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

On Fri, Mar 13, 2015 at 03:12:10PM -0400, Stephane Viau wrote:

From: Beeresh Gopal gbeeresh@codeaurora.org

Using fb modifier flag, support NV12MT format in MDP4.

v2:

• rework the modifier's description [Daniel Vetter's comment]
• drop .set_mode_config() callback [Rob Clark's comment]

Signed-off-by: Beeresh Gopal gbeeresh@codeaurora.org Signed-off-by: Stephane Viau sviau@codeaurora.org

---

drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 2 ++ drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 22 ++++++++++++++++++++++ include/uapi/drm/drm_fourcc.h | 5 +++++ 3 files changed, 29 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..6387881 100644 --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c @@ -119,6 +119,8 @@ static int mdp4_hw_init(struct msm_kms *kms) if (mdp4_kms->rev > 1) mdp4_write(mdp4_kms, REG_MDP4_RESET_STATUS, 1);

• dev->mode_config.allow_fb_modifiers = true;

out: pm_runtime_put_sync(dev->dev);

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/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 188e61f..2ff79cb 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -161,4 +161,9 @@

• authoritative source for all of these.

*/

+/* Samsung framebuffer modifiers */

+/* Tiled: 64x32 pixel macroblocks */

Since this seems shared by a lot of vendors (I still don't believe Samsung invented this really ...) can you please describe this thing a bit in more detail? Somewhat important how macroblocks are laid out and pixels within. Also with a planar format like NV12 "pixel" is a bit unclear, e.g. what happens if you throw 10bit plane formats at this? So maybe also add a note that for now this is only used together with NV12T.

Then there's the question of validating the input - stride should probably be a full multiple of the macroblock size. Since this is a shared format imo this kind of checking should be done in drm core. -Daniel

+#define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1)

#endif /* DRM_FOURCC_H */

Qualcomm Innovation Center, Inc.

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project