mbox series

[00/28] OpenChrome DRM for Linux 5.20

Message ID 20220624202633.3978-1-kevinbrace@gmx.com (mailing list archive)
Headers show
Series OpenChrome DRM for Linux 5.20 | expand

Message

Kevin Brace June 24, 2022, 8:26 p.m. UTC
From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Hi Dave and Daniel,

This is my first attempt (this is not a RFC posting) in trying to get
OpenChrome DRM pulled in for Linux 5.20.
I started to work on this seriously around the summer of 2017, so it has
been a long journey.
I know that the code is not quite perfect, but I have done as much work
as I can do on my own, and I now think that I should post the code on
dri-devel mailing list for other people to take a look at it.
Whether or not the code itself works or not, no, OpenChrome DRM is not
some vaporware, and the code reliability is comparable to the existing
OpenChrome DDX UMS code path.
The code reliability is more than good enough to the point where I utilize
the module almost every time I work on developing the code.
I am aware that the hardware (silicon) is quite old by today's standards,
so not too many people "still" (the word one open source software focused
journalist often uses to describe greater than 5 year old computer
hardware) own the VIA silicon hardware to actually test the code
themselves.
The current code is developed against drm-next branch
drm-next-2022-06-03-1 tag.
The current iteration of the code has no support for acceleration.
It is currently a mode setting only DRM module.
Even if the code is pulled into the Linux kernel tree, I will consider
the code experimental until at least 2D acceleration is implemented for
all supported devices.
Because of this, the DRM module requires via.modeset=1 to be added to
Linux kernel boot option line for it to function, and I intend to keep
this arrangement in place until at least 2D acceleration is fully
supported.
As a result, I think the code is fairly low risk to be pulled into the
Linux kernel tree.
The current module version is 3.5.2, but when the code is about to
be pulled into the kernel tree, I will like to up the version to 4.0.0.
This is because James Simmons era OpenChrome DRM used version number
3.0.x, but the current OpenChrome DRM uAPI implementation is different
from his implementation, so I will like to assign a new major version
(i.e., 4) so that DDX can distinguish the old and new OpenChrome DRM.
The current uAPI has no backward compatibility with the older DRI1 based
implementation, although some remnants of old DRI1 based uAPI still needs
to be there inside via_drm.h for XvMC based libraries on the DDX side to
properly compile.
It is my intention to replace the old DRI1 based VIA DRM located at
drivers/gpu/drm/via/ with OpenChrome DRM at the same location.
As I indicated previously, I do not wish to get pulled into the staging
area of the kernel tree.
I personally will like to have the option of porting the code to
BSD someday, however, the I2C bus access module (via_i2c.c) is GPL
based, so the DRM module license type is GPL (everything else is MIT
license based), for now.
I hope to replace this module someday with equivalent functionality
code that will be MIT license based.
I hope the uAPI variable types are compatible with BSD (please give
me advise on this since I do not know too much about this) since James
Simmons brought this up back in 2013 when he posted the code as RFC.

https://lists.freedesktop.org/archives/dri-devel/2013-June/039594.html
https://lists.freedesktop.org/archives/dri-devel/2013-June/040811.html


Features:

- Supports 12 different generations of VIA Technologies Chrome based
integrated graphics (CLE266 / KM400 / K8M800 / P4M800 Pro / PM800 /
P4M890 / K8M890 / P4M900 / CX700 / VX800 / VX855 / VX900 chipset) and
their variants
- Support for DAC (VGA), LVDS flat panel, DVI (CX700 / VX800 chipset
integrated, VX900 chipset integrated, and Silicon Image SiI164 /
VIA Technologies VT1632(A) DVI transmitter), and HDMI (VX900H chipset
integrated)
- Support for standby resume (ACPI S3 State)
- Support for dual head operation
- Supports atomic mode setting (implementation might still be incomplete
especially around gamma correction / palette initialization)
- Utilizes GEM / TTM for memory management
- Utilizes DRM FB Helper for FBDEV emulation


Known issues:

- via_lvds.c gives out 3 unused function warnings (willing to delete
the offending functions since they are not used)
- Gamma correction / palette initialization code is still legacy KMS
based (Daniel, how do I convert it for atomic mode setting?)
- uAPI might not be quite right (Do I need something like
DRM_IOCTL_VIA_GEM_DESTROY or DRM_IOCTL_VIA_GEM_FREE call to pair it
against DRM_IOCTL_VIA_GEM_CREATE?)


Code repository:
https://cgit.freedesktop.org/openchrome/drm-openchrome/

Current bleeding edge branch (drm-next-5.20 branch):
https://cgit.freedesktop.org/openchrome/drm-openchrome/?h=drm-next-5.20

