[PATCH 14/14] Documentation: Update the todo

Anusha Srivatsa posted 14 patches 1 year ago
There is a newer version of this series
[PATCH 14/14] Documentation: Update the todo
Posted by Anusha Srivatsa 1 year ago
Remove the TODO now that this series addresses
the changes needed.

Cc: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 Documentation/gpu/todo.rst | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 256d0d1cb2164bd94f9b610a751b907834d96a21..b5aa5f776cfe0916b98bddf791c65abe974834cf 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -441,21 +441,6 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>
 
 Level: Intermediate
 
-Request memory regions in all drivers
--------------------------------------
-
-Go through all drivers and add code to request the memory regions that the
-driver uses. This requires adding calls to request_mem_region(),
-pci_request_region() or similar functions. Use helpers for managed cleanup
-where possible.
-
-Drivers are pretty bad at doing this and there used to be conflicts among
-DRM and fbdev drivers. Still, it's the correct thing to do.
-
-Contact: Thomas Zimmermann <tzimmermann@suse.de>
-
-Level: Starter
-
 Remove driver dependencies on FB_DEVICE
 ---------------------------------------
 

-- 
2.47.0
Re: [PATCH 14/14] Documentation: Update the todo
Posted by Thomas Zimmermann 1 year ago
Hi


Am 28.01.25 um 23:29 schrieb Anusha Srivatsa:
> Remove the TODO now that this series addresses
> the changes needed.

While your series is fine, this TODO item is unrelated. It's about 
various ancient fbdev drivers that do not reserve their memory regions 
correctly. So please drop patch 14 form the series.

Best regards
Thomas

>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
> ---
>   Documentation/gpu/todo.rst | 15 ---------------
>   1 file changed, 15 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 256d0d1cb2164bd94f9b610a751b907834d96a21..b5aa5f776cfe0916b98bddf791c65abe974834cf 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -441,21 +441,6 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>
>   
>   Level: Intermediate
>   
> -Request memory regions in all drivers
> --------------------------------------
> -
> -Go through all drivers and add code to request the memory regions that the
> -driver uses. This requires adding calls to request_mem_region(),
> -pci_request_region() or similar functions. Use helpers for managed cleanup
> -where possible.
> -
> -Drivers are pretty bad at doing this and there used to be conflicts among
> -DRM and fbdev drivers. Still, it's the correct thing to do.
> -
> -Contact: Thomas Zimmermann <tzimmermann@suse.de>
> -
> -Level: Starter
> -
>   Remove driver dependencies on FB_DEVICE
>   ---------------------------------------
>   
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [PATCH 14/14] Documentation: Update the todo
Posted by Maxime Ripard 1 year ago
Hi Thomas,

On Wed, Jan 29, 2025 at 02:06:15PM +0100, Thomas Zimmermann wrote:
> Am 28.01.25 um 23:29 schrieb Anusha Srivatsa:
> > Remove the TODO now that this series addresses
> > the changes needed.
> 
> While your series is fine, this TODO item is unrelated. It's about various
> ancient fbdev drivers that do not reserve their memory regions correctly. So
> please drop patch 14 form the series.

Is it? Could we rephrase the entry then? I'm the one that suggested
Anusha to work on this, and it's still not clear to me what it means
exactly if it's not what she worked on :)

Maxime
Re: [PATCH 14/14] Documentation: Update the todo
Posted by Thomas Zimmermann 1 year ago
Hi Maxime


Am 29.01.25 um 15:31 schrieb Maxime Ripard:
> Hi Thomas,
>
> On Wed, Jan 29, 2025 at 02:06:15PM +0100, Thomas Zimmermann wrote:
>> Am 28.01.25 um 23:29 schrieb Anusha Srivatsa:
>>> Remove the TODO now that this series addresses
>>> the changes needed.
>> While your series is fine, this TODO item is unrelated. It's about various
>> ancient fbdev drivers that do not reserve their memory regions correctly. So
>> please drop patch 14 form the series.
> Is it? Could we rephrase the entry then? I'm the one that suggested
> Anusha to work on this, and it's still not clear to me what it means
> exactly if it's not what she worked on :)

I guess, we could make this more precise.

Some old graphics drivers don't request their memory ranges correctly. 
It's usually a problem with hardware that has exclusive ranges, such as 
the VGA. Vga16fb doesn't request the range as it should. Someone needs 
to audit all those old drivers and fix them.

Best regards
Thomas


