2022-02-03 17:23:30

by Maxime Ripard

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

Hi,

On Thu, Feb 03, 2022 at 04:25:44PM +0800, Sui Jingfeng wrote:
> From: suijingfeng <[email protected]>
>
> There is a display controller in loongson's LS2K1000 SoC and LS7A1000
> bridge, and the DC in those chip is a PCI device. This patch provide
> a minimal support for this display controller which is mainly for
> graphic environment bring up.
>
> This display controller has two display pipes but with only one hardware
> cursor. Each way has a DVO output interface and the CRTC is able to scanout
> from 1920x1080 resolution at 60Hz. The maxmium resolution is 2048x2048@60hz.
>
> LS2K1000 is a SoC, only system memory is available. Therefore scanout from
> system memory is the only choice. We prefer the CMA helper base solution
> because the gc1000 gpu can use etnaviv driver, in this case etnaviv and
> lsdc could made a compatible pair. Even through it is possible to use VRAM
> helper base solution on ls2k1000 by carving out part of system memory as
> VRAM.
>
> For LS7A1000, there are 4 gpios whos control register is located at the dc
> register space which is not the geneal purpose GPIO. The 4 gpios can emulate
> two way i2c. One for DVO0, another for DVO1. This is the reason the why we
> are not using the drm bridge framework.
>
> LS2K1000 and LS2K0500 SoC don't have such hardware, they use general purpose
> GPIO emulated i2c or hardware i2c adapter from other module to serve this
> purpose. Drm bridge and drm panel is suitable for those SoC, we have already
> implement it on our own downstream kernel. But due to the upstream kernel
> don't have gpio, pwm and i2c driver support for LS2K1000. We just can not
> upstream our support for the drm bridge subsystem.
>
> The DC in LS7A1000 can scanout from both the system memory and the dedicate
> VRAM attached to the ddr3 memory controller. Sadly, only scanout from the
> VRAM is proved to be a reliable solution for massive product. Scanout from
> the system memory suffer from some hardware deficiency which cause the
> screen flickering under RAM pressure. This is the reason why we integrate
> two distict helpers into one piece of device driver. But CMA base helper is
> still usable on ls7a1000 for normal usage, expecially on ls7a1000+ bridge
> chip. We have also implemented demage update on top of CMA helper which
> copy the demaged shadow framebuffer region from system RAM to the real
> framebuffer in VRAM manually. Using "lsdc.dirty_update=1" in the commmand
> line will enable this driver mode.
>
> LS7A1000 have a 32x32 harware cursor, we just let the two CRTC share it
> simply with the help of universe plane. LS7A2000 have two 64x64 harware
> cursor. Surport for LS7A2000 is on the way.
>
> In short, we have built-in gpio emulated i2c support, we also have hardware
> cursor support. The kind of tiny drivers in drm/tiny is not suitable for us,
> we are not "tiny".
>
> +------+ HyperTransport 3.0
> | DDR4 | |
> +------+ | +------------------------------------+
> || MC0 | | LS7A1000 +------------|
> +----------+ | | | DDR3 | +------+
> | LS3A4000 |<--------->| +--------+ +-------+ | memory |<->| VRAM |
> | CPU |<--------->| | GC1000 | | LSDC | | controller | +------+
> +----------+ | +--------+ +-+---+-+ +------------|
> || MC1 +---------------|---|----------------+
> +------+ | |
> | DDR4 | +-------+ DVO0 | | DVO1 +------+
> +------+ VGA <--|ADV7125|<---------+ +------->|TFP410|--> DVI/HDMI
> +-------+ +------+
>
> The above picture give a simple usage of LS7A1000, note that the encoder
> is not necessary adv7125 or tfp410, it is a choice of the downstream board
> manufacturer. Other candicate encoder can be ch7034b, sil9022 and ite66121
> etc. Therefore, we need device tree to provide board specific information.
> Besides, the dc in both ls2k1000 and ls7k1000 have the vendor:device id of
> 0x0014:0x7a06, the reverison id is also same. We can't tell it apart simply
> (this is the firmware engineer's mistake). But firmware already flushed to
> the board and borad already sold out, we choose to resolve those issues by
> introduing device tree with board specific device support.
>
> For lsdc, there is only a 1:1 mapping of encoders and connectors.
>
> +-------------------+ _________
> | | | |
> | CRTC0 --> DVO0 ---------> Encoder0 --> Connector0 ---> | Monitor |
> | | ^ ^ |_________|
> | | | |
> | <----- i2c0 ----------------+
> | LSDC IP CORE |
> | <----- i2c1 ----------------+
> | | | | _________
> | | | | | |
> | CRTC1 --> DVO1 ---------> Encoder1 --> Connector1 ---> | Panel |
> | | |_________|
> +-------------------+
>
> Below is a brief introduction of loongson's CPU, bridge chip and SoC.
> LS2K1000 is a double core 1.0Ghz mips64r2 compatible SoC[1]. LS7A1000 is
> a bridge chip made by Loongson corporation which act as north and/or south
> bridge of loongson's desktop and server level processor. It is equivalent
> to AMD RS780E+SB710 or something like that. More details can be read from
> its user manual[2].
>
> This bridge chip is typically use with LS3A3000, LS3A4000 and LS3A5000 cpu.
> LS3A3000 is 4 core 1.45gHz mips64r2 compatible cpu.
> LS3A4000 is 4 core 1.8gHz mips64r5 compatible cpu.
> LS3A5000 is 4 core 2.5gHz loongarch cpu[3].
>
> Nearly all mordern loongson CPU's cache coherency is maintained by hardware,
> except for early version of ls2k1000. So we using cached coherent memory by
> default, not writecombine.
>
> v2: fixup warnings reported by kernel test robot
>
> v3: fix more grammar mistakes in Kconfig reported by Randy Dunlap and give
> more details about lsdc.
>
> v4:
> 1) Add dts required and explain why device tree is required.
> 2) Give more description about lsdc and vram helper base driver.
> 3) Fix warnings reported by kernel test robot.
> 4) Introduce stride_alignment member into struct lsdc_chip_desc, the
> stride alignment is 256 bytes for ls7a1000, ls2k1000 and ls2k0500 SoC.
> But ls7a2000 improve it to 32 bytes, We are prepare for extend the
> support for the on coming device.
>
> v5:
> 1) using writel and readl replace writeq and readq, to fix kernel test
> robot report build error on other archtecture
> 2) set default fb format to XRGB8888 at crtc reset time.
> 3) fix typos.
>
> v6:
> 1) Explain why we are not switch to drm dridge subsystem on ls2k1000.
> 2) Explain why tiny drm driver is not suitable for us.
> 3) Give a short description of the trival dirty uppdate implement based
> on CMA helper.
> 4) code clean up
>
> [1] https://wiki.debian.org/InstallingDebianOn/Lemote/Loongson2K1000
> [2] https://loongson.github.io/LoongArch-Documentation/Loongson-7A1000-usermanual-EN.html
> [3] https://loongson.github.io/LoongArch-Documentation/Loongson-3A5000-usermanual-EN.html
>
> Reported-by: Randy Dunlap <[email protected]>
> Reported-by: kernel test robot
> Signed-off-by: suijingfeng <[email protected]>
> Signed-off-by: Sui Jingfeng <[email protected]>
> ---
> drivers/gpu/drm/Kconfig | 2 +
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/lsdc/Kconfig | 38 ++
> drivers/gpu/drm/lsdc/Makefile | 15 +
> drivers/gpu/drm/lsdc/lsdc_connector.c | 443 ++++++++++++++
> drivers/gpu/drm/lsdc/lsdc_connector.h | 60 ++
> drivers/gpu/drm/lsdc/lsdc_crtc.c | 440 ++++++++++++++
> drivers/gpu/drm/lsdc/lsdc_drv.c | 846 ++++++++++++++++++++++++++
> drivers/gpu/drm/lsdc/lsdc_drv.h | 216 +++++++
> drivers/gpu/drm/lsdc/lsdc_encoder.c | 79 +++
> drivers/gpu/drm/lsdc/lsdc_i2c.c | 220 +++++++
> drivers/gpu/drm/lsdc/lsdc_i2c.h | 61 ++
> drivers/gpu/drm/lsdc/lsdc_irq.c | 77 +++
> drivers/gpu/drm/lsdc/lsdc_irq.h | 37 ++
> drivers/gpu/drm/lsdc/lsdc_plane.c | 681 +++++++++++++++++++++
> drivers/gpu/drm/lsdc/lsdc_pll.c | 657 ++++++++++++++++++++
> drivers/gpu/drm/lsdc/lsdc_pll.h | 109 ++++
> drivers/gpu/drm/lsdc/lsdc_regs.h | 246 ++++++++
> 18 files changed, 4228 insertions(+)
> create mode 100644 drivers/gpu/drm/lsdc/Kconfig
> create mode 100644 drivers/gpu/drm/lsdc/Makefile
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_connector.c
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_connector.h
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_crtc.c
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.c
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.h
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_encoder.c
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.c
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.h
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.c
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.h
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_plane.c
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.c
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.h
> create mode 100644 drivers/gpu/drm/lsdc/lsdc_regs.h
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index dfdd3ec5f793..18de1485e2ed 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -405,6 +405,8 @@ source "drivers/gpu/drm/gud/Kconfig"
>
> source "drivers/gpu/drm/sprd/Kconfig"
>
> +source "drivers/gpu/drm/lsdc/Kconfig"
> +
> config DRM_HYPERV
> tristate "DRM Support for Hyper-V synthetic video device"
> depends on DRM && PCI && MMU && HYPERV
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 8675c2af7ae1..2c5a76ced323 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -133,3 +133,4 @@ obj-y += xlnx/
> obj-y += gud/
> obj-$(CONFIG_DRM_HYPERV) += hyperv/
> obj-$(CONFIG_DRM_SPRD) += sprd/
> +obj-$(CONFIG_DRM_LSDC) += lsdc/
> 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.

> diff --git a/drivers/gpu/drm/lsdc/Makefile b/drivers/gpu/drm/lsdc/Makefile
> new file mode 100644
> index 000000000000..342990654478
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/Makefile
> @@ -0,0 +1,15 @@
> +#
> +# Makefile for the lsdc drm device driver.
> +#
> +
> +lsdc-y := \
> + lsdc_drv.o \
> + lsdc_crtc.o \
> + lsdc_irq.o \
> + lsdc_plane.o \
> + lsdc_pll.o \
> + lsdc_i2c.o \
> + lsdc_encoder.o \
> + lsdc_connector.o
> +
> +obj-$(CONFIG_DRM_LSDC) += lsdc.o
> diff --git a/drivers/gpu/drm/lsdc/lsdc_connector.c b/drivers/gpu/drm/lsdc/lsdc_connector.c
> new file mode 100644
> index 000000000000..ae5fc0c90961
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_connector.c
> @@ -0,0 +1,443 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */

That's the MIT license, yet you claim the driver to be licensed under
the GPLv2 or later?

> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +#include <drm/drm_print.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_connector.h>
> +
> +#include <video/videomode.h>
> +#include <video/of_display_timing.h>
> +
> +#include "lsdc_drv.h"
> +#include "lsdc_i2c.h"
> +#include "lsdc_connector.h"
> +
> +
> +static int lsdc_get_modes_from_edid(struct drm_connector *connector)
> +{
> + struct drm_device *ddev = connector->dev;
> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
> + struct edid *edid_p = (struct edid *)lconn->edid_data;
> + int num = drm_add_edid_modes(connector, edid_p);
> +
> + if (num)
> + drm_connector_update_edid_property(connector, edid_p);
> +
> + drm_dbg_kms(ddev, "%d modes added\n", num);
> +
> + return num;
> +}
> +
> +
> +static int lsdc_get_modes_from_timings(struct drm_connector *connector)
> +{
> + struct drm_device *ddev = connector->dev;
> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
> + struct display_timings *disp_tim = lconn->disp_tim;
> + unsigned int num = 0;
> + unsigned int i;
> +
> + for (i = 0; i < disp_tim->num_timings; i++) {
> + const struct display_timing *dt = disp_tim->timings[i];
> + struct drm_display_mode *mode;
> + struct videomode vm;
> +
> + videomode_from_timing(dt, &vm);
> + mode = drm_mode_create(ddev);
> + if (!mode) {
> + drm_err(ddev, "failed to add mode %ux%u\n",
> + dt->hactive.typ, dt->vactive.typ);
> + continue;
> + }
> +
> + drm_display_mode_from_videomode(&vm, mode);
> +
> + mode->type |= DRM_MODE_TYPE_DRIVER;
> +
> + if (i == disp_tim->native_mode)
> + mode->type |= DRM_MODE_TYPE_PREFERRED;
> +
> + drm_mode_probed_add(connector, mode);
> + num++;
> + }
> +
> + drm_dbg_kms(ddev, "%d modes added\n", num);
> +
> + return num;
> +}
> +
> +
> +static int lsdc_get_modes_from_ddc(struct drm_connector *connector,
> + struct i2c_adapter *ddc)
> +{
> + unsigned int num = 0;
> + struct edid *edid;
> +
> + edid = drm_get_edid(connector, ddc);
> + if (edid) {
> + drm_connector_update_edid_property(connector, edid);
> + num = drm_add_edid_modes(connector, edid);
> + kfree(edid);
> + }
> +
> + return num;
> +}
> +
> +
> +static int lsdc_get_modes(struct drm_connector *connector)
> +{
> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
> + unsigned int num = 0;
> +
> + if (lconn->has_edid)
> + return lsdc_get_modes_from_edid(connector);
> +
> + if (lconn->has_disp_tim)
> + return lsdc_get_modes_from_timings(connector);
> +
> + if (IS_ERR_OR_NULL(lconn->ddc) == false)
> + return lsdc_get_modes_from_ddc(connector, lconn->ddc);
> +
> + if (connector->connector_type == DRM_MODE_CONNECTOR_VIRTUAL) {
> + num = drm_add_modes_noedid(connector,
> + connector->dev->mode_config.max_width,
> + connector->dev->mode_config.max_height);
> +
> + drm_set_preferred_mode(connector, 1024, 768);
> +
> + return num;
> + }
> +
> +
> + /*
> + * In case we cannot retrieve the EDIDs (broken or missing i2c
> + * bus), fallback on the XGA standards, because we are for board
> + * bringup.
> + */
> + num = drm_add_modes_noedid(connector, 1920, 1200);
> +
> + /* And prefer a mode pretty much anyone can handle */
> + drm_set_preferred_mode(connector, 1024, 768);
> +
> + return num;
> +
> +}
> +
> +
> +static enum drm_connector_status
> +lsdc_connector_detect(struct drm_connector *connector, bool force)
> +{
> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
> +
> + if (lconn->has_edid == true)
> + return connector_status_connected;
> +
> + if (lconn->has_disp_tim == true)
> + return connector_status_connected;
> +
> + if (IS_ERR_OR_NULL(lconn->ddc) == false)
> + return drm_probe_ddc(lconn->ddc);
> +
> + if (lconn->ddc && drm_probe_ddc(lconn->ddc))
> + return connector_status_connected;
> +
> + if (connector->connector_type == DRM_MODE_CONNECTOR_VIRTUAL)
> + return connector_status_connected;
> +
> + if ((connector->connector_type == DRM_MODE_CONNECTOR_DVIA) ||
> + (connector->connector_type == DRM_MODE_CONNECTOR_DVID) ||
> + (connector->connector_type == DRM_MODE_CONNECTOR_DVII))
> + return connector_status_disconnected;
> +
> + if ((connector->connector_type == DRM_MODE_CONNECTOR_HDMIA) ||
> + (connector->connector_type == DRM_MODE_CONNECTOR_HDMIB))
> + return connector_status_disconnected;
> +
> + return connector_status_unknown;
> +}
> +
> +
> +/*
> + * @connector: point to the drm_connector structure
> + *
> + * Clean up connector resources
> + */
> +static void lsdc_connector_destroy(struct drm_connector *connector)
> +{
> + struct drm_device *ddev = connector->dev;
> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
> +
> + if (lconn) {
> + if (lconn->ddc)
> + lsdc_destroy_i2c(connector->dev, lconn->ddc);
> +
> + drm_info(ddev, "destroying connector%u\n", lconn->index);
> +
> + devm_kfree(ddev->dev, lconn);
> + }
> +
> + drm_connector_cleanup(connector);
> +}
> +
> +
> +static const struct drm_connector_helper_funcs lsdc_connector_helpers = {
> + .get_modes = lsdc_get_modes,
> +};
> +
> +/*
> + * These provide the minimum set of functions required to handle a connector
> + *
> + * Control connectors on a given device.
> + *
> + * Each CRTC may have one or more connectors attached to it.
> + * The functions below allow the core DRM code to control
> + * connectors, enumerate available modes, etc.
> + */
> +static const struct drm_connector_funcs lsdc_connector_funcs = {
> + .dpms = drm_helper_connector_dpms,
> + .detect = lsdc_connector_detect,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .destroy = lsdc_connector_destroy,
> + .reset = drm_atomic_helper_connector_reset,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +
> +/* Get the simple EDID data from the device tree
> + * the length must be EDID_LENGTH, since it is simple.
> + *
> + * @np: device node contain edid data
> + * @edid_data: where the edid data to store to
> + */
> +static bool lsdc_get_edid_from_dtb(struct device_node *np,
> + unsigned char *edid_data)
> +{
> + int length;
> + const void *prop;
> +
> + if (np == NULL)
> + return false;
> +
> + prop = of_get_property(np, "edid", &length);
> + if (prop && (length == EDID_LENGTH)) {
> + memcpy(edid_data, prop, EDID_LENGTH);
> + return true;
> + }
> +
> + return false;
> +}

You don't have a device tree binding for that driver, this is something
that is required. And it's not clear to me why you'd want EDID in the
DTB?

> +
> +/* Get display timings from the device tree
> + *
> + * @np: device node contain the display timings
> + * @pptim: point to where the pointer of struct display_timings store to
> + */
> +static void lsdc_get_display_timings_from_dtb(struct device_node *np,
> + struct display_timings **pptim)
> +{
> + struct display_timings *timings;
> +
> + if (np == NULL)
> + return;
> +
> + timings = of_get_display_timings(np);
> + if (timings)
> + *pptim = timings;
> +}
> +
> +
> +static int lsdc_get_connector_type(struct drm_device *ddev,
> + struct device_node *output,
> + unsigned int index)
> +{
> + const char *name;
> + int ret;
> +
> + ret = of_property_read_string(output, "connector", &name);
> + if (ret < 0)
> + return DRM_MODE_CONNECTOR_Unknown;
> +
> + if (strncmp(name, "vga-connector", 13) == 0) {
> + ret = DRM_MODE_CONNECTOR_VGA;
> + drm_info(ddev, "connector%d is VGA\n", index);
> + } else if (strncmp(name, "dvi-connector", 13) == 0) {
> + bool analog, digital;
> +
> + analog = of_property_read_bool(output, "analog");
> + digital = of_property_read_bool(output, "digital");
> +
> + if (analog && !digital)
> + ret = DRM_MODE_CONNECTOR_DVIA;
> + else if (analog && digital)
> + ret = DRM_MODE_CONNECTOR_DVII;
> + else
> + ret = DRM_MODE_CONNECTOR_DVID;
> +
> + drm_info(ddev, "connector%d is DVI\n", index);
> + } else if (strncmp(name, "virtual-connector", 17) == 0) {
> + ret = DRM_MODE_CONNECTOR_VIRTUAL;
> + drm_info(ddev, "connector%d is virtual\n", index);
> + } else if (strncmp(name, "dpi-connector", 13) == 0) {
> + ret = DRM_MODE_CONNECTOR_DPI;
> + drm_info(ddev, "connector%d is DPI\n", index);
> + } else if (strncmp(name, "hdmi-connector", 14) == 0) {
> + int res;
> + const char *hdmi_type;
> +
> + res = of_property_read_string(output, "type", &hdmi_type);
> + if (res == 0) {
> + if (!strcmp(hdmi_type, "b"))
> + ret = DRM_MODE_CONNECTOR_HDMIB;
> + else
> + ret = DRM_MODE_CONNECTOR_HDMIA;
> + } else
> + ret = DRM_MODE_CONNECTOR_HDMIA;
> +
> + drm_info(ddev, "connector%d is HDMI, type is %s\n",
> + index, hdmi_type);
> + } else {
> + ret = DRM_MODE_CONNECTOR_Unknown;
> + drm_info(ddev, "The type of connector%d is unknown\n", index);
> + }
> +
> + return ret;
> +}
> +
> +
> +struct lsdc_connector *lsdc_connector_init(struct lsdc_device *ldev,
> + unsigned int index)
> +{
> + struct drm_device *ddev = &ldev->drm;
> + struct device_node *np = ddev->dev->of_node;
> + struct device_node *output = NULL;
> + struct lsdc_connector *lconn;
> + struct drm_connector *connector;
> + bool available = false;
> + unsigned int connector_type;
> + int ret;
> +
> + lconn = devm_kzalloc(ddev->dev, sizeof(*lconn), GFP_KERNEL);
> + if (lconn == NULL)
> + return ERR_PTR(-ENOMEM);
> +
> + lconn->index = index;
> +
> + output = of_parse_phandle(np, "output-ports", index);
> + if (output) {
> + struct device_node *disp_tims_np;
> +
> + available = of_device_is_available(output);
> +
> + if (available == false) {
> + drm_info(ddev, "connector%d is not available\n", index);
> + of_node_put(output);
> + return NULL;
> + }
> +
> + lconn->has_edid = of_property_read_bool(output, "edid");
> + disp_tims_np = of_get_child_by_name(output, "display-timings");
> + if (disp_tims_np) {
> + of_node_put(disp_tims_np);
> + lconn->has_disp_tim = true;
> + } else
> + lconn->has_disp_tim = false;
> + } else
> + drm_warn(ddev, "no output-ports property, please update dtb\n");
> +
> + /*
> + * Providing a blindly support even through there is
> + * no output-ports property in the dtb.
> + */
> + if (lconn->has_edid) {
> + lsdc_get_edid_from_dtb(output, lconn->edid_data);
> + drm_info(ddev, "connector%d provide edid\n", index);
> + }
> +
> + if (lconn->has_disp_tim) {
> + lsdc_get_display_timings_from_dtb(output, &lconn->disp_tim);
> + drm_info(ddev, "connector%d provide display timings\n", index);
> + }
> +
> + connector_type = lsdc_get_connector_type(ddev, output, index);
> +
> + if (output)
> + of_node_put(output);
> +
> + connector = &lconn->base;
> +
> + if (connector_type == DRM_MODE_CONNECTOR_VIRTUAL)
> + goto SKIPED_CREATE_DDC;
> +
> + /* bypass the ddc creation if the edid or display timing is provided */
> + if ((lconn->has_edid == false) &&
> + (lconn->has_disp_tim == false)) {
> + const struct lsdc_chip_desc * const dc = ldev->desc;
> +
> + if (dc->have_builtin_i2c)
> + lconn->ddc = lsdc_create_i2c_chan(ddev, index);
> + else
> + lconn->ddc = lsdc_get_i2c_adapter(ddev, index);
> +
> + if (lconn->ddc && (IS_ERR(lconn->ddc) == false)) {
> + drm_info(ddev, "i2c%d for connector%d created\n",
> + i2c_adapter_id(lconn->ddc), index);
> +
> + } else
> + drm_warn(ddev, "Get i2c adapter failed: %ld\n",
> + PTR_ERR(lconn->ddc));
> + }
> +
> + /* only pull if the connector have a ddc */
> + connector->polled = DRM_CONNECTOR_POLL_CONNECT |
> + DRM_CONNECTOR_POLL_DISCONNECT;
> +
> +SKIPED_CREATE_DDC:
> + ret = drm_connector_init_with_ddc(ddev,
> + connector,
> + &lsdc_connector_funcs,
> + connector_type,
> + lconn->ddc);
> + if (ret) {
> + drm_err(ddev, "init connector%d failed\n", index);
> + goto err_i2c_destroy;
> + }
> +
> + drm_connector_helper_add(connector, &lsdc_connector_helpers);
> +
> + return lconn;
> +
> +err_i2c_destroy:
> + lsdc_destroy_i2c(ddev, lconn->ddc);
> + return NULL;
> +}
> diff --git a/drivers/gpu/drm/lsdc/lsdc_connector.h b/drivers/gpu/drm/lsdc/lsdc_connector.h
> new file mode 100644
> index 000000000000..e9f94a969f74
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_connector.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +
> +#ifndef __LSDC_CONNECTOR_H__
> +#define __LSDC_CONNECTOR_H__
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_connector.h>
> +
> +struct lsdc_connector {
> + struct drm_connector base;
> +
> + struct i2c_adapter *ddc;
> +
> + /* pass EDID from dtb support */
> + unsigned char edid_data[EDID_LENGTH];
> + bool has_edid;
> +
> + /* pass display timmings from dtb support */
> + struct display_timings *disp_tim;
> + bool has_disp_tim;
> +
> + int index;
> +};
> +
> +#define to_lsdc_connector(x) \
> + container_of(x, struct lsdc_connector, base)
> +
> +struct lsdc_connector *lsdc_connector_init(struct lsdc_device *ldev,
> + unsigned int index);
> +
> +#endif
> diff --git a/drivers/gpu/drm/lsdc/lsdc_crtc.c b/drivers/gpu/drm/lsdc/lsdc_crtc.c
> new file mode 100644
> index 000000000000..7531389f4896
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_crtc.c
> @@ -0,0 +1,440 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +#include <drm/drm_print.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_plane.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_vblank.h>
> +#include <drm/drm_drv.h>
> +
> +
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
> +
> +#include "lsdc_drv.h"
> +#include "lsdc_regs.h"
> +#include "lsdc_pll.h"
> +
> +
> +static int lsdc_crtc_enable_vblank(struct drm_crtc *crtc)
> +{
> + struct lsdc_device *ldev = to_lsdc(crtc->dev);
> + unsigned int index = drm_crtc_index(crtc);
> + struct drm_crtc_state *state = crtc->state;
> + u32 val;
> +
> + if (state->enable) {
> + val = lsdc_reg_read32(ldev, LSDC_INT_REG);
> +
> + if (index == 0)
> + val |= INT_CRTC0_VS_EN;
> + else if (index == 1)
> + val |= INT_CRTC1_VS_EN;
> +
> + lsdc_reg_write32(ldev, LSDC_INT_REG, val);
> + }
> +
> + return 0;
> +}
> +
> +
> +static void lsdc_crtc_disable_vblank(struct drm_crtc *crtc)
> +{
> + struct lsdc_device *ldev = to_lsdc(crtc->dev);
> + unsigned int index = drm_crtc_index(crtc);
> + u32 val;
> +
> + val = lsdc_reg_read32(ldev, LSDC_INT_REG);
> +
> + if (index == 0)
> + val &= ~INT_CRTC0_VS_EN;
> + else if (index == 1)
> + val &= ~INT_CRTC1_VS_EN;
> +
> + lsdc_reg_write32(ldev, LSDC_INT_REG, val);
> +}
> +
> +
> +static void lsdc_crtc_reset(struct drm_crtc *crtc)
> +{
> + struct drm_device *ddev = crtc->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + unsigned int index = drm_crtc_index(crtc);
> + struct lsdc_crtc_state *priv_crtc_state;
> + u32 val;
> +
> + /* The crtc get soft reset if bit 20 of CRTC*_CFG_REG
> + * is write with falling edge.
> + *
> + * Doing this to switch from soft reset state to working state
> + */
> + if (index == 0) {
> + val = CFG_RESET_BIT | CFG_OUTPUT_EN_BIT | LSDC_PF_XRGB8888;
> + lsdc_reg_write32(ldev, LSDC_CRTC0_CFG_REG, val);
> + } else if (index == 1) {
> + val = CFG_RESET_BIT | CFG_OUTPUT_EN_BIT | LSDC_PF_XRGB8888;
> + lsdc_reg_write32(ldev, LSDC_CRTC1_CFG_REG, val);
> + }
> +
> +
> + if (crtc->state) {
> + priv_crtc_state = to_lsdc_crtc_state(crtc->state);
> + __drm_atomic_helper_crtc_destroy_state(&priv_crtc_state->base);
> + kfree(priv_crtc_state);
> + }
> +
> + priv_crtc_state = kzalloc(sizeof(*priv_crtc_state), GFP_KERNEL);
> + if (!priv_crtc_state)
> + return;
> +
> + priv_crtc_state->pix_fmt = val & CFG_PIX_FMT_MASK;
> +
> + __drm_atomic_helper_crtc_reset(crtc, &priv_crtc_state->base);
> +
> + drm_info(ddev, "crtc%u reset\n", index);
> +}
> +
> +
> +static void lsdc_crtc_atomic_destroy_state(struct drm_crtc *crtc,
> + struct drm_crtc_state *state)
> +{
> + struct lsdc_crtc_state *priv_crtc_state = to_lsdc_crtc_state(state);
> +
> + __drm_atomic_helper_crtc_destroy_state(&priv_crtc_state->base);
> +
> + kfree(priv_crtc_state);
> +}
> +
> +
> +static struct drm_crtc_state *lsdc_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
> +{
> + struct lsdc_crtc_state *new_priv_state;
> + struct lsdc_crtc_state *old_priv_state;
> + struct drm_device *ddev = crtc->dev;
> +
> + if (drm_WARN_ON(ddev, !crtc->state))
> + return NULL;
> +
> + new_priv_state = kmalloc(sizeof(*new_priv_state), GFP_KERNEL);
> + if (!new_priv_state)
> + return NULL;
> +
> + __drm_atomic_helper_crtc_duplicate_state(crtc, &new_priv_state->base);
> +
> + old_priv_state = to_lsdc_crtc_state(crtc->state);
> +
> + memcpy(&new_priv_state->pparams, &old_priv_state->pparams,
> + sizeof(new_priv_state->pparams));
> +
> + new_priv_state->pix_fmt = old_priv_state->pix_fmt;
> +
> + return &new_priv_state->base;
> +}
> +
> +
> +static const struct drm_crtc_funcs lsdc_crtc_funcs = {
> + .reset = lsdc_crtc_reset,
> + .destroy = drm_crtc_cleanup,
> + .set_config = drm_atomic_helper_set_config,
> + .page_flip = drm_atomic_helper_page_flip,
> + .atomic_duplicate_state = lsdc_crtc_atomic_duplicate_state,
> + .atomic_destroy_state = lsdc_crtc_atomic_destroy_state,
> + .enable_vblank = lsdc_crtc_enable_vblank,
> + .disable_vblank = lsdc_crtc_disable_vblank,
> +};
> +
> +
> +static enum drm_mode_status
> +lsdc_crtc_helper_mode_valid(struct drm_crtc *crtc,
> + const struct drm_display_mode *mode)
> +{
> + struct drm_device *ddev = crtc->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + const struct lsdc_chip_desc *desc = ldev->desc;
> +
> + if (mode->hdisplay > desc->max_width)
> + return MODE_BAD_HVALUE;
> + if (mode->vdisplay > desc->max_height)
> + return MODE_BAD_VVALUE;
> +
> + if (mode->clock > desc->max_pixel_clk) {
> + drm_dbg_kms(ddev, "mode %dx%d, pixel clock=%d is too high\n",
> + mode->hdisplay, mode->vdisplay, mode->clock);
> + return MODE_CLOCK_HIGH;
> + }
> +
> + /* the crtc hardware dma take 256 bytes once a time
> + * TODO: check RGB565 support
> + */
> + if ((mode->hdisplay * 4) % desc->stride_alignment) {
> + drm_dbg_kms(ddev, "stride is not %u bytes aligned\n",
> + desc->stride_alignment);
> + return MODE_BAD;
> + }
> +
> + return MODE_OK;
> +}

mode_valid will only prevent the mode from being advertised to the
userspace, but you need atomic_check if you want to prevent those modes
to be used by anybody.

> +
> +static void lsdc_update_pixclk(struct drm_crtc *crtc, unsigned int pixclk, bool verbose)
> +{
> + struct lsdc_display_pipe *dispipe;
> + struct lsdc_pll *pixpll;
> + const struct lsdc_pixpll_funcs *clkfun;
> + struct lsdc_crtc_state *priv_crtc_state;
> +
> + priv_crtc_state = to_lsdc_crtc_state(crtc->state);
> +
> + dispipe = container_of(crtc, struct lsdc_display_pipe, crtc);
> + pixpll = &dispipe->pixpll;
> + clkfun = pixpll->funcs;
> +
> + /* config the pixel pll */
> + clkfun->update(pixpll, &priv_crtc_state->pparams);
> +
> + if (verbose)
> + clkfun->print(pixpll, pixclk);
> +}
> +
> +
> +static void lsdc_crtc_helper_mode_set_nofb(struct drm_crtc *crtc)
> +{
> + struct drm_device *ddev = crtc->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + struct drm_display_mode *mode = &crtc->state->adjusted_mode;
> + unsigned int hr = mode->hdisplay;
> + unsigned int hss = mode->hsync_start;
> + unsigned int hse = mode->hsync_end;
> + unsigned int hfl = mode->htotal;
> + unsigned int vr = mode->vdisplay;
> + unsigned int vss = mode->vsync_start;
> + unsigned int vse = mode->vsync_end;
> + unsigned int vfl = mode->vtotal;
> + unsigned int pixclock = mode->clock;
> + unsigned int index = drm_crtc_index(crtc);
> +
> +
> + if (index == 0) {
> + /* CRTC 0 */
> + u32 hsync, vsync;
> +
> + lsdc_reg_write32(ldev, LSDC_CRTC0_FB_ORIGIN_REG, 0);
> +
> + /* 26:16 total pixels, 10:0 visiable pixels, in horizontal */
> + lsdc_reg_write32(ldev, LSDC_CRTC0_HDISPLAY_REG,
> + (mode->crtc_htotal << 16) | mode->crtc_hdisplay);
> +
> + /* 26:16 total pixels, 10:0 visiable pixels, in vertical */
> + lsdc_reg_write32(ldev, LSDC_CRTC0_VDISPLAY_REG,
> + (mode->crtc_vtotal << 16) | mode->crtc_vdisplay);
> +
> + /* 26:16 hsync end, 10:0 hsync start */
> + hsync = (mode->crtc_hsync_end << 16) | mode->crtc_hsync_start;
> +
> + if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> + hsync |= INV_HSYNC_BIT;
> +
> + lsdc_reg_write32(ldev, LSDC_CRTC0_HSYNC_REG, EN_HSYNC_BIT | hsync);
> +
> + /* 26:16 vsync end, 10:0 vsync start */
> + vsync = (mode->crtc_vsync_end << 16) | mode->crtc_vsync_start;
> +
> + if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> + vsync |= INV_VSYNC_BIT;
> +
> + lsdc_reg_write32(ldev, LSDC_CRTC0_VSYNC_REG, EN_VSYNC_BIT | vsync);
> +
> + } else if (index == 1) {
> + /* CRTC 1 */
> + u32 hsync, vsync;
> +
> + lsdc_reg_write32(ldev, LSDC_CRTC1_FB_ORIGIN_REG, 0);
> +
> + /* 26:16 total pixels, 10:0 visiable pixels, in horizontal */
> + lsdc_reg_write32(ldev, LSDC_CRTC1_HDISPLAY_REG,
> + (mode->crtc_htotal << 16) | mode->crtc_hdisplay);
> +
> + /* 26:16 total pixels, 10:0 visiable pixels, in vertical */
> + lsdc_reg_write32(ldev, LSDC_CRTC1_VDISPLAY_REG,
> + (mode->crtc_vtotal << 16) | mode->crtc_vdisplay);
> +
> + /* 26:16 hsync end, 10:0 hsync start */
> + hsync = (mode->crtc_hsync_end << 16) | mode->crtc_hsync_start;
> +
> + if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> + hsync |= INV_HSYNC_BIT;
> +
> + lsdc_reg_write32(ldev, LSDC_CRTC1_HSYNC_REG, EN_HSYNC_BIT | hsync);
> +
> + /* 26:16 vsync end, 10:0 vsync start */
> + vsync = (mode->crtc_vsync_end << 16) | mode->crtc_vsync_start;
> +
> + if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> + vsync |= INV_VSYNC_BIT;
> +
> + lsdc_reg_write32(ldev, LSDC_CRTC1_VSYNC_REG, EN_VSYNC_BIT | vsync);
> + }
> +
> + drm_dbg_kms(ddev, "hdisplay=%d, hsync_start=%d, hsync_end=%d, htotal=%d\n",
> + hr, hss, hse, hfl);
> +
> + drm_dbg_kms(ddev, "vdisplay=%d, vsync_start=%d, vsync_end=%d, vtotal=%d\n",
> + vr, vss, vse, vfl);
> +
> + drm_dbg_kms(ddev, "%s modeset: %ux%u\n", crtc->name, hr, vr);
> +
> + lsdc_update_pixclk(crtc, pixclock, ldev->verbose);
> +}
> +
> +
> +static void lsdc_enable_display(struct lsdc_device *ldev, unsigned int index)
> +{
> + u32 val;
> +
> + if (index == 0) {
> + val = lsdc_reg_read32(ldev, LSDC_CRTC0_CFG_REG);
> + val |= CFG_OUTPUT_EN_BIT;
> + lsdc_reg_write32(ldev, LSDC_CRTC0_CFG_REG, val);
> + } else if (index == 1) {
> + val = lsdc_reg_read32(ldev, LSDC_CRTC1_CFG_REG);
> + val |= CFG_OUTPUT_EN_BIT;
> + lsdc_reg_write32(ldev, LSDC_CRTC1_CFG_REG, val);
> + }
> +}
> +
> +
> +static void lsdc_disable_display(struct lsdc_device *ldev, unsigned int index)
> +{
> + u32 val;
> +
> + if (index == 0) {
> + val = lsdc_reg_read32(ldev, LSDC_CRTC0_CFG_REG);
> + val &= ~CFG_OUTPUT_EN_BIT;
> + lsdc_reg_write32(ldev, LSDC_CRTC0_CFG_REG, val);
> + } else if (index == 1) {
> + val = lsdc_reg_read32(ldev, LSDC_CRTC1_CFG_REG);
> + val &= ~CFG_OUTPUT_EN_BIT;
> + lsdc_reg_write32(ldev, LSDC_CRTC1_CFG_REG, val);
> + }
> +}
> +
> +
> +static void lsdc_crtc_helper_atomic_enable(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct drm_device *ddev = crtc->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> +
> + drm_crtc_vblank_on(crtc);
> +
> + lsdc_enable_display(ldev, drm_crtc_index(crtc));
> +
> + drm_dbg_kms(ddev, "%s: enabled\n", crtc->name);
> +}
> +
> +
> +static void lsdc_crtc_helper_atomic_disable(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct drm_device *ddev = crtc->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> +
> + drm_crtc_vblank_off(crtc);
> +
> + lsdc_disable_display(ldev, drm_crtc_index(crtc));
> +
> + drm_dbg_kms(ddev, "%s: disabled\n", crtc->name);
> +}
> +
> +
> +static void lsdc_crtc_atomic_flush(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct drm_pending_vblank_event *event = crtc->state->event;
> +
> + if (event) {
> + crtc->state->event = NULL;
> +
> + spin_lock_irq(&crtc->dev->event_lock);
> + if (drm_crtc_vblank_get(crtc) == 0)
> + drm_crtc_arm_vblank_event(crtc, event);
> + else
> + drm_crtc_send_vblank_event(crtc, event);
> + spin_unlock_irq(&crtc->dev->event_lock);
> + }
> +}
> +
> +
> +static const struct drm_crtc_helper_funcs lsdc_crtc_helper_funcs = {
> + .mode_valid = lsdc_crtc_helper_mode_valid,
> + .mode_set_nofb = lsdc_crtc_helper_mode_set_nofb,
> + .atomic_enable = lsdc_crtc_helper_atomic_enable,
> + .atomic_disable = lsdc_crtc_helper_atomic_disable,
> + .atomic_flush = lsdc_crtc_atomic_flush,
> +};
> +
> +
> +
> +/**
> + * lsdc_crtc_init
> + *
> + * @ddev: point to the drm_device structure
> + * @index: hardware crtc index
> + *
> + * Init CRTC
> + */
> +int lsdc_crtc_init(struct drm_device *ddev,
> + struct drm_crtc *crtc,
> + unsigned int index,
> + struct drm_plane *primary,
> + struct drm_plane *cursor)
> +{
> + int ret;
> +
> + drm_crtc_helper_add(crtc, &lsdc_crtc_helper_funcs);
> +
> + ret = drm_mode_crtc_set_gamma_size(crtc, 256);
> + if (ret)
> + drm_warn(ddev, "set the gamma table size failed\n");
> +
> + return drm_crtc_init_with_planes(ddev,
> + crtc,
> + primary,
> + cursor,
> + &lsdc_crtc_funcs,
> + "crtc%d",
> + index);
> +}
> diff --git a/drivers/gpu/drm/lsdc/lsdc_drv.c b/drivers/gpu/drm/lsdc/lsdc_drv.c
> new file mode 100644
> index 000000000000..aac8901c3431
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_drv.c
> @@ -0,0 +1,846 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/of_reserved_mem.h>
> +
> +#include <drm/drm_drv.h>
> +#include <drm/drm_aperture.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_plane.h>
> +#include <drm/drm_vblank.h>
> +#include <drm/drm_debugfs.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
> +#include <drm/drm_probe_helper.h>
> +
> +#include "lsdc_drv.h"
> +#include "lsdc_irq.h"
> +#include "lsdc_regs.h"
> +#include "lsdc_connector.h"
> +#include "lsdc_pll.h"
> +
> +
> +#define DRIVER_AUTHOR "Sui Jingfeng <[email protected]>"
> +#define DRIVER_NAME "lsdc"
> +#define DRIVER_DESC "drm driver for loongson's display controller"
> +#define DRIVER_DATE "20200701"
> +#define DRIVER_MAJOR 1
> +#define DRIVER_MINOR 0
> +#define DRIVER_PATCHLEVEL 0
> +
> +static int lsdc_modeset = 1;
> +MODULE_PARM_DESC(modeset, "Enable/disable CMA-based KMS(1 = enabled(default), 0 = disabled)");
> +module_param_named(modeset, lsdc_modeset, int, 0644);
> +
> +static int lsdc_cached_coherent = 1;
> +MODULE_PARM_DESC(cached_coherent, "uss cached coherent mapping(1 = enabled(default), 0 = disabled)");
> +module_param_named(cached_coherent, lsdc_cached_coherent, int, 0644);
> +
> +static int lsdc_dirty_update = -1;
> +MODULE_PARM_DESC(dirty_update, "enable dirty update(1 = enabled, 0 = disabled(default))");
> +module_param_named(dirty_update, lsdc_dirty_update, int, 0644);
> +
> +static int lsdc_use_vram_helper = -1;
> +MODULE_PARM_DESC(use_vram_helper, "use vram helper based solution(1 = enabled, 0 = disabled(default))");
> +module_param_named(use_vram_helper, lsdc_use_vram_helper, int, 0644);
> +
> +static int lsdc_verbose = -1;
> +MODULE_PARM_DESC(verbose, "Enable/disable print some key information");
> +module_param_named(verbose, lsdc_verbose, int, 0644);

It's not really clear to me why you need any of those parameters. Why
would a user want to use a non coherent mapping for example?

> +
> +static const struct lsdc_chip_desc dc_in_ls2k1000 = {
> + .chip = LSDC_CHIP_2K1000,
> + .num_of_crtc = LSDC_MAX_CRTC,
> + /* ls2k1000 user manual say the max pixel clock can be about 200MHz */
> + .max_pixel_clk = 200000,
> + .max_width = 2560,
> + .max_height = 2048,
> + .num_of_hw_cursor = 1,
> + .hw_cursor_w = 32,
> + .hw_cursor_h = 32,
> + .have_builtin_i2c = false,
> + .stride_alignment = 256,
> +};
> +
> +static const struct lsdc_chip_desc dc_in_ls2k0500 = {
> + .chip = LSDC_CHIP_2K0500,
> + .num_of_crtc = LSDC_MAX_CRTC,
> + .max_pixel_clk = 200000,
> + .max_width = 2048,
> + .max_height = 2048,
> + .num_of_hw_cursor = 1,
> + .hw_cursor_w = 32,
> + .hw_cursor_h = 32,
> + .have_builtin_i2c = false,
> + .stride_alignment = 256,
> +};
> +
> +static const struct lsdc_chip_desc dc_in_ls7a1000 = {
> + .chip = LSDC_CHIP_7A1000,
> + .num_of_crtc = LSDC_MAX_CRTC,
> + .max_pixel_clk = 200000,
> + .max_width = 2048,
> + .max_height = 2048,
> + .num_of_hw_cursor = 1,
> + .hw_cursor_w = 32,
> + .hw_cursor_h = 32,
> + .have_builtin_i2c = true,
> + .stride_alignment = 256,
> +};
> +
> +/*
> + * We need the device tree provided additional information to tell the DC
> + * in different chip apart.
> + */
> +static const struct of_device_id lsdc_drm_of_match[] = {
> + { .compatible = "loongson,loongson64c-4core-ls7a", .data = &dc_in_ls7a1000 },
> + { .compatible = "loongson,loongson64g-4core-ls7a", .data = &dc_in_ls7a1000 },
> + { .compatible = "loongson,loongson64-2core-2k1000", .data = &dc_in_ls2k1000 },
> + { .compatible = "loongson,loongson2k1000", .data = &dc_in_ls2k1000 },
> + { .compatible = "loongson,ls2k1000", .data = &dc_in_ls2k1000 },
> + { .compatible = "loongson,display-subsystem", .data = &dc_in_ls2k1000 },
> + { .compatible = "loongson,ls-fb", .data = &dc_in_ls2k1000 },
> + { .compatible = "loongson,loongson2k0500", .data = &dc_in_ls2k0500 },
> + { /* sentinel */ },
> +};
> +
> +
> +static struct drm_framebuffer *
> +lsdc_drm_gem_fb_create(struct drm_device *ddev, struct drm_file *file,
> + const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + struct lsdc_device *ldev = to_lsdc(ddev);
> +
> + if (ldev->dirty_update)
> + return drm_gem_fb_create_with_dirty(ddev, file, mode_cmd);
> +
> + return drm_gem_fb_create(ddev, file, mode_cmd);
> +}
> +
> +
> +static enum drm_mode_status lsdc_device_mode_valid(struct drm_device *ddev,
> + const struct drm_display_mode *mode)
> +{
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> + struct lsdc_device *ldev = to_lsdc(ddev);
> +
> + if (ldev->use_vram_helper == true)
> + return drm_vram_helper_mode_valid(ddev, mode);
> +#endif
> +
> + return MODE_OK;
> +}
> +
> +
> +static const struct drm_mode_config_funcs lsdc_mode_config_funcs = {
> + .fb_create = lsdc_drm_gem_fb_create,
> + .output_poll_changed = drm_fb_helper_output_poll_changed,
> + .atomic_check = drm_atomic_helper_check,
> + .atomic_commit = drm_atomic_helper_commit,
> + .mode_valid = lsdc_device_mode_valid,
> +};
> +
> +
> +#ifdef CONFIG_DEBUG_FS
> +static int lsdc_show_pxlclock(struct seq_file *m, void *arg)
> +{
> + struct drm_info_node *node = (struct drm_info_node *)m->private;
> + struct drm_device *ddev = node->minor->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + struct drm_crtc *crtc;
> +
> + drm_for_each_crtc(crtc, ddev) {
> + struct drm_crtc_state *state = crtc->state;
> + int index = drm_crtc_index(crtc);
> + struct lsdc_display_pipe *pipe = &ldev->disp_pipe[index];
> + struct lsdc_pll *pixpll = &pipe->pixpll;
> + const struct lsdc_pixpll_funcs *clkfun = pixpll->funcs;
> + unsigned int clkrate = clkfun->get_clock_rate(pixpll);
> + unsigned int mode_clock = crtc->mode.crtc_clock;
> +
> + seq_printf(m, "\n");
> + seq_printf(m, "CRTC%u, %s, %s\n", index,
> + state->active ? "active" : "non-active",
> + state->enable ? "enabled" : "disabled");
> + seq_printf(m, "hw clock: %u kHz\n", clkrate);
> + seq_printf(m, "mode: %u kHz\n", mode_clock);
> +
> + seq_printf(m, "div_out=%u, loopc=%u, div_ref=%u\n",
> + pixpll->core_params.div_out,
> + pixpll->core_params.loopc,
> + pixpll->core_params.div_ref);
> + }
> +
> + return 0;
> +}
> +
> +
> +static int lsdc_mm_show(struct seq_file *m, void *arg)
> +{
> + struct drm_info_node *node = (struct drm_info_node *) m->private;
> + struct drm_device *ddev = node->minor->dev;
> + struct drm_printer p = drm_seq_file_printer(m);
> +
> + drm_mm_print(&ddev->vma_offset_manager->vm_addr_space_mm, &p);
> + return 0;
> +}
> +
> +static struct drm_info_list lsdc_debugfs_list[] = {
> + { "clocks", lsdc_show_pxlclock, 0 },
> + { "mm", lsdc_mm_show, 0, NULL },
> +};
> +
> +static void lsdc_debugfs_init(struct drm_minor *minor)
> +{
> + drm_debugfs_create_files(lsdc_debugfs_list,
> + ARRAY_SIZE(lsdc_debugfs_list),
> + minor->debugfs_root,
> + minor);
> +}
> +#endif
> +
> +
> +static struct drm_gem_object *
> +lsdc_drm_gem_create_object(struct drm_device *ddev, size_t size)
> +{
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + struct drm_gem_cma_object *obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> +
> + if (!obj)
> + return ERR_PTR(-ENOMEM);
> +
> + if (ldev->cached_coherent)
> + obj->map_noncoherent = true;
> +
> + return &obj->base;
> +}
> +
> +
> +static int lsdc_gem_cma_dumb_create(struct drm_file *file,
> + struct drm_device *ddev,
> + struct drm_mode_create_dumb *args)
> +{
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + const struct lsdc_chip_desc *desc = ldev->desc;
> + unsigned int bytes_per_pixel = (args->bpp + 7) / 8;
> + unsigned int pitch = bytes_per_pixel * args->width;
> +
> + /*
> + * The dc in ls7a1000/ls2k1000/ls2k0500 require the stride be a
> + * multiple of 256 bytes, which is for sake of optimize dma data
> + * transfer.
> + */
> + args->pitch = roundup(pitch, desc->stride_alignment);
> +
> + return drm_gem_cma_dumb_create_internal(file, ddev, args);
> +}
> +
> +
> +DEFINE_DRM_GEM_CMA_FOPS(lsdc_drv_fops);
> +
> +
> +static const struct drm_driver lsdc_drm_driver_cma_stub = {
> + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> +
> + .lastclose = drm_fb_helper_lastclose,
> + .fops = &lsdc_drv_fops,
> + .gem_create_object = lsdc_drm_gem_create_object,
> +
> + .name = DRIVER_NAME,
> + .desc = DRIVER_DESC,
> + .date = DRIVER_DATE,
> + .major = DRIVER_MAJOR,
> + .minor = DRIVER_MINOR,
> + .patchlevel = DRIVER_PATCHLEVEL,
> +
> + DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(lsdc_gem_cma_dumb_create),
> +
> +#ifdef CONFIG_DEBUG_FS
> + .debugfs_init = lsdc_debugfs_init,
> +#endif
> +};
> +
> +
> +DEFINE_DRM_GEM_FOPS(lsdc_fops);
> +
> +
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> +static const struct drm_driver lsdc_vram_driver_stub = {
> + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
> + .fops = &lsdc_fops,
> +
> + .name = DRIVER_NAME,
> + .desc = DRIVER_DESC,
> + .date = DRIVER_DATE,
> + .major = DRIVER_MAJOR,
> + .minor = DRIVER_MINOR,
> + .patchlevel = DRIVER_PATCHLEVEL,
> +
> + DRM_GEM_VRAM_DRIVER,
> +};
> +#endif
> +
> +
> +static int lsdc_modeset_init(struct lsdc_device *ldev, const uint32_t num_crtc)
> +{
> + struct drm_device *ddev = &ldev->drm;
> + struct lsdc_display_pipe *dispipe;
> + struct lsdc_connector *lconn;
> + unsigned int i;
> + int ret;
> +
> + /* first find all of connector available */
> + for (i = 0; i < num_crtc; i++) {
> + lconn = lsdc_connector_init(ldev, i);
> + dispipe = &ldev->disp_pipe[i];
> +
> + if (IS_ERR(lconn))
> + return PTR_ERR(lconn);
> + else if (lconn == NULL) {
> + dispipe->lconn = NULL;
> + dispipe->available = false;
> + continue;
> + }
> +
> + /* take a record */
> + dispipe->available = true;
> + dispipe->lconn = lconn;
> + ldev->num_output++;
> + }
> +
> + drm_info(ddev, "number of outputs: %u\n", ldev->num_output);
> +
> + for (i = 0; i < num_crtc; i++) {
> + struct lsdc_display_pipe * const dispipe = &ldev->disp_pipe[i];
> + struct drm_plane * const primary = &dispipe->primary;
> + struct drm_plane * const cursor = &dispipe->cursor;
> + struct drm_encoder * const encoder = &dispipe->encoder;
> + struct drm_crtc * const crtc = &dispipe->crtc;
> + struct lsdc_pll * const pixpll = &dispipe->pixpll;
> +
> + dispipe->index = i;
> +
> + ret = lsdc_pixpll_init(pixpll, ddev, i);
> + if (ret)
> + return ret;
> +
> + ret = lsdc_plane_init(ldev, primary, DRM_PLANE_TYPE_PRIMARY, i);
> + if (ret)
> + return ret;
> +
> + ret = lsdc_plane_init(ldev, cursor, DRM_PLANE_TYPE_CURSOR, i);
> + if (ret)
> + return ret;
> +
> + /*
> + * Initial all of the CRTC available, in this way the crtc
> + * index is equal to the hardware crtc index. That is:
> + * display pipe 0 = crtc0 + dvo0 + encoder0
> + * display pipe 1 = crtc1 + dvo1 + encoder1
> + */
> + ret = lsdc_crtc_init(ddev, crtc, i, primary, cursor);
> + if (ret)
> + return ret;
> +
> + if (dispipe->available) {
> + ret = lsdc_encoder_init(encoder,
> + &dispipe->lconn->base,
> + ddev,
> + i,
> + ldev->num_output);
> + if (ret)
> + return ret;
> + }
> +
> + drm_info(ddev, "display pipe %u initialized\n", i);
> + }
> +
> + return 0;
> +}
> +
> +
> +static int lsdc_mode_config_init(struct lsdc_device *ldev)
> +{
> + const struct lsdc_chip_desc * const descp = ldev->desc;
> + struct drm_device *ddev = &ldev->drm;
> + int ret;
> +
> + spin_lock_init(&ldev->reglock);
> +
> + drm_mode_config_init(ddev);
> +
> + ddev->mode_config.funcs = &lsdc_mode_config_funcs;
> + ddev->mode_config.min_width = 1;
> + ddev->mode_config.min_height = 1;
> + ddev->mode_config.preferred_depth = 24;
> + ddev->mode_config.prefer_shadow = 0;
> +
> + ddev->mode_config.max_width = 4096;
> + ddev->mode_config.max_height = 4096;
> +
> + ddev->mode_config.cursor_width = descp->hw_cursor_h;
> + ddev->mode_config.cursor_height = descp->hw_cursor_h;
> +
> + if (ldev->vram_base)
> + ddev->mode_config.fb_base = ldev->vram_base;
> +
> + ret = lsdc_modeset_init(ldev, descp->num_of_crtc);
> + if (ret)
> + goto out_mode_config;
> +
> + drm_mode_config_reset(ddev);
> +
> + return 0;
> +
> +out_mode_config:
> + drm_mode_config_cleanup(ddev);
> +
> + return ret;
> +}
> +
> +
> +static void lsdc_mode_config_fini(struct drm_device *ddev)
> +{
> + struct lsdc_device *ldev = to_lsdc(ddev);
> +
> + /* disable output polling */
> + drm_kms_helper_poll_fini(ddev);
> +
> + drm_dev_unregister(ddev);
> +
> + devm_free_irq(ddev->dev, ldev->irq, ddev);
> +
> + /* shutdown all CRTC, for driver unloading */
> + drm_atomic_helper_shutdown(ddev);
> +
> + drm_mode_config_cleanup(ddev);
> +}
> +
> +/*
> + * We need a function to tell different chips apart.
> + * Because there are difference between the dc in ls7a1000 and the dc in
> + * ls2k1000, ls7a1000 have two gpio emulated i2c built-in, but ls2k1000
> + * don't have such hardware feature. ls2k1000 grab i2c adapter from other
> + * module, eithor hardware i2c or external gpio-emulated i2c.
> + *
> + * Beside, the hardware pixel pll unit is also different.
> + */
> +static int lsdc_determine_chip(struct lsdc_device *ldev)
> +{
> + struct device_node *np;
> + const char *model;
> + const char *compat;
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(lsdc_drm_of_match); ++i) {
> + compat = lsdc_drm_of_match[i].compatible;
> +
> + np = of_find_compatible_node(NULL, NULL, compat);
> + if (np) {
> +
> + /* get additional information */
> + of_property_read_string(np, "model", &model);
> +
> + of_node_put(np);
> +
> + ldev->desc = lsdc_drm_of_match[i].data;
> +
> + break;
> + }
> + }
> +
> + if (ldev->desc == NULL) {
> + drm_err(&ldev->drm, "unknown dc ip core, abort\n");
> + return -ENOENT;
> + }
> +
> + drm_info(&ldev->drm, "%s found, model: %s\n", compat, model);
> +
> + return 0;
> +}
> +
> +
> +static int lsdc_drm_suspend(struct device *dev)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> +
> + return drm_mode_config_helper_suspend(ddev);
> +}
> +
> +
> +static int lsdc_drm_resume(struct device *dev)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> +
> + return drm_mode_config_helper_resume(ddev);
> +}
> +
> +
> +static int lsdc_pm_freeze(struct device *dev)
> +{
> + return lsdc_drm_suspend(dev);
> +}
> +
> +
> +static int lsdc_pm_thaw(struct device *dev)
> +{
> + return lsdc_drm_resume(dev);
> +}
> +
> +
> +static int lsdc_pm_suspend(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + int error;
> +
> + error = lsdc_pm_freeze(dev);
> + if (error)
> + return error;
> +
> + pci_save_state(pdev);
> + /* Shut down the device */
> + pci_disable_device(pdev);
> + pci_set_power_state(pdev, PCI_D3hot);
> +
> + return 0;
> +}
> +
> +
> +static int lsdc_pm_resume(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> +
> + if (pcim_enable_device(pdev))
> + return -EIO;
> +
> + pci_set_power_state(pdev, PCI_D0);
> +
> + pci_restore_state(pdev);
> +
> + return lsdc_pm_thaw(dev);
> +}
> +
> +
> +static const struct dev_pm_ops lsdc_pm_ops = {
> + .suspend = lsdc_pm_suspend,
> + .resume = lsdc_pm_resume,
> + .freeze = lsdc_pm_freeze,
> + .thaw = lsdc_pm_thaw,
> + .poweroff = lsdc_pm_freeze,
> + .restore = lsdc_pm_resume,
> +};
> +
> +
> +static int lsdc_remove_conflicting_framebuffers(const struct drm_driver *drv)
> +{
> + /* lsdc is a pci device, but it don't have a dedicate vram bar because
> + * of historic reason(The display controller is ported from SoC product
> + * of Loongson, Loongson 2H series which date back to 2012)
> + * And simplefb node may have been located anywhere in memory.
> + */
> + return drm_aperture_remove_conflicting_framebuffers(0, ~0, false, drv);
> +}
> +
> +
> +static int lsdc_vram_init(struct lsdc_device *ldev)
> +{
> + struct drm_device *ddev = &ldev->drm;
> + struct pci_dev *gpu;
> + resource_size_t base, size;
> + int ret;
> +
> + /* BAR 2 of LS7A1000's GPU contain VRAM, It's GC1000 */
> + gpu = pci_get_device(PCI_VENDOR_ID_LOONGSON, 0x7a15, NULL);
> + base = pci_resource_start(gpu, 2);
> + size = pci_resource_len(gpu, 2);
> +
> + drm_info(ddev, "vram start: 0x%llx, size: %uMB\n",
> + base, (unsigned int)(size >> 20));
> +
> + if (!request_mem_region(base, size, "lsdc_vram")) {
> + drm_err(ddev, "can't reserve VRAM memory region\n");
> + return -ENXIO;
> + }
> +
> + if (ldev->use_vram_helper) {
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> + ret = drmm_vram_helper_init(ddev, base, size);
> + if (ret) {
> + drm_err(ddev, "can't init vram helper\n");
> + return ret;
> + }
> +#endif
> + } else if (ldev->dirty_update) {
> + ldev->vram = devm_ioremap_wc(ddev->dev, base, size);
> + if (ldev->vram == NULL)
> + return -ENOMEM;
> +
> + drm_info(ddev, "vram virtual addr: 0x%llx\n", (u64)ldev->vram);
> + }
> +
> + ldev->vram_base = base;
> + ldev->vram_size = size;
> +
> + return 0;
> +}
> +
> +/*
> + * For the dc in ls7a1000, it is more reliable scanout from the VRAM.
> + * scanout from the system memory suffer from some hardware deficiency
> + * which cause the screen flickering under RAM pressure.
> + */
> +static bool lsdc_should_vram_helper_based(void)
> +{
> + static const char * const dc_compat[] = { "pci0014,7a06.0",
> + "pci0014,7a06" };
> + bool ret = false;
> + struct device_node *np;
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(dc_compat); ++i) {
> + np = of_find_compatible_node(NULL, NULL, dc_compat[i]);
> + if (!np)
> + continue;
> +
> + ret = of_property_read_bool(np, "use_vram_helper");
> + of_node_put(np);
> + break;
> + }
> +
> + if (ret)
> + DRM_INFO("using vram base solution dictated by device tree\n");
> +
> + return ret;
> +}
> +
> +
> +static int lsdc_pci_probe(struct pci_dev *pdev,
> + const struct pci_device_id * const ent)
> +{
> + const struct drm_driver *driver = &lsdc_drm_driver_cma_stub;
> + struct lsdc_device *ldev;
> + struct drm_device *ddev;
> + int ret;
> +
> + lsdc_remove_conflicting_framebuffers(driver);
> +
> + ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> + if (ret) {
> + dev_err(&pdev->dev, "Set DMA Mask failed\n");
> + return ret;
> + }
> +
> + ret = pcim_enable_device(pdev);
> + if (ret) {
> + dev_err(&pdev->dev, "Enable pci devive failed\n");
> + return ret;
> + }
> +
> + pci_set_master(pdev);
> +
> + /* Get the optional framebuffer memory resource */
> + ret = of_reserved_mem_device_init(&pdev->dev);
> + if (ret && ret != -ENODEV)
> + return ret;
> +
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> + if (lsdc_use_vram_helper && lsdc_should_vram_helper_based()) {
> + driver = &lsdc_vram_driver_stub;
> + lsdc_use_vram_helper = 1;
> + DRM_INFO("using vram helper based solution\n");
> + }
> +#endif
> +
> + ldev = devm_drm_dev_alloc(&pdev->dev,
> + driver,
> + struct lsdc_device,
> + drm);
> + if (IS_ERR(ldev))
> + return PTR_ERR(ldev);
> +
> + ddev = &ldev->drm;
> +
> + pci_set_drvdata(pdev, ddev);
> +
> + if (lsdc_use_vram_helper > 0)
> + ldev->use_vram_helper = true;
> + else {
> + if (lsdc_cached_coherent > 0) {
> + ldev->cached_coherent = true;
> + drm_info(ddev, "cache coherency is maintained by hardware\n");
> + }
> +
> + if (lsdc_dirty_update > 0) {
> + ldev->dirty_update = true;
> + drm_info(ddev, "dirty update enabled\n");
> + }
> + }
> +
> + if (lsdc_verbose > 0)
> + ldev->verbose = true;
> +
> + ret = lsdc_determine_chip(ldev);
> + if (ret)
> + return ret;
> +
> + /* BAR 0 contains registers */
> + ldev->reg_base = devm_ioremap_resource(&pdev->dev, &pdev->resource[0]);
> + if (IS_ERR(ldev->reg_base))
> + return PTR_ERR(ldev->reg_base);
> +
> + /* LS2K1000/LS2K0500 is SoC, don't have a VRAM */
> + if ((ldev->desc->chip == LSDC_CHIP_7A1000) &&
> + (ldev->use_vram_helper || ldev->dirty_update))
> + lsdc_vram_init(ldev);
> +
> + ret = lsdc_mode_config_init(ldev);
> + if (ret)
> + goto err_out;
> +
> +
> + ldev->irq = pdev->irq;
> +
> + dev_info(&pdev->dev, "irq = %d\n", ldev->irq);
> +
> + ret = devm_request_threaded_irq(&pdev->dev,
> + pdev->irq,
> + lsdc_irq_handler_cb,
> + lsdc_irq_thread_cb,
> + IRQF_ONESHOT,
> + dev_name(&pdev->dev),
> + ddev);
> +
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register lsdc interrupt\n");
> + goto err_out;
> + }
> +
> + ret = drm_vblank_init(ddev, LSDC_MAX_CRTC);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "Fatal error during vblank init: %d\n", ret);
> + goto err_out;
> + }
> +
> + /* Initialize and enable output polling */
> + drm_kms_helper_poll_init(ddev);
> +
> + ret = drm_dev_register(ddev, 0);
> + if (ret)
> + goto err_out;
> +
> + drm_fbdev_generic_setup(ddev, 32);
> +
> + return 0;
> +
> +err_out:
> + drm_dev_put(ddev);
> +
> + return ret;
> +}
> +
> +
> +static void lsdc_pci_remove(struct pci_dev *pdev)
> +{
> + struct drm_device *ddev = pci_get_drvdata(pdev);
> +
> + lsdc_mode_config_fini(ddev);
> +
> + drm_dev_put(ddev);
> +
> + pci_clear_master(pdev);
> +
> + pci_release_regions(pdev);
> +}
> +
> +
> +static const struct pci_device_id lsdc_pciid_list[] = {
> + {PCI_VENDOR_ID_LOONGSON, 0x7a06, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> + {0, 0, 0, 0, 0, 0, 0}
> +};
> +
> +
> +static struct pci_driver lsdc_pci_driver = {
> + .name = DRIVER_NAME,
> + .id_table = lsdc_pciid_list,
> + .probe = lsdc_pci_probe,
> + .remove = lsdc_pci_remove,
> + .driver.pm = &lsdc_pm_ops,
> +};
> +
> +
> +static int __init lsdc_drm_init(void)
> +{
> + struct pci_dev *pdev = NULL;
> +
> + if (drm_firmware_drivers_only())
> + return -EINVAL;
> +
> + if (lsdc_modeset == 0)
> + return -ENOENT;
> +
> + while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev))) {
> + /*
> + * Multiple video card workaround
> + *
> + * This integrated video driver will always be selected as
> + * default boot device by vgaarb system.
> + */
> + if (pdev->vendor != PCI_VENDOR_ID_LOONGSON) {
> + DRM_INFO("Discrete graphic card detected, abort\n");
> + return 0;
> + }
> + }
> +
> + return pci_register_driver(&lsdc_pci_driver);
> +}
> +module_init(lsdc_drm_init);
> +
> +static void __exit lsdc_drm_exit(void)
> +{
> + pci_unregister_driver(&lsdc_pci_driver);
> +}
> +module_exit(lsdc_drm_exit);
> +
> +
> +MODULE_DEVICE_TABLE(pci, lsdc_pciid_list);
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/lsdc/lsdc_drv.h b/drivers/gpu/drm/lsdc/lsdc_drv.h
> new file mode 100644
> index 000000000000..89cf15248c3a
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_drv.h
> @@ -0,0 +1,216 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +#ifndef __LSDC_DRV_H__
> +#define __LSDC_DRV_H__
> +
> +#include <drm/drm_print.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_plane.h>
> +#include <drm/drm_encoder.h>
> +
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> +#include <drm/drm_gem_vram_helper.h>
> +#endif
> +
> +#include "lsdc_regs.h"
> +#include "lsdc_pll.h"
> +
> +#define LSDC_MAX_CRTC 2
> +
> +/* There is only a 1:1 mapping of encoders and connectors for lsdc */
> +/*
> + * +-------------------+ _________
> + * | | | |
> + * | CRTC0 --> DVO0 ---------> Encoder0 --> Connector0 ---> | Monotor |
> + * | | ^ ^ |_________|
> + * | | | |
> + * | <----- i2c0 ----------------+
> + * | LSDC IP CORE |
> + * | <----- i2c1 ----------------+
> + * | | | | _________
> + * | | | | | |
> + * | CRTC1 --> DVO1 ---------> Encoder1 --> Connector1 ---> | Panel |
> + * | | |_________|
> + * +-------------------+
> + */
> +
> +enum loongson_dc_family {
> + LSDC_CHIP_UNKNOWN = 0,
> + LSDC_CHIP_2K1000 = 1, /* 2-Core SoC, 64-bit */
> + LSDC_CHIP_7A1000 = 2, /* North bridge of LS3A3000/LS3A4000/LS3A5000 */
> + LSDC_CHIP_2K0500 = 3, /* Reduced version of LS2K1000, single core */
> + LSDC_CHIP_7A2000 = 4, /* Enhancement version of 7A1000 */
> + LSDC_CHIP_LAST,
> +};
> +
> +enum lsdc_pixel_format {
> + LSDC_PF_NONE = 0,
> + LSDC_PF_ARGB4444 = 1, /* ARGB A:4 bits R/G/B: 4 bits each [16 bits] */
> + LSDC_PF_ARGB1555 = 2, /* ARGB A:1 bit RGB:15 bits [16 bits] */
> + LSDC_PF_RGB565 = 3, /* RGB [16 bits] */
> + LSDC_PF_XRGB8888 = 4, /* XRGB [32 bits] */
> +};
> +
> +struct lsdc_chip_desc {
> + enum loongson_dc_family chip;
> + uint32_t num_of_crtc;
> +
> + uint32_t max_pixel_clk;
> +
> + uint32_t max_width;
> + uint32_t max_height;
> +
> + uint32_t num_of_hw_cursor;
> + uint32_t hw_cursor_w;
> + uint32_t hw_cursor_h;
> + bool have_builtin_i2c;
> + uint32_t stride_alignment;
> +};
> +
> +
> +/**
> + * struct lsdc_display_pipe - simple display pipeline
> + * @crtc: CRTC control structure
> + * @plane: Plane control structure
> + * @encoder: Encoder control structure
> + * @pixpll: Pll control structure
> + * @connector: point to connector control structure
> + *
> + * display pipeline with plane, crtc, encoder, pll collapsed into one
> + * entity. It should be initialized by calling drm_simple_display_pipe_init().
> + */
> +struct lsdc_display_pipe {
> + struct drm_crtc crtc;
> + struct drm_plane primary;
> + struct drm_plane cursor;
> + struct drm_encoder encoder;
> + struct lsdc_pll pixpll;
> + struct lsdc_connector *lconn;
> +
> + int index;
> + bool available;
> +};
> +
> +
> +struct lsdc_crtc_state {
> + struct drm_crtc_state base;
> + struct lsdc_pll_core_values pparams;
> + unsigned int pix_fmt;
> +};
> +
> +
> +struct lsdc_device {
> + struct drm_device drm;
> +
> + void __iomem *reg_base;
> + void __iomem *vram;
> + resource_size_t vram_base;
> + resource_size_t vram_size;
> +
> + struct lsdc_display_pipe disp_pipe[LSDC_MAX_CRTC];
> +
> + unsigned int num_output;
> +
> + /* platform specific data */
> + const struct lsdc_chip_desc *desc;
> +
> + /* @reglock: protects concurrent register access */
> + spinlock_t reglock;
> +
> + /*
> + * @dirty_update: true if manual dirty update is wantted
> + */
> + bool dirty_update;
> + /*
> + * @cached_coherent: true if the host platform is hardware maintained
> + * cached coherent.
> + */
> + bool cached_coherent;
> + /*
> + * @use_vram_helper: using vram helper instead of cma helper base
> + * solution. As ls7a1000 has a dediacted video ram, the dc scanout
> + * from the vram is more reliable.
> + */
> + bool use_vram_helper;
> +
> + /*
> + * @verbose: set it to true if print useful information is wantted
> + */
> + bool verbose;
> +
> + int irq;
> + u32 irq_status;
> +};
> +
> +#define to_lsdc(x) container_of(x, struct lsdc_device, drm)
> +
> +static inline struct lsdc_crtc_state *
> +to_lsdc_crtc_state(struct drm_crtc_state *base)
> +{
> + return container_of(base, struct lsdc_crtc_state, base);
> +}
> +
> +static inline u32 lsdc_reg_read32(struct lsdc_device * const ldev, u32 offset)
> +{
> + u32 val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ldev->reglock, flags);
> + val = readl(ldev->reg_base + offset);
> + spin_unlock_irqrestore(&ldev->reglock, flags);
> +
> + return val;
> +}
> +
> +static inline void lsdc_reg_write32(struct lsdc_device * const ldev, u32 offset, u32 val)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ldev->reglock, flags);
> + writel(val, ldev->reg_base + offset);
> + spin_unlock_irqrestore(&ldev->reglock, flags);
> +}
> +
> +int lsdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc,
> + unsigned int index, struct drm_plane *primary,
> + struct drm_plane *cursor);
> +
> +int lsdc_plane_init(struct lsdc_device *ldev, struct drm_plane *plane,
> + enum drm_plane_type type, unsigned int index);
> +
> +int lsdc_encoder_init(struct drm_encoder * const encoder,
> + struct drm_connector *connector,
> + struct drm_device *ddev,
> + unsigned int index,
> + unsigned int total);
> +
> +#endif
> diff --git a/drivers/gpu/drm/lsdc/lsdc_encoder.c b/drivers/gpu/drm/lsdc/lsdc_encoder.c
> new file mode 100644
> index 000000000000..0cdd95f5bc37
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_encoder.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +#include <drm/drm_print.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +#include "lsdc_drv.h"
> +
> +static const struct drm_encoder_funcs lsdc_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> +
> +int lsdc_encoder_init(struct drm_encoder * const encoder,
> + struct drm_connector *connector,
> + struct drm_device *ddev,
> + unsigned int index,
> + unsigned int total)
> +{
> + int ret;
> + int type;
> +
> + encoder->possible_crtcs = BIT(index);
> +
> + if (total == 2)
> + encoder->possible_clones = BIT(1) | BIT(0);
> + else if (total < 2)
> + encoder->possible_clones = 0;
> +
> + if (connector->connector_type == DRM_MODE_CONNECTOR_VGA)
> + type = DRM_MODE_ENCODER_DAC;
> + else if ((connector->connector_type == DRM_MODE_CONNECTOR_HDMIA) ||
> + (connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) ||
> + (connector->connector_type == DRM_MODE_CONNECTOR_DVID))
> + type = DRM_MODE_ENCODER_TMDS;
> + else if (connector->connector_type == DRM_MODE_CONNECTOR_DPI)
> + type = DRM_MODE_ENCODER_DPI;
> + else if (connector->connector_type == DRM_MODE_CONNECTOR_VIRTUAL)
> + type = DRM_MODE_ENCODER_VIRTUAL;
> + else
> + type = DRM_MODE_ENCODER_NONE;
> +
> + ret = drm_encoder_init(ddev,
> + encoder,
> + &lsdc_encoder_funcs,
> + type,
> + "encoder%d",
> + index);
> + if (ret)
> + return ret;
> +
> + return drm_connector_attach_encoder(connector, encoder);
> +}
> diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.c b/drivers/gpu/drm/lsdc/lsdc_i2c.c
> new file mode 100644
> index 000000000000..895e94ae6827
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.c
> @@ -0,0 +1,220 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +#include <linux/string.h>
> +#include <linux/i2c.h>
> +
> +#include "lsdc_drv.h"
> +#include "lsdc_regs.h"
> +#include "lsdc_i2c.h"
> +
> +/*
> + * set the state of a gpio pin indicated by mask
> + * @mask: gpio pin mask
> + */
> +static void ls7a_gpio_i2c_set(struct lsdc_i2c * const i2c, int mask, int state)
> +{
> + struct lsdc_device *ldev = to_lsdc(i2c->ddev);
> + u8 val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ldev->reglock, flags);
> +
> + if (state) {
> + val = readb(i2c->dir_reg);
> + val |= mask;
> + writeb(val, i2c->dir_reg);
> + } else {
> + val = readb(i2c->dir_reg);
> + val &= ~mask;
> + writeb(val, i2c->dir_reg);
> +
> + val = readb(i2c->dat_reg);
> + if (state)
> + val |= mask;
> + else
> + val &= ~mask;
> + writeb(val, i2c->dat_reg);
> + }
> +
> + spin_unlock_irqrestore(&ldev->reglock, flags);
> +}
> +
> +/*
> + * read value back from gpio pin
> + * @mask: gpio pin mask
> + */
> +static int ls7a_gpio_i2c_get(struct lsdc_i2c * const i2c, int mask)
> +{
> + struct lsdc_device *ldev = to_lsdc(i2c->ddev);
> + u8 val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ldev->reglock, flags);
> +
> + /* first set this pin as input */
> + val = readb(i2c->dir_reg);
> + val |= mask;
> + writeb(val, i2c->dir_reg);
> +
> + /* then get level state from this pin */
> + val = readb(i2c->dat_reg);
> +
> + spin_unlock_irqrestore(&ldev->reglock, flags);
> +
> + return (val & mask) ? 1 : 0;
> +}
> +
> +
> +/* set the state on the i2c->sda pin */
> +static void ls7a_i2c_set_sda(void *i2c, int state)
> +{
> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c;
> +
> + return ls7a_gpio_i2c_set(li2c, li2c->sda, state);
> +}
> +
> +/* set the state on the i2c->scl pin */
> +static void ls7a_i2c_set_scl(void *i2c, int state)
> +{
> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c;
> +
> + return ls7a_gpio_i2c_set(li2c, li2c->scl, state);
> +}
> +
> +/* read the value from the i2c->sda pin */
> +static int ls7a_i2c_get_sda(void *i2c)
> +{
> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c;
> +
> + return ls7a_gpio_i2c_get(li2c, li2c->sda);
> +}
> +
> +/* read the value from the i2c->scl pin */
> +static int ls7a_i2c_get_scl(void *i2c)
> +{
> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c;
> +
> + return ls7a_gpio_i2c_get(li2c, li2c->scl);
> +}
> +
> +/*
> + * Get i2c id from connector id
> + *
> + * TODO: get it from dtb
> + */
> +static int lsdc_get_i2c_id(struct drm_device *ddev, unsigned int index)
> +{
> + return index;
> +}
> +
> +/*
> + * mainly for dc in ls7a1000 which have builtin gpio emulated i2c
> + *
> + * @index : output channel index, 0 for DVO0, 1 for DVO1
> + */
> +struct i2c_adapter *lsdc_create_i2c_chan(struct drm_device *ddev,
> + unsigned int index)
> +{
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + struct i2c_adapter *adapter;
> + struct lsdc_i2c *li2c;
> + int ret;
> +
> + li2c = devm_kzalloc(ddev->dev, sizeof(*li2c), GFP_KERNEL);
> + if (li2c == NULL)
> + return ERR_PTR(-ENOMEM);
> +
> + li2c->ddev = ddev;
> +
> + if (index == 0) {
> + li2c->sda = 0x01;
> + li2c->scl = 0x02;
> + } else if (index == 1) {
> + li2c->sda = 0x04;
> + li2c->scl = 0x08;
> + }
> +
> + li2c->dir_reg = ldev->reg_base + LS7A_DC_GPIO_DIR_REG;
> + li2c->dat_reg = ldev->reg_base + LS7A_DC_GPIO_DAT_REG;
> +
> + li2c->bit.setsda = ls7a_i2c_set_sda;
> + li2c->bit.setscl = ls7a_i2c_set_scl;
> + li2c->bit.getsda = ls7a_i2c_get_sda;
> + li2c->bit.getscl = ls7a_i2c_get_scl;
> + li2c->bit.udelay = 5;
> + li2c->bit.timeout = usecs_to_jiffies(2200); /* from VESA */
> + li2c->bit.data = li2c;
> +
> + adapter = &li2c->adapter;
> +
> + adapter->algo_data = &li2c->bit;
> + adapter->owner = THIS_MODULE;
> + adapter->class = I2C_CLASS_DDC;
> + adapter->dev.parent = ddev->dev;
> + adapter->nr = -1;
> +
> + snprintf(adapter->name, sizeof(adapter->name),
> + "%s-%d", "lsdc_gpio_i2c", index);
> +
> + i2c_set_adapdata(adapter, li2c);
> +
> + ret = i2c_bit_add_numbered_bus(adapter);
> + if (ret) {
> + devm_kfree(ddev->dev, li2c);
> + return ERR_PTR(ret);
> + }
> +
> + return adapter;
> +}
> +
> +
> +/*
> + * Mainly for dc in ls2k1000, ls2k0500 SoC which should get a i2c adapter
> + * from i2c susystem.
> + *
> + * @index : output channel index, 0 for DVO0, 1 for DVO1
> + */
> +struct i2c_adapter *lsdc_get_i2c_adapter(struct drm_device *ddev,
> + unsigned int index)
> +{
> + unsigned int i2c_id;
> +
> + /* find mapping between i2c id and connector id */
> + i2c_id = lsdc_get_i2c_id(ddev, index);
> +
> + return i2c_get_adapter(i2c_id);
> +}
> +
> +
> +void lsdc_destroy_i2c(struct drm_device *ddev, struct i2c_adapter *adapter)
> +{
> + i2c_put_adapter(adapter);
> +}
> diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.h b/drivers/gpu/drm/lsdc/lsdc_i2c.h
> new file mode 100644
> index 000000000000..f4560db3694a
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +
> +#ifndef __LSDC_I2C__
> +#define __LSDC_I2C__
> +
> +#include <linux/i2c.h>
> +#include <linux/i2c-algo-bit.h>
> +
> +
> +struct lsdc_i2c {
> + struct drm_device *ddev;
> + struct i2c_adapter adapter;
> + struct i2c_algo_bit_data bit;
> + /* pin bit mask */
> + u8 sda;
> + u8 scl;
> +
> + void __iomem *dir_reg;
> + void __iomem *dat_reg;
> +};
> +
> +
> +void lsdc_destroy_i2c(struct drm_device *ddev, struct i2c_adapter *i2c);
> +
> +struct i2c_adapter *lsdc_create_i2c_chan(struct drm_device *ddev,
> + unsigned int con_id);
> +
> +struct i2c_adapter *lsdc_get_i2c_adapter(struct drm_device *ddev,
> + unsigned int con_id);
> +
> +
> +#endif
> diff --git a/drivers/gpu/drm/lsdc/lsdc_irq.c b/drivers/gpu/drm/lsdc/lsdc_irq.c
> new file mode 100644
> index 000000000000..7620de54aae4
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_irq.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +#include <drm/drm_vblank.h>
> +
> +#include "lsdc_drv.h"
> +#include "lsdc_regs.h"
> +#include "lsdc_irq.h"
> +
> +/* Function to be called in a threaded interrupt context. */
> +irqreturn_t lsdc_irq_thread_cb(int irq, void *arg)
> +{
> + struct drm_device *ddev = arg;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + struct drm_crtc *crtc;
> +
> + /* trigger the vblank event */
> + if (ldev->irq_status & INT_CRTC0_VS) {
> + crtc = drm_crtc_from_index(ddev, 0);
> + drm_crtc_handle_vblank(crtc);
> + }
> +
> + if (ldev->irq_status & INT_CRTC1_VS) {
> + crtc = drm_crtc_from_index(ddev, 1);
> + drm_crtc_handle_vblank(crtc);
> + }
> +
> + lsdc_reg_write32(ldev, LSDC_INT_REG, INT_CRTC0_VS_EN | INT_CRTC1_VS_EN);
> +
> + return IRQ_HANDLED;
> +}
> +
> +
> +/* Function to be called when the IRQ occurs */
> +irqreturn_t lsdc_irq_handler_cb(int irq, void *arg)
> +{
> + struct drm_device *ddev = arg;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> +
> + /* Read & Clear the interrupt status */
> + ldev->irq_status = lsdc_reg_read32(ldev, LSDC_INT_REG);
> + if ((ldev->irq_status & INT_STATUS_MASK) == 0) {
> + drm_warn(ddev, "no interrupt occurs\n");
> + return IRQ_NONE;
> + }
> +
> + /* clear all interrupt */
> + lsdc_reg_write32(ldev, LSDC_INT_REG, ldev->irq_status);
> +
> + return IRQ_WAKE_THREAD;
> +}
> diff --git a/drivers/gpu/drm/lsdc/lsdc_irq.h b/drivers/gpu/drm/lsdc/lsdc_irq.h
> new file mode 100644
> index 000000000000..ac187538d746
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_irq.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +#ifndef __LSDC_IRQ_H__
> +#define __LSDC_IRQ_H__
> +
> +irqreturn_t lsdc_irq_thread_cb(int irq, void *arg);
> +irqreturn_t lsdc_irq_handler_cb(int irq, void *arg);
> +
> +#endif
> diff --git a/drivers/gpu/drm/lsdc/lsdc_plane.c b/drivers/gpu/drm/lsdc/lsdc_plane.c
> new file mode 100644
> index 000000000000..62801e100a13
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_plane.c
> @@ -0,0 +1,681 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +#include <drm/drm_print.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_plane.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_vblank.h>
> +#include <drm/drm_drv.h>
> +
> +#include <drm/drm_format_helper.h>
> +#include <drm/drm_plane_helper.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
> +
> +
> +#include "lsdc_drv.h"
> +#include "lsdc_regs.h"
> +#include "lsdc_pll.h"
> +
> +
> +static const uint32_t lsdc_primary_formats[] = {
> + DRM_FORMAT_RGB565,
> + DRM_FORMAT_XRGB8888,
> + DRM_FORMAT_ARGB8888,
> +};
> +
> +static const uint32_t lsdc_cursor_formats[] = {
> + DRM_FORMAT_ARGB8888,
> +};
> +
> +static const u64 lsdc_fb_format_modifiers[] = {
> + DRM_FORMAT_MOD_LINEAR,
> + DRM_FORMAT_MOD_INVALID
> +};
> +
> +
> +static u32 lsdc_pixfmt_to_drm_pixfmt(enum lsdc_pixel_format pf)
> +{
> + switch (pf) {
> + case LSDC_PF_XRGB8888:
> + return DRM_FORMAT_XRGB8888;
> + case LSDC_PF_RGB565:
> + return DRM_FORMAT_RGB565;
> + case LSDC_PF_ARGB1555:
> + return DRM_FORMAT_ARGB1555;
> + case LSDC_PF_ARGB4444:
> + return DRM_FORMAT_ARGB4444;
> + case LSDC_PF_NONE:
> + default:
> + return 0;
> + }
> +}
> +
> +static const char *lsdc_pixfmt_to_string(u32 reg)
> +{
> + switch (reg & CFG_PIX_FMT_MASK) {
> + case LSDC_PF_XRGB8888:
> + return "XRGB8888";
> + case LSDC_PF_RGB565:
> + return "RGB565";
> + case LSDC_PF_ARGB1555:
> + return "ARGB1555";
> + case LSDC_PF_ARGB4444:
> + return "ARGB4444";
> + case LSDC_PF_NONE:
> + return "NONE";
> + default:
> + return "unknown";
> + }
> +}
> +
> +
> +
> +static void lsdc_update_fb_format(struct lsdc_device *ldev,
> + struct drm_crtc *crtc,
> + const struct drm_format_info *fmt_info)
> +{
> + unsigned int index = drm_crtc_index(crtc);
> + u32 val = 0;
> + u32 fmt;
> +
> + switch (fmt_info->format) {
> + case DRM_FORMAT_RGB565:
> + fmt = LSDC_PF_RGB565;
> + break;
> + case DRM_FORMAT_XRGB8888:
> + fmt = LSDC_PF_XRGB8888;
> + break;
> + case DRM_FORMAT_ARGB8888:
> + fmt = LSDC_PF_XRGB8888;
> + break;
> + default:
> + fmt = LSDC_PF_XRGB8888;
> + break;
> + }
> +
> + if (ldev->verbose)
> + drm_info(&ldev->drm, "fmt wanted is %s\n",
> + lsdc_pixfmt_to_string(fmt));
> +
> + if (index == 0) {
> + val = lsdc_reg_read32(ldev, LSDC_CRTC0_CFG_REG);
> + val = (val & ~CFG_PIX_FMT_MASK) | fmt;
> + lsdc_reg_write32(ldev, LSDC_CRTC0_CFG_REG, val);
> + val = lsdc_reg_read32(ldev, LSDC_CRTC0_CFG_REG);
> + } else if (index == 1) {
> + val = lsdc_reg_read32(ldev, LSDC_CRTC1_CFG_REG);
> + val = (val & ~CFG_PIX_FMT_MASK) | fmt;
> + lsdc_reg_write32(ldev, LSDC_CRTC1_CFG_REG, val);
> + val = lsdc_reg_read32(ldev, LSDC_CRTC1_CFG_REG);
> + }
> +
> + if (ldev->verbose)
> + drm_info(&ldev->drm, "after update fb%d format is %s\n",
> + index, lsdc_pixfmt_to_string(val));
> +}
> +
> +
> +static u32 lsdc_primary_get_default_format(struct drm_crtc *crtc)
> +{
> + struct lsdc_device *ldev = to_lsdc(crtc->dev);
> + unsigned int index = drm_crtc_index(crtc);
> + u32 val;
> +
> + if (index == 0)
> + val = lsdc_reg_read32(ldev, LSDC_CRTC0_CFG_REG);
> + else if (index == 1)
> + val = lsdc_reg_read32(ldev, LSDC_CRTC1_CFG_REG);
> +
> + return val & CFG_PIX_FMT_MASK;
> +}
> +
> +
> +static void lsdc_update_fb_start_addr(struct lsdc_device *ldev,
> + struct drm_crtc *crtc,
> + u64 paddr)
> +{
> + unsigned int index = drm_crtc_index(crtc);
> + u32 addr_reg;
> + u32 cfg_reg;
> + u32 val;
> +
> + /*
> + * Find which framebuffer address register should update.
> + * if FB_ADDR0_REG is in using, we write the addr to FB_ADDR1_REG,
> + * if FB_ADDR1_REG is in using, we write the addr to FB_ADDR0_REG
> + */
> + if (index == 0) {
> + /* CRTC0 */
> + val = lsdc_reg_read32(ldev, LSDC_CRTC0_CFG_REG);
> +
> + cfg_reg = LSDC_CRTC0_CFG_REG;
> +
> + if (val & CFG_FB_IDX_BIT) {
> + addr_reg = LSDC_CRTC0_FB_ADDR0_REG;
> + drm_dbg_kms(&ldev->drm, "CRTC0 FB0 will be use\n");
> + } else {
> + addr_reg = LSDC_CRTC0_FB_ADDR1_REG;
> + drm_dbg_kms(&ldev->drm, "CRTC0 FBq will be use\n");
> + }
> + } else if (index == 1) {
> + /* CRTC1 */
> + val = lsdc_reg_read32(ldev, LSDC_CRTC1_CFG_REG);
> +
> + cfg_reg = LSDC_CRTC1_CFG_REG;
> +
> + if (val & CFG_FB_IDX_BIT) {
> + addr_reg = LSDC_CRTC1_FB_ADDR0_REG;
> + drm_dbg_kms(&ldev->drm, "CRTC1 FB0 will be use\n");
> + } else {
> + addr_reg = LSDC_CRTC1_FB_ADDR1_REG;
> + drm_dbg_kms(&ldev->drm, "CRTC1 FBq will be use\n");
> + }
> + }
> +
> + lsdc_reg_write32(ldev, addr_reg, paddr);
> +
> + /*
> + * Then, we triger the fb switch, the switch of the framebuffer
> + * to be scanout will complete at the next vblank.
> + */
> + lsdc_reg_write32(ldev, cfg_reg, val | CFG_PAGE_FLIP_BIT);
> +
> + drm_dbg_kms(&ldev->drm, "crtc%u scantout from 0x%llx\n", index, paddr);
> +}
> +
> +
> +static void lsdc_handle_damage(struct lsdc_device *ldev,
> + struct drm_framebuffer *fb,
> + struct drm_rect *clip,
> + void *src)
> +{
> + unsigned int offset;
> + void __iomem *dst;
> +
> + offset = drm_fb_clip_offset(fb->pitches[0], fb->format, clip);
> + dst = ldev->vram + offset;
> + drm_fb_memcpy_toio(dst, fb->pitches[0], src, fb, clip);
> +}
> +
> +
> +static unsigned int lsdc_get_fb_offset(struct drm_framebuffer *fb,
> + struct drm_plane_state *state,
> + unsigned int plane)
> +{
> + unsigned int offset;
> +
> + offset = fb->offsets[plane];
> + offset += fb->format->cpp[plane] * (state->src_x >> 16);
> + offset += fb->pitches[plane] * (state->src_y >> 16);
> +
> + return offset;
> +}
> +
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> +static s64 lsdc_get_vram_bo_offset(struct drm_framebuffer *fb)
> +{
> + struct drm_gem_vram_object *gbo;
> + s64 gpu_addr;
> +
> + gbo = drm_gem_vram_of_gem(fb->obj[0]);
> + gpu_addr = drm_gem_vram_offset(gbo);
> +
> + return gpu_addr;
> +}
> +#endif
> +
> +
> +static int lsdc_primary_plane_atomic_check(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_device *ddev = plane->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> + struct drm_framebuffer *new_fb = new_plane_state->fb;
> + struct drm_framebuffer *old_fb = old_plane_state->fb;
> + struct drm_crtc *crtc = new_plane_state->crtc;
> + u32 new_format = new_fb->format->format;
> + struct drm_crtc_state *new_crtc_state;
> + struct lsdc_crtc_state *priv_crtc_state;
> + int ret;
> +
> + if (!crtc)
> + return 0;
> +
> + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> + if (WARN_ON(!new_crtc_state))
> + return -EINVAL;
> +
> + priv_crtc_state = to_lsdc_crtc_state(new_crtc_state);
> +
> + ret = drm_atomic_helper_check_plane_state(new_plane_state,
> + new_crtc_state,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false,
> + true);
> + if (ret)
> + return ret;
> +
> + /*
> + * Require full modeset if enabling or disabling a plane,
> + * or changing its position, size, depth or format.
> + */
> + if ((!new_fb || !old_fb ||
> + old_plane_state->crtc_x != new_plane_state->crtc_x ||
> + old_plane_state->crtc_y != new_plane_state->crtc_y ||
> + old_plane_state->crtc_w != new_plane_state->crtc_w ||
> + old_plane_state->crtc_h != new_plane_state->crtc_h ||
> + old_fb->format->format != new_format))
> + new_crtc_state->mode_changed = true;
> +
> +
> + priv_crtc_state->pix_fmt = lsdc_primary_get_default_format(crtc);

Storing the pixel format in the CRTC state is weird? What would happen
if you have a primary plane and a cursor in different formats?

Also, reading the default format from a register doesn't look right.
atomic_check can occur at any time, including before a previous commit,
or while the hardware is disabled. You should rely on either a constant
or the previous state here.

> + if (lsdc_pixfmt_to_drm_pixfmt(priv_crtc_state->pix_fmt) != new_format) {
> + drm_info(&ldev->drm, "mode changed\n");
> + new_crtc_state->mode_changed = true;
> + }
> +
> + if (new_crtc_state->mode_changed) {
> + struct lsdc_display_pipe *dispipe = container_of(plane, struct lsdc_display_pipe, primary);
> + struct lsdc_pll *pixpll = &dispipe->pixpll;
> + const struct lsdc_pixpll_funcs *pfuncs = pixpll->funcs;
> +
> + ret = pfuncs->compute(pixpll,
> + new_crtc_state->mode.clock,
> + true,
> + &priv_crtc_state->pparams);
> + if (ret == false) {
> + drm_warn(plane->dev,
> + "failed find a set of pll param for mode %u\n",
> + new_crtc_state->mode.clock);
> + return -EINVAL;
> + }
> + }
> +
> + if (ldev->dirty_update)
> + drm_atomic_helper_check_plane_damage(state, new_plane_state);
> +
> + return 0;
> +}
> +
> +static void lsdc_update_stride(struct lsdc_device *ldev,
> + struct drm_crtc *crtc,
> + unsigned int stride)
> +{
> + unsigned int index = drm_crtc_index(crtc);
> +
> + if (index == 0)
> + lsdc_reg_write32(ldev, LSDC_CRTC0_STRIDE_REG, stride);
> + else if (index == 1)
> + lsdc_reg_write32(ldev, LSDC_CRTC1_STRIDE_REG, stride);
> +
> + drm_dbg_kms(&ldev->drm, "update stride to %u\n", stride);
> +}
> +
> +
> +
> +static void lsdc_primary_plane_atomic_update(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct lsdc_device *ldev = to_lsdc(plane->dev);
> + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> + struct drm_crtc *crtc = new_plane_state->crtc;
> + struct drm_framebuffer *fb = new_plane_state->fb;
> + u32 fb_offset = lsdc_get_fb_offset(fb, new_plane_state, 0);
> + struct drm_gem_cma_object *obj;
> + dma_addr_t fb_addr;
> +
> + if (ldev->use_vram_helper) {
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> + s64 gpu_addr;
> +
> + gpu_addr = lsdc_get_vram_bo_offset(fb);
> + if (gpu_addr < 0)
> + return;
> +
> + fb_addr = ldev->vram_base + gpu_addr + fb_offset;
> +#endif
> + } else {
> + obj = drm_fb_cma_get_gem_obj(fb, 0);
> +
> + if (ldev->dirty_update)
> + fb_addr = ldev->vram_base + fb_offset;
> + else
> + fb_addr = obj->paddr + fb_offset;
> + }
> +
> + lsdc_update_fb_start_addr(ldev, crtc, fb_addr);
> +
> + lsdc_update_stride(ldev, crtc, fb->pitches[0]);
> +
> + if (drm_atomic_crtc_needs_modeset(crtc->state))
> + lsdc_update_fb_format(ldev, crtc, fb->format);
> +
> + if (ldev->dirty_update) {
> + struct drm_plane_state *old_plane_state;
> + struct drm_rect damage;
> + bool valid;
> +
> + old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> +
> + valid = drm_atomic_helper_damage_merged(old_plane_state,
> + new_plane_state,
> + &damage);
> + if (valid)
> + lsdc_handle_damage(ldev, fb, &damage, obj->vaddr);
> + }
> +}
> +
> +
> +static void lsdc_primary_plane_atomic_disable(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + drm_dbg_kms(plane->dev, "%s disabled\n", plane->name);
> +}
> +
> +
> +static int lsdc_plane_prepare_fb(struct drm_plane *plane,
> + struct drm_plane_state *new_state)
> +{
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> + struct lsdc_device *ldev = to_lsdc(plane->dev);
> +
> + if (ldev->use_vram_helper)
> + return drm_gem_vram_plane_helper_prepare_fb(plane, new_state);
> +#endif
> + return drm_gem_plane_helper_prepare_fb(plane, new_state);
> +}
> +
> +
> +static void lsdc_plane_cleanup_fb(struct drm_plane *plane,
> + struct drm_plane_state *old_state)
> +{
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> + struct drm_device *ddev = plane->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> +
> + if (ldev->use_vram_helper)
> + return drm_gem_vram_plane_helper_cleanup_fb(plane, old_state);
> +#endif
> +}
> +
> +static const struct drm_plane_helper_funcs lsdc_primary_plane_helpers = {
> + .prepare_fb = lsdc_plane_prepare_fb,
> + .cleanup_fb = lsdc_plane_cleanup_fb,
> + .atomic_check = lsdc_primary_plane_atomic_check,
> + .atomic_update = lsdc_primary_plane_atomic_update,
> + .atomic_disable = lsdc_primary_plane_atomic_disable,
> +};
> +
> +
> +

