Re: [PATCH 2/2] drm/msm: Support NV12MT format in mdp4

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 20, 2015 at 9:42 AM, Beeresh Gopal <gbeeresh@xxxxxxxxxxxxxx> wrote:
> Using fb modifier flag, support NV12MT format
> in MDP4
>
> Change-Id: I3de92b0c3b6d0cb56647aed6e4e35e56eff7adab
> Signed-off-by: Beeresh Gopal <gbeeresh@xxxxxxxxxxxxxx>
> Signed-off-by: Stephane Viau <sviau@xxxxxxxxxxxxxx>

on small comment below, and one open question at the end, but other
than that it looks good.

oh, and you should drop the Change-Id


> ---
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c   |  9 +++++++++
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 22 ++++++++++++++++++++++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   |  5 +++++
>  drivers/gpu/drm/msm/msm_drv.c             |  2 ++
>  drivers/gpu/drm/msm/msm_kms.h             |  1 +
>  include/uapi/drm/drm_fourcc.h             | 14 ++++++++++++++
>  6 files changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> index d81e19d..24c38d4 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> @@ -157,6 +157,14 @@ static void mdp4_complete_commit(struct msm_kms *kms, struct drm_atomic_state *s
>         mdp4_disable(mdp4_kms);
>  }
>
> +static void mdp4_set_mode_config(struct msm_kms *kms)
> +{
> +       struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
> +       struct drm_device *dev = mdp4_kms->dev;
> +
> +       dev->mode_config.allow_fb_modifiers = true;

why not just add this in mdp4_kms_init() or hw_init()?  That would
avoid having to add a new vfunc and keep things a bit simpler

> +}
> +
>  static long mdp4_round_pixclk(struct msm_kms *kms, unsigned long rate,
>                 struct drm_encoder *encoder)
>  {
> @@ -197,6 +205,7 @@ static const struct mdp_kms_funcs kms_funcs = {
>                 .complete_commit = mdp4_complete_commit,
>                 .get_format      = mdp_get_format,
>                 .round_pixclk    = mdp4_round_pixclk,
> +               .set_mode_config = mdp4_set_mode_config,
>                 .preclose        = mdp4_preclose,
>                 .destroy         = mdp4_destroy,
>         },
> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> index cde2500..2c2d6a5 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> @@ -33,6 +33,21 @@ struct mdp4_plane {
>  };
>  #define to_mdp4_plane(x) container_of(x, struct mdp4_plane, base)
>
> +/* MDP format helper functions */
> +static inline
> +enum mdp4_frame_format mdp4_get_frame_format(struct drm_framebuffer *fb)
> +{
> +       bool is_tile = false;
> +
> +       if (fb->modifier[1] == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE)
> +               is_tile = true;
> +
> +       if (fb->pixel_format == DRM_FORMAT_NV12 && is_tile)
> +               return FRAME_TILE_YCBCR_420;
> +
> +       return FRAME_LINEAR;
> +}
> +
>  static void mdp4_plane_set_scanout(struct drm_plane *plane,
>                 struct drm_framebuffer *fb);
>  static int mdp4_plane_mode_set(struct drm_plane *plane,
> @@ -203,6 +218,7 @@ static int mdp4_plane_mode_set(struct drm_plane *plane,
>         uint32_t op_mode = 0;
>         uint32_t phasex_step = MDP4_VG_PHASE_STEP_DEFAULT;
>         uint32_t phasey_step = MDP4_VG_PHASE_STEP_DEFAULT;
> +       enum mdp4_frame_format frame_type = mdp4_get_frame_format(fb);
>
>         if (!(crtc && fb)) {
>                 DBG("%s: disabled!", mdp4_plane->name);
> @@ -302,6 +318,7 @@ static int mdp4_plane_mode_set(struct drm_plane *plane,
>                         MDP4_PIPE_SRC_FORMAT_UNPACK_COUNT(format->unpack_count - 1) |
>                         MDP4_PIPE_SRC_FORMAT_FETCH_PLANES(format->fetch_type) |
>                         MDP4_PIPE_SRC_FORMAT_CHROMA_SAMP(format->chroma_sample) |
> +                       MDP4_PIPE_SRC_FORMAT_FRAME_FORMAT(frame_type) |
>                         COND(format->unpack_tight, MDP4_PIPE_SRC_FORMAT_UNPACK_TIGHT));
>
>         mdp4_write(mdp4_kms, REG_MDP4_PIPE_SRC_UNPACK(pipe),
> @@ -322,6 +339,11 @@ static int mdp4_plane_mode_set(struct drm_plane *plane,
>         mdp4_write(mdp4_kms, REG_MDP4_PIPE_PHASEX_STEP(pipe), phasex_step);
>         mdp4_write(mdp4_kms, REG_MDP4_PIPE_PHASEY_STEP(pipe), phasey_step);
>
> +       if (frame_type != FRAME_LINEAR)
> +               mdp4_write(mdp4_kms, REG_MDP4_PIPE_SSTILE_FRAME_SIZE(pipe),
> +                               MDP4_PIPE_SSTILE_FRAME_SIZE_WIDTH(src_w) |
> +                               MDP4_PIPE_SSTILE_FRAME_SIZE_HEIGHT(src_h));
> +
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> index 92b61db..9cb5a5d 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> @@ -80,6 +80,10 @@ static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *s
>         mdp5_disable(mdp5_kms);
>  }
>
> +static void mdp5_set_mode_config(struct msm_kms *kms)
> +{
> +}
> +
>  static long mdp5_round_pixclk(struct msm_kms *kms, unsigned long rate,
>                 struct drm_encoder *encoder)
>  {
> @@ -131,6 +135,7 @@ static const struct mdp_kms_funcs kms_funcs = {
>                 .complete_commit = mdp5_complete_commit,
>                 .get_format      = mdp_get_format,
>                 .round_pixclk    = mdp5_round_pixclk,
> +               .set_mode_config = mdp5_set_mode_config,
>                 .preclose        = mdp5_preclose,
>                 .destroy         = mdp5_destroy,
>         },
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index dfd583f..a72ed0a 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -281,6 +281,8 @@ static int msm_load(struct drm_device *dev, unsigned long flags)
>                         dev_err(dev->dev, "kms hw init failed: %d\n", ret);
>                         goto fail;
>                 }
> +
> +               kms->funcs->set_mode_config(kms);
>         }
>
>         dev->mode_config.min_width = 0;
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 3a78cb4..d6e3bb4 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -47,6 +47,7 @@ struct msm_kms_funcs {
>         const struct msm_format *(*get_format)(struct msm_kms *kms, uint32_t format);
>         long (*round_pixclk)(struct msm_kms *kms, unsigned long rate,
>                         struct drm_encoder *encoder);
> +       void (*set_mode_config)(struct msm_kms *kms);
>         /* cleanup: */
>         void (*preclose)(struct msm_kms *kms, struct drm_file *file);
>         void (*destroy)(struct msm_kms *kms);
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 188e61f..1546302 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -161,4 +161,18 @@
>   * authoritative source for all of these.
>   */
>
> +/* Samsung framebuffer modifiers */
> +
> +/*
> + * NV12 64x32 Tiled
> + *
> + * 2 planes Y and CbCr, grouped into 64x32 macro tiles,
> + * with a non-standard order in memory (Z-shape).
> + *
> + * Pixel layout identical to DRM_FORMAT_NV21 format:
> + * index 0 = Y plane, [7:0] Y
> + * index 1 = Cb:Cr plane, [15:0] Cb:Cr little endian
> + */
> +#define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1)
> +

so, the one thing I couldn't quite figure out...  I'm assuming the
untiling is before un-sub-sampling the UV, meaning that the pattern in
the UV plane is 32x16 pairs of pixels (so 64x16).  I found some
gstreamer code that untile a buffer in this layout, but there were too
many levels of indirection ;-)

I'm not sure if we should then define separate modifiers for Y and UV.
Or maybe it is simpler the define that the layout depends on the
plane?



>  #endif /* DRM_FOURCC_H */
> --
> 1.8.2.1
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux