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

[Sean Paul]

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.

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.


> 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.


> 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.

Sean


> Thanks,
> Inki Dae
>