[PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Laurent Pinchart]

Hello,

This patch series is a second, extended version of the code previously posted
as "[PATCH/RFC 0/7] Remove the omapdrm device from platform code".

The omapdss/omapdrm initialization code is quite a mess. The physical devices
are instantiated from DT, but two virtual devices named omapdrm and omapdss
are instanciated from platform code to pass various pieces of platform data to
the drivers.

The omapdrm and omapdss platform devices are currently used to pass data and
function pointers from board code to the drivers for the purpose of

- identifying the OMAP SoC revision
- controlling the DSI pads
- configuring bus throughput

It turns out that all these can be handled in the omapdrm and omapdss drivers
without the need for platform data.

- The SoC revision is used to identify the version of various DSS IP cores,
which can instead be done through compatible string matching (with the help of
soc_device_match() in two cases where ES version is needed).

- The DSI pads control can be performed by the driver directly without calling
board code, accessing the related registers through syscon.

- Bus throughput control is implemented in mach-omap2 as a no-op, so we can
just drop the code.

This patch series starts with a few small cleanups and unused features removal
(01/28 to 04/28). It then slowly replaces all uses of the omapdrm and omapdss
platform data as explained above (05/28 to 20/28).

The next step is to remove the omapdss platform driver (for the virtual
omapdss platform device, also known as core code, not to be confused with the
omapdss_dss driver for the DSS hardware device). Patches 21/28 to 23/28 move
the useful features of the core to the omapdss_dss driver. Patch 24/28 adds
omapdrm platform device registration to the omapdss_dss driver to replace
board code, and patch 25/28 finally removes the omapdss platform driver.

Note that registering the omapdrm platform device from within the omapdss_dss
driver is a hack, but isn't worse than the current situation. Quite the
contrary, given that the omapdrm device exists for the sole purpose of
supporting the omapdrm/omapdss driver architecture, moving it out of platform
code can be considered as (slightly) cleaner. In any case, it will be easier
to refactor the code as everything is now isolated on the driver side.

Patches 26/28 and 27/28 remove the now unnecessary platform devices from
platform code, and patch 28/28 removes the now unused omapdrm platform data
structure.

The series will be annoying to merge given that it touches multiple subsystems
(ARM core, DRM, FBDEV and ALSA). The easiest solution would be to merge
everything through the DRM tree as that's where the bulk of changes lies. This
would require an ack from Bartlomiej for patch 04/28, from Peter, Liam or Mark
on patch 18/28 and from Tony on patches 26/28 and 27/28.

The patches are currently based on top of v4.11.

Laurent Pinchart (28):
  drm: omapdrm: Remove duplicate error messages when mapping memory
  drm: omapdrm: Drop support for non-DT devices
  drm: omapdrm: Remove unused dss_get_core_pdev() function
  drm: omapdrm: Remove unused default display name support
  drm: omapdrm: Infer the OMAP version from the SoC family
  drm: omapdrm: dispc: Select features based on compatible string
  drm: omapdrm: dpi: Remove platform driver
  drm: omapdrm: dpi: Replace OMAP SoC model checks with DSS device type
  drm: omapdrm: dsi: Store DSI type and PLL hardware data in OF data
  drm: omapdrm: dsi: Handle pin muxing internally
  drm: omapdrm: dss: Select features based on compatible string
  drm: omapdrm: dss: Split operations out of dss_features structure
  drm: omapdrm: dss: Initialize DSS internal features at probe time
  drm: omapdrm: hdmi: Store PHY features in PHY data structure
  drm: omapdrm: hdmi: Store PHY features in HDMI transmitter drivers
  drm: omapdrm: hdmi: Store PLL hardware data in HDMI transmitter
    drivers
  drm: omapdrm: hdmi: Replace OMAP SoC model check with HDMI xmit
    version
  drm: omapdrm: hdmi: Pass HDMI core version as integer to HDMI audio
  drm: omapdrm: sdi: Remove platform driver
  drm: omapdrm: Don't forward set_min_bus_tput() to no-op platform code
  drm: omapdrm: Move all debugfs code from core to dss
  drm: omapdrm: Move shutdown() handler from core to dss
  drm: omapdrm: Merge the dss_features and omap_dss_features structures
  drm: omapdrm: Register omapdrm platform device in omapdss driver
  drm: omapdrm: Remove the omapdss driver
  ARM: OMAP2+: Remove unused omapdrm platform device
  ARM: OMAP2+: Don't register omapdss device for omapdrm
  drm: omapdrm: Remove omapdrm platform data

 arch/arm/mach-omap2/Makefile                |   2 +-
 arch/arm/mach-omap2/display.c               | 118 +++++------
 arch/arm/mach-omap2/display.h               |   1 -
 arch/arm/mach-omap2/drm.c                   |  53 -----
 drivers/gpu/drm/omapdrm/dss/core.c          | 230 ++-------------------
 drivers/gpu/drm/omapdrm/dss/dispc.c         |  96 +++------
 drivers/gpu/drm/omapdrm/dss/display.c       |  19 +-
 drivers/gpu/drm/omapdrm/dss/dpi.c           | 150 +++-----------
 drivers/gpu/drm/omapdrm/dss/dsi.c           | 298 ++++++++++++++--------------
 drivers/gpu/drm/omapdrm/dss/dss.c           | 274 +++++++++++--------------
 drivers/gpu/drm/omapdrm/dss/dss.h           |  46 +++--
 drivers/gpu/drm/omapdrm/dss/dss_features.c  | 220 +++++++++++++-------
 drivers/gpu/drm/omapdrm/dss/dss_features.h  |  41 +++-
 drivers/gpu/drm/omapdrm/dss/hdmi.h          |  16 +-
 drivers/gpu/drm/omapdrm/dss/hdmi4.c         |  47 ++++-
 drivers/gpu/drm/omapdrm/dss/hdmi4_core.c    |   9 +-
 drivers/gpu/drm/omapdrm/dss/hdmi5.c         |  48 ++++-
 drivers/gpu/drm/omapdrm/dss/hdmi5_core.c    |   9 +-
 drivers/gpu/drm/omapdrm/dss/hdmi_phy.c      |  79 +-------
 drivers/gpu/drm/omapdrm/dss/hdmi_pll.c      |  86 +-------
 drivers/gpu/drm/omapdrm/dss/hdmi_wp.c       |  24 +--
 drivers/gpu/drm/omapdrm/dss/omapdss.h       |   2 -
 drivers/gpu/drm/omapdrm/dss/rfbi.c          |  12 +-
 drivers/gpu/drm/omapdrm/dss/sdi.c           |  54 -----
 drivers/gpu/drm/omapdrm/dss/venc.c          |  28 +--
 drivers/gpu/drm/omapdrm/dss/video-pll.c     |  20 +-
 drivers/gpu/drm/omapdrm/omap_drv.c          |  16 +-
 drivers/gpu/drm/omapdrm/omap_drv.h          |   1 -
 drivers/video/fbdev/omap2/omapfb/dss/core.c |   2 -
 include/linux/platform_data/omap_drm.h      |  53 -----
 include/linux/platform_data/omapdss.h       |   1 -
 include/sound/omap-hdmi-audio.h             |   2 +-
 sound/soc/omap/omap-hdmi-audio.c            |   9 +-
 33 files changed, 763 insertions(+), 1303 deletions(-)
 delete mode 100644 arch/arm/mach-omap2/drm.c
 delete mode 100644 include/linux/platform_data/omap_drm.h

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 26/28] ARM: OMAP2+: Remove unused omapdrm platform device

[Laurent Pinchart]

The omapdrm platform device is unused, as a replacement is now
registered in the omapdss driver. Remove it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm/mach-omap2/Makefile  |  2 +-
 arch/arm/mach-omap2/display.c |  7 ------
 arch/arm/mach-omap2/display.h |  1 -
 arch/arm/mach-omap2/drm.c     | 53 -------------------------------------------
 4 files changed, 1 insertion(+), 62 deletions(-)
 delete mode 100644 arch/arm/mach-omap2/drm.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index c89757abb0ae..9d3ba1e0119c 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -8,7 +8,7 @@ ccflags-y := -I$(srctree)/$(src)/include \
 # Common support
 obj-y := id.o io.o control.o devices.o fb.o timer.o pm.o \
 	 common.o dma.o wd_timer.o display.o i2c.o hdq1w.o omap_hwmod.o \
