Re: [PATCH v6 1/3] drm/lsdc: add drm driver for loongson display controller

[Maxime Ripard]

On Thu, Feb 03, 2022 at 11:47:16PM +0800, Sui Jingfeng wrote:
> On 2022/2/3 16:58, Maxime Ripard wrote:
> > > diff --git a/drivers/gpu/drm/lsdc/Kconfig b/drivers/gpu/drm/lsdc/Kconfig
> > > new file mode 100644
> > > index 000000000000..7ed1b0fdbe1b
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/lsdc/Kconfig
> > > @@ -0,0 +1,38 @@
> > > +config DRM_LSDC
> > > +	tristate "DRM Support for loongson's display controller"
> > > +	depends on DRM && PCI
> > > +	depends on MACH_LOONGSON64 || LOONGARCH || MIPS || COMPILE_TEST
> > > +	select OF
> > > +	select CMA if HAVE_DMA_CONTIGUOUS
> > > +	select DMA_CMA if HAVE_DMA_CONTIGUOUS
> > > +	select DRM_KMS_HELPER
> > > +	select DRM_KMS_FB_HELPER
> > > +	select DRM_GEM_CMA_HELPER
> > > +	select VIDEOMODE_HELPERS
> > > +	select BACKLIGHT_PWM if CPU_LOONGSON2K
> > > +	select I2C_GPIO if CPU_LOONGSON2K
> > > +	select I2C_LS2X if CPU_LOONGSON2K
> > > +	default m
> > > +	help
> > > +	  This is a KMS driver for the display controller in the LS7A1000
> > > +	  bridge chip and LS2K1000 SoC. The display controller has two
> > > +	  display pipes and it is a PCI device.
> > > +	  When using this driver on LS2K1000/LS2K0500 SoC, its framebuffer
> > > +	  is located at system memory.
> > > +	  If "M" is selected, the module will be called lsdc.
> > > +
> > > +	  If in doubt, say "Y".
> > > +
> > > +config DRM_LSDC_VRAM_DRIVER
> > > +	bool "vram helper based device driver support"
> > > +	depends on DRM_LSDC
> > > +	select DRM_VRAM_HELPER
> > > +	default y
> > > +	help
> > > +	  When using this driver on LS7A1000 + LS3A3000/LS3A4000/LS3A5000
> > > +	  platform, the LS7A1000 bridge chip has dedicated video RAM. Using
> > > +	  "lsdc.use_vram_helper=1" in the kernel command line will enable
> > > +	  this driver mode and then the framebuffer will be located at the
> > > +	  VRAM at the price of losing PRIME support.
> > > +
> > > +	  If in doubt, say "Y".
> > This doesn't sound right. The driver should make the proper decision
> > depending on the platform, not the user or the distribution.
> 
> The LS7A1000 north bridge chip has dedicated video RAM, but the DC in LS7A1000
> can also scanout from the system memory directly like a display controller in a
> SoC does. In fact, this display controller is envolved from early product of
> Loongson 2H SoC. This driver still works even if the downstream PC board
> manufacturer doesn't solder a video RAM on the mother board.
> 
> The lsdc_should_vram_helper_based() function in lsdc_drv.c will examine
> if the DC device node contain a use_vram_helper property at driver loading time.
> If there is no use_vram_helper property, CMA helper based DRM driver will be used.
> Doing this way allow the user using "lsdc.use_vram_helper=0" override the default
> behavior even through there is a "use_vram_helper;" property in the DTS.
> 
> In short, It is intend to let the command line passed from kernel has higher
> priority than the device tree. Otherwise the end user can not switch different
> driver mode through the kernel command line once the DC device node contain
> "use_vram_helper;" property.
> 
> This driver's author already made the decision by the time when the patch is
> sent out, even through this**may not proper.
> 
> The CMA helper based driver will be used by default, if the DC device node contain
> "use_vram_helper;" then VRAM based driver will be used. This is my initial intention.

DT isn't really a solution either. Let's take the distribution
perspective there. Suppose you're a Fedora or Debian developer, and want
to make a single kernel image, and ship a DT to the user for their board
without any modification. How is either the Kconfig solution or DT flags
solution any good there? It doesn't help them at all.

Maxime

Attachment: signature.asc
Description: PGP signature