Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>
Let me know when you've pushed this upstream and I'll go ahead and send out a
rebased version of my backlight series.
On Fri, 2021-01-08 at 17:28 +0200, Jani Nikula wrote:
> The pch_get_backlight(), lpt_get_backlight(), and lpt_set_backlight()
> functions operate directly on the hardware registers. If inverting the
> value is needed, using intel_panel_compute_brightness(), it should only
> be done in the interface between hardware registers and
> panel->backlight.level.
>
> The CPU mode takeover code added in commit 5b1ec9ac7ab5
> ("drm/i915/backlight: Fix backlight takeover on LPT, v3.") reads the
> hardware register and converts to panel->backlight.level correctly,
> however the value written back should remain in the hardware register
> "domain".
>
> This hasn't been an issue, because GM45 machines are the only known
> users of i915.invert_brightness and the brightness invert quirk, and
> without one of them no conversion is made. It's likely nobody's ever hit
> the problem.
>
> Fixes: 5b1ec9ac7ab5 ("drm/i915/backlight: Fix backlight takeover on LPT, v3.")
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Lyude Paul <lyude@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v5.1+
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_panel.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
> b/drivers/gpu/drm/i915/display/intel_panel.c
> index 67f81ae995c4..7a4239d1c241 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -1649,16 +1649,13 @@ static int lpt_setup_backlight(struct intel_connector
> *connector, enum pipe unus
> val = pch_get_backlight(connector);
> else
> val = lpt_get_backlight(connector);
> - val = intel_panel_compute_brightness(connector, val);
> - panel->backlight.level = clamp(val, panel->backlight.min,
> - panel->backlight.max);
>
> if (cpu_mode) {
> drm_dbg_kms(&dev_priv->drm,
> "CPU backlight register was enabled, switching to
> PCH override\n");
>
> /* Write converted CPU PWM value to PCH override register */
> - lpt_set_backlight(connector->base.state, panel-
> >backlight.level);
> + lpt_set_backlight(connector->base.state, val);
> intel_de_write(dev_priv, BLC_PWM_PCH_CTL1,
> pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE);
>
> @@ -1666,6 +1663,10 @@ static int lpt_setup_backlight(struct intel_connector
> *connector, enum pipe unus
> cpu_ctl2 & ~BLM_PWM_ENABLE);
> }
>
> + val = intel_panel_compute_brightness(connector, val);
> + panel->backlight.level = clamp(val, panel->backlight.min,
> + panel->backlight.max);
> +
> return 0;
> }
>
--
Sincerely,
Lyude Paul (she/her)
Software Engineer at Red Hat
Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
asked me a question, are waiting for a review/merge on a patch, etc. and I
haven't responded in a while, please feel free to send me another email to check
on my status. I don't bite!