-	 omap_device.o omap-headsmp.o sram.o drm.o
+	 omap_device.o omap-headsmp.o sram.o
 
 hwmod-common				= omap_hwmod.o omap_hwmod_reset.o \
 					  omap_hwmod_common_data.o
diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index 8fa01c0ecdb2..5f5697668de8 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -384,13 +384,6 @@ int __init omapdss_init_of(void)
 		return r;
 	}
 
-	/* create DRM device */
-	r = omap_init_drm();
-	if (r < 0) {
-		pr_err("Unable to register omapdrm device\n");
-		return r;
-	}
-
 	/* create vrfb device */
 	r = omap_init_vrfb();
 	if (r < 0) {
diff --git a/arch/arm/mach-omap2/display.h b/arch/arm/mach-omap2/display.h
index 9a39646d4316..42ec2e99a2f4 100644
--- a/arch/arm/mach-omap2/display.h
+++ b/arch/arm/mach-omap2/display.h
@@ -26,7 +26,6 @@ struct omap_dss_dispc_dev_attr {
 	bool	has_framedonetv_irq;
 };
 
-int omap_init_drm(void);
 int omap_init_vrfb(void);
 int omap_init_fb(void);
 int omap_init_vout(void);
diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c
deleted file mode 100644
index 44fef961bb70..000000000000
--- a/arch/arm/mach-omap2/drm.c
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * DRM/KMS device registration for TI OMAP platforms
- *
- * Copyright (C) 2012 Texas Instruments
- * Author: Rob Clark <rob.clark@linaro.org>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published by
- * the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/mm.h>
-#include <linux/init.h>
-#include <linux/platform_device.h>
-#include <linux/dma-mapping.h>
-#include <linux/platform_data/omap_drm.h>
-
-#include "soc.h"
-#include "display.h"
-
-#if IS_ENABLED(CONFIG_DRM_OMAP)
-
-static struct omap_drm_platform_data platform_data;
-
-static struct platform_device omap_drm_device = {
-	.dev = {
-		.coherent_dma_mask = DMA_BIT_MASK(32),
-		.platform_data = &platform_data,
-	},
-	.name = "omapdrm",
-	.id = 0,
-};
-
-int __init omap_init_drm(void)
-{
-	platform_data.omaprev = GET_OMAP_TYPE;
-
-	return platform_device_register(&omap_drm_device);
-
-}
-#else
-int __init omap_init_drm(void) { return 0; }
-#endif
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 27/28] ARM: OMAP2+: Don't register omapdss device for omapdrm

[Laurent Pinchart]

The omapdrm driver doesn't need the omapdss device anymore. Although it
can't be removed completely as the fbdev driver still requires it, we
can condition its registration to the usage of the omapfb driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm/mach-omap2/display.c | 111 +++++++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 51 deletions(-)

diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index 5f5697668de8..798fc718fffe 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -66,6 +66,7 @@
  */
 #define FRAMEDONE_IRQ_TIMEOUT		100
 
+#if defined(CONFIG_FB_OMAP2)
 static struct platform_device omap_display_device = {
 	.name          = "omapdss",
 	.id            = -1,
@@ -163,6 +164,64 @@ static enum omapdss_version __init omap_display_get_version(void)
 		return OMAPDSS_VER_UNKNOWN;
 }
 
