On Wed Nov 6, 2024 at 1:33 PM CET, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@xxxxxxxxx>
>
> This adds the glue code for the MIPI DSI2 bridge on Rockchip SoCs and
> enables its use on the RK3588.
>
> Right now the DSI2 controller is always paired with a DC-phy based on a
> Samsung IP, so the interface values are set statically for now.
> This stays true for the upcoming RK3576 as well.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@xxxxxxxxx>
> ---
> drivers/gpu/drm/rockchip/Kconfig | 10 +
> drivers/gpu/drm/rockchip/Makefile | 1 +
> .../gpu/drm/rockchip/dw-mipi-dsi2-rockchip.c | 524 ++++++++++++++++++
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 +
> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 +
> 5 files changed, 538 insertions(+)
> create mode 100644 drivers/gpu/drm/rockchip/dw-mipi-dsi2-rockchip.c
>
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index 448fadd4ba15..99d4b260de85 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -10,6 +10,7 @@ config DRM_ROCKCHIP
> select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
> select DRM_DW_HDMI_QP if ROCKCHIP_DW_HDMI_QP
> select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
> + select DRM_DW_MIPI_DSI2 if ROCKCHIP_DW_MIPI_DSI2
> select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
> select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI
> select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
> @@ -81,6 +82,15 @@ config ROCKCHIP_DW_MIPI_DSI
> enable MIPI DSI on RK3288 or RK3399 based SoC, you should
> select this option.
>
> +config ROCKCHIP_DW_MIPI_DSI2
> + bool "Rockchip specific extensions for Synopsys DW MIPI DSI"
> + select GENERIC_PHY_MIPI_DPHY
> + help
> + This selects support for Rockchip SoC specific extensions
> + for the Synopsys DesignWare dsi driver. If you want to
> + enable MIPI DSI on RK3288 or RK3399 based SoC, you should
> + select this option.
The commit message and the cover letter talk about RK3588 (and RK3576),
so this seems like a copy-n-paste error (from DSI '1')?
I'd also prefer 'dsi' to be in capitals (thus 'DSI').
> +
> config ROCKCHIP_INNO_HDMI
> bool "Rockchip specific extensions for Innosilicon HDMI"
> select DRM_DISPLAY_HDMI_HELPER
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index 3eab662a5a1d..2b867cebbc12 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -13,6 +13,7 @@ rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI_QP) += dw_hdmi_qp-rockchip.o
> rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi-rockchip.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI2) += dw-mipi-dsi2-rockchip.o
> rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
> rockchipdrm-$(CONFIG_ROCKCHIP_LVDS) += rockchip_lvds.o
> rockchipdrm-$(CONFIG_ROCKCHIP_RGB) += rockchip_rgb.o
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi2-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi2-rockchip.c
> new file mode 100644
> index 000000000000..42a7a80e2138
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi2-rockchip.c
> @@ -0,0 +1,524 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2024 Rockchip Electronics Co.Ltd
> + * Author:
> + * Guochun Huang <hero.huang@xxxxxxxxxxxxxx>
> + * Heiko Stuebner <heiko.stuebner@xxxxxxxxx>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/gpio.h>
> +#include <linux/iopoll.h>
> +#include <linux/math64.h>
> +#include <linux/media-bus-format.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_platform.h>
In commit 722d4f06e560 ("drm: Explicitly include correct DT includes")
several changes were made to header includes.
One of the things I noticed is that ``of_device.h`` often was removed
and ``of.h`` added.
Thus are the above headers as they should be or should they be updated
like was done in commit 722d4f06e560?
> +#include <linux/pm_runtime.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/phy/phy.h>
> +
> +#include <drm/bridge/dw_mipi_dsi2.h>
> +
> +#include <drm/display/drm_dsc.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>
> +#include <video/mipi_display.h>
> +#include <video/videomode.h>
> +#include <drm/drm_connector.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +
> +#include <uapi/linux/videodev2.h>
> +
> +#include "rockchip_drm_drv.h"
> +#include "rockchip_drm_vop.h"
> +
> +#define PSEC_PER_SEC 1000000000000LL
> +
> +struct dsigrf_reg {
> + u16 offset;
> + u16 lsb;
> + u16 msb;
> +};
> +
> +enum grf_reg_fields {
> + TXREQCLKHS_EN,
> + GATING_EN,
> + IPI_SHUTDN,
> + IPI_COLORM,
> + IPI_COLOR_DEPTH,
> + IPI_FORMAT,
> + MAX_FIELDS,
> +};
> +
> +#define IPI_DEPTH_5_6_5_BITS 0x02
> +#define IPI_DEPTH_6_BITS 0x03
> +#define IPI_DEPTH_8_BITS 0x05
> +#define IPI_DEPTH_10_BITS 0x06
Possibly dumb remark (sorry):
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c also defines these
values, so wouldn't it be better if they're defined in 1 place?
Cheers,
Diederik
Attachment:
signature.asc
Description: PGP signature
