Cc: Thierry the drm panel maintainer.
On Mon, 29 Feb 2016, Deepak M <m.deepak@xxxxxxxxx> wrote:
> Currently there are few pair of functions which
> are called during the panel enable/disable sequence.
> To improve the granularity, adding few more wrapper
> functions so that the functions are more specific
> on what they are doing.
>
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Signed-off-by: Deepak M <m.deepak@xxxxxxxxx>
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@xxxxxxxxx>
> ---
> include/drm/drm_panel.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 13ff44b..c729f6d 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -73,6 +73,12 @@ struct drm_panel_funcs {
> int (*get_modes)(struct drm_panel *panel);
> int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> struct display_timing *timings);
> + int (*power_on)(struct drm_panel *panel);
> + int (*power_off)(struct drm_panel *panel);
> + int (*backlight_on)(struct drm_panel *panel);
> + int (*backlight_off)(struct drm_panel *panel);
Please read the kernel-doc comment above the struct, and justify the
need to add new ones. Please update the kernel-doc comment to reflect
the changes.
For example, the documentation for .prepare() says, "drivers can use
this to turn the panel on". What's the order of power_on and prepare
(and their counterparts)? What's the division of responsibilities?
Similarly, the documentation for .enable() says, "This will typically
enable the backlight". What's the order of backlight_on and prepare (and
their counterparts)? What's the division of responsibilities?
> + int (*get_info)(struct drm_panel *panel,
> + struct drm_connector *connector);
As I said in my earlier review, I don't think this is needed.
BR,
Jani.
> };
>
> struct drm_panel {
> @@ -117,6 +123,47 @@ static inline int drm_panel_enable(struct drm_panel *panel)
> return panel ? -ENOSYS : -EINVAL;
> }
>
> +static inline int drm_panel_power_on(struct drm_panel *panel)
> +{
> + if (panel && panel->funcs && panel->funcs->power_on)
> + return panel->funcs->power_on(panel);
> +
> + return panel ? -ENOSYS : -EINVAL;
> +}
> +
> +static inline int drm_panel_power_off(struct drm_panel *panel)
> +{
> + if (panel && panel->funcs && panel->funcs->power_off)
> + return panel->funcs->power_off(panel);
> +
> + return panel ? -ENOSYS : -EINVAL;
> +}
> +
> +static inline int drm_panel_backlight_on(struct drm_panel *panel)
> +{
> + if (panel && panel->funcs && panel->funcs->backlight_on)
> + return panel->funcs->backlight_on(panel);
> +
> + return panel ? -ENOSYS : -EINVAL;
> +}
> +
> +static inline int drm_panel_backlight_off(struct drm_panel *panel)
> +{
> + if (panel && panel->funcs && panel->funcs->backlight_off)
> + return panel->funcs->backlight_off(panel);
> +
> + return panel ? -ENOSYS : -EINVAL;
> +}
> +
> +static inline int drm_panel_get_info(struct drm_panel *panel,
> + struct drm_connector *connector)
> +{
> + if (connector && panel && panel->funcs && panel->funcs->get_info)
> + return panel->funcs->get_info(panel, connector);
> +
> + return panel ? -ENOSYS : -EINVAL;
> +}
> +
> static inline int drm_panel_get_modes(struct drm_panel *panel)
> {
> if (panel && panel->funcs && panel->funcs->get_modes)
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
