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);
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;
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;- /* 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;
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;- /* 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;
dri-devel@lists.freedesktop.org
-
Joonyoung Shim -
Tobias Jakobi