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

[Dhinakaran Pandiyan]

On Fri, 2018-10-05 at 12:53 -0700, Souza, Jose wrote:
> On Fri, 2018-10-05 at 10:51 -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)."
> > 
> > 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 at 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.
> 
> When I added this bit I first named it "EDP_Y_COORDINATE_INVALID" but
> was decided to go back to BSpec name.

I remember it now, something like EDP_Y_COORDINATE_VALID_DISABLE would
be more appropriate. 

-DK