> -----Original Message-----
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Sent: Friday, February 9, 2024 2:19 PM
> To: Kahola, Mika <mika.kah...@intel.com>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>; intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/i915/display: Force full modeset for eDP
>
> On Fri, Feb 09, 2024 at 12:13:02PM +0000, Kahola, Mika wrote:
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > Sent: Friday, February 9, 2024 2:07 PM
> > > To: Kahola, Mika <mika.kah...@intel.com>
> > > Cc: Jani Nikula <jani.nik...@linux.intel.com>;
> > > intel-gfx@lists.freedesktop.org
> > > Subject: Re: [PATCH 2/2] drm/i915/display: Force full modeset for
> > > eDP
> > >
> > > On Fri, Feb 09, 2024 at 11:55:58AM +0000, Kahola, Mika wrote:
> > > > > -----Original Message-----
> > > > > From: Jani Nikula <jani.nik...@linux.intel.com>
> > > > > Sent: Friday, February 9, 2024 11:06 AM
> > > > > To: Kahola, Mika <mika.kah...@intel.com>;
> > > > > intel-gfx@lists.freedesktop.org
> > > > > Cc: Kahola, Mika <mika.kah...@intel.com>
> > > > > Subject: Re: [PATCH 2/2] drm/i915/display: Force full modeset
> > > > > for eDP
> > > > >
> > > > > On Tue, 06 Feb 2024, Mika Kahola <mika.kah...@intel.com> wrote:
> > > > > > Force full modeset for eDP when booting up. GOP programs PLL
> > > > > > parameters and hence, we would be able to use fastset for eDP.
> > > > > > However, with fastset we are not setting PLL values from the
> > > > > > driver and rely that GOP and driver PLL values match.
> > > > > > We have discovered that with some of the panels this is not
> > > > > > true and hence we would need to program PLL values by the
> > > > > > driver. The patch suggests a workaround as enabling full
> > > > > > modeset when booting up. This way we force the driver to write the
> > > > > > PLL values to the hw.
> > > > >
> > > > > No. We want to avoid full modesets if possible, both in general and
> > > > > at probe.
> > > > >
> > > > > And when we do end up with modesets, the decision needs to be
> > > > > based on changes in the state that we can't write to the hardware
> > > > > without a modeset.
> > > > >
> > > > > We can't unconditionally force a modeset on eDP panels at probe.
> > > >
> > > > Thanks! Just wondering what the options here might be? With
> > > > fastest we end up having a mismatch with one PLL value with a
> > > few panels.
> > >
> > > You seem to be stuck in some infinite loop. If your PLL parameters
> > > are mismatching that should prevent the fastset, but then I guess
> > > you added some hack to allow the fastset despite the mismatch, and now
> > > you're trying to undo that hack by blindly forcing a
> full modeset?
> >
> > That's right, I found myself to be between a rock and a hard place. I did
> > discard the fastest but found out that we cannot do that.
>
> If you discarded it then why are you not already getting the full modeset you
> want?
>
Poor choice of words, I guess. What I meant that I discarded the state
verification in case of fastest. This way the mismatch is hidden under the
carpet.
> > Here, another hack is introduced to force the full modeset to ensure that
> > the driver programs these PLL values. As Jani already
> mentioned, this is a no go option as well.
> >
> > >
> > > >
> > > > Should we try identify the panels and setup some sort of quirks for
> > > > these problematic panels or what would be the best
> solution?
> > > >
> > > > -Mika-
> > > >
> > > > >
> > > > >
> > > > > BR,
> > > > > Jani.
> > > > >
> > > > > >
> > > > > > Signed-off-by: Mika Kahola <mika.kah...@intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 13 +++++++++++++
> > > > > > 1 file changed, 13 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > index ab415f41924d..9699ded1eb5f 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > @@ -3319,6 +3319,7 @@ bool intel_dp_initial_fastset_check(struct
> > > > > > intel_encoder *encoder,
> > > > > > * of crtc_state->dsc, we have no way to ensure reliable
> > > > > > fastset.
> > > > > > * Remove once we have readout for DSC.
> > > > > > */
> > > > > > +
> > > > > > if (crtc_state->dsc.compression_enable) {
> > > > > > drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s] Forcing full
> > > > > > modeset due to DSC being enabled\n",
> > > > > > encoder->base.base.id, encoder->base.name);
> > > > > > @@ -3326,6
> > > > > > +3327,18 @@ bool intel_dp_initial_fastset_check(struct
> > > > > > +intel_encoder *encoder,
> > > > > > fastset = false;
> > > > > > }
> > > > > >
> > > > > > + /*
> > > > > > + * FIXME hack to force full modeset for eDP as not always BIOS
> > > > > > written PLL
> > > > > > + * values does not match with the ones defined in the driver
> > > > > > code
> > > > > > + */
> > > > > > + if (!crtc_state->uapi.mode_changed &&
> > > > > > + intel_dp_is_edp(intel_dp)) {
> > > > > > + drm_dbg_kms(&i915->drm, "Forcing full modeset for
> > > > > > eDP\n");
> > > > > > + crtc_state->uapi.mode_changed = true;
> > > > > > + fastset = false;
> > > > > > + }
> > > > > > +
> > > > > > +
> > > > > > return fastset;
> > > > > > }
> > > > >
> > > > > --
> > > > > Jani Nikula, Intel
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
>
> --
> Ville Syrjälä
> Intel
