Hello members, I'm Forcha Pearl, a final-year Computer Science postgraduate. 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.
Could anyone kindly assist me in making my initial contribution to X.ORG foundation by suggesting beginner-friendly issues? On Tue, Nov 7, 2023 at 12:49 AM <[email protected]> wrote: > Send Freedreno mailing list submissions to > [email protected] > > To subscribe or unsubscribe via the World Wide Web, visit > https://lists.freedesktop.org/mailman/listinfo/freedreno > or, via email, send a message with subject or body 'help' to > [email protected] > > You can reach the person managing the list at > [email protected] > > 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 <[email protected]> > To: [email protected] > Cc: [email protected], [email protected], > Daniel Vetter <[email protected]>, Daniel Stone < > [email protected]>, > Dmitry Baryshkov <[email protected]>, Rob Clark > <[email protected]>, Rob Clark <[email protected]>, Abhinav > Kumar <[email protected]>, Sean Paul <[email protected]>, > Marijn > Suijten <[email protected]>, David Airlie > <[email protected]>, [email protected] (open list) > Subject: [Freedreno] [PATCH v3 2/2] drm/msm/gem: Add metadata > Message-ID: <[email protected]> > > From: Rob Clark <[email protected]> > > 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 <[email protected]> > --- > 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 <[email protected]> > To: Abhinav Kumar <[email protected]> > Cc: Rob Clark <[email protected]>, Sean Paul <[email protected]>, > Marijn Suijten <[email protected]>, Stephen Boyd > <[email protected]>, David Airlie <[email protected]>, Daniel > Vetter <[email protected]>, Bjorn Andersson <[email protected]>, > [email protected], [email protected], > [email protected] > Subject: Re: [Freedreno] [PATCH] drm/msm/dpu: correct clk bit for WB2 > block > Message-ID: > <CAA8EJprpBy6UhtScRkFS24TgKevBOb9nVBFCqPhEof=- > [email protected]> > Content-Type: text/plain; charset="UTF-8" > > On Mon, 6 Nov 2023 at 20:39, Abhinav Kumar <[email protected]> > 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 <[email protected]> > > > --- > > > 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 <[email protected]> > To: Dmitry Baryshkov <[email protected]> > Cc: Rob Clark <[email protected]>, Sean Paul <[email protected]>, > Marijn Suijten <[email protected]>, Stephen Boyd > <[email protected]>, David Airlie <[email protected]>, Daniel > Vetter > <[email protected]>, Bjorn Andersson <[email protected]>, > <[email protected]>, <[email protected] > >, > <[email protected]> > Subject: Re: [Freedreno] [PATCH] drm/msm/dpu: correct clk bit for WB2 > block > Message-ID: <[email protected]> > 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 <[email protected]> > 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 <[email protected]> > >>> --- > >>> 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 <[email protected]> > To: Abhinav Kumar <[email protected]> > Cc: Rob Clark <[email protected]>, Sean Paul <[email protected]>, > Marijn Suijten <[email protected]>, Stephen Boyd > <[email protected]>, David Airlie <[email protected]>, Daniel > Vetter <[email protected]>, Bjorn Andersson <[email protected]>, > [email protected], [email protected], > [email protected] > Subject: Re: [Freedreno] [PATCH] drm/msm/dpu: correct clk bit for WB2 > block > Message-ID: > < > caa8ejppy0v20rf1kv-b8x2xucq6zhy_+0ygp5s6b_srvq-l...@mail.gmail.com> > Content-Type: text/plain; charset="UTF-8" > > On Tue, 7 Nov 2023 at 01:30, Abhinav Kumar <[email protected]> > wrote: > > > > > > > > On 11/6/2023 2:11 PM, Dmitry Baryshkov wrote: > > > On Mon, 6 Nov 2023 at 20:39, Abhinav Kumar <[email protected]> > 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 <[email protected]> > > >>> --- > > >>> 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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/freedreno > > > ------------------------------ > > End of Freedreno Digest, Vol 117, Issue 10 > ****************************************** >