Don't use multiple blank lines. Generally speaking, there's a lot of
checkpatch warnings to fix. Make sure to run checkpatch.pl --strict and
fix whatever comes up.

> +static int lsdc_cursor_atomic_check(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> + struct drm_crtc *crtc = new_plane_state->crtc;
> + struct drm_framebuffer *fb = new_plane_state->fb;
> + struct drm_crtc_state *crtc_state;
> + int ret;
> +
> + /* no need for further checks if the plane is being disabled */
> + if (!crtc || !fb)
> + return 0;
> +
> + if (!new_plane_state->visible)
> + return 0;
> +
> + crtc_state = drm_atomic_get_new_crtc_state(state,
> + new_plane_state->crtc);
> +
> + ret = drm_atomic_helper_check_plane_state(new_plane_state,
> + crtc_state,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + true,
> + true);
> + if (ret)
> + return ret;
> +
> + if ((fb->width < LSDC_CURS_MIN_SIZE) ||
> + (fb->height < LSDC_CURS_MIN_SIZE) ||
> + (fb->width > LSDC_CURS_MAX_SIZE) ||
> + (fb->height > LSDC_CURS_MAX_SIZE)) {
> + drm_err(plane->dev, "Invalid cursor size: %dx%d\n",
> + fb->width, fb->height);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +
> +/* Update the location of the cursor */
> +static void lsdc_cursor_update_location(struct lsdc_device *ldev,
> + struct drm_crtc *crtc)
> +{
> + u32 val;
> +
> + val = lsdc_reg_read32(ldev, LSDC_CURSOR_CFG_REG);
> +
> + if ((val & CURSOR_FORMAT_MASK) == 0)
> + val |= CURSOR_FORMAT_ARGB8888;
> +
> + /* Update the location of the cursor */
> + if (drm_crtc_index(crtc))
> + val |= CURSOR_LOCATION_BIT;
> +
> + lsdc_reg_write32(ldev, LSDC_CURSOR_CFG_REG, val);
> +}
> +
> +/* update the position of the cursor */
> +static void lsdc_cursor_update_position(struct lsdc_device *ldev, int x, int y)
> +{
> + if (x < 0)
> + x = 0;
> +
> + if (y < 0)
> + y = 0;
> +
> + lsdc_reg_write32(ldev, LSDC_CURSOR_POSITION_REG, (y << 16) | x);
> +}
> +
> +
> +static void lsdc_cursor_atomic_update(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_device *ddev = plane->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> + struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> + struct drm_framebuffer *new_fb = new_plane_state->fb;
> + struct drm_framebuffer *old_fb = old_plane_state->fb;
> +
> + if (new_fb != old_fb) {
> + u64 cursor_addr;
> +
> + if (ldev->use_vram_helper) {
> +#ifdef CONFIG_DRM_LSDC_VRAM_DRIVER
> + s64 offset;
> +
> + offset = lsdc_get_vram_bo_offset(new_fb);
> + cursor_addr = ldev->vram_base + offset;
> +
> + drm_dbg_kms(ddev, "%s offset: %llx\n",
> + plane->name, offset);
> +#endif
> + } else {
> + struct drm_gem_cma_object *cursor_obj;
> +
> + cursor_obj = drm_fb_cma_get_gem_obj(new_fb, 0);
> + if (!cursor_obj)
> + return;
> +
> + cursor_addr = cursor_obj->paddr;
> + }
> +
> + lsdc_reg_write32(ldev, LSDC_CURSOR_ADDR_REG, cursor_addr);
> + }
> +
> + lsdc_cursor_update_position(ldev, new_plane_state->crtc_x,
> + new_plane_state->crtc_y);
> +
> + lsdc_cursor_update_location(ldev, new_plane_state->crtc);
> +}
> +
> +
> +static void lsdc_cursor_atomic_disable(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_device *ddev = plane->dev;
> + struct lsdc_device *ldev = to_lsdc(ddev);
> + struct drm_plane_state *old_plane_state;
> + struct drm_crtc *crtc;
> +
> + old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> +
> + if (old_plane_state)
> + crtc = old_plane_state->crtc;
> +
> + lsdc_reg_write32(ldev, LSDC_CURSOR_CFG_REG, 0);
> +
> + drm_dbg_kms(ddev, "%s disable\n", plane->name);
> +}
> +
> +
> +static const struct drm_plane_helper_funcs lsdc_cursor_plane_helpers = {
> + .prepare_fb = lsdc_plane_prepare_fb,
> + .cleanup_fb = lsdc_plane_cleanup_fb,
> + .atomic_check = lsdc_cursor_atomic_check,
> + .atomic_update = lsdc_cursor_atomic_update,
> + .atomic_disable = lsdc_cursor_atomic_disable,
> +};
> +
> +
> +static int lsdc_plane_get_default_zpos(enum drm_plane_type type)
> +{
> + switch (type) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + return 0;
> + case DRM_PLANE_TYPE_OVERLAY:
> + return 1;
> + case DRM_PLANE_TYPE_CURSOR:
> + return 7;
> + }
> + return 0;
> +}
> +
> +
> +static void lsdc_plane_reset(struct drm_plane *plane)
> +{
> + drm_atomic_helper_plane_reset(plane);
> +
> + plane->state->zpos = lsdc_plane_get_default_zpos(plane->type);
> +
> + drm_dbg_kms(plane->dev, "%s reset\n", plane->name);
> +}
> +
> +
> +static const struct drm_plane_funcs lsdc_plane_funcs = {
> + .update_plane = drm_atomic_helper_update_plane,
> + .disable_plane = drm_atomic_helper_disable_plane,
> + .destroy = drm_plane_cleanup,
> + .reset = lsdc_plane_reset,
> + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> +};
> +
> +
> +int lsdc_plane_init(struct lsdc_device *ldev, struct drm_plane *plane,
> + enum drm_plane_type type, unsigned int index)
> +{
> + struct drm_device *ddev = &ldev->drm;
> + int zpos = lsdc_plane_get_default_zpos(type);
> + unsigned int format_count;
> + const uint32_t *formats;
> + const char *name;
> + int ret;
> +
> + switch (type) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + formats = lsdc_primary_formats;
> + format_count = ARRAY_SIZE(lsdc_primary_formats);
> + name = "primary-%u";
> + break;
> + case DRM_PLANE_TYPE_CURSOR:
> + formats = lsdc_cursor_formats;
> + format_count = ARRAY_SIZE(lsdc_cursor_formats);
> + name = "cursor-%u";
> + break;
> + case DRM_PLANE_TYPE_OVERLAY:
> + drm_err(ddev, "overlay plane is not supported\n");
> + break;
> + }
> +
> + ret = drm_universal_plane_init(ddev, plane, 1 << index,
> + &lsdc_plane_funcs,
> + formats, format_count,
> + lsdc_fb_format_modifiers,
> + type, name, index);
> + if (ret) {
> + drm_err(ddev, "%s failed: %d\n", __func__, ret);
> + return ret;
> + }
> +
> + switch (type) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + drm_plane_helper_add(plane, &lsdc_primary_plane_helpers);
> + drm_plane_create_zpos_property(plane, zpos, 0, 6);
> + if (ldev->dirty_update)
> + drm_plane_enable_fb_damage_clips(plane);
> + break;
> + case DRM_PLANE_TYPE_CURSOR:
> + drm_plane_helper_add(plane, &lsdc_cursor_plane_helpers);
> + drm_plane_create_zpos_immutable_property(plane, zpos);
> + break;
> + case DRM_PLANE_TYPE_OVERLAY:
> + drm_err(ddev, "overlay plane is not supported\n");
> + break;
> + }
> +
> + drm_plane_create_alpha_property(plane);
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/lsdc/lsdc_pll.c b/drivers/gpu/drm/lsdc/lsdc_pll.c
> new file mode 100644
> index 000000000000..fcf3728a9067
> --- /dev/null
> +++ b/drivers/gpu/drm/lsdc/lsdc_pll.c
> @@ -0,0 +1,657 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 Loongson Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + */
> +
> +/*
> + * Authors:
> + * Sui Jingfeng <[email protected]>
> + */
> +
> +#include <drm/drm_print.h>
> +
> +#include "lsdc_drv.h"
> +#include "lsdc_regs.h"
> +#include "lsdc_pll.h"
> +
> +/* device dependent pixpll regs */
> +
> +/* u64 */
> +struct ls7a1000_pixpll_bitmap {
> + /* Byte 0 ~ Byte 3 */
> + unsigned div_out : 7; /* 0 : 6 output clock divider */
> + unsigned reserved_1 : 14; /* 7 : 20 */
> + unsigned loopc : 9; /* 21 : 29 */
> + unsigned reserved_2 : 2; /* 30 : 31 */
> +
> + /* Byte 4 ~ Byte 7 */
> + unsigned div_ref : 7; /* 0 : 6 input clock divider */
> + unsigned locked : 1; /* 7 PLL locked flag */
> + unsigned sel_out : 1; /* 8 output clk selector */
> + unsigned reserved_3 : 2; /* 9 : 10 reserved */
> + unsigned set_param : 1; /* 11 set pll param */
> + unsigned bypass : 1; /* 12 */
> + unsigned powerdown : 1; /* 13 */
> + unsigned reserved_4 : 18; /* 14 : 31 */
> +};
> +
> +
> +/* u128 */
> +struct ls2k1000_pixpll_bitmap {
> + /* Byte 0 ~ Byte 3 */
> + unsigned sel_out : 1; /* 0 select this PLL */
> + unsigned reserved_1 : 1; /* 1 */
> + unsigned sw_adj_en : 1; /* 2 allow software adjust */
> + unsigned bypass : 1; /* 3 bypass L1 PLL */
> + unsigned reserved_2 : 3; /* 4:6 */
> + unsigned lock_en : 1; /* 7 enable lock L1 PLL */
> + unsigned reserved_3 : 2; /* 8:9 */
> + unsigned lock_check : 2; /* 10:11 precision check */
> + unsigned reserved_4 : 4; /* 12:15 */
> +
> + unsigned locked : 1; /* 16 PLL locked flag bit */
> + unsigned reserved_5 : 2; /* 17:18 */
> + unsigned powerdown : 1; /* 19 powerdown the pll if set */
> + unsigned reserved_6 : 6; /* 20:25 */
> + unsigned div_ref : 6; /* 26:31 L1 Prescaler */
> +
> + /* Byte 4 ~ Byte 7 */
> + unsigned loopc : 10; /* 32:41 Clock Multiplier */
> + unsigned l1_div : 6; /* 42:47 not used */
> + unsigned reserved_7 : 16; /* 48:63 */
> +
> + /* Byte 8 ~ Byte 15 */
> + unsigned div_out : 6; /* 0 : 5 output clock divider */
> + unsigned reserved_8 : 26; /* 6 : 31 */
> + unsigned reserved_9 : 32; /* 70: 127 */
> +};
> +
> +
> +/* u32 */
> +struct ls2k0500_pixpll_bitmap {
> + /* Byte 0 ~ Byte 1 */
> + unsigned sel_out : 1;
> + unsigned reserved_1 : 2;
> + unsigned sw_adj_en : 1; /* allow software adjust */
> + unsigned bypass : 1; /* bypass L1 PLL */
> + unsigned powerdown : 1; /* write 1 to powerdown the PLL */
> + unsigned reserved_2 : 1;
> + unsigned locked : 1; /* 7 Is L1 PLL locked, read only */
> + unsigned div_ref : 6; /* 8:13 ref clock divider */
> + unsigned reserved_3 : 2; /* 14:15 */
> + /* Byte 2 ~ Byte 3 */
> + unsigned loopc : 8; /* 16:23 Clock Multiplier */
> + unsigned div_out : 6; /* 24:29 output clock divider */
> + unsigned reserved_4 : 2; /* 30:31 */
> +};
> +
> +
> +/*
> + * NOTE: All loongson's cpu is little endian.
> + */
> +union lsdc_pix_pll_param {
> + struct ls7a1000_pixpll_bitmap ls7a1000;
> + struct ls2k1000_pixpll_bitmap ls2k1000;
> + struct ls2k0500_pixpll_bitmap ls2k0500;
> +
> + u32 dword[4];
> +};
> +
> +/*
> + * pixel clock to pll parameters translation table
> + */
> +struct pixclk_to_pll_parm {
> + /* kHz */
> + unsigned int clock;
> +
> + /* unrelated information */
> + unsigned short width;
> + unsigned short height;
> + unsigned short vrefresh;
> +
> + /* Stores parameters for programming the Hardware PLLs */
> + unsigned short div_out;
> + unsigned short loopc;
> + unsigned short div_ref;
> +};
> +
> +
> +/*
> + * Small cached value to speed up pll parameter calculation
> + */
> +static const struct pixclk_to_pll_parm pll_param_table[] = {
> + {148500, 1920, 1080, 60, 11, 49, 3}, /* 1920x1080@60Hz */
> + /* 1920x1080@50Hz */
> + {174500, 1920, 1080, 75, 17, 89, 3}, /* 1920x1080@75Hz */
> + {181250, 2560, 1080, 75, 8, 58, 4}, /* 2560x1080@75Hz */
> + {146250, 1680, 1050, 60, 16, 117, 5}, /* 1680x1050@60Hz */
> + {135000, 1280, 1024, 75, 10, 54, 4}, /* 1280x1024@75Hz */
> +
> + {108000, 1600, 900, 60, 15, 81, 5}, /* 1600x900@60Hz */
> + /* 1280x1024@60Hz */
> + /* 1280x960@60Hz */
> + /* 1152x864@75Hz */
> +
> + {106500, 1440, 900, 60, 19, 81, 4}, /* 1440x900@60Hz */
> + {88750, 1440, 900, 60, 16, 71, 5}, /* 1440x900@60Hz */
> + {83500, 1280, 800, 60, 17, 71, 5}, /* 1280x800@60Hz */
> + {71000, 1280, 800, 60, 20, 71, 5}, /* 1280x800@60Hz */
> +
> + {74250, 1280, 720, 60, 22, 49, 3}, /* 1280x720@60Hz */
> + /* 1280x720@50Hz */
> +
> + {78750, 1024, 768, 75, 16, 63, 5}, /* 1024x768@75Hz */
> + {75000, 1024, 768, 70, 29, 87, 4}, /* 1024x768@70Hz */
> + {65000, 1024, 768, 60, 20, 39, 3}, /* 1024x768@60Hz */
> +
> + {51200, 1024, 600, 60, 25, 64, 5}, /* 1024x600@60Hz */
> +
> + {57284, 832, 624, 75, 24, 55, 4}, /* 832x624@75Hz */
> + {49500, 800, 600, 75, 40, 99, 5}, /* 800x600@75Hz */
> + {50000, 800, 600, 72, 44, 88, 4}, /* 800x600@72Hz */
> + {40000, 800, 600, 60, 30, 36, 3}, /* 800x600@60Hz */
> + {36000, 800, 600, 56, 50, 72, 4}, /* 800x600@56Hz */
> + {31500, 640, 480, 75, 40, 63, 5}, /* 640x480@75Hz */
> + /* 640x480@73Hz */
> +
> + {30240, 640, 480, 67, 62, 75, 4}, /* 640x480@67Hz */
> + {27000, 720, 576, 50, 50, 54, 4}, /* 720x576@60Hz */
> + {25175, 640, 480, 60, 85, 107, 5}, /* 640x480@60Hz */
> + {25200, 640, 480, 60, 50, 63, 5}, /* 640x480@60Hz */
> + /* 720x480@60Hz */
> +};
> +
> +/**
> + * lsdc_pixpll_setup
> + *
> + * @this: point to the object which calling this function
> + *
> + * ioremap the device dependent register before using it
> + */
> +static int lsdc_pixpll_setup(struct lsdc_pll * const this)
> +{
> + this->mmio = ioremap(this->reg_base, this->reg_size);
> +
> + drm_info(this->ddev, "PIXPLL%u REG[%x, %u] map to %llx\n",
> + this->index, this->reg_base, this->reg_size, (u64)this->mmio);
> +
> + return 0;
> +}
> +
> +
> +
> +/*
> + * Find a set of pll parameters (to generate pixel clock) from a static
> + * local table, which avoid to compute the pll parameter everytime a
> + * modeset is triggered.
> + *
> + * @this: point to the object which calling this function
> + * @clock: the desired pixel clock wanted to generate, the unit is kHz
> + * @pout: pointer to where hardware pll parameters(if found) to save
> + *
> + * Return true if a parameter is found, otherwise return false.
> + */
> +static bool lsdc_pixpll_find(struct lsdc_pll * const this,
> + unsigned int clock,
> + struct lsdc_pll_core_values * const pout)
> +{
> + unsigned int num = ARRAY_SIZE(pll_param_table);
> + unsigned int i;
> +
> + for (i = 0; i < num; i++) {
> + if (clock != pll_param_table[i].clock)
> + continue;
> +
> + pout->div_ref = pll_param_table[i].div_ref;
> + pout->loopc = pll_param_table[i].loopc;
> + pout->div_out = pll_param_table[i].div_out;
> +
> + return true;
> + }
> +
> + drm_dbg(this->ddev, "pixel clock %u: miss\n", clock);
> +
> + return false;
> +}
> +
> +/*
> + * Find a set of pll parameters which have minimal difference with desired
> + * clock frequency. It does that by computing the all of pll parameters
> + * combines possible and compare the diff and find the minimal.
> + *
> + * clock_out = refclk / div_ref * loopc / div_out
> + *
> + * refclk is fixed as 100MHz in ls7a1000, ls2k1000 and ls2k0500
> + *
> + * @this: point to the object which calling this function
> + * @clk: the desired pixel clock wanted to generate, the unit is kHz
> + * @verbose: print the pll parameter and the actual pixel clock.
> + * @pout: pointer to where hardware pll parameters(if found) to save
> + *
> + * Return true if a parameter is found, otherwise return false.
> + */
> +static bool lsdc_pixpll_compute(struct lsdc_pll * const this,
> + unsigned int clk,
> + bool verbose,
> + struct lsdc_pll_core_values * const pout)
> +{
> + unsigned int refclk = this->ref_clock;
> + const unsigned int tolerance = 1000;
> + unsigned int min = tolerance;
> + unsigned int div_out, loopc, div_ref;
> +
> + if (lsdc_pixpll_find(this, clk, pout))
> + return true;
> +
> + for (div_out = 6; div_out < 64; div_out++) {
> + for (div_ref = 3; div_ref < 6; div_ref++) {
> + for (loopc = 6; loopc < 161; loopc++) {
> + int diff;
> +
> + if (loopc < 12 * div_ref)
> + continue;
> + if (loopc > 32 * div_ref)
> + continue;
> +
> + diff = clk * div_out - refclk * loopc / div_ref;
> +
> + if (diff < 0)
> + diff = -diff;
> +
> + if (diff < min) {
> + min = diff;
> + pout->div_ref = div_ref;
> + pout->div_out = div_out;
> + pout->loopc = loopc;
> +
> + if (diff == 0)
> + return true;
> + }
> + }
> + }
> + }
> +
> + if (verbose) {
> + unsigned int clk_out;
> +
> + clk_out = refclk / pout->div_ref * pout->loopc / pout->div_out;
> +
> + drm_info(this->ddev, "pixpll%u\n", this->index);
> +
> + drm_info(this->ddev, "div_ref=%u, loopc=%u, div_out=%u\n",
> + pout->div_ref, pout->loopc, pout->div_out);
> +
> + drm_info(this->ddev, "desired clk=%u, actual out=%u, diff=%d\n",
> + clk, clk_out, clk_out - clk);
> + }
> +
> + return min < tolerance;
> +}
> +
> +/*
> + * Update the pll parameters to hardware, target to the pixpll in ls7a1000
> + *
> + * @this: point to the object which calling this function
> + * @param: pointer to where the parameters passed in
> + *
> + * Return true if a parameter is found, otherwise return false.
> + */
> +static int ls7a1000_pixpll_param_update(struct lsdc_pll * const this,
> + const struct lsdc_pll_core_values * const param)
> +{
> + u32 val;
> + unsigned int counter = 0;
> + void __iomem *reg = this->mmio;
> + bool locked;
> +
> +
> + /* clear sel_pll_out0 */
> + val = readl(reg + 0x4);
> + val &= ~(1 << 8);
> + writel(val, reg + 0x4);
> +
> + /* set pll_pd */
> + val = readl(reg + 0x4);
> + val |= (1 << 13);
> + writel(val, reg + 0x4);
> +
> + /* clear set_pll_param */
> + val = readl(reg + 0x4);
> + val &= ~(1 << 11);
> + writel(val, reg + 0x4);
> +
> + /* clear old value & config new value */
> + val = readl(reg + 0x04);
> + val &= ~0x7F;
> +
> + val |= param->div_ref; /* div_ref */
> + writel(val, reg + 0x4);
> +
> + val = readl(reg);
> + val &= ~(0x7f << 0);
> + val |= param->div_out; /* div_out */
> + val &= ~(0x1ffUL << 21);
> + val |= param->loopc << 21; /* loopc */
> + writel(val, reg);
> +
> + /* set set_pll_param */
> + val = readl(reg + 0x4);
> + val |= (1 << 11);
> + writel(val, reg + 0x4);
> +
> + /* clear pll_pd */
> + val = readl(reg + 0x4);
> + val &= ~(1 << 13);
> + writel(val, reg + 0x4);
> +
> + /* wait pll lock */
> + do {
> + val = readl(reg + 0x4);
> + locked = val & 0x80;
> + counter++;
> + } while (locked == false);
> +
> + drm_dbg_kms(this->ddev, "%u loop waited\n", counter);
> +
> + /* set sel_pll_out0 */
> + val = readl(reg + 0x4);
> + val |= (1UL << 8);
> + writel(val, reg + 0x4);
> +
> + return 0;
> +}
> +
> +
> +/*
> + * Update the pll parameters to hardware, target to the pixpll in ls2k1000
> + *
> + * @this: point to the object which calling this function
> + * @param: pointer to where the parameters passed in
> + *
> + * Return true if a parameter is found, otherwise return false.
> + */
> +static int ls2k1000_pixpll_param_update(struct lsdc_pll * const this,
> + const struct lsdc_pll_core_values * const param)
> +{
> + void __iomem *reg = this->mmio;
> + unsigned int counter = 0;
> + bool locked = false;
> + u32 val;
> +
> + val = readl(reg);
> + /* Bypass the software configured PLL, using refclk directly */
> + val &= ~(1 << 0);
> + writel(val, reg);
> +
> + /* powerdown the PLL */
> + val |= (1 << 19);
> + writel(val, reg);
> +
> + /* Allow the software configuration */
> + val &= ~(1 << 2);
> + writel(val, reg);
> +
> + /* allow L1 PLL lock */
> + val = (1L << 7) | (3L << 10);
> + writel(val, reg);
> +
> + /* clear div_ref bit field */
> + val &= ~(0x3f << 26);
> + /* set div_ref bit field */
> + val = val | (param->div_ref << 26);
> + writel(val, reg);
> +
> + val = readl(reg + 4);
> + /* clear loopc bit field */
> + val &= ~0x0fff;
> + /* set loopc bit field */
> + val |= param->loopc;
> + writel(val, reg + 4);
> +
> + /* set div_out */
> + writel(param->div_out, reg + 8);
> +
> + val = readl(reg);
> + /* use the software configure param */
> + val |= (1 << 2);
> + /* powerup the PLL */
> + val &= ~(1 << 19);
> + writel(val, reg);
> +
> + /* wait pll setup and locked */
> + do {
> + val = readl(reg);
> + locked = val & 0x10000;
> + counter++;
> + } while (locked == false);
> +
> + drm_dbg_kms(this->ddev, "%u loop waited\n", counter);
> +
> + /* Switch to the above software configured PLL instead of refclk */
> + val |= 1;
> + writel(val, reg);
> +
> + return 0;
> +}
> +
> +/*
> + * Update the pll parameters to hardware, target to the pixpll in ls2k0500
> + *
> + * @this: point to the object which calling this function
> + * @param: pointer to where the parameters passed in
> + *
> + * Return true if a parameter is found, otherwise return false.
> + */
> +
> +static int ls2k0500_pixpll_param_update(struct lsdc_pll * const this,
> + const struct lsdc_pll_core_values * const param)
> +{
> + void __iomem *reg = this->mmio;
> + unsigned int counter = 0;
> + bool locked = false;
> + u32 val;
> +
> + /* Bypass the software configured PLL, using refclk directly */
> + val = readl(reg);
> + val &= ~(1 << 0);
> + writel(val, reg);
> +
> + /* Powerdown the PLL */
> + val = readl(reg);
> + val |= (1 << 5);
> + writel(val, reg);
> +
> + /* Allow the software configuration */
> + val |= (1 << 3);
> + writel(val, reg);
> +
> + /* Update the pll params */
> + val = (param->div_out << 24) |
> + (param->loopc << 16) |
> + (param->div_ref << 8);
> +
> + writel(val, reg);
> +
> + /* Powerup the PLL */
> + val = readl(reg);
> + val &= ~(1 << 5);
> + writel(val, reg);
> +
> + /* wait pll setup and locked */
> + do {
> + val = readl(reg);
> + locked = val & 0x80;
> + counter++;
> + } while (locked == false);
> +
> + drm_dbg_kms(this->ddev, "%u loop waited\n", counter);
> +
> + /* Switch to the above software configured PLL instead of refclk */
> + writel((val | 1), reg);
> +
> + return 0;
> +}
> +
> +
> +#define LSDC_PIXPLL_BITMAP(type,var,parms) \
> + struct type ## _pixpll_bitmap *var = &parms.type
> +
> +#define LSDC_PIXPLL_PRINT_CODE_BLOCK(ddev,var,index,refclk) \
> +{ \
> + out_clk = refclk / var->div_ref * var->loopc / var->div_out; \
> + drm_info(ddev, "div_ref=%u, loopc=%u, div_out=%u\n", \
> + var->div_ref, var->loopc, var->div_out); \
> + drm_info(ddev, "locked: %s\n", var->locked ? "Yes" : "No"); \
> + drm_info(ddev, "bypass: %s\n", var->bypass ? "Yes" : "No"); \
> + drm_info(ddev, "powerdown: %s\n", var->powerdown ? "Yes" : "No"); \
> + drm_info(ddev, "set_out: %s\n", var->sel_out ? "Yes" : "No"); \
> + drm_info(ddev, "pixpll%u generate %ukHz\n", index, out_clk); \
> + drm_info(ddev, "\n"); \
> +}

This should be a debug-level trace

Maxime


Attachments:
(No filename) (129.03 kB)
signature.asc (235.00 B)
Download all attachments

2022-02-03 20:31:17

by Sui Jingfeng

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


>> +static enum drm_mode_status
>> +lsdc_crtc_helper_mode_valid(struct drm_crtc *crtc,
>> + const struct drm_display_mode *mode)
>> +{
>> + struct drm_device *ddev = crtc->dev;
>> + struct lsdc_device *ldev = to_lsdc(ddev);
>> + const struct lsdc_chip_desc *desc = ldev->desc;
>> +
>> + if (mode->hdisplay > desc->max_width)
>> + return MODE_BAD_HVALUE;
>> + if (mode->vdisplay > desc->max_height)
>> + return MODE_BAD_VVALUE;
>> +
>> + if (mode->clock > desc->max_pixel_clk) {
>> + drm_dbg_kms(ddev, "mode %dx%d, pixel clock=%d is too high\n",
>> + mode->hdisplay, mode->vdisplay, mode->clock);
>> + return MODE_CLOCK_HIGH;
>> + }
>> +
>> + /* the crtc hardware dma take 256 bytes once a time
>> + * TODO: check RGB565 support
>> + */
>> + if ((mode->hdisplay * 4) % desc->stride_alignment) {
>> + drm_dbg_kms(ddev, "stride is not %u bytes aligned\n",
>> + desc->stride_alignment);
>> + return MODE_BAD;
>> + }
>> +
>> + return MODE_OK;
>> +}
> mode_valid will only prevent the mode from being advertised to the
> userspace, but you need atomic_check if you want to prevent those modes
> to be used by anybody.

