Re: [PATCH 2/2] drm/i915/psr: Set Y coordinate valid for Gen10+ display

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Oct 05, 2018 at 10:51:28AM -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-10-05 at 10:38 -0700, Rodrigo Vivi wrote:
> > On Thu, Oct 04, 2018 at 08:01:30PM -0700, Dhinakaran Pandiyan wrote:
> > > PSR2 sinks that require Y coordinates for selective update also
> > > need the
> > > Y coordinate Valid bit in VSC SDP.
> > > Spec: eDP 1.4b VSC payload extension for PSR2 operation (Table 6-
> > > 12)
> > 
> > I couldn't get any meaningful information about Y coordinate valid
> > bit
> > looking at this table...
> > 
> > what am I missing?
> "
> Y-Coordinate_Valid
> If the Sink device indicates that Y-coordinate is required, the Source
> device must program HB2 (VSC Revision Number) to 04h or 05h.
> Additionally, the Source device must set this bit to 1 to indicate that
> the Y-coordinate provided in DB12 through DB13 is valid. However, if
> the Y-coordinate provided in DB12 through DB13 is not valid, this bit
> must be cleared to 0 (this includes when DB12 through DB13 are not set
> and remain at a default value of 0000h, because a Y-coordinate value of
> 0000h represents the first active video line)."

Interesting... I keep forgetting I need to delete my old spec and download
a new one from VESA website...

On the one that I have here I couldn't find this text and also HB2 doesn't
have these options:

4:0 Revision Number
01h = VSC packet supports only 3D stereo.
02h = 3D stereo + PSR1.
03h = 3D stereo + PSR2.
7:5 RESERVED

> 
> We enable PSR2 only when the sink says Y-coordinate is required and the
> hardware needs to be told to send Y coordinate valid bit in the VSC
> packets for such sinks.
> 
> 
> > 
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 105b7ea2cd98..92672954dfef 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -431,7 +431,7 @@ static void hsw_activate_psr2(struct intel_dp
> > > *intel_dp)
> > >  	 * good enough. */
> > >  	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> > >  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > > -		val |= EDP_Y_COORDINATE_ENABLE;
> > > +		val |= EDP_Y_COORDINATE_ENABLE |
> > > EDP_Y_COORDINATE_VALID;
> > 
> > But also, this seems to be doing the opposite what you wrote on the
> > commit message since this bit means:
> > 
> > "Do not include Y-coordinate valid eDP 1.4" (Bspec: 7713)
> 
> Oops! You are right, the bit name threw me off.

yeap... the name is so bad indeed.

> 
> 
> > >  
> > >  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
> > >  	    dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx