On Tue, 07 Feb 2023, Ville Syrjala <[email protected]> wrote:
> From: Ville Syrjälä <[email protected]>
>
> Use the second backlight controller on ICP+ if the VBT asks
> us to do so.
>
> On pre-MTP we also check the chicken bit to make sure the
> pins have been correctly muxed by the firmware.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8016
> Signed-off-by: Ville Syrjälä <[email protected]>
> ---
> .../gpu/drm/i915/display/intel_backlight.c | 34 +++++++++++++++++--
> 1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c
> b/drivers/gpu/drm/i915/display/intel_backlight.c
> index 5b7da72c95b8..a4e4b7f79e4d 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -1431,6 +1431,30 @@ bxt_setup_backlight(struct intel_connector *connector,
> enum pipe unused)
> return 0;
> }
>
> +static int cnp_num_backlight_controllers(struct drm_i915_private *i915)
> +{
> + if (INTEL_PCH_TYPE(i915) >= PCH_DG1)
> + return 1;
> +
> + if (INTEL_PCH_TYPE(i915) >= PCH_ICP)
> + return 2;
> +
> + return 1;
> +}
At some point I think we should clean this up between backlight and
pps. There's already intel_num_pps(). But let's get this merged as-is
now.
> +
> +static bool cnp_backlight_controller_is_valid(struct drm_i915_private *i915,
> int controller)
> +{
> + if (controller < 0 || controller >= cnp_num_backlight_controllers(i915))
> + return false;
> +
> + if (controller == 1 &&
> + INTEL_PCH_TYPE(i915) >= PCH_ICP &&
> + INTEL_PCH_TYPE(i915) < PCH_MTP)
> + return intel_de_read(i915, SOUTH_CHICKEN1) &
> ICP_SECOND_PPS_IO_SELECT;
I got a bit confused that MTP has two controllers but it doesn't have
that bit. I first thought you were off by one with that < PCH_MTP. But
looks like it's correct.
Anyway,
Reviewed-by: Jani Nikula <[email protected]>
> +
> + return true;
> +}
> +
> static int
> cnp_setup_backlight(struct intel_connector *connector, enum pipe unused)
> {
> @@ -1440,10 +1464,14 @@ cnp_setup_backlight(struct intel_connector
> *connector, enum pipe unused)
>
> /*
> * CNP has the BXT implementation of backlight, but with only one
> - * controller. TODO: ICP has multiple controllers but we only use
> - * controller 0 for now.
> + * controller. ICP+ can have two controllers, depending on pin muxing.
> */
> - panel->backlight.controller = 0;
> + panel->backlight.controller = connector->panel.vbt.backlight.controller;
> + if (!cnp_backlight_controller_is_valid(i915,
> panel->backlight.controller)) {
> + drm_dbg_kms(&i915->drm, "Invalid backlight controller %d,
> assuming 0\n",
> + panel->backlight.controller);
> + panel->backlight.controller = 0;
> + }
>
> pwm_ctl = intel_de_read(i915,
> BXT_BLC_PWM_CTL(panel->backlight.controller));
--
Jani Nikula, Intel Open Source Graphics Center
