From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Yuti Amonkar <yamonkar@cadence.com> Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, robh+dt@kernel.org, maxime@cerno.tech, airlied@linux.ie, daniel@ffwll.ch, mark.rutland@arm.com, a.hajda@samsung.com, narmstrong@baylibre.com, jonas@kwiboo.se, jernej.skrabec@siol.net, praneeth@ti.com, jsarha@ti.com, tomi.valkeinen@ti.com, mparab@cadence.com, sjakhade@cadence.com Subject: Re: [PATCH v6 3/3] drm: bridge: cdns-mhdp: add j721e wrapper Date: Wed, 11 Mar 2020 22:52:17 +0200 [thread overview] Message-ID: <20200311205217.GD4863@pendragon.ideasonboard.com> (raw) In-Reply-To: <1582712579-28504-4-git-send-email-yamonkar@cadence.com> Hi Yuti, Thank you for the patch. On Wed, Feb 26, 2020 at 11:22:59AM +0100, Yuti Amonkar wrote: > Add j721e wrapper for mhdp, which sets up the clock and data muxes. > > Signed-off-by: Yuti Amonkar <yamonkar@cadence.com> > Signed-off-by: Jyri Sarha <jsarha@ti.com> > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/bridge/Kconfig | 12 ++++ > drivers/gpu/drm/bridge/Makefile | 4 ++ > drivers/gpu/drm/bridge/cdns-mhdp-core.c | 14 +++++ > drivers/gpu/drm/bridge/cdns-mhdp-core.h | 1 + > drivers/gpu/drm/bridge/cdns-mhdp-j721e.c | 79 ++++++++++++++++++++++++ > drivers/gpu/drm/bridge/cdns-mhdp-j721e.h | 55 +++++++++++++++++ > 6 files changed, 165 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.h > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 3bfabb76f2bb..ba945071bb0b 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -38,6 +38,18 @@ config DRM_CDNS_MHDP > It takes a DPI stream as input and output it encoded > in DP format. > > +if DRM_CDNS_MHDP > + > +config DRM_CDNS_MHDP_J721E > + bool "J721E Cadence DPI/DP wrapper support" > + default y Should this be automatically selected when support for the J721E platform is enabled, instead of being user-selectable ? > + help > + Support J721E Cadence DPI/DP wrapper. This is a wrapper > + which adds support for J721E related platform ops. It > + initializes the J721e Display Port and sets up the > + clock and data muxes. > +endif > + > config DRM_DUMB_VGA_DAC > tristate "Dumb VGA DAC Bridge support" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 2e2c5be7c714..fa575ad57b95 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -19,5 +19,9 @@ obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o > obj-$(CONFIG_DRM_CDNS_MHDP) += cdns-mhdp.o > cdns-mhdp-objs := cdns-mhdp-core.o > > +ifeq ($(CONFIG_DRM_CDNS_MHDP_J721E),y) > + cdns-mhdp-objs += cdns-mhdp-j721e.o > +endif > + > obj-y += analogix/ > obj-y += synopsys/ > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.c b/drivers/gpu/drm/bridge/cdns-mhdp-core.c > index cc642893baa8..8d07ffe2d791 100644 > --- a/drivers/gpu/drm/bridge/cdns-mhdp-core.c > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.c > @@ -36,8 +36,22 @@ > > #include "cdns-mhdp-core.h" > You can drop the blank line here. > +#include "cdns-mhdp-j721e.h" > + > +#ifdef CONFIG_DRM_CDNS_MHDP_J721E > +static const struct mhdp_platform_ops mhdp_ti_j721e_ops = { > + .init = cdns_mhdp_j721e_init, > + .exit = cdns_mhdp_j721e_fini, > + .enable = cdns_mhdp_j721e_enable, > + .disable = cdns_mhdp_j721e_disable, > +}; > +#endif > + How about moving this structure to cdns-mhdp-j721e.c instead of exposing the four functions ? > static const struct of_device_id mhdp_ids[] = { > { .compatible = "cdns,mhdp8546", }, > +#ifdef CONFIG_DRM_CDNS_MHDP_J721E > + { .compatible = "ti,j721e-mhdp8546", .data = &mhdp_ti_j721e_ops }, > +#endif > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, mhdp_ids); > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.h b/drivers/gpu/drm/bridge/cdns-mhdp-core.h > index f8df54917816..0878a6e3fd31 100644 > --- a/drivers/gpu/drm/bridge/cdns-mhdp-core.h > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.h > @@ -335,6 +335,7 @@ struct mhdp_platform_ops { > > struct cdns_mhdp_device { > void __iomem *regs; > + void __iomem *j721e_regs; > > struct device *dev; > struct clk *clk; > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > new file mode 100644 > index 000000000000..a87faf55c065 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > @@ -0,0 +1,79 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * TI j721e Cadence MHDP DP wrapper > + * > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Jyri Sarha <jsarha@ti.com > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. You can drop this paragraph, it's implied by the SPDX header. > + */ > + > +#include <linux/device.h> This should be linux/platform_device.h > +#include <linux/io.h> > + > +#include "cdns-mhdp-j721e.h" > + > +#define REVISION 0x00 > +#define DPTX_IPCFG 0x04 > +#define ECC_MEM_CFG 0x08 > +#define DPTX_DSC_CFG 0x0c > +#define DPTX_SRC_CFG 0x10 > +#define DPTX_VIF_SECURE_MODE_CFG 0x14 > +#define DPTX_VIF_CONN_STATUS 0x18 > +#define PHY_CLK_STATUS 0x1c > + > +#define DPTX_SRC_AIF_EN BIT(16) > +#define DPTX_SRC_VIF_3_IN30B BIT(11) > +#define DPTX_SRC_VIF_2_IN30B BIT(10) > +#define DPTX_SRC_VIF_1_IN30B BIT(9) > +#define DPTX_SRC_VIF_0_IN30B BIT(8) > +#define DPTX_SRC_VIF_3_SEL_DPI5 BIT(7) > +#define DPTX_SRC_VIF_3_SEL_DPI3 0 > +#define DPTX_SRC_VIF_2_SEL_DPI4 BIT(6) > +#define DPTX_SRC_VIF_2_SEL_DPI2 0 > +#define DPTX_SRC_VIF_1_SEL_DPI3 BIT(5) > +#define DPTX_SRC_VIF_1_SEL_DPI1 0 > +#define DPTX_SRC_VIF_0_SEL_DPI2 BIT(4) > +#define DPTX_SRC_VIF_0_SEL_DPI0 0 > +#define DPTX_SRC_VIF_3_EN BIT(3) > +#define DPTX_SRC_VIF_2_EN BIT(2) > +#define DPTX_SRC_VIF_1_EN BIT(1) > +#define DPTX_SRC_VIF_0_EN BIT(0) > + > +/* TODO turn DPTX_IPCFG fw_mem_clk_en at pm_runtime_suspend. */ > + > +int cdns_mhdp_j721e_init(struct cdns_mhdp_device *mhdp) > +{ > + struct platform_device *pdev = to_platform_device(mhdp->dev); > + struct resource *regs; > + > + regs = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + mhdp->j721e_regs = devm_ioremap_resource(&pdev->dev, regs); You can use mhdp->j721e_regs = devm_platform_ioremap_resource(&pdev->dev, 1); > + if (IS_ERR(mhdp->j721e_regs)) > + return PTR_ERR(mhdp->j721e_regs); > + > + return 0; > +} > + > +void cdns_mhdp_j721e_fini(struct cdns_mhdp_device *mhdp) > +{ > +} > + To avoid the need for empty functions, how about a NULL check in the caller ? > +void cdns_mhdp_j721e_enable(struct cdns_mhdp_device *mhdp) > +{ > + /* > + * Eneble VIF_0 and select DPI2 as its input. DSS0 DPI0 is connected > + * to eDP DPI2. This is the only supported SST configuration on > + * J721E. Without hardware documentation I can't really comment on this, but I'd like to make sure it doesn't imply that the MHDP has more than one input and one output. > + */ > + writel(DPTX_SRC_VIF_0_EN | DPTX_SRC_VIF_0_SEL_DPI2, > + mhdp->j721e_regs + DPTX_SRC_CFG); > +} > + > +void cdns_mhdp_j721e_disable(struct cdns_mhdp_device *mhdp) > +{ > + /* Put everything to defaults */ > + writel(0, mhdp->j721e_regs + DPTX_DSC_CFG); > +} > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-j721e.h b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.h > new file mode 100644 > index 000000000000..c7f9e8bc9391 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.h > @@ -0,0 +1,55 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * TI j721e Cadence MHDP DP wrapper > + * > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Jyri Sarha <jsarha@ti.com > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. You can drop this paragraph too. > + */ > + > +#ifndef CDNS_MHDP_J721E_H > +#define CDNS_MHDP_J721E_H > + > +#include <linux/platform_device.h> > +#include "cdns-mhdp-core.h" > + > +struct cdns_mhdp_j721e_wrap; This is unused. > + > +#ifdef CONFIG_DRM_CDNS_MHDP_J721E > + > +int cdns_mhdp_j721e_init(struct cdns_mhdp_device *mhdp); > + > +void cdns_mhdp_j721e_fini(struct cdns_mhdp_device *mhdp); > + > +void cdns_mhdp_j721e_enable(struct cdns_mhdp_device *mhdp); > + > +void cdns_mhdp_j721e_disable(struct cdns_mhdp_device *mhdp); > + > +#else > + > +static inline > +int cdns_mhdp_j721e_init(struct cdns_mhdp_device *mhdp) > +{ > + return 0; > +} > + > +static inline > +void cdns_mhdp_j721e_fini(struct cdns_mhdp_device *mhdp) > +{ > +} > + > +static inline > +void cdns_mhdp_j721e_sst_enable(struct cdns_mhdp_device *mhdp) > +{ > +} > + > +static inline > +void cdns_mhdp_j721e_sst_disable(struct cdns_mhdp_device *mhdp) > +{ > +} > +#endif /* CONFIG_DRM_CDNS_MHDP_J721E */ No need for the CONFIG_DRM_CDNS_MHDP_J721E check, there's already one in cdns-mhdp-core.c. If you follow my above suggestion, the above should just become struct mhdp_platform_ops; extern const struct mhdp_platform_ops mhdp_ti_j721e_ops; Lots of small comments but nothing blocking. After addressing them, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > +#endif /* !CDNS_MHDP_J721E_H */ -- Regards, Laurent Pinchart
WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Yuti Amonkar <yamonkar@cadence.com> Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, jernej.skrabec@siol.net, praneeth@ti.com, narmstrong@baylibre.com, airlied@linux.ie, tomi.valkeinen@ti.com, jonas@kwiboo.se, jsarha@ti.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, a.hajda@samsung.com, robh+dt@kernel.org, maxime@cerno.tech, sjakhade@cadence.com, mparab@cadence.com Subject: Re: [PATCH v6 3/3] drm: bridge: cdns-mhdp: add j721e wrapper Date: Wed, 11 Mar 2020 22:52:17 +0200 [thread overview] Message-ID: <20200311205217.GD4863@pendragon.ideasonboard.com> (raw) In-Reply-To: <1582712579-28504-4-git-send-email-yamonkar@cadence.com> Hi Yuti, Thank you for the patch. On Wed, Feb 26, 2020 at 11:22:59AM +0100, Yuti Amonkar wrote: > Add j721e wrapper for mhdp, which sets up the clock and data muxes. > > Signed-off-by: Yuti Amonkar <yamonkar@cadence.com> > Signed-off-by: Jyri Sarha <jsarha@ti.com> > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/bridge/Kconfig | 12 ++++ > drivers/gpu/drm/bridge/Makefile | 4 ++ > drivers/gpu/drm/bridge/cdns-mhdp-core.c | 14 +++++ > drivers/gpu/drm/bridge/cdns-mhdp-core.h | 1 + > drivers/gpu/drm/bridge/cdns-mhdp-j721e.c | 79 ++++++++++++++++++++++++ > drivers/gpu/drm/bridge/cdns-mhdp-j721e.h | 55 +++++++++++++++++ > 6 files changed, 165 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.h > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 3bfabb76f2bb..ba945071bb0b 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -38,6 +38,18 @@ config DRM_CDNS_MHDP > It takes a DPI stream as input and output it encoded > in DP format. > > +if DRM_CDNS_MHDP > + > +config DRM_CDNS_MHDP_J721E > + bool "J721E Cadence DPI/DP wrapper support" > + default y Should this be automatically selected when support for the J721E platform is enabled, instead of being user-selectable ? > + help > + Support J721E Cadence DPI/DP wrapper. This is a wrapper > + which adds support for J721E related platform ops. It > + initializes the J721e Display Port and sets up the > + clock and data muxes. > +endif > + > config DRM_DUMB_VGA_DAC > tristate "Dumb VGA DAC Bridge support" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 2e2c5be7c714..fa575ad57b95 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -19,5 +19,9 @@ obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o > obj-$(CONFIG_DRM_CDNS_MHDP) += cdns-mhdp.o > cdns-mhdp-objs := cdns-mhdp-core.o > > +ifeq ($(CONFIG_DRM_CDNS_MHDP_J721E),y) > + cdns-mhdp-objs += cdns-mhdp-j721e.o > +endif > + > obj-y += analogix/ > obj-y += synopsys/ > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.c b/drivers/gpu/drm/bridge/cdns-mhdp-core.c > index cc642893baa8..8d07ffe2d791 100644 > --- a/drivers/gpu/drm/bridge/cdns-mhdp-core.c > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.c > @@ -36,8 +36,22 @@ > > #include "cdns-mhdp-core.h" > You can drop the blank line here. > +#include "cdns-mhdp-j721e.h" > + > +#ifdef CONFIG_DRM_CDNS_MHDP_J721E > +static const struct mhdp_platform_ops mhdp_ti_j721e_ops = { > + .init = cdns_mhdp_j721e_init, > + .exit = cdns_mhdp_j721e_fini, > + .enable = cdns_mhdp_j721e_enable, > + .disable = cdns_mhdp_j721e_disable, > +}; > +#endif > + How about moving this structure to cdns-mhdp-j721e.c instead of exposing the four functions ? > static const struct of_device_id mhdp_ids[] = { > { .compatible = "cdns,mhdp8546", }, > +#ifdef CONFIG_DRM_CDNS_MHDP_J721E > + { .compatible = "ti,j721e-mhdp8546", .data = &mhdp_ti_j721e_ops }, > +#endif > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, mhdp_ids); > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.h b/drivers/gpu/drm/bridge/cdns-mhdp-core.h > index f8df54917816..0878a6e3fd31 100644 > --- a/drivers/gpu/drm/bridge/cdns-mhdp-core.h > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.h > @@ -335,6 +335,7 @@ struct mhdp_platform_ops { > > struct cdns_mhdp_device { > void __iomem *regs; > + void __iomem *j721e_regs; > > struct device *dev; > struct clk *clk; > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > new file mode 100644 > index 000000000000..a87faf55c065 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > @@ -0,0 +1,79 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * TI j721e Cadence MHDP DP wrapper > + * > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Jyri Sarha <jsarha@ti.com > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. You can drop this paragraph, it's implied by the SPDX header. > + */ > + > +#include <linux/device.h> This should be linux/platform_device.h > +#include <linux/io.h> > + > +#include "cdns-mhdp-j721e.h" > + > +#define REVISION 0x00 > +#define DPTX_IPCFG 0x04 > +#define ECC_MEM_CFG 0x08 > +#define DPTX_DSC_CFG 0x0c > +#define DPTX_SRC_CFG 0x10 > +#define DPTX_VIF_SECURE_MODE_CFG 0x14 > +#define DPTX_VIF_CONN_STATUS 0x18 > +#define PHY_CLK_STATUS 0x1c > + > +#define DPTX_SRC_AIF_EN BIT(16) > +#define DPTX_SRC_VIF_3_IN30B BIT(11) > +#define DPTX_SRC_VIF_2_IN30B BIT(10) > +#define DPTX_SRC_VIF_1_IN30B BIT(9) > +#define DPTX_SRC_VIF_0_IN30B BIT(8) > +#define DPTX_SRC_VIF_3_SEL_DPI5 BIT(7) > +#define DPTX_SRC_VIF_3_SEL_DPI3 0 > +#define DPTX_SRC_VIF_2_SEL_DPI4 BIT(6) > +#define DPTX_SRC_VIF_2_SEL_DPI2 0 > +#define DPTX_SRC_VIF_1_SEL_DPI3 BIT(5) > +#define DPTX_SRC_VIF_1_SEL_DPI1 0 > +#define DPTX_SRC_VIF_0_SEL_DPI2 BIT(4) > +#define DPTX_SRC_VIF_0_SEL_DPI0 0 > +#define DPTX_SRC_VIF_3_EN BIT(3) > +#define DPTX_SRC_VIF_2_EN BIT(2) > +#define DPTX_SRC_VIF_1_EN BIT(1) > +#define DPTX_SRC_VIF_0_EN BIT(0) > + > +/* TODO turn DPTX_IPCFG fw_mem_clk_en at pm_runtime_suspend. */ > + > +int cdns_mhdp_j721e_init(struct cdns_mhdp_device *mhdp) > +{ > + struct platform_device *pdev = to_platform_device(mhdp->dev); > + struct resource *regs; > + > + regs = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + mhdp->j721e_regs = devm_ioremap_resource(&pdev->dev, regs); You can use mhdp->j721e_regs = devm_platform_ioremap_resource(&pdev->dev, 1); > + if (IS_ERR(mhdp->j721e_regs)) > + return PTR_ERR(mhdp->j721e_regs); > + > + return 0; > +} > + > +void cdns_mhdp_j721e_fini(struct cdns_mhdp_device *mhdp) > +{ > +} > + To avoid the need for empty functions, how about a NULL check in the caller ? > +void cdns_mhdp_j721e_enable(struct cdns_mhdp_device *mhdp) > +{ > + /* > + * Eneble VIF_0 and select DPI2 as its input. DSS0 DPI0 is connected > + * to eDP DPI2. This is the only supported SST configuration on > + * J721E. Without hardware documentation I can't really comment on this, but I'd like to make sure it doesn't imply that the MHDP has more than one input and one output. > + */ > + writel(DPTX_SRC_VIF_0_EN | DPTX_SRC_VIF_0_SEL_DPI2, > + mhdp->j721e_regs + DPTX_SRC_CFG); > +} > + > +void cdns_mhdp_j721e_disable(struct cdns_mhdp_device *mhdp) > +{ > + /* Put everything to defaults */ > + writel(0, mhdp->j721e_regs + DPTX_DSC_CFG); > +} > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-j721e.h b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.h > new file mode 100644 > index 000000000000..c7f9e8bc9391 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.h > @@ -0,0 +1,55 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * TI j721e Cadence MHDP DP wrapper > + * > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Jyri Sarha <jsarha@ti.com > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. You can drop this paragraph too. > + */ > + > +#ifndef CDNS_MHDP_J721E_H > +#define CDNS_MHDP_J721E_H > + > +#include <linux/platform_device.h> > +#include "cdns-mhdp-core.h" > + > +struct cdns_mhdp_j721e_wrap; This is unused. > + > +#ifdef CONFIG_DRM_CDNS_MHDP_J721E > + > +int cdns_mhdp_j721e_init(struct cdns_mhdp_device *mhdp); > + > +void cdns_mhdp_j721e_fini(struct cdns_mhdp_device *mhdp); > + > +void cdns_mhdp_j721e_enable(struct cdns_mhdp_device *mhdp); > + > +void cdns_mhdp_j721e_disable(struct cdns_mhdp_device *mhdp); > + > +#else > + > +static inline > +int cdns_mhdp_j721e_init(struct cdns_mhdp_device *mhdp) > +{ > + return 0; > +} > + > +static inline > +void cdns_mhdp_j721e_fini(struct cdns_mhdp_device *mhdp) > +{ > +} > + > +static inline > +void cdns_mhdp_j721e_sst_enable(struct cdns_mhdp_device *mhdp) > +{ > +} > + > +static inline > +void cdns_mhdp_j721e_sst_disable(struct cdns_mhdp_device *mhdp) > +{ > +} > +#endif /* CONFIG_DRM_CDNS_MHDP_J721E */ No need for the CONFIG_DRM_CDNS_MHDP_J721E check, there's already one in cdns-mhdp-core.c. If you follow my above suggestion, the above should just become struct mhdp_platform_ops; extern const struct mhdp_platform_ops mhdp_ti_j721e_ops; Lots of small comments but nothing blocking. After addressing them, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > +#endif /* !CDNS_MHDP_J721E_H */ -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-03-11 20:52 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-26 10:22 [PATCH v6 0/3] drm: Add support for Cadence MHDP DPI/DP bridge and J721E wrapper Yuti Amonkar 2020-02-26 10:22 ` Yuti Amonkar 2020-02-26 10:22 ` [PATCH v6 1/3] dt-bindings: drm/bridge: Document Cadence MHDP bridge bindings Yuti Amonkar 2020-02-26 10:22 ` Yuti Amonkar 2020-02-26 10:22 ` [PATCH v6 2/3] drm: bridge: Add support for Cadence MHDP DPI/DP bridge Yuti Amonkar 2020-02-26 10:22 ` Yuti Amonkar 2020-03-11 22:20 ` Laurent Pinchart 2020-03-11 22:20 ` Laurent Pinchart 2020-03-12 7:13 ` Tomi Valkeinen 2020-03-12 7:13 ` Tomi Valkeinen 2020-08-11 2:43 ` Laurent Pinchart 2020-08-11 2:43 ` Laurent Pinchart 2020-05-01 7:32 ` Yuti Suresh Amonkar 2020-05-01 7:32 ` Yuti Suresh Amonkar 2020-08-11 0:00 ` Laurent Pinchart 2020-08-11 0:00 ` Laurent Pinchart 2020-08-26 15:38 ` Yuti Suresh Amonkar 2020-08-26 15:38 ` Yuti Suresh Amonkar 2020-02-26 10:22 ` [PATCH v6 3/3] drm: bridge: cdns-mhdp: add j721e wrapper Yuti Amonkar 2020-02-26 10:22 ` Yuti Amonkar 2020-03-11 20:52 ` Laurent Pinchart [this message] 2020-03-11 20:52 ` Laurent Pinchart 2020-03-12 7:01 ` Tomi Valkeinen 2020-03-12 7:01 ` Tomi Valkeinen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200311205217.GD4863@pendragon.ideasonboard.com \ --to=laurent.pinchart@ideasonboard.com \ --cc=a.hajda@samsung.com \ --cc=airlied@linux.ie \ --cc=daniel@ffwll.ch \ --cc=devicetree@vger.kernel.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=jernej.skrabec@siol.net \ --cc=jonas@kwiboo.se \ --cc=jsarha@ti.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=maxime@cerno.tech \ --cc=mparab@cadence.com \ --cc=narmstrong@baylibre.com \ --cc=praneeth@ti.com \ --cc=robh+dt@kernel.org \ --cc=sjakhade@cadence.com \ --cc=tomi.valkeinen@ti.com \ --cc=yamonkar@cadence.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.