[v2] media: adv7604: extend deep color mode to ADV7611

Message ID 20191018112955.13652-1-alexandru.ardelean@analog.com (mailing list archive)
State Accepted, archived
Delegated to: Hans Verkuil
Headers
Series [v2] media: adv7604: extend deep color mode to ADV7611 |

Commit Message

Ardelean, Alexandru Oct. 18, 2019, 11:29 a.m. UTC
  From: Dragos Bogdan <dragos.bogdan@analog.com>

The AD7611 chip supports the same Deep Color Mode settings as the AD7604.
This change extends support for this feature to the AD7611 by adding a
wrapper function for the `read_hdmi_pixelclock` hook and adding the same
frequency adjustment logic.

Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v1 -> v2:
* address checkpatch complaints about 80 col limit and
  use of `unsigned` vs `unsigned int`

 drivers/media/i2c/adv7604.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)
  

Comments

Ardelean, Alexandru Nov. 25, 2019, 3:06 p.m. UTC | #1
On Fri, 2019-10-18 at 14:29 +0300, Alexandru Ardelean wrote:
> From: Dragos Bogdan <dragos.bogdan@analog.com>
> 
> The AD7611 chip supports the same Deep Color Mode settings as the AD7604.
> This change extends support for this feature to the AD7611 by adding a
> wrapper function for the `read_hdmi_pixelclock` hook and adding the same
> frequency adjustment logic.
> 

ping here

> Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> 
> Changelog v1 -> v2:
> * address checkpatch complaints about 80 col limit and
>   use of `unsigned` vs `unsigned int`
> 
>  drivers/media/i2c/adv7604.c | 32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 28a84bf9f8a9..c6fe3aa69733 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1503,23 +1503,14 @@ static void
> adv76xx_fill_optional_dv_timings_fields(struct v4l2_subdev *sd,
>  
>  static unsigned int adv7604_read_hdmi_pixelclock(struct v4l2_subdev *sd)
>  {
> -	unsigned int freq;
>  	int a, b;
>  
>  	a = hdmi_read(sd, 0x06);
>  	b = hdmi_read(sd, 0x3b);
>  	if (a < 0 || b < 0)
>  		return 0;
> -	freq =  a * 1000000 + ((b & 0x30) >> 4) * 250000;
>  
> -	if (is_hdmi(sd)) {
> -		/* adjust for deep color mode */
> -		unsigned bits_per_channel = ((hdmi_read(sd, 0x0b) & 0x60)
> >> 4) + 8;
> -
> -		freq = freq * 8 / bits_per_channel;
> -	}
> -
> -	return freq;
> +	return a * 1000000 + ((b & 0x30) >> 4) * 250000;
>  }
>  
>  static unsigned int adv7611_read_hdmi_pixelclock(struct v4l2_subdev *sd)
> @@ -1530,9 +1521,28 @@ static unsigned int
> adv7611_read_hdmi_pixelclock(struct v4l2_subdev *sd)
>  	b = hdmi_read(sd, 0x52);
>  	if (a < 0 || b < 0)
>  		return 0;
> +
>  	return ((a << 1) | (b >> 7)) * 1000000 + (b & 0x7f) * 1000000 /
> 128;
>  }
>  
> +static unsigned int adv76xx_read_hdmi_pixelclock(struct v4l2_subdev *sd)
> +{
> +	struct adv76xx_state *state = to_state(sd);
> +	const struct adv76xx_chip_info *info = state->info;
> +	unsigned int freq, bits_per_channel, pixelrepetition;
> +
> +	freq = info->read_hdmi_pixelclock(sd);
> +	if (is_hdmi(sd)) {
> +		/* adjust for deep color mode and pixel repetition */
> +		bits_per_channel = ((hdmi_read(sd, 0x0b) & 0x60) >> 4) + 8;
> +		pixelrepetition = (hdmi_read(sd, 0x05) & 0x0f) + 1;
> +
> +		freq = freq * 8 / bits_per_channel / pixelrepetition;
> +	}
> +
> +	return freq;
> +}
> +
>  static int adv76xx_query_dv_timings(struct v4l2_subdev *sd,
>  			struct v4l2_dv_timings *timings)
>  {
> @@ -1579,7 +1589,7 @@ static int adv76xx_query_dv_timings(struct
> v4l2_subdev *sd,
>  
>  		bt->width = w;
>  		bt->height = h;
> -		bt->pixelclock = info->read_hdmi_pixelclock(sd);
> +		bt->pixelclock = adv76xx_read_hdmi_pixelclock(sd);
>  		bt->hfrontporch = hdmi_read16(sd, 0x20, info-
> >hfrontporch_mask);
>  		bt->hsync = hdmi_read16(sd, 0x22, info->hsync_mask);
>  		bt->hbackporch = hdmi_read16(sd, 0x24, info-
> >hbackporch_mask);
  
Hans Verkuil Nov. 25, 2019, 4:05 p.m. UTC | #2
On 11/25/19 4:06 PM, Ardelean, Alexandru wrote:
> On Fri, 2019-10-18 at 14:29 +0300, Alexandru Ardelean wrote:
>> From: Dragos Bogdan <dragos.bogdan@analog.com>
>>
>> The AD7611 chip supports the same Deep Color Mode settings as the AD7604.
>> This change extends support for this feature to the AD7611 by adding a
>> wrapper function for the `read_hdmi_pixelclock` hook and adding the same
>> frequency adjustment logic.
>>
> 
> ping here

Not forgotten, it's on my TODO list. But I need some time to dig a bit
deeper into this code.

Regards,

	Hans

> 
>> Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
>> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>> ---
>>
>> Changelog v1 -> v2:
>> * address checkpatch complaints about 80 col limit and
>>   use of `unsigned` vs `unsigned int`
>>
>>  drivers/media/i2c/adv7604.c | 32 +++++++++++++++++++++-----------
>>  1 file changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
>> index 28a84bf9f8a9..c6fe3aa69733 100644
>> --- a/drivers/media/i2c/adv7604.c
>> +++ b/drivers/media/i2c/adv7604.c
>> @@ -1503,23 +1503,14 @@ static void
>> adv76xx_fill_optional_dv_timings_fields(struct v4l2_subdev *sd,
>>  
>>  static unsigned int adv7604_read_hdmi_pixelclock(struct v4l2_subdev *sd)
>>  {
>> -	unsigned int freq;
>>  	int a, b;
>>  
>>  	a = hdmi_read(sd, 0x06);
>>  	b = hdmi_read(sd, 0x3b);
>>  	if (a < 0 || b < 0)
>>  		return 0;
>> -	freq =  a * 1000000 + ((b & 0x30) >> 4) * 250000;
>>  
>> -	if (is_hdmi(sd)) {
>> -		/* adjust for deep color mode */
>> -		unsigned bits_per_channel = ((hdmi_read(sd, 0x0b) & 0x60)
>>>> 4) + 8;
>> -
>> -		freq = freq * 8 / bits_per_channel;
>> -	}
>> -
>> -	return freq;
>> +	return a * 1000000 + ((b & 0x30) >> 4) * 250000;
>>  }
>>  
>>  static unsigned int adv7611_read_hdmi_pixelclock(struct v4l2_subdev *sd)
>> @@ -1530,9 +1521,28 @@ static unsigned int
>> adv7611_read_hdmi_pixelclock(struct v4l2_subdev *sd)
>>  	b = hdmi_read(sd, 0x52);
>>  	if (a < 0 || b < 0)
>>  		return 0;
>> +
>>  	return ((a << 1) | (b >> 7)) * 1000000 + (b & 0x7f) * 1000000 /
>> 128;
>>  }
>>  
>> +static unsigned int adv76xx_read_hdmi_pixelclock(struct v4l2_subdev *sd)
>> +{
>> +	struct adv76xx_state *state = to_state(sd);
>> +	const struct adv76xx_chip_info *info = state->info;
>> +	unsigned int freq, bits_per_channel, pixelrepetition;
>> +
>> +	freq = info->read_hdmi_pixelclock(sd);
>> +	if (is_hdmi(sd)) {
>> +		/* adjust for deep color mode and pixel repetition */
>> +		bits_per_channel = ((hdmi_read(sd, 0x0b) & 0x60) >> 4) + 8;
>> +		pixelrepetition = (hdmi_read(sd, 0x05) & 0x0f) + 1;
>> +
>> +		freq = freq * 8 / bits_per_channel / pixelrepetition;
>> +	}
>> +
>> +	return freq;
>> +}
>> +
>>  static int adv76xx_query_dv_timings(struct v4l2_subdev *sd,
>>  			struct v4l2_dv_timings *timings)
>>  {
>> @@ -1579,7 +1589,7 @@ static int adv76xx_query_dv_timings(struct
>> v4l2_subdev *sd,
>>  
>>  		bt->width = w;
>>  		bt->height = h;
>> -		bt->pixelclock = info->read_hdmi_pixelclock(sd);
>> +		bt->pixelclock = adv76xx_read_hdmi_pixelclock(sd);
>>  		bt->hfrontporch = hdmi_read16(sd, 0x20, info-
>>> hfrontporch_mask);
>>  		bt->hsync = hdmi_read16(sd, 0x22, info->hsync_mask);
>>  		bt->hbackporch = hdmi_read16(sd, 0x24, info-
>>> hbackporch_mask);
  
Ardelean, Alexandru Nov. 26, 2019, 6:28 a.m. UTC | #3
On Mon, 2019-11-25 at 17:05 +0100, Hans Verkuil wrote:
> On 11/25/19 4:06 PM, Ardelean, Alexandru wrote:
> > On Fri, 2019-10-18 at 14:29 +0300, Alexandru Ardelean wrote:
> > > From: Dragos Bogdan <dragos.bogdan@analog.com>
> > > 
> > > The AD7611 chip supports the same Deep Color Mode settings as the
> > > AD7604.
> > > This change extends support for this feature to the AD7611 by adding
> > > a
> > > wrapper function for the `read_hdmi_pixelclock` hook and adding the
> > > same
> > > frequency adjustment logic.
> > > 
> > 
> > ping here
> 
> Not forgotten, it's on my TODO list. But I need some time to dig a bit
> deeper into this code.

Ack.

Thanks for replying.

Alex

> 
> Regards,
> 
> 	Hans
> 
> > > Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > ---
> > > 
> > > Changelog v1 -> v2:
> > > * address checkpatch complaints about 80 col limit and
> > >   use of `unsigned` vs `unsigned int`
> > > 
> > >  drivers/media/i2c/adv7604.c | 32 +++++++++++++++++++++-----------
> > >  1 file changed, 21 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/adv7604.c
> > > b/drivers/media/i2c/adv7604.c
> > > index 28a84bf9f8a9..c6fe3aa69733 100644
> > > --- a/drivers/media/i2c/adv7604.c
> > > +++ b/drivers/media/i2c/adv7604.c
> > > @@ -1503,23 +1503,14 @@ static void
> > > adv76xx_fill_optional_dv_timings_fields(struct v4l2_subdev *sd,
> > >  
> > >  static unsigned int adv7604_read_hdmi_pixelclock(struct v4l2_subdev
> > > *sd)
> > >  {
> > > -	unsigned int freq;
> > >  	int a, b;
> > >  
> > >  	a = hdmi_read(sd, 0x06);
> > >  	b = hdmi_read(sd, 0x3b);
> > >  	if (a < 0 || b < 0)
> > >  		return 0;
> > > -	freq =  a * 1000000 + ((b & 0x30) >> 4) * 250000;
> > >  
> > > -	if (is_hdmi(sd)) {
> > > -		/* adjust for deep color mode */
> > > -		unsigned bits_per_channel = ((hdmi_read(sd, 0x0b) & 0x60)
> > > > > 4) + 8;
> > > -
> > > -		freq = freq * 8 / bits_per_channel;
> > > -	}
> > > -
> > > -	return freq;
> > > +	return a * 1000000 + ((b & 0x30) >> 4) * 250000;
> > >  }
> > >  
> > >  static unsigned int adv7611_read_hdmi_pixelclock(struct v4l2_subdev
> > > *sd)
> > > @@ -1530,9 +1521,28 @@ static unsigned int
> > > adv7611_read_hdmi_pixelclock(struct v4l2_subdev *sd)
> > >  	b = hdmi_read(sd, 0x52);
> > >  	if (a < 0 || b < 0)
> > >  		return 0;
> > > +
> > >  	return ((a << 1) | (b >> 7)) * 1000000 + (b & 0x7f) * 1000000 /
> > > 128;
> > >  }
> > >  
> > > +static unsigned int adv76xx_read_hdmi_pixelclock(struct v4l2_subdev
> > > *sd)
> > > +{
> > > +	struct adv76xx_state *state = to_state(sd);
> > > +	const struct adv76xx_chip_info *info = state->info;
> > > +	unsigned int freq, bits_per_channel, pixelrepetition;
> > > +
> > > +	freq = info->read_hdmi_pixelclock(sd);
> > > +	if (is_hdmi(sd)) {
> > > +		/* adjust for deep color mode and pixel repetition */
> > > +		bits_per_channel = ((hdmi_read(sd, 0x0b) & 0x60) >> 4) + 8;
> > > +		pixelrepetition = (hdmi_read(sd, 0x05) & 0x0f) + 1;
> > > +
> > > +		freq = freq * 8 / bits_per_channel / pixelrepetition;
> > > +	}
> > > +
> > > +	return freq;
> > > +}
> > > +
> > >  static int adv76xx_query_dv_timings(struct v4l2_subdev *sd,
> > >  			struct v4l2_dv_timings *timings)
> > >  {
> > > @@ -1579,7 +1589,7 @@ static int adv76xx_query_dv_timings(struct
> > > v4l2_subdev *sd,
> > >  
> > >  		bt->width = w;
> > >  		bt->height = h;
> > > -		bt->pixelclock = info->read_hdmi_pixelclock(sd);
> > > +		bt->pixelclock = adv76xx_read_hdmi_pixelclock(sd);
> > >  		bt->hfrontporch = hdmi_read16(sd, 0x20, info-
> > > > hfrontporch_mask);
> > >  		bt->hsync = hdmi_read16(sd, 0x22, info->hsync_mask);
> > >  		bt->hbackporch = hdmi_read16(sd, 0x24, info-
> > > > hbackporch_mask);
  

Patch

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 28a84bf9f8a9..c6fe3aa69733 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1503,23 +1503,14 @@  static void adv76xx_fill_optional_dv_timings_fields(struct v4l2_subdev *sd,
 
 static unsigned int adv7604_read_hdmi_pixelclock(struct v4l2_subdev *sd)
 {
-	unsigned int freq;
 	int a, b;
 
 	a = hdmi_read(sd, 0x06);
 	b = hdmi_read(sd, 0x3b);
 	if (a < 0 || b < 0)
 		return 0;
-	freq =  a * 1000000 + ((b & 0x30) >> 4) * 250000;
 
-	if (is_hdmi(sd)) {
-		/* adjust for deep color mode */
-		unsigned bits_per_channel = ((hdmi_read(sd, 0x0b) & 0x60) >> 4) + 8;
-
-		freq = freq * 8 / bits_per_channel;
-	}
-
-	return freq;
+	return a * 1000000 + ((b & 0x30) >> 4) * 250000;
 }
 
 static unsigned int adv7611_read_hdmi_pixelclock(struct v4l2_subdev *sd)
@@ -1530,9 +1521,28 @@  static unsigned int adv7611_read_hdmi_pixelclock(struct v4l2_subdev *sd)
 	b = hdmi_read(sd, 0x52);
 	if (a < 0 || b < 0)
 		return 0;
+
 	return ((a << 1) | (b >> 7)) * 1000000 + (b & 0x7f) * 1000000 / 128;
 }
 
+static unsigned int adv76xx_read_hdmi_pixelclock(struct v4l2_subdev *sd)
+{
+	struct adv76xx_state *state = to_state(sd);
+	const struct adv76xx_chip_info *info = state->info;
+	unsigned int freq, bits_per_channel, pixelrepetition;
+
+	freq = info->read_hdmi_pixelclock(sd);
+	if (is_hdmi(sd)) {
+		/* adjust for deep color mode and pixel repetition */
+		bits_per_channel = ((hdmi_read(sd, 0x0b) & 0x60) >> 4) + 8;
+		pixelrepetition = (hdmi_read(sd, 0x05) & 0x0f) + 1;
+
+		freq = freq * 8 / bits_per_channel / pixelrepetition;
+	}
+
+	return freq;
+}
+
 static int adv76xx_query_dv_timings(struct v4l2_subdev *sd,
 			struct v4l2_dv_timings *timings)
 {
@@ -1579,7 +1589,7 @@  static int adv76xx_query_dv_timings(struct v4l2_subdev *sd,
 
 		bt->width = w;
 		bt->height = h;
-		bt->pixelclock = info->read_hdmi_pixelclock(sd);
+		bt->pixelclock = adv76xx_read_hdmi_pixelclock(sd);
 		bt->hfrontporch = hdmi_read16(sd, 0x20, info->hfrontporch_mask);
 		bt->hsync = hdmi_read16(sd, 0x22, info->hsync_mask);
 		bt->hbackporch = hdmi_read16(sd, 0x24, info->hbackporch_mask);