All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V10 0/4] drm/panel: Add Magnachip D53E6EA8966 Panel Controller
@ 2023-01-12 17:53 ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add the Magnachip D53E6EA8966 panel IC controller for display panels
such as the Samsung AMS495QA01 panel as found on the Anbernic RG503.
This panel uses DSI to receive video signals, but 3-wire SPI to receive
command signals using DBI.

Changes since V9:
 - Set an ifdef to not add the drm_of_get_dsi_bus when MIPI_DSI is not
   part of the current kernel config.
 - Made "info" optional in the drm_of_get_dsi_bus() function.

Changes since V8:
 - Set "placeholder" drm_of_get_dsi_bus in drm_of.h to static inline
   to hopefully eliminate the reported errors once and for all. Tested
   with 4 different kernel configurations provided by Intel's kernel
   test robot and no new warnings or errors were introduced.
   Reported-by: kernel test robot <lkp@intel.com>

Changes since V7:
 - Removed Linus Walleij review note due to substantial changes.
 - Corrected documentation of drm_of_get_dsi_bus function.
 - Updated the drm_of_get_dsi_bus function to return pointer to
   mipi_dsi_host and use ERR_PTR macros.
 - Refactored drm_panel_funcs so that the prepare function calls
   panel specific function for init sequence and uses generic
   functions otherwise.
 - Renamed non-panel specific functions.
 - Changed backlight value to int instead of u32.
 - Corrected brightness function to use backlight_get_brightness().
 - Fix an error reported when CONFIG_OF is selected but
   CONFIG_DRM_MIPI_DSI is not. Add an if function to drm_of_get_dsi_bus
   function to return -EINVAL in this instance.
   Reported-by: kernel test robot <lkp@intel.com>

Changes since V6:
 - Fixed a trivial error with definition of drm_of_get_dsi_bus().
   Reported-by: kernel test robot <lkp@intel.com>

Changes since V5:
 - Reverted dt binding documentation name back to
   samsung,ams495qa01.yaml.
 - Removed no longer needed of_graph.h header file.
 - Added backlight as a dependency.

Changes since V4:
 - Renamed driver from the panel model to the panel IC controller per
   DRM team.
 - Added a drm_of helper function of drm_of_get_dsi_bus() to handle
   finding and populating the DSI node when the DSI node is not the
   parent of the DSI controlled display.
 - Converted the documented commands to constants to make it more
   readable.
 - Reset GPIO is now required and documented as GPIO_ACTIVE_LOW.
 - Removed "prepared" logic from panel.

Changes since V3:
 - Updated documentation to add spi-peripheral-props.yaml per updates
   made for similar devices. Note that I removed a "Reviewed-by" tag
   from Rob Herring since this change probably needs to be confirmed.
 - Added binding for RG503, since this device is now accepted with this
   request: https://lore.kernel.org/linux-rockchip/166274831283.21181.6861718157177507544.b4-ty@sntech.de/

Changes since V2:
 - Added 50hz mode at request of userspace devs.
 - Renamed "dupa" to panel name. Good catch Maya.
 - Added Maya's Signed-off-by.
 - Removed check for max backlight, since it is already done by
   backlight_device_set_brightness.
 - Fixed minor formatting issues on devicetree binding documentation
   and added port to provided example.

Changes since V1:
 - Removed errant reference to backlight in documentation. This is an
   OLED panel.
 - Made elvss regulator optional. In my case its hard wired and not
   controllable.
 - Added "prepared" enum to track panel status to prevent unbalanced
   regulator enable/disable.

Chris Morgan (4):
  drm: of: Add drm_of_get_dsi_bus helper function
  dt-bindings: display: panel: Add Samsung AMS495QA01
  drm/panel: Add Magnachip D53E6EA8966 Panel Driver
  arm64: dts: rockchip: add display to RG503

 .../display/panel/samsung,ams495qa01.yaml     |  57 ++
 .../dts/rockchip/rk3566-anbernic-rg503.dts    |  55 ++
 drivers/gpu/drm/drm_of.c                      |  70 +++
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../drm/panel/panel-magnachip-d53e6ea8966.c   | 522 ++++++++++++++++++
 include/drm/drm_of.h                          |  10 +
 7 files changed, 726 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c

-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH V10 0/4] drm/panel: Add Magnachip D53E6EA8966 Panel Controller
@ 2023-01-12 17:53 ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, Chris Morgan, krzysztof.kozlowski+dt, sam,
	linux-rockchip, robh+dt, thierry.reding, tzimmermann,
	maccraft123mc

From: Chris Morgan <macromorgan@hotmail.com>

Add the Magnachip D53E6EA8966 panel IC controller for display panels
such as the Samsung AMS495QA01 panel as found on the Anbernic RG503.
This panel uses DSI to receive video signals, but 3-wire SPI to receive
command signals using DBI.

Changes since V9:
 - Set an ifdef to not add the drm_of_get_dsi_bus when MIPI_DSI is not
   part of the current kernel config.
 - Made "info" optional in the drm_of_get_dsi_bus() function.

Changes since V8:
 - Set "placeholder" drm_of_get_dsi_bus in drm_of.h to static inline
   to hopefully eliminate the reported errors once and for all. Tested
   with 4 different kernel configurations provided by Intel's kernel
   test robot and no new warnings or errors were introduced.
   Reported-by: kernel test robot <lkp@intel.com>

Changes since V7:
 - Removed Linus Walleij review note due to substantial changes.
 - Corrected documentation of drm_of_get_dsi_bus function.
 - Updated the drm_of_get_dsi_bus function to return pointer to
   mipi_dsi_host and use ERR_PTR macros.
 - Refactored drm_panel_funcs so that the prepare function calls
   panel specific function for init sequence and uses generic
   functions otherwise.
 - Renamed non-panel specific functions.
 - Changed backlight value to int instead of u32.
 - Corrected brightness function to use backlight_get_brightness().
 - Fix an error reported when CONFIG_OF is selected but
   CONFIG_DRM_MIPI_DSI is not. Add an if function to drm_of_get_dsi_bus
   function to return -EINVAL in this instance.
   Reported-by: kernel test robot <lkp@intel.com>

Changes since V6:
 - Fixed a trivial error with definition of drm_of_get_dsi_bus().
   Reported-by: kernel test robot <lkp@intel.com>

Changes since V5:
 - Reverted dt binding documentation name back to
   samsung,ams495qa01.yaml.
 - Removed no longer needed of_graph.h header file.
 - Added backlight as a dependency.

Changes since V4:
 - Renamed driver from the panel model to the panel IC controller per
   DRM team.
 - Added a drm_of helper function of drm_of_get_dsi_bus() to handle
   finding and populating the DSI node when the DSI node is not the
   parent of the DSI controlled display.
 - Converted the documented commands to constants to make it more
   readable.
 - Reset GPIO is now required and documented as GPIO_ACTIVE_LOW.
 - Removed "prepared" logic from panel.

Changes since V3:
 - Updated documentation to add spi-peripheral-props.yaml per updates
   made for similar devices. Note that I removed a "Reviewed-by" tag
   from Rob Herring since this change probably needs to be confirmed.
 - Added binding for RG503, since this device is now accepted with this
   request: https://lore.kernel.org/linux-rockchip/166274831283.21181.6861718157177507544.b4-ty@sntech.de/

Changes since V2:
 - Added 50hz mode at request of userspace devs.
 - Renamed "dupa" to panel name. Good catch Maya.
 - Added Maya's Signed-off-by.
 - Removed check for max backlight, since it is already done by
   backlight_device_set_brightness.
 - Fixed minor formatting issues on devicetree binding documentation
   and added port to provided example.

Changes since V1:
 - Removed errant reference to backlight in documentation. This is an
   OLED panel.
 - Made elvss regulator optional. In my case its hard wired and not
   controllable.
 - Added "prepared" enum to track panel status to prevent unbalanced
   regulator enable/disable.

Chris Morgan (4):
  drm: of: Add drm_of_get_dsi_bus helper function
  dt-bindings: display: panel: Add Samsung AMS495QA01
  drm/panel: Add Magnachip D53E6EA8966 Panel Driver
  arm64: dts: rockchip: add display to RG503

 .../display/panel/samsung,ams495qa01.yaml     |  57 ++
 .../dts/rockchip/rk3566-anbernic-rg503.dts    |  55 ++
 drivers/gpu/drm/drm_of.c                      |  70 +++
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../drm/panel/panel-magnachip-d53e6ea8966.c   | 522 ++++++++++++++++++
 include/drm/drm_of.h                          |  10 +
 7 files changed, 726 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c

-- 
2.34.1


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

* [PATCH V10 0/4] drm/panel: Add Magnachip D53E6EA8966 Panel Controller
@ 2023-01-12 17:53 ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add the Magnachip D53E6EA8966 panel IC controller for display panels
such as the Samsung AMS495QA01 panel as found on the Anbernic RG503.
This panel uses DSI to receive video signals, but 3-wire SPI to receive
command signals using DBI.

Changes since V9:
 - Set an ifdef to not add the drm_of_get_dsi_bus when MIPI_DSI is not
   part of the current kernel config.
 - Made "info" optional in the drm_of_get_dsi_bus() function.

Changes since V8:
 - Set "placeholder" drm_of_get_dsi_bus in drm_of.h to static inline
   to hopefully eliminate the reported errors once and for all. Tested
   with 4 different kernel configurations provided by Intel's kernel
   test robot and no new warnings or errors were introduced.
   Reported-by: kernel test robot <lkp@intel.com>

Changes since V7:
 - Removed Linus Walleij review note due to substantial changes.
 - Corrected documentation of drm_of_get_dsi_bus function.
 - Updated the drm_of_get_dsi_bus function to return pointer to
   mipi_dsi_host and use ERR_PTR macros.
 - Refactored drm_panel_funcs so that the prepare function calls
   panel specific function for init sequence and uses generic
   functions otherwise.
 - Renamed non-panel specific functions.
 - Changed backlight value to int instead of u32.
 - Corrected brightness function to use backlight_get_brightness().
 - Fix an error reported when CONFIG_OF is selected but
   CONFIG_DRM_MIPI_DSI is not. Add an if function to drm_of_get_dsi_bus
   function to return -EINVAL in this instance.
   Reported-by: kernel test robot <lkp@intel.com>

Changes since V6:
 - Fixed a trivial error with definition of drm_of_get_dsi_bus().
   Reported-by: kernel test robot <lkp@intel.com>

Changes since V5:
 - Reverted dt binding documentation name back to
   samsung,ams495qa01.yaml.
 - Removed no longer needed of_graph.h header file.
 - Added backlight as a dependency.

Changes since V4:
 - Renamed driver from the panel model to the panel IC controller per
   DRM team.
 - Added a drm_of helper function of drm_of_get_dsi_bus() to handle
   finding and populating the DSI node when the DSI node is not the
   parent of the DSI controlled display.
 - Converted the documented commands to constants to make it more
   readable.
 - Reset GPIO is now required and documented as GPIO_ACTIVE_LOW.
 - Removed "prepared" logic from panel.

Changes since V3:
 - Updated documentation to add spi-peripheral-props.yaml per updates
   made for similar devices. Note that I removed a "Reviewed-by" tag
   from Rob Herring since this change probably needs to be confirmed.
 - Added binding for RG503, since this device is now accepted with this
   request: https://lore.kernel.org/linux-rockchip/166274831283.21181.6861718157177507544.b4-ty@sntech.de/

Changes since V2:
 - Added 50hz mode at request of userspace devs.
 - Renamed "dupa" to panel name. Good catch Maya.
 - Added Maya's Signed-off-by.
 - Removed check for max backlight, since it is already done by
   backlight_device_set_brightness.
 - Fixed minor formatting issues on devicetree binding documentation
   and added port to provided example.

Changes since V1:
 - Removed errant reference to backlight in documentation. This is an
   OLED panel.
 - Made elvss regulator optional. In my case its hard wired and not
   controllable.
 - Added "prepared" enum to track panel status to prevent unbalanced
   regulator enable/disable.

Chris Morgan (4):
  drm: of: Add drm_of_get_dsi_bus helper function
  dt-bindings: display: panel: Add Samsung AMS495QA01
  drm/panel: Add Magnachip D53E6EA8966 Panel Driver
  arm64: dts: rockchip: add display to RG503

 .../display/panel/samsung,ams495qa01.yaml     |  57 ++
 .../dts/rockchip/rk3566-anbernic-rg503.dts    |  55 ++
 drivers/gpu/drm/drm_of.c                      |  70 +++
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../drm/panel/panel-magnachip-d53e6ea8966.c   | 522 ++++++++++++++++++
 include/drm/drm_of.h                          |  10 +
 7 files changed, 726 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c

-- 
2.34.1


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

* [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
  2023-01-12 17:53 ` Chris Morgan
  (?)
@ 2023-01-12 17:53   ` Chris Morgan
  -1 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add helper function to find DSI host for devices where DSI panel is not
a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
official Raspberry Pi touchscreen display).

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_of.h     | 10 ++++++
 2 files changed, 80 insertions(+)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 7bbcb999bb75..6c2c97a716fe 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -10,6 +10,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
 #include <drm/drm_encoder.h>
+#include <drm/drm_mipi_dsi.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 
@@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
+
+#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
+
+/**
+ * drm_of_get_dsi_bus - find the DSI bus for a given device
+ * @dev: parent device of display (SPI, I2C)
+ * @info: DSI device info to be updated with DSI node. This is optional
+ * and if not needed can be NULL.
+ *
+ * Gets parent DSI bus for a DSI device controlled through a bus other
+ * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
+ *
+ * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
+ * request is unsupported, -EPROBE_DEFER if the DSI host is found but
+ * not available, or -ENODEV otherwise.
+ */
+struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
+					 struct mipi_dsi_device_info *info)
+{
+	struct mipi_dsi_host *dsi_host;
+	struct device_node *endpoint, *dsi_host_node;
+
+	/*
+	 * Get first endpoint child from device.
+	 */
+	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
+	if (!endpoint)
+		return ERR_PTR(-ENODEV);
+
+	/*
+	 * Follow the first endpoint to get the DSI host node.
+	 */
+	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
+	if (!dsi_host_node)
+		goto error;
+
+	/*
+	 * Get the DSI host from the DSI host node. If we get an error
+	 * or the return is null assume we're not ready to probe just
+	 * yet. Release the DSI host node since we're done with it.
+	 */
+	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
+	of_node_put(dsi_host_node);
+	if (IS_ERR_OR_NULL(dsi_host)) {
+		of_node_put(endpoint);
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	/*
+	 * Set the node of the mipi_dsi_device_info to the correct node
+	 * and then release the endpoint node since we're done with it.
+	 * since this is optional, check if the info is NULL first.
+	 */
+	if (info) {
+		info->node = of_graph_get_remote_port(endpoint);
+		if (IS_ERR_OR_NULL(info->node))
+			goto error;
+	}
+
+	of_node_put(endpoint);
+	return dsi_host;
+
+error:
+	of_node_put(endpoint);
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL_GPL(drm_of_get_dsi_bus);
+
+#endif /* CONFIG_DRM_MIPI_DSI */
diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
index 10ab58c40746..705ea2caa494 100644
--- a/include/drm/drm_of.h
+++ b/include/drm/drm_of.h
@@ -15,6 +15,8 @@ struct drm_encoder;
 struct drm_panel;
 struct drm_bridge;
 struct device_node;
+struct mipi_dsi_device_info;
+struct mipi_dsi_host;
 
 /**
  * enum drm_lvds_dual_link_pixels - Pixel order of an LVDS dual-link connection
@@ -56,6 +58,8 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
 				   int port_reg, int reg,
 				   const unsigned int min,
 				   const unsigned int max);
+struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
+					 struct mipi_dsi_device_info *info);
 #else
 static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
 					  struct device_node *port)
@@ -127,6 +131,12 @@ drm_of_get_data_lanes_count_ep(const struct device_node *port,
 {
 	return -EINVAL;
 }
+static inline struct
+mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
+					 struct mipi_dsi_device_info *info)
+{
+	return ERR_PTR(-EINVAL);
+}
 #endif
 
 /*
-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
@ 2023-01-12 17:53   ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, Chris Morgan, krzysztof.kozlowski+dt, sam,
	linux-rockchip, robh+dt, thierry.reding, tzimmermann,
	maccraft123mc

From: Chris Morgan <macromorgan@hotmail.com>

Add helper function to find DSI host for devices where DSI panel is not
a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
official Raspberry Pi touchscreen display).

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_of.h     | 10 ++++++
 2 files changed, 80 insertions(+)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 7bbcb999bb75..6c2c97a716fe 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -10,6 +10,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
 #include <drm/drm_encoder.h>
+#include <drm/drm_mipi_dsi.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 
@@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
+
+#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
+
+/**
+ * drm_of_get_dsi_bus - find the DSI bus for a given device
+ * @dev: parent device of display (SPI, I2C)
+ * @info: DSI device info to be updated with DSI node. This is optional
+ * and if not needed can be NULL.
+ *
+ * Gets parent DSI bus for a DSI device controlled through a bus other
+ * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
+ *
+ * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
+ * request is unsupported, -EPROBE_DEFER if the DSI host is found but
+ * not available, or -ENODEV otherwise.
+ */
+struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
+					 struct mipi_dsi_device_info *info)
+{
+	struct mipi_dsi_host *dsi_host;
+	struct device_node *endpoint, *dsi_host_node;
+
+	/*
+	 * Get first endpoint child from device.
+	 */
+	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
+	if (!endpoint)
+		return ERR_PTR(-ENODEV);
+
+	/*
+	 * Follow the first endpoint to get the DSI host node.
+	 */
+	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
+	if (!dsi_host_node)
+		goto error;
+
+	/*
+	 * Get the DSI host from the DSI host node. If we get an error
+	 * or the return is null assume we're not ready to probe just
+	 * yet. Release the DSI host node since we're done with it.
+	 */
+	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
+	of_node_put(dsi_host_node);
+	if (IS_ERR_OR_NULL(dsi_host)) {
+		of_node_put(endpoint);
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	/*
+	 * Set the node of the mipi_dsi_device_info to the correct node
+	 * and then release the endpoint node since we're done with it.
+	 * since this is optional, check if the info is NULL first.
+	 */
+	if (info) {
+		info->node = of_graph_get_remote_port(endpoint);
+		if (IS_ERR_OR_NULL(info->node))
+			goto error;
+	}
+
+	of_node_put(endpoint);
+	return dsi_host;
+
+error:
+	of_node_put(endpoint);
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL_GPL(drm_of_get_dsi_bus);
+
+#endif /* CONFIG_DRM_MIPI_DSI */
diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
index 10ab58c40746..705ea2caa494 100644
--- a/include/drm/drm_of.h
+++ b/include/drm/drm_of.h
@@ -15,6 +15,8 @@ struct drm_encoder;
 struct drm_panel;
 struct drm_bridge;
 struct device_node;
+struct mipi_dsi_device_info;
+struct mipi_dsi_host;
 
 /**
  * enum drm_lvds_dual_link_pixels - Pixel order of an LVDS dual-link connection
@@ -56,6 +58,8 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
 				   int port_reg, int reg,
 				   const unsigned int min,
 				   const unsigned int max);
+struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
+					 struct mipi_dsi_device_info *info);
 #else
 static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
 					  struct device_node *port)
@@ -127,6 +131,12 @@ drm_of_get_data_lanes_count_ep(const struct device_node *port,
 {
 	return -EINVAL;
 }
+static inline struct
+mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
+					 struct mipi_dsi_device_info *info)
+{
+	return ERR_PTR(-EINVAL);
+}
 #endif
 
 /*
-- 
2.34.1


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

* [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
@ 2023-01-12 17:53   ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add helper function to find DSI host for devices where DSI panel is not
a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
official Raspberry Pi touchscreen display).

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_of.h     | 10 ++++++
 2 files changed, 80 insertions(+)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 7bbcb999bb75..6c2c97a716fe 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -10,6 +10,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
 #include <drm/drm_encoder.h>
+#include <drm/drm_mipi_dsi.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 
@@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
+
+#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
+
+/**
+ * drm_of_get_dsi_bus - find the DSI bus for a given device
+ * @dev: parent device of display (SPI, I2C)
+ * @info: DSI device info to be updated with DSI node. This is optional
+ * and if not needed can be NULL.
+ *
+ * Gets parent DSI bus for a DSI device controlled through a bus other
+ * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
+ *
+ * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
+ * request is unsupported, -EPROBE_DEFER if the DSI host is found but
+ * not available, or -ENODEV otherwise.
+ */
+struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
+					 struct mipi_dsi_device_info *info)
+{
+	struct mipi_dsi_host *dsi_host;
+	struct device_node *endpoint, *dsi_host_node;
+
+	/*
+	 * Get first endpoint child from device.
+	 */
+	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
+	if (!endpoint)
+		return ERR_PTR(-ENODEV);
+
+	/*
+	 * Follow the first endpoint to get the DSI host node.
+	 */
+	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
+	if (!dsi_host_node)
+		goto error;
+
+	/*
+	 * Get the DSI host from the DSI host node. If we get an error
+	 * or the return is null assume we're not ready to probe just
+	 * yet. Release the DSI host node since we're done with it.
+	 */
+	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
+	of_node_put(dsi_host_node);
+	if (IS_ERR_OR_NULL(dsi_host)) {
+		of_node_put(endpoint);
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	/*
+	 * Set the node of the mipi_dsi_device_info to the correct node
+	 * and then release the endpoint node since we're done with it.
+	 * since this is optional, check if the info is NULL first.
+	 */
+	if (info) {
+		info->node = of_graph_get_remote_port(endpoint);
+		if (IS_ERR_OR_NULL(info->node))
+			goto error;
+	}
+
+	of_node_put(endpoint);
+	return dsi_host;
+
+error:
+	of_node_put(endpoint);
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL_GPL(drm_of_get_dsi_bus);
+
+#endif /* CONFIG_DRM_MIPI_DSI */
diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
index 10ab58c40746..705ea2caa494 100644
--- a/include/drm/drm_of.h
+++ b/include/drm/drm_of.h
@@ -15,6 +15,8 @@ struct drm_encoder;
 struct drm_panel;
 struct drm_bridge;
 struct device_node;
+struct mipi_dsi_device_info;
+struct mipi_dsi_host;
 
 /**
  * enum drm_lvds_dual_link_pixels - Pixel order of an LVDS dual-link connection
@@ -56,6 +58,8 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
 				   int port_reg, int reg,
 				   const unsigned int min,
 				   const unsigned int max);
+struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
+					 struct mipi_dsi_device_info *info);
 #else
 static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
 					  struct device_node *port)
@@ -127,6 +131,12 @@ drm_of_get_data_lanes_count_ep(const struct device_node *port,
 {
 	return -EINVAL;
 }
+static inline struct
+mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
+					 struct mipi_dsi_device_info *info)
+{
+	return ERR_PTR(-EINVAL);
+}
 #endif
 
 /*
-- 
2.34.1


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

* [PATCH V10 2/4] dt-bindings: display: panel: Add Samsung AMS495QA01
  2023-01-12 17:53 ` Chris Morgan
  (?)
@ 2023-01-12 17:53   ` Chris Morgan
  -1 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan, Rob Herring

From: Chris Morgan <macromorgan@hotmail.com>

Add documentation for Samsung AMS495QA01 panel (with Magnachip
D53E6EA8966 controller IC).

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../display/panel/samsung,ams495qa01.yaml     | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
new file mode 100644
index 000000000000..58fa073ce258
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/samsung,ams495qa01.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung AMS495QA01 panel with Magnachip D53E6EA8966 controller
+
+maintainers:
+  - Chris Morgan <macromorgan@hotmail.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: samsung,ams495qa01
+
+  reg: true
+  reset-gpios:
+    description: reset gpio, must be GPIO_ACTIVE_LOW
+  elvdd-supply:
+    description: regulator that supplies voltage to the panel display
+  enable-gpios: true
+  port: true
+  vdd-supply:
+    description: regulator that supplies voltage to panel logic
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "samsung,ams495qa01";
+            reg = <0>;
+            reset-gpios = <&gpio4 0 GPIO_ACTIVE_LOW>;
+            vdd-supply = <&vcc_3v3>;
+
+            port {
+                mipi_in_panel: endpoint {
+                  remote-endpoint = <&mipi_out_panel>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH V10 2/4] dt-bindings: display: panel: Add Samsung AMS495QA01
@ 2023-01-12 17:53   ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, Chris Morgan, krzysztof.kozlowski+dt, sam,
	linux-rockchip, robh+dt, thierry.reding, tzimmermann,
	maccraft123mc

From: Chris Morgan <macromorgan@hotmail.com>

Add documentation for Samsung AMS495QA01 panel (with Magnachip
D53E6EA8966 controller IC).

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../display/panel/samsung,ams495qa01.yaml     | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
new file mode 100644
index 000000000000..58fa073ce258
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/samsung,ams495qa01.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung AMS495QA01 panel with Magnachip D53E6EA8966 controller
+
+maintainers:
+  - Chris Morgan <macromorgan@hotmail.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: samsung,ams495qa01
+
+  reg: true
+  reset-gpios:
+    description: reset gpio, must be GPIO_ACTIVE_LOW
+  elvdd-supply:
+    description: regulator that supplies voltage to the panel display
+  enable-gpios: true
+  port: true
+  vdd-supply:
+    description: regulator that supplies voltage to panel logic
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "samsung,ams495qa01";
+            reg = <0>;
+            reset-gpios = <&gpio4 0 GPIO_ACTIVE_LOW>;
+            vdd-supply = <&vcc_3v3>;
+
+            port {
+                mipi_in_panel: endpoint {
+                  remote-endpoint = <&mipi_out_panel>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.34.1


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

* [PATCH V10 2/4] dt-bindings: display: panel: Add Samsung AMS495QA01
@ 2023-01-12 17:53   ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan, Rob Herring

From: Chris Morgan <macromorgan@hotmail.com>

Add documentation for Samsung AMS495QA01 panel (with Magnachip
D53E6EA8966 controller IC).

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../display/panel/samsung,ams495qa01.yaml     | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
new file mode 100644
index 000000000000..58fa073ce258
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/samsung,ams495qa01.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung AMS495QA01 panel with Magnachip D53E6EA8966 controller
+
+maintainers:
+  - Chris Morgan <macromorgan@hotmail.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: samsung,ams495qa01
+
+  reg: true
+  reset-gpios:
+    description: reset gpio, must be GPIO_ACTIVE_LOW
+  elvdd-supply:
+    description: regulator that supplies voltage to the panel display
+  enable-gpios: true
+  port: true
+  vdd-supply:
+    description: regulator that supplies voltage to panel logic
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "samsung,ams495qa01";
+            reg = <0>;
+            reset-gpios = <&gpio4 0 GPIO_ACTIVE_LOW>;
+            vdd-supply = <&vcc_3v3>;
+
+            port {
+                mipi_in_panel: endpoint {
+                  remote-endpoint = <&mipi_out_panel>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.34.1


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

* [PATCH V10 3/4] drm/panel: Add Magnachip D53E6EA8966 Panel Driver
  2023-01-12 17:53 ` Chris Morgan
  (?)
@ 2023-01-12 17:53   ` Chris Morgan
  -1 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Support Magnachip D53E6EA8966 based panels such as the Samsung
AMS495QA01 panel as found on the Anbernic RG503. Note this driver
supports only the AMS495QA01 today which receives video signals via DSI,
however it receives commands via 3-wire SPI using DBI.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../drm/panel/panel-magnachip-d53e6ea8966.c   | 522 ++++++++++++++++++
 3 files changed, 534 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 737edcdf9eef..204b84a83604 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -298,6 +298,17 @@ config DRM_PANEL_LG_LG4573
 	  Say Y here if you want to enable support for LG4573 RGB panel.
 	  To compile this driver as a module, choose M here.
 
+config DRM_PANEL_MAGNACHIP_D53E6EA8966
+	tristate "Magnachip D53E6EA8966 DSI panel"
+	depends on OF && SPI
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	select DRM_MIPI_DBI
+	help
+	  DRM panel driver for the Samsung AMS495QA01 panel controlled
+	  with the Magnachip D53E6EA8966 panel IC. This panel receives
+	  video data via DSI but commands via 9-bit SPI using DBI.
+
 config DRM_PANEL_NEC_NL8048HL11
 	tristate "NEC NL8048HL11 RGB panel"
 	depends on GPIOLIB && OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index f8f9d9f6a307..20de312aa5e9 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
 obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
 obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
+obj-$(CONFIG_DRM_PANEL_MAGNACHIP_D53E6EA8966) += panel-magnachip-d53e6ea8966.o
 obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
 obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3051D) += panel-newvision-nv3051d.o
 obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
diff --git a/drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c b/drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c
new file mode 100644
index 000000000000..09ee12f0a147
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c
@@ -0,0 +1,522 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Magnachip d53e6ea8966 MIPI-DSI panel driver
+ * Copyright (C) 2023 Chris Morgan
+ */
+
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/media-bus-format.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+/* Forward declaration for use in backlight function */
+struct d53e6ea8966;
+
+/* Panel info, unique to each panel */
+struct d53e6ea8966_panel_info {
+	/** @display_modes: the supported display modes */
+	const struct drm_display_mode *display_modes;
+	/** @num_modes: the number of supported display modes */
+	unsigned int num_modes;
+	/** @width_mm: panel width in mm */
+	u16 width_mm;
+	/** @height_mm: panel height in mm */
+	u16 height_mm;
+	/** @bus_flags: drm bus flags for panel */
+	u32 bus_flags;
+	/** @panel_init_seq: panel specific init sequence */
+	void (*panel_init_seq)(struct d53e6ea8966 *db);
+	/** @backlight_register: panel backlight registration or NULL */
+	int (*backlight_register)(struct d53e6ea8966 *db);
+};
+
+struct d53e6ea8966 {
+	/** @dev: the container device */
+	struct device *dev;
+	/** @dbi: the DBI bus abstraction handle */
+	struct mipi_dbi dbi;
+	/** @panel: the DRM panel instance for this device */
+	struct drm_panel panel;
+	/** @reset: reset GPIO line */
+	struct gpio_desc *reset;
+	/** @enable: enable GPIO line */
+	struct gpio_desc *enable;
+	/** @reg_vdd: VDD supply regulator for panel logic */
+	struct regulator *reg_vdd;
+	/** @reg_elvdd: ELVDD supply regulator for panel display */
+	struct regulator *reg_elvdd;
+	/** @dsi_dev: DSI child device (panel) */
+	struct mipi_dsi_device *dsi_dev;
+	/** @bl_dev: pseudo-backlight device for oled panel */
+	struct backlight_device *bl_dev;
+	/** @panel_info: struct containing panel timing and info */
+	const struct d53e6ea8966_panel_info *panel_info;
+};
+
+#define NUM_GAMMA_LEVELS	16
+#define GAMMA_TABLE_COUNT	23
+#define MAX_BRIGHTNESS		(NUM_GAMMA_LEVELS - 1)
+
+#define MCS_ELVSS_ON			0xb1
+#define MCS_TEMP_SWIRE			0xb2
+#define MCS_PASSWORD_0			0xf0
+#define MCS_PASSWORD_1			0xf1
+#define MCS_ANALOG_PWR_CTL_0		0xf4
+#define MCS_ANALOG_PWR_CTL_1		0xf5
+#define MCS_GTCON_SET			0xf7
+#define MCS_GATELESS_SIGNAL_SET		0xf8
+#define MCS_SET_GAMMA			0xf9
+
+static inline struct d53e6ea8966 *to_d53e6ea8966(struct drm_panel *panel)
+{
+	return container_of(panel, struct d53e6ea8966, panel);
+}
+
+/* Table of gamma values provided in datasheet */
+static u8 ams495qa01_gamma[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
+	{0x01, 0x79, 0x78, 0x8d, 0xd9, 0xdf, 0xd5, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe0, 0xe4, 0xdc, 0xb8, 0xd4, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7d, 0x7c, 0x92, 0xd7, 0xdd, 0xd2, 0xcb, 0xd0, 0xc6,
+	 0xe5, 0xe1, 0xe3, 0xda, 0xbd, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7f, 0x7e, 0x95, 0xd7, 0xde, 0xd2, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe3, 0xe3, 0xda, 0xbf, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x82, 0x81, 0x99, 0xd6, 0xdd, 0xd1, 0xca, 0xcf, 0xc3,
+	 0xe4, 0xe3, 0xe3, 0xda, 0xc2, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x84, 0x83, 0x9b, 0xd7, 0xde, 0xd2, 0xc8, 0xce, 0xc2,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc3, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x87, 0x86, 0x9f, 0xd6, 0xdd, 0xd1, 0xc7, 0xce, 0xc1,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc6, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x89, 0x89, 0xa2, 0xd5, 0xdb, 0xcf, 0xc8, 0xcf, 0xc2,
+	 0xe3, 0xe3, 0xe1, 0xd9, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8b, 0x8b, 0xa5, 0xd5, 0xdb, 0xcf, 0xc7, 0xce, 0xc0,
+	 0xe3, 0xe3, 0xe1, 0xd8, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8d, 0x8d, 0xa7, 0xd5, 0xdb, 0xcf, 0xc6, 0xce, 0xc0,
+	 0xe4, 0xe4, 0xe1, 0xd7, 0xc8, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8f, 0x8f, 0xaa, 0xd4, 0xdb, 0xce, 0xc6, 0xcd, 0xbf,
+	 0xe3, 0xe3, 0xe1, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x91, 0x91, 0xac, 0xd3, 0xda, 0xce, 0xc5, 0xcd, 0xbe,
+	 0xe3, 0xe3, 0xe0, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x93, 0x93, 0xaf, 0xd3, 0xda, 0xcd, 0xc5, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd6, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x95, 0x95, 0xb1, 0xd2, 0xd9, 0xcc, 0xc4, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd7, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x99, 0x99, 0xb6, 0xd1, 0xd9, 0xcc, 0xc3, 0xcb, 0xbc,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9c, 0x9c, 0xba, 0xd0, 0xd8, 0xcb, 0xc3, 0xcb, 0xbb,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xce, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9f, 0x9f, 0xbe, 0xcf, 0xd7, 0xc9, 0xc2, 0xcb, 0xbb,
+	 0xe1, 0xe3, 0xde, 0xd6, 0xd0, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+};
+
+/*
+ * Table of elvss values provided in datasheet and corresponds to
+ * gamma values.
+ */
+static u8 ams495qa01_elvss[NUM_GAMMA_LEVELS] = {
+	0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15,
+	0x15, 0x15, 0x14, 0x14, 0x13, 0x12,
+};
+
+static int ams495qa01_update_gamma(struct mipi_dbi *dbi, int brightness)
+{
+	int tmp = brightness;
+
+	mipi_dbi_command_buf(dbi, MCS_SET_GAMMA, ams495qa01_gamma[tmp],
+			     ARRAY_SIZE(ams495qa01_gamma[tmp]));
+	mipi_dbi_command(dbi, MCS_SET_GAMMA, 0x00);
+
+	/* Undocumented command */
+	mipi_dbi_command(dbi, 0x26, 0x00);
+
+	mipi_dbi_command(dbi, MCS_TEMP_SWIRE, ams495qa01_elvss[tmp]);
+
+	return 0;
+}
+
+static void ams495qa01_panel_init(struct d53e6ea8966 *db)
+{
+	struct mipi_dbi *dbi = &db->dbi;
+
+	mipi_dbi_command(dbi, MCS_PASSWORD_0, 0x5a, 0x5a);
+	mipi_dbi_command(dbi, MCS_PASSWORD_1, 0x5a, 0x5a);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb0, 0x02);
+	mipi_dbi_command(dbi, 0xf3, 0x3b);
+
+	mipi_dbi_command(dbi, MCS_ANALOG_PWR_CTL_0, 0x33, 0x42, 0x00, 0x08);
+	mipi_dbi_command(dbi, MCS_ANALOG_PWR_CTL_1, 0x00, 0x06, 0x26, 0x35, 0x03);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xf6, 0x02);
+	mipi_dbi_command(dbi, 0xc6, 0x0b, 0x00, 0x00, 0x3c, 0x00, 0x22,
+			 0x00, 0x00, 0x00, 0x00);
+
+	mipi_dbi_command(dbi, MCS_GTCON_SET, 0x20);
+	mipi_dbi_command(dbi, MCS_TEMP_SWIRE, 0x06, 0x06, 0x06, 0x06);
+	mipi_dbi_command(dbi, MCS_ELVSS_ON, 0x07, 0x00, 0x10);
+	mipi_dbi_command(dbi, MCS_GATELESS_SIGNAL_SET, 0x7f, 0x7a,
+			 0x89, 0x67, 0x26, 0x38, 0x00, 0x00, 0x09,
+			 0x67, 0x70, 0x88, 0x7a, 0x76, 0x05, 0x09,
+			 0x23, 0x23, 0x23);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb5, 0xff, 0xef, 0x35, 0x42, 0x0d, 0xd7,
+			 0xff, 0x07, 0xff, 0xff, 0xfd, 0x00, 0x01,
+			 0xff, 0x05, 0x12, 0x0f, 0xff, 0xff, 0xff,
+			 0xff);
+	mipi_dbi_command(dbi, 0xb4, 0x15);
+	mipi_dbi_command(dbi, 0xb3, 0x00);
+
+	ams495qa01_update_gamma(dbi, MAX_BRIGHTNESS);
+}
+
+static int d53e6ea8966_prepare(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	int ret;
+
+	/* Power up */
+	ret = regulator_enable(db->reg_vdd);
+	if (ret) {
+		dev_err(db->dev, "failed to enable vdd regulator: %d\n", ret);
+		return ret;
+	}
+
+	if (db->reg_elvdd) {
+		ret = regulator_enable(db->reg_elvdd);
+		if (ret) {
+			dev_err(db->dev,
+				"failed to enable elvdd regulator: %d\n", ret);
+			regulator_disable(db->reg_vdd);
+			return ret;
+		}
+	}
+
+	/* Enable */
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 1);
+
+	msleep(50);
+
+	/* Reset */
+	gpiod_set_value_cansleep(db->reset, 1);
+	usleep_range(1000, 5000);
+	gpiod_set_value_cansleep(db->reset, 0);
+	msleep(20);
+
+	db->panel_info->panel_init_seq(db);
+
+	return 0;
+}
+
+static int d53e6ea8966_enable(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(200);
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
+	usleep_range(10000, 15000);
+
+	return 0;
+}
+
+static int d53e6ea8966_disable(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(20);
+	mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
+	msleep(100);
+
+	return 0;
+}
+
+static int d53e6ea8966_unprepare(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 0);
+
+	gpiod_set_value_cansleep(db->reset, 1);
+
+	if (db->reg_elvdd)
+		regulator_disable(db->reg_elvdd);
+
+	regulator_disable(db->reg_vdd);
+	msleep(100);
+
+	return 0;
+}
+
+static int d53e6ea8966_get_modes(struct drm_panel *panel,
+				struct drm_connector *connector)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	const struct d53e6ea8966_panel_info *panel_info = db->panel_info;
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	unsigned int i;
+
+	for (i = 0; i < panel_info->num_modes; i++) {
+		mode = drm_mode_duplicate(connector->dev,
+					  &panel_info->display_modes[i]);
+		if (!mode)
+			return -ENOMEM;
+
+		drm_mode_set_name(mode);
+		drm_mode_probed_add(connector, mode);
+	}
+
+	connector->display_info.bpc = 8;
+	connector->display_info.width_mm = panel_info->width_mm;
+	connector->display_info.height_mm = panel_info->height_mm;
+	connector->display_info.bus_flags = panel_info->bus_flags;
+
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 &bus_format, 1);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs d53e6ea8966_panel_funcs = {
+	.disable = d53e6ea8966_disable,
+	.enable = d53e6ea8966_enable,
+	.get_modes = d53e6ea8966_get_modes,
+	.prepare = d53e6ea8966_prepare,
+	.unprepare = d53e6ea8966_unprepare,
+};
+
+static int ams495qa01_set_brightness(struct backlight_device *bd)
+{
+	struct d53e6ea8966 *db = bl_get_data(bd);
+	struct mipi_dbi *dbi = &db->dbi;
+	int brightness = backlight_get_brightness(bd);
+
+	ams495qa01_update_gamma(dbi, brightness);
+
+	return 0;
+}
+
+static const struct backlight_ops ams495qa01_backlight_ops = {
+	.update_status	= ams495qa01_set_brightness,
+};
+
+static int ams495qa01_backlight_register(struct d53e6ea8966 *db)
+{
+	struct backlight_properties props = {
+		.type		= BACKLIGHT_RAW,
+		.brightness	= MAX_BRIGHTNESS,
+		.max_brightness = MAX_BRIGHTNESS,
+	};
+	struct device *dev = db->dev;
+	int ret = 0;
+
+	db->bl_dev = devm_backlight_device_register(dev, "panel", dev, db,
+						     &ams495qa01_backlight_ops,
+						     &props);
+	if (IS_ERR(db->bl_dev)) {
+		ret = PTR_ERR(db->bl_dev);
+		dev_err(dev, "error registering backlight device (%d)\n", ret);
+	}
+
+	return ret;
+}
+
+static int d53e6ea8966_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct mipi_dsi_host *dsi_host;
+	struct d53e6ea8966 *db;
+	int ret;
+	struct mipi_dsi_device_info info = {
+		.type = "d53e6ea8966",
+		.channel = 0,
+		.node = NULL,
+	};
+
+	db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
+	if (!db)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, db);
+
+	db->dev = dev;
+
+	db->panel_info = of_device_get_match_data(dev);
+	if (!db->panel_info)
+		return -EINVAL;
+
+	db->reg_vdd = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(db->reg_vdd))
+		return dev_err_probe(dev, PTR_ERR(db->reg_vdd),
+				     "Failed to get vdd supply\n");
+
+	db->reg_elvdd = devm_regulator_get_optional(dev, "elvdd");
+	if (IS_ERR(db->reg_elvdd))
+		db->reg_elvdd = NULL;
+
+	db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(db->reset)) {
+		ret = PTR_ERR(db->reset);
+		return dev_err_probe(dev, ret, "no RESET GPIO\n");
+	}
+
+	db->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(db->enable)) {
+		ret = PTR_ERR(db->enable);
+		return dev_err_probe(dev, ret, "cannot get ENABLE GPIO\n");
+	}
+
+	ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
+
+	dsi_host = drm_of_get_dsi_bus(dev, &info);
+	if (IS_ERR(dsi_host)) {
+		ret = PTR_ERR(dsi_host);
+		return dev_err_probe(dev, ret, "Error attaching DSI bus\n");
+	}
+
+	db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
+	if (IS_ERR(db->dsi_dev)) {
+		dev_err(dev, "failed to register dsi device: %ld\n",
+			PTR_ERR(db->dsi_dev));
+		ret = PTR_ERR(db->dsi_dev);
+	}
+
+	db->dsi_dev->lanes = 2;
+	db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
+	db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
+
+	drm_panel_init(&db->panel, dev, &d53e6ea8966_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+
+	if (db->panel_info->backlight_register != NULL) {
+		ret = db->panel_info->backlight_register(db);
+		if (ret < 0)
+			return ret;
+		db->panel.backlight = db->bl_dev;
+	}
+
+	drm_panel_add(&db->panel);
+
+	ret = devm_mipi_dsi_attach(dev, db->dsi_dev);
+	if (ret < 0) {
+		dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
+		drm_panel_remove(&db->panel);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void d53e6ea8966_remove(struct spi_device *spi)
+{
+	struct d53e6ea8966 *db = spi_get_drvdata(spi);
+
+	drm_panel_remove(&db->panel);
+}
+
+static const struct drm_display_mode ams495qa01_modes[] = {
+	{ /* 60hz */
+		.clock = 33500,
+		.hdisplay = 960,
+		.hsync_start = 960 + 10,
+		.hsync_end = 960 + 10 + 2,
+		.htotal = 960 + 10 + 2 + 10,
+		.vdisplay = 544,
+		.vsync_start = 544 + 10,
+		.vsync_end = 544 + 10 + 2,
+		.vtotal = 544 + 10 + 2 + 10,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+		.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+		},
+	{ /* 50hz */
+		.clock = 27800,
+		.hdisplay = 960,
+		.hsync_start = 960 + 10,
+		.hsync_end = 960 + 10 + 2,
+		.htotal = 960 + 10 + 2 + 10,
+		.vdisplay = 544,
+		.vsync_start = 544 + 10,
+		.vsync_end = 544 + 10 + 2,
+		.vtotal = 544 + 10 + 2 + 10,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+		.type = DRM_MODE_TYPE_DRIVER,
+	},
+};
+
+static const struct d53e6ea8966_panel_info ams495qa01_info = {
+	.display_modes = ams495qa01_modes,
+	.num_modes = ARRAY_SIZE(ams495qa01_modes),
+	.width_mm = 117,
+	.height_mm = 74,
+	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+	.panel_init_seq = ams495qa01_panel_init,
+	.backlight_register = ams495qa01_backlight_register,
+};
+
+static const struct of_device_id d53e6ea8966_match[] = {
+	{ .compatible = "samsung,ams495qa01", .data = &ams495qa01_info },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, d53e6ea8966_match);
+
+static const struct spi_device_id d53e6ea8966_ids[] = {
+	{ "ams495qa01", 0 },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(spi, d53e6ea8966_ids);
+
+static struct spi_driver d53e6ea8966_driver = {
+	.driver		= {
+		.name	= "d53e6ea8966-panel",
+		.of_match_table = d53e6ea8966_match,
+	},
+	.id_table	= d53e6ea8966_ids,
+	.probe		= d53e6ea8966_probe,
+	.remove		= d53e6ea8966_remove,
+};
+module_spi_driver(d53e6ea8966_driver);
+
+MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
+MODULE_DESCRIPTION("Magnachip d53e6ea8966 panel driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH V10 3/4] drm/panel: Add Magnachip D53E6EA8966 Panel Driver
@ 2023-01-12 17:53   ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, Chris Morgan, krzysztof.kozlowski+dt, sam,
	linux-rockchip, robh+dt, thierry.reding, tzimmermann,
	maccraft123mc

From: Chris Morgan <macromorgan@hotmail.com>

Support Magnachip D53E6EA8966 based panels such as the Samsung
AMS495QA01 panel as found on the Anbernic RG503. Note this driver
supports only the AMS495QA01 today which receives video signals via DSI,
however it receives commands via 3-wire SPI using DBI.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../drm/panel/panel-magnachip-d53e6ea8966.c   | 522 ++++++++++++++++++
 3 files changed, 534 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 737edcdf9eef..204b84a83604 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -298,6 +298,17 @@ config DRM_PANEL_LG_LG4573
 	  Say Y here if you want to enable support for LG4573 RGB panel.
 	  To compile this driver as a module, choose M here.
 
+config DRM_PANEL_MAGNACHIP_D53E6EA8966
+	tristate "Magnachip D53E6EA8966 DSI panel"
+	depends on OF && SPI
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	select DRM_MIPI_DBI
+	help
+	  DRM panel driver for the Samsung AMS495QA01 panel controlled
+	  with the Magnachip D53E6EA8966 panel IC. This panel receives
+	  video data via DSI but commands via 9-bit SPI using DBI.
+
 config DRM_PANEL_NEC_NL8048HL11
 	tristate "NEC NL8048HL11 RGB panel"
 	depends on GPIOLIB && OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index f8f9d9f6a307..20de312aa5e9 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
 obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
 obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
+obj-$(CONFIG_DRM_PANEL_MAGNACHIP_D53E6EA8966) += panel-magnachip-d53e6ea8966.o
 obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
 obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3051D) += panel-newvision-nv3051d.o
 obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
diff --git a/drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c b/drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c
new file mode 100644
index 000000000000..09ee12f0a147
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c
@@ -0,0 +1,522 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Magnachip d53e6ea8966 MIPI-DSI panel driver
+ * Copyright (C) 2023 Chris Morgan
+ */
+
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/media-bus-format.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+/* Forward declaration for use in backlight function */
+struct d53e6ea8966;
+
+/* Panel info, unique to each panel */
+struct d53e6ea8966_panel_info {
+	/** @display_modes: the supported display modes */
+	const struct drm_display_mode *display_modes;
+	/** @num_modes: the number of supported display modes */
+	unsigned int num_modes;
+	/** @width_mm: panel width in mm */
+	u16 width_mm;
+	/** @height_mm: panel height in mm */
+	u16 height_mm;
+	/** @bus_flags: drm bus flags for panel */
+	u32 bus_flags;
+	/** @panel_init_seq: panel specific init sequence */
+	void (*panel_init_seq)(struct d53e6ea8966 *db);
+	/** @backlight_register: panel backlight registration or NULL */
+	int (*backlight_register)(struct d53e6ea8966 *db);
+};
+
+struct d53e6ea8966 {
+	/** @dev: the container device */
+	struct device *dev;
+	/** @dbi: the DBI bus abstraction handle */
+	struct mipi_dbi dbi;
+	/** @panel: the DRM panel instance for this device */
+	struct drm_panel panel;
+	/** @reset: reset GPIO line */
+	struct gpio_desc *reset;
+	/** @enable: enable GPIO line */
+	struct gpio_desc *enable;
+	/** @reg_vdd: VDD supply regulator for panel logic */
+	struct regulator *reg_vdd;
+	/** @reg_elvdd: ELVDD supply regulator for panel display */
+	struct regulator *reg_elvdd;
+	/** @dsi_dev: DSI child device (panel) */
+	struct mipi_dsi_device *dsi_dev;
+	/** @bl_dev: pseudo-backlight device for oled panel */
+	struct backlight_device *bl_dev;
+	/** @panel_info: struct containing panel timing and info */
+	const struct d53e6ea8966_panel_info *panel_info;
+};
+
+#define NUM_GAMMA_LEVELS	16
+#define GAMMA_TABLE_COUNT	23
+#define MAX_BRIGHTNESS		(NUM_GAMMA_LEVELS - 1)
+
+#define MCS_ELVSS_ON			0xb1
+#define MCS_TEMP_SWIRE			0xb2
+#define MCS_PASSWORD_0			0xf0
+#define MCS_PASSWORD_1			0xf1
+#define MCS_ANALOG_PWR_CTL_0		0xf4
+#define MCS_ANALOG_PWR_CTL_1		0xf5
+#define MCS_GTCON_SET			0xf7
+#define MCS_GATELESS_SIGNAL_SET		0xf8
+#define MCS_SET_GAMMA			0xf9
+
+static inline struct d53e6ea8966 *to_d53e6ea8966(struct drm_panel *panel)
+{
+	return container_of(panel, struct d53e6ea8966, panel);
+}
+
+/* Table of gamma values provided in datasheet */
+static u8 ams495qa01_gamma[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
+	{0x01, 0x79, 0x78, 0x8d, 0xd9, 0xdf, 0xd5, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe0, 0xe4, 0xdc, 0xb8, 0xd4, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7d, 0x7c, 0x92, 0xd7, 0xdd, 0xd2, 0xcb, 0xd0, 0xc6,
+	 0xe5, 0xe1, 0xe3, 0xda, 0xbd, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7f, 0x7e, 0x95, 0xd7, 0xde, 0xd2, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe3, 0xe3, 0xda, 0xbf, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x82, 0x81, 0x99, 0xd6, 0xdd, 0xd1, 0xca, 0xcf, 0xc3,
+	 0xe4, 0xe3, 0xe3, 0xda, 0xc2, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x84, 0x83, 0x9b, 0xd7, 0xde, 0xd2, 0xc8, 0xce, 0xc2,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc3, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x87, 0x86, 0x9f, 0xd6, 0xdd, 0xd1, 0xc7, 0xce, 0xc1,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc6, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x89, 0x89, 0xa2, 0xd5, 0xdb, 0xcf, 0xc8, 0xcf, 0xc2,
+	 0xe3, 0xe3, 0xe1, 0xd9, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8b, 0x8b, 0xa5, 0xd5, 0xdb, 0xcf, 0xc7, 0xce, 0xc0,
+	 0xe3, 0xe3, 0xe1, 0xd8, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8d, 0x8d, 0xa7, 0xd5, 0xdb, 0xcf, 0xc6, 0xce, 0xc0,
+	 0xe4, 0xe4, 0xe1, 0xd7, 0xc8, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8f, 0x8f, 0xaa, 0xd4, 0xdb, 0xce, 0xc6, 0xcd, 0xbf,
+	 0xe3, 0xe3, 0xe1, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x91, 0x91, 0xac, 0xd3, 0xda, 0xce, 0xc5, 0xcd, 0xbe,
+	 0xe3, 0xe3, 0xe0, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x93, 0x93, 0xaf, 0xd3, 0xda, 0xcd, 0xc5, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd6, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x95, 0x95, 0xb1, 0xd2, 0xd9, 0xcc, 0xc4, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd7, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x99, 0x99, 0xb6, 0xd1, 0xd9, 0xcc, 0xc3, 0xcb, 0xbc,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9c, 0x9c, 0xba, 0xd0, 0xd8, 0xcb, 0xc3, 0xcb, 0xbb,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xce, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9f, 0x9f, 0xbe, 0xcf, 0xd7, 0xc9, 0xc2, 0xcb, 0xbb,
+	 0xe1, 0xe3, 0xde, 0xd6, 0xd0, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+};
+
+/*
+ * Table of elvss values provided in datasheet and corresponds to
+ * gamma values.
+ */
+static u8 ams495qa01_elvss[NUM_GAMMA_LEVELS] = {
+	0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15,
+	0x15, 0x15, 0x14, 0x14, 0x13, 0x12,
+};
+
+static int ams495qa01_update_gamma(struct mipi_dbi *dbi, int brightness)
+{
+	int tmp = brightness;
+
+	mipi_dbi_command_buf(dbi, MCS_SET_GAMMA, ams495qa01_gamma[tmp],
+			     ARRAY_SIZE(ams495qa01_gamma[tmp]));
+	mipi_dbi_command(dbi, MCS_SET_GAMMA, 0x00);
+
+	/* Undocumented command */
+	mipi_dbi_command(dbi, 0x26, 0x00);
+
+	mipi_dbi_command(dbi, MCS_TEMP_SWIRE, ams495qa01_elvss[tmp]);
+
+	return 0;
+}
+
+static void ams495qa01_panel_init(struct d53e6ea8966 *db)
+{
+	struct mipi_dbi *dbi = &db->dbi;
+
+	mipi_dbi_command(dbi, MCS_PASSWORD_0, 0x5a, 0x5a);
+	mipi_dbi_command(dbi, MCS_PASSWORD_1, 0x5a, 0x5a);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb0, 0x02);
+	mipi_dbi_command(dbi, 0xf3, 0x3b);
+
+	mipi_dbi_command(dbi, MCS_ANALOG_PWR_CTL_0, 0x33, 0x42, 0x00, 0x08);
+	mipi_dbi_command(dbi, MCS_ANALOG_PWR_CTL_1, 0x00, 0x06, 0x26, 0x35, 0x03);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xf6, 0x02);
+	mipi_dbi_command(dbi, 0xc6, 0x0b, 0x00, 0x00, 0x3c, 0x00, 0x22,
+			 0x00, 0x00, 0x00, 0x00);
+
+	mipi_dbi_command(dbi, MCS_GTCON_SET, 0x20);
+	mipi_dbi_command(dbi, MCS_TEMP_SWIRE, 0x06, 0x06, 0x06, 0x06);
+	mipi_dbi_command(dbi, MCS_ELVSS_ON, 0x07, 0x00, 0x10);
+	mipi_dbi_command(dbi, MCS_GATELESS_SIGNAL_SET, 0x7f, 0x7a,
+			 0x89, 0x67, 0x26, 0x38, 0x00, 0x00, 0x09,
+			 0x67, 0x70, 0x88, 0x7a, 0x76, 0x05, 0x09,
+			 0x23, 0x23, 0x23);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb5, 0xff, 0xef, 0x35, 0x42, 0x0d, 0xd7,
+			 0xff, 0x07, 0xff, 0xff, 0xfd, 0x00, 0x01,
+			 0xff, 0x05, 0x12, 0x0f, 0xff, 0xff, 0xff,
+			 0xff);
+	mipi_dbi_command(dbi, 0xb4, 0x15);
+	mipi_dbi_command(dbi, 0xb3, 0x00);
+
+	ams495qa01_update_gamma(dbi, MAX_BRIGHTNESS);
+}
+
+static int d53e6ea8966_prepare(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	int ret;
+
+	/* Power up */
+	ret = regulator_enable(db->reg_vdd);
+	if (ret) {
+		dev_err(db->dev, "failed to enable vdd regulator: %d\n", ret);
+		return ret;
+	}
+
+	if (db->reg_elvdd) {
+		ret = regulator_enable(db->reg_elvdd);
+		if (ret) {
+			dev_err(db->dev,
+				"failed to enable elvdd regulator: %d\n", ret);
+			regulator_disable(db->reg_vdd);
+			return ret;
+		}
+	}
+
+	/* Enable */
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 1);
+
+	msleep(50);
+
+	/* Reset */
+	gpiod_set_value_cansleep(db->reset, 1);
+	usleep_range(1000, 5000);
+	gpiod_set_value_cansleep(db->reset, 0);
+	msleep(20);
+
+	db->panel_info->panel_init_seq(db);
+
+	return 0;
+}
+
+static int d53e6ea8966_enable(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(200);
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
+	usleep_range(10000, 15000);
+
+	return 0;
+}
+
+static int d53e6ea8966_disable(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(20);
+	mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
+	msleep(100);
+
+	return 0;
+}
+
+static int d53e6ea8966_unprepare(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 0);
+
+	gpiod_set_value_cansleep(db->reset, 1);
+
+	if (db->reg_elvdd)
+		regulator_disable(db->reg_elvdd);
+
+	regulator_disable(db->reg_vdd);
+	msleep(100);
+
+	return 0;
+}
+
+static int d53e6ea8966_get_modes(struct drm_panel *panel,
+				struct drm_connector *connector)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	const struct d53e6ea8966_panel_info *panel_info = db->panel_info;
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	unsigned int i;
+
+	for (i = 0; i < panel_info->num_modes; i++) {
+		mode = drm_mode_duplicate(connector->dev,
+					  &panel_info->display_modes[i]);
+		if (!mode)
+			return -ENOMEM;
+
+		drm_mode_set_name(mode);
+		drm_mode_probed_add(connector, mode);
+	}
+
+	connector->display_info.bpc = 8;
+	connector->display_info.width_mm = panel_info->width_mm;
+	connector->display_info.height_mm = panel_info->height_mm;
+	connector->display_info.bus_flags = panel_info->bus_flags;
+
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 &bus_format, 1);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs d53e6ea8966_panel_funcs = {
+	.disable = d53e6ea8966_disable,
+	.enable = d53e6ea8966_enable,
+	.get_modes = d53e6ea8966_get_modes,
+	.prepare = d53e6ea8966_prepare,
+	.unprepare = d53e6ea8966_unprepare,
+};
+
+static int ams495qa01_set_brightness(struct backlight_device *bd)
+{
+	struct d53e6ea8966 *db = bl_get_data(bd);
+	struct mipi_dbi *dbi = &db->dbi;
+	int brightness = backlight_get_brightness(bd);
+
+	ams495qa01_update_gamma(dbi, brightness);
+
+	return 0;
+}
+
+static const struct backlight_ops ams495qa01_backlight_ops = {
+	.update_status	= ams495qa01_set_brightness,
+};
+
+static int ams495qa01_backlight_register(struct d53e6ea8966 *db)
+{
+	struct backlight_properties props = {
+		.type		= BACKLIGHT_RAW,
+		.brightness	= MAX_BRIGHTNESS,
+		.max_brightness = MAX_BRIGHTNESS,
+	};
+	struct device *dev = db->dev;
+	int ret = 0;
+
+	db->bl_dev = devm_backlight_device_register(dev, "panel", dev, db,
+						     &ams495qa01_backlight_ops,
+						     &props);
+	if (IS_ERR(db->bl_dev)) {
+		ret = PTR_ERR(db->bl_dev);
+		dev_err(dev, "error registering backlight device (%d)\n", ret);
+	}
+
+	return ret;
+}
+
+static int d53e6ea8966_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct mipi_dsi_host *dsi_host;
+	struct d53e6ea8966 *db;
+	int ret;
+	struct mipi_dsi_device_info info = {
+		.type = "d53e6ea8966",
+		.channel = 0,
+		.node = NULL,
+	};
+
+	db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
+	if (!db)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, db);
+
+	db->dev = dev;
+
+	db->panel_info = of_device_get_match_data(dev);
+	if (!db->panel_info)
+		return -EINVAL;
+
+	db->reg_vdd = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(db->reg_vdd))
+		return dev_err_probe(dev, PTR_ERR(db->reg_vdd),
+				     "Failed to get vdd supply\n");
+
+	db->reg_elvdd = devm_regulator_get_optional(dev, "elvdd");
+	if (IS_ERR(db->reg_elvdd))
+		db->reg_elvdd = NULL;
+
+	db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(db->reset)) {
+		ret = PTR_ERR(db->reset);
+		return dev_err_probe(dev, ret, "no RESET GPIO\n");
+	}
+
+	db->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(db->enable)) {
+		ret = PTR_ERR(db->enable);
+		return dev_err_probe(dev, ret, "cannot get ENABLE GPIO\n");
+	}
+
+	ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
+
+	dsi_host = drm_of_get_dsi_bus(dev, &info);
+	if (IS_ERR(dsi_host)) {
+		ret = PTR_ERR(dsi_host);
+		return dev_err_probe(dev, ret, "Error attaching DSI bus\n");
+	}
+
+	db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
+	if (IS_ERR(db->dsi_dev)) {
+		dev_err(dev, "failed to register dsi device: %ld\n",
+			PTR_ERR(db->dsi_dev));
+		ret = PTR_ERR(db->dsi_dev);
+	}
+
+	db->dsi_dev->lanes = 2;
+	db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
+	db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
+
+	drm_panel_init(&db->panel, dev, &d53e6ea8966_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+
+	if (db->panel_info->backlight_register != NULL) {
+		ret = db->panel_info->backlight_register(db);
+		if (ret < 0)
+			return ret;
+		db->panel.backlight = db->bl_dev;
+	}
+
+	drm_panel_add(&db->panel);
+
+	ret = devm_mipi_dsi_attach(dev, db->dsi_dev);
+	if (ret < 0) {
+		dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
+		drm_panel_remove(&db->panel);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void d53e6ea8966_remove(struct spi_device *spi)
+{
+	struct d53e6ea8966 *db = spi_get_drvdata(spi);
+
+	drm_panel_remove(&db->panel);
+}
+
+static const struct drm_display_mode ams495qa01_modes[] = {
+	{ /* 60hz */
+		.clock = 33500,
+		.hdisplay = 960,
+		.hsync_start = 960 + 10,
+		.hsync_end = 960 + 10 + 2,
+		.htotal = 960 + 10 + 2 + 10,
+		.vdisplay = 544,
+		.vsync_start = 544 + 10,
+		.vsync_end = 544 + 10 + 2,
+		.vtotal = 544 + 10 + 2 + 10,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+		.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+		},
+	{ /* 50hz */
+		.clock = 27800,
+		.hdisplay = 960,
+		.hsync_start = 960 + 10,
+		.hsync_end = 960 + 10 + 2,
+		.htotal = 960 + 10 + 2 + 10,
+		.vdisplay = 544,
+		.vsync_start = 544 + 10,
+		.vsync_end = 544 + 10 + 2,
+		.vtotal = 544 + 10 + 2 + 10,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+		.type = DRM_MODE_TYPE_DRIVER,
+	},
+};
+
+static const struct d53e6ea8966_panel_info ams495qa01_info = {
+	.display_modes = ams495qa01_modes,
+	.num_modes = ARRAY_SIZE(ams495qa01_modes),
+	.width_mm = 117,
+	.height_mm = 74,
+	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+	.panel_init_seq = ams495qa01_panel_init,
+	.backlight_register = ams495qa01_backlight_register,
+};
+
+static const struct of_device_id d53e6ea8966_match[] = {
+	{ .compatible = "samsung,ams495qa01", .data = &ams495qa01_info },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, d53e6ea8966_match);
+
+static const struct spi_device_id d53e6ea8966_ids[] = {
+	{ "ams495qa01", 0 },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(spi, d53e6ea8966_ids);
+
+static struct spi_driver d53e6ea8966_driver = {
+	.driver		= {
+		.name	= "d53e6ea8966-panel",
+		.of_match_table = d53e6ea8966_match,
+	},
+	.id_table	= d53e6ea8966_ids,
+	.probe		= d53e6ea8966_probe,
+	.remove		= d53e6ea8966_remove,
+};
+module_spi_driver(d53e6ea8966_driver);
+
+MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
+MODULE_DESCRIPTION("Magnachip d53e6ea8966 panel driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1


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

* [PATCH V10 3/4] drm/panel: Add Magnachip D53E6EA8966 Panel Driver
@ 2023-01-12 17:53   ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Support Magnachip D53E6EA8966 based panels such as the Samsung
AMS495QA01 panel as found on the Anbernic RG503. Note this driver
supports only the AMS495QA01 today which receives video signals via DSI,
however it receives commands via 3-wire SPI using DBI.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../drm/panel/panel-magnachip-d53e6ea8966.c   | 522 ++++++++++++++++++
 3 files changed, 534 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 737edcdf9eef..204b84a83604 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -298,6 +298,17 @@ config DRM_PANEL_LG_LG4573
 	  Say Y here if you want to enable support for LG4573 RGB panel.
 	  To compile this driver as a module, choose M here.
 
+config DRM_PANEL_MAGNACHIP_D53E6EA8966
+	tristate "Magnachip D53E6EA8966 DSI panel"
+	depends on OF && SPI
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	select DRM_MIPI_DBI
+	help
+	  DRM panel driver for the Samsung AMS495QA01 panel controlled
+	  with the Magnachip D53E6EA8966 panel IC. This panel receives
+	  video data via DSI but commands via 9-bit SPI using DBI.
+
 config DRM_PANEL_NEC_NL8048HL11
 	tristate "NEC NL8048HL11 RGB panel"
 	depends on GPIOLIB && OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index f8f9d9f6a307..20de312aa5e9 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
 obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
 obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
+obj-$(CONFIG_DRM_PANEL_MAGNACHIP_D53E6EA8966) += panel-magnachip-d53e6ea8966.o
 obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
 obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3051D) += panel-newvision-nv3051d.o
 obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
diff --git a/drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c b/drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c
new file mode 100644
index 000000000000..09ee12f0a147
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-magnachip-d53e6ea8966.c
@@ -0,0 +1,522 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Magnachip d53e6ea8966 MIPI-DSI panel driver
+ * Copyright (C) 2023 Chris Morgan
+ */
+
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/media-bus-format.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+/* Forward declaration for use in backlight function */
+struct d53e6ea8966;
+
+/* Panel info, unique to each panel */
+struct d53e6ea8966_panel_info {
+	/** @display_modes: the supported display modes */
+	const struct drm_display_mode *display_modes;
+	/** @num_modes: the number of supported display modes */
+	unsigned int num_modes;
+	/** @width_mm: panel width in mm */
+	u16 width_mm;
+	/** @height_mm: panel height in mm */
+	u16 height_mm;
+	/** @bus_flags: drm bus flags for panel */
+	u32 bus_flags;
+	/** @panel_init_seq: panel specific init sequence */
+	void (*panel_init_seq)(struct d53e6ea8966 *db);
+	/** @backlight_register: panel backlight registration or NULL */
+	int (*backlight_register)(struct d53e6ea8966 *db);
+};
+
+struct d53e6ea8966 {
+	/** @dev: the container device */
+	struct device *dev;
+	/** @dbi: the DBI bus abstraction handle */
+	struct mipi_dbi dbi;
+	/** @panel: the DRM panel instance for this device */
+	struct drm_panel panel;
+	/** @reset: reset GPIO line */
+	struct gpio_desc *reset;
+	/** @enable: enable GPIO line */
+	struct gpio_desc *enable;
+	/** @reg_vdd: VDD supply regulator for panel logic */
+	struct regulator *reg_vdd;
+	/** @reg_elvdd: ELVDD supply regulator for panel display */
+	struct regulator *reg_elvdd;
+	/** @dsi_dev: DSI child device (panel) */
+	struct mipi_dsi_device *dsi_dev;
+	/** @bl_dev: pseudo-backlight device for oled panel */
+	struct backlight_device *bl_dev;
+	/** @panel_info: struct containing panel timing and info */
+	const struct d53e6ea8966_panel_info *panel_info;
+};
+
+#define NUM_GAMMA_LEVELS	16
+#define GAMMA_TABLE_COUNT	23
+#define MAX_BRIGHTNESS		(NUM_GAMMA_LEVELS - 1)
+
+#define MCS_ELVSS_ON			0xb1
+#define MCS_TEMP_SWIRE			0xb2
+#define MCS_PASSWORD_0			0xf0
+#define MCS_PASSWORD_1			0xf1
+#define MCS_ANALOG_PWR_CTL_0		0xf4
+#define MCS_ANALOG_PWR_CTL_1		0xf5
+#define MCS_GTCON_SET			0xf7
+#define MCS_GATELESS_SIGNAL_SET		0xf8
+#define MCS_SET_GAMMA			0xf9
+
+static inline struct d53e6ea8966 *to_d53e6ea8966(struct drm_panel *panel)
+{
+	return container_of(panel, struct d53e6ea8966, panel);
+}
+
+/* Table of gamma values provided in datasheet */
+static u8 ams495qa01_gamma[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
+	{0x01, 0x79, 0x78, 0x8d, 0xd9, 0xdf, 0xd5, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe0, 0xe4, 0xdc, 0xb8, 0xd4, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7d, 0x7c, 0x92, 0xd7, 0xdd, 0xd2, 0xcb, 0xd0, 0xc6,
+	 0xe5, 0xe1, 0xe3, 0xda, 0xbd, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7f, 0x7e, 0x95, 0xd7, 0xde, 0xd2, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe3, 0xe3, 0xda, 0xbf, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x82, 0x81, 0x99, 0xd6, 0xdd, 0xd1, 0xca, 0xcf, 0xc3,
+	 0xe4, 0xe3, 0xe3, 0xda, 0xc2, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x84, 0x83, 0x9b, 0xd7, 0xde, 0xd2, 0xc8, 0xce, 0xc2,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc3, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x87, 0x86, 0x9f, 0xd6, 0xdd, 0xd1, 0xc7, 0xce, 0xc1,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc6, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x89, 0x89, 0xa2, 0xd5, 0xdb, 0xcf, 0xc8, 0xcf, 0xc2,
+	 0xe3, 0xe3, 0xe1, 0xd9, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8b, 0x8b, 0xa5, 0xd5, 0xdb, 0xcf, 0xc7, 0xce, 0xc0,
+	 0xe3, 0xe3, 0xe1, 0xd8, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8d, 0x8d, 0xa7, 0xd5, 0xdb, 0xcf, 0xc6, 0xce, 0xc0,
+	 0xe4, 0xe4, 0xe1, 0xd7, 0xc8, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8f, 0x8f, 0xaa, 0xd4, 0xdb, 0xce, 0xc6, 0xcd, 0xbf,
+	 0xe3, 0xe3, 0xe1, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x91, 0x91, 0xac, 0xd3, 0xda, 0xce, 0xc5, 0xcd, 0xbe,
+	 0xe3, 0xe3, 0xe0, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x93, 0x93, 0xaf, 0xd3, 0xda, 0xcd, 0xc5, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd6, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x95, 0x95, 0xb1, 0xd2, 0xd9, 0xcc, 0xc4, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd7, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x99, 0x99, 0xb6, 0xd1, 0xd9, 0xcc, 0xc3, 0xcb, 0xbc,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9c, 0x9c, 0xba, 0xd0, 0xd8, 0xcb, 0xc3, 0xcb, 0xbb,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xce, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9f, 0x9f, 0xbe, 0xcf, 0xd7, 0xc9, 0xc2, 0xcb, 0xbb,
+	 0xe1, 0xe3, 0xde, 0xd6, 0xd0, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+};
+
+/*
+ * Table of elvss values provided in datasheet and corresponds to
+ * gamma values.
+ */
+static u8 ams495qa01_elvss[NUM_GAMMA_LEVELS] = {
+	0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15,
+	0x15, 0x15, 0x14, 0x14, 0x13, 0x12,
+};
+
+static int ams495qa01_update_gamma(struct mipi_dbi *dbi, int brightness)
+{
+	int tmp = brightness;
+
+	mipi_dbi_command_buf(dbi, MCS_SET_GAMMA, ams495qa01_gamma[tmp],
+			     ARRAY_SIZE(ams495qa01_gamma[tmp]));
+	mipi_dbi_command(dbi, MCS_SET_GAMMA, 0x00);
+
+	/* Undocumented command */
+	mipi_dbi_command(dbi, 0x26, 0x00);
+
+	mipi_dbi_command(dbi, MCS_TEMP_SWIRE, ams495qa01_elvss[tmp]);
+
+	return 0;
+}
+
+static void ams495qa01_panel_init(struct d53e6ea8966 *db)
+{
+	struct mipi_dbi *dbi = &db->dbi;
+
+	mipi_dbi_command(dbi, MCS_PASSWORD_0, 0x5a, 0x5a);
+	mipi_dbi_command(dbi, MCS_PASSWORD_1, 0x5a, 0x5a);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb0, 0x02);
+	mipi_dbi_command(dbi, 0xf3, 0x3b);
+
+	mipi_dbi_command(dbi, MCS_ANALOG_PWR_CTL_0, 0x33, 0x42, 0x00, 0x08);
+	mipi_dbi_command(dbi, MCS_ANALOG_PWR_CTL_1, 0x00, 0x06, 0x26, 0x35, 0x03);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xf6, 0x02);
+	mipi_dbi_command(dbi, 0xc6, 0x0b, 0x00, 0x00, 0x3c, 0x00, 0x22,
+			 0x00, 0x00, 0x00, 0x00);
+
+	mipi_dbi_command(dbi, MCS_GTCON_SET, 0x20);
+	mipi_dbi_command(dbi, MCS_TEMP_SWIRE, 0x06, 0x06, 0x06, 0x06);
+	mipi_dbi_command(dbi, MCS_ELVSS_ON, 0x07, 0x00, 0x10);
+	mipi_dbi_command(dbi, MCS_GATELESS_SIGNAL_SET, 0x7f, 0x7a,
+			 0x89, 0x67, 0x26, 0x38, 0x00, 0x00, 0x09,
+			 0x67, 0x70, 0x88, 0x7a, 0x76, 0x05, 0x09,
+			 0x23, 0x23, 0x23);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb5, 0xff, 0xef, 0x35, 0x42, 0x0d, 0xd7,
+			 0xff, 0x07, 0xff, 0xff, 0xfd, 0x00, 0x01,
+			 0xff, 0x05, 0x12, 0x0f, 0xff, 0xff, 0xff,
+			 0xff);
+	mipi_dbi_command(dbi, 0xb4, 0x15);
+	mipi_dbi_command(dbi, 0xb3, 0x00);
+
+	ams495qa01_update_gamma(dbi, MAX_BRIGHTNESS);
+}
+
+static int d53e6ea8966_prepare(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	int ret;
+
+	/* Power up */
+	ret = regulator_enable(db->reg_vdd);
+	if (ret) {
+		dev_err(db->dev, "failed to enable vdd regulator: %d\n", ret);
+		return ret;
+	}
+
+	if (db->reg_elvdd) {
+		ret = regulator_enable(db->reg_elvdd);
+		if (ret) {
+			dev_err(db->dev,
+				"failed to enable elvdd regulator: %d\n", ret);
+			regulator_disable(db->reg_vdd);
+			return ret;
+		}
+	}
+
+	/* Enable */
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 1);
+
+	msleep(50);
+
+	/* Reset */
+	gpiod_set_value_cansleep(db->reset, 1);
+	usleep_range(1000, 5000);
+	gpiod_set_value_cansleep(db->reset, 0);
+	msleep(20);
+
+	db->panel_info->panel_init_seq(db);
+
+	return 0;
+}
+
+static int d53e6ea8966_enable(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(200);
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
+	usleep_range(10000, 15000);
+
+	return 0;
+}
+
+static int d53e6ea8966_disable(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(20);
+	mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
+	msleep(100);
+
+	return 0;
+}
+
+static int d53e6ea8966_unprepare(struct drm_panel *panel)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 0);
+
+	gpiod_set_value_cansleep(db->reset, 1);
+
+	if (db->reg_elvdd)
+		regulator_disable(db->reg_elvdd);
+
+	regulator_disable(db->reg_vdd);
+	msleep(100);
+
+	return 0;
+}
+
+static int d53e6ea8966_get_modes(struct drm_panel *panel,
+				struct drm_connector *connector)
+{
+	struct d53e6ea8966 *db = to_d53e6ea8966(panel);
+	const struct d53e6ea8966_panel_info *panel_info = db->panel_info;
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	unsigned int i;
+
+	for (i = 0; i < panel_info->num_modes; i++) {
+		mode = drm_mode_duplicate(connector->dev,
+					  &panel_info->display_modes[i]);
+		if (!mode)
+			return -ENOMEM;
+
+		drm_mode_set_name(mode);
+		drm_mode_probed_add(connector, mode);
+	}
+
+	connector->display_info.bpc = 8;
+	connector->display_info.width_mm = panel_info->width_mm;
+	connector->display_info.height_mm = panel_info->height_mm;
+	connector->display_info.bus_flags = panel_info->bus_flags;
+
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 &bus_format, 1);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs d53e6ea8966_panel_funcs = {
+	.disable = d53e6ea8966_disable,
+	.enable = d53e6ea8966_enable,
+	.get_modes = d53e6ea8966_get_modes,
+	.prepare = d53e6ea8966_prepare,
+	.unprepare = d53e6ea8966_unprepare,
+};
+
+static int ams495qa01_set_brightness(struct backlight_device *bd)
+{
+	struct d53e6ea8966 *db = bl_get_data(bd);
+	struct mipi_dbi *dbi = &db->dbi;
+	int brightness = backlight_get_brightness(bd);
+
+	ams495qa01_update_gamma(dbi, brightness);
+
+	return 0;
+}
+
+static const struct backlight_ops ams495qa01_backlight_ops = {
+	.update_status	= ams495qa01_set_brightness,
+};
+
+static int ams495qa01_backlight_register(struct d53e6ea8966 *db)
+{
+	struct backlight_properties props = {
+		.type		= BACKLIGHT_RAW,
+		.brightness	= MAX_BRIGHTNESS,
+		.max_brightness = MAX_BRIGHTNESS,
+	};
+	struct device *dev = db->dev;
+	int ret = 0;
+
+	db->bl_dev = devm_backlight_device_register(dev, "panel", dev, db,
+						     &ams495qa01_backlight_ops,
+						     &props);
+	if (IS_ERR(db->bl_dev)) {
+		ret = PTR_ERR(db->bl_dev);
+		dev_err(dev, "error registering backlight device (%d)\n", ret);
+	}
+
+	return ret;
+}
+
+static int d53e6ea8966_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct mipi_dsi_host *dsi_host;
+	struct d53e6ea8966 *db;
+	int ret;
+	struct mipi_dsi_device_info info = {
+		.type = "d53e6ea8966",
+		.channel = 0,
+		.node = NULL,
+	};
+
+	db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
+	if (!db)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, db);
+
+	db->dev = dev;
+
+	db->panel_info = of_device_get_match_data(dev);
+	if (!db->panel_info)
+		return -EINVAL;
+
+	db->reg_vdd = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(db->reg_vdd))
+		return dev_err_probe(dev, PTR_ERR(db->reg_vdd),
+				     "Failed to get vdd supply\n");
+
+	db->reg_elvdd = devm_regulator_get_optional(dev, "elvdd");
+	if (IS_ERR(db->reg_elvdd))
+		db->reg_elvdd = NULL;
+
+	db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(db->reset)) {
+		ret = PTR_ERR(db->reset);
+		return dev_err_probe(dev, ret, "no RESET GPIO\n");
+	}
+
+	db->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(db->enable)) {
+		ret = PTR_ERR(db->enable);
+		return dev_err_probe(dev, ret, "cannot get ENABLE GPIO\n");
+	}
+
+	ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
+
+	dsi_host = drm_of_get_dsi_bus(dev, &info);
+	if (IS_ERR(dsi_host)) {
+		ret = PTR_ERR(dsi_host);
+		return dev_err_probe(dev, ret, "Error attaching DSI bus\n");
+	}
+
+	db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
+	if (IS_ERR(db->dsi_dev)) {
+		dev_err(dev, "failed to register dsi device: %ld\n",
+			PTR_ERR(db->dsi_dev));
+		ret = PTR_ERR(db->dsi_dev);
+	}
+
+	db->dsi_dev->lanes = 2;
+	db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
+	db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
+
+	drm_panel_init(&db->panel, dev, &d53e6ea8966_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+
+	if (db->panel_info->backlight_register != NULL) {
+		ret = db->panel_info->backlight_register(db);
+		if (ret < 0)
+			return ret;
+		db->panel.backlight = db->bl_dev;
+	}
+
+	drm_panel_add(&db->panel);
+
+	ret = devm_mipi_dsi_attach(dev, db->dsi_dev);
+	if (ret < 0) {
+		dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
+		drm_panel_remove(&db->panel);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void d53e6ea8966_remove(struct spi_device *spi)
+{
+	struct d53e6ea8966 *db = spi_get_drvdata(spi);
+
+	drm_panel_remove(&db->panel);
+}
+
+static const struct drm_display_mode ams495qa01_modes[] = {
+	{ /* 60hz */
+		.clock = 33500,
+		.hdisplay = 960,
+		.hsync_start = 960 + 10,
+		.hsync_end = 960 + 10 + 2,
+		.htotal = 960 + 10 + 2 + 10,
+		.vdisplay = 544,
+		.vsync_start = 544 + 10,
+		.vsync_end = 544 + 10 + 2,
+		.vtotal = 544 + 10 + 2 + 10,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+		.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+		},
+	{ /* 50hz */
+		.clock = 27800,
+		.hdisplay = 960,
+		.hsync_start = 960 + 10,
+		.hsync_end = 960 + 10 + 2,
+		.htotal = 960 + 10 + 2 + 10,
+		.vdisplay = 544,
+		.vsync_start = 544 + 10,
+		.vsync_end = 544 + 10 + 2,
+		.vtotal = 544 + 10 + 2 + 10,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+		.type = DRM_MODE_TYPE_DRIVER,
+	},
+};
+
+static const struct d53e6ea8966_panel_info ams495qa01_info = {
+	.display_modes = ams495qa01_modes,
+	.num_modes = ARRAY_SIZE(ams495qa01_modes),
+	.width_mm = 117,
+	.height_mm = 74,
+	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+	.panel_init_seq = ams495qa01_panel_init,
+	.backlight_register = ams495qa01_backlight_register,
+};
+
+static const struct of_device_id d53e6ea8966_match[] = {
+	{ .compatible = "samsung,ams495qa01", .data = &ams495qa01_info },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, d53e6ea8966_match);
+
+static const struct spi_device_id d53e6ea8966_ids[] = {
+	{ "ams495qa01", 0 },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(spi, d53e6ea8966_ids);
+
+static struct spi_driver d53e6ea8966_driver = {
+	.driver		= {
+		.name	= "d53e6ea8966-panel",
+		.of_match_table = d53e6ea8966_match,
+	},
+	.id_table	= d53e6ea8966_ids,
+	.probe		= d53e6ea8966_probe,
+	.remove		= d53e6ea8966_remove,
+};
+module_spi_driver(d53e6ea8966_driver);
+
+MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
+MODULE_DESCRIPTION("Magnachip d53e6ea8966 panel driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1


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

* [PATCH V10 4/4] arm64: dts: rockchip: add display to RG503
  2023-01-12 17:53 ` Chris Morgan
  (?)
@ 2023-01-12 17:53   ` Chris Morgan
  -1 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add Samsung AMS495QA01 panel to RG503.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 .../dts/rockchip/rk3566-anbernic-rg503.dts    | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
index 5dafcc86296b..b4b2df821cba 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
@@ -47,6 +47,21 @@ gpio_spi: spi {
 		mosi-gpios = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
 		cs-gpios = <&gpio4 RK_PA7 GPIO_ACTIVE_HIGH>;
 		num-chipselects = <0>;
+
+		panel@0 {
+			compatible = "samsung,ams495qa01";
+			reg = <0>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&lcd_reset>;
+			reset-gpios = <&gpio4 RK_PA0 GPIO_ACTIVE_LOW>;
+			vdd-supply = <&vcc_3v3>;
+
+			port {
+				mipi_in_panel: endpoint {
+					remote-endpoint = <&mipi_out_panel>;
+				};
+			};
+		};
 	};
 
 	/* Channels reversed for both headphones and speakers. */
@@ -94,6 +109,32 @@ &cru {
 	assigned-clock-rates = <1200000000>, <200000000>, <500000000>;
 };
 
+&dsi_dphy0 {
+	status = "okay";
+};
+
+&dsi0 {
+	status = "okay";
+
+	ports {
+		dsi0_in: port@0 {
+			reg = <0>;
+
+			dsi0_in_vp1: endpoint {
+				remote-endpoint = <&vp1_out_dsi0>;
+			};
+		};
+
+		dsi0_out: port@1 {
+			reg = <1>;
+
+			mipi_out_panel: endpoint {
+				remote-endpoint = <&mipi_in_panel>;
+			};
+		};
+	};
+};
+
 &gpio_keys_control {
 	button-a {
 		gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_LOW>;
@@ -146,6 +187,13 @@ spk_amp_enable_h: spk-amp-enable-h {
 		};
 	};
 
+	gpio-lcd {
+		lcd_reset: lcd-reset {
+			rockchip,pins =
+				<4 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	gpio-spi {
 		spi_pins: spi-pins {
 			rockchip,pins =
@@ -164,3 +212,10 @@ rk817_charger: charger {
 		rockchip,sleep-filter-current-microamp = <100000>;
 	};
 };
+
+&vp1 {
+	vp1_out_dsi0: endpoint@ROCKCHIP_VOP2_EP_MIPI0 {
+		reg = <ROCKCHIP_VOP2_EP_MIPI0>;
+		remote-endpoint = <&dsi0_in_vp1>;
+	};
+};
-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH V10 4/4] arm64: dts: rockchip: add display to RG503
@ 2023-01-12 17:53   ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, Chris Morgan, krzysztof.kozlowski+dt, sam,
	linux-rockchip, robh+dt, thierry.reding, tzimmermann,
	maccraft123mc

From: Chris Morgan <macromorgan@hotmail.com>

Add Samsung AMS495QA01 panel to RG503.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 .../dts/rockchip/rk3566-anbernic-rg503.dts    | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
index 5dafcc86296b..b4b2df821cba 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
@@ -47,6 +47,21 @@ gpio_spi: spi {
 		mosi-gpios = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
 		cs-gpios = <&gpio4 RK_PA7 GPIO_ACTIVE_HIGH>;
 		num-chipselects = <0>;
+
+		panel@0 {
+			compatible = "samsung,ams495qa01";
+			reg = <0>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&lcd_reset>;
+			reset-gpios = <&gpio4 RK_PA0 GPIO_ACTIVE_LOW>;
+			vdd-supply = <&vcc_3v3>;
+
+			port {
+				mipi_in_panel: endpoint {
+					remote-endpoint = <&mipi_out_panel>;
+				};
+			};
+		};
 	};
 
 	/* Channels reversed for both headphones and speakers. */
@@ -94,6 +109,32 @@ &cru {
 	assigned-clock-rates = <1200000000>, <200000000>, <500000000>;
 };
 
+&dsi_dphy0 {
+	status = "okay";
+};
+
+&dsi0 {
+	status = "okay";
+
+	ports {
+		dsi0_in: port@0 {
+			reg = <0>;
+
+			dsi0_in_vp1: endpoint {
+				remote-endpoint = <&vp1_out_dsi0>;
+			};
+		};
+
+		dsi0_out: port@1 {
+			reg = <1>;
+
+			mipi_out_panel: endpoint {
+				remote-endpoint = <&mipi_in_panel>;
+			};
+		};
+	};
+};
+
 &gpio_keys_control {
 	button-a {
 		gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_LOW>;
@@ -146,6 +187,13 @@ spk_amp_enable_h: spk-amp-enable-h {
 		};
 	};
 
+	gpio-lcd {
+		lcd_reset: lcd-reset {
+			rockchip,pins =
+				<4 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	gpio-spi {
 		spi_pins: spi-pins {
 			rockchip,pins =
@@ -164,3 +212,10 @@ rk817_charger: charger {
 		rockchip,sleep-filter-current-microamp = <100000>;
 	};
 };
+
+&vp1 {
+	vp1_out_dsi0: endpoint@ROCKCHIP_VOP2_EP_MIPI0 {
+		reg = <ROCKCHIP_VOP2_EP_MIPI0>;
+		remote-endpoint = <&dsi0_in_vp1>;
+	};
+};
-- 
2.34.1


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

* [PATCH V10 4/4] arm64: dts: rockchip: add display to RG503
@ 2023-01-12 17:53   ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-12 17:53 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-rockchip, devicetree, linus.walleij, maccraft123mc,
	tzimmermann, mripard, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add Samsung AMS495QA01 panel to RG503.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 .../dts/rockchip/rk3566-anbernic-rg503.dts    | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
index 5dafcc86296b..b4b2df821cba 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
@@ -47,6 +47,21 @@ gpio_spi: spi {
 		mosi-gpios = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
 		cs-gpios = <&gpio4 RK_PA7 GPIO_ACTIVE_HIGH>;
 		num-chipselects = <0>;
+
+		panel@0 {
+			compatible = "samsung,ams495qa01";
+			reg = <0>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&lcd_reset>;
+			reset-gpios = <&gpio4 RK_PA0 GPIO_ACTIVE_LOW>;
+			vdd-supply = <&vcc_3v3>;
+
+			port {
+				mipi_in_panel: endpoint {
+					remote-endpoint = <&mipi_out_panel>;
+				};
+			};
+		};
 	};
 
 	/* Channels reversed for both headphones and speakers. */
@@ -94,6 +109,32 @@ &cru {
 	assigned-clock-rates = <1200000000>, <200000000>, <500000000>;
 };
 
+&dsi_dphy0 {
+	status = "okay";
+};
+
+&dsi0 {
+	status = "okay";
+
+	ports {
+		dsi0_in: port@0 {
+			reg = <0>;
+
+			dsi0_in_vp1: endpoint {
+				remote-endpoint = <&vp1_out_dsi0>;
+			};
+		};
+
+		dsi0_out: port@1 {
+			reg = <1>;
+
+			mipi_out_panel: endpoint {
+				remote-endpoint = <&mipi_in_panel>;
+			};
+		};
+	};
+};
+
 &gpio_keys_control {
 	button-a {
 		gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_LOW>;
@@ -146,6 +187,13 @@ spk_amp_enable_h: spk-amp-enable-h {
 		};
 	};
 
+	gpio-lcd {
+		lcd_reset: lcd-reset {
+			rockchip,pins =
+				<4 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	gpio-spi {
 		spi_pins: spi-pins {
 			rockchip,pins =
@@ -164,3 +212,10 @@ rk817_charger: charger {
 		rockchip,sleep-filter-current-microamp = <100000>;
 	};
 };
+
+&vp1 {
+	vp1_out_dsi0: endpoint@ROCKCHIP_VOP2_EP_MIPI0 {
+		reg = <ROCKCHIP_VOP2_EP_MIPI0>;
+		remote-endpoint = <&dsi0_in_vp1>;
+	};
+};
-- 
2.34.1


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

* Re: [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
  2023-01-12 17:53   ` Chris Morgan
  (?)
@ 2023-01-17 16:58     ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2023-01-17 16:58 UTC (permalink / raw)
  To: Chris Morgan
  Cc: devicetree, Chris Morgan, krzysztof.kozlowski+dt, sam, dri-devel,
	linux-rockchip, robh+dt, thierry.reding, tzimmermann,
	maccraft123mc

[-- Attachment #1: Type: text/plain, Size: 5447 bytes --]

Hi,

On Thu, Jan 12, 2023 at 11:53:55AM -0600, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add helper function to find DSI host for devices where DSI panel is not
> a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
> official Raspberry Pi touchscreen display).
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> ---
>  drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_of.h     | 10 ++++++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index 7bbcb999bb75..6c2c97a716fe 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -10,6 +10,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_device.h>
>  #include <drm/drm_encoder.h>
> +#include <drm/drm_mipi_dsi.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_panel.h>
>  
> @@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
> +
> +#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> +
> +/**
> + * drm_of_get_dsi_bus - find the DSI bus for a given device
> + * @dev: parent device of display (SPI, I2C)
> + * @info: DSI device info to be updated with DSI node. This is optional
> + * and if not needed can be NULL.
> + *
> + * Gets parent DSI bus for a DSI device controlled through a bus other
> + * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
> + *
> + * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
> + * request is unsupported, -EPROBE_DEFER if the DSI host is found but
> + * not available, or -ENODEV otherwise.
> + */
> +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> +					 struct mipi_dsi_device_info *info)
> +{
> +	struct mipi_dsi_host *dsi_host;
> +	struct device_node *endpoint, *dsi_host_node;
> +
> +	/*
> +	 * Get first endpoint child from device.
> +	 */
> +	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> +	if (!endpoint)
> +		return ERR_PTR(-ENODEV);
> +
> +	/*
> +	 * Follow the first endpoint to get the DSI host node.
> +	 */
> +	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> +	if (!dsi_host_node)
> +		goto error;
> +
> +	/*
> +	 * Get the DSI host from the DSI host node. If we get an error
> +	 * or the return is null assume we're not ready to probe just
> +	 * yet. Release the DSI host node since we're done with it.
> +	 */
> +	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> +	of_node_put(dsi_host_node);
> +	if (IS_ERR_OR_NULL(dsi_host)) {
> +		of_node_put(endpoint);
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	/*
> +	 * Set the node of the mipi_dsi_device_info to the correct node
> +	 * and then release the endpoint node since we're done with it.
> +	 * since this is optional, check if the info is NULL first.
> +	 */
> +	if (info) {
> +		info->node = of_graph_get_remote_port(endpoint);

it looks to me that the info->node is actually the DSI device OF node,
not its host port. Which begs the question, why should we even return it
there, since there's a pretty big chance that dev->of.node ==
info->node, and you obviously don't care about the channel and type fields.

I've had a look and node of the current users of
mipi_dsi_device_register_full actually register a mipi_dsi_device_info
with a node pointer set to !NULL, including the driver in this series.

So, why do we care about the device info at all?

> +		if (IS_ERR_OR_NULL(info->node))

of_graph_get_remote_port doesn't return an error pointer.

> --- a/include/drm/drm_of.h
> +++ b/include/drm/drm_of.h
> @@ -15,6 +15,8 @@ struct drm_encoder;
>  struct drm_panel;
>  struct drm_bridge;
>  struct device_node;
> +struct mipi_dsi_device_info;
> +struct mipi_dsi_host;
>  
>  /**
>   * enum drm_lvds_dual_link_pixels - Pixel order of an LVDS dual-link connection
> @@ -56,6 +58,8 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
>  				   int port_reg, int reg,
>  				   const unsigned int min,
>  				   const unsigned int max);
> +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> +					 struct mipi_dsi_device_info *info);
>  #else
>  static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
>  					  struct device_node *port)
> @@ -127,6 +131,12 @@ drm_of_get_data_lanes_count_ep(const struct device_node *port,
>  {
>  	return -EINVAL;
>  }
> +static inline struct
> +mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> +					 struct mipi_dsi_device_info *info)
> +{
> +	return ERR_PTR(-EINVAL);
> +}
>  #endif

So it looks to me that if CONFIG_OF is defined, we'll define an external
symbol declared for drm_of_get_dsi_bus, but that function will only be
compiled if CONFIG_DRM_MIPI_DSI is enabled.

What happens if we have CONFIG_OF but not CONFIG_DRM_MIPI_DSI?

If think you need to have here something like:

#ifdef IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_MIPI_DSI)
struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
       					 struct mipi_dsi_device_info *info);
#else
static inline struct
mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
				  struct mipi_dsi_device_info *info)
{
	return ERR_PTR(-EINVAL);
}
#endif

Maxime

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

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

* Re: [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
@ 2023-01-17 16:58     ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2023-01-17 16:58 UTC (permalink / raw)
  To: Chris Morgan
  Cc: dri-devel, linux-rockchip, devicetree, linus.walleij,
	maccraft123mc, tzimmermann, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

[-- Attachment #1: Type: text/plain, Size: 5447 bytes --]

Hi,

On Thu, Jan 12, 2023 at 11:53:55AM -0600, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add helper function to find DSI host for devices where DSI panel is not
> a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
> official Raspberry Pi touchscreen display).
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> ---
>  drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_of.h     | 10 ++++++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index 7bbcb999bb75..6c2c97a716fe 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -10,6 +10,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_device.h>
>  #include <drm/drm_encoder.h>
> +#include <drm/drm_mipi_dsi.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_panel.h>
>  
> @@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
> +
> +#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> +
> +/**
> + * drm_of_get_dsi_bus - find the DSI bus for a given device
> + * @dev: parent device of display (SPI, I2C)
> + * @info: DSI device info to be updated with DSI node. This is optional
> + * and if not needed can be NULL.
> + *
> + * Gets parent DSI bus for a DSI device controlled through a bus other
> + * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
> + *
> + * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
> + * request is unsupported, -EPROBE_DEFER if the DSI host is found but
> + * not available, or -ENODEV otherwise.
> + */
> +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> +					 struct mipi_dsi_device_info *info)
> +{
> +	struct mipi_dsi_host *dsi_host;
> +	struct device_node *endpoint, *dsi_host_node;
> +
> +	/*
> +	 * Get first endpoint child from device.
> +	 */
> +	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> +	if (!endpoint)
> +		return ERR_PTR(-ENODEV);
> +
> +	/*
> +	 * Follow the first endpoint to get the DSI host node.
> +	 */
> +	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> +	if (!dsi_host_node)
> +		goto error;
> +
> +	/*
> +	 * Get the DSI host from the DSI host node. If we get an error
> +	 * or the return is null assume we're not ready to probe just
> +	 * yet. Release the DSI host node since we're done with it.
> +	 */
> +	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> +	of_node_put(dsi_host_node);
> +	if (IS_ERR_OR_NULL(dsi_host)) {
> +		of_node_put(endpoint);
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	/*
> +	 * Set the node of the mipi_dsi_device_info to the correct node
> +	 * and then release the endpoint node since we're done with it.
> +	 * since this is optional, check if the info is NULL first.
> +	 */
> +	if (info) {
> +		info->node = of_graph_get_remote_port(endpoint);

it looks to me that the info->node is actually the DSI device OF node,
not its host port. Which begs the question, why should we even return it
there, since there's a pretty big chance that dev->of.node ==
info->node, and you obviously don't care about the channel and type fields.

I've had a look and node of the current users of
mipi_dsi_device_register_full actually register a mipi_dsi_device_info
with a node pointer set to !NULL, including the driver in this series.

So, why do we care about the device info at all?

> +		if (IS_ERR_OR_NULL(info->node))

of_graph_get_remote_port doesn't return an error pointer.

> --- a/include/drm/drm_of.h
> +++ b/include/drm/drm_of.h
> @@ -15,6 +15,8 @@ struct drm_encoder;
>  struct drm_panel;
>  struct drm_bridge;
>  struct device_node;
> +struct mipi_dsi_device_info;
> +struct mipi_dsi_host;
>  
>  /**
>   * enum drm_lvds_dual_link_pixels - Pixel order of an LVDS dual-link connection
> @@ -56,6 +58,8 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
>  				   int port_reg, int reg,
>  				   const unsigned int min,
>  				   const unsigned int max);
> +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> +					 struct mipi_dsi_device_info *info);
>  #else
>  static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
>  					  struct device_node *port)
> @@ -127,6 +131,12 @@ drm_of_get_data_lanes_count_ep(const struct device_node *port,
>  {
>  	return -EINVAL;
>  }
> +static inline struct
> +mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> +					 struct mipi_dsi_device_info *info)
> +{
> +	return ERR_PTR(-EINVAL);
> +}
>  #endif

So it looks to me that if CONFIG_OF is defined, we'll define an external
symbol declared for drm_of_get_dsi_bus, but that function will only be
compiled if CONFIG_DRM_MIPI_DSI is enabled.

What happens if we have CONFIG_OF but not CONFIG_DRM_MIPI_DSI?

If think you need to have here something like:

#ifdef IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_MIPI_DSI)
struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
       					 struct mipi_dsi_device_info *info);
#else
static inline struct
mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
				  struct mipi_dsi_device_info *info)
{
	return ERR_PTR(-EINVAL);
}
#endif

Maxime

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

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

* Re: [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
@ 2023-01-17 16:58     ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2023-01-17 16:58 UTC (permalink / raw)
  To: Chris Morgan
  Cc: dri-devel, linux-rockchip, devicetree, linus.walleij,
	maccraft123mc, tzimmermann, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan


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

Hi,

On Thu, Jan 12, 2023 at 11:53:55AM -0600, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add helper function to find DSI host for devices where DSI panel is not
> a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
> official Raspberry Pi touchscreen display).
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> ---
>  drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_of.h     | 10 ++++++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index 7bbcb999bb75..6c2c97a716fe 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -10,6 +10,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_device.h>
>  #include <drm/drm_encoder.h>
> +#include <drm/drm_mipi_dsi.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_panel.h>
>  
> @@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
> +
> +#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> +
> +/**
> + * drm_of_get_dsi_bus - find the DSI bus for a given device
> + * @dev: parent device of display (SPI, I2C)
> + * @info: DSI device info to be updated with DSI node. This is optional
> + * and if not needed can be NULL.
> + *
> + * Gets parent DSI bus for a DSI device controlled through a bus other
> + * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
> + *
> + * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
> + * request is unsupported, -EPROBE_DEFER if the DSI host is found but
> + * not available, or -ENODEV otherwise.
> + */
> +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> +					 struct mipi_dsi_device_info *info)
> +{
> +	struct mipi_dsi_host *dsi_host;
> +	struct device_node *endpoint, *dsi_host_node;
> +
> +	/*
> +	 * Get first endpoint child from device.
> +	 */
> +	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> +	if (!endpoint)
> +		return ERR_PTR(-ENODEV);
> +
> +	/*
> +	 * Follow the first endpoint to get the DSI host node.
> +	 */
> +	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> +	if (!dsi_host_node)
> +		goto error;
> +
> +	/*
> +	 * Get the DSI host from the DSI host node. If we get an error
> +	 * or the return is null assume we're not ready to probe just
> +	 * yet. Release the DSI host node since we're done with it.
> +	 */
> +	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> +	of_node_put(dsi_host_node);
> +	if (IS_ERR_OR_NULL(dsi_host)) {
> +		of_node_put(endpoint);
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	/*
> +	 * Set the node of the mipi_dsi_device_info to the correct node
> +	 * and then release the endpoint node since we're done with it.
> +	 * since this is optional, check if the info is NULL first.
> +	 */
> +	if (info) {
> +		info->node = of_graph_get_remote_port(endpoint);

it looks to me that the info->node is actually the DSI device OF node,
not its host port. Which begs the question, why should we even return it
there, since there's a pretty big chance that dev->of.node ==
info->node, and you obviously don't care about the channel and type fields.

I've had a look and node of the current users of
mipi_dsi_device_register_full actually register a mipi_dsi_device_info
with a node pointer set to !NULL, including the driver in this series.

So, why do we care about the device info at all?

> +		if (IS_ERR_OR_NULL(info->node))

of_graph_get_remote_port doesn't return an error pointer.

> --- a/include/drm/drm_of.h
> +++ b/include/drm/drm_of.h
> @@ -15,6 +15,8 @@ struct drm_encoder;
>  struct drm_panel;
>  struct drm_bridge;
>  struct device_node;
> +struct mipi_dsi_device_info;
> +struct mipi_dsi_host;
>  
>  /**
>   * enum drm_lvds_dual_link_pixels - Pixel order of an LVDS dual-link connection
> @@ -56,6 +58,8 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
>  				   int port_reg, int reg,
>  				   const unsigned int min,
>  				   const unsigned int max);
> +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> +					 struct mipi_dsi_device_info *info);
>  #else
>  static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
>  					  struct device_node *port)
> @@ -127,6 +131,12 @@ drm_of_get_data_lanes_count_ep(const struct device_node *port,
>  {
>  	return -EINVAL;
>  }
> +static inline struct
> +mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> +					 struct mipi_dsi_device_info *info)
> +{
> +	return ERR_PTR(-EINVAL);
> +}
>  #endif

So it looks to me that if CONFIG_OF is defined, we'll define an external
symbol declared for drm_of_get_dsi_bus, but that function will only be
compiled if CONFIG_DRM_MIPI_DSI is enabled.

What happens if we have CONFIG_OF but not CONFIG_DRM_MIPI_DSI?

If think you need to have here something like:

#ifdef IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_MIPI_DSI)
struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
       					 struct mipi_dsi_device_info *info);
#else
static inline struct
mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
				  struct mipi_dsi_device_info *info)
{
	return ERR_PTR(-EINVAL);
}
#endif

Maxime

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

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
  2023-01-17 16:58     ` Maxime Ripard
  (?)
@ 2023-01-17 19:12       ` Chris Morgan
  -1 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-17 19:12 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: dri-devel, linux-rockchip, devicetree, linus.walleij,
	maccraft123mc, tzimmermann, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

On Tue, Jan 17, 2023 at 05:58:19PM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Jan 12, 2023 at 11:53:55AM -0600, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add helper function to find DSI host for devices where DSI panel is not
> > a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
> > official Raspberry Pi touchscreen display).
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_of.h     | 10 ++++++
> >  2 files changed, 80 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> > index 7bbcb999bb75..6c2c97a716fe 100644
> > --- a/drivers/gpu/drm/drm_of.c
> > +++ b/drivers/gpu/drm/drm_of.c
> > @@ -10,6 +10,7 @@
> >  #include <drm/drm_crtc.h>
> >  #include <drm/drm_device.h>
> >  #include <drm/drm_encoder.h>
> > +#include <drm/drm_mipi_dsi.h>
> >  #include <drm/drm_of.h>
> >  #include <drm/drm_panel.h>
> >  
> > @@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
> > +
> > +#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> > +
> > +/**
> > + * drm_of_get_dsi_bus - find the DSI bus for a given device
> > + * @dev: parent device of display (SPI, I2C)
> > + * @info: DSI device info to be updated with DSI node. This is optional
> > + * and if not needed can be NULL.
> > + *
> > + * Gets parent DSI bus for a DSI device controlled through a bus other
> > + * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
> > + *
> > + * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
> > + * request is unsupported, -EPROBE_DEFER if the DSI host is found but
> > + * not available, or -ENODEV otherwise.
> > + */
> > +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > +					 struct mipi_dsi_device_info *info)
> > +{
> > +	struct mipi_dsi_host *dsi_host;
> > +	struct device_node *endpoint, *dsi_host_node;
> > +
> > +	/*
> > +	 * Get first endpoint child from device.
> > +	 */
> > +	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > +	if (!endpoint)
> > +		return ERR_PTR(-ENODEV);
> > +
> > +	/*
> > +	 * Follow the first endpoint to get the DSI host node.
> > +	 */
> > +	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> > +	if (!dsi_host_node)
> > +		goto error;
> > +
> > +	/*
> > +	 * Get the DSI host from the DSI host node. If we get an error
> > +	 * or the return is null assume we're not ready to probe just
> > +	 * yet. Release the DSI host node since we're done with it.
> > +	 */
> > +	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> > +	of_node_put(dsi_host_node);
> > +	if (IS_ERR_OR_NULL(dsi_host)) {
> > +		of_node_put(endpoint);
> > +		return ERR_PTR(-EPROBE_DEFER);
> > +	}
> > +
> > +	/*
> > +	 * Set the node of the mipi_dsi_device_info to the correct node
> > +	 * and then release the endpoint node since we're done with it.
> > +	 * since this is optional, check if the info is NULL first.
> > +	 */
> > +	if (info) {
> > +		info->node = of_graph_get_remote_port(endpoint);
> 
> it looks to me that the info->node is actually the DSI device OF node,
> not its host port. Which begs the question, why should we even return it
> there, since there's a pretty big chance that dev->of.node ==
> info->node, and you obviously don't care about the channel and type fields.
> 
> I've had a look and node of the current users of
> mipi_dsi_device_register_full actually register a mipi_dsi_device_info
> with a node pointer set to !NULL, including the driver in this series.
> 
> So, why do we care about the device info at all?
> 

I honestly thought it might be useful, but I can try without it.

> > +		if (IS_ERR_OR_NULL(info->node))
> 
> of_graph_get_remote_port doesn't return an error pointer.
> 
> > --- a/include/drm/drm_of.h
> > +++ b/include/drm/drm_of.h
> > @@ -15,6 +15,8 @@ struct drm_encoder;
> >  struct drm_panel;
> >  struct drm_bridge;
> >  struct device_node;
> > +struct mipi_dsi_device_info;
> > +struct mipi_dsi_host;
> >  
> >  /**
> >   * enum drm_lvds_dual_link_pixels - Pixel order of an LVDS dual-link connection
> > @@ -56,6 +58,8 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
> >  				   int port_reg, int reg,
> >  				   const unsigned int min,
> >  				   const unsigned int max);
> > +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > +					 struct mipi_dsi_device_info *info);
> >  #else
> >  static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
> >  					  struct device_node *port)
> > @@ -127,6 +131,12 @@ drm_of_get_data_lanes_count_ep(const struct device_node *port,
> >  {
> >  	return -EINVAL;
> >  }
> > +static inline struct
> > +mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > +					 struct mipi_dsi_device_info *info)
> > +{
> > +	return ERR_PTR(-EINVAL);
> > +}
> >  #endif
> 
> So it looks to me that if CONFIG_OF is defined, we'll define an external
> symbol declared for drm_of_get_dsi_bus, but that function will only be
> compiled if CONFIG_DRM_MIPI_DSI is enabled.
> 
> What happens if we have CONFIG_OF but not CONFIG_DRM_MIPI_DSI?
> 

Will do, thank you.

> If think you need to have here something like:
> 
> #ifdef IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
>        					 struct mipi_dsi_device_info *info);
> #else
> static inline struct
> mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> 				  struct mipi_dsi_device_info *info)
> {
> 	return ERR_PTR(-EINVAL);
> }
> #endif
> 
> Maxime



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
@ 2023-01-17 19:12       ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-17 19:12 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: devicetree, Chris Morgan, krzysztof.kozlowski+dt, sam, dri-devel,
	linux-rockchip, robh+dt, thierry.reding, tzimmermann,
	maccraft123mc

On Tue, Jan 17, 2023 at 05:58:19PM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Jan 12, 2023 at 11:53:55AM -0600, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add helper function to find DSI host for devices where DSI panel is not
> > a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
> > official Raspberry Pi touchscreen display).
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_of.h     | 10 ++++++
> >  2 files changed, 80 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> > index 7bbcb999bb75..6c2c97a716fe 100644
> > --- a/drivers/gpu/drm/drm_of.c
> > +++ b/drivers/gpu/drm/drm_of.c
> > @@ -10,6 +10,7 @@
> >  #include <drm/drm_crtc.h>
> >  #include <drm/drm_device.h>
> >  #include <drm/drm_encoder.h>
> > +#include <drm/drm_mipi_dsi.h>
> >  #include <drm/drm_of.h>
> >  #include <drm/drm_panel.h>
> >  
> > @@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
> > +
> > +#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> > +
> > +/**
> > + * drm_of_get_dsi_bus - find the DSI bus for a given device
> > + * @dev: parent device of display (SPI, I2C)
> > + * @info: DSI device info to be updated with DSI node. This is optional
> > + * and if not needed can be NULL.
> > + *
> > + * Gets parent DSI bus for a DSI device controlled through a bus other
> > + * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
> > + *
> > + * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
> > + * request is unsupported, -EPROBE_DEFER if the DSI host is found but
> > + * not available, or -ENODEV otherwise.
> > + */
> > +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > +					 struct mipi_dsi_device_info *info)
> > +{
> > +	struct mipi_dsi_host *dsi_host;
> > +	struct device_node *endpoint, *dsi_host_node;
> > +
> > +	/*
> > +	 * Get first endpoint child from device.
> > +	 */
> > +	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > +	if (!endpoint)
> > +		return ERR_PTR(-ENODEV);
> > +
> > +	/*
> > +	 * Follow the first endpoint to get the DSI host node.
> > +	 */
> > +	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> > +	if (!dsi_host_node)
> > +		goto error;
> > +
> > +	/*
> > +	 * Get the DSI host from the DSI host node. If we get an error
> > +	 * or the return is null assume we're not ready to probe just
> > +	 * yet. Release the DSI host node since we're done with it.
> > +	 */
> > +	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> > +	of_node_put(dsi_host_node);
> > +	if (IS_ERR_OR_NULL(dsi_host)) {
> > +		of_node_put(endpoint);
> > +		return ERR_PTR(-EPROBE_DEFER);
> > +	}
> > +
> > +	/*
> > +	 * Set the node of the mipi_dsi_device_info to the correct node
> > +	 * and then release the endpoint node since we're done with it.
> > +	 * since this is optional, check if the info is NULL first.
> > +	 */
> > +	if (info) {
> > +		info->node = of_graph_get_remote_port(endpoint);
> 
> it looks to me that the info->node is actually the DSI device OF node,
> not its host port. Which begs the question, why should we even return it
> there, since there's a pretty big chance that dev->of.node ==
> info->node, and you obviously don't care about the channel and type fields.
> 
> I've had a look and node of the current users of
> mipi_dsi_device_register_full actually register a mipi_dsi_device_info
> with a node pointer set to !NULL, including the driver in this series.
> 
> So, why do we care about the device info at all?
> 

I honestly thought it might be useful, but I can try without it.

> > +		if (IS_ERR_OR_NULL(info->node))
> 
> of_graph_get_remote_port doesn't return an error pointer.
> 
> > --- a/include/drm/drm_of.h
> > +++ b/include/drm/drm_of.h
> > @@ -15,6 +15,8 @@ struct drm_encoder;
> >  struct drm_panel;
> >  struct drm_bridge;
> >  struct device_node;
> > +struct mipi_dsi_device_info;
> > +struct mipi_dsi_host;
> >  
> >  /**
> >   * enum drm_lvds_dual_link_pixels - Pixel order of an LVDS dual-link connection
> > @@ -56,6 +58,8 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
> >  				   int port_reg, int reg,
> >  				   const unsigned int min,
> >  				   const unsigned int max);
> > +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > +					 struct mipi_dsi_device_info *info);
> >  #else
> >  static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
> >  					  struct device_node *port)
> > @@ -127,6 +131,12 @@ drm_of_get_data_lanes_count_ep(const struct device_node *port,
> >  {
> >  	return -EINVAL;
> >  }
> > +static inline struct
> > +mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > +					 struct mipi_dsi_device_info *info)
> > +{
> > +	return ERR_PTR(-EINVAL);
> > +}
> >  #endif
> 
> So it looks to me that if CONFIG_OF is defined, we'll define an external
> symbol declared for drm_of_get_dsi_bus, but that function will only be
> compiled if CONFIG_DRM_MIPI_DSI is enabled.
> 
> What happens if we have CONFIG_OF but not CONFIG_DRM_MIPI_DSI?
> 

Will do, thank you.

> If think you need to have here something like:
> 
> #ifdef IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
>        					 struct mipi_dsi_device_info *info);
> #else
> static inline struct
> mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> 				  struct mipi_dsi_device_info *info)
> {
> 	return ERR_PTR(-EINVAL);
> }
> #endif
> 
> Maxime



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

* Re: [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
@ 2023-01-17 19:12       ` Chris Morgan
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Morgan @ 2023-01-17 19:12 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: dri-devel, linux-rockchip, devicetree, linus.walleij,
	maccraft123mc, tzimmermann, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

On Tue, Jan 17, 2023 at 05:58:19PM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Jan 12, 2023 at 11:53:55AM -0600, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add helper function to find DSI host for devices where DSI panel is not
> > a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
> > official Raspberry Pi touchscreen display).
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_of.h     | 10 ++++++
> >  2 files changed, 80 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> > index 7bbcb999bb75..6c2c97a716fe 100644
> > --- a/drivers/gpu/drm/drm_of.c
> > +++ b/drivers/gpu/drm/drm_of.c
> > @@ -10,6 +10,7 @@
> >  #include <drm/drm_crtc.h>
> >  #include <drm/drm_device.h>
> >  #include <drm/drm_encoder.h>
> > +#include <drm/drm_mipi_dsi.h>
> >  #include <drm/drm_of.h>
> >  #include <drm/drm_panel.h>
> >  
> > @@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
> > +
> > +#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> > +
> > +/**
> > + * drm_of_get_dsi_bus - find the DSI bus for a given device
> > + * @dev: parent device of display (SPI, I2C)
> > + * @info: DSI device info to be updated with DSI node. This is optional
> > + * and if not needed can be NULL.
> > + *
> > + * Gets parent DSI bus for a DSI device controlled through a bus other
> > + * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
> > + *
> > + * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
> > + * request is unsupported, -EPROBE_DEFER if the DSI host is found but
> > + * not available, or -ENODEV otherwise.
> > + */
> > +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > +					 struct mipi_dsi_device_info *info)
> > +{
> > +	struct mipi_dsi_host *dsi_host;
> > +	struct device_node *endpoint, *dsi_host_node;
> > +
> > +	/*
> > +	 * Get first endpoint child from device.
> > +	 */
> > +	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > +	if (!endpoint)
> > +		return ERR_PTR(-ENODEV);
> > +
> > +	/*
> > +	 * Follow the first endpoint to get the DSI host node.
> > +	 */
> > +	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> > +	if (!dsi_host_node)
> > +		goto error;
> > +
> > +	/*
> > +	 * Get the DSI host from the DSI host node. If we get an error
> > +	 * or the return is null assume we're not ready to probe just
> > +	 * yet. Release the DSI host node since we're done with it.
> > +	 */
> > +	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> > +	of_node_put(dsi_host_node);
> > +	if (IS_ERR_OR_NULL(dsi_host)) {
> > +		of_node_put(endpoint);
> > +		return ERR_PTR(-EPROBE_DEFER);
> > +	}
> > +
> > +	/*
> > +	 * Set the node of the mipi_dsi_device_info to the correct node
> > +	 * and then release the endpoint node since we're done with it.
> > +	 * since this is optional, check if the info is NULL first.
> > +	 */
> > +	if (info) {
> > +		info->node = of_graph_get_remote_port(endpoint);
> 
> it looks to me that the info->node is actually the DSI device OF node,
> not its host port. Which begs the question, why should we even return it
> there, since there's a pretty big chance that dev->of.node ==
> info->node, and you obviously don't care about the channel and type fields.
> 
> I've had a look and node of the current users of
> mipi_dsi_device_register_full actually register a mipi_dsi_device_info
> with a node pointer set to !NULL, including the driver in this series.
> 
> So, why do we care about the device info at all?
> 

I honestly thought it might be useful, but I can try without it.

> > +		if (IS_ERR_OR_NULL(info->node))
> 
> of_graph_get_remote_port doesn't return an error pointer.
> 
> > --- a/include/drm/drm_of.h
> > +++ b/include/drm/drm_of.h
> > @@ -15,6 +15,8 @@ struct drm_encoder;
> >  struct drm_panel;
> >  struct drm_bridge;
> >  struct device_node;
> > +struct mipi_dsi_device_info;
> > +struct mipi_dsi_host;
> >  
> >  /**
> >   * enum drm_lvds_dual_link_pixels - Pixel order of an LVDS dual-link connection
> > @@ -56,6 +58,8 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
> >  				   int port_reg, int reg,
> >  				   const unsigned int min,
> >  				   const unsigned int max);
> > +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > +					 struct mipi_dsi_device_info *info);
> >  #else
> >  static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
> >  					  struct device_node *port)
> > @@ -127,6 +131,12 @@ drm_of_get_data_lanes_count_ep(const struct device_node *port,
> >  {
> >  	return -EINVAL;
> >  }
> > +static inline struct
> > +mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > +					 struct mipi_dsi_device_info *info)
> > +{
> > +	return ERR_PTR(-EINVAL);
> > +}
> >  #endif
> 
> So it looks to me that if CONFIG_OF is defined, we'll define an external
> symbol declared for drm_of_get_dsi_bus, but that function will only be
> compiled if CONFIG_DRM_MIPI_DSI is enabled.
> 
> What happens if we have CONFIG_OF but not CONFIG_DRM_MIPI_DSI?
> 

Will do, thank you.

> If think you need to have here something like:
> 
> #ifdef IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
>        					 struct mipi_dsi_device_info *info);
> #else
> static inline struct
> mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> 				  struct mipi_dsi_device_info *info)
> {
> 	return ERR_PTR(-EINVAL);
> }
> #endif
> 
> Maxime



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

* Re: [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
  2023-01-17 19:12       ` Chris Morgan
  (?)
@ 2023-01-18  8:51         ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2023-01-18  8:51 UTC (permalink / raw)
  To: Chris Morgan
  Cc: dri-devel, linux-rockchip, devicetree, linus.walleij,
	maccraft123mc, tzimmermann, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan


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

On Tue, Jan 17, 2023 at 01:12:39PM -0600, Chris Morgan wrote:
> On Tue, Jan 17, 2023 at 05:58:19PM +0100, Maxime Ripard wrote:
> > Hi,
> > 
> > On Thu, Jan 12, 2023 at 11:53:55AM -0600, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add helper function to find DSI host for devices where DSI panel is not
> > > a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
> > > official Raspberry Pi touchscreen display).
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> > > ---
> > >  drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
> > >  include/drm/drm_of.h     | 10 ++++++
> > >  2 files changed, 80 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> > > index 7bbcb999bb75..6c2c97a716fe 100644
> > > --- a/drivers/gpu/drm/drm_of.c
> > > +++ b/drivers/gpu/drm/drm_of.c
> > > @@ -10,6 +10,7 @@
> > >  #include <drm/drm_crtc.h>
> > >  #include <drm/drm_device.h>
> > >  #include <drm/drm_encoder.h>
> > > +#include <drm/drm_mipi_dsi.h>
> > >  #include <drm/drm_of.h>
> > >  #include <drm/drm_panel.h>
> > >  
> > > @@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
> > >  	return ret;
> > >  }
> > >  EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
> > > +
> > > +#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> > > +
> > > +/**
> > > + * drm_of_get_dsi_bus - find the DSI bus for a given device
> > > + * @dev: parent device of display (SPI, I2C)
> > > + * @info: DSI device info to be updated with DSI node. This is optional
> > > + * and if not needed can be NULL.
> > > + *
> > > + * Gets parent DSI bus for a DSI device controlled through a bus other
> > > + * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
> > > + *
> > > + * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
> > > + * request is unsupported, -EPROBE_DEFER if the DSI host is found but
> > > + * not available, or -ENODEV otherwise.
> > > + */
> > > +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > > +					 struct mipi_dsi_device_info *info)
> > > +{
> > > +	struct mipi_dsi_host *dsi_host;
> > > +	struct device_node *endpoint, *dsi_host_node;
> > > +
> > > +	/*
> > > +	 * Get first endpoint child from device.
> > > +	 */
> > > +	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > > +	if (!endpoint)
> > > +		return ERR_PTR(-ENODEV);
> > > +
> > > +	/*
> > > +	 * Follow the first endpoint to get the DSI host node.
> > > +	 */
> > > +	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> > > +	if (!dsi_host_node)
> > > +		goto error;
> > > +
> > > +	/*
> > > +	 * Get the DSI host from the DSI host node. If we get an error
> > > +	 * or the return is null assume we're not ready to probe just
> > > +	 * yet. Release the DSI host node since we're done with it.
> > > +	 */
> > > +	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> > > +	of_node_put(dsi_host_node);
> > > +	if (IS_ERR_OR_NULL(dsi_host)) {
> > > +		of_node_put(endpoint);
> > > +		return ERR_PTR(-EPROBE_DEFER);
> > > +	}
> > > +
> > > +	/*
> > > +	 * Set the node of the mipi_dsi_device_info to the correct node
> > > +	 * and then release the endpoint node since we're done with it.
> > > +	 * since this is optional, check if the info is NULL first.
> > > +	 */
> > > +	if (info) {
> > > +		info->node = of_graph_get_remote_port(endpoint);
> > 
> > it looks to me that the info->node is actually the DSI device OF node,
> > not its host port. Which begs the question, why should we even return it
> > there, since there's a pretty big chance that dev->of.node ==
> > info->node, and you obviously don't care about the channel and type fields.
> > 
> > I've had a look and node of the current users of
> > mipi_dsi_device_register_full actually register a mipi_dsi_device_info
> > with a node pointer set to !NULL, including the driver in this series.
> > 
> > So, why do we care about the device info at all?
>
> I honestly thought it might be useful, but I can try without it.

It might tbh, but it doesn't look like you use it in your driver. You have:

struct mipi_dsi_device_info info = {
	.type = "d53e6ea8966",
	.channel = 0,
	.node = NULL,
};

...

// info.node is NULL so far
dsi_host = drm_of_get_dsi_bus(dev, &info);

...

// info.node has been filled to the port node by drm_of_get_dsi_bus()
db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);

// db->dsi_dev.dev.of_node is now set to the port node

But if we grep through drm_mipi_dsi.c, we can see that the of_node is
only really useful if we're using of_find_mipi_dsi_device_by_node, and
it looks like you don't.

So nothing uses info->node, which also explains why not reporting the
proper node has been working.

Looking more into the code, it really looks to me that info->node should
be equal to the your panel device tree node, that's what
of_mipi_dsi_device_add does at least.

if info->node == dev->of_node, and if info->node is the only thing
filled by drm_of_get_dsi_bus(), then it doesn't need to fill it at all
because it's already accessible easily to the caller (and even more
easily than to the callee).

So yeah, until we have a real-world need to retrieve the info function I
think we should leave it aside for now, and we can always change the API
later if we need to.

Maxime

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

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
@ 2023-01-18  8:51         ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2023-01-18  8:51 UTC (permalink / raw)
  To: Chris Morgan
  Cc: devicetree, Chris Morgan, krzysztof.kozlowski+dt, sam, dri-devel,
	linux-rockchip, robh+dt, thierry.reding, tzimmermann,
	maccraft123mc

[-- Attachment #1: Type: text/plain, Size: 5563 bytes --]

On Tue, Jan 17, 2023 at 01:12:39PM -0600, Chris Morgan wrote:
> On Tue, Jan 17, 2023 at 05:58:19PM +0100, Maxime Ripard wrote:
> > Hi,
> > 
> > On Thu, Jan 12, 2023 at 11:53:55AM -0600, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add helper function to find DSI host for devices where DSI panel is not
> > > a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
> > > official Raspberry Pi touchscreen display).
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> > > ---
> > >  drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
> > >  include/drm/drm_of.h     | 10 ++++++
> > >  2 files changed, 80 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> > > index 7bbcb999bb75..6c2c97a716fe 100644
> > > --- a/drivers/gpu/drm/drm_of.c
> > > +++ b/drivers/gpu/drm/drm_of.c
> > > @@ -10,6 +10,7 @@
> > >  #include <drm/drm_crtc.h>
> > >  #include <drm/drm_device.h>
> > >  #include <drm/drm_encoder.h>
> > > +#include <drm/drm_mipi_dsi.h>
> > >  #include <drm/drm_of.h>
> > >  #include <drm/drm_panel.h>
> > >  
> > > @@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
> > >  	return ret;
> > >  }
> > >  EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
> > > +
> > > +#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> > > +
> > > +/**
> > > + * drm_of_get_dsi_bus - find the DSI bus for a given device
> > > + * @dev: parent device of display (SPI, I2C)
> > > + * @info: DSI device info to be updated with DSI node. This is optional
> > > + * and if not needed can be NULL.
> > > + *
> > > + * Gets parent DSI bus for a DSI device controlled through a bus other
> > > + * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
> > > + *
> > > + * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
> > > + * request is unsupported, -EPROBE_DEFER if the DSI host is found but
> > > + * not available, or -ENODEV otherwise.
> > > + */
> > > +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > > +					 struct mipi_dsi_device_info *info)
> > > +{
> > > +	struct mipi_dsi_host *dsi_host;
> > > +	struct device_node *endpoint, *dsi_host_node;
> > > +
> > > +	/*
> > > +	 * Get first endpoint child from device.
> > > +	 */
> > > +	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > > +	if (!endpoint)
> > > +		return ERR_PTR(-ENODEV);
> > > +
> > > +	/*
> > > +	 * Follow the first endpoint to get the DSI host node.
> > > +	 */
> > > +	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> > > +	if (!dsi_host_node)
> > > +		goto error;
> > > +
> > > +	/*
> > > +	 * Get the DSI host from the DSI host node. If we get an error
> > > +	 * or the return is null assume we're not ready to probe just
> > > +	 * yet. Release the DSI host node since we're done with it.
> > > +	 */
> > > +	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> > > +	of_node_put(dsi_host_node);
> > > +	if (IS_ERR_OR_NULL(dsi_host)) {
> > > +		of_node_put(endpoint);
> > > +		return ERR_PTR(-EPROBE_DEFER);
> > > +	}
> > > +
> > > +	/*
> > > +	 * Set the node of the mipi_dsi_device_info to the correct node
> > > +	 * and then release the endpoint node since we're done with it.
> > > +	 * since this is optional, check if the info is NULL first.
> > > +	 */
> > > +	if (info) {
> > > +		info->node = of_graph_get_remote_port(endpoint);
> > 
> > it looks to me that the info->node is actually the DSI device OF node,
> > not its host port. Which begs the question, why should we even return it
> > there, since there's a pretty big chance that dev->of.node ==
> > info->node, and you obviously don't care about the channel and type fields.
> > 
> > I've had a look and node of the current users of
> > mipi_dsi_device_register_full actually register a mipi_dsi_device_info
> > with a node pointer set to !NULL, including the driver in this series.
> > 
> > So, why do we care about the device info at all?
>
> I honestly thought it might be useful, but I can try without it.

It might tbh, but it doesn't look like you use it in your driver. You have:

struct mipi_dsi_device_info info = {
	.type = "d53e6ea8966",
	.channel = 0,
	.node = NULL,
};

...

// info.node is NULL so far
dsi_host = drm_of_get_dsi_bus(dev, &info);

...

// info.node has been filled to the port node by drm_of_get_dsi_bus()
db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);

// db->dsi_dev.dev.of_node is now set to the port node

But if we grep through drm_mipi_dsi.c, we can see that the of_node is
only really useful if we're using of_find_mipi_dsi_device_by_node, and
it looks like you don't.

So nothing uses info->node, which also explains why not reporting the
proper node has been working.

Looking more into the code, it really looks to me that info->node should
be equal to the your panel device tree node, that's what
of_mipi_dsi_device_add does at least.

if info->node == dev->of_node, and if info->node is the only thing
filled by drm_of_get_dsi_bus(), then it doesn't need to fill it at all
because it's already accessible easily to the caller (and even more
easily than to the callee).

So yeah, until we have a real-world need to retrieve the info function I
think we should leave it aside for now, and we can always change the API
later if we need to.

Maxime

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

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

* Re: [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function
@ 2023-01-18  8:51         ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2023-01-18  8:51 UTC (permalink / raw)
  To: Chris Morgan
  Cc: dri-devel, linux-rockchip, devicetree, linus.walleij,
	maccraft123mc, tzimmermann, maarten.lankhorst, heiko,
	krzysztof.kozlowski+dt, robh+dt, daniel, airlied, sam,
	thierry.reding, Chris Morgan

[-- Attachment #1: Type: text/plain, Size: 5563 bytes --]

On Tue, Jan 17, 2023 at 01:12:39PM -0600, Chris Morgan wrote:
> On Tue, Jan 17, 2023 at 05:58:19PM +0100, Maxime Ripard wrote:
> > Hi,
> > 
> > On Thu, Jan 12, 2023 at 11:53:55AM -0600, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add helper function to find DSI host for devices where DSI panel is not
> > > a minor of a DSI bus (such as the Samsung AMS495QA01 panel or the
> > > official Raspberry Pi touchscreen display).
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> > > ---
> > >  drivers/gpu/drm/drm_of.c | 70 ++++++++++++++++++++++++++++++++++++++++
> > >  include/drm/drm_of.h     | 10 ++++++
> > >  2 files changed, 80 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> > > index 7bbcb999bb75..6c2c97a716fe 100644
> > > --- a/drivers/gpu/drm/drm_of.c
> > > +++ b/drivers/gpu/drm/drm_of.c
> > > @@ -10,6 +10,7 @@
> > >  #include <drm/drm_crtc.h>
> > >  #include <drm/drm_device.h>
> > >  #include <drm/drm_encoder.h>
> > > +#include <drm/drm_mipi_dsi.h>
> > >  #include <drm/drm_of.h>
> > >  #include <drm/drm_panel.h>
> > >  
> > > @@ -493,3 +494,72 @@ int drm_of_get_data_lanes_count_ep(const struct device_node *port,
> > >  	return ret;
> > >  }
> > >  EXPORT_SYMBOL_GPL(drm_of_get_data_lanes_count_ep);
> > > +
> > > +#if IS_ENABLED(CONFIG_DRM_MIPI_DSI)
> > > +
> > > +/**
> > > + * drm_of_get_dsi_bus - find the DSI bus for a given device
> > > + * @dev: parent device of display (SPI, I2C)
> > > + * @info: DSI device info to be updated with DSI node. This is optional
> > > + * and if not needed can be NULL.
> > > + *
> > > + * Gets parent DSI bus for a DSI device controlled through a bus other
> > > + * than MIPI-DCS (SPI, I2C, etc.) using the Device Tree.
> > > + *
> > > + * Returns pointer to mipi_dsi_host if successful, -EINVAL if the
> > > + * request is unsupported, -EPROBE_DEFER if the DSI host is found but
> > > + * not available, or -ENODEV otherwise.
> > > + */
> > > +struct mipi_dsi_host *drm_of_get_dsi_bus(struct device *dev,
> > > +					 struct mipi_dsi_device_info *info)
> > > +{
> > > +	struct mipi_dsi_host *dsi_host;
> > > +	struct device_node *endpoint, *dsi_host_node;
> > > +
> > > +	/*
> > > +	 * Get first endpoint child from device.
> > > +	 */
> > > +	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > > +	if (!endpoint)
> > > +		return ERR_PTR(-ENODEV);
> > > +
> > > +	/*
> > > +	 * Follow the first endpoint to get the DSI host node.
> > > +	 */
> > > +	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> > > +	if (!dsi_host_node)
> > > +		goto error;
> > > +
> > > +	/*
> > > +	 * Get the DSI host from the DSI host node. If we get an error
> > > +	 * or the return is null assume we're not ready to probe just
> > > +	 * yet. Release the DSI host node since we're done with it.
> > > +	 */
> > > +	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> > > +	of_node_put(dsi_host_node);
> > > +	if (IS_ERR_OR_NULL(dsi_host)) {
> > > +		of_node_put(endpoint);
> > > +		return ERR_PTR(-EPROBE_DEFER);
> > > +	}
> > > +
> > > +	/*
> > > +	 * Set the node of the mipi_dsi_device_info to the correct node
> > > +	 * and then release the endpoint node since we're done with it.
> > > +	 * since this is optional, check if the info is NULL first.
> > > +	 */
> > > +	if (info) {
> > > +		info->node = of_graph_get_remote_port(endpoint);
> > 
> > it looks to me that the info->node is actually the DSI device OF node,
> > not its host port. Which begs the question, why should we even return it
> > there, since there's a pretty big chance that dev->of.node ==
> > info->node, and you obviously don't care about the channel and type fields.
> > 
> > I've had a look and node of the current users of
> > mipi_dsi_device_register_full actually register a mipi_dsi_device_info
> > with a node pointer set to !NULL, including the driver in this series.
> > 
> > So, why do we care about the device info at all?
>
> I honestly thought it might be useful, but I can try without it.

It might tbh, but it doesn't look like you use it in your driver. You have:

struct mipi_dsi_device_info info = {
	.type = "d53e6ea8966",
	.channel = 0,
	.node = NULL,
};

...

// info.node is NULL so far
dsi_host = drm_of_get_dsi_bus(dev, &info);

...

// info.node has been filled to the port node by drm_of_get_dsi_bus()
db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);

// db->dsi_dev.dev.of_node is now set to the port node

But if we grep through drm_mipi_dsi.c, we can see that the of_node is
only really useful if we're using of_find_mipi_dsi_device_by_node, and
it looks like you don't.

So nothing uses info->node, which also explains why not reporting the
proper node has been working.

Looking more into the code, it really looks to me that info->node should
be equal to the your panel device tree node, that's what
of_mipi_dsi_device_add does at least.

if info->node == dev->of_node, and if info->node is the only thing
filled by drm_of_get_dsi_bus(), then it doesn't need to fill it at all
because it's already accessible easily to the caller (and even more
easily than to the callee).

So yeah, until we have a real-world need to retrieve the info function I
think we should leave it aside for now, and we can always change the API
later if we need to.

Maxime

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

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

end of thread, other threads:[~2023-01-18  9:28 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-12 17:53 [PATCH V10 0/4] drm/panel: Add Magnachip D53E6EA8966 Panel Controller Chris Morgan
2023-01-12 17:53 ` Chris Morgan
2023-01-12 17:53 ` Chris Morgan
2023-01-12 17:53 ` [PATCH V10 1/4] drm: of: Add drm_of_get_dsi_bus helper function Chris Morgan
2023-01-12 17:53   ` Chris Morgan
2023-01-12 17:53   ` Chris Morgan
2023-01-17 16:58   ` Maxime Ripard
2023-01-17 16:58     ` Maxime Ripard
2023-01-17 16:58     ` Maxime Ripard
2023-01-17 19:12     ` Chris Morgan
2023-01-17 19:12       ` Chris Morgan
2023-01-17 19:12       ` Chris Morgan
2023-01-18  8:51       ` Maxime Ripard
2023-01-18  8:51         ` Maxime Ripard
2023-01-18  8:51         ` Maxime Ripard
2023-01-12 17:53 ` [PATCH V10 2/4] dt-bindings: display: panel: Add Samsung AMS495QA01 Chris Morgan
2023-01-12 17:53   ` Chris Morgan
2023-01-12 17:53   ` Chris Morgan
2023-01-12 17:53 ` [PATCH V10 3/4] drm/panel: Add Magnachip D53E6EA8966 Panel Driver Chris Morgan
2023-01-12 17:53   ` Chris Morgan
2023-01-12 17:53   ` Chris Morgan
2023-01-12 17:53 ` [PATCH V10 4/4] arm64: dts: rockchip: add display to RG503 Chris Morgan
2023-01-12 17:53   ` Chris Morgan
2023-01-12 17:53   ` Chris Morgan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.