drm/bridge: ite-it6263: Support VESA input format

Commit Message

Tommaso Merciai Dec. 3, 2024, 5:21 p.m. UTC

From: Tommaso Merciai <[e-mail]>

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 <[e-mail]>
---
 drivers/gpu/drm/bridge/ite-it6263.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

Comments

Dmitry Baryshkov Dec. 3, 2024, 5:37 p.m. UTC | #1

On Tue, Dec 03, 2024 at 06:21:29PM +0100, [e-mail] wrote:
> From: Tommaso Merciai <[e-mail]>
> 
> 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 <[e-mail]>

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 <[e-mail]>
> ---
>  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;
> +}
> +

Liu Ying Dec. 4, 2024, 3:34 a.m. UTC | #2

On 12/04/2024, [e-mail] wrote:
> From: Tommaso Merciai <[e-mail]>
> 
> 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 <[e-mail]>

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;

Liu Ying Dec. 4, 2024, 3:42 a.m. UTC | #3

On 12/04/2024, Dmitry Baryshkov wrote:
> On Tue, Dec 03, 2024 at 06:21:29PM +0100, [e-mail] wrote:
>> From: Tommaso Merciai <[e-mail]>
>>
>> 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 <[e-mail]>
> 
> 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 <[e-mail]>
>> ---
>>  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;
>> +}
>> +
>

Biju Das Dec. 4, 2024, 6:27 a.m. UTC | #4

Hi Liu Ying,

> -----Original Message-----
> From: Liu Ying <[e-mail]>
> Sent: 04 December 2024 03:34
> To: [e-mail]
> Subject: Re: [PATCH] drm/bridge: ite-it6263: Support VESA input format
> 
> On 12/04/2024, [e-mail] wrote:
> > From: Tommaso Merciai <[e-mail]>
> >
> > 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 <[e-mail]>
> 
> 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 <[e-mail]>

Introduce it6263_is_input_bus_fmt_valid() and refactor the
</snippet>


Cheers,
Biju

Biju Das Dec. 4, 2024, 6:45 a.m. UTC | #5

Hi Liu Ying,

> -----Original Message-----
> From: Liu Ying <[e-mail]>
> 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, [e-mail] wrote:
> >> From: Tommaso Merciai <[e-mail]>
> >>
> >> 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 <[e-mail]>
> >
> > 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

Liu Ying Dec. 4, 2024, 6:47 a.m. UTC | #6

On 12/04/2024, Biju Das wrote:
> Hi Liu Ying,

Hi Biju,

> 
>> -----Original Message-----
>> From: Liu Ying <[e-mail]>
>> Sent: 04 December 2024 03:34
>> To: [e-mail]
>> Subject: Re: [PATCH] drm/bridge: ite-it6263: Support VESA input format
>>
>> On 12/04/2024, [e-mail] wrote:
>>> From: Tommaso Merciai <[e-mail]>
>>>
>>> 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 <[e-mail]>
>>
>> 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 <[e-mail]>
> 
> Introduce it6263_is_input_bus_fmt_valid() and refactor the
> </snippet>
> 
> 
> Cheers,
> Biju

Tommaso Merciai Dec. 4, 2024, 8:11 a.m. UTC | #7

-----Original Message-----
From: [e-mail] <[e-mail]>
Sent: Tuesday, December 3, 2024 6:21 PM
To: [e-mail]
Cc: [e-mail]; [e-mail]; Biju Das <[e-mail]>; Tommaso Merciai <[e-mail]>; Liu Ying <[e-mail]>; Andrzej Hajda <[e-mail]>; Neil Armstrong <[e-mail]>; Robert Foss <[e-mail]>; laurent.pinchart <[e-mail]>; Jonas Karlman <[e-mail]>; Jernej Skrabec <[e-mail]>; Maarten Lankhorst <[e-mail]>; Maxime Ripard <[e-mail]>; Thomas Zimmermann <[e-mail]>; David Airlie <[e-mail]>; Simona Vetter <[e-mail]>; [e-mail]
Subject: [PATCH] drm/bridge: ite-it6263: Support VESA input format

From: Tommaso Merciai <[e-mail]>

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 <[e-mail]>
---
 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 <[e-mail]>

Tommaso Merciai Dec. 4, 2024, 10:37 a.m. UTC | #8

Hi Liu Ying,
Thanks for your review.

On Wed, Dec 04, 2024 at 11:34:23AM +0800, Liu Ying wrote:
> On 12/04/2024, [e-mail] wrote:
> > From: Tommaso Merciai <[e-mail]>
> > 
> > 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".

Fully agree, thanks.
I will fix that in v2 specifying vesa-24 like you suggest.

> 
> > 
> > Signed-off-by: Tommaso Merciai <[e-mail]>
> 
> 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.

