On Tue, 23 Mar 2021, Lyude Paul <lyude@xxxxxxxxxx> wrote: > On Tue, 2021-03-23 at 16:06 +0200, Jani Nikula wrote: >> On Thu, 18 Mar 2021, Lyude Paul <lyude@xxxxxxxxxx> wrote: >> > 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 >> >> Well, I think there are actually multiple issues on multiple >> machines. This fixes the issue on ThinkPad X1 Titanium Gen1 [1]. >> >> I suspect reverting 98e497e203a5 ("drm/i915/dpcd_bl: uncheck PWM_PIN_CAP >> when detect eDP backlight capabilities") would too. But then that would >> break *other* machines that claim support for *both* eDP PWM pin and >> DPCD backlight control, I think. >> >> I think there are issues with how we try setup DPCD backlight if the GOP >> has set up PWM backlight. For example, we don't set the backlight >> control mode correctly until the next disable/enable sequence. However, >> I tried to fix this, and I think I was doing all the right things, and >> DPCD reads seemed to confirm this, yet I was not able to control >> brightness using DPCD. I don't know what gives, but I do know eDP PWM >> pin control works. > > Should we go ahead and push the VESA fix for this then? If you're willing to > test future patches on that machine, we could give another shot at enabling this > in the future if we find reason to. Yes, let's go with this first. Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> > >> >> >> BR, >> Jani. >> >> >> [1] https://gitlab.freedesktop.org/drm/intel/-/issues/3158 >> >> >> > >> > 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; >> -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel