Re: [PATCH] drm/msm/dpu: correct clk bit for WB2 block

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

 





On 11/6/2023 2:11 PM, Dmitry Baryshkov wrote:
On Mon, 6 Nov 2023 at 20:39, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:

Sorry for the delay in getting back on this. There was quite a bit of
history digging I had to do myself to give a certain response.


On 10/9/2023 10:11 AM, Dmitry Baryshkov wrote:
On sc7280 there are two clk bits for WB2: control and status. While
programming the VBIF params of WB, the driver should be toggling the
former bit, while the sc7280_mdp struct lists the latter one.


No, this is not correct. Both are control bits. But for the context of
where this is being used today, that is setting the VBIF OT limit, we
should be using the VBIF_CLI one. So the below change itself is correct
but not the commit text.

Maybe you can update dt bindings for the SDE driver? Because they
clearly speak about the control and status bits.


There is nothing to update here if we both are referring to the same entries in the dt bindings.

qcom,sde-wb-clk-status = <0x3bc 20>;

the clk status is indeed bit 20 of 0x3bc.

What we have before your patch was bit 24 of 0x3b8 which was indeed clk_ctl bit for wb2. But the only issue was it was not the vbif_cli one.

So we are talking about two different registers?


We need to make the same change on sm8250 WB2 as well as this register
is present there too. In fact, anything >=msm8994 for setting VBIF OT
for WB2 we should be using VBIF_CLI bits of register MDP_CLK_CTRL2
(offset 0x2bc).

For anything >=sm8550, we need to use WB_2_CLK_CTRL present within the
WB block and not the one in the top.

Hence for this change, we can do below:

-> update the commit text to indicate both are control bits but for the
vbif ot context we should using the corrected one
-> if you can also add sm8250 in the same change i can ack it and pick it up

Have you re-validated WB with this change? If not, let me know I shall
while picking this up for -fixes.

No, I haven't validated this on sc7280. I'll try this on sm8250 and
then I'll send v2.


Correct that to ensure proper programming sequence for WB2 on sc7280.

Fixes: 3ce166380567 ("drm/msm/dpu: add writeback support for sc7280")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
index 3b5061c4402a..9195cb996f44 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
@@ -25,7 +25,7 @@ static const struct dpu_mdp_cfg sc7280_mdp = {
               [DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8 },
               [DPU_CLK_CTRL_DMA1] = { .reg_off = 0x2b4, .bit_off = 8 },
               [DPU_CLK_CTRL_DMA2] = { .reg_off = 0x2c4, .bit_off = 8 },
-             [DPU_CLK_CTRL_WB2] = { .reg_off = 0x3b8, .bit_off = 24 },
+             [DPU_CLK_CTRL_WB2] = { .reg_off = 0x2bc, .bit_off = 16 },
       },
   };








[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