diff mbox

[4/5] drm/i915: Write AVI infoframes for Parade LSPCON

Message ID 1510672646-29918-5-git-send-email-shashank.sharma@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sharma, Shashank Nov. 14, 2017, 3:17 p.m. UTC
Different LSPCON vendors specify their custom methods to pass
AVI infoframes to the LSPCON chip, so does Parade tech.

This patch adds functions to arrange and write AVI infoframes
into Parade LSPCON chips.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_lspcon.c | 119 +++++++++++++++++++++++++++++++++++-
 1 file changed, 118 insertions(+), 1 deletion(-)

Comments

Maarten Lankhorst Dec. 18, 2017, 7:15 p.m. UTC | #1
Op 14-11-17 om 16:17 schreef Shashank Sharma:
> Different LSPCON vendors specify their custom methods to pass
> AVI infoframes to the LSPCON chip, so does Parade tech.
>
> This patch adds functions to arrange and write AVI infoframes
> into Parade LSPCON chips.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lspcon.c | 119 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 118 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index 1ac4c47..77f0687 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -37,6 +37,12 @@
>  #define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
>  #define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
>  
> +/* AUX addresses to write Parade AVI IF */
> +#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
> +#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
> +#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
> +#define LSPCON_PARADE_AVI_IF_DATA_SIZE 32
> +
>  static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
>  {
>  	struct intel_digital_port *dig_port =
> @@ -241,6 +247,113 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
>  	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
>  }
>  
> +static bool lspcon_parade_fw_ready(struct drm_dp_aux *aux)
> +{
> +	u8 avi_if_ctrl;
> +	u8 retry;
> +	ssize_t ret;
> +
> +	/* Check if LSPCON FW is ready for data */
> +	for (retry = 0; retry < 5; retry++) {
> +
> +		if (retry)
> +			usleep_range(200, 300);
> +
> +		ret = drm_dp_dpcd_read(aux, LSPCON_PARADE_AVI_IF_CTRL,
> +				       &avi_if_ctrl, 1);
> +		if (ret < 0) {
> +			DRM_ERROR("Failed to read AVI IF control\n");
> +			return false;
> +		}
> +
> +		if ((avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF) == 0)
> +			return true;
> +	}
> +
> +	DRM_ERROR("Parade FW not ready to accept AVI IF\n");
> +	return false;
> +}
> +
> +static bool _lspcon_parade_write_infoframe_blocks(struct drm_dp_aux *aux,
> +					       uint8_t *avi_buf)
> +{
> +	u8 avi_if_ctrl;
> +	u8 block_count = 0;
> +	u8 *data;
> +	uint16_t reg;
> +	ssize_t ret;
> +
> +	while (block_count < 4) {
> +
> +		if (!lspcon_parade_fw_ready(aux)) {
> +			DRM_DEBUG_KMS("LSPCON FW not ready, block %d\n",
> +				       block_count);
> +			return false;
> +		}
> +
> +		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
> +		data = avi_buf + block_count * 8;
> +		ret = drm_dp_dpcd_write(aux, reg, data, 8);
> +		if (ret < 0) {
> +			DRM_ERROR("Failed to write AVI IF block %d\n",
> +				   block_count);
> +			return false;
> +		}
> +
> +		/*
> +		 * Once a block of data is written, we have to inform the FW
> +		 * about this by writing into avi infoframe control register:
> +		 * - set the kickoff bit[7] to 1
> +		 * - write the block no. to bits[1:0]
> +		 */
> +		reg = LSPCON_PARADE_AVI_IF_CTRL;
> +		avi_if_ctrl = LSPCON_PARADE_AVI_IF_KICKOFF | block_count;
> +		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
> +		if (ret < 0) {
> +			DRM_ERROR("Failed to update (0x%x), block %d\n",
> +					reg, block_count);
> +			return false;
> +		}
> +
> +		block_count++;
> +	}
> +
> +	DRM_DEBUG_KMS("Wrote AVI IF blocks successfully\n");
> +	return true;
> +}
> +
> +static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
> +					       const uint8_t *frame,
> +					       ssize_t len)
> +{
> +	uint8_t avi_if[LSPCON_PARADE_AVI_IF_DATA_SIZE] = {1, };
> +
> +	/*
> +	 * Parade's frames contains 32 bytes of data, divided
> +	 * into 4 frames:
> +	 *	Token byte (first byte of first frame, must be non-zero)
> +	 *	HB0 to HB2	 from AVI IF (3 bytes header)
> +	 *	PB0 to PB27 from AVI IF (28 bytes data)
> +	 * So it should look like this
> +	 *	first block: | <token> <HB0-HB2> <DB0-DB3> |
> +	 *	next 3 blocks: |<DB4-DB11>|<DB12-DB19>|<DB20-DB28>|
> +	 */
> +
> +	if (len > LSPCON_PARADE_AVI_IF_DATA_SIZE - 1) {
> +		DRM_ERROR("Invalid length of infoframes\n");
> +		return false;
> +	}
> +
> +	memcpy(&avi_if[1], frame, len);
> +
> +	if (!_lspcon_parade_write_infoframe_blocks(aux, avi_if)) {
> +		DRM_DEBUG_KMS("Failed to write infoframe blocks\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
>  					     const uint8_t *buffer, ssize_t len)
>  {
> @@ -295,7 +408,7 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
>  			     enum hdmi_infoframe_type type,
>  			     const void *frame, ssize_t len)
>  {
> -	bool ret = true;
> +	bool ret;
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
>  
> @@ -306,6 +419,10 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
>  	if (lspcon->vendor == LSPCON_VENDOR_MCA)
>  		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
>  						      frame, len);
> +	else
> +		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
> +							 frame, len);
If you make it a switch (lspcon->vendor) and don't add a default case, you will get a compiler warning when you add a new vendor to the enumeration.

With that changed for first 4 patches:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
David Weinehall Dec. 19, 2017, 10:13 a.m. UTC | #2
On Mon, Dec 18, 2017 at 08:15:30PM +0100, Maarten Lankhorst wrote:
> Op 14-11-17 om 16:17 schreef Shashank Sharma:
> > Different LSPCON vendors specify their custom methods to pass
> > AVI infoframes to the LSPCON chip, so does Parade tech.
> >
> > This patch adds functions to arrange and write AVI infoframes
> > into Parade LSPCON chips.
> >
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_lspcon.c | 119 +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 118 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> > index 1ac4c47..77f0687 100644
> > --- a/drivers/gpu/drm/i915/intel_lspcon.c
> > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> > @@ -37,6 +37,12 @@
> >  #define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
> >  #define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
> >  
> > +/* AUX addresses to write Parade AVI IF */
> > +#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
> > +#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
> > +#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
> > +#define LSPCON_PARADE_AVI_IF_DATA_SIZE 32
> > +
> >  static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
> >  {
> >  	struct intel_digital_port *dig_port =
> > @@ -241,6 +247,113 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
> >  	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
> >  }
> >  
> > +static bool lspcon_parade_fw_ready(struct drm_dp_aux *aux)
> > +{
> > +	u8 avi_if_ctrl;
> > +	u8 retry;
> > +	ssize_t ret;
> > +
> > +	/* Check if LSPCON FW is ready for data */
> > +	for (retry = 0; retry < 5; retry++) {
> > +

Remove this newline.

> > +		if (retry)
> > +			usleep_range(200, 300);
> > +
> > +		ret = drm_dp_dpcd_read(aux, LSPCON_PARADE_AVI_IF_CTRL,
> > +				       &avi_if_ctrl, 1);
> > +		if (ret < 0) {
> > +			DRM_ERROR("Failed to read AVI IF control\n");
> > +			return false;
> > +		}
> > +
> > +		if ((avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF) == 0)
> > +			return true;
> > +	}
> > +
> > +	DRM_ERROR("Parade FW not ready to accept AVI IF\n");
> > +	return false;
> > +}
> > +
> > +static bool _lspcon_parade_write_infoframe_blocks(struct drm_dp_aux *aux,
> > +					       uint8_t *avi_buf)
> > +{
> > +	u8 avi_if_ctrl;
> > +	u8 block_count = 0;
> > +	u8 *data;
> > +	uint16_t reg;
> > +	ssize_t ret;
> > +
> > +	while (block_count < 4) {
> > +

And this one.

> > +		if (!lspcon_parade_fw_ready(aux)) {
> > +			DRM_DEBUG_KMS("LSPCON FW not ready, block %d\n",
> > +				       block_count);
> > +			return false;
> > +		}
> > +
> > +		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
> > +		data = avi_buf + block_count * 8;
> > +		ret = drm_dp_dpcd_write(aux, reg, data, 8);
> > +		if (ret < 0) {
> > +			DRM_ERROR("Failed to write AVI IF block %d\n",
> > +				   block_count);
> > +			return false;
> > +		}
> > +
> > +		/*
> > +		 * Once a block of data is written, we have to inform the FW
> > +		 * about this by writing into avi infoframe control register:
> > +		 * - set the kickoff bit[7] to 1
> > +		 * - write the block no. to bits[1:0]
> > +		 */
> > +		reg = LSPCON_PARADE_AVI_IF_CTRL;
> > +		avi_if_ctrl = LSPCON_PARADE_AVI_IF_KICKOFF | block_count;
> > +		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
> > +		if (ret < 0) {
> > +			DRM_ERROR("Failed to update (0x%x), block %d\n",
> > +					reg, block_count);
> > +			return false;
> > +		}
> > +
> > +		block_count++;
> > +	}
> > +
> > +	DRM_DEBUG_KMS("Wrote AVI IF blocks successfully\n");
> > +	return true;
> > +}
> > +
> > +static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
> > +					       const uint8_t *frame,
> > +					       ssize_t len)
> > +{
> > +	uint8_t avi_if[LSPCON_PARADE_AVI_IF_DATA_SIZE] = {1, };
> > +
> > +	/*
> > +	 * Parade's frames contains 32 bytes of data, divided
> > +	 * into 4 frames:
> > +	 *	Token byte (first byte of first frame, must be non-zero)
> > +	 *	HB0 to HB2	 from AVI IF (3 bytes header)
> > +	 *	PB0 to PB27 from AVI IF (28 bytes data)
> > +	 * So it should look like this
> > +	 *	first block: | <token> <HB0-HB2> <DB0-DB3> |
> > +	 *	next 3 blocks: |<DB4-DB11>|<DB12-DB19>|<DB20-DB28>|
> > +	 */
> > +
> > +	if (len > LSPCON_PARADE_AVI_IF_DATA_SIZE - 1) {
> > +		DRM_ERROR("Invalid length of infoframes\n");
> > +		return false;
> > +	}
> > +
> > +	memcpy(&avi_if[1], frame, len);
> > +
> > +	if (!_lspcon_parade_write_infoframe_blocks(aux, avi_if)) {
> > +		DRM_DEBUG_KMS("Failed to write infoframe blocks\n");
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> >  static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
> >  					     const uint8_t *buffer, ssize_t len)
> >  {
> > @@ -295,7 +408,7 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
> >  			     enum hdmi_infoframe_type type,
> >  			     const void *frame, ssize_t len)
> >  {
> > -	bool ret = true;
> > +	bool ret;
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >  	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
> >  
> > @@ -306,6 +419,10 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
> >  	if (lspcon->vendor == LSPCON_VENDOR_MCA)
> >  		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
> >  						      frame, len);
> > +	else
> > +		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
> > +							 frame, len);
> If you make it a switch (lspcon->vendor) and don't add a default case, you will get a compiler warning when you add a new vendor to the enumeration.
> 
> With that changed for first 4 patches:
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Sharma, Shashank Dec. 20, 2017, 4:38 a.m. UTC | #3
Thanks for the review, David.

