| Message ID | 20210812154237.13911-7-shawn.c.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MIPI DSI driver enhancements | expand |
> -----Original Message----- > From: Lee, Shawn C <shawn.c.lee@intel.com> > Sent: Thursday, August 12, 2021 9:13 PM > To: intel-gfx@lists.freedesktop.org > Cc: Nikula, Jani <jani.nikula@intel.com>; ville.syrjala@linux.intel.com; > Kulkarni, Vandita <vandita.kulkarni@intel.com>; Chiou, Cooper > <cooper.chiou@intel.com>; Tseng, William <william.tseng@intel.com>; Lee, > Shawn C <shawn.c.lee@intel.com>; Jani Nikula <jani.nikula@linux.intel.com> > Subject: [v4 6/7] drm/i915/dsi: Retrieve max brightness level from VBT. > > So far, DCS backlight driver hardcode (0xFF) for max brightness level. > MIPI DCS spec allow max 0xFFFF for set_display_brightness (51h) command. > And VBT brightness precision bits can support 8 ~ 16 bits. > > We should set correct precision bits in VBT that meet panel's request. > Driver can refer to this setting then configure max brightness level in DCS > backlight driver properly. > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > Cc: Cooper Chiou <cooper.chiou@intel.com> > Cc: William Tseng <william.tseng@intel.com> > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > --- > drivers/gpu/drm/i915/display/intel_bios.c | 3 +++ > .../gpu/drm/i915/display/intel_dsi_dcs_backlight.c | 12 +++++++++--- > drivers/gpu/drm/i915/i915_drv.h | 1 + > 3 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > b/drivers/gpu/drm/i915/display/intel_bios.c > index e86e6ed2d3bf..1affd679d1d1 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -483,6 +483,9 @@ parse_lfp_backlight(struct drm_i915_private *i915, > level = 255; > } > i915->vbt.backlight.min_brightness = min_level; > + > + i915->vbt.backlight.max_brightness_level = > + (1 << backlight_data- > >brightness_precision_bits[panel_type]) - 1; > } else { > level = backlight_data->level[panel_type]; > i915->vbt.backlight.min_brightness = entry->min_brightness; > diff --git a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c > b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c > index 584c14c4cbd0..cd85520d36e2 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c > @@ -41,7 +41,7 @@ > #define POWER_SAVE_HIGH (3 << 0) > #define POWER_SAVE_OUTDOOR_MODE (4 << 0) > > -#define PANEL_PWM_MAX_VALUE 0xFF Shouldn't this be panel dependant too? > +#define PANEL_PWM_MAX_VALUE 0xFFFF > > static u32 dcs_get_backlight(struct intel_connector *connector, enum pipe > unused) { @@ -147,10 +147,16 @@ static void dcs_enable_backlight(const > struct intel_crtc_state *crtc_state, static int dcs_setup_backlight(struct > intel_connector *connector, > enum pipe unused) > { > + struct drm_device *dev = connector->base.dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_panel *panel = &connector->panel; > > - panel->backlight.max = PANEL_PWM_MAX_VALUE; > - panel->backlight.level = PANEL_PWM_MAX_VALUE; > + panel->backlight.max = dev_priv- > >vbt.backlight.max_brightness_level \ > + ? dev_priv->vbt.backlight.max_brightness_level \ > + : PANEL_PWM_MAX_VALUE; > + panel->backlight.level = dev_priv- > >vbt.backlight.max_brightness_level \ > + ? dev_priv- > >vbt.backlight.max_brightness_level \ > + : PANEL_PWM_MAX_VALUE; > > return 0; > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h index 005b1cec7007..fbb3f18e7b8e > 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -706,6 +706,7 @@ struct intel_vbt_data { > > struct { > u16 pwm_freq_hz; VBT doesn't provide max_brightness_level, instead it gives us Brightness Precision bits from version 236 onwards. > + u16 max_brightness_level; > bool present; > bool active_low_pwm; > u8 min_brightness; /* min_brightness/255 of max */ > -- > 2.17.1
On Mon, 23 Aug 2021, Kulkarni, Vandita <vandita.kulkarni@intel.com> wrote: >> >> So far, DCS backlight driver hardcode (0xFF) for max brightness level. >> MIPI DCS spec allow max 0xFFFF for set_display_brightness (51h) command. >> And VBT brightness precision bits can support 8 ~ 16 bits. >> >> We should set correct precision bits in VBT that meet panel's request. >> Driver can refer to this setting then configure max brightness level >> in DCS backlight driver properly. >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> >> Cc: Cooper Chiou <cooper.chiou@intel.com> >> Cc: William Tseng <william.tseng@intel.com> >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_bios.c | 3 +++ >> .../gpu/drm/i915/display/intel_dsi_dcs_backlight.c | 12 +++++++++--- >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> 3 files changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c >> b/drivers/gpu/drm/i915/display/intel_bios.c >> index e86e6ed2d3bf..1affd679d1d1 100644 >> --- a/drivers/gpu/drm/i915/display/intel_bios.c >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c >> @@ -483,6 +483,9 @@ parse_lfp_backlight(struct drm_i915_private *i915, >> level = 255; } i915->vbt.backlight.min_brightness = min_level; >> + >> +i915->vbt.backlight.max_brightness_level = >> +(1 << backlight_data- >> >brightness_precision_bits[panel_type]) - 1; >> } else { >> level = backlight_data->level[panel_type]; >> i915->vbt.backlight.min_brightness = entry->min_brightness; diff >> --git a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c >> b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c >> index 584c14c4cbd0..cd85520d36e2 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c >> +++ b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c >> @@ -41,7 +41,7 @@ >> #define POWER_SAVE_HIGH(3 << 0) >> #define POWER_SAVE_OUTDOOR_MODE(4 << 0) >> >> -#define PANEL_PWM_MAX_VALUE0xFF > >Shouldn't this be panel dependant too? If driver did not retrieve proper precision bits, we will use PANEL_PWM_MAX_VALUE as default max backlight. Do you recommend to keep 0xFF here? >> +#define PANEL_PWM_MAX_VALUE0xFFFF >> >> static u32 dcs_get_backlight(struct intel_connector *connector, enum >> pipe >> unused) { @@ -147,10 +147,16 @@ static void >> dcs_enable_backlight(const struct intel_crtc_state *crtc_state, >> static int dcs_setup_backlight(struct intel_connector *connector, >> enum pipe unused) >> { >> +struct drm_device *dev = connector->base.dev; struct drm_i915_private >> +*dev_priv = to_i915(dev); >> struct intel_panel *panel = &connector->panel; >> >> -panel->backlight.max = PANEL_PWM_MAX_VALUE; backlight.level = >> -panel->PANEL_PWM_MAX_VALUE; >> +panel->backlight.max = dev_priv- >> >vbt.backlight.max_brightness_level \ >> + ? dev_priv->vbt.backlight.max_brightness_level \ >> + : PANEL_PWM_MAX_VALUE; >> +panel->backlight.level = dev_priv- >> >vbt.backlight.max_brightness_level \ >> + ? dev_priv- >> >vbt.backlight.max_brightness_level \ >> + : PANEL_PWM_MAX_VALUE; >> >> return 0; >> } >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h index 005b1cec7007..fbb3f18e7b8e >> 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -706,6 +706,7 @@ struct intel_vbt_data { >> >> struct { >> u16 pwm_freq_hz; > >VBT doesn't provide max_brightness_level, instead it gives us Brightness Precision bits from version 236 onwards. I will modify this variable name to "brightness_precision_bits". And move the max brightness calculation into dcs_setup_backlight(). Best regards, Shawn >> +u16 max_brightness_level; >> bool present; >> bool active_low_pwm; >> u8 min_brightness;/* min_brightness/255 of max */ >> -- >> 2.17.1
> -----Original Message----- > From: > Sent: Monday, August 23, 2021 2:34 PM > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Nikula, Jani <jani.nikula@intel.com>; ville.syrjala@linux.intel.com; Chiou, > Cooper <cooper.chiou@intel.com>; Tseng, William > <william.tseng@intel.com>; Jani Nikula <jani.nikula@linux.intel.com> > Subject: RE: [v4 6/7] drm/i915/dsi: Retrieve max brightness level from VBT. > > On Mon, 23 Aug 2021, Kulkarni, Vandita <vandita.kulkarni@intel.com> wrote: > >> > >> So far, DCS backlight driver hardcode (0xFF) for max brightness level. > >> MIPI DCS spec allow max 0xFFFF for set_display_brightness (51h) > command. > >> And VBT brightness precision bits can support 8 ~ 16 bits. > >> > >> We should set correct precision bits in VBT that meet panel's request. > >> Driver can refer to this setting then configure max brightness level > >> in DCS backlight driver properly. > >> > >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > >> Cc: Jani Nikula <jani.nikula@linux.intel.com> > >> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > >> Cc: Cooper Chiou <cooper.chiou@intel.com> > >> Cc: William Tseng <william.tseng@intel.com> > >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_bios.c | 3 +++ > >> .../gpu/drm/i915/display/intel_dsi_dcs_backlight.c | 12 +++++++++--- > >> drivers/gpu/drm/i915/i915_drv.h | 1 + > >> 3 files changed, 13 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > >> b/drivers/gpu/drm/i915/display/intel_bios.c > >> index e86e6ed2d3bf..1affd679d1d1 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_bios.c > >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c > >> @@ -483,6 +483,9 @@ parse_lfp_backlight(struct drm_i915_private > >> *i915, level = 255; } i915->vbt.backlight.min_brightness = > >> min_level; > >> + > >> +i915->vbt.backlight.max_brightness_level = > >> +(1 << backlight_data- > >> >brightness_precision_bits[panel_type]) - 1; > >> } else { > >> level = backlight_data->level[panel_type]; > >> i915->vbt.backlight.min_brightness = entry->min_brightness; diff > >> --git a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c > >> b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c > >> index 584c14c4cbd0..cd85520d36e2 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c > >> +++ b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c > >> @@ -41,7 +41,7 @@ > >> #define POWER_SAVE_HIGH(3 << 0) > >> #define POWER_SAVE_OUTDOOR_MODE(4 << 0) > >> > >> -#define PANEL_PWM_MAX_VALUE0xFF > > > >Shouldn't this be panel dependant too? > > If driver did not retrieve proper precision bits, we will use > PANEL_PWM_MAX_VALUE as default max backlight. > Do you recommend to keep 0xFF here? Precision bits are defined from version 236 of VBT , if that is not valid or is equal to 8 then max brightness should be 0xFF. > > >> +#define PANEL_PWM_MAX_VALUE0xFFFF > >> > >> static u32 dcs_get_backlight(struct intel_connector *connector, enum > >> pipe > >> unused) { @@ -147,10 +147,16 @@ static void > >> dcs_enable_backlight(const struct intel_crtc_state *crtc_state, > >> static int dcs_setup_backlight(struct intel_connector *connector, > >> enum pipe unused) > >> { > >> +struct drm_device *dev = connector->base.dev; struct > >> +drm_i915_private *dev_priv = to_i915(dev); > >> struct intel_panel *panel = &connector->panel; > >> > >> -panel->backlight.max = PANEL_PWM_MAX_VALUE; backlight.level = > >> -panel->PANEL_PWM_MAX_VALUE; > >> +panel->backlight.max = dev_priv- > >> >vbt.backlight.max_brightness_level \ > >> + ? dev_priv->vbt.backlight.max_brightness_level \ > >> + : PANEL_PWM_MAX_VALUE; > >> +panel->backlight.level = dev_priv- > >> >vbt.backlight.max_brightness_level \ > >> + ? dev_priv- > >> >vbt.backlight.max_brightness_level \ > >> + : PANEL_PWM_MAX_VALUE; > >> > >> return 0; > >> } > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h > >> b/drivers/gpu/drm/i915/i915_drv.h index 005b1cec7007..fbb3f18e7b8e > >> 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -706,6 +706,7 @@ struct intel_vbt_data { > >> > >> struct { > >> u16 pwm_freq_hz; > > > >VBT doesn't provide max_brightness_level, instead it gives us Brightness > Precision bits from version 236 onwards. > > I will modify this variable name to "brightness_precision_bits". And move the > max brightness calculation into dcs_setup_backlight(). > > Best regards, > Shawn > > >> +u16 max_brightness_level; > >> bool present; > >> bool active_low_pwm; > >> u8 min_brightness;/* min_brightness/255 of max */ > >> -- > >> 2.17.1
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index e86e6ed2d3bf..1affd679d1d1 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -483,6 +483,9 @@ parse_lfp_backlight(struct drm_i915_private *i915, level = 255; } i915->vbt.backlight.min_brightness = min_level; + + i915->vbt.backlight.max_brightness_level = + (1 << backlight_data->brightness_precision_bits[panel_type]) - 1; } else { level = backlight_data->level[panel_type]; i915->vbt.backlight.min_brightness = entry->min_brightness; diff --git a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c index 584c14c4cbd0..cd85520d36e2 100644 --- a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c @@ -41,7 +41,7 @@ #define POWER_SAVE_HIGH (3 << 0) #define POWER_SAVE_OUTDOOR_MODE (4 << 0) -#define PANEL_PWM_MAX_VALUE 0xFF +#define PANEL_PWM_MAX_VALUE 0xFFFF static u32 dcs_get_backlight(struct intel_connector *connector, enum pipe unused) { @@ -147,10 +147,16 @@ static void dcs_enable_backlight(const struct intel_crtc_state *crtc_state, static int dcs_setup_backlight(struct intel_connector *connector, enum pipe unused) { + struct drm_device *dev = connector->base.dev; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_panel *panel = &connector->panel; - panel->backlight.max = PANEL_PWM_MAX_VALUE; - panel->backlight.level = PANEL_PWM_MAX_VALUE; + panel->backlight.max = dev_priv->vbt.backlight.max_brightness_level \ + ? dev_priv->vbt.backlight.max_brightness_level \ + : PANEL_PWM_MAX_VALUE; + panel->backlight.level = dev_priv->vbt.backlight.max_brightness_level \ + ? dev_priv->vbt.backlight.max_brightness_level \ + : PANEL_PWM_MAX_VALUE; return 0; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 005b1cec7007..fbb3f18e7b8e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -706,6 +706,7 @@ struct intel_vbt_data { struct { u16 pwm_freq_hz; + u16 max_brightness_level; bool present; bool active_low_pwm; u8 min_brightness; /* min_brightness/255 of max */
So far, DCS backlight driver hardcode (0xFF) for max brightness level. MIPI DCS spec allow max 0xFFFF for set_display_brightness (51h) command. And VBT brightness precision bits can support 8 ~ 16 bits. We should set correct precision bits in VBT that meet panel's request. Driver can refer to this setting then configure max brightness level in DCS backlight driver properly. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> Cc: Cooper Chiou <cooper.chiou@intel.com> Cc: William Tseng <william.tseng@intel.com> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> --- drivers/gpu/drm/i915/display/intel_bios.c | 3 +++ .../gpu/drm/i915/display/intel_dsi_dcs_backlight.c | 12 +++++++++--- drivers/gpu/drm/i915/i915_drv.h | 1 + 3 files changed, 13 insertions(+), 3 deletions(-)