+static int __init omapdss_init_fbdev(void)
+{
+	static struct omap_dss_board_info board_data = {
+		.dsi_enable_pads = omap_dsi_enable_pads,
+		.dsi_disable_pads = omap_dsi_disable_pads,
+		.set_min_bus_tput = omap_dss_set_min_bus_tput,
+	};
+	struct device_node *node;
+
+	board_data.version = omap_display_get_version();
+	if (board_data.version == OMAPDSS_VER_UNKNOWN) {
+		pr_err("DSS not supported on this SoC\n");
+		return -ENODEV;
+	}
+
+	omap_display_device.dev.platform_data = &board_data;
+
+	r = platform_device_register(&omap_display_device);
+	if (r < 0) {
+		pr_err("Unable to register omapdss device\n");
+		return r;
+	}
+
+	/* create vrfb device */
+	r = omap_init_vrfb();
+	if (r < 0) {
+		pr_err("Unable to register omapvrfb device\n");
+		return r;
+	}
+
+	/* create FB device */
+	r = omap_init_fb();
+	if (r < 0) {
+		pr_err("Unable to register omapfb device\n");
+		return r;
+	}
+
+	/* create V4L2 display device */
+	r = omap_init_vout();
+	if (r < 0) {
+		pr_err("Unable to register omap_vout device\n");
+		return r;
+	}
+
+	/* add DSI info for omap4 */
+	node = of_find_node_by_name(NULL, "omap4_padconf_global");
+	if (node)
+		omap4_dsi_mux_syscon = syscon_node_to_regmap(node);
+
+	return 0;
+}
+#else
+static inline int omapdss_init_fbdev(void)
+{
+	return 0;
+}
+#endif /* CONFIG_FB_OMAP2 */
+
 static void dispc_disable_outputs(void)
 {
 	u32 v, irq_mask = 0;
@@ -335,16 +394,9 @@ static struct device_node * __init omapdss_find_dss_of_node(void)
 int __init omapdss_init_of(void)
 {
 	int r;
-	enum omapdss_version ver;
 	struct device_node *node;
 	struct platform_device *pdev;
 
-	static struct omap_dss_board_info board_data = {
-		.dsi_enable_pads = omap_dsi_enable_pads,
-		.dsi_disable_pads = omap_dsi_disable_pads,
-		.set_min_bus_tput = omap_dss_set_min_bus_tput,
-	};
-
 	/* only create dss helper devices if dss is enabled in the .dts */
 
 	node = omapdss_find_dss_of_node();
@@ -354,13 +406,6 @@ int __init omapdss_init_of(void)
 	if (!of_device_is_available(node))
 		return 0;
 
-	ver = omap_display_get_version();
-
-	if (ver == OMAPDSS_VER_UNKNOWN) {
-		pr_err("DSS not supported on this SoC\n");
-		return -ENODEV;
-	}
-
 	pdev = of_find_device_by_node(node);
 
 	if (!pdev) {
@@ -374,41 +419,5 @@ int __init omapdss_init_of(void)
 		return r;
 	}
 
-	board_data.version = ver;
-
-	omap_display_device.dev.platform_data = &board_data;
-
-	r = platform_device_register(&omap_display_device);
-	if (r < 0) {
-		pr_err("Unable to register omapdss device\n");
-		return r;
-	}
-
-	/* create vrfb device */
-	r = omap_init_vrfb();
-	if (r < 0) {
-		pr_err("Unable to register omapvrfb device\n");
-		return r;
-	}
-
-	/* create FB device */
-	r = omap_init_fb();
-	if (r < 0) {
-		pr_err("Unable to register omapfb device\n");
-		return r;
-	}
-
-	/* create V4L2 display device */
-	r = omap_init_vout();
-	if (r < 0) {
-		pr_err("Unable to register omap_vout device\n");
-		return r;
-	}
-
-	/* add DSI info for omap4 */
-	node = of_find_node_by_name(NULL, "omap4_padconf_global");
-	if (node)
-		omap4_dsi_mux_syscon = syscon_node_to_regmap(node);
-
-	return 0;
+	return omapdss_init_fbdev();
 }
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [170508 04:36]:
> The next step is to remove the omapdss platform driver (for the virtual
> omapdss platform device, also known as core code, not to be confused with the
> omapdss_dss driver for the DSS hardware device). Patches 21/28 to 23/28 move
> the useful features of the core to the omapdss_dss driver. Patch 24/28 adds
> omapdrm platform device registration to the omapdss_dss driver to replace
> board code, and patch 25/28 finally removes the omapdss platform driver.
> 
> Note that registering the omapdrm platform device from within the omapdss_dss
> driver is a hack, but isn't worse than the current situation. Quite the
> contrary, given that the omapdrm device exists for the sole purpose of
> supporting the omapdrm/omapdss driver architecture, moving it out of platform
> code can be considered as (slightly) cleaner. In any case, it will be easier
> to refactor the code as everything is now isolated on the driver side.

Good to see this happening. While at it, can you please also check that
the struct device entries follow what's in the hardware to avoid more
headaches later on.

The rule of thumb for struct device use here should be that each
interconnect target module is a separate device. If a single interconnect
target module has multiple IP blocks within it, then you need to have a
minimal wrapper driver to deal with the shared hardware resources like
the clkctrl clock ("ti,hwmods") along the lines of what we're doing in
drivers/usb/musb/musb_am335x.c to wrap two musb instances and cppi41 dma
all in a single interconnect target module.

You can detect the interconnect target module with SYSCONFIG and
SYSSTATUS entries listed for it in the TRM. It might be already set
up that way if we're lucky.

Regards,

Tony
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 26/28] ARM: OMAP2+: Remove unused omapdrm platform device

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [170508 04:36]:
> The omapdrm platform device is unused, as a replacement is now
> registered in the omapdss driver. Remove it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