Current bleeding edge branch (drm-next-5.20 branch) code location:
https://cgit.freedesktop.org/openchrome/drm-openchrome/tree/drivers/gpu/drm/via?h=drm-next-5.20
https://cgit.freedesktop.org/openchrome/drm-openchrome/tree/include/uapi/drm?h=drm-next-5.20

Obviously, I do not expect to get the code pulled in on my first try,
but if other people can point out what I need to improve, I will try to
improve the code so that I can get the code pulled in as soon as possible.
Personally, I will like the code to be pulled in for Linux 5.20, but I
sort of expect the next release after 5.20 is more likely.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


---
Kevin Brace (28):
  drm/via: Add via_3d_reg.h
  drm/via: Add via_crtc_hw.h
  drm/via: Add via_disp_reg.h
  drm/via: Add via_drv.h
  drm/via: Add via_regs.h
  drm/via: Add via_crtc.c
  drm/via: Add via_crtc_hw.c
  drm/via: Add via_cursor.c
  drm/via: Add via_dac.c
  drm/via: Add via_display.c
  drm/via: Add via_drv.c
  drm/via: Add via_encoder.c
  drm/via: Add via_hdmi.c
  drm/via: Add via_i2c.c
  drm/via: Add via_init.c
  drm/via: Add via_ioctl.c
  drm/via: Add via_lvds.c
  drm/via: Add via_object.c
  drm/via: Add via_pll.c
  drm/via: Add via_pm.c
  drm/via: Add via_sii164.c
  drm/via: Add via_tmds.c
  drm/via: Add via_ttm.c
  drm/via: Add via_vt1632.c
  drm/via: Add via_drm.h
  drm/via: Add Kconfig
  drm/via: Add Makefile
  drm/via: Modify DRM main Makefile to be able to build OpenChrome DRM

 drivers/gpu/drm/Makefile           |    1 +
 drivers/gpu/drm/via/Kconfig        |   10 +
 drivers/gpu/drm/via/Makefile       |   26 +
 drivers/gpu/drm/via/via_3d_reg.h   | 1863 ++++++++++++++++++++++
 drivers/gpu/drm/via/via_crtc.c     | 2324 ++++++++++++++++++++++++++++
 drivers/gpu/drm/via/via_crtc_hw.c  |   91 ++
 drivers/gpu/drm/via/via_crtc_hw.h  | 1048 +++++++++++++
 drivers/gpu/drm/via/via_cursor.c   |  419 +++++
 drivers/gpu/drm/via/via_dac.c      |  504 ++++++
 drivers/gpu/drm/via/via_disp_reg.h |  513 ++++++
 drivers/gpu/drm/via/via_display.c  |  125 ++
 drivers/gpu/drm/via/via_drv.c      |  313 ++++
 drivers/gpu/drm/via/via_drv.h      |  437 ++++++
 drivers/gpu/drm/via/via_encoder.c  |  173 +++
 drivers/gpu/drm/via/via_hdmi.c     |  719 +++++++++
 drivers/gpu/drm/via/via_i2c.c      |  209 +++
 drivers/gpu/drm/via/via_init.c     | 1300 ++++++++++++++++
 drivers/gpu/drm/via/via_ioctl.c    |  122 ++
 drivers/gpu/drm/via/via_lvds.c     | 1420 +++++++++++++++++
 drivers/gpu/drm/via/via_object.c   |  324 ++++
 drivers/gpu/drm/via/via_pll.c      |  263 ++++
 drivers/gpu/drm/via/via_pm.c       |  187 +++
 drivers/gpu/drm/via/via_regs.h     |  296 ++++
 drivers/gpu/drm/via/via_sii164.c   |  563 +++++++
 drivers/gpu/drm/via/via_tmds.c     |  714 +++++++++
 drivers/gpu/drm/via/via_ttm.c      |  168 ++
 drivers/gpu/drm/via/via_vt1632.c   |  583 +++++++
 include/uapi/drm/via_drm.h         |  309 ++++
 28 files changed, 15024 insertions(+)
 create mode 100644 drivers/gpu/drm/via/Kconfig
 create mode 100644 drivers/gpu/drm/via/Makefile
 create mode 100644 drivers/gpu/drm/via/via_3d_reg.h
 create mode 100644 drivers/gpu/drm/via/via_crtc.c
 create mode 100644 drivers/gpu/drm/via/via_crtc_hw.c
 create mode 100644 drivers/gpu/drm/via/via_crtc_hw.h
 create mode 100644 drivers/gpu/drm/via/via_cursor.c
 create mode 100644 drivers/gpu/drm/via/via_dac.c
 create mode 100644 drivers/gpu/drm/via/via_disp_reg.h
 create mode 100644 drivers/gpu/drm/via/via_display.c
 create mode 100644 drivers/gpu/drm/via/via_drv.c
 create mode 100644 drivers/gpu/drm/via/via_drv.h
 create mode 100644 drivers/gpu/drm/via/via_encoder.c
 create mode 100644 drivers/gpu/drm/via/via_hdmi.c
 create mode 100644 drivers/gpu/drm/via/via_i2c.c
 create mode 100644 drivers/gpu/drm/via/via_init.c
 create mode 100644 drivers/gpu/drm/via/via_ioctl.c
 create mode 100644 drivers/gpu/drm/via/via_lvds.c
 create mode 100644 drivers/gpu/drm/via/via_object.c
 create mode 100644 drivers/gpu/drm/via/via_pll.c
 create mode 100644 drivers/gpu/drm/via/via_pm.c
 create mode 100644 drivers/gpu/drm/via/via_regs.h
 create mode 100644 drivers/gpu/drm/via/via_sii164.c
 create mode 100644 drivers/gpu/drm/via/via_tmds.c
 create mode 100644 drivers/gpu/drm/via/via_ttm.c
 create mode 100644 drivers/gpu/drm/via/via_vt1632.c
 create mode 100644 include/uapi/drm/via_drm.h

