[v2,6/6] drm/i915: Do not do fb src adjustments for NV12
diff mbox

Message ID 1523621644-32363-7-git-send-email-vidya.srinivas@intel.com
State New
Headers show

Commit Message

vsrini4 April 13, 2018, 12:14 p.m. UTC
We skip src trunction/adjustments for
NV12 case and handle the sizes directly.
Without this, pipe fifo underruns are seen on APL/KBL.

v2: For NV12, making the src coordinates multiplier of 4

Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
 drivers/gpu/drm/i915/intel_sprite.c  | 15 +++++++++++++++
 2 files changed, 26 insertions(+)

Comments

Kahola, Mika April 17, 2018, 9:18 a.m. UTC | #1
On Fri, 2018-04-13 at 17:44 +0530, Vidya Srinivas wrote:
> We skip src trunction/adjustments for
> NV12 case and handle the sizes directly.
> Without this, pipe fifo underruns are seen on APL/KBL.
> 
> v2: For NV12, making the src coordinates multiplier of 4
> 
> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
>  drivers/gpu/drm/i915/intel_sprite.c  | 15 +++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index bc83f10..f64708f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12978,6 +12978,10 @@ intel_check_primary_plane(struct intel_plane
> *plane,
>  	bool can_position = false;
>  	int ret;
>  	uint32_t pixel_format = 0;
> +	struct drm_plane_state *plane_state = &state->base;
> +	struct drm_rect *src = &plane_state->src;
This seems unnecessary. We could use the line below to get the src's.

> +
> +	*src = drm_plane_state_src(plane_state);
>  
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		/* use scaler when colorkey is not required */
> @@ -13001,6 +13005,13 @@ intel_check_primary_plane(struct intel_plane
> *plane,
>  	if (!state->base.fb)
>  		return 0;
>  
> +	if (pixel_format == DRM_FORMAT_NV12) {
> +		src->x1 = (((src->x1 >> 16)/4)*4) << 16;
> +		src->x2 = (((src->x2 >> 16)/4)*4) << 16;
> +		src->y1 = (((src->y1 >> 16)/4)*4) << 16;
> +		src->y2 = (((src->y2 >> 16)/4)*4) << 16;
> +	}
> +
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		ret = skl_check_plane_surface(crtc_state, state);
>  		if (ret)
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 8b7947d..c1dd85e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1035,11 +1035,20 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
>  			return vscale;
>  		}
>  
> +		if (fb->format->format == DRM_FORMAT_NV12) {
> +			if (src->x2 >> 16 == 16)
> +				src->x2 = 16 << 16;
> +			if (src->y2 >> 16 == 16)
> +				src->y2 = 16 << 16;
> +			goto nv12_min_no_clip;
> +		}
> +
>  		/* Make the source viewport size an exact multiple
> of the scaling factors. */
>  		drm_rect_adjust_size(src,
>  				     drm_rect_width(dst) * hscale -
> drm_rect_width(src),
>  				     drm_rect_height(dst) * vscale -
> drm_rect_height(src));
>  
> +nv12_min_no_clip:
>  		drm_rect_rotate_inv(src, fb->width << 16, fb->height 
> << 16,
>  				    state->base.rotation);
>  
> @@ -1105,6 +1114,12 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
>  		src->x2 = (src_x + src_w) << 16;
>  		src->y1 = src_y << 16;
>  		src->y2 = (src_y + src_h) << 16;
> +		if (fb->format->format == DRM_FORMAT_NV12) {
> +			src->x1 = (((src->x1 >> 16)/4)*4) << 16;
> +			src->x2 = (((src->x2 >> 16)/4)*4) << 16;
> +			src->y1 = (((src->y1 >> 16)/4)*4) << 16;
> +			src->y2 = (((src->y2 >> 16)/4)*4) << 16;
> +		}
>  	}
>  
>  	dst->x1 = crtc_x;
vsrini4 April 17, 2018, 9:20 a.m. UTC | #2
> -----Original Message-----

> From: Kahola, Mika

> Sent: Tuesday, April 17, 2018 2:49 PM

> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-

> gfx@lists.freedesktop.org

> Subject: Re: [Intel-gfx] [PATCH v2 6/6] drm/i915: Do not do fb src

> adjustments for NV12

> 

> On Fri, 2018-04-13 at 17:44 +0530, Vidya Srinivas wrote:

> > We skip src trunction/adjustments for

> > NV12 case and handle the sizes directly.

> > Without this, pipe fifo underruns are seen on APL/KBL.

> >

> > v2: For NV12, making the src coordinates multiplier of 4

> >

> > Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>

> > ---

> >  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++

> >  drivers/gpu/drm/i915/intel_sprite.c  | 15 +++++++++++++++

> >  2 files changed, 26 insertions(+)

> >

> > diff --git a/drivers/gpu/drm/i915/intel_display.c

> > b/drivers/gpu/drm/i915/intel_display.c

> > index bc83f10..f64708f 100644

> > --- a/drivers/gpu/drm/i915/intel_display.c

> > +++ b/drivers/gpu/drm/i915/intel_display.c

> > @@ -12978,6 +12978,10 @@ intel_check_primary_plane(struct

> intel_plane

> > *plane,

> >  	bool can_position = false;

> >  	int ret;

> >  	uint32_t pixel_format = 0;

> > +	struct drm_plane_state *plane_state = &state->base;

> > +	struct drm_rect *src = &plane_state->src;

> This seems unnecessary. We could use the line below to get the src's.

Sure thank you, am moving all the handling code to skl_check_nv12_aux_surface as suggested
By Maarten. So this wont be necessary.

Regards
Vidya
> 

> > +

> > +	*src = drm_plane_state_src(plane_state);

> >

> >  	if (INTEL_GEN(dev_priv) >= 9) {

> >  		/* use scaler when colorkey is not required */ @@ -13001,6

> > +13005,13 @@ intel_check_primary_plane(struct intel_plane *plane,

> >  	if (!state->base.fb)

> >  		return 0;

> >

> > +	if (pixel_format == DRM_FORMAT_NV12) {

> > +		src->x1 = (((src->x1 >> 16)/4)*4) << 16;

> > +		src->x2 = (((src->x2 >> 16)/4)*4) << 16;

> > +		src->y1 = (((src->y1 >> 16)/4)*4) << 16;

> > +		src->y2 = (((src->y2 >> 16)/4)*4) << 16;

> > +	}

> > +

> >  	if (INTEL_GEN(dev_priv) >= 9) {

> >  		ret = skl_check_plane_surface(crtc_state, state);

> >  		if (ret)

> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c

> > b/drivers/gpu/drm/i915/intel_sprite.c

> > index 8b7947d..c1dd85e 100644

> > --- a/drivers/gpu/drm/i915/intel_sprite.c

> > +++ b/drivers/gpu/drm/i915/intel_sprite.c

> > @@ -1035,11 +1035,20 @@ intel_check_sprite_plane(struct intel_plane

> > *plane,

> >  			return vscale;

> >  		}

> >

> > +		if (fb->format->format == DRM_FORMAT_NV12) {

> > +			if (src->x2 >> 16 == 16)

> > +				src->x2 = 16 << 16;

> > +			if (src->y2 >> 16 == 16)

> > +				src->y2 = 16 << 16;

> > +			goto nv12_min_no_clip;

> > +		}

> > +

> >  		/* Make the source viewport size an exact multiple of the

> scaling

> > factors. */

> >  		drm_rect_adjust_size(src,

> >  				     drm_rect_width(dst) * hscale -

> drm_rect_width(src),

> >  				     drm_rect_height(dst) * vscale -

> drm_rect_height(src));

> >

> > +nv12_min_no_clip:

> >  		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,

> >  				    state->base.rotation);

> >

> > @@ -1105,6 +1114,12 @@ intel_check_sprite_plane(struct intel_plane

> > *plane,

> >  		src->x2 = (src_x + src_w) << 16;

> >  		src->y1 = src_y << 16;

> >  		src->y2 = (src_y + src_h) << 16;

> > +		if (fb->format->format == DRM_FORMAT_NV12) {

> > +			src->x1 = (((src->x1 >> 16)/4)*4) << 16;

> > +			src->x2 = (((src->x2 >> 16)/4)*4) << 16;

> > +			src->y1 = (((src->y1 >> 16)/4)*4) << 16;

> > +			src->y2 = (((src->y2 >> 16)/4)*4) << 16;

> > +		}

> >  	}

> >

> >  	dst->x1 = crtc_x;

> --

> Mika Kahola - Intel OTC

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bc83f10..f64708f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12978,6 +12978,10 @@  intel_check_primary_plane(struct intel_plane *plane,
 	bool can_position = false;
 	int ret;
 	uint32_t pixel_format = 0;
+	struct drm_plane_state *plane_state = &state->base;
+	struct drm_rect *src = &plane_state->src;
+
+	*src = drm_plane_state_src(plane_state);
 
 	if (INTEL_GEN(dev_priv) >= 9) {
 		/* use scaler when colorkey is not required */
@@ -13001,6 +13005,13 @@  intel_check_primary_plane(struct intel_plane *plane,
 	if (!state->base.fb)
 		return 0;
 
+	if (pixel_format == DRM_FORMAT_NV12) {
+		src->x1 = (((src->x1 >> 16)/4)*4) << 16;
+		src->x2 = (((src->x2 >> 16)/4)*4) << 16;
+		src->y1 = (((src->y1 >> 16)/4)*4) << 16;
+		src->y2 = (((src->y2 >> 16)/4)*4) << 16;
+	}
+
 	if (INTEL_GEN(dev_priv) >= 9) {
 		ret = skl_check_plane_surface(crtc_state, state);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 8b7947d..c1dd85e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1035,11 +1035,20 @@  intel_check_sprite_plane(struct intel_plane *plane,
 			return vscale;
 		}
 
+		if (fb->format->format == DRM_FORMAT_NV12) {
+			if (src->x2 >> 16 == 16)
+				src->x2 = 16 << 16;
+			if (src->y2 >> 16 == 16)
+				src->y2 = 16 << 16;
+			goto nv12_min_no_clip;
+		}
+
 		/* Make the source viewport size an exact multiple of the scaling factors. */
 		drm_rect_adjust_size(src,
 				     drm_rect_width(dst) * hscale - drm_rect_width(src),
 				     drm_rect_height(dst) * vscale - drm_rect_height(src));
 
+nv12_min_no_clip:
 		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
 				    state->base.rotation);
 
@@ -1105,6 +1114,12 @@  intel_check_sprite_plane(struct intel_plane *plane,
 		src->x2 = (src_x + src_w) << 16;
 		src->y1 = src_y << 16;
 		src->y2 = (src_y + src_h) << 16;
+		if (fb->format->format == DRM_FORMAT_NV12) {
+			src->x1 = (((src->x1 >> 16)/4)*4) << 16;
+			src->x2 = (((src->x2 >> 16)/4)*4) << 16;
+			src->y1 = (((src->y1 >> 16)/4)*4) << 16;
+			src->y2 = (((src->y2 >> 16)/4)*4) << 16;
+		}
 	}
 
 	dst->x1 = crtc_x;