Re: [Freedreno] [PATCH v3 3/3] drm/komeda: Fix IS_ERR() vs NULL check in komeda_component_get_avail_scaler()
If you're going to update the other patches to use IS_ERR_OR_NULL() please do so
here too. You can keep my R-b for that change.
Thanks for taking time to review the patch.
I have update the patch set and add this change to v5.
On 2023/7/21 18:24, Liviu Dudau wrote:
Hi Gaosheng,
On Fri, Jul 14, 2023 at 09:48:20AM +0800, Gaosheng Cui wrote:
The komeda_pipeline_get_state() returns an ERR_PTR() on failure, we should
use IS_ERR() to check the return value.
Fixes: 502932a03fce ("drm/komeda: Add the initial scaler support for CORE")
Signed-off-by: Gaosheng Cui
Reviewed-by: Liviu Dudau
---
drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 3276a3e82c62..e9c92439398d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -259,7 +259,7 @@ komeda_component_get_avail_scaler(struct komeda_component
*c,
u32 avail_scalers;
pipe_st = komeda_pipeline_get_state(c->pipeline, state);
- if (!pipe_st)
+ if (IS_ERR(pipe_st))
If you're going to update the other patches to use IS_ERR_OR_NULL() please do so
here too. You can keep my R-b for that change.
Best regards,
Liviu
return NULL;
avail_scalers = (pipe_st->active_comps & KOMEDA_PIPELINE_SCALERS) ^
--
2.25.1
Re: [Freedreno] [PATCH v2 3/3] drm/komeda: Fix IS_ERR() vs NULL check in komeda_component_get_avail_scaler()
Thanks for taking time to review this patch.
Maybe we can submit another separate patch to fix ERR_CAST(st), because I'm not
sure which commit should be used as a fixes-tag.
Also, Maybe we should fix ERR_CAST(st) in
komeda_pipeline_get_state_and_set_crtc()
and komeda_component_get_state_and_set_user(), please make another separate
patch.
Let me know if there's anything I can do, thanks for your work again!
Gaosheng,
On 2023/7/13 16:54, Liviu Dudau wrote:
Hello,
On Thu, Jul 13, 2023 at 10:05:56AM +0800, Gaosheng Cui wrote:
The komeda_pipeline_get_state() returns an ERR_PTR() on failure, we should
use IS_ERR() to check the return value.
While reviewing the change I've realised that
komeda_pipeline_get_state_and_set_crtc()
is also mishandling the return value from komeda_pipeline_get_state(). If
IS_ERR(st) is
true it should use return ERR_CAST(st), following the same pattern as
komeda_pipeline_get_state().
If you don't want to update this patch I can send a separate patch. Otherwise,
the change looks good to me.
Reviewed-by: Liviu Dudau
Best regards,
Liviu
Fixes: 502932a03fce ("drm/komeda: Add the initial scaler support for CORE")
Signed-off-by: Gaosheng Cui
---
drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 3276a3e82c62..e9c92439398d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -259,7 +259,7 @@ komeda_component_get_avail_scaler(struct komeda_component
*c,
u32 avail_scalers;
pipe_st = komeda_pipeline_get_state(c->pipeline, state);
- if (!pipe_st)
+ if (IS_ERR(pipe_st))
return NULL;
avail_scalers = (pipe_st->active_comps & KOMEDA_PIPELINE_SCALERS) ^
--
2.25.1
Re: [Freedreno] [PATCH 3/5] drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()
NAK. This path should be returned if it is NON-NULL, otherwise we defer
to of_icc_get() on the parent device. See the code-comment below.
Thanks for taking time to review this patch, how do you think of the following
changes:
diff --git a/drivers/gpu/drm/msm/msm_io_utils.c
b/drivers/gpu/drm/msm/msm_io_utils.c index d02cd29ce829..a112d8c74d59
100644 --- a/drivers/gpu/drm/msm/msm_io_utils.c +++
b/drivers/gpu/drm/msm/msm_io_utils.c @@ -133,7 +133,7 @@ struct
icc_path *msm_icc_get(struct device *dev, const char *name) struct
icc_path *path; path = of_icc_get(dev, name); - if (path) + if
(!IS_ERR_OR_NULL(path)) return path;
Looking forward to your reply, thanks again!
On 2022/11/11 18:02, Marijn Suijten wrote:
On 2022-11-10 17:44:43, Gaosheng Cui wrote:
The of_icc_get() function returns NULL or error pointers on error path,
so we should use IS_ERR_OR_NULL() to check the return value.
Fixes: 5ccdcecaf8f7 ("drm/msm: lookup the ICC paths in both mdp5/dpu and mdss
devices")
Signed-off-by: Gaosheng Cui
---
drivers/gpu/drm/msm/msm_io_utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_io_utils.c
b/drivers/gpu/drm/msm/msm_io_utils.c
index d02cd29ce829..950083b2f092 100644
--- a/drivers/gpu/drm/msm/msm_io_utils.c
+++ b/drivers/gpu/drm/msm/msm_io_utils.c
@@ -133,7 +133,7 @@ struct icc_path *msm_icc_get(struct device *dev, const char
*name)
struct icc_path *path;
path = of_icc_get(dev, name);
- if (path)
+ if (IS_ERR_OR_NULL(path))
NAK. This path should be returned if it is NON-NULL, otherwise we defer
to of_icc_get() on the parent device. See the code-comment below.
- Marijn
return path;
/*
--
2.25.1
.