My comments, inline.


Regards

Shashank


On 12/19/2017 3:43 PM, David Weinehall wrote:
> On Mon, Dec 18, 2017 at 08:15:30PM +0100, Maarten Lankhorst wrote:
>> Op 14-11-17 om 16:17 schreef Shashank Sharma:
>>> Different LSPCON vendors specify their custom methods to pass
>>> AVI infoframes to the LSPCON chip, so does Parade tech.
>>>
>>> This patch adds functions to arrange and write AVI infoframes
>>> into Parade LSPCON chips.
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_lspcon.c | 119 +++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 118 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>>> index 1ac4c47..77f0687 100644
>>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>>> @@ -37,6 +37,12 @@
>>>   #define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
>>>   #define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
>>>   
>>> +/* AUX addresses to write Parade AVI IF */
>>> +#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
>>> +#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
>>> +#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
>>> +#define LSPCON_PARADE_AVI_IF_DATA_SIZE 32
>>> +
>>>   static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
>>>   {
>>>   	struct intel_digital_port *dig_port =
>>> @@ -241,6 +247,113 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
>>>   	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
>>>   }
>>>   
>>> +static bool lspcon_parade_fw_ready(struct drm_dp_aux *aux)
>>> +{
>>> +	u8 avi_if_ctrl;
>>> +	u8 retry;
>>> +	ssize_t ret;
>>> +
>>> +	/* Check if LSPCON FW is ready for data */
>>> +	for (retry = 0; retry < 5; retry++) {
>>> +
> Remove this newline.
Why ? this is not accidental.
>>> +		if (retry)
>>> +			usleep_range(200, 300);
>>> +
>>> +		ret = drm_dp_dpcd_read(aux, LSPCON_PARADE_AVI_IF_CTRL,
>>> +				       &avi_if_ctrl, 1);
>>> +		if (ret < 0) {
>>> +			DRM_ERROR("Failed to read AVI IF control\n");
>>> +			return false;
>>> +		}
>>> +
>>> +		if ((avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF) == 0)
>>> +			return true;
>>> +	}
>>> +
>>> +	DRM_ERROR("Parade FW not ready to accept AVI IF\n");
>>> +	return false;
>>> +}
>>> +
>>> +static bool _lspcon_parade_write_infoframe_blocks(struct drm_dp_aux *aux,
>>> +					       uint8_t *avi_buf)
>>> +{
>>> +	u8 avi_if_ctrl;
>>> +	u8 block_count = 0;
>>> +	u8 *data;
>>> +	uint16_t reg;
>>> +	ssize_t ret;
>>> +
>>> +	while (block_count < 4) {
>>> +
> And this one.
Same as above.
- Shashank
>
>>> +		if (!lspcon_parade_fw_ready(aux)) {
>>> +			DRM_DEBUG_KMS("LSPCON FW not ready, block %d\n",
>>> +				       block_count);
>>> +			return false;
>>> +		}
>>> +
>>> +		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
>>> +		data = avi_buf + block_count * 8;
>>> +		ret = drm_dp_dpcd_write(aux, reg, data, 8);
>>> +		if (ret < 0) {
>>> +			DRM_ERROR("Failed to write AVI IF block %d\n",
>>> +				   block_count);
>>> +			return false;
>>> +		}
>>> +
>>> +		/*
>>> +		 * Once a block of data is written, we have to inform the FW
>>> +		 * about this by writing into avi infoframe control register:
>>> +		 * - set the kickoff bit[7] to 1
>>> +		 * - write the block no. to bits[1:0]
>>> +		 */
>>> +		reg = LSPCON_PARADE_AVI_IF_CTRL;
>>> +		avi_if_ctrl = LSPCON_PARADE_AVI_IF_KICKOFF | block_count;
>>> +		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
>>> +		if (ret < 0) {
>>> +			DRM_ERROR("Failed to update (0x%x), block %d\n",
>>> +					reg, block_count);
>>> +			return false;
>>> +		}
>>> +
>>> +		block_count++;
>>> +	}
>>> +
>>> +	DRM_DEBUG_KMS("Wrote AVI IF blocks successfully\n");
>>> +	return true;
>>> +}
>>> +
>>> +static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
>>> +					       const uint8_t *frame,
>>> +					       ssize_t len)
>>> +{
>>> +	uint8_t avi_if[LSPCON_PARADE_AVI_IF_DATA_SIZE] = {1, };
>>> +
>>> +	/*
>>> +	 * Parade's frames contains 32 bytes of data, divided
>>> +	 * into 4 frames:
>>> +	 *	Token byte (first byte of first frame, must be non-zero)
>>> +	 *	HB0 to HB2	 from AVI IF (3 bytes header)
>>> +	 *	PB0 to PB27 from AVI IF (28 bytes data)
>>> +	 * So it should look like this
>>> +	 *	first block: | <token> <HB0-HB2> <DB0-DB3> |
>>> +	 *	next 3 blocks: |<DB4-DB11>|<DB12-DB19>|<DB20-DB28>|
>>> +	 */
>>> +
>>> +	if (len > LSPCON_PARADE_AVI_IF_DATA_SIZE - 1) {
>>> +		DRM_ERROR("Invalid length of infoframes\n");
>>> +		return false;
>>> +	}
>>> +
>>> +	memcpy(&avi_if[1], frame, len);
>>> +
>>> +	if (!_lspcon_parade_write_infoframe_blocks(aux, avi_if)) {
>>> +		DRM_DEBUG_KMS("Failed to write infoframe blocks\n");
>>> +		return false;
>>> +	}
>>> +
>>> +	return true;
>>> +}
>>> +
>>>   static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
>>>   					     const uint8_t *buffer, ssize_t len)
>>>   {
>>> @@ -295,7 +408,7 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
>>>   			     enum hdmi_infoframe_type type,
>>>   			     const void *frame, ssize_t len)
>>>   {
>>> -	bool ret = true;
>>> +	bool ret;
>>>   	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>>>   	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
>>>   
>>> @@ -306,6 +419,10 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
>>>   	if (lspcon->vendor == LSPCON_VENDOR_MCA)
>>>   		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
>>>   						      frame, len);
>>> +	else
>>> +		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
>>> +							 frame, len);
>> If you make it a switch (lspcon->vendor) and don't add a default case, you will get a compiler warning when you add a new vendor to the enumeration.
>>
>> With that changed for first 4 patches:
>>
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
David Weinehall Dec. 20, 2017, 10:36 a.m. UTC | #4
On Wed, Dec 20, 2017 at 10:08:53AM +0530, Sharma, Shashank wrote:
> Thanks for the review, David.
> 
> My comments, inline.
> 
> 
> Regards
> 
> Shashank
> 
> 
> On 12/19/2017 3:43 PM, David Weinehall wrote:
> > On Mon, Dec 18, 2017 at 08:15:30PM +0100, Maarten Lankhorst wrote:
> > > Op 14-11-17 om 16:17 schreef Shashank Sharma:
> > > > Different LSPCON vendors specify their custom methods to pass
> > > > AVI infoframes to the LSPCON chip, so does Parade tech.
> > > > 
> > > > This patch adds functions to arrange and write AVI infoframes
> > > > into Parade LSPCON chips.
> > > > 
> > > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/intel_lspcon.c | 119 +++++++++++++++++++++++++++++++++++-
> > > >   1 file changed, 118 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> > > > index 1ac4c47..77f0687 100644
> > > > --- a/drivers/gpu/drm/i915/intel_lspcon.c
> > > > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> > > > @@ -37,6 +37,12 @@
> > > >   #define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
> > > >   #define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
> > > > +/* AUX addresses to write Parade AVI IF */
> > > > +#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
> > > > +#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
> > > > +#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
> > > > +#define LSPCON_PARADE_AVI_IF_DATA_SIZE 32
> > > > +
> > > >   static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
> > > >   {
> > > >   	struct intel_digital_port *dig_port =
> > > > @@ -241,6 +247,113 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
> > > >   	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
> > > >   }
> > > > +static bool lspcon_parade_fw_ready(struct drm_dp_aux *aux)
> > > > +{
> > > > +	u8 avi_if_ctrl;
> > > > +	u8 retry;
> > > > +	ssize_t ret;
> > > > +
> > > > +	/* Check if LSPCON FW is ready for data */
> > > > +	for (retry = 0; retry < 5; retry++) {
> > > > +
> > Remove this newline.
> Why ? this is not accidental.

Because accidental or not you're not allowed to impose personal coding
style on the driver. Follow the example of the rest of the driver;
we don't insert leading blanklines inside for-blocks (or other blocks,
for that matter).

> > > > +		if (retry)
> > > > +			usleep_range(200, 300);
> > > > +
> > > > +		ret = drm_dp_dpcd_read(aux, LSPCON_PARADE_AVI_IF_CTRL,
> > > > +				       &avi_if_ctrl, 1);
> > > > +		if (ret < 0) {
> > > > +			DRM_ERROR("Failed to read AVI IF control\n");
> > > > +			return false;
> > > > +		}
> > > > +
> > > > +		if ((avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF) == 0)
> > > > +			return true;
> > > > +	}
> > > > +
> > > > +	DRM_ERROR("Parade FW not ready to accept AVI IF\n");
> > > > +	return false;
> > > > +}
> > > > +
> > > > +static bool _lspcon_parade_write_infoframe_blocks(struct drm_dp_aux *aux,
> > > > +					       uint8_t *avi_buf)
> > > > +{
> > > > +	u8 avi_if_ctrl;
> > > > +	u8 block_count = 0;
> > > > +	u8 *data;
> > > > +	uint16_t reg;
> > > > +	ssize_t ret;
> > > > +
> > > > +	while (block_count < 4) {
> > > > +
> > And this one.
> Same as above.
> - Shashank
> > 
> > > > +		if (!lspcon_parade_fw_ready(aux)) {
> > > > +			DRM_DEBUG_KMS("LSPCON FW not ready, block %d\n",
> > > > +				       block_count);
> > > > +			return false;
> > > > +		}
> > > > +
> > > > +		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
> > > > +		data = avi_buf + block_count * 8;
> > > > +		ret = drm_dp_dpcd_write(aux, reg, data, 8);
> > > > +		if (ret < 0) {
> > > > +			DRM_ERROR("Failed to write AVI IF block %d\n",
> > > > +				   block_count);
> > > > +			return false;
> > > > +		}
> > > > +
> > > > +		/*
> > > > +		 * Once a block of data is written, we have to inform the FW
> > > > +		 * about this by writing into avi infoframe control register:
> > > > +		 * - set the kickoff bit[7] to 1
> > > > +		 * - write the block no. to bits[1:0]
> > > > +		 */
> > > > +		reg = LSPCON_PARADE_AVI_IF_CTRL;
> > > > +		avi_if_ctrl = LSPCON_PARADE_AVI_IF_KICKOFF | block_count;
> > > > +		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
> > > > +		if (ret < 0) {
> > > > +			DRM_ERROR("Failed to update (0x%x), block %d\n",
> > > > +					reg, block_count);
> > > > +			return false;
> > > > +		}
> > > > +
> > > > +		block_count++;
> > > > +	}
> > > > +
> > > > +	DRM_DEBUG_KMS("Wrote AVI IF blocks successfully\n");
> > > > +	return true;
> > > > +}
> > > > +
> > > > +static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
> > > > +					       const uint8_t *frame,
> > > > +					       ssize_t len)
> > > > +{
> > > > +	uint8_t avi_if[LSPCON_PARADE_AVI_IF_DATA_SIZE] = {1, };
> > > > +
> > > > +	/*
> > > > +	 * Parade's frames contains 32 bytes of data, divided
> > > > +	 * into 4 frames:
> > > > +	 *	Token byte (first byte of first frame, must be non-zero)
> > > > +	 *	HB0 to HB2	 from AVI IF (3 bytes header)
> > > > +	 *	PB0 to PB27 from AVI IF (28 bytes data)
> > > > +	 * So it should look like this
> > > > +	 *	first block: | <token> <HB0-HB2> <DB0-DB3> |
> > > > +	 *	next 3 blocks: |<DB4-DB11>|<DB12-DB19>|<DB20-DB28>|
> > > > +	 */
> > > > +
> > > > +	if (len > LSPCON_PARADE_AVI_IF_DATA_SIZE - 1) {
> > > > +		DRM_ERROR("Invalid length of infoframes\n");
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	memcpy(&avi_if[1], frame, len);
> > > > +
> > > > +	if (!_lspcon_parade_write_infoframe_blocks(aux, avi_if)) {
> > > > +		DRM_DEBUG_KMS("Failed to write infoframe blocks\n");
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	return true;
> > > > +}
> > > > +
> > > >   static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
> > > >   					     const uint8_t *buffer, ssize_t len)
> > > >   {
> > > > @@ -295,7 +408,7 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
> > > >   			     enum hdmi_infoframe_type type,
> > > >   			     const void *frame, ssize_t len)
> > > >   {
> > > > -	bool ret = true;
> > > > +	bool ret;
> > > >   	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > >   	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
> > > > @@ -306,6 +419,10 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
> > > >   	if (lspcon->vendor == LSPCON_VENDOR_MCA)
> > > >   		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
> > > >   						      frame, len);
> > > > +	else
> > > > +		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
> > > > +							 frame, len);
> > > If you make it a switch (lspcon->vendor) and don't add a default case, you will get a compiler warning when you add a new vendor to the enumeration.
> > > 
> > > With that changed for first 4 patches:
> > > 
> > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 1ac4c47..77f0687 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -37,6 +37,12 @@ 
 #define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
 #define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
 
