LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Stephane Viau <sviau@codeaurora.org>
Cc: dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org, robdclark@gmail.com,
	daniel@ffwll.ch, Beeresh Gopal <gbeeresh@codeaurora.org>
Subject: Re: [PATCH v2 2/2] drm/msm/mdp4: Support NV12MT format in mdp4
Date: Mon, 16 Mar 2015 09:05:29 +0100	[thread overview]
Message-ID: <20150316080238.GA21993@phenom.ffwll.local> (raw)
In-Reply-To: <1426273930-12788-3-git-send-email-sviau@codeaurora.org>

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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2015-03-16  8:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-20 14:42 [PATCH 1/2] drm/msm: update generated headers Beeresh Gopal
2015-02-20 14:42 ` [PATCH 2/2] drm/msm: Support NV12MT format in mdp4 Beeresh Gopal
2015-02-20 15:26   ` Rob Clark
2015-03-13 19:12     ` [PATCH v2 0/2] drm/msm: Add support for " Stephane Viau
2015-03-13 19:12       ` [PATCH v2 1/2] drm/msm: update generated headers Stephane Viau
2015-03-13 19:14         ` [PATCH] rnndb: " Stephane Viau
2015-03-13 19:12       ` [PATCH v2 2/2] drm/msm/mdp4: Support NV12MT format in mdp4 Stephane Viau
2015-03-16  8:05         ` Daniel Vetter [this message]
2015-03-24 18:32           ` Rob Clark
2015-03-25  2:26             ` Seung-Woo Kim
2015-02-22 10:34   ` [PATCH 2/2] drm/msm: " Daniel Vetter
2015-03-13 14:51     ` "Stéphane Viau"

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150316080238.GA21993@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gbeeresh@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robdclark@gmail.com \
    --cc=sviau@codeaurora.org \
    --subject='Re: [PATCH v2 2/2] drm/msm/mdp4: Support NV12MT format in mdp4' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).