This should be OK to merge via the drm patches as long as things
have been properly tested to not cause regressions:

Acked-by: Tony Lindgren <tony@atomide.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 27/28] ARM: OMAP2+: Don't register omapdss device for omapdrm

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [170508 04:36]:
> The omapdrm driver doesn't need the omapdss device anymore. Although it
> can't be removed completely as the fbdev driver still requires it, we
> can condition its registration to the usage of the omapfb driver.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

This too:

Acked-by: Tony Lindgren <tony@atomide.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 26/28] ARM: OMAP2+: Remove unused omapdrm platform device

[Laurent Pinchart]

Hi Tony,

On Monday 08 May 2017 10:09:06 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [170508 04:36]:
> > The omapdrm platform device is unused, as a replacement is now
> > registered in the omapdss driver. Remove it.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> This should be OK to merge via the drm patches as long as things
> have been properly tested to not cause regressions:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>

I'll do my best to ensure that. I can't test the code on all OMAP platforms, 
but given that this patch (and the next one) are not dependent on the 
particular OMAP SoC, it should be fine.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 26/28] ARM: OMAP2+: Remove unused omapdrm platform device

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 620 bytes --]

On 08/05/17 14:33, Laurent Pinchart wrote:
> The omapdrm platform device is unused, as a replacement is now
> registered in the omapdss driver. Remove it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  arch/arm/mach-omap2/Makefile  |  2 +-
>  arch/arm/mach-omap2/display.c |  7 ------
>  arch/arm/mach-omap2/display.h |  1 -
>  arch/arm/mach-omap2/drm.c     | 53 -------------------------------------------
>  4 files changed, 1 insertion(+), 62 deletions(-)
>  delete mode 100644 arch/arm/mach-omap2/drm.c

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 27/28] ARM: OMAP2+: Don't register omapdss device for omapdrm

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 546 bytes --]

On 08/05/17 14:33, Laurent Pinchart wrote:
> The omapdrm driver doesn't need the omapdss device anymore. Although it
> can't be removed completely as the fbdev driver still requires it, we
> can condition its registration to the usage of the omapfb driver.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  arch/arm/mach-omap2/display.c | 111 +++++++++++++++++++++++-------------------
>  1 file changed, 60 insertions(+), 51 deletions(-)

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1382 bytes --]

On 08/05/17 20:07, Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [170508 04:36]:
>> The next step is to remove the omapdss platform driver (for the virtual
>> omapdss platform device, also known as core code, not to be confused with the
>> omapdss_dss driver for the DSS hardware device). Patches 21/28 to 23/28 move
>> the useful features of the core to the omapdss_dss driver. Patch 24/28 adds
>> omapdrm platform device registration to the omapdss_dss driver to replace
>> board code, and patch 25/28 finally removes the omapdss platform driver.
>>
>> Note that registering the omapdrm platform device from within the omapdss_dss
>> driver is a hack, but isn't worse than the current situation. Quite the
>> contrary, given that the omapdrm device exists for the sole purpose of
>> supporting the omapdrm/omapdss driver architecture, moving it out of platform
>> code can be considered as (slightly) cleaner. In any case, it will be easier
>> to refactor the code as everything is now isolated on the driver side.
> 
> Good to see this happening. While at it, can you please also check that
> the struct device entries follow what's in the hardware to avoid more
> headaches later on.

This has been the case for many years. We've just had some extra stuff
on top, due to legacy reasons (from time before hwmods).

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 559 bytes --]

On 08/05/17 14:32, Laurent Pinchart wrote:
> Hello,
> 
> This patch series is a second, extended version of the code previously posted
> as "[PATCH/RFC 0/7] Remove the omapdrm device from platform code".

