* [bug report] drm/amd/display: If one stream full updates, full update all planes
@ 2019-04-23 14:10 Dan Carpenter
2019-04-23 14:16 ` Kazlauskas, Nicholas
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2019-04-23 14:10 UTC (permalink / raw)
To: David.Francis-5C7GfCeVMHo; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Hello David Francis,
This is a semi-automatic email about new static checker warnings.
The patch c238bfe0be9e: "drm/amd/display: If one stream full updates,
full update all planes" from Mar 29, 2019, leads to the following
Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1492 det_surface_update()
warn: variable dereferenced before check 'u->surface' (see line 1467)
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c
1466
1467 if (u->surface->force_full_update) {
^^^^^^^^^^^^
The patch adds a new dereference
1468 update_flags->bits.full_update = 1;
1469 return UPDATE_TYPE_FULL;
1470 }
1471
1472 type = get_plane_info_update_type(u);
1473 elevate_update_type(&overall_type, type);
1474
1475 type = get_scaling_info_update_type(u);
1476 elevate_update_type(&overall_type, type);
1477
1478 if (u->in_transfer_func)
1479 update_flags->bits.in_transfer_func_change = 1;
1480
1481 if (u->input_csc_color_matrix)
1482 update_flags->bits.input_csc_change = 1;
1483
1484 if (u->coeff_reduction_factor)
1485 update_flags->bits.coeff_reduction_change = 1;
1486
1487 if (u->gamma) {
1488 enum surface_pixel_format format = SURFACE_PIXEL_FORMAT_GRPH_BEGIN;
1489
1490 if (u->plane_info)
1491 format = u->plane_info->format;
1492 else if (u->surface)
^^^^^^^^^^
But the old code assumed it could be NULL
1493 format = u->surface->format;
1494
regards,
dan carpenter
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] drm/amd/display: If one stream full updates, full update all planes
2019-04-23 14:10 [bug report] drm/amd/display: If one stream full updates, full update all planes Dan Carpenter
@ 2019-04-23 14:16 ` Kazlauskas, Nicholas
[not found] ` <c5d7c58c-3c38-a2e7-4584-df3ac347de95-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Kazlauskas, Nicholas @ 2019-04-23 14:16 UTC (permalink / raw)
To: Dan Carpenter, Francis, David; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 4/23/19 10:10 AM, Dan Carpenter wrote:
> Hello David Francis,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch c238bfe0be9e: "drm/amd/display: If one stream full updates,
> full update all planes" from Mar 29, 2019, leads to the following
> Smatch complaint:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1492 det_surface_update()
> warn: variable dereferenced before check 'u->surface' (see line 1467)
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c
> 1466
> 1467 if (u->surface->force_full_update) {
> ^^^^^^^^^^^^
> The patch adds a new dereference
>
> 1468 update_flags->bits.full_update = 1;
> 1469 return UPDATE_TYPE_FULL;
> 1470 }
> 1471
> 1472 type = get_plane_info_update_type(u);
> 1473 elevate_update_type(&overall_type, type);
> 1474
> 1475 type = get_scaling_info_update_type(u);
> 1476 elevate_update_type(&overall_type, type);
> 1477
> 1478 if (u->in_transfer_func)
> 1479 update_flags->bits.in_transfer_func_change = 1;
> 1480
> 1481 if (u->input_csc_color_matrix)
> 1482 update_flags->bits.input_csc_change = 1;
> 1483
> 1484 if (u->coeff_reduction_factor)
> 1485 update_flags->bits.coeff_reduction_change = 1;
> 1486
> 1487 if (u->gamma) {
> 1488 enum surface_pixel_format format = SURFACE_PIXEL_FORMAT_GRPH_BEGIN;
> 1489
> 1490 if (u->plane_info)
> 1491 format = u->plane_info->format;
> 1492 else if (u->surface)
> ^^^^^^^^^^
> But the old code assumed it could be NULL
This looks like a check that could probably be dropped. This function
shouldn't be called with a u->surface = NULL - there's even that other
deference at the top of the function with &u->surface->update_flags;
Nicholas Kazlauskas
>
> 1493 format = u->surface->format;
> 1494
>
> regards,
> dan carpenter
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] drm/amd/display: If one stream full updates, full update all planes
[not found] ` <c5d7c58c-3c38-a2e7-4584-df3ac347de95-5C7GfCeVMHo@public.gmane.org>
@ 2019-04-23 15:20 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2019-04-23 15:20 UTC (permalink / raw)
To: Kazlauskas, Nicholas
Cc: Francis, David, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Tue, Apr 23, 2019 at 02:16:53PM +0000, Kazlauskas, Nicholas wrote:
> On 4/23/19 10:10 AM, Dan Carpenter wrote:
> > Hello David Francis,
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > The patch c238bfe0be9e: "drm/amd/display: If one stream full updates,
> > full update all planes" from Mar 29, 2019, leads to the following
> > Smatch complaint:
> >
> > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1492 det_surface_update()
> > warn: variable dereferenced before check 'u->surface' (see line 1467)
> >
> > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c
> > 1466
> > 1467 if (u->surface->force_full_update) {
> > ^^^^^^^^^^^^
> > The patch adds a new dereference
> >
> > 1468 update_flags->bits.full_update = 1;
> > 1469 return UPDATE_TYPE_FULL;
> > 1470 }
> > 1471
> > 1472 type = get_plane_info_update_type(u);
> > 1473 elevate_update_type(&overall_type, type);
> > 1474
> > 1475 type = get_scaling_info_update_type(u);
> > 1476 elevate_update_type(&overall_type, type);
> > 1477
> > 1478 if (u->in_transfer_func)
> > 1479 update_flags->bits.in_transfer_func_change = 1;
> > 1480
> > 1481 if (u->input_csc_color_matrix)
> > 1482 update_flags->bits.input_csc_change = 1;
> > 1483
> > 1484 if (u->coeff_reduction_factor)
> > 1485 update_flags->bits.coeff_reduction_change = 1;
> > 1486
> > 1487 if (u->gamma) {
> > 1488 enum surface_pixel_format format = SURFACE_PIXEL_FORMAT_GRPH_BEGIN;
> > 1489
> > 1490 if (u->plane_info)
> > 1491 format = u->plane_info->format;
> > 1492 else if (u->surface)
> > ^^^^^^^^^^
> > But the old code assumed it could be NULL
>
> This looks like a check that could probably be dropped. This function
> shouldn't be called with a u->surface = NULL - there's even that other
> deference at the top of the function with &u->surface->update_flags;
>
Yeah... Techincally, that's not a dereference though so it doesn't
generate a static checker warning. (It's just taking the address).
But then we dereference the address on the next line when we do
"update_flags->raw = 0;". That code is complicated for static analysis.
regards,
dan carpenter
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-23 15:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-23 14:10 [bug report] drm/amd/display: If one stream full updates, full update all planes Dan Carpenter
2019-04-23 14:16 ` Kazlauskas, Nicholas
[not found] ` <c5d7c58c-3c38-a2e7-4584-df3ac347de95-5C7GfCeVMHo@public.gmane.org>
2019-04-23 15:20 ` Dan Carpenter
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.