--
2.35.1

Comments

Sam Ravnborg June 24, 2022, 9:15 p.m. UTC | #1
Hi Kevin,

On Fri, Jun 24, 2022 at 03:26:05PM -0500, Kevin Brace wrote:
> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> 
> Hi Dave and Daniel,
> 
> This is my first attempt (this is not a RFC posting) in trying to get
> OpenChrome DRM pulled in for Linux 5.20.

I tried to apply the patches one-by-one but they fail as they assume all
old via files are gone. Please fix v2 so all patches apply clean to the
chosen base.

There is no need for a respin now, give people a bit time to look at the
patches first. At least unless someone else needs it.

	Sam
Kevin Brace June 24, 2022, 11:14 p.m. UTC | #2
Hi Sam,

Oh, sorry that I assumed drivers/gpu/drm/via/ is completely cleared out.
The problem is, I used drm-openchrome repository (customized drm-next branch for OpenChrome DRM development) to generate the patches.
I happened not to have the original DRM maintainers version installed on my regular development system.
As of now, I am downloading it onto my development system.
I will have a few patches to supplement the patches I posted ready in about 2 hours per your suggestion.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Friday, June 24, 2022 at 4:15 PM
> From: "Sam Ravnborg" <sam@ravnborg.org>
> To: "Kevin Brace" <kevinbrace@gmx.com>
> Cc: dri-devel@lists.freedesktop.org, "Kevin Brace" <kevinbrace@bracecomputerlab.com>
> Subject: Re: [PATCH 00/28] OpenChrome DRM for Linux 5.20
>
> Hi Kevin,
>
> On Fri, Jun 24, 2022 at 03:26:05PM -0500, Kevin Brace wrote:
> > From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> >
> > Hi Dave and Daniel,
> >
> > This is my first attempt (this is not a RFC posting) in trying to get
> > OpenChrome DRM pulled in for Linux 5.20.
>
> I tried to apply the patches one-by-one but they fail as they assume all
> old via files are gone. Please fix v2 so all patches apply clean to the
> chosen base.
>
> There is no need for a respin now, give people a bit time to look at the
> patches first. At least unless someone else needs it.
>
> 	Sam
>
Thomas Zimmermann June 27, 2022, 7:37 a.m. UTC | #3
Hi Kevin

Am 24.06.22 um 22:26 schrieb Kevin Brace:
> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> 
> Hi Dave and Daniel,
> 
> This is my first attempt (this is not a RFC posting) in trying to get
> OpenChrome DRM pulled in for Linux 5.20.

First of all, thank you so much for working on this.

> I started to work on this seriously around the summer of 2017, so it has
> been a long journey.

> I know that the code is not quite perfect, but I have done as much work
> as I can do on my own, and I now think that I should post the code on
> dri-devel mailing list for other people to take a look at it.

I had a brief look over the patches. Even though the code looks fairly 
rough in places, I think we should get this driver merged ASAP. Once 
merged, cleanups become *a lot* easier.


> Whether or not the code itself works or not, no, OpenChrome DRM is not
> some vaporware, and the code reliability is comparable to the existing
> OpenChrome DDX UMS code path.
> The code reliability is more than good enough to the point where I utilize
> the module almost every time I work on developing the code.
> I am aware that the hardware (silicon) is quite old by today's standards,
> so not too many people "still" (the word one open source software focused
> journalist often uses to describe greater than 5 year old computer
> hardware) own the VIA silicon hardware to actually test the code
> themselves.

