Received-SPF: pass (google.com: domain of linux-kernel+bounces-83856-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <592e5da7-7aac-4735-ae8f-625402e381ae@riseup.net>
Date: Tue, 27 Feb 2024 15:47:08 -0300
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH v3 3/9] drm/vkms: write/update the documentation for pixel
 conversion and pixel write functions
Content-Language: en-US
To: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>,
 Melissa Wen <melissa.srw@gmail.com>, =?UTF-8?Q?Ma=C3=ADra_Canal?=
 <mairacanal@riseup.net>, Haneen Mohammed <hamohammed.sa@gmail.com>,
 Daniel Vetter <daniel@ffwll.ch>,
 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
 Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>,
 David Airlie <airlied@gmail.com>, Jonathan Corbet <corbet@lwn.net>,
 pekka.paalanen@haloniitty.fi, dri-devel@lists.freedesktop.org,
 linux-kernel@vger.kernel.org, jeremie.dautheribes@bootlin.com,
 miquel.raynal@bootlin.com, thomas.petazzoni@bootlin.com,
 seanpaul@google.com, marcheu@google.com, nicolejadeyee@google.com
References: <20240226-yuv-v3-0-ff662f0994db@bootlin.com>
 <20240226-yuv-v3-3-ff662f0994db@bootlin.com>
 <406988be-48a4-4762-9c03-7a27c8e7b91e@riseup.net>
 <Zd35csjqRMstzElA@localhost.localdomain>
From: Arthur Grillo <arthurgrillo@riseup.net>
In-Reply-To: <Zd35csjqRMstzElA@localhost.localdomain>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


On 27/02/24 12:02, Louis Chauvet wrote:
> Le 26/02/24 - 10:07, Arthur Grillo a écrit :
>>
>>
>> On 26/02/24 05:46, Louis Chauvet wrote:
>>> Add some documentation on pixel conversion functions.
>>> Update of outdated comments for pixel_write functions.
>>>
>>> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
>>> ---
>>>  drivers/gpu/drm/vkms/vkms_composer.c |  4 +++
>>>  drivers/gpu/drm/vkms/vkms_drv.h      | 13 ++++++++
>>>  drivers/gpu/drm/vkms/vkms_formats.c  | 58 ++++++++++++++++++++++++++++++------
>>>  3 files changed, 66 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
>>> index c6d9b4a65809..5b341222d239 100644
>>> --- a/drivers/gpu/drm/vkms/vkms_composer.c
>>> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
>>> @@ -189,6 +189,10 @@ static void blend(struct vkms_writeback_job *wb,
>>>  
>>>  	size_t crtc_y_limit = crtc_state->base.crtc->mode.vdisplay;
>>>  
>>> +	/*
>>> +	 * The planes are composed line-by-line. It is a necessary complexity to avoid poor
>>> +	 * blending performance.
>>
>> At this moment in the series, you have not yet reintroduced the
>> line-by-line algorithm yet. Maybe it's better to add this comment when
>> you do.
> 
> Is it better with this:
> 
> 	/*
> 	 * The planes are composed line-by-line to avoid heavy memory usage. It is a necessary
> 	 * complexity to avoid poor blending performance.
> 	 *
> 	 * The function vkms_compose_row is used to read a line, pixel-by-pixel, into the staging
> 	 * buffer.
> 	 */
>  
>> Also, I think it's good to give more context, like:
>> "The planes are composed line-by-line, instead of pixel-by-pixel"
> 
> And after PATCHv3 5/9:
> 
> 	/*
> 	 * The planes are composed line-by-line to avoid heavy memory usage. It is a necessary
> 	 * complexity to avoid poor blending performance.
> 	 *
> 	 * The function pixel_read_line callback is used to read a line, using an efficient 
> 	 * algorithm for a specific format, into the staging buffer.
> 	 */
> 

Hi,

This looks good to me.

Best Regards,
~Arthur Grillo

> Kind regards,
> Louis Chauvet
> 
>> Best Regards,
>> ~Arthur Grillo
> 
> [...]
>