| Message ID | 1426273930-12788-3-git-send-email-sviau@codeaurora.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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 >
On Mon, Mar 16, 2015 at 4:05 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > 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. + 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
Hello, On 2015? 03? 25? 03:32, Rob Clark wrote: > On Mon, Mar 16, 2015 at 4:05 AM, Daniel Vetter <daniel@ffwll.ch> wrote: >> 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(+) >>> <snip.> >>> 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) Yes, I have a plan to apply fb_modifier for exynos with kms interface of hdmi. > > vl4 also has this format, but last I looked was rather light on the details. I am not sure msm mdp uses exactly same format with exynos, but anyway v4l2 NV12MT format was introduced for exynos hw video codec. macro blocks for the format is laid z-order and each pixel data in each macro block is just normal NV12 style. I think Marek and Sylwester can help understanding the format. > > 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 >
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 */ +#define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1) + #endif /* DRM_FOURCC_H */