[PATCH v2 12/26] drm/exynos: Split manager/display/subdrv

[Sean Paul]

On Wed, Oct 23, 2013 at 1:42 AM, Inki Dae <inki.dae at samsung.com> wrote:
> 2013/10/23 Sean Paul <seanpaul at chromium.org>:
>> On Wed, Oct 23, 2013 at 12:48 AM, Inki Dae <inki.dae at samsung.com> wrote:
>>> 2013/10/23 Stéphane Marchesin <stephane.marchesin at gmail.com>:
>>>>
>>>>
>>>>
>>>> On Tue, Oct 22, 2013 at 9:15 PM, Inki Dae <inki.dae at samsung.com> wrote:
>>>>>
>>>>> 2013/10/23 Stéphane Marchesin <stephane.marchesin at gmail.com>:
>>>>> >
>>>>> >
>>>>> >
>>>>> > On Tue, Oct 22, 2013 at 8:38 PM, Inki Dae <inki.dae at samsung.com> wrote:
>>>>> >>
>>>>> >> 2013/10/23 Stéphane Marchesin <stephane.marchesin at gmail.com>:
>>>>> >> >
>>>>> >> >
>>>>> >> >
>>>>> >> > On Tue, Oct 22, 2013 at 7:28 PM, Inki Dae <inki.dae at samsung.com>
>>>>> >> > wrote:
>>>>> >> >>
>>>>> >> >> 2013/10/22 Sean Paul <seanpaul at chromium.org>:
>>>>> >> >> > On Tue, Oct 22, 2013 at 1:30 AM, Inki Dae <inki.dae at samsung.com>
>>>>> >> >> > wrote:
>>>>> >> >> >>
>>>>> >> >> >>
>>>>> >> >> >>> -----Original Message-----
>>>>> >> >> >>> From: Sean Paul [mailto:seanpaul at chromium.org]
>>>>> >> >> >>> Sent: Tuesday, October 22, 2013 6:18 AM
>>>>> >> >> >>> To: Inki Dae
>>>>> >> >> >>> Cc: dri-devel; Dave Airlie; Tomasz Figa; Stéphane Marchesin
>>>>> >> >> >>> Subject: Re: [PATCH v2 12/26] drm/exynos: Split
>>>>> >> >> >>> manager/display/subdrv
>>>>> >> >> >>>
>>>>> >> >> >>> On Mon, Oct 21, 2013 at 10:46 AM, Sean Paul
>>>>> >> >> >>> <seanpaul at chromium.org>
>>>>> >> >> >>> wrote:
>>>>> >> >> >>> > On Thu, Oct 17, 2013 at 10:31 PM, Inki Dae
>>>>> >> >> >>> > <inki.dae at samsung.com>
>>>>> >> >> >>> > wrote:
>>>>> >> >> >>> >>
>>>>> >> >> >>> >>
>>>>> >> >> >>> >>> -----Original Message-----
>>>>> >> >> >>> >>> From: Sean Paul [mailto:seanpaul at chromium.org]
>>>>> >> >> >>> >>> Sent: Thursday, October 17, 2013 11:37 PM
>>>>> >> >> >>> >>> To: Inki Dae
>>>>> >> >> >>> >>> Cc: dri-devel; Dave Airlie; Tomasz Figa; Stéphane Marchesin
>>>>> >> >> >>> >>> Subject: Re: [PATCH v2 12/26] drm/exynos: Split
>>>>> >> >> >>> >>> manager/display/subdrv
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>> On Thu, Oct 17, 2013 at 4:21 AM, Inki Dae
>>>>> >> >> >>> >>> <inki.dae at samsung.com>
>>>>> >> >> >> wrote:
>>>>> >> >> >>> >>> >
>>>>> >> >> >>> >>> >
>>>>> >> >> >>> >>> >> -----Original Message-----
>>>>> >> >> >>> >>> >> From: Sean Paul [mailto:seanpaul at chromium.org]
>>>>> >> >> >>> >>> >> Sent: Thursday, October 17, 2013 4:27 AM
>>>>> >> >> >>> >>> >> To: dri-devel at lists.freedesktop.org; inki.dae at samsung.com
>>>>> >> >> >>> >>> >> Cc: airlied at linux.ie; tomasz.figa at gmail.com;
>>>>> >> >> >>> >>> >> marcheu at chromium.org;
>>>>> >> >> >>> Sean
>>>>> >> >> >>> >>> >> Paul
>>>>> >> >> >>> >>> >> Subject: [PATCH v2 12/26] drm/exynos: Split
>>>>> >> >> >>> >>> >> manager/display/subdrv
>>>>> >> >> >>> >>> >>
>>>>> >> >> >>> >>> >> This patch splits display and manager from subdrv. The
>>>>> >> >> >>> >>> >> result
>>>>> >> >> >>> >>> >> is
>>>>> >> >> >>> that
>>>>> >> >> >>> >>> >> crtc functions can directly call into manager callbacks
>>>>> >> >> >>> >>> >> and
>>>>> >> >> >>> >>> >> encoder
>>>>> >> >> >>> >>> >> functions can directly call into display callbacks. This
>>>>> >> >> >>> >>> >> will
>>>>> >> >> >>> >>> >> allow
>>>>> >> >> >>> >>> >> us to remove the exynos_drm_hdmi shim and support
>>>>> >> >> >>> >>> >> mixer/hdmi
>>>>> >> >> >>> >>> >> &
>>>>> >> >> >>> fimd/dp
>>>>> >> >> >>> >>> >> with common code.
>>>>> >> >> >>> >>> >>
>>>>> >> >> >>> >>> >> Signed-off-by: Sean Paul <seanpaul at chromium.org>
>>>>> >> >> >>> >>> >> ---
>>>>> >> >> >>> >>> >>
>>>>> >> >> >>> >>> >> Changes in v2:
>>>>> >> >> >>> >>> >>       - Pass display into display_ops instead of context
>>>>> >> >> >>> >>> >
>>>>> >> >> >>> >>> > Sorry but it seems like more reasonable to pass device
>>>>> >> >> >>> >>> > object
>>>>> >> >> >>> >>> > into
>>>>> >> >> >>> >>> > display_ops and manager_ops.
>>>>> >> >> >>> >>> >
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>> So you've changed your mind from when you said the
>>>>> >> >> >>> >>> following?
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>> >>> manager->ops->xxx(manager, ...);
>>>>> >> >> >>> >>> >>> display->ops->xxx(display, ...);
>>>>> >> >> >>> >>> >>>
>>>>> >> >> >>> >>> >>> Agree.
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>
>>>>> >> >> >>> >>
>>>>> >> >> >>> >> True. Before that, My comment was to pass device object into
>>>>> >> >> >>> display_ops and
>>>>> >> >> >>> >> manager_ops, and then you said the good solution is to pass
>>>>> >> >> >>> >> manager
>>>>> >> >> >>> >> and
>>>>> >> >> >>> >> display to each driver. At that time, I thought no matter how
>>>>> >> >> >>> >> the
>>>>> >> >> >>> callback
>>>>> >> >> >>> >> is called if the framework doesn't call callbacks of each
>>>>> >> >> >>> >> driver
>>>>> >> >> >>> >> with
>>>>> >> >> >>> ctx.
>>>>> >> >> >>> >> So I agreed.
>>>>> >> >> >>> >>
>>>>> >> >> >>> >>
>>>>> >> >> >>> >>> It would have been nice if you had changed your mind
>>>>> >> >> >>> >>> *before* I
>>>>> >> >> >>> >>> reworked everything. At any rate, I think it's still the
>>>>> >> >> >>> >>> right
>>>>> >> >> >>> >>> thing
>>>>> >> >> >>> >>> to do.
>>>>> >> >> >>> >>
>>>>> >> >> >>> >> Really sorry about that. And I will add new patch for it so
>>>>> >> >> >>> >> you
>>>>> >> >> >>> >> don't
>>>>> >> >> >>> need
>>>>> >> >> >>> >> to concern about that.
>>>>> >> >> >>> >>
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>> > I'm not sure but display_ops could be implemented in other
>>>>> >> >> >>> >>> > framework
>>>>> >> >> >>> >>> based
>>>>> >> >> >>> >>> > driver such as CDF based lcd panel driver. So if you pass
>>>>> >> >> >>> >>> > display -
>>>>> >> >> >>> it's
>>>>> >> >> >>> >>> > specific to exynos drm framework - into display_ops, the
>>>>> >> >> >>> >>> > other
>>>>> >> >> >>> framework
>>>>> >> >> >>> >>> > based driver should include specific exynos drm header.
>>>>> >> >> >>> >>> >
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>> AFAIK, CDF will not land in its current separate-from-drm
>>>>> >> >> >>> >>> form,
>>>>> >> >> >>> >>> we
>>>>> >> >> >>> >>> don't need to worry about this. Furthermore, these ops
>>>>> >> >> >>> >>> should
>>>>> >> >> >>> >>> just
>>>>> >> >> >>> >>> go
>>>>> >> >> >>> >>> away and become drm_crtc/drm_encoder/drm_connector funcs
>>>>> >> >> >>> >>> anyways.
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>
>>>>> >> >> >>> >> Can you assure the display_ops never implemented in other
>>>>> >> >> >>> >> framework
>>>>> >> >> >>> based
>>>>> >> >> >>> >> driver, not CDF? At any rate, I think all possibilities
>>>>> >> >> >>> >> should
>>>>> >> >> >>> >> be
>>>>> >> >> >>> opened.
>>>>> >> >> >>> >>
>>>>> >> >> >>> >
>>>>> >> >> >>> > I don't think we should let an RFC framework make the code
>>>>> >> >> >>> > more
>>>>> >> >> >>> > complicated for unclear benefit. By removing manager/display
>>>>> >> >> >>> > entirely,
>>>>> >> >> >>> > we can get rid of a *lot* of other code that is basically just
>>>>> >> >> >>> > plumbing drm hooks (exynos_drm_connector is a good example).
>>>>> >> >> >>> >
>>>>> >> >> >>>
>>>>> >> >> >>> I hacked this up today to prove it out. Check out the top 5
>>>>> >> >> >>> commits
>>>>> >> >> >>> in
>>>>> >> >> >>>
>>>>> >> >> >>>
>>>>> >> >> >>>
>>>>> >> >> >>> https://github.com/crseanpaul/exynos-drm-next/commits/linux-next-exynos-
>>>>> >> >> >>> staging.
>>>>> >> >> >>> It removes exynos_drm_connector in favor of just implementing
>>>>> >> >> >>> drm_connector directly. This same treatment should be done for
>>>>> >> >> >>> exynos_drm_encoder and exynos_drm_crtc, but I didn't get around
>>>>> >> >> >>> to
>>>>> >> >> >>> doing this.
>>>>> >> >> >>>
>>>>> >> >> >>> As you can see, it cuts out a lot of code and removes an entire
>>>>> >> >> >>> abstraction layer. Much nicer :)
>>>>> >> >> >>>
>>>>> >> >> >>
>>>>> >> >> >> It seems that you implements connector in each device driver.
>>>>> >> >> >> Can't
>>>>> >> >> >> they be
>>>>> >> >> >> combined as common spot, exynos_connector, again to avoid codes
>>>>> >> >> >> from
>>>>> >> >> >> duplicated? :)
>>>>> >> >> >
>>>>> >> >> > There's nothing of substance being duplicated.
>>>>> >> >>
>>>>> >> >> Not true. xxx_create_connector is duplicated.
>>>>> >> >>
>>>>> >> >> > In fact, by getting rid
>>>>> >> >> > of the exynos_drm_connector layer, we deleted 150 lines. If you
>>>>> >> >> > really
>>>>> >> >> > take a look at exynos_drm_connector, it's not doing anything
>>>>> >> >> > useful.
>>>>> >> >>
>>>>> >> >> No, That is for each driver has no any dependency of drm framework.
>>>>> >> >>
>>>>> >> >> > All it does is translate the drm callbacks into display callbacks,
>>>>> >> >> > so
>>>>> >> >> > I think it's much better to just implement the drm callbacks
>>>>> >> >> > directly.
>>>>> >> >> >
>>>>> >> >>
>>>>> >> >> No, It has strongly dependency of drm framework. Assume that we
>>>>> >> >> implemented the drm callbacks directly, and then some features are
>>>>> >> >> added to drm framework, drm_connector side. At this time, we will
>>>>> >> >> have
>>>>> >> >> to take care of each device driver according to the change. That is
>>>>> >> >> really not good. Why device drivers should have dependency of drm
>>>>> >> >> framework? Just to reduce line counts?
>>>>> >> >
>>>>> >> >
>>>>> >> >
>>>>> >> > You seem to miss the point here and elsewhere in the discussion.
>>>>> >> > drm/exynos is a drm driver, and as such it should use the drm
>>>>> >> > framework,
>>>>> >>
>>>>> >> Hm.. you seem to miss something. Exynos drm based drivers are based on
>>>>> >> exynos drm framework, not drm framework directly. So I mean that
>>>>> >> Exynos drm framework based drivers should include only Exynos drm
>>>>> >> headers, _not drm header_ directly.
>>>>> >
>>>>> >
>>>>> > Well, I think everyone sees that exynos is different. But my point still
>>>>> > remains: why is the exynos driver in drm/ if it wants to use a different
>>>>> > framework? Right now it is blocking work on a proper drm driver...
>>>>> >
>>>>>
>>>>> Noooooo. It's not to use a different framework. It's to use a wrapper
>>>>> instead.
>>>>
>>>>
>>>> Ok, if you want to call it a wrapper, then what is the point of doing this
>>>> wrapping given that it prevents a proper drm-style implementation?
>>>>
>>>
>>> I already commented. That is for only Exynos drm framework has
>>> dependency of drm framework directly, and Exynos drm based drivers
>>> include only Exynos drm headers.
>>>
>>>>
>>>>>
>>>>> >
>>>>> >>
>>>>> >>
>>>>> >> > especially if this reduces the line count and the code
>>>>> >> > complexity (as is the case for this patch series). If you don't want
>>>>> >> > to maintain a drm driver, it simply should be moved away from drm/,
>>>>> >> > and it should be replaced by a real drm driver in my opinion.
>>>>> >>
>>>>> >> So those drivers should be in drm/exynos. Isn't that you really mean
>>>>> >> those drivers should be driver/gpu/drm?
>>>>> >
>>>>> >
>>>>> > I don't understand this sentence, sorry.
>>>>>
>>>>> Sorry, again, you mean Exynos drm based drivers should be in
>>>>> drivers/gpu/drm, not drivers/gpu/drm/exynos?
>>>>>
>>>> Is the exynos drm useful in its current shape at all? My recommendation
>>>> would be to fork off a real drm driver in gpu/drm/exynos with the current
>>>> code as a base.
>>>>
>>>
>>> Yes as of now. of course, There could be a better way. However, I
>>> don't want for Exynos drm based drivers have dependency of drm
>>> framework directly.
>>>
>>
>> Just to satisfy my curiosity, do you actually have something that uses
>> these drivers outside of drm?
>>
>
> Never no.
>
>
>> So I think we've reached somewhat of an impasse. I'd like to move the
>> driver towards a proper drm driver (ie: no exynos
>> framework/wrapper/whatever), you'd like to keep things separate. So
>> should we create a new exynos driver drivers/drm/gpu/exynos5 to house
>> the drm driver?
>
> Did you check out other drm drivers using similar way? And If so, the
> other drm framework based drivers should be moved?

