| Message ID | 20230420135901.3889747-1-trix@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/display: return status of dmub_srv_get_fw_boot_status | expand |
On 4/20/23 09:59, Tom Rix wrote: > gcc with W=1 reports > drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c: > In function ‘dc_dmub_srv_optimized_init_done’: > drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:184:26: > error: variable ‘dmub’ set but not used [-Werror=unused-but-set-variable] > 184 | struct dmub_srv *dmub; > | ^~~~ > > The return status is never set. > It looks like a call to dmub_srv_get_fw_boot_status is missing. > > Fixes: 499e4b1c722e ("drm/amd/display: add mechanism to skip DCN init") What tree is this based on? I am unable to find that exact commit on amd-staging-drm-next. > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > index d15ec32243e2..36d936ab4300 100644 > --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > @@ -182,14 +182,23 @@ bool dc_dmub_srv_cmd_run_list(struct dc_dmub_srv *dc_dmub_srv, unsigned int coun > bool dc_dmub_srv_optimized_init_done(struct dc_dmub_srv *dc_dmub_srv) > { > struct dmub_srv *dmub; > - union dmub_fw_boot_status status; > + struct dc_context *dc_ctx; > + union dmub_fw_boot_status boot_status; > + enum dmub_status status; > > if (!dc_dmub_srv || !dc_dmub_srv->dmub) > return false; > > dmub = dc_dmub_srv->dmub; > + dc_ctx = dc_dmub_srv->ctx; > + > + status = dmub_srv_get_fw_boot_status(dmub, &boot_status); > + if (status != DMUB_STATUS_OK) { > + DC_ERROR("Error querying DMUB boot status: error=%d\n", status); > + return false; > + } > > - return status.bits.optimized_init_done; > + return boot_status.bits.optimized_init_done; > } > > bool dc_dmub_srv_notify_stream_mask(struct dc_dmub_srv *dc_dmub_srv,
On 4/24/23 10:02 AM, Hamza Mahfooz wrote: > > On 4/20/23 09:59, Tom Rix wrote: >> gcc with W=1 reports >> drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c: >> In function ‘dc_dmub_srv_optimized_init_done’: >> drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:184:26: >> error: variable ‘dmub’ set but not used >> [-Werror=unused-but-set-variable] >> 184 | struct dmub_srv *dmub; >> | ^~~~ >> >> The return status is never set. >> It looks like a call to dmub_srv_get_fw_boot_status is missing. >> >> Fixes: 499e4b1c722e ("drm/amd/display: add mechanism to skip DCN init") > > What tree is this based on? I am unable to find that exact commit on > amd-staging-drm-next. linux-next Tom > >> Signed-off-by: Tom Rix <trix@redhat.com> >> --- >> drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c >> b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c >> index d15ec32243e2..36d936ab4300 100644 >> --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c >> +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c >> @@ -182,14 +182,23 @@ bool dc_dmub_srv_cmd_run_list(struct >> dc_dmub_srv *dc_dmub_srv, unsigned int coun >> bool dc_dmub_srv_optimized_init_done(struct dc_dmub_srv *dc_dmub_srv) >> { >> struct dmub_srv *dmub; >> - union dmub_fw_boot_status status; >> + struct dc_context *dc_ctx; >> + union dmub_fw_boot_status boot_status; >> + enum dmub_status status; >> if (!dc_dmub_srv || !dc_dmub_srv->dmub) >> return false; >> dmub = dc_dmub_srv->dmub; >> + dc_ctx = dc_dmub_srv->ctx; >> + >> + status = dmub_srv_get_fw_boot_status(dmub, &boot_status); >> + if (status != DMUB_STATUS_OK) { >> + DC_ERROR("Error querying DMUB boot status: error=%d\n", >> status); >> + return false; >> + } >> - return status.bits.optimized_init_done; >> + return boot_status.bits.optimized_init_done; >> } >> bool dc_dmub_srv_notify_stream_mask(struct dc_dmub_srv *dc_dmub_srv,
On 4/20/23 09:59, Tom Rix wrote: > gcc with W=1 reports > drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c: > In function ‘dc_dmub_srv_optimized_init_done’: > drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:184:26: > error: variable ‘dmub’ set but not used [-Werror=unused-but-set-variable] > 184 | struct dmub_srv *dmub; > | ^~~~ > > The return status is never set. > It looks like a call to dmub_srv_get_fw_boot_status is missing. > > Fixes: 499e4b1c722e ("drm/amd/display: add mechanism to skip DCN init") > Signed-off-by: Tom Rix <trix@redhat.com> Applied, thanks! > --- > drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > index d15ec32243e2..36d936ab4300 100644 > --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > @@ -182,14 +182,23 @@ bool dc_dmub_srv_cmd_run_list(struct dc_dmub_srv *dc_dmub_srv, unsigned int coun > bool dc_dmub_srv_optimized_init_done(struct dc_dmub_srv *dc_dmub_srv) > { > struct dmub_srv *dmub; > - union dmub_fw_boot_status status; > + struct dc_context *dc_ctx; > + union dmub_fw_boot_status boot_status; > + enum dmub_status status; > > if (!dc_dmub_srv || !dc_dmub_srv->dmub) > return false; > > dmub = dc_dmub_srv->dmub; > + dc_ctx = dc_dmub_srv->ctx; > + > + status = dmub_srv_get_fw_boot_status(dmub, &boot_status); > + if (status != DMUB_STATUS_OK) { > + DC_ERROR("Error querying DMUB boot status: error=%d\n", status); > + return false; > + } > > - return status.bits.optimized_init_done; > + return boot_status.bits.optimized_init_done; > } > > bool dc_dmub_srv_notify_stream_mask(struct dc_dmub_srv *dc_dmub_srv,
diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c index d15ec32243e2..36d936ab4300 100644 --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c @@ -182,14 +182,23 @@ bool dc_dmub_srv_cmd_run_list(struct dc_dmub_srv *dc_dmub_srv, unsigned int coun bool dc_dmub_srv_optimized_init_done(struct dc_dmub_srv *dc_dmub_srv) { struct dmub_srv *dmub; - union dmub_fw_boot_status status; + struct dc_context *dc_ctx; + union dmub_fw_boot_status boot_status; + enum dmub_status status; if (!dc_dmub_srv || !dc_dmub_srv->dmub) return false; dmub = dc_dmub_srv->dmub; + dc_ctx = dc_dmub_srv->ctx; + + status = dmub_srv_get_fw_boot_status(dmub, &boot_status); + if (status != DMUB_STATUS_OK) { + DC_ERROR("Error querying DMUB boot status: error=%d\n", status); + return false; + } - return status.bits.optimized_init_done; + return boot_status.bits.optimized_init_done; } bool dc_dmub_srv_notify_stream_mask(struct dc_dmub_srv *dc_dmub_srv,
gcc with W=1 reports drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c: In function ‘dc_dmub_srv_optimized_init_done’: drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:184:26: error: variable ‘dmub’ set but not used [-Werror=unused-but-set-variable] 184 | struct dmub_srv *dmub; | ^~~~ The return status is never set. It looks like a call to dmub_srv_get_fw_boot_status is missing. Fixes: 499e4b1c722e ("drm/amd/display: add mechanism to skip DCN init") Signed-off-by: Tom Rix <trix@redhat.com> --- drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)