linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/vkms: Add information on how to benchmark
@ 2024-02-26 20:42 Arthur Grillo
  2024-02-27  9:19 ` Pekka Paalanen
  0 siblings, 1 reply; 8+ messages in thread
From: Arthur Grillo @ 2024-02-26 20:42 UTC (permalink / raw)
  To: Rodrigo Siqueira, Melissa Wen, Maíra Canal, Haneen Mohammed,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Jonathan Corbet, Pekka Paalanen
  Cc: dri-devel, linux-doc, linux-kernel, Arthur Grillo

Now that we have a defined benchmark for testing the driver, add
documentation on how to run it.

Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
---
 Documentation/gpu/vkms.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index ba04ac7c2167..6d07f79f77ff 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -89,6 +89,12 @@ You can also run subtests if you do not want to run the entire test::
   sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device "sys:/sys/devices/platform/vkms"
   sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip --run-subtest basic-plain-flip
 
+If you are developing features that may affect performance, you can run the kms_fb_stress
+benchmark::
+
+  sudo ./build/benchmarks/kms_fb_stress --device "sys:/sys/devices/platform/vkms"
+  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/benchmarks/kms_fb_stress
+
 TODO
 ====
 

---
base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
change-id: 20240226-bench-vkms-5b8b7aab255e

Best regards,
-- 
Arthur Grillo <arthurgrillo@riseup.net>


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/vkms: Add information on how to benchmark
  2024-02-26 20:42 [PATCH] drm/vkms: Add information on how to benchmark Arthur Grillo
@ 2024-02-27  9:19 ` Pekka Paalanen
  2024-02-27 11:44   ` Arthur Grillo
  0 siblings, 1 reply; 8+ messages in thread
From: Pekka Paalanen @ 2024-02-27  9:19 UTC (permalink / raw)
  To: Arthur Grillo
  Cc: Rodrigo Siqueira, Melissa Wen, Maíra Canal, Haneen Mohammed,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Jonathan Corbet, dri-devel,
	linux-doc, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]

On Mon, 26 Feb 2024 17:42:11 -0300
Arthur Grillo <arthurgrillo@riseup.net> wrote:

> Now that we have a defined benchmark for testing the driver, add
> documentation on how to run it.
> 
> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
> ---
>  Documentation/gpu/vkms.rst | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index ba04ac7c2167..6d07f79f77ff 100644
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -89,6 +89,12 @@ You can also run subtests if you do not want to run the entire test::
>    sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device "sys:/sys/devices/platform/vkms"
>    sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip --run-subtest basic-plain-flip
>  
> +If you are developing features that may affect performance, you can run the kms_fb_stress

s/can/must/

> +benchmark::

before and after, and report the numbers.

> +
> +  sudo ./build/benchmarks/kms_fb_stress --device "sys:/sys/devices/platform/vkms"
> +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/benchmarks/kms_fb_stress

Do people need to run both commands?

Anyway, a good idea.

Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>


Thanks,
pq

> +
>  TODO
>  ====
>  
> 
> ---
> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
> change-id: 20240226-bench-vkms-5b8b7aab255e
> 
> Best regards,


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/vkms: Add information on how to benchmark
  2024-02-27  9:19 ` Pekka Paalanen
@ 2024-02-27 11:44   ` Arthur Grillo
  2024-02-27 11:55     ` Pekka Paalanen
  0 siblings, 1 reply; 8+ messages in thread
From: Arthur Grillo @ 2024-02-27 11:44 UTC (permalink / raw)
  To: Pekka Paalanen
  Cc: Rodrigo Siqueira, Melissa Wen, Maíra Canal, Haneen Mohammed,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Jonathan Corbet, dri-devel,
	linux-doc, linux-kernel



