On Fri, Sep 15, 2017 at 06:10:20PM +0530, Subhransu S. Prusty wrote:
> On Fri, Sep 08, 2017 at 08:41:54AM -0500, Pierre-Louis Bossart wrote:
> > On 9/8/17 12:01 AM, Subhransu S. Prusty wrote:
> > >On Fri, Sep 08, 2017 at 09:01:36AM +0530, Subhransu S. Prusty wrote:
> > >>On Thu, Sep 07, 2017 at 08:48:38PM -0500, Pierre-Louis Bossart wrote:
> > >>>
> > >>>
> > >>>On 09/07/2017 09:29 AM, Subhransu S. Prusty wrote:
> > >>>>From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@xxxxxxxxx>
> > >>>>
> > >>>>Create a platform device and register the clock ops. Clock
> > >>>>prepare/unprepare are used to enable/disable the clock as the IPC will be
> > >>>>sent in non-atomic context. The clk set_dma_control IPC structures are
> > >>>>populated during the set_rate callback and IPC is sent to enable the clock
> > >>>>during prepare callback.
> > >>>>
> > >>>[snip]
> > >>>>+
> > >>>>+static int skl_clk_prepare(void *pvt_data, u32 id, unsigned long rate)
> > >>>>+{
> > >>>>+ struct skl *skl = pvt_data;
> > >>>>+ struct skl_clk_rate_cfg_table *rcfg;
> > >>>>+ int vbus_id, clk_type, ret;
> > >>>>+
> > >>>>+ clk_type = skl_get_clk_type(id);
> > >>>>+ if (clk_type < 0)
> > >>>>+ return -EINVAL;
> > >>>>+
> > >>>>+ ret = skl_get_vbus_id(id, clk_type);
> > >>>>+ if (ret < 0)
> > >>>>+ return ret;
> > >>>>+
> > >>>>+ vbus_id = ret;
> > >>>>+
> > >>>>+ rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, rate);
> > >>>>+ if (!rcfg)
> > >>>>+ return -EINVAL;
> > >>>>+
> > >>>>+ ret = skl_send_clk_dma_control(skl, rcfg, vbus_id, clk_type, true);
> > >>>>+
> > >>>>+ return ret;
> > >>>>+}
> > >>>In this patchset, the clocks are configured from the machine driver,
> > >>>and the enable/disable conveniently placed in
> > >>>platform_clock_control() or hw_params(), where the DSP is most
> > >>>likely active.
> > >>>If you expose a clock, codec driver implementers may want to use
> > >>>them directly instead of relying on a machine driver. A number of
> > >>>existing codecs do use the clk API, so there could be a case where a
> > >>>codec driver calls devm_clk_get and clk_prepare_enable(), without
> > >>>any ability to know what state the DSP is in.
> > >>>What happens then if the DSP is in suspend? Does this force it back
> > >>>to D0? Does the virtual clock driver return an error? Or are you
> > >>>using the clk API with some restrictions on when the clock can be
> > >>>configured?
> > >>
> > >>No, clk enable will not force the DSP to D0. So if the DSP is not active,
> > >>the IPC will timeout and error will be propagated to the caller.
> > >
> > >Or may be it makes sense to enable the runtime pm for clk driver so that it
> > >can activate the DSP. I will check this.
> >
> > I was thinking of another case: we should not make the assumption
> > that there is always a platform clock control and a hw_params
> > callback, e.g. when an external component seen as a dummy codec
> > needs the mclk/bitclock at all times to drive a second-level set of
> > audio devices. In those cases the machine driver will get/enable the
> > clock at startup and it needs to remain on no matter what the DSP
> > state is. That's probably another case for disabling runtime-pm for
> > as long as the machine driver wants the clock.
>
> With the series "[PATCH v9 0/5] Add runtime PM support for clocks (on Exynos
> SoC example)", runtime support is added in the common clock framework. This
> is expected to be merged to clk-next after -rc1 drop.
>
> Reference: http://www.spinics.net/lists/linux-clk/msg19755.html
>
> So marking the parent clock with skylake device will help keep the DSP
> active on call to enable clock.
Will fix this in v2.
>
> Regards,
> Subhransu
>
> --
--
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
