Re: [PATCH 07/12] drm/panic: Retry in dispatch function if panic output fails

Am 18.08.26 um 15:05 schrieb [email protected]:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Removing the internal fallback from `draw_panic_screen_qr_code` breaks the `drm_panic` KUnit tests.

I think that's exactly the right thing to do. When we unit-test 
draw_panic_screen_qr_code() , we better not fallback silently to 
something that works; thus masking the error.
> --
>
> --- Patch [7]: [PATCH 07/12] drm/panic: Retry in dispatch function if panic output fails ---
> commit 23392eb57457ad383d9c4741f6fc2f3c107aabb3
> Author: Thomas Zimmermann <[email protected]>
>
> drm/panic: Retry in dispatch function if panic output fails
>
> On failed output of QR codes and kmsg buffers, fall back to a user
> message. QR-code output already did this internally, but now formally
> support it in drm_panic_dispatch(). Streamlines the existing logic.
>
>> --- a/drivers/gpu/drm/drm_panic.c
>> +++ b/drivers/gpu/drm/drm_panic.c
> [ ... ]
>> @@ -816,15 +816,6 @@ static int _draw_panic_screen_qr_code(struct drm_scanout_buffer *sb,
>>   	drm_panic_blit(sb, &r_qr, qr_image, qr_pitch, scale, fg_color);
>>   	return 0;
>>   }
>> -
>> -static int draw_panic_screen_qr_code(struct drm_scanout_buffer *sb,
>> -				     u32 fg_color, u32 bg_color,
>> -				     unsigned int qr_version)
>> -{
>> -	if (_draw_panic_screen_qr_code(sb, fg_color, bg_color, qr_version))
>> -		draw_panic_screen_user(sb, fg_color, bg_color);
>> -	return 0;
>> -}
> [Severity: Medium]
> Does removing this fallback cause the drm_panic KUnit tests to fail?
>
> Looking at the test suite in drivers/gpu/drm/tests/drm_panic_test.c, it
> calls this function directly without going through the new dispatch logic:
>
> static int draw_panic_screen_qr_code_default(struct drm_scanout_buffer *sb)
> {
> 	return draw_panic_screen_qr_code(sb, 0x00ffffff, 0x00000000, 40);
> }
>
> When tested with a small resolution like 300x200, the scale evaluates to 0
> and returns -ENOSPC. Previously, this wrapper caught the error, fell back
> to draw_panic_screen_user, and safely returned 0, satisfying the KUnit
> check.
>
> Since the fallback was moved to draw_panic_dispatch and the test calls
> the lower-level function directly, will this now propagate the -ENOSPC
> error and spuriously fail the test?
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)