soundwire: intel: fix PDI/stream mapping for Bulk
diff mbox series

Message ID 20191022232948.17156-1-pierre-louis.bossart@linux.intel.com
State New
Headers show
Series
  • soundwire: intel: fix PDI/stream mapping for Bulk
Related show

Commit Message

Pierre-Louis Bossart Oct. 22, 2019, 11:29 p.m. UTC
The previous formula is incorrect for PDI0/1, the mapping is not
linear but has a discontinuity between PDI1 and PDI2.

This change has no effect on PCM PDIs (same mapping).

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Vinod Koul Oct. 24, 2019, 11:23 a.m. UTC | #1
On 22-10-19, 18:29, Pierre-Louis Bossart wrote:
> The previous formula is incorrect for PDI0/1, the mapping is not
> linear but has a discontinuity between PDI1 and PDI2.
> 
> This change has no effect on PCM PDIs (same mapping).
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/soundwire/intel.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index b403ccc832b6..c984261fcc33 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -480,7 +480,10 @@ intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
>  	unsigned int link_id = sdw->instance;
>  	int pdi_conf = 0;
>  
> -	pdi->intel_alh_id = (link_id * 16) + pdi->num + 5;
> +	/* the Bulk and PCM streams are not contiguous */
> +	pdi->intel_alh_id = (link_id * 16) + pdi->num + 3;
> +	if (pdi->num >= 2)
> +		pdi->intel_alh_id += 2;
>  
>  	/*
>  	 * Program stream parameters to stream SHIM register
> @@ -509,7 +512,10 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
>  	unsigned int link_id = sdw->instance;
>  	unsigned int conf;
>  
> -	pdi->intel_alh_id = (link_id * 16) + pdi->num + 5;
> +	/* the Bulk and PCM streams are not contiguous */
> +	pdi->intel_alh_id = (link_id * 16) + pdi->num + 3;
> +	if (pdi->num >= 2)
> +		pdi->intel_alh_id += 2;

The change is repeated so how about:

        intel_pdi_update_alh() or similar which does this rather than
repeat the pattern

>  
>  	/* Program Stream config ALH register */
>  	conf = intel_readl(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id));
> -- 
> 2.20.1
Pierre-Louis Bossart Oct. 24, 2019, 12:37 p.m. UTC | #2
On 10/24/19 6:23 AM, Vinod Koul wrote:
> On 22-10-19, 18:29, Pierre-Louis Bossart wrote:
>> The previous formula is incorrect for PDI0/1, the mapping is not
>> linear but has a discontinuity between PDI1 and PDI2.
>>
>> This change has no effect on PCM PDIs (same mapping).
>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> ---
>>   drivers/soundwire/intel.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
>> index b403ccc832b6..c984261fcc33 100644
>> --- a/drivers/soundwire/intel.c
>> +++ b/drivers/soundwire/intel.c
>> @@ -480,7 +480,10 @@ intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
>>   	unsigned int link_id = sdw->instance;
>>   	int pdi_conf = 0;
>>   
>> -	pdi->intel_alh_id = (link_id * 16) + pdi->num + 5;
>> +	/* the Bulk and PCM streams are not contiguous */
>> +	pdi->intel_alh_id = (link_id * 16) + pdi->num + 3;
>> +	if (pdi->num >= 2)
>> +		pdi->intel_alh_id += 2;
>>   
>>   	/*
>>   	 * Program stream parameters to stream SHIM register
>> @@ -509,7 +512,10 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
>>   	unsigned int link_id = sdw->instance;
>>   	unsigned int conf;
>>   
>> -	pdi->intel_alh_id = (link_id * 16) + pdi->num + 5;
>> +	/* the Bulk and PCM streams are not contiguous */
>> +	pdi->intel_alh_id = (link_id * 16) + pdi->num + 3;
>> +	if (pdi->num >= 2)
>> +		pdi->intel_alh_id += 2;
> 
> The change is repeated so how about:
> 
>          intel_pdi_update_alh() or similar which does this rather than
> repeat the pattern

The initial code was also repeated by the initial contributors, this 
patch does not refactor the code but corrects an invalid mapping. We 
will do this refactoring at a later point, when we add the clock-stop mode.

> 
>>   
>>   	/* Program Stream config ALH register */
>>   	conf = intel_readl(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id));
>> -- 
>> 2.20.1
>
Vinod Koul Nov. 9, 2019, 11:15 a.m. UTC | #3
On 22-10-19, 18:29, Pierre-Louis Bossart wrote:
> The previous formula is incorrect for PDI0/1, the mapping is not
> linear but has a discontinuity between PDI1 and PDI2.
> 
> This change has no effect on PCM PDIs (same mapping).

Applied now

Patch
diff mbox series

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index b403ccc832b6..c984261fcc33 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -480,7 +480,10 @@  intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
 	unsigned int link_id = sdw->instance;
 	int pdi_conf = 0;
 
-	pdi->intel_alh_id = (link_id * 16) + pdi->num + 5;
+	/* the Bulk and PCM streams are not contiguous */
+	pdi->intel_alh_id = (link_id * 16) + pdi->num + 3;
+	if (pdi->num >= 2)
+		pdi->intel_alh_id += 2;
 
 	/*
 	 * Program stream parameters to stream SHIM register
@@ -509,7 +512,10 @@  intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
 	unsigned int link_id = sdw->instance;
 	unsigned int conf;
 
-	pdi->intel_alh_id = (link_id * 16) + pdi->num + 5;
+	/* the Bulk and PCM streams are not contiguous */
+	pdi->intel_alh_id = (link_id * 16) + pdi->num + 3;
+	if (pdi->num >= 2)
+		pdi->intel_alh_id += 2;
 
 	/* Program Stream config ALH register */
 	conf = intel_readl(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id));