On 27/02/24 06:19, Pekka Paalanen wrote:
> On Mon, 26 Feb 2024 17:42:11 -0300
> Arthur Grillo <arthurgrillo@riseup.net> wrote:
> 
>> Now that we have a defined benchmark for testing the driver, add
>> documentation on how to run it.
>>
>> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
>> ---
>>  Documentation/gpu/vkms.rst | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
>> index ba04ac7c2167..6d07f79f77ff 100644
>> --- a/Documentation/gpu/vkms.rst
>> +++ b/Documentation/gpu/vkms.rst
>> @@ -89,6 +89,12 @@ You can also run subtests if you do not want to run the entire test::
>>    sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device "sys:/sys/devices/platform/vkms"
>>    sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip --run-subtest basic-plain-flip
>>  
>> +If you are developing features that may affect performance, you can run the kms_fb_stress
> 
> s/can/must/
> 
>> +benchmark::
> 
> before and after, and report the numbers.

Did you mean to write the benchmarks logs here?

> 
>> +
>> +  sudo ./build/benchmarks/kms_fb_stress --device "sys:/sys/devices/platform/vkms"
>> +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/benchmarks/kms_fb_stress
> 
> Do people need to run both commands?

No, they don't, just two options.

Best Regards,
~Arthur Grillo

> 
> Anyway, a good idea.
> 
> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> 
> 
> Thanks,
> pq
> 
>> +
>>  TODO
>>  ====
>>  
>>
>> ---
>> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
>> change-id: 20240226-bench-vkms-5b8b7aab255e
>>
>> Best regards,
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/vkms: Add information on how to benchmark
  2024-02-27 11:44   ` Arthur Grillo
@ 2024-02-27 11:55     ` Pekka Paalanen
  2024-02-27 12:29       ` Arthur Grillo
  0 siblings, 1 reply; 8+ messages in thread
From: Pekka Paalanen @ 2024-02-27 11:55 UTC (permalink / raw)
  To: Arthur Grillo
  Cc: Rodrigo Siqueira, Melissa Wen, Maíra Canal, Haneen Mohammed,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Jonathan Corbet, dri-devel,
	linux-doc, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2300 bytes --]

On Tue, 27 Feb 2024 08:44:52 -0300
Arthur Grillo <arthurgrillo@riseup.net> wrote:

> On 27/02/24 06:19, Pekka Paalanen wrote:
> > On Mon, 26 Feb 2024 17:42:11 -0300
> > Arthur Grillo <arthurgrillo@riseup.net> wrote:
> >   
> >> Now that we have a defined benchmark for testing the driver, add
> >> documentation on how to run it.
> >>
> >> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
> >> ---
> >>  Documentation/gpu/vkms.rst | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> >> index ba04ac7c2167..6d07f79f77ff 100644
> >> --- a/Documentation/gpu/vkms.rst
> >> +++ b/Documentation/gpu/vkms.rst
> >> @@ -89,6 +89,12 @@ You can also run subtests if you do not want to run the entire test::
> >>    sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device "sys:/sys/devices/platform/vkms"
> >>    sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip --run-subtest basic-plain-flip
> >>  
> >> +If you are developing features that may affect performance, you can run the kms_fb_stress  
> > 
> > s/can/must/
> >   
> >> +benchmark::  
> > 
> > before and after, and report the numbers.  
> 
> Did you mean to write the benchmarks logs here?

I mean people should be required tell their before and after numbers in
either commit message (my preference) or in series cover letter (if
benchmarking commits is not useful).

With the addition of YUV support in VKMS, maybe the benchmark needs to
start printing YUV numbers separately as a new case.


Thanks,
pq

> 
> >   
> >> +
> >> +  sudo ./build/benchmarks/kms_fb_stress --device "sys:/sys/devices/platform/vkms"
> >> +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/benchmarks/kms_fb_stress  
> > 
> > Do people need to run both commands?  
> 
> No, they don't, just two options.
> 
> Best Regards,
> ~Arthur Grillo
> 
> > 
> > Anyway, a good idea.
> > 
> > Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> > 
> > 
> > Thanks,
> > pq
> >   
> >> +
> >>  TODO
> >>  ====
> >>  
> >>
> >> ---
> >> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
> >> change-id: 20240226-bench-vkms-5b8b7aab255e
> >>
> >> Best regards,  
> >   


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/vkms: Add information on how to benchmark
  2024-02-27 11:55     ` Pekka Paalanen
@ 2024-02-27 12:29       ` Arthur Grillo
  2024-02-27 13:26         ` Pekka Paalanen
  0 siblings, 1 reply; 8+ messages in thread
From: Arthur Grillo @ 2024-02-27 12:29 UTC (permalink / raw)
  To: Pekka Paalanen
  Cc: Rodrigo Siqueira, Melissa Wen, Maíra Canal, Haneen Mohammed,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Jonathan Corbet, dri-devel,
	linux-doc, linux-kernel



On 27/02/24 08:55, Pekka Paalanen wrote:
> On Tue, 27 Feb 2024 08:44:52 -0300
> Arthur Grillo <arthurgrillo@riseup.net> wrote:
> 
>> On 27/02/24 06:19, Pekka Paalanen wrote:
>>> On Mon, 26 Feb 2024 17:42:11 -0300
>>> Arthur Grillo <arthurgrillo@riseup.net> wrote:
>>>   
>>>> Now that we have a defined benchmark for testing the driver, add
>>>> documentation on how to run it.
>>>>
>>>> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
>>>> ---
>>>>  Documentation/gpu/vkms.rst | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
>>>> index ba04ac7c2167..6d07f79f77ff 100644
>>>> --- a/Documentation/gpu/vkms.rst
>>>> +++ b/Documentation/gpu/vkms.rst
>>>> @@ -89,6 +89,12 @@ You can also run subtests if you do not want to run the entire test::
>>>>    sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device "sys:/sys/devices/platform/vkms"
>>>>    sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip --run-subtest basic-plain-flip
>>>>  
>>>> +If you are developing features that may affect performance, you can run the kms_fb_stress  
>>>
>>> s/can/must/
>>>   
>>>> +benchmark::  
>>>
>>> before and after, and report the numbers.  
>>
>> Did you mean to write the benchmarks logs here?
> 
> I mean people should be required tell their before and after numbers in
> either commit message (my preference) or in series cover letter (if
> benchmarking commits is not useful).
> 
> With the addition of YUV support in VKMS, maybe the benchmark needs to

With the upcoming addition, I've sent a patch to arbitrarily change the
formats on the benchmark via command-line options. It's not adding a new
case, but maybe just this could already help.

https://lore.kernel.org/all/20240226-kms_fb_stress-dev-v1-0-1c14942b1244@riseup.net/

Best Regards,
~Arthur Grillo

