[Intel-gfx] [PATCH 11/11] drm/i915: Hook PSR functionality

[Daniel Vetter]

On Thu, Jul 11, 2013 at 06:45:05PM -0300, Rodrigo Vivi wrote:
> PSR must be enabled after transcoder and port are running.
> And it is only available for HSW.
> 
> v2: move enable/disable to intel_ddi
> v3: The spec suggests PSR should be disabled even before backlight (by pzanoni)
> v4: also disabling and enabling whenever panel is disabled/enabled.
> v5: make it last patch to avoid breaking whenever bisecting. So calling for
>     update and force exit came to this patch along with enable/disable calls.
> v6: Remove unused and unecessary psr_enable/disable calls, as notice by Paulo.
> 
> CC: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>

Ok, I've slurped this series in with the few bikesheds from Paulo applied
and two patches not merged:
- The userspace interface part, since I don't want to commit yet to an
  interface as long as things are unclear.
- And the invalidation part since imo that parts isn't properly thought
  through yet. See my other mail to Chris' RFC for ideas.

Now on the topic fbc, psr and frontbuffer rendering:

Now that I've appeased our dear PDT it's time to stop building castles on
sand imo. Items to pay back a bit of the technical dept:

- Untangle the "is fbc/psr" allowed mess. Imo we should add more static
  (i.e. only changes at modesets) information to the pipe config and track
  dynamic reasons for disabling fbc/psr somewhere in the crtc. Also this
  way update_fbc/psr would only need to check dynamic state and more
  important would never need to frob the basic setup (like reallocating
  the compressed buffer and similar things).

- Implement precise frontbuffer rendering tracking and disabling of
  fbc/psr for legacy userspace and rip out the hw tracking. If we have to
  add tons of workarounds (like for fbc), have performance this or just
  can't use it in a bunch of cases (sprites for psr) we might as well just
  track everything in the kernel.

- Testcases. With pipe CRC checksums we should be able to make sure that
  the frontbuffer invalidation actually happens. And if we do it all in
  the kernel the difference should be all-or-nothing, so much easier to
  test than with hw tracking where sometimes something seems to slip
  through.

- low-level improvements. I've only really reviewed the fbc code in-depth
  and discussed a few things with Art, but there's definitely a few things
  we need to do. One of the important things imo is to enable fbc
  everywhere we can to have as wide test coverage as possible for this
  feature.

I'll block future hw enabling in this area on the above list (i.e. vlv psr
or patches for -internal). I'll reconsider my stance if e.g. vlv psr is
used as a demonstration vehicle for the refactored psr tracking. But since
fbc can be used on many more machines (even desktops and apparently even
when other pipes are enabled) I think we should push that forward first
and use fbc to create solid tests and interfaces for userspace&kernel to
cooperate on frontbuffer rendering.

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c      | 2 ++
>  drivers/gpu/drm/i915/intel_ddi.c     | 2 ++
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  3 files changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 46bf7e3..703bc69 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3758,6 +3758,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>  		goto unlock;
>  	}
>  
> +	intel_edp_psr_force_exit(dev);
> +
>  	/* Count all active objects as busy, even if they are currently not used
>  	 * by the gpu. Users of this interface expect objects to eventually
>  	 * become non-busy without any further actions, therefore emit any
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 324211a..4211925 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1117,6 +1117,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
>  			intel_dp_stop_link_train(intel_dp);
>  
>  		ironlake_edp_backlight_on(intel_dp);
> +		intel_edp_psr_enable(intel_dp);
>  	}
>  
>  	if (intel_crtc->eld_vld && type != INTEL_OUTPUT_EDP) {
> @@ -1147,6 +1148,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
>  	if (type == INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  
> +		intel_edp_psr_disable(intel_dp);
>  		ironlake_edp_backlight_off(intel_dp);
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 10a3629..eb4e49b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2272,6 +2272,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>  	}
>  
>  	intel_update_fbc(dev);
> +	intel_edp_psr_update(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	intel_crtc_update_sarea_pos(crtc, x, y);
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch