Re: [Freedreno] [PATCH v10 3/5] drm/msm/dp: add support for DP PLL driver

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 16/08/2020 01:45, Rob Clark wrote:
> On Sat, Aug 15, 2020 at 2:21 PM Jonathan Marek <jonathan@marek.ca> wrote:
>>
>> On 8/15/20 4:20 PM, Rob Clark wrote:
>>> On Fri, Aug 14, 2020 at 10:05 AM Dmitry Baryshkov
>>> <dmitry.baryshkov@linaro.org> wrote:
>>>>
>>>>
>>>> On 12/08/2020 07:42, Tanmay Shah wrote:
>>>>    > From: Chandan Uddaraju <chandanu@codeaurora.org>
>>>>    >
>>>>    > Add the needed DP PLL specific files to support
>>>>    > display port interface on msm targets.
>>>>
>>>> [skipped]
>>>>
>>>>    > diff --git a/drivers/gpu/drm/msm/dp/dp_pll_private.h
>>>> b/drivers/gpu/drm/msm/dp/dp_pll_private.h
>>>>    > new file mode 100644
>>>>    > index 000000000000..475ba6ed59ab
>>>>    > --- /dev/null
>>>>    > +++ b/drivers/gpu/drm/msm/dp/dp_pll_private.h
>>>>    > @@ -0,0 +1,98 @@
>>>>    > +/* SPDX-License-Identifier: GPL-2.0-only */
>>>>    > +/*
>>>>    > + * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved.
>>>>    > + */
>>>>    > +
>>>>    > +#ifndef __DP_PLL_10NM_H
>>>>    > +#define __DP_PLL_10NM_H
>>>>    > +
>>>>    > +#include "dp_pll.h"
>>>>    > +#include "dp_reg.h"
>>>>    > +
>>>>    > +#define DP_VCO_HSCLK_RATE_1620MHZDIV1000    1620000UL
>>>>    > +#define DP_VCO_HSCLK_RATE_2700MHZDIV1000    2700000UL
>>>>    > +#define DP_VCO_HSCLK_RATE_5400MHZDIV1000    5400000UL
>>>>    > +#define DP_VCO_HSCLK_RATE_8100MHZDIV1000    8100000UL
>>>>    > +
>>>>    > +#define NUM_DP_CLOCKS_MAX            6
>>>>    > +
>>>>    > +#define DP_PHY_PLL_POLL_SLEEP_US        500
>>>>    > +#define DP_PHY_PLL_POLL_TIMEOUT_US        10000
>>>>    > +
>>>>    > +#define DP_VCO_RATE_8100MHZDIV1000        8100000UL
>>>>    > +#define DP_VCO_RATE_9720MHZDIV1000        9720000UL
>>>>    > +#define DP_VCO_RATE_10800MHZDIV1000        10800000UL
>>>>    > +
>>>>    > +struct dp_pll_vco_clk {
>>>>    > +    struct clk_hw hw;
>>>>    > +    unsigned long    rate;        /* current vco rate */
>>>>    > +    u64        min_rate;    /* min vco rate */
>>>>    > +    u64        max_rate;    /* max vco rate */
>>>>    > +    void        *priv;
>>>>    > +};
>>>>    > +
>>>>    > +struct dp_pll_db {
>>>>
>>>> This struct should probably go into dp_pll_10nm.c. dp_pll_7nm.c, for
>>>> example, will use slightly different structure.
>>>
>>> Note that sboyd has a WIP series to move all of the pll code out to a
>>> phy driver.  If there is work already happening on 7nm support, it
>>> might be better to go with the separate phy driver approach?  I'm
>>> still a bit undecided about whether to land the dp code initially with
>>> the pll stuff in drm, and then continue refactoring to move to
>>> separate phy driver upstream, or to strip out the pll code from the
>>> beginning.  If you/someone is working on 7nm support, then feedback
>>> about which approach is easier is welcome.
>>>
>>> https://lore.kernel.org/dri-devel/20200611091919.108018-1-swboyd@chromium.org/
>>>
>>
>> I have a sm8150/sm8250 (7nm) upstream kernel stack with DP enabled, and
>> I have done something similar, with the PLL driver in the QMP phy,
>> although not based on sboyd's series (along with some typec changes to
>> negotiate the DP alt mode and get HPD events, etc.). I don't think
>> having PLL in drm/msm makes sense, the drm/msm DP driver shouldn't need
>> to be aware of the DP PLL/PHY driver, it only needs to set the
>> link/pixel clock rates which are in dispcc (and those then have the PLL
>> clocks as a parent).
> 
> yeah, in the dp case, having phy split out makes a ton of sense.. it
> would maybe be a nice cleanup in other cases (dsi, hdmi) but the
> combination of usb+dp makes burying this in drm not so great..
> 
> It would be good if you could work w/ sboyd on this.. based on what
> I've seen on previous gens, it is probably a different phy driver for
> 7nm vs 10nm, but I think where we want to end up upstream is with phy
> split out of drm.

7nm differs in registers programming, so it would end up with a separate 
set of tables in qmp phy driver. There is also a 14nm version of dp phy, 
but I don't know if it usable in any actual hardware design.


-- 
With best wishes
Dmitry