Re: [PATCH] drm/i915/dp: add a delay before setting panel brightness after power on

[Vasily Khoruzhick <anarsoul@gmail.com>]

On Wed, Sep 15, 2021 at 1:47 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Tue, 14 Sep 2021, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> > On Tue, Sep 14, 2021 at 3:31 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >>
> >> On Tue, 14 Sep 2021, Lyude Paul <lyude@redhat.com> wrote:
> >> > On Tue, 2021-09-14 at 12:09 +0300, Jani Nikula wrote:
> >> >> On Mon, 13 Sep 2021, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> >> >> > Panel in my Dell XPS 7590, that uses Intel's HDR backlight interface to
> >> >> > control brightness, apparently needs a delay before setting brightness
> >> >> > after power on. Without this delay the panel does accept the setting
> >> >> > and may come up with some arbitrary brightness (sometimes it's too dark,
> >> >> > sometimes it's too bright, I wasn't able to find a system).
> >> >> >
> >> >> > I don't have access to the spec, so I'm not sure if it's expected
> >> >> > behavior or a quirk for particular device.
> >> >> >
> >> >> > Delay was chosen by experiment: it works with 100ms, but fails with
> >> >> > anything lower than 75ms.
> >> >>
> >> >> Looks like we don't respect the panel delays for DPCD backlight. The
> >> >> values are used for setting up the panel power sequencer, and thus PWM
> >> >> based backlight, but we should probably use the delays in DPCD backlight
> >> >> code too.
> >> >
> >> > This makes sense to me, you're referring to the panel delays in the VBT
> >> > correct?
> >>
> >> Yes. See pps_init_delays() and intel_pps_backlight_on(). The delays
> >> aren't applied to DPCD backlight, I think it would make sense if they
> >> were.
> >
> > I guess it explains why it usually stops working after suspend.
> > Probably BIOS doesn't re-init the power sequencer on resume.
>
> The point is, the DPCD backlight isn't driven via the power sequencer,
> while the PWM pin would be.
>
> Please file a bug at [1], and attach /sys/kernel/debug/dri/0/i915_vbt as
> well as dmesg from boot with drm.debug=14 module parameter set.

Done, see https://gitlab.freedesktop.org/drm/intel/-/issues/4170

> Thanks,
> Jani.
>
>
> [1] https://gitlab.freedesktop.org/drm/intel/issues/new
>
>
>
> >
> >> BR,
> >> Jani.
> >>
> >> >
> >> >>
> >> >> BR,
> >> >> Jani.
> >> >>
> >> >>
> >> >> >
> >> >> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> >> >> > ---
> >> >> >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 4 ++++
> >> >> >  1 file changed, 4 insertions(+)
> >> >> >
> >> >> > 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 4f8337c7fd2e..c4f35e1b5870 100644
> >> >> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> >> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> >> >> > @@ -210,6 +210,10 @@ intel_dp_aux_hdr_enable_backlight(const struct
> >> >> > intel_crtc_state *crtc_state,
> >> >> >
> >> >> >         ctrl = old_ctrl;
> >> >> >         if (panel->backlight.edp.intel.sdr_uses_aux) {
> >> >> > +               /* Wait 100ms to ensure that panel is ready otherwise it
> >> >> > may not
> >> >> > +                * set chosen backlight level
> >> >> > +                */
> >> >> > +               msleep(100);
> >> >> >                 ctrl |= INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE;
> >> >> >                 intel_dp_aux_hdr_set_aux_backlight(conn_state, level);
> >> >> >         } else {
> >> >>
> >>
> >> --
> >> Jani Nikula, Intel Open Source Graphics Center
>
> --
> Jani Nikula, Intel Open Source Graphics Center