Thanks, for the point.
What about adding Acked-by: from my renesas 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.

Will fix that in v2, thanks :)

> 
> 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;
> 
> -- 
> Regards,
> Liu Ying

Thanks & Regards,
Tommaso


>

Dmitry Baryshkov Dec. 4, 2024, 11:53 a.m. UTC | #9

On Wed, Dec 04, 2024 at 11:37:05AM +0100, Tommaso Merciai wrote:
> Hi Liu Ying,
> Thanks for your review.
> 
> On Wed, Dec 04, 2024 at 11:34:23AM +0800, Liu Ying wrote:
> > On 12/04/2024, [e-mail] wrote:
> > > From: Tommaso Merciai <[e-mail]>
> > > 
> > > 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".
> 
> Fully agree, thanks.
> I will fix that in v2 specifying vesa-24 like you suggest.
> 
> > 
> > > 
> > > Signed-off-by: Tommaso Merciai <[e-mail]>
> > 
> > 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.
> 
> Thanks, for the point.
> What about adding Acked-by: from my renesas email address?

Acked-by has a different meaning. I'd say that generally it's okay to
have this light mismatch, see [1] or any of the emails that B4 generates
for web-based submission.

[1] https://lore.kernel.org/dri-devel/[e-mail]/

>

Tommaso Merciai Dec. 4, 2024, 1:11 p.m. UTC | #10

Hi Dmitry,

On Wed, Dec 04, 2024 at 01:53:44PM +0200, Dmitry Baryshkov wrote:
> On Wed, Dec 04, 2024 at 11:37:05AM +0100, Tommaso Merciai wrote:
> > Hi Liu Ying,
> > Thanks for your review.
> > 
> > On Wed, Dec 04, 2024 at 11:34:23AM +0800, Liu Ying wrote:
> > > On 12/04/2024, [e-mail] wrote:
> > > > From: Tommaso Merciai <[e-mail]>
> > > > 
> > > > 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".
> > 
> > Fully agree, thanks.
> > I will fix that in v2 specifying vesa-24 like you suggest.
> > 
> > > 
> > > > 
> > > > Signed-off-by: Tommaso Merciai <[e-mail]>
> > > 
> > > 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.
> > 
> > Thanks, for the point.
> > What about adding Acked-by: from my renesas email address?
> 
> Acked-by has a different meaning. I'd say that generally it's okay to
> have this light mismatch, see [1] or any of the emails that B4 generates
> for web-based submission.
> 
> [1] https://lore.kernel.org/dri-devel/[e-mail]/

Thanks for sharing this example and for the clarification.
This is similar to my case :)

Then v1 procedure is correct?
I can use the same formatting (From: Sob:) for sending v2?

Thanks & Regards,
Tommaso
> 
> > 
> 
> -- 
> With best wishes
> Dmitry

Dmitry Baryshkov Dec. 4, 2024, 1:33 p.m. UTC | #11

On Wed, Dec 04, 2024 at 02:11:28PM +0100, Tommaso Merciai wrote:
> Hi Dmitry,
> 
> On Wed, Dec 04, 2024 at 01:53:44PM +0200, Dmitry Baryshkov wrote:
> > On Wed, Dec 04, 2024 at 11:37:05AM +0100, Tommaso Merciai wrote:
> > > Hi Liu Ying,
> > > Thanks for your review.
> > > 
> > > On Wed, Dec 04, 2024 at 11:34:23AM +0800, Liu Ying wrote:
> > > > On 12/04/2024, [e-mail] wrote:
> > > > > From: Tommaso Merciai <[e-mail]>
> > > > > 
> > > > > 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".
> > > 
> > > Fully agree, thanks.
> > > I will fix that in v2 specifying vesa-24 like you suggest.
> > > 
> > > > 
> > > > > 
> > > > > Signed-off-by: Tommaso Merciai <[e-mail]>
> > > > 
> > > > 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.
> > > 
> > > Thanks, for the point.
> > > What about adding Acked-by: from my renesas email address?
> > 
> > Acked-by has a different meaning. I'd say that generally it's okay to
> > have this light mismatch, see [1] or any of the emails that B4 generates
> > for web-based submission.
> > 
> > [1] https://lore.kernel.org/dri-devel/[e-mail]/
> 
> Thanks for sharing this example and for the clarification.
> This is similar to my case :)
> 
> Then v1 procedure is correct?
> I can use the same formatting (From: Sob:) for sending v2?

I think so

> 
> Thanks & Regards,
> Tommaso
> > 
> > > 
> > 
> > -- 
> > With best wishes
> > Dmitry

Tommaso Merciai Dec. 4, 2024, 1:36 p.m. UTC | #12

On Wed, Dec 04, 2024 at 03:33:09PM +0200, Dmitry Baryshkov wrote:
> On Wed, Dec 04, 2024 at 02:11:28PM +0100, Tommaso Merciai wrote:
> > Hi Dmitry,
> > 
> > On Wed, Dec 04, 2024 at 01:53:44PM +0200, Dmitry Baryshkov wrote:
> > > On Wed, Dec 04, 2024 at 11:37:05AM +0100, Tommaso Merciai wrote:
> > > > Hi Liu Ying,
> > > > Thanks for your review.
> > > > 
> > > > On Wed, Dec 04, 2024 at 11:34:23AM +0800, Liu Ying wrote:
> > > > > On 12/04/2024, [e-mail] wrote:
> > > > > > From: Tommaso Merciai <[e-mail]>
> > > > > > 
> > > > > > 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".
> > > > 
> > > > Fully agree, thanks.
> > > > I will fix that in v2 specifying vesa-24 like you suggest.
> > > > 
> > > > > 
> > > > > > 
> > > > > > Signed-off-by: Tommaso Merciai <[e-mail]>
> > > > > 
> > > > > 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.
> > > > 
> > > > Thanks, for the point.
> > > > What about adding Acked-by: from my renesas email address?
> > > 
> > > Acked-by has a different meaning. I'd say that generally it's okay to
> > > have this light mismatch, see [1] or any of the emails that B4 generates
> > > for web-based submission.
> > > 
> > > [1] https://lore.kernel.org/dri-devel/[e-mail]/
> > 
> > Thanks for sharing this example and for the clarification.
> > This is similar to my case :)
> > 
> > Then v1 procedure is correct?
> > I can use the same formatting (From: Sob:) for sending v2?
> 
> I think so

Oks, thanks for the feedback.

Regards,
Tommaso

> 
> > 
> > Thanks & Regards,
> > Tommaso
> > > 
> > > > 
> > > 
> > > -- 
> > > With best wishes
> > > Dmitry
> 
> -- 
> With best wishes
> Dmitry

Liu Ying Dec. 5, 2024, 9:02 a.m. UTC | #13

On 12/04/2024, Biju Das wrote:
> Hi Liu Ying,

Hi Biju,

> 
>> -----Original Message-----
>> From: Liu Ying <[e-mail]>
>> 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, [e-mail] wrote:
>>>> From: Tommaso Merciai <[e-mail]>
>>>>
>>>> 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 <[e-mail]>
>>>
>>> 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?

The adapter cards I'm using to connect with i.MX8MP EVK have only 4
LVDS data lanes routed out. 

> 
> Cheers,
> Biju

Dmitry Baryshkov Dec. 5, 2024, 11:42 a.m. UTC | #14

On Thu, 5 Dec 2024 at 11:01, Liu Ying <[e-mail]> wrote:
>
> On 12/04/2024, Biju Das wrote:
> > Hi Liu Ying,
>
> Hi Biju,
>
> >
> >> -----Original Message-----
> >> From: Liu Ying <[e-mail]>
> >> 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, [e-mail] wrote:
> >>>> From: Tommaso Merciai <[e-mail]>
> >>>>
> >>>> 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 <[e-mail]>
> >>>
> >>> 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?
>
> The adapter cards I'm using to connect with i.MX8MP EVK have only 4
> LVDS data lanes routed out.

What about the bridge SoC. I don't understand why the driver gets
limited by a particular add-on card.

Biju Das Dec. 5, 2024, 12:19 p.m. UTC | #15

Hi Dmitry Baryshkov,

> -----Original Message-----
> From: Dmitry Baryshkov <[e-mail]>
> Sent: 05 December 2024 11:43
> Subject: Re: [PATCH] drm/bridge: ite-it6263: Support VESA input format
> 
> On Thu, 5 Dec 2024 at 11:01, Liu Ying <[e-mail]> wrote:
> >
> > On 12/04/2024, Biju Das wrote:
> > > Hi Liu Ying,
> >
> > Hi Biju,
> >
> > >
> > >> -----Original Message-----
> > >> From: Liu Ying <[e-mail]>
> > >> 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, [e-mail] wrote:
> > >>>> From: Tommaso Merciai <[e-mail]>
> > >>>>
> > >>>> 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 <[e-mail]>
> > >>>
> > >>> 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?
> >
> > The adapter cards I'm using to connect with i.MX8MP EVK have only 4
> > LVDS data lanes routed out.
> 
> What about the bridge SoC. I don't understand why the driver gets limited by a particular add-on card.

Bridge SoC supports up to 5 Rx Lanes. But vendors do the wiring to bridge SoC and 
currently as far as we know there are 2 boards with ITE 6263 bridge SoC and both of
them has 4 LVDS rx lanes connected and both vesa-24 and Jeida-24 formats are tested.

Cheers,
Biju

Patch

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;