lkml.org 
[lkml]   [2019]   [Aug]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [alsa-devel] [RFC PATCH 25/40] soundwire: intel: use BIOS information to set clock dividers
    From
    Date


    On 8/5/19 5:28 AM, Sanyog Kale wrote:
    > On Thu, Jul 25, 2019 at 06:40:17PM -0500, Pierre-Louis Bossart wrote:
    >> The BIOS provides an Intel-specific property, let's use it to avoid
    >> hard-coded clock dividers.
    >>
    >> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
    >> ---
    >> drivers/soundwire/cadence_master.c | 26 ++++++++++++++++++++++----
    >> drivers/soundwire/intel.c | 26 ++++++++++++++++++++++++++
    >> include/linux/soundwire/sdw.h | 2 ++
    >> 3 files changed, 50 insertions(+), 4 deletions(-)
    >>
    >> diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
    >> index d84344e29f71..10ebcef2e84e 100644
    >> --- a/drivers/soundwire/cadence_master.c
    >> +++ b/drivers/soundwire/cadence_master.c
    >> @@ -173,8 +173,6 @@
    >> #define CDNS_PDI_CONFIG_PORT GENMASK(4, 0)
    >>
    >> /* Driver defaults */
    >> -
    >> -#define CDNS_DEFAULT_CLK_DIVIDER 0
    >> #define CDNS_DEFAULT_SSP_INTERVAL 0x18
    >> #define CDNS_TX_TIMEOUT 2000
    >>
    >> @@ -973,7 +971,10 @@ static u32 cdns_set_default_frame_shape(int n_rows, int n_cols)
    >> */
    >> int sdw_cdns_init(struct sdw_cdns *cdns)
    >> {
    >> + struct sdw_bus *bus = &cdns->bus;
    >> + struct sdw_master_prop *prop = &bus->prop;
    >> u32 val;
    >> + int divider;
    >> int ret;
    >>
    >> /* Exit clock stop */
    >> @@ -985,9 +986,17 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
    >> }
    >>
    >> /* Set clock divider */
    >> + divider = (prop->mclk_freq / prop->max_clk_freq) - 1;
    >
    > Do you expect mclk_freq and max_clk_freq to be same?

    Nope. For Icelake the MCLK is 38.4 MHz and is, but the max_clk needs to
    be 9.6 max (can't be higher per the SoundWire spec).

    The max_clk_freq may even be lower thank 9.6 MHz due to specific
    topologies where the higher frequencies are problematic if the trace
    lengths are too long.

    For CNL/CML the MCLK is 24 MHz but ironically we run the bus at 12 MHz,
    so the divider is smaller.

    >
    >> val = cdns_readl(cdns, CDNS_MCP_CLK_CTRL0);
    >> - val |= CDNS_DEFAULT_CLK_DIVIDER;
    >> + val |= divider;
    >> cdns_writel(cdns, CDNS_MCP_CLK_CTRL0, val);
    >> + cdns_writel(cdns, CDNS_MCP_CLK_CTRL1, val);
    >> +
    >> + pr_err("plb: mclk %d max_freq %d divider %d register %x\n",
    >> + prop->mclk_freq,
    >> + prop->max_clk_freq,
    >> + divider,
    >> + val);
    >
    > This can be removed.

    yes, done already.

    >
    >>
    >> /* Set the default frame shape */
    >> val = cdns_set_default_frame_shape(prop->default_row,
    >> @@ -1035,6 +1044,7 @@ EXPORT_SYMBOL(sdw_cdns_init);
    >>
    >> int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params)
    >> {
    >> + struct sdw_master_prop *prop = &bus->prop;
    >> struct sdw_cdns *cdns = bus_to_cdns(bus);
    >> int mcp_clkctrl_off, mcp_clkctrl;
    >> int divider;
    >> @@ -1044,7 +1054,9 @@ int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params)
    >> return -EINVAL;
    >> }
    >>
    >> - divider = (params->max_dr_freq / params->curr_dr_freq) - 1;
    >> + divider = prop->mclk_freq * SDW_DOUBLE_RATE_FACTOR /
    >
    > What is the reason for not using max_dr_freq? Its precomputed as
    > prop->max_clk_freq * SDW_DOUBLE_RATE_FACTOR;

    no, as explained above the divider needs to start from the clock
    provided to the IP, which is different from the max frequency clock the
    bus operates at. the MCLK is a fixed value for all platforms using the
    same SOC/PCH, the max_clk is platform-dependent and its value is
    provided by the firmware (BIOS/DT).

    \
     
     \ /
      Last update: 2019-08-05 17:40    [W:2.155 / U:0.140 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site