| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8499738.T7Z3S40VBb@steina-w>
Date: Tue, 04 Mar 2025 10:52:21 +0100
From: Alexander Stein <alexander.stein@...tq-group.com>
To: devicetree@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org, Liu Ying <victor.liu@....com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
festevam@...il.com, victor.liu@....com, andrzej.hajda@...el.com,
neil.armstrong@...aro.org, rfoss@...nel.org,
Laurent.pinchart@...asonboard.com, jonas@...boo.se, jernej.skrabec@...il.com,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
airlied@...il.com, simona@...ll.ch, peng.fan@....com
Subject:
Re: [PATCH v5 2/2] drm/bridge: imx: Add i.MX93 parallel display format
configuration support
Hi,
thanks for the update.
Am Dienstag, 4. März 2025, 09:24:34 CET schrieb Liu Ying:
> 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@....com>
> ---
> v4->v5:
> * Rebase upon next-20250303. This causes the drop of .remove_new by using
> devm_drm_bridge_add(). Also, this causes API change for
> imx93_pdfc_bridge_atomic_enable().
> * Update year of copyright.
>
> v3->v4:
> * Use dev_err_probe() in imx93_pdfc_bridge_probe(). (Krzysztof)
> * Drop MODULE_ALIAS(). (Krzysztof)
> * Update year of Copyright.
>
> 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 | 186 ++++++++++++++++++++++++
> 3 files changed, 195 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 9a480c6abb85..51138d74ddfb 100644
> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> @@ -88,4 +88,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 dd5d48584806..f4ccc5cbef72 100644
> --- a/drivers/gpu/drm/bridge/imx/Makefile
> +++ b/drivers/gpu/drm/bridge/imx/Makefile
> @@ -8,3 +8,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..7dfb87e64197
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/imx/imx93-pdfc.c
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2022-2025 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 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,
> +};
How about?
#define PARALLEL_DISP_FORMAT GENMASK(10, 8)
#define FORMAT_RGB888_TO_RGB888 FIELD_PREP(PARALLEL_DISP_FORMAT, 0)
#define FORMAT_RGB888_TO_RGB666 FIELD_PREP(PARALLEL_DISP_FORMAT, 1)
#define FORMAT_RGB565_TO_RGB565 FIELD_PREP(PARALLEL_DISP_FORMAT, 2)
#define FORMAT_RGB555_TO_RGB555 FIELD_PREP(PARALLEL_DISP_FORMAT, 3)
#define FORMAT_YUV_TO_YCBCR24 FIELD_PREP(PARALLEL_DISP_FORMAT, 4)
#define FORMAT_YUV_TO_YUV444 FIELD_PREP(PARALLEL_DISP_FORMAT, 5)
> +
> +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_atomic_state *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;
Do really need to store the bus format in device struct?
It's possible to access the bridge state in atomic_enable using
drm_atomic_get_bridge_state, no? TBH I don't know what are the best
practices though.
> + 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;
> +
> + 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))
> + return dev_err_probe(dev, PTR_ERR(pdfc->regmap),
> + "failed to get regmap\n");
> +
> + pdfc->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> + if (IS_ERR(pdfc->next_bridge))
> + return dev_err_probe(dev, PTR_ERR(pdfc->next_bridge),
> + "failed to get next bridge\n");
> +
> + pdfc->dev = dev;
> + pdfc->bridge.driver_private = pdfc;
> + pdfc->bridge.funcs = &imx93_pdfc_bridge_funcs;
> + pdfc->bridge.of_node = dev->of_node;
pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI;
Despite that looks god to me.
Best regards
Alexander
> +
> + return devm_drm_bridge_add(dev, &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,
> + .driver = {
> + .of_match_table = imx93_pdfc_dt_ids,
> + .name = "imx93_pdfc",
> + },
> +};
> +module_platform_driver(imx93_pdfc_bridge_driver);
> +
> +MODULE_DESCRIPTION("NXP i.MX93 parallel display format configuration driver");
> +MODULE_AUTHOR("Liu Ying <victor.liu@....com>");
> +MODULE_LICENSE("GPL v2");
>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Powered by blists - more mailing lists