[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <874kgjl0dn.fsf@intel.com>
Date: Tue, 06 Apr 2021 21:43:32 +0300
From: Jani Nikula <jani.nikula@...el.com>
To: lyude@...hat.com, intel-gfx@...ts.freedesktop.org
Cc: David Airlie <airlied@...ux.ie>,
open list <linux-kernel@...r.kernel.org>,
"open list\:DRM DRIVERS" <dri-devel@...ts.freedesktop.org>,
Sean Paul <seanpaul@...omium.org>,
Aaron Ma <aaron.ma@...onical.com>,
Dave Airlie <airlied@...hat.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/dpcd_bl: Don't try vesa interface unless specified by VBT
On Tue, 23 Mar 2021, Lyude Paul <lyude@...hat.com> wrote:
> On Tue, 2021-03-23 at 16:06 +0200, Jani Nikula wrote:
>> On Thu, 18 Mar 2021, Lyude Paul <lyude@...hat.com> 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@...el.com>
>
>>
>>
>> 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@...el.com>
>> > > Cc: Rodrigo Vivi <rodrigo.vivi@...el.com>
>> > > Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/3169
>> > > Signed-off-by: Lyude Paul <lyude@...hat.com>
>> > > ---
>> > > 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
Powered by blists - more mailing lists