[PATCH 0/2] soundwire: set frame shape and divider based on actual clk freq

[PATCH 0/2] soundwire: set frame shape and divider based on actual clk freq

[Bard Liao]

Set frame shape and clock divider based on actual clock frequency. And
with that change, we can support ckock change in Intel platforms.

Bard Liao (2):
  soundwire: cadence_master: set frame shape and divider based on actual
    clk freq
  Revert "soundwire: intel_auxdevice: start the bus at default
    frequency"

 drivers/soundwire/cadence_master.c  | 24 +++++++++++++++++++++---
 drivers/soundwire/intel_auxdevice.c | 21 ---------------------
 2 files changed, 21 insertions(+), 24 deletions(-)

-- 
2.43.0

[PATCH 1/2] soundwire: cadence_master: set frame shape and divider based on actual clk freq

[Bard Liao]

Frame shape and curr_dr_freq could be updated by sdw_compute_bus_params().
Peripherals will set curr_dr_freq as their frequency. Managers
should do the same. Then update frame shape according to the actual
bus frequency.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 drivers/soundwire/cadence_master.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index f367670ea991..a49d4e521a8f 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -1341,13 +1341,14 @@ static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
 	return val;
 }
 
-static void cdns_init_clock_ctrl(struct sdw_cdns *cdns)
+static int cdns_init_clock_ctrl(struct sdw_cdns *cdns)
 {
 	struct sdw_bus *bus = &cdns->bus;
 	struct sdw_master_prop *prop = &bus->prop;
 	u32 val;
 	u32 ssp_interval;
 	int divider;
+	int freq;
 
 	dev_dbg(cdns->dev, "mclk %d max %d row %d col %d\n",
 		prop->mclk_freq,
@@ -1356,13 +1357,25 @@ static void cdns_init_clock_ctrl(struct sdw_cdns *cdns)
 		prop->default_col);
 
 	/* Set clock divider */
-	divider	= (prop->mclk_freq / prop->max_clk_freq) - 1;
+	divider	= (prop->mclk_freq * SDW_DOUBLE_RATE_FACTOR /
+		bus->params.curr_dr_freq) - 1;
+	freq = bus->params.curr_dr_freq >> 1;
 
 	cdns_updatel(cdns, CDNS_MCP_CLK_CTRL0,
 		     CDNS_MCP_CLK_MCLKD_MASK, divider);
 	cdns_updatel(cdns, CDNS_MCP_CLK_CTRL1,
 		     CDNS_MCP_CLK_MCLKD_MASK, divider);
 
+	/* Set frame shape base on the actual bus frequency. */
+	if (!prop->default_frame_rate || !prop->default_row) {
+		dev_err(cdns->dev, "Default frame_rate %d or row %d is invalid\n",
+			prop->default_frame_rate, prop->default_row);
+		return -EINVAL;
+	}
+
+	prop->default_col = freq * SDW_DOUBLE_RATE_FACTOR /
+			    prop->default_frame_rate / prop->default_row;
+
 	/*
 	 * Frame shape changes after initialization have to be done
 	 * with the bank switch mechanism
@@ -1375,6 +1388,8 @@ static void cdns_init_clock_ctrl(struct sdw_cdns *cdns)
 	ssp_interval = prop->default_frame_rate / SDW_CADENCE_GSYNC_HZ;
 	cdns_writel(cdns, CDNS_MCP_SSP_CTRL0, ssp_interval);
 	cdns_writel(cdns, CDNS_MCP_SSP_CTRL1, ssp_interval);
+
+	return 0;
 }
 
 /**
@@ -1408,9 +1423,12 @@ EXPORT_SYMBOL(sdw_cdns_soft_reset);
  */
 int sdw_cdns_init(struct sdw_cdns *cdns)
 {
+	int ret;
 	u32 val;
 
-	cdns_init_clock_ctrl(cdns);
+	ret = cdns_init_clock_ctrl(cdns);
+	if (ret)
+		return ret;
 
 	sdw_cdns_check_self_clearing_bits(cdns, __func__, false, 0);
 
-- 
2.43.0

Re: [PATCH 1/2] soundwire: cadence_master: set frame shape and divider based on actual clk freq

[Pierre-Louis Bossart]

> -static void cdns_init_clock_ctrl(struct sdw_cdns *cdns)
> +static int cdns_init_clock_ctrl(struct sdw_cdns *cdns)
>  {
>  	struct sdw_bus *bus = &cdns->bus;
>  	struct sdw_master_prop *prop = &bus->prop;
>  	u32 val;
>  	u32 ssp_interval;
>  	int divider;
> +	int freq;
>  
>  	dev_dbg(cdns->dev, "mclk %d max %d row %d col %d\n",
>  		prop->mclk_freq,
> @@ -1356,13 +1357,25 @@ static void cdns_init_clock_ctrl(struct sdw_cdns *cdns)
>  		prop->default_col);
>  
>  	/* Set clock divider */
> -	divider	= (prop->mclk_freq / prop->max_clk_freq) - 1;
> +	divider	= (prop->mclk_freq * SDW_DOUBLE_RATE_FACTOR /
> +		bus->params.curr_dr_freq) - 1;
> +	freq = bus->params.curr_dr_freq >> 1;

do you actually need this intermediate variable? see below [1] ...

>  
>  	cdns_updatel(cdns, CDNS_MCP_CLK_CTRL0,
>  		     CDNS_MCP_CLK_MCLKD_MASK, divider);
>  	cdns_updatel(cdns, CDNS_MCP_CLK_CTRL1,
>  		     CDNS_MCP_CLK_MCLKD_MASK, divider);
>  
> +	/* Set frame shape base on the actual bus frequency. */
> +	if (!prop->default_frame_rate || !prop->default_row) {
> +		dev_err(cdns->dev, "Default frame_rate %d or row %d is invalid\n",
> +			prop->default_frame_rate, prop->default_row);
> +		return -EINVAL;
> +	}

maybe check the values before writing the divider registers?

> +
> +	prop->default_col = freq * SDW_DOUBLE_RATE_FACTOR /
> +			    prop->default_frame_rate / prop->default_row;

[1] ... this is the only place where 'freq' is used, and you multiply it by two after dividing it by two.

couldn't this just be:

prop->default_col = bus->params.curr_dr_freq /
		    prop->default_frame_rate / prop->default_row;

[PATCH 2/2] Revert "soundwire: intel_auxdevice: start the bus at default frequency"

[Bard Liao]

Now, we can support more than 1 soundwire bus clock frequency.

This reverts commit c326356188f1dc2d7a2c55b30dac6a8b76087bc6.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 drivers/soundwire/intel_auxdevice.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
index 599954d92752..dee126f6d9d5 100644
--- a/drivers/soundwire/intel_auxdevice.c
+++ b/drivers/soundwire/intel_auxdevice.c
@@ -222,30 +222,9 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
 
 static int intel_prop_read(struct sdw_bus *bus)
 {
-	struct sdw_master_prop *prop;
-
 	/* Initialize with default handler to read all DisCo properties */
 	sdw_master_read_prop(bus);
 
-	/*
-	 * Only one bus frequency is supported so far, filter
-	 * frequencies reported in the DSDT
-	 */
-	prop = &bus->prop;
-	if (prop->clk_freq && prop->num_clk_freq > 1) {
-		unsigned int default_bus_frequency;
-
-		default_bus_frequency =
-			prop->default_frame_rate *
-			prop->default_row *
-			prop->default_col /
-			SDW_DOUBLE_RATE_FACTOR;
-
-		prop->num_clk_freq = 1;
-		prop->clk_freq[0] = default_bus_frequency;
-		prop->max_clk_freq = default_bus_frequency;
-	}
-
 	/* read Intel-specific properties */
 	sdw_master_read_intel_prop(bus);
 
-- 
2.43.0

Re: [PATCH 2/2] Revert "soundwire: intel_auxdevice: start the bus at default frequency"

[Pierre-Louis Bossart]

On 1/6/25 7:58 PM, Bard Liao wrote:
> Now, we can support more than 1 soundwire bus clock frequency.
> 
> This reverts commit c326356188f1dc2d7a2c55b30dac6a8b76087bc6.

maybe clarify that this has no impact on "legacy" devices prior to LunarLake, where only one frequency is listed in the _DSD properties.

 
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
> ---
>  drivers/soundwire/intel_auxdevice.c | 21 ---------------------
>  1 file changed, 21 deletions(-)
> 
> diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
> index 599954d92752..dee126f6d9d5 100644
> --- a/drivers/soundwire/intel_auxdevice.c
> +++ b/drivers/soundwire/intel_auxdevice.c
> @@ -222,30 +222,9 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
>  
>  static int intel_prop_read(struct sdw_bus *bus)
>  {
> -	struct sdw_master_prop *prop;
> -
>  	/* Initialize with default handler to read all DisCo properties */
>  	sdw_master_read_prop(bus);
>  
> -	/*
> -	 * Only one bus frequency is supported so far, filter
> -	 * frequencies reported in the DSDT
> -	 */
> -	prop = &bus->prop;
> -	if (prop->clk_freq && prop->num_clk_freq > 1) {
> -		unsigned int default_bus_frequency;
> -
> -		default_bus_frequency =
> -			prop->default_frame_rate *
> -			prop->default_row *
> -			prop->default_col /
> -			SDW_DOUBLE_RATE_FACTOR;
> -
> -		prop->num_clk_freq = 1;
> -		prop->clk_freq[0] = default_bus_frequency;
> -		prop->max_clk_freq = default_bus_frequency;
> -	}
> -
>  	/* read Intel-specific properties */
>  	sdw_master_read_intel_prop(bus);
>  

[PATCH 0/2] soundwire: Intel: support more then 1 sdw bus clock frequency

[Bard Liao]

This series set frame shape and divider based on actual clk freq to
support dynamic clock change. The revert the commit that limits Intel
platform to use fixed clock frequency.

Bard Liao (2):
  soundwire: cadence_master: set frame shape and divider based on actual
    clk freq
  Revert "soundwire: intel_auxdevice: start the bus at default
    frequency"

 drivers/soundwire/cadence_master.c  | 22 +++++++++++++++++++---
 drivers/soundwire/intel_auxdevice.c | 21 ---------------------
 2 files changed, 19 insertions(+), 24 deletions(-)

-- 
2.43.0

[PATCH 2/2] Revert "soundwire: intel_auxdevice: start the bus at default frequency"

[Bard Liao]

Now, we can support more than 1 soundwire bus clock frequency.

This reverts commit c326356188f1dc2d7a2c55b30dac6a8b76087bc6.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 drivers/soundwire/intel_auxdevice.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
index b34b897400fc..5ea6399e6c9b 100644
--- a/drivers/soundwire/intel_auxdevice.c
+++ b/drivers/soundwire/intel_auxdevice.c
@@ -243,30 +243,9 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
 
 static int intel_prop_read(struct sdw_bus *bus)
 {
-	struct sdw_master_prop *prop;
-
 	/* Initialize with default handler to read all DisCo properties */
 	sdw_master_read_prop(bus);
 
-	/*
-	 * Only one bus frequency is supported so far, filter
-	 * frequencies reported in the DSDT
-	 */
-	prop = &bus->prop;
-	if (prop->clk_freq && prop->num_clk_freq > 1) {
-		unsigned int default_bus_frequency;
-
-		default_bus_frequency =
-			prop->default_frame_rate *
-			prop->default_row *
-			prop->default_col /
-			SDW_DOUBLE_RATE_FACTOR;
-
-		prop->num_clk_freq = 1;
-		prop->clk_freq[0] = default_bus_frequency;
-		prop->max_clk_freq = default_bus_frequency;
-	}
-
 	/* read Intel-specific properties */
 	sdw_master_read_intel_prop(bus);
 
-- 
2.43.0

Re: [PATCH 2/2] Revert "soundwire: intel_auxdevice: start the bus at default frequency"

[Vinod Koul]

On 05-02-25, 15:42, Bard Liao wrote:
> Now, we can support more than 1 soundwire bus clock frequency.
> 
> This reverts commit c326356188f1dc2d7a2c55b30dac6a8b76087bc6.

Pls use full patch title and not just the sha

> 
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
> ---
>  drivers/soundwire/intel_auxdevice.c | 21 ---------------------
>  1 file changed, 21 deletions(-)
> 
> diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
> index b34b897400fc..5ea6399e6c9b 100644
> --- a/drivers/soundwire/intel_auxdevice.c
> +++ b/drivers/soundwire/intel_auxdevice.c
> @@ -243,30 +243,9 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
>  
>  static int intel_prop_read(struct sdw_bus *bus)
>  {
> -	struct sdw_master_prop *prop;
> -
>  	/* Initialize with default handler to read all DisCo properties */
>  	sdw_master_read_prop(bus);
>  
> -	/*
> -	 * Only one bus frequency is supported so far, filter
> -	 * frequencies reported in the DSDT
> -	 */
> -	prop = &bus->prop;
> -	if (prop->clk_freq && prop->num_clk_freq > 1) {
> -		unsigned int default_bus_frequency;
> -
> -		default_bus_frequency =
> -			prop->default_frame_rate *
> -			prop->default_row *
> -			prop->default_col /
> -			SDW_DOUBLE_RATE_FACTOR;
> -
> -		prop->num_clk_freq = 1;
> -		prop->clk_freq[0] = default_bus_frequency;
> -		prop->max_clk_freq = default_bus_frequency;
> -	}
> -
>  	/* read Intel-specific properties */
>  	sdw_master_read_intel_prop(bus);
>  
> -- 
> 2.43.0

-- 
~Vinod