> start printing YUV numbers separately as a new case.
> 
> 
> Thanks,
> pq
> 
>>
>>>   
>>>> +
>>>> +  sudo ./build/benchmarks/kms_fb_stress --device "sys:/sys/devices/platform/vkms"
>>>> +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/benchmarks/kms_fb_stress  
>>>
>>> Do people need to run both commands?  
>>
>> No, they don't, just two options.
>>
>> Best Regards,
>> ~Arthur Grillo
>>
>>>
>>> Anyway, a good idea.
>>>
>>> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
>>>
>>>
>>> Thanks,
>>> pq
>>>   
>>>> +
>>>>  TODO
>>>>  ====
>>>>  
>>>>
>>>> ---
>>>> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
>>>> change-id: 20240226-bench-vkms-5b8b7aab255e
>>>>
>>>> Best regards,  
>>>   
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/vkms: Add information on how to benchmark
  2024-02-27 12:29       ` Arthur Grillo
@ 2024-02-27 13:26         ` Pekka Paalanen
  2024-02-27 15:02           ` Louis Chauvet
  0 siblings, 1 reply; 8+ messages in thread
From: Pekka Paalanen @ 2024-02-27 13:26 UTC (permalink / raw)
  To: Arthur Grillo
  Cc: Rodrigo Siqueira, Melissa Wen, Maíra Canal, Haneen Mohammed,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Jonathan Corbet, dri-devel,
	linux-doc, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3624 bytes --]

On Tue, 27 Feb 2024 09:29:58 -0300
Arthur Grillo <arthurgrillo@riseup.net> wrote:

> On 27/02/24 08:55, Pekka Paalanen wrote:
> > On Tue, 27 Feb 2024 08:44:52 -0300
> > Arthur Grillo <arthurgrillo@riseup.net> wrote:
> >   
> >> On 27/02/24 06:19, Pekka Paalanen wrote:  
> >>> On Mon, 26 Feb 2024 17:42:11 -0300
> >>> Arthur Grillo <arthurgrillo@riseup.net> wrote:
> >>>     
> >>>> Now that we have a defined benchmark for testing the driver, add
> >>>> documentation on how to run it.
> >>>>
> >>>> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
> >>>> ---
> >>>>  Documentation/gpu/vkms.rst | 6 ++++++
> >>>>  1 file changed, 6 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> >>>> index ba04ac7c2167..6d07f79f77ff 100644
> >>>> --- a/Documentation/gpu/vkms.rst
> >>>> +++ b/Documentation/gpu/vkms.rst
> >>>> @@ -89,6 +89,12 @@ You can also run subtests if you do not want to run the entire test::
> >>>>    sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device "sys:/sys/devices/platform/vkms"
> >>>>    sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip --run-subtest basic-plain-flip
> >>>>  
> >>>> +If you are developing features that may affect performance, you can run the kms_fb_stress    
> >>>
> >>> s/can/must/
> >>>     
> >>>> +benchmark::    
> >>>
> >>> before and after, and report the numbers.    
> >>
> >> Did you mean to write the benchmarks logs here?  
> > 
> > I mean people should be required tell their before and after numbers in
> > either commit message (my preference) or in series cover letter (if
> > benchmarking commits is not useful).
> > 
> > With the addition of YUV support in VKMS, maybe the benchmark needs to  
> 
> With the upcoming addition, I've sent a patch to arbitrarily change the
> formats on the benchmark via command-line options. It's not adding a new
> case, but maybe just this could already help.
> 
> https://lore.kernel.org/all/20240226-kms_fb_stress-dev-v1-0-1c14942b1244@riseup.net/

In that case you would need to document exactly what command line
options to use, and ask people to report the numbers of each test
case.

That works. Alternatively or additionally, the test cases could be
built in to the benchmark, and it just reports numbers for all of them
in a single invocation. Then people running the standard benchmark do
not need to worry about getting the command line options right, or
running multiple cases. And reviewers do not need to ask to re-run with
the correct options.

I suppose rotations might get added, too.

Or maybe you'd provide a script that covers all the standard
performance test cases?


Thanks,
pq

> > start printing YUV numbers separately as a new case.
> > 
> > 
> > Thanks,
> > pq
> >   
> >>  
> >>>     
> >>>> +
> >>>> +  sudo ./build/benchmarks/kms_fb_stress --device "sys:/sys/devices/platform/vkms"
> >>>> +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/benchmarks/kms_fb_stress    
> >>>
> >>> Do people need to run both commands?    
> >>
> >> No, they don't, just two options.
> >>
> >> Best Regards,
> >> ~Arthur Grillo
> >>  
> >>>
> >>> Anyway, a good idea.
> >>>
> >>> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> >>>
> >>>
> >>> Thanks,
> >>> pq
> >>>     
> >>>> +
> >>>>  TODO
> >>>>  ====
> >>>>  
> >>>>
> >>>> ---
> >>>> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
> >>>> change-id: 20240226-bench-vkms-5b8b7aab255e
> >>>>
> >>>> Best regards,    
> >>>     
> >   


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/vkms: Add information on how to benchmark
  2024-02-27 13:26         ` Pekka Paalanen
