How to support various hardware blocks in drm driver

[Rob Clark]

On Tue, Mar 18, 2014 at 4:26 AM, Inki Dae <inki.dae at samsung.com> wrote:
>
> enum {
>         DRM_HW_BLOCK_PANEL,
>         DRM_HW_BLOCK_BRIDGE,
>         DRM_HW_BLOCK_ENHANCE,
> };
>
> struct drm_hw_block {
>         unsigned int type;
>         struct device *dev;

just fyi, drop the 'struct device' ptr.. that sort of thing shouldn't
be in the base struct.

(and it should subclass 'struct drm_mode_object base' too)

>         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);

so, other than this part, the rest is already in drm_bridge_funcs.
But seems like something that only makes sense at the end of the chain
(ie. connector/panel end of things).. to me it makes sense to instead
just keep panel as it's own thing.

>         /* 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;
> };
>
> Of course, we could separate above callbacks to each structure like below,

yup callbacks should be split out (and normally in a 'const' data
structure if possible)

BR,
-R