linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 RESEND 0/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support
@ 2024-08-16  8:09 Liu Ying
  2024-08-16  8:09 ` [PATCH v3 RESEND 1/2] dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example Liu Ying
  2024-08-16  8:09 ` [PATCH v3 RESEND 2/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support Liu Ying
  0 siblings, 2 replies; 6+ messages in thread
From: Liu Ying @ 2024-08-16  8:09 UTC (permalink / raw)
  To: devicetree, imx, linux-arm-kernel, linux-kernel, dri-devel
  Cc: robh, krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
	victor.liu, andrzej.hajda, neil.armstrong, rfoss,
	Laurent.pinchart, jonas, jernej.skrabec, maarten.lankhorst,
	mripard, tzimmermann, airlied, daniel, peng.fan

Hi,

This patch set aims to add NXP i.MX93 parallel display format configuration
DRM bridge driver support. i.MX93 mediamix blk-ctrl contains one
DISPLAY_MUX register which configures parallel display format by using
the "PARALLEL_DISP_FORMAT" field. i.MX93 LCDIF display controller's
parallel output connects with this piece of small logic to configure
parallel display format.

Patch 1/2 adds NXP i.MX93 parallel display format configuration subnode
in i.MX93 mediamix blk-ctrl dt-binding.

Patch 2/2 adds NXP i.MX93 parallel display format configuration DRM bridge
driver support.

v2->v3:
* Define i.MX93 parallel display format configuration subnode in
  i.MX93 mediamix blk-ctrl dt-binding. (Rob)
* Resend with Conor's R-b tag on patch 1/2 and with the patch set rebased
  upon v6.11-rc1.

v1->v2:
* Set *num_input_fmts to zero in case
  imx93_pdfc_bridge_atomic_get_input_bus_fmts() returns NULL in patch 2/2.
* Replace .remove callback with .remove_new callback in
  imx93_pdfc_bridge_driver in patch 2/2.


Liu Ying (2):
  dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and
    example
  drm/bridge: imx: Add i.MX93 parallel display format configuration
    support

 .../soc/imx/fsl,imx93-media-blk-ctrl.yaml     |  68 ++++++
 drivers/gpu/drm/bridge/imx/Kconfig            |   8 +
 drivers/gpu/drm/bridge/imx/Makefile           |   1 +
 drivers/gpu/drm/bridge/imx/imx93-pdfc.c       | 209 ++++++++++++++++++
 4 files changed, 286 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/imx/imx93-pdfc.c

-- 
2.34.1



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

* [PATCH v3 RESEND 1/2] dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example
  2024-08-16  8:09 [PATCH v3 RESEND 0/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support Liu Ying
@ 2024-08-16  8:09 ` Liu Ying
  2024-08-16  8:09 ` [PATCH v3 RESEND 2/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support Liu Ying
  1 sibling, 0 replies; 6+ messages in thread
From: Liu Ying @ 2024-08-16  8:09 UTC (permalink / raw)
  To: devicetree, imx, linux-arm-kernel, linux-kernel, dri-devel
  Cc: robh, krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
	victor.liu, andrzej.hajda, neil.armstrong, rfoss,
	Laurent.pinchart, jonas, jernej.skrabec, maarten.lankhorst,
	mripard, tzimmermann, airlied, daniel, peng.fan, Conor Dooley

i.MX93 SoC mediamix blk-ctrl contains one DISPLAY_MUX register which
configures parallel display format by using the "PARALLEL_DISP_FORMAT"
field. Document the Parallel Display Format Configuration(PDFC) subnode
and add the subnode to example.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
v2->v3:
* Newly introduced to replace the standalone dt-binding in v1 and v2. (Rob)
* Resend with Conor's R-b tag and with the patch rebased upon v6.11-rc1.

 .../soc/imx/fsl,imx93-media-blk-ctrl.yaml     | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml
index b3554e7f9e76..3f550c30d93d 100644
--- a/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml
@@ -24,6 +24,12 @@ properties:
   reg:
     maxItems: 1
 
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
   '#power-domain-cells':
     const: 1
 
@@ -46,9 +52,43 @@ properties:
       - const: csi
       - const: dsi
 
+  bridge@60:
+    type: object
+    additionalProperties: false
+
+    properties:
+      compatible:
+        const: nxp,imx93-pdfc
+
+      reg:
+        maxItems: 1
+
+      ports:
+        $ref: /schemas/graph.yaml#/properties/ports
+
+        properties:
+          port@0:
+            $ref: /schemas/graph.yaml#/properties/port
+            description: Input port node to receive pixel data.
+
+          port@1:
+            $ref: /schemas/graph.yaml#/properties/port
+            description: Output port node to downstream pixel data receivers.
+
+        required:
+          - port@0
+          - port@1
+
+    required:
+      - compatible
+      - reg
+      - ports
+
 required:
   - compatible
   - reg
+  - '#address-cells'
+  - '#size-cells'
   - power-domains
   - clocks
   - clock-names
@@ -76,5 +116,33 @@ examples:
                <&clk IMX93_CLK_MIPI_DSI_GATE>;
                clock-names = "apb", "axi", "nic", "disp", "cam",
                              "pxp", "lcdif", "isi", "csi", "dsi";
+      #address-cells = <1>;
+      #size-cells = <1>;
       #power-domain-cells = <1>;
+
+      bridge@60 {
+        compatible = "nxp,imx93-pdfc";
+        reg = <0x60 0x4>;
+
+        ports {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          port@0 {
+            reg = <0>;
+
+            pdfc_from_lcdif: endpoint {
+              remote-endpoint = <&lcdif_to_pdfc>;
+            };
+          };
+
+          port@1 {
+            reg = <1>;
+
+            pdfc_to_panel: endpoint {
+              remote-endpoint = <&panel_from_pdfc>;
+            };
+          };
+        };
+      };
     };
-- 
2.34.1



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

* [PATCH v3 RESEND 2/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support
  2024-08-16  8:09 [PATCH v3 RESEND 0/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support Liu Ying
  2024-08-16  8:09 ` [PATCH v3 RESEND 1/2] dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example Liu Ying
@ 2024-08-16  8:09 ` Liu Ying
  2024-08-16  9:27   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 6+ messages in thread
From: Liu Ying @ 2024-08-16  8:09 UTC (permalink / raw)
  To: devicetree, imx, linux-arm-kernel, linux-kernel, dri-devel
  Cc: robh, krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
	victor.liu, andrzej.hajda, neil.armstrong, rfoss,
	Laurent.pinchart, jonas, jernej.skrabec, maarten.lankhorst,
	mripard, tzimmermann, airlied, daniel, peng.fan

NXP i.MX93 mediamix blk-ctrl contains one DISPLAY_MUX register which
configures parallel display format by using the "PARALLEL_DISP_FORMAT"
field. Add a DRM bridge driver to support the display format configuration.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v2->v3:
* No change.
* Resend with the patch rebased upon v6.11-rc1.

v1->v2:
* Set *num_input_fmts to zero in case
  imx93_pdfc_bridge_atomic_get_input_bus_fmts() returns NULL.
* Replace .remove callback with .remove_new callback in
  imx93_pdfc_bridge_driver.

 drivers/gpu/drm/bridge/imx/Kconfig      |   8 +
 drivers/gpu/drm/bridge/imx/Makefile     |   1 +
 drivers/gpu/drm/bridge/imx/imx93-pdfc.c | 209 ++++++++++++++++++++++++
 3 files changed, 218 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/imx/imx93-pdfc.c

diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
index 8dd89efa8ea7..088241575857 100644
--- a/drivers/gpu/drm/bridge/imx/Kconfig
+++ b/drivers/gpu/drm/bridge/imx/Kconfig
@@ -78,4 +78,12 @@ config DRM_IMX93_MIPI_DSI
 	  Choose this to enable MIPI DSI controller found in Freescale i.MX93
 	  processor.
 
+config DRM_IMX93_PARALLEL_DISP_FMT_CONFIG
+	tristate "NXP i.MX93 parallel display format configuration"
+	depends on OF
+	select DRM_KMS_HELPER
+	help
+	  Choose this to enable parallel display format configuration
+	  found in NXP i.MX93 processor.
+
 endif # ARCH_MXC || COMPILE_TEST
diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
index edb0a7b71b30..8d3499fb7fba 100644
--- a/drivers/gpu/drm/bridge/imx/Makefile
+++ b/drivers/gpu/drm/bridge/imx/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
 obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK) += imx8qxp-pixel-link.o
 obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK_TO_DPI) += imx8qxp-pxl2dpi.o
 obj-$(CONFIG_DRM_IMX93_MIPI_DSI) += imx93-mipi-dsi.o
+obj-$(CONFIG_DRM_IMX93_PARALLEL_DISP_FMT_CONFIG) += imx93-pdfc.o
diff --git a/drivers/gpu/drm/bridge/imx/imx93-pdfc.c b/drivers/gpu/drm/bridge/imx/imx93-pdfc.c
new file mode 100644
index 000000000000..61e01b503be5
--- /dev/null
+++ b/drivers/gpu/drm/bridge/imx/imx93-pdfc.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright 2022,2023 NXP
+ */
+
+#include <linux/media-bus-format.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_print.h>
+
+#define DRIVER_NAME		"imx93_pdfc"
+
+#define DISPLAY_MUX		0x60
+#define  PARALLEL_DISP_FORMAT	0x700
+
+enum imx93_pdfc_format {
+	RGB888_TO_RGB888 = 0x0,
+	RGB888_TO_RGB666 = 0x1 << 8,
+	RGB565_TO_RGB565 = 0x2 << 8,
+};
+
+struct imx93_pdfc {
+	struct drm_bridge bridge;
+	struct drm_bridge *next_bridge;
+	struct device *dev;
+	struct regmap *regmap;
+	u32 format;
+};
+
+static int imx93_pdfc_bridge_attach(struct drm_bridge *bridge,
+				    enum drm_bridge_attach_flags flags)
+{
+	struct imx93_pdfc *pdfc = bridge->driver_private;
+
+	return drm_bridge_attach(bridge->encoder, pdfc->next_bridge, bridge, flags);
+}
+
+static void
+imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
+				struct drm_bridge_state *old_bridge_state)
+{
+	struct imx93_pdfc *pdfc = bridge->driver_private;
+
+	regmap_update_bits(pdfc->regmap, DISPLAY_MUX, PARALLEL_DISP_FORMAT,
+			   pdfc->format);
+}
+
+static const u32 imx93_pdfc_bus_output_fmts[] = {
+	MEDIA_BUS_FMT_RGB888_1X24,
+	MEDIA_BUS_FMT_RGB666_1X18,
+	MEDIA_BUS_FMT_RGB565_1X16,
+	MEDIA_BUS_FMT_FIXED
+};
+
+static bool imx93_pdfc_bus_output_fmt_supported(u32 fmt)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(imx93_pdfc_bus_output_fmts); i++) {
+		if (imx93_pdfc_bus_output_fmts[i] == fmt)
+			return true;
+	}
+
+	return false;
+}
+
+static u32 *
+imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+					    struct drm_bridge_state *bridge_state,
+					    struct drm_crtc_state *crtc_state,
+					    struct drm_connector_state *conn_state,
+					    u32 output_fmt,
+					    unsigned int *num_input_fmts)
+{
+	u32 *input_fmts;
+
+	*num_input_fmts = 0;
+
+	if (!imx93_pdfc_bus_output_fmt_supported(output_fmt))
+		return NULL;
+
+	input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	switch (output_fmt) {
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		input_fmts[0] = output_fmt;
+		break;
+	case MEDIA_BUS_FMT_RGB666_1X18:
+	case MEDIA_BUS_FMT_FIXED:
+		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
+	}
+
+	*num_input_fmts = 1;
+
+	return input_fmts;
+}
+
+static int imx93_pdfc_bridge_atomic_check(struct drm_bridge *bridge,
+					  struct drm_bridge_state *bridge_state,
+					  struct drm_crtc_state *crtc_state,
+					  struct drm_connector_state *conn_state)
+{
+	struct imx93_pdfc *pdfc = bridge->driver_private;
+
+	switch (bridge_state->output_bus_cfg.format) {
+	case MEDIA_BUS_FMT_RGB888_1X24:
+		pdfc->format = RGB888_TO_RGB888;
+		break;
+	case MEDIA_BUS_FMT_RGB666_1X18:
+		pdfc->format = RGB888_TO_RGB666;
+		break;
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		pdfc->format = RGB565_TO_RGB565;
+		break;
+	default:
+		DRM_DEV_DEBUG_DRIVER(pdfc->dev, "Unsupported output bus format: 0x%x\n",
+				     bridge_state->output_bus_cfg.format);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct drm_bridge_funcs imx93_pdfc_bridge_funcs = {
+	.attach			= imx93_pdfc_bridge_attach,
+	.atomic_enable		= imx93_pdfc_bridge_atomic_enable,
+	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
+	.atomic_get_input_bus_fmts	= imx93_pdfc_bridge_atomic_get_input_bus_fmts,
+	.atomic_check		= imx93_pdfc_bridge_atomic_check,
+	.atomic_reset		= drm_atomic_helper_bridge_reset,
+};
+
+static int imx93_pdfc_bridge_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct imx93_pdfc *pdfc;
+	int ret;
+
+	pdfc = devm_kzalloc(dev, sizeof(*pdfc), GFP_KERNEL);
+	if (!pdfc)
+		return -ENOMEM;
+
+	pdfc->regmap = syscon_node_to_regmap(dev->of_node->parent);
+	if (IS_ERR(pdfc->regmap)) {
+		ret = PTR_ERR(pdfc->regmap);
+		if (ret != -EPROBE_DEFER)
+			DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
+		return ret;
+	}
+
+	pdfc->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	if (IS_ERR(pdfc->next_bridge)) {
+		ret = PTR_ERR(pdfc->next_bridge);
+		if (ret != -EPROBE_DEFER)
+			DRM_DEV_ERROR(dev, "failed to get next bridge: %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, pdfc);
+
+	pdfc->dev = dev;
+	pdfc->bridge.driver_private = pdfc;
+	pdfc->bridge.funcs = &imx93_pdfc_bridge_funcs;
+	pdfc->bridge.of_node = dev->of_node;
+
+	drm_bridge_add(&pdfc->bridge);
+
+	return 0;
+}
+
+static void imx93_pdfc_bridge_remove(struct platform_device *pdev)
+{
+	struct imx93_pdfc *pdfc = platform_get_drvdata(pdev);
+
+	drm_bridge_remove(&pdfc->bridge);
+}
+
+static const struct of_device_id imx93_pdfc_dt_ids[] = {
+	{ .compatible = "nxp,imx93-pdfc", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx93_pdfc_dt_ids);
+
+static struct platform_driver imx93_pdfc_bridge_driver = {
+	.probe	= imx93_pdfc_bridge_probe,
+	.remove_new = imx93_pdfc_bridge_remove,
+	.driver	= {
+		.of_match_table = imx93_pdfc_dt_ids,
+		.name = DRIVER_NAME,
+	},
+};
+module_platform_driver(imx93_pdfc_bridge_driver);
+
+MODULE_DESCRIPTION("NXP i.MX93 parallel display format configuration driver");
+MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
2.34.1



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

* Re: [PATCH v3 RESEND 2/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support
  2024-08-16  8:09 ` [PATCH v3 RESEND 2/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support Liu Ying
@ 2024-08-16  9:27   ` Krzysztof Kozlowski
  2024-08-16  9:44     ` Liu Ying
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-16  9:27 UTC (permalink / raw)
  To: Liu Ying, devicetree, imx, linux-arm-kernel, linux-kernel,
	dri-devel
  Cc: robh, krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
	andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, jonas,
	jernej.skrabec, maarten.lankhorst, mripard, tzimmermann, airlied,
	daniel, peng.fan

On 16/08/2024 10:09, Liu Ying wrote:
> NXP i.MX93 mediamix blk-ctrl contains one DISPLAY_MUX register which
> configures parallel display format by using the "PARALLEL_DISP_FORMAT"
> field. Add a DRM bridge driver to support the display format configuration.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---

...

> +
> +static int imx93_pdfc_bridge_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct imx93_pdfc *pdfc;
> +	int ret;
> +
> +	pdfc = devm_kzalloc(dev, sizeof(*pdfc), GFP_KERNEL);
> +	if (!pdfc)
> +		return -ENOMEM;
> +
> +	pdfc->regmap = syscon_node_to_regmap(dev->of_node->parent);
> +	if (IS_ERR(pdfc->regmap)) {
> +		ret = PTR_ERR(pdfc->regmap);
> +		if (ret != -EPROBE_DEFER)
> +			DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
> +		return ret;

Nope, you just open-coded dev_err_probe. Syntax is - return
dev_err_probe(). if you need wrapper for DRM, add such.

> +	}
> +
> +	pdfc->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +	if (IS_ERR(pdfc->next_bridge)) {
> +		ret = PTR_ERR(pdfc->next_bridge);
> +		if (ret != -EPROBE_DEFER)
> +			DRM_DEV_ERROR(dev, "failed to get next bridge: %d\n", ret);
> +		return ret;

Ditto


> +	}
> +

...

> +MODULE_DESCRIPTION("NXP i.MX93 parallel display format configuration driver");
> +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRIVER_NAME);

Which other driver needs this platform alias?

Best regards,
Krzysztof



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

* Re: [PATCH v3 RESEND 2/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support
  2024-08-16  9:27   ` Krzysztof Kozlowski
@ 2024-08-16  9:44     ` Liu Ying
  2024-08-16 12:24       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Liu Ying @ 2024-08-16  9:44 UTC (permalink / raw)
  To: Krzysztof Kozlowski, devicetree, imx, linux-arm-kernel,
	linux-kernel, dri-devel
  Cc: robh, krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
	andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, jonas,
	jernej.skrabec, maarten.lankhorst, mripard, tzimmermann, airlied,
	daniel, peng.fan

On 08/16/2024, Krzysztof Kozlowski wrote:
> On 16/08/2024 10:09, Liu Ying wrote:
>> NXP i.MX93 mediamix blk-ctrl contains one DISPLAY_MUX register which
>> configures parallel display format by using the "PARALLEL_DISP_FORMAT"
>> field. Add a DRM bridge driver to support the display format configuration.
>>
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
> 
> ...
> 
>> +
>> +static int imx93_pdfc_bridge_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct imx93_pdfc *pdfc;
>> +	int ret;
>> +
>> +	pdfc = devm_kzalloc(dev, sizeof(*pdfc), GFP_KERNEL);
>> +	if (!pdfc)
>> +		return -ENOMEM;
>> +
>> +	pdfc->regmap = syscon_node_to_regmap(dev->of_node->parent);
>> +	if (IS_ERR(pdfc->regmap)) {
>> +		ret = PTR_ERR(pdfc->regmap);
>> +		if (ret != -EPROBE_DEFER)
>> +			DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
>> +		return ret;
> 
> Nope, you just open-coded dev_err_probe. Syntax is - return
> dev_err_probe(). if you need wrapper for DRM, add such.

Will use dev_err_probe().

> 
>> +	}
>> +
>> +	pdfc->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
>> +	if (IS_ERR(pdfc->next_bridge)) {
>> +		ret = PTR_ERR(pdfc->next_bridge);
>> +		if (ret != -EPROBE_DEFER)
>> +			DRM_DEV_ERROR(dev, "failed to get next bridge: %d\n", ret);
>> +		return ret;
> 
> Ditto

Will use dev_err_probe().

> 
> 
>> +	}
>> +
> 
> ...
> 
>> +MODULE_DESCRIPTION("NXP i.MX93 parallel display format configuration driver");
>> +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:" DRIVER_NAME);
> 
> Which other driver needs this platform alias?

Quote include/linux/module.h:

"
/* For userspace: you can also call me... */                                     
#define MODULE_ALIAS(_alias) MODULE_INFO(alias, _alias)   
"

Anything wrong with using MODULE_ALIAS() here?

> 
> Best regards,
> Krzysztof
> 

-- 
Regards,
Liu Ying



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

* Re: [PATCH v3 RESEND 2/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support
  2024-08-16  9:44     ` Liu Ying
@ 2024-08-16 12:24       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-16 12:24 UTC (permalink / raw)
  To: Liu Ying, devicetree, imx, linux-arm-kernel, linux-kernel,
	dri-devel
  Cc: robh, krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
	andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, jonas,
	jernej.skrabec, maarten.lankhorst, mripard, tzimmermann, airlied,
	daniel, peng.fan

On 16/08/2024 11:44, Liu Ying wrote:
> On 08/16/2024, Krzysztof Kozlowski wrote:
>> On 16/08/2024 10:09, Liu Ying wrote:
>>> NXP i.MX93 mediamix blk-ctrl contains one DISPLAY_MUX register which
>>> configures parallel display format by using the "PARALLEL_DISP_FORMAT"
>>> field. Add a DRM bridge driver to support the display format configuration.
>>>
>>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>>> ---
>>
>> ...
>>
>>> +
>>> +static int imx93_pdfc_bridge_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct imx93_pdfc *pdfc;
>>> +	int ret;
>>> +
>>> +	pdfc = devm_kzalloc(dev, sizeof(*pdfc), GFP_KERNEL);
>>> +	if (!pdfc)
>>> +		return -ENOMEM;
>>> +
>>> +	pdfc->regmap = syscon_node_to_regmap(dev->of_node->parent);
>>> +	if (IS_ERR(pdfc->regmap)) {
>>> +		ret = PTR_ERR(pdfc->regmap);
>>> +		if (ret != -EPROBE_DEFER)
>>> +			DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
>>> +		return ret;
>>
>> Nope, you just open-coded dev_err_probe. Syntax is - return
>> dev_err_probe(). if you need wrapper for DRM, add such.
> 
> Will use dev_err_probe().
> 
>>
>>> +	}
>>> +
>>> +	pdfc->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
>>> +	if (IS_ERR(pdfc->next_bridge)) {
>>> +		ret = PTR_ERR(pdfc->next_bridge);
>>> +		if (ret != -EPROBE_DEFER)
>>> +			DRM_DEV_ERROR(dev, "failed to get next bridge: %d\n", ret);
>>> +		return ret;
>>
>> Ditto
> 
> Will use dev_err_probe().
> 
>>
>>
>>> +	}
>>> +
>>
>> ...
>>
>>> +MODULE_DESCRIPTION("NXP i.MX93 parallel display format configuration driver");
>>> +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_ALIAS("platform:" DRIVER_NAME);
>>
>> Which other driver needs this platform alias?
> 
> Quote include/linux/module.h:
> 
> "
> /* For userspace: you can also call me... */                                     
> #define MODULE_ALIAS(_alias) MODULE_INFO(alias, _alias)   
> "
> 
> Anything wrong with using MODULE_ALIAS() here?

Yes, it redundant. Do not answer with question to the question.

Are you adding random code that you cannot justify? It looks like this
if you cannot answer why do you need it.

Form letter:

You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.

Best regards,
Krzysztof



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

end of thread, other threads:[~2024-08-16 12:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-16  8:09 [PATCH v3 RESEND 0/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support Liu Ying
2024-08-16  8:09 ` [PATCH v3 RESEND 1/2] dt-bindings: soc: imx93-media-blk-ctrl: Add PDFC subnode to schema and example Liu Ying
2024-08-16  8:09 ` [PATCH v3 RESEND 2/2] drm/bridge: imx: Add i.MX93 parallel display format configuration support Liu Ying
2024-08-16  9:27   ` Krzysztof Kozlowski
2024-08-16  9:44     ` Liu Ying
2024-08-16 12:24       ` Krzysztof Kozlowski

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