Hi Marijn
On 4/11/2023 3:24 PM, Marijn Suijten wrote:
Again, don't forget to include previous reviewers in cc, please :)
On 2023-04-11 14:09:40, Kuogee Hsieh wrote:
Perform DSC range checking to make sure correct DSC is requested before
reserve resource for it.
This isn't performing any range checking for resource reservations /
requests: this is only validating the constants written in our catalog
and seems rather useless. It isn't fixing any real bug either, so the
Fixes: tag below seems extraneous.
Given prior comments from Abhinav that "the kernel should be trusted",
we should remove this validation for all the other blocks instead.
The purpose of this check is that today all our blocks in RM use the
DSC_* enum as the size.
struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
If the device tree ends up with more DSC blocks than the DSC_* enum, how
can we avoid this issue today? Not because its a bug in device tree but
how many static number of DSCs are hard-coded in RM.
And like you said, this is not specific to DSC. Such checks are present
for other blocks too.
Fixes: c985d7bb64ff ("drm/msm/disp/dpu1: Add DSC support in RM")
Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f4dda88..95e58f1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#define pr_fmt(fmt) "[drm:%s] " fmt, __func__
@@ -250,6 +251,11 @@ int dpu_rm_init(struct dpu_rm *rm,
struct dpu_hw_dsc *hw;
const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
+ if (dsc->id < DSC_0 || dsc->id >= DSC_MAX) {
+ DPU_ERROR("skip dsc %d with invalid id\n", dsc->id);
+ continue;
+ }
+
hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
if (IS_ERR_OR_NULL(hw)) {
rc = PTR_ERR(hw);
@@ -557,8 +563,10 @@ static int _dpu_rm_make_reservation(
}
ret = _dpu_rm_reserve_dsc(rm, global_state, enc, &reqs->topology);
- if (ret)
+ if (ret) {
+ DPU_ERROR("unable to find appropriate DSC\n");
This, while a nice addition, should go in a different patch.
Thanks!
- Marijn
return ret;
+ }
return ret;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