As this is a long series, I'd like to pick a bunch of patches from this
series already:

1-5, 7, 19, 21.

I didn't test yet, but I think those should not cause conflicts with the
rest of the series.

I can then push the branch, which contains also the fences and cache
patches, so you can base the rest on that.

Is this ok?

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [170509 04:56]:
> On 08/05/17 20:07, Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [170508 04:36]:
> >> The next step is to remove the omapdss platform driver (for the virtual
> >> omapdss platform device, also known as core code, not to be confused with the
> >> omapdss_dss driver for the DSS hardware device). Patches 21/28 to 23/28 move
> >> the useful features of the core to the omapdss_dss driver. Patch 24/28 adds
> >> omapdrm platform device registration to the omapdss_dss driver to replace
> >> board code, and patch 25/28 finally removes the omapdss platform driver.
> >>
> >> Note that registering the omapdrm platform device from within the omapdss_dss
> >> driver is a hack, but isn't worse than the current situation. Quite the
> >> contrary, given that the omapdrm device exists for the sole purpose of
> >> supporting the omapdrm/omapdss driver architecture, moving it out of platform
> >> code can be considered as (slightly) cleaner. In any case, it will be easier
> >> to refactor the code as everything is now isolated on the driver side.
> > 
> > Good to see this happening. While at it, can you please also check that
> > the struct device entries follow what's in the hardware to avoid more
> > headaches later on.
> 
> This has been the case for many years. We've just had some extra stuff
> on top, due to legacy reasons (from time before hwmods).

OK thanks for confirming that.

Tony



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Sebastian Reichel]

[-- Attachment #1.1: Type: text/plain, Size: 1283 bytes --]

Hi,

On Tue, May 09, 2017 at 03:10:40PM +0300, Tomi Valkeinen wrote:
> On 08/05/17 14:32, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series is a second, extended version of the code previously posted
> > as "[PATCH/RFC 0/7] Remove the omapdrm device from platform code".
> 
> As this is a long series, I'd like to pick a bunch of patches from this
> series already:
> 
> 1-5, 7, 19, 21.

I can't reply directly, since I was not subscribed to dri-devel when
this was sent, but I also noticed, the incorrect pattern in patch 1:

foo = devm_ioremap_resource(...)
if (!foo)
    return -ENOMEM;

The same pattern is in patch 2, so you may want a PATCHv2 of that
one first. Also I have a few more notes:

patch 10:
I think dsi pin muxing should be done by providing pinmux
info via DT.

patch 11:
There is a typo in the comment "can be told apart" => "can't be told
apart".

patch 21:
As far as I can see dss.h contains an empty #if
defined(CONFIG_OMAP2_DSS_DEBUGFS) after the patch.

> I didn't test yet, but I think those should not cause conflicts with the
> rest of the series.
> 
> I can then push the branch, which contains also the fences and cache
> patches, so you can base the rest on that.
> 
> Is this ok?

-- Sebastian

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Laurent Pinchart]

Hi Tomi,

On Tuesday 09 May 2017 15:10:40 Tomi Valkeinen wrote:
> On 08/05/17 14:32, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series is a second, extended version of the code previously
> > posted as "[PATCH/RFC 0/7] Remove the omapdrm device from platform code".
> As this is a long series, I'd like to pick a bunch of patches from this
> series already:
> 
> 1-5, 7, 19, 21.
> 
> I didn't test yet, but I think those should not cause conflicts with the
> rest of the series.
> 
> I can then push the branch, which contains also the fences and cache
> patches, so you can base the rest on that.
> 
> Is this ok?

Patch 21 will conflict, the other ones shouldn't. For your convenience I've 
picked all the Reviewed-by tags, fixed a typo in patch 01/28 that broke 
compilation, rebased the patches with 1-5, 7 and 19 moved to the beginning of 
the series, tested the result and pushed it to

	git://linuxtv.org/pinchartl/media.git omapdrm/platform

You can just pick the 7 first patches from there.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1216 bytes --]



On 10/05/17 01:18, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Tuesday 09 May 2017 15:10:40 Tomi Valkeinen wrote:
>> On 08/05/17 14:32, Laurent Pinchart wrote:
>>> Hello,
>>>
>>> This patch series is a second, extended version of the code previously
>>> posted as "[PATCH/RFC 0/7] Remove the omapdrm device from platform code".
>> As this is a long series, I'd like to pick a bunch of patches from this
>> series already:
>>
>> 1-5, 7, 19, 21.
>>
>> I didn't test yet, but I think those should not cause conflicts with the
>> rest of the series.
>>
>> I can then push the branch, which contains also the fences and cache
>> patches, so you can base the rest on that.
>>
>> Is this ok?
> 
> Patch 21 will conflict, the other ones shouldn't. For your convenience I've 
> picked all the Reviewed-by tags, fixed a typo in patch 01/28 that broke 
> compilation, rebased the patches with 1-5, 7 and 19 moved to the beginning of 
> the series, tested the result and pushed it to
> 
> 	git://linuxtv.org/pinchartl/media.git omapdrm/platform
> 
> You can just pick the 7 first patches from there.

Thanks, I picked those. A few minor conflicts with my branch, but I
fixed them.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 279 bytes --]

On 09/05/17 18:05, Sebastian Reichel wrote:

> patch 10:
> I think dsi pin muxing should be done by providing pinmux
> info via DT.

Unfortunately there's no pinmux driver for the kind of register we have
for DSI. At least this was the case not that long ago.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [170510 00:26]:
> On 09/05/17 18:05, Sebastian Reichel wrote:
> 
> > patch 10:
> > I think dsi pin muxing should be done by providing pinmux
> > info via DT.
> 
> Unfortunately there's no pinmux driver for the kind of register we have
> for DSI. At least this was the case not that long ago.

What are the TRM names for the registers you need to mux?

AFAIK pinctrl-single should be capable of muxing anything in
the padconf registers area. It should be only used for external
pins though, not internal muxes.

Regards,

Tony



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 486 bytes --]



On 10/05/17 19:46, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [170510 00:26]:
>> On 09/05/17 18:05, Sebastian Reichel wrote:
>>
>>> patch 10:
>>> I think dsi pin muxing should be done by providing pinmux
>>> info via DT.
>>
>> Unfortunately there's no pinmux driver for the kind of register we have
>> for DSI. At least this was the case not that long ago.
> 
> What are the TRM names for the registers you need to mux?

CONTROL_DSIPHY

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [170510 10:44]:
> 
> 
> On 10/05/17 19:46, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [170510 00:26]:
> >> On 09/05/17 18:05, Sebastian Reichel wrote:
> >>
> >>> patch 10:
> >>> I think dsi pin muxing should be done by providing pinmux
> >>> info via DT.
> >>
> >> Unfortunately there's no pinmux driver for the kind of register we have
> >> for DSI. At least this was the case not that long ago.
> > 
> > What are the TRM names for the registers you need to mux?
> 
> CONTROL_DSIPHY

Oh just one register with few bits? That should be doable in
dts with pinctrl-single,bit-per-mux and pinctrl-single,bits.

There are examples in the kernel tree:

$ git grep -e pinctrl-single,bits -e pinctrl-single,bit-per-mux \
	arch/arm/boot/dts

Regards,

Tony



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1097 bytes --]

On 10/05/17 21:29, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [170510 10:44]:
>>
>>
>> On 10/05/17 19:46, Tony Lindgren wrote:
>>> * Tomi Valkeinen <tomi.valkeinen@ti.com> [170510 00:26]:
>>>> On 09/05/17 18:05, Sebastian Reichel wrote:
>>>>
>>>>> patch 10:
>>>>> I think dsi pin muxing should be done by providing pinmux
>>>>> info via DT.
>>>>
>>>> Unfortunately there's no pinmux driver for the kind of register we have
>>>> for DSI. At least this was the case not that long ago.
>>>
>>> What are the TRM names for the registers you need to mux?
>>
>> CONTROL_DSIPHY
> 
> Oh just one register with few bits? That should be doable in
> dts with pinctrl-single,bit-per-mux and pinctrl-single,bits.

No, I don't think it works. Or at least I can't figure out how.

pinctrl-single doesn't allow to freely set the bits, but requires the
pins to have similar bit structure (function-mask). In CONTROL_DSIPHY,
DSI1 and DSI2 have different bit structures.

I don't understand why pinctrl-single tries so hard to fit things into
one mold...

 tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [170511 01:37]:
> On 10/05/17 21:29, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [170510 10:44]:
> >>
> >>
> >> On 10/05/17 19:46, Tony Lindgren wrote:
> >>> * Tomi Valkeinen <tomi.valkeinen@ti.com> [170510 00:26]:
> >>>> On 09/05/17 18:05, Sebastian Reichel wrote:
> >>>>
> >>>>> patch 10:
> >>>>> I think dsi pin muxing should be done by providing pinmux
> >>>>> info via DT.
> >>>>
> >>>> Unfortunately there's no pinmux driver for the kind of register we have
> >>>> for DSI. At least this was the case not that long ago.
> >>>
> >>> What are the TRM names for the registers you need to mux?
> >>
> >> CONTROL_DSIPHY
> > 
> > Oh just one register with few bits? That should be doable in
> > dts with pinctrl-single,bit-per-mux and pinctrl-single,bits.
> 
> No, I don't think it works. Or at least I can't figure out how.
> 
> pinctrl-single doesn't allow to freely set the bits, but requires the
> pins to have similar bit structure (function-mask). In CONTROL_DSIPHY,
> DSI1 and DSI2 have different bit structures.

OK if the register mixes different types of controllers that
can't be partitioned into separate 8 or 16 bit instances then
you're out of luck with pinctrl-single. If it does not fit, no
point trying to force it, then you need a custom pinctrl driver.

> I don't understand why pinctrl-single tries so hard to fit things into
> one mold...

Basically on many SoCs pinctrl is just the same exact control
register repeated for each pin on the SoC:

$ git grep '"pinctrl-single"' arch/arm*/boot/dts | sort | uniq | wc -l
20

Regards,

Tony


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1241 bytes --]

On 11/05/17 17:16, Tony Lindgren wrote:

>> pinctrl-single doesn't allow to freely set the bits, but requires the
>> pins to have similar bit structure (function-mask). In CONTROL_DSIPHY,
>> DSI1 and DSI2 have different bit structures.
> 
> OK if the register mixes different types of controllers that
> can't be partitioned into separate 8 or 16 bit instances then
> you're out of luck with pinctrl-single. If it does not fit, no
> point trying to force it, then you need a custom pinctrl driver.

Writing a driver for a single register on a legacy SoC feels like an
overkill... But I guess a generic pinctrl driver which allows free
writes to registers would do the trick, but, then again, if so far we
have a single register in a single SoC that needs this, maybe it's not
worth the effort.

>> I don't understand why pinctrl-single tries so hard to fit things into
>> one mold...
> 
> Basically on many SoCs pinctrl is just the same exact control
> register repeated for each pin on the SoC:

Right, I was just wondering why it forces one to have a function mask,
versus allowing it to be left out and thus making it possible to handle
also cases where the pins require different kinds of bit masks.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/29] Remove the omapdrm and omapdss devices from platform code

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [170512 00:32]:
> On 11/05/17 17:16, Tony Lindgren wrote:
> 
> >> pinctrl-single doesn't allow to freely set the bits, but requires the
> >> pins to have similar bit structure (function-mask). In CONTROL_DSIPHY,
> >> DSI1 and DSI2 have different bit structures.
> > 
> > OK if the register mixes different types of controllers that
> > can't be partitioned into separate 8 or 16 bit instances then
> > you're out of luck with pinctrl-single. If it does not fit, no
> > point trying to force it, then you need a custom pinctrl driver.
> 
> Writing a driver for a single register on a legacy SoC feels like an
> overkill... But I guess a generic pinctrl driver which allows free
> writes to registers would do the trick, but, then again, if so far we
> have a single register in a single SoC that needs this, maybe it's not
> worth the effort.

Yeah..

> >> I don't understand why pinctrl-single tries so hard to fit things into
> >> one mold...
> > 
> > Basically on many SoCs pinctrl is just the same exact control
> > register repeated for each pin on the SoC:
> 
> Right, I was just wondering why it forces one to have a function mask,
> versus allowing it to be left out and thus making it possible to handle
> also cases where the pins require different kinds of bit masks.

Some of the bits are not usable typically. I think what you're
describing could probably be done with a custom compatible plus
struct pcs_soc_data.

Regards,

Tony
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel