| Message ID | 20220426193222.3422-11-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/bios: Rework BDB block handling and PNPID->panel_type matching | expand |
On Tue, 26 Apr 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > During the eDP probe we may not yet know the panel_type used > to index the VBT panel tables. So the initial eDP probe will have > to be done without that, and thus we won't yet have the PPS delays > from the VBT. Once the VBT has been fully parse we should reinit > the PPS delays to make sure it's fully accounted for. > > TODO: I wonder if we should do the eDP probe with some super safe > PPS delayes (eg. max of all VBT PPS delays) just to make sure we > don't violate the timings. Though typically the VBIOS/GOP do leave > VDD enabled after boot in which case we don't actually have to care > about the delays at all. The trouble here is that the first init writes the registers, and the second init reads them back as "bios set values", and never goes lower than them. The late init can only increase the values based on VBT data. So I'm holding off on the r-b on both the PPS changes for now because I think this will need some kind of a redesign. Maybe we'll need to keep track of the origins of the values at each step, and choose the right ones at late init and skip the register writes if there are no changes. I don't know. BR, Jani. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_pps.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c > index 0ae2be5c5318..15cbdc465a86 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.c > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > @@ -1396,6 +1396,11 @@ void intel_pps_init_late(struct intel_dp *intel_dp) > intel_wakeref_t wakeref; > > with_intel_pps_lock(intel_dp, wakeref) { > + /* Reinit delays after per-panel info has been parsed from VBT */ > + memset(&intel_dp->pps.pps_delays, 0, sizeof(intel_dp->pps.pps_delays)); > + pps_init_delays(intel_dp); > + pps_init_registers(intel_dp, false); > + > if (edp_have_panel_vdd(intel_dp)) > edp_panel_vdd_schedule_off(intel_dp); > }
On Tue, May 03, 2022 at 02:46:35PM +0300, Jani Nikula wrote: > On Tue, 26 Apr 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > During the eDP probe we may not yet know the panel_type used > > to index the VBT panel tables. So the initial eDP probe will have > > to be done without that, and thus we won't yet have the PPS delays > > from the VBT. Once the VBT has been fully parse we should reinit > > the PPS delays to make sure it's fully accounted for. > > > > TODO: I wonder if we should do the eDP probe with some super safe > > PPS delayes (eg. max of all VBT PPS delays) just to make sure we > > don't violate the timings. Though typically the VBIOS/GOP do leave > > VDD enabled after boot in which case we don't actually have to care > > about the delays at all. > > The trouble here is that the first init writes the registers, and the > second init reads them back as "bios set values", and never goes lower > than them. The late init can only increase the values based on VBT data. Hmm. I thought we just took the max of hw,VBT,spec, but looks like the spec stuff has some other magic going on :( > > So I'm holding off on the r-b on both the PPS changes for now because I > think this will need some kind of a redesign. Maybe we'll need to keep > track of the origins of the values at each step, and choose the right > ones at late init and skip the register writes if there are no > changes. I don't know. > > BR, > Jani. > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_pps.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c > > index 0ae2be5c5318..15cbdc465a86 100644 > > --- a/drivers/gpu/drm/i915/display/intel_pps.c > > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > > @@ -1396,6 +1396,11 @@ void intel_pps_init_late(struct intel_dp *intel_dp) > > intel_wakeref_t wakeref; > > > > with_intel_pps_lock(intel_dp, wakeref) { > > + /* Reinit delays after per-panel info has been parsed from VBT */ > > + memset(&intel_dp->pps.pps_delays, 0, sizeof(intel_dp->pps.pps_delays)); > > + pps_init_delays(intel_dp); > > + pps_init_registers(intel_dp, false); > > + > > if (edp_have_panel_vdd(intel_dp)) > > edp_panel_vdd_schedule_off(intel_dp); > > } > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c index 0ae2be5c5318..15cbdc465a86 100644 --- a/drivers/gpu/drm/i915/display/intel_pps.c +++ b/drivers/gpu/drm/i915/display/intel_pps.c @@ -1396,6 +1396,11 @@ void intel_pps_init_late(struct intel_dp *intel_dp) intel_wakeref_t wakeref; with_intel_pps_lock(intel_dp, wakeref) { + /* Reinit delays after per-panel info has been parsed from VBT */ + memset(&intel_dp->pps.pps_delays, 0, sizeof(intel_dp->pps.pps_delays)); + pps_init_delays(intel_dp); + pps_init_registers(intel_dp, false); + if (edp_have_panel_vdd(intel_dp)) edp_panel_vdd_schedule_off(intel_dp); }