Re: [PATCH v3 4/7] drm/msm/dpu: add PINGPONG_NONE to disconnect DSC from PINGPONG

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

 




On 5/3/2023 1:03 AM, Marijn Suijten wrote:
On 2023-05-02 14:02:59, Kuogee Hsieh wrote:
During DSC setup, the crossbar mux need to be programmed to engage
DSC to specified PINGPONG. Hence during tear down, the crossbar mux
need to be reset to disengage DSC from PINGPONG. This patch add
PINGPONG_NONE to serve as disable to reset crossbar mux.
This patch doesn't *just add* PINGPONG_NONE to reset the crossbar; that
functionality was already available thanks to a `bool enable` function
parameter.  Instead it should explain why you think PINGPONG_NONE is
more convenient than passing a bool that warrants this replacement.
(Hint: I think because you don't have a hw_pp->idx available in the
  teardown path, and/or its value is not relevant for the disable case
  anyway.)

In addition I don't see this series use PINGPONG_NONE anywhere yet: will
that be added in the DSC 1.2 series for DP (to support hotplug)?

PINGPONG_NONE will be used to tear down DSC data path later at DP DSC patch series.

Current it is not used because DSI does not do tear down.


Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c  | 7 +++----
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h  | 1 -
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 ++-
  4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1dc5dbe..d9ad334 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1839,7 +1839,7 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
  		hw_pp->ops.setup_dsc(hw_pp);
if (hw_dsc->ops.dsc_bind_pingpong_blk)
-		hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, true, hw_pp->idx);
+		hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, hw_pp->idx);
if (hw_pp->ops.enable_dsc)
  		hw_pp->ops.enable_dsc(hw_pp);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
index 4a6bbcc..3e68d47 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -157,7 +157,6 @@ static void dpu_hw_dsc_config_thresh(struct dpu_hw_dsc *hw_dsc,
static void dpu_hw_dsc_bind_pingpong_blk(
  		struct dpu_hw_dsc *hw_dsc,
-		bool enable,
  		const enum dpu_pingpong pp)
  {
  	struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
@@ -166,13 +165,13 @@ static void dpu_hw_dsc_bind_pingpong_blk(
dsc_ctl_offset = DSC_CTL(hw_dsc->idx); - if (enable)
+	if (pp)
  		mux_cfg = (pp - PINGPONG_0) & 0x7;
DRM_DEBUG_KMS("%s dsc:%d %s pp:%d\n",
-			enable ? "Binding" : "Unbinding",
+			pp ? "Binding" : "Unbinding",
  			hw_dsc->idx - DSC_0,
-			enable ? "to" : "from",
+			pp ? "to" : "from",
  			pp - PINGPONG_0);
PINGPONG_NONE - PINGPONG_0 = -1, so this whole debug log likely needs to
be rewritten for the disable case as we don't know what PINGPONG it is
being unbound from.  How about:

	if (pp)
		DRM_DEBUG_KMS("Binding dsc:%d to pp:%d\n",
				hw_dsc->idx - DSC_0,
				pp - PINGPONG_0);
	else
		DRM_DEBUG_KMS("Unbinding dsc:%d from any pp\n",
				hw_dsc->idx - DSC_0);

- Marijn

DPU_REG_WRITE(c, dsc_ctl_offset, mux_cfg);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
index 287ec5f..138080a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -44,7 +44,6 @@ struct dpu_hw_dsc_ops {
  				  struct drm_dsc_config *dsc);
void (*dsc_bind_pingpong_blk)(struct dpu_hw_dsc *hw_dsc,
-				  bool enable,
  				  enum dpu_pingpong pp);
  };
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 2d9192a..56826a9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -191,7 +191,8 @@ enum dpu_dsc {
  };
enum dpu_pingpong {
-	PINGPONG_0 = 1,
+	PINGPONG_NONE,
+	PINGPONG_0,
  	PINGPONG_1,
  	PINGPONG_2,
  	PINGPONG_3,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project




[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