Re: [RFC 1/3] drm/i915: remove !enabled handling from commit primary plane step

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 09, 2014 at 11:43:19AM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> The !crtc->enabled case will now be handled by the !visible code,
> since the handling is basically the same.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 --------------------------
>  1 file changed, 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5279b99..2ccf7c0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11837,32 +11837,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
>  	struct drm_rect *src = &state->src;
>  	int ret;
>  
> -	/*
> -	 * If the CRTC isn't enabled, we're just pinning the framebuffer,
> -	 * updating the fb pointer, and returning without touching the
> -	 * hardware.  This allows us to later do a drmModeSetCrtc with fb=-1 to
> -	 * turn on the display with all planes setup as desired.
> -	 */
> -	if (!crtc->enabled) {
> -		mutex_lock(&dev->struct_mutex);
> -
> -		/*
> -		 * If we already called setplane while the crtc was disabled,
> -		 * we may have an fb pinned; unpin it.
> -		 */
> -		if (plane->fb)
> -			intel_unpin_fb_obj(old_obj);
> -
> -		i915_gem_track_fb(old_obj, obj,
> -				  INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
> -
> -		/* Pin and return without programming hardware */
> -		ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
> -		mutex_unlock(&dev->struct_mutex);
> -
> -		return ret;
> -	}
> -
>  	intel_crtc_wait_for_pending_flips(crtc);

Yeah this should work just fine.

One difference between the code paths is the intel_crtc_wait_for_pending_flips()
call, but since the crtc isn't enabled there can't be any pending flip.

The other difference is the pin vs. unpin order, but that shouldn't matter
unless there's tons of other stuff pinned as well. It's not worth optimizing
setplane calls on disabled CRTCs too much IMO.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	/*
> -- 
> 1.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC