linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver
       [not found] <202206191404.Z1X0BOIH-lkp@intel.com>
@ 2022-06-20  2:05 ` kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-06-20  2:05 UTC (permalink / raw)
  To: Venkateshwar Rao Gannavarapu
  Cc: kbuild-all, Laurent Pinchart, Sam Ravnborg, dri-devel, airlied,
	vgannava, Venkateshwar Rao Gannavarapu,
	Linux Kernel Mailing List

Hi Venkateshwar,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: riscv-randconfig-s031-20220619 (https://download.01.org/0day-ci/archive/20220619/202206191404.Z1X0BOIH-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 11.3.0
reproduce:
         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # apt-get install sparse
         # sparse version: v0.6.4-30-g92122700-dirty
         # https://github.com/intel-lab-lkp/linux/commit/28aa62ffdc1901029bf75961166f4ebba948b9b7
         git remote add linux-review https://github.com/intel-lab-lkp/linux
         git fetch --no-tags linux-review Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
         git checkout 28aa62ffdc1901029bf75961166f4ebba948b9b7
         # save the config file
         mkdir build_dir && cp config build_dir/.config
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <yujie.liu@intel.com>


sparse warnings: (new ones prefixed by >>)

 >> drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct xlnx_dsi *dsi @@     got void [noderef] __iomem *iomem @@
    drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse:     expected struct xlnx_dsi *dsi
    drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse:     got void [noderef] __iomem *iomem

 >> drivers/gpu/drm/xlnx/xlnx_dsi.c:256:9: sparse: sparse: statement expected after case label

vim +256 drivers/gpu/drm/xlnx/xlnx_dsi.c

28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  179
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  180  static void
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  181  xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  182  		       struct drm_bridge_state *old_bridge_state)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  183  {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  184  	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  185  	struct drm_atomic_state *state = old_bridge_state->base.state;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  186  	struct drm_connector *connector;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  187  	struct drm_crtc *crtc;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  188  	const struct drm_crtc_state *crtc_state;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  189  	const struct drm_display_mode *mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  190  	u32 reg, video_mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  191
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  192  	connector = drm_atomic_get_new_connector_for_encoder(state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  193  							     bridge->encoder);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  194  	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  195  	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  196  	mode = &crtc_state->adjusted_mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  197
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  198  	reg = xlnx_dsi_read(dsi, XDSI_PCR);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  199  	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  200
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  201  	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  202  		reg = XDSI_TIME1_HSA(mode->hsync_end -
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  203  				     mode->hsync_start);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  204  		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  205  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  206
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  207  	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  208  		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  209  		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  210  	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  211
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  212  	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  213  		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  214  	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  215
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  216  	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  217  		XDSI_TIME2_VACT(mode->vdisplay);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16 @218  	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  219
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  220  	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  221
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  222  	/* Enable Core */
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  223  	reg = xlnx_dsi_read(dsi, XDSI_CCR);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  224  	reg |= XDSI_CCR_COREENB;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  225  	xlnx_dsi_write(dsi, XDSI_CCR, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  226  }
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  227
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  228  #define MAX_INPUT_SEL_FORMATS   3
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  229  static u32
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  230  *xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  231  					   struct drm_bridge_state *bridge_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  232  					   struct drm_crtc_state *crtc_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  233  					   struct drm_connector_state *conn_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  234  					   u32 output_fmt,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  235  					   unsigned int *num_input_fmts)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  236  {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  237  	u32 *input_fmts;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  238  	unsigned int i = 0;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  239
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  240  	*num_input_fmts = 0;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  241  	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  242  	if (!input_fmts)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  243  		return NULL;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  244
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  245  	switch (output_fmt) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  246  	case MEDIA_BUS_FMT_FIXED:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  247  		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  248  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  249  	case MEDIA_BUS_FMT_RGB666_1X18:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  250  		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  251  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  252  	case MEDIA_BUS_FMT_RGB565_1X16:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  253  		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  254  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  255  	default: /* define */
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16 @256  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  257
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  258  	*num_input_fmts = i;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  259  	if (*num_input_fmts == 0) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  260  		kfree(input_fmts);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  261  		input_fmts = NULL;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  262  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  263
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  264  	return input_fmts;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  265  }
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  266

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver
  2022-06-16 14:17 ` [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver Venkateshwar Rao Gannavarapu
  2022-06-16 15:48   ` Randy Dunlap
  2022-06-22 22:08   ` Laurent Pinchart
@ 2022-06-26 22:44   ` kernel test robot
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-06-26 22:44 UTC (permalink / raw)
  To: Venkateshwar Rao Gannavarapu, laurent.pinchart, sam, dri-devel
  Cc: llvm, kbuild-all, airlied, vgannava,
	Venkateshwar Rao Gannavarapu, linux-kernel

Hi Venkateshwar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on linus/master v5.19-rc3 next-20220624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: mips-randconfig-c004-20220626
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project b0d6dd3905db145853c7c744ac92d49b00b1fa20)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/28aa62ffdc1901029bf75961166f4ebba948b9b7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
        git checkout 28aa62ffdc1901029bf75961166f4ebba948b9b7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/gpu/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/xlnx/xlnx_dsi.c:255:10: error: label at end of compound statement: expected statement
           default: /* define */
                   ^
                    ;
   1 error generated.


vim +255 drivers/gpu/drm/xlnx/xlnx_dsi.c

   227	
   228	#define MAX_INPUT_SEL_FORMATS   3
   229	static u32
   230	*xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
   231						   struct drm_bridge_state *bridge_state,
   232						   struct drm_crtc_state *crtc_state,
   233						   struct drm_connector_state *conn_state,
   234						   u32 output_fmt,
   235						   unsigned int *num_input_fmts)
   236	{
   237		u32 *input_fmts;
   238		unsigned int i = 0;
   239	
   240		*num_input_fmts = 0;
   241		input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
   242		if (!input_fmts)
   243			return NULL;
   244	
   245		switch (output_fmt) {
   246		case MEDIA_BUS_FMT_FIXED:
   247			input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
   248			break;
   249		case MEDIA_BUS_FMT_RGB666_1X18:
   250			input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
   251			break;
   252		case MEDIA_BUS_FMT_RGB565_1X16:
   253			input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
   254			break;
 > 255		default: /* define */
   256		}
   257	
   258		*num_input_fmts = i;
   259		if (*num_input_fmts == 0) {
   260			kfree(input_fmts);
   261			input_fmts = NULL;
   262		}
   263	
   264		return input_fmts;
   265	}
   266	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver
  2022-06-16 14:17 ` [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver Venkateshwar Rao Gannavarapu
  2022-06-16 15:48   ` Randy Dunlap
@ 2022-06-22 22:08   ` Laurent Pinchart
  2022-06-26 22:44   ` kernel test robot
  2 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2022-06-22 22:08 UTC (permalink / raw)
  To: Venkateshwar Rao Gannavarapu
  Cc: sam, dri-devel, airlied, daniel, linux-kernel, vgannava

Hi GVRao,

Thank you for the patch.

On Thu, Jun 16, 2022 at 07:47:36PM +0530, Venkateshwar Rao Gannavarapu wrote:
> The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
> data from AXI-4 stream interface.
> 
> It supports upto 4 lanes, optional register interface for the DPHY
> and multiple RGB color formats.
> This is a MIPI-DSI host driver and provides DSI bus for panels.
> This driver also helps to communicate with its panel using panel
> framework.
> 
> Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> ---
>  drivers/gpu/drm/xlnx/Kconfig    |  12 ++
>  drivers/gpu/drm/xlnx/Makefile   |   1 +
>  drivers/gpu/drm/xlnx/xlnx_dsi.c | 429 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 442 insertions(+)
>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
> 
> diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
> index f9cf93c..a75bd76 100644
> --- a/drivers/gpu/drm/xlnx/Kconfig
> +++ b/drivers/gpu/drm/xlnx/Kconfig
> @@ -1,3 +1,15 @@
> +config DRM_XLNX_DSI
> +	tristate "Xilinx DRM DSI Subsystem Driver"
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	depends on DRM && OF
> +	select DRM_KMS_HELPER
> +	select DRM_MIPI_DSI
> +	select DRM_PANEL_BRIDGE

You can drop DRM_PANEL_BRIDGE, the driver doesn't need it.

> +	help
> +	  DRM bridge driver for Xilinx programmable DSI subsystem controller.
> +	  choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in

s/choose/Choose/

> +	  video pipeline.
> +
>  config DRM_ZYNQMP_DPSUB
>  	tristate "ZynqMP DisplayPort Controller Driver"
>  	depends on ARCH_ZYNQMP || COMPILE_TEST
> diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile
> index 51c24b7..f90849b 100644
> --- a/drivers/gpu/drm/xlnx/Makefile
> +++ b/drivers/gpu/drm/xlnx/Makefile
> @@ -1,2 +1,3 @@
> +obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o
>  zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o
>  obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o
> diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c
> new file mode 100644
> index 0000000..39d8947
> --- /dev/null
> +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c
> @@ -0,0 +1,429 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx FPGA MIPI DSI Tx Controller driver.
> + *
> + * Copyright (C) 2022 Xilinx, Inc.
> + *
> + * Author: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>

I'd add at least platform_device.h to avoid depending on indirect
inclusion of headers.

> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>

Not needed.

> +#include <drm/drm_print.h>
> +
> +/* DSI Tx IP registers */
> +#define XDSI_CCR			0x00
> +#define XDSI_CCR_COREENB		BIT(0)
> +#define XDSI_CCR_SOFTRST		BIT(1)
> +#define XDSI_CCR_CRREADY		BIT(2)
> +#define XDSI_CCR_CMDMODE		BIT(3)
> +#define XDSI_CCR_DFIFORST		BIT(4)
> +#define XDSI_CCR_CMDFIFORST		BIT(5)
> +#define XDSI_PCR			0x04
> +#define XDSI_PCR_LANES_MASK		3
> +#define XDSI_PCR_VIDEOMODE(x)		(((x) & 0x3) << 3)
> +#define XDSI_PCR_VIDEOMODE_MASK	GENMASK(4, 3)
> +#define XDSI_PCR_VIDEOMODE_SHIFT	3
> +#define XDSI_PCR_BLLPTYPE(x)		((x) << 5)
> +#define XDSI_PCR_BLLPMODE(x)		((x) << 6)
> +#define XDSI_PCR_PIXELFORMAT_MASK	GENMASK(12, 11)
> +#define XDSI_PCR_PIXELFORMAT_SHIFT	11
> +#define XDSI_PCR_EOTPENABLE(x)		((x) << 13)
> +#define XDSI_GIER			0x20
> +#define XDSI_ISR			0x24
> +#define XDSI_IER			0x28
> +#define XDSI_STR			0x2C
> +#define XDSI_STR_RDY_SHPKT		BIT(6)
> +#define XDSI_STR_RDY_LNGPKT		BIT(7)
> +#define XDSI_STR_DFIFO_FULL		BIT(8)
> +#define XDSI_STR_DFIFO_EMPTY		BIT(9)
> +#define XDSI_STR_WAITFR_DATA		BIT(10)
> +#define XDSI_STR_CMD_EXE_PGS		BIT(11)
> +#define XDSI_STR_CCMD_PROC		BIT(12)
> +#define XDSI_CMD			0x30
> +#define XDSI_CMD_QUEUE_PACKET(x)	((x) & GENMASK(23, 0))
> +#define XDSI_DFR			0x34
> +#define XDSI_TIME1			0x50
> +#define XDSI_TIME1_BLLP_BURST(x)	((x) & GENMASK(15, 0))
> +#define XDSI_TIME1_HSA(x)		(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_TIME2			0x54
> +#define XDSI_TIME2_VACT(x)		((x) & GENMASK(15, 0))
> +#define XDSI_TIME2_HACT(x)		(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_HACT_MULTIPLIER		GENMASK(1, 0)
> +#define XDSI_TIME3			0x58
> +#define XDSI_TIME3_HFP(x)		((x) & GENMASK(15, 0))
> +#define XDSI_TIME3_HBP(x)		(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_TIME4			0x5c
> +#define XDSI_TIME4_VFP(x)		((x) & GENMASK(7, 0))
> +#define XDSI_TIME4_VBP(x)		(((x) & GENMASK(7, 0)) << 8)
> +#define XDSI_TIME4_VSA(x)		(((x) & GENMASK(7, 0)) << 16)
> +#define XDSI_NUM_DATA_T		4
> +
> +/**
> + * struct xlnx_dsi - Xilinx DSI-TX core
> + * @bridge: DRM bridge structure
> + * @dsi_host: DSI host device
> + * @next_bridge: bridge structure
> + * @dev: device structure
> + * @clks: clock source structure
> + * @iomem: Base address of DSI subsystem
> + * @mode_flags: DSI operation mode related flags
> + * @lanes: number of active data lanes supported by DSI controller
> + * @mul_factor: multiplication factor for HACT timing
> + * @format: pixel format for video mode of DSI controller
> + * @device_found: Flag to indicate device presence
> + */
> +struct xlnx_dsi {
> +	struct drm_bridge bridge;
> +	struct mipi_dsi_host dsi_host;
> +	struct drm_bridge *next_bridge;
> +	struct device *dev;
> +	struct clk_bulk_data *clks;
> +	void __iomem *iomem;
> +	unsigned long mode_flags;
> +	u32 lanes;
> +	u32 mul_factor;
> +	enum mipi_dsi_pixel_format format;
> +	bool device_found;
> +};
> +
> +static const struct clk_bulk_data xdsi_clks[] = {
> +	{ .id = "s_axis_aclk" },
> +	{ .id = "dphy_clk_200M" },
> +};
> +
> +static inline struct xlnx_dsi *host_to_dsi(struct mipi_dsi_host *host)
> +{
> +	return container_of(host, struct xlnx_dsi, dsi_host);
> +}
> +
> +static inline struct xlnx_dsi *bridge_to_dsi(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct xlnx_dsi, bridge);
> +}
> +
> +static inline void xlnx_dsi_write(struct xlnx_dsi *dsi, int offset, u32 val)
> +{
> +	writel(val, dsi->iomem + offset);
> +}
> +
> +static inline u32 xlnx_dsi_read(struct xlnx_dsi *dsi, int offset)
> +{
> +	return readl(dsi->iomem + offset);
> +}
> +
> +static int xlnx_dsi_host_attach(struct mipi_dsi_host *host,
> +				struct mipi_dsi_device *device)
> +{
> +	struct xlnx_dsi *dsi = host_to_dsi(host);
> +	struct device *dev = host->dev;
> +
> +	dsi->mode_flags = device->mode_flags;
> +
> +	if (dsi->lanes != device->lanes) {
> +		dev_err(dsi->dev, "Mismatch of lanes. panel = %d, DSI = %d\n",

It's not necessarily a panel. You can write

		dev_err(dsi->dev, "Mismatch of lanes, host: %u != device: %u\n",
			dsi->lanes, device->lanes);

> +			device->lanes, dsi->lanes);
> +		return -EINVAL;
> +	}
> +
> +	if (dsi->format != device->format) {
> +		dev_err(dsi->dev, "Mismatch of format. panel = %d, DSI = %d\n",
> +			device->format, dsi->format);

Same here.

> +		return -EINVAL;
> +	}
> +
> +	if (!dsi->device_found) {
> +		dsi->next_bridge = devm_drm_of_get_bridge(dev,

As dev is the same as dsi->dev, I'd use dev->dsi here and drop the local
variable.

> +							  dev->of_node, 0, 0);
> +		if (IS_ERR(dsi->next_bridge))
> +			return PTR_ERR(dsi->next_bridge);
> +		drm_bridge_add(&dsi->bridge);
> +		dsi->device_found = true;

I don't think the device_found flag mechanism is right. You remove the
bridge in xlnx_dsi_host_detach(), so it should be added back here,
shouldn't it ?

The next question would then be what to do with the next bridge acquired
with devm_drm_of_get_bridge(). It looks like we have a lifetime
management issue here, and I don't think it's fair to ask you to fix the
bridge/panel core to get this driver merged, so I'm fine keeping this
for now even if it's incorrect. I'd like feedback from others though.

> +	}
> +
> +	return 0;
> +}
> +
> +static int xlnx_dsi_host_detach(struct mipi_dsi_host *host,
> +				struct mipi_dsi_device *device)
> +{
> +	struct xlnx_dsi *dsi = host_to_dsi(host);
> +
> +	drm_bridge_remove(&dsi->bridge);
> +	return 0;
> +}
> +
> +static const struct mipi_dsi_host_ops xlnx_dsi_ops = {
> +	.attach = xlnx_dsi_host_attach,
> +	.detach	= xlnx_dsi_host_detach,
> +};
> +
> +static void
> +xlnx_dsi_bridge_disable(struct drm_bridge *bridge,
> +			struct drm_bridge_state *old_bridge_state)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +	u32 reg = xlnx_dsi_read(dsi, XDSI_CCR);
> +
> +	reg &= ~XDSI_CCR_COREENB;
> +	xlnx_dsi_write(dsi, XDSI_CCR, reg);
> +}
> +
> +static void
> +xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
> +		       struct drm_bridge_state *old_bridge_state)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +	struct drm_atomic_state *state = old_bridge_state->base.state;
> +	struct drm_connector *connector;
> +	struct drm_crtc *crtc;
> +	const struct drm_crtc_state *crtc_state;
> +	const struct drm_display_mode *mode;
> +	u32 reg, video_mode;
> +
> +	connector = drm_atomic_get_new_connector_for_encoder(state,
> +							     bridge->encoder);
> +	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> +	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +	mode = &crtc_state->adjusted_mode;
> +
> +	reg = xlnx_dsi_read(dsi, XDSI_PCR);
> +	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
> +
> +	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
> +		reg = XDSI_TIME1_HSA(mode->hsync_end -
> +				     mode->hsync_start);

This holds on a single line.

> +		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
> +	}
> +
> +	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
> +		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
> +		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
> +	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
> +
> +	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
> +		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
> +	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
> +
> +	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
> +		XDSI_TIME2_VACT(mode->vdisplay);
> +	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
> +
> +	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
> +
> +	/* Enable Core */
> +	reg = xlnx_dsi_read(dsi, XDSI_CCR);
> +	reg |= XDSI_CCR_COREENB;
> +	xlnx_dsi_write(dsi, XDSI_CCR, reg);
> +}
> +
> +#define MAX_INPUT_SEL_FORMATS   3
> +static u32
> +*xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> +					   struct drm_bridge_state *bridge_state,
> +					   struct drm_crtc_state *crtc_state,
> +					   struct drm_connector_state *conn_state,
> +					   u32 output_fmt,
> +					   unsigned int *num_input_fmts)
> +{
> +	u32 *input_fmts;
> +	unsigned int i = 0;
> +
> +	*num_input_fmts = 0;
> +	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
> +	if (!input_fmts)
> +		return NULL;
> +
> +	switch (output_fmt) {
> +	case MEDIA_BUS_FMT_FIXED:
> +		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> +		break;
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
> +		break;
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
> +		break;
> +	default: /* define */
> +	}

As the cases are mutually exclusive, i will always be equal to 1. You
can drop the MAX_INPUT_SEL_FORMATS macro, the i variable, allocate a
single entry for input_fmts, and set inputs_fmts[0].

> +
> +	*num_input_fmts = i;
> +	if (*num_input_fmts == 0) {
> +		kfree(input_fmts);
> +		input_fmts = NULL;
> +	}
> +
> +	return input_fmts;
> +}
> +
> +static int xlnx_dsi_bridge_attach(struct drm_bridge *bridge,
> +				  enum drm_bridge_attach_flags flags)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +
> +	if (!dsi->next_bridge)

Can this ever happen ?

> +		return 0;
> +
> +	/* Attach the next bridge */
> +	return drm_bridge_attach(bridge->encoder, dsi->next_bridge, bridge,
> +				 flags);
> +}
> +
> +static void xlnx_dsi_bridge_detach(struct drm_bridge *bridge)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +
> +	drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);

This doesn't look right. Is it a leftover ? You should instead call
drm_bridge_detach() on the next bridge.

> +}
> +
> +static enum drm_mode_status
> +xlnx_dsi_bridge_mode_valid(struct drm_bridge *bridge,
> +			   const struct drm_display_info *info,
> +			   const struct drm_display_mode *mode)
> +{
> +	if ((mode->hdisplay & XDSI_HACT_MULTIPLIER) != 0)
> +		return MODE_BAD_WIDTH;
> +
> +	return MODE_OK;
> +}
> +
> +static const struct drm_bridge_funcs xlnx_dsi_bridge_funcs = {
> +	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
> +	.atomic_reset			= drm_atomic_helper_bridge_reset,
> +	.atomic_disable                 = xlnx_dsi_bridge_disable,
> +	.atomic_enable			= xlnx_dsi_bridge_enable,
> +	.atomic_get_input_bus_fmts      = xlnx_dsi_bridge_atomic_get_input_bus_fmts,
> +	.attach				= xlnx_dsi_bridge_attach,
> +	.detach				= xlnx_dsi_bridge_detach,
> +	.mode_valid			= xlnx_dsi_bridge_mode_valid,
> +};
> +
> +static int xlnx_dsi_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	struct xlnx_dsi *dsi;
> +	int ret;
> +	const int xdsi_mul_fact[XDSI_NUM_DATA_T] = {300, 225, 225, 200};

static const, and you can move it as the first variable of the function.

> +	u32 reg;
> +
> +	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> +	if (!dsi)
> +		return -ENOMEM;
> +
> +	dsi->dev = dev;
> +	dsi->clks = devm_kmemdup(dev, xdsi_clks, sizeof(xdsi_clks),
> +				 GFP_KERNEL);
> +	if (!dsi->clks)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	dsi->iomem = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(dsi->iomem))
> +		return PTR_ERR(dsi->iomem);
> +
> +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(xdsi_clks), dsi->clks);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(xdsi_clks), dsi->clks);
> +	if (ret)
> +		return ret;

Shouldn't this be done in the .atomic_enable() handler instead, possible
through runtime PM ?

> +
> +	platform_set_drvdata(pdev, dsi);
> +	dsi->dsi_host.ops = &xlnx_dsi_ops;
> +	dsi->dsi_host.dev = dev;
> +
> +	ret = mipi_dsi_host_register(&dsi->dsi_host);
> +	if (ret) {
> +		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
> +		goto err_clk_put;
> +	}
> +
> +	dsi->bridge.driver_private = dsi;
> +	dsi->bridge.funcs = &xlnx_dsi_bridge_funcs;
> +	dsi->bridge.of_node = pdev->dev.of_node;
> +
> +	reg = xlnx_dsi_read(dsi, XDSI_PCR);
> +	dsi->lanes = reg & XDSI_PCR_LANES_MASK;
> +	dsi->format = (reg & XDSI_PCR_PIXELFORMAT_MASK) >>
> +		XDSI_PCR_PIXELFORMAT_SHIFT;

OK, you'll need to enable clocks for this too, so runtime PM could be a
good option.

> +
> +	if (dsi->lanes > 4 || dsi->lanes < 1) {
> +		dev_err(dsi->dev, "%d invalid lanes\n",	dsi->lanes);

dsi->lanes is unsigned, so %u.

> +		return -EINVAL;

This leaves the clocks enabled.

> +	}
> +
> +	if (dsi->format > MIPI_DSI_FMT_RGB565) {
> +		dev_err(dsi->dev, "Invalid xlnx,dsi-data-type string\n");

This is read from a register, not parsed from DT, so the message isn't
valid.

> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Used as a multiplication factor for HACT based on used
> +	 * DSI data type.
> +	 *
> +	 * e.g. for RGB666_L datatype and 1920x1080 resolution,
> +	 * the Hact (WC) would be as follows -
> +	 * 1920 pixels * 18 bits per pixel / 8 bits per byte
> +	 * = 1920 pixels * 2.25 bytes per pixel = 4320 bytes.
> +	 *
> +	 * Data Type - Multiplication factor
> +	 * RGB888    - 3
> +	 * RGB666_L  - 2.25
> +-	 * RGB666_P  - 2.25
> +	 * RGB565    - 2
> +	 *
> +	 * Since the multiplication factor is a floating number,
> +	 * a 100x multiplication factor is used.
> +	 */
> +	dsi->mul_factor = xdsi_mul_fact[dsi->format];
> +
> +	dev_dbg(dsi->dev, "DSI controller num lanes = %d\n", dsi->lanes);
> +	dev_dbg(dsi->dev, "DSI controller format = %d\n", dsi->format);

You could combine both messages into a single one.

Are you missing a return 0 here ?

> +
> +err_clk_put:
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
> +
> +	return ret;
> +}
> +
> +static int xlnx_dsi_remove(struct platform_device *pdev)
> +{
> +	struct xlnx_dsi *dsi = platform_get_drvdata(pdev);
> +
> +	mipi_dsi_host_unregister(&dsi->dsi_host);
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id xlnx_dsi_of_match[] = {
> +	{ .compatible = "xlnx,dsi-tx-v2.0"},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, xlnx_dsi_of_match);
> +
> +static struct platform_driver dsi_driver = {
> +	.probe = xlnx_dsi_probe,
> +	.remove = xlnx_dsi_remove,
> +	.driver = {
> +		.name = "xlnx-dsi",
> +		.of_match_table = xlnx_dsi_of_match,
> +	},
> +};
> +
> +module_platform_driver(dsi_driver);
> +
> +MODULE_AUTHOR("Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>");
> +MODULE_DESCRIPTION("Xilinx MIPI DSI host controller driver");
> +MODULE_LICENSE("GPL");

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver
  2022-06-16 14:17 ` [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver Venkateshwar Rao Gannavarapu
@ 2022-06-16 15:48   ` Randy Dunlap
  2022-06-22 22:08   ` Laurent Pinchart
  2022-06-26 22:44   ` kernel test robot
  2 siblings, 0 replies; 5+ messages in thread
From: Randy Dunlap @ 2022-06-16 15:48 UTC (permalink / raw)
  To: Venkateshwar Rao Gannavarapu, laurent.pinchart, sam, dri-devel
  Cc: airlied, daniel, linux-kernel, vgannava



On 6/16/22 07:17, Venkateshwar Rao Gannavarapu wrote:
> diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
> index f9cf93c..a75bd76 100644
> --- a/drivers/gpu/drm/xlnx/Kconfig
> +++ b/drivers/gpu/drm/xlnx/Kconfig
> @@ -1,3 +1,15 @@
> +config DRM_XLNX_DSI
> +	tristate "Xilinx DRM DSI Subsystem Driver"
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	depends on DRM && OF
> +	select DRM_KMS_HELPER
> +	select DRM_MIPI_DSI
> +	select DRM_PANEL_BRIDGE
> +	help
> +	  DRM bridge driver for Xilinx programmable DSI subsystem controller.
> +	  choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in

	  Choose

> +	  video pipeline.

-- 
~Randy

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver
  2022-06-16 14:17 [PATCH V2 0/2] Add Xilinx DSI-Tx subsystem DRM driver Venkateshwar Rao Gannavarapu
@ 2022-06-16 14:17 ` Venkateshwar Rao Gannavarapu
  2022-06-16 15:48   ` Randy Dunlap
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Venkateshwar Rao Gannavarapu @ 2022-06-16 14:17 UTC (permalink / raw)
  To: laurent.pinchart, sam, dri-devel
  Cc: airlied, daniel, linux-kernel, vgannava, Venkateshwar Rao Gannavarapu

The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
data from AXI-4 stream interface.

It supports upto 4 lanes, optional register interface for the DPHY
and multiple RGB color formats.
This is a MIPI-DSI host driver and provides DSI bus for panels.
This driver also helps to communicate with its panel using panel
framework.

Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
---
 drivers/gpu/drm/xlnx/Kconfig    |  12 ++
 drivers/gpu/drm/xlnx/Makefile   |   1 +
 drivers/gpu/drm/xlnx/xlnx_dsi.c | 429 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 442 insertions(+)
 create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c

diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
index f9cf93c..a75bd76 100644
--- a/drivers/gpu/drm/xlnx/Kconfig
+++ b/drivers/gpu/drm/xlnx/Kconfig
@@ -1,3 +1,15 @@
+config DRM_XLNX_DSI
+	tristate "Xilinx DRM DSI Subsystem Driver"
+	depends on ARCH_ZYNQMP || COMPILE_TEST
+	depends on DRM && OF
+	select DRM_KMS_HELPER
+	select DRM_MIPI_DSI
+	select DRM_PANEL_BRIDGE
+	help
+	  DRM bridge driver for Xilinx programmable DSI subsystem controller.
+	  choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in
+	  video pipeline.
+
 config DRM_ZYNQMP_DPSUB
 	tristate "ZynqMP DisplayPort Controller Driver"
 	depends on ARCH_ZYNQMP || COMPILE_TEST
diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile
index 51c24b7..f90849b 100644
--- a/drivers/gpu/drm/xlnx/Makefile
+++ b/drivers/gpu/drm/xlnx/Makefile
@@ -1,2 +1,3 @@
+obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o
 zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o
 obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o
diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c
new file mode 100644
index 0000000..39d8947
--- /dev/null
+++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c
@@ -0,0 +1,429 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx FPGA MIPI DSI Tx Controller driver.
+ *
+ * Copyright (C) 2022 Xilinx, Inc.
+ *
+ * Author: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+
+/* DSI Tx IP registers */
+#define XDSI_CCR			0x00
+#define XDSI_CCR_COREENB		BIT(0)
+#define XDSI_CCR_SOFTRST		BIT(1)
+#define XDSI_CCR_CRREADY		BIT(2)
+#define XDSI_CCR_CMDMODE		BIT(3)
+#define XDSI_CCR_DFIFORST		BIT(4)
+#define XDSI_CCR_CMDFIFORST		BIT(5)
+#define XDSI_PCR			0x04
+#define XDSI_PCR_LANES_MASK		3
+#define XDSI_PCR_VIDEOMODE(x)		(((x) & 0x3) << 3)
+#define XDSI_PCR_VIDEOMODE_MASK	GENMASK(4, 3)
+#define XDSI_PCR_VIDEOMODE_SHIFT	3
+#define XDSI_PCR_BLLPTYPE(x)		((x) << 5)
+#define XDSI_PCR_BLLPMODE(x)		((x) << 6)
+#define XDSI_PCR_PIXELFORMAT_MASK	GENMASK(12, 11)
+#define XDSI_PCR_PIXELFORMAT_SHIFT	11
+#define XDSI_PCR_EOTPENABLE(x)		((x) << 13)
+#define XDSI_GIER			0x20
+#define XDSI_ISR			0x24
+#define XDSI_IER			0x28
+#define XDSI_STR			0x2C
+#define XDSI_STR_RDY_SHPKT		BIT(6)
+#define XDSI_STR_RDY_LNGPKT		BIT(7)
+#define XDSI_STR_DFIFO_FULL		BIT(8)
+#define XDSI_STR_DFIFO_EMPTY		BIT(9)
+#define XDSI_STR_WAITFR_DATA		BIT(10)
+#define XDSI_STR_CMD_EXE_PGS		BIT(11)
+#define XDSI_STR_CCMD_PROC		BIT(12)
+#define XDSI_CMD			0x30
+#define XDSI_CMD_QUEUE_PACKET(x)	((x) & GENMASK(23, 0))
+#define XDSI_DFR			0x34
+#define XDSI_TIME1			0x50
+#define XDSI_TIME1_BLLP_BURST(x)	((x) & GENMASK(15, 0))
+#define XDSI_TIME1_HSA(x)		(((x) & GENMASK(15, 0)) << 16)
+#define XDSI_TIME2			0x54
+#define XDSI_TIME2_VACT(x)		((x) & GENMASK(15, 0))
+#define XDSI_TIME2_HACT(x)		(((x) & GENMASK(15, 0)) << 16)
+#define XDSI_HACT_MULTIPLIER		GENMASK(1, 0)
+#define XDSI_TIME3			0x58
+#define XDSI_TIME3_HFP(x)		((x) & GENMASK(15, 0))
+#define XDSI_TIME3_HBP(x)		(((x) & GENMASK(15, 0)) << 16)
+#define XDSI_TIME4			0x5c
+#define XDSI_TIME4_VFP(x)		((x) & GENMASK(7, 0))
+#define XDSI_TIME4_VBP(x)		(((x) & GENMASK(7, 0)) << 8)
+#define XDSI_TIME4_VSA(x)		(((x) & GENMASK(7, 0)) << 16)
+#define XDSI_NUM_DATA_T		4
+
+/**
+ * struct xlnx_dsi - Xilinx DSI-TX core
+ * @bridge: DRM bridge structure
+ * @dsi_host: DSI host device
+ * @next_bridge: bridge structure
+ * @dev: device structure
+ * @clks: clock source structure
+ * @iomem: Base address of DSI subsystem
+ * @mode_flags: DSI operation mode related flags
+ * @lanes: number of active data lanes supported by DSI controller
+ * @mul_factor: multiplication factor for HACT timing
+ * @format: pixel format for video mode of DSI controller
+ * @device_found: Flag to indicate device presence
+ */
+struct xlnx_dsi {
+	struct drm_bridge bridge;
+	struct mipi_dsi_host dsi_host;
+	struct drm_bridge *next_bridge;
+	struct device *dev;
+	struct clk_bulk_data *clks;
+	void __iomem *iomem;
+	unsigned long mode_flags;
+	u32 lanes;
+	u32 mul_factor;
+	enum mipi_dsi_pixel_format format;
+	bool device_found;
+};
+
+static const struct clk_bulk_data xdsi_clks[] = {
+	{ .id = "s_axis_aclk" },
+	{ .id = "dphy_clk_200M" },
+};
+
+static inline struct xlnx_dsi *host_to_dsi(struct mipi_dsi_host *host)
+{
+	return container_of(host, struct xlnx_dsi, dsi_host);
+}
+
+static inline struct xlnx_dsi *bridge_to_dsi(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct xlnx_dsi, bridge);
+}
+
+static inline void xlnx_dsi_write(struct xlnx_dsi *dsi, int offset, u32 val)
+{
+	writel(val, dsi->iomem + offset);
+}
+
+static inline u32 xlnx_dsi_read(struct xlnx_dsi *dsi, int offset)
+{
+	return readl(dsi->iomem + offset);
+}
+
+static int xlnx_dsi_host_attach(struct mipi_dsi_host *host,
+				struct mipi_dsi_device *device)
+{
+	struct xlnx_dsi *dsi = host_to_dsi(host);
+	struct device *dev = host->dev;
+
+	dsi->mode_flags = device->mode_flags;
+
+	if (dsi->lanes != device->lanes) {
+		dev_err(dsi->dev, "Mismatch of lanes. panel = %d, DSI = %d\n",
+			device->lanes, dsi->lanes);
+		return -EINVAL;
+	}
+
+	if (dsi->format != device->format) {
+		dev_err(dsi->dev, "Mismatch of format. panel = %d, DSI = %d\n",
+			device->format, dsi->format);
+		return -EINVAL;
+	}
+
+	if (!dsi->device_found) {
+		dsi->next_bridge = devm_drm_of_get_bridge(dev,
+							  dev->of_node, 0, 0);
+		if (IS_ERR(dsi->next_bridge))
+			return PTR_ERR(dsi->next_bridge);
+		drm_bridge_add(&dsi->bridge);
+		dsi->device_found = true;
+	}
+
+	return 0;
+}
+
+static int xlnx_dsi_host_detach(struct mipi_dsi_host *host,
+				struct mipi_dsi_device *device)
+{
+	struct xlnx_dsi *dsi = host_to_dsi(host);
+
+	drm_bridge_remove(&dsi->bridge);
+	return 0;
+}
+
+static const struct mipi_dsi_host_ops xlnx_dsi_ops = {
+	.attach = xlnx_dsi_host_attach,
+	.detach	= xlnx_dsi_host_detach,
+};
+
+static void
+xlnx_dsi_bridge_disable(struct drm_bridge *bridge,
+			struct drm_bridge_state *old_bridge_state)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+	u32 reg = xlnx_dsi_read(dsi, XDSI_CCR);
+
+	reg &= ~XDSI_CCR_COREENB;
+	xlnx_dsi_write(dsi, XDSI_CCR, reg);
+}
+
+static void
+xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
+		       struct drm_bridge_state *old_bridge_state)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+	struct drm_atomic_state *state = old_bridge_state->base.state;
+	struct drm_connector *connector;
+	struct drm_crtc *crtc;
+	const struct drm_crtc_state *crtc_state;
+	const struct drm_display_mode *mode;
+	u32 reg, video_mode;
+
+	connector = drm_atomic_get_new_connector_for_encoder(state,
+							     bridge->encoder);
+	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	mode = &crtc_state->adjusted_mode;
+
+	reg = xlnx_dsi_read(dsi, XDSI_PCR);
+	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
+
+	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
+		reg = XDSI_TIME1_HSA(mode->hsync_end -
+				     mode->hsync_start);
+		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
+	}
+
+	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
+		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
+		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
+	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
+
+	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
+		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
+	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
+
+	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
+		XDSI_TIME2_VACT(mode->vdisplay);
+	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
+
+	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
+
+	/* Enable Core */
+	reg = xlnx_dsi_read(dsi, XDSI_CCR);
+	reg |= XDSI_CCR_COREENB;
+	xlnx_dsi_write(dsi, XDSI_CCR, reg);
+}
+
+#define MAX_INPUT_SEL_FORMATS   3
+static u32
+*xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+					   struct drm_bridge_state *bridge_state,
+					   struct drm_crtc_state *crtc_state,
+					   struct drm_connector_state *conn_state,
+					   u32 output_fmt,
+					   unsigned int *num_input_fmts)
+{
+	u32 *input_fmts;
+	unsigned int i = 0;
+
+	*num_input_fmts = 0;
+	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	switch (output_fmt) {
+	case MEDIA_BUS_FMT_FIXED:
+		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
+	case MEDIA_BUS_FMT_RGB666_1X18:
+		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
+		break;
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
+		break;
+	default: /* define */
+	}
+
+	*num_input_fmts = i;
+	if (*num_input_fmts == 0) {
+		kfree(input_fmts);
+		input_fmts = NULL;
+	}
+
+	return input_fmts;
+}
+
+static int xlnx_dsi_bridge_attach(struct drm_bridge *bridge,
+				  enum drm_bridge_attach_flags flags)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+
+	if (!dsi->next_bridge)
+		return 0;
+
+	/* Attach the next bridge */
+	return drm_bridge_attach(bridge->encoder, dsi->next_bridge, bridge,
+				 flags);
+}
+
+static void xlnx_dsi_bridge_detach(struct drm_bridge *bridge)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+
+	drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);
+}
+
+static enum drm_mode_status
+xlnx_dsi_bridge_mode_valid(struct drm_bridge *bridge,
+			   const struct drm_display_info *info,
+			   const struct drm_display_mode *mode)
+{
+	if ((mode->hdisplay & XDSI_HACT_MULTIPLIER) != 0)
+		return MODE_BAD_WIDTH;
+
+	return MODE_OK;
+}
+
+static const struct drm_bridge_funcs xlnx_dsi_bridge_funcs = {
+	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset			= drm_atomic_helper_bridge_reset,
+	.atomic_disable                 = xlnx_dsi_bridge_disable,
+	.atomic_enable			= xlnx_dsi_bridge_enable,
+	.atomic_get_input_bus_fmts      = xlnx_dsi_bridge_atomic_get_input_bus_fmts,
+	.attach				= xlnx_dsi_bridge_attach,
+	.detach				= xlnx_dsi_bridge_detach,
+	.mode_valid			= xlnx_dsi_bridge_mode_valid,
+};
+
+static int xlnx_dsi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct xlnx_dsi *dsi;
+	int ret;
+	const int xdsi_mul_fact[XDSI_NUM_DATA_T] = {300, 225, 225, 200};
+	u32 reg;
+
+	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
+	if (!dsi)
+		return -ENOMEM;
+
+	dsi->dev = dev;
+	dsi->clks = devm_kmemdup(dev, xdsi_clks, sizeof(xdsi_clks),
+				 GFP_KERNEL);
+	if (!dsi->clks)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dsi->iomem = devm_ioremap_resource(dev, res);
+	if (IS_ERR(dsi->iomem))
+		return PTR_ERR(dsi->iomem);
+
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(xdsi_clks), dsi->clks);
+	if (ret)
+		return ret;
+
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(xdsi_clks), dsi->clks);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, dsi);
+	dsi->dsi_host.ops = &xlnx_dsi_ops;
+	dsi->dsi_host.dev = dev;
+
+	ret = mipi_dsi_host_register(&dsi->dsi_host);
+	if (ret) {
+		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
+		goto err_clk_put;
+	}
+
+	dsi->bridge.driver_private = dsi;
+	dsi->bridge.funcs = &xlnx_dsi_bridge_funcs;
+	dsi->bridge.of_node = pdev->dev.of_node;
+
+	reg = xlnx_dsi_read(dsi, XDSI_PCR);
+	dsi->lanes = reg & XDSI_PCR_LANES_MASK;
+	dsi->format = (reg & XDSI_PCR_PIXELFORMAT_MASK) >>
+		XDSI_PCR_PIXELFORMAT_SHIFT;
+
+	if (dsi->lanes > 4 || dsi->lanes < 1) {
+		dev_err(dsi->dev, "%d invalid lanes\n",	dsi->lanes);
+		return -EINVAL;
+	}
+
+	if (dsi->format > MIPI_DSI_FMT_RGB565) {
+		dev_err(dsi->dev, "Invalid xlnx,dsi-data-type string\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Used as a multiplication factor for HACT based on used
+	 * DSI data type.
+	 *
+	 * e.g. for RGB666_L datatype and 1920x1080 resolution,
+	 * the Hact (WC) would be as follows -
+	 * 1920 pixels * 18 bits per pixel / 8 bits per byte
+	 * = 1920 pixels * 2.25 bytes per pixel = 4320 bytes.
+	 *
+	 * Data Type - Multiplication factor
+	 * RGB888    - 3
+	 * RGB666_L  - 2.25
+-	 * RGB666_P  - 2.25
+	 * RGB565    - 2
+	 *
+	 * Since the multiplication factor is a floating number,
+	 * a 100x multiplication factor is used.
+	 */
+	dsi->mul_factor = xdsi_mul_fact[dsi->format];
+
+	dev_dbg(dsi->dev, "DSI controller num lanes = %d\n", dsi->lanes);
+	dev_dbg(dsi->dev, "DSI controller format = %d\n", dsi->format);
+
+err_clk_put:
+	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
+
+	return ret;
+}
+
+static int xlnx_dsi_remove(struct platform_device *pdev)
+{
+	struct xlnx_dsi *dsi = platform_get_drvdata(pdev);
+
+	mipi_dsi_host_unregister(&dsi->dsi_host);
+	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
+
+	return 0;
+}
+
+static const struct of_device_id xlnx_dsi_of_match[] = {
+	{ .compatible = "xlnx,dsi-tx-v2.0"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, xlnx_dsi_of_match);
+
+static struct platform_driver dsi_driver = {
+	.probe = xlnx_dsi_probe,
+	.remove = xlnx_dsi_remove,
+	.driver = {
+		.name = "xlnx-dsi",
+		.of_match_table = xlnx_dsi_of_match,
+	},
+};
+
+module_platform_driver(dsi_driver);
+
+MODULE_AUTHOR("Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>");
+MODULE_DESCRIPTION("Xilinx MIPI DSI host controller driver");
+MODULE_LICENSE("GPL");
--
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-06-26 22:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <202206191404.Z1X0BOIH-lkp@intel.com>
2022-06-20  2:05 ` [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver kernel test robot
2022-06-16 14:17 [PATCH V2 0/2] Add Xilinx DSI-Tx subsystem DRM driver Venkateshwar Rao Gannavarapu
2022-06-16 14:17 ` [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver Venkateshwar Rao Gannavarapu
2022-06-16 15:48   ` Randy Dunlap
2022-06-22 22:08   ` Laurent Pinchart
2022-06-26 22:44   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).