Messages in this thread | | | Date | Tue, 13 Jul 2021 12:57:42 +0530 | From | rajeevny@codeauro ... | Subject | Re: [PATCH v2] drm/dp: Move panel DP AUX backlight support to drm_dp_helper |
| |
Hi Doug,
On 12-07-2021 20:30, Douglas Anderson wrote: > We were getting a depmod error: > depmod: ERROR: Cycle detected: drm_kms_helper -> drm -> > drm_kms_helper > > It looks like the rule is that drm_kms_helper can call into drm, but > drm can't call into drm_kms_helper. That means we've got to move the > DP AUX backlight support into drm_dp_helper. > > NOTE: as part of this, I didn't try to do any renames of the main > registration function. Even though it's in the drm_dp_helper, it still > feels very parallel to drm_panel_of_backlight(). > > Fixes: 10f7b40e4f30 ("drm/panel: add basic DP AUX backlight support") > Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Reported-by: Thomas Zimmermann <tzimmermann@suse.de> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > Note that I've compile tested this, but I don't have a device setup > yet that uses this code. Since the code is basically the same as it > was this should be OK, but if Rajeev could confirm that nothing is > broken that'd be nice. >
Reviewed-by: Rajeev Nandan <rajeevny@codeaurora.org>
Thanks, Rajeev
> Changes in v2: > - Guard new functions by the proper configs. > > drivers/gpu/drm/drm_dp_helper.c | 113 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_panel.c | 108 ------------------------------ > include/drm/drm_dp_helper.h | 16 +++++ > include/drm/drm_panel.h | 8 --- > 4 files changed, 129 insertions(+), 116 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > b/drivers/gpu/drm/drm_dp_helper.c > index 24bbc710c825..e8eec20ab364 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -33,9 +33,17 @@ > #include <drm/drm_print.h> > #include <drm/drm_vblank.h> > #include <drm/drm_dp_mst_helper.h> > +#include <drm/drm_panel.h> > > #include "drm_crtc_helper_internal.h" > > +struct dp_aux_backlight { > + struct backlight_device *base; > + struct drm_dp_aux *aux; > + struct drm_edp_backlight_info info; > + bool enabled; > +}; > + > /** > * DOC: dp helpers > * > @@ -3462,3 +3470,108 @@ drm_edp_backlight_init(struct drm_dp_aux *aux, > struct drm_edp_backlight_info *bl > return 0; > } > EXPORT_SYMBOL(drm_edp_backlight_init); > + > +#if IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \ > + (IS_MODULE(CONFIG_DRM_KMS_HELPER) && > IS_MODULE(CONFIG_BACKLIGHT_CLASS_DEVICE)) > + > +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 > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index 4fa1e3bb1b78..f634371c717a 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c > @@ -26,20 +26,12 @@ > #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 > * > @@ -350,106 +342,6 @@ 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_dp_helper.h b/include/drm/drm_dp_helper.h > index 729d5d82475e..a1b2d945def6 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -30,6 +30,7 @@ > > struct drm_device; > struct drm_dp_aux; > +struct drm_panel; > > /* > * Unless otherwise noted, all values are from the DP 1.1a spec. Note > that > @@ -2200,6 +2201,21 @@ int drm_edp_backlight_enable(struct drm_dp_aux > *aux, const struct drm_edp_backli > u16 level); > int drm_edp_backlight_disable(struct drm_dp_aux *aux, const struct > drm_edp_backlight_info *bl); > > +#if IS_ENABLED(CONFIG_DRM_KMS_HELPER) && > (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \ > + (IS_MODULE(CONFIG_DRM_KMS_HELPER) && > IS_MODULE(CONFIG_BACKLIGHT_CLASS_DEVICE))) > + > +int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct > drm_dp_aux *aux); > + > +#else > + > +static inline int drm_panel_dp_aux_backlight(struct drm_panel *panel, > + struct drm_dp_aux *aux) > +{ > + return 0; > +} > + > +#endif > + > #ifdef CONFIG_DRM_DP_CEC > void drm_dp_cec_irq(struct drm_dp_aux *aux); > void drm_dp_cec_register_connector(struct drm_dp_aux *aux, > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h > index 71aac751a032..4602f833eb51 100644 > --- a/include/drm/drm_panel.h > +++ b/include/drm/drm_panel.h > @@ -32,7 +32,6 @@ struct backlight_device; > struct device_node; > struct drm_connector; > struct drm_device; > -struct drm_dp_aux; > struct drm_panel; > struct display_timing; > > @@ -209,18 +208,11 @@ 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
| |