| 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: <4911f05b-ecef-4883-b88c-f01dbe136acf@ideasonboard.com>
Date: Tue, 11 Feb 2025 14:25:39 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Aradhya Bhatia <aradhya.bhatia@...ux.dev>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>, Maxime Ripard <mripard@...nel.org>,
David Airlie <airlied@...il.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Simona Vetter <simona@...ll.ch>, Nishanth Menon <nm@...com>,
Vignesh Raghavendra <vigneshr@...com>, Devarsh Thakkar <devarsht@...com>,
Praneeth Bajjuri <praneeth@...com>, Udit Kumar <u-kumar1@...com>,
Jayesh Choudhary <j-choudhary@...com>,
Francesco Dolcini <francesco@...cini.it>,
DRI Development List <dri-devel@...ts.freedesktop.org>,
Devicetree List <devicetree@...r.kernel.org>,
Linux Kernel List <linux-kernel@...r.kernel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Jyri Sarha <jyri.sarha@....fi>
Subject: Re: [PATCH v5 3/3] drm/tidss: Add OLDI bridge support
Hi,
On 11/02/2025 12:57, Tomi Valkeinen wrote:
> Hi,
>
> On 09/02/2025 18:09, Aradhya Bhatia wrote:
>> From: Aradhya Bhatia <a-bhatia1@...com>
>>
>> The AM62x and AM62Px SoCs feature 2 OLDI TXes each, which makes it
>> possible to connect them in dual-link or cloned single-link OLDI display
>> modes. The current OLDI support in tidss_dispc.c can only support for
>> a single OLDI TX, connected to a VP and doesn't really support
>> configuration of OLDIs in the other modes. The current OLDI support in
>> tidss_dispc.c also works on the principle that the OLDI output can only
>> be served by one, and only one, DSS video-port. This isn't the case in
>> the AM62Px SoC, where there are 2 DSS controllers present that share the
>> OLDI TXes.
>>
>> Having their own devicetree and their own bridge entity will help
>> support the various display modes and sharing possiblilities of the OLDI
>> hardware.
>>
>> For all these reasons, add support for the OLDI TXes as DRM bridges.
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@...com>
>> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@...ux.dev>
>> ---
>> drivers/gpu/drm/tidss/Makefile | 3 +-
>> drivers/gpu/drm/tidss/tidss_dispc.c | 20 +-
>> drivers/gpu/drm/tidss/tidss_dispc.h | 4 +
>> drivers/gpu/drm/tidss/tidss_dispc_regs.h | 14 +
>> drivers/gpu/drm/tidss/tidss_drv.c | 9 +
>> drivers/gpu/drm/tidss/tidss_drv.h | 5 +
>> drivers/gpu/drm/tidss/tidss_oldi.c | 558 +++++++++++++++++++++++
>> drivers/gpu/drm/tidss/tidss_oldi.h | 51 +++
>> 8 files changed, 662 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.c
>> create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.h
>>
>> diff --git a/drivers/gpu/drm/tidss/Makefile b/drivers/gpu/drm/tidss/
>> Makefile
>> index 312645271014..b6d6becf1683 100644
>> --- a/drivers/gpu/drm/tidss/Makefile
>> +++ b/drivers/gpu/drm/tidss/Makefile
>> @@ -7,6 +7,7 @@ tidss-y := tidss_crtc.o \
>> tidss_irq.o \
>> tidss_plane.o \
>> tidss_scale_coefs.o \
>> - tidss_dispc.o
>> + tidss_dispc.o \
>> + tidss_oldi.o
>> obj-$(CONFIG_DRM_TIDSS) += tidss.o
>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/
>> tidss/tidss_dispc.c
>> index 1ad711f8d2a8..8631a89e6155 100644
>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>> @@ -466,6 +466,25 @@ static u32 dispc_vp_read(struct dispc_device
>> *dispc, u32 hw_videoport, u16 reg)
>> return ioread32(base + reg);
>> }
>> +void tidss_configure_oldi(struct tidss_device *tidss, u32 hw_videoport,
>> + u32 oldi_cfg)
>> +{
>> + u32 count = 0;
>> + u32 oldi_reset_bit = BIT(5 + hw_videoport);
>> +
>> + dispc_vp_write(tidss->dispc, hw_videoport, DISPC_VP_DSS_OLDI_CFG,
>> oldi_cfg);
>> +
>> + if (oldi_cfg != 0) {
>> + while (!(oldi_reset_bit & dispc_read(tidss->dispc,
>> DSS_SYSSTATUS)) &&
>> + count < 10000)
>> + count++;
>> +
>> + if (!(oldi_reset_bit & dispc_read(tidss->dispc, DSS_SYSSTATUS)))
>> + dev_warn(tidss->dispc->dev, "%s: timeout waiting OLDI
>> reset done\n",
>> + __func__);
>> + }
>> +}
>
> The timeout sounds like an error. Better to return an error value, and
> handle it in tidss_oldi_config()?
>
> You could also
>
> if (!oldi_cfg)
> return 0;
>
> But would it actually be nicer to have a separate void function for
> disabling?
>
>> +
>> /*
>> * TRM gives bitfields as start:end, where start is the higher bit
>> * number. For example 7:0
>> @@ -1310,7 +1329,6 @@ void dispc_vp_disable_clk(struct dispc_device
>> *dispc, u32 hw_videoport)
>> * Calculate the percentage difference between the requested pixel
>> clock rate
>> * and the effective rate resulting from calculating the clock
>> divider value.
>> */
>> -static
>> unsigned int dispc_pclk_diff(unsigned long rate, unsigned long
>> real_rate)
>> {
>> int r = rate / 100, rr = real_rate / 100;
>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/
>> tidss/tidss_dispc.h
>> index 086327d51a90..fab248f2055a 100644
>> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
>> @@ -94,6 +94,10 @@ extern const struct dispc_features dispc_am62a7_feats;
>> extern const struct dispc_features dispc_am65x_feats;
>> extern const struct dispc_features dispc_j721e_feats;
>> +void tidss_configure_oldi(struct tidss_device *tidss, u32 hw_videoport,
>> + u32 oldi_cfg);
>> +unsigned int dispc_pclk_diff(unsigned long rate, unsigned long
>> real_rate);
>> +
>> void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask);
>> dispc_irq_t dispc_read_and_clear_irqstatus(struct dispc_device *dispc);
>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc_regs.h b/drivers/gpu/
>> drm/tidss/tidss_dispc_regs.h
>> index 13feedfe5d6d..03f7098029e6 100644
>> --- a/drivers/gpu/drm/tidss/tidss_dispc_regs.h
>> +++ b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
>> @@ -226,6 +226,20 @@ enum dispc_common_regs {
>> #define DISPC_VP_DSS_DMA_THREADSIZE 0x170 /* J721E */
>> #define DISPC_VP_DSS_DMA_THREADSIZE_STATUS 0x174 /* J721E */
>> +/* OLDI Config Bits (DISPC_VP_DSS_OLDI_CFG) */
>> +#define OLDI_ENABLE BIT(0)
>> +#define OLDI_MAP (BIT(1) | BIT(2) | BIT(3))
>> +#define OLDI_SRC BIT(4)
>> +#define OLDI_CLONE_MODE BIT(5)
>> +#define OLDI_MASTERSLAVE BIT(6)
>> +#define OLDI_DEPOL BIT(7)
>> +#define OLDI_MSB BIT(8)
>> +#define OLDI_LBEN BIT(9)
>> +#define OLDI_LBDATA BIT(10)
>> +#define OLDI_DUALMODESYNC BIT(11)
>> +#define OLDI_SOFTRST BIT(12)
>> +#define OLDI_TPATCFG BIT(13)
>> +
>> /*
>> * OLDI IO_CTRL register offsets. On AM654 the registers are found
>> * from CTRL_MMR0, there the syscon regmap should map 0x14 bytes from
>> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/
>> tidss/tidss_drv.c
>> index 7c8fd6407d82..27b9f86f1eb2 100644
>> --- a/drivers/gpu/drm/tidss/tidss_drv.c
>> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
>> @@ -24,6 +24,7 @@
>> #include "tidss_drv.h"
>> #include "tidss_kms.h"
>> #include "tidss_irq.h"
>> +#include "tidss_oldi.h"
>> /* Power management */
>> @@ -148,6 +149,10 @@ static int tidss_probe(struct platform_device *pdev)
>> return ret;
>> }
>> + ret = tidss_oldi_init(tidss);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to init OLDI\n");
>> +
>> pm_runtime_enable(dev);
>> pm_runtime_set_autosuspend_delay(dev, 1000);
>> @@ -204,6 +209,8 @@ static int tidss_probe(struct platform_device *pdev)
>> pm_runtime_dont_use_autosuspend(dev);
>> pm_runtime_disable(dev);
>> + tidss_oldi_deinit(tidss);
>> +
>> return ret;
>> }
>> @@ -228,6 +235,8 @@ static void tidss_remove(struct platform_device
>> *pdev)
>> pm_runtime_dont_use_autosuspend(dev);
>> pm_runtime_disable(dev);
>> + tidss_oldi_deinit(tidss);
>> +
>> /* devm allocated dispc goes away with the dev so mark it NULL */
>> dispc_remove(tidss);
>> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/
>> tidss/tidss_drv.h
>> index d7f27b0b0315..6c0fe1d989ee 100644
>> --- a/drivers/gpu/drm/tidss/tidss_drv.h
>> +++ b/drivers/gpu/drm/tidss/tidss_drv.h
>> @@ -11,8 +11,10 @@
>> #define TIDSS_MAX_PORTS 4
>> #define TIDSS_MAX_PLANES 4
>> +#define TIDSS_MAX_OLDI_TXES 2
>> typedef u32 dispc_irq_t;
>> +struct tidss_oldi;
>> struct tidss_device {
>> struct drm_device ddev; /* DRM device for DSS */
>> @@ -27,6 +29,9 @@ struct tidss_device {
>> unsigned int num_planes;
>> struct drm_plane *planes[TIDSS_MAX_PLANES];
>> + unsigned int num_oldis;
>> + struct tidss_oldi *oldis[TIDSS_MAX_OLDI_TXES];
>> +
>> unsigned int irq;
>> spinlock_t wait_lock; /* protects the irq masks */
>> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c b/drivers/gpu/drm/
>> tidss/tidss_oldi.c
>> new file mode 100644
>> index 000000000000..4af13a01f546
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tidss/tidss_oldi.c
>> @@ -0,0 +1,558 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2024 - Texas Instruments Incorporated
>> + *
>> + * Aradhya Bhatia <a-bhatia1@...com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/of.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/media-bus-format.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_bridge.h>
>> +#include <drm/drm_of.h>
>> +
>> +#include "tidss_dispc.h"
>> +#include "tidss_dispc_regs.h"
>> +#include "tidss_oldi.h"
>> +
>> +struct tidss_oldi {
>> + struct tidss_device *tidss;
>> + struct device *dev;
>> +
>> + struct drm_bridge bridge;
>> + struct drm_bridge *next_bridge;
>> +
>> + enum tidss_oldi_link_type link_type;
>> + const struct oldi_bus_format *bus_format;
>> + u32 oldi_instance;
>> + u32 companion_instance;
>> + u32 parent_vp;
>> +
>> + struct clk *serial;
>> + struct regmap *io_ctrl;
>> +};
>> +
>> +static const struct oldi_bus_format oldi_bus_formats[] = {
>> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, SPWG_18,
>> MEDIA_BUS_FMT_RGB666_1X18 },
>> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, SPWG_24,
>> MEDIA_BUS_FMT_RGB888_1X24 },
>> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, JEIDA_24,
>> MEDIA_BUS_FMT_RGB888_1X24 },
>> +};
>> +
>> +#define OLDI_IDLE_CLK_HZ 25000000 /*25 MHz */
>> +
>> +static inline struct tidss_oldi *
>> +drm_bridge_to_tidss_oldi(struct drm_bridge *bridge)
>> +{
>> + return container_of(bridge, struct tidss_oldi, bridge);
>> +}
>> +
>> +static int tidss_oldi_bridge_attach(struct drm_bridge *bridge,
>> + enum drm_bridge_attach_flags flags)
>> +{
>> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
>> +
>> + if (!oldi->next_bridge) {
>> + dev_err(oldi->dev,
>> + "%s: OLDI%u Failure attach next bridge\n",
>> + __func__, oldi->oldi_instance);
>> + return -ENODEV;
>> + }
>> +
>> + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
>> + dev_err(oldi->dev,
>> + "%s: OLDI%u DRM_BRIDGE_ATTACH_NO_CONNECTOR is mandatory.\n",
>> + __func__, oldi->oldi_instance);
>> + return -EINVAL;
>> + }
>> +
>> + return drm_bridge_attach(bridge->encoder, oldi->next_bridge,
>> + bridge, flags);
>> +}
>> +
>> +static int
>> +tidss_oldi_set_serial_clk(struct tidss_oldi *oldi, unsigned long rate)
>> +{
>> + unsigned long new_rate;
>> + int ret;
>> +
>> + ret = clk_set_rate(oldi->serial, rate);
>> + if (ret) {
>> + dev_err(oldi->dev,
>> + "OLDI%u: failed to set serial clk rate to %lu Hz\n",
>> + oldi->oldi_instance, rate);
>> + return ret;
>> + }
>> +
>> + new_rate = clk_get_rate(oldi->serial);
>> +
>> + if (dispc_pclk_diff(rate, new_rate) > 5)
>> + dev_warn(oldi->dev,
>> + "OLDI%u Clock rate %lu differs over 5%% from requested
>> %lu\n",
>> + oldi->oldi_instance, new_rate, rate);
>> +
>> + dev_dbg(oldi->dev, "OLDI%u: new rate %lu Hz (requested %lu Hz)\n",
>> + oldi->oldi_instance, clk_get_rate(oldi->serial), rate);
>> +
>> + return 0;
>> +}
>> +
>> +static void tidss_oldi_tx_power(struct tidss_oldi *oldi, bool enable)
>> +{
>> + u32 mask;
>> +
>> + /*
>> + * The power control bits are Active Low, and remain powered off by
>> + * default. That is, the bits are set to 1. To power on the OLDI
>> TXes,
>> + * the bits must be cleared to 0. Since there are cases where not
>> all
>> + * OLDI TXes are being used, the power logic selectively powers them
>> + * on.
>> + * Setting the variable 'val' to particular bit masks, makes sure
>> that
>> + * the unrequired OLDI TXes remain powered off.
>> + */
>> +
>> + if (enable) {
>> + switch (oldi->link_type) {
>> + case OLDI_MODE_SINGLE_LINK:
>> + /* Power-on only the required OLDI TX's IO*/
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) | OLDI_PWRDN_BG;
>> + break;
>> + case OLDI_MODE_CLONE_SINGLE_LINK:
>> + case OLDI_MODE_DUAL_LINK:
>> + /* Power-on both the OLDI TXes' IOs */
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) |
>> + OLDI_PWRDOWN_TX(oldi->companion_instance) |
>> + OLDI_PWRDN_BG;
>> + break;
>> + default:
>> + /*
>> + * This code execution should never reach here as any
>> + * OLDI with an unsupported OLDI mode would never get
>> + * registered in the first place.
>> + * However, power-off the OLDI in concern just in case.
>> + */
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance);
>> + enable = false;
>> + break;
>> + }
>> + } else {
>> + switch (oldi->link_type) {
>> + case OLDI_MODE_CLONE_SINGLE_LINK:
>> + case OLDI_MODE_DUAL_LINK:
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) |
>> + OLDI_PWRDOWN_TX(oldi->companion_instance) |
>> + OLDI_PWRDN_BG;
>> + break;
>> + case OLDI_MODE_SINGLE_LINK:
>> + default:
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance);
>> + break;
>> + }
>> + }
>> +
>> + regmap_update_bits(oldi->io_ctrl, OLDI_PD_CTRL, mask, enable ?
>> 0 : mask);
>> +}
>> +
>> +static int tidss_oldi_config(struct tidss_oldi *oldi)
>> +{
>> + const struct oldi_bus_format *bus_fmt = NULL;
>> + u32 oldi_cfg = 0;
>> +
>> + bus_fmt = oldi->bus_format;
>> +
>> + /*
>> + * MASTERSLAVE and SRC bits of OLDI Config are always set to 0.
>> + */
>> +
>> + if (bus_fmt->data_width == 24)
>> + oldi_cfg |= OLDI_MSB;
>> + else if (bus_fmt->data_width != 18)
>> + dev_warn(oldi->dev,
>> + "OLDI%u: DSS port width %d not supported\n",
>> + oldi->oldi_instance, bus_fmt->data_width);
>> +
>> + oldi_cfg |= OLDI_DEPOL;
>> +
>> + oldi_cfg = (oldi_cfg & (~OLDI_MAP)) | (bus_fmt->oldi_mode_reg_val
>> << 1);
>> +
>> + oldi_cfg |= OLDI_SOFTRST;
>> +
>> + oldi_cfg |= OLDI_ENABLE;
>> +
>> + switch (oldi->link_type) {
>> + case OLDI_MODE_SINGLE_LINK:
>> + /* All configuration is done for this mode. */
>> + break;
>> +
>> + case OLDI_MODE_CLONE_SINGLE_LINK:
>> + oldi_cfg |= OLDI_CLONE_MODE;
>> + break;
>> +
>> + case OLDI_MODE_DUAL_LINK:
>> + /* data-mapping field also indicates dual-link mode */
>> + oldi_cfg |= BIT(3);
>> + oldi_cfg |= OLDI_DUALMODESYNC;
>> + break;
>> +
>> + default:
>> + dev_err(oldi->dev, "OLDI%u: Unsupported mode.\n",
>> + oldi->oldi_instance);
>> + return -EINVAL;
>> + }
>> +
>> + tidss_configure_oldi(oldi->tidss, oldi->parent_vp, oldi_cfg);
>> +
>> + return 0;
>> +}
>> +
>> +static void tidss_oldi_atomic_pre_enable(struct drm_bridge *bridge,
>> + struct drm_bridge_state *old_bridge_state)
>> +{
>> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
>> + struct drm_atomic_state *state = old_bridge_state->base.state;
>> + struct drm_connector *connector;
>> + struct drm_connector_state *conn_state;
>> + struct drm_crtc_state *crtc_state;
>> + struct drm_display_mode *mode;
>> +
>> + connector = drm_atomic_get_new_connector_for_encoder(state,
>> + bridge->encoder);
>> + if (WARN_ON(!connector))
>> + return;
>> +
>> + conn_state = drm_atomic_get_new_connector_state(state, connector);
>> + if (WARN_ON(!conn_state))
>> + return;
>> +
>> + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
>> + if (WARN_ON(!crtc_state))
>> + return;
>> +
>> + mode = &crtc_state->adjusted_mode;
>> +
>> + /* Configure the OLDI params*/
>> + tidss_oldi_config(oldi);
>> +
>> + /* Set the OLDI serial clock (7 times the pixel clock) */
>> + tidss_oldi_set_serial_clk(oldi, mode->clock * 7 * 1000);
>> +
>> + /* Enable OLDI IO power */
>> + tidss_oldi_tx_power(oldi, true);
>> +}
>> +
>> +static void tidss_oldi_atomic_post_disable(struct drm_bridge *bridge,
>> + struct drm_bridge_state *old_bridge_state)
>> +{
>> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
>> +
>> + /* Disable OLDI IO power */
>> + tidss_oldi_tx_power(oldi, false);
>> +
>> + /* Set the OLDI serial clock to IDLE Frequency */
>> + tidss_oldi_set_serial_clk(oldi, OLDI_IDLE_CLK_HZ);
>> +
>> + /* Clear OLDI Config */
>> + tidss_configure_oldi(oldi->tidss, oldi->parent_vp, 0);
>> +}
>> +
>> +#define MAX_INPUT_SEL_FORMATS 1
>> +
>> +static u32 *tidss_oldi_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)
>> +{
>> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
>> + u32 *input_fmts;
>> + int i;
>> +
>> + *num_input_fmts = 0;
>> +
>> + for (i = 0; i < ARRAY_SIZE(oldi_bus_formats); i++)
>> + if (oldi_bus_formats[i].bus_fmt == output_fmt)
>> + break;
>> +
>> + if (i == ARRAY_SIZE(oldi_bus_formats))
>> + return NULL;
>> +
>> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>> + GFP_KERNEL);
>> + if (!input_fmts)
>> + return NULL;
>> +
>> + *num_input_fmts = 1;
>> + input_fmts[0] = oldi_bus_formats[i].input_bus_fmt;
>> + oldi->bus_format = &oldi_bus_formats[i];
>> +
>> + return input_fmts;
>> +}
>> +
>> +static const struct drm_bridge_funcs tidss_oldi_bridge_funcs = {
>> + .attach = tidss_oldi_bridge_attach,
>
> Looks like an extra tab there?
>
> Other than those two cosmetic issues, I think this looks fine. I also
> tested on AM62-SK.
Also, feel free to add:
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Tomi
> However, one more thing. We'll have a separate OLDI bridge, but we still
> will have the old OLDI code for AM65x in the tidss_dispc.c. And to mix
> things up, we will have some new OLDI code there too
> (tidss_configure_oldi). Could you check the AM65x specific code and
> perhaps rename the functions to am65x or such, to make this clearer.
> Perhaps also the DISPC_VP_OLDI should be DISPC_VP_OLDI_AM65X, as that VP
> type shouldn't be used for anything else.
>
> Tomi
>
Powered by blists - more mailing lists