I wanted to test the patches (with that other additional patch), but 
they don't build with the current DRM development tree.

> The current code is developed against drm-next branch
> drm-next-2022-06-03-1 tag.

This will go through drm-misc-next, presumably. You need to rebase on 
top of that branch. Please don't merge but rebase, so that you actually 
have all required changes in the next patchset. I've found that at least 
DRM's Kconfig file misses a change and that <linux/pci_ids.h> doesn't 
contain all necessary PCI ids.

This should be the next step.

> The current iteration of the code has no support for acceleration.
> It is currently a mode setting only DRM module.
> Even if the code is pulled into the Linux kernel tree, I will consider
> the code experimental until at least 2D acceleration is implemented for
> all supported devices.

Just forget about 2d acceleration (aka blitters). Daniel did a write-up 
about the specific problems of 2d acceleration at [1].

(There's also video acceleration, which is a different story.)

> Because of this, the DRM module requires via.modeset=1 to be added to
> Linux kernel boot option line for it to function, and I intend to keep
> this arrangement in place until at least 2D acceleration is fully
> supported.

Don't do that. Enable modesetting from day 1. You want to get the driver 
out to users ASAP. Otherwise, it'll take another 5 years.

We plenty of options to disable your driver on systems where it fails.

> As a result, I think the code is fairly low risk to be pulled into the
> Linux kernel tree.

Exactly.

> The current module version is 3.5.2, but when the code is about to
> be pulled into the kernel tree, I will like to up the version to 4.0.0.
> This is because James Simmons era OpenChrome DRM used version number
> 3.0.x, but the current OpenChrome DRM uAPI implementation is different
> from his implementation, so I will like to assign a new major version
> (i.e., 4) so that DDX can distinguish the old and new OpenChrome DRM.
> The current uAPI has no backward compatibility with the older DRI1 based
> implementation, although some remnants of old DRI1 based uAPI still needs
> to be there inside via_drm.h for XvMC based libraries on the DDX side to
> properly compile.

What is the DDX required for? Would the regular X11 modesetting driver 
work as well?

> It is my intention to replace the old DRI1 based VIA DRM located at
> drivers/gpu/drm/via/ with OpenChrome DRM at the same location.
> As I indicated previously, I do not wish to get pulled into the staging
> area of the kernel tree.

DRM is not allowed in staging.

For the old via driver, I think we need a better apprach. IMHO it would 
be preferable to put the new driver into via/ but keep the old driver 
there as well.  A build option would control which is being used.

Best regards
Thomas

[1] https://blog.ffwll.ch/2018/08/no-2d-in-drm.html

> I personally will like to have the option of porting the code to
> BSD someday, however, the I2C bus access module (via_i2c.c) is GPL
> based, so the DRM module license type is GPL (everything else is MIT
> license based), for now.
> I hope to replace this module someday with equivalent functionality
> code that will be MIT license based.
> I hope the uAPI variable types are compatible with BSD (please give
> me advise on this since I do not know too much about this) since James
> Simmons brought this up back in 2013 when he posted the code as RFC.
> 
> https://lists.freedesktop.org/archives/dri-devel/2013-June/039594.html
> https://lists.freedesktop.org/archives/dri-devel/2013-June/040811.html
> 
> 
> Features:
> 
> - Supports 12 different generations of VIA Technologies Chrome based
> integrated graphics (CLE266 / KM400 / K8M800 / P4M800 Pro / PM800 /
> P4M890 / K8M890 / P4M900 / CX700 / VX800 / VX855 / VX900 chipset) and
> their variants
> - Support for DAC (VGA), LVDS flat panel, DVI (CX700 / VX800 chipset
> integrated, VX900 chipset integrated, and Silicon Image SiI164 /
> VIA Technologies VT1632(A) DVI transmitter), and HDMI (VX900H chipset
> integrated)
> - Support for standby resume (ACPI S3 State)
> - Support for dual head operation
> - Supports atomic mode setting (implementation might still be incomplete
> especially around gamma correction / palette initialization)
> - Utilizes GEM / TTM for memory management
> - Utilizes DRM FB Helper for FBDEV emulation
> 
> 
> Known issues:
> 
> - via_lvds.c gives out 3 unused function warnings (willing to delete
> the offending functions since they are not used)
> - Gamma correction / palette initialization code is still legacy KMS
> based (Daniel, how do I convert it for atomic mode setting?)
> - uAPI might not be quite right (Do I need something like
> DRM_IOCTL_VIA_GEM_DESTROY or DRM_IOCTL_VIA_GEM_FREE call to pair it
> against DRM_IOCTL_VIA_GEM_CREATE?)
> 
> 
> Code repository:
> https://cgit.freedesktop.org/openchrome/drm-openchrome/
> 
> Current bleeding edge branch (drm-next-5.20 branch):
> https://cgit.freedesktop.org/openchrome/drm-openchrome/?h=drm-next-5.20
> 
> Current bleeding edge branch (drm-next-5.20 branch) code location:
> https://cgit.freedesktop.org/openchrome/drm-openchrome/tree/drivers/gpu/drm/via?h=drm-next-5.20
> https://cgit.freedesktop.org/openchrome/drm-openchrome/tree/include/uapi/drm?h=drm-next-5.20
> 
> Obviously, I do not expect to get the code pulled in on my first try,
> but if other people can point out what I need to improve, I will try to
> improve the code so that I can get the code pulled in as soon as possible.
> Personally, I will like the code to be pulled in for Linux 5.20, but I
> sort of expect the next release after 5.20 is more likely.
> 
> Regards,
> 
> Kevin Brace
> Brace Computer Laboratory blog
> https://bracecomputerlab.com
> 
> 
> ---
> Kevin Brace (28):
>    drm/via: Add via_3d_reg.h
>    drm/via: Add via_crtc_hw.h
>    drm/via: Add via_disp_reg.h
>    drm/via: Add via_drv.h
>    drm/via: Add via_regs.h
>    drm/via: Add via_crtc.c
>    drm/via: Add via_crtc_hw.c
>    drm/via: Add via_cursor.c
>    drm/via: Add via_dac.c
>    drm/via: Add via_display.c
>    drm/via: Add via_drv.c
>    drm/via: Add via_encoder.c
>    drm/via: Add via_hdmi.c
>    drm/via: Add via_i2c.c
>    drm/via: Add via_init.c
>    drm/via: Add via_ioctl.c
>    drm/via: Add via_lvds.c
>    drm/via: Add via_object.c
>    drm/via: Add via_pll.c
>    drm/via: Add via_pm.c
>    drm/via: Add via_sii164.c
>    drm/via: Add via_tmds.c
>    drm/via: Add via_ttm.c
>    drm/via: Add via_vt1632.c
>    drm/via: Add via_drm.h
>    drm/via: Add Kconfig
>    drm/via: Add Makefile
>    drm/via: Modify DRM main Makefile to be able to build OpenChrome DRM
> 
>   drivers/gpu/drm/Makefile           |    1 +
>   drivers/gpu/drm/via/Kconfig        |   10 +
>   drivers/gpu/drm/via/Makefile       |   26 +
>   drivers/gpu/drm/via/via_3d_reg.h   | 1863 ++++++++++++++++++++++
>   drivers/gpu/drm/via/via_crtc.c     | 2324 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/via/via_crtc_hw.c  |   91 ++
>   drivers/gpu/drm/via/via_crtc_hw.h  | 1048 +++++++++++++
>   drivers/gpu/drm/via/via_cursor.c   |  419 +++++
>   drivers/gpu/drm/via/via_dac.c      |  504 ++++++
>   drivers/gpu/drm/via/via_disp_reg.h |  513 ++++++
>   drivers/gpu/drm/via/via_display.c  |  125 ++
>   drivers/gpu/drm/via/via_drv.c      |  313 ++++
>   drivers/gpu/drm/via/via_drv.h      |  437 ++++++
>   drivers/gpu/drm/via/via_encoder.c  |  173 +++
>   drivers/gpu/drm/via/via_hdmi.c     |  719 +++++++++
>   drivers/gpu/drm/via/via_i2c.c      |  209 +++
>   drivers/gpu/drm/via/via_init.c     | 1300 ++++++++++++++++
>   drivers/gpu/drm/via/via_ioctl.c    |  122 ++
>   drivers/gpu/drm/via/via_lvds.c     | 1420 +++++++++++++++++
>   drivers/gpu/drm/via/via_object.c   |  324 ++++
>   drivers/gpu/drm/via/via_pll.c      |  263 ++++
>   drivers/gpu/drm/via/via_pm.c       |  187 +++
>   drivers/gpu/drm/via/via_regs.h     |  296 ++++
>   drivers/gpu/drm/via/via_sii164.c   |  563 +++++++
>   drivers/gpu/drm/via/via_tmds.c     |  714 +++++++++
>   drivers/gpu/drm/via/via_ttm.c      |  168 ++
>   drivers/gpu/drm/via/via_vt1632.c   |  583 +++++++
>   include/uapi/drm/via_drm.h         |  309 ++++
>   28 files changed, 15024 insertions(+)
>   create mode 100644 drivers/gpu/drm/via/Kconfig
>   create mode 100644 drivers/gpu/drm/via/Makefile
>   create mode 100644 drivers/gpu/drm/via/via_3d_reg.h
>   create mode 100644 drivers/gpu/drm/via/via_crtc.c
>   create mode 100644 drivers/gpu/drm/via/via_crtc_hw.c
>   create mode 100644 drivers/gpu/drm/via/via_crtc_hw.h
>   create mode 100644 drivers/gpu/drm/via/via_cursor.c
>   create mode 100644 drivers/gpu/drm/via/via_dac.c
>   create mode 100644 drivers/gpu/drm/via/via_disp_reg.h
>   create mode 100644 drivers/gpu/drm/via/via_display.c
>   create mode 100644 drivers/gpu/drm/via/via_drv.c
>   create mode 100644 drivers/gpu/drm/via/via_drv.h
>   create mode 100644 drivers/gpu/drm/via/via_encoder.c
>   create mode 100644 drivers/gpu/drm/via/via_hdmi.c
>   create mode 100644 drivers/gpu/drm/via/via_i2c.c
>   create mode 100644 drivers/gpu/drm/via/via_init.c
>   create mode 100644 drivers/gpu/drm/via/via_ioctl.c
>   create mode 100644 drivers/gpu/drm/via/via_lvds.c
>   create mode 100644 drivers/gpu/drm/via/via_object.c
>   create mode 100644 drivers/gpu/drm/via/via_pll.c
>   create mode 100644 drivers/gpu/drm/via/via_pm.c
>   create mode 100644 drivers/gpu/drm/via/via_regs.h
>   create mode 100644 drivers/gpu/drm/via/via_sii164.c
>   create mode 100644 drivers/gpu/drm/via/via_tmds.c
>   create mode 100644 drivers/gpu/drm/via/via_ttm.c
>   create mode 100644 drivers/gpu/drm/via/via_vt1632.c
>   create mode 100644 include/uapi/drm/via_drm.h
> 
> --
> 2.35.1
>
Sam Ravnborg June 27, 2022, 9:32 p.m. UTC | #4
Hi Kevin/Thomas,

> 
> I had a brief look over the patches. Even though the code looks fairly rough
> in places, I think we should get this driver merged ASAP.

Agreed, we have a had a few cases where we dragged out committing much
too long time.

Timing wise, it would be good if we can already hit drm-misc-next right
_after_ the pull -so we have a full cycle to fix anything before it hits
mainline.

> For the old via driver, I think we need a better apprach. IMHO it would be
> preferable to put the new driver into via/ but keep the old driver there as
> well.  A build option would control which is being used.

I assume the user base for via drivers are very small and they have the
fbdev driver already.
So I support replacing the current via drm driver as Kevin tries to do.

But anyway we need a patch set that applies without extra
delete file / add file cycles as the next step.

	Sam
Thomas Zimmermann June 28, 2022, 12:21 p.m. UTC | #5
Hi

Am 27.06.22 um 23:32 schrieb Sam Ravnborg:
> Hi Kevin/Thomas,
> 
>>
>> I had a brief look over the patches. Even though the code looks fairly rough
>> in places, I think we should get this driver merged ASAP.
> 
> Agreed, we have a had a few cases where we dragged out committing much
> too long time.
> 
> Timing wise, it would be good if we can already hit drm-misc-next right
> _after_ the pull -so we have a full cycle to fix anything before it hits
> mainline.
> 
>> For the old via driver, I think we need a better apprach. IMHO it would be
>> preferable to put the new driver into via/ but keep the old driver there as
>> well.  A build option would control which is being used.
> 
> I assume the user base for via drivers are very small and they have the
> fbdev driver already.
> So I support replacing the current via drm driver as Kevin tries to do.

I don't know if there are still users of the old userspace, but if so I 
would consider this removal a regression. I think the old code supports 
3d and video decoding. Depending on the feature set, 3d support might 
not be useful any longer, but video decoding probably is.  (I might be 
wrong about all this.) IMHO we should not simply remove this at least 
until we can verify that it's no longer useful to anyone.

However, legacy support is trivial. Kevin, please see the attached files 
for two cleanup patches. You're welcome to add them to the start of your 
patchset to get the legacy code out of the way.

Best regards
Thomas


> 
> But anyway we need a patch set that applies without extra
> delete file / add file cycles as the next step.
> 
> 	Sam
Javier Martinez Canillas June 30, 2022, 8:07 a.m. UTC | #6
Hello,

On 6/28/22 14:21, Thomas Zimmermann wrote:
> Hi
> 
> Am 27.06.22 um 23:32 schrieb Sam Ravnborg:
>> Hi Kevin/Thomas,
>>
>>>
>>> I had a brief look over the patches. Even though the code looks fairly rough
>>> in places, I think we should get this driver merged ASAP.
>>
>> Agreed, we have a had a few cases where we dragged out committing much
>> too long time.
>>
>> Timing wise, it would be good if we can already hit drm-misc-next right
>> _after_ the pull -so we have a full cycle to fix anything before it hits
>> mainline.
>>
>>> For the old via driver, I think we need a better apprach. IMHO it would be
>>> preferable to put the new driver into via/ but keep the old driver there as
>>> well.  A build option would control which is being used.
>>
>> I assume the user base for via drivers are very small and they have the
>> fbdev driver already.
>> So I support replacing the current via drm driver as Kevin tries to do.
> 
> I don't know if there are still users of the old userspace, but if so I 
> would consider this removal a regression. I think the old code supports 
> 3d and video decoding. Depending on the feature set, 3d support might 
> not be useful any longer, but video decoding probably is.  (I might be 
> wrong about all this.) IMHO we should not simply remove this at least 
> until we can verify that it's no longer useful to anyone.
>

I strongly agree with Thomas on this.
 
> However, legacy support is trivial. Kevin, please see the attached files 
> for two cleanup patches. You're welcome to add them to the start of your 
> patchset to get the legacy code out of the way.
>

I'm not sure about this approach, I think that having completely separated
drivers would be better to maintain in the long run since it's likely that
the legacy VIA driver will only get bug fixes (if any) and could be removed
once the new modsetting driver has feature parity, the legacy can be dropped.
 
Maybe an alternative could be to add a drivers/gpu/drm/legacy directory and
move all the legacy DRM drivers there ? And the Kconfig symbol could be i.e:
CONFIG_DRM_LEGACY_VIA and same for the others legacy DRM drivers.

And the directory could only be sourced from Kconfig when CONFIG_DRM_LEGACY
is enabled and make it default to n. If in a few of releases nobody complains
then the whole directory could be removed.
Thomas Zimmermann June 30, 2022, 8:19 a.m. UTC | #7
Hi

Am 30.06.22 um 10:07 schrieb Javier Martinez Canillas:
> Hello,
> 
> On 6/28/22 14:21, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 27.06.22 um 23:32 schrieb Sam Ravnborg:
>>> Hi Kevin/Thomas,
>>>
>>>>
>>>> I had a brief look over the patches. Even though the code looks fairly rough
>>>> in places, I think we should get this driver merged ASAP.
>>>
>>> Agreed, we have a had a few cases where we dragged out committing much
>>> too long time.
>>>
>>> Timing wise, it would be good if we can already hit drm-misc-next right
>>> _after_ the pull -so we have a full cycle to fix anything before it hits
>>> mainline.
>>>
>>>> For the old via driver, I think we need a better apprach. IMHO it would be
>>>> preferable to put the new driver into via/ but keep the old driver there as
>>>> well.  A build option would control which is being used.
>>>
>>> I assume the user base for via drivers are very small and they have the
>>> fbdev driver already.
>>> So I support replacing the current via drm driver as Kevin tries to do.
>>
>> I don't know if there are still users of the old userspace, but if so I
>> would consider this removal a regression. I think the old code supports
>> 3d and video decoding. Depending on the feature set, 3d support might
>> not be useful any longer, but video decoding probably is.  (I might be
>> wrong about all this.) IMHO we should not simply remove this at least
>> until we can verify that it's no longer useful to anyone.
>>
> 
> I strongly agree with Thomas on this.
>   
>> However, legacy support is trivial. Kevin, please see the attached files
>> for two cleanup patches. You're welcome to add them to the start of your
>> patchset to get the legacy code out of the way.
>>
> 
> I'm not sure about this approach, I think that having completely separated
> drivers would be better to maintain in the long run since it's likely that
> the legacy VIA driver will only get bug fixes (if any) and could be removed
> once the new modsetting driver has feature parity, the legacy can be dropped.
>   
> Maybe an alternative could be to add a drivers/gpu/drm/legacy directory and
> move all the legacy DRM drivers there ? And the Kconfig symbol could be i.e:
> CONFIG_DRM_LEGACY_VIA and same for the others legacy DRM drivers.
> 
> And the directory could only be sourced from Kconfig when CONFIG_DRM_LEGACY
> is enabled and make it default to n. If in a few of releases nobody complains
> then the whole directory could be removed.
> 

That seems a lot of work for simply removing something. And I'm sure 
that people will only complain after legacy/via got removed.

If we want to separate code for the old and new VIA driver, let's put 
the new code into  unichrome/ and be done with it.

Best regards
Thomas
Javier Martinez Canillas June 30, 2022, 8:27 a.m. UTC | #8
Hello Thomas,

On 6/30/22 10:19, Thomas Zimmermann wrote:

[snip]

>>
>> And the directory could only be sourced from Kconfig when CONFIG_DRM_LEGACY
>> is enabled and make it default to n. If in a few of releases nobody complains
>> then the whole directory could be removed.
>>
> 
> That seems a lot of work for simply removing something. And I'm sure 
> that people will only complain after legacy/via got removed.
> 
> If we want to separate code for the old and new VIA driver, let's put 
> the new code into  unichrome/ and be done with it.
>

I believe that there's value in having all the legacy drivers in a single
directory rather than having them scattered all across the subsystem. But
don't have a strong opinion either. I just wanted to share my thoughts.
 
> Best regards
> Thomas
> 
>
Thomas Zimmermann June 30, 2022, 8:33 a.m. UTC | #9
Hi

Am 30.06.22 um 10:19 schrieb Thomas Zimmermann:
> Hi
> 
> Am 30.06.22 um 10:07 schrieb Javier Martinez Canillas:
>> Hello,
>>
>> On 6/28/22 14:21, Thomas Zimmermann wrote:
>>> Hi
>>>
>>> Am 27.06.22 um 23:32 schrieb Sam Ravnborg:
>>>> Hi Kevin/Thomas,
>>>>
>>>>>
>>>>> I had a brief look over the patches. Even though the code looks 
>>>>> fairly rough
>>>>> in places, I think we should get this driver merged ASAP.
>>>>
>>>> Agreed, we have a had a few cases where we dragged out committing much
>>>> too long time.
>>>>
>>>> Timing wise, it would be good if we can already hit drm-misc-next right
>>>> _after_ the pull -so we have a full cycle to fix anything before it 
>>>> hits
>>>> mainline.
>>>>
>>>>> For the old via driver, I think we need a better apprach. IMHO it 
>>>>> would be
>>>>> preferable to put the new driver into via/ but keep the old driver 
>>>>> there as
>>>>> well.  A build option would control which is being used.
>>>>
>>>> I assume the user base for via drivers are very small and they have the
>>>> fbdev driver already.
>>>> So I support replacing the current via drm driver as Kevin tries to do.
>>>
>>> I don't know if there are still users of the old userspace, but if so I
>>> would consider this removal a regression. I think the old code supports
>>> 3d and video decoding. Depending on the feature set, 3d support might
>>> not be useful any longer, but video decoding probably is.  (I might be
>>> wrong about all this.) IMHO we should not simply remove this at least
>>> until we can verify that it's no longer useful to anyone.
>>>
>>
>> I strongly agree with Thomas on this.
>>> However, legacy support is trivial. Kevin, please see the attached files
>>> for two cleanup patches. You're welcome to add them to the start of your
>>> patchset to get the legacy code out of the way.
>>>
>>
>> I'm not sure about this approach, I think that having completely 
>> separated
>> drivers would be better to maintain in the long run since it's likely 
>> that
>> the legacy VIA driver will only get bug fixes (if any) and could be 
>> removed
>> once the new modsetting driver has feature parity, the legacy can be 
>> dropped.
>> Maybe an alternative could be to add a drivers/gpu/drm/legacy 
>> directory and
>> move all the legacy DRM drivers there ? And the Kconfig symbol could 
>> be i.e:
>> CONFIG_DRM_LEGACY_VIA and same for the others legacy DRM drivers.
>>
>> And the directory could only be sourced from Kconfig when 
>> CONFIG_DRM_LEGACY
>> is enabled and make it default to n. If in a few of releases nobody 
>> complains
>> then the whole directory could be removed.

To add another comment here, if we do this then let's please not call it 
legacy.  We also have non-atomic modesetting, non-atomic color mgmt, 
non-universal planes, etc. All that is also legacy in some way.

If we're going to rename and move the old drivers, let's call it 
consistently CONFIG_DRI1 and dri1/.

Best regard
Thomas

>>
> 
> That seems a lot of work for simply removing something. And I'm sure 
> that people will only complain after legacy/via got removed.
> 
> If we want to separate code for the old and new VIA driver, let's put 
> the new code into  unichrome/ and be done with it.
> 
> Best regards
> Thomas
> 
>