Hello Aradhya, On Sun, May 12, 2024 at 08:53:12PM +0530, Aradhya Bhatia wrote: > On 12/05/24 17:18, Francesco Dolcini wrote: > > On Sun, May 12, 2024 at 01:00:55AM +0530, Aradhya Bhatia wrote: > >> Up till now, the OLDI support in tidss was integrated within the tidss dispc. > >> This was fine till the OLDI was one-to-mapped with the DSS video-port (VP). > >> The AM62 and AM62P SoCs have 2 OLDI TXes that can support dual-lvds / lvds-clone > >> modes. > >> > >> Add OLDI TXes as separate DRM bridge entities to better support the new LVDS > >> configurations. > >> > >> Signed-off-by: Aradhya Bhatia <a-bhatia1@xxxxxx> > >> --- > >> drivers/gpu/drm/tidss/Makefile | 3 +- > >> drivers/gpu/drm/tidss/tidss_dispc.c | 11 +- > >> drivers/gpu/drm/tidss/tidss_dispc.h | 4 + > >> drivers/gpu/drm/tidss/tidss_drv.c | 13 +- > >> drivers/gpu/drm/tidss/tidss_drv.h | 4 + > >> drivers/gpu/drm/tidss/tidss_oldi.c | 568 ++++++++++++++++++++++++++++ > >> drivers/gpu/drm/tidss/tidss_oldi.h | 73 ++++ > >> 7 files changed, 673 insertions(+), 3 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/tidss_oldi.h b/drivers/gpu/drm/tidss/tidss_oldi.h > >> new file mode 100644 > >> index 000000000000..5ad02ddea11a > >> --- /dev/null > >> +++ b/drivers/gpu/drm/tidss/tidss_oldi.h > >> @@ -0,0 +1,73 @@ > >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ > >> +/* > >> + * Copyright (C) 2023 - Texas Instruments Incorporated > >> + * > >> + * Aradhya Bhatia <a-bhati1@xxxxxx> > >> + */ > >> + > >> +#ifndef __TIDSS_OLDI_H__ > >> +#define __TIDSS_OLDI_H__ > >> + > >> +#include <linux/media-bus-format.h> > >> + > >> +#include "tidss_drv.h" > >> +#include "tidss_dispc.h" > >> + > >> +struct tidss_oldi; > > > > why do you need this here? > > So that struct tidss_device can store pointers to struct tidss_oldi > instances. on this and ... > >> +#define OLDI_IDLE_CLK_HZ 25000000 /*25 MHz */ > > this is used only on a single C files, move it there? > > > > I would consider this comment in general for this header file, > > from a quick check most of this is used only in tidss_oldi.c. > > Apart from struct tidss_device being able to access struct tidss_oldi, > there is no direct access to any of the above. > > Perhaps I can move the idle clock definition into the C file. > > However, before tidss_oldi.h, all the register definitions have been > stored in tidss_dispc_regs.h. It just seemed right to keep them out in > the header file and maintain the status quo. ... this they are probably more of a personal taste topic, just go for whatever you and the actual maintainer (tomi?) prefer. Thanks, Francesco
