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 <s...@ravnborg.org>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Maxime Ripard <maxime.rip...@bootlin.com>
> Cc: Sean Paul <s...@poorly.run>
> Cc: Thierry Reding <thierry.red...@gmail.com>
> Cc: Sam Ravnborg <s...@ravnborg.org>
> Cc: David Airlie <airl...@linux.ie>
> Cc: Daniel Vetter <dan...@ffwll.ch>

Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

> ---
>  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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to