[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d67ffcd6-b10c-92a4-55a7-40521b3be68f@linaro.org>
Date: Mon, 17 Aug 2020 23:32:37 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Rob Clark <robdclark@...il.com>, Jonathan Marek <jonathan@...ek.ca>
Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>, David Airlie <airlied@...ux.ie>,
linux-arm-msm <linux-arm-msm@...r.kernel.org>,
Abhinav Kumar <abhinavk@...eaurora.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
dri-devel <dri-devel@...ts.freedesktop.org>,
Stephen Boyd <swboyd@...omium.org>, khsieh@...eaurora.org,
Sean Paul <seanpaul@...omium.org>,
Tanmay Shah <tanmay@...eaurora.org>,
Daniel Vetter <daniel@...ll.ch>,
Vara Reddy <varar@...eaurora.org>, aravindh@...eaurora.org,
freedreno <freedreno@...ts.freedesktop.org>,
Chandan Uddaraju <chandanu@...eaurora.org>
Subject: Re: [Freedreno] [PATCH v10 3/5] drm/msm/dp: add support for DP PLL
driver
On 16/08/2020 01:45, Rob Clark wrote:
> On Sat, Aug 15, 2020 at 2:21 PM Jonathan Marek <jonathan@...ek.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@...aro.org> wrote:
>>>>
>>>>
>>>> On 12/08/2020 07:42, Tanmay Shah wrote:
>>>> > From: Chandan Uddaraju <chandanu@...eaurora.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
Powered by blists - more mailing lists