@ 2024-02-27 15:02           ` Louis Chauvet
  2024-02-27 16:52             ` Arthur Grillo
  0 siblings, 1 reply; 8+ messages in thread
From: Louis Chauvet @ 2024-02-27 15:02 UTC (permalink / raw)
  To: Pekka Paalanen
  Cc: Arthur Grillo, Rodrigo Siqueira, Melissa Wen, Maíra Canal,
	Haneen Mohammed, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Jonathan Corbet, dri-devel,
	linux-doc, linux-kernel

Le 27/02/24 - 15:26, Pekka Paalanen a écrit :
> On Tue, 27 Feb 2024 09:29:58 -0300
> Arthur Grillo <arthurgrillo@riseup.net> wrote:
> 
> > On 27/02/24 08:55, Pekka Paalanen wrote:
> > > On Tue, 27 Feb 2024 08:44:52 -0300
> > > Arthur Grillo <arthurgrillo@riseup.net> wrote:
> > >   
> > >> On 27/02/24 06:19, Pekka Paalanen wrote:  
> > >>> On Mon, 26 Feb 2024 17:42:11 -0300
> > >>> Arthur Grillo <arthurgrillo@riseup.net> wrote:
> > >>>     
> > >>>> Now that we have a defined benchmark for testing the driver, add
> > >>>> documentation on how to run it.
> > >>>>
> > >>>> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
> > >>>> ---
> > >>>>  Documentation/gpu/vkms.rst | 6 ++++++
> > >>>>  1 file changed, 6 insertions(+)
> > >>>>
> > >>>> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> > >>>> index ba04ac7c2167..6d07f79f77ff 100644
> > >>>> --- a/Documentation/gpu/vkms.rst
> > >>>> +++ b/Documentation/gpu/vkms.rst
> > >>>> @@ -89,6 +89,12 @@ You can also run subtests if you do not want to run the entire test::
> > >>>>    sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device "sys:/sys/devices/platform/vkms"
> > >>>>    sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip --run-subtest basic-plain-flip
> > >>>>  
> > >>>> +If you are developing features that may affect performance, you can run the kms_fb_stress    
> > >>>
> > >>> s/can/must/
> > >>>     
> > >>>> +benchmark::    
> > >>>
> > >>> before and after, and report the numbers.    
> > >>
> > >> Did you mean to write the benchmarks logs here?  
> > > 
> > > I mean people should be required tell their before and after numbers in
> > > either commit message (my preference) or in series cover letter (if
> > > benchmarking commits is not useful).
> > > 
> > > With the addition of YUV support in VKMS, maybe the benchmark needs to  
> > 
> > With the upcoming addition, I've sent a patch to arbitrarily change the
> > formats on the benchmark via command-line options. It's not adding a new
> > case, but maybe just this could already help.
> > 
> > https://lore.kernel.org/all/20240226-kms_fb_stress-dev-v1-0-1c14942b1244@riseup.net/
> 
> In that case you would need to document exactly what command line
> options to use, and ask people to report the numbers of each test
> case.
> 
> That works. Alternatively or additionally, the test cases could be
> built in to the benchmark, and it just reports numbers for all of them
> in a single invocation. Then people running the standard benchmark do
> not need to worry about getting the command line options right, or
> running multiple cases. And reviewers do not need to ask to re-run with
> the correct options.
> 
> I suppose rotations might get added, too.
> 
> Or maybe you'd provide a script that covers all the standard
> performance test cases?

I agree with Pekka, it would be nice to have a simple "bench everything" 
tool. Like kms_rotation is a test for all rotations, kms_plane for color 
conversions... kms_fb_test can run a few combinations (rgb+norotation, 
rgb+yuv, rgb+rotation...) and report a "global result" (this way it's easy 
to spot a regression not related directly to your changes).

