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
