Re: [DPU PATCH 2/7] drm/msm/dpu: clean up dpu plane custom properties

[Jeykumar Sankaran <jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On 2018-06-04 12:53, Sean Paul wrote:
> On Wed, May 23, 2018 at 12:30:57PM -0700, Jeykumar Sankaran wrote:
>> This change removes all the dpu plane custom properties
>> and its handlers.
>> 
>> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
>> ---
>>  Makefile                                           |    2 +-
>>  drivers/gpu/drm/msm/Makefile                       |    8 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_ad4.h            |   99 --
>>  .../gpu/drm/msm/disp/dpu1/dpu_color_processing.c   | 1521
> --------------------
>>  .../gpu/drm/msm/disp/dpu1/dpu_color_processing.h   |  120 --
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c           |  148 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h           |    3 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |    2 -
>>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |    1 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ad4.c         | 1443
> -------------------
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     |   72 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |   89 --
>>  .../msm/disp/dpu1/dpu_hw_color_proc_common_v4.h    |   69 -
>>  .../gpu/drm/msm/disp/dpu1/dpu_hw_color_proc_v4.c   |  242 ----
>>  .../gpu/drm/msm/disp/dpu1/dpu_hw_color_proc_v4.h   |   40 -
>>  .../drm/msm/disp/dpu1/dpu_hw_color_processing.h    |   20 -
>>  .../msm/disp/dpu1/dpu_hw_color_processing_v1_7.c   |  565 --------
>>  .../msm/disp/dpu1/dpu_hw_color_processing_v1_7.h   |   92 --
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c         |   44 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h         |   15 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c        |  209 ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.h        |  220 ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c          |    1 +
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h        |   44 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c    |   68 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h    |    6 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_reg_dma_v1.c  |  757 ----------
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_reg_dma_v1.h  |   27 -
>>  .../msm/disp/dpu1/dpu_hw_reg_dma_v1_color_proc.c   |  943 
>> ------------
>>  .../msm/disp/dpu1/dpu_hw_reg_dma_v1_color_proc.h   |   75 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c        |  219 ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h        |   73 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c        |    1 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h        |  156 ++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |    3 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          | 1267
> +---------------
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h          |   31 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_reg_dma.c        |  139 --
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_reg_dma.h        |  310 ----
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             |  102 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h             |    2 -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_wb.c             |    2 -
>>  drivers/gpu/drm/msm/msm_drv.h                      |   28 -
>>  include/uapi/drm/dpu_drm.h                         |  187 ---
>>  include/uapi/drm/msm_drm.h                         |    1 -
>>  45 files changed, 277 insertions(+), 9189 deletions(-)
> 
> Doing all of this at once is really hard to review. I would have 
> preferred
> to
> review each feature removal in a separate patch. However, since this is
> just
> going to be squashed into the DPU megapatch anyways, I guess it's fine.
> 
Sure. I thought I was helping by squashing them beforehand.
Will take care by spliting them for review on future patches.
> I only paid close attention to the additions, there are some unrelated
> whitespace changes, but also meh on account of the squash (and non seem
> objectionable).
> 
>>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_ad4.h
>>  delete mode 100644 
>> drivers/gpu/drm/msm/disp/dpu1/dpu_color_processing.c
>>  delete mode 100644 
>> drivers/gpu/drm/msm/disp/dpu1/dpu_color_processing.h
>>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ad4.c
>>  delete mode 100644
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_color_proc_common_v4.h
>>  delete mode 100644 
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_color_proc_v4.c
>>  delete mode 100644 
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_color_proc_v4.h
>>  delete mode 100644
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_color_processing.h
>>  delete mode 100644
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_color_processing_v1_7.c
>>  delete mode 100644
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_color_processing_v1_7.h
>>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c
>>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.h
>>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_reg_dma_v1.c
>>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_reg_dma_v1.h
>>  delete mode 100644
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_reg_dma_v1_color_proc.c
>>  delete mode 100644
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_reg_dma_v1_color_proc.h
>>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_reg_dma.c
>>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_reg_dma.h
>> 
>> diff --git a/Makefile b/Makefile
>> index 3c00040..1f23c66 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -364,7 +364,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS)
>>  HOSTCC       = gcc
>>  HOSTCXX      = g++
>>  HOSTCFLAGS   := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \
>> -		-fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS)
>> +		-fomit-frame-pointer -std=gnu89 -Wmaybe-uninitialized
> $(HOST_LFS_CFLAGS)
> 
> What's up with the compiler flag warnings? Seems unrelated (and we 
> should
> probably remove all of them in a separate patch anyways).
> 
Unintentional. Will clean up in V2.
>>  HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS)
>>  HOSTLDFLAGS  := $(HOST_LFS_LDFLAGS)
>>  HOST_LOADLIBES := $(HOST_LFS_LIBS)
> 
> /snip
> 
>> index 48920b05..d439a9e 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -20,7 +20,6 @@
>>  #include <linux/sort.h>
>>  #include <linux/debugfs.h>
>>  #include <linux/ktime.h>
>> -#include <uapi/drm/dpu_drm.h>
>>  #include <drm/drm_mode.h>
>>  #include <drm/drm_crtc.h>
>>  #include <drm/drm_crtc_helper.h>
>> @@ -31,13 +30,18 @@
>>  #include "dpu_hw_ctl.h"
>>  #include "dpu_crtc.h"
>>  #include "dpu_plane.h"
>> -#include "dpu_color_processing.h"
>>  #include "dpu_encoder.h"
>>  #include "dpu_vbif.h"
>>  #include "dpu_power_handle.h"
>>  #include "dpu_core_perf.h"
>>  #include "dpu_trace.h"
>> 
>> +#define DPU_DRM_BLEND_OP_NOT_DEFINED    0
>> +#define DPU_DRM_BLEND_OP_OPAQUE         1
>> +#define DPU_DRM_BLEND_OP_PREMULTIPLIED  2
>> +#define DPU_DRM_BLEND_OP_COVERAGE       3
>> +#define DPU_DRM_BLEND_OP_MAX            4
>> +
>>  /* layer mixer index on dpu_crtc */
>>  #define LEFT_MIXER 0
>>  #define RIGHT_MIXER 1
>> @@ -590,7 +594,6 @@ static void dpu_crtc_destroy(struct drm_crtc 
>> *crtc)
>>  	if (dpu_crtc->blob_info)
>>  		drm_property_blob_put(dpu_crtc->blob_info);
>>  	msm_property_destroy(&dpu_crtc->property_info);
>> -	dpu_cp_crtc_destroy_properties(crtc);
>>  	_dpu_crtc_destroy_dest_scaler(dpu_crtc);
>> 
>>  	_dpu_crtc_deinit_events(dpu_crtc);
>> @@ -624,15 +627,11 @@ static void _dpu_crtc_setup_blend_cfg(struct
> dpu_crtc_mixer *mixer,
>>  	struct dpu_hw_mixer *lm = mixer->hw_lm;
>> 
>>  	/* default to opaque blending */
>> -	fg_alpha = dpu_plane_get_property(pstate, PLANE_PROP_ALPHA);
>> +	fg_alpha = 0XFF;
>>  	bg_alpha = 0xFF - fg_alpha;
> 
> This goes to 0 and the fg_alpha != 0xff checks are always false. So 
> let's
> clean
> the rest of the function to remove the dead code and variables.
> 
>> -	blend_op = DPU_BLEND_FG_ALPHA_FG_CONST |
> DPU_BLEND_BG_ALPHA_BG_CONST;
>> -	blend_type = dpu_plane_get_property(pstate, PLANE_PROP_BLEND_OP);
>> -
>> -	DPU_DEBUG("blend type:0x%x blend alpha:0x%x\n", blend_type,
> fg_alpha);
>> +	blend_type = DPU_DRM_BLEND_OP_OPAQUE;
>> 
>>  	switch (blend_type) {
>> -
>>  	case DPU_DRM_BLEND_OP_OPAQUE:
>>  		blend_op = DPU_BLEND_FG_ALPHA_FG_CONST |
>>  			DPU_BLEND_BG_ALPHA_BG_CONST;
>> 
> 
> 
> /snip

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno