Re: [Freedreno] [PATCH v9 3/5] drm/msm/dp: add support for DP PLL driver
On 2020-08-11 13:21, Randy Dunlap wrote: On 8/11/20 12:49 PM, tan...@codeaurora.org wrote: On 2020-08-07 13:28, Randy Dunlap wrote: On 8/7/20 1:24 PM, Stephen Boyd wrote: Quoting Rob Clark (2020-08-07 08:51:48) On Fri, Aug 7, 2020 at 8:27 AM Randy Dunlap wrote: On 8/7/20 12:17 AM, Tanmay Shah wrote: diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index ea3c4d094d09..cc1392b29022 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -60,6 +60,7 @@ config DRM_MSM_HDMI_HDCP config DRM_MSM_DP bool "Enable DP support in MSM DRM driver" depends on DRM_MSM + default y help Compile in support for DP driver in msm drm driver. DP external display support is enabled through this config option. It can Hi, You need a very strong justification to make an optional part of a driver to be "default y". My opinion is that if the driver is built, everything should be built. This is what makes sense for distro's. It is only the embedded case where you want to trim down unneeded features where you might want to disable some parts. So 'default y' makes sense to me. We don't set defaults for distro convenience. Maybe use 'default DRM_MSM' so that it doesn't trigger the 'default y' filters people have? Most people can figure that one out. ;) I don't have any automated filters. After after further reviews, I agree with Rob. Display Port is required module as of now so it makes sense to keep 'default y'. If it is required, then you don't need to have a Kconfig entry/symbol for it. Kconfig makes driver flexible. Other moudles in the driver are also 'default y' such as DSI. I will let Rob guide us further on this as he is the maintainer. ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v9 3/5] drm/msm/dp: add support for DP PLL driver
On 8/11/20 12:49 PM, tan...@codeaurora.org wrote: > On 2020-08-07 13:28, Randy Dunlap wrote: >> On 8/7/20 1:24 PM, Stephen Boyd wrote: >>> Quoting Rob Clark (2020-08-07 08:51:48) On Fri, Aug 7, 2020 at 8:27 AM Randy Dunlap wrote: > > On 8/7/20 12:17 AM, Tanmay Shah wrote: >> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig >> index ea3c4d094d09..cc1392b29022 100644 >> --- a/drivers/gpu/drm/msm/Kconfig >> +++ b/drivers/gpu/drm/msm/Kconfig >> @@ -60,6 +60,7 @@ config DRM_MSM_HDMI_HDCP >> config DRM_MSM_DP >> bool "Enable DP support in MSM DRM driver" >> depends on DRM_MSM >> + default y >> help >> Compile in support for DP driver in msm drm driver. DP external >> display support is enabled through this config option. It can > > Hi, > > You need a very strong justification to make an optional part of a > driver > to be "default y". My opinion is that if the driver is built, everything should be built. This is what makes sense for distro's. It is only the embedded case where you want to trim down unneeded features where you might want to disable some parts. So 'default y' makes sense to me. >> >> We don't set defaults for distro convenience. >> >>> >>> Maybe use 'default DRM_MSM' so that it doesn't trigger the 'default y' >>> filters people have? >> >> Most people can figure that one out. ;) >> I don't have any automated filters. > > After after further reviews, I agree with Rob. Display Port is required > module as of now so it makes sense to keep 'default y'. If it is required, then you don't need to have a Kconfig entry/symbol for it. -- ~Randy ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v9 3/5] drm/msm/dp: add support for DP PLL driver
On 2020-08-07 13:28, Randy Dunlap wrote: On 8/7/20 1:24 PM, Stephen Boyd wrote: Quoting Rob Clark (2020-08-07 08:51:48) On Fri, Aug 7, 2020 at 8:27 AM Randy Dunlap wrote: On 8/7/20 12:17 AM, Tanmay Shah wrote: diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index ea3c4d094d09..cc1392b29022 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -60,6 +60,7 @@ config DRM_MSM_HDMI_HDCP config DRM_MSM_DP bool "Enable DP support in MSM DRM driver" depends on DRM_MSM + default y help Compile in support for DP driver in msm drm driver. DP external display support is enabled through this config option. It can Hi, You need a very strong justification to make an optional part of a driver to be "default y". My opinion is that if the driver is built, everything should be built. This is what makes sense for distro's. It is only the embedded case where you want to trim down unneeded features where you might want to disable some parts. So 'default y' makes sense to me. We don't set defaults for distro convenience. Maybe use 'default DRM_MSM' so that it doesn't trigger the 'default y' filters people have? Most people can figure that one out. ;) I don't have any automated filters. After after further reviews, I agree with Rob. Display Port is required module as of now so it makes sense to keep 'default y'. ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v9 3/5] drm/msm/dp: add support for DP PLL driver
On 8/7/20 1:24 PM, Stephen Boyd wrote: > Quoting Rob Clark (2020-08-07 08:51:48) >> On Fri, Aug 7, 2020 at 8:27 AM Randy Dunlap wrote: >>> >>> On 8/7/20 12:17 AM, Tanmay Shah wrote: diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index ea3c4d094d09..cc1392b29022 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -60,6 +60,7 @@ config DRM_MSM_HDMI_HDCP config DRM_MSM_DP bool "Enable DP support in MSM DRM driver" depends on DRM_MSM + default y help Compile in support for DP driver in msm drm driver. DP external display support is enabled through this config option. It can >>> >>> Hi, >>> >>> You need a very strong justification to make an optional part of a driver >>> to be "default y". >> >> My opinion is that if the driver is built, everything should be built. >> This is what makes sense for distro's. It is only the embedded case >> where you want to trim down unneeded features where you might want to >> disable some parts. So 'default y' makes sense to me. We don't set defaults for distro convenience. > > Maybe use 'default DRM_MSM' so that it doesn't trigger the 'default y' > filters people have? Most people can figure that one out. ;) I don't have any automated filters. -- ~Randy ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v9 3/5] drm/msm/dp: add support for DP PLL driver
Quoting Rob Clark (2020-08-07 08:51:48) > On Fri, Aug 7, 2020 at 8:27 AM Randy Dunlap wrote: > > > > On 8/7/20 12:17 AM, Tanmay Shah wrote: > > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > > > index ea3c4d094d09..cc1392b29022 100644 > > > --- a/drivers/gpu/drm/msm/Kconfig > > > +++ b/drivers/gpu/drm/msm/Kconfig > > > @@ -60,6 +60,7 @@ config DRM_MSM_HDMI_HDCP > > > config DRM_MSM_DP > > > bool "Enable DP support in MSM DRM driver" > > > depends on DRM_MSM > > > + default y > > > help > > > Compile in support for DP driver in msm drm driver. DP external > > > display support is enabled through this config option. It can > > > > Hi, > > > > You need a very strong justification to make an optional part of a driver > > to be "default y". > > My opinion is that if the driver is built, everything should be built. > This is what makes sense for distro's. It is only the embedded case > where you want to trim down unneeded features where you might want to > disable some parts. So 'default y' makes sense to me. > Maybe use 'default DRM_MSM' so that it doesn't trigger the 'default y' filters people have? ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v9 3/5] drm/msm/dp: add support for DP PLL driver
On 2020-08-07 08:27, Randy Dunlap wrote: On 8/7/20 12:17 AM, Tanmay Shah wrote: diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index ea3c4d094d09..cc1392b29022 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -60,6 +60,7 @@ config DRM_MSM_HDMI_HDCP config DRM_MSM_DP bool "Enable DP support in MSM DRM driver" depends on DRM_MSM + default y help Compile in support for DP driver in msm drm driver. DP external display support is enabled through this config option. It can Hi, You need a very strong justification to make an optional part of a driver to be "default y". so why? Thanks Randy for reviews. "default y" doesn't belong there. Thanks for pointing that. It will be fixed in next patch. thanks. ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v9 3/5] drm/msm/dp: add support for DP PLL driver
On Fri, Aug 7, 2020 at 8:27 AM Randy Dunlap wrote: > > On 8/7/20 12:17 AM, Tanmay Shah wrote: > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > > index ea3c4d094d09..cc1392b29022 100644 > > --- a/drivers/gpu/drm/msm/Kconfig > > +++ b/drivers/gpu/drm/msm/Kconfig > > @@ -60,6 +60,7 @@ config DRM_MSM_HDMI_HDCP > > config DRM_MSM_DP > > bool "Enable DP support in MSM DRM driver" > > depends on DRM_MSM > > + default y > > help > > Compile in support for DP driver in msm drm driver. DP external > > display support is enabled through this config option. It can > > Hi, > > You need a very strong justification to make an optional part of a driver > to be "default y". My opinion is that if the driver is built, everything should be built. This is what makes sense for distro's. It is only the embedded case where you want to trim down unneeded features where you might want to disable some parts. So 'default y' makes sense to me. BR, -R > so why? > > thanks. > -- > ~Randy > ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v9 3/5] drm/msm/dp: add support for DP PLL driver
On 8/7/20 12:17 AM, Tanmay Shah wrote: > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > index ea3c4d094d09..cc1392b29022 100644 > --- a/drivers/gpu/drm/msm/Kconfig > +++ b/drivers/gpu/drm/msm/Kconfig > @@ -60,6 +60,7 @@ config DRM_MSM_HDMI_HDCP > config DRM_MSM_DP > bool "Enable DP support in MSM DRM driver" > depends on DRM_MSM > + default y > help > Compile in support for DP driver in msm drm driver. DP external > display support is enabled through this config option. It can Hi, You need a very strong justification to make an optional part of a driver to be "default y". so why? thanks. -- ~Randy ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v9 3/5] drm/msm/dp: add support for DP PLL driver
From: Chandan Uddaraju
Add the needed DP PLL specific files to support
display port interface on msm targets.
The DP driver calls the DP PLL driver registration.
The DP driver sets the link and pixel clock sources.
Changes in v2:
-- Update copyright markings on all relevant files.
-- Use DRM_DEBUG_DP for debug msgs.
Changes in v4:
-- Update the DP link clock provider names
Changes in V5:
-- Addressed comments from Stephen Boyd, Rob clark.
Changes in V6:
-- Remove PLL as separate driver and include PLL as DP module
-- Remove redundant clock parsing from PLL module and make DP as
clock provider
-- Map USB3 DPCOM and PHY IO using hardcoded register address and
move mapping form parser to PLL module
-- Access DP PHY modules from same base address using offsets instead of
deriving base address of individual module from device tree.
-- Remove dp_pll_10nm_util.c and include its functionality in
dp_pll_10nm.c
-- Introduce new data structures private to PLL module
Changes in v7:
-- Remove DRM_MSM_DP_PLL config from Makefile and Kconfig
-- Remove set_parent from determin_rate API
-- Remove phy_pll_vco_div_clk from parent list
-- Remove flag CLK_DIVIDER_ONE_BASED
-- Remove redundant cell-index property parsing
Changes in v8:
-- Unregister hardware clocks during driver cleanup
Changes in v9:
-- Remove redundant Kconfig option DRM_MSM_DP_10NM_PLL
Signed-off-by: Chandan Uddaraju
Signed-off-by: Vara Reddy
Signed-off-by: Tanmay Shah
---
drivers/gpu/drm/msm/Kconfig | 1 +
drivers/gpu/drm/msm/Makefile| 4 +-
drivers/gpu/drm/msm/dp/dp_catalog.c | 31 +-
drivers/gpu/drm/msm/dp/dp_display.c | 18 +-
drivers/gpu/drm/msm/dp/dp_display.h | 3 +
drivers/gpu/drm/msm/dp/dp_parser.c | 2 +
drivers/gpu/drm/msm/dp/dp_parser.h | 7 +-
drivers/gpu/drm/msm/dp/dp_pll.c | 99 +++
drivers/gpu/drm/msm/dp/dp_pll.h | 61 ++
drivers/gpu/drm/msm/dp/dp_pll_10nm.c| 917
drivers/gpu/drm/msm/dp/dp_pll_private.h | 98 +++
drivers/gpu/drm/msm/dp/dp_power.c | 10 +
drivers/gpu/drm/msm/dp/dp_power.h | 40 +-
drivers/gpu/drm/msm/dp/dp_reg.h | 16 +
14 files changed, 1290 insertions(+), 17 deletions(-)
create mode 100644 drivers/gpu/drm/msm/dp/dp_pll.c
create mode 100644 drivers/gpu/drm/msm/dp/dp_pll.h
create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_10nm.c
create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_private.h
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index ea3c4d094d09..cc1392b29022 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -60,6 +60,7 @@ config DRM_MSM_HDMI_HDCP
config DRM_MSM_DP
bool "Enable DP support in MSM DRM driver"
depends on DRM_MSM
+ default y
help
Compile in support for DP driver in msm drm driver. DP external
display support is enabled through this config option. It can
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index af868e791210..6d31188cc776 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -109,7 +109,9 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
dp/dp_link.o \
dp/dp_panel.o \
dp/dp_parser.o \
- dp/dp_power.o
+ dp/dp_power.o \
+ dp/dp_pll.o \
+ dp/dp_pll_10nm.o
msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
msm-$(CONFIG_COMMON_CLK) += disp/mdp4/mdp4_lvds_pll.o
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 497f97f86c82..e506e0756e92 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -5,6 +5,7 @@
#define pr_fmt(fmt)"[drm-dp] %s: " fmt, __func__
+#include
#include
#include
#include
@@ -131,51 +132,58 @@ static inline void dp_write_ahb(struct dp_catalog_private
*catalog,
static inline void dp_write_phy(struct dp_catalog_private *catalog,
u32 offset, u32 data)
{
+ offset += DP_PHY_REG_OFFSET;
/*
* To make sure phy reg writes happens before any other operation,
* this function uses writel() instread of writel_relaxed()
*/
- writel(data, catalog->io->phy_io.base + offset);
+ writel(data, catalog->io->phy_reg.base + offset);
}
static inline u32 dp_read_phy(struct dp_catalog_private *catalog,
u32 offset)
{
+ offset += DP_PHY_REG_OFFSET;
/*
* To make sure phy reg writes happens before any other operation,
* this function uses writel() instread of writel_relaxed()
*/
- return readl_relaxed(catalog->io->phy_io.base + offset);
+ return readl_relaxed(catalog->io->phy_reg.base + offset);
}
static inline void dp_write_pll(struct dp_catalog_private *catalog,
u32 offset, u32 data)
{
- writel_relaxed(data, catalog->io->dp_pll_io.base + offset);
+
