Re: [Freedreno] [PATCH v12 01/10] drm/msm/dpu: set DSC flush bit correctly at MDP CTL flush register

[Marijn Suijten]

On 2023-05-22 23:18:02, Dmitry Baryshkov wrote:
> On 22/05/2023 22:30, Kuogee Hsieh wrote:
> > DSC flush bit is the bit 22 of CTL flush register. BIT(22) is expected
> > to be written to CTL flush register to indicates that DSC is ready for
> > flush. However 0x22 (BIT(1) and BIT(5)) was written mistakenly at current
> 
> DSC_IDX is 22, not 0x22.
> 
> > implementation. Fix this problem by writing BIT(22) to CTL flush register
> > for DSC to work properly.

Perhaps this whole comment needs a rewrite anyway, instead of pointing
out grammar errors and confusion:

The DSC CTL_FLUSH register should be programmed with the 22th bit
(DSC_IDX) to flush the DSC hardware blocks, not the literal value of
22 (which corresponds to flushing VIG1, VIG2 and RGB1 instead).

Patch contents are good though, thanks!

- Marijn

> > Changes in V12:
> > -- split this patch out of "separate DSC flush update out of interface"
> > 
> > Fixes: 77f6da90487c ("drm/msm/disp/dpu1: Add DSC support in hw_ctl")
> > Signed-off-by: Kuogee Hsieh 
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > index 4f7cfa9..69d0ea2 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > @@ -525,7 +525,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl 
> > *ctx,
> > DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
> >   BIT(cfg->merge_3d - MERGE_3D_0));
> > if (cfg->dsc) {
> > -   DPU_REG_WRITE(>hw, CTL_FLUSH, DSC_IDX);
> > +   DPU_REG_WRITE(>hw, CTL_FLUSH, BIT(DSC_IDX));
> > DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
> > }
> >   }
> 
> -- 
> With best wishes
> Dmitry
> 

Re: [Freedreno] [PATCH v12 01/10] drm/msm/dpu: set DSC flush bit correctly at MDP CTL flush register

[Dmitry Baryshkov]

On 22/05/2023 22:30, Kuogee Hsieh wrote:

DSC flush bit is the bit 22 of CTL flush register. BIT(22) is expected
to be written to CTL flush register to indicates that DSC is ready for
flush. However 0x22 (BIT(1) and BIT(5)) was written mistakenly at current


DSC_IDX is 22, not 0x22.


implementation. Fix this problem by writing BIT(22) to CTL flush register
for DSC to work properly.

Changes in V12:
-- split this patch out of "separate DSC flush update out of interface"

Fixes: 77f6da90487c ("drm/msm/disp/dpu1: Add DSC support in hw_ctl")
Signed-off-by: Kuogee Hsieh 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 4f7cfa9..69d0ea2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -525,7 +525,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
  BIT(cfg->merge_3d - MERGE_3D_0));
if (cfg->dsc) {
-   DPU_REG_WRITE(>hw, CTL_FLUSH, DSC_IDX);
+   DPU_REG_WRITE(>hw, CTL_FLUSH, BIT(DSC_IDX));
DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
}
  }


--
With best wishes
Dmitry

[Freedreno] [PATCH v12 01/10] drm/msm/dpu: set DSC flush bit correctly at MDP CTL flush register

[Kuogee Hsieh]

DSC flush bit is the bit 22 of CTL flush register. BIT(22) is expected
to be written to CTL flush register to indicates that DSC is ready for
flush. However 0x22 (BIT(1) and BIT(5)) was written mistakenly at current
implementation. Fix this problem by writing BIT(22) to CTL flush register
for DSC to work properly.

Changes in V12:
-- split this patch out of "separate DSC flush update out of interface"

Fixes: 77f6da90487c ("drm/msm/disp/dpu1: Add DSC support in hw_ctl")
Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 4f7cfa9..69d0ea2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -525,7 +525,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
  BIT(cfg->merge_3d - MERGE_3D_0));
if (cfg->dsc) {
-   DPU_REG_WRITE(>hw, CTL_FLUSH, DSC_IDX);
+   DPU_REG_WRITE(>hw, CTL_FLUSH, BIT(DSC_IDX));
DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
}
 }
-- 
2.7.4