Yes, I used to change mode with mate-display-properties tools,
what I though is the end user can't see it, they can't set it.
I will add atomic_check() support at next version, thanks.

>> +
>> +static void lsdc_update_pixclk(struct drm_crtc *crtc, unsigned int pixclk, bool verbose)
>> +{
>> + struct lsdc_display_pipe *dispipe;
>> + struct lsdc_pll *pixpll;
>> + const struct lsdc_pixpll_funcs *clkfun;
>> + struct lsdc_crtc_state *priv_crtc_state;
>> +
>> + priv_crtc_state = to_lsdc_crtc_state(crtc->state);
>> +
>> + dispipe = container_of(crtc, struct lsdc_display_pipe, crtc);
>> + pixpll = &dispipe->pixpll;
>> + clkfun = pixpll->funcs;
>> +
>> + /* config the pixel pll */
>> + clkfun->update(pixpll, &priv_crtc_state->pparams);
>> +
>> + if (verbose)
>> + clkfun->print(pixpll, pixclk);
>> +}
>> +
>> +
>> +static void lsdc_crtc_helper_mode_set_nofb(struct drm_crtc *crtc)
>> +{
>> + struct drm_device *ddev = crtc->dev;
>> + struct lsdc_device *ldev = to_lsdc(ddev);
>> + struct drm_display_mode *mode = &crtc->state->adjusted_mode;
>> + unsigned int hr = mode->hdisplay;
>> + unsigned int hss = mode->hsync_start;
>> + unsigned int hse = mode->hsync_end;
>> + unsigned int hfl = mode->htotal;
>> + unsigned int vr = mode->vdisplay;
>> + unsigned int vss = mode->vsync_start;
>> + unsigned int vse = mode->vsync_end;
>> + unsigned int vfl = mode->vtotal;
>> + unsigned int pixclock = mode->clock;
>> + unsigned int index = drm_crtc_index(crtc);
>> +
>> +
>> + if (index == 0) {
>> + /* CRTC 0 */
>> + u32 hsync, vsync;
>> +
>> + lsdc_reg_write32(ldev, LSDC_CRTC0_FB_ORIGIN_REG, 0);
>> +
>> + /* 26:16 total pixels, 10:0 visiable pixels, in horizontal */
>> + lsdc_reg_write32(ldev, LSDC_CRTC0_HDISPLAY_REG,
>> + (mode->crtc_htotal << 16) | mode->crtc_hdisplay);
>> +
>> + /* 26:16 total pixels, 10:0 visiable pixels, in vertical */
>> + lsdc_reg_write32(ldev, LSDC_CRTC0_VDISPLAY_REG,
>> + (mode->crtc_vtotal << 16) | mode->crtc_vdisplay);
>> +
>> + /* 26:16 hsync end, 10:0 hsync start */
>> + hsync = (mode->crtc_hsync_end << 16) | mode->crtc_hsync_start;
>> +
>> + if (mode->flags & DRM_MODE_FLAG_NHSYNC)
>> + hsync |= INV_HSYNC_BIT;
>> +
>> + lsdc_reg_write32(ldev, LSDC_CRTC0_HSYNC_REG, EN_HSYNC_BIT | hsync);
>> +
>> + /* 26:16 vsync end, 10:0 vsync start */
>> + vsync = (mode->crtc_vsync_end << 16) | mode->crtc_vsync_start;
>> +
>> + if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>> + vsync |= INV_VSYNC_BIT;
>> +
>> + lsdc_reg_write32(ldev, LSDC_CRTC0_VSYNC_REG, EN_VSYNC_BIT | vsync);
>> +
>> + } else if (index == 1) {
>> + /* CRTC 1 */
>> + u32 hsync, vsync;
>> +
>> + lsdc_reg_write32(ldev, LSDC_CRTC1_FB_ORIGIN_REG, 0);
>> +
>> + /* 26:16 total pixels, 10:0 visiable pixels, in horizontal */
>> + lsdc_reg_write32(ldev, LSDC_CRTC1_HDISPLAY_REG,
>> + (mode->crtc_htotal << 16) | mode->crtc_hdisplay);
>> +
>> + /* 26:16 total pixels, 10:0 visiable pixels, in vertical */
>> + lsdc_reg_write32(ldev, LSDC_CRTC1_VDISPLAY_REG,
>> + (mode->crtc_vtotal << 16) | mode->crtc_vdisplay);
>> +
>> + /* 26:16 hsync end, 10:0 hsync start */
>> + hsync = (mode->crtc_hsync_end << 16) | mode->crtc_hsync_start;
>> +
>> + if (mode->flags & DRM_MODE_FLAG_NHSYNC)
>> + hsync |= INV_HSYNC_BIT;
>> +
>> + lsdc_reg_write32(ldev, LSDC_CRTC1_HSYNC_REG, EN_HSYNC_BIT | hsync);
>> +
>> + /* 26:16 vsync end, 10:0 vsync start */
>> + vsync = (mode->crtc_vsync_end << 16) | mode->crtc_vsync_start;
>> +
>> + if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>> + vsync |= INV_VSYNC_BIT;
>> +
>> + lsdc_reg_write32(ldev, LSDC_CRTC1_VSYNC_REG, EN_VSYNC_BIT | vsync);
>> + }
>> +
>> + drm_dbg_kms(ddev, "hdisplay=%d, hsync_start=%d, hsync_end=%d, htotal=%d\n",
>> + hr, hss, hse, hfl);
>> +
>> + drm_dbg_kms(ddev, "vdisplay=%d, vsync_start=%d, vsync_end=%d, vtotal=%d\n",
>> + vr, vss, vse, vfl);
>> +
>> + drm_dbg_kms(ddev, "%s modeset: %ux%u\n", crtc->name, hr, vr);
>> +
>> + lsdc_update_pixclk(crtc, pixclock, ldev->verbose);
>> +}
>> +
>> +
>> +static void lsdc_enable_display(struct lsdc_device *ldev, unsigned int index)
>> +{
>> + u32 val;
>> +
>> + if (index == 0) {
>> + val = lsdc_reg_read32(ldev, LSDC_CRTC0_CFG_REG);
>> + val |= CFG_OUTPUT_EN_BIT;
>> + lsdc_reg_write32(ldev, LSDC_CRTC0_CFG_REG, val);
>> + } else if (index == 1) {
>> + val = lsdc_reg_read32(ldev, LSDC_CRTC1_CFG_REG);
>> + val |= CFG_OUTPUT_EN_BIT;
>> + lsdc_reg_write32(ldev, LSDC_CRTC1_CFG_REG, val);
>> + }
>> +}
>> +
>> +
>> +static void lsdc_disable_display(struct lsdc_device *ldev, unsigned int index)
>> +{
>> + u32 val;
>> +
>> + if (index == 0) {
>> + val = lsdc_reg_read32(ldev, LSDC_CRTC0_CFG_REG);
>> + val &= ~CFG_OUTPUT_EN_BIT;
>> + lsdc_reg_write32(ldev, LSDC_CRTC0_CFG_REG, val);
>> + } else if (index == 1) {
>> + val = lsdc_reg_read32(ldev, LSDC_CRTC1_CFG_REG);
>> + val &= ~CFG_OUTPUT_EN_BIT;
>> + lsdc_reg_write32(ldev, LSDC_CRTC1_CFG_REG, val);
>> + }
>> +}
>> +
>> +
>> +static void lsdc_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>> + struct drm_atomic_state *state)
>> +{
>> + struct drm_device *ddev = crtc->dev;
>> + struct lsdc_device *ldev = to_lsdc(ddev);
>> +
>> + drm_crtc_vblank_on(crtc);
>> +
>> + lsdc_enable_display(ldev, drm_crtc_index(crtc));
>> +
>> + drm_dbg_kms(ddev, "%s: enabled\n", crtc->name);
>> +}
>> +
>> +
>> +static void lsdc_crtc_helper_atomic_disable(struct drm_crtc *crtc,
>> + struct drm_atomic_state *state)
>> +{
>> + struct drm_device *ddev = crtc->dev;
>> + struct lsdc_device *ldev = to_lsdc(ddev);
>> +
>> + drm_crtc_vblank_off(crtc);
>> +
>> + lsdc_disable_display(ldev, drm_crtc_index(crtc));
>> +
>> + drm_dbg_kms(ddev, "%s: disabled\n", crtc->name);
>> +}
>> +
>> +
>> +static void lsdc_crtc_atomic_flush(struct drm_crtc *crtc,
>> + struct drm_atomic_state *state)
>> +{
>> + struct drm_pending_vblank_event *event = crtc->state->event;
>> +
>> + if (event) {
>> + crtc->state->event = NULL;
>> +
>> + spin_lock_irq(&crtc->dev->event_lock);
>> + if (drm_crtc_vblank_get(crtc) == 0)
>> + drm_crtc_arm_vblank_event(crtc, event);
>> + else
>> + drm_crtc_send_vblank_event(crtc, event);
>> + spin_unlock_irq(&crtc->dev->event_lock);
>> + }
>> +}
>> +
>> +
>> +static const struct drm_crtc_helper_funcs lsdc_crtc_helper_funcs = {
>> + .mode_valid = lsdc_crtc_helper_mode_valid,
>> + .mode_set_nofb = lsdc_crtc_helper_mode_set_nofb,
>> + .atomic_enable = lsdc_crtc_helper_atomic_enable,
>> + .atomic_disable = lsdc_crtc_helper_atomic_disable,
>> + .atomic_flush = lsdc_crtc_atomic_flush,
>> +};
>> +
>> +
>> +
>> +/**
>> + * lsdc_crtc_init
>> + *
>> + * @ddev: point to the drm_device structure
>> + * @index: hardware crtc index
>> + *
>> + * Init CRTC
>> + */
>> +int lsdc_crtc_init(struct drm_device *ddev,
>> + struct drm_crtc *crtc,
>> + unsigned int index,
>> + struct drm_plane *primary,
>> + struct drm_plane *cursor)
>> +{
>> + int ret;
>> +
>> + drm_crtc_helper_add(crtc, &lsdc_crtc_helper_funcs);
>> +
>> + ret = drm_mode_crtc_set_gamma_size(crtc, 256);
>> + if (ret)
>> + drm_warn(ddev, "set the gamma table size failed\n");
>> +
>> + return drm_crtc_init_with_planes(ddev,
>> + crtc,
>> + primary,
>> + cursor,
>> + &lsdc_crtc_funcs,
>> + "crtc%d",
>> + index);
>> +}
>> diff --git a/drivers/gpu/drm/lsdc/lsdc_drv.c b/drivers/gpu/drm/lsdc/lsdc_drv.c
>> new file mode 100644
>> index 000000000000..aac8901c3431
>> --- /dev/null
>> +++ b/drivers/gpu/drm/lsdc/lsdc_drv.c
>> @@ -0,0 +1,846 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2020 Loongson Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the
>> + * "Software"), to deal in the Software without restriction, including
>> + * without limitation the rights to use, copy, modify, merge, publish,
>> + * distribute, sub license, and/or sell copies of the Software, and to
>> + * permit persons to whom the Software is furnished to do so, subject to
>> + * the following conditions:
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
>> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
>> + *
>> + * The above copyright notice and this permission notice (including the
>> + * next paragraph) shall be included in all copies or substantial portions
>> + * of the Software.
>> + */
>> +
>> +/*
>> + * Authors:
>> + * Sui Jingfeng <[email protected]>
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/string.h>
>> +#include <linux/module.h>
>> +#include <linux/pci.h>
>> +#include <linux/of_reserved_mem.h>
>> +
>> +#include <drm/drm_drv.h>
>> +#include <drm/drm_aperture.h>
>> +#include <drm/drm_of.h>
>> +#include <drm/drm_plane.h>
>> +#include <drm/drm_vblank.h>
>> +#include <drm/drm_debugfs.h>
>> +#include <drm/drm_fb_helper.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_gem_cma_helper.h>
>> +#include <drm/drm_fb_cma_helper.h>
>> +#include <drm/drm_gem_framebuffer_helper.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_damage_helper.h>
>> +#include <drm/drm_probe_helper.h>
>> +
>> +#include "lsdc_drv.h"
>> +#include "lsdc_irq.h"
>> +#include "lsdc_regs.h"
>> +#include "lsdc_connector.h"
>> +#include "lsdc_pll.h"
>> +
>> +
>> +#define DRIVER_AUTHOR "Sui Jingfeng <[email protected]>"
>> +#define DRIVER_NAME "lsdc"
>> +#define DRIVER_DESC "drm driver for loongson's display controller"
>> +#define DRIVER_DATE "20200701"
>> +#define DRIVER_MAJOR 1
>> +#define DRIVER_MINOR 0
>> +#define DRIVER_PATCHLEVEL 0
>> +
>> +static int lsdc_modeset = 1;
>> +MODULE_PARM_DESC(modeset, "Enable/disable CMA-based KMS(1 = enabled(default), 0 = disabled)");
>> +module_param_named(modeset, lsdc_modeset, int, 0644);
>> +
>> +static int lsdc_cached_coherent = 1;
>> +MODULE_PARM_DESC(cached_coherent, "uss cached coherent mapping(1 = enabled(default), 0 = disabled)");
>> +module_param_named(cached_coherent, lsdc_cached_coherent, int, 0644);
>> +
>> +static int lsdc_dirty_update = -1;
>> +MODULE_PARM_DESC(dirty_update, "enable dirty update(1 = enabled, 0 = disabled(default))");
>> +module_param_named(dirty_update, lsdc_dirty_update, int, 0644);
>> +
>> +static int lsdc_use_vram_helper = -1;
>> +MODULE_PARM_DESC(use_vram_helper, "use vram helper based solution(1 = enabled, 0 = disabled(default))");
>> +module_param_named(use_vram_helper, lsdc_use_vram_helper, int, 0644);
>> +
>> +static int lsdc_verbose = -1;
>> +MODULE_PARM_DESC(verbose, "Enable/disable print some key information");
>> +module_param_named(verbose, lsdc_verbose, int, 0644);
> It's not really clear to me why you need any of those parameters. Why
> would a user want to use a non coherent mapping for example?
>
Because we are Mips architecture. Paul Cercueil already explained it
in his mmap GEM buffers cachedpatch <https://lkml.kernel.org/lkml/[email protected]/T/>. I drag part of it to here for
convenient to reading:

/Traditionally, GEM buffers are mapped write-combine. Writes to the
buffer are accelerated, and reads are slow. Application doing lots////of alpha-blending paint inside shadow buffers, which is then memcpy'd////into the final GEM buffer.///
"non coherent mapping" is actually cached and it is for CMA helpers
base driver, not for VRAM helper based driver. For Loongson CPU/SoCs.
The cache coherency is maintained by hardware, therefore there no
need to worry about coherency problems. This is true at least for
ls3a3000, ls3a4000 and ls3a5000.

"non coherent" or "coherent" is not important here, the key point is
that the backing memory of the framebuffer is cached with non coherent
mapping, you don't need a shadow buffer layer when using X server's
modesetting driver.

Read and write to the framebuffer in system memory is much faster than
read and write to the framebuffer in the VRAM.

Why CMA helper based solution is faster than the VRAM based solution on Mips platform?

Partly because of the CPU have L1, L2 and L3 cache, especially L3 cache
is as large as 8MB, read and write from the cache is fast.

Another reason is as Paul Cercueil said, read from VRAM with write-combine
cache mode is slow. it is just uncache read.
Please note that we don't have a GPU here, we are just a display controller.

For the VRAM helper based driver case, the backing memory of the framebuffer
is located at VRAM, When using X server's modesetting driver, we have to enable
the ShadowFB option, Uncache acceleration support(at the kernel size) should
also be enabled. Otherwise the performance of graphic application is just slow.

Beside write-combine cache mode have bugs on our platform, a kernel side
developer have disabled it. Write-combine cache mode just boil down to uncached
now. See [1] and [2]

[1]https://lkml.org/lkml/2020/8/10/255
[2]https://lkml.kernel.org/lkml/[email protected]/T/


This is the reason why we prefer CMA helper base solution with non coherent mapping,
simply because it is fast.

As far as I know, Loongson's CPU does not has the concept of write-combine,
it only support three caching mode: uncached, cached and uncache acceleration.
write-combine is implemented with uncache acceleration on Mips.


2022-02-03 23:48:21

by Sui Jingfeng

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



>> diff --git a/drivers/gpu/drm/lsdc/Makefile b/drivers/gpu/drm/lsdc/Makefile
>> new file mode 100644
>> index 000000000000..342990654478
>> --- /dev/null
>> +++ b/drivers/gpu/drm/lsdc/Makefile
>> @@ -0,0 +1,15 @@
>> +#
>> +# Makefile for the lsdc drm device driver.
>> +#
>> +
>> +lsdc-y := \
>> + lsdc_drv.o \
>> + lsdc_crtc.o \
>> + lsdc_irq.o \
>> + lsdc_plane.o \
>> + lsdc_pll.o \
>> + lsdc_i2c.o \
>> + lsdc_encoder.o \
>> + lsdc_connector.o
>> +
>> +obj-$(CONFIG_DRM_LSDC) += lsdc.o
>> diff --git a/drivers/gpu/drm/lsdc/lsdc_connector.c b/drivers/gpu/drm/lsdc/lsdc_connector.c
>> new file mode 100644
>> index 000000000000..ae5fc0c90961
>> --- /dev/null
>> +++ b/drivers/gpu/drm/lsdc/lsdc_connector.c
>> @@ -0,0 +1,443 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2020 Loongson Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the
>> + * "Software"), to deal in the Software without restriction, including
>> + * without limitation the rights to use, copy, modify, merge, publish,
>> + * distribute, sub license, and/or sell copies of the Software, and to
>> + * permit persons to whom the Software is furnished to do so, subject to
>> + * the following conditions:
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
>> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
>> + *
>> + * The above copyright notice and this permission notice (including the
>> + * next paragraph) shall be included in all copies or substantial portions
>> + * of the Software.
>> + */
> That's the MIT license, yet you claim the driver to be licensed under
> the GPLv2 or later?

