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

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

 



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.

>
> 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 },
> >       },
> >   };
> >



-- 
With best wishes
Dmitry



[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