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

Reply via email to