All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Paul <sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
To: Bruce Wang <bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	abhinavk-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH v2 1/7] drm/msm/dpu: Remove unneeded checks in dpu_plane.c
Date: Fri, 21 Sep 2018 11:43:38 -0400	[thread overview]
Message-ID: <20180921154338.GG72545@art_vandelay> (raw)
In-Reply-To: <20180920164924.225847-2-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

On Thu, Sep 20, 2018 at 12:49:18PM -0400, Bruce Wang wrote:
> Removes some checks from dpu_plane.c that will never result in an error.
> Subsequent variable assignments become part of the initialization wherever
> possible. Unused variables are removed.
> 
> Signed-off-by: Bruce Wang <bzwang@chromium.org>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 190 ++--------------------
>  1 file changed, 17 insertions(+), 173 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 1ce76460d710..99887c804e4e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -123,13 +123,8 @@ struct dpu_plane {
>  
>  static struct dpu_kms *_dpu_plane_get_kms(struct drm_plane *plane)
>  {
> -	struct msm_drm_private *priv;
> +	struct msm_drm_private *priv = plane->dev->dev_private;
>  
> -	if (!plane || !plane->dev)
> -		return NULL;
> -	priv = plane->dev->dev_private;
> -	if (!priv)
> -		return NULL;
>  	return to_dpu_kms(priv->kms);
>  }
>  
> @@ -148,7 +143,7 @@ static inline int _dpu_plane_calc_fill_level(struct drm_plane *plane,
>  	u32 fixed_buff_size;
>  	u32 total_fl;
>  
> -	if (!plane || !fmt || !plane->state || !src_width || !fmt->bpp) {
> +	if (!fmt || !plane->state || !src_width || !fmt->bpp) {
>  		DPU_ERROR("invalid arguments\n");
>  		return 0;
>  	}
> @@ -229,26 +224,11 @@ static u64 _dpu_plane_get_qos_lut(const struct dpu_qos_lut_tbl *tbl,
>  static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
>  		struct drm_framebuffer *fb)
>  {
> -	struct dpu_plane *pdpu;
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
>  	const struct dpu_format *fmt = NULL;
>  	u64 qos_lut;
>  	u32 total_fl = 0, lut_usage;
>  
> -	if (!plane || !fb) {
> -		DPU_ERROR("invalid arguments plane %d fb %d\n",
> -				plane != 0, fb != 0);
> -		return;
> -	}
> -
> -	pdpu = to_dpu_plane(plane);
> -
> -	if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
> -		DPU_ERROR("invalid arguments\n");
> -		return;
> -	} else if (!pdpu->pipe_hw->ops.setup_creq_lut) {
> -		return;
> -	}
> -
>  	if (!pdpu->is_rt_pipe) {
>  		lut_usage = DPU_QOS_LUT_USAGE_NRT;
>  	} else {
> @@ -290,24 +270,10 @@ static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
>  static void _dpu_plane_set_danger_lut(struct drm_plane *plane,
>  		struct drm_framebuffer *fb)
>  {
> -	struct dpu_plane *pdpu;
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
>  	const struct dpu_format *fmt = NULL;
>  	u32 danger_lut, safe_lut;
>  
> -	if (!plane || !fb) {
> -		DPU_ERROR("invalid arguments\n");
> -		return;
> -	}
> -
> -	pdpu = to_dpu_plane(plane);
> -
> -	if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
> -		DPU_ERROR("invalid arguments\n");
> -		return;
> -	} else if (!pdpu->pipe_hw->ops.setup_danger_safe_lut) {
> -		return;
> -	}
> -
>  	if (!pdpu->is_rt_pipe) {
>  		danger_lut = pdpu->catalog->perf.danger_lut_tbl
>  				[DPU_QOS_LUT_USAGE_NRT];
> @@ -361,21 +327,7 @@ static void _dpu_plane_set_danger_lut(struct drm_plane *plane,
>  static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
>  	bool enable, u32 flags)
>  {
> -	struct dpu_plane *pdpu;
> -
> -	if (!plane) {
> -		DPU_ERROR("invalid arguments\n");
> -		return;
> -	}
> -
> -	pdpu = to_dpu_plane(plane);
> -
> -	if (!pdpu->pipe_hw || !pdpu->pipe_sblk) {
> -		DPU_ERROR("invalid arguments\n");
> -		return;
> -	} else if (!pdpu->pipe_hw->ops.setup_qos_ctrl) {
> -		return;
> -	}
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
>  
>  	if (flags & DPU_PLANE_QOS_VBLANK_CTRL) {
>  		pdpu->pipe_qos_cfg.creq_vblank = pdpu->pipe_sblk->creq_vblank;
> @@ -450,29 +402,10 @@ int dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable)
>  static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>  		struct drm_crtc *crtc)
>  {
> -	struct dpu_plane *pdpu;
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
>  	struct dpu_vbif_set_ot_params ot_params;
> -	struct msm_drm_private *priv;
> -	struct dpu_kms *dpu_kms;
> -
> -	if (!plane || !plane->dev || !crtc) {
> -		DPU_ERROR("invalid arguments plane %d crtc %d\n",
> -				plane != 0, crtc != 0);
> -		return;
> -	}
> -
> -	priv = plane->dev->dev_private;
> -	if (!priv || !priv->kms) {
> -		DPU_ERROR("invalid KMS reference\n");
> -		return;
> -	}
> -
> -	dpu_kms = to_dpu_kms(priv->kms);
> -	pdpu = to_dpu_plane(plane);
> -	if (!pdpu->pipe_hw) {
> -		DPU_ERROR("invalid pipe reference\n");
> -		return;
> -	}
> +	struct msm_drm_private *priv = plane->dev->dev_private;
> +	struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
>  
>  	memset(&ot_params, 0, sizeof(ot_params));
>  	ot_params.xin_id = pdpu->pipe_hw->cap->xin_id;
> @@ -494,28 +427,10 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>   */
>  static void _dpu_plane_set_qos_remap(struct drm_plane *plane)
>  {
> -	struct dpu_plane *pdpu;
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
>  	struct dpu_vbif_set_qos_params qos_params;
> -	struct msm_drm_private *priv;
> -	struct dpu_kms *dpu_kms;
> -
> -	if (!plane || !plane->dev) {
> -		DPU_ERROR("invalid arguments\n");
> -		return;
> -	}
> -
> -	priv = plane->dev->dev_private;
> -	if (!priv || !priv->kms) {
> -		DPU_ERROR("invalid KMS reference\n");
> -		return;
> -	}
> -
> -	dpu_kms = to_dpu_kms(priv->kms);
> -	pdpu = to_dpu_plane(plane);
> -	if (!pdpu->pipe_hw) {
> -		DPU_ERROR("invalid pipe reference\n");
> -		return;
> -	}
> +	struct msm_drm_private *priv = plane->dev->dev_private;
> +	struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
>  
>  	memset(&qos_params, 0, sizeof(qos_params));
>  	qos_params.vbif_idx = VBIF_RT;
> @@ -549,10 +464,6 @@ static int _dpu_plane_get_aspace(
>  	}
>  
>  	kms = _dpu_plane_get_kms(&pdpu->base);
> -	if (!kms) {
> -		DPU_ERROR("invalid kms\n");
> -		return -EINVAL;
> -	}
>  
>  	*aspace = kms->base.aspace;
>  
> @@ -576,10 +487,6 @@ static inline void _dpu_plane_set_scanout(struct drm_plane *plane,
>  	}
>  
>  	pdpu = to_dpu_plane(plane);
> -	if (!pdpu->pipe_hw) {
> -		DPU_ERROR_PLANE(pdpu, "invalid pipe_hw\n");
> -		return;
> -	}
>  
>  	ret = _dpu_plane_get_aspace(pdpu, pstate, &aspace);
>  	if (ret) {
> @@ -610,15 +517,6 @@ static void _dpu_plane_setup_scaler3(struct dpu_plane *pdpu,
>  {
>  	uint32_t i;
>  
> -	if (!pdpu || !pstate || !scale_cfg || !fmt || !chroma_subsmpl_h ||
> -			!chroma_subsmpl_v) {
> -		DPU_ERROR(
> -			"pdpu %d pstate %d scale_cfg %d fmt %d smp_h %d smp_v %d\n",
> -			!!pdpu, !!pstate, !!scale_cfg, !!fmt, chroma_subsmpl_h,
> -			chroma_subsmpl_v);
> -		return;
> -	}
> -
>  	memset(scale_cfg, 0, sizeof(*scale_cfg));
>  	memset(&pstate->pixel_ext, 0, sizeof(struct dpu_hw_pixel_ext));
>  
> @@ -722,17 +620,8 @@ static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu,
>  		struct dpu_plane_state *pstate,
>  		const struct dpu_format *fmt, bool color_fill)
>  {
> -	struct dpu_hw_pixel_ext *pe;
>  	uint32_t chroma_subsmpl_h, chroma_subsmpl_v;
>  
> -	if (!pdpu || !fmt || !pstate) {
> -		DPU_ERROR("invalid arg(s), plane %d fmt %d state %d\n",
> -				pdpu != 0, fmt != 0, pstate != 0);
> -		return;
> -	}
> -
> -	pe = &pstate->pixel_ext;
> -
>  	/* don't chroma subsample if decimating */
>  	chroma_subsmpl_h =
>  		drm_format_horz_chroma_subsampling(fmt->base.pixel_format);
> @@ -760,21 +649,8 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
>  		uint32_t color, uint32_t alpha)
>  {
>  	const struct dpu_format *fmt;
> -	const struct drm_plane *plane;
> -	struct dpu_plane_state *pstate;
> -
> -	if (!pdpu || !pdpu->base.state) {
> -		DPU_ERROR("invalid plane\n");
> -		return -EINVAL;
> -	}
> -
> -	if (!pdpu->pipe_hw) {
> -		DPU_ERROR_PLANE(pdpu, "invalid plane h/w pointer\n");
> -		return -EINVAL;
> -	}
> -
> -	plane = &pdpu->base;
> -	pstate = to_dpu_plane_state(plane->state);
> +	const struct drm_plane *plane = &pdpu->base;
> +	struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
>  
>  	DPU_DEBUG_PLANE(pdpu, "\n");
>  
> @@ -825,12 +701,7 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
>  
>  void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state)
>  {
> -	struct dpu_plane_state *pstate;
> -
> -	if (!drm_state)
> -		return;
> -
> -	pstate = to_dpu_plane_state(drm_state);
> +	struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
>  
>  	pstate->multirect_index = DPU_SSPP_RECT_SOLO;
>  	pstate->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> @@ -961,15 +832,6 @@ int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
>  void dpu_plane_get_ctl_flush(struct drm_plane *plane, struct dpu_hw_ctl *ctl,
>  		u32 *flush_sspp)
>  {
> -	struct dpu_plane_state *pstate;
> -
> -	if (!plane || !flush_sspp) {
> -		DPU_ERROR("invalid parameters\n");
> -		return;
> -	}
> -
> -	pstate = to_dpu_plane_state(plane->state);
> -
>  	*flush_sspp = ctl->ops.get_bitmask_sspp(ctl, dpu_plane_pipe(plane));
>  }
>  
> @@ -1389,8 +1251,7 @@ static void dpu_plane_destroy(struct drm_plane *plane)
>  		/* this will destroy the states as well */
>  		drm_plane_cleanup(plane);
>  
> -		if (pdpu->pipe_hw)
> -			dpu_hw_sspp_destroy(pdpu->pipe_hw);
> +		dpu_hw_sspp_destroy(pdpu->pipe_hw);
>  
>  		kfree(pdpu);
>  	}
> @@ -1737,28 +1598,11 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>  	struct drm_plane *plane = NULL, *master_plane = NULL;
>  	const struct dpu_format_extended *format_list;
>  	struct dpu_plane *pdpu;
> -	struct msm_drm_private *priv;
> -	struct dpu_kms *kms;
> +	struct msm_drm_private *priv = dev->dev_private;
> +	struct dpu_kms *kms = to_dpu_kms(priv->kms);
>  	int zpos_max = DPU_ZPOS_MAX;
>  	int ret = -EINVAL;
>  
> -	if (!dev) {
> -		DPU_ERROR("[%u]device is NULL\n", pipe);
> -		goto exit;
> -	}
> -
> -	priv = dev->dev_private;
> -	if (!priv) {
> -		DPU_ERROR("[%u]private data is NULL\n", pipe);
> -		goto exit;
> -	}
> -
> -	if (!priv->kms) {
> -		DPU_ERROR("[%u]invalid KMS reference\n", pipe);
> -		goto exit;
> -	}
> -	kms = to_dpu_kms(priv->kms);
> -
>  	if (!kms->catalog) {
>  		DPU_ERROR("[%u]invalid catalog reference\n", pipe);
>  		goto exit;
> -- 
> 2.19.0.444.g18242da7ef-goog
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

  parent reply	other threads:[~2018-09-21 15:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20 16:49 [PATCH v2 0/7] drm/msm/dpu: Clean up dpu code Bruce Wang
     [not found] ` <20180920164924.225847-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-20 16:49   ` [PATCH v2 1/7] drm/msm/dpu: Remove unneeded checks in dpu_plane.c Bruce Wang
     [not found]     ` <20180920164924.225847-2-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-20 17:29       ` Jordan Crouse
2018-09-21 15:43       ` Sean Paul [this message]
2018-09-20 16:49   ` [PATCH v2 2/7] drm/msm/dpu: Clean up plane atomic disable/update Bruce Wang
     [not found]     ` <20180920164924.225847-3-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-21 15:44       ` Sean Paul
2018-09-20 16:49   ` [PATCH v2 3/7] drm/msm/dpu: Remove unneeded checks in dpu_crtc.c Bruce Wang
     [not found]     ` <20180920164924.225847-4-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-21 15:49       ` Sean Paul
2018-09-20 16:49   ` [PATCH v2 4/7] drm/msm/dpu: Change _dpu_crtc_power_enable to void Bruce Wang
     [not found]     ` <20180920164924.225847-5-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-20 17:36       ` Jordan Crouse
     [not found]         ` <20180920173634.GD809-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org>
2018-09-21 15:51           ` Sean Paul
2018-09-24 16:00             ` Bruce Wang
2018-09-20 16:49   ` [PATCH v2 5/7] drm/msm/dpu: Change _dpu_crtc_vblank_enable_no_lock " Bruce Wang
     [not found]     ` <20180920164924.225847-6-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-21 15:51       ` Sean Paul
2018-09-20 16:49   ` [PATCH v2 6/7] drm/msm/dpu: Make dpu_plane_danger_signal_ctrl void Bruce Wang
     [not found]     ` <20180920164924.225847-7-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-20 17:38       ` Jordan Crouse
     [not found]         ` <20180920173853.GE809-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org>
2018-09-21 15:52           ` Sean Paul
2018-09-20 16:49   ` [PATCH v2 7/7] drm/msm/dpu: Make _dpu_plane_get_aspace void Bruce Wang
     [not found]     ` <20180920164924.225847-8-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-21 15:55       ` Sean Paul

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=20180921154338.GG72545@art_vandelay \
    --to=sean-p7ytbzm4h96eqtr555yldq@public.gmane.org \
    --cc=abhinavk-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    /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.