[PATCH v1 0/11] drm/via: Make via a single file driver

Thomas Zimmermann tzimmermann at suse.de
Mon Jul 11 07:01:50 UTC 2022


Hi Sam,

this looks like a good solution to me. I'd give you a detailed review, 
but dri-devel decided to only send me the color letter. I had to use 
patchwork get the patches.

For the series

Acked-by: Thomas Zimmermann <tzimmermann at suse.de>

with suggestions below.

Am 10.07.22 um 10:54 schrieb Sam Ravnborg:
> We have an upcoming openchrome driver for VIA where some
> of the files conflicts with the existing via driver.
> 
> It is not acceptable to just delete the existing DRI1
> based driver as there are most likely users out there,
> so a different approach was required.
> 
> It was disccussed what to do and the least invasive
> solution was to keep the DRI1 driver in the current
> directory as a single file.
> 
> Thomas Zimmermann already posted a patch to do the
> same but it attemped to have a single driver
> for the DRI1 and the upcoming new driver.

After the openchrome patches land, there will be an option in Kconfig to 
build the old driver instead of of the new one?

> 
> This patchset embeds the files one by one to make the
> diffs remotely readable and end up with an independent
> DRI1 driver.
> 
> The driver was built tested for each step.
> 
> The driver is in the end renamed to via_dri1.
> Some feedback on this would be good as I do not know
> what impact it will have on users.

I don't know how Mesa decides about which userspace driver to load, but 
It seems related to the name of the kernel driver. Renaming the module 
might interfere somehow.  But if the old and new driver are mutually 
exclusive at build time, it should not be a problem to use the same name 
for both modules.

> 
> The very last patch synchronize the via_3d_reg header
> file with the one used in the openchrome driver.
> This was added to verify that the new header would not
> break the DRI1 driver.

Some of the macros introduce line wraps. I don't know if you did that 
intentionally, but it shouldn't be necessary. We have a 100-character 
limit per line.

Maybe you can also add an extra patch that adds SPDX license tags at the 
top of the files.

Best regard
Thomas

> 
> 	Sam
> 
> Sam Ravnborg (11):
>        drm/via: Rename via_drv to via_dri1
>        drm/via: Embed via_dma in via_dri1
>        drm/via: Embed via_map in via_dri1
>        drm/via: Embed via_mm in via_dri1
>        drm/via: Embed via_video in via_dri1
>        drm/via: Embed via_irq in via_dri1
>        drm/via: Embed via_dmablit in via_dri1
>        drm/via: Embed via_verifier in via_dri1
>        drm/via: Embed via_drv.h in via_dri1
>        drm/via: Rename the via driver to via_dri1
>        drm/via: Update to the latest via_3d_reg file
> 
>   drivers/gpu/drm/via/Makefile       |    4 +-
>   drivers/gpu/drm/via/via_3d_reg.h   |  395 +++-
>   drivers/gpu/drm/via/via_dma.c      |  744 --------
>   drivers/gpu/drm/via/via_dmablit.c  |  807 --------
>   drivers/gpu/drm/via/via_dmablit.h  |  140 --
>   drivers/gpu/drm/via/via_dri1.c     | 3630 ++++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/via/via_drv.c      |  124 --
>   drivers/gpu/drm/via/via_drv.h      |  229 ---
>   drivers/gpu/drm/via/via_irq.c      |  388 ----
>   drivers/gpu/drm/via/via_map.c      |  132 --
>   drivers/gpu/drm/via/via_mm.c       |  241 ---
>   drivers/gpu/drm/via/via_verifier.c | 1110 -----------
>   drivers/gpu/drm/via/via_verifier.h |   62 -
>   drivers/gpu/drm/via/via_video.c    |   94 -
>   14 files changed, 3935 insertions(+), 4165 deletions(-)
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220711/30610d99/attachment-0001.sig>


More information about the dri-devel mailing list