drm/exynos: mixer: avoid Oops in vp_video_buffer()
diff mbox

Message ID 20171123141440.29911-1-tjakobi@math.uni-bielefeld.de
State New
Headers show

Commit Message

Tobias Jakobi Nov. 23, 2017, 2:14 p.m. UTC
If an interlaced video mode is selected, a IOMMU pagefault is
triggered by vp_video_buffer().

Fix the most apparent bugs:
- pitch value for chroma plane
- divide by two of height and vpos

This is more like a band-aid at this point. The VP plane is
still corrupted, but at least there are no more pagefaults.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Comments

Tobias Jakobi Nov. 23, 2017, 2:16 p.m. UTC | #1
Please disregard this one, need to rebase. v2 in a minute...

- Tobias


Tobias Jakobi wrote:
> If an interlaced video mode is selected, a IOMMU pagefault is
> triggered by vp_video_buffer().
> 
> Fix the most apparent bugs:
> - pitch value for chroma plane
> - divide by two of height and vpos
> 
> This is more like a band-aid at this point. The VP plane is
> still corrupted, but at least there are no more pagefaults.
> 
> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index dc5d79465f9b..c382f99e0f5b 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -485,7 +485,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  			chroma_addr[1] = chroma_addr[0] + 0x40;
>  		} else {
>  			luma_addr[1] = luma_addr[0] + fb->pitches[0];
> -			chroma_addr[1] = chroma_addr[0] + fb->pitches[0];
> +			chroma_addr[1] = chroma_addr[0] + fb->pitches[1];
>  		}
>  	} else {
>  		luma_addr[1] = 0;
> @@ -507,25 +507,26 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  	vp_reg_write(ctx, VP_IMG_SIZE_Y, VP_IMG_HSIZE(fb->pitches[0]) |
>  		VP_IMG_VSIZE(fb->height));
>  	/* chroma plane for NV12/NV21 is half the height of the luma plane */
> -	vp_reg_write(ctx, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[0]) |
> +	vp_reg_write(ctx, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[1]) |
>  		VP_IMG_VSIZE(fb->height / 2));
>  
>  	vp_reg_write(ctx, VP_SRC_WIDTH, state->src.w);
> -	vp_reg_write(ctx, VP_SRC_HEIGHT, state->src.h);
>  	vp_reg_write(ctx, VP_SRC_H_POSITION,
>  			VP_SRC_H_POSITION_VAL(state->src.x));
> -	vp_reg_write(ctx, VP_SRC_V_POSITION, state->src.y);
>  
> -	vp_reg_write(ctx, VP_DST_WIDTH, state->crtc.w);
> -	vp_reg_write(ctx, VP_DST_H_POSITION, state->crtc.x);
>  	if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
> -		vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h / 2);
> -		vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y / 2);
> +		vp_reg_write(res, VP_SRC_HEIGHT, state->src.h / 2);
> +		vp_reg_write(res, VP_SRC_V_POSITION, state->src.y / 2);
>  	} else {
> -		vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h);
> -		vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y);
> +		vp_reg_write(res, VP_SRC_HEIGHT, state->src.h);
> +		vp_reg_write(res, VP_SRC_V_POSITION, state->src.y);
>  	}
>  
> +	vp_reg_write(res, VP_DST_WIDTH, state->crtc.w);
> +	vp_reg_write(res, VP_DST_H_POSITION, state->crtc.x);
> +	vp_reg_write(res, VP_DST_HEIGHT, state->crtc.h);
> +	vp_reg_write(res, VP_DST_V_POSITION, state->crtc.y);
> +
>  	vp_reg_write(ctx, VP_H_RATIO, state->h_ratio);
>  	vp_reg_write(ctx, VP_V_RATIO, state->v_ratio);
>  
>
kbuild test robot Nov. 23, 2017, 8:09 p.m. UTC | #2
Hi Tobias,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20171122]
[cannot apply to drm-exynos/exynos-drm/for-next v4.14 v4.14-rc8 v4.14-rc7 v4.14]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Tobias-Jakobi/drm-exynos-mixer-avoid-Oops-in-vp_video_buffer/20171124-012413
config: arm-exynos_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/exynos/exynos_mixer.c: In function 'vp_video_buffer':
>> drivers/gpu/drm/exynos/exynos_mixer.c:518:16: error: 'res' undeclared (first use in this function)
      vp_reg_write(res, VP_SRC_HEIGHT, state->src.h / 2);
                   ^~~
   drivers/gpu/drm/exynos/exynos_mixer.c:518:16: note: each undeclared identifier is reported only once for each function it appears in