+/* AUX addresses to write Parade AVI IF */
+#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
+#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
+#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
+#define LSPCON_PARADE_AVI_IF_DATA_SIZE 32
+
 static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
 {
 	struct intel_digital_port *dig_port =
@@ -241,6 +247,113 @@  static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
 	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
 }
 
+static bool lspcon_parade_fw_ready(struct drm_dp_aux *aux)
+{
+	u8 avi_if_ctrl;
+	u8 retry;
+	ssize_t ret;
+
+	/* Check if LSPCON FW is ready for data */
+	for (retry = 0; retry < 5; retry++) {
+
+		if (retry)
+			usleep_range(200, 300);
+
+		ret = drm_dp_dpcd_read(aux, LSPCON_PARADE_AVI_IF_CTRL,
+				       &avi_if_ctrl, 1);
+		if (ret < 0) {
+			DRM_ERROR("Failed to read AVI IF control\n");
+			return false;
+		}
+
+		if ((avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF) == 0)
+			return true;
+	}
+
+	DRM_ERROR("Parade FW not ready to accept AVI IF\n");
+	return false;
+}
+
+static bool _lspcon_parade_write_infoframe_blocks(struct drm_dp_aux *aux,
+					       uint8_t *avi_buf)
+{
+	u8 avi_if_ctrl;
+	u8 block_count = 0;
+	u8 *data;
+	uint16_t reg;
+	ssize_t ret;
+
+	while (block_count < 4) {
+
+		if (!lspcon_parade_fw_ready(aux)) {
+			DRM_DEBUG_KMS("LSPCON FW not ready, block %d\n",
+				       block_count);
+			return false;
+		}
+
+		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
+		data = avi_buf + block_count * 8;
+		ret = drm_dp_dpcd_write(aux, reg, data, 8);
+		if (ret < 0) {
+			DRM_ERROR("Failed to write AVI IF block %d\n",
+				   block_count);
+			return false;
+		}
+
+		/*
+		 * Once a block of data is written, we have to inform the FW
+		 * about this by writing into avi infoframe control register:
+		 * - set the kickoff bit[7] to 1
+		 * - write the block no. to bits[1:0]
+		 */
+		reg = LSPCON_PARADE_AVI_IF_CTRL;
+		avi_if_ctrl = LSPCON_PARADE_AVI_IF_KICKOFF | block_count;
+		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
+		if (ret < 0) {
+			DRM_ERROR("Failed to update (0x%x), block %d\n",
+					reg, block_count);
+			return false;
+		}
+
+		block_count++;
+	}
+
+	DRM_DEBUG_KMS("Wrote AVI IF blocks successfully\n");
+	return true;
+}
+
+static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
+					       const uint8_t *frame,
+					       ssize_t len)
+{
+	uint8_t avi_if[LSPCON_PARADE_AVI_IF_DATA_SIZE] = {1, };
+
+	/*
+	 * Parade's frames contains 32 bytes of data, divided
+	 * into 4 frames:
+	 *	Token byte (first byte of first frame, must be non-zero)
+	 *	HB0 to HB2	 from AVI IF (3 bytes header)
+	 *	PB0 to PB27 from AVI IF (28 bytes data)
+	 * So it should look like this
+	 *	first block: | <token> <HB0-HB2> <DB0-DB3> |
+	 *	next 3 blocks: |<DB4-DB11>|<DB12-DB19>|<DB20-DB28>|
+	 */
+
+	if (len > LSPCON_PARADE_AVI_IF_DATA_SIZE - 1) {
+		DRM_ERROR("Invalid length of infoframes\n");
+		return false;
+	}
+
+	memcpy(&avi_if[1], frame, len);
+
+	if (!_lspcon_parade_write_infoframe_blocks(aux, avi_if)) {
+		DRM_DEBUG_KMS("Failed to write infoframe blocks\n");
+		return false;
+	}
+
+	return true;
+}
+
 static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
 					     const uint8_t *buffer, ssize_t len)
 {
@@ -295,7 +408,7 @@  void lspcon_write_infoframe(struct drm_encoder *encoder,
 			     enum hdmi_infoframe_type type,
 			     const void *frame, ssize_t len)
 {
-	bool ret = true;
+	bool ret;
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
 
@@ -306,6 +419,10 @@  void lspcon_write_infoframe(struct drm_encoder *encoder,
 	if (lspcon->vendor == LSPCON_VENDOR_MCA)
 		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
 						      frame, len);
+	else
+		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
+							 frame, len);
+
 	if (!ret) {
 		DRM_ERROR("Failed to write AVI infoframes\n");
 		return;