Re: [PATCH v2 2/2] drm/msm/mdp4: Support NV12MT format in mdp4
From: Rob Clark
Date: Tue Mar 24 2015 - 14:33:16 EST
On Mon, Mar 16, 2015 at 4:05 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Fri, Mar 13, 2015 at 03:12:10PM -0400, Stephane Viau wrote:
>> From: Beeresh Gopal <gbeeresh@xxxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxx>
>> Signed-off-by: Stephane Viau <sviau@xxxxxxxxxxxxxx>
>> ---
>> 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.
+ a couple folks from Samsung, since I expect they want this for
exynos as well (and might be able to help with the description)
vl4 also has this format, but last I looked was rather light on the details.
http://linuxtv.org/downloads/v4l-dvb-apis/re31.html
I know up in userspace, GStreamer seems to have some support for this format:
http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=f8d3b9b4fcc5e08b771314fa95e9ed8f750b54e6
> 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.
afaiu, stride (and maybe even width?) should be a multiple of the
block size (but height does not)
BR,
-R
> -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
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/