Message ID | 1624726268-14869-2-git-send-email-rajeevny@codeaurora.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20 | expand |
Am 26.06.21 um 18:51 schrieb Rajeev Nandan: > Some panels support backlight control over DP AUX channel using > VESA's standard backlight control interface. > Using new DRM eDP backlight helpers, add support to create and > register a backlight for those panels in drm_panel to simplify > the panel drivers. > > The panel driver with access to "struct drm_dp_aux" can create and > register a backlight device using following code snippet in its > probe() function: > > err = drm_panel_dp_aux_backlight(panel, aux); > if (err) > return err; > > Then drm_panel will handle backlight_(enable|disable) calls > similar to the case when drm_panel_of_backlight() is used. > > Currently, we are not supporting one feature where the source > device can combine the backlight brightness levels set through > DP AUX and the BL_PWM_DIM eDP connector pin. Since it's not > required for the basic backlight controls, it can be added later. > > Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org> > Reviewed-by: Douglas Anderson <dianders@chromium.org> > Reviewed-by: Lyude Paul <lyude@redhat.com> > --- > > Changes in v5: > - New > > Changes in v6: > - Fixed ordering of memory allocation (Douglas) > - Updated word wrapping in a comment (Douglas) > > Changes in v8: > - Now using backlight_is_blank() to get the backlight blank status (Sam Ravnborg) > > drivers/gpu/drm/drm_panel.c | 108 ++++++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_panel.h | 15 ++++-- > 2 files changed, 119 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index f634371..4fa1e3b 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c > @@ -26,12 +26,20 @@ > #include <linux/module.h> > > #include <drm/drm_crtc.h> > +#include <drm/drm_dp_helper.h> > #include <drm/drm_panel.h> > #include <drm/drm_print.h> > > static DEFINE_MUTEX(panel_lock); > static LIST_HEAD(panel_list); > > +struct dp_aux_backlight { > + struct backlight_device *base; > + struct drm_dp_aux *aux; > + struct drm_edp_backlight_info info; > + bool enabled; > +}; > + > /** > * DOC: drm panel > * > @@ -342,6 +350,106 @@ int drm_panel_of_backlight(struct drm_panel *panel) > return 0; > } > EXPORT_SYMBOL(drm_panel_of_backlight); > + > +static int dp_aux_backlight_update_status(struct backlight_device *bd) > +{ > + struct dp_aux_backlight *bl = bl_get_data(bd); > + u16 brightness = backlight_get_brightness(bd); > + int ret = 0; > + > + if (!backlight_is_blank(bd)) { > + if (!bl->enabled) { > + drm_edp_backlight_enable(bl->aux, &bl->info, brightness); > + bl->enabled = true; > + return 0; > + } > + ret = drm_edp_backlight_set_level(bl->aux, &bl->info, brightness); > + } else { > + if (bl->enabled) { > + drm_edp_backlight_disable(bl->aux, &bl->info); > + bl->enabled = false; > + } > + } > + > + return ret; > +} > + > +static const struct backlight_ops dp_aux_bl_ops = { > + .update_status = dp_aux_backlight_update_status, > +}; > + > +/** > + * drm_panel_dp_aux_backlight - create and use DP AUX backlight > + * @panel: DRM panel > + * @aux: The DP AUX channel to use > + * > + * Use this function to create and handle backlight if your panel > + * supports backlight control over DP AUX channel using DPCD > + * registers as per VESA's standard backlight control interface. > + * > + * When the panel is enabled backlight will be enabled after a > + * successful call to &drm_panel_funcs.enable() > + * > + * When the panel is disabled backlight will be disabled before the > + * call to &drm_panel_funcs.disable(). > + * > + * A typical implementation for a panel driver supporting backlight > + * control over DP AUX will call this function at probe time. > + * Backlight will then be handled transparently without requiring > + * any intervention from the driver. > + * > + * drm_panel_dp_aux_backlight() must be called after the call to drm_panel_init(). > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux) > +{ > + struct dp_aux_backlight *bl; > + struct backlight_properties props = { 0 }; > + u16 current_level; > + u8 current_mode; > + u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE]; > + int ret; > + > + if (!panel || !panel->dev || !aux) > + return -EINVAL; > + > + ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd, > + EDP_DISPLAY_CTL_CAP_SIZE); This creates a cyclic dependency between drm_kms_helper-ko and drm.ko. drm_panel.c is in the latter, while drm_dp_dpcd_read() in drm_dp_helper.c is in the former. Please fix. Best regards Thomas > + if (ret < 0) > + return ret; > + > + if (!drm_edp_backlight_supported(edp_dpcd)) { > + DRM_DEV_INFO(panel->dev, "DP AUX backlight is not supported\n"); > + return 0; > + } > + > + bl = devm_kzalloc(panel->dev, sizeof(*bl), GFP_KERNEL); > + if (!bl) > + return -ENOMEM; > + > + bl->aux = aux; > + > + ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd, > + ¤t_level, ¤t_mode); > + if (ret < 0) > + return ret; > + > + props.type = BACKLIGHT_RAW; > + props.brightness = current_level; > + props.max_brightness = bl->info.max; > + > + bl->base = devm_backlight_device_register(panel->dev, "dp_aux_backlight", > + panel->dev, bl, > + &dp_aux_bl_ops, &props); > + if (IS_ERR(bl->base)) > + return PTR_ERR(bl->base); > + > + panel->backlight = bl->base; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_panel_dp_aux_backlight); > #endif > > MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>"); > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h > index 33605c3..3ebfaa6 100644 > --- a/include/drm/drm_panel.h > +++ b/include/drm/drm_panel.h > @@ -32,6 +32,7 @@ struct backlight_device; > struct device_node; > struct drm_connector; > struct drm_device; > +struct drm_dp_aux; > struct drm_panel; > struct display_timing; > > @@ -64,8 +65,8 @@ enum drm_panel_orientation; > * the panel. This is the job of the .unprepare() function. > * > * Backlight can be handled automatically if configured using > - * drm_panel_of_backlight(). Then the driver does not need to implement the > - * functionality to enable/disable backlight. > + * drm_panel_of_backlight() or drm_panel_dp_aux_backlight(). Then the driver > + * does not need to implement the functionality to enable/disable backlight. > */ > struct drm_panel_funcs { > /** > @@ -144,8 +145,8 @@ struct drm_panel { > * Backlight device, used to turn on backlight after the call > * to enable(), and to turn off backlight before the call to > * disable(). > - * backlight is set by drm_panel_of_backlight() and drivers > - * shall not assign it. > + * backlight is set by drm_panel_of_backlight() or > + * drm_panel_dp_aux_backlight() and drivers shall not assign it. > */ > struct backlight_device *backlight; > > @@ -208,11 +209,17 @@ static inline int of_drm_get_panel_orientation(const struct device_node *np, > #if IS_ENABLED(CONFIG_DRM_PANEL) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \ > (IS_MODULE(CONFIG_DRM) && IS_MODULE(CONFIG_BACKLIGHT_CLASS_DEVICE))) > int drm_panel_of_backlight(struct drm_panel *panel); > +int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux); > #else > static inline int drm_panel_of_backlight(struct drm_panel *panel) > { > return 0; > } > +static inline int drm_panel_dp_aux_backlight(struct drm_panel *panel, > + struct drm_dp_aux *aux) > +{ > + return 0; > +} > #endif > > #endif >
Hi Am 12.07.21 um 11:41 schrieb Thomas Zimmermann: > > > Am 26.06.21 um 18:51 schrieb Rajeev Nandan: >> Some panels support backlight control over DP AUX channel using >> VESA's standard backlight control interface. >> Using new DRM eDP backlight helpers, add support to create and >> register a backlight for those panels in drm_panel to simplify >> the panel drivers. >> >> The panel driver with access to "struct drm_dp_aux" can create and >> register a backlight device using following code snippet in its >> probe() function: >> >> err = drm_panel_dp_aux_backlight(panel, aux); >> if (err) >> return err; >> >> Then drm_panel will handle backlight_(enable|disable) calls >> similar to the case when drm_panel_of_backlight() is used. >> >> Currently, we are not supporting one feature where the source >> device can combine the backlight brightness levels set through >> DP AUX and the BL_PWM_DIM eDP connector pin. Since it's not >> required for the basic backlight controls, it can be added later. >> >> Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org> >> Reviewed-by: Douglas Anderson <dianders@chromium.org> >> Reviewed-by: Lyude Paul <lyude@redhat.com> >> --- >> >> Changes in v5: >> - New >> >> Changes in v6: >> - Fixed ordering of memory allocation (Douglas) >> - Updated word wrapping in a comment (Douglas) >> >> Changes in v8: >> - Now using backlight_is_blank() to get the backlight blank status >> (Sam Ravnborg) >> >> drivers/gpu/drm/drm_panel.c | 108 >> ++++++++++++++++++++++++++++++++++++++++++++ >> include/drm/drm_panel.h | 15 ++++-- >> 2 files changed, 119 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c >> index f634371..4fa1e3b 100644 >> --- a/drivers/gpu/drm/drm_panel.c >> +++ b/drivers/gpu/drm/drm_panel.c >> @@ -26,12 +26,20 @@ >> #include <linux/module.h> >> #include <drm/drm_crtc.h> >> +#include <drm/drm_dp_helper.h> >> #include <drm/drm_panel.h> >> #include <drm/drm_print.h> >> static DEFINE_MUTEX(panel_lock); >> static LIST_HEAD(panel_list); >> +struct dp_aux_backlight { >> + struct backlight_device *base; >> + struct drm_dp_aux *aux; >> + struct drm_edp_backlight_info info; >> + bool enabled; >> +}; >> + >> /** >> * DOC: drm panel >> * >> @@ -342,6 +350,106 @@ int drm_panel_of_backlight(struct drm_panel *panel) >> return 0; >> } >> EXPORT_SYMBOL(drm_panel_of_backlight); >> + >> +static int dp_aux_backlight_update_status(struct backlight_device *bd) >> +{ >> + struct dp_aux_backlight *bl = bl_get_data(bd); >> + u16 brightness = backlight_get_brightness(bd); >> + int ret = 0; >> + >> + if (!backlight_is_blank(bd)) { >> + if (!bl->enabled) { >> + drm_edp_backlight_enable(bl->aux, &bl->info, brightness); >> + bl->enabled = true; >> + return 0; >> + } >> + ret = drm_edp_backlight_set_level(bl->aux, &bl->info, >> brightness); >> + } else { >> + if (bl->enabled) { >> + drm_edp_backlight_disable(bl->aux, &bl->info); >> + bl->enabled = false; >> + } >> + } >> + >> + return ret; >> +} >> + >> +static const struct backlight_ops dp_aux_bl_ops = { >> + .update_status = dp_aux_backlight_update_status, >> +}; >> + >> +/** >> + * drm_panel_dp_aux_backlight - create and use DP AUX backlight >> + * @panel: DRM panel >> + * @aux: The DP AUX channel to use >> + * >> + * Use this function to create and handle backlight if your panel >> + * supports backlight control over DP AUX channel using DPCD >> + * registers as per VESA's standard backlight control interface. >> + * >> + * When the panel is enabled backlight will be enabled after a >> + * successful call to &drm_panel_funcs.enable() >> + * >> + * When the panel is disabled backlight will be disabled before the >> + * call to &drm_panel_funcs.disable(). >> + * >> + * A typical implementation for a panel driver supporting backlight >> + * control over DP AUX will call this function at probe time. >> + * Backlight will then be handled transparently without requiring >> + * any intervention from the driver. >> + * >> + * drm_panel_dp_aux_backlight() must be called after the call to >> drm_panel_init(). >> + * >> + * Return: 0 on success or a negative error code on failure. >> + */ >> +int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct >> drm_dp_aux *aux) >> +{ >> + struct dp_aux_backlight *bl; >> + struct backlight_properties props = { 0 }; >> + u16 current_level; >> + u8 current_mode; >> + u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE]; >> + int ret; >> + >> + if (!panel || !panel->dev || !aux) >> + return -EINVAL; >> + >> + ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd, >> + EDP_DISPLAY_CTL_CAP_SIZE); > > This creates a cyclic dependency between drm_kms_helper-ko and drm.ko. > drm_panel.c is in the latter, while drm_dp_dpcd_read() in > drm_dp_helper.c is in the former. Please fix. FYI, build DRM as modules and the error shows up during make module_install. Best regards Thomas
Hi, On Mon, Jul 12, 2021 at 2:41 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > > + ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd, > > + EDP_DISPLAY_CTL_CAP_SIZE); > > This creates a cyclic dependency between drm_kms_helper-ko and drm.ko. > drm_panel.c is in the latter, while drm_dp_dpcd_read() in > drm_dp_helper.c is in the former. Please fix. Yeah, this was reported Friday and I posted a patch to try to fix it: https://lore.kernel.org/lkml/20210709152909.1.I23eb4cc5a680341e7b3e791632a635566fa5806a@changeid/ I see Rajeev had some feedback on it. Once I've dug out of my weekend email hole I'll take a look at plan to post a v2 ASAP. -Doug
Hi, On Mon, Jul 12, 2021 at 6:39 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Mon, Jul 12, 2021 at 2:41 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > > > > + ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd, > > > + EDP_DISPLAY_CTL_CAP_SIZE); > > > > This creates a cyclic dependency between drm_kms_helper-ko and drm.ko. > > drm_panel.c is in the latter, while drm_dp_dpcd_read() in > > drm_dp_helper.c is in the former. Please fix. > > Yeah, this was reported Friday and I posted a patch to try to fix it: > > https://lore.kernel.org/lkml/20210709152909.1.I23eb4cc5a680341e7b3e791632a635566fa5806a@changeid/ > > I see Rajeev had some feedback on it. Once I've dug out of my weekend > email hole I'll take a look at plan to post a v2 ASAP. Breadcrumbs: v2 of the fix is at: https://lore.kernel.org/lkml/20210712075933.v2.1.I23eb4cc5a680341e7b3e791632a635566fa5806a@changeid/ I'm hoping this looks OK so we can get this resolved before it trips too many people up. -Doug
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index f634371..4fa1e3b 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -26,12 +26,20 @@ #include <linux/module.h> #include <drm/drm_crtc.h> +#include <drm/drm_dp_helper.h> #include <drm/drm_panel.h> #include <drm/drm_print.h> static DEFINE_MUTEX(panel_lock); static LIST_HEAD(panel_list); +struct dp_aux_backlight { + struct backlight_device *base; + struct drm_dp_aux *aux; + struct drm_edp_backlight_info info; + bool enabled; +}; + /** * DOC: drm panel * @@ -342,6 +350,106 @@ int drm_panel_of_backlight(struct drm_panel *panel) return 0; } EXPORT_SYMBOL(drm_panel_of_backlight); + +static int dp_aux_backlight_update_status(struct backlight_device *bd) +{ + struct dp_aux_backlight *bl = bl_get_data(bd); + u16 brightness = backlight_get_brightness(bd); + int ret = 0; + + if (!backlight_is_blank(bd)) { + if (!bl->enabled) { + drm_edp_backlight_enable(bl->aux, &bl->info, brightness); + bl->enabled = true; + return 0; + } + ret = drm_edp_backlight_set_level(bl->aux, &bl->info, brightness); + } else { + if (bl->enabled) { + drm_edp_backlight_disable(bl->aux, &bl->info); + bl->enabled = false; + } + } + + return ret; +} + +static const struct backlight_ops dp_aux_bl_ops = { + .update_status = dp_aux_backlight_update_status, +}; + +/** + * drm_panel_dp_aux_backlight - create and use DP AUX backlight + * @panel: DRM panel + * @aux: The DP AUX channel to use + * + * Use this function to create and handle backlight if your panel + * supports backlight control over DP AUX channel using DPCD + * registers as per VESA's standard backlight control interface. + * + * When the panel is enabled backlight will be enabled after a + * successful call to &drm_panel_funcs.enable() + * + * When the panel is disabled backlight will be disabled before the + * call to &drm_panel_funcs.disable(). + * + * A typical implementation for a panel driver supporting backlight + * control over DP AUX will call this function at probe time. + * Backlight will then be handled transparently without requiring + * any intervention from the driver. + * + * drm_panel_dp_aux_backlight() must be called after the call to drm_panel_init(). + * + * Return: 0 on success or a negative error code on failure. + */ +int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux) +{ + struct dp_aux_backlight *bl; + struct backlight_properties props = { 0 }; + u16 current_level; + u8 current_mode; + u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE]; + int ret; + + if (!panel || !panel->dev || !aux) + return -EINVAL; + + ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd, + EDP_DISPLAY_CTL_CAP_SIZE); + if (ret < 0) + return ret; + + if (!drm_edp_backlight_supported(edp_dpcd)) { + DRM_DEV_INFO(panel->dev, "DP AUX backlight is not supported\n"); + return 0; + } + + bl = devm_kzalloc(panel->dev, sizeof(*bl), GFP_KERNEL); + if (!bl) + return -ENOMEM; + + bl->aux = aux; + + ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd, + ¤t_level, ¤t_mode); + if (ret < 0) + return ret; + + props.type = BACKLIGHT_RAW; + props.brightness = current_level; + props.max_brightness = bl->info.max; + + bl->base = devm_backlight_device_register(panel->dev, "dp_aux_backlight", + panel->dev, bl, + &dp_aux_bl_ops, &props); + if (IS_ERR(bl->base)) + return PTR_ERR(bl->base); + + panel->backlight = bl->base; + + return 0; +} +EXPORT_SYMBOL(drm_panel_dp_aux_backlight); #endif MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>"); diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 33605c3..3ebfaa6 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -32,6 +32,7 @@ struct backlight_device; struct device_node; struct drm_connector; struct drm_device; +struct drm_dp_aux; struct drm_panel; struct display_timing; @@ -64,8 +65,8 @@ enum drm_panel_orientation; * the panel. This is the job of the .unprepare() function. * * Backlight can be handled automatically if configured using - * drm_panel_of_backlight(). Then the driver does not need to implement the - * functionality to enable/disable backlight. + * drm_panel_of_backlight() or drm_panel_dp_aux_backlight(). Then the driver + * does not need to implement the functionality to enable/disable backlight. */ struct drm_panel_funcs { /** @@ -144,8 +145,8 @@ struct drm_panel { * Backlight device, used to turn on backlight after the call * to enable(), and to turn off backlight before the call to * disable(). - * backlight is set by drm_panel_of_backlight() and drivers - * shall not assign it. + * backlight is set by drm_panel_of_backlight() or + * drm_panel_dp_aux_backlight() and drivers shall not assign it. */ struct backlight_device *backlight; @@ -208,11 +209,17 @@ static inline int of_drm_get_panel_orientation(const struct device_node *np, #if IS_ENABLED(CONFIG_DRM_PANEL) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \ (IS_MODULE(CONFIG_DRM) && IS_MODULE(CONFIG_BACKLIGHT_CLASS_DEVICE))) int drm_panel_of_backlight(struct drm_panel *panel); +int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux); #else static inline int drm_panel_of_backlight(struct drm_panel *panel) { return 0; } +static inline int drm_panel_dp_aux_backlight(struct drm_panel *panel, + struct drm_dp_aux *aux) +{ + return 0; +} #endif #endif