Re: [PATCH v8 01/14] drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags
On Sun, Nov 13, 2022 at 8:29 AM Marek Vasut wrote: > > On 11/11/22 13:12, Nicolas Boichat wrote: > > [...] > > >>> BTW, are you sure DSIM_HSE_MODE is correct now? > >> > >> Yes, we have tested in imx8m platforms as well. Sébastien Szymanski > >> initially observed this issue on the imx8m platform. > > > > I'll repeat, are you sure about HSE specifically? You invert the > > polarity for HBP, HFP, and HSA, which makes sense given your patch > > 02/14. > > > > I'm concerned about HSE. Is the bit really a disable bit? > > MIPI_DSI_MODE_VIDEO_HSE is supposed to be an enable flag, so you > > should not do `reg |= DSIM_HSE_DISABLE;`, probably. > > I suspect the HSE bit is a misnomer, but its handling in the driver is > correct. > > i.MX 8M Plus Applications Processor Reference Manual, Rev. 1, 06/2021 > Page 5436 > > 23 HseDisableMode > > In Vsync pulse and Vporch area, MIPI DSI master transfers only Hsync > start packet to MIPI DSI slave at MIPI DSI spec 1.1r02. This bit > transfers Hsync end packet in Vsync pulse and Vporch area (optional). > > 0 = Disables transfer > 1 = Enables transfer > > In command mode, this bit is ignored. Okay. I'd suggest adding a comment in the code, it'd be so tempting to attempt to "fix" this as the if/or pattern looks different from the others. But it's up to you all. Thanks,
Re: [PATCH v8 01/14] drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags
On Fri, Nov 11, 2022 at 4:49 PM Jagan Teki wrote: > > On Fri, Nov 11, 2022 at 6:19 AM Nicolas Boichat wrote: > > > > On Fri, Nov 11, 2022 at 2:40 AM Jagan Teki > > wrote: > > > > > > HFP/HBP/HSA/EOT_PACKET modes in Exynos DSI host specifies > > > 0 = Enable and 1 = Disable. > > > > Oh I see, that's confusing... IMHO you might want to change the > > register macro name... (but if that's what the datasheet uses, it > > might not be ideal either). At the _very_ least, I'd add a comment in > > the code so the next person doesn't attempt to "fix" it again... > > 02/14 on the same series doing the name change. > https://lore.kernel.org/all/20221110183853.3678209-3-ja...@amarulasolutions.com/ Oh thanks I wasn't cc'ed on that one, makes sense. You can add my reviewed tag to this one, as my HSE comment doesn't change this. Reviewed-by: Nicolas Boichat But please double check HSE. > > > > > BTW, are you sure DSIM_HSE_MODE is correct now? > > Yes, we have tested in imx8m platforms as well. Sébastien Szymanski > initially observed this issue on the imx8m platform. I'll repeat, are you sure about HSE specifically? You invert the polarity for HBP, HFP, and HSA, which makes sense given your patch 02/14. I'm concerned about HSE. Is the bit really a disable bit? MIPI_DSI_MODE_VIDEO_HSE is supposed to be an enable flag, so you should not do `reg |= DSIM_HSE_DISABLE;`, probably. Thanks, > > Jagan.
Re: [PATCH v8 01/14] drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags
On Fri, Nov 11, 2022 at 2:40 AM Jagan Teki wrote: > > HFP/HBP/HSA/EOT_PACKET modes in Exynos DSI host specifies > 0 = Enable and 1 = Disable. Oh I see, that's confusing... IMHO you might want to change the register macro name... (but if that's what the datasheet uses, it might not be ideal either). At the _very_ least, I'd add a comment in the code so the next person doesn't attempt to "fix" it again... BTW, are you sure DSIM_HSE_MODE is correct now? > > The logic for checking these mode flags was correct before > the MIPI_DSI*_NO_* mode flag conversion. > > Fix the MIPI_DSI*_NO_* mode flags handling. > > Fixes: 0f3b68b66a6d ("drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling > features") > Cc: Nicolas Boichat > Reported-by: Sébastien Szymanski > Signed-off-by: Jagan Teki > --- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > index ec673223d6b7..b5305b145ddb 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > @@ -805,15 +805,15 @@ static int exynos_dsi_init_link(struct exynos_dsi *dsi) > reg |= DSIM_AUTO_MODE; > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_HSE) > reg |= DSIM_HSE_MODE; > - if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP)) > + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP) > reg |= DSIM_HFP_MODE; > - if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP)) > + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP) > reg |= DSIM_HBP_MODE; > - if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA)) > + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA) > reg |= DSIM_HSA_MODE; > } > > - if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) > + if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) > reg |= DSIM_EOT_DISABLE; > > switch (dsi->format) { > -- > 2.25.1 >
Re: [PATCH v3] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Tue, Jul 27, 2021 at 4:35 PM Sam Ravnborg wrote: > > Hi Nicolas, > On Tue, Jul 27, 2021 at 09:45:21AM +0800, Nicolas Boichat wrote: > > Many of the DSI flags have names opposite to their actual effects, > > e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually > > be disabled. Fix this by including _NO_ in the flag names, e.g. > > MIPI_DSI_MODE_NO_EOT_PACKET. > > > > Signed-off-by: Nicolas Boichat > > Reviewed-by: Linus Walleij > > Reviewed-by: Robert Foss > > Reviewed-by: Laurent Pinchart > > Reviewed-by: Andrzej Hajda > > Reviewed-by: Xin Ji # anx7625.c > > Reviewed-by: Abhinav Kumar # msm/dsi > > --- > > I considered adding _DISABLE_ instead, but that'd make the > > flag names a big too long. > > > > Generated with: > > flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {} > > flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {} > > flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {} > > flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {} > > (then minor format changes) > > > > Changes in v3: > > - Added all R-b tags from v1 and v2 (hopefully didn't miss any). > > > > Changes in v2: > > - Rebased on latest linux-next, after some of the flags got fixed > >(Linus Walleij). > > Thanks for the update, applied to drm-misc-next. Thanks Sam! > > Sam
[PATCH v3] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
Many of the DSI flags have names opposite to their actual effects, e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually be disabled. Fix this by including _NO_ in the flag names, e.g. MIPI_DSI_MODE_NO_EOT_PACKET. Signed-off-by: Nicolas Boichat Reviewed-by: Linus Walleij Reviewed-by: Robert Foss Reviewed-by: Laurent Pinchart Reviewed-by: Andrzej Hajda Reviewed-by: Xin Ji # anx7625.c Reviewed-by: Abhinav Kumar # msm/dsi --- I considered adding _DISABLE_ instead, but that'd make the flag names a big too long. Generated with: flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {} flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {} flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {} flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {} (then minor format changes) Changes in v3: - Added all R-b tags from v1 and v2 (hopefully didn't miss any). Changes in v2: - Rebased on latest linux-next, after some of the flags got fixed (Linus Walleij). drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +- drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +- drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++-- drivers/gpu/drm/bridge/lontium-lt8912b.c | 2 +- drivers/gpu/drm/bridge/tc358768.c| 2 +- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++-- drivers/gpu/drm/msm/dsi/dsi_host.c | 8 drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +- drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +- drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +- drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +- drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +- drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +- drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +- drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +- drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++-- drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +- drivers/gpu/drm/panel/panel-simple.c | 2 +- drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +- include/drm/drm_mipi_dsi.h | 8 22 files changed, 34 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c index aa19d5a40e31..59d718bde8c4 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c @@ -165,7 +165,7 @@ int adv7533_attach_dsi(struct adv7511 *adv) dsi->lanes = adv->num_dsi_lanes; dsi->format = MIPI_DSI_FMT_RGB888; dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | - MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; + MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; ret = mipi_dsi_attach(dsi); if (ret < 0) { diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index a3d82377066b..be987c836891 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1307,7 +1307,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx) dsi->format = MIPI_DSI_FMT_RGB888; dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | - MIPI_DSI_MODE_EOT_PACKET| + MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; if (mipi_dsi_attach(dsi) < 0) { diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c index b31281f76117..e6e331071a00 100644 --- a/drivers/gpu/drm/bridge/cdns-dsi.c +++ b/drivers/gpu/drm/bridge/cdns-dsi.c @@ -829,7 +829,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) tmp = DIV_ROUND_UP(dsi_cfg.htotal, nlanes) - DIV_ROUND_UP(dsi_cfg.hsa, nlanes); - if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET)) + if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) tmp -= DIV_ROUND_UP(DSI_EOT_PKT_SIZE, nlanes); tx_byte_period = DIV_ROUND_DOWN_ULL((u64)NSEC_PER_SEC * 8, @@ -902,7 +902,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) tmp = readl(dsi->regs + MCTL_MAIN_DATA_CTL); tmp &= ~(IF_VID_SELECT_MASK | HOST_EOT_GEN | IF_VID_MODE); - if (!
Re: [PATCH v2] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Sun, Jul 25, 2021 at 9:31 PM Sam Ravnborg wrote: > > On Tue, Jun 29, 2021 at 07:47:21AM +0800, Nicolas Boichat wrote: > > Many of the DSI flags have names opposite to their actual effects, > > e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually > > be disabled. Fix this by including _NO_ in the flag names, e.g. > > MIPI_DSI_MODE_NO_EOT_PACKET. > > > > Signed-off-by: Nicolas Boichat > > Hi Nicolas, > > in this thread: > https://lore.kernel.org/dri-devel/2021023309.1.I629b2366a6591410359c7fcf6d385b474b705ca2@changeid/ > I see that several people added their Reviewed-by. > > Please either add the tgas if missing, or elaborate why you left them out. Oh simple, I just forgot. I regenerated the patch so it's a bit different from v1... Not 100% sure if I can add those, since those were for the overall patch: Reviewed-by: Robert Foss Reviewed-by: Laurent Pinchart Reviewed-by: Andrzej Hajda . Those 2 shouldn't be a problem: Reviewed-by: Xin Ji # anx7625.c Reviewed-by: Abhinav Kumar # msm/dsi > I was suprised this had not landed yet. Yep. Let me know if you want me to send a v3 with those tags. Best, > > Sam > > > --- > > I considered adding _DISABLE_ instead, but that'd make the > > flag names a big too long. > > > > Generated with: > > flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {} > > flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {} > > flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {} > > flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {} > > (then minor format changes) > > > > Changes in v2: > > - Rebased on latest linux-next, after some of the flags got fixed > >(Linus Walleij). > > > > drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +- > > drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +- > > drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++-- > > drivers/gpu/drm/bridge/lontium-lt8912b.c | 2 +- > > drivers/gpu/drm/bridge/tc358768.c| 2 +- > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 > > drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- > > drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++-- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 8 > > drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +- > > drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +- > > drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +- > > drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +- > > drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +- > > drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +- > > drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +- > > drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +- > > drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++-- > > drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +- > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +- > > include/drm/drm_mipi_dsi.h | 8 > > 22 files changed, 34 insertions(+), 34 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c > > b/drivers/gpu/drm/bridge/adv7511/adv7533.c > > index aa19d5a40e31..59d718bde8c4 100644 > > --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c > > @@ -165,7 +165,7 @@ int adv7533_attach_dsi(struct adv7511 *adv) > > dsi->lanes = adv->num_dsi_lanes; > > dsi->format = MIPI_DSI_FMT_RGB888; > > dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > > MIPI_DSI_MODE_VIDEO_SYNC_PULSE | > > - MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; > > + MIPI_DSI_MODE_NO_EOT_PACKET | > > MIPI_DSI_MODE_VIDEO_HSE; > > > > ret = mipi_dsi_attach(dsi); > > if (ret < 0) { > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > > b/drivers/gpu/drm/bridge/analogix/anx7625.c > > index 7519b7a0f29d..6ca9f7e00064 100644 > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > +++ b/drivers/gpu/drm/bridge/anal
Re: [PATCH v9 1/4] dt-bindings:drm/bridge:anx7625:add vendor define flags
On Fri, Jul 16, 2021 at 1:32 PM Xin Ji wrote: > > On Tue, Jul 13, 2021 at 04:10:10PM -0600, Rob Herring wrote: > > On Wed, Jul 07, 2021 at 03:30:51PM +0800, Xin Ji wrote: > > > On Thu, Jun 24, 2021 at 01:57:22PM +0200, Robert Foss wrote: > > > > Hey Xin, > > > > > > > > I would like to merge this series now, but this patch needs a review > > > > first. Maybe Laurent/Rob Herring are good candidates. > > > > > > > > > > > > Rob. > > > Hi Rob, I get Laurent/Rob comments before, and explained why we needs > > > these DT properties, so far, I didn't get any response. > > > > Do I have to go dig that up? If it was more than a week ago, assume I > > don't remember. This is 1 of 100 bindings a week. > > > > Justify why this is needed in your commit message. > Hi Rob, I'll give more detail description in commit message. > > > > > Hi Rob Herring and Laurent, for the DT property lane0/1-swing, Google > > > engineer has strong demond for them, they don't want to move DP swing > > > adjusting to kernel, thus may cause change the driver code in each > > > project, so config them in DT is a best option. > > > > Where's the ack from a Google engineer? > They didn't give the review ack, but we discussed it offline. Nicolas > Boichat known this. Yeah I suggested this here: https://lore.kernel.org/patchwork/patch/1359670/#1564682 . I also looked at the ANX7625 datasheet at the time and it was pretty clear to me that this was not something customers could tune without ANX's help, but it'd be great if Xin Ji can describe a bit more. > > Thanks, > Xin > [snip]
[PATCH v2] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
Many of the DSI flags have names opposite to their actual effects, e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually be disabled. Fix this by including _NO_ in the flag names, e.g. MIPI_DSI_MODE_NO_EOT_PACKET. Signed-off-by: Nicolas Boichat --- I considered adding _DISABLE_ instead, but that'd make the flag names a big too long. Generated with: flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {} flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {} flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {} flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {} (then minor format changes) Changes in v2: - Rebased on latest linux-next, after some of the flags got fixed (Linus Walleij). drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +- drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +- drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++-- drivers/gpu/drm/bridge/lontium-lt8912b.c | 2 +- drivers/gpu/drm/bridge/tc358768.c| 2 +- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++-- drivers/gpu/drm/msm/dsi/dsi_host.c | 8 drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +- drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +- drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +- drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +- drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +- drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +- drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +- drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +- drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++-- drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +- drivers/gpu/drm/panel/panel-simple.c | 2 +- drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +- include/drm/drm_mipi_dsi.h | 8 22 files changed, 34 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c index aa19d5a40e31..59d718bde8c4 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c @@ -165,7 +165,7 @@ int adv7533_attach_dsi(struct adv7511 *adv) dsi->lanes = adv->num_dsi_lanes; dsi->format = MIPI_DSI_FMT_RGB888; dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | - MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; + MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; ret = mipi_dsi_attach(dsi); if (ret < 0) { diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 7519b7a0f29d..6ca9f7e00064 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1307,7 +1307,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx) dsi->format = MIPI_DSI_FMT_RGB888; dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | - MIPI_DSI_MODE_EOT_PACKET| + MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; if (mipi_dsi_attach(dsi) < 0) { diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c index b31281f76117..e6e331071a00 100644 --- a/drivers/gpu/drm/bridge/cdns-dsi.c +++ b/drivers/gpu/drm/bridge/cdns-dsi.c @@ -829,7 +829,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) tmp = DIV_ROUND_UP(dsi_cfg.htotal, nlanes) - DIV_ROUND_UP(dsi_cfg.hsa, nlanes); - if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET)) + if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) tmp -= DIV_ROUND_UP(DSI_EOT_PKT_SIZE, nlanes); tx_byte_period = DIV_ROUND_DOWN_ULL((u64)NSEC_PER_SEC * 8, @@ -902,7 +902,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) tmp = readl(dsi->regs + MCTL_MAIN_DATA_CTL); tmp &= ~(IF_VID_SELECT_MASK | HOST_EOT_GEN | IF_VID_MODE); - if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET)) + if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) tmp |= HOST_EOT_GEN; if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO) diff --git a/dri
Re: [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Mon, Jun 28, 2021 at 7:10 PM Linus Walleij wrote: > > On Wed, Mar 3, 2021 at 11:31 AM Nicolas Boichat wrote: > > On Mon, Mar 1, 2021 at 4:59 PM Linus Walleij > > wrote: > > > > > dsi->mode_flags = > > > > MIPI_DSI_CLOCK_NON_CONTINUOUS | > > > > - MIPI_DSI_MODE_EOT_PACKET; > > > > + MIPI_DSI_MODE_NO_EOT_PACKET; > > > > > > Same, just delete the flag. > > > > > > These are all just semantic bugs due to the ambiguity of the flags, it is > > > possible to provide a Fixes: flag for each file using this flag the wrong > > > way > > > but I dunno if it's worth it. > > > > Wow nice catch. > > > > I think we should fix all of those _before_ my patch is applied, with > > proper Fixes tags so that this can be backported to stable branches, > > even if it's a no-op. I can look into it but that may take a bit of > > time. > > This is fixed now, please will you proceed with this patch, because I > really like it! Thanks for the ping, that fell off my radar, v2 on its way ,-) > Yours, > Linus Walleij
Re: [PATCH v13 0/4] drm/panfrost: Add support for mt8183 GPU
On Fri, May 14, 2021 at 11:27 PM Steven Price wrote: > [snip] > >>> Seems this version is ready to be applied if we get a review on the DT ? > >>> > >>> Mathias ? could you have a look ? > >>> > >> > >> Given Rob has Acked the DT bindings, I think it's OK to apply patches > >> 1, 3 and 4 via drm-misc, letting Mediatek people sort out the DT changes. > >> > >> My two unsolicited cents :-) > > You make a convincing point - and if everyone is happy for the DT > changes to be handled separately I don't see a reason for the other > patches to be held up. > > > Yeah sure, is there a panfrost maintainer in the room ? I can apply them if > > you ack me. > > I seem to be applying most Panfrost changes these days, so I'll save you > the effort and push 1,3,4 to drm-misc-next. Thanks Steve!! > > Thanks, > > Steve
[PATCH v13 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso Reviewed-by: Steven Price --- (no changes since v7) Changes in v7: - Fix GPU ID in commit message Changes in v6: - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Change power domain name from 2d to core2. Changes in v4: - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 83a461bdeea8..ca07098a6141 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = { .vendor_quirk = panfrost_gpu_amlogic_quirk, }; +const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; +static const struct panfrost_compatible mediatek_mt8183_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .supply_names = mediatek_mt8183_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = _data, }, { .compatible = "arm,mali-t880", .data = _data, }, { .compatible = "arm,mali-bifrost", .data = _data, }, + { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v13 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will require platform-specific handling for devfreq, for 2 reasons: 1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator) does not support multiple regulators, so we'll need custom handlers. 2. Generally, platforms with 2 regulators have platform-specific constraints on how the voltages should be set (e.g. minimum/maximum voltage difference between them), so we should not just create generic handlers that simply change the voltages without taking care of those constraints. Disable devfreq for now on those GPUs. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso Reviewed-by: Steven Price --- Changes in v13: - devfreq: Fix conflict resolution mistake when rebasing, didn't even compile. Oops. Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message Changes in v6: - devfreq: New change drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 47d27e54a34f..3644652f726f 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = >pfdevfreq; + if (pfdev->comp->num_supplies > 1) { + /* +* GPUs with more than 1 supply require platform-specific handling: +* continue without devfreq +*/ + DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n"); + return 0; + } + ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names, pfdev->comp->num_supplies); if (ret) { -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v13 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat --- (no changes since v12) Changes in v12: - binding: Fix min/maxItems logic (Rob Herring) Changes in v11: - binding: power-domain-names not power-domainS-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v6: - Rebased, actually tested with recent mesa driver. Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) .../bindings/gpu/arm,mali-bifrost.yaml| 30 ++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..b22cd8f1b015 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -41,10 +42,13 @@ properties: mali-supply: true + sram-supply: true + operating-points-v2: true power-domains: -maxItems: 1 +minItems: 1 +maxItems: 3 resets: maxItems: 2 @@ -87,6 +91,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +power-domains: + minItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domain-names +else: + properties: +power-domains: + maxItems: 1 +sram-supply: false examples: - | -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v13 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! This is just a rebase of the v11, untested (but it seems like Neil Armstrong recently tested it), with small changes in binding and dts. v11 cover follows: Follow-up on the v5 [1], things have gotten significantly better in the last year, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v13: - devfreq: Fix conflict resolution mistake when rebasing, didn't even compile. Oops. Changes in v12: - binding: Fix min/maxItems logic (Rob Herring) - Add gpu node to mt8183-pumpkin.dts as well (Neil Armstrong). Changes in v11: - binding: power-domain-names not power-domainS-names - mt8183*.dts: remove incorrect supply-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. - Add gpu regulators to kukui dtsi as well. - Power domains are now attached to spm, not scpsys - Drop R-B. - devfreq: New change - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Rename "2d" power domain to "core2" - Rename "2d" power domain to "core2" (keep R-B again). - Change power domain name from 2d to core2. Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) - Add power-domain-names to describe the 3 domains. (kept Alyssa's reviewed-by as the change is minor) - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. Changes in v2: - Use sram instead of mali_sram as SRAM supply name. - Rename mali@ to gpu@. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 30 - arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 + .../boot/dts/mediatek/mt8183-pumpkin.dts | 5 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 7 files changed, 168 insertions(+), 1 deletion(-) -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v12 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
Argh sorry I messed up the rebase and this doesn't even build... I'll send v13. On Wed, Apr 21, 2021 at 8:19 AM Nicolas Boichat wrote: > > GPUs with more than a single regulator (e.g. G72 on MT8183) will > require platform-specific handling for devfreq, for 2 reasons: > 1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator) > does not support multiple regulators, so we'll need custom > handlers. > 2. Generally, platforms with 2 regulators have platform-specific > constraints on how the voltages should be set (e.g. > minimum/maximum voltage difference between them), so we > should not just create generic handlers that simply > change the voltages without taking care of those constraints. > > Disable devfreq for now on those GPUs. > > Signed-off-by: Nicolas Boichat > Reviewed-by: Tomeu Vizoso > Reviewed-by: Steven Price > --- > > (no changes since v9) > > Changes in v9: > - Explain why devfreq needs to be disabled for GPUs with >1 >regulators. > > Changes in v8: > - Use DRM_DEV_INFO instead of ERROR > > Changes in v7: > - Fix GPU ID in commit message > > Changes in v6: > - devfreq: New change > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > index 47d27e54a34f..aca3bb9a12e4 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > @@ -92,9 +92,19 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > struct thermal_cooling_device *cooling; > struct panfrost_devfreq *pfdevfreq = >pfdevfreq; > > - ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names, > -pfdev->comp->num_supplies); > - if (ret) { > + if (pfdev->comp->num_supplies > 1) { > + /* > +* GPUs with more than 1 supply require platform-specific > handling: > +* continue without devfreq > +*/ > + DRM_DEV_INFO(dev, "More than 1 supply is not supported > yet\n"); > + return 0; > + } > + > + opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, > + pfdev->comp->num_supplies); > + if (IS_ERR(opp_table)) { > + ret = PTR_ERR(opp_table); > /* Continue if the optional regulator is missing */ > if (ret != -ENODEV) { > DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n"); > -- > 2.31.1.368.gbe11c130af-goog > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso Reviewed-by: Steven Price --- (no changes since v7) Changes in v7: - Fix GPU ID in commit message Changes in v6: - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Change power domain name from 2d to core2. Changes in v4: - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 83a461bdeea8..ca07098a6141 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = { .vendor_quirk = panfrost_gpu_amlogic_quirk, }; +const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; +static const struct panfrost_compatible mediatek_mt8183_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .supply_names = mediatek_mt8183_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = _data, }, { .compatible = "arm,mali-t880", .data = _data, }, { .compatible = "arm,mali-bifrost", .data = _data, }, + { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will require platform-specific handling for devfreq, for 2 reasons: 1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator) does not support multiple regulators, so we'll need custom handlers. 2. Generally, platforms with 2 regulators have platform-specific constraints on how the voltages should be set (e.g. minimum/maximum voltage difference between them), so we should not just create generic handlers that simply change the voltages without taking care of those constraints. Disable devfreq for now on those GPUs. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso Reviewed-by: Steven Price --- (no changes since v9) Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message Changes in v6: - devfreq: New change drivers/gpu/drm/panfrost/panfrost_devfreq.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 47d27e54a34f..aca3bb9a12e4 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -92,9 +92,19 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = >pfdevfreq; - ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names, -pfdev->comp->num_supplies); - if (ret) { + if (pfdev->comp->num_supplies > 1) { + /* +* GPUs with more than 1 supply require platform-specific handling: +* continue without devfreq +*/ + DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n"); + return 0; + } + + opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, + pfdev->comp->num_supplies); + if (IS_ERR(opp_table)) { + ret = PTR_ERR(opp_table); /* Continue if the optional regulator is missing */ if (ret != -ENODEV) { DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n"); -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat --- Changes in v12: - binding: Fix min/maxItems logic (Rob Herring) Changes in v11: - binding: power-domain-names not power-domainS-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v6: - Rebased, actually tested with recent mesa driver. Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) .../bindings/gpu/arm,mali-bifrost.yaml| 30 ++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..b22cd8f1b015 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -41,10 +42,13 @@ properties: mali-supply: true + sram-supply: true + operating-points-v2: true power-domains: -maxItems: 1 +minItems: 1 +maxItems: 3 resets: maxItems: 2 @@ -87,6 +91,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +power-domains: + minItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domain-names +else: + properties: +power-domains: + maxItems: 1 +sram-supply: false examples: - | -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! This is just a rebase of the v11, untested (but it seems like Neil Armstrong recently tested it), with small changes in binding and dts. v11 cover follows: Follow-up on the v5 [1], things have gotten significantly better in the last year, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v12: - binding: Fix min/maxItems logic (Rob Herring) - Add gpu node to mt8183-pumpkin.dts as well (Neil Armstrong). Changes in v11: - binding: power-domain-names not power-domainS-names - mt8183*.dts: remove incorrect supply-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. - Add gpu regulators to kukui dtsi as well. - Power domains are now attached to spm, not scpsys - Drop R-B. - devfreq: New change - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Rename "2d" power domain to "core2" - Rename "2d" power domain to "core2" (keep R-B again). - Change power domain name from 2d to core2. Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) - Add power-domain-names to describe the 3 domains. (kept Alyssa's reviewed-by as the change is minor) - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. Changes in v2: - Use sram instead of mali_sram as SRAM supply name. - Rename mali@ to gpu@. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 30 - arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 + .../boot/dts/mediatek/mt8183-pumpkin.dts | 5 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 16 ++- drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 7 files changed, 172 insertions(+), 4 deletions(-) -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
On Tue, Apr 20, 2021 at 9:01 PM Rob Herring wrote: > > On Mon, Jan 25, 2021 at 7:18 PM Nicolas Boichat wrote: > > > > Define a compatible string for the Mali Bifrost GPU found in > > Mediatek's MT8183 SoCs. > > > > Signed-off-by: Nicolas Boichat > > --- > > > > Changes in v11: > > - binding: power-domain-names not power-domainS-names > > > > Changes in v10: > > - Fix the binding to make sure sram-supply property can be provided. > > > > Changes in v9: None > > Changes in v8: None > > Changes in v7: None > > Changes in v6: > > - Rebased, actually tested with recent mesa driver. > > > > Changes in v5: > > - Rename "2d" power domain to "core2" > > > > Changes in v4: > > - Add power-domain-names description > >(kept Alyssa's reviewed-by as the change is minor) > > > > Changes in v3: None > > Changes in v2: None > > > > .../bindings/gpu/arm,mali-bifrost.yaml| 28 +++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > index 184492162e7e..3e758f88e2cd 100644 > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > @@ -17,6 +17,7 @@ properties: > > items: > >- enum: > >- amlogic,meson-g12a-mali > > + - mediatek,mt8183-mali > >- realtek,rtd1619-mali > >- rockchip,px30-mali > >- const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully > > discoverable > > @@ -41,6 +42,8 @@ properties: > > > >mali-supply: true > > > > + sram-supply: true > > + > >operating-points-v2: true > > > >power-domains: > > @@ -87,6 +90,31 @@ allOf: > > then: > >required: > > - resets > > + - if: > > + properties: > > +compatible: > > + contains: > > +const: mediatek,mt8183-mali > > +then: > > + properties: > > +power-domains: > > + description: > > +List of phandle and PM domain specifier as documented in > > +Documentation/devicetree/bindings/power/power_domain.txt > > + minItems: 3 > > + maxItems: 3 > > This won't work because the top level schema restricts this to 1. The > top level needs to say: > > power-domains: > minItems: 1 > maxItems: 3 > > And you need just 'minItems: 3' here and 'maxItems: 1' in the else clause. > > And drop the description. That's every 'power-domains' property. > > > +power-domain-names: > > + items: > > +- const: core0 > > +- const: core1 > > +- const: core2 > > Blank line Thanks, hopefully all fixed in v12. > > + required: > > +- sram-supply > > +- power-domains > > +- power-domain-names > > +else: > > + properties: > > +sram-supply: false > > > > examples: > >- | > > -- > > 2.30.0.280.ga3ce27912f-goog > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mcde/panel: Inverse misunderstood flag
On Thu, Mar 4, 2021 at 8:41 AM Linus Walleij wrote: > > A recent patch renaming MIPI_DSI_MODE_EOT_PACKET to > MIPI_DSI_MODE_NO_EOT_PACKET brought to light the > misunderstanding in the current MCDE driver and all > its associated panel drivers that MIPI_DSI_MODE_EOT_PACKET > would mean "use EOT packet" when in fact it means the > reverse. > > Fix it up by implementing the flag right in the MCDE > DSI driver and remove the flag from panels that actually > want the EOT packet. > > Suggested-by: Nicolas Boichat > Signed-off-by: Linus Walleij Reviewed-by: Nicolas Boichat I wonder if it's worth adding the fixes, should be: Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE") Fixes: 899f24ed8d3a ("drm/panel: Add driver for Novatek NT35510-based panels") Fixes: ac1d6d74884e ("drm/panel: Add driver for Samsung S6D16D0 panel") Fixes: 435e06c06cb2 ("drm/panel: s6e63m0: Add DSI transport") Fixes: 8152c2bfd780 ("drm/panel: Add driver for Sony ACX424AKP panel") But then you'd almost need to separate the patches in multiple bits (these patches landed in very different releases). I'll leave that up to the maintainers to decide: this would only matter if anybody tried to use these panels on LTS releases with a non-MCDE driver (or MCDE with other panels). > --- > drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- > drivers/gpu/drm/panel/panel-novatek-nt35510.c | 3 +-- > drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 4 +--- > drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c | 1 - > drivers/gpu/drm/panel/panel-sony-acx424akp.c | 3 +-- > 5 files changed, 4 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c > index 2314c8122992..b3fd3501c412 100644 > --- a/drivers/gpu/drm/mcde/mcde_dsi.c > +++ b/drivers/gpu/drm/mcde/mcde_dsi.c > @@ -760,7 +760,7 @@ static void mcde_dsi_start(struct mcde_dsi *d) > DSI_MCTL_MAIN_DATA_CTL_BTA_EN | > DSI_MCTL_MAIN_DATA_CTL_READ_EN | > DSI_MCTL_MAIN_DATA_CTL_REG_TE_EN; > - if (d->mdsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET) > + if (!(d->mdsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET)) > val |= DSI_MCTL_MAIN_DATA_CTL_HOST_EOT_GEN; > writel(val, d->regs + DSI_MCTL_MAIN_DATA_CTL); > > diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c > b/drivers/gpu/drm/panel/panel-novatek-nt35510.c > index b9a0e56f33e2..ef70140c5b09 100644 > --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c > +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c > @@ -898,8 +898,7 @@ static int nt35510_probe(struct mipi_dsi_device *dsi) > */ > dsi->hs_rate = 34944; > dsi->lp_rate = 960; > - dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS | > - MIPI_DSI_MODE_EOT_PACKET; > + dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS; > > /* > * Every new incarnation of this display must have a unique > diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > index 4aac0d1573dd..70560cac53a9 100644 > --- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > +++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > @@ -184,9 +184,7 @@ static int s6d16d0_probe(struct mipi_dsi_device *dsi) > * As we only send commands we do not need to be continuously > * clocked. > */ > - dsi->mode_flags = > - MIPI_DSI_CLOCK_NON_CONTINUOUS | > - MIPI_DSI_MODE_EOT_PACKET; > + dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS; > > s6->supply = devm_regulator_get(dev, "vdd1"); > if (IS_ERR(s6->supply)) > diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c > b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c > index eec74c10ddda..9c3563c61e8c 100644 > --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c > +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c > @@ -97,7 +97,6 @@ static int s6e63m0_dsi_probe(struct mipi_dsi_device *dsi) > dsi->hs_rate = 34944; > dsi->lp_rate = 960; > dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > - MIPI_DSI_MODE_EOT_PACKET | > MIPI_DSI_MODE_VIDEO_BURST; > > ret = s6e63m0_probe(dev, s6e63m0_dsi_dcs_read, s6e63m0_dsi_dcs_write, > diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c > b/drivers/gpu/drm/panel/panel-sony-acx424akp.c > index 065efae213f5..95659a4d15e9 100644 > --- a/drivers/gpu/drm/panel/panel-sony-acx424akp.c > +++ b/drivers/gpu/drm/panel/panel-sony-acx424
Re: [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Mon, Mar 1, 2021 at 4:59 PM Linus Walleij wrote: > > On Thu, Feb 11, 2021 at 4:34 AM Nicolas Boichat wrote: > > > Many of the DSI flags have names opposite to their actual effects, > > e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually > > be disabled. Fix this by including _NO_ in the flag names, e.g. > > MIPI_DSI_MODE_NO_EOT_PACKET. > > Unless someone like me interpreted it literally... > > Like in these: > > > drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- > > drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +- > > drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +- > > drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +- > > > diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c > > b/drivers/gpu/drm/mcde/mcde_dsi.c > > index 2314c8122992..f4cdc3cfd7d0 100644 > > --- a/drivers/gpu/drm/mcde/mcde_dsi.c > > +++ b/drivers/gpu/drm/mcde/mcde_dsi.c > > @@ -760,7 +760,7 @@ static void mcde_dsi_start(struct mcde_dsi *d) > > DSI_MCTL_MAIN_DATA_CTL_BTA_EN | > > DSI_MCTL_MAIN_DATA_CTL_READ_EN | > > DSI_MCTL_MAIN_DATA_CTL_REG_TE_EN; > > - if (d->mdsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET) > > + if (d->mdsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) > > val |= DSI_MCTL_MAIN_DATA_CTL_HOST_EOT_GEN; > > If you read the code you can see that this is interpreted as inserting > an EOT packet, so here you need to change the logic such: > > if (!d->mdsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) > val |= DSI_MCTL_MAIN_DATA_CTL_HOST_EOT_GEN; > > This will make sure the host generates the EOT packet in HS mode > *unless* the flag is set. > > (I checked the data sheet.) > > > diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c > > b/drivers/gpu/drm/panel/panel-novatek-nt35510.c > > index b9a0e56f33e2..9d9334656803 100644 > > --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c > > +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c > > @@ -899,7 +899,7 @@ static int nt35510_probe(struct mipi_dsi_device *dsi) > > dsi->hs_rate = 34944; > > dsi->lp_rate = 960; > > dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS | > > - MIPI_DSI_MODE_EOT_PACKET; > > + MIPI_DSI_MODE_NO_EOT_PACKET; > > Here you should just delete the MIPI_DSI_MODE_EOT_PACKET > flag because this was used with the MCDE driver which interpret the > flag literally. > > > diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > > b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > > index 4aac0d1573dd..b04b9975e9b2 100644 > > --- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > > +++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > > @@ -186,7 +186,7 @@ static int s6d16d0_probe(struct mipi_dsi_device *dsi) > > */ > > dsi->mode_flags = > > MIPI_DSI_CLOCK_NON_CONTINUOUS | > > - MIPI_DSI_MODE_EOT_PACKET; > > + MIPI_DSI_MODE_NO_EOT_PACKET; > > Same, just delete the flag. > > > --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c > > +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c > > @@ -97,7 +97,7 @@ static int s6e63m0_dsi_probe(struct mipi_dsi_device *dsi) > > dsi->hs_rate = 34944; > > dsi->lp_rate = 960; > > dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > > - MIPI_DSI_MODE_EOT_PACKET | > > + MIPI_DSI_MODE_NO_EOT_PACKET | > > MIPI_DSI_MODE_VIDEO_BURST; > > Same, just delete the flag. > > > diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c > > b/drivers/gpu/drm/panel/panel-sony-acx424akp.c > > index 065efae213f5..6b706cbf2f9c 100644 > > --- a/drivers/gpu/drm/panel/panel-sony-acx424akp.c > > +++ b/drivers/gpu/drm/panel/panel-sony-acx424akp.c > > @@ -450,7 +450,7 @@ static int acx424akp_probe(struct mipi_dsi_device *dsi) > > else > > dsi->mode_flags = > > MIPI_DSI_CLOCK_NON_CONTINUOUS | > > - MIPI_DSI_MODE_EOT_PACKET; > > + MIPI_DSI_MODE_NO_EOT_PACKET; > > Same, just delete the flag. > > These are all just semantic bugs due to the ambiguity of the flags, it is > possible to provide a Fixes: flag for each file using this flag the wrong way > but I dunno if it's worth it. Wow nice catch. I think we should fix all of those _before_ my patch is applied, with proper Fixes tags so that this can be backported to stable branches, even if it's a no-op. I can look into it but that may take a bit of time. Thanks, > > Yours, > Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Mon, Feb 22, 2021 at 3:21 PM Andrzej Hajda wrote: > > Hi Nicolas, > > W dniu 22.02.2021 o 06:31, Nicolas Boichat pisze: > > On Mon, Feb 22, 2021 at 3:08 AM Laurent Pinchart > > wrote: > >> Hi Nicolas, > >> > >> Thank you for the patch. > >> > >> On Thu, Feb 11, 2021 at 11:33:55AM +0800, Nicolas Boichat wrote: > >>> Many of the DSI flags have names opposite to their actual effects, > >>> e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually > >>> be disabled. Fix this by including _NO_ in the flag names, e.g. > >>> MIPI_DSI_MODE_NO_EOT_PACKET. > >>> > >>> Signed-off-by: Nicolas Boichat > >> This looks good to me, it increases readability. > >> > >> Reviewed-by: Laurent Pinchart > >> > >> Please however see the end of the mail for a comment. > > > Reviewed-by: Andrzej Hajda > > And comment at the end. > > >> > >>> --- > >>> I considered adding _DISABLE_ instead, but that'd make the > >>> flag names a big too long. > >>> > >>> Generated with: > >>> flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \ > >>>xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {} > >>> flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \ > >>>xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {} > >>> flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \ > >>>xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {} > >>> flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \ > >>>xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {} > >>> (then minor format changes) > >> Ever tried coccinelle ? :-) > > Fun project for next time ,-) > > > >>> drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +- > >>> drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +- > >>> drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++-- > >>> drivers/gpu/drm/bridge/tc358768.c| 2 +- > >>> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 > >>> drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- > >>> drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +- > >>> drivers/gpu/drm/msm/dsi/dsi_host.c | 8 > >>> drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +- > >>> drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +- > >>> drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +- > >>> drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +- > >>> drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +- > >>> drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +- > >>> drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +- > >>> drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +- > >>> drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +- > >>> drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +- > >>> drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c| 2 +- > >>> drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++-- > >>> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +- > >>> drivers/gpu/drm/panel/panel-simple.c | 2 +- > >>> drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +- > >>> drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +- > >>> include/drm/drm_mipi_dsi.h | 8 > >>> 25 files changed, 36 insertions(+), 36 deletions(-) > >>> > >>> [] > >>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > >>> index 360e6377e84b..ba91cf22af51 100644 > >>> --- a/include/drm/drm_mipi_dsi.h > >>> +++ b/include/drm/drm_mipi_dsi.h > >>> @@ -119,15 +119,15 @@ struct mipi_dsi_host > >>> *of_find_mipi_dsi_host_by_node(struct device_node *node); > >>> /* enable hsync-end packets in vsync-pulse and v-porch area */ > >>> #define MIPI_DSI_MODE_VIDEO_HSE BIT(4) > >> We're mixing bits that enable a feature and bits that disable a feature. > >> Are these bits defined in the DSI spec, or internal to DRM ? In the > >> latter case, would it make sense to standardi
Re: [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Mon, Feb 22, 2021 at 3:08 AM Laurent Pinchart wrote: > > Hi Nicolas, > > Thank you for the patch. > > On Thu, Feb 11, 2021 at 11:33:55AM +0800, Nicolas Boichat wrote: > > Many of the DSI flags have names opposite to their actual effects, > > e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually > > be disabled. Fix this by including _NO_ in the flag names, e.g. > > MIPI_DSI_MODE_NO_EOT_PACKET. > > > > Signed-off-by: Nicolas Boichat > > This looks good to me, it increases readability. > > Reviewed-by: Laurent Pinchart > > Please however see the end of the mail for a comment. > > > --- > > I considered adding _DISABLE_ instead, but that'd make the > > flag names a big too long. > > > > Generated with: > > flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {} > > flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {} > > flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {} > > flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \ > > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {} > > (then minor format changes) > > Ever tried coccinelle ? :-) Fun project for next time ,-) > > > drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +- > > drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +- > > drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++-- > > drivers/gpu/drm/bridge/tc358768.c| 2 +- > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 > > drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- > > drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 8 > > drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +- > > drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +- > > drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +- > > drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +- > > drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +- > > drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +- > > drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +- > > drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +- > > drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +- > > drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +- > > drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c| 2 +- > > drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++-- > > drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +- > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +- > > drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +- > > include/drm/drm_mipi_dsi.h | 8 > > 25 files changed, 36 insertions(+), 36 deletions(-) > > > > [] > > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > > index 360e6377e84b..ba91cf22af51 100644 > > --- a/include/drm/drm_mipi_dsi.h > > +++ b/include/drm/drm_mipi_dsi.h > > @@ -119,15 +119,15 @@ struct mipi_dsi_host > > *of_find_mipi_dsi_host_by_node(struct device_node *node); > > /* enable hsync-end packets in vsync-pulse and v-porch area */ > > #define MIPI_DSI_MODE_VIDEO_HSE BIT(4) > > We're mixing bits that enable a feature and bits that disable a feature. > Are these bits defined in the DSI spec, or internal to DRM ? In the > latter case, would it make sense to standardize on one "polarity" ? That > would be a more intrusive change in drivers though. Yes, that'd require auditing every single code path and reverse the logic as needed. I'm not volunteering for that ,-P (hopefully the current change is still an improvement). Hopefully real DSI experts can comment (Andrzej?), I think the default are sensible settings? > > > /* disable hfront-porch area */ > > -#define MIPI_DSI_MODE_VIDEO_HFP BIT(5) > > +#define MIPI_DSI_MODE_VIDEO_NO_HFP BIT(5) > > /* disable hback-porch area */ > > -#define MIPI_DSI_MODE_VIDEO_HBP BIT(6) > > +#define MIPI_DSI_MODE_VIDEO_NO_HBP BIT(6) > > /* disable hsync-active area */ > > -#define MIPI_DSI_MODE_VIDEO_HSA BIT(7) > > +#define MIPI_DSI_MODE_VIDEO_NO_HSA B
Re: [PATCH v3 1/3] drm/mediatek: mtk_dpi: Add check for max clock rate in mode_valid
+Pi-Hsun Shih On Mon, Feb 8, 2021 at 9:42 AM Jitao Shi wrote: > > Add per-platform max clock rate check in mtk_dpi_bridge_mode_valid. > > Signed-off-by: Jitao Shi I believe this patch (and the following) were actually authored by Pi-Hsun: https://crrev.com/c/2628812 . Would be best to keep the author information (unless I'm missing something of course). > --- > drivers/gpu/drm/mediatek/mtk_dpi.c | 17 + > 1 file changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c > b/drivers/gpu/drm/mediatek/mtk_dpi.c > index 52f11a63a330..ffa4a0f1989f 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > @@ -118,6 +118,7 @@ struct mtk_dpi_yc_limit { > struct mtk_dpi_conf { > unsigned int (*cal_factor)(int clock); > u32 reg_h_fre_con; > + u32 max_clock_khz; > bool edge_sel_en; > }; > > @@ -555,9 +556,22 @@ static void mtk_dpi_bridge_enable(struct drm_bridge > *bridge) > mtk_dpi_set_display_mode(dpi, >mode); > } > > +static enum drm_mode_status > +mtk_dpi_bridge_mode_valid(struct drm_bridge *bridge, > + const struct drm_display_mode *mode) > +{ > + struct mtk_dpi *dpi = bridge_to_dpi(bridge); > + > + if (dpi->conf->max_clock_khz && mode->clock > > dpi->conf->max_clock_khz) > + return MODE_CLOCK_HIGH; > + > + return MODE_OK; > +} > + > static const struct drm_bridge_funcs mtk_dpi_bridge_funcs = { > .attach = mtk_dpi_bridge_attach, > .mode_set = mtk_dpi_bridge_mode_set, > + .mode_valid = mtk_dpi_bridge_mode_valid, > .disable = mtk_dpi_bridge_disable, > .enable = mtk_dpi_bridge_enable, > }; > @@ -673,17 +687,20 @@ static unsigned int mt8183_calculate_factor(int clock) > static const struct mtk_dpi_conf mt8173_conf = { > .cal_factor = mt8173_calculate_factor, > .reg_h_fre_con = 0xe0, > + .max_clock_khz = 30, > }; > > static const struct mtk_dpi_conf mt2701_conf = { > .cal_factor = mt2701_calculate_factor, > .reg_h_fre_con = 0xb0, > .edge_sel_en = true, > + .max_clock_khz = 15, > }; > > static const struct mtk_dpi_conf mt8183_conf = { > .cal_factor = mt8183_calculate_factor, > .reg_h_fre_con = 0xe0, > + .max_clock_khz = 10, > }; > > static int mtk_dpi_probe(struct platform_device *pdev) > -- > 2.25.1 > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
Many of the DSI flags have names opposite to their actual effects, e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually be disabled. Fix this by including _NO_ in the flag names, e.g. MIPI_DSI_MODE_NO_EOT_PACKET. Signed-off-by: Nicolas Boichat --- I considered adding _DISABLE_ instead, but that'd make the flag names a big too long. Generated with: flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {} flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {} flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {} flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \ xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {} (then minor format changes) drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +- drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +- drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++-- drivers/gpu/drm/bridge/tc358768.c| 2 +- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 8 drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +- drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +- drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +- drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +- drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +- drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +- drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +- drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +- drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +- drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +- drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c| 2 +- drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++-- drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +- drivers/gpu/drm/panel/panel-simple.c | 2 +- drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +- drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +- include/drm/drm_mipi_dsi.h | 8 25 files changed, 36 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c index aa19d5a40e31..59d718bde8c4 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c @@ -165,7 +165,7 @@ int adv7533_attach_dsi(struct adv7511 *adv) dsi->lanes = adv->num_dsi_lanes; dsi->format = MIPI_DSI_FMT_RGB888; dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | - MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; + MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; ret = mipi_dsi_attach(dsi); if (ret < 0) { diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 65cc05982f82..beecfe6bf359 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1334,7 +1334,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx) dsi->format = MIPI_DSI_FMT_RGB888; dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | - MIPI_DSI_MODE_EOT_PACKET| + MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE; if (mipi_dsi_attach(dsi) < 0) { diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c index 76373e31df92..34aa24269a57 100644 --- a/drivers/gpu/drm/bridge/cdns-dsi.c +++ b/drivers/gpu/drm/bridge/cdns-dsi.c @@ -829,7 +829,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) tmp = DIV_ROUND_UP(dsi_cfg.htotal, nlanes) - DIV_ROUND_UP(dsi_cfg.hsa, nlanes); - if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET)) + if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) tmp -= DIV_ROUND_UP(DSI_EOT_PKT_SIZE, nlanes); tx_byte_period = DIV_ROUND_DOWN_ULL((u64)NSEC_PER_SEC * 8, @@ -902,7 +902,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) tmp = readl(dsi->regs + MCTL_MAIN_DATA_CTL); tmp &= ~(IF_VID_SELECT_MASK | HOST_EOT_GEN | IF_VID_MODE); - if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET)) + if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)) tmp |= HOST_EOT_GEN; i
Re: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
On Sat, Feb 6, 2021 at 1:55 AM Rob Herring wrote: > > On Tue, 26 Jan 2021 09:17:56 +0800, Nicolas Boichat wrote: > > Define a compatible string for the Mali Bifrost GPU found in > > Mediatek's MT8183 SoCs. > > > > Signed-off-by: Nicolas Boichat > > --- > > > > Changes in v11: > > - binding: power-domain-names not power-domainS-names > > > > Changes in v10: > > - Fix the binding to make sure sram-supply property can be provided. > > > > Changes in v9: None > > Changes in v8: None > > Changes in v7: None > > Changes in v6: > > - Rebased, actually tested with recent mesa driver. > > > > Changes in v5: > > - Rename "2d" power domain to "core2" > > > > Changes in v4: > > - Add power-domain-names description > >(kept Alyssa's reviewed-by as the change is minor) > > > > Changes in v3: None > > Changes in v2: None > > > > .../bindings/gpu/arm,mali-bifrost.yaml| 28 +++ > > 1 file changed, 28 insertions(+) > > > > > Please add Acked-by/Reviewed-by tags when posting new versions. However, > there's no need to repost patches *only* to add the tags. The upstream > maintainer will do that for acks received on the version they apply. > > If a tag was not added on purpose, please state why and what changed. There were changes in v11, I thought you'd want to review again? Anyway, I can resend a v12 with all the Rb/Ab if that works better for you. > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: anx7625: enable DSI EOTP
On Thu, Feb 4, 2021 at 8:59 PM Andrzej Hajda wrote: > > > W dniu 04.02.2021 o 13:34, Nicolas Boichat pisze: > > On Thu, Feb 4, 2021 at 8:07 PM Robert Foss wrote: > >> Hi Xin, > >> > >> Thanks for the patch. > >> > >> On Thu, 28 Jan 2021 at 12:17, Xin Ji wrote: > >>> Enable DSI EOTP feature for fixing some panel screen constance > >>> shift issue. > >>> Removing MIPI flag MIPI_DSI_MODE_EOT_PACKET to enable DSI EOTP. > >> I don't think I quite understand how removing the > >> MIPI_DSI_MODE_EOT_PACKET flag will cause DSI EOTP to be enabled. Could > >> you extrapolate on this in the commit message? > > That confused me as well, but it turns out that's how the flag is defined: > > ``` > > /* disable EoT packets in HS mode */ > > #define MIPI_DSI_MODE_EOT_PACKET BIT(9) > > ``` > > (https://protect2.fireeye.com/v1/url?k=5bd95ebd-044267fb-5bd8d5f2-0cc47a3003e8-ce9db8ea264d6901=1=900556dc-d199-4c18-9432-5c3465a98eae=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Finclude%2Fdrm%2Fdrm_mipi_dsi.h%23L129) > > > > I'm almost tempted to put together a mass patch to rename all of these > > flags... > > > Yes that would be good, many of these flags were just copy pasted from > some hw datasheet, without good analysis how to adapt them to the framework. I'll look into it (but that shouldn't block this patch). > > > Regards > > Andrzej > > > > > >>> Signed-off-by: Xin Ji > >>> --- > >>> drivers/gpu/drm/bridge/analogix/anx7625.c | 1 - > >>> 1 file changed, 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > >>> b/drivers/gpu/drm/bridge/analogix/anx7625.c > >>> index 65cc059..e31eeb1b 100644 > >>> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > >>> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > >>> @@ -1334,7 +1334,6 @@ static int anx7625_attach_dsi(struct anx7625_data > >>> *ctx) > >>> dsi->format = MIPI_DSI_FMT_RGB888; > >>> dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > >>> MIPI_DSI_MODE_VIDEO_SYNC_PULSE | > >>> - MIPI_DSI_MODE_EOT_PACKET| > >>> MIPI_DSI_MODE_VIDEO_HSE; > >>> > >>> if (mipi_dsi_attach(dsi) < 0) { > >>> -- > >>> 2.7.4 > >>> > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://protect2.fireeye.com/v1/url?k=457f3f39-1ae4067f-457eb476-0cc47a3003e8-b702072da729d8c9=1=900556dc-d199-4c18-9432-5c3465a98eae=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: anx7625: enable DSI EOTP
On Thu, Feb 4, 2021 at 8:07 PM Robert Foss wrote: > > Hi Xin, > > Thanks for the patch. > > On Thu, 28 Jan 2021 at 12:17, Xin Ji wrote: > > > > Enable DSI EOTP feature for fixing some panel screen constance > > shift issue. > > Removing MIPI flag MIPI_DSI_MODE_EOT_PACKET to enable DSI EOTP. > > I don't think I quite understand how removing the > MIPI_DSI_MODE_EOT_PACKET flag will cause DSI EOTP to be enabled. Could > you extrapolate on this in the commit message? That confused me as well, but it turns out that's how the flag is defined: ``` /* disable EoT packets in HS mode */ #define MIPI_DSI_MODE_EOT_PACKET BIT(9) ``` (https://elixir.bootlin.com/linux/latest/source/include/drm/drm_mipi_dsi.h#L129) I'm almost tempted to put together a mass patch to rename all of these flags... > > > > > Signed-off-by: Xin Ji > > --- > > drivers/gpu/drm/bridge/analogix/anx7625.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > > b/drivers/gpu/drm/bridge/analogix/anx7625.c > > index 65cc059..e31eeb1b 100644 > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > > @@ -1334,7 +1334,6 @@ static int anx7625_attach_dsi(struct anx7625_data > > *ctx) > > dsi->format = MIPI_DSI_FMT_RGB888; > > dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > > MIPI_DSI_MODE_VIDEO_SYNC_PULSE | > > - MIPI_DSI_MODE_EOT_PACKET| > > MIPI_DSI_MODE_VIDEO_HSE; > > > > if (mipi_dsi_attach(dsi) < 0) { > > -- > > 2.7.4 > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mediatek: fine tune the data lane trail by project dts
On Mon, Feb 1, 2021 at 11:48 AM Jitao Shi wrote: > > Some panels or bridges require customized hs_da_trail time. > So add a property in devicetree for this panels and bridges. Since this changes the device tree, you also need to upload a binding document change. > > Signed-off-by: Jitao Shi > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 8c70ec39bfe1..6e7092fa2fee 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -194,6 +194,7 @@ struct mtk_dsi { > struct clk *hs_clk; > > u32 data_rate; > + u32 da_trail_delta; > > unsigned long mode_flags; > enum mipi_dsi_pixel_format format; > @@ -234,7 +235,7 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi) > timing->da_hs_prepare = (80 * data_rate_mhz + 4 * 1000) / 8000; > timing->da_hs_zero = (170 * data_rate_mhz + 10 * 1000) / 8000 + 1 - > timing->da_hs_prepare; > - timing->da_hs_trail = timing->da_hs_prepare + 1; > + timing->da_hs_trail = timing->da_hs_prepare + 1 + dsi->da_trail_delta; > > timing->ta_go = 4 * timing->lpx - 2; > timing->ta_sure = timing->lpx + 2; > @@ -1094,6 +1095,13 @@ static int mtk_dsi_probe(struct platform_device *pdev) > goto err_unregister_host; > } > > + ret = of_property_read_u32_index(dev->of_node, "da_trail_delta", 0, > +>da_trail_delta); > + if (ret) { > + dev_info(dev, "Can't get da_trail_delta, keep it as 0: %d\n", > ret); > + dsi->da_trail_delta = 0; > + } > + > comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DSI); > if (comp_id < 0) { > dev_err(dev, "Failed to identify by alias: %d\n", comp_id); > -- > 2.12.5 > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v11 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso Reviewed-by: Steven Price --- Changes in v11: None Changes in v10: None Changes in v9: None Changes in v8: None Changes in v7: - Fix GPU ID in commit message Changes in v6: - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Change power domain name from 2d to core2. Changes in v4: - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. Changes in v2: None drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 83a461bdeea8..ca07098a6141 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = { .vendor_quirk = panfrost_gpu_amlogic_quirk, }; +const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; +static const struct panfrost_compatible mediatek_mt8183_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .supply_names = mediatek_mt8183_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = _data, }, { .compatible = "arm,mali-t880", .data = _data, }, { .compatible = "arm,mali-bifrost", .data = _data, }, + { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.30.0.280.ga3ce27912f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v11 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will require platform-specific handling for devfreq, for 2 reasons: 1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator) does not support multiple regulators, so we'll need custom handlers. 2. Generally, platforms with 2 regulators have platform-specific constraints on how the voltages should be set (e.g. minimum/maximum voltage difference between them), so we should not just create generic handlers that simply change the voltages without taking care of those constraints. Disable devfreq for now on those GPUs. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso Reviewed-by: Steven Price --- Changes in v11: None Changes in v10: None Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message Changes in v6: - devfreq: New change Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 7c5ffc81dce1..a544ae1e560a 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -93,6 +93,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = >pfdevfreq; + if (pfdev->comp->num_supplies > 1) { + /* +* GPUs with more than 1 supply require platform-specific handling: +* continue without devfreq +*/ + DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n"); + return 0; + } + opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, pfdev->comp->num_supplies); if (IS_ERR(opp_table)) { -- 2.30.0.280.ga3ce27912f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat --- Changes in v11: - binding: power-domain-names not power-domainS-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v9: None Changes in v8: None Changes in v7: None Changes in v6: - Rebased, actually tested with recent mesa driver. Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: None Changes in v2: None .../bindings/gpu/arm,mali-bifrost.yaml| 28 +++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..3e758f88e2cd 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -41,6 +42,8 @@ properties: mali-supply: true + sram-supply: true + operating-points-v2: true power-domains: @@ -87,6 +90,31 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + required: +- sram-supply +- power-domains +- power-domain-names +else: + properties: +sram-supply: false examples: - | -- 2.30.0.280.ga3ce27912f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v11 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v11: - binding: power-domain-names not power-domainS-names - mt8183*.dts: remove incorrect supply-names Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. - Add gpu regulators to kukui dtsi as well. - Power domains are now attached to spm, not scpsys - Drop R-B. - devfreq: New change - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Rename "2d" power domain to "core2" - Rename "2d" power domain to "core2" (keep R-B again). - Change power domain name from 2d to core2. Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) - Add power-domain-names to describe the 3 domains. (kept Alyssa's reviewed-by as the change is minor) - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. Changes in v2: - Use sram instead of mali_sram as SRAM supply name. - Rename mali@ to gpu@. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 28 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 162 insertions(+) -- 2.30.0.280.ga3ce27912f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] dt-bindings: drm/bridge: anx7625: add DPI flag and swing setting
On Tue, Jan 12, 2021 at 4:59 PM Xin Ji wrote: > > Hi Rob Herring, thanks for the comments. > > On Mon, Jan 11, 2021 at 04:14:35PM -0600, Rob Herring wrote: > > On Thu, Dec 31, 2020 at 10:21:12AM +0800, Xin Ji wrote: > > > Add DPI flag for distinguish MIPI input signal type, DSI or DPI. Add > > > swing setting for adjusting DP tx PHY swing > > > > > > Signed-off-by: Xin Ji > > > --- > > > .../bindings/display/bridge/analogix,anx7625.yaml | 25 > > > -- > > > 1 file changed, 23 insertions(+), 2 deletions(-) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > index 60585a4..4eb0ea3 100644 > > > --- > > > a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > +++ > > > b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > @@ -34,6 +34,16 @@ properties: > > > description: used for reset chip control, RESET_N pin B7. > > > maxItems: 1 > > > > > > + analogix,swing-setting: > > > +type: uint8-array > > > > Humm, this should have be rejected by the meta-schema. > We needs define an array to adjust DP tx PHY swing, the developer hopes these > settings are changeable, so I moved the register data to DT. Can you > give me some suggestion if it is rejected by the meta-schema? > > > > > +$ref: /schemas/types.yaml#/definitions/uint32-array > > > > This is how types are defined other than boolean or nodes (object). > > > > > +description: an array of swing register setting for DP tx PHY > > > + > > > + analogix,mipi-dpi-in: > > > +type: int > > > +$ref: /schemas/types.yaml#/definitions/uint32 > > > +description: indicate the MIPI rx signal type is DPI or DSI > > > > Why does this need to be in DT, you should be able to determine this > > based on what you are connected to. > As the anx7625 can receive MIPI DSI and DPI data (depends on hardware > implement, we have a project which have two anx7625, one is DSI input, > the other is DPI input), we needs to let driver know what kind of MIPI > rx signal input. And there is no other way to tell driver the MIPI rx > signal type, we needs define this flag. > > > > > + > > >ports: > > > type: object > > > > > > @@ -49,8 +59,8 @@ properties: > > >Video port for panel or connector. > > > > > > required: > > > -- port@0 > > > -- port@1 > > > + - port@0 > > > + - port@1 > > > > > > required: > > >- compatible > > > @@ -72,6 +82,17 @@ examples: > > > reg = <0x58>; > > > enable-gpios = < 45 GPIO_ACTIVE_HIGH>; > > > reset-gpios = < 73 GPIO_ACTIVE_HIGH>; > > > +analogix,swing-setting = <0x00 0x14>, <0x01 0x54>, > > > +<0x02 0x64>, <0x03 0x74>, <0x04 0x29>, > > > +<0x05 0x7b>, <0x06 0x77>, <0x07 0x5b>, > > > +<0x08 0x7f>, <0x0c 0x20>, <0x0d 0x60>, > > > +<0x10 0x60>, <0x12 0x40>, <0x13 0x60>, > > > +<0x14 0x14>, <0x15 0x54>, <0x16 0x64>, > > > +<0x17 0x74>, <0x18 0x29>, <0x19 0x7b>, > > > +<0x1a 0x77>, <0x1b 0x5b>, <0x1c 0x7f>, > > > +<0x20 0x20>, <0x21 0x60>, <0x24 0x60>, > > > +<0x26 0x40>, <0x27 0x60>; > > > > This is a matrix, which is different from an array type. > OK, I'll change to array if this vendor define has been accepted. I also wonder if we want to split the parameters per lane, and simply have a flat array (instead of reg/value pairs like you have now). Registers 0x00 to 0x13 have to do with Lane 0 (and 0x14 to 0x27 with Lane 1): you can almost tell from the example values, they repeat. I'm not sure if there's any value splitting those further (I don't think anybody will be able to tune those without ANX's help). So, maybe something like: anx,swing-setting = <<[reg values for lane 0]>, <[reg values for lane 1]>> where <[reg values for lane 0]> would be something like <0x14, 0x54, 0x64, ...> (that is, all the values between 0x00 and 0x13). > > > > > +analogix,mipi-dpi-in = <0>; > > > > > > ports { > > > #address-cells = <1>; > > > -- > > > 2.7.4 > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso --- (no changes since v7) Changes in v7: - Fix GPU ID in commit message Changes in v6: - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Change power domain name from 2d to core2. Changes in v4: - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 83a461bdeea8..ca07098a6141 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = { .vendor_quirk = panfrost_gpu_amlogic_quirk, }; +const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; +static const struct panfrost_compatible mediatek_mt8183_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .supply_names = mediatek_mt8183_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = _data, }, { .compatible = "arm,mali-t880", .data = _data, }, { .compatible = "arm,mali-bifrost", .data = _data, }, + { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.30.0.284.gd98b1dd5eaa7-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will require platform-specific handling for devfreq, for 2 reasons: 1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator) does not support multiple regulators, so we'll need custom handlers. 2. Generally, platforms with 2 regulators have platform-specific constraints on how the voltages should be set (e.g. minimum/maximum voltage difference between them), so we should not just create generic handlers that simply change the voltages without taking care of those constraints. Disable devfreq for now on those GPUs. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso --- (no changes since v9) Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message Changes in v6: - New change drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index f44d28fad085..812cfecdee3b 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = >pfdevfreq; + if (pfdev->comp->num_supplies > 1) { + /* +* GPUs with more than 1 supply require platform-specific handling: +* continue without devfreq +*/ + DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n"); + return 0; + } + opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, pfdev->comp->num_supplies); if (IS_ERR(opp_table)) { -- 2.30.0.284.gd98b1dd5eaa7-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat --- Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 28 +++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..eac561582063 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -41,6 +42,8 @@ properties: mali-supply: true + sram-supply: true + operating-points-v2: true power-domains: @@ -87,6 +90,31 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + required: +- sram-supply +- power-domains +- power-domains-names +else: + properties: +sram-supply: false examples: - | -- 2.30.0.284.gd98b1dd5eaa7-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v10: - Fix the binding to make sure sram-supply property can be provided. Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 28 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 164 insertions(+) -- 2.30.0.284.gd98b1dd5eaa7-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
On Tue, Jan 12, 2021 at 11:07 PM Rob Herring wrote: > > On Fri, Jan 08, 2021 at 09:10:08AM +0800, Nicolas Boichat wrote: > > Define a compatible string for the Mali Bifrost GPU found in > > Mediatek's MT8183 SoCs. > > > > Signed-off-by: Nicolas Boichat > > Reviewed-by: Alyssa Rosenzweig > > --- > > > > (no changes since v6) > > > > Changes in v6: > > - Rebased, actually tested with recent mesa driver. > > - No change > > > > Changes in v5: > > - Rename "2d" power domain to "core2" > > > > Changes in v4: > > - Add power-domain-names description > >(kept Alyssa's reviewed-by as the change is minor) > > > > Changes in v3: > > - No change > > > > .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > index 184492162e7e..71b613ee5bd7 100644 > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > @@ -17,6 +17,7 @@ properties: > > items: > >- enum: > >- amlogic,meson-g12a-mali > > + - mediatek,mt8183-mali > >- realtek,rtd1619-mali > >- rockchip,px30-mali > >- const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully > > discoverable > > @@ -87,6 +88,30 @@ allOf: > > then: > >required: > > - resets > > + - if: > > + properties: > > +compatible: > > + contains: > > +const: mediatek,mt8183-mali > > +then: > > + properties: > > +sram-supply: true > > This has to be defined at the top-level or there will be an error when > it is present (due to additionalProperties). > > In this if/then you can do: > > else: > sram-supply: false > > to disallow it if not 'mediatek,mt8183-mali' I see. Thanks Rob, will send a v10. > > > +power-domains: > > + description: > > +List of phandle and PM domain specifier as documented in > > +Documentation/devicetree/bindings/power/power_domain.txt > > + minItems: 3 > > + maxItems: 3 > > +power-domain-names: > > + items: > > +- const: core0 > > +- const: core1 > > +- const: core2 > > + > > + required: > > +- sram-supply > > +- power-domains > > +- power-domains-names > > > > examples: > >- | > > -- > > 2.29.2.729.g45daf8777d-goog > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso --- (no changes since v7) Changes in v7: - Fix GPU ID in commit message Changes in v6: - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Change power domain name from 2d to core2. Changes in v4: - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 83a461bdeea8..ca07098a6141 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = { .vendor_quirk = panfrost_gpu_amlogic_quirk, }; +const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; +static const struct panfrost_compatible mediatek_mt8183_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .supply_names = mediatek_mt8183_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = _data, }, { .compatible = "arm,mali-t880", .data = _data, }, { .compatible = "arm,mali-bifrost", .data = _data, }, + { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will require platform-specific handling for devfreq, for 2 reasons: 1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator) does not support multiple regulators, so we'll need custom handlers. 2. Generally, platforms with 2 regulators have platform-specific constraints on how the voltages should be set (e.g. minimum/maximum voltage difference between them), so we should not just create generic handlers that simply change the voltages without taking care of those constraints. Disable devfreq for now on those GPUs. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso --- Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message Changes in v6: - New change drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index f44d28fad085..812cfecdee3b 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = >pfdevfreq; + if (pfdev->comp->num_supplies > 1) { + /* +* GPUs with more than 1 supply require platform-specific handling: +* continue without devfreq +*/ + DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n"); + return 0; + } + opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, pfdev->comp->num_supplies); if (IS_ERR(opp_table)) { -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- (no changes since v6) Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..71b613ee5bd7 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -87,6 +88,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v9: - Explain why devfreq needs to be disabled for GPUs with >1 regulators. Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 25 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 161 insertions(+) -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
On Thu, Jan 7, 2021 at 11:59 PM Steven Price wrote: > > On 05/01/2021 00:11, Nicolas Boichat wrote: > > GPUs with more than a single regulator (e.g. G-57 on MT8183) will > > require platform-specific handling, disable devfreq for now. > > Can you explain what actually goes wrong here? AFAICT the existing code > does support controlling multiple regulators - but clearly this is the > first platform that exercises that code with num_supplies>1. Sure, I should have expanded in the commit message, will do in v9. We have support for >1 supplies, and we need to enable them to get the GPU running _at all_ (and the default voltages should be safe by design). For devfreq though: 1. There are constraints on the voltage difference between the core and SRAM, we have this caterpillar logic downstream [1], so somebody will need to port it (TBH I don't think it's overly critical at this point, as Bifrost support is still not very mature from what I can see, and devfreq is purely a performance thing). 2. The core [2] does not support multiple regulators, so we'll need custom code anyway. Even if we didn't have constraints. [1] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-4.19/drivers/gpu/arm/bifrost/platform/mediatek/mali_kbase_runtime_pm.c#367 [2] https://elixir.bootlin.com/linux/latest/source/drivers/opp/core.c#L679 > > Steve > > > > > Signed-off-by: Nicolas Boichat > > --- > > > > Changes in v6: > > - New change > > > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 + > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > index f44d28fad085..1f49043aae73 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > > struct thermal_cooling_device *cooling; > > struct panfrost_devfreq *pfdevfreq = >pfdevfreq; > > > > + if (pfdev->comp->num_supplies > 1) { > > + /* > > + * GPUs with more than 1 supply require platform-specific > > handling: > > + * continue without devfreq > > + */ > > + DRM_DEV_ERROR(dev, "More than 1 supply is not supported > > yet\n"); > > + return 0; > > + } > > + > > opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, > > pfdev->comp->num_supplies); > > if (IS_ERR(opp_table)) { > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso --- (no changes since v7) Changes in v7: - Fix GPU ID in commit message Changes in v6: - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Change power domain name from 2d to core2. Changes in v4: - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 83a461bdeea8..ca07098a6141 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = { .vendor_quirk = panfrost_gpu_amlogic_quirk, }; +const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; +static const struct panfrost_compatible mediatek_mt8183_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .supply_names = mediatek_mt8183_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = _data, }, { .compatible = "arm,mali-t880", .data = _data, }, { .compatible = "arm,mali-bifrost", .data = _data, }, + { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will require platform-specific handling, disable devfreq for now. Signed-off-by: Nicolas Boichat Reviewed-by: Tomeu Vizoso --- Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message Changes in v6: - New change drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index f44d28fad085..812cfecdee3b 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = >pfdevfreq; + if (pfdev->comp->num_supplies > 1) { + /* +* GPUs with more than 1 supply require platform-specific handling: +* continue without devfreq +*/ + DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n"); + return 0; + } + opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, pfdev->comp->num_supplies); if (IS_ERR(opp_table)) { -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- (no changes since v6) Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..71b613ee5bd7 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -87,6 +88,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v8: - Use DRM_DEV_INFO instead of ERROR Changes in v7: - Fix GPU ID in commit message - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 25 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 161 insertions(+) -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
On Thu, Jan 7, 2021 at 4:31 PM Tomeu Vizoso wrote: > > On 1/7/21 9:26 AM, Nicolas Boichat wrote: > > GPUs with more than a single regulator (e.g. G72 on MT8183) will > > require platform-specific handling, disable devfreq for now. > > > > Signed-off-by: Nicolas Boichat > > --- > > > > Changes in v7: > > - Fix GPU ID in commit message > > > > Changes in v6: > > - New change > > > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 + > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > index f44d28fad085..1f49043aae73 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > > struct thermal_cooling_device *cooling; > > struct panfrost_devfreq *pfdevfreq = >pfdevfreq; > > > > + if (pfdev->comp->num_supplies > 1) { > > + /* > > + * GPUs with more than 1 supply require platform-specific > > handling: > > + * continue without devfreq > > + */ > > + DRM_DEV_ERROR(dev, "More than 1 supply is not supported > > yet\n"); > > Should this be info instead? Sure, will fix in v8. > > Regards, > > Tomeu > > > + return 0; > > + } > > + > > opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, > > pfdev->comp->num_supplies); > > if (IS_ERR(opp_table)) { > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G72 Bifrost. Signed-off-by: Nicolas Boichat --- Changes in v7: - Fix GPU ID in commit message Changes in v6: - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Change power domain name from 2d to core2. Changes in v4: - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 83a461bdeea8..ca07098a6141 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = { .vendor_quirk = panfrost_gpu_amlogic_quirk, }; +const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; +static const struct panfrost_compatible mediatek_mt8183_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .supply_names = mediatek_mt8183_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = _data, }, { .compatible = "arm,mali-t880", .data = _data, }, { .compatible = "arm,mali-bifrost", .data = _data, }, + { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G72 on MT8183) will require platform-specific handling, disable devfreq for now. Signed-off-by: Nicolas Boichat --- Changes in v7: - Fix GPU ID in commit message Changes in v6: - New change drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index f44d28fad085..1f49043aae73 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = >pfdevfreq; + if (pfdev->comp->num_supplies > 1) { + /* +* GPUs with more than 1 supply require platform-specific handling: +* continue without devfreq +*/ + DRM_DEV_ERROR(dev, "More than 1 supply is not supported yet\n"); + return 0; + } + opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, pfdev->comp->num_supplies); if (IS_ERR(opp_table)) { -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- Changes in v7: - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..71b613ee5bd7 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -87,6 +88,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v7: - Fix GPU ID in commit message Changes in v6: - Rebased, actually tested with recent mesa driver. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 25 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 161 insertions(+) -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
On Tue, Jan 5, 2021 at 8:34 AM Alyssa Rosenzweig wrote: > > > GPUs with more than a single regulator (e.g. G-57 on MT8183) will > > G72 Duh, sorry, yes. I will fix that in v7. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 4/4] drm/panfrost: Add mt8183-mali compatible string
Add support for MT8183's G-57 Bifrost. Signed-off-by: Nicolas Boichat --- Changes in v6: - Context conflicts, reflow the code. - Use ARRAY_SIZE for power domains too. Changes in v5: - Change power domain name from 2d to core2. Changes in v4: - Add power domain names. Changes in v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 83a461bdeea8..ca07098a6141 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = { .vendor_quirk = panfrost_gpu_amlogic_quirk, }; +const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; +static const struct panfrost_compatible mediatek_mt8183_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .supply_names = mediatek_mt8183_supplies, + .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = _data, }, { .compatible = "arm,mali-t880", .data = _data, }, { .compatible = "arm,mali-bifrost", .data = _data, }, + { .compatible = "mediatek,mt8183-mali", .data = _mt8183_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G-57 on MT8183) will require platform-specific handling, disable devfreq for now. Signed-off-by: Nicolas Boichat --- Changes in v6: - New change drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index f44d28fad085..1f49043aae73 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = >pfdevfreq; + if (pfdev->comp->num_supplies > 1) { + /* +* GPUs with more than 1 supply require platform-specific handling: +* continue without devfreq +*/ + DRM_DEV_ERROR(dev, "More than 1 supply is not supported yet\n"); + return 0; + } + opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, pfdev->comp->num_supplies); if (IS_ERR(opp_table)) { -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- Changes in v6: - Rebased, actually tested with recent mesa driver. - No change Changes in v5: - Rename "2d" power domain to "core2" Changes in v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) Changes in v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 184492162e7e..71b613ee5bd7 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -87,6 +88,30 @@ allOf: then: required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 0/4] drm/panfrost: Add support for mt8183 GPU
Hi! Follow-up on the v5 [1], things have gotten significantly better in the last 9 months, thanks to the efforts on Bifrost support by the Collabora team (and probably others I'm not aware of). I've been testing this series on a MT8183/kukui device, with a chromeos-5.10 kernel [2], and got basic Chromium OS UI up with mesa 20.3.2 (lots of artifacts though). devfreq is currently not supported, as we'll need: - Clock core support for switching the GPU core clock (see 2/4). - Platform-specific handling of the 2-regulator (see 3/4). Since the latter is easy to detect, patch 3/4 just disables devfreq if the more than one regulator is specified in the compatible matching table. [1] https://patchwork.kernel.org/project/linux-mediatek/cover/20200306041345.259332-1-drink...@chromium.org/ [2] https://crrev.com/c/2608070 Changes in v6: - Rebased, actually tested with recent mesa driver. Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 drm/panfrost: Add mt8183-mali compatible string .../bindings/gpu/arm,mali-bifrost.yaml| 25 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 6 + .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 6 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++ drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++ 6 files changed, 161 insertions(+) -- 2.29.2.729.g45daf8777d-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3, 1/8] soc: mediatek: mmsys: create mmsys folder
On Mon, Dec 28, 2020 at 4:38 PM Yongqiang Niu wrote: > > the mmsys will more and more complicated after support > more and more SoCs, add an independent folder will be > more clear > > Signed-off-by: Yongqiang Niu > --- > drivers/soc/mediatek/Makefile | 2 +- > drivers/soc/mediatek/mmsys/Makefile| 2 + > drivers/soc/mediatek/mmsys/mtk-mmsys.c | 380 > + > drivers/soc/mediatek/mtk-mmsys.c | 380 > - I wonder why this doesn't get detected as a rename? > 4 files changed, 383 insertions(+), 381 deletions(-) > create mode 100644 drivers/soc/mediatek/mmsys/Makefile > create mode 100644 drivers/soc/mediatek/mmsys/mtk-mmsys.c > delete mode 100644 drivers/soc/mediatek/mtk-mmsys.c > > diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile > index 01f9f87..b5987ca 100644 > --- a/drivers/soc/mediatek/Makefile > +++ b/drivers/soc/mediatek/Makefile > @@ -3,4 +3,4 @@ obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o > obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o > obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o > obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o > -obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o > +obj-$(CONFIG_MTK_MMSYS) += mmsys/ > diff --git a/drivers/soc/mediatek/mmsys/Makefile > b/drivers/soc/mediatek/mmsys/Makefile > new file mode 100644 > index 000..5d976d7 > --- /dev/null > +++ b/drivers/soc/mediatek/mmsys/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o > \ No newline at end of file Nit: newline at end of file please. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2, 14/17] soc: mediatek: mmsys: Use function call for setting mmsys ovl mout register
On Sat, Dec 12, 2020 at 12:13 PM Yongqiang Niu wrote: > > Use function call for setting mmsys ovl mout register > > Signed-off-by: Yongqiang Niu > --- > drivers/soc/mediatek/mmsys/mtk-mmsys.c | 18 ++ > include/linux/soc/mediatek/mtk-mmsys.h | 3 +++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/soc/mediatek/mmsys/mtk-mmsys.c > b/drivers/soc/mediatek/mmsys/mtk-mmsys.c > index cb76e64..2558b42 100644 > --- a/drivers/soc/mediatek/mmsys/mtk-mmsys.c > +++ b/drivers/soc/mediatek/mmsys/mtk-mmsys.c > @@ -78,6 +78,15 @@ void mtk_mmsys_ddp_connect(struct device *dev, > reg = readl_relaxed(mmsys->regs + addr) | value; > writel_relaxed(reg, mmsys->regs + addr); > } > + > + if (!funcs->ovl_mout_en) > + return; > + > + value = funcs->ovl_mout_en(cur, next, ); > + if (value) { > + reg = readl_relaxed(mmsys->regs + addr) | value; > + writel_relaxed(reg, mmsys->regs + addr); > + } This is technically correct, but I'm afraid this may become and issue later if we have another function like ovl_mout_en. So maybe it's better to do: if (funcs->ovl_mout_en) { value = funcs->ovl_mout_en(cur, next, ); ... } Or another option: Create a new function static unsigned int mtk_mmsys_ovl_mout_en(...) { if (!funcs->ovl_mout_en) return 0; } and call that, following the same pattern as mtk_mmsys_ddp_mout_en/mtk_mmsys_ddp_sel_in? > } > EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect); > > @@ -103,6 +112,15 @@ void mtk_mmsys_ddp_disconnect(struct device *dev, > reg = readl_relaxed(mmsys->regs + addr) & ~value; > writel_relaxed(reg, mmsys->regs + addr); > } > + > + if (!funcs->ovl_mout_en) > + return; > + > + value = funcs->ovl_mout_en(cur, next, ); > + if (value) { > + reg = readl_relaxed(mmsys->regs + addr) & ~value; > + writel_relaxed(reg, mmsys->regs + addr); > + } > } > EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect); > > diff --git a/include/linux/soc/mediatek/mtk-mmsys.h > b/include/linux/soc/mediatek/mtk-mmsys.h > index aa4f60e..220203d 100644 > --- a/include/linux/soc/mediatek/mtk-mmsys.h > +++ b/include/linux/soc/mediatek/mtk-mmsys.h > @@ -49,6 +49,9 @@ struct mtk_mmsys_conn_funcs { > u32 (*mout_en)(enum mtk_ddp_comp_id cur, >enum mtk_ddp_comp_id next, >unsigned int *addr); > + u32 (*ovl_mout_en)(enum mtk_ddp_comp_id cur, > + enum mtk_ddp_comp_id next, > + unsigned int *addr); > u32 (*sel_in)(enum mtk_ddp_comp_id cur, > enum mtk_ddp_comp_id next, > unsigned int *addr); > -- > 1.8.1.1.dirty > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2, 07/17] drm/mediatek: add disp config and mm 26mhz clock into mutex device
On Sat, Dec 12, 2020 at 12:12 PM Yongqiang Niu wrote: > > there are 2 more clock need enable for display. > parser these clock when mutex device probe, > enable and disable when mutex on/off > > Signed-off-by: Yongqiang Niu > --- > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 49 > -- > 1 file changed, 41 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > index 60788c1..de618a1 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > @@ -118,7 +118,7 @@ struct mtk_ddp_data { > > struct mtk_ddp { > struct device *dev; > - struct clk *clk; > + struct clk *clk[3]; > void __iomem*regs; > struct mtk_disp_mutex mutex[10]; > const struct mtk_ddp_data *data; > @@ -257,14 +257,39 @@ int mtk_disp_mutex_prepare(struct mtk_disp_mutex *mutex) > { > struct mtk_ddp *ddp = container_of(mutex, struct mtk_ddp, >mutex[mutex->id]); > - return clk_prepare_enable(ddp->clk); > + int ret; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(ddp->clk); i++) { > + if (IS_ERR(ddp->clk[i])) > + continue; > + ret = clk_prepare_enable(ddp->clk[i]); > + if (ret) { > + pr_err("failed to enable clock, err %d. i:%d\n", > + ret, i); > + goto err; > + } > + } > + > + return 0; > + > +err: > + while (--i >= 0) > + clk_disable_unprepare(ddp->clk[i]); > + return ret; > } > > void mtk_disp_mutex_unprepare(struct mtk_disp_mutex *mutex) > { > struct mtk_ddp *ddp = container_of(mutex, struct mtk_ddp, >mutex[mutex->id]); > - clk_disable_unprepare(ddp->clk); > + int i; > + > +for (i = 0; i < ARRAY_SIZE(ddp->clk); i++) { > + if (IS_ERR(ddp->clk[i])) > + continue; > + clk_disable_unprepare(ddp->clk[i]); > + } > } > > void mtk_disp_mutex_add_comp(struct mtk_disp_mutex *mutex, > @@ -415,11 +440,19 @@ static int mtk_ddp_probe(struct platform_device *pdev) > ddp->data = of_device_get_match_data(dev); > > if (!ddp->data->no_clk) { > - ddp->clk = devm_clk_get(dev, NULL); > - if (IS_ERR(ddp->clk)) { > - if (PTR_ERR(ddp->clk) != -EPROBE_DEFER) > - dev_err(dev, "Failed to get clock\n"); > - return PTR_ERR(ddp->clk); > + int ret; > + > + for (i = 0; i < ARRAY_SIZE(ddp->clk); i++) { > + ddp->clk[i] = of_clk_get(dev->of_node, i); > + > + if (IS_ERR(ddp->clk[i])) { > + ret = PTR_ERR(ddp->clk[i]); > + if (ret != EPROBE_DEFER) > + dev_err(dev, "Failed to get clock > %d\n", > + ret); > + > + return ret; > + } Use of_clk_bulk_get_all instead? ddp->num_clks = of_clk_bulk_get_all(dev->of_node, >clks); ... Then the calls above can be clk_bulk_enable/clk_bulk_disable using num_clks and clks. > } > } > > -- > 1.8.1.1.dirty > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/1] drm/mediatek: dsi: fix scrolling of panel with small hfp or hbp
On Tue, Oct 13, 2020 at 6:06 PM Jitao Shi wrote: > > Replace horizontal_backporch_byte with vm->hback_porch * bpp to aovid > flowing judgement negative number. > > if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) > > data_phy_cycles * dsi->lanes + delta) > > Signed-off-by: Jitao Shi > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 65 > +++--- > 1 file changed, 25 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 80b7a082e874..f69ebeaf 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -445,6 +445,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) > u32 horizontal_backporch_byte; > u32 horizontal_frontporch_byte; > u32 dsi_tmp_buf_bpp, data_phy_cycles; > + u32 delta; > struct mtk_phy_timing *timing = >phy_timing; > > struct videomode *vm = >vm; > @@ -466,50 +467,34 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi > *dsi) > horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10); > > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) > - horizontal_backporch_byte = vm->hback_porch * dsi_tmp_buf_bpp; > + horizontal_backporch_byte = > + (vm->hback_porch * dsi_tmp_buf_bpp - 10); These parentheses are not required, but it might be a little clearer to write: (vm->hback_porch * dsi_tmp_buf_bpp) - 10; > else > - horizontal_backporch_byte = (vm->hback_porch + vm->hsync_len) > * > - dsi_tmp_buf_bpp; > + horizontal_backporch_byte = ((vm->hback_porch + > vm->hsync_len) * > + dsi_tmp_buf_bpp - 10); ditto: ((vm->hback_porch + vm->hsync_len) * dsi_tmp_buf_bpp) - 10; But then, _maybe_ it's clearer to drop this hunk and just add this below the if/else: horizontal_backporch_byte -= 10; > > data_phy_cycles = timing->lpx + timing->da_hs_prepare + > - timing->da_hs_zero + timing->da_hs_exit; > - > - if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) { > - if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp > > - data_phy_cycles * dsi->lanes + 18) { > - horizontal_frontporch_byte = > - vm->hfront_porch * dsi_tmp_buf_bpp - > - (data_phy_cycles * dsi->lanes + 18) * > - vm->hfront_porch / > - (vm->hfront_porch + vm->hback_porch); > - > - horizontal_backporch_byte = > - horizontal_backporch_byte - > - (data_phy_cycles * dsi->lanes + 18) * > - vm->hback_porch / > - (vm->hfront_porch + vm->hback_porch); > - } else { > - DRM_WARN("HFP less than d-phy, FPS will under > 60Hz\n"); > - horizontal_frontporch_byte = vm->hfront_porch * > -dsi_tmp_buf_bpp; > - } > + timing->da_hs_zero + timing->da_hs_exit + 3; (for reference, apart from this `+ 3`, there is no functional change in this hunk: this just moves delta outside of the if/else block, which is a good idea for readability) > + > + delta = (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) ? 18 : 12; > + > + if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) > > + data_phy_cycles * dsi->lanes + delta) { > + horizontal_frontporch_byte = > + vm->hfront_porch * dsi_tmp_buf_bpp - > + (data_phy_cycles * dsi->lanes + delta) * > + vm->hfront_porch / > + (vm->hfront_porch + vm->hback_porch); > + > + horizontal_backporch_byte = > + horizontal_backporch_byte - > + (data_phy_cycles * dsi->lanes + delta) * > + vm->hback_porch / > + (vm->hfront_porch + vm->hback_porch); > } else { > - if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp > > - data_phy_cycles * dsi->lanes + 12) { > - horizontal_frontporch_byte = > - vm->hfront_porch * dsi_tmp_buf_bpp - > - (data_phy_cycles * dsi->lanes + 12) * > - vm->hfront_porch / > - (vm->hfront_porch + vm->hback_porch); > - horizontal_backporch_byte = horizontal_backporch_byte > - > - (data_phy_cycles * dsi->lanes + 12) * > -
[PATCH v4 2/3] kernel/trace: Add TRACING_ALLOW_PRINTK config option
trace_printk is meant as a debugging tool, and should not be compiled into production code without specific debug Kconfig options enabled, or source code changes, as indicated by the warning that shows up on boot if any trace_printk is called: ** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE ** ** ** ** trace_printk() being used. Allocating extra memory. ** ** ** ** This means that this is a DEBUG kernel and it is ** ** unsafe for production use. ** If this option is set to n, the kernel will generate a build-time error if trace_printk is used. Note that the code to handle trace_printk is still present, so this does not prevent people from compiling out-of-tree kernel modules with the option set to =y, or BPF programs. Signed-off-by: Nicolas Boichat --- Changes since v2/v3: - Rebase only, v3 didn't exist as I just split out the other necessary patches. - Added patch 3/3 to fix atomisp_compat_css20.c Changes since v1: - Use static_assert instead of __static_assert (Jason Gunthorpe) - Fix issues that can be detected by this patch (running some randconfig in a loop, kernel test robot, or manual inspection), by: - Making some debug config options that use trace_printk depend on the new config option. - Adding 3 patches before this one. There is a question from Alexei whether the warning is warranted, and it's possibly too strongly worded, but the fact is, we do not want trace_printk to be sprinkled in kernel code by default, unless a very specific Kconfig command is enabled (or preprocessor macro). There's at least 3 reasons that I can come up with: 1. trace_printk introduces some overhead. 2. If the kernel keeps adding always-enabled trace_printk, it will be much harder for developers to make use of trace_printk for debugging. 3. People may assume that trace_printk is for debugging only, and may accidentally output sensitive data (theoritical at this stage). drivers/gpu/drm/i915/Kconfig.debug | 4 ++-- fs/f2fs/Kconfig| 1 + include/linux/kernel.h | 17 - kernel/trace/Kconfig | 10 ++ samples/Kconfig| 2 ++ 5 files changed, 31 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug index 1cb28c20807c59d..fa30f9bdc82311f 100644 --- a/drivers/gpu/drm/i915/Kconfig.debug +++ b/drivers/gpu/drm/i915/Kconfig.debug @@ -84,7 +84,7 @@ config DRM_I915_ERRLOG_GEM config DRM_I915_TRACE_GEM bool "Insert extra ftrace output from the GEM internals" depends on DRM_I915_DEBUG_GEM - select TRACING + depends on TRACING_ALLOW_PRINTK default n help Enable additional and verbose debugging output that will spam @@ -98,7 +98,7 @@ config DRM_I915_TRACE_GEM config DRM_I915_TRACE_GTT bool "Insert extra ftrace output from the GTT internals" depends on DRM_I915_DEBUG_GEM - select TRACING + depends on TRACING_ALLOW_PRINTK default n help Enable additional and verbose debugging output that will spam diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig index d13c5c6a978769b..d161d96cc1b7418 100644 --- a/fs/f2fs/Kconfig +++ b/fs/f2fs/Kconfig @@ -80,6 +80,7 @@ config F2FS_IO_TRACE bool "F2FS IO tracer" depends on F2FS_FS depends on FUNCTION_TRACER + depends on TRACING_ALLOW_PRINTK help F2FS IO trace is based on a function trace, which gathers process information and block IO patterns in the filesystem level. diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 500def620d8f493..7cf24fa16a479ed 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -720,10 +720,15 @@ do { \ #define trace_printk(fmt, ...) \ do { \ char ___STR[] = __stringify((__VA_ARGS__)); \ + \ + static_assert( \ + IS_ENABLED(CONFIG_TRACING_ALLOW_PRINTK),\ + "trace_printk called, please enable CONFIG_TRACING_ALLOW_PRINTK."); \ + \ if (sizeof(___STR) > 3) \ do_trace_printk(fmt, ##__VA_ARGS__);\ else\ - trace_puts(fmt);\ + do_trace_puts(fmt); \ } while (0) #define do_trace_printk(fmt, args...) \ @@ -772,6 +777,13 @@ int __trace_printk(unsigned long ip, const char
Re: [v7, PATCH 3/7] mtk-mmsys: add mt8183 mmsys support
On Thu, Jul 23, 2020 at 10:05 AM Yongqiang Niu wrote: > > add mt8183 mmsys support > > Feature: drm/mediatek > Signed-off-by: Yongqiang Niu > --- > drivers/soc/mediatek/mmsys/Makefile | 1 + > drivers/soc/mediatek/mmsys/mt8183-mmsys.c | 161 > ++ > drivers/soc/mediatek/mtk-mmsys.c | 1 + > 3 files changed, 163 insertions(+) > create mode 100644 drivers/soc/mediatek/mmsys/mt8183-mmsys.c > > diff --git a/drivers/soc/mediatek/mmsys/Makefile > b/drivers/soc/mediatek/mmsys/Makefile > index 33b0dab..62cfedf 100644 > --- a/drivers/soc/mediatek/mmsys/Makefile > +++ b/drivers/soc/mediatek/mmsys/Makefile > @@ -1,2 +1,3 @@ > # SPDX-License-Identifier: GPL-2.0-only > obj-y += mt2701-mmsys.o > +obj-y += mt8183-mmsys.o > diff --git a/drivers/soc/mediatek/mmsys/mt8183-mmsys.c > b/drivers/soc/mediatek/mmsys/mt8183-mmsys.c > new file mode 100644 > index 000..9d5f276 > --- /dev/null > +++ b/drivers/soc/mediatek/mmsys/mt8183-mmsys.c > @@ -0,0 +1,161 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// Copyright (c) 2020 MediaTek Inc. > + > +#include > +#include > +#include > +#include > +#include > + > +#define DISP_OVL0_MOUT_EN 0xf00 > +#define DISP_OVL0_2L_MOUT_EN 0xf04 > +#define DISP_OVL1_2L_MOUT_EN 0xf08 > +#define DISP_DITHER0_MOUT_EN 0xf0c > +#define DISP_PATH0_SEL_IN 0xf24 > +#define DISP_DSI0_SEL_IN 0xf2c > +#define DISP_DPI0_SEL_IN 0xf30 > +#define DISP_RDMA0_SOUT_SEL_IN 0xf50 > +#define DISP_RDMA1_SOUT_SEL_IN 0xf54 > + > +#define OVL0_MOUT_EN_OVL0_2L BIT(4) > +#define OVL0_2L_MOUT_EN_DISP_PATH0 BIT(0) > +#define OVL1_2L_MOUT_EN_RDMA1 BIT(4) > +#define DITHER0_MOUT_IN_DSI0 BIT(0) > +#define DISP_PATH0_SEL_IN_OVL0_2L 0x1 > +#define DSI0_SEL_IN_RDMA0 0x1 > +#define DSI0_SEL_IN_RDMA1 0x3 > +#define DPI0_SEL_IN_RDMA0 0x1 > +#define DPI0_SEL_IN_RDMA1 0x2 > +#define RDMA0_SOUT_COLOR0 0x1 > +#define RDMA1_SOUT_DSI00x1 > + > +struct mmsys_path_sel { > + enum mtk_ddp_comp_id cur; > + enum mtk_ddp_comp_id next; > + u32 addr; > + u32 val; > +}; > + > +static struct mmsys_path_sel mmsys_mout_en[] = { > + { > + DDP_COMPONENT_OVL0, DDP_COMPONENT_OVL_2L0, > + DISP_OVL0_MOUT_EN, OVL0_MOUT_EN_OVL0_2L, > + }, > + { > + DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0, > + DISP_OVL0_2L_MOUT_EN, OVL0_2L_MOUT_EN_DISP_PATH0, > + }, > + { > + DDP_COMPONENT_OVL_2L1, DDP_COMPONENT_RDMA1, > + DISP_OVL1_2L_MOUT_EN, OVL1_2L_MOUT_EN_RDMA1, > + }, > + { > + DDP_COMPONENT_DITHER, DDP_COMPONENT_DSI0, > + DISP_DITHER0_MOUT_EN, DITHER0_MOUT_IN_DSI0, > + }, > +}; > + > +static struct mmsys_path_sel mmsys_sel_in[] = { > + { > + DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0, > + DISP_PATH0_SEL_IN, DISP_PATH0_SEL_IN_OVL0_2L, > + }, > + { > + DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0, > + DISP_DPI0_SEL_IN, DPI0_SEL_IN_RDMA1, > + }, > +}; > + > +static struct mmsys_path_sel mmsys_sout_sel[] = { > + { > + DDP_COMPONENT_RDMA0, DDP_COMPONENT_COLOR0, > + DISP_RDMA0_SOUT_SEL_IN, RDMA0_SOUT_COLOR0, > + }, > +}; > + > +static unsigned int mtk_mmsys_ddp_mout_en(enum mtk_ddp_comp_id cur, > + enum mtk_ddp_comp_id next, > + unsigned int *addr) > +{ > + u32 i; > + u32 val = 0; > + struct mmsys_path_sel *path; > + > + for (i = 0; i < ARRAY_SIZE(mmsys_mout_en); i++) { > + path = _mout_en[i]; > + if (cur == path->cur && next == path->next) { > + *addr = path->addr; > + val = path->val; return path->val? > + break; > + } > + } > + > + return val; Then this becomes just return 0; > +} > + > +static unsigned int mtk_mmsys_ddp_sel_in(enum mtk_ddp_comp_id cur, > +enum mtk_ddp_comp_id next, > +unsigned int *addr) > +{ > + u32 i; > + u32 val = 0; > + struct mmsys_path_sel *path; > + > + for (i = 0; i < ARRAY_SIZE(mmsys_sel_in); i++) { > + path = _sel_in[i]; > + if (cur == path->cur && next == path->next) { > + *addr = path->addr; > + val = path->val; > + break; > + } > + } > + > + return val; ditto > +} > + > +static void mtk_mmsys_ddp_sout_sel(void __iomem
Re: [PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
On Thu, Jul 23, 2020 at 9:17 PM Felipe Balbi wrote: > > Nicolas Boichat writes: > > > trace_printk should not be used in production code, replace it > > call with dev_dbg. > > > > Signed-off-by: Nicolas Boichat > > > > --- > > > > Unclear why a trace_printk was used in the first place, it's > > possible that some rate-limiting is necessary here. > > > > drivers/usb/cdns3/gadget.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > > index 5e24c2e57c0d8c8..c303ab7c62d1651 100644 > > --- a/drivers/usb/cdns3/gadget.c > > +++ b/drivers/usb/cdns3/gadget.c > > @@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device > > *priv_dev, > > if ((priv_req->flags & REQUEST_INTERNAL) || > > (priv_ep->flags & EP_TDLCHK_EN) || > > priv_ep->use_streams) { > > - trace_printk("Blocking external request\n"); > > + dev_dbg(priv_dev->dev, "Blocking external request\n"); > > Instead, I would suggest adding a proper trace event here; one that > includes "priv_ep->flags" in the output. The patch was already merged by Greg (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/usb/cdns3/gadget.c?id=b3a5ce874c2619c9b8a6c5bbcfefdb95e0227600), but feel free to do that as a follow-up CL. Looks like Peter -- the main author, is ok with dev_dbg (also, apologies for missing the R-b tag when I sent a v2 -- which is the one that was merged by Greg). Thanks, > > -- > balbi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v13 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP
On Tue, Jun 9, 2020 at 3:20 PM Xin Ji wrote: > > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed > for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K. > > Signed-off-by: Xin Ji > --- > drivers/gpu/drm/bridge/analogix/Kconfig |9 + > drivers/gpu/drm/bridge/analogix/Makefile |1 + > drivers/gpu/drm/bridge/analogix/anx7625.c | 1999 > + > drivers/gpu/drm/bridge/analogix/anx7625.h | 397 ++ > 4 files changed, 2406 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h > > [snip] > +static int anx7625_parse_dt(struct device *dev, > + struct anx7625_platform_data *pdata) > +{ > + struct device_node *np = dev->of_node; > + struct device_node *panel_node, *out_ep; > + > + pdata->node.mipi_dsi_host_node = of_graph_get_remote_node(np, 0, 0); > + if (!pdata->node.mipi_dsi_host_node) { > + DRM_DEV_ERROR(dev, "fail to get internal panel.\n"); > + return -EPROBE_DEFER; This does not look correct. I don't think of_graph_get_remote_node will ever return NULL if the device tree is configured properly, and it's useless to retry later (EPROBE_DEFER). You should just fail (e.g. return EINVAL). > + } > + > + of_node_put(pdata->node.mipi_dsi_host_node); You are using pdata->node.mipi_dsi_host_node in other places in the code, so I don't think it's ok to call of_node_put? > + DRM_DEV_DEBUG_DRIVER(dev, "found dsi host node.\n"); > + > + pdata->node.panel_node = of_graph_get_port_by_id(np, 1); > + if (!pdata->node.panel_node) { > + DRM_DEV_ERROR(dev, "fail to get panel node.\n"); > + return -EPROBE_DEFER; -EINVAL. > + } > + > + of_node_put(pdata->node.panel_node); > + out_ep = of_get_child_by_name(pdata->node.panel_node, > + "endpoint"); > + if (!out_ep) { > + DRM_DEV_DEBUG_DRIVER(dev, "cannot get endpoint.\n"); DRM_DEV_ERROR seems more appropriate > + return -EPROBE_DEFER; -EINVAL > + } > + > + panel_node = of_graph_get_remote_port_parent(out_ep); > + of_node_put(out_ep); > + pdata->panel = of_drm_find_panel(panel_node); > + DRM_DEV_DEBUG_DRIVER(dev, "get panel node.\n"); > + > + of_node_put(panel_node); > + if (IS_ERR_OR_NULL(pdata->panel)) > + return -EPROBE_DEFER; of_drm_find_panel cannot return NULL, so, do this instead: if (IS_ERR(pdata->panel)) return PTR_ERR(pdata->panel); (which actually _may_ return EPROBE_DEFER) > + > + return 0; > +} > [snip] > +static int anx7625_i2c_probe(struct i2c_client *client, > +const struct i2c_device_id *id) > +{ > + struct anx7625_data *platform; > + struct anx7625_platform_data *pdata; > + int ret = 0; > + struct device *dev = >dev; > + > + if (!i2c_check_functionality(client->adapter, > +I2C_FUNC_SMBUS_I2C_BLOCK)) { > + DRM_DEV_ERROR(dev, "anx7625's i2c bus doesn't support\n"); > + return -ENODEV; > + } > + > + platform = kzalloc(sizeof(*platform), GFP_KERNEL); > + if (!platform) { > + DRM_DEV_ERROR(dev, "fail to allocate driver data\n"); > + return -ENOMEM; > + } > + > + pdata = >pdata; > + > + ret = anx7625_parse_dt(dev, pdata); > + if (ret) { > + DRM_DEV_ERROR(dev, "fail to parse devicetree.\n"); Please do not print this error (or at least not if err == -EPROBE_DEFER). > + goto free_platform; > + } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] media: camss: vfe: Use trace_printk for debugging only
trace_printk should not be used in production code. Since tracing interrupts is presumably latency sensitive, pr_dbg is not appropriate, so guard the call with a preprocessor symbol that can be defined for debugging purpose. Signed-off-by: Nicolas Boichat --- drivers/media/platform/qcom/camss/camss-vfe-4-1.c | 2 ++ drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c index 174a36be6f5d866..0c57171fae4f9e9 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c @@ -936,8 +936,10 @@ static irqreturn_t vfe_isr(int irq, void *dev) vfe->ops->isr_read(vfe, , ); +#ifdef CAMSS_VFE_TRACE_IRQ trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n", value0, value1); +#endif if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK) vfe->isr_ops.reset_ack(vfe); diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c index 0dca8bf9281e774..307675925e5c779 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c @@ -1058,8 +1058,10 @@ static irqreturn_t vfe_isr(int irq, void *dev) vfe->ops->isr_read(vfe, , ); +#ifdef CAMSS_VFE_TRACE_IRQ trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n", value0, value1); +#endif if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK) vfe->isr_ops.reset_ack(vfe); -- 2.27.0.212.ge8ba1cc988-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/4] kernel/trace: Add TRACING_ALLOW_PRINTK config option
trace_printk is meant as a debugging tool, and should not be compiled into production code without specific debug Kconfig options enabled, or source code changes, as indicated by the warning that shows up on boot if any trace_printk is called: ** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE ** ** ** ** trace_printk() being used. Allocating extra memory. ** ** ** ** This means that this is a DEBUG kernel and it is ** ** unsafe for production use. ** If this option is set to n, the kernel will generate a build-time error if trace_printk is used. Note that the code to handle trace_printk is still present, so this does not prevent people from compiling out-of-tree kernel modules with the option set to =y, or BPF programs. Signed-off-by: Nicolas Boichat --- Changes since v1: - Use static_assert instead of __static_assert (Jason Gunthorpe) - Fix issues that can be detected by this patch (running some randconfig in a loop, kernel test robot, or manual inspection), by: - Making some debug config options that use trace_printk depend on the new config option. - Adding 3 patches before this one. There is a question from Alexei whether the warning is warranted, and it's possibly too strongly worded, but the fact is, we do not want trace_printk to be sprinkled in kernel code by default, unless a very specific Kconfig command is enabled (or preprocessor macro). There's at least 3 reasons that I can come up with: 1. trace_printk introduces some overhead. 2. If the kernel keeps adding always-enabled trace_printk, it will be much harder for developers to make use of trace_printk for debugging. 3. People may assume that trace_printk is for debugging only, and may accidentally output sensitive data (theoritical at this stage). drivers/gpu/drm/i915/Kconfig.debug | 4 ++-- fs/f2fs/Kconfig| 1 + include/linux/kernel.h | 17 - kernel/trace/Kconfig | 10 ++ samples/Kconfig| 2 ++ 5 files changed, 31 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug index 1cb28c20807c59d..fa30f9bdc82311f 100644 --- a/drivers/gpu/drm/i915/Kconfig.debug +++ b/drivers/gpu/drm/i915/Kconfig.debug @@ -84,7 +84,7 @@ config DRM_I915_ERRLOG_GEM config DRM_I915_TRACE_GEM bool "Insert extra ftrace output from the GEM internals" depends on DRM_I915_DEBUG_GEM - select TRACING + depends on TRACING_ALLOW_PRINTK default n help Enable additional and verbose debugging output that will spam @@ -98,7 +98,7 @@ config DRM_I915_TRACE_GEM config DRM_I915_TRACE_GTT bool "Insert extra ftrace output from the GTT internals" depends on DRM_I915_DEBUG_GEM - select TRACING + depends on TRACING_ALLOW_PRINTK default n help Enable additional and verbose debugging output that will spam diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig index d13c5c6a978769b..d161d96cc1b7418 100644 --- a/fs/f2fs/Kconfig +++ b/fs/f2fs/Kconfig @@ -80,6 +80,7 @@ config F2FS_IO_TRACE bool "F2FS IO tracer" depends on F2FS_FS depends on FUNCTION_TRACER + depends on TRACING_ALLOW_PRINTK help F2FS IO trace is based on a function trace, which gathers process information and block IO patterns in the filesystem level. diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 196607aaf653082..8abce95b0c95a0e 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -721,10 +721,15 @@ do { \ #define trace_printk(fmt, ...) \ do { \ char ___STR[] = __stringify((__VA_ARGS__)); \ + \ + static_assert( \ + IS_ENABLED(CONFIG_TRACING_ALLOW_PRINTK),\ + "trace_printk called, please enable CONFIG_TRACING_ALLOW_PRINTK."); \ + \ if (sizeof(___STR) > 3) \ do_trace_printk(fmt, ##__VA_ARGS__);\ else\ - trace_puts(fmt);\ + do_trace_puts(fmt); \ } while (0) #define do_trace_printk(fmt, args...) \ @@ -773,6 +778,13 @@ int __trace_printk(unsigned long ip, const char *fmt, ...); */ #define trace_puts(str) ({ \ + static_assert(
[PATCH 2/4] media: atomisp: Replace trace_printk by pr_info
trace_printk should not be used in production code, replace it call with pr_info. Signed-off-by: Nicolas Boichat --- drivers/staging/media/atomisp/pci/hmm/hmm.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c index 42fef17798622f1..2bd39b4939f16d2 100644 --- a/drivers/staging/media/atomisp/pci/hmm/hmm.c +++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c @@ -735,11 +735,11 @@ ia_css_ptr hmm_host_vaddr_to_hrt_vaddr(const void *ptr) void hmm_show_mem_stat(const char *func, const int line) { - trace_printk("tol_cnt=%d usr_size=%d res_size=%d res_cnt=%d sys_size=%d dyc_thr=%d dyc_size=%d.\n", -hmm_mem_stat.tol_cnt, -hmm_mem_stat.usr_size, hmm_mem_stat.res_size, -hmm_mem_stat.res_cnt, hmm_mem_stat.sys_size, -hmm_mem_stat.dyc_thr, hmm_mem_stat.dyc_size); + pr_info("tol_cnt=%d usr_size=%d res_size=%d res_cnt=%d sys_size=%d dyc_thr=%d dyc_size=%d.\n", + hmm_mem_stat.tol_cnt, + hmm_mem_stat.usr_size, hmm_mem_stat.res_size, + hmm_mem_stat.res_cnt, hmm_mem_stat.sys_size, + hmm_mem_stat.dyc_thr, hmm_mem_stat.dyc_size); } void hmm_init_mem_stat(int res_pgnr, int dyc_en, int dyc_pgnr) -- 2.27.0.212.ge8ba1cc988-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
trace_printk should not be used in production code, replace it call with dev_dbg. Signed-off-by: Nicolas Boichat --- Unclear why a trace_printk was used in the first place, it's possible that some rate-limiting is necessary here. drivers/usb/cdns3/gadget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index 5e24c2e57c0d8c8..c303ab7c62d1651 100644 --- a/drivers/usb/cdns3/gadget.c +++ b/drivers/usb/cdns3/gadget.c @@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device *priv_dev, if ((priv_req->flags & REQUEST_INTERNAL) || (priv_ep->flags & EP_TDLCHK_EN) || priv_ep->use_streams) { - trace_printk("Blocking external request\n"); + dev_dbg(priv_dev->dev, "Blocking external request\n"); return ret; } } -- 2.27.0.212.ge8ba1cc988-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/4] Detect and remove trace_printk
trace_printk is meant as a debugging tool, and should not be compiled into production code without specific debug Kconfig options enabled or source code changes. Patches 1 to 3 remove/disable trace_printk that should not be enabled by default. Patch 4 adds a config option that can be used to detect such trace_printk at compile time (instead of printing a nasty warning at runtime). Nicolas Boichat (4): usb: cdns3: gadget: Replace trace_printk by dev_dbg media: atomisp: Replace trace_printk by pr_info media: camss: vfe: Use trace_printk for debugging only kernel/trace: Add TRACING_ALLOW_PRINTK config option drivers/gpu/drm/i915/Kconfig.debug | 4 ++-- .../media/platform/qcom/camss/camss-vfe-4-1.c | 2 ++ .../media/platform/qcom/camss/camss-vfe-4-7.c | 2 ++ drivers/staging/media/atomisp/pci/hmm/hmm.c | 10 +- drivers/usb/cdns3/gadget.c | 2 +- fs/f2fs/Kconfig | 1 + include/linux/kernel.h | 17 - kernel/trace/Kconfig| 10 ++ samples/Kconfig | 2 ++ 9 files changed, 41 insertions(+), 9 deletions(-) -- 2.27.0.212.ge8ba1cc988-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
On Mon, Apr 27, 2020 at 2:18 PM Xin Ji wrote: > > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed > for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K. > > The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI > to DP feature. This driver only enabled MIPI DSI/DPI to DP feature. > > Signed-off-by: Xin Ji > --- > drivers/gpu/drm/bridge/Makefile |2 +- > drivers/gpu/drm/bridge/analogix/Kconfig |6 + > drivers/gpu/drm/bridge/analogix/Makefile |1 + > drivers/gpu/drm/bridge/analogix/anx7625.c | 2158 > + > drivers/gpu/drm/bridge/analogix/anx7625.h | 410 ++ > 5 files changed, 2576 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 4934fcf..bcd388a 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -12,8 +12,8 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o > obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o > obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o > obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o > -obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ > obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o > obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o > +obj-y += analogix/ > obj-y += synopsys/ > diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig > b/drivers/gpu/drm/bridge/analogix/Kconfig > index e930ff9..b2f127e 100644 > --- a/drivers/gpu/drm/bridge/analogix/Kconfig > +++ b/drivers/gpu/drm/bridge/analogix/Kconfig > @@ -2,3 +2,9 @@ > config DRM_ANALOGIX_DP > tristate > depends on DRM > + > +config ANALOGIX_ANX7625 > + tristate "Analogix MIPI to DP interface support" > + help > + ANX7625 is an ultra-low power 4K mobile HD transmitter > designed > + for portable devices. It converts MIPI/DPI to DisplayPort1.3 > 4K. > diff --git a/drivers/gpu/drm/bridge/analogix/Makefile > b/drivers/gpu/drm/bridge/analogix/Makefile > index fdbf3fd..8a52867 100644 > --- a/drivers/gpu/drm/bridge/analogix/Makefile > +++ b/drivers/gpu/drm/bridge/analogix/Makefile > @@ -1,3 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0-only > +obj-$(CONFIG_ANALOGIX_ANX7625) += anx7625.o > analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > b/drivers/gpu/drm/bridge/analogix/anx7625.c > new file mode 100644 > index 000..fff7a49 > [snip] > +static int anx7625_attach_dsi(struct anx7625_data *ctx) > +{ > + struct mipi_dsi_host *host; > + struct mipi_dsi_device *dsi; > + struct device_node *mipi_host_node; > + struct device *dev = >client->dev; > + const struct mipi_dsi_device_info info = { > + .type = "anx7625", > + .channel = 0, > + .node = NULL, > + }; > + > + DRM_DEV_DEBUG_DRIVER(dev, "attach dsi\n"); > + > + if (ctx->pdata.dsi_supported) > + mipi_host_node = ctx->pdata.node.mipi_dsi_host_node; > + else > + mipi_host_node = ctx->pdata.node.mipi_dpi_host_node; > + > + if (!mipi_host_node) { > + DRM_ERROR("dsi host is not configured.\n"); > + return -EINVAL; > + } > + > + host = of_find_mipi_dsi_host_by_node(mipi_host_node); I tried this driver when connected to a dpi interface, and this fails, as of_find_mipi_dsi_host_by_node is not able to find the dpi interface from the SoC. I'm not too familiar with how dpi bridges are supposed to work in the kernel, but should we even call "anx7625_attach_dsi" for DPI interface? > + if (!host) { > + DRM_ERROR("failed to find dsi host.\n"); > + return -EINVAL; > + } > + > + dsi = mipi_dsi_device_register_full(host, ); > + if (IS_ERR(dsi)) { > + DRM_ERROR("failed to create dsi device.\n"); > + return -EINVAL; > + } > + > + dsi->lanes = 4; > + dsi->format = MIPI_DSI_FMT_RGB888; > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > + MIPI_DSI_MODE_VIDEO_SYNC_PULSE | > + MIPI_DSI_MODE_EOT_PACKET| > + MIPI_DSI_MODE_VIDEO_HSE; > + > + if (mipi_dsi_attach(dsi) < 0) { > + DRM_ERROR("failed to attach dsi to host.\n"); > + mipi_dsi_device_unregister(dsi); > + return -EINVAL; > + } > + > + ctx->dsi = dsi; > + > + DRM_DEV_DEBUG_DRIVER(dev, "attach dsi succeeded.\n"); > + > + return 0; > +} > + > [snip] > +static int anx7625_bridge_attach(struct drm_bridge *bridge) > +{ > + struct anx7625_data *ctx = bridge_to_anx7625(bridge); > + int err; > + struct device
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
On Fri, Apr 24, 2020 at 2:51 PM Xin Ji wrote: > > On Thu, Apr 23, 2020 at 07:55:15PM +0800, Nicolas Boichat wrote: > > Hi, > > > > Just commenting on the mode_fixup function that was added in v7. > > > [snip] > > > + /* > > > +* once illegal timing detected, use default HFP, HSYNC, HBP > > > +*/ > > > + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) { > > > > should this be adj_hblanking/adj_hfp/adj_hbp? > NO, need check original HFP and HBP, if they are not legal, driver need > set default value to adj_hsync, adj_hfp, adj_hbp. > > > > > + adj_hsync = SYNC_LEN_DEF; > > > + adj_hfp = HFP_HBP_DEF; > > > + adj_hbp = HFP_HBP_DEF; > > > + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal); > > > + if (hblanking < HBLANKING_MIN) { > > > + delta_adj = HBLANKING_MIN - hblanking; > > > + adj_clock = vref * delta_adj * adj->vtotal; > > > + adj->clock += DIV_ROUND_UP(adj_clock, 1000); > > > + } else { > > > + delta_adj = hblanking - HBLANKING_MIN; > > > + adj_clock = vref * delta_adj * adj->vtotal; > > > + adj->clock -= DIV_ROUND_UP(adj_clock, 1000); > > > + } > > > + > > > + DRM_WARN("illegal hblanking timing, use default.\n"); > > > + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync); > > > > How likely is it that this mode is going to work? Can you just return > > false here to reject the mode? > We want to set the default minimal Hblancking value, then it may display, > otherwise. If we just return false, there is no display for sure. Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Hi, Just commenting on the mode_fixup function that was added in v7. On Tue, Feb 25, 2020 at 2:15 PM Xin Ji wrote: > > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed > for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K. > > The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI > to DP feature. This driver only enabled MIPI DSI/DPI to DP feature. > > Signed-off-by: Xin Ji > --- > drivers/gpu/drm/bridge/Makefile |2 +- > drivers/gpu/drm/bridge/analogix/Kconfig |6 + > drivers/gpu/drm/bridge/analogix/Makefile |1 + > drivers/gpu/drm/bridge/analogix/anx7625.c | 2172 > + > drivers/gpu/drm/bridge/analogix/anx7625.h | 410 ++ > 5 files changed, 2590 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 4934fcf..bcd388a 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile [snip] > +static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, > + const struct drm_display_mode *mode, > + struct drm_display_mode *adj) > +{ > + struct anx7625_data *ctx = bridge_to_anx7625(bridge); > + struct device *dev = >client->dev; > + u32 hsync, hfp, hbp, hactive, hblanking; > + u32 adj_hsync, adj_hfp, adj_hbp, adj_hblanking, delta_adj; > + u32 vref, adj_clock; > + > + DRM_DEV_DEBUG_DRIVER(dev, "drm mode fixup set\n"); > + > + mutex_lock(>lock); Why do you need this lock? > + > + hactive = mode->hdisplay; This is never used, drop it? > + hsync = mode->hsync_end - mode->hsync_start; > + hfp = mode->hsync_start - mode->hdisplay; > + hbp = mode->htotal - mode->hsync_end; > + hblanking = mode->htotal - mode->hdisplay; > + > + DRM_DEV_DEBUG_DRIVER(dev, "before mode fixup\n"); > + DRM_DEV_DEBUG_DRIVER(dev, "hsync(%d),hfp(%d),hbp(%d),clock(%d)\n", > +hsync, > +hfp, > +hbp, > +adj->clock); > + DRM_DEV_DEBUG_DRIVER(dev, > "hsync_start(%d),hsync_end(%d),htotal(%d)\n", > +adj->hsync_start, > +adj->hsync_end, > +adj->htotal); > + > + adj_hfp = hfp; > + adj_hsync = hsync; > + adj_hbp = hbp; > + adj_hblanking = hblanking; > + > + /* plus 1 if hfp is odd */ A better way to word these comments is to say "hfp needs to be even", otherwise, you're just repeating what we can already see in the code. > + if (hfp & 0x1) { > + adj_hfp = hfp + 1; adj_hfp -= 1 for consistency? > + adj_hblanking += 1; > + } > + > + /* minus 1 if hbp is odd */ > + if (hbp & 0x1) { > + adj_hbp = hbp - 1; ditto, adj_hbp -= 1; > + adj_hblanking -= 1; > + } > + > + /* plus 1 if hsync is odd */ > + if (hsync & 0x1) { > + if (adj_hblanking < hblanking) > + adj_hsync = hsync + 1; ditto > + else > + adj_hsync = hsync - 1; ditto > + } > + > + /* > +* once illegal timing detected, use default HFP, HSYNC, HBP > +*/ > + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) { should this be adj_hblanking/adj_hfp/adj_hbp? > + adj_hsync = SYNC_LEN_DEF; > + adj_hfp = HFP_HBP_DEF; > + adj_hbp = HFP_HBP_DEF; > + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal); > + if (hblanking < HBLANKING_MIN) { > + delta_adj = HBLANKING_MIN - hblanking; > + adj_clock = vref * delta_adj * adj->vtotal; > + adj->clock += DIV_ROUND_UP(adj_clock, 1000); > + } else { > + delta_adj = hblanking - HBLANKING_MIN; > + adj_clock = vref * delta_adj * adj->vtotal; > + adj->clock -= DIV_ROUND_UP(adj_clock, 1000); > + } > + > + DRM_WARN("illegal hblanking timing, use default.\n"); > + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync); How likely is it that this mode is going to work? Can you just return false here to reject the mode? > + } else if (adj_hfp < HP_MIN) { > + /* adjust hfp if hfp less than HP_MIN */ > + delta_adj = HP_MIN - adj_hfp; > + adj_hfp = HP_MIN; > + > + /* > +* balance total HBlanking pixel, if HBP hasn't enough space, "does not have enough space" > +* adjust HSYNC length, otherwize
Re: [PATCH] drm/panel: boe-tv101wum-n16: extend LCD support list
On Wed, Mar 25, 2020 at 4:17 PM David Lu wrote: > No very strong opinion, but for consistency, I'd have this as a title: drm/panel: support for boe,tv105wum-nw0 dsi video mode panel > Add entries for BOE TV105WUM-NW0 10.5" WUXGA TFT LCD panel. > > Signed-off-by: David Lu > Change-Id: I5b1cef259de5fb498220dcc47baa035083c41667 No Change-Id please. Otherwise: Reviewed-by: Nicolas Boichat > --- > .../gpu/drm/panel/panel-boe-tv101wum-nl6.c| 31 +++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c > b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c > index 48a164257d18..f89861c8598a 100644 > --- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c > +++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c > @@ -696,6 +696,34 @@ static const struct panel_desc auo_b101uan08_3_desc = { > .init_cmds = auo_b101uan08_3_init_cmd, > }; > > +static const struct drm_display_mode boe_tv105wum_nw0_default_mode = { > + .clock = 159260, > + .hdisplay = 1200, > + .hsync_start = 1200 + 80, > + .hsync_end = 1200 + 80 + 24, > + .htotal = 1200 + 80 + 24 + 60, > + .vdisplay = 1920, > + .vsync_start = 1920 + 10, > + .vsync_end = 1920 + 10 + 2, > + .vtotal = 1920 + 10 + 2 + 14, > + .vrefresh = 60, > + .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED, > +}; > + > +static const struct panel_desc boe_tv105wum_nw0_desc = { > + .modes = _tv105wum_nw0_default_mode, > + .bpc = 8, > + .size = { > + .width_mm = 141, > + .height_mm = 226, > + }, > + .lanes = 4, > + .format = MIPI_DSI_FMT_RGB888, > + .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | > + MIPI_DSI_MODE_LPM, > + .init_cmds = boe_init_cmd, > +}; > + > static int boe_panel_get_modes(struct drm_panel *panel, >struct drm_connector *connector) > { > @@ -834,6 +862,9 @@ static const struct of_device_id boe_of_match[] = { > { .compatible = "auo,b101uan08.3", > .data = _b101uan08_3_desc > }, > + { .compatible = "boe,tv105wum-nw0", > + .data = _tv105wum_nw0_desc > + }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, boe_of_match); > -- > 2.24.1 > > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 7/7] RFC: drm/panfrost: devfreq: Add support for 2 regulators
Looping back on this, after digging a bit deeper... On Fri, Feb 14, 2020 at 9:38 AM Nick Fan (范哲維) wrote: > [snip] > > > Another thing that I'm not implementing is the dance that Mediatek > > > does in their kbase driver when changing the clock (described in > > > patch > > > 2/7): > > > "" > > > The binding we use with out-of-tree Mali drivers includes more > > > clocks, this is used for devfreq: the out-of-tree driver switches > > > clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then > > > switches clk_mux back to clk_main_parent: > > > (see > > > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ch > > > romeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_run > > > time_pm.c#423) > > > clocks = > > > < CLK_TOP_MFGPLL_CK>, > > > < CLK_TOP_MUX_MFG>, > > > <>, > > > < CLK_MFG_BG3D>; > > > clock-names = > > > "clk_main_parent", > > > "clk_mux", > > > "clk_sub_parent", > > > "subsys_mfg_cg"; > > > "" > > > Is there a clean/simple way to implement this in the clock > > > framework/device tree? Or should we implement something in the > > > panfrost driver? > > > > Putting parent clocks into 'clocks' for a device is a pretty common > > abuse. The 'assigned-clocks' binding is what's used for parent clock > > setup. Not sure that's going to help here though. Is this dance > > because the parent clock frequency can't be changed cleanly? > > Nick/Weiyi, any idea why we do that dance in the first place? (maybe the PLL > clock is unstable while it's being changed?) > > Clock source may become unstable during clock frequency changes, so it is > always safer to switch to a more reliable clock source. > Otherwise, it may cause some problem in some corner case. > I would suggest to keep it. The Mediatek CPUfreq driver actually does a very similar dance: https://github.com/torvalds/linux/blob/master/drivers/cpufreq/mediatek-cpufreq.c#L249 What they have in the device tree is the main clock, and the "intermediate" clock that is required during switching: clocks = < CLK_MCU_MP0_SEL>, < CLK_TOP_ARMPLL_DIV_PLL1>; clock-names = "cpu", "intermediate"; The topology looks like this: clk26m 15 1512600 0 0 5 armpll_ll 110 141700 0 0 5 mcu_mp0_sel000 141700 0 0 5 And device tree provides mcu_mp0_sel as "cpu", and the armpll_div_pll1 as "intermediate". The driver looks up armpll_ll by calling get_parent, then: - set_parent(mcu_mp0_sel, armpll_div_pll1) - set_rate(armpll_ll, new_rate) - set_parent(mcu_mp0_sel, armpll_ll) On MT8183's GPU, the topology is a little bit more complicated (but I think there should be a way to merge mfg_bg3d an mfg_sel in the clock core) clk26m 15 1512600 0 0 5 mfgpll110 41817 0 0 5 mfgpll_ck 220 41817 0 0 5 mfg_sel 330 41817 0 0 5 mfg_bg3d 110 41817 0 0 5 We're going to need a special panfrost devfreq driver for mt8183 anyway (to handle the 2 regulators), so it would be easy to take a similar approach: - Add "intermediate" clock in the device tree (clk26m) - Find mfg_sel/mfgpll_ck using 1/2 clk_get_parent calls. - Switch mfg_sel to clk26m, set mfgpll_ck rate, switch mfg_sel back to mfgpll_ck. (BTW, I tried to look, and couldn't find examples or reparenting during clock changes in drivers/clk, are there existing drivers doing similar things? Or this would be new?). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] drm/bridge: anx7688: Add anx7688 bridge driver support
On Fri, Mar 6, 2020 at 8:07 PM Ondřej Jirman wrote: > > On Fri, Mar 06, 2020 at 04:53:46PM +0800, Icenowy Zheng wrote: > > 在 2020-03-06星期五的 09:46 +0100,Enric Balletbo i Serra写道: > > > Hi Ondrej, > > > > > > On 5/3/20 20:35, Ondřej Jirman wrote: > > > > Hi, > > > > > > > > On Thu, Mar 05, 2020 at 10:29:33AM -0800, Vasily Khoruzhick wrote: > > > > > On Thu, Mar 5, 2020 at 7:28 AM Enric Balletbo i Serra > > > > > wrote: > > > > > > Hi Vasily, > > > > > > > > > > CC: Icenowy and Ondrej > > > > > > Would you mind to check which firmware version is running the > > > > > > anx7688 in > > > > > > PinePhone, I think should be easy to check with i2c-tools. > > > > > > > > > > Icenowy, Ondrej, can you guys please check anx7688 firmware > > > > > version? > > > > > > > > i2cget 0 0x28 0x00 w > > > > 0x > > > > > > > > i2cget 0 0x28 0x02 w > > > > 0x7688 > > > > > > > > i2cget 0 0x28 0x80 w > > > > 0x > > > > > > > > > > Can you check the value for 0x81 too? > > > > root@ice-pinephone [ ~ ] # i2cdump 0 0x28 > > No size specified (using byte-data access) > > WARNING! This program can confuse your I2C bus, cause data loss and > > worse! > > I will probe file /dev/i2c-0, address 0x28, mode byte > > Continue? [Y/n] > > 0 1 2 3 4 5 6 7 8 9 a b c d e f0123456789abcdef > > 00: aa aa 88 76 ac 00 00 00 00 00 00 00 00 00 05 05???v?.?? > > 10: 30 00 00 00 00 00 00 00 00 00 00 00 00 00 00 000... > > 20: 00 00 00 00 00 00 00 00 00 00 00 24 f2 e4 ff 00...$??.. > > 30: 06 40 00 04 94 11 20 ff ff 03 00 bf ff ff 10 01?@.??? ..?.?..?? > > 40: 72 a4 00 09 00 08 05 84 15 40 17 00 00 0a 00 e0r?.?.@?..?.? > > 50: 00 00 00 0a 10 00 e0 df ff ff 00 00 00 10 71 00...??.??.?q. > > 60: 10 10 04 29 2d 21 10 01 09 13 00 03 e8 13 88 00???)-!.. > > 70: 00 19 18 83 16 5c 11 00 ff 00 00 0d 04 38 42 07.\???8B? > > 80: 00 00 00 00 00 74 1b 19 44 08 75 00 00 00 00 00.t??D?u. > > 90: 01 02 00 00 00 00 03 00 ff 30 00 59 01 00 00 00???..0.Y?... > > a0: 00 ff fe ff ff 00 00 00 00 00 00 00 00 00 00 02..?? > > b0: 00 00 00 00 00 00 40 00 28 00 00 00 00 44 08 00..@.(D?. > > c0: 00 00 00 00 80 00 10 01 0a 10 18 00 00 fd 00 00?.?..?.. > > d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > e0: 50 10 08 50 00 02 00 70 00 00 30 10 0b 02 1c 01P??P.?.p..0? > > f0: 00 0b 07 00 94 11 7f 00 00 00 00 00 00 01 0e ff.??.???..??. > > My values for 0x28 address match this. ^ What about the values at 0x2c? > Interesting that it returns different register values for different > device addresses. Yes the registers have different meanings for the 2 addresses. Thanks, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 4/4] RFC: drm/panfrost: devfreq: Add support for 2 regulators
The Bifrost GPU on MT8183 uses 2 regulators (core and SRAM) for devfreq, and provides OPP table with 2 sets of voltages. TODO: This is incomplete as we'll need add support for setting a pair of voltages as well. I also realized that the out-of-tree driver has complex logic to ensure voltage delta between the regulators stays within a specific range, so we'd need to port that as well. Signed-off-by: Nicolas Boichat --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 + drivers/gpu/drm/panfrost/panfrost_device.h | 1 + 2 files changed, 18 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 413987038fbfccb..9c0987a3d71c597 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -79,6 +79,21 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct devfreq *devfreq; struct thermal_cooling_device *cooling; + /* If we have 2 regulator, we need an OPP table with 2 voltages. */ + if (pfdev->comp->num_supplies > 1) { + pfdev->devfreq.dev_opp_table = + dev_pm_opp_set_regulators(dev, + pfdev->comp->supply_names, + pfdev->comp->num_supplies); + if (IS_ERR(pfdev->devfreq.dev_opp_table)) { + ret = PTR_ERR(pfdev->devfreq.dev_opp_table); + pfdev->devfreq.dev_opp_table = NULL; + dev_err(dev, + "Failed to init devfreq opp table: %d\n", ret); + return ret; + } + } + ret = dev_pm_opp_of_add_table(dev); if (ret == -ENODEV) /* Optional, continue without devfreq */ return 0; @@ -119,6 +134,8 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev) if (pfdev->devfreq.cooling) devfreq_cooling_unregister(pfdev->devfreq.cooling); dev_pm_opp_of_remove_table(>pdev->dev); + if (pfdev->devfreq.dev_opp_table) + dev_pm_opp_put_regulators(pfdev->devfreq.dev_opp_table); } void panfrost_devfreq_resume(struct panfrost_device *pfdev) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index c30c719a805940a..5009a8b7c853ea1 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -110,6 +110,7 @@ struct panfrost_device { struct { struct devfreq *devfreq; struct thermal_cooling_device *cooling; + struct opp_table *dev_opp_table; ktime_t busy_time; ktime_t idle_time; ktime_t time_last_update; -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- v5: - Rename "2d" power domain to "core2" v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index e8b99adcb1bd292..c5ceca513192f99 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -62,6 +63,30 @@ allOf: minItems: 2 required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: core2 + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 0/4] Add dts for mt8183 GPU (and misc panfrost patches)
Hi! Follow-up on the v4: https://patchwork.kernel.org/cover/11369777/, some of the core patches got merged already (thanks Rob!). The main purpose of this series is to upstream the dts change and the binding document, but I wanted to see how far I could probe the GPU, to check that the binding is indeed correct. The rest of the patches are RFC/work-in-progress. So this is tested on MT8183 with a chromeos-4.19 kernel, and a ton of backports to get the latest panfrost driver (I should probably try on linux-next at some point but this was the path of least resistance). I tested it as a module as it's more challenging (originally probing would work built-in, on boot, but not as a module, as I didn't have the power domain changes, and all power domains are on by default during boot). Probing logs looks like this, currently. They look sane. [ 501.319728] panfrost 1304.gpu: clock rate = 51170 [ 501.320041] panfrost 1304.gpu: Linked as a consumer to regulator.14 [ 501.320102] panfrost 1304.gpu: Linked as a consumer to regulator.31 [ 501.320651] panfrost 1304.gpu: Linked as a consumer to genpd:0:1304.gpu [ 501.320954] panfrost 1304.gpu: Linked as a consumer to genpd:1:1304.gpu [ 501.321062] panfrost 1304.gpu: Linked as a consumer to genpd:2:1304.gpu [ 501.321734] panfrost 1304.gpu: mali-g72 id 0x6221 major 0x0 minor 0x3 status 0x0 [ 501.321741] panfrost 1304.gpu: features: ,13de77ff, issues: ,0400 [ 501.321747] panfrost 1304.gpu: Features: L2:0x07120206 Shader:0x Tiler:0x0809 Mem:0x1 MMU:0x2830 AS:0xff JS:0x7 [ 501.321752] panfrost 1304.gpu: shader_present=0x7 l2_present=0x1 [ 501.324951] [drm] Initialized panfrost 1.1.0 20180908 for 1304.gpu on minor 2 Some more changes are still required to get devfreq working, and of course I do not have a userspace driver to test this with. I believe at least patches 1 & 2 can be merged (2 depends on another patch series, so maybe we could start with 1 only for now...). Thanks! Nicolas Boichat (4): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU RFC: drm/panfrost: Add mt8183-mali compatible string RFC: drm/panfrost: devfreq: Add support for 2 regulators .../bindings/gpu/arm,mali-bifrost.yaml| 25 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 7 ++ arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +++ drivers/gpu/drm/panfrost/panfrost_device.h| 1 + drivers/gpu/drm/panfrost/panfrost_drv.c | 11 ++ 6 files changed, 166 insertions(+) -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/4] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- Upstreaming what matches existing bindings from our Chromium OS tree: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/arch/arm64/boot/dts/mediatek/mt8183.dtsi#1411 The evb part of this change depends on this patch to add PMIC dtsi: https://patchwork.kernel.org/patch/10928161/ The binding we use with out-of-tree Mali drivers includes more clocks, this is used for devfreq: the out-of-tree driver switches clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then switches clk_mux back to clk_main_parent: (see https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423) clocks = < CLK_TOP_MFGPLL_CK>, < CLK_TOP_MUX_MFG>, <>, < CLK_MFG_BG3D>; clock-names = "clk_main_parent", "clk_mux", "clk_sub_parent", "subsys_mfg_cg"; (based on discussions, this probably belongs in the clock core) v5: - Rename "2d" power domain to "core2" (keep R-B again). v4: - Add power-domain-names to describe the 3 domains. (kept Alyssa's reviewed-by as the change is minor) v3: - No changes v2: - Use sram instead of mali_sram as SRAM supply name. - Rename mali@ to gpu@. arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 7 ++ arch/arm64/boot/dts/mediatek/mt8183.dtsi| 105 2 files changed, 112 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts index 1fb195c683c3d01..7d609e0cd9b4975 100644 --- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts @@ -7,6 +7,7 @@ /dts-v1/; #include "mt8183.dtsi" +#include "mt6358.dtsi" / { model = "MediaTek MT8183 evaluation board"; @@ -30,6 +31,12 @@ { status = "okay"; }; + { + supply-names = "mali", "sram"; + mali-supply = <_vgpu_reg>; + sram-supply = <_vsram_gpu_reg>; +}; + { pinctrl-names = "default"; pinctrl-0 = <_pins_0>; diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi index 97863adb7bc02b4..fc690c54988c8cc 100644 --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi @@ -652,6 +652,111 @@ mfgcfg: syscon@1300 { #clock-cells = <1>; }; + gpu: gpu@1304 { + compatible = "mediatek,mt8183-mali", "arm,mali-bifrost"; + reg = <0 0x1304 0 0x4000>; + interrupts = + , + , + ; + interrupt-names = "job", "mmu", "gpu"; + + clocks = < CLK_TOP_MFGPLL_CK>; + + power-domains = + < MT8183_POWER_DOMAIN_MFG_CORE0>, + < MT8183_POWER_DOMAIN_MFG_CORE1>, + < MT8183_POWER_DOMAIN_MFG_2D>; + power-domain-names = "core0", "core1", "core2"; + + operating-points-v2 = <_opp_table>; + }; + + gpu_opp_table: opp_table0 { + compatible = "operating-points-v2"; + opp-shared; + + opp-3 { + opp-hz = /bits/ 64 <3>; + opp-microvolt = <625000>, <85>; + }; + + opp-32000 { + opp-hz = /bits/ 64 <32000>; + opp-microvolt = <631250>, <85>; + }; + + opp-34000 { + opp-hz = /bits/ 64 <34000>; + opp-microvolt = <637500>, <85>; + }; + + opp-36000 { + opp-hz = /bits/ 64 <36000>; + opp-microvolt = <643750>, <85>; + }; + + opp-38000 { + opp-hz = /bits/ 64 <38000>; + opp-microvolt = <65>, <85>; + }; + + opp-4 { + opp-hz = /bits/ 64 <4>;
[PATCH v5 3/4] RFC: drm/panfrost: Add mt8183-mali compatible string
For testing only, the driver doesn't really work yet, AFAICT. Signed-off-by: Nicolas Boichat --- v5: - Change power domain name from 2d to core2. v4: - Add power domain names. v3: - Match mt8183-mali instead of bifrost, as we require special handling for the 2 regulators and 3 power domains. drivers/gpu/drm/panfrost/panfrost_drv.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index a6e162236d67fdf..ff76b29b373e105 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -667,6 +667,15 @@ static const struct panfrost_compatible default_data = { .pm_domain_names = NULL, }; +const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; +static const struct panfrost_compatible mediatek_mt8183_data = { + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .supply_names = mediatek_mt8183_supplies, + .num_pm_domains = 3, + .pm_domain_names = mediatek_mt8183_pm_domains, +}; + static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t604", .data = _data, }, { .compatible = "arm,mali-t624", .data = _data, }, @@ -677,6 +686,8 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t830", .data = _data, }, { .compatible = "arm,mali-t860", .data = _data, }, { .compatible = "arm,mali-t880", .data = _data, }, + { .compatible = "mediatek,mt8183-mali", + .data = _mt8183_data }, {} }; MODULE_DEVICE_TABLE(of, dt_match); -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/7] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
On Fri, Mar 6, 2020 at 10:34 AM Nick Fan wrote: > > Sorry for my late reply. > I have checked internally. > The MT8183_POWER_DOMAIN_MFG_2D is just a legacy name, not really 2D > domain. > > If the naming too confusing, we can change this name to > MT8183_POWER_DOMAIN_MFG_CORE2 for consistency. Thanks! I think I'll keep MT8183_POWER_DOMAIN_MFG_2D (that's fine if that's the domain name you use internally in your HW design), but I'll modify power-domain-names to core0/1/2 in the binding. > Thanks > > Nick Fan > > On Wed, 2020-02-26 at 08:55 +0800, Nicolas Boichat wrote: > > > +Nick Fan +Sj Huang @ MTK > > > > On Wed, Feb 26, 2020 at 1:16 AM Rob Herring wrote: > > > > > > On Fri, Feb 07, 2020 at 01:26:21PM +0800, Nicolas Boichat wrote: > > > > Define a compatible string for the Mali Bifrost GPU found in > > > > Mediatek's MT8183 SoCs. > > > > > > > > Signed-off-by: Nicolas Boichat > > > > Reviewed-by: Alyssa Rosenzweig > > > > --- > > > > > > > > v4: > > > > - Add power-domain-names description > > > >(kept Alyssa's reviewed-by as the change is minor) > > > > v3: > > > > - No change > > > > > > > > .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ > > > > 1 file changed, 25 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > > index 4ea6a8789699709..0d93b3981445977 100644 > > > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > > @@ -17,6 +17,7 @@ properties: > > > > items: > > > >- enum: > > > >- amlogic,meson-g12a-mali > > > > + - mediatek,mt8183-mali > > > >- realtek,rtd1619-mali > > > >- rockchip,px30-mali > > > >- const: arm,mali-bifrost # Mali Bifrost GPU model/revision is > > > > fully discoverable > > > > @@ -62,6 +63,30 @@ allOf: > > > >minItems: 2 > > > >required: > > > > - resets > > > > + - if: > > > > + properties: > > > > +compatible: > > > > + contains: > > > > +const: mediatek,mt8183-mali > > > > +then: > > > > + properties: > > > > +sram-supply: true > > > > +power-domains: > > > > + description: > > > > +List of phandle and PM domain specifier as documented in > > > > +Documentation/devicetree/bindings/power/power_domain.txt > > > > + minItems: 3 > > > > + maxItems: 3 > > > > +power-domain-names: > > > > + items: > > > > +- const: core0 > > > > +- const: core1 > > > > +- const: 2d > > > > > > AFAIK, there's no '2d' block in bifrost GPUs. A power domain for each > > > core group is correct though. > > > > Good question... Hopefully Nick/SJ@MTK can comment, the non-upstream DTS > > has: > > gpu: mali@1304 { > > compatible = "mediatek,mt8183-mali", "arm,mali-bifrost"; > > power-domains = < MT8183_POWER_DOMAIN_MFG_CORE0>; > > ... > > } > > > > gpu_core1: mali_gpu_core1 { > > compatible = "mediatek,gpu_core1"; > > power-domains = < MT8183_POWER_DOMAIN_MFG_CORE1>; > > }; > > > > gpu_core2: mali_gpu_core2 { > > compatible = "mediatek,gpu_core2"; > > power-domains = < MT8183_POWER_DOMAIN_MFG_2D>; > > }; > > > > So I picked core0/core1/2d as names, but looking at this, it's likely > > core2 is more appropriate (and MT8183_POWER_DOMAIN_MFG_2D might just > > be a internal/legacy name, if there is no real 2d domain). > > > > Thanks. > > > > > Rob > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/7] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
+Nick Fan +Sj Huang @ MTK On Wed, Feb 26, 2020 at 1:16 AM Rob Herring wrote: > > On Fri, Feb 07, 2020 at 01:26:21PM +0800, Nicolas Boichat wrote: > > Define a compatible string for the Mali Bifrost GPU found in > > Mediatek's MT8183 SoCs. > > > > Signed-off-by: Nicolas Boichat > > Reviewed-by: Alyssa Rosenzweig > > --- > > > > v4: > > - Add power-domain-names description > >(kept Alyssa's reviewed-by as the change is minor) > > v3: > > - No change > > > > .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > index 4ea6a8789699709..0d93b3981445977 100644 > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > @@ -17,6 +17,7 @@ properties: > > items: > >- enum: > >- amlogic,meson-g12a-mali > > + - mediatek,mt8183-mali > >- realtek,rtd1619-mali > >- rockchip,px30-mali > >- const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully > > discoverable > > @@ -62,6 +63,30 @@ allOf: > >minItems: 2 > >required: > > - resets > > + - if: > > + properties: > > +compatible: > > + contains: > > +const: mediatek,mt8183-mali > > +then: > > + properties: > > +sram-supply: true > > +power-domains: > > + description: > > +List of phandle and PM domain specifier as documented in > > +Documentation/devicetree/bindings/power/power_domain.txt > > + minItems: 3 > > + maxItems: 3 > > +power-domain-names: > > + items: > > +- const: core0 > > +- const: core1 > > +- const: 2d > > AFAIK, there's no '2d' block in bifrost GPUs. A power domain for each > core group is correct though. Good question... Hopefully Nick/SJ@MTK can comment, the non-upstream DTS has: gpu: mali@1304 { compatible = "mediatek,mt8183-mali", "arm,mali-bifrost"; power-domains = < MT8183_POWER_DOMAIN_MFG_CORE0>; ... } gpu_core1: mali_gpu_core1 { compatible = "mediatek,gpu_core1"; power-domains = < MT8183_POWER_DOMAIN_MFG_CORE1>; }; gpu_core2: mali_gpu_core2 { compatible = "mediatek,gpu_core2"; power-domains = < MT8183_POWER_DOMAIN_MFG_2D>; }; So I picked core0/core1/2d as names, but looking at this, it's likely core2 is more appropriate (and MT8183_POWER_DOMAIN_MFG_2D might just be a internal/legacy name, if there is no real 2d domain). Thanks. > Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] drm/bridge: anx7688: Add anx7688 bridge driver support
On Sat, Feb 15, 2020 at 5:36 AM Vasily Khoruzhick wrote: > > On Thu, Feb 13, 2020 at 6:54 AM Enric Balletbo i Serra > wrote: > > > > From: Nicolas Boichat > > > > ANX7688 is a HDMI to DP converter (as well as USB-C port controller), > > that has an internal microcontroller. > > > > The only reason a Linux kernel driver is necessary is to reject > > resolutions that require more bandwidth than what is available on > > the DP side. DP bandwidth and lane count are reported by the bridge > > via 2 registers on I2C. > > It is true only for your particular platform where usb-c part is > managed by firmware. Pinephone has the same anx7688 but linux will > need a driver that manages usb-c in addition to DP. > > I'd suggest making it MFD driver from the beginning, or at least make > proper bindings so we don't have to rework it and introduce binding > incompatibilities in future. If that helps for the binding, ANX7688 is indeed a MFD (TCPC, HDMI to DP converter, USB-C mux between USB 3.0 lanes and the DP output of the embedded converter), with 2 I2C addresses: - 0x2c is the TCPC/mux, used by the Embedded Controller [1] on Chrome OS, and the code in this patch (my understanding is that lane count/BW registers in the kernel driver below may only be available to FW on Chromebooks). - 0x28: - Used to update the embedded FW in the anx7688 (on Chrome OS we do this in depthcharge [2]). This is a EEPROM-based FW (so even without implementing this, it'll usually "just work"). - Used to workaround some TCPC issues (see [1] again). [1] EC driver: https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/driver/tcpm/anx7688.c [2] depthcharge driver to update ANX7688 FW: https://chromium.googlesource.com/chromiumos/platform/depthcharge/+/master/src/drivers/ec/anx7688/anx7688.c ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] drm/bridge: anx7688: Add anx7688 bridge driver support
On Fri, Feb 14, 2020 at 8:18 PM Andrzej Hajda wrote: > > On 13.02.2020 15:54, Enric Balletbo i Serra wrote: > > From: Nicolas Boichat > > > > ANX7688 is a HDMI to DP converter (as well as USB-C port controller), > > that has an internal microcontroller. > > > > The only reason a Linux kernel driver is necessary is to reject > > resolutions that require more bandwidth than what is available on > > the DP side. DP bandwidth and lane count are reported by the bridge > > via 2 registers on I2C. > > > > Signed-off-by: Nicolas Boichat > > Signed-off-by: Hsin-Yi Wang > > Signed-off-by: Enric Balletbo i Serra > > --- > > > > Changes in v2: > > - Move driver to drivers/gpu/drm/bridge/analogix. > > - Make the driver OF only so we can reduce the ifdefs. > > - Update the Copyright to 2020. > > - Use probe_new so we can get rid of the i2c_device_id table. > > > > drivers/gpu/drm/bridge/analogix/Kconfig | 12 ++ > > drivers/gpu/drm/bridge/analogix/Makefile | 1 + > > .../drm/bridge/analogix/analogix-anx7688.c| 188 ++ > > 3 files changed, 201 insertions(+) > > create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-anx7688.c > > > > diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig > > b/drivers/gpu/drm/bridge/analogix/Kconfig > > index e1fa7d820373..af7c2939403c 100644 > > --- a/drivers/gpu/drm/bridge/analogix/Kconfig > > +++ b/drivers/gpu/drm/bridge/analogix/Kconfig > > @@ -11,6 +11,18 @@ config DRM_ANALOGIX_ANX6345 > > ANX6345 transforms the LVTTL RGB output of an > > application processor to eDP or DisplayPort. > > > > +config DRM_ANALOGIX_ANX7688 > > + tristate "Analogix ANX7688 bridge" > > + depends on OF > > + select DRM_KMS_HELPER > > + select REGMAP_I2C > > + help > > + ANX7688 is an ultra-low power 4k Ultra-HD (4096x2160p60) > > + mobile HD transmitter designed for portable devices. The > > + ANX7688 converts HDMI 2.0 to DisplayPort 1.3 Ultra-HD > > + including an intelligent crosspoint switch to support > > + USB Type-C. > > + > > config DRM_ANALOGIX_ANX78XX > > tristate "Analogix ANX78XX bridge" > > select DRM_ANALOGIX_DP > > diff --git a/drivers/gpu/drm/bridge/analogix/Makefile > > b/drivers/gpu/drm/bridge/analogix/Makefile > > index 97669b374098..27cd73635c8c 100644 > > --- a/drivers/gpu/drm/bridge/analogix/Makefile > > +++ b/drivers/gpu/drm/bridge/analogix/Makefile > > @@ -1,5 +1,6 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o > > analogix-i2c-dptx.o > > obj-$(CONFIG_DRM_ANALOGIX_ANX6345) += analogix-anx6345.o > > +obj-$(CONFIG_DRM_ANALOGIX_ANX7688) += analogix-anx7688.o > > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o > > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c > > b/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c > > new file mode 100644 > > index ..10a7cd0f9126 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c > > @@ -0,0 +1,188 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * ANX7688 HDMI->DP bridge driver > > + * > > + * Copyright 2020 Google LLC > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +/* Register addresses */ > > +#define VENDOR_ID_REG 0x00 > > +#define DEVICE_ID_REG 0x02 > > + > > +#define FW_VERSION_REG 0x80 > > + > > +#define DP_BANDWIDTH_REG 0x85 > > +#define DP_LANE_COUNT_REG 0x86 > > + > > +#define VENDOR_ID 0x1f29 > > +#define DEVICE_ID 0x7688 > > + > > +/* First supported firmware version (0.85) */ > > +#define MINIMUM_FW_VERSION 0x0085 > > + > > +struct anx7688 { > > + struct drm_bridge bridge; > > + struct i2c_client *client; > > + struct regmap *regmap; > > + > > + bool filter; > > +}; > > + > > +static inline struct anx7688 *bridge_to_anx7688(struct drm_bridge *bridge) > > +{ > > + return container_of(bridge, struct anx7688, bridge); > > +} > > + > > +static bool anx7688_bridge_mode_fixup(struct drm_bridge *bridge, > > + const struct drm_display_mode *mode, > > + struct drm_display_mode *adjusted_mode) > > +{
Re: [PATCH v2 2/2] drm/bridge: anx7688: Add anx7688 bridge driver support
On Fri, Feb 14, 2020 at 8:18 PM Andrzej Hajda wrote: > > On 13.02.2020 15:54, Enric Balletbo i Serra wrote: > > From: Nicolas Boichat > > > > ANX7688 is a HDMI to DP converter (as well as USB-C port controller), > > that has an internal microcontroller. > > > > The only reason a Linux kernel driver is necessary is to reject > > resolutions that require more bandwidth than what is available on > > the DP side. DP bandwidth and lane count are reported by the bridge > > via 2 registers on I2C. > > > > Signed-off-by: Nicolas Boichat > > Signed-off-by: Hsin-Yi Wang > > Signed-off-by: Enric Balletbo i Serra > > --- > > > > Changes in v2: > > - Move driver to drivers/gpu/drm/bridge/analogix. > > - Make the driver OF only so we can reduce the ifdefs. > > - Update the Copyright to 2020. > > - Use probe_new so we can get rid of the i2c_device_id table. > > > > drivers/gpu/drm/bridge/analogix/Kconfig | 12 ++ > > drivers/gpu/drm/bridge/analogix/Makefile | 1 + > > .../drm/bridge/analogix/analogix-anx7688.c| 188 ++ > > 3 files changed, 201 insertions(+) > > create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-anx7688.c > > > > diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig > > b/drivers/gpu/drm/bridge/analogix/Kconfig > > index e1fa7d820373..af7c2939403c 100644 > > --- a/drivers/gpu/drm/bridge/analogix/Kconfig > > +++ b/drivers/gpu/drm/bridge/analogix/Kconfig > > @@ -11,6 +11,18 @@ config DRM_ANALOGIX_ANX6345 > > ANX6345 transforms the LVTTL RGB output of an > > application processor to eDP or DisplayPort. > > > > +config DRM_ANALOGIX_ANX7688 > > + tristate "Analogix ANX7688 bridge" > > + depends on OF > > + select DRM_KMS_HELPER > > + select REGMAP_I2C > > + help > > + ANX7688 is an ultra-low power 4k Ultra-HD (4096x2160p60) > > + mobile HD transmitter designed for portable devices. The > > + ANX7688 converts HDMI 2.0 to DisplayPort 1.3 Ultra-HD > > + including an intelligent crosspoint switch to support > > + USB Type-C. > > + > > config DRM_ANALOGIX_ANX78XX > > tristate "Analogix ANX78XX bridge" > > select DRM_ANALOGIX_DP > > diff --git a/drivers/gpu/drm/bridge/analogix/Makefile > > b/drivers/gpu/drm/bridge/analogix/Makefile > > index 97669b374098..27cd73635c8c 100644 > > --- a/drivers/gpu/drm/bridge/analogix/Makefile > > +++ b/drivers/gpu/drm/bridge/analogix/Makefile > > @@ -1,5 +1,6 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o > > analogix-i2c-dptx.o > > obj-$(CONFIG_DRM_ANALOGIX_ANX6345) += analogix-anx6345.o > > +obj-$(CONFIG_DRM_ANALOGIX_ANX7688) += analogix-anx7688.o > > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o > > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c > > b/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c > > new file mode 100644 > > index ..10a7cd0f9126 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c > > @@ -0,0 +1,188 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * ANX7688 HDMI->DP bridge driver > > + * > > + * Copyright 2020 Google LLC > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +/* Register addresses */ > > +#define VENDOR_ID_REG 0x00 > > +#define DEVICE_ID_REG 0x02 > > + > > +#define FW_VERSION_REG 0x80 > > + > > +#define DP_BANDWIDTH_REG 0x85 > > +#define DP_LANE_COUNT_REG 0x86 > > + > > +#define VENDOR_ID 0x1f29 > > +#define DEVICE_ID 0x7688 > > + > > +/* First supported firmware version (0.85) */ > > +#define MINIMUM_FW_VERSION 0x0085 > > + > > +struct anx7688 { > > + struct drm_bridge bridge; > > + struct i2c_client *client; > > + struct regmap *regmap; > > + > > + bool filter; > > +}; > > + > > +static inline struct anx7688 *bridge_to_anx7688(struct drm_bridge *bridge) > > +{ > > + return container_of(bridge, struct anx7688, bridge); > > +} > > + > > +static bool anx7688_bridge_mode_fixup(struct drm_bridge *bridge, > > + const struct drm_display_mode *mode, > > + struct drm_display_mode *adjusted_mode) > > +{
Re: [PATCH v4 7/7] RFC: drm/panfrost: devfreq: Add support for 2 regulators
On Thu, Feb 13, 2020 at 3:57 PM Nicolas Boichat wrote: > > > [snip] > > > But then there's a slight problem: panfrost_devfreq uses a bunch of > > > clk_get_rate calls, and the clock PLLs (at least on MTK platform) are > > > never fully precise, so we get back 29955 for 300 Mhz and > > > 79878 for 800 Mhz. That means that the kernel is unable to keep > > > devfreq stats as neither of these values are in the table: > > > [ 4802.470952] devfreq devfreq1: Couldn't update frequency transition > > > information. > > > The kbase driver fixes this by remembering the last set frequency, and > > > reporting that to devfreq. Should we do that as well or is there a > > > better fix? This one is my bad, I was missing this patch in my forklift to 4.19: 22bd4df9dadf46f drm/panfrost: devfreq: Round frequencies to OPPs (should really try to boot that board on linux-next, but that's for another time) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 7/7] RFC: drm/panfrost: devfreq: Add support for 2 regulators
+Weiyi Lu +Nick Fan @MTK who may have more ideas. On Thu, Feb 13, 2020 at 2:14 AM Rob Herring wrote: > > On Wed, Feb 12, 2020 at 2:49 AM Nicolas Boichat wrote: > > > > +Viresh Kumar +Stephen Boyd for clock advice. > > > > On Fri, Feb 7, 2020 at 1:27 PM Nicolas Boichat > > wrote: > > > > > > The Bifrost GPU on MT8183 uses 2 regulators (core and SRAM) for > > > devfreq, and provides OPP table with 2 sets of voltages. > > > > > > TODO: This is incomplete as we'll need add support for setting > > > a pair of voltages as well. > > > > So all we need for this to work (at least apparently, that is, I can > > change frequency) is this: > > https://lore.kernel.org/patchwork/patch/1192945/ > > (ah well, Viresh just replied, so, probably not, I'll check that out > > and use the correct API) > > > > But then there's a slight problem: panfrost_devfreq uses a bunch of > > clk_get_rate calls, and the clock PLLs (at least on MTK platform) are > > never fully precise, so we get back 29955 for 300 Mhz and > > 79878 for 800 Mhz. That means that the kernel is unable to keep > > devfreq stats as neither of these values are in the table: > > [ 4802.470952] devfreq devfreq1: Couldn't update frequency transition > > information. > > The kbase driver fixes this by remembering the last set frequency, and > > reporting that to devfreq. Should we do that as well or is there a > > better fix? > > > > Another thing that I'm not implementing is the dance that Mediatek > > does in their kbase driver when changing the clock (described in patch > > 2/7): > > "" > > The binding we use with out-of-tree Mali drivers includes more > > clocks, this is used for devfreq: the out-of-tree driver switches > > clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then > > switches clk_mux back to clk_main_parent: > > (see > > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423) > > clocks = > > < CLK_TOP_MFGPLL_CK>, > > < CLK_TOP_MUX_MFG>, > > <>, > > < CLK_MFG_BG3D>; > > clock-names = > > "clk_main_parent", > > "clk_mux", > > "clk_sub_parent", > > "subsys_mfg_cg"; > > "" > > Is there a clean/simple way to implement this in the clock > > framework/device tree? Or should we implement something in the > > panfrost driver? > > Putting parent clocks into 'clocks' for a device is a pretty common > abuse. The 'assigned-clocks' binding is what's used for parent clock > setup. Not sure that's going to help here though. Is this dance > because the parent clock frequency can't be changed cleanly? Nick/Weiyi, any idea why we do that dance in the first place? (maybe the PLL clock is unstable while it's being changed?) If we really need it, can we move that logic to the clock core? > If up to > me, I'd put that dance in the clock driver. The GPU shouldn't have to > care. > > Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 7/7] RFC: drm/panfrost: devfreq: Add support for 2 regulators
+Viresh Kumar +Stephen Boyd for clock advice. On Fri, Feb 7, 2020 at 1:27 PM Nicolas Boichat wrote: > > The Bifrost GPU on MT8183 uses 2 regulators (core and SRAM) for > devfreq, and provides OPP table with 2 sets of voltages. > > TODO: This is incomplete as we'll need add support for setting > a pair of voltages as well. So all we need for this to work (at least apparently, that is, I can change frequency) is this: https://lore.kernel.org/patchwork/patch/1192945/ (ah well, Viresh just replied, so, probably not, I'll check that out and use the correct API) But then there's a slight problem: panfrost_devfreq uses a bunch of clk_get_rate calls, and the clock PLLs (at least on MTK platform) are never fully precise, so we get back 29955 for 300 Mhz and 79878 for 800 Mhz. That means that the kernel is unable to keep devfreq stats as neither of these values are in the table: [ 4802.470952] devfreq devfreq1: Couldn't update frequency transition information. The kbase driver fixes this by remembering the last set frequency, and reporting that to devfreq. Should we do that as well or is there a better fix? Another thing that I'm not implementing is the dance that Mediatek does in their kbase driver when changing the clock (described in patch 2/7): "" The binding we use with out-of-tree Mali drivers includes more clocks, this is used for devfreq: the out-of-tree driver switches clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then switches clk_mux back to clk_main_parent: (see https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423) clocks = < CLK_TOP_MFGPLL_CK>, < CLK_TOP_MUX_MFG>, <>, < CLK_MFG_BG3D>; clock-names = "clk_main_parent", "clk_mux", "clk_sub_parent", "subsys_mfg_cg"; "" Is there a clean/simple way to implement this in the clock framework/device tree? Or should we implement something in the panfrost driver? Thanks! > > Signed-off-by: Nicolas Boichat > > --- > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 + > drivers/gpu/drm/panfrost/panfrost_device.h | 1 + > 2 files changed, 18 insertions(+) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > index 413987038fbfccb..9c0987a3d71c597 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > @@ -79,6 +79,21 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > struct devfreq *devfreq; > struct thermal_cooling_device *cooling; > > + /* If we have 2 regulator, we need an OPP table with 2 voltages. */ > + if (pfdev->comp->num_supplies > 1) { > + pfdev->devfreq.dev_opp_table = > + dev_pm_opp_set_regulators(dev, > + pfdev->comp->supply_names, > + pfdev->comp->num_supplies); > + if (IS_ERR(pfdev->devfreq.dev_opp_table)) { > + ret = PTR_ERR(pfdev->devfreq.dev_opp_table); > + pfdev->devfreq.dev_opp_table = NULL; > + dev_err(dev, > + "Failed to init devfreq opp table: %d\n", > ret); > + return ret; > + } > + } > + > ret = dev_pm_opp_of_add_table(dev); > if (ret == -ENODEV) /* Optional, continue without devfreq */ > return 0; > @@ -119,6 +134,8 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev) > if (pfdev->devfreq.cooling) > devfreq_cooling_unregister(pfdev->devfreq.cooling); > dev_pm_opp_of_remove_table(>pdev->dev); > + if (pfdev->devfreq.dev_opp_table) > + dev_pm_opp_put_regulators(pfdev->devfreq.dev_opp_table); > } > > void panfrost_devfreq_resume(struct panfrost_device *pfdev) > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h > b/drivers/gpu/drm/panfrost/panfrost_device.h > index c30c719a805940a..5009a8b7c853ea1 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -110,6 +110,7 @@ struct panfrost_device { > struct { > struct devfreq *devfreq; > struct thermal_cooling_device *cooling; > + struct opp_table *dev_opp_table; > ktime_t busy_time; > ktime_t idle_time; > ktime_t time_last_update; > -- > 2.25.0.341.g760bfbb309-goog > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/7] drm/panfrost: Add support for multiple power domains
On Wed, Feb 12, 2020 at 4:09 AM Saravana Kannan wrote: > > On Tue, Feb 11, 2020 at 11:44 AM Rob Herring wrote: > > > > +Saravana > > > > On Thu, Feb 6, 2020 at 11:27 PM Nicolas Boichat > > wrote: > > > > > > When there is a single power domain per device, the core will > > > ensure the power domain is switched on (so it is technically > > > equivalent to having not power domain specified at all). > > > > > > However, when there are multiple domains, as in MT8183 Bifrost > > > GPU, we need to handle them in driver code. > > > > > > Signed-off-by: Nicolas Boichat > > > > > > --- > > > [snip] > > > + pfdev->pm_domain_links[i] = device_link_add(pfdev->dev, > > > + pfdev->pm_domain_devs[i], > > > DL_FLAG_PM_RUNTIME | > > > + DL_FLAG_STATELESS | DL_FLAG_RPM_ACTIVE); > > > > We're in the process of adding device links based on DT properties. > > Shouldn't we add power domains to that? See drivers/of/property.c for > > what's handled. > > Rob, > > drivers/of/property.c doesn't enable the RPM_ACTIVE AND PM_RUNTIME > flags. Wanted to start off conservative. But adding command line ops > to change the default flags shouldn't be difficult. But before I do > that, I want to change of_devlink to > fw_devlink=. May be I can extend that to > "disabled, permissive, suspend, runtime". > > Nicholas, > > And the adding and removing of device links for power domains will be > a 2 line change. I've been meaning to add a few more bindings like > hwspinlocks and pinctrl. I can roll power domains support into that if > you want. Adding power domains makes sense to me, as there are a bunch of other users (git grep dev_pm_domain_attach_by_name). This seems to be a bit orthogonal to this patch though. If your solution lands before this (and not something that is behind a command line option), then I'm happy to make use of it. Either way, happy to test things. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 0/7] Add dts for mt8183 GPU (and misc panfrost patches)
On Fri, Feb 7, 2020 at 4:13 PM Tomeu Vizoso wrote: > > On 2/7/20 8:42 AM, Nicolas Boichat wrote: > > On Fri, Feb 7, 2020 at 2:18 PM Tomeu Vizoso > > wrote: > >> > >>> Some more changes are still required to get devfreq working, and of course > >>> I do not have a userspace driver to test this with. > >> > >> Have you tried the Panfrost tests in IGT? They are atm quite basic, but > >> could be interesting to check that the different HW units are correctly > >> powered on. > > > > I haven't, you mean this right? > > https://gitlab.freedesktop.org/tomeu/igt-gpu-tools/tree/panfrost > > Yes, though may be better to use the upstream repo: > > https://gitlab.freedesktop.org/drm/igt-gpu-tools > > > Any specific test you have in mind? > > All the panfrost ones, but looks like panfrost_prime:gem-prime-import is > failing atm: > > https://lava.collabora.co.uk/scheduler/job/2214987 (I first removed opp table from device tree to avoid constant spew about devfreq not supporting 2 regulators, I should get around to fix that...) # /usr/libexec/igt-gpu-tools/panfrost_gem_new IGT-Version: 1.24-gd4d574a4 (arm) (Linux: 4.19.99 aarch64) Starting subtest: gem-new-4096 Subtest gem-new-4096: SUCCESS (0.000s) Starting subtest: gem-new-0 Subtest gem-new-0: SUCCESS (0.000s) Starting subtest: gem-new-zeroed Subtest gem-new-zeroed: SUCCESS (0.001s) # /usr/libexec/igt-gpu-tools/panfrost_get_param IGT-Version: 1.24-gd4d574a4 (arm) (Linux: 4.19.99 aarch64) Starting subtest: base-params Subtest base-params: SUCCESS (0.000s) Starting subtest: get-bad-param Subtest get-bad-param: SUCCESS (0.000s) Starting subtest: get-bad-padding Subtest get-bad-padding: SUCCESS (0.000s) # /usr/libexec/igt-gpu-tools/panfrost_prime IGT-Version: 1.24-gd4d574a4 (arm) (Linux: 4.19.99 aarch64) Starting subtest: gem-prime-import (panfrost_prime:1527) ioctl_wrappers-CRITICAL: Test assertion failure function prime_fd_to_handle, file ../igt-gpu-tools-/lib/ioctl_wrappers.c:1336: (panfrost_prime:1527) ioctl_wrappers-CRITICAL: Failed assertion: igt_ioctl((fd), 2U|1U) << (((0+8)+8)+14)) | ((('d')) << (0+8)) | (((0x2e)) << 0) | sizeof(struct drm_prime_handle << ((0+8)+8, ()) == 0 (panfrost_prime:1527) ioctl_wrappers-CRITICAL: Last errno: 95, Operation not supported (panfrost_prime:1527) ioctl_wrappers-CRITICAL: error: -1 != 0 Stack trace: Subtest gem-prime-import failed. Subtest gem-prime-import: FAIL (0.004s) (but that looks expected?) Now the trickier ones, I guess we're either missing something, or my dirty 4.19 backport is very broken: # /usr/libexec/igt-gpu-tools/panfrost_submit IGT-Version: 1.24-gd4d574a4 (arm) (Linux: 4.19.99 aarch64) Starting subtest: pan-submit (panfrost_submit:1643) CRITICAL: Test assertion failure function __real_main86, file ../igt-gpu-tools-/tests/panfrost_submit.c:103: (panfrost_submit:1643) CRITICAL: Failed assertion: syncobj_wait(fd, >args->out_sync, 1, abs_timeout(SHORT_TIME_NSEC), 0, NULL) Stack trace: Subtest pan-submit failed. DEBUG (panfrost_submit:1643) CRITICAL: Test assertion failure function __real_main86, file ../igt-gpu-tools-/tests/panfrost_submit.c:103: (panfrost_submit:1643) CRITICAL: Failed assertion: syncobj_wait(fd, >args->out_sync, 1, abs_timeout(SHORT_TIME_NSEC), 0, NULL) (panfrost_submit:1643) igt_core-INFO: Stack trace: END Subtest pan-submit: FAIL (0.119s) Starting subtest: pan-submit-error-no-jc Subtest pan-submit-error-no-jc: SUCCESS (0.000s) Starting subtest: pan-submit-error-bad-in-syncs Subtest pan-submit-error-bad-in-syncs: SUCCESS (0.012s) Starting subtest: pan-submit-error-bad-bo-handles Subtest pan-submit-error-bad-bo-handles: SUCCESS (0.012s) Starting subtest: pan-submit-error-bad-requirements Subtest pan-submit-error-bad-requirements: SUCCESS (0.012s) Starting subtest: pan-submit-error-bad-out-sync Subtest pan-submit-error-bad-out-sync: SUCCESS (0.012s) Starting subtest: pan-reset (panfrost_submit:1643) CRITICAL: Test assertion failure function __real_main86, file ../igt-gpu-tools-/tests/panfrost_submit.c:173: (panfrost_submit:1643) CRITICAL: Failed assertion: syncobj_wait(fd, >args->out_sync, 1, abs_timeout(BAD_JOB_TIME_NSEC), 0, NULL) Stack trace: Subtest pan-reset failed. DEBUG (panfrost_submit:1643) CRITICAL: Test assertion failure function __real_main86, file ../igt-gpu-tools-/tests/panfrost_submit.c:173: (panfrost_submit:1643) CRITICAL: Failed assertion: syncobj_wait(fd, >args->out_sync, 1, abs_timeout(BAD_JOB_TIME_NSEC), 0, NULL) (panfrost_submit:1643) igt_core-INFO: Stack trace: END Subtest pan-reset: FAIL (0.840s) The pan-submit case causes an MMU fault: (full log: https://gist.github.com/drinkcat/1ae36cb1b1b71f30cc4fc29759612d76) [ 1215.234937] [IGT] panfrost_submit: executing [ 1215.318446] [IGT] panfrost_submit: sta
Re: [PATCH v4 5/7] drm/panfrost: Add support for multiple power domains
On Fri, Feb 7, 2020 at 10:26 PM Ulf Hansson wrote: > > On Fri, 7 Feb 2020 at 06:27, Nicolas Boichat wrote: > > > > When there is a single power domain per device, the core will > > ensure the power domain is switched on (so it is technically > > equivalent to having not power domain specified at all). > > > > However, when there are multiple domains, as in MT8183 Bifrost > > GPU, we need to handle them in driver code. > > > > Signed-off-by: Nicolas Boichat > > Besides a minor nitpick, feel free to add: > > Reviewed-by: Ulf Hansson > > Kind regards > Uffe > > [snip] > > +static int panfrost_pm_domain_init(struct panfrost_device *pfdev) > > +{ > > + int err; > > + int i, num_domains; > > + > > + num_domains = of_count_phandle_with_args(pfdev->dev->of_node, > > +"power-domains", > > +"#power-domain-cells"); > > + > > + /* > > +* Single domain is handled by the core, and, if only a single power > > +* the power domain is requested, the property is optional. > > +*/ > > + if (num_domains < 2 && pfdev->comp->num_pm_domains < 2) > > + return 0; > > + > > + if (num_domains != pfdev->comp->num_pm_domains) { > > + dev_err(pfdev->dev, > > + "Incorrect number of power domains: %d provided, %d > > needed\n", > > + num_domains, pfdev->comp->num_pm_domains); > > + return -EINVAL; > > + } > > + > > + if (WARN(num_domains > ARRAY_SIZE(pfdev->pm_domain_devs), > > + "Too many supplies in compatible structure.\n")) > > Nitpick: > Not sure this deserves a WARN. Perhaps a regular dev_err() is sufficient. Ah well I had a BUG_ON before so presumably this is already a little better ,-) You can only reach there if you set pfdev->comp->num_pm_domains > MAX_PM_DOMAINS in the currently matched struct panfrost_compatible (pfdev->comp->num_pm_domains == num_domains, and see below too), so the kernel code would actually be actually broken (not the device tree, nor anything that could be probed). So I'm wondering if the loudness of a WARN is better in this case? Arguable ,-) > > + return -EINVAL; > [snip] > > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > > @@ -21,6 +21,7 @@ struct panfrost_perfcnt; > > > > #define NUM_JOB_SLOTS 3 > > #define MAX_REGULATORS 2 > > +#define MAX_PM_DOMAINS 3 > > > > struct panfrost_features { > > u16 id; > > @@ -61,6 +62,13 @@ struct panfrost_compatible { > > /* Supplies count and names. */ > > int num_supplies; > > const char * const *supply_names; > > + /* > > +* Number of power domains required, note that values 0 and 1 are > > +* handled identically, as only values > 1 need special handling. > > +*/ > > + int num_pm_domains; > > + /* Only required if num_pm_domains > 1. */ > > + const char * const *pm_domain_names; > > }; > > > > struct panfrost_device { > > @@ -73,6 +81,9 @@ struct panfrost_device { > > struct clk *bus_clock; > > struct regulator_bulk_data regulators[MAX_REGULATORS]; > > struct reset_control *rstc; > > + /* pm_domains for devices with more than one. */ > > + struct device *pm_domain_devs[MAX_PM_DOMAINS]; > > + struct device_link *pm_domain_links[MAX_PM_DOMAINS]; > > > > struct panfrost_features features; > > const struct panfrost_compatible *comp; > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c > > b/drivers/gpu/drm/panfrost/panfrost_drv.c > > index 4d08507526239f2..a6e162236d67fdf 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > @@ -663,6 +663,8 @@ const char * const default_supplies[] = { "mali" }; > > static const struct panfrost_compatible default_data = { > > .num_supplies = ARRAY_SIZE(default_supplies), > > .supply_names = default_supplies, > > + .num_pm_domains = 1, /* optional */ > > + .pm_domain_names = NULL, > > }; > > > > static const struct of_device_id dt_match[] = { > > -- > > 2.25.0.341.g760bfbb309-goog > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/7] drm/panfrost: Add support for multiple power domains
On Fri, Feb 7, 2020 at 9:52 PM Alyssa Rosenzweig wrote: > > > + for (i = 0; i < ARRAY_SIZE(pfdev->pm_domain_devs); i++) { > > + if (!pfdev->pm_domain_devs[i]) > > + break; (next time, please provide a tiny bit more context when quoting, I had to look up to see where this comes from) So this comes from panfrost_pm_domain_fini. > I'm not totally familiar with this code, but should this be a break or > just a continue? Check how the domains are initialized in panfrost_pm_domain_init, they are guaranteed to be "packed" at the beginning of the array, so there can't be any holes, so break is safe. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 0/7] Add dts for mt8183 GPU (and misc panfrost patches)
On Fri, Feb 7, 2020 at 2:18 PM Tomeu Vizoso wrote: > > On 2/7/20 6:26 AM, Nicolas Boichat wrote: > > Hi! > > > > Follow-up on the v3: https://patchwork.kernel.org/cover/11331343/. > > > > The main purpose of this series is to upstream the dts change and the > > binding document, but I wanted to see how far I could probe the GPU, to > > check that the binding is indeed correct. The rest of the patches are > > RFC/work-in-progress, but I think some of them could already be picked up. > > > > So this is tested on MT8183 with a chromeos-4.19 kernel, and a ton of > > backports to get the latest panfrost driver (I should probably try on > > linux-next at some point but this was the path of least resistance). > > > > I tested it as a module as it's more challenging (originally probing would > > work built-in, on boot, but not as a module, as I didn't have the power > > domain changes, and all power domains are on by default during boot). > > > > Probing logs looks like this, currently. They look sane. > > [ 501.319728] panfrost 1304.gpu: clock rate = 51170 > > [ 501.320041] panfrost 1304.gpu: Linked as a consumer to regulator.14 > > [ 501.320102] panfrost 1304.gpu: Linked as a consumer to regulator.31 > > [ 501.320651] panfrost 1304.gpu: Linked as a consumer to > > genpd:0:1304.gpu > > [ 501.320954] panfrost 1304.gpu: Linked as a consumer to > > genpd:1:1304.gpu > > [ 501.321062] panfrost 1304.gpu: Linked as a consumer to > > genpd:2:1304.gpu > > [ 501.321734] panfrost 1304.gpu: mali-g72 id 0x6221 major 0x0 minor > > 0x3 status 0x0 > > [ 501.321741] panfrost 1304.gpu: features: ,13de77ff, issues: > > ,0400 > > [ 501.321747] panfrost 1304.gpu: Features: L2:0x07120206 > > Shader:0x Tiler:0x0809 Mem:0x1 MMU:0x2830 AS:0xff JS:0x7 > > [ 501.321752] panfrost 1304.gpu: shader_present=0x7 l2_present=0x1 > > [ 501.324951] [drm] Initialized panfrost 1.1.0 20180908 for 1304.gpu > > on minor 2 > > > > Some more changes are still required to get devfreq working, and of course > > I do not have a userspace driver to test this with. > > Have you tried the Panfrost tests in IGT? They are atm quite basic, but > could be interesting to check that the different HW units are correctly > powered on. I haven't, you mean this right? https://gitlab.freedesktop.org/tomeu/igt-gpu-tools/tree/panfrost Any specific test you have in mind? Thanks, > Regards, > > Tomeu > > > I believe at least patches 1, 2, and 3 can be merged. 4 and 5 are mostly > > useful in conjunction with 6 and 7 (which are not ready yet), so I'll let > > maintainers decide. > > > > Thanks! > > > > Nicolas Boichat (7): > >dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 > >arm64: dts: mt8183: Add node for the Mali GPU > >drm/panfrost: Improve error reporting in panfrost_gpu_power_on > >drm/panfrost: Add support for multiple regulators > >drm/panfrost: Add support for multiple power domains > >RFC: drm/panfrost: Add mt8183-mali compatible string > >RFC: drm/panfrost: devfreq: Add support for 2 regulators > > > > .../bindings/gpu/arm,mali-bifrost.yaml| 25 > > arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 7 + > > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 +++ > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +++ > > drivers/gpu/drm/panfrost/panfrost_device.c| 123 +++--- > > drivers/gpu/drm/panfrost/panfrost_device.h| 27 +++- > > drivers/gpu/drm/panfrost/panfrost_drv.c | 41 -- > > drivers/gpu/drm/panfrost/panfrost_gpu.c | 11 +- > > 8 files changed, 326 insertions(+), 30 deletions(-) > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/7] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- v4: - Add power-domain-names description (kept Alyssa's reviewed-by as the change is minor) v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml| 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 4ea6a8789699709..0d93b3981445977 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -62,6 +63,30 @@ allOf: minItems: 2 required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 +power-domain-names: + items: +- const: core0 +- const: core1 +- const: 2d + + required: +- sram-supply +- power-domains +- power-domains-names examples: - | -- 2.25.0.341.g760bfbb309-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 4/7] drm/panfrost: Add support for multiple regulators
Some GPUs, namely, the bifrost/g72 part on MT8183, have a second regulator for their SRAM, let's add support for that. We extend the framework in a generic manner so that we could support more than 2 regulators, if required. Signed-off-by: Nicolas Boichat --- v4: - nits: Run through latest version of checkpatch: - Use WARN instead of BUG_ON. - Drop braces in single expression for loop. - *comp not * comp v3: - Make this more generic, by allowing any number of regulators (in practice we fix the maximum number of regulators to 2, but this could be increased easily). - We only probe the second regulator if the device tree matching data asks for it. - I couldn't find a way to detect the number of regulators in the device tree, if we wanted to refuse to probe the device if there are too many regulators, which might be required for safety, see the thread on v2 [1]. - The discussion also included the idea of a separate device tree entry for a "soft PDC", or at least a separate driver. I'm not sure to understand the full picture, and how different vendors implement this, so I'm still integrating everything in the main driver. I'd be happy to try to make mt8183 fit into such a framework after it's created, but I don't think I'm best placed to implement (and again, the main purpose of this was to test if the binding is correct). [1] https://patchwork.kernel.org/patch/11322839/ drivers/gpu/drm/panfrost/panfrost_device.c | 26 +--- drivers/gpu/drm/panfrost/panfrost_device.h | 15 +++- drivers/gpu/drm/panfrost/panfrost_drv.c| 28 +++--- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 238fb6d54df4732..3720d50f6d9f965 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -87,18 +87,27 @@ static void panfrost_clk_fini(struct panfrost_device *pfdev) static int panfrost_regulator_init(struct panfrost_device *pfdev) { - int ret; + int ret, i; - pfdev->regulator = devm_regulator_get(pfdev->dev, "mali"); - if (IS_ERR(pfdev->regulator)) { - ret = PTR_ERR(pfdev->regulator); - dev_err(pfdev->dev, "failed to get regulator: %d\n", ret); + if (WARN(pfdev->comp->num_supplies > ARRAY_SIZE(pfdev->regulators), + "Too many supplies in compatible structure.\n")) + return -EINVAL; + + for (i = 0; i < pfdev->comp->num_supplies; i++) + pfdev->regulators[i].supply = pfdev->comp->supply_names[i]; + + ret = devm_regulator_bulk_get(pfdev->dev, + pfdev->comp->num_supplies, + pfdev->regulators); + if (ret < 0) { + dev_err(pfdev->dev, "failed to get regulators: %d\n", ret); return ret; } - ret = regulator_enable(pfdev->regulator); + ret = regulator_bulk_enable(pfdev->comp->num_supplies, + pfdev->regulators); if (ret < 0) { - dev_err(pfdev->dev, "failed to enable regulator: %d\n", ret); + dev_err(pfdev->dev, "failed to enable regulators: %d\n", ret); return ret; } @@ -107,7 +116,8 @@ static int panfrost_regulator_init(struct panfrost_device *pfdev) static void panfrost_regulator_fini(struct panfrost_device *pfdev) { - regulator_disable(pfdev->regulator); + regulator_bulk_disable(pfdev->comp->num_supplies, + pfdev->regulators); } int panfrost_device_init(struct panfrost_device *pfdev) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index 06713811b92cdf7..c9468bc5573ac9d 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -19,6 +20,7 @@ struct panfrost_job; struct panfrost_perfcnt; #define NUM_JOB_SLOTS 3 +#define MAX_REGULATORS 2 struct panfrost_features { u16 id; @@ -51,6 +53,16 @@ struct panfrost_features { unsigned long hw_issues[64 / BITS_PER_LONG]; }; +/* + * Features that cannot be automatically detected and need matching using the + * compatible string, typically SoC-specific. + */ +struct panfrost_compatible { + /* Supplies count and names. */ + int num_supplies; + const char * const *supply_names; +}; + struct panfrost_device { struct device *dev; struct drm_device *ddev; @@ -59,10 +71,11 @@ struct panfrost_device { void __iomem *iomem; struct clk *clock;
[PATCH v4 0/7] Add dts for mt8183 GPU (and misc panfrost patches)
Hi! Follow-up on the v3: https://patchwork.kernel.org/cover/11331343/. The main purpose of this series is to upstream the dts change and the binding document, but I wanted to see how far I could probe the GPU, to check that the binding is indeed correct. The rest of the patches are RFC/work-in-progress, but I think some of them could already be picked up. So this is tested on MT8183 with a chromeos-4.19 kernel, and a ton of backports to get the latest panfrost driver (I should probably try on linux-next at some point but this was the path of least resistance). I tested it as a module as it's more challenging (originally probing would work built-in, on boot, but not as a module, as I didn't have the power domain changes, and all power domains are on by default during boot). Probing logs looks like this, currently. They look sane. [ 501.319728] panfrost 1304.gpu: clock rate = 51170 [ 501.320041] panfrost 1304.gpu: Linked as a consumer to regulator.14 [ 501.320102] panfrost 1304.gpu: Linked as a consumer to regulator.31 [ 501.320651] panfrost 1304.gpu: Linked as a consumer to genpd:0:1304.gpu [ 501.320954] panfrost 1304.gpu: Linked as a consumer to genpd:1:1304.gpu [ 501.321062] panfrost 1304.gpu: Linked as a consumer to genpd:2:1304.gpu [ 501.321734] panfrost 1304.gpu: mali-g72 id 0x6221 major 0x0 minor 0x3 status 0x0 [ 501.321741] panfrost 1304.gpu: features: ,13de77ff, issues: ,0400 [ 501.321747] panfrost 1304.gpu: Features: L2:0x07120206 Shader:0x Tiler:0x0809 Mem:0x1 MMU:0x2830 AS:0xff JS:0x7 [ 501.321752] panfrost 1304.gpu: shader_present=0x7 l2_present=0x1 [ 501.324951] [drm] Initialized panfrost 1.1.0 20180908 for 1304.gpu on minor 2 Some more changes are still required to get devfreq working, and of course I do not have a userspace driver to test this with. I believe at least patches 1, 2, and 3 can be merged. 4 and 5 are mostly useful in conjunction with 6 and 7 (which are not ready yet), so I'll let maintainers decide. Thanks! Nicolas Boichat (7): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: Improve error reporting in panfrost_gpu_power_on drm/panfrost: Add support for multiple regulators drm/panfrost: Add support for multiple power domains RFC: drm/panfrost: Add mt8183-mali compatible string RFC: drm/panfrost: devfreq: Add support for 2 regulators .../bindings/gpu/arm,mali-bifrost.yaml| 25 arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 7 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 +++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +++ drivers/gpu/drm/panfrost/panfrost_device.c| 123 +++--- drivers/gpu/drm/panfrost/panfrost_device.h| 27 +++- drivers/gpu/drm/panfrost/panfrost_drv.c | 41 -- drivers/gpu/drm/panfrost/panfrost_gpu.c | 11 +- 8 files changed, 326 insertions(+), 30 deletions(-) -- 2.25.0.341.g760bfbb309-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 2/7] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- Upstreaming what matches existing bindings from our Chromium OS tree: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/arch/arm64/boot/dts/mediatek/mt8183.dtsi#1348 The evb part of this change depends on this patch to add PMIC dtsi: https://patchwork.kernel.org/patch/10928161/ The binding we use with out-of-tree Mali drivers includes more clocks, this is used for devfreq: the out-of-tree driver switches clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then switches clk_mux back to clk_main_parent: (see https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423) clocks = < CLK_TOP_MFGPLL_CK>, < CLK_TOP_MUX_MFG>, <>, < CLK_MFG_BG3D>; clock-names = "clk_main_parent", "clk_mux", "clk_sub_parent", "subsys_mfg_cg"; v4: - Add power-domain-names to describe the 3 domains. (kept Alyssa's reviewed-by as the change is minor) v3: - No changes v2: - Use sram instead of mali_sram as SRAM supply name. - Rename mali@ to gpu@. arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 7 ++ arch/arm64/boot/dts/mediatek/mt8183.dtsi| 105 2 files changed, 112 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts index 1fb195c683c3d01..7d609e0cd9b4975 100644 --- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts @@ -7,6 +7,7 @@ /dts-v1/; #include "mt8183.dtsi" +#include "mt6358.dtsi" / { model = "MediaTek MT8183 evaluation board"; @@ -30,6 +31,12 @@ { status = "okay"; }; + { + supply-names = "mali", "sram"; + mali-supply = <_vgpu_reg>; + sram-supply = <_vsram_gpu_reg>; +}; + { pinctrl-names = "default"; pinctrl-0 = <_pins_0>; diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi index 124f9d3e09f532c..74b5305f663f740 100644 --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi @@ -599,6 +599,111 @@ mfgcfg: syscon@1300 { #clock-cells = <1>; }; + gpu: gpu@1304 { + compatible = "mediatek,mt8183-mali", "arm,mali-bifrost"; + reg = <0 0x1304 0 0x4000>; + interrupts = + , + , + ; + interrupt-names = "job", "mmu", "gpu"; + + clocks = < CLK_TOP_MFGPLL_CK>; + + power-domains = + < MT8183_POWER_DOMAIN_MFG_CORE0>, + < MT8183_POWER_DOMAIN_MFG_CORE1>, + < MT8183_POWER_DOMAIN_MFG_2D>; + power-domain-names = "core0", "core1", "2d"; + + operating-points-v2 = <_opp_table>; + }; + + gpu_opp_table: opp_table0 { + compatible = "operating-points-v2"; + opp-shared; + + opp-3 { + opp-hz = /bits/ 64 <3>; + opp-microvolt = <625000>, <85>; + }; + + opp-32000 { + opp-hz = /bits/ 64 <32000>; + opp-microvolt = <631250>, <85>; + }; + + opp-34000 { + opp-hz = /bits/ 64 <34000>; + opp-microvolt = <637500>, <85>; + }; + + opp-36000 { + opp-hz = /bits/ 64 <36000>; + opp-microvolt = <643750>, <85>; + }; + + opp-38000 { + opp-hz = /bits/ 64 <38000>; + opp-microvolt = <65>, <85>; + }; + + opp-4 { + opp-hz = /bits/ 64 <4>; + opp-microvolt = <656250>, <85>; + }; + + opp-42000 { +