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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 17, 2013 at 4:21 AM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote:
>
>
>> -----Original Message-----
>> From: Sean Paul [mailto:seanpaul@xxxxxxxxxxxx]
>> Sent: Thursday, October 17, 2013 4:27 AM
>> To: dri-devel@xxxxxxxxxxxxxxxxxxxxx; inki.dae@xxxxxxxxxxx
>> Cc: airlied@xxxxxxxx; tomasz.figa@xxxxxxxxx; marcheu@xxxxxxxxxxxx; 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@xxxxxxxxxxxx>
>> ---
>>
>> 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
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux