<div dir="ltr"><div style="font-size:large;color:rgb(68,68,68)">Hello members, </div><div style="font-size:large;color:rgb(68,68,68)">I'm Forcha Pearl, a final-year Computer Science postgraduate. </div><div style="font-size:large;color:rgb(68,68,68)">I'm good at Python, C++,C and JavaScript languages and have completed a few projects, excited to explore open source by contributing and learning. </div><div style="font-size:large;color:rgb(68,68,68)">Could anyone kindly assist me in making my initial contribution to <a href="http://X.ORG">X.ORG</a> foundation by suggesting beginner-friendly issues?</div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 7, 2023 at 12:49 AM <<a href="mailto:freedreno-request@lists.freedesktop.org">freedreno-request@lists.freedesktop.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Send Freedreno mailing list submissions to <a href="mailto:freedreno@lists.freedesktop.org" target="_blank">freedreno@lists.freedesktop.org</a> To subscribe or unsubscribe via the World Wide Web, visit <a href="https://lists.freedesktop.org/mailman/listinfo/freedreno" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/freedreno</a> or, via email, send a message with subject or body 'help' to <a href="mailto:freedreno-request@lists.freedesktop.org" target="_blank">freedreno-request@lists.freedesktop.org</a> You can reach the person managing the list at <a href="mailto:freedreno-owner@lists.freedesktop.org" target="_blank">freedreno-owner@lists.freedesktop.org</a> When replying, please edit your Subject line so it is more specific than "Re: Contents of Freedreno digest..." Today's Topics: 1. [PATCH v3 2/2] drm/msm/gem: Add metadata (Rob Clark) 2. Re: [PATCH] drm/msm/dpu: correct clk bit for WB2 block (Dmitry Baryshkov) 3. Re: [PATCH] drm/msm/dpu: correct clk bit for WB2 block (Abhinav Kumar) 4. Re: [PATCH] drm/msm/dpu: correct clk bit for WB2 block (Dmitry Baryshkov) ---------------------------------------------------------------------- Message: 1 Date: Mon, 6 Nov 2023 10:50:26 -0800 From: Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>> To: <a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a> Cc: <a href="mailto:freedreno@lists.freedesktop.org" target="_blank">freedreno@lists.freedesktop.org</a>, <a href="mailto:linux-arm-msm@vger.kernel.org" target="_blank">linux-arm-msm@vger.kernel.org</a>, Daniel Vetter <<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>>, Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>>, Dmitry Baryshkov <<a href="mailto:dmitry.baryshkov@linaro.org" target="_blank">dmitry.baryshkov@linaro.org</a>>, Rob Clark <<a href="mailto:robdclark@chromium.org" target="_blank">robdclark@chromium.org</a>>, Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>>, Abhinav Kumar <<a href="mailto:quic_abhinavk@quicinc.com" target="_blank">quic_abhinavk@quicinc.com</a>>, Sean Paul <sean@poorly.run>, Marijn Suijten <<a href="mailto:marijn.suijten@somainline.org" target="_blank">marijn.suijten@somainline.org</a>>, David Airlie <<a href="mailto:airlied@gmail.com" target="_blank">airlied@gmail.com</a>>, <a href="mailto:linux-kernel@vger.kernel.org" target="_blank">linux-kernel@vger.kernel.org</a> (open list) Subject: [Freedreno] [PATCH v3 2/2] drm/msm/gem: Add metadata Message-ID: <<a href="mailto:20231106185028.209462-3-robdclark@gmail.com" target="_blank">20231106185028.209462-3-robdclark@gmail.com</a>> From: Rob Clark <<a href="mailto:robdclark@chromium.org" target="_blank">robdclark@chromium.org</a>> The EXT_external_objects extension is a bit awkward as it doesn't pass explicit modifiers, leaving the importer to guess with incomplete information. In the case of vk (turnip) exporting and gl (freedreno) importing, the "OPTIMAL_TILING_EXT" layout depends on VkImageCreateInfo flags (among other things), which the importer does not know. Which unfortunately leaves us with the need for a metadata back-channel. The contents of the metadata are defined by userspace. The EXT_external_objects extension is only required to work between compatible versions of gl and vk drivers, as defined by device and driver UUIDs. v2: add missing metadata kfree v3: Rework to move copy_from/to_user out from under gem obj lock to avoid angering lockdep about deadlocks against fs-reclaim Signed-off-by: Rob Clark <<a href="mailto:robdclark@chromium.org" target="_blank">robdclark@chromium.org</a>> --- Note, I dropped Dmitry's r-b on this version because it was a bit of a re-write of the original patch. drivers/gpu/drm/msm/msm_drv.c | 92 ++++++++++++++++++++++++++++++++++- drivers/gpu/drm/msm/msm_gem.c | 1 + drivers/gpu/drm/msm/msm_gem.h | 4 ++ include/uapi/drm/msm_drm.h | 2 + 4 files changed, 98 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 781db689fb16..c05c27a70c34 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -49,9 +49,10 @@ * - 1.9.0 - Add MSM_SUBMIT_FENCE_SN_IN * - 1.10.0 - Add MSM_SUBMIT_BO_NO_IMPLICIT * - 1.11.0 - Add wait boost (MSM_WAIT_FENCE_BOOST, MSM_PREP_BOOST) + * - 1.12.0 - Add MSM_INFO_SET_METADATA and MSM_INFO_GET_METADATA */ #define MSM_VERSION_MAJOR 1 -#define MSM_VERSION_MINOR 11 +#define MSM_VERSION_MINOR 12 #define MSM_VERSION_PATCHLEVEL 0 static void msm_deinit_vram(struct drm_device *ddev); @@ -822,6 +823,85 @@ static int msm_ioctl_gem_info_set_iova(struct drm_device *dev, return msm_gem_set_iova(obj, ctx->aspace, iova); } +static int msm_ioctl_gem_info_set_metadata(struct drm_gem_object *obj, + __user void *metadata, + u32 metadata_size) +{ + struct msm_gem_object *msm_obj = to_msm_bo(obj); + void *buf; + int ret; + + /* Impose a moderate upper bound on metadata size: */ + if (metadata_size > 128) { + return -EOVERFLOW; + } + + /* Use a temporary buf to keep copy_from_user() outside of gem obj lock: */ + buf = memdup_user(metadata, metadata_size); + if (IS_ERR(buf)) + return PTR_ERR(buf); + + ret = msm_gem_lock_interruptible(obj); + if (ret) + goto out; + + msm_obj->metadata = + krealloc(msm_obj->metadata, metadata_size, GFP_KERNEL); + msm_obj->metadata_size = metadata_size; + memcpy(msm_obj->metadata, buf, metadata_size); + + msm_gem_unlock(obj); + +out: + kfree(buf); + + return ret; +} + +static int msm_ioctl_gem_info_get_metadata(struct drm_gem_object *obj, + __user void *metadata, + u32 *metadata_size) +{ + struct msm_gem_object *msm_obj = to_msm_bo(obj); + void *buf; + int ret, len; + + if (!metadata) { + /* + * Querying the size is inherently racey, but + * EXT_external_objects expects the app to confirm + * via device and driver UUIDs that the exporter and + * importer versions match. All we can do from the + * kernel side is check the length under obj lock + * when userspace tries to retrieve the metadata + */ + *metadata_size = msm_obj->metadata_size; + return 0; + } + + ret = msm_gem_lock_interruptible(obj); + if (ret) + return ret; + + /* Avoid copy_to_user() under gem obj lock: */ + len = msm_obj->metadata_size; + buf = kmemdup(msm_obj->metadata, len, GFP_KERNEL); + + msm_gem_unlock(obj); + + if (*metadata_size < len) { + ret = -ETOOSMALL; + } else if (copy_to_user(metadata, buf, len)) { + ret = -EFAULT; + } else { + *metadata_size = len; + } + + kfree(buf); + + return 0; +} + static int msm_ioctl_gem_info(struct drm_device *dev, void *data, struct drm_file *file) { @@ -844,6 +924,8 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, break; case MSM_INFO_SET_NAME: case MSM_INFO_GET_NAME: + case MSM_INFO_SET_METADATA: + case MSM_INFO_GET_METADATA: break; default: return -EINVAL; @@ -906,6 +988,14 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, ret = -EFAULT; } break; + case MSM_INFO_SET_METADATA: + ret = msm_ioctl_gem_info_set_metadata( + obj, u64_to_user_ptr(args->value), args->len); + break; + case MSM_INFO_GET_METADATA: + ret = msm_ioctl_gem_info_get_metadata( + obj, u64_to_user_ptr(args->value), &args->len); + break; } drm_gem_object_put(obj); diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 1113e6b2ec8e..175ee4ab8a6f 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -1058,6 +1058,7 @@ static void msm_gem_free_object(struct drm_gem_object *obj) drm_gem_object_release(obj); + kfree(msm_obj->metadata); kfree(msm_obj); } diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 7f34263048a3..8d414b072c29 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -109,6 +109,10 @@ struct msm_gem_object { char name[32]; /* Identifier to print for the debugfs files */ + /* userspace metadata backchannel */ + void *metadata; + u32 metadata_size; + /** * pin_count: Number of times the pages are pinned * diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h index 6c34272a13fd..6f2a7ad04aa4 100644 --- a/include/uapi/drm/msm_drm.h +++ b/include/uapi/drm/msm_drm.h @@ -139,6 +139,8 @@ struct drm_msm_gem_new { #define MSM_INFO_GET_NAME 0x03 /* get debug name, returned by pointer */ #define MSM_INFO_SET_IOVA 0x04 /* set the iova, passed by value */ #define MSM_INFO_GET_FLAGS 0x05 /* get the MSM_BO_x flags */ +#define MSM_INFO_SET_METADATA 0x06 /* set userspace metadata */ +#define MSM_INFO_GET_METADATA 0x07 /* get userspace metadata */ struct drm_msm_gem_info { __u32 handle; /* in */ -- 2.41.0 ------------------------------ Message: 2 Date: Tue, 7 Nov 2023 00:11:47 +0200 From: Dmitry Baryshkov <<a href="mailto:dmitry.baryshkov@linaro.org" target="_blank">dmitry.baryshkov@linaro.org</a>> To: Abhinav Kumar <<a href="mailto:quic_abhinavk@quicinc.com" target="_blank">quic_abhinavk@quicinc.com</a>> Cc: Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>>, Sean Paul <sean@poorly.run>, Marijn Suijten <<a href="mailto:marijn.suijten@somainline.org" target="_blank">marijn.suijten@somainline.org</a>>, Stephen Boyd <<a href="mailto:swboyd@chromium.org" target="_blank">swboyd@chromium.org</a>>, David Airlie <<a href="mailto:airlied@gmail.com" target="_blank">airlied@gmail.com</a>>, Daniel Vetter <<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>>, Bjorn Andersson <<a href="mailto:andersson@kernel.org" target="_blank">andersson@kernel.org</a>>, <a href="mailto:linux-arm-msm@vger.kernel.org" target="_blank">linux-arm-msm@vger.kernel.org</a>, <a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a>, <a href="mailto:freedreno@lists.freedesktop.org" target="_blank">freedreno@lists.freedesktop.org</a> Subject: Re: [Freedreno] [PATCH] drm/msm/dpu: correct clk bit for WB2 block Message-ID: <CAA8EJprpBy6UhtScRkFS24TgKevBOb9nVBFCqPhEof=-<a href="mailto:k58Mwg@mail.gmail.com" target="_blank">k58Mwg@mail.gmail.com</a>> Content-Type: text/plain; charset="UTF-8" On Mon, 6 Nov 2023 at 20:39, Abhinav Kumar <<a href="mailto:quic_abhinavk@quicinc.com" target="_blank">quic_abhinavk@quicinc.com</a>> 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 <<a href="mailto:dmitry.baryshkov@linaro.org" target="_blank">dmitry.baryshkov@linaro.org</a>> > > --- > > 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 ------------------------------ Message: 3 Date: Mon, 6 Nov 2023 15:30:32 -0800 From: Abhinav Kumar <<a href="mailto:quic_abhinavk@quicinc.com" target="_blank">quic_abhinavk@quicinc.com</a>> To: Dmitry Baryshkov <<a href="mailto:dmitry.baryshkov@linaro.org" target="_blank">dmitry.baryshkov@linaro.org</a>> Cc: Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>>, Sean Paul <sean@poorly.run>, Marijn Suijten <<a href="mailto:marijn.suijten@somainline.org" target="_blank">marijn.suijten@somainline.org</a>>, Stephen Boyd <<a href="mailto:swboyd@chromium.org" target="_blank">swboyd@chromium.org</a>>, David Airlie <<a href="mailto:airlied@gmail.com" target="_blank">airlied@gmail.com</a>>, Daniel Vetter <<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>>, Bjorn Andersson <<a href="mailto:andersson@kernel.org" target="_blank">andersson@kernel.org</a>>, <<a href="mailto:linux-arm-msm@vger.kernel.org" target="_blank">linux-arm-msm@vger.kernel.org</a>>, <<a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a>>, <<a href="mailto:freedreno@lists.freedesktop.org" target="_blank">freedreno@lists.freedesktop.org</a>> Subject: Re: [Freedreno] [PATCH] drm/msm/dpu: correct clk bit for WB2 block Message-ID: <<a href="mailto:1ac30bfd-86d9-8186-1aee-f201b8ff4370@quicinc.com" target="_blank">1ac30bfd-86d9-8186-1aee-f201b8ff4370@quicinc.com</a>> Content-Type: text/plain; charset="UTF-8"; format=flowed On 11/6/2023 2:11 PM, Dmitry Baryshkov wrote: > On Mon, 6 Nov 2023 at 20:39, Abhinav Kumar <<a href="mailto:quic_abhinavk@quicinc.com" target="_blank">quic_abhinavk@quicinc.com</a>> 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 <<a href="mailto:dmitry.baryshkov@linaro.org" target="_blank">dmitry.baryshkov@linaro.org</a>> >>> --- >>> 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 }, >>> }, >>> }; >>> > > > ------------------------------ Message: 4 Date: Tue, 7 Nov 2023 01:49:03 +0200 From: Dmitry Baryshkov <<a href="mailto:dmitry.baryshkov@linaro.org" target="_blank">dmitry.baryshkov@linaro.org</a>> To: Abhinav Kumar <<a href="mailto:quic_abhinavk@quicinc.com" target="_blank">quic_abhinavk@quicinc.com</a>> Cc: Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>>, Sean Paul <sean@poorly.run>, Marijn Suijten <<a href="mailto:marijn.suijten@somainline.org" target="_blank">marijn.suijten@somainline.org</a>>, Stephen Boyd <<a href="mailto:swboyd@chromium.org" target="_blank">swboyd@chromium.org</a>>, David Airlie <<a href="mailto:airlied@gmail.com" target="_blank">airlied@gmail.com</a>>, Daniel Vetter <<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>>, Bjorn Andersson <<a href="mailto:andersson@kernel.org" target="_blank">andersson@kernel.org</a>>, <a href="mailto:linux-arm-msm@vger.kernel.org" target="_blank">linux-arm-msm@vger.kernel.org</a>, <a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a>, <a href="mailto:freedreno@lists.freedesktop.org" target="_blank">freedreno@lists.freedesktop.org</a> Subject: Re: [Freedreno] [PATCH] drm/msm/dpu: correct clk bit for WB2 block Message-ID: <<a href="mailto:CAA8EJppY0V20rF1kV-b8X2xuCQ6ZHy_%2B0YGp5s6b_Srvq-LLNg@mail.gmail.com" target="_blank">CAA8EJppY0V20rF1kV-b8X2xuCQ6ZHy_+0YGp5s6b_Srvq-LLNg@mail.gmail.com</a>> Content-Type: text/plain; charset="UTF-8" On Tue, 7 Nov 2023 at 01:30, Abhinav Kumar <<a href="mailto:quic_abhinavk@quicinc.com" target="_blank">quic_abhinavk@quicinc.com</a>> wrote: > > > > On 11/6/2023 2:11 PM, Dmitry Baryshkov wrote: > > On Mon, 6 Nov 2023 at 20:39, Abhinav Kumar <<a href="mailto:quic_abhinavk@quicinc.com" target="_blank">quic_abhinavk@quicinc.com</a>> 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? Ah, excuse me then, I didn't notice 24 vs 20. > > >> > >> 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 <<a href="mailto:dmitry.baryshkov@linaro.org" target="_blank">dmitry.baryshkov@linaro.org</a>> > >>> --- > >>> 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 ------------------------------ Subject: Digest Footer _______________________________________________ Freedreno mailing list <a href="mailto:Freedreno@lists.freedesktop.org" target="_blank">Freedreno@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/freedreno" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/freedreno</a> ------------------------------ End of Freedreno Digest, Vol 117, Issue 10 ****************************************** </blockquote></div>