vim +/res +518 drivers/gpu/drm/exynos/exynos_mixer.c

   463	
   464	static void vp_video_buffer(struct mixer_context *ctx,
   465				    struct exynos_drm_plane *plane)
   466	{
   467		struct exynos_drm_plane_state *state =
   468					to_exynos_plane_state(plane->base.state);
   469		struct drm_framebuffer *fb = state->base.fb;
   470		unsigned int priority = state->base.normalized_zpos + 1;
   471		unsigned long flags;
   472		dma_addr_t luma_addr[2], chroma_addr[2];
   473		bool is_tiled, is_nv21;
   474		u32 val;
   475	
   476		is_nv21 = (fb->format->format == DRM_FORMAT_NV21);
   477		is_tiled = (fb->modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE);
   478	
   479		luma_addr[0] = exynos_drm_fb_dma_addr(fb, 0);
   480		chroma_addr[0] = exynos_drm_fb_dma_addr(fb, 1);
   481	
   482		if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
   483			if (is_tiled) {
   484				luma_addr[1] = luma_addr[0] + 0x40;
   485				chroma_addr[1] = chroma_addr[0] + 0x40;
   486			} else {
   487				luma_addr[1] = luma_addr[0] + fb->pitches[0];
   488				chroma_addr[1] = chroma_addr[0] + fb->pitches[1];
   489			}
   490		} else {
   491			luma_addr[1] = 0;
   492			chroma_addr[1] = 0;
   493		}
   494	
   495		spin_lock_irqsave(&ctx->reg_slock, flags);
   496	
   497		/* interlace or progressive scan mode */
   498		val = (test_bit(MXR_BIT_INTERLACE, &ctx->flags) ? ~0 : 0);
   499		vp_reg_writemask(ctx, VP_MODE, val, VP_MODE_LINE_SKIP);
   500	
   501		/* setup format */
   502		val = (is_nv21 ? VP_MODE_NV21 : VP_MODE_NV12);
   503		val |= (is_tiled ? VP_MODE_MEM_TILED : VP_MODE_MEM_LINEAR);
   504		vp_reg_writemask(ctx, VP_MODE, val, VP_MODE_FMT_MASK);
   505	
   506		/* setting size of input image */
   507		vp_reg_write(ctx, VP_IMG_SIZE_Y, VP_IMG_HSIZE(fb->pitches[0]) |
   508			VP_IMG_VSIZE(fb->height));
   509		/* chroma plane for NV12/NV21 is half the height of the luma plane */
   510		vp_reg_write(ctx, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[1]) |
   511			VP_IMG_VSIZE(fb->height / 2));
   512	
   513		vp_reg_write(ctx, VP_SRC_WIDTH, state->src.w);
   514		vp_reg_write(ctx, VP_SRC_H_POSITION,
   515				VP_SRC_H_POSITION_VAL(state->src.x));
   516	
   517		if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
 > 518			vp_reg_write(res, VP_SRC_HEIGHT, state->src.h / 2);
   519			vp_reg_write(res, VP_SRC_V_POSITION, state->src.y / 2);
   520		} else {
   521			vp_reg_write(res, VP_SRC_HEIGHT, state->src.h);
   522			vp_reg_write(res, VP_SRC_V_POSITION, state->src.y);
   523		}
   524	
   525		vp_reg_write(res, VP_DST_WIDTH, state->crtc.w);
   526		vp_reg_write(res, VP_DST_H_POSITION, state->crtc.x);
   527		vp_reg_write(res, VP_DST_HEIGHT, state->crtc.h);
   528		vp_reg_write(res, VP_DST_V_POSITION, state->crtc.y);
   529	
   530		vp_reg_write(ctx, VP_H_RATIO, state->h_ratio);
   531		vp_reg_write(ctx, VP_V_RATIO, state->v_ratio);
   532	
   533		vp_reg_write(ctx, VP_ENDIAN_MODE, VP_ENDIAN_MODE_LITTLE);
   534	
   535		/* set buffer address to vp */
   536		vp_reg_write(ctx, VP_TOP_Y_PTR, luma_addr[0]);
   537		vp_reg_write(ctx, VP_BOT_Y_PTR, luma_addr[1]);
   538		vp_reg_write(ctx, VP_TOP_C_PTR, chroma_addr[0]);
   539		vp_reg_write(ctx, VP_BOT_C_PTR, chroma_addr[1]);
   540	
   541		mixer_cfg_layer(ctx, plane->index, priority, true);
   542		mixer_cfg_vp_blend(ctx);
   543	
   544		spin_unlock_irqrestore(&ctx->reg_slock, flags);
   545	
   546		mixer_regs_dump(ctx);
   547		vp_regs_dump(ctx);
   548	}
   549	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch
diff mbox

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index dc5d79465f9b..c382f99e0f5b 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -485,7 +485,7 @@  static void vp_video_buffer(struct mixer_context *ctx,
 			chroma_addr[1] = chroma_addr[0] + 0x40;
 		} else {
 			luma_addr[1] = luma_addr[0] + fb->pitches[0];
-			chroma_addr[1] = chroma_addr[0] + fb->pitches[0];
+			chroma_addr[1] = chroma_addr[0] + fb->pitches[1];
 		}
 	} else {
 		luma_addr[1] = 0;
@@ -507,25 +507,26 @@  static void vp_video_buffer(struct mixer_context *ctx,
 	vp_reg_write(ctx, VP_IMG_SIZE_Y, VP_IMG_HSIZE(fb->pitches[0]) |
 		VP_IMG_VSIZE(fb->height));
 	/* chroma plane for NV12/NV21 is half the height of the luma plane */
-	vp_reg_write(ctx, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[0]) |
+	vp_reg_write(ctx, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[1]) |
 		VP_IMG_VSIZE(fb->height / 2));
 
 	vp_reg_write(ctx, VP_SRC_WIDTH, state->src.w);
-	vp_reg_write(ctx, VP_SRC_HEIGHT, state->src.h);
 	vp_reg_write(ctx, VP_SRC_H_POSITION,
 			VP_SRC_H_POSITION_VAL(state->src.x));
-	vp_reg_write(ctx, VP_SRC_V_POSITION, state->src.y);
 
-	vp_reg_write(ctx, VP_DST_WIDTH, state->crtc.w);
-	vp_reg_write(ctx, VP_DST_H_POSITION, state->crtc.x);
 	if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
-		vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h / 2);
-		vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y / 2);
+		vp_reg_write(res, VP_SRC_HEIGHT, state->src.h / 2);
+		vp_reg_write(res, VP_SRC_V_POSITION, state->src.y / 2);
 	} else {
-		vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h);
-		vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y);
+		vp_reg_write(res, VP_SRC_HEIGHT, state->src.h);
+		vp_reg_write(res, VP_SRC_V_POSITION, state->src.y);
 	}
 
+	vp_reg_write(res, VP_DST_WIDTH, state->crtc.w);
+	vp_reg_write(res, VP_DST_H_POSITION, state->crtc.x);
+	vp_reg_write(res, VP_DST_HEIGHT, state->crtc.h);
+	vp_reg_write(res, VP_DST_V_POSITION, state->crtc.y);
+
 	vp_reg_write(ctx, VP_H_RATIO, state->h_ratio);
 	vp_reg_write(ctx, VP_V_RATIO, state->v_ratio);