Hi Sam,
Thank you for the patch.
On Sun, Aug 04, 2019 at 10:16:33PM +0200, Sam Ravnborg wrote:
> Inline comments provide better space for additional comments.
> Comments was slightly edited to follow the normal style,
> but no change to actual content.
> Used the opportuniy to change the order in drm_panel_funcs
> to follow the order they will be used by a panel.
>
> Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> Cc: Sean Paul <sean@xxxxxxxxxx>
> Cc: Thierry Reding <thierry.reding@xxxxxxxxx>
> Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> include/drm/drm_panel.h | 82 +++++++++++++++++++++++++++++++++--------
> 1 file changed, 66 insertions(+), 16 deletions(-)
>
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 053d611656b9..5e62deea49ba 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -36,14 +36,6 @@ struct display_timing;
>
> /**
> * struct drm_panel_funcs - perform operations on a given panel
> - * @disable: disable panel (turn off back light, etc.)
> - * @unprepare: turn off panel
> - * @prepare: turn on panel and perform set up
> - * @enable: enable panel (turn on back light, etc.)
> - * @get_modes: add modes to the connector that the panel is attached to and
> - * return the number of modes added
> - * @get_timings: copy display timings into the provided array and return
> - * the number of display timings available
> *
> * The .prepare() function is typically called before the display controller
> * starts to transmit video data. Panel drivers can use this to turn the panel
> @@ -69,31 +61,89 @@ struct display_timing;
> * the panel. This is the job of the .unprepare() function.
> */
> struct drm_panel_funcs {
> - int (*disable)(struct drm_panel *panel);
> - int (*unprepare)(struct drm_panel *panel);
> + /**
> + * @prepare:
> + *
> + * Turn on panel and perform set up.
> + */
> int (*prepare)(struct drm_panel *panel);
> +
> + /**
> + * @enable:
> + *
> + * Enable panel (turn on back light, etc.).
> + */
> int (*enable)(struct drm_panel *panel);
> +
> + /**
> + * @disable:
> + *
> + * Disable panel (turn off back light, etc.).
> + */
> + int (*disable)(struct drm_panel *panel);
> +
> + /**
> + * @unprepare:
> + *
> + * Turn off panel.
> + */
> + int (*unprepare)(struct drm_panel *panel);
> +
> + /**
> + * @get_modes:
> + *
> + * Add modes to the connector that the panel is attached to and
> + * return the number of modes added.
> + */
> int (*get_modes)(struct drm_panel *panel);
> +
> + /**
> + * @get_timings:
> + *
> + * Copy display timings into the provided array and return
> + * the number of display timings available.
> + */
> int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> struct display_timing *timings);
> };
>
> /**
> * struct drm_panel - DRM panel object
> - * @drm: DRM device owning the panel
> - * @connector: DRM connector that the panel is attached to
> - * @dev: parent device of the panel
> - * @link: link from panel device (supplier) to DRM device (consumer)
> - * @funcs: operations that can be performed on the panel
> - * @list: panel entry in registry
> */
> struct drm_panel {
> + /**
> + * @drm:
> + *
> + * DRM device owning the panel.
> + */
> struct drm_device *drm;
> +
> + /**
> + * @connector:
> + *
> + * DRM connector that the panel is attached to.
> + */
> struct drm_connector *connector;
> +
> + /**
> + * @dev:
> + *
> + * Parent device of the panel.
> + */
> struct device *dev;
>
> + /**
> + * @funcs:
> + *
> + * Operations that can be performed on the panel.
> + */
> const struct drm_panel_funcs *funcs;
>
> + /**
> + * @list:
> + *
> + * Panel entry in registry.
> + */
> struct list_head list;
> };
>
--
Regards,
Laurent Pinchart
