How to support various hardware blocks in drm driver

[Inki Dae]

2014-03-19 13:07 GMT+09:00 Inki Dae <inki.dae at samsung.com>:
> 2014-03-19 2:37 GMT+09:00 Daniel Vetter <daniel at ffwll.ch>:
>> On Tue, Mar 18, 2014 at 09:58:25PM +0900, Inki Dae wrote:
>>> 2014-03-18 21:47 GMT+09:00 Daniel Vetter <daniel at ffwll.ch>:
>>> > On Tue, Mar 18, 2014 at 1:42 PM, Inki Dae <inki.dae at samsung.com> wrote:
>>> >> I think now drm_bridge couldn't do what we want for embedded systems
>>> >> as long as drm_encoder has drm_bridge.
>>> >> See the blow hardware pipeline,
>>> >> Display Controller-----Image Enhancement chip-----MIP DSI-----MIPI TO
>>> >> LVDS Bridge-----LCD Panel
>>> >>
>>> >> In above hardware pipeline, Display controller is controlled by crtc,
>>> >> and Image Enhancement chip receives output from display controller.
>>> >> So the use of existing drm_bridge would be suitable to only bridge
>>> >> devices between MIPI DSI and LCD Panel, but not to Image Enhancement
>>> >> chip.
>>> >>
>>> >> For such hardware, drm_panel infrastructure is more reasonable to me,
>>> >> and that is why I try to integrate drm_panel and drm_bridge to one
>>> >> integrated framework which has infrastructure same as existing
>>> >> drm_panel.
>>> >> The important thing is to free this integrated framework from
>>> >> drm_encoder so that crtc device can also use this framework.
>>> >
>>> > Hm, what is this image enhancement chip? Is that some IP block on the
>>> > SoC? Is it optional? Can it be attached to different crtcs?
>>>
>>> In case of Exynos, this chip is in SoC, and can be only attached to
>>> one crtc, display controller. But I'm not sure other SoC have similar
>>> chip.
>>>
>>> >
>>> > I think we have similar things on intel hardware, but without details
>>> > on what it does and how it works I can't really say how to best expose
>>> > it to userspace and how to best handle it internally in the driver.
>>> > -Daniel
>>>
>>> Simply saying, the image enhancement chip can enhance image data from
>>> display controller, i.e. saturation enhancement, color reproduction,
>>> dithering, and so on.
>>> And this chip receives image data through hardware wired lines
>>> connected internally between display controller and this chip.
>>
>> To me this sounds like you simply need to expose all these capabilities to
>> userspace as crtc properties. Which addresses one part of this issue.
>
> No, what I want is really not to expose some capabilities to
> suerspace. If you think so, there might be your missing point.
>
>>
>> The other side is how you are going to track this in the driver, and there
>> you can do whatever you want - just add a pointer/structure to the exynos
>> crtc structure for the display enhancement block.
>
> Hm.. that is what I try to do extending existing drm_panel and
> drm_bridge frameworks. As you may know, now existing drm_bridge could
> support only bridge device of encoder side.
> So now I am trying for this drm_bridge can support both sides of crtc
> and encoder.
>
>>
>> The MIPI DSI block would then be treated as a drm_encoder, and all the
>> later stages as drm_bridges up to the very last (the actual lvds panel)
>> which would be a simple drm_panel.
>>
>
> Existing drm_panel only supports real panel device, and existing
> drm_bridge only supports bridge device between encoder and panel.
> In here, the problem is that there can be other bridge devices between
> crtc and encoder. So crtc driver would want to control such bridge
> device in timely manner.
> How the crtc driver could control this bridge between crtc and encoder?
>
> See the below a example that is what I try to do for this,
>
> struct drm_bridge {       <- just change name from drm_hw_block to drm_bridge
>         ...
>         struct drm_panel *panel;    <- existing drm_panel
>         struct drm_lvds *lvds;        <- existing drm_bridge
>         struct drm_enhancer *enhander; < - new one
>         ...
> };
>
> Or,
>
> struct drm_panel {    <- existing drm_panel
>        ...
> };
>
> No change for drm_panel.
>
> struct drm_bridge {
>         ...
>         struct drm_lvds *lvds;   <- existing drm_bridge
>         struct drm_enhancer *enhancer;   <- new one
> };
>
> My idea is to extend existing drm_bridge so that this drm_bridge could
> support both of them, one between crtc and encoder, and another one
> between encoder and panel.
>
> Display controller ----- bridge1 ----- mipi dsi ----- bridge2 ----- panel
>                                       /|\
>       /|\                /|\
>                                        |
>        |                  |
>                               drm_enhancer                   drm_lvds
>   drm_panel
>

Oops, sorry for broken diagram.

bridge1 -----> drm_enhancer
bridge2 -----> drm_lvds
panel -----> drm_panel


> And now is just ready for supporting such bridge later. So this
> integrated drm_bridge would have drm_panel and drm_lvds. For Image
> Enhancer, later.
>
>> I don't really see what additional complexity you need here. Especially
>> since this image enhancer is on your SoC (and I guess a samgsung IP block
>> not shared with any other SoC manufacture) you can easily keep the driver
>> code for it in the exynos driver. So really no need to have a generic
>> interface here.
>
> Yes, that might be the point. As I already mentioned, I'm not sure
> this device exists only on Exynos SoC. But I guess there could be
> other SoC with image enhancer now or later.
>
> To other SoC people,
> Does your SoC have bridge device such as Image Enhancer between crtc
> and encoder? If so, this time would be a good opportunity that we
> could support all hardware blocks in drm world.
>
> Thanks,
> Inki Dae
>
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel