diff mbox series

[7/7] drm/i915/dp: Limit brightness level to vbt min brightness

Message ID 20240405083704.393996-2-suraj.kandpal@intel.com (mailing list archive)
State New
Headers show
Series None | expand

Commit Message

Suraj Kandpal April 5, 2024, 8:37 a.m. UTC
Limit minimum brightness to vbt min brightness when using aux
based brightness control to avoid letting the screen
from going completely blank.
Sometimes vbt can have some bogus values hence clamping the value
for sanity in case of corner case.

--v2
-Use something same mechanism to limit minimum brightness
that PWM method uses [Jani]

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Sebastian Wick April 5, 2024, 5:55 p.m. UTC | #1
On Fri, Apr 05, 2024 at 02:07:05PM +0530, Suraj Kandpal wrote:
> Limit minimum brightness to vbt min brightness when using aux
> based brightness control to avoid letting the screen
> from going completely blank.
> Sometimes vbt can have some bogus values hence clamping the value
> for sanity in case of corner case.

So, you're completely ignoring the value from the EDID now instead?

> 
> --v2
> -Use something same mechanism to limit minimum brightness
> that PWM method uses [Jani]
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index 20dd5a6a0f3f..eb2a7225dfaa 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -411,6 +411,8 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi
>  	struct intel_panel *panel = &connector->panel;
>  	struct drm_luminance_range_info *luminance_range =
>  		&connector->base.display_info.luminance_range;
> +	u32 min_level = clamp_t(u32,
> +				connector->panel.vbt.backlight.min_brightness, 0, 64);
>  	int ret;
>  
>  	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] SDR backlight is controlled through %s\n",
> @@ -427,14 +429,12 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi
>  		}
>  	}
>  
> -	if (luminance_range->max_luminance) {
> +	if (luminance_range->max_luminance)
>  		panel->backlight.max = luminance_range->max_luminance;
> -		panel->backlight.min = luminance_range->min_luminance;
> -	} else {
> +	else
>  		panel->backlight.max = 512;
> -		panel->backlight.min = 0;
> -	}
>  
> +	panel->backlight.min = min_level;
>  	intel_dp_aux_write_panel_luminance_override(connector);
>  
>  	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Using AUX HDR interface for backlight control (range %d..%d)\n",
Suraj Kandpal April 8, 2024, 3:49 a.m. UTC | #2
> -----Original Message-----
> From: Sebastian Wick <sebastian.wick@redhat.com>
> Sent: Friday, April 5, 2024 11:26 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>;
> Murthy, Arun R <arun.r.murthy@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Kumar, Naveen1 <naveen1.kumar@intel.com>
> Subject: Re: [7/7] drm/i915/dp: Limit brightness level to vbt min brightness
> 
> On Fri, Apr 05, 2024 at 02:07:05PM +0530, Suraj Kandpal wrote:
> > Limit minimum brightness to vbt min brightness when using aux based
> > brightness control to avoid letting the screen from going completely
> > blank.
> > Sometimes vbt can have some bogus values hence clamping the value for
> > sanity in case of corner case.
> 
> So, you're completely ignoring the value from the EDID now instead?
> 

Yes we will be giving vbt value a preference but yes in case I can add a change wherein
If vbt value ends up being 0 to prefer edid val and if that too is 0 to take a min value of 20