No, I haven't looked at other ARM drivers. It's besides the point,
really, that's not a compelling argument. For the exynos driver, in
particular, it doesn't make sense to wrapping everything in
exynos_drm_*

> What is the problem
> that Exynos drm based drivers include only Exynos drm headers, not drm
> framework header directly? Is that really a big issue? I _cannot_
> understand your behavior.

The problem is that it makes things unnecessarily complex for no benefit.

For example, that you wanted to implement
connector_funcs->set_property() just in the hdmi driver. This requires
that you add a new display_op in exynos_drm_drv.h, then you implement
the connector_func callback in exynos_drm_connector to check if the op
is implemented then call it, and then you implement the set_property
hook in the hdmi driver. If you didn't have the wrapper, you just
implement the hook in the hdmi driver.

Sean


> Anyway, if there are any reasons that Exynos
> drm based drivers should necessary include drm framework header
> directly, I will agree it.
>
> Thanks,
> Inki Dae
>
>>
>> Sean
>>
>>
>>> Thanks for your opinions.
>>> Inki Dae
>>>
>>>> Stéphane
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> Inki Dae
>>>>>
>>>>> >
>>>>> > Stéphane
>>>>> >
>>>>> >
>>>>> >>
>>>>> >> If so, That would really be
>>>>> >> horrible. :(
>>>>> >>
>>>>> >
>>>>> >
>>>>> >>
>>>>> >> Please, know that only Exynos drm framework, _not device drivers_, has
>>>>> >> all dependencies of drm framework, and also I know that other ARM
>>>>> >> based drm drivers are using same way.
>>>>> >>
>>>>> >> Thanks,
>>>>> >> Inki Dae
>>>>> >>
>>>>> >> >
>>>>> >> > Stéphane
>>>>> >> >
>>>>> >> >>
>>>>> >> >>
>>>>> >> >> > There are a bunch of real bugs that we've found as a result of
>>>>> >> >> > having
>>>>> >> >> > these abstraction layers. Take, for example, dpms. Before this
>>>>> >> >> > patchset, dpms for fimd was being tracked separately in fimd
>>>>> >> >> > driver,
>>>>> >> >> > exynos_drm_encoder, exynos_drm_crtc, and exynos_drm_connector.
>>>>> >> >> > Furthermore, during suspend, only fimd driver's dpms state was
>>>>> >> >> > updated, so the others were incorrect. There was also this weird
>>>>> >> >> > gymnastics that had to happen when dpms was changed in the encoder
>>>>> >> >> > since it had to walk up to the connector level to change its dpms
>>>>> >> >> > state. If fimd just directly implemented
>>>>> >> >> > drm_crtc/drm_encoder/drm_connector (before dp was moved in), this
>>>>> >> >> > problem wouldn't exist. The same goes for HDMI/mixer.
>>>>> >> >> >
>>>>> >> >>
>>>>> >> >> That is a issue we should take care of by using the independent
>>>>> >> >> layer.
>>>>> >> >> Then, aren't you take care of that well with the re-factoring patch
>>>>> >> >> set? :)  It seems that you are outside real point.
>>>>> >> >>
>>>>> >> >> > Take a look at exynos_drm_encoder.c  in my tree
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> > (https://github.com/crseanpaul/exynos-drm-next/blob/linux-next-exynos-staging/drivers/gpu/drm/exynos/exynos_drm_encoder.c),
>>>>> >> >> > what does it do that's useful to abstract? All that it does is
>>>>> >> >> > just
>>>>> >> >> > call display ops, it's completely useless. The same is true for
>>>>> >> >> > exynos_drm_connector, it's just dead weight. There is some useful
>>>>> >> >> > stuff in exynos_drm_crtc for page flipping, that would be better
>>>>> >> >> > served as a helper library, though.
>>>>> >> >> >
>>>>> >> >> >> The abstraction layer you mentioned also means a common spot.
>>>>> >> >> >> Another one, you patch also makes each sub driver have strongly
>>>>> >> >> >> dependency
>>>>> >> >> >> of drm framework. So how we can support existing backlight and
>>>>> >> >> >> lcd
>>>>> >> >> >> class
>>>>> >> >> >> based lcd panel drivers if the connector is implemented in each
>>>>> >> >> >> device
>>>>> >> >> >> driver later?  the drm header files should be included in
>>>>> >> >> >> drivers/video/backlight/xxx_lcd.c?
>>>>> >> >> >>
>>>>> >> >> >
>>>>> >> >> > drm_bridge or drm_panel seem like good candidates for this.
>>>>> >> >> >
>>>>> >> >>
>>>>> >> >> Yes, exynos_drm_display could be replaced with drm_panel later if
>>>>> >> >> the
>>>>> >> >> drm_panel can be merged to mainline.
>>>>> >> >>
>>>>> >> >> >
>>>>> >> >> >> And, I will introduce a new framework to support existing lcd
>>>>> >> >> >> panel
>>>>> >> >> >> drivers
>>>>> >> >> >> and display bus drivers soon; as of now for Exynos drm, and the
>>>>> >> >> >> framework is
>>>>> >> >> >> being tested internally. With this framework, encoder and
>>>>> >> >> >> connector
>>>>> >> >> >> will be
>>>>> >> >> >> created when lcd panel or display bus driver such as eDP is
>>>>> >> >> >> probed:
>>>>> >> >> >> it
>>>>> >> >> >> doesn’t really need to create encoder and connector in advance if
>>>>> >> >> >> lcd
>>>>> >> >> >> panel
>>>>> >> >> >> or display bus driver isn't probed yet. Regardless of crtc, and
>>>>> >> >> >> encoder
>>>>> >> >> >> and
>>>>> >> >> >> connector creation order, when last one is created, crtc and
>>>>> >> >> >> connector
>>>>> >> >> >> will
>>>>> >> >> >> be connected each other. And exynos_drm_display could be
>>>>> >> >> >> implemented
>>>>> >> >> >> in
>>>>> >> >> >> other frameworks if we have common structure for display device
>>>>> >> >> >> driver.
>>>>> >> >> >> And
>>>>> >> >> >> also the framework will support lvds driver according to Linux
>>>>> >> >> >> device
>>>>> >> >> >> driver
>>>>> >> >> >> model.
>>>>> >> >> >>
>>>>> >> >> >
>>>>> >> >> > I don't really follow what you're trying to do here, but I think
>>>>> >> >> > we
>>>>> >> >> > should be moving in the direction of fewer abstractions in the
>>>>> >> >> > exynos
>>>>> >> >> > driver, not more :)
>>>>> >> >> >
>>>>> >> >>
>>>>> >> >> Not abstraction layer, just a bridge for connecting crtc and its
>>>>> >> >> corresponding encoder/connector, and lvds regardless of creation
>>>>> >> >> order, and for connecting drm connector and other framework based
>>>>> >> >> display ops such as drm_panel later.
>>>>> >> >>
>>>>> >> >> > Sean
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >> Thanks,
>>>>> >> >> >> Inki Dae
>>>>> >> >> >>
>>>>> >> >> >>> Sean
>>>>> >> >> >>>
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>> > And another one, the patch 6 passes manager object to
>>>>> >> >> >>> >>> > manager_ops,
>>>>> >> >> >>> and
>>>>> >> >> >>> >>> for
>>>>> >> >> >>> >>> > this, you made the manager object to be set to driver
>>>>> >> >> >>> >>> > data;
>>>>> >> >> >>> >>> > platform_set_drvdata(pdev, &manager). That isn't
>>>>> >> >> >>> >>> > reasonable.
>>>>> >> >> >>> Generally,
>>>>> >> >> >>> >>> > driver_data would point to device driver's context object.
>>>>> >> >> >>> >>> >
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>> I'm not sure why this isn't reasonable, but it's a moot
>>>>> >> >> >>> >>> point.
>>>>> >> >> >>> >>> The
>>>>> >> >> >>> >>> driver data is only used up until we get rid of the pm ops,
>>>>> >> >> >>> >>> it
>>>>> >> >> >>> >>> needn't
>>>>> >> >> >>> >>> be set at all once things go through dpms.
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>
>>>>> >> >> >>> >> Generally, device drivers can call its own internal
>>>>> >> >> >>> >> functions,
>>>>> >> >> >>> >> and
>>>>> >> >> >>> >> they
>>>>> >> >> >>> will
>>>>> >> >> >>> >> call that functions with ctx. However, if you set manager to
>>>>> >> >> >>> driver_data
>>>>> >> >> >>> >> then that functions should be called with manager object and
>>>>> >> >> >>> >> also
>>>>> >> >> >>> internally
>>>>> >> >> >>> >> that functions should get ctx from the manager object. What
>>>>> >> >> >>> >> is
>>>>> >> >> >>> >> the
>>>>> >> >> >>> purpose
>>>>> >> >> >>> >> of manager? Do you think it's reasonable?
>>>>> >> >> >>> >>
>>>>> >> >> >>> >
>>>>> >> >> >>> > So, to avoid setting the manager as the drvdata, we could
>>>>> >> >> >>> > implement
>>>>> >> >> >>> > something like fimd_dpms_ctx(ctx, mode) that takes ctx and the
>>>>> >> >> >>> > manager
>>>>> >> >> >>> > callback calls it fimd_dpms(mgr, mode) { ctx = mgr->ctx;
>>>>> >> >> >>> > fimd_dpms_ctx(ctx, mode); }. Alternatively, you can save a
>>>>> >> >> >>> > pointer
>>>>> >> >> >>> > to
>>>>> >> >> >>> > mgr in ctx, but that creates a circular link between the two.
>>>>> >> >> >>> > IMO,
>>>>> >> >> >>> > both of those solutions suck :)
>>>>> >> >> >>> >
>>>>> >> >> >>> > I'd much rather just set drvdata to the manager and call the
>>>>> >> >> >>> > hook
>>>>> >> >> >>> > directly. Like I said earlier, this is just a temporary step
>>>>> >> >> >>> > since
>>>>> >> >> >>> > we
>>>>> >> >> >>> > remove these pm ops later in the patch series.
>>>>> >> >> >>> >
>>>>> >> >> >>> > Sean
>>>>> >> >> >>> >
>>>>> >> >> >>> >
>>>>> >> >> >>> >> Anyway, I'd like to say really sorry about inconvenient
>>>>> >> >> >>> >> again.
>>>>> >> >> >>> >> So I
>>>>> >> >> >>> will fix
>>>>> >> >> >>> >> it.
>>>>> >> >> >>> >>
>>>>> >> >> >>> >> Thanks,
>>>>> >> >> >>> >> Inki Dae
>>>>> >> >> >>> >>
>>>>> >> >> >>> >>> Sean
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>>
>>>>> >> >> >>> >>> > Thanks,
>>>>> >> >> >>> >>> > Inki Dae
>>>>> >> >> >>> >>> >
>>>>> >> >> >>> >>
>>>>> >> >> >>
>>>>> >> >> > _______________________________________________
>>>>> >> >> > dri-devel mailing list
>>>>> >> >> > dri-devel at lists.freedesktop.org
>>>>> >> >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>> >> >> _______________________________________________
>>>>> >> >> dri-devel mailing list
>>>>> >> >> dri-devel at lists.freedesktop.org
>>>>> >> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>> >> >
>>>>> >> >
>>>>> >> >
>>>>> >> > _______________________________________________
>>>>> >> > dri-devel mailing list
>>>>> >> > dri-devel at lists.freedesktop.org
>>>>> >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>> >> >
>>>>> >
>>>>> >
>>>>> >
>>>>> > _______________________________________________
>>>>> > dri-devel mailing list
>>>>> > dri-devel at lists.freedesktop.org
>>>>> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>> >
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel