[PATCH 1/2] drm/exynos: fb: use drm_format_num_planes to get buffer count

[PATCH 1/2] drm/exynos: fb: use drm_format_num_planes to get buffer count

[Tobias Jakobi]

The previous code had some special case handling for the buffer
count in exynos_drm_format_num_buffers().

This code was incorrect though, since this special case doesn't
exist for DRM. It stemmed from the existence of the special NV12M
V4L2 format. NV12 is a bi-planar format (separate planes for luma
and chroma) and V4L2 differentiates between a NV12 buffer where
luma and chroma is contiguous in memory (so no data between
luma/chroma), and a NV12 buffer where luma and chroma have two
explicit memory locations (which is then called NV12M).

This distinction doesn't exist for DRM. A bi-planar format always
explicitly comes with the information about its two planes (even
if these planes should be contiguous).

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_drm_fb.c | 39 +---------------------------------
 1 file changed, 1 insertion(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index 929cb03..142eb4e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -171,43 +171,6 @@ exynos_drm_framebuffer_init(struct drm_device *dev,
 	return &exynos_fb->fb;
 }
 
-static u32 exynos_drm_format_num_buffers(struct drm_mode_fb_cmd2 *mode_cmd)
-{
-	unsigned int cnt = 0;
-
-	if (mode_cmd->pixel_format != DRM_FORMAT_NV12)
-		return drm_format_num_planes(mode_cmd->pixel_format);
-
-	while (cnt != MAX_FB_BUFFER) {
-		if (!mode_cmd->handles[cnt])
-			break;
-		cnt++;
-	}
-
-	/*
-	 * check if NV12 or NV12M.
-	 *
-	 * NV12
-	 * handles[0] = base1, offsets[0] = 0
-	 * handles[1] = base1, offsets[1] = Y_size
-	 *
-	 * NV12M
-	 * handles[0] = base1, offsets[0] = 0
-	 * handles[1] = base2, offsets[1] = 0
-	 */
-	if (cnt == 2) {
-		/*
-		 * in case of NV12 format, offsets[1] is not 0 and
-		 * handles[0] is same as handles[1].
-		 */
-		if (mode_cmd->offsets[1] &&
-			mode_cmd->handles[0] == mode_cmd->handles[1])
-			cnt = 1;
-	}
-
-	return cnt;
-}
-
 static struct drm_framebuffer *
 exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
 		      struct drm_mode_fb_cmd2 *mode_cmd)
@@ -230,7 +193,7 @@ exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
 
 	drm_helper_mode_fill_fb_struct(&exynos_fb->fb, mode_cmd);
 	exynos_fb->exynos_gem_obj[0] = to_exynos_gem_obj(obj);
-	exynos_fb->buf_cnt = exynos_drm_format_num_buffers(mode_cmd);
+	exynos_fb->buf_cnt = drm_format_num_planes(mode_cmd->pixel_format);
 
 	DRM_DEBUG_KMS("buf_cnt = %d\n", exynos_fb->buf_cnt);
 
-- 
2.0.5

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

[PATCH 2/2] drm/exynos: mixer: remove buffer count handling in vp_video_buffer()

[Tobias Jakobi]

The video processor (VP) supports four formats: NV12, NV21 and its
tiled variants. All these formats are bi-planar, so the buffer
count in vp_video_buffer() is always 2.

While we're at it, also add support for NV21 and properly exit
if we're called with an invalid (non-VP) pixelformat.

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

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 534a594..6fb4faa 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -388,7 +388,6 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
 	struct mixer_resources *res = &ctx->mixer_res;
 	unsigned long flags;
 	struct exynos_drm_plane *plane;
-	unsigned int buf_num = 1;
 	dma_addr_t luma_addr[2], chroma_addr[2];
 	bool tiled_mode = false;
 	bool crcb_mode = false;
@@ -399,27 +398,18 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
 	switch (plane->pixel_format) {
 	case DRM_FORMAT_NV12:
 		crcb_mode = false;
-		buf_num = 2;
 		break;
-	/* TODO: single buffer format NV12, NV21 */
+	case DRM_FORMAT_NV21:
+		crcb_mode = true;
+		break;
 	default:
-		/* ignore pixel format at disable time */
-		if (!plane->dma_addr[0])
-			break;
-
 		DRM_ERROR("pixel format for vp is wrong [%d].\n",
 				plane->pixel_format);
 		return;
 	}
 
-	if (buf_num == 2) {
-		luma_addr[0] = plane->dma_addr[0];
-		chroma_addr[0] = plane->dma_addr[1];
-	} else {
-		luma_addr[0] = plane->dma_addr[0];
-		chroma_addr[0] = plane->dma_addr[0]
-			+ (plane->pitch * plane->fb_height);
-	}
+	luma_addr[0] = plane->dma_addr[0];
+	chroma_addr[0] = plane->dma_addr[1];
 
 	if (plane->scan_flag & DRM_MODE_FLAG_INTERLACE) {
 		ctx->interlace = true;
-- 
2.0.5

Re: [PATCH 2/2] drm/exynos: mixer: remove buffer count handling in vp_video_buffer()

[Joonyoung Shim]

Hi Tobias,

On 04/26/2015 05:31 AM, Tobias Jakobi wrote:
> The video processor (VP) supports four formats: NV12, NV21 and its
> tiled variants. All these formats are bi-planar, so the buffer
> count in vp_video_buffer() is always 2.
> 
> While we're at it, also add support for NV21 and properly exit
> if we're called with an invalid (non-VP) pixelformat.
> 
> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 534a594..6fb4faa 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -388,7 +388,6 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
>  	struct mixer_resources *res = &ctx->mixer_res;
>  	unsigned long flags;
>  	struct exynos_drm_plane *plane;
> -	unsigned int buf_num = 1;
>  	dma_addr_t luma_addr[2], chroma_addr[2];
>  	bool tiled_mode = false;
>  	bool crcb_mode = false;
> @@ -399,27 +398,18 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
>  	switch (plane->pixel_format) {
>  	case DRM_FORMAT_NV12:
>  		crcb_mode = false;
> -		buf_num = 2;
>  		break;
> -	/* TODO: single buffer format NV12, NV21 */
> +	case DRM_FORMAT_NV21:
> +		crcb_mode = true;
> +		break;

To support NV21, how about make to other patch? because this patch is to
fix buffer number.

>  	default:
> -		/* ignore pixel format at disable time */
> -		if (!plane->dma_addr[0])
> -			break;
> -
>  		DRM_ERROR("pixel format for vp is wrong [%d].\n",
>  				plane->pixel_format);
>  		return;
>  	}
>  
> -	if (buf_num == 2) {
> -		luma_addr[0] = plane->dma_addr[0];
> -		chroma_addr[0] = plane->dma_addr[1];
> -	} else {
> -		luma_addr[0] = plane->dma_addr[0];
> -		chroma_addr[0] = plane->dma_addr[0]
> -			+ (plane->pitch * plane->fb_height);
> -	}
> +	luma_addr[0] = plane->dma_addr[0];
> +	chroma_addr[0] = plane->dma_addr[1];

Hmm, dma_addr[0] and dma_addr[1] are same address if it uses just one
buffer for two planes, then need offset information of each plane.

Thanks.

>  
>  	if (plane->scan_flag & DRM_MODE_FLAG_INTERLACE) {
>  		ctx->interlace = true;
> 

Re: [PATCH 2/2] drm/exynos: mixer: remove buffer count handling in vp_video_buffer()

[Tobias Jakobi]

On 2015-04-27 09:24, Joonyoung Shim wrote:
> Hi Tobias,
> 
> On 04/26/2015 05:31 AM, Tobias Jakobi wrote:
>> The video processor (VP) supports four formats: NV12, NV21 and its
>> tiled variants. All these formats are bi-planar, so the buffer
>> count in vp_video_buffer() is always 2.
>> 
>> While we're at it, also add support for NV21 and properly exit
>> if we're called with an invalid (non-VP) pixelformat.
>> 
>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 20 +++++---------------
>>  1 file changed, 5 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 534a594..6fb4faa 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -388,7 +388,6 @@ static void vp_video_buffer(struct mixer_context 
>> *ctx, int win)
>>  	struct mixer_resources *res = &ctx->mixer_res;
>>  	unsigned long flags;
>>  	struct exynos_drm_plane *plane;
>> -	unsigned int buf_num = 1;
>>  	dma_addr_t luma_addr[2], chroma_addr[2];
>>  	bool tiled_mode = false;
>>  	bool crcb_mode = false;
>> @@ -399,27 +398,18 @@ static void vp_video_buffer(struct mixer_context 
>> *ctx, int win)
>>  	switch (plane->pixel_format) {
>>  	case DRM_FORMAT_NV12:
>>  		crcb_mode = false;
>> -		buf_num = 2;
>>  		break;
>> -	/* TODO: single buffer format NV12, NV21 */
>> +	case DRM_FORMAT_NV21:
>> +		crcb_mode = true;
>> +		break;
> 
> To support NV21, how about make to other patch? because this patch is 
> to
> fix buffer number.
Sure, going to split this off.



>>  	default:
>> -		/* ignore pixel format at disable time */
>> -		if (!plane->dma_addr[0])
>> -			break;
>> -
>>  		DRM_ERROR("pixel format for vp is wrong [%d].\n",
>>  				plane->pixel_format);
>>  		return;
>>  	}
>> 
>> -	if (buf_num == 2) {
>> -		luma_addr[0] = plane->dma_addr[0];
>> -		chroma_addr[0] = plane->dma_addr[1];
>> -	} else {
>> -		luma_addr[0] = plane->dma_addr[0];
>> -		chroma_addr[0] = plane->dma_addr[0]
>> -			+ (plane->pitch * plane->fb_height);
>> -	}
>> +	luma_addr[0] = plane->dma_addr[0];
>> +	chroma_addr[0] = plane->dma_addr[1];
> 
> Hmm, dma_addr[0] and dma_addr[1] are same address if it uses just one
> buffer for two planes, then need offset information of each plane.
Good catch! I was already wondering why the colors were wrong, but 
that's of course to be expected if luma/chroma addr are the same.

I should probably apply offsets when writing dma_addr[i] to the plane 
object.


With best wishes,
Tobias


> Thanks.
> 
>> 
>>  	if (plane->scan_flag & DRM_MODE_FLAG_INTERLACE) {
>>  		ctx->interlace = true;
>>