I don't know the IGT benchmark API, but I think there is a way to create 
"sub-benchmarks" so you can run a specific benchmark when developping and 
the whole thing before pushing your series.

Kind regards,
Louis Chauvet

> 
> Thanks,
> pq
> 
> > > start printing YUV numbers separately as a new case.
> > > 
> > > 
> > > Thanks,
> > > pq
> > >   
> > >>  
> > >>>     
> > >>>> +
> > >>>> +  sudo ./build/benchmarks/kms_fb_stress --device "sys:/sys/devices/platform/vkms"
> > >>>> +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/benchmarks/kms_fb_stress    
> > >>>
> > >>> Do people need to run both commands?    
> > >>
> > >> No, they don't, just two options.
> > >>
> > >> Best Regards,
> > >> ~Arthur Grillo
> > >>  
> > >>>
> > >>> Anyway, a good idea.
> > >>>
> > >>> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> > >>>
> > >>>
> > >>> Thanks,
> > >>> pq
> > >>>     
> > >>>> +
> > >>>>  TODO
> > >>>>  ====
> > >>>>  
> > >>>>
> > >>>> ---
> > >>>> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
> > >>>> change-id: 20240226-bench-vkms-5b8b7aab255e
> > >>>>
> > >>>> Best regards,    
> > >>>     
> > >   
> 



-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/vkms: Add information on how to benchmark
  2024-02-27 15:02           ` Louis Chauvet
@ 2024-02-27 16:52             ` Arthur Grillo
  0 siblings, 0 replies; 8+ messages in thread
From: Arthur Grillo @ 2024-02-27 16:52 UTC (permalink / raw)
  To: Pekka Paalanen, Rodrigo Siqueira, Melissa Wen, Maíra Canal,
	Haneen Mohammed, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Jonathan Corbet, dri-devel,
	linux-doc, linux-kernel



On 27/02/24 12:02, Louis Chauvet wrote:
> Le 27/02/24 - 15:26, Pekka Paalanen a écrit :
>> On Tue, 27 Feb 2024 09:29:58 -0300
>> Arthur Grillo <arthurgrillo@riseup.net> wrote:
>>
>>> On 27/02/24 08:55, Pekka Paalanen wrote:
>>>> On Tue, 27 Feb 2024 08:44:52 -0300
>>>> Arthur Grillo <arthurgrillo@riseup.net> wrote:
>>>>   
>>>>> On 27/02/24 06:19, Pekka Paalanen wrote:  
>>>>>> On Mon, 26 Feb 2024 17:42:11 -0300
>>>>>> Arthur Grillo <arthurgrillo@riseup.net> wrote:
>>>>>>     
>>>>>>> Now that we have a defined benchmark for testing the driver, add
>>>>>>> documentation on how to run it.
>>>>>>>
>>>>>>> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
>>>>>>> ---
>>>>>>>  Documentation/gpu/vkms.rst | 6 ++++++
>>>>>>>  1 file changed, 6 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
>>>>>>> index ba04ac7c2167..6d07f79f77ff 100644
>>>>>>> --- a/Documentation/gpu/vkms.rst
>>>>>>> +++ b/Documentation/gpu/vkms.rst
>>>>>>> @@ -89,6 +89,12 @@ You can also run subtests if you do not want to run the entire test::
>>>>>>>    sudo ./build/tests/kms_flip --run-subtest basic-plain-flip --device "sys:/sys/devices/platform/vkms"
>>>>>>>    sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/tests/kms_flip --run-subtest basic-plain-flip
>>>>>>>  
>>>>>>> +If you are developing features that may affect performance, you can run the kms_fb_stress    
>>>>>>
>>>>>> s/can/must/
>>>>>>     
>>>>>>> +benchmark::    
>>>>>>
>>>>>> before and after, and report the numbers.    
>>>>>
>>>>> Did you mean to write the benchmarks logs here?  
>>>>
>>>> I mean people should be required tell their before and after numbers in
>>>> either commit message (my preference) or in series cover letter (if
>>>> benchmarking commits is not useful).
>>>>
>>>> With the addition of YUV support in VKMS, maybe the benchmark needs to  
>>>
>>> With the upcoming addition, I've sent a patch to arbitrarily change the
>>> formats on the benchmark via command-line options. It's not adding a new
>>> case, but maybe just this could already help.
>>>
>>> https://lore.kernel.org/all/20240226-kms_fb_stress-dev-v1-0-1c14942b1244@riseup.net/
>>
>> In that case you would need to document exactly what command line
>> options to use, and ask people to report the numbers of each test
>> case.
>>
>> That works. Alternatively or additionally, the test cases could be
>> built in to the benchmark, and it just reports numbers for all of them
>> in a single invocation. Then people running the standard benchmark do
>> not need to worry about getting the command line options right, or
>> running multiple cases. And reviewers do not need to ask to re-run with
>> the correct options.
>>
>> I suppose rotations might get added, too.
>>
>> Or maybe you'd provide a script that covers all the standard
>> performance test cases?
> 
> I agree with Pekka, it would be nice to have a simple "bench everything" 
> tool. Like kms_rotation is a test for all rotations, kms_plane for color 
> conversions... kms_fb_test can run a few combinations (rgb+norotation, 
> rgb+yuv, rgb+rotation...) and report a "global result" (this way it's easy 
> to spot a regression not related directly to your changes).
> 
> I don't know the IGT benchmark API, but I think there is a way to create 
> "sub-benchmarks" so you can run a specific benchmark when developping and 
> the whole thing before pushing your series.
> 
> Kind regards,
> Louis Chauvet
> 