I just copy then paste it blindly, sorry about that.
I do not know the difference, we want open the source anyway.
I will correct it in next version, thanks.

>> +
>> +/*
>> + * Authors:
>> + * Sui Jingfeng <[email protected]>
>> + */
>> +
>> +#include <drm/drm_print.h>
>> +#include <drm/drm_edid.h>
>> +#include <drm/drm_probe_helper.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_connector.h>
>> +
>> +#include <video/videomode.h>
>> +#include <video/of_display_timing.h>
>> +
>> +#include "lsdc_drv.h"
>> +#include "lsdc_i2c.h"
>> +#include "lsdc_connector.h"
>> +
>> +
>> +static int lsdc_get_modes_from_edid(struct drm_connector *connector)
>> +{
>> + struct drm_device *ddev = connector->dev;
>> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
>> + struct edid *edid_p = (struct edid *)lconn->edid_data;
>> + int num = drm_add_edid_modes(connector, edid_p);
>> +
>> + if (num)
>> + drm_connector_update_edid_property(connector, edid_p);
>> +
>> + drm_dbg_kms(ddev, "%d modes added\n", num);
>> +
>> + return num;
>> +}
>> +
>> +
>> +static int lsdc_get_modes_from_timings(struct drm_connector *connector)
>> +{
>> + struct drm_device *ddev = connector->dev;
>> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
>> + struct display_timings *disp_tim = lconn->disp_tim;
>> + unsigned int num = 0;
>> + unsigned int i;
>> +
>> + for (i = 0; i < disp_tim->num_timings; i++) {
>> + const struct display_timing *dt = disp_tim->timings[i];
>> + struct drm_display_mode *mode;
>> + struct videomode vm;
>> +
>> + videomode_from_timing(dt, &vm);
>> + mode = drm_mode_create(ddev);
>> + if (!mode) {
>> + drm_err(ddev, "failed to add mode %ux%u\n",
>> + dt->hactive.typ, dt->vactive.typ);
>> + continue;
>> + }
>> +
>> + drm_display_mode_from_videomode(&vm, mode);
>> +
>> + mode->type |= DRM_MODE_TYPE_DRIVER;
>> +
>> + if (i == disp_tim->native_mode)
>> + mode->type |= DRM_MODE_TYPE_PREFERRED;
>> +
>> + drm_mode_probed_add(connector, mode);
>> + num++;
>> + }
>> +
>> + drm_dbg_kms(ddev, "%d modes added\n", num);
>> +
>> + return num;
>> +}
>> +
>> +
>> +static int lsdc_get_modes_from_ddc(struct drm_connector *connector,
>> + struct i2c_adapter *ddc)
>> +{
>> + unsigned int num = 0;
>> + struct edid *edid;
>> +
>> + edid = drm_get_edid(connector, ddc);
>> + if (edid) {
>> + drm_connector_update_edid_property(connector, edid);
>> + num = drm_add_edid_modes(connector, edid);
>> + kfree(edid);
>> + }
>> +
>> + return num;
>> +}
>> +
>> +
>> +static int lsdc_get_modes(struct drm_connector *connector)
>> +{
>> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
>> + unsigned int num = 0;
>> +
>> + if (lconn->has_edid)
>> + return lsdc_get_modes_from_edid(connector);
>> +
>> + if (lconn->has_disp_tim)
>> + return lsdc_get_modes_from_timings(connector);
>> +
>> + if (IS_ERR_OR_NULL(lconn->ddc) == false)
>> + return lsdc_get_modes_from_ddc(connector, lconn->ddc);
>> +
>> + if (connector->connector_type == DRM_MODE_CONNECTOR_VIRTUAL) {
>> + num = drm_add_modes_noedid(connector,
>> + connector->dev->mode_config.max_width,
>> + connector->dev->mode_config.max_height);
>> +
>> + drm_set_preferred_mode(connector, 1024, 768);
>> +
>> + return num;
>> + }
>> +
>> +
>> + /*
>> + * In case we cannot retrieve the EDIDs (broken or missing i2c
>> + * bus), fallback on the XGA standards, because we are for board
>> + * bringup.
>> + */
>> + num = drm_add_modes_noedid(connector, 1920, 1200);
>> +
>> + /* And prefer a mode pretty much anyone can handle */
>> + drm_set_preferred_mode(connector, 1024, 768);
>> +
>> + return num;
>> +
>> +}
>> +
>> +
>> +static enum drm_connector_status
>> +lsdc_connector_detect(struct drm_connector *connector, bool force)
>> +{
>> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
>> +
>> + if (lconn->has_edid == true)
>> + return connector_status_connected;
>> +
>> + if (lconn->has_disp_tim == true)
>> + return connector_status_connected;
>> +
>> + if (IS_ERR_OR_NULL(lconn->ddc) == false)
>> + return drm_probe_ddc(lconn->ddc);
>> +
>> + if (lconn->ddc && drm_probe_ddc(lconn->ddc))
>> + return connector_status_connected;
>> +
>> + if (connector->connector_type == DRM_MODE_CONNECTOR_VIRTUAL)
>> + return connector_status_connected;
>> +
>> + if ((connector->connector_type == DRM_MODE_CONNECTOR_DVIA) ||
>> + (connector->connector_type == DRM_MODE_CONNECTOR_DVID) ||
>> + (connector->connector_type == DRM_MODE_CONNECTOR_DVII))
>> + return connector_status_disconnected;
>> +
>> + if ((connector->connector_type == DRM_MODE_CONNECTOR_HDMIA) ||
>> + (connector->connector_type == DRM_MODE_CONNECTOR_HDMIB))
>> + return connector_status_disconnected;
>> +
>> + return connector_status_unknown;
>> +}
>> +
>> +
>> +/*
>> + * @connector: point to the drm_connector structure
>> + *
>> + * Clean up connector resources
>> + */
>> +static void lsdc_connector_destroy(struct drm_connector *connector)
>> +{
>> + struct drm_device *ddev = connector->dev;
>> + struct lsdc_connector *lconn = to_lsdc_connector(connector);
>> +
>> + if (lconn) {
>> + if (lconn->ddc)
>> + lsdc_destroy_i2c(connector->dev, lconn->ddc);
>> +
>> + drm_info(ddev, "destroying connector%u\n", lconn->index);
>> +
>> + devm_kfree(ddev->dev, lconn);
>> + }
>> +
>> + drm_connector_cleanup(connector);
>> +}
>> +
>> +
>> +static const struct drm_connector_helper_funcs lsdc_connector_helpers = {
>> + .get_modes = lsdc_get_modes,
>> +};
>> +
>> +/*
>> + * These provide the minimum set of functions required to handle a connector
>> + *
>> + * Control connectors on a given device.
>> + *
>> + * Each CRTC may have one or more connectors attached to it.
>> + * The functions below allow the core DRM code to control
>> + * connectors, enumerate available modes, etc.
>> + */
>> +static const struct drm_connector_funcs lsdc_connector_funcs = {
>> + .dpms = drm_helper_connector_dpms,
>> + .detect = lsdc_connector_detect,
>> + .fill_modes = drm_helper_probe_single_connector_modes,
>> + .destroy = lsdc_connector_destroy,
>> + .reset = drm_atomic_helper_connector_reset,
>> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +
>> +/* Get the simple EDID data from the device tree
>> + * the length must be EDID_LENGTH, since it is simple.
>> + *
>> + * @np: device node contain edid data
>> + * @edid_data: where the edid data to store to
>> + */
>> +static bool lsdc_get_edid_from_dtb(struct device_node *np,
>> + unsigned char *edid_data)
>> +{
>> + int length;
>> + const void *prop;
>> +
>> + if (np == NULL)
>> + return false;
>> +
>> + prop = of_get_property(np, "edid", &length);
>> + if (prop && (length == EDID_LENGTH)) {
>> + memcpy(edid_data, prop, EDID_LENGTH);
>> + return true;
>> + }
>> +
>> + return false;
>> +}
> You don't have a device tree binding for that driver, this is something
> that is required. And it's not clear to me why you'd want EDID in the
> DTB?

1) It is left to the end user of this driver.

The downstream motherboard maker may use a dpi(XRGB888) or LVDS panel
which don't have DDC support either, doing this way allow them put a
EDID property into the dc device node in the DTS. Then the entire system works.
Note those panel usually support only one display mode.

2) That is for the display controller in ls2k1000 SoC.

Currently, the upstream kernel still don't have GPIO, PWM and I2C driver support
for LS2K1000 SoC.

How dose you read EDID from the monitor without a I2C driver?

without reading EDID the device tree support, the screen just black,
the lsdc driver just stall. With reading EDID from device tree support
we do not need a i2c driver to light up the monitor.

This make lsdc drm driver work on various ls2k1000 development board
before I2C driver and GPIO driver and PWM backlight driver is upstream.

I have many local private dts with the bindings, those local change just can not
upstream at this time, below is an example.

3) Again, doing this way is for graphic environment bring up.

&lsdc {

    output-ports = <&dvo0 &dvo1>;
    #address-cells = <1>;
    #size-cells = <0>;
    dvo0: dvo@0 {
        reg = <0>;

        connector = "dpi-connector";
        encoder = "none";
        status = "ok";

        display-timings {
            native-mode = <&mode_0_1024x600_60>;

            mode_0_1024x600_60: panel-timing@0 {
                clock-frequency = <51200000>;
                hactive = <1024>;
                vactive = <600>;
                hsync-len = <4>;
                hfront-porch = <160>;
                hback-porch = <156>;
                vfront-porch = <11>;
                vback-porch = <23>;
                vsync-len = <1>;
            };

            mode_1_800x480_60: panel-timing@1 {
                clock-frequency = <30066000>;
                hactive = <800>;
                vactive = <480>;
                hfront-porch = <50>;
                hback-porch = <70>;
                hsync-len = <50>;
                vback-porch = <0>;
                vfront-porch = <0>;
                vsync-len = <50>;
            };
        };
    };

    dvo1: dvo@1 {
        reg = <1>;

        connector = "hdmi-connector";
        type = "a";
        encoder = "sil9022";

        edid = [ 00 ff ff ff ff ff ff 00 1e 6d 54 5b 0b cc 04 00
             02 1c 01 03 6c 30 1b 78 ea 31 35 a5 55 4e a1 26
             0c 50 54 a5 4b 00 71 4f 81 80 95 00 b3 00 a9 c0
             81 00 81 c0 90 40 02 3a 80 18 71 38 2d 40 58 2c
             45 00 e0 0e 11 00 00 1e 00 00 00 fd 00 38 4b 1e
             53 0f 00 0a 20 20 20 20 20 20 00 00 00 fc 00 4c
             47 20 46 55 4c 4c 20 48 44 0a 20 20 00 00 00 ff
             00 38 30 32 4e 54 43 5a 39 38 33 37 39 0a 00 35 ];

        status = "ok";
    };
};


2022-02-04 00:21:21

by Sui Jingfeng

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