>
> Maxime

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [PATCH 14/14] Documentation: Update the todo
Posted by Thierry Reding 1 year ago
On Wed, Jan 29, 2025 at 03:31:50PM +0100, Maxime Ripard wrote:
> Hi Thomas,
> 
> On Wed, Jan 29, 2025 at 02:06:15PM +0100, Thomas Zimmermann wrote:
> > Am 28.01.25 um 23:29 schrieb Anusha Srivatsa:
> > > Remove the TODO now that this series addresses
> > > the changes needed.
> > 
> > While your series is fine, this TODO item is unrelated. It's about various
> > ancient fbdev drivers that do not reserve their memory regions correctly. So
> > please drop patch 14 form the series.
> 
> Is it? Could we rephrase the entry then? I'm the one that suggested
> Anusha to work on this, and it's still not clear to me what it means
> exactly if it's not what she worked on :)

The text in the TODO sounds pretty clear to me. It says that not all
drivers request the memory that they are going to use, and suggests to
add those missing calls. But all of the drivers in this series already
do that and the only change here is to convert them to use some of the
newer helpers.

Thierry
Re: [PATCH 14/14] Documentation: Update the todo
Posted by Maxime Ripard 1 year ago
On Wed, Jan 29, 2025 at 03:41:32PM +0100, Thierry Reding wrote:
> On Wed, Jan 29, 2025 at 03:31:50PM +0100, Maxime Ripard wrote:
> > Hi Thomas,
> > 
> > On Wed, Jan 29, 2025 at 02:06:15PM +0100, Thomas Zimmermann wrote:
> > > Am 28.01.25 um 23:29 schrieb Anusha Srivatsa:
> > > > Remove the TODO now that this series addresses
> > > > the changes needed.
> > > 
> > > While your series is fine, this TODO item is unrelated. It's about various
> > > ancient fbdev drivers that do not reserve their memory regions correctly. So
> > > please drop patch 14 form the series.
> > 
> > Is it? Could we rephrase the entry then? I'm the one that suggested
> > Anusha to work on this, and it's still not clear to me what it means
> > exactly if it's not what she worked on :)
> 
> The text in the TODO sounds pretty clear to me.

The title is "Request memory regions in all drivers", and the first
sentence is "Go through all drivers and add code to request the memory
regions that the driver uses". It's definitely ambiguous if only fbdev
drivers should be considered, even more so in the DRM documentation.

> It says that not all drivers request the memory that they are going to
> use, and suggests to add those missing calls.

Right.

> But all of the drivers in this series already do that

Nope.

> and the only change here is to convert them to use some of the newer
> helpers.

For some, yes. For others, it actually adds request_mem_region.

Maxime
Re: [PATCH 14/14] Documentation: Update the todo
Posted by Thierry Reding 1 year ago
On Wed, Jan 29, 2025 at 04:28:49PM +0100, Maxime Ripard wrote:
> On Wed, Jan 29, 2025 at 03:41:32PM +0100, Thierry Reding wrote:
> > On Wed, Jan 29, 2025 at 03:31:50PM +0100, Maxime Ripard wrote:
> > > Hi Thomas,
> > > 
> > > On Wed, Jan 29, 2025 at 02:06:15PM +0100, Thomas Zimmermann wrote:
> > > > Am 28.01.25 um 23:29 schrieb Anusha Srivatsa:
> > > > > Remove the TODO now that this series addresses
> > > > > the changes needed.
> > > > 
> > > > While your series is fine, this TODO item is unrelated. It's about various
> > > > ancient fbdev drivers that do not reserve their memory regions correctly. So
> > > > please drop patch 14 form the series.
> > > 
> > > Is it? Could we rephrase the entry then? I'm the one that suggested
> > > Anusha to work on this, and it's still not clear to me what it means
> > > exactly if it's not what she worked on :)
> > 
> > The text in the TODO sounds pretty clear to me.
> 
> The title is "Request memory regions in all drivers", and the first
> sentence is "Go through all drivers and add code to request the memory
> regions that the driver uses". It's definitely ambiguous if only fbdev
> drivers should be considered, even more so in the DRM documentation.
> 
> > It says that not all drivers request the memory that they are going to
> > use, and suggests to add those missing calls.
> 
> Right.
> 
> > But all of the drivers in this series already do that
> 
> Nope.
> 
> > and the only change here is to convert them to use some of the newer
> > helpers.
> 
> For some, yes. For others, it actually adds request_mem_region.

Ah... indeed. Well, on the face of it this looks like just another mass-
conversion to the devm_platform_ioremap_resource() helper, so that's
also confusing.

Maybe the right way to do this would be to split this into two series,
one that actually does what the TODO suggests (and maybe updates the
TODO to make it more obvious that after this only fbdev drivers are
left) and another series that does the helper conversion.

Thierry