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 at 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; >>