Re: [Freedreno] [PATCH 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane

2018-09-19 Thread Sean Paul
On Wed, Sep 19, 2018 at 11:44:11AM -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.
> 
> Signed-off-by: Bruce Wang 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 53 +++
>  1 file changed, 6 insertions(+), 47 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..a8d00f57f06a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -229,19 +229,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) {

As we discussed, it looks like we can remove all of these too \o/

>   DPU_ERROR("invalid arguments\n");
>   return;
> @@ -290,17 +282,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) {

and here

>   DPU_ERROR("invalid arguments\n");
>   return;
> @@ -361,14 +346,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);
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
>  
>   if (!pdpu->pipe_hw || !pdpu->pipe_sblk) {

and here

>   DPU_ERROR("invalid arguments\n");
> @@ -452,16 +430,9 @@ static void _dpu_plane_set_ot_limit(struct drm_plane 
> *plane,
>  {
>   struct dpu_plane *pdpu;
>   struct dpu_vbif_set_ot_params ot_params;
> - struct msm_drm_private *priv;
> + struct msm_drm_private *priv = plane->dev->dev_private;
>   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) {

These are also guaranteed to be non-NULL (dev_private checked in msm_drv and kms
is checked in dpu_kms.c (or maybe msm_kms.c, but either way)).

>   DPU_ERROR("invalid KMS reference\n");
>   return;
> @@ -496,15 +467,9 @@ static void _dpu_plane_set_qos_remap(struct drm_plane 
> *plane)
>  {
>   struct dpu_plane *pdpu;
>   struct dpu_vbif_set_qos_params qos_params;
> - struct msm_drm_private *priv;
> + struct msm_drm_private *priv = plane->dev->dev_private;
>   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;
> @@ -1737,17 +1702,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 msm_drm_private *priv = dev->dev_private;
>   struct dpu_kms *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) {

So let's do another pass of all dev_private checks along with kms, 
pipe_hw/sblk/etc.

Sean

>   DPU_ERROR("[%u]private data is NULL\n", pipe);
>   goto exit;
> -- 
> 2.19.0.397.gdd90340f6a-goog
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[Freedreno] [PATCH 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane

2018-09-19 Thread Bruce Wang
Removes some checks from dpu_plane.c that will never result in an error.
Subsequent variable assignments become part of the initialization wherever
possible.

Signed-off-by: Bruce Wang 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 53 +++
 1 file changed, 6 insertions(+), 47 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..a8d00f57f06a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -229,19 +229,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;
@@ -290,17 +282,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;
@@ -361,14 +346,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);
+   struct dpu_plane *pdpu = to_dpu_plane(plane);
 
if (!pdpu->pipe_hw || !pdpu->pipe_sblk) {
DPU_ERROR("invalid arguments\n");
@@ -452,16 +430,9 @@ static void _dpu_plane_set_ot_limit(struct drm_plane 
*plane,
 {
struct dpu_plane *pdpu;
struct dpu_vbif_set_ot_params ot_params;
-   struct msm_drm_private *priv;
+   struct msm_drm_private *priv = plane->dev->dev_private;
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;
@@ -496,15 +467,9 @@ static void _dpu_plane_set_qos_remap(struct drm_plane 
*plane)
 {
struct dpu_plane *pdpu;
struct dpu_vbif_set_qos_params qos_params;
-   struct msm_drm_private *priv;
+   struct msm_drm_private *priv = plane->dev->dev_private;
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;
@@ -1737,17 +1702,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 msm_drm_private *priv = dev->dev_private;
struct dpu_kms *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;
-- 
2.19.0.397.gdd90340f6a-goog

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno