Re: [PATCH 09/13] drm/exynos: check NV12M format specific to Exynos properly

[Joonyoung Shim <jy0922.shim@samsung.com>]

On 08/20/2012 11:23 AM, InKi Dae wrote:
> 2012/8/20 Joonyoung Shim <jy0922.shim@samsung.com>:
>> On 08/17/2012 06:50 PM, Inki Dae wrote:
>>> this patch adds buf_cnt variable in exynos_drm_fb structure and
>>> that means a buffer count to drm framebuffer and also adds two
>>> functions to get/set the buffer count from/to exynos_drm_fb structure.
>>> if pixel format is not DRM_FORMAT_NV12MT then it gets a buffer count
>>> to drm framebuffer refering to mode_cmd->handles and offsets.
>>> but when booted, the buffer count will always be 1 because pixel
>>> format of console framebuffer is RGB format.
>>>
>>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>>    drivers/gpu/drm/exynos/exynos_drm_fb.c    |   65
>>> +++++++++++++++++++++++++++-
>>>    drivers/gpu/drm/exynos/exynos_drm_fb.h    |   20 +++------
>>>    drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    3 +
>>>    drivers/gpu/drm/exynos/exynos_drm_plane.c |    2 +-
>>>    4 files changed, 73 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_fb.c
>>> index 4ccfe43..2d1bc3a 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
>>> @@ -41,10 +41,12 @@
>>>     * exynos specific framebuffer structure.
>>>     *
>>>     * @fb: drm framebuffer obejct.
>>> + * @buf_cnt: a buffer count to drm framebuffer.
>>>     * @exynos_gem_obj: array of exynos specific gem object containing a gem
>>> object.
>>>     */
>>>    struct exynos_drm_fb {
>>>          struct drm_framebuffer          fb;
>>> +       unsigned int                    buf_cnt;
>>>          struct exynos_drm_gem_obj       *exynos_gem_obj[MAX_FB_BUFFER];
>>>    };
>>>    @@ -101,6 +103,25 @@ static struct drm_framebuffer_funcs
>>> exynos_drm_fb_funcs = {
>>>          .dirty          = exynos_drm_fb_dirty,
>>>    };
>>>    +void exynos_drm_fb_set_buf_cnt(struct drm_framebuffer *fb,
>>> +                                               unsigned int cnt)
>>> +{
>>> +       struct exynos_drm_fb *exynos_fb;
>>> +
>>> +       exynos_fb = to_exynos_fb(fb);
>>> +
>>> +       exynos_fb->buf_cnt = cnt;
>>> +}
>>> +
>>> +unsigned int exynos_drm_fb_get_buf_cnt(struct drm_framebuffer *fb)
>>> +{
>>> +       struct exynos_drm_fb *exynos_fb;
>>> +
>>> +       exynos_fb = to_exynos_fb(fb);
>>> +
>>> +       return exynos_fb->buf_cnt;
>>> +}
>>> +
>>>    struct drm_framebuffer *
>>>    exynos_drm_framebuffer_init(struct drm_device *dev,
>>>                              struct drm_mode_fb_cmd2 *mode_cmd,
>>> @@ -127,6 +148,43 @@ 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_NV12MT)
>>> +               return 2;
>>> +
>>> +       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;
>>> +}
>>
>> No, please don't add specific function. There is already
>> drm_format_num_planes() function
>>
>>
> I know that, but NV12M format is specific to Exynos. for this, we
> already had a discussion and you can refer to below link,
> http://web.archiveorange.com/archive/v/hhSc5JAv767vo7fKZLPf

Yes, but this implementation is not clear, just get plane number using 
drm_format_num_planes()
and check handle and offset argument when format is NV12.

>>> +
>>>    static struct drm_framebuffer *
>>>    exynos_user_fb_create(struct drm_device *dev, struct drm_file
>>> *file_priv,
>>>                        struct drm_mode_fb_cmd2 *mode_cmd)
>>> @@ -134,7 +192,6 @@ exynos_user_fb_create(struct drm_device *dev, struct
>>> drm_file *file_priv,
>>>          struct drm_gem_object *obj;
>>>          struct drm_framebuffer *fb;
>>>          struct exynos_drm_fb *exynos_fb;
>>> -       int nr;
>>>          int i;
>>>          DRM_DEBUG_KMS("%s\n", __FILE__);
>>> @@ -152,9 +209,11 @@ exynos_user_fb_create(struct drm_device *dev, struct
>>> drm_file *file_priv,
>>>          }
>>>          exynos_fb = to_exynos_fb(fb);
>>> -       nr = exynos_drm_format_num_buffers(fb->pixel_format);
>>> +       exynos_fb->buf_cnt = exynos_drm_format_num_buffers(mode_cmd);
>>> +
>>> +       DRM_DEBUG_KMS("buf_cnt = %d\n", exynos_fb->buf_cnt);
>>>    -     for (i = 1; i < nr; i++) {
>>> +       for (i = 1; i < exynos_fb->buf_cnt; i++) {
>>>                  obj = drm_gem_object_lookup(dev, file_priv,
>>>                                  mode_cmd->handles[i]);
>>>                  if (!obj) {
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.h
>>> b/drivers/gpu/drm/exynos/exynos_drm_fb.h
>>> index 5082375..96262e5 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.h
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.h
>>> @@ -28,19 +28,6 @@
>>>    #ifndef _EXYNOS_DRM_FB_H_
>>>    #define _EXYNOS_DRM_FB_H
>>>    -static inline int exynos_drm_format_num_buffers(uint32_t format)
>>> -{
>>> -       switch (format) {
>>> -       case DRM_FORMAT_NV12:
>>> -       case DRM_FORMAT_NV12MT:
>>> -               return 2;
>>> -       case DRM_FORMAT_YUV420:
>>> -               return 3;
>>> -       default:
>>> -               return 1;
>>> -       }
>>> -}
>>> -
>>>    struct drm_framebuffer *
>>>    exynos_drm_framebuffer_init(struct drm_device *dev,
>>>                              struct drm_mode_fb_cmd2 *mode_cmd,
>>> @@ -52,4 +39,11 @@ struct exynos_drm_gem_buf *exynos_drm_fb_buffer(struct
>>> drm_framebuffer *fb,
>>>      void exynos_drm_mode_config_init(struct drm_device *dev);
>>>    +/* set a buffer count to drm framebuffer. */
>>> +void exynos_drm_fb_set_buf_cnt(struct drm_framebuffer *fb,
>>> +                                               unsigned int cnt);
>>> +
>>> +/* get a buffer count to drm framebuffer. */
>>> +unsigned int exynos_drm_fb_get_buf_cnt(struct drm_framebuffer *fb);
>>> +
>>>    #endif
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> index d5586cc..5b125fe 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> @@ -79,6 +79,9 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper
>>> *helper,
>>>                  return -EFAULT;
>>>          }
>>>    +     /* buffer count to framebuffer always is 1 at booting time. */
>>> +       exynos_drm_fb_set_buf_cnt(fb, 1);
>>> +
>>>          offset = fbi->var.xoffset * (fb->bits_per_pixel >> 3);
>>>          offset += fbi->var.yoffset * fb->pitches[0];
>>>    diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_plane.c
>>> index b89829e..777e142 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
>>> @@ -48,7 +48,7 @@ int exynos_plane_mode_set(struct drm_plane *plane,
>>> struct drm_crtc *crtc,
>>>          DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>>>    -     nr = exynos_drm_format_num_buffers(fb->pixel_format);
>>> +       nr = exynos_drm_fb_get_buf_cnt(fb);
>>>          for (i = 0; i < nr; i++) {
>>>                  struct exynos_drm_gem_buf *buffer =
>>> exynos_drm_fb_buffer(fb, i);
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel