Re: [PATCH 1/2] drm/i915: Updating plane parameters for primary plane in setplane

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Aug 21, 2014 at 11:44:39AM +0530, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@intel.com>
> 
> v2: Moving setting of plane members in the end to take care of failure cases and
> not-visible cases (Matt).
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

Queued for -next, thanks for the patch.

But please when submitting anything non-trival (i.e. "fix typo" or similar
simple stuff) a proper commit message to explain the motivation for a
patch is required. I've added a small blurb for this one here, but
guessing at why a patch is necessary is much harder than just reading the
explanation. Please double-check the commit patch to make sure I've
actually understood it correctly.

Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c |   38 +++++++++++++++++++++++++++-------
>  1 file changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0b327eb..f2a8797 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11594,6 +11594,21 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
>  		.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
>  	};
> +	const struct {
> +		int crtc_x, crtc_y;
> +		unsigned int crtc_w, crtc_h;
> +		uint32_t src_x, src_y, src_w, src_h;
> +	} orig = {
> +		.crtc_x = crtc_x,
> +		.crtc_y = crtc_y,
> +		.crtc_w = crtc_w,
> +		.crtc_h = crtc_h,
> +		.src_x = src_x,
> +		.src_y = src_y,
> +		.src_w = src_w,
> +		.src_h = src_h,
> +	};
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	bool visible;
>  	int ret;
>  
> @@ -11668,15 +11683,24 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  		mutex_unlock(&dev->struct_mutex);
>  
> -		return 0;
> -	}
> +	} else {
> +		ret = intel_pipe_set_base(crtc, src.x1, src.y1, fb);
> +		if (ret)
> +			return ret;
>  
> -	ret = intel_pipe_set_base(crtc, src.x1, src.y1, fb);
> -	if (ret)
> -		return ret;
> +		if (!intel_crtc->primary_enabled)
> +			intel_enable_primary_hw_plane(plane, crtc);
> +	}
>  
> -	if (!intel_crtc->primary_enabled)
> -		intel_enable_primary_hw_plane(plane, crtc);
> +	intel_plane->crtc_x = orig.crtc_x;
> +	intel_plane->crtc_y = orig.crtc_y;
> +	intel_plane->crtc_w = orig.crtc_w;
> +	intel_plane->crtc_h = orig.crtc_h;
> +	intel_plane->src_x = orig.src_x;
> +	intel_plane->src_y = orig.src_y;
> +	intel_plane->src_w = orig.src_w;
> +	intel_plane->src_h = orig.src_h;
> +	intel_plane->obj = obj;
>  
>  	return 0;
>  }
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@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