>> +static int lsdc_primary_plane_atomic_check(struct drm_plane *plane,
>> + struct drm_atomic_state *state)
>> +{
>> + struct drm_device *ddev = plane->dev;
>> + struct lsdc_device *ldev = to_lsdc(ddev);
>> + struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
>> + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
>> + struct drm_framebuffer *new_fb = new_plane_state->fb;
>> + struct drm_framebuffer *old_fb = old_plane_state->fb;
>> + struct drm_crtc *crtc = new_plane_state->crtc;
>> + u32 new_format = new_fb->format->format;
>> + struct drm_crtc_state *new_crtc_state;
>> + struct lsdc_crtc_state *priv_crtc_state;
>> + int ret;
>> +
>> + if (!crtc)
>> + return 0;
>> +
>> + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>> + if (WARN_ON(!new_crtc_state))
>> + return -EINVAL;
>> +
>> + priv_crtc_state = to_lsdc_crtc_state(new_crtc_state);
>> +
>> + ret = drm_atomic_helper_check_plane_state(new_plane_state,
>> + new_crtc_state,
>> + DRM_PLANE_HELPER_NO_SCALING,
>> + DRM_PLANE_HELPER_NO_SCALING,
>> + false,
>> + true);
>> + if (ret)
>> + return ret;
>> +
>> + /*
>> + * Require full modeset if enabling or disabling a plane,
>> + * or changing its position, size, depth or format.
>> + */
>> + if ((!new_fb || !old_fb ||
>> + old_plane_state->crtc_x != new_plane_state->crtc_x ||
>> + old_plane_state->crtc_y != new_plane_state->crtc_y ||
>> + old_plane_state->crtc_w != new_plane_state->crtc_w ||
>> + old_plane_state->crtc_h != new_plane_state->crtc_h ||
>> + old_fb->format->format != new_format))
>> + new_crtc_state->mode_changed = true;
>> +
>> +
>> + priv_crtc_state->pix_fmt = lsdc_primary_get_default_format(crtc);
> Storing the pixel format in the CRTC state is weird? What would happen
> if you have a primary plane and a cursor in different formats?
>
> Also, reading the default format from a register doesn't look right.
> atomic_check can occur at any time, including before a previous commit,
> or while the hardware is disabled. You should rely on either a constant
> or the previous state here.
>
Currently, private CRTC state(priv_crtc_state) is not get used by the cursor's
atomic_check() and atomic_update(). I means this is only for the primary plane.
And both and the primary and the cursor using  XRGB8888 format, what I really
want here is let the atomic_update to update the framebuffer's format, because
the firmware (pmon) of some board set the framebuffer's format as RGB565.
If the hardware's format is same with the plane state, then there is no need to
update the FB's format register, save a function call, right?

When the plane is disabled the drm core will call the atomic_disable() function, right?

I will reconsider this, thank for your advice and i will correct other problems
at the next version. Thanks for you take time review my patch again.

2022-02-04 08:30:59

by Sui Jingfeng

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


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.


2022-02-09 12:12:20

by Maxime Ripard

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

On Fri, Feb 04, 2022 at 12:29:39AM +0800, Sui Jingfeng wrote:
> > > +static int lsdc_modeset = 1;
> > > +MODULE_PARM_DESC(modeset, "Enable/disable CMA-based KMS(1 = enabled(default), 0 = disabled)");
> > > +module_param_named(modeset, lsdc_modeset, int, 0644);
> > > +
> > > +static int lsdc_cached_coherent = 1;
> > > +MODULE_PARM_DESC(cached_coherent, "uss cached coherent mapping(1 = enabled(default), 0 = disabled)");
> > > +module_param_named(cached_coherent, lsdc_cached_coherent, int, 0644);
> > > +
> > > +static int lsdc_dirty_update = -1;
> > > +MODULE_PARM_DESC(dirty_update, "enable dirty update(1 = enabled, 0 = disabled(default))");
> > > +module_param_named(dirty_update, lsdc_dirty_update, int, 0644);
> > > +
> > > +static int lsdc_use_vram_helper = -1;
> > > +MODULE_PARM_DESC(use_vram_helper, "use vram helper based solution(1 = enabled, 0 = disabled(default))");
> > > +module_param_named(use_vram_helper, lsdc_use_vram_helper, int, 0644);
> > > +
> > > +static int lsdc_verbose = -1;
> > > +MODULE_PARM_DESC(verbose, "Enable/disable print some key information");
> > > +module_param_named(verbose, lsdc_verbose, int, 0644);
> >
> > It's not really clear to me why you need any of those parameters. Why
> > would a user want to use a non coherent mapping for example?
> >
> Because we are Mips architecture. Paul Cercueil already explained it
> in his mmap GEM buffers cachedpatch <https://lkml.kernel.org/lkml/[email protected]/T/>. I drag part of it to here for
> convenient to reading:
>
> /Traditionally, GEM buffers are mapped write-combine. Writes to the buffer
> are accelerated, and reads are slow. Application doing lots////of
> alpha-blending paint inside shadow buffers, which is then memcpy'd////into
> the final GEM buffer.///
> "non coherent mapping" is actually cached and it is for CMA helpers
> base driver, not for VRAM helper based driver. For Loongson CPU/SoCs.
> The cache coherency is maintained by hardware, therefore there no
> need to worry about coherency problems. This is true at least for
> ls3a3000, ls3a4000 and ls3a5000.
>
> "non coherent" or "coherent" is not important here, the key point is
> that the backing memory of the framebuffer is cached with non coherent
> mapping, you don't need a shadow buffer layer when using X server's
> modesetting driver.
>
> Read and write to the framebuffer in system memory is much faster than
> read and write to the framebuffer in the VRAM.
>
> Why CMA helper based solution is faster than the VRAM based solution on Mips platform?
>
> Partly because of the CPU have L1, L2 and L3 cache, especially L3 cache
> is as large as 8MB, read and write from the cache is fast.
>
> Another reason is as Paul Cercueil said, read from VRAM with write-combine
> cache mode is slow. it is just uncache read.
> Please note that we don't have a GPU here, we are just a display controller.
>
> For the VRAM helper based driver case, the backing memory of the framebuffer
> is located at VRAM, When using X server's modesetting driver, we have to enable
> the ShadowFB option, Uncache acceleration support(at the kernel size) should
> also be enabled. Otherwise the performance of graphic application is just slow.
>
> Beside write-combine cache mode have bugs on our platform, a kernel side
> developer have disabled it. Write-combine cache mode just boil down to uncached
> now. See [1] and [2]
>
> [1]https://lkml.org/lkml/2020/8/10/255
> [2]https://lkml.kernel.org/lkml/[email protected]/T/
>
> This is the reason why we prefer CMA helper base solution with non coherent mapping,
> simply because it is fast.
>
> As far as I know, Loongson's CPU does not has the concept of write-combine,
> it only support three caching mode: uncached, cached and uncache acceleration.
> write-combine is implemented with uncache acceleration on Mips.

My point wasn't just about the VRAM vs CMA stuff, it was about why do
you need all those switches in the first place?

Take the verbose parameter for example: it's entirely redundant with the
already existing, documented, DRM logging infrastructure.

Then, you have "modeset", and I'm not sure why it's supposed to be
there, at all. This is a modesetting driver, why would I want to disable
modesetting entirely?

More fundamentally (and this extends to the CMA, caching and VRAM stuff
you explained above), why can't the driver pick the right decision all
the time and why would that be under the user control?

You were mentioning that you need to work-around MIPS memory management.
Then fine, just do that on MIPS, and don't it on the other architectures
that don't need it. There's no need for a knob.

Maxime


Attachments:
(No filename) (4.69 kB)
signature.asc (235.00 B)
Download all attachments

2022-02-09 13:10:55

by Maxime Ripard

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

On Fri, Feb 04, 2022 at 12:04:19AM +0800, Sui Jingfeng wrote:
> > > +/* Get the simple EDID data from the device tree
> > > + * the length must be EDID_LENGTH, since it is simple.
> > > + *
> > > + * @np: device node contain edid data
> > > + * @edid_data: where the edid data to store to
> > > + */
> > > +static bool lsdc_get_edid_from_dtb(struct device_node *np,
> > > + unsigned char *edid_data)
> > > +{
> > > + int length;
> > > + const void *prop;
> > > +
> > > + if (np == NULL)
> > > + return false;
> > > +
> > > + prop = of_get_property(np, "edid", &length);
> > > + if (prop && (length == EDID_LENGTH)) {
> > > + memcpy(edid_data, prop, EDID_LENGTH);
> > > + return true;
> > > + }
> > > +
> > > + return false;
> > > +}
> >
> > You don't have a device tree binding for that driver, this is something
> > that is required. And it's not clear to me why you'd want EDID in the
> > DTB?
>
> 1) It is left to the end user of this driver.
>
> The downstream motherboard maker may use a dpi(XRGB888) or LVDS panel
> which don't have DDC support either, doing this way allow them put a
> EDID property into the dc device node in the DTS. Then the entire system works.
> Note those panel usually support only one display mode.

I guess it depends on what we mean exactly by the user, but the DTB
usually isn't under the (end) user control. And the drm.edid_firmware is
here already to address exactly this issue.

On the other end, if the board has a static panel without any DDC lines,
then just put the timings in the device tree, there's no need for an
EDID blob.

> 2) That is for the display controller in ls2k1000 SoC.
>
> Currently, the upstream kernel still don't have GPIO, PWM and I2C driver support
> for LS2K1000 SoC.
>
> How dose you read EDID from the monitor without a I2C driver?
>
> without reading EDID the device tree support, the screen just black,
> the lsdc driver just stall.?With reading EDID from device tree support
> we do not need?a i2c driver to light up the monitor.
>
> This make lsdc drm driver work on various ls2k1000 development board
> before I2C driver and GPIO driver and PWM backlight driver is upstream.
>
> I have many local private dts with the bindings,?those local change just can not
> upstream at this time, below is an example.

The device tree is a platform description language. It's there to let
the OS know what the hardware is, but the state of hardware support in
the said OS isn't a parameter we have to take into account for a new
binding.

If you don't have any DDC support at the moment, use the firmware
mechanism above, or add fixed modes using drm_add_modes_noedid in the
driver, and leave the DT out of it. Once you'll gain support for the
EDID readout in the driver, then it'll just work and you won't need to
change the DT again.

> 3) Again, doing this way is for graphic environment bring up.
>
> &lsdc {
>
> ??? output-ports = <&dvo0 &dvo1>;
> ?? ?#address-cells = <1>;
> ?? ?#size-cells = <0>;
> ?? ?dvo0: dvo@0 {
> ?? ???? reg = <0>;
>
> ?? ???? connector = "dpi-connector";
> ?? ???? encoder = "none";
> ?? ???? status = "ok";
>
> ?? ???? display-timings {
> ?? ???? ??? native-mode = <&mode_0_1024x600_60>;
>
> ?? ???? ??? mode_0_1024x600_60: panel-timing@0 {
> ?? ???? ??? ??? clock-frequency = <51200000>;
> ?? ???? ??? ??? hactive = <1024>;
> ?? ???? ??? ??? vactive = <600>;
> ?? ???? ??? ??? hsync-len = <4>;
> ?? ???? ??? ??? hfront-porch = <160>;
> ?? ???? ??? ??? hback-porch = <156>;
> ?? ???? ??? ??? vfront-porch = <11>;
> ?? ???? ??? ??? vback-porch = <23>;
> ?? ???? ??? ??? vsync-len = <1>;
> ?? ???? ??? };
>
> ?? ???? ??? mode_1_800x480_60: panel-timing@1 {
> ?? ???? ??? ??? clock-frequency = <30066000>;
> ?? ???? ??? ??? hactive = <800>;
> ?? ???? ??? ??? vactive = <480>;
> ?? ???? ??? ??? hfront-porch = <50>;
> ?? ???? ??? ??? hback-porch = <70>;
> ?? ???? ??? ??? hsync-len = <50>;
> ?? ???? ??? ??? vback-porch = <0>;
> ?? ???? ??? ??? vfront-porch = <0>;
> ?? ???? ??? ??? vsync-len = <50>;
> ?? ???? ??? };
> ?? ???? };
> ?? ?};
>
> ?? ?dvo1: dvo@1 {
> ?? ???? reg = <1>;
>
> ?? ???? connector = "hdmi-connector";
> ?? ???? type = "a";
> ?? ???? encoder = "sil9022";
>
> ?? ???? edid = [ 00 ff ff ff ff ff ff 00 1e 6d 54 5b 0b cc 04 00
> ?? ???? ??? ?02 1c 01 03 6c 30 1b 78 ea 31 35 a5 55 4e a1 26
> ?? ???? ??? ?0c 50 54 a5 4b 00 71 4f 81 80 95 00 b3 00 a9 c0
> ?? ???? ??? ?81 00 81 c0 90 40 02 3a 80 18 71 38 2d 40 58 2c
> ?? ???? ??? ?45 00 e0 0e 11 00 00 1e 00 00 00 fd 00 38 4b 1e
> ?? ???? ??? ?53 0f 00 0a 20 20 20 20 20 20 00 00 00 fc 00 4c
> ?? ???? ??? ?47 20 46 55 4c 4c 20 48 44 0a 20 20 00 00 00 ff
> ?? ???? ??? ?00 38 30 32 4e 54 43 5a 39 38 33 37 39 0a 00 35 ];
>
> ?? ???? status = "ok";
> ?? ?};
> };

Yeah, this needs to be documented with a YAML schema

Maxime


Attachments:
(No filename) (4.83 kB)
signature.asc (235.00 B)
Download all attachments

2022-02-09 13:57:46

by Maxime Ripard

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

On Fri, Feb 04, 2022 at 12:41:37AM +0800, Sui Jingfeng wrote:
> > > +static int lsdc_primary_plane_atomic_check(struct drm_plane *plane,
> > > + struct drm_atomic_state *state)
> > > +{
> > > + struct drm_device *ddev = plane->dev;
> > > + struct lsdc_device *ldev = to_lsdc(ddev);
> > > + struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> > > + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> > > + struct drm_framebuffer *new_fb = new_plane_state->fb;
> > > + struct drm_framebuffer *old_fb = old_plane_state->fb;
> > > + struct drm_crtc *crtc = new_plane_state->crtc;
> > > + u32 new_format = new_fb->format->format;
> > > + struct drm_crtc_state *new_crtc_state;
> > > + struct lsdc_crtc_state *priv_crtc_state;
> > > + int ret;
> > > +
> > > + if (!crtc)
> > > + return 0;
> > > +
> > > + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> > > + if (WARN_ON(!new_crtc_state))
> > > + return -EINVAL;
> > > +
> > > + priv_crtc_state = to_lsdc_crtc_state(new_crtc_state);
> > > +
> > > + ret = drm_atomic_helper_check_plane_state(new_plane_state,
> > > + new_crtc_state,
> > > + DRM_PLANE_HELPER_NO_SCALING,
> > > + DRM_PLANE_HELPER_NO_SCALING,
> > > + false,
> > > + true);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /*
> > > + * Require full modeset if enabling or disabling a plane,
> > > + * or changing its position, size, depth or format.
> > > + */
> > > + if ((!new_fb || !old_fb ||
> > > + old_plane_state->crtc_x != new_plane_state->crtc_x ||
> > > + old_plane_state->crtc_y != new_plane_state->crtc_y ||
> > > + old_plane_state->crtc_w != new_plane_state->crtc_w ||
> > > + old_plane_state->crtc_h != new_plane_state->crtc_h ||
> > > + old_fb->format->format != new_format))
> > > + new_crtc_state->mode_changed = true;
> > > +
> > > +
> > > + priv_crtc_state->pix_fmt = lsdc_primary_get_default_format(crtc);
> > Storing the pixel format in the CRTC state is weird? What would happen
> > if you have a primary plane and a cursor in different formats?
> >
> > Also, reading the default format from a register doesn't look right.
> > atomic_check can occur at any time, including before a previous commit,
> > or while the hardware is disabled. You should rely on either a constant
> > or the previous state here.
> >
> Currently, private CRTC state(priv_crtc_state) is not get used by the cursor's
> atomic_check() and atomic_update(). I means this is only for the primary plane.
> And both and the primary and the cursor using? XRGB8888 format, what I really
> want?here is let the atomic_update to update the framebuffer's format, because
> the firmware (pmon) of some board set the framebuffer's format as RGB565.

atomic_update will be called each time the plane state is changed, so it
won't be an issue: when the first state will be committed, your
atomic_update function will be called and thus you'll overwrite what was
left of the firmware setup.

> If the hardware's format is same with the plane state, then there is no need to
> update the FB's format register, save a function call, right?

My point was more about the fact that you're using the wrong abstraction
there. The format is a property of the plane, not from the CRTC. In KMS
(and in most drivers), you can have multiple planes with different
formats all attached to the same CRTC just fine.

Maxime


Attachments:
(No filename) (3.46 kB)
signature.asc (235.00 B)
Download all attachments

2022-02-09 13:59:24

by Maxime Ripard

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

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


Attachments:
(No filename) (3.75 kB)
signature.asc (235.00 B)
Download all attachments

2022-02-09 14:53:24

by Sui Jingfeng

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


On 2022/2/9 16:49, Maxime Ripard wrote:
> On Fri, Feb 04, 2022 at 12:04:19AM +0800, Sui Jingfeng wrote:
>>>> +/* Get the simple EDID data from the device tree
>>>> + * the length must be EDID_LENGTH, since it is simple.
>>>> + *
>>>> + * @np: device node contain edid data
>>>> + * @edid_data: where the edid data to store to
>>>> + */
>>>> +static bool lsdc_get_edid_from_dtb(struct device_node *np,
>>>> + unsigned char *edid_data)
>>>> +{
>>>> + int length;
>>>> + const void *prop;
>>>> +
>>>> + if (np == NULL)
>>>> + return false;
>>>> +
>>>> + prop = of_get_property(np, "edid", &length);
>>>> + if (prop && (length == EDID_LENGTH)) {
>>>> + memcpy(edid_data, prop, EDID_LENGTH);
>>>> + return true;
>>>> + }
>>>> +
>>>> + return false;
>>>> +}
>>> You don't have a device tree binding for that driver, this is something
>>> that is required. And it's not clear to me why you'd want EDID in the
>>> DTB?
>> 1) It is left to the end user of this driver.
>>
>> The downstream motherboard maker may use a dpi(XRGB888) or LVDS panel
>> which don't have DDC support either, doing this way allow them put a
>> EDID property into the dc device node in the DTS. Then the entire system works.
>> Note those panel usually support only one display mode.
> I guess it depends on what we mean exactly by the user, but the DTB
> usually isn't under the (end) user control. And the drm.edid_firmware is
> here already to address exactly this issue.
>
> On the other end, if the board has a static panel without any DDC lines,
> then just put the timings in the device tree, there's no need for an
> EDID blob.

Loongson have a long history of using PMON firmware, The PMON firmware
support flush the dtb into the the firmware before grub loading the kernel.
You press 'c' key, then the PMON will give you a shell. it is much like a
UEFI shell. Suppose foo.dtb is what you want to pass to the vmlinuz.
Then type the follow single command can flush the dtb into the PMON firmware.

|load_dtb /dev/fs/fat@usb0/foo.dtb|

For our PMON firmware, it**is** totally under developer/pc board maker's control.
You can flush whatever dtb every time you bootup until you satisfied.
It(the pmon firmware) is designed to let downstream motherboard maker and/or
customers to play easily.

Support of reading EDID from the dtb is really a feature which downstream
motherboard maker or customer wanted. They sometimes using eDP also whose
resolution is not 1024x768. This is out of control for a graphic driver
developer like me. And drm.edid_firmware have only a few limited resolution
which is weak.

I will consider to adding drm.edid_firmware support, thanks.

>> 2) That is for the display controller in ls2k1000 SoC.
>>
>> Currently, the upstream kernel still don't have GPIO, PWM and I2C driver support
>> for LS2K1000 SoC.
>>
>> How dose you read EDID from the monitor without a I2C driver?
>>
>> without reading EDID the device tree support, the screen just black,
>> the lsdc driver just stall. With reading EDID from device tree support
>> we do not need a i2c driver to light up the monitor.
>>
>> This make lsdc drm driver work on various ls2k1000 development board
>> before I2C driver and GPIO driver and PWM backlight driver is upstream.
>>
>> I have many local private dts with the bindings, those local change just can not
>> upstream at this time, below is an example.
>>
>> The device tree is a platform description language. It's there to let
>> the OS know what the hardware is, but the state of hardware support in
>> the said OS isn't a parameter we have to take into account for a new
>> binding.
>>
>> If you don't have any DDC support at the moment, use the firmware
>> mechanism above, or add fixed modes using drm_add_modes_noedid in the
>> driver, and leave the DT out of it. Once you'll gain support for the
>> EDID readout in the driver, then it'll just work and you won't need to
>> change the DT again.
>>
The resolution will be 1024x768, it will also add a lot modes which may
not supported by the specific panel. Take 1024x600 as an example,
Both drm_add_modes_noedid() and firmware mechanism above will fail.

Because the user supply EDID only and manufacturer of some strange panel
supply EDID only.

>> 3) Again, doing this way is for graphic environment bring up.
>>
>> &lsdc {
>>
>>     output-ports = <&dvo0 &dvo1>;
>>     #address-cells = <1>;
>>     #size-cells = <0>;
>>     dvo0: dvo@0 {
>>         reg = <0>;
>>
>>         connector = "dpi-connector";
>>         encoder = "none";
>>         status = "ok";
>>
>>         display-timings {
>>             native-mode = <&mode_0_1024x600_60>;
>>
>>             mode_0_1024x600_60: panel-timing@0 {
>>                 clock-frequency = <51200000>;
>>                 hactive = <1024>;
>>                 vactive = <600>;
>>                 hsync-len = <4>;
>>                 hfront-porch = <160>;
>>                 hback-porch = <156>;
>>                 vfront-porch = <11>;
>>                 vback-porch = <23>;
>>                 vsync-len = <1>;
>>             };
>>
>>             mode_1_800x480_60: panel-timing@1 {
>>                 clock-frequency = <30066000>;
>>                 hactive = <800>;
>>                 vactive = <480>;
>>                 hfront-porch = <50>;
>>                 hback-porch = <70>;
>>                 hsync-len = <50>;
>>                 vback-porch = <0>;
>>                 vfront-porch = <0>;
>>                 vsync-len = <50>;
>>             };
>>         };
>>     };
>>
>>     dvo1: dvo@1 {
>>         reg = <1>;
>>
>>         connector = "hdmi-connector";
>>         type = "a";
>>         encoder = "sil9022";
>>
>>         edid = [ 00 ff ff ff ff ff ff 00 1e 6d 54 5b 0b cc 04 00
>>              02 1c 01 03 6c 30 1b 78 ea 31 35 a5 55 4e a1 26
>>              0c 50 54 a5 4b 00 71 4f 81 80 95 00 b3 00 a9 c0
>>              81 00 81 c0 90 40 02 3a 80 18 71 38 2d 40 58 2c
>>              45 00 e0 0e 11 00 00 1e 00 00 00 fd 00 38 4b 1e
>>              53 0f 00 0a 20 20 20 20 20 20 00 00 00 fc 00 4c
>>              47 20 46 55 4c 4c 20 48 44 0a 20 20 00 00 00 ff
>>              00 38 30 32 4e 54 43 5a 39 38 33 37 39 0a 00 35 ];
>>
>>         status = "ok";
>>     };
>> };
> Yeah, this needs to be documented with a YAML schema
>
> Maxime
Yes, It takes time to learn that.

2022-02-09 18:52:32

by Sui Jingfeng

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


On 2022/2/9 16:43, Maxime Ripard wrote:
> On Fri, Feb 04, 2022 at 12:29:39AM +0800, Sui Jingfeng wrote:
>>>> +static int lsdc_modeset = 1;
>>>> +MODULE_PARM_DESC(modeset, "Enable/disable CMA-based KMS(1 = enabled(default), 0 = disabled)");
>>>> +module_param_named(modeset, lsdc_modeset, int, 0644);
>>>> +
>>>> +static int lsdc_cached_coherent = 1;
>>>> +MODULE_PARM_DESC(cached_coherent, "uss cached coherent mapping(1 = enabled(default), 0 = disabled)");
>>>> +module_param_named(cached_coherent, lsdc_cached_coherent, int, 0644);
>>>> +
>>>> +static int lsdc_dirty_update = -1;
>>>> +MODULE_PARM_DESC(dirty_update, "enable dirty update(1 = enabled, 0 = disabled(default))");
>>>> +module_param_named(dirty_update, lsdc_dirty_update, int, 0644);
>>>> +
>>>> +static int lsdc_use_vram_helper = -1;
>>>> +MODULE_PARM_DESC(use_vram_helper, "use vram helper based solution(1 = enabled, 0 = disabled(default))");
>>>> +module_param_named(use_vram_helper, lsdc_use_vram_helper, int, 0644);
>>>> +
>>>> +static int lsdc_verbose = -1;
>>>> +MODULE_PARM_DESC(verbose, "Enable/disable print some key information");
>>>> +module_param_named(verbose, lsdc_verbose, int, 0644);
>>> It's not really clear to me why you need any of those parameters. Why
>>> would a user want to use a non coherent mapping for example?
>>>
>> Because we are Mips architecture. Paul Cercueil already explained it
>> in his mmap GEM buffers cachedpatch <https://lkml.kernel.org/lkml/[email protected]/T/>. I drag part of it to here for
>> convenient to reading:
>>
>> /Traditionally, GEM buffers are mapped write-combine. Writes to the buffer
>> are accelerated, and reads are slow. Application doing lots////of
>> alpha-blending paint inside shadow buffers, which is then memcpy'd////into
>> the final GEM buffer.///
>> "non coherent mapping" is actually cached and it is for CMA helpers
>> base driver, not for VRAM helper based driver. For Loongson CPU/SoCs.
>> The cache coherency is maintained by hardware, therefore there no
>> need to worry about coherency problems. This is true at least for
>> ls3a3000, ls3a4000 and ls3a5000.
>>
>> "non coherent" or "coherent" is not important here, the key point is
>> that the backing memory of the framebuffer is cached with non coherent
>> mapping, you don't need a shadow buffer layer when using X server's
>> modesetting driver.
>>
>> Read and write to the framebuffer in system memory is much faster than
>> read and write to the framebuffer in the VRAM.
>>
>> Why CMA helper based solution is faster than the VRAM based solution on Mips platform?
>>
>> Partly because of the CPU have L1, L2 and L3 cache, especially L3 cache
>> is as large as 8MB, read and write from the cache is fast.
>>
>> Another reason is as Paul Cercueil said, read from VRAM with write-combine
>> cache mode is slow. it is just uncache read.
>> Please note that we don't have a GPU here, we are just a display controller.
>>
>> For the VRAM helper based driver case, the backing memory of the framebuffer
>> is located at VRAM, When using X server's modesetting driver, we have to enable
>> the ShadowFB option, Uncache acceleration support(at the kernel size) should
>> also be enabled. Otherwise the performance of graphic application is just slow.
>>
>> Beside write-combine cache mode have bugs on our platform, a kernel side
>> developer have disabled it. Write-combine cache mode just boil down to uncached
>> now. See [1] and [2]
>>
>> [1]https://lkml.org/lkml/2020/8/10/255
>> [2]https://lkml.kernel.org/lkml/[email protected]/T/
>>
>> This is the reason why we prefer CMA helper base solution with non coherent mapping,
>> simply because it is fast.
>>
>> As far as I know, Loongson's CPU does not has the concept of write-combine,
>> it only support three caching mode: uncached, cached and uncache acceleration.
>> write-combine is implemented with uncache acceleration on Mips.
> My point wasn't just about the VRAM vs CMA stuff, it was about why do
> you need all those switches in the first place?
>
> Take the verbose parameter for example: it's entirely redundant with the
> already existing, documented, DRM logging infrastructure.

Yes, verbose is redundant, we will use drm_dbg() instead of verbose. 
thanks.

I am correcting.

> Then, you have "modeset", and I'm not sure why it's supposed to be
> there, at all. This is a modesetting driver, why would I want to disable
> modesetting entirely?

