How to support various hardware blocks in drm driver

[Rob Clark]

On Tue, Mar 18, 2014 at 6:27 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Tue, Mar 18, 2014 at 05:26:42PM +0900, Inki Dae wrote:
>> Hello all,
>>
>> The purpose of this email is for discussion to how we could support
>> various hardware blocks such as LVDS bridges, Image Enhancement chips
>> and parallel panels in drm driver.
>>
>> Now we have drm_panel framework for controlling parallel panels, and
>> drm_bridge for controlling LVDS bridge devices.
>> However, drm_panel has only some callbacks, enable, disable and
>> get_modes, and drm_bridge has some callbacks only for bridge devices
>> so these seems like only for their devices.
>>
>> However, some SoC could have other type of hardware blocks. i.e. Image
>> Enhancement blocks.
>> So it seems that these frameworks don't cover all hardware blocks that
>> can be placed between Display controller and LCD Panel device.
>>
>> Below shows various hardware pipe lines for Exynos SoC,
>>
>> Display Controller-----LCD Panel
>> Display Controller-----LVDS Bridge-----LCD Panel
>> Display Controller-----MIP DSI-----MIPI TO LVDS Bridge-----LCD Panel
>> Display Controller-----Image Enhancement chip-----LCD Panel
>> Display Controller-----Image Enhancement chip-----LVDS Bridge-----LCD Panel
>> Display Controller-----Image Enhancement chip-----MIP DSI-----MIPI TO
>> LVDS Bridge-----LCD Panel
>> ...
>>
>> As you know, these hardware blocks should be probed without the probe
>> order issue, and also these hardware blocks should be controlled by
>> crtc and connector drivers.
>> However, it seems that there is no any good way for this yet. So I
>> think we would need a common framework, and for this, we could
>> integrate these two frameworks, drm_panel and drm_bridge to one
>> integrated framework.
>> This integrated framework could be used commonly for other hardware blocks.
>>
>> Below is just pseudo structure for this,
>>
>> enum {
>>         DRM_HW_BLOCK_PANEL,
>>         DRM_HW_BLOCK_BRIDGE,
>>         DRM_HW_BLOCK_ENHANCE,
>> };
>>
>> struct drm_hw_block {
>>         unsigned int type;
>>         struct device *dev;
>>         struct drm_device *drm;
>>
>>         /* Used commonly. */
>>         int (*disable)(struct drm_hw_block *block);
>>         int (*enable)(struct drm_hw_block *block);
>>
>>         /* For parallel panels. */
>>         struct drm_connector *connector;
>>         int (*get_modes)(struct drm_hw_block *block);
>>
>>         /* For LVDS bridge devices. */
>>         void (*mode_fixup)(struct drm_hw_block *block);
>>         void (*mode_set)(struct drm_hw_block *block);
>>         ....
>>
>>         /* For Image Enhancement devices. */
>>         ....
>>
>>         struct list_head list;
>> };
>
> What's the difference here compared to an encoder_slave? I don't really
> see the point of adding yet another such thing to the drm core ...

so I think at one point the rough idea was to add additional fxn ptrs
to bridge as the need arose.  While I still need to give a closer read
to the original msg, but seems like if you s/drm_hw_block/drm_bridge/g
it is basically the same thing.  (Not saying "bridge" is the best
name.. it was just one of those things where no one had a better
suggestion at the time ;-))

> If you don't like the helper libraries provided by the drm core then
> either try to improve them, or just roll your own (like i915 does for
> modesetting).
>

I think the question is how to go from zero or one
bridge/hwblock/widget/whatever to zero or more..

an alternate set of helpers is one option.  But it didn't turn out to
be too intrusive to the existing helpers to add bridge in the first
place, so I'm not sure it will be necessary.

> Cheers, Daniel
>