Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix 90/270 rotated coordinates for FBC

2017-05-19 Thread Tvrtko Ursulin


On 31/03/2017 19:00, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä 

The clipped src coordinates have already been rotated by 270 degrees for
when the plane rotation is 90/270 degrees, hence the FBC code should no
longer swap the width and height.

Cc: sta...@vger.kernel.org
Cc: Tvrtko Ursulin 
Cc: Paulo Zanoni 
Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check 
hook for SKL+")
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_fbc.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index ded2add18b26..d93c58410bff 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -82,20 +82,10 @@ static unsigned int get_crtc_fence_y_offset(struct 
intel_crtc *crtc)
 static void intel_fbc_get_plane_source_size(struct intel_fbc_state_cache 
*cache,
int *width, int *height)
 {
-   int w, h;
-
-   if (drm_rotation_90_or_270(cache->plane.rotation)) {
-   w = cache->plane.src_h;
-   h = cache->plane.src_w;
-   } else {
-   w = cache->plane.src_w;
-   h = cache->plane.src_h;
-   }
-
if (width)
-   *width = w;
+   *width = cache->plane.src_w;
if (height)
-   *height = h;
+   *height = cache->plane.src_h;
 }

 static int intel_fbc_calculate_cfb_size(struct drm_i915_private *dev_priv,
@@ -746,6 +736,11 @@ static void intel_fbc_update_state_cache(struct intel_crtc 
*crtc,
cache->crtc.hsw_bdw_pixel_rate = crtc_state->pixel_rate;

cache->plane.rotation = plane_state->base.rotation;
+   /*
+* Src coordinates are already rotated by 270 degrees for
+* the 90/270 degree plane rotation cases (to match the
+* GTT mapping), hence no need to account for rotation here.
+*/
cache->plane.src_w = drm_rect_width(&plane_state->base.src) >> 16;
cache->plane.src_h = drm_rect_height(&plane_state->base.src) >> 16;
cache->plane.visible = plane_state->base.visible;



For the series:

Tested-by: Tvrtko Ursulin 

Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix 90/270 rotated coordinates for FBC

2017-04-03 Thread Paulo Zanoni
Em Sex, 2017-03-31 às 21:00 +0300, ville.syrj...@linux.intel.com
escreveu:
> From: Ville Syrjälä 
> 
> The clipped src coordinates have already been rotated by 270 degrees
> for
> when the plane rotation is 90/270 degrees, hence the FBC code should
> no
> longer swap the width and height.

I've never payed too much attention to rotation, but based on the
mentioned commits, what's said on the messages and my understanding of
the code, this looks sane, so:

Reviewed-by: Paulo Zanoni 

And in case someone suggests to just kill
intel_fbc_get_plane_source_size(), I'd like to point that "plane source
size" is wording used by our spec and there's a nice comment explaining
what exactly it's supposed to be, so I'd be in favor of keeping it.

Super bonus point if you end up writing some sort of rotation test for
kms_frontbuffer_tracking or kms_fbc_crc. The problem is that I'm not
entirely too sure about how much the current code structure for those
tests is ready to easily support such a test with minimal efforts.
Needs to be studied.

> 
> Cc: sta...@vger.kernel.org
> Cc: Tvrtko Ursulin 
> Cc: Paulo Zanoni 
> Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the
> plane check hook for SKL+")
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index ded2add18b26..d93c58410bff 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -82,20 +82,10 @@ static unsigned int
> get_crtc_fence_y_offset(struct intel_crtc *crtc)
>  static void intel_fbc_get_plane_source_size(struct
> intel_fbc_state_cache *cache,
>   int *width, int *height)
>  {
> - int w, h;
> -
> - if (drm_rotation_90_or_270(cache->plane.rotation)) {
> - w = cache->plane.src_h;
> - h = cache->plane.src_w;
> - } else {
> - w = cache->plane.src_w;
> - h = cache->plane.src_h;
> - }
> -
>   if (width)
> - *width = w;
> + *width = cache->plane.src_w;
>   if (height)
> - *height = h;
> + *height = cache->plane.src_h;
>  }
>  
>  static int intel_fbc_calculate_cfb_size(struct drm_i915_private
> *dev_priv,
> @@ -746,6 +736,11 @@ static void intel_fbc_update_state_cache(struct
> intel_crtc *crtc,
>   cache->crtc.hsw_bdw_pixel_rate = crtc_state-
> >pixel_rate;
>  
>   cache->plane.rotation = plane_state->base.rotation;
> + /*
> +  * Src coordinates are already rotated by 270 degrees for
> +  * the 90/270 degree plane rotation cases (to match the
> +  * GTT mapping), hence no need to account for rotation here.
> +  */
>   cache->plane.src_w = drm_rect_width(&plane_state->base.src)
> >> 16;
>   cache->plane.src_h = drm_rect_height(&plane_state->base.src) 
> >> 16;
>   cache->plane.visible = plane_state->base.visible;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 3/3] drm/i915: Fix 90/270 rotated coordinates for FBC

2017-03-31 Thread ville . syrjala
From: Ville Syrjälä 

The clipped src coordinates have already been rotated by 270 degrees for
when the plane rotation is 90/270 degrees, hence the FBC code should no
longer swap the width and height.

Cc: sta...@vger.kernel.org
Cc: Tvrtko Ursulin 
Cc: Paulo Zanoni 
Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane 
check hook for SKL+")
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_fbc.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index ded2add18b26..d93c58410bff 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -82,20 +82,10 @@ static unsigned int get_crtc_fence_y_offset(struct 
intel_crtc *crtc)
 static void intel_fbc_get_plane_source_size(struct intel_fbc_state_cache 
*cache,
int *width, int *height)
 {
-   int w, h;
-
-   if (drm_rotation_90_or_270(cache->plane.rotation)) {
-   w = cache->plane.src_h;
-   h = cache->plane.src_w;
-   } else {
-   w = cache->plane.src_w;
-   h = cache->plane.src_h;
-   }
-
if (width)
-   *width = w;
+   *width = cache->plane.src_w;
if (height)
-   *height = h;
+   *height = cache->plane.src_h;
 }
 
 static int intel_fbc_calculate_cfb_size(struct drm_i915_private *dev_priv,
@@ -746,6 +736,11 @@ static void intel_fbc_update_state_cache(struct intel_crtc 
*crtc,
cache->crtc.hsw_bdw_pixel_rate = crtc_state->pixel_rate;
 
cache->plane.rotation = plane_state->base.rotation;
+   /*
+* Src coordinates are already rotated by 270 degrees for
+* the 90/270 degree plane rotation cases (to match the
+* GTT mapping), hence no need to account for rotation here.
+*/
cache->plane.src_w = drm_rect_width(&plane_state->base.src) >> 16;
cache->plane.src_h = drm_rect_height(&plane_state->base.src) >> 16;
cache->plane.visible = plane_state->base.visible;
-- 
2.10.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx