| Message ID | 20191207140353.23967-2-sam@ravnborg.org |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/panel infrastructure + backlight update | expand |
Hi Sam, Thank you for the patch. On Sat, Dec 07, 2019 at 03:03:29PM +0100, Sam Ravnborg wrote: > The callbacks in drm_panel_funcs are optional, so do not > return an error just because no callback is assigned. > > v2: > - Document what functions in drm_panel_funcs are optional (Laurent) > - Return -EOPNOTSUPP if get_modes() is not assigned (Laurent) > (Sam: -EOPNOTSUPP seems to best error code in this situation) > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/drm_panel.c | 35 +++++++++++++++++++++++++---------- > include/drm/drm_panel.h | 18 ++++++++++++++++-- > 2 files changed, 41 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index ed7985c0535a..4ab7229fb22b 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c > @@ -151,10 +151,13 @@ EXPORT_SYMBOL(drm_panel_detach); > */ > int drm_panel_prepare(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->prepare) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->prepare) > return panel->funcs->prepare(panel); > > - return panel ? -ENOSYS : -EINVAL; > + return 0; > } > EXPORT_SYMBOL(drm_panel_prepare); > > @@ -171,10 +174,13 @@ EXPORT_SYMBOL(drm_panel_prepare); > */ > int drm_panel_unprepare(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->unprepare) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->unprepare) > return panel->funcs->unprepare(panel); > > - return panel ? -ENOSYS : -EINVAL; > + return 0; > } > EXPORT_SYMBOL(drm_panel_unprepare); > > @@ -190,10 +196,13 @@ EXPORT_SYMBOL(drm_panel_unprepare); > */ > int drm_panel_enable(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->enable) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->enable) > return panel->funcs->enable(panel); > > - return panel ? -ENOSYS : -EINVAL; > + return 0; > } > EXPORT_SYMBOL(drm_panel_enable); > > @@ -209,10 +218,13 @@ EXPORT_SYMBOL(drm_panel_enable); > */ > int drm_panel_disable(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->disable) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->disable) > return panel->funcs->disable(panel); > > - return panel ? -ENOSYS : -EINVAL; > + return 0; > } > EXPORT_SYMBOL(drm_panel_disable); > > @@ -228,10 +240,13 @@ EXPORT_SYMBOL(drm_panel_disable); > */ > int drm_panel_get_modes(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->get_modes) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->get_modes) > return panel->funcs->get_modes(panel); > > - return panel ? -ENOSYS : -EINVAL; > + return -EOPNOTSUPP; > } > EXPORT_SYMBOL(drm_panel_get_modes); > > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h > index ce8da64022b4..d71655b2634c 100644 > --- a/include/drm/drm_panel.h > +++ b/include/drm/drm_panel.h > @@ -65,6 +65,8 @@ struct drm_panel_funcs { > * @prepare: > * > * Turn on panel and perform set up. > + * > + * This function is optional. > */ > int (*prepare)(struct drm_panel *panel); > > @@ -72,6 +74,8 @@ struct drm_panel_funcs { > * @enable: > * > * Enable panel (turn on back light, etc.). > + * > + * This function is optional. > */ > int (*enable)(struct drm_panel *panel); > > @@ -79,6 +83,8 @@ struct drm_panel_funcs { > * @disable: > * > * Disable panel (turn off back light, etc.). > + * > + * This function is optional. > */ > int (*disable)(struct drm_panel *panel); > > @@ -86,14 +92,20 @@ struct drm_panel_funcs { > * @unprepare: > * > * Turn off panel. > + * > + * This function is optional. > */ > 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. > + * Add modes to the connector that the panel is attached to. > + * > + * This function is mandatory. > + * > + * Returns the number of modes added, -EOPNOTSUPP if callback > + * is missing, -EINVAL if panel is NULL. This applies to drm_panel_get_modes, not &drm_panel_funcs.get_modes. Here you should just have * * Add modes to the connector that the panel is attached to and * return the number of modes added. + * + * This function is mandatory. */ int (*get_modes)(struct drm_panel *panel); > */ > int (*get_modes)(struct drm_panel *panel); > > @@ -102,6 +114,8 @@ struct drm_panel_funcs { > * > * Copy display timings into the provided array and return > * the number of display timings available. > + * > + * This function is optional. > */ > int (*get_timings)(struct drm_panel *panel, unsigned int num_timings, > struct display_timing *timings);
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index ed7985c0535a..4ab7229fb22b 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -151,10 +151,13 @@ EXPORT_SYMBOL(drm_panel_detach); */ int drm_panel_prepare(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->prepare) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->prepare) return panel->funcs->prepare(panel); - return panel ? -ENOSYS : -EINVAL; + return 0; } EXPORT_SYMBOL(drm_panel_prepare); @@ -171,10 +174,13 @@ EXPORT_SYMBOL(drm_panel_prepare); */ int drm_panel_unprepare(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->unprepare) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->unprepare) return panel->funcs->unprepare(panel); - return panel ? -ENOSYS : -EINVAL; + return 0; } EXPORT_SYMBOL(drm_panel_unprepare); @@ -190,10 +196,13 @@ EXPORT_SYMBOL(drm_panel_unprepare); */ int drm_panel_enable(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->enable) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->enable) return panel->funcs->enable(panel); - return panel ? -ENOSYS : -EINVAL; + return 0; } EXPORT_SYMBOL(drm_panel_enable); @@ -209,10 +218,13 @@ EXPORT_SYMBOL(drm_panel_enable); */ int drm_panel_disable(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->disable) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->disable) return panel->funcs->disable(panel); - return panel ? -ENOSYS : -EINVAL; + return 0; } EXPORT_SYMBOL(drm_panel_disable); @@ -228,10 +240,13 @@ EXPORT_SYMBOL(drm_panel_disable); */ int drm_panel_get_modes(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->get_modes) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->get_modes) return panel->funcs->get_modes(panel); - return panel ? -ENOSYS : -EINVAL; + return -EOPNOTSUPP; } EXPORT_SYMBOL(drm_panel_get_modes); diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index ce8da64022b4..d71655b2634c 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -65,6 +65,8 @@ struct drm_panel_funcs { * @prepare: * * Turn on panel and perform set up. + * + * This function is optional. */ int (*prepare)(struct drm_panel *panel); @@ -72,6 +74,8 @@ struct drm_panel_funcs { * @enable: * * Enable panel (turn on back light, etc.). + * + * This function is optional. */ int (*enable)(struct drm_panel *panel); @@ -79,6 +83,8 @@ struct drm_panel_funcs { * @disable: * * Disable panel (turn off back light, etc.). + * + * This function is optional. */ int (*disable)(struct drm_panel *panel); @@ -86,14 +92,20 @@ struct drm_panel_funcs { * @unprepare: * * Turn off panel. + * + * This function is optional. */ 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. + * Add modes to the connector that the panel is attached to. + * + * This function is mandatory. + * + * Returns the number of modes added, -EOPNOTSUPP if callback + * is missing, -EINVAL if panel is NULL. */ int (*get_modes)(struct drm_panel *panel); @@ -102,6 +114,8 @@ struct drm_panel_funcs { * * Copy display timings into the provided array and return * the number of display timings available. + * + * This function is optional. */ int (*get_timings)(struct drm_panel *panel, unsigned int num_timings, struct display_timing *timings);