[Intel-gfx] [RFC PATCH 5/5] drm/i915/display/tgl+: Use PPS index from vbt

[Manna, Animesh]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula at intel.com>
> Sent: Thursday, June 2, 2022 9:03 PM
> To: Manna, Animesh <animesh.manna at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: ville.syrjala at linux.intel.com; Shankar, Uma <uma.shankar at intel.com>;
> Varide, Nischal <nischal.varide at intel.com>; Manna, Animesh
> <animesh.manna at intel.com>
> Subject: Re: [RFC PATCH 5/5] drm/i915/display/tgl+: Use PPS index from vbt
> 
> On Thu, 02 Jun 2022, Animesh Manna <animesh.manna at intel.com> wrote:
> > From: Nischal Varide <nischal.varide at intel.com>
> >
> > Tigerlake and newer has two instances of PPS, to support up to two eDP
> > panels.
> >
> > Signed-off-by: Nischal Varide <nischal.varide at intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_pps.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c
> > b/drivers/gpu/drm/i915/display/intel_pps.c
> > index 1b21a341962f..52cb5be4e901 100644
> > --- a/drivers/gpu/drm/i915/display/intel_pps.c
> > +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> > @@ -365,7 +365,8 @@ static void intel_pps_get_registers(struct
> > intel_dp *intel_dp,
> >
> >  	memset(regs, 0, sizeof(*regs));
> >
> > -	if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
> > +	if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> > +	    DISPLAY_VER(dev_priv) >= 12)
> >  		pps_idx = bxt_power_sequencer_idx(intel_dp);
> 
> There are two things that need to be checked, but I don't have the time right
> now:
> 
> - We'll probably need this *before* we've parsed the panel specific info
>   from VBT. Ville has looked into this somewhat with the PNPID panel
>   type stuff.

Currently intel_pps_init() get called before intel_bios_init_panel() where panel specific info in parsed from VBT.

> 
> - bxt_power_sequencer_idx() does pps_init_registers() which has always
>   struck me as a really odd place to do it. As if we don't know when the
>   first time we do it is, so we do it there just in case.

Will try to check on this.

Regards,
Animesh 
> 
> BR,
> Jani.
> 
> 
> 
> >  	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> >  		pps_idx = vlv_power_sequencer_pipe(intel_dp);
> 
> --
> Jani Nikula, Intel Open Source Graphics Center