| Message ID | 20241203172129.778123-1-tommaso.merciai.xr@bp.renesas.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm/bridge: ite-it6263: Support VESA input format | expand |
On Tue, Dec 03, 2024 at 06:21:29PM +0100, tomm.merciai@gmail.com wrote: > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> > > Introduce it6263_is_input_bus_fmt_valid() and refactor the > it6263_bridge_atomic_get_input_bus_fmts() function to support VESA > format by selecting the LVDS input format based on the LVDS data mapping > and thereby support both JEIDA and VESA input formats. For the patch itself, Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> A more generic question: is the bridge limited to 4 lanes or does it support 3-lane or 5-lane configurations? > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> > --- > drivers/gpu/drm/bridge/ite-it6263.c | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c > index cbabd4e20d3e..83d1db29157a 100644 > --- a/drivers/gpu/drm/bridge/ite-it6263.c > +++ b/drivers/gpu/drm/bridge/ite-it6263.c > @@ -48,6 +48,7 @@ > #define REG_COL_DEP GENMASK(1, 0) > #define BIT8 FIELD_PREP(REG_COL_DEP, 1) > #define OUT_MAP BIT(4) > +#define VESA BIT(4) > #define JEIDA 0 > #define REG_DESSC_ENB BIT(6) > #define DMODE BIT(7) > @@ -428,12 +429,30 @@ static inline void it6263_lvds_reset(struct it6263 *it) > fsleep(10000); > } > > +static bool it6263_is_input_bus_fmt_valid(u32 input_fmt) > +{ > + switch (input_fmt) { > + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA: > + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG: > + return true; > + } > + return false; > +} > +
On 12/04/2024, tomm.merciai@gmail.com wrote: > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> > > Introduce it6263_is_input_bus_fmt_valid() and refactor the > it6263_bridge_atomic_get_input_bus_fmts() function to support VESA > format by selecting the LVDS input format based on the LVDS data mapping > and thereby support both JEIDA and VESA input formats. ite,it6263.yaml says IT6263 supports vesa-24 and vesa-30, while this patch actually only adds vesa-24 support. So, to be more specific, the patch subject and commit message should reflect this rather than claim "Support VESA input format". > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> Can you please send this patch with your Renesas email address instead of Gmail email address? Otherwise, add a Signed-off-by tag with your Gmail email address. > --- > drivers/gpu/drm/bridge/ite-it6263.c | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c > index cbabd4e20d3e..83d1db29157a 100644 > --- a/drivers/gpu/drm/bridge/ite-it6263.c > +++ b/drivers/gpu/drm/bridge/ite-it6263.c > @@ -48,6 +48,7 @@ > #define REG_COL_DEP GENMASK(1, 0) > #define BIT8 FIELD_PREP(REG_COL_DEP, 1) > #define OUT_MAP BIT(4) > +#define VESA BIT(4) > #define JEIDA 0 > #define REG_DESSC_ENB BIT(6) > #define DMODE BIT(7) > @@ -428,12 +429,30 @@ static inline void it6263_lvds_reset(struct it6263 *it) > fsleep(10000); > } > > +static bool it6263_is_input_bus_fmt_valid(u32 input_fmt) 1) Inline this small function. 2) Change the argument input_fmt type from u32 to int to match the type of it->lvds_data_mapping. static inline bool it6263_is_input_bus_fmt_valid(int input_fmt) > +{ > + switch (input_fmt) { > + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA: > + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG: > + return true; > + } > + return false; > +} > + > static inline void it6263_lvds_set_interface(struct it6263 *it) > { > + u8 fmt; > + > /* color depth */ > regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, REG_COL_DEP, BIT8); > + > + if (it->lvds_data_mapping == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) > + fmt = VESA; > + else > + fmt = JEIDA; > + > /* output mapping */ > - regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, OUT_MAP, JEIDA); > + regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, OUT_MAP, fmt); > > if (it->lvds_dual_link) { > regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, DMODE, DISO); > @@ -714,14 +733,14 @@ it6263_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, > > *num_input_fmts = 0; > > - if (it->lvds_data_mapping != MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA) > + if (!it6263_is_input_bus_fmt_valid(it->lvds_data_mapping)) > return NULL; > > input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL); > if (!input_fmts) > return NULL; > > - input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA; > + input_fmts[0] = it->lvds_data_mapping; > *num_input_fmts = 1; > > return input_fmts;
On 12/04/2024, Dmitry Baryshkov wrote: > On Tue, Dec 03, 2024 at 06:21:29PM +0100, tomm.merciai@gmail.com wrote: >> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> >> >> Introduce it6263_is_input_bus_fmt_valid() and refactor the >> it6263_bridge_atomic_get_input_bus_fmts() function to support VESA >> format by selecting the LVDS input format based on the LVDS data mapping >> and thereby support both JEIDA and VESA input formats. > > For the patch itself, > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > A more generic question: is the bridge limited to 4 lanes or does it > support 3-lane or 5-lane configurations? According to ite,it6263.yaml, the bridge supports all the data mappings(jeida-{18,24,30} and vesa-{24,30}) listed in lvds-data-mapping.yaml. lvds-data-mapping.yaml specifies the data lanes(3/4/5) used by each of the data mappings. So, the bridge supports 3, 4 or 5 data lanes. > >> >> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> >> --- >> drivers/gpu/drm/bridge/ite-it6263.c | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c >> index cbabd4e20d3e..83d1db29157a 100644 >> --- a/drivers/gpu/drm/bridge/ite-it6263.c >> +++ b/drivers/gpu/drm/bridge/ite-it6263.c >> @@ -48,6 +48,7 @@ >> #define REG_COL_DEP GENMASK(1, 0) >> #define BIT8 FIELD_PREP(REG_COL_DEP, 1) >> #define OUT_MAP BIT(4) >> +#define VESA BIT(4) >> #define JEIDA 0 >> #define REG_DESSC_ENB BIT(6) >> #define DMODE BIT(7) >> @@ -428,12 +429,30 @@ static inline void it6263_lvds_reset(struct it6263 *it) >> fsleep(10000); >> } >> >> +static bool it6263_is_input_bus_fmt_valid(u32 input_fmt) >> +{ >> + switch (input_fmt) { >> + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA: >> + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG: >> + return true; >> + } >> + return false; >> +} >> + >
Hi Liu Ying, > -----Original Message----- > From: Liu Ying <victor.liu@nxp.com> > Sent: 04 December 2024 03:34 > To: tomm.merciai@gmail.com > Subject: Re: [PATCH] drm/bridge: ite-it6263: Support VESA input format > > On 12/04/2024, tomm.merciai@gmail.com wrote: > > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> > > > > Introduce it6263_is_input_bus_fmt_valid() and refactor the > > it6263_bridge_atomic_get_input_bus_fmts() function to support VESA > > format by selecting the LVDS input format based on the LVDS data > > mapping and thereby support both JEIDA and VESA input formats. > > ite,it6263.yaml says IT6263 supports vesa-24 and vesa-30, while this patch actually only adds vesa-24 > support. So, to be more specific, the patch subject and commit message should reflect this rather > than claim "Support VESA input format". > > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> > > Can you please send this patch with your Renesas email address instead of Gmail email address? > Otherwise, add a Signed-off-by tag with your Gmail email address. If I am correct, you can clearly see the Renesas email address after subject. So, the procedure followed by Tommaso is correct. You can use gmail account for send patch, but you just need to add From tag and SoB tag after subject with company address. Please shout if anyone think this procedure is wrong. <snippet> Subject: Re: [PATCH] drm/bridge: ite-it6263: Support VESA input format From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> Introduce it6263_is_input_bus_fmt_valid() and refactor the </snippet> Cheers, Biju
Hi Liu Ying, > -----Original Message----- > From: Liu Ying <victor.liu@nxp.com> > Sent: 04 December 2024 03:43 > Subject: Re: [PATCH] drm/bridge: ite-it6263: Support VESA input format > > On 12/04/2024, Dmitry Baryshkov wrote: > > On Tue, Dec 03, 2024 at 06:21:29PM +0100, tomm.merciai@gmail.com wrote: > >> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> > >> > >> Introduce it6263_is_input_bus_fmt_valid() and refactor the > >> it6263_bridge_atomic_get_input_bus_fmts() function to support VESA > >> format by selecting the LVDS input format based on the LVDS data > >> mapping and thereby support both JEIDA and VESA input formats. > > > > For the patch itself, > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > A more generic question: is the bridge limited to 4 lanes or does it > > support 3-lane or 5-lane configurations? > > According to ite,it6263.yaml, the bridge supports all the data mappings(jeida-{18,24,30} and vesa- > {24,30}) listed in lvds-data-mapping.yaml. lvds-data-mapping.yaml specifies the data lanes(3/4/5) > used by each of the data mappings. So, the bridge supports 3, 4 or 5 data lanes. In Renesas SMARC RZ/G3E LVDS add on board, only 4 LVDS Rx lanes connected. The 5th one is unconnected. What is the situation in your board Liu Ying? Cheers, Biju
On 12/04/2024, Biju Das wrote: > Hi Liu Ying, Hi Biju, > >> -----Original Message----- >> From: Liu Ying <victor.liu@nxp.com> >> Sent: 04 December 2024 03:34 >> To: tomm.merciai@gmail.com >> Subject: Re: [PATCH] drm/bridge: ite-it6263: Support VESA input format >> >> On 12/04/2024, tomm.merciai@gmail.com wrote: >>> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> >>> >>> Introduce it6263_is_input_bus_fmt_valid() and refactor the >>> it6263_bridge_atomic_get_input_bus_fmts() function to support VESA >>> format by selecting the LVDS input format based on the LVDS data >>> mapping and thereby support both JEIDA and VESA input formats. >> >> ite,it6263.yaml says IT6263 supports vesa-24 and vesa-30, while this patch actually only adds vesa-24 >> support. So, to be more specific, the patch subject and commit message should reflect this rather >> than claim "Support VESA input format". >> >>> >>> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> >> >> Can you please send this patch with your Renesas email address instead of Gmail email address? >> Otherwise, add a Signed-off-by tag with your Gmail email address. > > If I am correct, you can clearly see the Renesas email address after subject. Yes. > So, the procedure followed by Tommaso is correct. No, I think it's wrong. We don't know whether the person(s) behind the two Email addresses are the same Tommaso. Likely it's a person from two different domains - still kind of two people. AFAIK, sending other's patch needs to add sender's SoB tag. > > You can use gmail account for send patch, but you just need to add From tag and SoB tag > after subject with company address. Again, we don't know sender and author are the same person if their Email addresses are different. > > Please shout if anyone think this procedure is wrong. > > <snippet> > Subject: Re: [PATCH] drm/bridge: ite-it6263: Support VESA input format > > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> > > Introduce it6263_is_input_bus_fmt_valid() and refactor the > </snippet> > > > Cheers, > Biju
-----Original Message----- From: tomm.merciai@gmail.com <tomm.merciai@gmail.com> Sent: Tuesday, December 3, 2024 6:21 PM To: tomm.merciai@gmail.com Cc: linux-renesas-soc@vger.kernel.org; dri-devel@lists.freedesktop.org; Biju Das <biju.das.jz@bp.renesas.com>; Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>; Liu Ying <victor.liu@nxp.com>; Andrzej Hajda <andrzej.hajda@intel.com>; Neil Armstrong <neil.armstrong@linaro.org>; Robert Foss <rfoss@kernel.org>; laurent.pinchart <laurent.pinchart@ideasonboard.com>; Jonas Karlman <jonas@kwiboo.se>; Jernej Skrabec <jernej.skrabec@gmail.com>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; linux-kernel@vger.kernel.org Subject: [PATCH] drm/bridge: ite-it6263: Support VESA input format From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> Introduce it6263_is_input_bus_fmt_valid() and refactor the it6263_bridge_atomic_get_input_bus_fmts() function to support VESA format by selecting the LVDS input format based on the LVDS data mapping and thereby support both JEIDA and VESA input formats. Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> --- drivers/gpu/drm/bridge/ite-it6263.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c index cbabd4e20d3e..83d1db29157a 100644 --- a/drivers/gpu/drm/bridge/ite-it6263.c +++ b/drivers/gpu/drm/bridge/ite-it6263.c @@ -48,6 +48,7 @@ #define REG_COL_DEP GENMASK(1, 0) #define BIT8 FIELD_PREP(REG_COL_DEP, 1) #define OUT_MAP BIT(4) +#define VESA BIT(4) #define JEIDA 0 #define REG_DESSC_ENB BIT(6) #define DMODE BIT(7) @@ -428,12 +429,30 @@ static inline void it6263_lvds_reset(struct it6263 *it) fsleep(10000); } +static bool it6263_is_input_bus_fmt_valid(u32 input_fmt) { + switch (input_fmt) { + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA: + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG: + return true; + } + return false; +} + static inline void it6263_lvds_set_interface(struct it6263 *it) { + u8 fmt; + /* color depth */ regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, REG_COL_DEP, BIT8); + + if (it->lvds_data_mapping == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) + fmt = VESA; + else + fmt = JEIDA; + /* output mapping */ - regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, OUT_MAP, JEIDA); + regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, OUT_MAP, fmt); if (it->lvds_dual_link) { regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, DMODE, DISO); @@ -714,14 +733,14 @@ it6263_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, *num_input_fmts = 0; - if (it->lvds_data_mapping != MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA) + if (!it6263_is_input_bus_fmt_valid(it->lvds_data_mapping)) return NULL; input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL); if (!input_fmts) return NULL; - input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA; + input_fmts[0] = it->lvds_data_mapping; *num_input_fmts = 1; return input_fmts; -- 2.34.1 Acked-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c index cbabd4e20d3e..83d1db29157a 100644 --- a/drivers/gpu/drm/bridge/ite-it6263.c +++ b/drivers/gpu/drm/bridge/ite-it6263.c @@ -48,6 +48,7 @@ #define REG_COL_DEP GENMASK(1, 0) #define BIT8 FIELD_PREP(REG_COL_DEP, 1) #define OUT_MAP BIT(4) +#define VESA BIT(4) #define JEIDA 0 #define REG_DESSC_ENB BIT(6) #define DMODE BIT(7) @@ -428,12 +429,30 @@ static inline void it6263_lvds_reset(struct it6263 *it) fsleep(10000); } +static bool it6263_is_input_bus_fmt_valid(u32 input_fmt) +{ + switch (input_fmt) { + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA: + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG: + return true; + } + return false; +} + static inline void it6263_lvds_set_interface(struct it6263 *it) { + u8 fmt; + /* color depth */ regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, REG_COL_DEP, BIT8); + + if (it->lvds_data_mapping == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) + fmt = VESA; + else + fmt = JEIDA; + /* output mapping */ - regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, OUT_MAP, JEIDA); + regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, OUT_MAP, fmt); if (it->lvds_dual_link) { regmap_write_bits(it->lvds_regmap, LVDS_REG_2C, DMODE, DISO); @@ -714,14 +733,14 @@ it6263_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, *num_input_fmts = 0; - if (it->lvds_data_mapping != MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA) + if (!it6263_is_input_bus_fmt_valid(it->lvds_data_mapping)) return NULL; input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL); if (!input_fmts) return NULL; - input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA; + input_fmts[0] = it->lvds_data_mapping; *num_input_fmts = 1; return input_fmts;