All of lore.kernel.org
 help / color / mirror / Atom feed
From: abhinavk@codeaurora.org
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
	Jonathan Marek <jonathan@marek.ca>,
	Stephen Boyd <sboyd@kernel.org>, David Airlie <airlied@linux.ie>,
	Daniel Vetter <daniel@ffwll.ch>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org
Subject: Re: [Freedreno] [PATCH 07/11] drm/msm/dpu: drop dpu_csc_cfg from dpu_plane
Date: Thu, 21 Oct 2021 16:17:05 -0700	[thread overview]
Message-ID: <afe17ee7977bc967ddc0b35253d4e8f0@codeaurora.org> (raw)
In-Reply-To: <20210930140002.308628-8-dmitry.baryshkov@linaro.org>

On 2021-09-30 06:59, Dmitry Baryshkov wrote:
> Simplify code surrounding CSC table setup by removing struct 
> dpu_csc_cfg
> pointer from dpu_plane and getting it directly at the CSC setup time.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 96 +++++++++++----------
>  5 files changed, 54 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> index cbafb61404d0..103d4bd7585b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> @@ -537,7 +537,7 @@ static void dpu_hw_sspp_setup_sourceaddress(struct
> dpu_hw_pipe *ctx,
>  }
> 
>  static void dpu_hw_sspp_setup_csc(struct dpu_hw_pipe *ctx,
> -		struct dpu_csc_cfg *data)
> +		const struct dpu_csc_cfg *data)
>  {
>  	u32 idx;
>  	bool csc10 = false;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> index 27263bc1a1ef..e8939d7387cb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> @@ -262,7 +262,7 @@ struct dpu_hw_sspp_ops {
>  	 * @ctx: Pointer to pipe context
>  	 * @data: Pointer to config structure
>  	 */
> -	void (*setup_csc)(struct dpu_hw_pipe *ctx, struct dpu_csc_cfg *data);
> +	void (*setup_csc)(struct dpu_hw_pipe *ctx, const struct dpu_csc_cfg 
> *data);
> 
>  	/**
>  	 * setup_solidfill - enable/disable colorfill
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
> index f94584c982cd..aad85116b0a0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
> @@ -374,7 +374,7 @@ u32 dpu_hw_get_scaler3_ver(struct 
> dpu_hw_blk_reg_map *c,
> 
>  void dpu_hw_csc_setup(struct dpu_hw_blk_reg_map *c,
>  		u32 csc_reg_off,
> -		struct dpu_csc_cfg *data, bool csc10)
> +		const struct dpu_csc_cfg *data, bool csc10)
>  {
>  	static const u32 matrix_shift = 7;
>  	u32 clamp_shift = csc10 ? 16 : 8;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> index ff3cffde84cd..bc2fdb2b8f5f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> @@ -321,6 +321,6 @@ u32 dpu_hw_get_scaler3_ver(struct 
> dpu_hw_blk_reg_map *c,
> 
>  void dpu_hw_csc_setup(struct dpu_hw_blk_reg_map  *c,
>  		u32 csc_reg_off,
> -		struct dpu_csc_cfg *data, bool csc10);
> +		const struct dpu_csc_cfg *data, bool csc10);
> 
>  #endif /* _DPU_HW_UTIL_H */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 4259c4ecde9b..b8836c089863 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -90,7 +90,6 @@ enum dpu_plane_qos {
>  /*
>   * struct dpu_plane - local dpu plane structure
>   * @aspace: address space pointer
> - * @csc_ptr: Points to dpu_csc_cfg structure to use for current
>   * @mplane_list: List of multirect planes of the same pipe
>   * @catalog: Points to dpu catalog structure
>   * @revalidate: force revalidation of all the plane properties
> @@ -111,8 +110,6 @@ struct dpu_plane {
>  	struct list_head mplane_list;
>  	struct dpu_mdss_cfg *catalog;
> 
> -	struct dpu_csc_cfg *csc_ptr;
> -
>  	const struct dpu_sspp_sub_blks *pipe_sblk;
> 
>  	/* debugfs related stuff */
> @@ -605,51 +602,59 @@ static void _dpu_plane_setup_scaler3(struct
> dpu_plane *pdpu,
>  	scale_cfg->enable = 1;
>  }
> 
> -static void _dpu_plane_setup_csc(struct dpu_plane *pdpu)
> -{
> -	static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = {
> -		{
> -			/* S15.16 format */
> -			0x00012A00, 0x00000000, 0x00019880,
> -			0x00012A00, 0xFFFF9B80, 0xFFFF3000,
> -			0x00012A00, 0x00020480, 0x00000000,
> +static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = {
> +	{
> +		/* S15.16 format */
> +		0x00012A00, 0x00000000, 0x00019880,
> +		0x00012A00, 0xFFFF9B80, 0xFFFF3000,
> +		0x00012A00, 0x00020480, 0x00000000,
> +	},
> +	/* signed bias */
> +	{ 0xfff0, 0xff80, 0xff80,},
> +	{ 0x0, 0x0, 0x0,},
> +	/* unsigned clamp */
> +	{ 0x10, 0xeb, 0x10, 0xf0, 0x10, 0xf0,},
> +	{ 0x00, 0xff, 0x00, 0xff, 0x00, 0xff,},
> +};
> +
> +static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
> +	{
> +		/* S15.16 format */
> +		0x00012A00, 0x00000000, 0x00019880,
> +		0x00012A00, 0xFFFF9B80, 0xFFFF3000,
> +		0x00012A00, 0x00020480, 0x00000000,
>  		},
> -		/* signed bias */
> -		{ 0xfff0, 0xff80, 0xff80,},
> -		{ 0x0, 0x0, 0x0,},
> -		/* unsigned clamp */
> -		{ 0x10, 0xeb, 0x10, 0xf0, 0x10, 0xf0,},
> -		{ 0x00, 0xff, 0x00, 0xff, 0x00, 0xff,},
> -	};
> -	static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
> -		{
> -			/* S15.16 format */
> -			0x00012A00, 0x00000000, 0x00019880,
> -			0x00012A00, 0xFFFF9B80, 0xFFFF3000,
> -			0x00012A00, 0x00020480, 0x00000000,
> -			},
> -		/* signed bias */
> -		{ 0xffc0, 0xfe00, 0xfe00,},
> -		{ 0x0, 0x0, 0x0,},
> -		/* unsigned clamp */
> -		{ 0x40, 0x3ac, 0x40, 0x3c0, 0x40, 0x3c0,},
> -		{ 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,},
> -	};
> +	/* signed bias */
> +	{ 0xffc0, 0xfe00, 0xfe00,},
> +	{ 0x0, 0x0, 0x0,},
> +	/* unsigned clamp */
> +	{ 0x40, 0x3ac, 0x40, 0x3c0, 0x40, 0x3c0,},
> +	{ 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,},
> +};
> +
> +static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct dpu_plane
> *pdpu, const struct dpu_format *fmt)
> +{
> +	const struct dpu_csc_cfg *csc_ptr;
> 
>  	if (!pdpu) {
>  		DPU_ERROR("invalid plane\n");
> -		return;
> +		return NULL;
>  	}
> 
> +	if (!DPU_FORMAT_IS_YUV(fmt))
> +		return NULL;
> +
>  	if (BIT(DPU_SSPP_CSC_10BIT) & pdpu->features)
> -		pdpu->csc_ptr = (struct dpu_csc_cfg *)&dpu_csc10_YUV2RGB_601L;
> +		csc_ptr = &dpu_csc10_YUV2RGB_601L;
>  	else
> -		pdpu->csc_ptr = (struct dpu_csc_cfg *)&dpu_csc_YUV2RGB_601L;
> +		csc_ptr = &dpu_csc_YUV2RGB_601L;
> 
>  	DPU_DEBUG_PLANE(pdpu, "using 0x%X 0x%X 0x%X...\n",
> -			pdpu->csc_ptr->csc_mv[0],
> -			pdpu->csc_ptr->csc_mv[1],
> -			pdpu->csc_ptr->csc_mv[2]);
> +			csc_ptr->csc_mv[0],
> +			csc_ptr->csc_mv[1],
> +			csc_ptr->csc_mv[2]);
> +
> +	return csc_ptr;
>  }
> 
>  static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu,
> @@ -1070,8 +1075,13 @@ void dpu_plane_flush(struct drm_plane *plane)
>  	else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
>  		/* force 100% alpha */
>  		_dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
> -	else if (pdpu->pipe_hw && pdpu->csc_ptr && 
> pdpu->pipe_hw->ops.setup_csc)
> -		pdpu->pipe_hw->ops.setup_csc(pdpu->pipe_hw, pdpu->csc_ptr);
> +	else if (pdpu->pipe_hw && pdpu->pipe_hw->ops.setup_csc) {
> +		const struct dpu_format *fmt =
> to_dpu_format(msm_framebuffer_format(plane->state->fb));
> +		const struct dpu_csc_cfg *csc_ptr = _dpu_plane_get_csc(pdpu, fmt);
> +
> +		if (csc_ptr)
> +			pdpu->pipe_hw->ops.setup_csc(pdpu->pipe_hw, csc_ptr);
> +	}
> 
>  	/* flag h/w flush complete */
>  	if (plane->state)
> @@ -1187,12 +1197,6 @@ static void dpu_plane_sspp_atomic_update(struct
> drm_plane *plane)
> 
>  			pdpu->pipe_hw->ops.setup_cdp(pdpu->pipe_hw, cdp_cfg);
>  		}
> -
> -		/* update csc */
> -		if (DPU_FORMAT_IS_YUV(fmt))
> -			_dpu_plane_setup_csc(pdpu);
> -		else
> -			pdpu->csc_ptr = 0;
>  	}
> 
>  	_dpu_plane_set_qos_lut(plane, fb, &pipe_cfg);

  reply	other threads:[~2021-10-21 23:17 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30 13:59 [PATCH 00/11] drm/msm/dpu: cleanup plane state Dmitry Baryshkov
2021-09-30 13:59 ` [PATCH 01/11] drm/msm/dpu: move LUT levels out of QOS config Dmitry Baryshkov
2021-10-21 22:22   ` [Freedreno] " abhinavk
2021-09-30 13:59 ` [PATCH 02/11] drm/msm/dpu: remove pipe_qos_cfg from struct dpu_plane Dmitry Baryshkov
2021-10-21 22:24   ` [Freedreno] " abhinavk
2021-09-30 13:59 ` [PATCH 03/11] drm/msm/dpu: drop pipe_name " Dmitry Baryshkov
2021-10-21 22:25   ` [Freedreno] " abhinavk
2021-09-30 13:59 ` [PATCH 04/11] drm/msm/dpu: remove stage_cfg from struct dpu_crtc Dmitry Baryshkov
2021-10-21 22:30   ` [Freedreno] " abhinavk
2021-09-30 13:59 ` [PATCH 05/11] drm/msm/dpu: move dpu_hw_pipe_cfg out of struct dpu_plane Dmitry Baryshkov
2021-10-21 22:35   ` [Freedreno] " abhinavk
2021-09-30 13:59 ` [PATCH 06/11] drm/msm/dpu: drop scaler config from plane state Dmitry Baryshkov
2021-10-21 22:41   ` [Freedreno] " abhinavk
2021-09-30 13:59 ` [PATCH 07/11] drm/msm/dpu: drop dpu_csc_cfg from dpu_plane Dmitry Baryshkov
2021-10-21 23:17   ` abhinavk [this message]
2021-09-30 13:59 ` [PATCH 08/11] drm/msm/dpu: remove dpu_hw_pipe_cdp_cfg " Dmitry Baryshkov
2021-10-21 23:19   ` [Freedreno] " abhinavk
2021-09-30 14:00 ` [PATCH 09/11] drm/msm/dpu: don't cache pipe->cap->features in dpu_plane Dmitry Baryshkov
2021-10-21 23:24   ` [Freedreno] " abhinavk
2021-09-30 14:00 ` [PATCH 10/11] drm/msm/dpu: don't cache pipe->cap->sblk " Dmitry Baryshkov
2021-10-21 23:29   ` [Freedreno] " abhinavk
2021-09-30 14:00 ` [PATCH 11/11] drm/msm/dpu: rip out debugfs support from dpu_plane Dmitry Baryshkov
2021-10-21 23:53   ` [Freedreno] " abhinavk
2021-10-22 11:35     ` Dmitry Baryshkov
2021-11-18 21:43       ` Rob Clark
2021-11-18 21:43         ` Rob Clark

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=afe17ee7977bc967ddc0b35253d4e8f0@codeaurora.org \
    --to=abhinavk@codeaurora.org \
    --cc=airlied@linux.ie \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=jonathan@marek.ca \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=robdclark@gmail.com \
    --cc=sboyd@kernel.org \
    --cc=sean@poorly.run \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.