Regards,
Suraj Kandpal
> >
> > --v2
> > -Use something same mechanism to limit minimum brightness that PWM
> > method uses [Jani]
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > index 20dd5a6a0f3f..eb2a7225dfaa 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > @@ -411,6 +411,8 @@ intel_dp_aux_hdr_setup_backlight(struct
> intel_connector *connector, enum pipe pi
> >  	struct intel_panel *panel = &connector->panel;
> >  	struct drm_luminance_range_info *luminance_range =
> >  		&connector->base.display_info.luminance_range;
> > +	u32 min_level = clamp_t(u32,
> > +				connector-
> >panel.vbt.backlight.min_brightness, 0, 64);
> >  	int ret;
> >
> >  	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] SDR backlight is
> > controlled through %s\n", @@ -427,14 +429,12 @@
> intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum
> pipe pi
> >  		}
> >  	}
> >
> > -	if (luminance_range->max_luminance) {
> > +	if (luminance_range->max_luminance)
> >  		panel->backlight.max = luminance_range->max_luminance;
> > -		panel->backlight.min = luminance_range->min_luminance;
> > -	} else {
> > +	else
> >  		panel->backlight.max = 512;
> > -		panel->backlight.min = 0;
> > -	}
> >
> > +	panel->backlight.min = min_level;
> >  	intel_dp_aux_write_panel_luminance_override(connector);
> >
> >  	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Using AUX HDR
> interface
> > for backlight control (range %d..%d)\n",
Suraj Kandpal April 8, 2024, 5:40 a.m. UTC | #3
> -----Original Message-----
> From: Sebastian Wick <sebastian.wick@redhat.com>
> Sent: Friday, April 5, 2024 11:26 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>;
> Murthy, Arun R <arun.r.murthy@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Kumar, Naveen1 <naveen1.kumar@intel.com>
> Subject: Re: [7/7] drm/i915/dp: Limit brightness level to vbt min brightness
> 
> On Fri, Apr 05, 2024 at 02:07:05PM +0530, Suraj Kandpal wrote:
> > Limit minimum brightness to vbt min brightness when using aux based
> > brightness control to avoid letting the screen from going completely
> > blank.
> > Sometimes vbt can have some bogus values hence clamping the value for
> > sanity in case of corner case.
> 
> So, you're completely ignoring the value from the EDID now instead?
> 
> >

Also after having  deeper look using the vbt value directly still will not get the job done
Since that too can end up being 0.
Usually when using the pwm path when we set brightness to 0 it usually results in a
25 percent brightness which is what ill be doing  in the next revision.
Set the min_luminance as max(25% of max_luminance, min_luminance) that way
We don't ignore values.

Regards,
Suraj Kandpal
> > --v2
> > -Use something same mechanism to limit minimum brightness that PWM
> > method uses [Jani]
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > index 20dd5a6a0f3f..eb2a7225dfaa 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > @@ -411,6 +411,8 @@ intel_dp_aux_hdr_setup_backlight(struct
> intel_connector *connector, enum pipe pi
> >  	struct intel_panel *panel = &connector->panel;
> >  	struct drm_luminance_range_info *luminance_range =
> >  		&connector->base.display_info.luminance_range;
> > +	u32 min_level = clamp_t(u32,
> > +				connector-
> >panel.vbt.backlight.min_brightness, 0, 64);
> >  	int ret;
> >
> >  	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] SDR backlight is
> > controlled through %s\n", @@ -427,14 +429,12 @@
> intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum
> pipe pi
> >  		}
> >  	}
> >
> > -	if (luminance_range->max_luminance) {
> > +	if (luminance_range->max_luminance)
> >  		panel->backlight.max = luminance_range->max_luminance;
> > -		panel->backlight.min = luminance_range->min_luminance;
> > -	} else {
> > +	else
> >  		panel->backlight.max = 512;
> > -		panel->backlight.min = 0;
> > -	}
> >
> > +	panel->backlight.min = min_level;
> >  	intel_dp_aux_write_panel_luminance_override(connector);
> >
> >  	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Using AUX HDR
> interface
> > for backlight control (range %d..%d)\n",
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index 20dd5a6a0f3f..eb2a7225dfaa 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -411,6 +411,8 @@  intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi
 	struct intel_panel *panel = &connector->panel;
 	struct drm_luminance_range_info *luminance_range =
 		&connector->base.display_info.luminance_range;
+	u32 min_level = clamp_t(u32,
+				connector->panel.vbt.backlight.min_brightness, 0, 64);
 	int ret;
 
 	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] SDR backlight is controlled through %s\n",
@@ -427,14 +429,12 @@  intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi
 		}
 	}
 
-	if (luminance_range->max_luminance) {
+	if (luminance_range->max_luminance)
 		panel->backlight.max = luminance_range->max_luminance;
-		panel->backlight.min = luminance_range->min_luminance;
-	} else {
+	else
 		panel->backlight.max = 512;
-		panel->backlight.min = 0;
-	}
 
+	panel->backlight.min = min_level;
 	intel_dp_aux_write_panel_luminance_override(connector);
 
 	drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Using AUX HDR interface for backlight control (range %d..%d)\n",