Re: [PATCH] drm/i915/dpcd_bl: Don't try vesa interface unless specified by VBT

[Lyude Paul]

Actually-NAK this. I just realized I've been misreading the bug and that this
doesn't actually seem to be fixed. Will resend once I figure out what's going on

On Thu, 2021-03-18 at 13:02 -0400, Lyude Paul wrote:
> Looks like that there actually are another subset of laptops on the market
> that don't support the Intel HDR backlight interface, but do advertise
> support for the VESA DPCD backlight interface despite the fact it doesn't
> seem to work.
> 
> Note though I'm not entirely clear on this - on one of the machines where
> this issue was observed, I also noticed that we appeared to be rejecting
> the VBT defined backlight frequency in
> intel_dp_aux_vesa_calc_max_backlight(). It's noted in this function that:
> 
> /* Use highest possible value of Pn for more granularity of brightness
>  * adjustment while satifying the conditions below.
>  * ...
>  * - FxP is within 25% of desired value.
>  *   Note: 25% is arbitrary value and may need some tweak.
>  */
> 
> So it's possible that this value might just need to be tweaked, but for now
> let's just disable the VESA backlight interface unless it's specified in
> the VBT just to be safe. We might be able to try enabling this again by
> default in the future.
> 
> Fixes: 2227816e647a ("drm/i915/dp: Allow forcing specific interfaces through
> enable_dpcd_backlight")
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/3169
> Signed-off-by: Lyude Paul <lyude@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> 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 651884390137..4f8337c7fd2e 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -646,7 +646,6 @@ int intel_dp_aux_init_backlight_funcs(struct
> intel_connector *connector)
>                         break;
>                 case INTEL_BACKLIGHT_DISPLAY_DDI:
>                         try_intel_interface = true;
> -                       try_vesa_interface = true;
>                         break;
>                 default:
>                         return -ENODEV;

-- 
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!