[PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[Carlos]

Hi Maxime, thanks for the reviews!

On 7/19/23 04:49, Maxime Ripard wrote:
> Hi,
>
> On Tue, Jul 18, 2023 at 03:17:24PM -0300, Carlos Eduardo Gallo Filho wrote:
>> Add a parametrized test for the drm_framebuffer_check_src_coords function.
>>
>> Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos at disroot.org>
>> ---
>>   drivers/gpu/drm/tests/drm_framebuffer_test.c | 126 +++++++++++++++++++
>>   1 file changed, 126 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c
>> index f759d9f3b76e..ee92120cd8e9 100644
>> --- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
>> +++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
>> @@ -9,6 +9,7 @@
>>   
>>   #include <drm/drm_device.h>
>>   #include <drm/drm_mode.h>
>> +#include <drm/drm_framebuffer.h>
>>   #include <drm/drm_fourcc.h>
>>   #include <drm/drm_print.h>
>>   
>> @@ -366,7 +367,132 @@ static void drm_framebuffer_test_to_desc(const struct drm_framebuffer_test *t, c
>>   KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases,
>>   		  drm_framebuffer_test_to_desc);
>>   
>> +/* Parameters for testing drm_framebuffer_check_src_coords function */
>> +struct check_src_coords_case {
>> +	const char *name; /* Description of the parameter case */
>> +	const int expect; /* Expected returned value by the function */
>> +
>> +	/* All function args */
>> +	const uint32_t src_x;
>> +	const uint32_t src_y;
>> +	const uint32_t src_w;
>> +	const uint32_t src_h;
>> +	const struct drm_framebuffer fb;
>> +};
>> +
>> +static const struct check_src_coords_case check_src_coords_cases[] = {
>> +	/* Regular case where the source just fit in the framebuffer */
>> +	{ .name = "source inside framebuffer with normal sizes and coordinates",
>> +	  .expect = 0,
>> +	  .src_x = 500 << 16, .src_y = 700 << 16,
>> +	  .src_w = 100 << 16, .src_h = 100 << 16,
> I don't think we need to duplicate the << 16 everywhere, this can be
> added by the test function.
>
> Maxime
I thought about it, but there's some cases where we have some values 
composed
by both a shifted part and a regular one, like ".src_x = (600 << 16) + 
1". If we
left to shift everything on the test function we won't be able to have 
this kind
of values, which would compromise the test. Or if we just put off the 
regular part,
we will deal with a test that won't cover the out-of-bound cases at the 
subpixel
level.

Of course this could be implemented by adding some new members to the 
case struct,
being each src_{x,y,w,h} composed by two, where one is always shifted, 
though I
guess it would be way worse than having the shifts everywhere on the 
cases array.

I'll be happy to know about if you have some suggestion of how it can 
implemented
without throwing away the non-shifted part!

Thanks,
Carlos