* [PATCH] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()'
@ 2024-01-31 3:28 Srinivasan Shanmugam
2024-02-01 9:58 ` [PATCH v2] " Srinivasan Shanmugam
0 siblings, 1 reply; 5+ messages in thread
From: Srinivasan Shanmugam @ 2024-01-31 3:28 UTC (permalink / raw)
To: Rodrigo Siqueira, Aurabindo Pillai, Anthony Koo
Cc: Yongqiang Sun, Srinivasan Shanmugam, amd-gfx
In "u32 otg_inst = pipe_ctx->stream_res.tg->inst;"
pipe_ctx->stream_res.tg could be NULL, it is relying on the caller to
ensure the tg is not NULL.
Fixes: 474ac4a875ca ("drm/amd/display: Implement some asic specific abm call backs.")
Cc: Yongqiang Sun <yongqiang.sun@amd.com>
Cc: Anthony Koo <Anthony.Koo@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
.../amd/display/dc/hwss/dcn21/dcn21_hwseq.c | 24 +++++++++++--------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
index 5d2d8fd64d98..4e21af0942ea 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
@@ -206,28 +206,32 @@ void dcn21_set_abm_immediate_disable(struct pipe_ctx *pipe_ctx)
void dcn21_set_pipe(struct pipe_ctx *pipe_ctx)
{
struct abm *abm = pipe_ctx->stream_res.abm;
- uint32_t otg_inst = pipe_ctx->stream_res.tg->inst;
+ struct timing_generator *tg = pipe_ctx->stream_res.tg;
struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
struct dmcu *dmcu = pipe_ctx->stream->ctx->dc->res_pool->dmcu;
+ u32 otg_inst;
+
+ if (!abm && !tg && !panel_cntl)
+ return;
+
+ otg_inst = tg->inst;
if (dmcu) {
dce110_set_pipe(pipe_ctx);
return;
}
- if (abm && panel_cntl) {
- if (abm->funcs && abm->funcs->set_pipe_ex) {
- abm->funcs->set_pipe_ex(abm,
+ if (abm->funcs && abm->funcs->set_pipe_ex) {
+ abm->funcs->set_pipe_ex(abm,
otg_inst,
SET_ABM_PIPE_NORMAL,
panel_cntl->inst,
panel_cntl->pwrseq_inst);
- } else {
- dmub_abm_set_pipe(abm, otg_inst,
- SET_ABM_PIPE_NORMAL,
- panel_cntl->inst,
- panel_cntl->pwrseq_inst);
- }
+ } else {
+ dmub_abm_set_pipe(abm, otg_inst,
+ SET_ABM_PIPE_NORMAL,
+ panel_cntl->inst,
+ panel_cntl->pwrseq_inst);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()'
2024-01-31 3:28 [PATCH] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()' Srinivasan Shanmugam
@ 2024-02-01 9:58 ` Srinivasan Shanmugam
2024-02-01 14:23 ` Koo, Anthony
2024-02-12 18:37 ` Kees Cook
0 siblings, 2 replies; 5+ messages in thread
From: Srinivasan Shanmugam @ 2024-02-01 9:58 UTC (permalink / raw)
To: Rodrigo Siqueira, Aurabindo Pillai, Anthony Koo
Cc: Yongqiang Sun, Srinivasan Shanmugam, amd-gfx
In "u32 otg_inst = pipe_ctx->stream_res.tg->inst;"
pipe_ctx->stream_res.tg could be NULL, it is relying on the caller to
ensure the tg is not NULL.
Fixes: 474ac4a875ca ("drm/amd/display: Implement some asic specific abm call backs.")
Cc: Yongqiang Sun <yongqiang.sun@amd.com>
Cc: Anthony Koo <Anthony.Koo@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
v2:
- s/u32/uint32_t for consistency (Anthony)
.../amd/display/dc/hwss/dcn21/dcn21_hwseq.c | 24 +++++++++++--------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
index 8e88dcaf88f5..8323077bba15 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
@@ -206,28 +206,32 @@ void dcn21_set_abm_immediate_disable(struct pipe_ctx *pipe_ctx)
void dcn21_set_pipe(struct pipe_ctx *pipe_ctx)
{
struct abm *abm = pipe_ctx->stream_res.abm;
- uint32_t otg_inst = pipe_ctx->stream_res.tg->inst;
+ struct timing_generator *tg = pipe_ctx->stream_res.tg;
struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
struct dmcu *dmcu = pipe_ctx->stream->ctx->dc->res_pool->dmcu;
+ uint32_t otg_inst;
+
+ if (!abm && !tg && !panel_cntl)
+ return;
+
+ otg_inst = tg->inst;
if (dmcu) {
dce110_set_pipe(pipe_ctx);
return;
}
- if (abm && panel_cntl) {
- if (abm->funcs && abm->funcs->set_pipe_ex) {
- abm->funcs->set_pipe_ex(abm,
+ if (abm->funcs && abm->funcs->set_pipe_ex) {
+ abm->funcs->set_pipe_ex(abm,
otg_inst,
SET_ABM_PIPE_NORMAL,
panel_cntl->inst,
panel_cntl->pwrseq_inst);
- } else {
- dmub_abm_set_pipe(abm, otg_inst,
- SET_ABM_PIPE_NORMAL,
- panel_cntl->inst,
- panel_cntl->pwrseq_inst);
- }
+ } else {
+ dmub_abm_set_pipe(abm, otg_inst,
+ SET_ABM_PIPE_NORMAL,
+ panel_cntl->inst,
+ panel_cntl->pwrseq_inst);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH v2] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()'
2024-02-01 9:58 ` [PATCH v2] " Srinivasan Shanmugam
@ 2024-02-01 14:23 ` Koo, Anthony
2024-02-12 18:37 ` Kees Cook
1 sibling, 0 replies; 5+ messages in thread
From: Koo, Anthony @ 2024-02-01 14:23 UTC (permalink / raw)
To: SHANMUGAM, SRINIVASAN, Siqueira, Rodrigo, Pillai, Aurabindo
Cc: amd-gfx@lists.freedesktop.org, Sun, Yongqiang
[AMD Official Use Only - General]
Reviewed-by: Anthony Koo <Anthony.Koo@amd.com>
Thanks,
Anthony
-----Original Message-----
From: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>
Sent: Thursday, February 1, 2024 4:59 AM
To: Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Pillai, Aurabindo <Aurabindo.Pillai@amd.com>; Koo, Anthony <Anthony.Koo@amd.com>
Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>; Sun, Yongqiang <Yongqiang.Sun@amd.com>
Subject: [PATCH v2] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()'
In "u32 otg_inst = pipe_ctx->stream_res.tg->inst;"
pipe_ctx->stream_res.tg could be NULL, it is relying on the caller to ensure the tg is not NULL.
Fixes: 474ac4a875ca ("drm/amd/display: Implement some asic specific abm call backs.")
Cc: Yongqiang Sun <yongqiang.sun@amd.com>
Cc: Anthony Koo <Anthony.Koo@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
v2:
- s/u32/uint32_t for consistency (Anthony)
.../amd/display/dc/hwss/dcn21/dcn21_hwseq.c | 24 +++++++++++--------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
index 8e88dcaf88f5..8323077bba15 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
@@ -206,28 +206,32 @@ void dcn21_set_abm_immediate_disable(struct pipe_ctx *pipe_ctx) void dcn21_set_pipe(struct pipe_ctx *pipe_ctx) {
struct abm *abm = pipe_ctx->stream_res.abm;
- uint32_t otg_inst = pipe_ctx->stream_res.tg->inst;
+ struct timing_generator *tg = pipe_ctx->stream_res.tg;
struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
struct dmcu *dmcu = pipe_ctx->stream->ctx->dc->res_pool->dmcu;
+ uint32_t otg_inst;
+
+ if (!abm && !tg && !panel_cntl)
+ return;
+
+ otg_inst = tg->inst;
if (dmcu) {
dce110_set_pipe(pipe_ctx);
return;
}
- if (abm && panel_cntl) {
- if (abm->funcs && abm->funcs->set_pipe_ex) {
- abm->funcs->set_pipe_ex(abm,
+ if (abm->funcs && abm->funcs->set_pipe_ex) {
+ abm->funcs->set_pipe_ex(abm,
otg_inst,
SET_ABM_PIPE_NORMAL,
panel_cntl->inst,
panel_cntl->pwrseq_inst);
- } else {
- dmub_abm_set_pipe(abm, otg_inst,
- SET_ABM_PIPE_NORMAL,
- panel_cntl->inst,
- panel_cntl->pwrseq_inst);
- }
+ } else {
+ dmub_abm_set_pipe(abm, otg_inst,
+ SET_ABM_PIPE_NORMAL,
+ panel_cntl->inst,
+ panel_cntl->pwrseq_inst);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()'
2024-02-01 9:58 ` [PATCH v2] " Srinivasan Shanmugam
2024-02-01 14:23 ` Koo, Anthony
@ 2024-02-12 18:37 ` Kees Cook
2024-02-13 1:27 ` SRINIVASAN SHANMUGAM
1 sibling, 1 reply; 5+ messages in thread
From: Kees Cook @ 2024-02-12 18:37 UTC (permalink / raw)
To: Srinivasan Shanmugam
Cc: Rodrigo Siqueira, Aurabindo Pillai, Anthony Koo, Yongqiang Sun,
amd-gfx
On Thu, Feb 01, 2024 at 03:28:45PM +0530, Srinivasan Shanmugam wrote:
> In "u32 otg_inst = pipe_ctx->stream_res.tg->inst;"
> pipe_ctx->stream_res.tg could be NULL, it is relying on the caller to
> ensure the tg is not NULL.
>
> Fixes: 474ac4a875ca ("drm/amd/display: Implement some asic specific abm call backs.")
> Cc: Yongqiang Sun <yongqiang.sun@amd.com>
> Cc: Anthony Koo <Anthony.Koo@amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
> v2:
> - s/u32/uint32_t for consistency (Anthony)
>
> .../amd/display/dc/hwss/dcn21/dcn21_hwseq.c | 24 +++++++++++--------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
> index 8e88dcaf88f5..8323077bba15 100644
> --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
> @@ -206,28 +206,32 @@ void dcn21_set_abm_immediate_disable(struct pipe_ctx *pipe_ctx)
> void dcn21_set_pipe(struct pipe_ctx *pipe_ctx)
> {
> struct abm *abm = pipe_ctx->stream_res.abm;
> - uint32_t otg_inst = pipe_ctx->stream_res.tg->inst;
> + struct timing_generator *tg = pipe_ctx->stream_res.tg;
> struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
> struct dmcu *dmcu = pipe_ctx->stream->ctx->dc->res_pool->dmcu;
> + uint32_t otg_inst;
> +
> + if (!abm && !tg && !panel_cntl)
> + return;
> +
> + otg_inst = tg->inst;
Is the "if" supposed to be using "||"s instead of "&&"s? I noticed
Coverity complained "tg may be NULL" for the "tg->inst" dereference...
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()'
2024-02-12 18:37 ` Kees Cook
@ 2024-02-13 1:27 ` SRINIVASAN SHANMUGAM
0 siblings, 0 replies; 5+ messages in thread
From: SRINIVASAN SHANMUGAM @ 2024-02-13 1:27 UTC (permalink / raw)
To: Kees Cook
Cc: Rodrigo Siqueira, Aurabindo Pillai, Anthony Koo, Yongqiang Sun,
amd-gfx
On 2/13/2024 12:07 AM, Kees Cook wrote:
> On Thu, Feb 01, 2024 at 03:28:45PM +0530, Srinivasan Shanmugam wrote:
>> In "u32 otg_inst = pipe_ctx->stream_res.tg->inst;"
>> pipe_ctx->stream_res.tg could be NULL, it is relying on the caller to
>> ensure the tg is not NULL.
>>
>> Fixes: 474ac4a875ca ("drm/amd/display: Implement some asic specific abm call backs.")
>> Cc: Yongqiang Sun <yongqiang.sun@amd.com>
>> Cc: Anthony Koo <Anthony.Koo@amd.com>
>> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
>> Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
>> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
>> ---
>> v2:
>> - s/u32/uint32_t for consistency (Anthony)
>>
>> .../amd/display/dc/hwss/dcn21/dcn21_hwseq.c | 24 +++++++++++--------
>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
>> index 8e88dcaf88f5..8323077bba15 100644
>> --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
>> +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
>> @@ -206,28 +206,32 @@ void dcn21_set_abm_immediate_disable(struct pipe_ctx *pipe_ctx)
>> void dcn21_set_pipe(struct pipe_ctx *pipe_ctx)
>> {
>> struct abm *abm = pipe_ctx->stream_res.abm;
>> - uint32_t otg_inst = pipe_ctx->stream_res.tg->inst;
>> + struct timing_generator *tg = pipe_ctx->stream_res.tg;
>> struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
>> struct dmcu *dmcu = pipe_ctx->stream->ctx->dc->res_pool->dmcu;
>> + uint32_t otg_inst;
>> +
>> + if (!abm && !tg && !panel_cntl)
>> + return;
>> +
>> + otg_inst = tg->inst;
> Is the "if" supposed to be using "||"s instead of "&&"s? I noticed
> Coverity complained "tg may be NULL" for the "tg->inst" dereference...
>
> -Kees
Thanks Kees!
It is fixed in the below commit:
commit ccc1e78470efb6572a71ba88d70995e8eee2f6e5
Author: Dan Carpenter <dan.carpenter@linaro.org>
Date: Fri Feb 9 16:02:42 2024 +0300
drm/amd/display: Fix && vs || typos
These ANDs should be ORs or it will lead to a NULL dereference.
Fixes: fb5a3d037082 ("drm/amd/display: Add NULL test for 'timing
generator' in 'dcn21_set_pipe()'")
Fixes: 886571d217d7 ("drm/amd/display: Fix 'panel_cntl' could be
null in 'dcn21_set_backlight_level()'")
Reviewed-by: Anthony Koo <anthony.koo@amd.com>
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
-Srini
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-13 8:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-31 3:28 [PATCH] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()' Srinivasan Shanmugam
2024-02-01 9:58 ` [PATCH v2] " Srinivasan Shanmugam
2024-02-01 14:23 ` Koo, Anthony
2024-02-12 18:37 ` Kees Cook
2024-02-13 1:27 ` SRINIVASAN SHANMUGAM
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.