Something you want fbdev driver, for example simple-framebuffer driver which
using the firmware passed fb (screeninfo).

besides, text mode support.

> More fundamentally (and this extends to the CMA, caching and VRAM stuff
> you explained above), why can't the driver pick the right decision all
> the time and why would that be under the user control?

The right decision for ls7a1000 is to use VRAM based helper, But sometimes
we need CMA helper based solution. Because: The PRIME support is lost, use
lsdc with etnaviv is not possible any more.

Buffer sharing with etnaviv is no longer possible, loongson display controllers
are simple which require scanout buffers to be physically contiguous.

We still need to develop userspace driver(say xf86-video-loongson)
on ls3a4000 + ls7a1000 platform then deploy those driver to ls2k1000 platform.

ls3a4000 and ls3a5000 is fast enough which can build the linux kernel directly.
Build mesa, libdrm, xf86-video-loongson just a piece of cake.

Is is so fast on ls3a5000+ls7a1000, developing driver on ls2k1000 is cumbersome,
embarrassing slow.

I means it(ls3a4000/ls3a5000 + ls7a1000) is not just target for end user, but
it is a platform which can be used to develop software for other platform.

The author of linux device driver told us that device driver is providing mechanism, not policy.

We are able to make the decision, but we want to give the user more choice.

"pick the right decision all the time" is also true, i am considering correct this,
thanks.

> You were mentioning that you need to work-around MIPS memory management.
> Then fine, just do that on MIPS, and don't it on the other architectures
> that don't need it. There's no need for a knob.
>
> Maxime

2022-02-09 18:55:37

by Maxime Ripard

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

On Wed, Feb 09, 2022 at 10:38:41PM +0800, Sui Jingfeng wrote:
> On 2022/2/9 16:49, Maxime Ripard wrote:
> > On Fri, Feb 04, 2022 at 12:04:19AM +0800, Sui Jingfeng wrote:
> > > > > +/* Get the simple EDID data from the device tree
> > > > > + * the length must be EDID_LENGTH, since it is simple.
> > > > > + *
> > > > > + * @np: device node contain edid data
> > > > > + * @edid_data: where the edid data to store to
> > > > > + */
> > > > > +static bool lsdc_get_edid_from_dtb(struct device_node *np,
> > > > > + unsigned char *edid_data)
> > > > > +{
> > > > > + int length;
> > > > > + const void *prop;
> > > > > +
> > > > > + if (np == NULL)
> > > > > + return false;
> > > > > +
> > > > > + prop = of_get_property(np, "edid", &length);
> > > > > + if (prop && (length == EDID_LENGTH)) {
> > > > > + memcpy(edid_data, prop, EDID_LENGTH);
> > > > > + return true;
> > > > > + }
> > > > > +
> > > > > + return false;
> > > > > +}
> > > > You don't have a device tree binding for that driver, this is something
> > > > that is required. And it's not clear to me why you'd want EDID in the
> > > > DTB?
> > > 1) It is left to the end user of this driver.
> > >
> > > The downstream motherboard maker may use a dpi(XRGB888) or LVDS panel
> > > which don't have DDC support either, doing this way allow them put a
> > > EDID property into the dc device node in the DTS. Then the entire system works.
> > > Note those panel usually support only one display mode.
> > I guess it depends on what we mean exactly by the user, but the DTB
> > usually isn't under the (end) user control. And the drm.edid_firmware is
> > here already to address exactly this issue.
> >
> > On the other end, if the board has a static panel without any DDC lines,
> > then just put the timings in the device tree, there's no need for an
> > EDID blob.
>
> Loongson have a long history of using PMON firmware, The PMON firmware
> support flush the dtb into the the firmware before grub loading the kernel.
> You press 'c' key, then the PMON will give you a shell. it is much like a
> UEFI shell. Suppose foo.dtb is what you want to pass to the vmlinuz.
> Then type the follow single command can flush the dtb into the PMON firmware.
>
> |load_dtb /dev/fs/fat@usb0/foo.dtb|
>
> For our PMON firmware, it**is** totally under developer/pc board maker's control.
> You can flush whatever dtb every time you bootup until you satisfied.
> It(the pmon firmware) is designed to let downstream motherboard maker and/or
> customers to play easily.
>
> Support of reading EDID from the dtb is really a feature which downstream
> motherboard maker or customer wanted. They sometimes using eDP also whose
> resolution is not 1024x768. This is out of control for a graphic driver
> developer like me.

And, to reinstate, we already have a mechanism to set an EDID, and if it
wasn't an option, the DT is not the place to store an EDID blob.

> And drm.edid_firmware have only a few limited resolution which is weak.

You're wrong. There's no limitation, it's just as limited as your
solution. You put the same thing, you get the same thing out of it. The
only difference is where the data are coming from.

> I will consider to adding drm.edid_firmware support, thanks.

It just works if you use drm_get_edid.

> > > 2) That is for the display controller in ls2k1000 SoC.
> > >
> > > Currently, the upstream kernel still don't have GPIO, PWM and I2C driver support
> > > for LS2K1000 SoC.
> > >
> > > How dose you read EDID from the monitor without a I2C driver?
> > >
> > > without reading EDID the device tree support, the screen just black,
> > > the lsdc driver just stall.?With reading EDID from device tree support
> > > we do not need?a i2c driver to light up the monitor.
> > >
> > > This make lsdc drm driver work on various ls2k1000 development board
> > > before I2C driver and GPIO driver and PWM backlight driver is upstream.
> > >
> > > I have many local private dts with the bindings,?those local change just can not
> > > upstream at this time, below is an example.
> > >
> > > The device tree is a platform description language. It's there to let
> > > the OS know what the hardware is, but the state of hardware support in
> > > the said OS isn't a parameter we have to take into account for a new
> > > binding.
> > >
> > > If you don't have any DDC support at the moment, use the firmware
> > > mechanism above, or add fixed modes using drm_add_modes_noedid in the
> > > driver, and leave the DT out of it. Once you'll gain support for the
> > > EDID readout in the driver, then it'll just work and you won't need to
> > > change the DT again.
> > >
> The resolution will be 1024x768, it will also add a lot modes which may
> not supported by the specific panel. Take 1024x600 as an example,
> Both drm_add_modes_noedid() and firmware mechanism above will fail.
>
> Because the user supply EDID only and manufacturer of some strange panel
> supply EDID only.

It's fairly easy to address: if the panel has some EDID, make the driver
able to read it; if it doesn't, describe the mode in the DT.

And if you want to be nice to your users, the firmware can even patch
the DT at boot time to add the necessary bits based on whatever info it
has, it doesn't have to be static.

Maxime


Attachments:
(No filename) (5.27 kB)
signature.asc (235.00 B)
Download all attachments

2022-02-14 09:50:44

by Sui Jingfeng

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


On 2022/2/10 00:16, Maxime Ripard wrote:
> On Wed, Feb 09, 2022 at 10:38:41PM +0800, Sui Jingfeng wrote:
>> On 2022/2/9 16:49, Maxime Ripard wrote:
>>> On Fri, Feb 04, 2022 at 12:04:19AM +0800, Sui Jingfeng wrote:
>>>>>> +/* Get the simple EDID data from the device tree
>>>>>> + * the length must be EDID_LENGTH, since it is simple.
>>>>>> + *
>>>>>> + * @np: device node contain edid data
>>>>>> + * @edid_data: where the edid data to store to
>>>>>> + */
>>>>>> +static bool lsdc_get_edid_from_dtb(struct device_node *np,
>>>>>> + unsigned char *edid_data)
>>>>>> +{
>>>>>> + int length;
>>>>>> + const void *prop;
>>>>>> +
>>>>>> + if (np == NULL)
>>>>>> + return false;
>>>>>> +
>>>>>> + prop = of_get_property(np, "edid", &length);
>>>>>> + if (prop && (length == EDID_LENGTH)) {
>>>>>> + memcpy(edid_data, prop, EDID_LENGTH);
>>>>>> + return true;
>>>>>> + }
>>>>>> +
>>>>>> + return false;
>>>>>> +}
>>>>> You don't have a device tree binding for that driver, this is something
>>>>> that is required. And it's not clear to me why you'd want EDID in the
>>>>> DTB?
>>>> 1) It is left to the end user of this driver.
>>>>
>>>> The downstream motherboard maker may use a dpi(XRGB888) or LVDS panel
>>>> which don't have DDC support either, doing this way allow them put a
>>>> EDID property into the dc device node in the DTS. Then the entire system works.
>>>> Note those panel usually support only one display mode.
>>> I guess it depends on what we mean exactly by the user, but the DTB
>>> usually isn't under the (end) user control. And the drm.edid_firmware is
>>> here already to address exactly this issue.
>>>
>>> On the other end, if the board has a static panel without any DDC lines,
>>> then just put the timings in the device tree, there's no need for an
>>> EDID blob.
>> Loongson have a long history of using PMON firmware, The PMON firmware
>> support flush the dtb into the the firmware before grub loading the kernel.
>> You press 'c' key, then the PMON will give you a shell. it is much like a
>> UEFI shell. Suppose foo.dtb is what you want to pass to the vmlinuz.
>> Then type the follow single command can flush the dtb into the PMON firmware.
>>
>> |load_dtb /dev/fs/fat@usb0/foo.dtb|
>>
>> For our PMON firmware, it**is** totally under developer/pc board maker's control.
>> You can flush whatever dtb every time you bootup until you satisfied.
>> It(the pmon firmware) is designed to let downstream motherboard maker and/or
>> customers to play easily.
>>
>> Support of reading EDID from the dtb is really a feature which downstream
>> motherboard maker or customer wanted. They sometimes using eDP also whose
>> resolution is not 1024x768. This is out of control for a graphic driver
>> developer like me.
> And, to reinstate, we already have a mechanism to set an EDID, and if it
> wasn't an option, the DT is not the place to store an EDID blob.

I know, put edid blob in the dts maybe abuse, but i am not push dts with edid blob either.

It is left to other people, and the ./arch/powerpc/boot/dts/ac14xx.dts already have edid blob.

>> And drm.edid_firmware have only a few limited resolution which is weak.
> You're wrong. There's no limitation, it's just as limited as your
> solution. You put the same thing, you get the same thing out of it. The
> only difference is where the data are coming from.

It is extremely difficult to use, it have difficulty to specify which firmware edid is for which connector.
because we have a 1024x600 panel and a 1920x1080 monitor.

It require you to know the connector's name at first, it is not as intuitive as my method.
I am exhausted by it.

>> I will consider to adding drm.edid_firmware support, thanks.
> It just works if you use drm_get_edid.
>
>>>> 2) That is for the display controller in ls2k1000 SoC.
>>>>
>>>> Currently, the upstream kernel still don't have GPIO, PWM and I2C driver support
>>>> for LS2K1000 SoC.
>>>>
>>>> How dose you read EDID from the monitor without a I2C driver?
>>>>
>>>> without reading EDID the device tree support, the screen just black,
>>>> the lsdc driver just stall. With reading EDID from device tree support
>>>> we do not need a i2c driver to light up the monitor.
>>>>
>>>> This make lsdc drm driver work on various ls2k1000 development board
>>>> before I2C driver and GPIO driver and PWM backlight driver is upstream.
>>>>
>>>> I have many local private dts with the bindings, those local change just can not
>>>> upstream at this time, below is an example.
>>>>
>>>> The device tree is a platform description language. It's there to let
>>>> the OS know what the hardware is, but the state of hardware support in
>>>> the said OS isn't a parameter we have to take into account for a new
>>>> binding.
>>>>
>>>> If you don't have any DDC support at the moment, use the firmware
>>>> mechanism above, or add fixed modes using drm_add_modes_noedid in the
>>>> driver, and leave the DT out of it. Once you'll gain support for the
>>>> EDID readout in the driver, then it'll just work and you won't need to
>>>> change the DT again.
>>>>
>> The resolution will be 1024x768, it will also add a lot modes which may
>> not supported by the specific panel. Take 1024x600 as an example,
>> Both drm_add_modes_noedid() and firmware mechanism above will fail.
>>
>> Because the user supply EDID only and manufacturer of some strange panel
>> supply EDID only.
> It's fairly easy to address: if the panel has some EDID, make the driver
> able to read it; if it doesn't, describe the mode in the DT.
>
> And if you want to be nice to your users, the firmware can even patch
> the DT at boot time to add the necessary bits based on whatever info it
> has, it doesn't have to be static.
>
> Maxime

2022-02-14 19:35:32

by Ilia Mirkin

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

On Wed, Feb 9, 2022 at 11:16 AM Maxime Ripard <[email protected]> wrote:
>
> On Wed, Feb 09, 2022 at 10:38:41PM +0800, Sui Jingfeng wrote:
> > On 2022/2/9 16:49, Maxime Ripard wrote:
> > > On Fri, Feb 04, 2022 at 12:04:19AM +0800, Sui Jingfeng wrote:
> > > > > > +/* Get the simple EDID data from the device tree
> > > > > > + * the length must be EDID_LENGTH, since it is simple.
> > > > > > + *
> > > > > > + * @np: device node contain edid data
> > > > > > + * @edid_data: where the edid data to store to
> > > > > > + */
> > > > > > +static bool lsdc_get_edid_from_dtb(struct device_node *np,
> > > > > > + unsigned char *edid_data)
> > > > > > +{
> > > > > > + int length;
> > > > > > + const void *prop;
> > > > > > +
> > > > > > + if (np == NULL)
> > > > > > + return false;
> > > > > > +
> > > > > > + prop = of_get_property(np, "edid", &length);
> > > > > > + if (prop && (length == EDID_LENGTH)) {
> > > > > > + memcpy(edid_data, prop, EDID_LENGTH);
> > > > > > + return true;
> > > > > > + }
> > > > > > +
> > > > > > + return false;
> > > > > > +}
> > > > > You don't have a device tree binding for that driver, this is something
> > > > > that is required. And it's not clear to me why you'd want EDID in the
> > > > > DTB?
> > > > 1) It is left to the end user of this driver.
> > > >
> > > > The downstream motherboard maker may use a dpi(XRGB888) or LVDS panel
> > > > which don't have DDC support either, doing this way allow them put a
> > > > EDID property into the dc device node in the DTS. Then the entire system works.
> > > > Note those panel usually support only one display mode.
> > > I guess it depends on what we mean exactly by the user, but the DTB
> > > usually isn't under the (end) user control. And the drm.edid_firmware is
> > > here already to address exactly this issue.
> > >
> > > On the other end, if the board has a static panel without any DDC lines,
> > > then just put the timings in the device tree, there's no need for an
> > > EDID blob.
> >
> > Loongson have a long history of using PMON firmware, The PMON firmware
> > support flush the dtb into the the firmware before grub loading the kernel.
> > You press 'c' key, then the PMON will give you a shell. it is much like a
> > UEFI shell. Suppose foo.dtb is what you want to pass to the vmlinuz.
> > Then type the follow single command can flush the dtb into the PMON firmware.
> >
> > |load_dtb /dev/fs/fat@usb0/foo.dtb|
> >
> > For our PMON firmware, it**is** totally under developer/pc board maker's control.
> > You can flush whatever dtb every time you bootup until you satisfied.
> > It(the pmon firmware) is designed to let downstream motherboard maker and/or
> > customers to play easily.
> >
> > Support of reading EDID from the dtb is really a feature which downstream
> > motherboard maker or customer wanted. They sometimes using eDP also whose
> > resolution is not 1024x768. This is out of control for a graphic driver
> > developer like me.
>
> And, to reinstate, we already have a mechanism to set an EDID, and if it
> wasn't an option, the DT is not the place to store an EDID blob.

Note that it's pretty common on laptop GPUs to have the attached
panel's EDID be baked into the VBIOS or ACPI (for LVDS / eDP). The hw
drivers in question (e.g. nouveau, radeon, probably i915) know how to
retrieve it specially. I'm no DT expert, but I'd imagine that it's
meant to allow mirroring those types of configurations. Stuff like
"drm.edid_firmware" isn't really meant for system-configuration-level
things, more as an out in case something doesn't work as it should.

Cheers,

-ilia

2022-02-16 14:50:55

by Maxime Ripard

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

On Sun, Feb 13, 2022 at 02:11:30AM +0800, Sui Jingfeng wrote:
>
> On 2022/2/10 00:16, Maxime Ripard wrote:
> > On Wed, Feb 09, 2022 at 10:38:41PM +0800, Sui Jingfeng wrote:
> > > On 2022/2/9 16:49, Maxime Ripard wrote:
> > > > On Fri, Feb 04, 2022 at 12:04:19AM +0800, Sui Jingfeng wrote:
> > > > > > > +/* Get the simple EDID data from the device tree
> > > > > > > + * the length must be EDID_LENGTH, since it is simple.
> > > > > > > + *
> > > > > > > + * @np: device node contain edid data
> > > > > > > + * @edid_data: where the edid data to store to
> > > > > > > + */
> > > > > > > +static bool lsdc_get_edid_from_dtb(struct device_node *np,
> > > > > > > + unsigned char *edid_data)
> > > > > > > +{
> > > > > > > + int length;
> > > > > > > + const void *prop;
> > > > > > > +
> > > > > > > + if (np == NULL)
> > > > > > > + return false;
> > > > > > > +
> > > > > > > + prop = of_get_property(np, "edid", &length);
> > > > > > > + if (prop && (length == EDID_LENGTH)) {
> > > > > > > + memcpy(edid_data, prop, EDID_LENGTH);
> > > > > > > + return true;
> > > > > > > + }
> > > > > > > +
> > > > > > > + return false;
> > > > > > > +}
> > > > > > You don't have a device tree binding for that driver, this is something
> > > > > > that is required. And it's not clear to me why you'd want EDID in the
> > > > > > DTB?
> > > > > 1) It is left to the end user of this driver.
> > > > >
> > > > > The downstream motherboard maker may use a dpi(XRGB888) or LVDS panel
> > > > > which don't have DDC support either, doing this way allow them put a
> > > > > EDID property into the dc device node in the DTS. Then the entire system works.
> > > > > Note those panel usually support only one display mode.
> > > > I guess it depends on what we mean exactly by the user, but the DTB
> > > > usually isn't under the (end) user control. And the drm.edid_firmware is
> > > > here already to address exactly this issue.
> > > >
> > > > On the other end, if the board has a static panel without any DDC lines,
> > > > then just put the timings in the device tree, there's no need for an
> > > > EDID blob.
> > > Loongson have a long history of using PMON firmware, The PMON firmware
> > > support flush the dtb into the the firmware before grub loading the kernel.
> > > You press 'c' key, then the PMON will give you a shell. it is much like a
> > > UEFI shell. Suppose foo.dtb is what you want to pass to the vmlinuz.
> > > Then type the follow single command can flush the dtb into the PMON firmware.
> > >
> > > |load_dtb /dev/fs/fat@usb0/foo.dtb|
> > >
> > > For our PMON firmware, it**is** totally under developer/pc board maker's control.
> > > You can flush whatever dtb every time you bootup until you satisfied.
> > > It(the pmon firmware) is designed to let downstream motherboard maker and/or
> > > customers to play easily.
> > >
> > > Support of reading EDID from the dtb is really a feature which downstream
> > > motherboard maker or customer wanted. They sometimes using eDP also whose
> > > resolution is not 1024x768. This is out of control for a graphic driver
> > > developer like me.
> > And, to reinstate, we already have a mechanism to set an EDID, and if it
> > wasn't an option, the DT is not the place to store an EDID blob.
>
> I know, put edid blob in the dts maybe abuse, but i am not push dts
> with edid blob either.
>
> It is left to other people, and the
> ./arch/powerpc/boot/dts/ac14xx.dts?already have edid blob.

There's one example across the entire tree, and that's not either
documented or used by any driver. I'm not sure it was really the point
you were trying to make, but the only thing it proves from my point of
view is that you don't need it.

> > > And drm.edid_firmware have only a few limited resolution which is weak.
> > You're wrong. There's no limitation, it's just as limited as your
> > solution. You put the same thing, you get the same thing out of it. The
> > only difference is where the data are coming from.
>
> It is extremely difficult to use, it have difficulty to specify which
> firmware edid is for which connector. because we have a 1024x600 panel
> and a 1920x1080 monitor.
>
> It require you to know the connector's name at first, it is not as
> intuitive as my method. I am exhausted by it.

Then you always have the option to implement DDC support, or get your
firmware to patch the DT at boot time with the proper display timing
node. Even more so if you have a single timing to provide.

Maxime


Attachments:
(No filename) (4.47 kB)
signature.asc (235.00 B)
Download all attachments

2022-02-16 16:04:49

by Daniel Stone

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

On Wed, 9 Feb 2022 at 15:41, Sui Jingfeng <[email protected]> wrote:
> On 2022/2/9 16:43, Maxime Ripard wrote:
> > More fundamentally (and this extends to the CMA, caching and VRAM stuff
> > you explained above), why can't the driver pick the right decision all
> > the time and why would that be under the user control?
>
> The right decision for ls7a1000 is to use VRAM based helper, But sometimes
> we need CMA helper based solution. Because: The PRIME support is lost, use
> lsdc with etnaviv is not possible any more.
>
> Buffer sharing with etnaviv is no longer possible, loongson display controllers
> are simple which require scanout buffers to be physically contiguous.

Other systems have this limitation, and Mesa's 'kmsro' concept makes
this work transparently, as long as your driver can export dmabufs
when running in 'VRAM' mode.

Cheers,
Daniel

2022-02-16 18:18:17

by Sui Jingfeng

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


On 2022/2/10 00:16, Maxime Ripard wrote:
> And, to reinstate, we already have a mechanism to set an EDID, and if it
> wasn't an option, the DT is not the place to store an EDID blob.

Hi,


if DT is not the place to store EDID blob, why nvidia can do that ?

output->edid = of_get_property(output->of_node, "nvidia,edid", &size);
[1][2]


[1] ./drivers/gpu/drm/tegra/output.c

[2]
https://docs.nvidia.com/drive/active/5.0.10.3L/nvvib_docs/index.html#page/NVIDIA%20DRIVE%20Linux%20SDK%20Development%20Guide/Appendix/AppendixDeviceTree.html


2023-02-02 02:58:43

by Sui Jingfeng

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


On 2022/2/9 16:36, Maxime Ripard wrote:
> On Fri, Feb 04, 2022 at 12:41:37AM +0800, Sui Jingfeng wrote:
>>>> +static int lsdc_primary_plane_atomic_check(struct drm_plane *plane,
>>>> + struct drm_atomic_state *state)
>>>> +{
>>>> + struct drm_device *ddev = plane->dev;
>>>> + struct lsdc_device *ldev = to_lsdc(ddev);
>>>> + struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
>>>> + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
>>>> + struct drm_framebuffer *new_fb = new_plane_state->fb;
>>>> + struct drm_framebuffer *old_fb = old_plane_state->fb;
>>>> + struct drm_crtc *crtc = new_plane_state->crtc;
>>>> + u32 new_format = new_fb->format->format;
>>>> + struct drm_crtc_state *new_crtc_state;
>>>> + struct lsdc_crtc_state *priv_crtc_state;
>>>> + int ret;
>>>> +
>>>> + if (!crtc)
>>>> + return 0;
>>>> +
>>>> + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>>>> + if (WARN_ON(!new_crtc_state))
>>>> + return -EINVAL;
>>>> +
>>>> + priv_crtc_state = to_lsdc_crtc_state(new_crtc_state);
>>>> +
>>>> + ret = drm_atomic_helper_check_plane_state(new_plane_state,
>>>> + new_crtc_state,
>>>> + DRM_PLANE_HELPER_NO_SCALING,
>>>> + DRM_PLANE_HELPER_NO_SCALING,
>>>> + false,
>>>> + true);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + /*
>>>> + * Require full modeset if enabling or disabling a plane,
>>>> + * or changing its position, size, depth or format.
>>>> + */
>>>> + if ((!new_fb || !old_fb ||
>>>> + old_plane_state->crtc_x != new_plane_state->crtc_x ||
>>>> + old_plane_state->crtc_y != new_plane_state->crtc_y ||
>>>> + old_plane_state->crtc_w != new_plane_state->crtc_w ||
>>>> + old_plane_state->crtc_h != new_plane_state->crtc_h ||
>>>> + old_fb->format->format != new_format))
>>>> + new_crtc_state->mode_changed = true;
>>>> +
>>>> +
>>>> + priv_crtc_state->pix_fmt = lsdc_primary_get_default_format(crtc);
>>> Storing the pixel format in the CRTC state is weird? What would happen
>>> if you have a primary plane and a cursor in different formats?
>>>
>>> Also, reading the default format from a register doesn't look right.
>>> atomic_check can occur at any time, including before a previous commit,
>>> or while the hardware is disabled. You should rely on either a constant
>>> or the previous state here.
>>>
>> Currently, private CRTC state(priv_crtc_state) is not get used by the cursor's
>> atomic_check() and atomic_update(). I means this is only for the primary plane.
>> And both and the primary and the cursor using  XRGB8888 format, what I really
>> want here is let the atomic_update to update the framebuffer's format, because
>> the firmware (pmon) of some board set the framebuffer's format as RGB565.
> atomic_update will be called each time the plane state is changed, so it
> won't be an issue: when the first state will be committed, your
> atomic_update function will be called and thus you'll overwrite what was
> left of the firmware setup.

I am agree with you that  the format is a property of the plane, the
code using wrong abstraction.

this is because i am confused by our DC hardware at that time.
atomic_update is failed to update

the FB format because our CRTC hardware has bug. It can only success
update fb format with CRTC

reset bit set in drm_crtc_funcs.reset(). Otherwise the CRTC still using
RGB565 format to parse the

RGBX8888 frame buffer.  We finally found the hardware bug via debugfs,
just dump the register value.

we found that the fb format didn't get update. The hardware bug leading
us using the wrong abstraction.

we do not understand the DRM core fully at that time. We have revised
our patch, thank you point out that.

>> If the hardware's format is same with the plane state, then there is no need to
>> update the FB's format register, save a function call, right?
> My point was more about the fact that you're using the wrong abstraction
> there. The format is a property of the plane, not from the CRTC. In KMS
> (and in most drivers), you can have multiple planes with different
> formats all attached to the same CRTC just fine.
>
> Maxime

Our CRTC have cursor plane and primary plane, composite is doing by the
CRTC hardware automatically.

there no depth property, the cursor is always on the top of the CRTC. 
If the CRTC format is not same format

I think we should convert the format of cursor to the format of primary
plane at atomic_update, by introduce

a shadow plane. Thanks for your guiding.