Thank you guys for the ideas!

I'm going to work on something along on those lines, having combinations
of formats, rotations and whatever property that might be useful for
stress testing the driver. Maybe query the driver for the supported
properties (e.g. what formats and rotations it supports) and do many/all
possible combinations.

Best Regards,
~Arthur Grillo


>>
>> Thanks,
>> pq
>>
>>>> start printing YUV numbers separately as a new case.
>>>>
>>>>
>>>> Thanks,
>>>> pq
>>>>   
>>>>>  
>>>>>>     
>>>>>>> +
>>>>>>> +  sudo ./build/benchmarks/kms_fb_stress --device "sys:/sys/devices/platform/vkms"
>>>>>>> +  sudo IGT_DEVICE="sys:/sys/devices/platform/vkms" ./build/benchmarks/kms_fb_stress    
>>>>>>
>>>>>> Do people need to run both commands?    
>>>>>
>>>>> No, they don't, just two options.
>>>>>
>>>>> Best Regards,
>>>>> ~Arthur Grillo
>>>>>  
>>>>>>
>>>>>> Anyway, a good idea.
>>>>>>
>>>>>> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> pq
>>>>>>     
>>>>>>> +
>>>>>>>  TODO
>>>>>>>  ====
>>>>>>>  
>>>>>>>
>>>>>>> ---
>>>>>>> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
>>>>>>> change-id: 20240226-bench-vkms-5b8b7aab255e
>>>>>>>
>>>>>>> Best regards,    
>>>>>>     
>>>>   
>>
> 
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-02-27 16:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-26 20:42 [PATCH] drm/vkms: Add information on how to benchmark Arthur Grillo
2024-02-27  9:19 ` Pekka Paalanen
2024-02-27 11:44   ` Arthur Grillo
2024-02-27 11:55     ` Pekka Paalanen
2024-02-27 12:29       ` Arthur Grillo
2024-02-27 13:26         ` Pekka Paalanen
2024-02-27 15:02           ` Louis Chauvet
2024-02-27 16:52             ` Arthur Grillo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).