Re: [PATCH v1 08/10] iommu/arm-smmu-qcom: Merge table from arm-smmu-qcom-debug into match data
On 2/13/2024 4:40 PM, Dmitry Baryshkov wrote: On Tue, 13 Feb 2024 at 12:29, Pratyush Brahma wrote: Hi Patch [1] introduces a use after free bug in the function "qcom_smmu_create()" in file: drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c wherein devm_krealloc() frees the old pointer marked by "smmu" but it is still being accessed later in qcom_smmu_impl_data() in the same function as: qsmmu->cfg = qcom_smmu_impl_data(smmu); The current patchset [2] implicitly fixes this issue as it doesn't access the freed ptr in the line: qsmmu->cfg = data->cfg; Hence, can this patchset[2] be propagated to branches where patchset[1] has been propagated already? The bug is currently present in all branches that have patchset[1] but do not have patchset[2]. Can you please comment on your thoughts on this as well? RFC: This bug would be reintroduced if patchset [3] is accepted. This makes the path prone to such errors as it relies on the developer's understanding on the internal implementation of devm_krealloc(). realloc is a basic function. Not understanding it is a significant problem for the developer. Hence, a better fix IMO, would be to remove the confusion around the freed "smmu" ptr in the following way: Could you please post a proper patch, which can be reviewed and accepted or declined? Sure, will do. diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 549ae4dba3a6..6dd142ce75d1 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -463,11 +463,12 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu, qsmmu = devm_krealloc(smmu->dev, smmu, sizeof(*qsmmu), GFP_KERNEL); if (!qsmmu) return ERR_PTR(-ENOMEM); + smmu = >smmu; qsmmu->smmu.impl = impl; qsmmu->cfg = data->cfg; - return >smmu; + return smmu; } This is similar to the patch[4] which I've sent in-reply-to patch[3]. Will send a formal patch if you think this approach is better. Please let me know your thoughts. None of the other implementations does this. If you are going to fix qcom implementation, please fix all implementations. Ohh okay. Wasn't aware that this may be an issue in other implementations as well. Will check and raise a formal patch. However a better option might be to change arm-smmu to remove devm_krealloc() usage at all. Can you please elaborate on your thoughts on how removing devm_krealloc() usage would be better? Is it because this implementation is error prone or do you think this isn't required at all? I agree on your previous comment that realloc is a basic function and developers should understand that before using it. But as you've pointed out that implementations other than qcom may also have this issue, I'm inclined to think that the usage of the api is quite error prone and there may be some room for improving the usage text perhaps or some other way. -- With best wishes Dmitry Thanks, Pratyush
[PATCH] drm/dp: move intel_dp_vsc_sdp_pack() to generic helper
intel_dp_vsc_sdp_pack() can be re-used by other DRM drivers as well.
Lets move this to drm_dp_helper to achieve this.
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/display/drm_dp_helper.c | 78 +
drivers/gpu/drm/i915/display/intel_dp.c | 73 +--
include/drm/display/drm_dp_helper.h | 3 +
3 files changed, 84 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
b/drivers/gpu/drm/display/drm_dp_helper.c
index b1ca3a1100da..066cfbbf7a91 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2916,6 +2916,84 @@ void drm_dp_vsc_sdp_log(const char *level, struct device
*dev,
}
EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
+/**
+ * drm_dp_vsc_sdp_pack() - pack a given vsc sdp into generic dp_sdp
+ * @vsc: vsc sdp initialized according to its purpose as defined in
+ * table 2-118 - table 2-120 in DP 1.4a specification
+ * @sdp: valid handle to the generic dp_sdp which will be packed
+ * @size: valid size of the passed sdp handle
+ *
+ * Returns length of sdp on success and error code on failure
+ */
+ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
+ struct dp_sdp *sdp, size_t size)
+{
+ size_t length = sizeof(struct dp_sdp);
+
+ if (size < length)
+ return -ENOSPC;
+
+ memset(sdp, 0, size);
+
+ /*
+* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
+* VSC SDP Header Bytes
+*/
+ sdp->sdp_header.HB0 = 0; /* Secondary-Data Packet ID = 0 */
+ sdp->sdp_header.HB1 = vsc->sdp_type; /* Secondary-data Packet Type */
+ sdp->sdp_header.HB2 = vsc->revision; /* Revision Number */
+ sdp->sdp_header.HB3 = vsc->length; /* Number of Valid Data Bytes */
+
+ if (vsc->revision == 0x6) {
+ sdp->db[0] = 1;
+ sdp->db[3] = 1;
+ }
+
+ /*
+* Revision 0x5 and revision 0x7 supports Pixel Encoding/Colorimetry
+* Format as per DP 1.4a spec and DP 2.0 respectively.
+*/
+ if (!(vsc->revision == 0x5 || vsc->revision == 0x7))
+ goto out;
+
+ /* VSC SDP Payload for DB16 through DB18 */
+ /* Pixel Encoding and Colorimetry Formats */
+ sdp->db[16] = (vsc->pixelformat & 0xf) << 4; /* DB16[7:4] */
+ sdp->db[16] |= vsc->colorimetry & 0xf; /* DB16[3:0] */
+
+ switch (vsc->bpc) {
+ case 6:
+ /* 6bpc: 0x0 */
+ break;
+ case 8:
+ sdp->db[17] = 0x1; /* DB17[3:0] */
+ break;
+ case 10:
+ sdp->db[17] = 0x2;
+ break;
+ case 12:
+ sdp->db[17] = 0x3;
+ break;
+ case 16:
+ sdp->db[17] = 0x4;
+ break;
+ default:
+ WARN(1, "Missing case %d\n", vsc->bpc);
+ return -EINVAL;
+ }
+
+ /* Dynamic Range and Component Bit Depth */
+ if (vsc->dynamic_range == DP_DYNAMIC_RANGE_CTA)
+ sdp->db[17] |= 0x80; /* DB17[7] */
+
+ /* Content Type */
+ sdp->db[18] = vsc->content_type & 0x7;
+
+out:
+ return length;
+}
+EXPORT_SYMBOL(drm_dp_vsc_sdp_pack);
+
/**
* drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON
* @dpcd: DisplayPort configuration data
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index f5ef95da5534..e94db51aeeb7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4110,73 +4110,6 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state
*crtc_state,
return false;
}
-static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
-struct dp_sdp *sdp, size_t size)
-{
- size_t length = sizeof(struct dp_sdp);
-
- if (size < length)
- return -ENOSPC;
-
- memset(sdp, 0, size);
-
- /*
-* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
-* VSC SDP Header Bytes
-*/
- sdp->sdp_header.HB0 = 0; /* Secondary-Data Packet ID = 0 */
- sdp->sdp_header.HB1 = vsc->sdp_type; /* Secondary-data Packet Type */
- sdp->sdp_header.HB2 = vsc->revision; /* Revision Number */
- sdp->sdp_header.HB3 = vsc->length; /* Number of Valid Data Bytes */
-
- if (vsc->revision == 0x6) {
- sdp->db[0] = 1;
- sdp->db[3] = 1;
- }
-
- /*
-* Revision 0x5 and revision 0x7 supports Pixel Encoding/Colorimetry
-* Format as per DP 1.4a spec and DP 2.0 respectively.
-*/
- if (!(vsc->revision == 0x5 || vsc->revision == 0x7))
- goto out;
-
- /* VSC SDP Payload for DB16 through DB18 */
- /* Pixel Encoding and Colorimetry Formats */
- sdp->db[16] = (vsc->pixelformat & 0xf) << 4; /* DB16[7:4] */
- sdp->db[16] |=
Re: [PATCH v2 16/19] drm/msm/dpu: modify encoder programming for CDM over DP
On 2/13/2024 1:16 PM, Dmitry Baryshkov wrote:
On Tue, 13 Feb 2024 at 23:10, Abhinav Kumar wrote:
On 2/13/2024 11:31 AM, Dmitry Baryshkov wrote:
On Tue, 13 Feb 2024 at 20:46, Abhinav Kumar wrote:
On 2/13/2024 10:23 AM, Dmitry Baryshkov wrote:
On Tue, 13 Feb 2024 at 19:32, Abhinav Kumar wrote:
On 2/13/2024 3:18 AM, Dmitry Baryshkov wrote:
On Sat, 10 Feb 2024 at 03:53, Paloma Arellano wrote:
Adjust the encoder format programming in the case of video mode for DP
to accommodate CDM related changes.
Changes in v2:
- Move timing engine programming to a separate patch from this
one
- Move update_pending_flush_periph() invocation completely to
this patch
- Change the logic of dpu_encoder_get_drm_fmt() so that it only
calls drm_mode_is_420_only() instead of doing additional
unnecessary checks
- Create new functions msm_dp_needs_periph_flush() and it's
supporting function dpu_encoder_needs_periph_flush() to check
if the mode is YUV420 and VSC SDP is enabled before doing a
peripheral flush
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35 +++
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 13 +++
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 ++
drivers/gpu/drm/msm/dp/dp_display.c | 18 ++
drivers/gpu/drm/msm/msm_drv.h | 17 -
5 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 7e7796561009a..6280c6be6dca9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -222,6 +222,41 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc;
+ struct dpu_encoder_virt *dpu_enc;
+ struct drm_display_info *info;
+ struct drm_display_mode *mode;
+
+ drm_enc = phys_enc->parent;
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+ info = _enc->connector->display_info;
+ mode = _enc->cached_mode;
+
+ if (drm_mode_is_420_only(info, mode))
+ return DRM_FORMAT_YUV420;
+
+ return DRM_FORMAT_RGB888;
+}
+
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc;
+ struct dpu_encoder_virt *dpu_enc;
+ struct msm_display_info *disp_info;
+ struct msm_drm_private *priv;
+ struct drm_display_mode *mode;
+
+ drm_enc = phys_enc->parent;
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+ disp_info = _enc->disp_info;
+ priv = drm_enc->dev->dev_private;
+ mode = _enc->cached_mode;
+
+ return phys_enc->hw_intf->cap->type == INTF_DP && phys_enc->hw_cdm &&
Do we really need to check for phys_enc->hw_cdm here?
hmmm I dont think so. If CDM was not there, then after the last patch,
YUV420 removes will not be present at all.
The only other case I could think of was, if for some reason CDM was
used by some other interface such as WB, then hw_cdm will not be assigned.
But, I think even for that msm_dp_needs_periph_flush() will take care of
it because we use the cached_mode which is assigned only in mode_set().
+
msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
+}
bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index f43d57d9c74e1..211a3d90eb690 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode
dpu_encoder_helper_get_3d_blend_mode(
*/
unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
+/**
+ * dpu_encoder_get_drm_fmt - return DRM fourcc format
+ * @phys_enc: Pointer to physical encoder structure
+ */
+u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
+
+/**
+ * dpu_encoder_needs_periph_flush - return true if physical encoder requires
+ * peripheral flush
+ * @phys_enc: Pointer to physical encoder structure
+ */
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc);
+
/**
* dpu_encoder_helper_split_config - split display configuration helper
function
* This helper function may be used by physical encoders to configure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index f562beb6f7971..3f102b2813ca8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++
Re: [PATCH v2 16/19] drm/msm/dpu: modify encoder programming for CDM over DP
On Tue, 13 Feb 2024 at 23:10, Abhinav Kumar wrote:
>
>
>
> On 2/13/2024 11:31 AM, Dmitry Baryshkov wrote:
> > On Tue, 13 Feb 2024 at 20:46, Abhinav Kumar
> > wrote:
> >>
> >>
> >>
> >> On 2/13/2024 10:23 AM, Dmitry Baryshkov wrote:
> >>> On Tue, 13 Feb 2024 at 19:32, Abhinav Kumar
> >>> wrote:
>
>
>
> On 2/13/2024 3:18 AM, Dmitry Baryshkov wrote:
> > On Sat, 10 Feb 2024 at 03:53, Paloma Arellano
> > wrote:
> >>
> >> Adjust the encoder format programming in the case of video mode for DP
> >> to accommodate CDM related changes.
> >>
> >> Changes in v2:
> >>- Move timing engine programming to a separate patch from
> >> this
> >> one
> >>- Move update_pending_flush_periph() invocation completely
> >> to
> >> this patch
> >>- Change the logic of dpu_encoder_get_drm_fmt() so that it
> >> only
> >> calls drm_mode_is_420_only() instead of doing additional
> >> unnecessary checks
> >>- Create new functions msm_dp_needs_periph_flush() and it's
> >> supporting function dpu_encoder_needs_periph_flush() to
> >> check
> >> if the mode is YUV420 and VSC SDP is enabled before doing
> >> a
> >> peripheral flush
> >>
> >> Signed-off-by: Paloma Arellano
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35
> >> +++
> >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 13 +++
> >> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 ++
> >> drivers/gpu/drm/msm/dp/dp_display.c | 18 ++
> >> drivers/gpu/drm/msm/msm_drv.h | 17 -
> >> 5 files changed, 101 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> index 7e7796561009a..6280c6be6dca9 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> @@ -222,6 +222,41 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
> >>15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
> >> };
> >>
> >> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc)
> >> +{
> >> + struct drm_encoder *drm_enc;
> >> + struct dpu_encoder_virt *dpu_enc;
> >> + struct drm_display_info *info;
> >> + struct drm_display_mode *mode;
> >> +
> >> + drm_enc = phys_enc->parent;
> >> + dpu_enc = to_dpu_encoder_virt(drm_enc);
> >> + info = _enc->connector->display_info;
> >> + mode = _enc->cached_mode;
> >> +
> >> + if (drm_mode_is_420_only(info, mode))
> >> + return DRM_FORMAT_YUV420;
> >> +
> >> + return DRM_FORMAT_RGB888;
> >> +}
> >> +
> >> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
> >> +{
> >> + struct drm_encoder *drm_enc;
> >> + struct dpu_encoder_virt *dpu_enc;
> >> + struct msm_display_info *disp_info;
> >> + struct msm_drm_private *priv;
> >> + struct drm_display_mode *mode;
> >> +
> >> + drm_enc = phys_enc->parent;
> >> + dpu_enc = to_dpu_encoder_virt(drm_enc);
> >> + disp_info = _enc->disp_info;
> >> + priv = drm_enc->dev->dev_private;
> >> + mode = _enc->cached_mode;
> >> +
> >> + return phys_enc->hw_intf->cap->type == INTF_DP &&
> >> phys_enc->hw_cdm &&
> >
> > Do we really need to check for phys_enc->hw_cdm here?
> >
>
> hmmm I dont think so. If CDM was not there, then after the last patch,
> YUV420 removes will not be present at all.
>
> The only other case I could think of was, if for some reason CDM was
> used by some other interface such as WB, then hw_cdm will not be
> assigned.
>
> But, I think even for that msm_dp_needs_periph_flush() will take care of
> it because we use the cached_mode which is assigned only in mode_set().
>
> >> +
> >> msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]],
> >> mode);
> >> +}
> >>
> >> bool dpu_encoder_is_widebus_enabled(const struct drm_encoder
> >> *drm_enc)
> >> {
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >> index f43d57d9c74e1..211a3d90eb690 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >> @@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode
> >> dpu_encoder_helper_get_3d_blend_mode(
> >> */
Re: [PATCH v2 16/19] drm/msm/dpu: modify encoder programming for CDM over DP
On 2/13/2024 11:31 AM, Dmitry Baryshkov wrote:
On Tue, 13 Feb 2024 at 20:46, Abhinav Kumar wrote:
On 2/13/2024 10:23 AM, Dmitry Baryshkov wrote:
On Tue, 13 Feb 2024 at 19:32, Abhinav Kumar wrote:
On 2/13/2024 3:18 AM, Dmitry Baryshkov wrote:
On Sat, 10 Feb 2024 at 03:53, Paloma Arellano wrote:
Adjust the encoder format programming in the case of video mode for DP
to accommodate CDM related changes.
Changes in v2:
- Move timing engine programming to a separate patch from this
one
- Move update_pending_flush_periph() invocation completely to
this patch
- Change the logic of dpu_encoder_get_drm_fmt() so that it only
calls drm_mode_is_420_only() instead of doing additional
unnecessary checks
- Create new functions msm_dp_needs_periph_flush() and it's
supporting function dpu_encoder_needs_periph_flush() to check
if the mode is YUV420 and VSC SDP is enabled before doing a
peripheral flush
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35 +++
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 13 +++
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 ++
drivers/gpu/drm/msm/dp/dp_display.c | 18 ++
drivers/gpu/drm/msm/msm_drv.h | 17 -
5 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 7e7796561009a..6280c6be6dca9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -222,6 +222,41 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc;
+ struct dpu_encoder_virt *dpu_enc;
+ struct drm_display_info *info;
+ struct drm_display_mode *mode;
+
+ drm_enc = phys_enc->parent;
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+ info = _enc->connector->display_info;
+ mode = _enc->cached_mode;
+
+ if (drm_mode_is_420_only(info, mode))
+ return DRM_FORMAT_YUV420;
+
+ return DRM_FORMAT_RGB888;
+}
+
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc;
+ struct dpu_encoder_virt *dpu_enc;
+ struct msm_display_info *disp_info;
+ struct msm_drm_private *priv;
+ struct drm_display_mode *mode;
+
+ drm_enc = phys_enc->parent;
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+ disp_info = _enc->disp_info;
+ priv = drm_enc->dev->dev_private;
+ mode = _enc->cached_mode;
+
+ return phys_enc->hw_intf->cap->type == INTF_DP && phys_enc->hw_cdm &&
Do we really need to check for phys_enc->hw_cdm here?
hmmm I dont think so. If CDM was not there, then after the last patch,
YUV420 removes will not be present at all.
The only other case I could think of was, if for some reason CDM was
used by some other interface such as WB, then hw_cdm will not be assigned.
But, I think even for that msm_dp_needs_periph_flush() will take care of
it because we use the cached_mode which is assigned only in mode_set().
+
msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
+}
bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index f43d57d9c74e1..211a3d90eb690 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode
dpu_encoder_helper_get_3d_blend_mode(
*/
unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
+/**
+ * dpu_encoder_get_drm_fmt - return DRM fourcc format
+ * @phys_enc: Pointer to physical encoder structure
+ */
+u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
+
+/**
+ * dpu_encoder_needs_periph_flush - return true if physical encoder requires
+ * peripheral flush
+ * @phys_enc: Pointer to physical encoder structure
+ */
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc);
+
/**
* dpu_encoder_helper_split_config - split display configuration helper
function
* This helper function may be used by physical encoders to configure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index f562beb6f7971..3f102b2813ca8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -413,8 +413,15 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
static
[PATCH] drm: ci: use clk_ignore_unused for apq8016
If the ADV7511 bridge driver is compiled as a module, while DRM_MSM is
built-in, the clk_disable_unused congests with the runtime PM handling
of the DSI PHY for the clk_prepare_lock(). This causes apq8016 runner to
fail without completing any jobs ([1]). Drop the BM_CMDLINE which
duplicate the command line from the .baremetal-igt-arm64 clause and
enforce the clk_ignore_unused kernelarg instead to make apq8016 runner
work.
[1] https://gitlab.freedesktop.org/drm/msm/-/jobs/54990475
Fixes: 0119c894ab0d ("drm: Add initial ci/ subdirectory")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/ci/test.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ci/test.yml b/drivers/gpu/drm/ci/test.yml
index 355b794ef2b1..b9f864e062df 100644
--- a/drivers/gpu/drm/ci/test.yml
+++ b/drivers/gpu/drm/ci/test.yml
@@ -119,7 +119,7 @@ msm:apq8016:
DRIVER_NAME: msm
BM_DTB: https://${PIPELINE_ARTIFACTS_BASE}/arm64/apq8016-sbc-usb-host.dtb
GPU_VERSION: apq8016
-BM_CMDLINE: "ip=dhcp console=ttyMSM0,115200n8 $BM_KERNEL_EXTRA_ARGS
root=/dev/nfs rw nfsrootdebug nfsroot=,tcp,nfsvers=4.2 init=/init
$BM_KERNELARGS"
+BM_KERNEL_EXTRA_ARGS: clk_ignore_unused
RUNNER_TAG: google-freedreno-db410c
script:
- ./install/bare-metal/fastboot.sh
--
2.39.2
Re: [PATCH v2 16/19] drm/msm/dpu: modify encoder programming for CDM over DP
On Tue, 13 Feb 2024 at 20:46, Abhinav Kumar wrote:
>
>
>
> On 2/13/2024 10:23 AM, Dmitry Baryshkov wrote:
> > On Tue, 13 Feb 2024 at 19:32, Abhinav Kumar
> > wrote:
> >>
> >>
> >>
> >> On 2/13/2024 3:18 AM, Dmitry Baryshkov wrote:
> >>> On Sat, 10 Feb 2024 at 03:53, Paloma Arellano
> >>> wrote:
>
> Adjust the encoder format programming in the case of video mode for DP
> to accommodate CDM related changes.
>
> Changes in v2:
> - Move timing engine programming to a separate patch from this
> one
> - Move update_pending_flush_periph() invocation completely to
> this patch
> - Change the logic of dpu_encoder_get_drm_fmt() so that it only
> calls drm_mode_is_420_only() instead of doing additional
> unnecessary checks
> - Create new functions msm_dp_needs_periph_flush() and it's
> supporting function dpu_encoder_needs_periph_flush() to check
> if the mode is YUV420 and VSC SDP is enabled before doing a
> peripheral flush
>
> Signed-off-by: Paloma Arellano
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35 +++
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 13 +++
> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 ++
> drivers/gpu/drm/msm/dp/dp_display.c | 18 ++
> drivers/gpu/drm/msm/msm_drv.h | 17 -
> 5 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 7e7796561009a..6280c6be6dca9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -222,6 +222,41 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
> };
>
> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc)
> +{
> + struct drm_encoder *drm_enc;
> + struct dpu_encoder_virt *dpu_enc;
> + struct drm_display_info *info;
> + struct drm_display_mode *mode;
> +
> + drm_enc = phys_enc->parent;
> + dpu_enc = to_dpu_encoder_virt(drm_enc);
> + info = _enc->connector->display_info;
> + mode = _enc->cached_mode;
> +
> + if (drm_mode_is_420_only(info, mode))
> + return DRM_FORMAT_YUV420;
> +
> + return DRM_FORMAT_RGB888;
> +}
> +
> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
> +{
> + struct drm_encoder *drm_enc;
> + struct dpu_encoder_virt *dpu_enc;
> + struct msm_display_info *disp_info;
> + struct msm_drm_private *priv;
> + struct drm_display_mode *mode;
> +
> + drm_enc = phys_enc->parent;
> + dpu_enc = to_dpu_encoder_virt(drm_enc);
> + disp_info = _enc->disp_info;
> + priv = drm_enc->dev->dev_private;
> + mode = _enc->cached_mode;
> +
> + return phys_enc->hw_intf->cap->type == INTF_DP &&
> phys_enc->hw_cdm &&
> >>>
> >>> Do we really need to check for phys_enc->hw_cdm here?
> >>>
> >>
> >> hmmm I dont think so. If CDM was not there, then after the last patch,
> >> YUV420 removes will not be present at all.
> >>
> >> The only other case I could think of was, if for some reason CDM was
> >> used by some other interface such as WB, then hw_cdm will not be assigned.
> >>
> >> But, I think even for that msm_dp_needs_periph_flush() will take care of
> >> it because we use the cached_mode which is assigned only in mode_set().
> >>
> +
> msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
> +}
>
> bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
> {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index f43d57d9c74e1..211a3d90eb690 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode
> dpu_encoder_helper_get_3d_blend_mode(
> */
> unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys
> *phys_enc);
>
> +/**
> + * dpu_encoder_get_drm_fmt - return DRM fourcc format
> + * @phys_enc: Pointer to physical encoder structure
> + */
> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
> +
> +/**
> + * dpu_encoder_needs_periph_flush - return true if physical encoder
> requires
> + *
Re: [PATCH v2 16/19] drm/msm/dpu: modify encoder programming for CDM over DP
On 2/13/2024 10:23 AM, Dmitry Baryshkov wrote:
On Tue, 13 Feb 2024 at 19:32, Abhinav Kumar wrote:
On 2/13/2024 3:18 AM, Dmitry Baryshkov wrote:
On Sat, 10 Feb 2024 at 03:53, Paloma Arellano wrote:
Adjust the encoder format programming in the case of video mode for DP
to accommodate CDM related changes.
Changes in v2:
- Move timing engine programming to a separate patch from this
one
- Move update_pending_flush_periph() invocation completely to
this patch
- Change the logic of dpu_encoder_get_drm_fmt() so that it only
calls drm_mode_is_420_only() instead of doing additional
unnecessary checks
- Create new functions msm_dp_needs_periph_flush() and it's
supporting function dpu_encoder_needs_periph_flush() to check
if the mode is YUV420 and VSC SDP is enabled before doing a
peripheral flush
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35 +++
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 13 +++
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 ++
drivers/gpu/drm/msm/dp/dp_display.c | 18 ++
drivers/gpu/drm/msm/msm_drv.h | 17 -
5 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 7e7796561009a..6280c6be6dca9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -222,6 +222,41 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc;
+ struct dpu_encoder_virt *dpu_enc;
+ struct drm_display_info *info;
+ struct drm_display_mode *mode;
+
+ drm_enc = phys_enc->parent;
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+ info = _enc->connector->display_info;
+ mode = _enc->cached_mode;
+
+ if (drm_mode_is_420_only(info, mode))
+ return DRM_FORMAT_YUV420;
+
+ return DRM_FORMAT_RGB888;
+}
+
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc;
+ struct dpu_encoder_virt *dpu_enc;
+ struct msm_display_info *disp_info;
+ struct msm_drm_private *priv;
+ struct drm_display_mode *mode;
+
+ drm_enc = phys_enc->parent;
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+ disp_info = _enc->disp_info;
+ priv = drm_enc->dev->dev_private;
+ mode = _enc->cached_mode;
+
+ return phys_enc->hw_intf->cap->type == INTF_DP && phys_enc->hw_cdm &&
Do we really need to check for phys_enc->hw_cdm here?
hmmm I dont think so. If CDM was not there, then after the last patch,
YUV420 removes will not be present at all.
The only other case I could think of was, if for some reason CDM was
used by some other interface such as WB, then hw_cdm will not be assigned.
But, I think even for that msm_dp_needs_periph_flush() will take care of
it because we use the cached_mode which is assigned only in mode_set().
+
msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
+}
bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index f43d57d9c74e1..211a3d90eb690 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode
dpu_encoder_helper_get_3d_blend_mode(
*/
unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
+/**
+ * dpu_encoder_get_drm_fmt - return DRM fourcc format
+ * @phys_enc: Pointer to physical encoder structure
+ */
+u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
+
+/**
+ * dpu_encoder_needs_periph_flush - return true if physical encoder requires
+ * peripheral flush
+ * @phys_enc: Pointer to physical encoder structure
+ */
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc);
+
/**
* dpu_encoder_helper_split_config - split display configuration helper
function
* This helper function may be used by physical encoders to configure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index f562beb6f7971..3f102b2813ca8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -413,8 +413,15 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
{
struct dpu_hw_ctl *ctl;
+ struct dpu_hw_cdm
Re: [PATCH v2 16/19] drm/msm/dpu: modify encoder programming for CDM over DP
On Tue, 13 Feb 2024 at 19:32, Abhinav Kumar wrote:
>
>
>
> On 2/13/2024 3:18 AM, Dmitry Baryshkov wrote:
> > On Sat, 10 Feb 2024 at 03:53, Paloma Arellano
> > wrote:
> >>
> >> Adjust the encoder format programming in the case of video mode for DP
> >> to accommodate CDM related changes.
> >>
> >> Changes in v2:
> >> - Move timing engine programming to a separate patch from this
> >>one
> >> - Move update_pending_flush_periph() invocation completely to
> >>this patch
> >> - Change the logic of dpu_encoder_get_drm_fmt() so that it only
> >>calls drm_mode_is_420_only() instead of doing additional
> >>unnecessary checks
> >> - Create new functions msm_dp_needs_periph_flush() and it's
> >>supporting function dpu_encoder_needs_periph_flush() to check
> >>if the mode is YUV420 and VSC SDP is enabled before doing a
> >>peripheral flush
> >>
> >> Signed-off-by: Paloma Arellano
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35 +++
> >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 13 +++
> >> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 ++
> >> drivers/gpu/drm/msm/dp/dp_display.c | 18 ++
> >> drivers/gpu/drm/msm/msm_drv.h | 17 -
> >> 5 files changed, 101 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> index 7e7796561009a..6280c6be6dca9 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> @@ -222,6 +222,41 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
> >> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
> >> };
> >>
> >> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc)
> >> +{
> >> + struct drm_encoder *drm_enc;
> >> + struct dpu_encoder_virt *dpu_enc;
> >> + struct drm_display_info *info;
> >> + struct drm_display_mode *mode;
> >> +
> >> + drm_enc = phys_enc->parent;
> >> + dpu_enc = to_dpu_encoder_virt(drm_enc);
> >> + info = _enc->connector->display_info;
> >> + mode = _enc->cached_mode;
> >> +
> >> + if (drm_mode_is_420_only(info, mode))
> >> + return DRM_FORMAT_YUV420;
> >> +
> >> + return DRM_FORMAT_RGB888;
> >> +}
> >> +
> >> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
> >> +{
> >> + struct drm_encoder *drm_enc;
> >> + struct dpu_encoder_virt *dpu_enc;
> >> + struct msm_display_info *disp_info;
> >> + struct msm_drm_private *priv;
> >> + struct drm_display_mode *mode;
> >> +
> >> + drm_enc = phys_enc->parent;
> >> + dpu_enc = to_dpu_encoder_virt(drm_enc);
> >> + disp_info = _enc->disp_info;
> >> + priv = drm_enc->dev->dev_private;
> >> + mode = _enc->cached_mode;
> >> +
> >> + return phys_enc->hw_intf->cap->type == INTF_DP && phys_enc->hw_cdm
> >> &&
> >
> > Do we really need to check for phys_enc->hw_cdm here?
> >
>
> hmmm I dont think so. If CDM was not there, then after the last patch,
> YUV420 removes will not be present at all.
>
> The only other case I could think of was, if for some reason CDM was
> used by some other interface such as WB, then hw_cdm will not be assigned.
>
> But, I think even for that msm_dp_needs_periph_flush() will take care of
> it because we use the cached_mode which is assigned only in mode_set().
>
> >> +
> >> msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
> >> +}
> >>
> >> bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
> >> {
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >> index f43d57d9c74e1..211a3d90eb690 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >> @@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode
> >> dpu_encoder_helper_get_3d_blend_mode(
> >>*/
> >> unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys
> >> *phys_enc);
> >>
> >> +/**
> >> + * dpu_encoder_get_drm_fmt - return DRM fourcc format
> >> + * @phys_enc: Pointer to physical encoder structure
> >> + */
> >> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
> >> +
> >> +/**
> >> + * dpu_encoder_needs_periph_flush - return true if physical encoder
> >> requires
> >> + * peripheral flush
> >> + * @phys_enc: Pointer to physical encoder structure
> >> + */
> >> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc);
> >> +
> >> /**
> >>* dpu_encoder_helper_split_config - split display configuration helper
> >> function
> >>* This helper function may be used by physical encoders to configure
> >> diff --git
Re: drm/msm: DisplayPort regressions in 6.8-rc1
Hi Johan Thanks for the report. I do agree that pm runtime eDP driver got merged that time but I think the issue is either a combination of that along with DRM aux bridge https://patchwork.freedesktop.org/series/122584/ OR just the latter as even that went in around the same time. Thats why perhaps this issue was not seen with the chromebooks we tested on as they do not use pmic_glink (aux bridge). So we will need to debug this on sc8280xp specifically or an equivalent device which uses aux bridge. Thanks Abhinav On 2/13/2024 3:42 AM, Johan Hovold wrote: Hi, Since 6.8-rc1 the internal eDP display on the Lenovo ThinkPad X13s does not always show up on boot. The logs indicate problems with the runtime PM and eDP rework that went into 6.8-rc1: [6.006236] Console: switching to colour dummy device 80x25 [6.007542] [drm:dpu_kms_hw_init:1048] dpu hardware revision:0x8000 [6.007872] [drm:drm_bridge_attach [drm]] *ERROR* failed to attach bridge /soc@0/phy@88eb000 to encoder TMDS-31: -16 [6.007934] [drm:dp_bridge_init [msm]] *ERROR* failed to attach panel bridge: -16 [6.007983] msm_dpu ae01000.display-controller: [drm:msm_dp_modeset_init [msm]] *ERROR* failed to create dp bridge: -16 [6.008030] [drm:_dpu_kms_initialize_displayport:588] [dpu error]modeset_init failed for DP, rc = -16 [6.008050] [drm:_dpu_kms_setup_displays:681] [dpu error]initialize_DP failed, rc = -16 [6.008068] [drm:dpu_kms_hw_init:1153] [dpu error]modeset init failed: -16 [6.008388] msm_dpu ae01000.display-controller: [drm:msm_drm_kms_init [msm]] *ERROR* kms hw init failed: -16 and this can also manifest itself as a NULL-pointer dereference: [7.339447] Unable to handle kernel NULL pointer dereference at virtual address [7.643705] pc : drm_bridge_attach+0x70/0x1a8 [drm] [7.686415] lr : drm_aux_bridge_attach+0x24/0x38 [aux_bridge] [7.769039] Call trace: [7.771564] drm_bridge_attach+0x70/0x1a8 [drm] [7.776234] drm_aux_bridge_attach+0x24/0x38 [aux_bridge] [7.781782] drm_bridge_attach+0x80/0x1a8 [drm] [7.786454] dp_bridge_init+0xa8/0x15c [msm] [7.790856] msm_dp_modeset_init+0x28/0xc4 [msm] [7.795617] _dpu_kms_drm_obj_init+0x19c/0x680 [msm] [7.800731] dpu_kms_hw_init+0x348/0x4c4 [msm] [7.805306] msm_drm_kms_init+0x84/0x324 [msm] [7.809891] msm_drm_bind+0x1d8/0x3a8 [msm] [7.814196] try_to_bring_up_aggregate_device+0x1f0/0x2f8 [7.819747] __component_add+0xa4/0x18c [7.823703] component_add+0x14/0x20 [7.827389] dp_display_probe+0x47c/0x568 [msm] [7.832052] platform_probe+0x68/0xd8 Users have also reported random crashes at boot since 6.8-rc1, and I've been able to trigger hard crashes twice when testing an external display (USB-C/DP), which may also be related to the DP regressions. I've opened an issue here: https://gitlab.freedesktop.org/drm/msm/-/issues/51 but I also want Thorsten's help to track this so that it gets fixed before 6.8 is released. #regzbot introduced: v6.7..v6.8-rc1 The following series is likely the culprit: https://lore.kernel.org/all/1701472789-25951-1-git-send-email-quic_khs...@quicinc.com/ Johan
Re: [PATCH v2 16/19] drm/msm/dpu: modify encoder programming for CDM over DP
On 2/13/2024 3:18 AM, Dmitry Baryshkov wrote:
On Sat, 10 Feb 2024 at 03:53, Paloma Arellano wrote:
Adjust the encoder format programming in the case of video mode for DP
to accommodate CDM related changes.
Changes in v2:
- Move timing engine programming to a separate patch from this
one
- Move update_pending_flush_periph() invocation completely to
this patch
- Change the logic of dpu_encoder_get_drm_fmt() so that it only
calls drm_mode_is_420_only() instead of doing additional
unnecessary checks
- Create new functions msm_dp_needs_periph_flush() and it's
supporting function dpu_encoder_needs_periph_flush() to check
if the mode is YUV420 and VSC SDP is enabled before doing a
peripheral flush
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35 +++
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 13 +++
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 ++
drivers/gpu/drm/msm/dp/dp_display.c | 18 ++
drivers/gpu/drm/msm/msm_drv.h | 17 -
5 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 7e7796561009a..6280c6be6dca9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -222,6 +222,41 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc;
+ struct dpu_encoder_virt *dpu_enc;
+ struct drm_display_info *info;
+ struct drm_display_mode *mode;
+
+ drm_enc = phys_enc->parent;
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+ info = _enc->connector->display_info;
+ mode = _enc->cached_mode;
+
+ if (drm_mode_is_420_only(info, mode))
+ return DRM_FORMAT_YUV420;
+
+ return DRM_FORMAT_RGB888;
+}
+
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc;
+ struct dpu_encoder_virt *dpu_enc;
+ struct msm_display_info *disp_info;
+ struct msm_drm_private *priv;
+ struct drm_display_mode *mode;
+
+ drm_enc = phys_enc->parent;
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+ disp_info = _enc->disp_info;
+ priv = drm_enc->dev->dev_private;
+ mode = _enc->cached_mode;
+
+ return phys_enc->hw_intf->cap->type == INTF_DP && phys_enc->hw_cdm &&
Do we really need to check for phys_enc->hw_cdm here?
hmmm I dont think so. If CDM was not there, then after the last patch,
YUV420 removes will not be present at all.
The only other case I could think of was, if for some reason CDM was
used by some other interface such as WB, then hw_cdm will not be assigned.
But, I think even for that msm_dp_needs_periph_flush() will take care of
it because we use the cached_mode which is assigned only in mode_set().
+
msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
+}
bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index f43d57d9c74e1..211a3d90eb690 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode
dpu_encoder_helper_get_3d_blend_mode(
*/
unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
+/**
+ * dpu_encoder_get_drm_fmt - return DRM fourcc format
+ * @phys_enc: Pointer to physical encoder structure
+ */
+u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
+
+/**
+ * dpu_encoder_needs_periph_flush - return true if physical encoder requires
+ * peripheral flush
+ * @phys_enc: Pointer to physical encoder structure
+ */
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc);
+
/**
* dpu_encoder_helper_split_config - split display configuration helper
function
* This helper function may be used by physical encoders to configure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index f562beb6f7971..3f102b2813ca8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -413,8 +413,15 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
{
struct dpu_hw_ctl *ctl;
+ struct dpu_hw_cdm *hw_cdm;
+ const struct dpu_format *fmt = NULL;
+ u32 fmt_fourcc = DRM_FORMAT_RGB888;
ctl =
[PATCH] drm/msm: Wire up tlb ops
From: Rob Clark
The brute force iommu_flush_iotlb_all() was good enough for unmap, but
in some cases a map operation could require removing a table pte entry
to replace with a block entry. This also requires tlb invalidation.
Missing this was resulting an obscure iova fault on what should be a
valid buffer address.
Thanks to Robin Murphy for helping me understand the cause of the fault.
Cc: Robin Murphy
Fixes: b145c6e65eb0 ("drm/msm: Add support to create a local pagetable")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_iommu.c | 32 +---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index 5cc8d358cc97..d5512037c38b 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -21,6 +21,8 @@ struct msm_iommu_pagetable {
struct msm_mmu base;
struct msm_mmu *parent;
struct io_pgtable_ops *pgtbl_ops;
+ const struct iommu_flush_ops *tlb;
+ struct device *iommu_dev;
unsigned long pgsize_bitmap;/* Bitmap of page sizes in use */
phys_addr_t ttbr;
u32 asid;
@@ -201,11 +203,33 @@ static const struct msm_mmu_funcs pagetable_funcs = {
static void msm_iommu_tlb_flush_all(void *cookie)
{
+ struct msm_iommu_pagetable *pagetable = cookie;
+ struct adreno_smmu_priv *adreno_smmu;
+
+ if (!pm_runtime_get_if_in_use(pagetable->iommu_dev))
+ return;
+
+ adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
+
+ pagetable->tlb->tlb_flush_all((void *)adreno_smmu->cookie);
+
+ pm_runtime_put_autosuspend(pagetable->iommu_dev);
}
static void msm_iommu_tlb_flush_walk(unsigned long iova, size_t size,
size_t granule, void *cookie)
{
+ struct msm_iommu_pagetable *pagetable = cookie;
+ struct adreno_smmu_priv *adreno_smmu;
+
+ if (!pm_runtime_get_if_in_use(pagetable->iommu_dev))
+ return;
+
+ adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
+
+ pagetable->tlb->tlb_flush_walk(iova, size, granule, (void
*)adreno_smmu->cookie);
+
+ pm_runtime_put_autosuspend(pagetable->iommu_dev);
}
static void msm_iommu_tlb_add_page(struct iommu_iotlb_gather *gather,
@@ -213,7 +237,7 @@ static void msm_iommu_tlb_add_page(struct
iommu_iotlb_gather *gather,
{
}
-static const struct iommu_flush_ops null_tlb_ops = {
+static const struct iommu_flush_ops tlb_ops = {
.tlb_flush_all = msm_iommu_tlb_flush_all,
.tlb_flush_walk = msm_iommu_tlb_flush_walk,
.tlb_add_page = msm_iommu_tlb_add_page,
@@ -254,10 +278,10 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu
*parent)
/* The incoming cfg will have the TTBR1 quirk enabled */
ttbr0_cfg.quirks &= ~IO_PGTABLE_QUIRK_ARM_TTBR1;
- ttbr0_cfg.tlb = _tlb_ops;
+ ttbr0_cfg.tlb = _ops;
pagetable->pgtbl_ops = alloc_io_pgtable_ops(ARM_64_LPAE_S1,
- _cfg, iommu->domain);
+ _cfg, pagetable);
if (!pagetable->pgtbl_ops) {
kfree(pagetable);
@@ -279,6 +303,8 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu
*parent)
/* Needed later for TLB flush */
pagetable->parent = parent;
+ pagetable->tlb = ttbr1_cfg->tlb;
+ pagetable->iommu_dev = ttbr1_cfg->iommu_dev;
pagetable->pgsize_bitmap = ttbr0_cfg.pgsize_bitmap;
pagetable->ttbr = ttbr0_cfg.arm_lpae_s1_cfg.ttbr;
--
2.43.0
drm/msm: DisplayPort regressions in 6.8-rc1
Hi, Since 6.8-rc1 the internal eDP display on the Lenovo ThinkPad X13s does not always show up on boot. The logs indicate problems with the runtime PM and eDP rework that went into 6.8-rc1: [6.006236] Console: switching to colour dummy device 80x25 [6.007542] [drm:dpu_kms_hw_init:1048] dpu hardware revision:0x8000 [6.007872] [drm:drm_bridge_attach [drm]] *ERROR* failed to attach bridge /soc@0/phy@88eb000 to encoder TMDS-31: -16 [6.007934] [drm:dp_bridge_init [msm]] *ERROR* failed to attach panel bridge: -16 [6.007983] msm_dpu ae01000.display-controller: [drm:msm_dp_modeset_init [msm]] *ERROR* failed to create dp bridge: -16 [6.008030] [drm:_dpu_kms_initialize_displayport:588] [dpu error]modeset_init failed for DP, rc = -16 [6.008050] [drm:_dpu_kms_setup_displays:681] [dpu error]initialize_DP failed, rc = -16 [6.008068] [drm:dpu_kms_hw_init:1153] [dpu error]modeset init failed: -16 [6.008388] msm_dpu ae01000.display-controller: [drm:msm_drm_kms_init [msm]] *ERROR* kms hw init failed: -16 and this can also manifest itself as a NULL-pointer dereference: [7.339447] Unable to handle kernel NULL pointer dereference at virtual address [7.643705] pc : drm_bridge_attach+0x70/0x1a8 [drm] [7.686415] lr : drm_aux_bridge_attach+0x24/0x38 [aux_bridge] [7.769039] Call trace: [7.771564] drm_bridge_attach+0x70/0x1a8 [drm] [7.776234] drm_aux_bridge_attach+0x24/0x38 [aux_bridge] [7.781782] drm_bridge_attach+0x80/0x1a8 [drm] [7.786454] dp_bridge_init+0xa8/0x15c [msm] [7.790856] msm_dp_modeset_init+0x28/0xc4 [msm] [7.795617] _dpu_kms_drm_obj_init+0x19c/0x680 [msm] [7.800731] dpu_kms_hw_init+0x348/0x4c4 [msm] [7.805306] msm_drm_kms_init+0x84/0x324 [msm] [7.809891] msm_drm_bind+0x1d8/0x3a8 [msm] [7.814196] try_to_bring_up_aggregate_device+0x1f0/0x2f8 [7.819747] __component_add+0xa4/0x18c [7.823703] component_add+0x14/0x20 [7.827389] dp_display_probe+0x47c/0x568 [msm] [7.832052] platform_probe+0x68/0xd8 Users have also reported random crashes at boot since 6.8-rc1, and I've been able to trigger hard crashes twice when testing an external display (USB-C/DP), which may also be related to the DP regressions. I've opened an issue here: https://gitlab.freedesktop.org/drm/msm/-/issues/51 but I also want Thorsten's help to track this so that it gets fixed before 6.8 is released. #regzbot introduced: v6.7..v6.8-rc1 The following series is likely the culprit: https://lore.kernel.org/all/1701472789-25951-1-git-send-email-quic_khs...@quicinc.com/ Johan
Re: [PATCH v2 16/19] drm/msm/dpu: modify encoder programming for CDM over DP
On Sat, 10 Feb 2024 at 03:53, Paloma Arellano wrote:
>
> Adjust the encoder format programming in the case of video mode for DP
> to accommodate CDM related changes.
>
> Changes in v2:
> - Move timing engine programming to a separate patch from this
> one
> - Move update_pending_flush_periph() invocation completely to
> this patch
> - Change the logic of dpu_encoder_get_drm_fmt() so that it only
> calls drm_mode_is_420_only() instead of doing additional
> unnecessary checks
> - Create new functions msm_dp_needs_periph_flush() and it's
> supporting function dpu_encoder_needs_periph_flush() to check
> if the mode is YUV420 and VSC SDP is enabled before doing a
> peripheral flush
>
> Signed-off-by: Paloma Arellano
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35 +++
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 13 +++
> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 ++
> drivers/gpu/drm/msm/dp/dp_display.c | 18 ++
> drivers/gpu/drm/msm/msm_drv.h | 17 -
> 5 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 7e7796561009a..6280c6be6dca9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -222,6 +222,41 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
> };
>
> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc)
> +{
> + struct drm_encoder *drm_enc;
> + struct dpu_encoder_virt *dpu_enc;
> + struct drm_display_info *info;
> + struct drm_display_mode *mode;
> +
> + drm_enc = phys_enc->parent;
> + dpu_enc = to_dpu_encoder_virt(drm_enc);
> + info = _enc->connector->display_info;
> + mode = _enc->cached_mode;
> +
> + if (drm_mode_is_420_only(info, mode))
> + return DRM_FORMAT_YUV420;
> +
> + return DRM_FORMAT_RGB888;
> +}
> +
> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
> +{
> + struct drm_encoder *drm_enc;
> + struct dpu_encoder_virt *dpu_enc;
> + struct msm_display_info *disp_info;
> + struct msm_drm_private *priv;
> + struct drm_display_mode *mode;
> +
> + drm_enc = phys_enc->parent;
> + dpu_enc = to_dpu_encoder_virt(drm_enc);
> + disp_info = _enc->disp_info;
> + priv = drm_enc->dev->dev_private;
> + mode = _enc->cached_mode;
> +
> + return phys_enc->hw_intf->cap->type == INTF_DP && phys_enc->hw_cdm &&
Do we really need to check for phys_enc->hw_cdm here?
> +
> msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
> +}
>
> bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
> {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index f43d57d9c74e1..211a3d90eb690 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode
> dpu_encoder_helper_get_3d_blend_mode(
> */
> unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
>
> +/**
> + * dpu_encoder_get_drm_fmt - return DRM fourcc format
> + * @phys_enc: Pointer to physical encoder structure
> + */
> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
> +
> +/**
> + * dpu_encoder_needs_periph_flush - return true if physical encoder requires
> + * peripheral flush
> + * @phys_enc: Pointer to physical encoder structure
> + */
> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc);
> +
> /**
> * dpu_encoder_helper_split_config - split display configuration helper
> function
> * This helper function may be used by physical encoders to configure
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index f562beb6f7971..3f102b2813ca8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -413,8 +413,15 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
> static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
> {
> struct dpu_hw_ctl *ctl;
> + struct dpu_hw_cdm *hw_cdm;
> + const struct dpu_format *fmt = NULL;
> + u32 fmt_fourcc = DRM_FORMAT_RGB888;
>
> ctl = phys_enc->hw_ctl;
> + hw_cdm = phys_enc->hw_cdm;
> + if (hw_cdm)
> + fmt_fourcc = dpu_encoder_get_drm_fmt(phys_enc);
Please move if(hw_cdm) inside dpu_encoder_get_drm_fmt().
> + fmt = dpu_get_dpu_format(fmt_fourcc);
Can this be moved into
Re: [PATCH v1 08/10] iommu/arm-smmu-qcom: Merge table from arm-smmu-qcom-debug into match data
On Tue, 13 Feb 2024 at 12:29, Pratyush Brahma wrote: > > Hi > > Patch [1] introduces a use after free bug in the function > "qcom_smmu_create()" in file: drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > wherein devm_krealloc() frees the old pointer marked by "smmu" but it is > still being accessed later in qcom_smmu_impl_data() in the same function > as: > > qsmmu->cfg = qcom_smmu_impl_data(smmu); > > The current patchset [2] implicitly fixes this issue as it doesn't > access the freed ptr in the line: > > qsmmu->cfg = data->cfg; > > Hence, can this patchset[2] be propagated to branches where patchset[1] > has been propagated already? The bug is currently present in all branches > that have patchset[1] but do not have patchset[2]. > > RFC: > > This bug would be reintroduced if patchset [3] is accepted. This makes > the path prone to such errors as it relies on the > developer's understanding on the internal implementation of devm_krealloc(). realloc is a basic function. Not understanding it is a significant problem for the developer. > Hence, a better fix IMO, would be to remove the confusion around the > freed "smmu" ptr in the following way: Could you please post a proper patch, which can be reviewed and accepted or declined? > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > index 549ae4dba3a6..6dd142ce75d1 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -463,11 +463,12 @@ static struct arm_smmu_device > *qcom_smmu_create(struct arm_smmu_device *smmu, > qsmmu = devm_krealloc(smmu->dev, smmu, sizeof(*qsmmu), GFP_KERNEL); > if (!qsmmu) > return ERR_PTR(-ENOMEM); > + smmu = >smmu; > > qsmmu->smmu.impl = impl; > qsmmu->cfg = data->cfg; > > - return >smmu; > + return smmu; > } > > This is similar to the patch[4] which I've sent in-reply-to patch[3]. > Will send a formal patch if you think this approach is better. > > Please let me know your thoughts. None of the other implementations does this. If you are going to fix qcom implementation, please fix all implementations. However a better option might be to change arm-smmu to remove devm_krealloc() usage at all. -- With best wishes Dmitry
Re: [PATCH v1 08/10] iommu/arm-smmu-qcom: Merge table from arm-smmu-qcom-debug into match data
Hi Patch [1] introduces a use after free bug in the function "qcom_smmu_create()" in file: drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c wherein devm_krealloc() frees the old pointer marked by "smmu" but it is still being accessed later in qcom_smmu_impl_data() in the same function as: qsmmu->cfg = qcom_smmu_impl_data(smmu); The current patchset [2] implicitly fixes this issue as it doesn't access the freed ptr in the line: qsmmu->cfg = data->cfg; Hence, can this patchset[2] be propagated to branches where patchset[1] has been propagated already? The bug is currently present in all branches that have patchset[1] but do not have patchset[2]. RFC: This bug would be reintroduced if patchset [3] is accepted. This makes the path prone to such errors as it relies on the developer's understanding on the internal implementation of devm_krealloc(). Hence, a better fix IMO, would be to remove the confusion around the freed "smmu" ptr in the following way: diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 549ae4dba3a6..6dd142ce75d1 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -463,11 +463,12 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu, qsmmu = devm_krealloc(smmu->dev, smmu, sizeof(*qsmmu), GFP_KERNEL); if (!qsmmu) return ERR_PTR(-ENOMEM); + smmu = >smmu; qsmmu->smmu.impl = impl; qsmmu->cfg = data->cfg; - return >smmu; + return smmu; } This is similar to the patch[4] which I've sent in-reply-to patch[3]. Will send a formal patch if you think this approach is better. Please let me know your thoughts. Thanks, Pratyush [1] https://lore.kernel.org/all/20220708094230.4349-1-quic_saipr...@quicinc.com/ [2] https://lore.kernel.org/all/20221114170635.1406534-9-dmitry.barysh...@linaro.org/ [3] https://lore.kernel.org/all/20240201210529.7728-4-quic_c_gdj...@quicinc.com/ [4] https://lore.kernel.org/all/20240213062608.13018-1-quic_pbra...@quicinc.com/
