Re: [PATCH v4 2/4] drm/i915: New drm crtc property for varying the Pipe Src size

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Apr 29, 2014 at 05:06:59PM +0300, Ville Syrjälä wrote:
> On Sun, Apr 20, 2014 at 04:14:18PM +0530, akash.goel@intel.com wrote:
> > From: Akash Goel <akash.goel@intel.com>
> > 
> > This patch adds a new drm crtc property for varying the Pipe Src size
> > or the Panel fitter input size. Pipe Src controls the size that is
> > scaled from.
> > This will allow to dynamically flip (without modeset) the frame buffers
> > of different resolutions
> > 
> > v2: Added a check to fail the set property call if Panel fitter is
> > disabled & new PIPESRC programming do not match with PIPE timings.
> > Removed the pitch mismatch check on frame buffer, when being flipped.
> > This is currently done only for VLV/HSW.
> > 
> > v3: Modified the check, added in v2, to consider the platforms having
> > Split PCH.
> > 
> > v4: Refactored based on latest codebase.
> > Used 'UINT_MAX' macro in place of constant.
> > 
> > Testcase: igt/kms_panel_fitter_test
> > 
> > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  6 +++
> >  drivers/gpu/drm/i915/intel_display.c | 76 +++++++++++++++++++++++++++++++++++-
> >  2 files changed, 80 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 7d6acb4..104a232 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1441,6 +1441,12 @@ struct drm_i915_private {
> >  	struct drm_property *broadcast_rgb_property;
> >  	struct drm_property *force_audio_property;
> >  
> > +	/*
> > +	 * Property to dynamically vary the size of the
> > +	 * PIPESRC or Panel fitter input size
> > +	 */
> > +	struct drm_property *input_size_property;
> > +
> >  	uint32_t hw_context_size;
> >  	struct list_head context_list;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f26be4e..5484ae2 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8902,8 +8902,18 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> >  	 * Note that pitch changes could also affect these register.
> >  	 */
> >  	if (INTEL_INFO(dev)->gen > 3 &&
> > -	    (fb->offsets[0] != crtc->primary->fb->offsets[0] ||
> > -	     fb->pitches[0] != crtc->primary->fb->pitches[0]))
> > +	    (fb->offsets[0] != crtc->primary->fb->offsets[0]))
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * Bypassing the fb pitch check for VLV/HSW, as purportedly there
> > +	 * is a dynamic flip support in VLV/HSW. This will allow to
> > +	 * flip fbs of different resolutions without doing a modeset.
> > +	 * TBD, confirm the same for other newer gen platforms also.
> > +	 */
> > +	if (INTEL_INFO(dev)->gen > 3 &&
> > +	    !IS_VALLEYVIEW(dev) && !IS_HASWELL(dev) &&
> > +	    (fb->pitches[0] != crtc->primary->fb->pitches[0]))
> 
> NAK. Please read the comment above the check to understand why we can't
> just change the stride here. I guess on HSW+ it might be possible since
> it uses the x/y offsets even with linear FBs so stride changes don't
> affect the offset. Also such changes sould be in separate patches
> anyway.
> 
> I guess one option would be update the linear offset with LRI, but
> since the offset register gets latched independently of the DSPSURF
> write the changes aren't guaranteed to happen atomically. Also on
> VLV LRI to display registers is apparently busted.
> 
> If you do find a way to make this work on some platforms, then we
> need to have a test for it to make sure it does the right thing.

Note that even on hsw we update the linear offset if it's out of range for
the x/y tiled offsets. Which can happen with very wide/high virtual
desktops.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch