On 11-03-20, 17:10, Pierre-Louis Bossart wrote:
From: Rander Wang rander.wang@intel.com
Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register
Is the documentation available public space so that we can correct
programming sequences are correct.
This includes making sure SHIM_SYNC is programmed only once, before the first link is powered on.
Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate.
Signed-off-by: Rander Wang rander.wang@intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
drivers/soundwire/intel.c | 186 ++++++++++++++++++++++++++++---------- 1 file changed, 139 insertions(+), 47 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 3c271a8044b8..9c6514fe1284 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -46,7 +46,8 @@ #define SDW_SHIM_LCTL_SPA BIT(0) #define SDW_SHIM_LCTL_CPA BIT(8)
-#define SDW_SHIM_SYNC_SYNCPRD_VAL 0x176F +#define SDW_SHIM_SYNC_SYNCPRD_VAL_24 (24000 / SDW_CADENCE_GSYNC_KHZ - 1) +#define SDW_SHIM_SYNC_SYNCPRD_VAL_38_4 (38400 / SDW_CADENCE_GSYNC_KHZ - 1) #define SDW_SHIM_SYNC_SYNCPRD GENMASK(14, 0) #define SDW_SHIM_SYNC_SYNCCPU BIT(15) #define SDW_SHIM_SYNC_CMDSYNC_MASK GENMASK(19, 16) @@ -283,11 +284,48 @@ static int intel_link_power_up(struct sdw_intel *sdw) { unsigned int link_id = sdw->instance; void __iomem *shim = sdw->link_res->shim;
- u32 *shim_mask = sdw->link_res->shim_mask;
this is a local pointer, so the one defined previously is not used.
- struct sdw_bus *bus = &sdw->cdns.bus;
- struct sdw_master_prop *prop = &bus->prop; int spa_mask, cpa_mask;
- int link_control, ret;
int link_control;
int ret = 0;
u32 syncprd;
u32 sync_reg;
mutex_lock(sdw->link_res->shim_lock);
/*
* The hardware relies on an internal counter,* typically 4kHz, to generate the SoundWire SSP -* which defines a 'safe' synchronization point* between commands and audio transport and allows for* multi link synchronization. The SYNCPRD value is* only dependent on the oscillator clock provided to* the IP, so adjust based on _DSD properties reported* in DSDT tables. The values reported are based on* either 24MHz (CNL/CML) or 38.4 MHz (ICL/TGL+).
Sorry this looks quite bad to read, we have 80 chars, so please use like below:
/* * The hardware relies on an internal counter, typically 4kHz, * to generate the SoundWire SSP - which defines a 'safe' * synchronization point between commands and audio transport * and allows for multi link synchronization. The SYNCPRD value * is only dependent on the oscillator clock provided to * the IP, so adjust based on _DSD properties reported in DSDT * tables. The values reported are based on either 24MHz * (CNL/CML) or 38.4 MHz (ICL/TGL+). */