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

[Rob Clark]

On Thu, Oct 24, 2013 at 3:46 AM, Inki Dae <inki.dae at samsung.com> wrote:
> 2013/10/23 Rob Clark <robdclark at gmail.com>:
>> On Wed, Oct 23, 2013 at 9:18 AM, Inki Dae <inki.dae at samsung.com> wrote:
>>> Look at omapdrm, nouveau, and radeon drm drivers.
>>
>>
>> btw, please don't look at omapdrm as a "good" example in this regard.
>> The layering is really not a good idea and for a long time caused a
>> lot of problems, which we essentially solved by bypassing parts of the
>> layering.  I still think omapdss and omapdrm should be flattened into
>> a single drm driver, then net result would be a drop in # of lines of
>
> It seems that you proper to use duplicated driver. I mean... do you
> proper that omapdss driver is placed in drm framework? Otherwise, is
> there any way that omapdss and omapdrm can be flattened into a single
> drm driver without moving omapdss into drm framework? As I mentioned
> earlier, we wanted to reuse the existing panel driver so Exynos drm
> framework has the layer, wrappers to connector and encoder. Of course,
> for this, we have TODO works yet, and I still think it's better way to
> keep the wrapper if we should reuse the existing panel drivers.

If it were my decision, I would duplicate (+refactor) the code into
drm..  there ends up being some duplication between fbdev and drm, but
the benefit is less risk of breaking other subsystem (fbdev) in the
short term and more flexibility to refactor things to fit better into
how drm/kms works.

In the long term, fbdev stops being something we care about, so the
duplicate code is just a transient problem.  But what ends up in drm
we live with for a long time, so it is easier in the long run to not
have to care about both fbdev and drm with the same code.

BR,
-R

> The below would be one design for the case,
>
>
>                                                               lcd panel drivers
>                                                                       \ | /
>                                    drm framework  -----  drm_bridge
>                                                                       / | \
>                                                       crtc
> drivers(display controller or hdmi)
>
>
> A header file of drm_bridge includes drm_panel structure having some
> callbacks related to connector, and some function to register crtc and
> panel driver's requests to the drm_bridge object. And the header file
> can be included in the existing panel drivers. In other words,
> drm_bridge will connect drm driver and lcd class based panel drivers.
>
> struct drm_bridge {
>         struct list_head list;
>         unsigned int type;
>         struct drm_device *drm_dev;
>         struct drm_panel *panel_ops;
>         ...
>         int (*drm_create_enc_conn)(....);
> };
>
> A drm_bridge object has drm_panel ops and drm_create_enc_conn
> callback. And once the crtc driver calls register function of the
> drm_bridge with drm_create_enc_conn callback pointer, a drm_crtc is
> created, and once the panel driver calls the register function with
> drm_panel ops, a encoder and a connector are created. At this time,
> drm_fb_helper_initial_config() can be called appropriately to connect
> crtc and connector, and to display framebuffer on lcd panel.
>
> The above is just my opinion for the case, and we are testing this way
> implementing it internally.
>
> Thanks,
> Inki Dae
>
>> code.  I wish there were folks like the Sean and Stéphane who cared
>> enough to do this for omapdrm ;-)
>>
>> Other drivers have some modularity to separate code that is
>> significantly different across genarations (but what form that takes
>> differs depending on what how the hw differs across generations).
>> This isn't a bad thing.  But you need to look at the end result.  For
>> example how I split out the phy code for the mdp4 code in msm (hdmi
>> and dsi phy differ between otherwise similar 28nm and 45nm parts, but
>> the rest of the display controller blocks are basically the same).
>>
>> BR,
>> -R
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel