Re: [PATCH v4 8/9] drm/msm/dpu: merge DPU_SSPP_SCALER_QSEED3, QSEED3LITE, QSEED4

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/11/2023 2:45 PM, Dmitry Baryshkov wrote:
Three different features, DPU_SSPP_SCALER_QSEED3, QSEED3LITE and QSEED4
are all related to different versions of the same HW scaling block.
Corresponding driver parts use scaler_blk.version to identify the
correct way to program the hardware. In order to simplify the driver
codepath, merge these three feature bits.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>

I am okay with some parts of this change but not all.

Please see below.

---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +-----
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c    | 9 ++-------
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h    | 4 +---
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c      | 3 +--
  5 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index b37b4076e53a..67d66319a825 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -31,10 +31,10 @@
  	(VIG_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
#define VIG_SC7180_MASK \
-	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4))
+	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3))
#define VIG_SM6125_MASK \
-	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE))
+	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3))

This is like a half-n-half solution. This is telling that SC7180 and SM6125 have a scaler blk version of 3.1 but are still qseed3. That gives a misleading picture.



  #define VIG_SC7180_MASK_SDMA \
  	(VIG_SC7180_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index fc5027b0123a..ba262b3f0bdc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -51,9 +51,7 @@ enum {
  /**
   * SSPP sub-blocks/features
   * @DPU_SSPP_SCALER_QSEED2,  QSEED2 algorithm support
- * @DPU_SSPP_SCALER_QSEED3,  QSEED3 alogorithm support
- * @DPU_SSPP_SCALER_QSEED3LITE,  QSEED3 Lite alogorithm support
- * @DPU_SSPP_SCALER_QSEED4,  QSEED4 algorithm support
+ * @DPU_SSPP_SCALER_QSEED3,  QSEED3 alogorithm support (also QSEED3LITE and QSEED4)
   * @DPU_SSPP_SCALER_RGB,     RGB Scaler, supported by RGB pipes
   * @DPU_SSPP_CSC,            Support of Color space converion
   * @DPU_SSPP_CSC_10BIT,      Support of 10-bit Color space conversion
@@ -72,8 +70,6 @@ enum {
  enum {
  	DPU_SSPP_SCALER_QSEED2 = 0x1,
  	DPU_SSPP_SCALER_QSEED3,
-	DPU_SSPP_SCALER_QSEED3LITE,
-	DPU_SSPP_SCALER_QSEED4,
  	DPU_SSPP_SCALER_RGB,
  	DPU_SSPP_CSC,
  	DPU_SSPP_CSC_10BIT,
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 7e9c87088e17..d1b70cf72eef 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -594,9 +594,7 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
  		test_bit(DPU_SSPP_SMART_DMA_V2, &c->cap->features))
  		c->ops.setup_multirect = dpu_hw_sspp_setup_multirect;
- if (test_bit(DPU_SSPP_SCALER_QSEED3, &features) ||
-			test_bit(DPU_SSPP_SCALER_QSEED3LITE, &features) ||
-			test_bit(DPU_SSPP_SCALER_QSEED4, &features))
+	if (test_bit(DPU_SSPP_SCALER_QSEED3, &features))
  		c->ops.setup_scaler = _dpu_hw_sspp_setup_scaler3;
any reason we cannot replace this with sblk->scaler_blk.version >= 1.2?

  	if (test_bit(DPU_SSPP_CDP, &features))
@@ -629,10 +627,7 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
  			cfg->len,
  			kms);
- if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
-			cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
-			cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
-			cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
+	if (sblk->scaler_blk.len)
  		dpu_debugfs_create_regset32("scaler_blk", 0400,
  				debugfs_root,
  				sblk->scaler_blk.base + cfg->base,

This part LGTM.

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 ca02f86c94ed..b157ed7da065 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -26,9 +26,7 @@ struct dpu_hw_sspp;
   */
  #define DPU_SSPP_SCALER (BIT(DPU_SSPP_SCALER_RGB) | \
  			 BIT(DPU_SSPP_SCALER_QSEED2) | \
-			 BIT(DPU_SSPP_SCALER_QSEED3) | \
-			 BIT(DPU_SSPP_SCALER_QSEED3LITE) | \
-			 BIT(DPU_SSPP_SCALER_QSEED4))
+			 BIT(DPU_SSPP_SCALER_QSEED3))

I am not seeing DPU_SSPP_SCALER_RGB being set by any chipset in the catalog? So we can drop it in a separate change and then just use sblk->scaler_blk.len in the place of this macro and drop this macro.

  /*
   * Define all CSC feature bits in catalog
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c2aaaded07ed..109355275ec5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -438,8 +438,7 @@ static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
  			scale_cfg->src_height[i] /= chroma_subsmpl_v;
  		}
- if (pipe_hw->cap->features &
-			BIT(DPU_SSPP_SCALER_QSEED4)) {
+		if (pipe_hw->cap->sblk->scaler_blk.version >= 0x3000) {
  			scale_cfg->preload_x[i] = DPU_QSEED4_DEFAULT_PRELOAD_H;
  			scale_cfg->preload_y[i] = DPU_QSEED4_DEFAULT_PRELOAD_V;
  		} else {

This part LGTM.



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux