Re: [PATCH] drm/msm/dpu: always program dsc active bits

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

 



On 12/04/2023 02:32, Abhinav Kumar wrote:


On 4/11/2023 3:17 PM, Dmitry Baryshkov wrote:
On 12/04/2023 00:04, Kuogee Hsieh wrote:
In current code, the dsc active bits are set only if the cfg->dsc is set.
However, for displays which are hot-pluggable, there can be a use-case
of disconnecting a DSC supported sink and connecting a non-DSC sink.

For those cases we need to clear DSC active bits during teardown.

Please correct me if I'm wrong here, shouldn't we start using reset_intf_cfg() during teardown / unplug?


This is actually a good point. Since PSR landed this cycle, we are doing dpu_encoder_helper_phys_cleanup() even for video mode path,

22cb02bc96ff ("drm/msm/disp/dpu: reset the datapath after timing engine disable")

I was doing it only for writeback path as I had not validated video mode enough with the dpu_encoder_helper_phys_cleanup() API.

But looking closely, I think there is an issue with the flush logic in that API for video mode.

The reset API, calls a ctl->ops.trigger_flush(ctl); but its getting called after timing engine turns off today so this wont take any effect.

We need to improve that API and add the missing pieces for it to work correctly with video mode and re-validate the issue for which PSR made that change. So needs more work there.

This change works because the timing engine is enabled right after this call and will trigger the flush with it.

The only drawback of this change is DSC_ACTIVE will always get written to either with 0 or the right value but only once during enable.

I think this change is fine till we finish the rest of the pieces. We can add the if (cfg->dsc) back to this when we fix the reset_intf_cfg() to handle DSC and dpu_encoder_helper_phys_cleanup() to handle flush correctly.

I'd agree here. Then a FIXME comment would be nice.


I will take up that work.


Fixes: ede3c6bb00c ("drm/msm/disp/dpu1: Add DSC support in hw_ctl")
Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 +++----
  1 file changed, 3 insertions(+), 4 deletions(-)

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 bbdc95c..88e4efe 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -541,10 +541,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
      if (cfg->merge_3d)
          DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
                    BIT(cfg->merge_3d - MERGE_3D_0));
-    if (cfg->dsc) {
-        DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
-        DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
-    }
+
+    DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
+    DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
  }
  static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,


--
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux