Re: [PATCH 0/3] lib/string_helpers: Add a few string helpers
On Tue, 18 Jan 2022, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nik...@intel.com/ > 2) > https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevche...@linux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > a. This version should be a drop-in replacement for what is currently in >the tree, with no change in behavior or binary size. For binary >size what I checked wat that the linked objects in the end have the >same size (gcc 11). From comments in the previous attempts this seems >also the case for earlier compiler versions > > b. I didn't change the function name to choice_* as suggested by Andrew >Morton in 20191023155619.43e0013f0c8c673a5c508...@linux-foundation.org >because other people argumented in favor of shorter names for these >simple helpers - if they are long and people simply not use due to >that, we failed > > c. Use string_helper.h for these helpers - pulling string.h in the >compilations units was one of the concerns and I think re-using this >already existing header is better than creating a new string-choice.h > > d. This doesn't bring onoff() helper as there are some places in the >kernel with onoff as variable - another name is probably needed for >this function in order not to shadow the variable, or those variables >could be renamed. Or if people wanting >try to find a short one > > e. One alternative to all of this suggested by Christian König >(43456ba7-c372-84cc-4949-dcb817188...@amd.com) would be to add a >printk format. But besides the comment, he also seemed to like >the common function. This brought the argument from others that the >simple yesno()/enabledisable() already used in the code is easier to >remember and use than e.g. %py[DOY] > > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. Thanks for picking this up again. I agree with the approach here. Acked-by: Jani Nikula > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. > > thanks > Lucas De Marchi > > Cc: Alex Deucher > Cc: Andrew Morton > Cc: Andy Shevchenko > Cc: Andy Shevchenko > Cc: Ben Skeggs > Cc: Christian König > Cc: Chris Wilson > Cc: Daniel Vetter > Cc: David Airlie > Cc: David S. Miller > Cc: Emma Anholt > Cc: Eryk Brol > Cc: Francis Laniel > Cc: Greg Kroah-Hartman > Cc: Harry Wentland > Cc: Jakub Kicinski > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Julia Lawall > Cc: Kentaro Takeda > Cc: Leo Li > Cc: Mikita Lipski > Cc: Petr Mladek > Cc: Rahul Lakkireddy > Cc: Raju Rangoju > Cc: Rasmus Villemoes > Cc: Rodrigo Vivi > Cc: Sakari Ailus > Cc: Sergey Senozhatsky > Cc: Steven Rostedt > Cc: Vishal Kulkarni > > Lucas De Marchi (3): > lib/string_helpers: Consolidate yesno() implementation > lib/string_helpers: Add helpers for enable[d]/disable[d] > drm: Convert open yes/no strings to yesno() > > drivers/gpu/drm/amd/amdgpu/atom.c | 3 ++- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 6 +- > drivers/gpu/drm/drm_client_modeset.c | 3 ++- > drivers/gpu/drm/drm_dp_helper.c| 3 ++- > drivers/gpu/drm/drm_gem.c | 3 ++- > drivers/gpu/drm/i915/i915_utils.h | 15 --- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 4 +++- > drivers/gpu/drm/radeon/atom.c | 3 ++- > drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++- > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 3 ++- > .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 --- > include/linux/string_helpers.h | 4 > security/tomoyo/audit.c| 2 +- > security/tomoyo/common.c | 18 -- > security/tomoyo/common.h | 1 - > 15 files changed, 31 insertions(+), 59 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] mgag200 fix memmapsl configuration in GCTL6 register
Hi Am 18.01.22 um 20:06 schrieb Lyude Paul: We should probably Cc: sta...@vger.kernel.org this as well, see: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for more info. As well, some useful tools for adding the appropriate Fixes: tags: https://drm.pages.freedesktop.org/maintainer-tools/dim.html At least on my end this is: Acked-by: Lyude Paul I'd very much like Thomas Zimmerman to verify that this patch is OK though with an R-b before we push anything upstream. Yep, I'll give it a try on my test system. I'll also add a TODO comment that summarizes the situation. A real fix would detect that the kdump kernel is running and not use the display then. Best regards Thomas On Fri, 2022-01-14 at 10:47 +0100, Jocelyn Falempe wrote: On some server with MGA G200e (rev 42), booting with Legacy BIOS, The hardware hangs when using kdump and kexec into the kdump kernel. This happens when the uncompress code tries to write "Decompressing Linux" to the VGA Console. It can be reproduced by writing to the VGA console (0xB8000) after booting to graphic mode, it generates the following error: kernel:NMI: PCI system error (SERR) for reason a0 on CPU 0. kernel:Dazed and confused, but trying to continue The root cause is a bad configuration of the MGA GCTL6 register According to the GCTL6 register documentation: bit 0 is gcgrmode: 0: Enables alpha mode, and the character generator addressing system is activated. 1: Enables graphics mode, and the character addressing system is not used. bit 1 is chainodd even: 0: The A0 signal of the memory address bus is used during system memory addressing. 1: Allows A0 to be replaced by either the A16 signal of the system address (if memmapsl is ‘00’), or by the hpgoddev (MISC<5>, odd/even page select) field, described on page 3-294). bit 3-2 are memmapsl: Memory map select bits 1 and 0. VGA. These bits select where the video memory is mapped, as shown below: 00 => Ah - Bh 01 => Ah - Ah 10 => Bh - B7FFFh 11 => B8000h - Bh bit 7-4 are reserved. Current driver code set it to 0x05 => memmapsl to b01 => 0xA but on x86, the VGA console is at 0xB8000 arch/x86/boot/compressed/misc.c define vidmem to 0xb8000 in extract_kernel() so it's better to configure it to b11 Thus changing the value 0x05 to 0x0d Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/mgag200/mgag200_mode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index b983541a4c53..c7f63610b278 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -529,7 +529,7 @@ static void mgag200_set_format_regs(struct mga_device *mdev, WREG_GFX(3, 0x00); WREG_GFX(4, 0x00); WREG_GFX(5, 0x40); - WREG_GFX(6, 0x05); + WREG_GFX(6, 0x0d); WREG_GFX(7, 0x0f); WREG_GFX(8, 0x0f); -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[PATCH] drm/panel: Add missing pm_rumtime_resume_and_get
pm_runtime_put_autosuspend() and pm_runtime_put_sync_suspend()
will decrease the rumtime PM counter even when it returns an error.
Thus a pairing decrement is needed to prevent refcount leak.
Fix this by adding pm_runtime_resume_and_get() on error handling path.
Signed-off-by: Yongzhi Liu
---
drivers/gpu/drm/panel/panel-samsung-atna33xc20.c | 4 +++-
drivers/gpu/drm/panel/panel-simple.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
index 20666b6..dd7e3f1 100644
--- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
+++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
@@ -189,8 +189,10 @@ static int atana33xc20_unprepare(struct drm_panel *panel)
* to get the EDID or otherwise send DP AUX commands to the panel.
*/
ret = pm_runtime_put_sync_suspend(panel->dev);
- if (ret < 0)
+ if (ret < 0) {
+ pm_runtime_resume_and_get(panel->dev);
return ret;
+ }
p->prepared = false;
return 0;
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 9e46db5..ad18965 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -323,8 +323,10 @@ static int panel_simple_unprepare(struct drm_panel *panel)
pm_runtime_mark_last_busy(panel->dev);
ret = pm_runtime_put_autosuspend(panel->dev);
- if (ret < 0)
+ if (ret < 0) {
+ pm_runtime_resume_and_get(panel->dev);
return ret;
+ }
p->prepared = false;
return 0;
--
2.7.4
Re: [PATCH] dt-bindings: Improve phandle-array schemas
On Tue, Jan 18, 2022 at 07:50:38PM -0600, Rob Herring wrote: > The 'phandle-array' type is a bit ambiguous. It can be either just an > array of phandles or an array of phandles plus args. Many schemas for > phandle-array properties aren't clear in the schema which case applies > though the description usually describes it. > > The array of phandles case boils down to needing: > > items: > maxItems: 1 > > The phandle plus args cases should typically take this form: > > items: > - items: > - description: A phandle > - description: 1st arg cell > - description: 2nd arg cell > > With this change, some examples need updating so that the bracketing of > property values matches the schema. > > Cc: Damien Le Moal > Cc: Herbert Xu > Cc: "David S. Miller" > Cc: Chun-Kuang Hu > Cc: Philipp Zabel > Cc: Laurent Pinchart > Cc: Kieran Bingham > Cc: Vinod Koul > Cc: Georgi Djakov > Cc: Thomas Gleixner > Cc: Marc Zyngier > Cc: Joerg Roedel > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han > Cc: Pavel Machek > Cc: Mauro Carvalho Chehab > Cc: Krzysztof Kozlowski > Cc: Jakub Kicinski > Cc: Wolfgang Grandegger > Cc: Marc Kleine-Budde > Cc: Andrew Lunn > Cc: Vivien Didelot > Cc: Florian Fainelli > Cc: Vladimir Oltean > Cc: Kalle Valo > Cc: Viresh Kumar > Cc: Stephen Boyd > Cc: Kishon Vijay Abraham I > Cc: Linus Walleij > Cc: "Rafael J. Wysocki" > Cc: Kevin Hilman > Cc: Ulf Hansson > Cc: Sebastian Reichel > Cc: Mark Brown > Cc: Mathieu Poirier > Cc: Daniel Lezcano > Cc: Zhang Rui > Cc: Greg Kroah-Hartman > Cc: Thierry Reding > Cc: Jonathan Hunter > Cc: Sudeep Holla > Cc: Geert Uytterhoeven > Cc: linux-...@vger.kernel.org > Cc: linux-cry...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: dmaeng...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: io...@lists.linux-foundation.org > Cc: linux-l...@vger.kernel.org > Cc: linux-me...@vger.kernel.org > Cc: net...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: linux-wirel...@vger.kernel.org > Cc: linux-...@lists.infradead.org > Cc: linux-g...@vger.kernel.org > Cc: linux-ri...@lists.infradead.org > Cc: linux-remotep...@vger.kernel.org > Cc: alsa-de...@alsa-project.org > Cc: linux-...@vger.kernel.org > Signed-off-by: Rob Herring Acked-by: Greg Kroah-Hartman
Re: [PATCH 1/2] drm/msm: add support for QCM2290 MDSS
Hi Dmitry,
On Tue, 18 Jan 2022 at 19:02, Dmitry Baryshkov
wrote:
>
> On 18/01/2022 18:47, Loic Poulain wrote:
> > Add compatibility for QCM2290 display subsystem, including
> > required entries in DPU hw catalog.
> >
> > Signed-off-by: Loic Poulain
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 175
> > -
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 +
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 1 +
> > drivers/gpu/drm/msm/msm_drv.c | 1 +
> > 4 files changed, 177 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > index ce6f32a..7fa3fc7 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > @@ -25,6 +25,8 @@
> > #define VIG_SM8250_MASK \
[...]
> > +static const struct dpu_vbif_cfg qcm2290_vbif[] = {
> > + {
> > + .name = "vbif_0", .id = VBIF_0,
> > + .base = 0, .len = 0x1040,
> > + .features = BIT(DPU_VBIF_QOS_REMAP),
> > + .xin_halt_timeout = 0x4000,
> > + .qos_rt_tbl = {
> > + .npriority_lvl = ARRAY_SIZE(sdm845_rt_pri_lvl),
> > + .priority_lvl = sdm845_rt_pri_lvl,
> > + },
> > + .memtype_count = 14,
> > + .memtype = {3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3},
> > + },
> > +};
>
> The only difference from sdm845_vbif is the lack of .qos_nrt_tbl. Is
> this on purpose?
Yes, I've not found any info related to non-rt for QCM2290 dpu, but I
assume it would be safe to just use sdm845_vbif here, as the others.
Regards,
Loic
Re: [PATCH v2] phy: dphy: Correct clk_pre parameter
Hi,
On 19/01/2022 03:37, Liu Ying wrote:
> The D-PHY specification (v1.2) explicitly mentions that the T-CLK-PRE
> parameter's unit is Unit Interval(UI) and the minimum value is 8. Also,
> kernel doc of the 'clk_pre' member of struct phy_configure_opts_mipi_dphy
> mentions that it should be in UI. However, the dphy core driver wrongly
> sets 'clk_pre' to 8000, which seems to hint that it's in picoseconds.
> And, the kernel doc of the 'clk_pre' member wrongly says the minimum value
> is '8 UI', instead of 8.
>
> So, let's fix both the dphy core driver and the kernel doc of the 'clk_pre'
> member to correctly reflect the T-CLK-PRE parameter's unit and the minimum
> value according to the D-PHY specification.
>
> I'm assuming that all impacted custom drivers shall program values in
> TxByteClkHS cycles into hardware for the T-CLK-PRE parameter. The D-PHY
> specification mentions that the frequency of TxByteClkHS is exactly 1/8
> the High-Speed(HS) bit rate(each HS bit consumes one UI). So, relevant
> custom driver code is changed to program those values as
> DIV_ROUND_UP(cfg->clk_pre, BITS_PER_BYTE), then.
>
> Note that I've only tested the patch with RM67191 DSI panel on i.MX8mq EVK.
> Help is needed to test with other i.MX8mq, Meson and Rockchip platforms,
> as I don't have the hardwares.
>
> Fixes: 2ed869990e14 ("phy: Add MIPI D-PHY configuration options")
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Kishon Vijay Abraham I
> Cc: Vinod Koul
> Cc: Kevin Hilman
> Cc: Jerome Brunet
> Cc: Martin Blumenstingl
> Cc: Heiko Stuebner
> Cc: Maxime Ripard
> Cc: Guido Günther
> Tested-by: Liu Ying # RM67191 DSI panel on i.MX8mq EVK
> Signed-off-by: Liu Ying
> ---
> v1->v2:
> * Use BITS_PER_BYTE macro. (Andrzej)
> * Drop dsi argument from ui2bc() in nwl-dsi.c.
>
> drivers/gpu/drm/bridge/nwl-dsi.c | 12 +---
> drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c| 3 ++-
> drivers/phy/phy-core-mipi-dphy.c | 4 ++--
> drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c | 3 ++-
> include/linux/phy/phy-mipi-dphy.h| 2 +-
> 5 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c
> b/drivers/gpu/drm/bridge/nwl-dsi.c
> index a7389a0facfb..af07eeb47ca0 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -196,12 +197,9 @@ static u32 ps2bc(struct nwl_dsi *dsi, unsigned long long
> ps)
> /*
> * ui2bc - UI time periods to byte clock cycles
> */
> -static u32 ui2bc(struct nwl_dsi *dsi, unsigned long long ui)
> +static u32 ui2bc(unsigned int ui)
> {
> - u32 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> -
> - return DIV64_U64_ROUND_UP(ui * dsi->lanes,
> - dsi->mode.clock * 1000 * bpp);
> + return DIV_ROUND_UP(ui, BITS_PER_BYTE);
> }
>
> /*
> @@ -232,12 +230,12 @@ static int nwl_dsi_config_host(struct nwl_dsi *dsi)
> }
>
> /* values in byte clock cycles */
> - cycles = ui2bc(dsi, cfg->clk_pre);
> + cycles = ui2bc(cfg->clk_pre);
> DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_t_pre: 0x%x\n", cycles);
> nwl_dsi_write(dsi, NWL_DSI_CFG_T_PRE, cycles);
> cycles = ps2bc(dsi, cfg->lpx + cfg->clk_prepare + cfg->clk_zero);
> DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap (pre): 0x%x\n", cycles);
> - cycles += ui2bc(dsi, cfg->clk_pre);
> + cycles += ui2bc(cfg->clk_pre);
> DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_t_post: 0x%x\n", cycles);
> nwl_dsi_write(dsi, NWL_DSI_CFG_T_POST, cycles);
> cycles = ps2bc(dsi, cfg->hs_exit);
> diff --git a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> index cd2332bf0e31..fdbd64c03e12 100644
> --- a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> +++ b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> @@ -9,6 +9,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -250,7 +251,7 @@ static int phy_meson_axg_mipi_dphy_power_on(struct phy
> *phy)
>(DIV_ROUND_UP(priv->config.clk_zero, temp) << 16) |
>(DIV_ROUND_UP(priv->config.clk_prepare, temp) << 24));
> regmap_write(priv->regmap, MIPI_DSI_CLK_TIM1,
> - DIV_ROUND_UP(priv->config.clk_pre, temp));
> + DIV_ROUND_UP(priv->config.clk_pre, BITS_PER_BYTE));
>
> regmap_write(priv->regmap, MIPI_DSI_HS_TIM,
>DIV_ROUND_UP(priv->config.hs_exit, temp) |
I'll try to run a test, currently the calculation gives 2, so this would give 1.
Neil
> diff --git a/drivers/phy/phy-core-mipi-dphy.c
> b/drivers/phy/phy-core-mipi-dphy.c
> index 288c9c67aa74..ccb4045685cd 100644
> --- a/drivers/phy/phy-core-
Re: [PATCH] MAINTAINERS: Add Helge as fbdev maintainer
On Tue, 18 Jan 2022 10:53:52 +0100 Gerd Hoffmann wrote: > On Tue, Jan 18, 2022 at 10:33:23AM +0200, Pekka Paalanen wrote: > > On Mon, 17 Jan 2022 19:47:39 +0100 > > Sven Schnelle wrote: > > > > > I also tested the speed on my Thinkpad X1 with Intel graphics, and there > > > a dmesg with 919 lines one the text console took about 2s to display. In > > > x11, i measure 22ms. This might be unfair because encoding might be > > > different, but i cannot confirm the 'memcpy' is faster than hardware > > > blitting' point. I think if that would be the case, no-one would care > > > about 2D acceleration. > > > > I think that is an extremely unfair comparison, because a graphical > > terminal app is not going to render every line of text streamed to it. > > It probably renders only the final view alone if you simply run > > 'dmesg', skipping the first 800-900 lines completely. > > Probably more like "render on every vblank", but yes, unlike fbcon it > surely wouldn't render every single character sent to the terminal. Yes, and since 1k lines of dmesg is such little data, I would guess even an old machine can chew that up in much less than one refresh period until it needs to draw, so there is only going to be one or two screen updates to be drawn. Also, since X11 does not have vblank or frame boundaries in the protocol, a terminal emulator app will do render throttling somehow else. Maybe when it temporarily exhausts input and a timer as a deadline in case input just keeps on flooding, would be my wild guess. Thanks, pq pgpp2H2F_BmJw.pgp Description: OpenPGP digital signature
Re: [PATCH] drm/vmwgfx: Stop requesting the pci regions
Hi Zack
Am 17.01.22 um 19:03 schrieb Zack Rusin:
From: Zack Rusin
When sysfb_simple is enabled loading vmwgfx fails because the regions
are held by the platform. In that case remove_conflicting*_framebuffers
only removes the simplefb but not the regions held by sysfb.
I don't understand this sentence. There's only one memory resource
claimed by the sysfb code. What else would block vmwgfx?
I appears to me as if simplefb should release the region (or the
simple-framebuffer device).
simpledrm does a hot-unplug of the simple-framebuffer, so the region
should be released afterwards.
Best regard
Thomas
Like the other drm drivers we need to stop requesting all the pci regions
to let the driver load with platform code enabled.
This allows vmwgfx to load correctly on systems with sysfb_simple enabled.
Signed-off-by: Zack Rusin
Fixes: 523375c943e5 ("drm/vmwgfx: Port vmwgfx to arm64")
Cc: dri-devel@lists.freedesktop.org
Cc:
Reviewed-by: Martin Krastev
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index fe36efdb7ff5..27feb19f3324 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -724,10 +724,6 @@ static int vmw_setup_pci_resources(struct vmw_private *dev,
pci_set_master(pdev);
- ret = pci_request_regions(pdev, "vmwgfx probe");
- if (ret)
- return ret;
-
dev->pci_id = pci_id;
if (pci_id == VMWGFX_PCI_ID_SVGA3) {
rmmio_start = pci_resource_start(pdev, 0);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
[RFC] How to add hardware rotation, scaling etc to a DRM/KMS driver
Hi all, I've copied and pasted my way to mostly working DRM/KMS driver for a low cost ARM SoC (Sigmastar SSD202D). The hardware is 2D only. One of the devices that uses this SoC has the screen upside down so it needs the screen rotated. The hardware doesn't have bits that change the scan out direction from what I can tell (so it can't mirror/flip while feeding it to the screen) but it does have a 2D blitter style block that can take a framebuffer and flip/mirror/scale/convert the colour space into another buffer. My idea was to create a buffer for the rotated image when allocating the framebuffer and trigger the hardware to do the conversion each vblank or something. While reading the discussion about maintaining fbdev I realised maybe I should ask instead of wasting too much time on something that's wrong. I got the feeling that maybe I should just provide an interface to the blitter from userspace and userspace should be doing the rotation. I'd like to do it in the kernel so stuff like SDL1 apps just work but maybe that isn't possible? Cheers, Daniel
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/gt: make a gt sysfs group and move power management files
Hi Joonas, > > > The GT has its own properties and in sysfs they should be grouped > > > in the 'gt/' directory. > > > > > > Create a 'gt/' directory in sysfs which will contain gt0...gtN > > > directories related to each tile configured in the GPU. Move the > > > power management files inside those directories. > > > > > > The previous power management files are kept in their original > > > root directory to avoid breaking the ABI. They point to the tile > > > '0' and a warning message is printed whenever accessed to. > > This is wrong. They should act as multiplexers to all the tiles. > > Needs to be fixed before merging. I have a patch for this and I planned to send it later. I have even been asked to split this one in more chunks as the review is a bit difficult. > > > The > > > deprecated interface needs for the CONFIG_SYSFS_DEPRECATED_V2 > > > flag in order to be generated. > > > > CONFIG_SYSFS_DEPRECATED_V2 idea was abandoned, no? This patch at least > > does not appear to contain it so please update the commit message to > > reflect current state. > > > > Adding Joonas to help address the question of how strict are userspace > > requirements for sysfs additions. Personally sysadmin use sounds fine to > > me, although it needs to be mentioned/documented as Matt requested, but > > I fear it may not be enough to upstream. Is Level0 at the stage where we > > can upstream for it I am also not sure. > > Sysadmin usage is fine for the simple interfaces that can truly be used > from the command line. This patch seems to just expose the existing > interface in per-tile manner, so should not be a concern. This will definitely help this patch (series) to get in, but I my understanding is that Level0 is a bit behind for upstreaming. > However, the controls not under gt directories, need to be converted to > apply to all tiles. (I've definitely given that feedback multiple > times). Otherwise it will be very unexpected to the end user when what > previously applied to whole device would only apply to part of it. It's not forgotten :) > Regards, Joonas Thank you, Andi
Re: [PATCH v3 00/10] drm: Make drivers to honour the nomodeset parameter
On 12/22/21 09:28, Javier Martinez Canillas wrote: > The nomodeset kernel command line parameter is used to prevent the KMS/DRM > drivers to be registered/probed. But only a few drivers implement support > for this and most DRM drivers just ignore it. > > This patch series is a v3 to make DRM drivers to honour nomodeset. It is > posted as separate patches to make easier for drivers maintainers to ack > or pick them independently at their own pace. > [snip] > > Thomas Zimmermann (5): > drm: Provide PCI module-init macros > drm/ast: Replace module-init boiler-plate code with DRM helpers > drm/bochs: Replace module-init boiler-plate code with DRM helpers > drm/cirrus: Replace module-init boiler-plate code with DRM helpers > drm/hisilicon/hibmc: Replace module initialization with DRM helpers > For Thomas' patches (1-5) Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: (subset) [PATCH] drm/vc4: Fix deadlock on DSI device attach error
On Tue, 18 Jan 2022 01:51:26 +0100, Padmanabha Srinivasaiah wrote: > DSI device attach to DSI host will be done with host device's lock > held. > > Un-registering host in "device attach" error path (ex: probe retry) > will result in deadlock with below call trace and non operational > DSI display. > > [...] Applied to drm/drm-misc (drm-misc-next-fixes). Thanks! Maxime
Re: [PATCH v2] phy: dphy: Correct clk_pre parameter
On 19/01/2022 09:40, Neil Armstrong wrote:
> Hi,
>
> On 19/01/2022 03:37, Liu Ying wrote:
>> The D-PHY specification (v1.2) explicitly mentions that the T-CLK-PRE
>> parameter's unit is Unit Interval(UI) and the minimum value is 8. Also,
>> kernel doc of the 'clk_pre' member of struct phy_configure_opts_mipi_dphy
>> mentions that it should be in UI. However, the dphy core driver wrongly
>> sets 'clk_pre' to 8000, which seems to hint that it's in picoseconds.
>> And, the kernel doc of the 'clk_pre' member wrongly says the minimum value
>> is '8 UI', instead of 8.
>>
>> So, let's fix both the dphy core driver and the kernel doc of the 'clk_pre'
>> member to correctly reflect the T-CLK-PRE parameter's unit and the minimum
>> value according to the D-PHY specification.
>>
>> I'm assuming that all impacted custom drivers shall program values in
>> TxByteClkHS cycles into hardware for the T-CLK-PRE parameter. The D-PHY
>> specification mentions that the frequency of TxByteClkHS is exactly 1/8
>> the High-Speed(HS) bit rate(each HS bit consumes one UI). So, relevant
>> custom driver code is changed to program those values as
>> DIV_ROUND_UP(cfg->clk_pre, BITS_PER_BYTE), then.
>>
>> Note that I've only tested the patch with RM67191 DSI panel on i.MX8mq EVK.
>> Help is needed to test with other i.MX8mq, Meson and Rockchip platforms,
>> as I don't have the hardwares.
>>
>> Fixes: 2ed869990e14 ("phy: Add MIPI D-PHY configuration options")
>> Cc: Andrzej Hajda
>> Cc: Neil Armstrong
>> Cc: Robert Foss
>> Cc: Laurent Pinchart
>> Cc: Jonas Karlman
>> Cc: Jernej Skrabec
>> Cc: David Airlie
>> Cc: Daniel Vetter
>> Cc: Kishon Vijay Abraham I
>> Cc: Vinod Koul
>> Cc: Kevin Hilman
>> Cc: Jerome Brunet
>> Cc: Martin Blumenstingl
>> Cc: Heiko Stuebner
>> Cc: Maxime Ripard
>> Cc: Guido Günther
>> Tested-by: Liu Ying # RM67191 DSI panel on i.MX8mq EVK
>> Signed-off-by: Liu Ying
>> ---
>> v1->v2:
>> * Use BITS_PER_BYTE macro. (Andrzej)
>> * Drop dsi argument from ui2bc() in nwl-dsi.c.
>>
>> drivers/gpu/drm/bridge/nwl-dsi.c | 12 +---
>> drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c| 3 ++-
>> drivers/phy/phy-core-mipi-dphy.c | 4 ++--
>> drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c | 3 ++-
>> include/linux/phy/phy-mipi-dphy.h| 2 +-
>> 5 files changed, 12 insertions(+), 12 deletions(-)
>>
[...]
>> diff --git a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
>> b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
>> index cd2332bf0e31..fdbd64c03e12 100644
>> --- a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
>> +++ b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
>> @@ -9,6 +9,7 @@
>>
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -250,7 +251,7 @@ static int phy_meson_axg_mipi_dphy_power_on(struct phy
>> *phy)
>> (DIV_ROUND_UP(priv->config.clk_zero, temp) << 16) |
>> (DIV_ROUND_UP(priv->config.clk_prepare, temp) << 24));
>> regmap_write(priv->regmap, MIPI_DSI_CLK_TIM1,
>> - DIV_ROUND_UP(priv->config.clk_pre, temp));
>> + DIV_ROUND_UP(priv->config.clk_pre, BITS_PER_BYTE));
>>
>> regmap_write(priv->regmap, MIPI_DSI_HS_TIM,
>> DIV_ROUND_UP(priv->config.hs_exit, temp) |
>
> I'll try to run a test, currently the calculation gives 2, so this would give
> 1.
The Amlogic vendor code does:
/* >8*ui */
#define DPHY_TIME_CLK_PRE(ui) (10 * ui)
t_ui = lcd_timing.bit_rate
t_ui = (100 * 100) / (dsi_ui / 1000); /*100*ns */
temp = t_ui * 8; /* lane_byte cycle time */
dphy->clk_pre = ((DPHY_TIME_CLK_PRE(t_ui) + temp - 1) / temp) & 0xff;
PHY Registers only says:
MIPI_DSI_CLK_TIM1 [31:0]
7:0 R/W 0tCLK_PRE
>
> Neil
>
[...]
Re: [PATCH] drm/vmwgfx: Stop requesting the pci regions
Hi
Am 19.01.22 um 03:15 schrieb Zack Rusin:
On Tue, 2022-01-18 at 20:00 +0100, Javier Martinez Canillas wrote:
Hello Zack,
On 1/17/22 19:03, Zack Rusin wrote:
From: Zack Rusin
When sysfb_simple is enabled loading vmwgfx fails because the regions
are held by the platform. In that case
remove_conflicting*_framebuffers
only removes the simplefb but not the regions held by sysfb.
Indeed, that's an issue. I wonder if we should drop the IORESOURCE_BUSY
flag from the memory resource added to the "simple-framebuffer" device
?
I think this is one of those cases where it depends on what we plan to
do after that. Sementically it makes sense to have it in there - the
framebuffer memory is claimed by the "simple-framebuffer" and it's
busy, it's just that it creates issues for drivers after unloading. I
think removing it, while making things easier for drivers, would be
confusing for people reading the code later, unless there's some kind
of cleanup that would happen with it (e.g. removing IORESOURCE_BUSY
altogether and making the drm drivers properly request their
resources). At least by itself it doesn't seem to be much better
solution than having the drm drivers not call pci_request_region[s],
which apart from hyperv and cirrus (iirc bochs does it for resources
other than fb which wouldn't have been claimed by "simple-framebuffer")
is already the case.
I do think we should do one of them to make the codebase coherent:
either remove IORESOURCE_BUSY from "simple-framebuffer" or remove
pci_request_region[s] from hyperv and cirrus.
I just discussed this a bit with Javier. It's a problem with the
simple-framebuffer code, rather then vmwgfx.
IMHO the best solution is to drop IORESOURCE_BUSY from sysfb and have
drivers register/release the range with _BUSY. That would signal the
memory belongs to the sysfb device but is not busy unless a driver has
been bound. After simplefb released the range, it should be 'non-busy'
again and available for vmwgfx. Simpledrm does a hot-unplug of the sysfb
device, so the memory range gets released entirely. If you want, I'll
prepare some patches for this scenario.
If this doesn't work, pushing all request/release pairs into drivers
would be my next option.
If none of this is feasible, we can still remove pci_request_region()
from vmwgfx.
Best regards
Thomas
Like the other drm drivers we need to stop requesting all the pci
regions
to let the driver load with platform code enabled.
This allows vmwgfx to load correctly on systems with sysfb_simple
enabled.
I read this very interesting thread from two years ago:
https://lkml.org/lkml/2020/11/5/248
Maybe is worth mentioning in the commit message what Daniel said there,
that is that only a few DRM drivers request explicitly the PCI regions
and the only reliable approach is for bus drivers to claim these.
Ah, great point. I'll update the commit log with that.
Signed-off-by: Zack Rusin
Fixes: 523375c943e5 ("drm/vmwgfx: Port vmwgfx to arm64")
Cc: dri-devel@lists.freedesktop.org
Cc:
Reviewed-by: Martin Krastev
---
The patch looks good to me, thanks a lot for fixing this:
Reviewed-by: Javier Martinez Canillas
Thanks for taking a look at this, I appreciate it!
z
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH 1/3] lib/string_helpers: Consolidate yesno() implementation
On Wednesday, January 19, 2022, Lucas De Marchi
wrote:
> There are a few implementations of yesno() in the tree. Consolidate them
> in include/linux/string_helpers.h. Quite a few users of open coded
> yesno() could later be converted to the new function:
>
> $ git grep '?\s*"yes"\s*' | wc -l
> 286
> $ git grep '?\s*"no"\s*' | wc -l
> 20
>
> The inlined function should keep the const strings local to each
> compilation unit, the same way it's now, thus not changing the current
> behavior.
>
> Signed-off-by: Lucas De Marchi
> ---
> .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 6 +-
> drivers/gpu/drm/i915/i915_utils.h | 5 -
> .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 ---
> include/linux/string_helpers.h | 2 ++
> security/tomoyo/audit.c| 2 +-
> security/tomoyo/common.c | 18 --
> security/tomoyo/common.h | 1 -
> 7 files changed, 8 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index 9d43ecb1f692..b59760f91bf6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -23,6 +23,7 @@
> *
> */
>
> +#include
> #include
>
> #include "dc.h"
> @@ -49,11 +50,6 @@ struct dmub_debugfs_trace_entry {
> uint32_t param1;
> };
>
> -static inline const char *yesno(bool v)
> -{
> - return v ? "yes" : "no";
> -}
> -
> /* parse_write_buffer_into_params - Helper function to parse debugfs
> write buffer into an array
> *
> * Function takes in attributes passed to debugfs write entry
> diff --git a/drivers/gpu/drm/i915/i915_utils.h
> b/drivers/gpu/drm/i915/i915_utils.h
> index 7a5925072466..2a8781cc648b 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -414,11 +414,6 @@ wait_remaining_ms_from_jiffies(unsigned long
> timestamp_jiffies, int to_wait_ms)
> #define MBps(x) KBps(1000 * (x))
> #define GBps(x) ((u64)1000 * MBps((x)))
>
> -static inline const char *yesno(bool v)
> -{
> - return v ? "yes" : "no";
> -}
> -
> static inline const char *onoff(bool v)
> {
> return v ? "on" : "off";
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
> index 7d49fd4edc9e..61a04d7abc1f 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
> @@ -2015,17 +2015,6 @@ static const struct file_operations
> rss_debugfs_fops = {
> /* RSS Configuration.
> */
>
> -/* Small utility function to return the strings "yes" or "no" if the
> supplied
> - * argument is non-zero.
> - */
> -static const char *yesno(int x)
> -{
> - static const char *yes = "yes";
> - static const char *no = "no";
> -
> - return x ? yes : no;
> -}
> -
> static int rss_config_show(struct seq_file *seq, void *v)
> {
> struct adapter *adapter = seq->private;
> diff --git a/include/linux/string_helpers.h b/include/linux/string_
> helpers.h
> index 4ba39e1403b2..e980dec05d31 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -102,4 +102,6 @@ char *kstrdup_quotable_file(struct file *file, gfp_t
> gfp);
>
> void kfree_strarray(char **array, size_t n);
>
> +static inline const char *yesno(bool v) { return v ? "yes" : "no"; }
Perhaps keep it on 4 lines? Yes, yes/no is short, but if we add others
(enable/disable) it will not be possible to keep on one line. And hence
style will be broken among similar functions.
Also it needs to be rebased and resend after -rc1, I expect conflict here.
> +
> #endif
> diff --git a/security/tomoyo/audit.c b/security/tomoyo/audit.c
> index d79bf07e16be..1458e27361e8 100644
> --- a/security/tomoyo/audit.c
> +++ b/security/tomoyo/audit.c
> @@ -166,7 +166,7 @@ static char *tomoyo_print_header(struct
> tomoyo_request_info *r)
>"#%04u/%02u/%02u %02u:%02u:%02u# profile=%u mode=%s
> granted=%s (global-pid=%u) task={ pid=%u ppid=%u uid=%u gid=%u euid=%u
> egid=%u suid=%u sgid=%u fsuid=%u fsgid=%u }",
>stamp.year, stamp.month, stamp.day, stamp.hour,
>stamp.min, stamp.sec, r->profile,
> tomoyo_mode[r->mode],
> - tomoyo_yesno(r->granted), gpid, tomoyo_sys_getpid(),
> + yesno(r->granted), gpid, tomoyo_sys_getpid(),
>tomoyo_sys_getppid(),
>from_kuid(&init_user_ns, current_uid()),
>from_kgid(&init_user_ns, current_gid()),
> diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
> index 5c64927bf2b3..304ed0f426dd 100644
> --- a/security/tomoyo/common.c
> +++ b/security/tomoyo/common.c
> @@ -8,6 +8,7 @@
> #include
> #inclu
Re: [PATCH v7 5/8] drm/mediatek: dpi: Add dpintf support
Hi, Guillaume:
On Fri, 2021-12-17 at 16:08 +0100, Guillaume Ranquet wrote:
> From: Markus Schneider-Pargmann
>
> dpintf is the displayport interface hardware unit. This unit is
> similar
> to dpi and can reuse most of the code.
>
> This patch adds support for mt8195-dpintf to this dpi driver. Main
> differences are:
> - Some features/functional components are not available for dpintf
>which are now excluded from code execution once is_dpintf is set
> - dpintf can and needs to choose between different clockdividers
> based
>on the clockspeed. This is done by choosing a different clock
> parent.
> - There are two additional clocks that need to be managed. These are
>only set for dpintf and will be set to NULL if not supplied. The
>clk_* calls handle these as normal clocks then.
> - Some register contents differ slightly between the two components.
> To
>work around this I added register bits/masks with a DPINTF_ prefix
>and use them where different.
>
> Based on a separate driver for dpintf created by
> Jason-JH.Lin .
>
> Signed-off-by: Markus Schneider-Pargmann
> Signed-off-by: Guillaume Ranquet
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delre...@collabora.com>
> ---
> drivers/gpu/drm/mediatek/mtk_dpi.c | 304
>
> drivers/gpu/drm/mediatek/mtk_dpi_regs.h | 38 +++
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 4 +
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +-
> include/linux/soc/mediatek/mtk-mmsys.h | 2 +
> 6 files changed, 299 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 4554e2de14309..fbc43ea4049b9 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -63,6 +63,14 @@ enum mtk_dpi_out_color_format {
> MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL
> };
>
> +enum TVDPLL_CLK {
> + TVDPLL_PLL = 0,
> + TVDPLL_D2 = 2,
> + TVDPLL_D4 = 4,
> + TVDPLL_D8 = 8,
> + TVDPLL_D16 = 16,
> +};
> +
> struct mtk_dpi {
> struct drm_encoder encoder;
> struct drm_bridge bridge;
> @@ -71,8 +79,10 @@ struct mtk_dpi {
> void __iomem *regs;
> struct device *dev;
> struct clk *engine_clk;
> + struct clk *dpi_ck_cg;
> struct clk *pixel_clk;
> struct clk *tvd_clk;
> + struct clk *pclk_src[5];
> int irq;
> struct drm_display_mode mode;
> const struct mtk_dpi_conf *conf;
> @@ -125,6 +135,18 @@ struct mtk_dpi_conf {
> bool edge_sel_en;
> const u32 *output_fmts;
> u32 num_output_fmts;
> + bool is_ck_de_pol;
Seperate is_ck_de_pol to an independent patch.
> + bool is_dpintf;
Ditto for is_dpintf. And I would like change the name to what this
actually do.
> + bool csc_support;
Ditto for csc_support.
> + bool swap_input_support;
Ditto for swap_input_support.
> + // Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH
/* ... */
> (no shift)
> + u32 dimension_mask;
Ditto for dimension_mask.
> + // Mask used for HSIZE and VSIZE (no shift)
> + u32 hvsize_mask;
Ditto for hvsize_mask.
> + u32 channel_swap_shift;
Ditto for channel_swap_shift.
> + u32 yuv422_en_bit;
Ditto for yuv422_en_bit.
> + u32 csc_enable_bit;
Ditto for csc_enable_bit.
> + const struct mtk_dpi_yc_limit *limit;
Ditto for limit.
> };
>
>
> +
> +static const struct mtk_dpi_conf mt8195_dpintf_conf = {
> + .cal_factor = mt8195_dpintf_calculate_factor,
> + .output_fmts = mt8195_output_fmts,
> + .num_output_fmts = ARRAY_SIZE(mt8195_output_fmts),
> + .is_dpintf = true,
> + .csc_support = true,
For every SoC, csc_support is true. Why do we need csc_support?
Regards,
CK
> + .dimension_mask = DPINTF_HPW_MASK,
> + .hvsize_mask = DPINTF_HSIZE_MASK,
> + .channel_swap_shift = DPINTF_CH_SWAP,
> + .yuv422_en_bit = DPINTF_YUV422_EN,
> + .csc_enable_bit = DPINTF_CSC_ENABLE,
> + .limit = &mtk_dpintf_limit,
> };
>
>
Re: [PATCH 1/3] lib/string_helpers: Consolidate yesno() implementation
Hi Lucas, On Tue, Jan 18, 2022 at 11:24:48PM -0800, Lucas De Marchi wrote: > @@ -1354,8 +1345,7 @@ static bool tomoyo_print_condition(struct > tomoyo_io_buffer *head, > case 3: > if (cond->grant_log != TOMOYO_GRANTLOG_AUTO) > tomoyo_io_printf(head, " grant_log=%s", > - tomoyo_yesno(cond->grant_log == > - TOMOYO_GRANTLOG_YES)); > + yesno(cond->grant_log == > TOMOYO_GRANTLOG_YES)); This would be better split on two lines. Then, Reviewed-by: Sakari Ailus -- Sakari Ailus
Re: [PATCH 2/3] lib/string_helpers: Add helpers for enable[d]/disable[d]
On Wednesday, January 19, 2022, Lucas De Marchi
wrote:
> Follow the yes/no logic and add helpers for enabled/disabled and
> enable/disable - those are not so common throughout the kernel,
> but they give a nice way to reuse the strings to log things as
> enabled/disabled or enable/disable.
>
> Signed-off-by: Lucas De Marchi
> ---
> drivers/gpu/drm/i915/i915_utils.h | 10 --
> include/linux/string_helpers.h| 2 ++
> 2 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_utils.h
> b/drivers/gpu/drm/i915/i915_utils.h
> index 2a8781cc648b..cbec79bae0d2 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -419,16 +419,6 @@ static inline const char *onoff(bool v)
> return v ? "on" : "off";
> }
>
> -static inline const char *enabledisable(bool v)
> -{
> - return v ? "enable" : "disable";
> -}
> -
> -static inline const char *enableddisabled(bool v)
> -{
> - return v ? "enabled" : "disabled";
> -}
> -
> void add_taint_for_CI(struct drm_i915_private *i915, unsigned int taint);
> static inline void __add_taint_for_CI(unsigned int taint)
> {
> diff --git a/include/linux/string_helpers.h b/include/linux/string_
> helpers.h
> index e980dec05d31..e4b82f364ee1 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -103,5 +103,7 @@ char *kstrdup_quotable_file(struct file *file, gfp_t
> gfp);
> void kfree_strarray(char **array, size_t n);
>
> static inline const char *yesno(bool v) { return v ? "yes" : "no"; }
> +static inline const char *enabledisable(bool v) { return v ? "enable" :
> "disable"; }
> +static inline const char *enableddisabled(bool v) { return v ? "enabled"
> : "disabled"; }
Looks not readable even if takes 80 characters. Please, keep original style.
I believe you wanted to have nice negative statistics from day 1, then you
may add more patches in the series to cleanup more users.
>
> #endif
> --
> 2.34.1
>
>
--
With Best Regards,
Andy Shevchenko
Re: [PATCH] drm/selftests: Select DRM_DP_HELPER
Hello Thomas,
On 1/18/22 16:44, Thomas Zimmermann wrote:
> Resolve warnings about non-existing symbols by selecting DRM_DP_HELPER.
>
> Signed-off-by: Thomas Zimmermann
> Fixes: adb9d5a2cc77 ("drm/dp: Move DisplayPort helpers into separate helper
> module")
> Reported-by: kernel test robot
> Cc: Lyude Paul
> Cc: Daniel Vetter
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 91f54aeb0b7c..6589d931 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -68,6 +68,7 @@ config DRM_DEBUG_SELFTEST
> depends on DRM
> depends on DEBUG_KERNEL
> select PRIME_NUMBERS
> + select DRM_DP_HELPER
> select DRM_LIB_RANDOM
> select DRM_KMS_HELPER
> select DRM_EXPORT_FOR_TESTS if m
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH] drm/msm: Fix include statements for DisplayPort
On 1/18/22 16:44, Thomas Zimmermann wrote:
> Update the include statements for DisplayPort helpers. The header
> files are in the dp/ subdirectory.
>
> Signed-off-by: Thomas Zimmermann
> Fixes: 5b529e8d9c38 ("drm/dp: Move public DisplayPort headers into dp/")
> Reported-by: kernel test robot
> Cc: Lyude Paul
> Cc: Daniel Vetter
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/msm/edp/edp.h | 2 +-
> drivers/gpu/drm/msm/edp/edp_ctrl.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
This patch looks good to me as well.
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH 0/3] lib/string_helpers: Add a few string helpers
On Wednesday, January 19, 2022, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani. > nik...@intel.com/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy. > shevche...@linux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > a. This version should be a drop-in replacement for what is currently in >the tree, with no change in behavior or binary size. For binary >size what I checked wat that the linked objects in the end have the >same size (gcc 11). From comments in the previous attempts this seems >also the case for earlier compiler versions > > b. I didn't change the function name to choice_* as suggested by Andrew >Morton in 20191023155619.43e0013f0c8c673a5c508...@linux-foundation.org >because other people argumented in favor of shorter names for these >simple helpers - if they are long and people simply not use due to >that, we failed > > c. Use string_helper.h for these helpers - pulling string.h in the >compilations units was one of the concerns and I think re-using this >already existing header is better than creating a new string-choice.h > > d. This doesn't bring onoff() helper as there are some places in the >kernel with onoff as variable - another name is probably needed for >this function in order not to shadow the variable, or those variables >could be renamed. Or if people wanting >try to find a short one > > e. One alternative to all of this suggested by Christian König >(43456ba7-c372-84cc-4949-dcb817188...@amd.com) would be to add a >printk format. But besides the comment, he also seemed to like >the common function. This brought the argument from others that the >simple yesno()/enabledisable() already used in the code is easier to >remember and use than e.g. %py[DOY] > > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. I believe the best if we go via drm-misc with the entire series. I have couple of comments, after addressing them: Reviewed-by: Andy Shevchenko > thanks > Lucas De Marchi > > Cc: Alex Deucher > Cc: Andrew Morton > Cc: Andy Shevchenko > Cc: Andy Shevchenko > Cc: Ben Skeggs > Cc: Christian König > Cc: Chris Wilson > Cc: Daniel Vetter > Cc: David Airlie > Cc: David S. Miller > Cc: Emma Anholt > Cc: Eryk Brol > Cc: Francis Laniel > Cc: Greg Kroah-Hartman > Cc: Harry Wentland > Cc: Jakub Kicinski > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Julia Lawall > Cc: Kentaro Takeda > Cc: Leo Li > Cc: Mikita Lipski > Cc: Petr Mladek > Cc: Rahul Lakkireddy > Cc: Raju Rangoju > Cc: Rasmus Villemoes > Cc: Rodrigo Vivi > Cc: Sakari Ailus > Cc: Sergey Senozhatsky > Cc: Steven Rostedt > Cc: Vishal Kulkarni > > Lucas De Marchi (3): > lib/string_helpers: Consolidate yesno() implementation > lib/string_helpers: Add helpers for enable[d]/disable[d] > drm: Convert open yes/no strings to yesno() > > drivers/gpu/drm/amd/amdgpu/atom.c | 3 ++- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 6 +- > drivers/gpu/drm/drm_client_modeset.c | 3 ++- > drivers/gpu/drm/drm_dp_helper.c| 3 ++- > drivers/gpu/drm/drm_gem.c | 3 ++- > drivers/gpu/drm/i915/i915_utils.h | 15 --- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 4 +++- > drivers/gpu/drm/radeon/atom.c | 3 ++- > drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++- > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 3 ++- > .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 --- > include/linux/string_helpers.h | 4 > security/tomoyo/audit.c| 2 +- > security/tomoyo/common.c | 18 -- > security/tomoyo/common.h | 1 - > 15 files changed, 31 insertions(+), 59 deletions(-) > > -- > 2.34.1 > > -- With Best Regards, Andy Shevchenko
Re: [PATCH] drm/edid: Support type 7 timings
On Wed, 19 Jan 2022, Yaroslav Bolyukin wrote:
> Per VESA DisplayID Standard v2.0: Type VII Timing – Detailed Timing Data
>
> Definitions were already provided as type I, but not used
Thanks for the patch. Functionally I think it looks correct, and
something we'll want. I do have some nitpicks though, comments inline.
For the next version, please consider Cc'ing the intel-gfx list as well
to get our CI results on it too.
BR,
Jani.
>
> Signed-off-by: Yaroslav Bolyukin
> ---
> drivers/gpu/drm/drm_edid.c | 26 +-
> include/drm/drm_displayid.h | 6 +++---
> 2 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 12893e7be..5fcefd9b5 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5404,13 +5404,17 @@ u32 drm_add_display_info(struct drm_connector
> *connector, const struct edid *edi
> return quirks;
> }
>
> -static struct drm_display_mode *drm_mode_displayid_detailed(struct
> drm_device *dev,
> - struct
> displayid_detailed_timings_1 *timings)
> +static struct drm_display_mode *drm_mode_displayid_detailed_1_7(struct
> drm_device *dev,
> + struct
> displayid_detailed_timings_1_7 *timings,
> + bool type_7)
I think the function rename here is unnecessary.
> {
> struct drm_display_mode *mode;
> unsigned pixel_clock = (timings->pixel_clock[0] |
> (timings->pixel_clock[1] << 8) |
> (timings->pixel_clock[2] << 16)) + 1;
> + // type 7 allows higher precision pixel clock
Please don't use // style comments.
For the comment contents, I think you should just state the units for
each; 10 kHz for type I, kHz for type VII.
> + if (!type_7)
> + pixel_clock *= 10;
Please don't mix declarations and code.
> unsigned hactive = (timings->hactive[0] | timings->hactive[1] << 8) + 1;
> unsigned hblank = (timings->hblank[0] | timings->hblank[1] << 8) + 1;
> unsigned hsync = (timings->hsync[0] | (timings->hsync[1] & 0x7f) << 8)
> + 1;
> @@ -5426,7 +5430,7 @@ static struct drm_display_mode
> *drm_mode_displayid_detailed(struct drm_device *d
> if (!mode)
> return NULL;
>
> - mode->clock = pixel_clock * 10;
> + mode->clock = pixel_clock;
Since we used to have the multiplication here (and we don't mix
declarations and code anyway) I'd keep it here.
Maybe:
mode->clock = type_7 ? pixel_clock : pixel_clock * 10;
> mode->hdisplay = hactive;
> mode->hsync_start = mode->hdisplay + hsync;
> mode->hsync_end = mode->hsync_start + hsync_width;
> @@ -5449,10 +5453,12 @@ static struct drm_display_mode
> *drm_mode_displayid_detailed(struct drm_device *d
> return mode;
> }
>
> -static int add_displayid_detailed_1_modes(struct drm_connector *connector,
> - const struct displayid_block *block)
> +static int add_displayid_detailed_1_7_modes(struct drm_connector *connector,
> + const struct displayid_block *block,
> + bool type_7)
> {
> - struct displayid_detailed_timing_block *det = (struct
> displayid_detailed_timing_block *)block;
> + struct displayid_detailed_timing_1_7_block *det =
> + (struct displayid_detailed_timing_1_7_block *)block;
I think the displayid_detailed_timing_block ->
displayid_detailed_timing_1_7_block rename is unnecessary.
> int i;
> int num_timings;
> struct drm_display_mode *newmode;
> @@ -5463,9 +5469,9 @@ static int add_displayid_detailed_1_modes(struct
> drm_connector *connector,
>
> num_timings = block->num_bytes / 20;
> for (i = 0; i < num_timings; i++) {
> - struct displayid_detailed_timings_1 *timings = &det->timings[i];
> + struct displayid_detailed_timings_1_7 *timings =
> &det->timings[i];
>
> - newmode = drm_mode_displayid_detailed(connector->dev, timings);
> + newmode = drm_mode_displayid_detailed_1_7(connector->dev,
> timings, type_7);
> if (!newmode)
> continue;
>
> @@ -5485,7 +5491,9 @@ static int add_displayid_detailed_modes(struct
> drm_connector *connector,
> displayid_iter_edid_begin(edid, &iter);
> displayid_iter_for_each(block, &iter) {
> if (block->tag == DATA_BLOCK_TYPE_1_DETAILED_TIMING)
> - num_modes += add_displayid_detailed_1_modes(connector,
> block);
> + num_modes +=
> add_displayid_detailed_1_7_modes(connector, block, false);
> + else if (block->tag == DATA_BLOCK_2_TYPE_7_DETAILED_TIMING)
> + num_modes +=
> add_displayid_detailed_1
Re: [PATCH 2/3] lib/string_helpers: Add helpers for enable[d]/disable[d]
On Wed, Jan 19, 2022 at 11:20:38AM +0200, Andy Shevchenko wrote:
On Wednesday, January 19, 2022, Lucas De Marchi
wrote:
Follow the yes/no logic and add helpers for enabled/disabled and
enable/disable - those are not so common throughout the kernel,
but they give a nice way to reuse the strings to log things as
enabled/disabled or enable/disable.
Signed-off-by: Lucas De Marchi
---
drivers/gpu/drm/i915/i915_utils.h | 10 --
include/linux/string_helpers.h| 2 ++
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_utils.h
b/drivers/gpu/drm/i915/i915_utils.h
index 2a8781cc648b..cbec79bae0d2 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -419,16 +419,6 @@ static inline const char *onoff(bool v)
return v ? "on" : "off";
}
-static inline const char *enabledisable(bool v)
-{
- return v ? "enable" : "disable";
-}
-
-static inline const char *enableddisabled(bool v)
-{
- return v ? "enabled" : "disabled";
-}
-
void add_taint_for_CI(struct drm_i915_private *i915, unsigned int taint);
static inline void __add_taint_for_CI(unsigned int taint)
{
diff --git a/include/linux/string_helpers.h b/include/linux/string_
helpers.h
index e980dec05d31..e4b82f364ee1 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -103,5 +103,7 @@ char *kstrdup_quotable_file(struct file *file, gfp_t
gfp);
void kfree_strarray(char **array, size_t n);
static inline const char *yesno(bool v) { return v ? "yes" : "no"; }
+static inline const char *enabledisable(bool v) { return v ? "enable" :
"disable"; }
+static inline const char *enableddisabled(bool v) { return v ? "enabled"
: "disabled"; }
Looks not readable even if takes 80 characters. Please, keep original style.
I believe you wanted to have nice negative statistics from day 1, then you
may add more patches in the series to cleanup more users.
not really the reason... it was just "this is small enough and
checkpatch doesn't complain" (it checks for 100 chars nowadays). But yes,
I can keep it in 4 lines.
thanks
Lucas De Marchi
Re: [PATCH] dt-bindings: Improve phandle-array schemas
Hi Rob,
On Wed, Jan 19, 2022 at 2:50 AM Rob Herring wrote:
> The 'phandle-array' type is a bit ambiguous. It can be either just an
> array of phandles or an array of phandles plus args. Many schemas for
> phandle-array properties aren't clear in the schema which case applies
> though the description usually describes it.
>
> The array of phandles case boils down to needing:
>
> items:
> maxItems: 1
>
> The phandle plus args cases should typically take this form:
>
> items:
> - items:
> - description: A phandle
> - description: 1st arg cell
> - description: 2nd arg cell
>
> With this change, some examples need updating so that the bracketing of
> property values matches the schema.
> Signed-off-by: Rob Herring
The Renesas parts look good to me.
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote: > On Fri, Jan 14, 2022 at 4:04 AM Guangming.Cao > wrote: > > > > On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote: > > > Am 14.01.22 um 00:26 schrieb John Stultz: > > > > On Thu, Jan 13, 2022 at 5:05 AM Christian König > > > > wrote: > > > > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J: > > > > > > > -Original Message- > > > > > > > From: dri-devel > > > > > > > On > > > > > > > Behalf Of > > > > > > > Ruhl, Michael J > > > > > > > > -Original Message- > > > > > > > > From: dri-devel < > > > > > > > > dri-devel-boun...@lists.freedesktop.org> > > > > > > > > On Behalf Of > > > > > > > > guangming@mediatek.com > > > > > > > > + /* > > > > > > > > +* Invalid size check. The "len" should be less > > > > > > > > than > > > > > > > > totalram. > > > > > > > > +* > > > > > > > > +* Without this check, once the invalid size > > > > > > > > allocation > > > > > > > > runs on a process > > > > > > > > that > > > > > > > > +* can't be killed by OOM flow(such as "gralloc" on > > > > > > > > Android devices), it > > > > > > > > will > > > > > > > > +* cause a kernel exception, and to make matters > > > > > > > > worse, > > > > > > > > we can't find > > > > > > > > who are using > > > > > > > > +* so many memory with "dma_buf_debug_show" since > > > > > > > > the > > > > > > > > relevant > > > > > > > > dma-buf hasn't exported. > > > > > > > > +*/ > > > > > > > > + if (len >> PAGE_SHIFT > totalram_pages()) > > > > > > > > > > > > > > If your "heap" is from cma, is this still a valid check? > > > > > > > > > > > > And thinking a bit further, if I create a heap from > > > > > > something > > > > > > else (say device memory), > > > > > > you will need to be able to figure out the maximum > > > > > > allowable > > > > > > check for the specific > > > > > > heap. > > > > > > > > > > > > Maybe the heap needs a callback for max size? > > > > Yes, I agree with this solution. > > If dma-heap framework support this via adding a callback to support > > it, > > seems it's more clear than adding a limitation in dma-heap > > framework > > since each heap maybe has different limitation. > > If you prefer adding callback, I can update this patch and add > > totalram > > limitation to system dma-heap. > > If the max value is per-heap, why not enforce that value in the > per-heap allocation function? > > Moving the check to the heap alloc to me seems simpler to me than > adding complexity to the infrastructure to add a heap max_size > callback. Is there some other use for the callback that you envision? > > thanks > -john Thanks for your comment. If you think max the value is per-heap, why not add an optional callback for dma-heap to solve this issue(prevent consuming too much time for a doomed to fail allocation), if the dma-heap doesn't have a special size check, just use the default value(totalram) in dma-heap framework to do the size check. Yes, for linux dma-heaps, only system-heap needs it, so adding it in system heap is the simplest. However, there are many vendor dma-heaps like system-heap which won't be uploaded to linux codebase, and maybe have same limitation, all these heaps need to add the same limitation. I just think it's boring. However, If you think discussing these absent cases based on current linux code is meaningless, I also agree to it. So, to summarize, if you still think adding it in system_heap.c is better, I also agree and I will update the patch to add it in system_heap.c Thanks~ Guangming > > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Re: [PATCH v2] phy: dphy: Correct clk_pre parameter
Hi Neil,
On Wed, 2022-01-19 at 10:11 +0100, Neil Armstrong wrote:
> On 19/01/2022 09:40, Neil Armstrong wrote:
> > Hi,
> >
> > On 19/01/2022 03:37, Liu Ying wrote:
> > > The D-PHY specification (v1.2) explicitly mentions that the T-CLK-PRE
> > > parameter's unit is Unit Interval(UI) and the minimum value is 8. Also,
> > > kernel doc of the 'clk_pre' member of struct phy_configure_opts_mipi_dphy
> > > mentions that it should be in UI. However, the dphy core driver wrongly
> > > sets 'clk_pre' to 8000, which seems to hint that it's in picoseconds.
> > > And, the kernel doc of the 'clk_pre' member wrongly says the minimum value
> > > is '8 UI', instead of 8.
> > >
> > > So, let's fix both the dphy core driver and the kernel doc of the
> > > 'clk_pre'
> > > member to correctly reflect the T-CLK-PRE parameter's unit and the minimum
> > > value according to the D-PHY specification.
> > >
> > > I'm assuming that all impacted custom drivers shall program values in
> > > TxByteClkHS cycles into hardware for the T-CLK-PRE parameter. The D-PHY
> > > specification mentions that the frequency of TxByteClkHS is exactly 1/8
> > > the High-Speed(HS) bit rate(each HS bit consumes one UI). So, relevant
> > > custom driver code is changed to program those values as
> > > DIV_ROUND_UP(cfg->clk_pre, BITS_PER_BYTE), then.
> > >
> > > Note that I've only tested the patch with RM67191 DSI panel on i.MX8mq
> > > EVK.
> > > Help is needed to test with other i.MX8mq, Meson and Rockchip platforms,
> > > as I don't have the hardwares.
> > >
> > > Fixes: 2ed869990e14 ("phy: Add MIPI D-PHY configuration options")
> > > Cc: Andrzej Hajda
> > > Cc: Neil Armstrong
> > > Cc: Robert Foss
> > > Cc: Laurent Pinchart
> > > Cc: Jonas Karlman
> > > Cc: Jernej Skrabec
> > > Cc: David Airlie
> > > Cc: Daniel Vetter
> > > Cc: Kishon Vijay Abraham I
> > > Cc: Vinod Koul
> > > Cc: Kevin Hilman
> > > Cc: Jerome Brunet
> > > Cc: Martin Blumenstingl
> > > Cc: Heiko Stuebner
> > > Cc: Maxime Ripard
> > > Cc: Guido Günther
> > > Tested-by: Liu Ying # RM67191 DSI panel on i.MX8mq
> > > EVK
> > > Signed-off-by: Liu Ying
> > > ---
> > > v1->v2:
> > > * Use BITS_PER_BYTE macro. (Andrzej)
> > > * Drop dsi argument from ui2bc() in nwl-dsi.c.
> > >
> > > drivers/gpu/drm/bridge/nwl-dsi.c | 12 +---
> > > drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c| 3 ++-
> > > drivers/phy/phy-core-mipi-dphy.c | 4 ++--
> > > drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c | 3 ++-
> > > include/linux/phy/phy-mipi-dphy.h| 2 +-
> > > 5 files changed, 12 insertions(+), 12 deletions(-)
> > >
> [...]
>
> > > diff --git a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> > > b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> > > index cd2332bf0e31..fdbd64c03e12 100644
> > > --- a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> > > +++ b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> > > @@ -9,6 +9,7 @@
> > >
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -250,7 +251,7 @@ static int phy_meson_axg_mipi_dphy_power_on(struct
> > > phy *phy)
> > >(DIV_ROUND_UP(priv->config.clk_zero, temp) << 16) |
> > >(DIV_ROUND_UP(priv->config.clk_prepare, temp) << 24));
> > > regmap_write(priv->regmap, MIPI_DSI_CLK_TIM1,
> > > - DIV_ROUND_UP(priv->config.clk_pre, temp));
> > > + DIV_ROUND_UP(priv->config.clk_pre, BITS_PER_BYTE));
> > >
> > > regmap_write(priv->regmap, MIPI_DSI_HS_TIM,
> > >DIV_ROUND_UP(priv->config.hs_exit, temp) |
> >
> > I'll try to run a test, currently the calculation gives 2, so this would
> > give 1.
>
> The Amlogic vendor code does:
>
> /* >8*ui */
> #define DPHY_TIME_CLK_PRE(ui) (10 * ui)
This looks like clk_pre time is 10 * ui, which matches the comment
'>8*ui' - longer time 8 * ui.
>
> t_ui = lcd_timing.bit_rate
>
> t_ui = (100 * 100) / (dsi_ui / 1000); /*100*ns */
> temp = t_ui * 8; /* lane_byte cycle time */
If I read correctly, this temp in vendor code is TxByteClkHS period
time in picoseconds. So...
>
> dphy->clk_pre = ((DPHY_TIME_CLK_PRE(t_ui) + temp - 1) / temp) & 0xff;
IIUC, 'dphy->clk_pre' essentially means dphy->clk_pre = DIV_ROUND_UP(10
* ui, temp), that is, the time for dphy->clk_pre is no less than
10 * ui. The D-PHY spec (v1.2)'s saying is that the minimum time for
dphy->clk_pre is 8 * ui. So, it looks like meson is stricter than the
spec.
However, the vendor code doesn't seem to match the current meson driver
implementation. The temp in driver code is also TxByteClkHS period
time in picoseconds. And, without this patch, I'm assuming that
priv->config.clk_pre is 8000. So, it looks like MIPI_DSI_CLK_TIM1 is
set to DIV_ROUND_UP(8000, temp). This '8000' does _not_ reflect
'10 * ui'.
So, 3 choices for meson, up to you:
1) If meson really requires the time for clk_pre no less than
10 * ui, then s
Re: [PATCH v2] phy: dphy: Correct clk_pre parameter
Hi,
On 19/01/2022 11:01, Liu Ying wrote:
> Hi Neil,
>
> On Wed, 2022-01-19 at 10:11 +0100, Neil Armstrong wrote:
>> On 19/01/2022 09:40, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On 19/01/2022 03:37, Liu Ying wrote:
The D-PHY specification (v1.2) explicitly mentions that the T-CLK-PRE
parameter's unit is Unit Interval(UI) and the minimum value is 8. Also,
kernel doc of the 'clk_pre' member of struct phy_configure_opts_mipi_dphy
mentions that it should be in UI. However, the dphy core driver wrongly
sets 'clk_pre' to 8000, which seems to hint that it's in picoseconds.
And, the kernel doc of the 'clk_pre' member wrongly says the minimum value
is '8 UI', instead of 8.
So, let's fix both the dphy core driver and the kernel doc of the 'clk_pre'
member to correctly reflect the T-CLK-PRE parameter's unit and the minimum
value according to the D-PHY specification.
I'm assuming that all impacted custom drivers shall program values in
TxByteClkHS cycles into hardware for the T-CLK-PRE parameter. The D-PHY
specification mentions that the frequency of TxByteClkHS is exactly 1/8
the High-Speed(HS) bit rate(each HS bit consumes one UI). So, relevant
custom driver code is changed to program those values as
DIV_ROUND_UP(cfg->clk_pre, BITS_PER_BYTE), then.
Note that I've only tested the patch with RM67191 DSI panel on i.MX8mq EVK.
Help is needed to test with other i.MX8mq, Meson and Rockchip platforms,
as I don't have the hardwares.
Fixes: 2ed869990e14 ("phy: Add MIPI D-PHY configuration options")
Cc: Andrzej Hajda
Cc: Neil Armstrong
Cc: Robert Foss
Cc: Laurent Pinchart
Cc: Jonas Karlman
Cc: Jernej Skrabec
Cc: David Airlie
Cc: Daniel Vetter
Cc: Kishon Vijay Abraham I
Cc: Vinod Koul
Cc: Kevin Hilman
Cc: Jerome Brunet
Cc: Martin Blumenstingl
Cc: Heiko Stuebner
Cc: Maxime Ripard
Cc: Guido Günther
Tested-by: Liu Ying # RM67191 DSI panel on i.MX8mq EVK
Signed-off-by: Liu Ying
---
v1->v2:
* Use BITS_PER_BYTE macro. (Andrzej)
* Drop dsi argument from ui2bc() in nwl-dsi.c.
drivers/gpu/drm/bridge/nwl-dsi.c | 12 +---
drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c| 3 ++-
drivers/phy/phy-core-mipi-dphy.c | 4 ++--
drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c | 3 ++-
include/linux/phy/phy-mipi-dphy.h| 2 +-
5 files changed, 12 insertions(+), 12 deletions(-)
>> [...]
>>
diff --git a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
index cd2332bf0e31..fdbd64c03e12 100644
--- a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
+++ b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
@@ -9,6 +9,7 @@
#include
#include
+#include
#include
#include
#include
@@ -250,7 +251,7 @@ static int phy_meson_axg_mipi_dphy_power_on(struct phy
*phy)
(DIV_ROUND_UP(priv->config.clk_zero, temp) << 16) |
(DIV_ROUND_UP(priv->config.clk_prepare, temp) << 24));
regmap_write(priv->regmap, MIPI_DSI_CLK_TIM1,
- DIV_ROUND_UP(priv->config.clk_pre, temp));
+ DIV_ROUND_UP(priv->config.clk_pre, BITS_PER_BYTE));
regmap_write(priv->regmap, MIPI_DSI_HS_TIM,
DIV_ROUND_UP(priv->config.hs_exit, temp) |
>>>
>>> I'll try to run a test, currently the calculation gives 2, so this would
>>> give 1.
>>
>> The Amlogic vendor code does:
>>
>> /* >8*ui */
>> #define DPHY_TIME_CLK_PRE(ui) (10 * ui)
>
> This looks like clk_pre time is 10 * ui, which matches the comment
> '>8*ui' - longer time 8 * ui.
>
>>
>> t_ui = lcd_timing.bit_rate
>>
>> t_ui = (100 * 100) / (dsi_ui / 1000); /*100*ns */
>> temp = t_ui * 8; /* lane_byte cycle time */
>
> If I read correctly, this temp in vendor code is TxByteClkHS period
> time in picoseconds. So...
>
>>
>> dphy->clk_pre = ((DPHY_TIME_CLK_PRE(t_ui) + temp - 1) / temp) & 0xff;
>
> IIUC, 'dphy->clk_pre' essentially means dphy->clk_pre = DIV_ROUND_UP(10
> * ui, temp), that is, the time for dphy->clk_pre is no less than
> 10 * ui. The D-PHY spec (v1.2)'s saying is that the minimum time for
> dphy->clk_pre is 8 * ui. So, it looks like meson is stricter than the
> spec.
>
> However, the vendor code doesn't seem to match the current meson driver
> implementation. The temp in driver code is also TxByteClkHS period
> time in picoseconds. And, without this patch, I'm assuming that
> priv->config.clk_pre is 8000. So, it looks like MIPI_DSI_CLK_TIM1 is
> set to DIV_ROUND_UP(8000, temp). This '8000' does _not_ reflect
> '10 * ui'.
>
> So, 3 choices for meson, up to you:
> 1) If meson really requires the time for clk_pre no less than
> 10 * ui
[PATCH v2] mgag200 fix memmapsl configuration in GCTL6 register
On some servers with MGA G200_SE_A (rev 42), booting with Legacy BIOS, the hardware hangs when using kdump and kexec into the kdump kernel. This happens when the uncompress code tries to write "Decompressing Linux" to the VGA Console. It can be reproduced by writing to the VGA console (0xB8000) after booting to graphic mode, it generates the following error: kernel:NMI: PCI system error (SERR) for reason a0 on CPU 0. kernel:Dazed and confused, but trying to continue The root cause is the configuration of the MGA GCTL6 register According to the GCTL6 register documentation: bit 0 is gcgrmode: 0: Enables alpha mode, and the character generator addressing system is activated. 1: Enables graphics mode, and the character addressing system is not used. bit 1 is chainodd even: 0: The A0 signal of the memory address bus is used during system memory addressing. 1: Allows A0 to be replaced by either the A16 signal of the system address (ifmemmapsl is ‘00’), or by the hpgoddev (MISC<5>, odd/even page select) field, described on page 3-294). bit 3-2 are memmapsl: Memory map select bits 1 and 0. VGA. These bits select where the video memory is mapped, as shown below: 00 => Ah - Bh 01 => Ah - Ah 10 => Bh - B7FFFh 11 => B8000h - Bh bit 7-4 are reserved. Current code set it to 0x05 => memmapsl to b01 => 0xa (graphic mode) But on x86, the VGA console is at 0xb8000 (text mode) In arch/x86/boot/compressed/misc.c debug strings are written to 0xb8000 As the driver doesn't use this mapping at 0xa, it is safe to set it to 0xb8000 instead, to avoid kernel hang on G200_SE_A rev42, with kexec/kdump. Thus changing the value 0x05 to 0x0d Signed-off-by: Jocelyn Falempe Reviewed-by: Javier Martinez Canillas Acked-by: Lyude Paul Cc: sta...@vger.kernel.org --- v2: Add clear statement that it's not the right configuration, but it prevents an annoying bug with kexec/kdump. drivers/gpu/drm/mgag200/mgag200_mode.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index b983541a4c53..cd9ba13ad5fc 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -529,7 +529,10 @@ static void mgag200_set_format_regs(struct mga_device *mdev, WREG_GFX(3, 0x00); WREG_GFX(4, 0x00); WREG_GFX(5, 0x40); - WREG_GFX(6, 0x05); + /* GCTL6 should be 0x05, but we configure memmapsl to 0xb8000 (text mode), +* so that it doesn't hang when running kexec/kdump on G200_SE rev42. +*/ + WREG_GFX(6, 0x0d); WREG_GFX(7, 0x0f); WREG_GFX(8, 0x0f); -- 2.34.1
Re: [PATCH v2 1/2] drm/panel: Add inx Himax8279d MIPI-DSI LCD panel driver
On Fri, Jan 7, 2022 at 8:22 PM xiazhengqiao
wrote:
>
> Add STARRY 2081101QFH032011-53G 10.1" WUXGA TFT LCD panel
>
> Signed-off-by: xiazhengqiao
> Tested-by: Hsin-Yi Wang
Reviewed-by: Hsin-Yi Wang
> ---
> drivers/gpu/drm/panel/Kconfig | 9 +
> drivers/gpu/drm/panel/Makefile| 1 +
> .../gpu/drm/panel/panel-innolux-himax8279d.c | 515 ++
> 3 files changed, 525 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-innolux-himax8279d.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index cfc8d644cedf..9458262ef1dd 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -167,6 +167,15 @@ config DRM_PANEL_INNOLUX_EJ030NA
>320x480 3.0" panel as found in the RS97 V2.1, RG300(non-ips)
>and LDK handheld gaming consoles.
>
> +config DRM_PANEL_INNOLUX_HIMAX8279D
> + tristate "INX 2081101qfh032011-53g 1200x1920 video panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to support for inx 2081101qfh032011-53g
> + 1200x1920 video panel.
> +
> config DRM_PANEL_INNOLUX_P079ZCA
> tristate "Innolux P079ZCA panel"
> depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index bca4cc1f2715..ec94fd5700fc 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) +=
> panel-ilitek-ili9322.o
> obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o
> obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
> obj-$(CONFIG_DRM_PANEL_INNOLUX_EJ030NA) += panel-innolux-ej030na.o
> +obj-$(CONFIG_DRM_PANEL_INNOLUX_HIMAX8279D) += panel-innolux-himax8279d.o
> obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
> obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
> obj-$(CONFIG_DRM_PANEL_KHADAS_TS050) += panel-khadas-ts050.o
> diff --git a/drivers/gpu/drm/panel/panel-innolux-himax8279d.c
> b/drivers/gpu/drm/panel/panel-innolux-himax8279d.c
> new file mode 100644
> index ..6840449548e4
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-innolux-himax8279d.c
> @@ -0,0 +1,515 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021, Huaqin Telecom Technology Co., Ltd
> + * Author: Zhengqiao Xia
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +struct panel_desc {
> + const struct drm_display_mode *modes;
> + unsigned int bpc;
> +
> + /**
> +* @width_mm: width of the panel's active display area
> +* @height_mm: height of the panel's active display area
> +*/
> + struct {
> + unsigned int width_mm;
> + unsigned int height_mm;
> + } size;
> +
> + unsigned long mode_flags;
> + enum mipi_dsi_pixel_format format;
> + const struct panel_init_cmd *init_cmds;
> + unsigned int lanes;
> + bool discharge_on_disable;
> +};
> +
> +struct inx_panel {
> + struct drm_panel base;
> + struct mipi_dsi_device *dsi;
> +
> + const struct panel_desc *desc;
> +
> + enum drm_panel_orientation orientation;
> + struct regulator *pp1800;
> + struct regulator *avee;
> + struct regulator *avdd;
> + struct gpio_desc *enable_gpio;
> +
> + bool prepared;
> +};
> +
> +enum dsi_cmd_type {
> + INIT_DCS_CMD,
> + DELAY_CMD,
> +};
> +
> +struct panel_init_cmd {
> + enum dsi_cmd_type type;
> + size_t len;
> + const char *data;
> +};
> +
> +#define _INIT_DCS_CMD(...) { \
> + .type = INIT_DCS_CMD, \
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
> +
> +#define _INIT_DELAY_CMD(...) { \
> + .type = DELAY_CMD,\
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
> +
> +static const struct panel_init_cmd starry_qfh032011_53g_init_cmd[] = {
> + _INIT_DCS_CMD(0xB0, 0x01),
> + _INIT_DCS_CMD(0xC3, 0x4F),
> + _INIT_DCS_CMD(0xC4, 0x40),
> + _INIT_DCS_CMD(0xC5, 0x40),
> + _INIT_DCS_CMD(0xC6, 0x40),
> + _INIT_DCS_CMD(0xC7, 0x40),
> + _INIT_DCS_CMD(0xC8, 0x4D),
> + _INIT_DCS_CMD(0xC9, 0x52),
> + _INIT_DCS_CMD(0xCA, 0x51),
> + _INIT_DCS_CMD(0xCD, 0x5D),
> + _INIT_DCS_CMD(0xCE, 0x5B),
> + _INIT_DCS_CMD(0xCF, 0x4B),
> + _INIT_DCS_CMD(0xD0, 0x49),
> + _INIT_DCS_CMD(0xD1, 0x47),
> + _INIT_DCS_CMD(0xD2, 0x45),
> + _INIT_DCS_CMD(0xD3, 0x41),
> + _INIT_DCS_CMD(0xD7, 0x50),
> + _INIT_DCS_CMD(0xD8, 0x40),
> + _INIT_DCS_CMD(0xD9, 0x40),
> + _INIT_DCS_CMD(0xDA, 0x40),
> + _INIT_DCS
Re: [PATCH] dt-bindings: Improve phandle-array schemas
On Wed, 19 Jan 2022 at 02:50, Rob Herring wrote: > > The 'phandle-array' type is a bit ambiguous. It can be either just an > array of phandles or an array of phandles plus args. Many schemas for > phandle-array properties aren't clear in the schema which case applies > though the description usually describes it. > > The array of phandles case boils down to needing: > > items: > maxItems: 1 > > The phandle plus args cases should typically take this form: > > items: > - items: > - description: A phandle > - description: 1st arg cell > - description: 2nd arg cell > > With this change, some examples need updating so that the bracketing of > property values matches the schema. > > Cc: Damien Le Moal > Cc: Herbert Xu > Cc: "David S. Miller" > Cc: Chun-Kuang Hu > Cc: Philipp Zabel > Cc: Laurent Pinchart > Cc: Kieran Bingham > Cc: Vinod Koul > Cc: Georgi Djakov > Cc: Thomas Gleixner > Cc: Marc Zyngier > Cc: Joerg Roedel > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han > Cc: Pavel Machek > Cc: Mauro Carvalho Chehab > Cc: Krzysztof Kozlowski > Cc: Jakub Kicinski > Cc: Wolfgang Grandegger > Cc: Marc Kleine-Budde > Cc: Andrew Lunn > Cc: Vivien Didelot > Cc: Florian Fainelli > Cc: Vladimir Oltean > Cc: Kalle Valo > Cc: Viresh Kumar > Cc: Stephen Boyd > Cc: Kishon Vijay Abraham I > Cc: Linus Walleij > Cc: "Rafael J. Wysocki" > Cc: Kevin Hilman > Cc: Ulf Hansson > Cc: Sebastian Reichel > Cc: Mark Brown > Cc: Mathieu Poirier > Cc: Daniel Lezcano > Cc: Zhang Rui > Cc: Greg Kroah-Hartman > Cc: Thierry Reding > Cc: Jonathan Hunter > Cc: Sudeep Holla > Cc: Geert Uytterhoeven > Cc: linux-...@vger.kernel.org > Cc: linux-cry...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: dmaeng...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: io...@lists.linux-foundation.org > Cc: linux-l...@vger.kernel.org > Cc: linux-me...@vger.kernel.org > Cc: net...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: linux-wirel...@vger.kernel.org > Cc: linux-...@lists.infradead.org > Cc: linux-g...@vger.kernel.org > Cc: linux-ri...@lists.infradead.org > Cc: linux-remotep...@vger.kernel.org > Cc: alsa-de...@alsa-project.org > Cc: linux-...@vger.kernel.org > Signed-off-by: Rob Herring > --- For CPUs and PM domains: Acked-by: Ulf Hansson Kind regards Uffe
Re: [PATCH v2] phy: dphy: Correct clk_pre parameter
Hi,
On 19/01/2022 11:11, Neil Armstrong wrote:
> Hi,
>
> On 19/01/2022 11:01, Liu Ying wrote:
>> Hi Neil,
>>
>> On Wed, 2022-01-19 at 10:11 +0100, Neil Armstrong wrote:
>>> On 19/01/2022 09:40, Neil Armstrong wrote:
Hi,
On 19/01/2022 03:37, Liu Ying wrote:
> The D-PHY specification (v1.2) explicitly mentions that the T-CLK-PRE
> parameter's unit is Unit Interval(UI) and the minimum value is 8. Also,
> kernel doc of the 'clk_pre' member of struct phy_configure_opts_mipi_dphy
> mentions that it should be in UI. However, the dphy core driver wrongly
> sets 'clk_pre' to 8000, which seems to hint that it's in picoseconds.
> And, the kernel doc of the 'clk_pre' member wrongly says the minimum value
> is '8 UI', instead of 8.
>
> So, let's fix both the dphy core driver and the kernel doc of the
> 'clk_pre'
> member to correctly reflect the T-CLK-PRE parameter's unit and the minimum
> value according to the D-PHY specification.
>
> I'm assuming that all impacted custom drivers shall program values in
> TxByteClkHS cycles into hardware for the T-CLK-PRE parameter. The D-PHY
> specification mentions that the frequency of TxByteClkHS is exactly 1/8
> the High-Speed(HS) bit rate(each HS bit consumes one UI). So, relevant
> custom driver code is changed to program those values as
> DIV_ROUND_UP(cfg->clk_pre, BITS_PER_BYTE), then.
>
> Note that I've only tested the patch with RM67191 DSI panel on i.MX8mq
> EVK.
> Help is needed to test with other i.MX8mq, Meson and Rockchip platforms,
> as I don't have the hardwares.
>
> Fixes: 2ed869990e14 ("phy: Add MIPI D-PHY configuration options")
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Kishon Vijay Abraham I
> Cc: Vinod Koul
> Cc: Kevin Hilman
> Cc: Jerome Brunet
> Cc: Martin Blumenstingl
> Cc: Heiko Stuebner
> Cc: Maxime Ripard
> Cc: Guido Günther
> Tested-by: Liu Ying # RM67191 DSI panel on i.MX8mq
> EVK
> Signed-off-by: Liu Ying
> ---
> v1->v2:
> * Use BITS_PER_BYTE macro. (Andrzej)
> * Drop dsi argument from ui2bc() in nwl-dsi.c.
>
> drivers/gpu/drm/bridge/nwl-dsi.c | 12 +---
> drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c| 3 ++-
> drivers/phy/phy-core-mipi-dphy.c | 4 ++--
> drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c | 3 ++-
> include/linux/phy/phy-mipi-dphy.h| 2 +-
> 5 files changed, 12 insertions(+), 12 deletions(-)
>
>>> [...]
>>>
> diff --git a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> index cd2332bf0e31..fdbd64c03e12 100644
> --- a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> +++ b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> @@ -9,6 +9,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -250,7 +251,7 @@ static int phy_meson_axg_mipi_dphy_power_on(struct
> phy *phy)
>(DIV_ROUND_UP(priv->config.clk_zero, temp) << 16) |
>(DIV_ROUND_UP(priv->config.clk_prepare, temp) << 24));
> regmap_write(priv->regmap, MIPI_DSI_CLK_TIM1,
> - DIV_ROUND_UP(priv->config.clk_pre, temp));
> + DIV_ROUND_UP(priv->config.clk_pre, BITS_PER_BYTE));
>
> regmap_write(priv->regmap, MIPI_DSI_HS_TIM,
>DIV_ROUND_UP(priv->config.hs_exit, temp) |
I'll try to run a test, currently the calculation gives 2, so this would
give 1.
>>>
>>> The Amlogic vendor code does:
>>>
>>> /* >8*ui */
>>> #define DPHY_TIME_CLK_PRE(ui) (10 * ui)
>>
>> This looks like clk_pre time is 10 * ui, which matches the comment
>> '>8*ui' - longer time 8 * ui.
>>
>>>
>>> t_ui = lcd_timing.bit_rate
>>>
>>> t_ui = (100 * 100) / (dsi_ui / 1000); /*100*ns */
>>> temp = t_ui * 8; /* lane_byte cycle time */
>>
>> If I read correctly, this temp in vendor code is TxByteClkHS period
>> time in picoseconds. So...
>>
>>>
>>> dphy->clk_pre = ((DPHY_TIME_CLK_PRE(t_ui) + temp - 1) / temp) & 0xff;
>>
>> IIUC, 'dphy->clk_pre' essentially means dphy->clk_pre = DIV_ROUND_UP(10
>> * ui, temp), that is, the time for dphy->clk_pre is no less than
>> 10 * ui. The D-PHY spec (v1.2)'s saying is that the minimum time for
>> dphy->clk_pre is 8 * ui. So, it looks like meson is stricter than the
>> spec.
>>
>> However, the vendor code doesn't seem to match the current meson driver
>> implementation. The temp in driver code is also TxByteClkHS period
>> time in picoseconds. And, without this patch, I'm assuming that
>> priv->config.clk_pre is 8000. So, it looks like MIPI_DSI_CLK_TIM1 is
>> set to DIV
Re: [PATCH] drm/etnaviv: Add missing pm_runtime_put
Am Dienstag, dem 18.01.2022 um 06:16 -0800 schrieb Yongzhi Liu: > pm_runtime_get_sync() increments the runtime PM usage counter even > when it returns an error code, thus a matching decrement is needed > on the error handling path to keep the counter balanced. > Instead of adding more error handling code here, I would prefer to convert this to pm_runtime_resume_and_get to avoid this issue. Regards, Lucas > Signed-off-by: Yongzhi Liu > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index 242a5fd..5e81a98 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -1714,6 +1714,9 @@ static int etnaviv_gpu_bind(struct device *dev, struct > device *master, > return 0; > > out_sched: > +#ifdef CONFIG_PM > + pm_runtime_put_autosuspend(gpu->dev); > +#endif > etnaviv_sched_fini(gpu); > > out_workqueue:
[PATCH 0/2] Fix regression introduced by disabling accelerated scrolling in fbcon
This series reverts two patches which disabled scrolling acceleration in fbcon/fbdev. Those patches introduced a regression for fbdev-supported graphic cards because of the performance penalty by doing screen scrolling by software instead of using hardware acceleration. Console scrolling acceleration was disabled by dropping code which checked at runtime the driver hardware possibilities for the BINFO_HWACCEL_COPYAREA or FBINFO_HWACCEL_FILLRECT flags and if set, it enabled scrollmode SCROLL_MOVE which uses hardware acceleration to move screen contents. After dropping those checks scrollmode was hard-wired to SCROLL_REDRAW instead, which forces all graphic cards to redraw every character at the new screen position when scrolling. This change effectively disabled all hardware-based scrolling acceleration for ALL drivers, because now all kind of 2D hardware acceleration (bitblt, fillrect) in the drivers isn't used any longer. The original commit message mentions that only 3 DRM drivers (nouveau, omapdrm and gma500) used hardware acceleration in the past and thus code for checking and using scrolling acceleration is obsolete. This statement is NOT TRUE, because beside the DRM drivers there are around 35 other fbdev drivers which depend on fbdev/fbcon and still provide hardware acceleration for fbdev/fbcon. The original commit message also states that syzbot found lots of bugs in fbcon and thus it's "often the solution to just delete code and remove features". This is true, and the bugs - which actually affected all users of fbcon, including DRM - were fixed, or code was dropped like e.g. the support for software scrollback in vgacon (commit 973c096f6a85). So to further analyze which bugs were found by syzbot, I've looked through all patches in drivers/video which were tagged with syzbot or syzkaller back to year 2005. The vast majority fixed the reported issues on a higher level, e.g. when screen is to be resized, or when font size is to be changed. The few ones which touched driver code fixed a real driver bug, e.g. by adding a check. But NONE of those patches touched code of either the SCROLL_MOVE or the SCROLL_REDRAW case. That means, there was no real reason why SCROLL_MOVE had to be ripped-out and just SCROLL_REDRAW had to be used instead. The only reason I can imagine so far was that SCROLL_MOVE wasn't used by DRM and as such it was assumed that it could go away. That argument completely missed the fact that SCROLL_MOVE is still heavily used by fbdev (non-DRM) drivers. Some people mention that using memcpy() instead of the hardware acceleration is pretty much the same speed. But that's not true, at least not for older graphic cards and machines where we see speed decreases by factor 10 and more and thus this change leads to console responsiveness way worse than before. That's why I propose to revert those patches, re-introduce hardware-based scrolling acceleration and fix the performance-regression for fbdev drivers. There isn't any impact on DRM when reverting those patches. Helge Deller (2): Revert "fbdev: Garbage collect fbdev scrolling acceleration, part 1 (from TODO list)" Revert "fbcon: Disable accelerated scrolling" Documentation/gpu/todo.rst | 24 -- drivers/video/fbdev/core/bitblit.c | 16 + drivers/video/fbdev/core/fbcon.c| 540 +++- drivers/video/fbdev/core/fbcon.h| 59 +++ drivers/video/fbdev/core/fbcon_ccw.c| 28 +- drivers/video/fbdev/core/fbcon_cw.c | 28 +- drivers/video/fbdev/core/fbcon_rotate.h | 9 + drivers/video/fbdev/core/fbcon_ud.c | 37 +- drivers/video/fbdev/core/tileblit.c | 16 + drivers/video/fbdev/skeletonfb.c| 12 +- include/linux/fb.h | 2 +- 11 files changed, 703 insertions(+), 68 deletions(-) -- 2.31.1
[PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
Revert this patch. This patch started to introduce the regression that
all hardware acceleration of more than 35 existing fbdev drivers were
bypassed and thus fbcon console output for those was dramatically slowed
down by factor of 10 and more.
Reverting this commit has no impact on DRM, since none of the DRM drivers are
tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
FBINFO_HWACCEL_FILLRECT or others.
Signed-off-by: Helge Deller
Cc: sta...@vger.kernel.org # v5.16
---
Documentation/gpu/todo.rst | 21 ---
drivers/video/fbdev/core/fbcon.c | 45 ++--
2 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 29506815d24a..a1212b5b3026 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -300,27 +300,6 @@ Contact: Daniel Vetter, Noralf Tronnes
Level: Advanced
-Garbage collect fbdev scrolling acceleration
-
-
-Scroll acceleration is disabled in fbcon by hard-wiring p->scrollmode =
-SCROLL_REDRAW. There's a ton of code this will allow us to remove:
-
-- lots of code in fbcon.c
-
-- a bunch of the hooks in fbcon_ops, maybe the remaining hooks could be called
- directly instead of the function table (with a switch on p->rotate)
-
-- fb_copyarea is unused after this, and can be deleted from all drivers
-
-Note that not all acceleration code can be deleted, since clearing and cursor
-support is still accelerated, which might be good candidates for further
-deletion projects.
-
-Contact: Daniel Vetter
-
-Level: Intermediate
-
idr_init_base()
---
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 22bb3892f6bd..b813985f1403 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1025,7 +1025,7 @@ static void fbcon_init(struct vc_data *vc, int init)
struct vc_data *svc = *default_mode;
struct fbcon_display *t, *p = &fb_display[vc->vc_num];
int logo = 1, new_rows, new_cols, rows, cols;
- int ret;
+ int cap, ret;
if (WARN_ON(info_idx == -1))
return;
@@ -1034,6 +1034,7 @@ static void fbcon_init(struct vc_data *vc, int init)
con2fb_map[vc->vc_num] = info_idx;
info = registered_fb[con2fb_map[vc->vc_num]];
+ cap = info->flags;
if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
logo_shown = FBCON_LOGO_DONTSHOW;
@@ -1135,13 +1136,11 @@ static void fbcon_init(struct vc_data *vc, int init)
ops->graphics = 0;
- /*
-* No more hw acceleration for fbcon.
-*
-* FIXME: Garbage collect all the now dead code after sufficient time
-* has passed.
-*/
- p->scrollmode = SCROLL_REDRAW;
+ if ((cap & FBINFO_HWACCEL_COPYAREA) &&
+ !(cap & FBINFO_HWACCEL_DISABLED))
+ p->scrollmode = SCROLL_MOVE;
+ else /* default to something safe */
+ p->scrollmode = SCROLL_REDRAW;
/*
* ++guenther: console.c:vc_allocate() relies on initializing
@@ -1953,15 +1952,45 @@ static void updatescrollmode(struct fbcon_display *p,
{
struct fbcon_ops *ops = info->fbcon_par;
int fh = vc->vc_font.height;
+ int cap = info->flags;
+ u16 t = 0;
+ int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep,
+ info->fix.xpanstep);
+ int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t);
int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual,
info->var.xres_virtual);
+ int good_pan = (cap & FBINFO_HWACCEL_YPAN) &&
+ divides(ypan, vc->vc_font.height) && vyres > yres;
+ int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) &&
+ divides(ywrap, vc->vc_font.height) &&
+ divides(vc->vc_font.height, vyres) &&
+ divides(vc->vc_font.height, yres);
+ int reading_fast = cap & FBINFO_READS_FAST;
+ int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) &&
+ !(cap & FBINFO_HWACCEL_DISABLED);
+ int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) &&
+ !(cap & FBINFO_HWACCEL_DISABLED);
p->vrows = vyres/fh;
if (yres > (fh * (vc->vc_rows + 1)))
p->vrows -= (yres - (fh * vc->vc_rows)) / fh;
if ((yres % fh) && (vyres % fh < yres % fh))
p->vrows--;
+
+ if (good_wrap || good_pan) {
+ if (reading_fast || fast_copyarea)
+ p->scrollmode = good_wrap ?
+ SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE;
+ else
+ p->scrollmode = good_wrap ? SCROLL_REDRAW :
+
[PATCH 1/2] Revert "fbdev: Garbage collect fbdev scrolling acceleration, part 1 (from TODO list)"
This reverts commit b3ec8cdf457e5e63d396fe1346cc788cf7c1b578.
Revert this patch. This and the previous patch introduced the
regression that all hardware acceleration of more than 35 existing fbdev
drivers were bypassed and thus fbcon console output for those was
dramatically slowed down by factor of 10 and more.
Reverting this commit has no impact on DRM, since none of the DRM drivers are
tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
FBINFO_HWACCEL_FILLRECT or others.
Signed-off-by: Helge Deller
Cc: sta...@vger.kernel.org # v5.16
---
Documentation/gpu/todo.rst | 13 +-
drivers/video/fbdev/core/bitblit.c | 16 +
drivers/video/fbdev/core/fbcon.c| 509 +++-
drivers/video/fbdev/core/fbcon.h| 59 +++
drivers/video/fbdev/core/fbcon_ccw.c| 28 +-
drivers/video/fbdev/core/fbcon_cw.c | 28 +-
drivers/video/fbdev/core/fbcon_rotate.h | 9 +
drivers/video/fbdev/core/fbcon_ud.c | 37 +-
drivers/video/fbdev/core/tileblit.c | 16 +
drivers/video/fbdev/skeletonfb.c| 12 +-
include/linux/fb.h | 2 +-
11 files changed, 678 insertions(+), 51 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index da138dd39883..29506815d24a 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -303,19 +303,16 @@ Level: Advanced
Garbage collect fbdev scrolling acceleration
-Scroll acceleration has been disabled in fbcon. Now it works as the old
-SCROLL_REDRAW mode. A ton of code was removed in fbcon.c and the hook bmove was
-removed from fbcon_ops.
-Remaining tasks:
+Scroll acceleration is disabled in fbcon by hard-wiring p->scrollmode =
+SCROLL_REDRAW. There's a ton of code this will allow us to remove:
-- a bunch of the hooks in fbcon_ops could be removed or simplified by calling
+- lots of code in fbcon.c
+
+- a bunch of the hooks in fbcon_ops, maybe the remaining hooks could be called
directly instead of the function table (with a switch on p->rotate)
- fb_copyarea is unused after this, and can be deleted from all drivers
-- after that, fb_copyarea can be deleted from fb_ops in include/linux/fb.h as
- well as cfb_copyarea
-
Note that not all acceleration code can be deleted, since clearing and cursor
support is still accelerated, which might be good candidates for further
deletion projects.
diff --git a/drivers/video/fbdev/core/bitblit.c
b/drivers/video/fbdev/core/bitblit.c
index 01fae2c96965..f98e8f298bc1 100644
--- a/drivers/video/fbdev/core/bitblit.c
+++ b/drivers/video/fbdev/core/bitblit.c
@@ -43,6 +43,21 @@ static void update_attr(u8 *dst, u8 *src, int attribute,
}
}
+static void bit_bmove(struct vc_data *vc, struct fb_info *info, int sy,
+ int sx, int dy, int dx, int height, int width)
+{
+ struct fb_copyarea area;
+
+ area.sx = sx * vc->vc_font.width;
+ area.sy = sy * vc->vc_font.height;
+ area.dx = dx * vc->vc_font.width;
+ area.dy = dy * vc->vc_font.height;
+ area.height = height * vc->vc_font.height;
+ area.width = width * vc->vc_font.width;
+
+ info->fbops->fb_copyarea(info, &area);
+}
+
static void bit_clear(struct vc_data *vc, struct fb_info *info, int sy,
int sx, int height, int width)
{
@@ -378,6 +393,7 @@ static int bit_update_start(struct fb_info *info)
void fbcon_set_bitops(struct fbcon_ops *ops)
{
+ ops->bmove = bit_bmove;
ops->clear = bit_clear;
ops->putcs = bit_putcs;
ops->clear_margins = bit_clear_margins;
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 99ecd9a6d844..22bb3892f6bd 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -173,6 +173,8 @@ static void fbcon_putcs(struct vc_data *vc, const unsigned
short *s,
int count, int ypos, int xpos);
static void fbcon_clear_margins(struct vc_data *vc, int bottom_only);
static void fbcon_cursor(struct vc_data *vc, int mode);
+static void fbcon_bmove(struct vc_data *vc, int sy, int sx, int dy, int dx,
+ int height, int width);
static int fbcon_switch(struct vc_data *vc);
static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch);
static void fbcon_set_palette(struct vc_data *vc, const unsigned char *table);
@@ -180,8 +182,16 @@ static void fbcon_set_palette(struct vc_data *vc, const
unsigned char *table);
/*
* Internal routines
*/
+static __inline__ void ywrap_up(struct vc_data *vc, int count);
+static __inline__ void ywrap_down(struct vc_data *vc, int count);
+static __inline__ void ypan_up(struct vc_data *vc, int count);
+static __inline__ void ypan_down(struct vc_data *vc, int count);
+static void fbcon_bmove_rec(struct vc_data *vc, struct fbcon_display *p, int
sy, int sx,
+ int dy, int dx, int height, int width, u_int
y_break
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote: > This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151. > > Revert this patch. This patch started to introduce the regression that > all hardware acceleration of more than 35 existing fbdev drivers were > bypassed and thus fbcon console output for those was dramatically slowed > down by factor of 10 and more. > > Reverting this commit has no impact on DRM, since none of the DRM drivers are > tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA, > FBINFO_HWACCEL_FILLRECT or others. > > Signed-off-by: Helge Deller > Cc: sta...@vger.kernel.org # v5.16 Why just 5.16? This commit came in on 5.11 and was backported to 5.10.5. As for "why", I think there was a number of private bugs that were reported in this code, which is why it was removed. I do not think it can be safely added back in without addressing them first. Let me go dig through my email to see if I can find them... thanks, greg k-h
Re: [PATCH v5, 15/15] media: mtk-vcodec: support stateless VP9 decoding
Il 17/01/22 10:40, Yunfei Dong ha scritto: Add support for VP9 decoding using the stateless API, as supported by MT8192. And the drivers is lat and core architecture. Signed-off-by: Yunfei Dong Signed-off-by: George Sun --- drivers/media/platform/mtk-vcodec/Makefile|1 + .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 26 +- .../platform/mtk-vcodec/mtk_vcodec_drv.h |1 + .../mtk-vcodec/vdec/vdec_vp9_req_lat_if.c | 1973 + .../media/platform/mtk-vcodec/vdec_drv_if.c |4 + .../media/platform/mtk-vcodec/vdec_drv_if.h |1 + 6 files changed, 2003 insertions(+), 3 deletions(-) create mode 100644 drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_req_lat_if.c Hello Yunfei, this driver is based on an older version of the VP9 stateless decoder uAPI, hence this is not applicable upstream. The latest linux-next tag (as of today) already contains the new and accepted code; can you please rebase over that one? Thanks, Angelo
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote: > On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote: > > This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151. > > > > Revert this patch. This patch started to introduce the regression that > > all hardware acceleration of more than 35 existing fbdev drivers were > > bypassed and thus fbcon console output for those was dramatically slowed > > down by factor of 10 and more. > > > > Reverting this commit has no impact on DRM, since none of the DRM drivers > > are > > tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA, > > FBINFO_HWACCEL_FILLRECT or others. > > > > Signed-off-by: Helge Deller > > Cc: sta...@vger.kernel.org # v5.16 > > Why just 5.16? This commit came in on 5.11 and was backported to > 5.10.5. > > As for "why", I think there was a number of private bugs that were > reported in this code, which is why it was removed. I do not think it > can be safely added back in without addressing them first. Let me go > dig through my email to see if I can find them... Ah, no, that was just the soft scrollback code I was thinking of, which was a different revert and is still gone, thankfully :) This one was just removed because Daniel noticed that only 3 drivers used this (nouveau, omapdrm, and gma600), so this shouldn't have caused any regressions in any other drivers like you are reporting here. So perhaps this regression is caused by something else? thanks, greg k-h
[PATCH v9 2/6] drm: improve drm_buddy_alloc function
- Make drm_buddy_alloc a single function to handle
range allocation and non-range allocation demands
- Implemented a new function alloc_range() which allocates
the requested power-of-two block comply with range limitations
- Moved order computation and memory alignment logic from
i915 driver to drm buddy
v2:
merged below changes to keep the build unbroken
- drm_buddy_alloc_range() becomes obsolete and may be removed
- enable ttm range allocation (fpfn / lpfn) support in i915 driver
- apply enhanced drm_buddy_alloc() function to i915 driver
v3(Matthew Auld):
- Fix alignment issues and remove unnecessary list_empty check
- add more validation checks for input arguments
- make alloc_range() block allocations as bottom-up
- optimize order computation logic
- replace uint64_t with u64, which is preferred in the kernel
v4(Matthew Auld):
- keep drm_buddy_alloc_range() function implementation for generic
actual range allocations
- keep alloc_range() implementation for end bias allocations
v5(Matthew Auld):
- modify drm_buddy_alloc() passing argument place->lpfn to lpfn
as place->lpfn will currently always be zero for i915
v6(Matthew Auld):
- fixup potential uaf - If we are unlucky and can't allocate
enough memory when splitting blocks, where we temporarily
end up with the given block and its buddy on the respective
free list, then we need to ensure we delete both blocks,
and no just the buddy, before potentially freeing them
- fix warnings reported by kernel test robot
Signed-off-by: Arunpravin
---
drivers/gpu/drm/drm_buddy.c | 326 +-
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 67 ++--
drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 2 +
include/drm/drm_buddy.h | 22 +-
4 files changed, 293 insertions(+), 124 deletions(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index d60878bc9c20..954e31962c74 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -282,23 +282,99 @@ void drm_buddy_free_list(struct drm_buddy *mm, struct
list_head *objects)
}
EXPORT_SYMBOL(drm_buddy_free_list);
-/**
- * drm_buddy_alloc_blocks - allocate power-of-two blocks
- *
- * @mm: DRM buddy manager to allocate from
- * @order: size of the allocation
- *
- * The order value here translates to:
- *
- * 0 = 2^0 * mm->chunk_size
- * 1 = 2^1 * mm->chunk_size
- * 2 = 2^2 * mm->chunk_size
- *
- * Returns:
- * allocated ptr to the &drm_buddy_block on success
- */
-struct drm_buddy_block *
-drm_buddy_alloc_blocks(struct drm_buddy *mm, unsigned int order)
+static inline bool overlaps(u64 s1, u64 e1, u64 s2, u64 e2)
+{
+ return s1 <= e2 && e1 >= s2;
+}
+
+static inline bool contains(u64 s1, u64 e1, u64 s2, u64 e2)
+{
+ return s1 <= s2 && e1 >= e2;
+}
+
+static struct drm_buddy_block *
+alloc_range_bias(struct drm_buddy *mm,
+u64 start, u64 end,
+unsigned int order)
+{
+ struct drm_buddy_block *block;
+ struct drm_buddy_block *buddy;
+ LIST_HEAD(dfs);
+ int err;
+ int i;
+
+ end = end - 1;
+
+ for (i = 0; i < mm->n_roots; ++i)
+ list_add_tail(&mm->roots[i]->tmp_link, &dfs);
+
+ do {
+ u64 block_start;
+ u64 block_end;
+
+ block = list_first_entry_or_null(&dfs,
+struct drm_buddy_block,
+tmp_link);
+ if (!block)
+ break;
+
+ list_del(&block->tmp_link);
+
+ if (drm_buddy_block_order(block) < order)
+ continue;
+
+ block_start = drm_buddy_block_offset(block);
+ block_end = block_start + drm_buddy_block_size(mm, block) - 1;
+
+ if (!overlaps(start, end, block_start, block_end))
+ continue;
+
+ if (drm_buddy_block_is_allocated(block))
+ continue;
+
+ if (contains(start, end, block_start, block_end) &&
+ order == drm_buddy_block_order(block)) {
+ /*
+* Find the free block within the range.
+*/
+ if (drm_buddy_block_is_free(block))
+ return block;
+
+ continue;
+ }
+
+ if (!drm_buddy_block_is_split(block)) {
+ err = split_block(mm, block);
+ if (unlikely(err))
+ goto err_undo;
+ }
+
+ list_add(&block->right->tmp_link, &dfs);
+ list_add(&block->left->tmp_link, &dfs);
+ } while (1);
+
+ return ERR_PTR(-ENOSPC);
+
+err_undo:
+ /*
+* We really don't want to leave around a bunch of split blocks, since
+*
[PATCH v9 3/6] drm: implement top-down allocation method
Implemented a function which walk through the order list,
compares the offset and returns the maximum offset block,
this method is unpredictable in obtaining the high range
address blocks which depends on allocation and deallocation.
for instance, if driver requests address at a low specific
range, allocator traverses from the root block and splits
the larger blocks until it reaches the specific block and
in the process of splitting, lower orders in the freelist
are occupied with low range address blocks and for the
subsequent TOPDOWN memory request we may return the low
range blocks.To overcome this issue, we may go with the
below approach.
The other approach, sorting each order list entries in
ascending order and compares the last entry of each
order list in the freelist and return the max block.
This creates sorting overhead on every drm_buddy_free()
request and split up of larger blocks for a single page
request.
v2:
- Fix alignment issues(Matthew Auld)
- Remove unnecessary list_empty check(Matthew Auld)
- merged the below patch to see the feature in action
- add top-down alloc support to i915 driver
Signed-off-by: Arunpravin
---
drivers/gpu/drm/drm_buddy.c | 36 ---
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 3 ++
include/drm/drm_buddy.h | 1 +
3 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 954e31962c74..6aa5c1ce25bf 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -371,6 +371,26 @@ alloc_range_bias(struct drm_buddy *mm,
return ERR_PTR(err);
}
+static struct drm_buddy_block *
+get_maxblock(struct list_head *head)
+{
+ struct drm_buddy_block *max_block = NULL, *node;
+
+ max_block = list_first_entry_or_null(head,
+struct drm_buddy_block,
+link);
+ if (!max_block)
+ return NULL;
+
+ list_for_each_entry(node, head, link) {
+ if (drm_buddy_block_offset(node) >
+ drm_buddy_block_offset(max_block))
+ max_block = node;
+ }
+
+ return max_block;
+}
+
static struct drm_buddy_block *
alloc_from_freelist(struct drm_buddy *mm,
unsigned int order,
@@ -381,11 +401,17 @@ alloc_from_freelist(struct drm_buddy *mm,
int err;
for (i = order; i <= mm->max_order; ++i) {
- block = list_first_entry_or_null(&mm->free_list[i],
-struct drm_buddy_block,
-link);
- if (block)
- break;
+ if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) {
+ block = get_maxblock(&mm->free_list[i]);
+ if (block)
+ break;
+ } else {
+ block = list_first_entry_or_null(&mm->free_list[i],
+struct drm_buddy_block,
+link);
+ if (block)
+ break;
+ }
}
if (!block)
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 1411f4cf1f21..3662434b64bb 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -53,6 +53,9 @@ static int i915_ttm_buddy_man_alloc(struct
ttm_resource_manager *man,
INIT_LIST_HEAD(&bman_res->blocks);
bman_res->mm = mm;
+ if (place->flags & TTM_PL_FLAG_TOPDOWN)
+ bman_res->flags |= DRM_BUDDY_TOPDOWN_ALLOCATION;
+
if (place->fpfn || lpfn != man->size)
bman_res->flags |= DRM_BUDDY_RANGE_ALLOCATION;
diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
index 865664b90a8a..424fc443115e 100644
--- a/include/drm/drm_buddy.h
+++ b/include/drm/drm_buddy.h
@@ -28,6 +28,7 @@
})
#define DRM_BUDDY_RANGE_ALLOCATION (1 << 0)
+#define DRM_BUDDY_TOPDOWN_ALLOCATION (1 << 1)
struct drm_buddy_block {
#define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
--
2.25.1
[PATCH v9 4/6] drm: implement a method to free unused pages
On contiguous allocation, we round up the size
to the *next* power of 2, implement a function
to free the unused pages after the newly allocate block.
v2(Matthew Auld):
- replace function name 'drm_buddy_free_unused_pages' with
drm_buddy_block_trim
- replace input argument name 'actual_size' with 'new_size'
- add more validation checks for input arguments
- add overlaps check to avoid needless searching and splitting
- merged the below patch to see the feature in action
- add free unused pages support to i915 driver
- lock drm_buddy_block_trim() function as it calls mark_free/mark_split
are all globally visible
v3(Matthew Auld):
- remove trim method error handling as we address the failure case
at drm_buddy_block_trim() function
v4:
- in case of trim, at __alloc_range() split_block failure path
marks the block as free and removes it from the original list,
potentially also freeing it, to overcome this problem, we turn
the drm_buddy_block_trim() input node into a temporary node to
prevent recursively freeing itself, but still retain the
un-splitting/freeing of the other nodes(Matthew Auld)
- modify the drm_buddy_block_trim() function return type
v5(Matthew Auld):
- revert drm_buddy_block_trim() function return type changes in v4
- modify drm_buddy_block_trim() passing argument n_pages to original_size
as n_pages has already been rounded up to the next power-of-two and
passing n_pages results noop
v6:
- fix warnings reported by kernel test robot
Signed-off-by: Arunpravin
---
drivers/gpu/drm/drm_buddy.c | 65 +++
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 10 +++
include/drm/drm_buddy.h | 4 ++
3 files changed, 79 insertions(+)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 6aa5c1ce25bf..c5902a81b8c5 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -546,6 +546,71 @@ static int __drm_buddy_alloc_range(struct drm_buddy *mm,
return __alloc_range(mm, &dfs, start, size, blocks);
}
+/**
+ * drm_buddy_block_trim - free unused pages
+ *
+ * @mm: DRM buddy manager
+ * @new_size: original size requested
+ * @blocks: output list head to add allocated blocks
+ *
+ * For contiguous allocation, we round up the size to the nearest
+ * power of two value, drivers consume *actual* size, so remaining
+ * portions are unused and it can be freed.
+ *
+ * Returns:
+ * 0 on success, error code on failure.
+ */
+int drm_buddy_block_trim(struct drm_buddy *mm,
+u64 new_size,
+struct list_head *blocks)
+{
+ struct drm_buddy_block *parent;
+ struct drm_buddy_block *block;
+ LIST_HEAD(dfs);
+ u64 new_start;
+ int err;
+
+ if (!list_is_singular(blocks))
+ return -EINVAL;
+
+ block = list_first_entry(blocks,
+struct drm_buddy_block,
+link);
+
+ if (!drm_buddy_block_is_allocated(block))
+ return -EINVAL;
+
+ if (new_size > drm_buddy_block_size(mm, block))
+ return -EINVAL;
+
+ if (!new_size && !IS_ALIGNED(new_size, mm->chunk_size))
+ return -EINVAL;
+
+ if (new_size == drm_buddy_block_size(mm, block))
+ return 0;
+
+ list_del(&block->link);
+ mark_free(mm, block);
+ mm->avail += drm_buddy_block_size(mm, block);
+
+ /* Prevent recursively freeing this node */
+ parent = block->parent;
+ block->parent = NULL;
+
+ new_start = drm_buddy_block_offset(block);
+ list_add(&block->tmp_link, &dfs);
+ err = __alloc_range(mm, &dfs, new_start, new_size, blocks);
+ if (err) {
+ mark_allocated(block);
+ mm->avail -= drm_buddy_block_size(mm, block);
+ list_add(&block->link, blocks);
+ }
+
+ block->parent = parent;
+ return err;
+}
+EXPORT_SYMBOL(drm_buddy_block_trim);
+
/**
* drm_buddy_alloc_blocks - allocate power-of-two blocks
*
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 3662434b64bb..53eb100688a6 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -97,6 +97,16 @@ static int i915_ttm_buddy_man_alloc(struct
ttm_resource_manager *man,
if (unlikely(err))
goto err_free_blocks;
+ if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
+ u64 original_size = (u64)bman_res->base.num_pages << PAGE_SHIFT;
+
+ mutex_lock(&bman->lock);
+ drm_buddy_block_trim(mm,
+original_size,
+&bman_res->blocks);
+ mutex_unlock(&bman->lock);
+ }
+
*res = &bman_res->base;
return 0;
diff --git a/include/dr
[PATCH v9 5/6] drm/amdgpu: move vram inline functions into a header
Move shared vram inline functions and structs
into a header file
Signed-off-by: Arunpravin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 51
1 file changed, 51 insertions(+)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
new file mode 100644
index ..59983464cce5
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: MIT
+ * Copyright 2021 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __AMDGPU_VRAM_MGR_H__
+#define __AMDGPU_VRAM_MGR_H__
+
+#include
+
+struct amdgpu_vram_mgr_node {
+ struct ttm_resource base;
+ struct list_head blocks;
+ unsigned long flags;
+};
+
+static inline u64 amdgpu_node_start(struct drm_buddy_block *block)
+{
+ return drm_buddy_block_offset(block);
+}
+
+static inline u64 amdgpu_node_size(struct drm_buddy_block *block)
+{
+ return PAGE_SIZE << drm_buddy_block_order(block);
+}
+
+static inline struct amdgpu_vram_mgr_node *
+to_amdgpu_vram_mgr_node(struct ttm_resource *res)
+{
+ return container_of(res, struct amdgpu_vram_mgr_node, base);
+}
+
+#endif
--
2.25.1
[PATCH v9 6/6] drm/amdgpu: add drm buddy support to amdgpu
- Remove drm_mm references and replace with drm buddy functionalities
- Add res cursor support for drm buddy
v2(Matthew Auld):
- replace spinlock with mutex as we call kmem_cache_zalloc
(..., GFP_KERNEL) in drm_buddy_alloc() function
- lock drm_buddy_block_trim() function as it calls
mark_free/mark_split are all globally visible
v3(Matthew Auld):
- remove trim method error handling as we address the failure case
at drm_buddy_block_trim() function
v4:
- fix warnings reported by kernel test robot
v5:
- fix merge conflict issue
Signed-off-by: Arunpravin
---
drivers/gpu/drm/Kconfig | 1 +
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 97 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 259 ++
4 files changed, 231 insertions(+), 133 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index dfdd3ec5f793..eb5a57ae3c5c 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -279,6 +279,7 @@ config DRM_AMDGPU
select HWMON
select BACKLIGHT_CLASS_DEVICE
select INTERVAL_TREE
+ select DRM_BUDDY
help
Choose this option if you have a recent AMD Radeon graphics card.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index acfa207cf970..da12b4ff2e45 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -30,12 +30,15 @@
#include
#include
+#include "amdgpu_vram_mgr.h"
+
/* state back for walking over vram_mgr and gtt_mgr allocations */
struct amdgpu_res_cursor {
uint64_tstart;
uint64_tsize;
uint64_tremaining;
- struct drm_mm_node *node;
+ void*node;
+ uint32_tmem_type;
};
/**
@@ -52,27 +55,63 @@ static inline void amdgpu_res_first(struct ttm_resource
*res,
uint64_t start, uint64_t size,
struct amdgpu_res_cursor *cur)
{
+ struct drm_buddy_block *block;
+ struct list_head *head, *next;
struct drm_mm_node *node;
- if (!res || res->mem_type == TTM_PL_SYSTEM) {
- cur->start = start;
- cur->size = size;
- cur->remaining = size;
- cur->node = NULL;
- WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
- return;
- }
+ if (!res)
+ goto err_out;
BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
- node = to_ttm_range_mgr_node(res)->mm_nodes;
- while (start >= node->size << PAGE_SHIFT)
- start -= node++->size << PAGE_SHIFT;
+ cur->mem_type = res->mem_type;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ head = &to_amdgpu_vram_mgr_node(res)->blocks;
+
+ block = list_first_entry_or_null(head,
+struct drm_buddy_block,
+link);
+ if (!block)
+ goto err_out;
+
+ while (start >= amdgpu_node_size(block)) {
+ start -= amdgpu_node_size(block);
+
+ next = block->link.next;
+ if (next != head)
+ block = list_entry(next, struct
drm_buddy_block, link);
+ }
+
+ cur->start = amdgpu_node_start(block) + start;
+ cur->size = min(amdgpu_node_size(block) - start, size);
+ cur->remaining = size;
+ cur->node = block;
+ break;
+ case TTM_PL_TT:
+ node = to_ttm_range_mgr_node(res)->mm_nodes;
+ while (start >= node->size << PAGE_SHIFT)
+ start -= node++->size << PAGE_SHIFT;
+
+ cur->start = (node->start << PAGE_SHIFT) + start;
+ cur->size = min((node->size << PAGE_SHIFT) - start, size);
+ cur->remaining = size;
+ cur->node = node;
+ break;
+ default:
+ goto err_out;
+ }
- cur->start = (node->start << PAGE_SHIFT) + start;
- cur->size = min((node->size << PAGE_SHIFT) - start, size);
+ return;
+
+err_out:
+ cur->start = start;
+ cur->size = size;
cur->remaining = size;
- cur->node = node;
+ cur->node = NULL;
+ WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
+ return;
}
/**
@@ -85,7 +124,9 @@ static inline void amdgpu_res_first(struct ttm_resource *res,
*/
static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t
size)
{
- struct drm_mm_node *node = cur->node;
+ struct drm_buddy_block *block;
+
[PATCH] drm/etnaviv: Fix runtime PM imbalance on error
pm_runtime_get_sync() will increase the rumtime PM counter even it returns an error. Thus a pairing decrement is needed to prevent refcount leak. Fix this by replacing this API with pm_runtime_resume_and_get(), which will not change the runtime PM counter on error. Signed-off-by: Yongzhi Liu --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 242a5fd..aa64f45 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1690,7 +1690,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master, goto out_workqueue; #ifdef CONFIG_PM - ret = pm_runtime_get_sync(gpu->dev); + ret = pm_runtime_resume_and_get(gpu->dev); #else ret = etnaviv_gpu_clk_enable(gpu); #endif -- 2.7.4
Re: Re: [PATCH] drm/etnaviv: Add missing pm_runtime_put
> -原始邮件- > 发件人: "Lucas Stach" > 发送时间: 2022-01-19 18:51:20 (星期三) > 收件人: "Yongzhi Liu" , linux+etna...@armlinux.org.uk, > christian.gmei...@gmail.com, airl...@linux.ie, dan...@ffwll.ch, > etna...@lists.freedesktop.org > 抄送: dri-devel@lists.freedesktop.org, linux-ker...@vger.kernel.org > 主题: Re: [PATCH] drm/etnaviv: Add missing pm_runtime_put > > Am Dienstag, dem 18.01.2022 um 06:16 -0800 schrieb Yongzhi Liu: > > pm_runtime_get_sync() increments the runtime PM usage counter even > > when it returns an error code, thus a matching decrement is needed > > on the error handling path to keep the counter balanced. > > > Instead of adding more error handling code here, I would prefer to > convert this to pm_runtime_resume_and_get to avoid this issue. > > Regards, > Lucas > I will resend my modified patch. Thanks for your reply. > > Signed-off-by: Yongzhi Liu > > --- > > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > > index 242a5fd..5e81a98 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > > @@ -1714,6 +1714,9 @@ static int etnaviv_gpu_bind(struct device *dev, > > struct device *master, > > return 0; > > > > out_sched: > > +#ifdef CONFIG_PM > > + pm_runtime_put_autosuspend(gpu->dev); > > +#endif > > etnaviv_sched_fini(gpu); > > > > out_workqueue: >
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
Hi Greg,
On Wed, Jan 19, 2022 at 12:28 PM Greg Kroah-Hartman
wrote:
> On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote:
> > > This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
> > >
> > > Revert this patch. This patch started to introduce the regression that
> > > all hardware acceleration of more than 35 existing fbdev drivers were
> > > bypassed and thus fbcon console output for those was dramatically slowed
> > > down by factor of 10 and more.
> > >
> > > Reverting this commit has no impact on DRM, since none of the DRM drivers
> > > are
> > > tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
> > > FBINFO_HWACCEL_FILLRECT or others.
> > >
> > > Signed-off-by: Helge Deller
> > As for "why", I think there was a number of private bugs that were
> > reported in this code, which is why it was removed. I do not think it
> > can be safely added back in without addressing them first. Let me go
> > dig through my email to see if I can find them...
>
> Ah, no, that was just the soft scrollback code I was thinking of, which
So the bugs argument is moot.
> was a different revert and is still gone, thankfully :)
FTR, not everyone else was thankful about that one...
> This one was just removed because Daniel noticed that only 3 drivers
> used this (nouveau, omapdrm, and gma600), so this shouldn't have caused
> any regressions in any other drivers like you are reporting here.
>
> So perhaps this regression is caused by something else?
1. Daniel's patch was not CCed to linux-fbdev,
2. When I discovered the patch, I pointed out that the premise of 3
drivers was not true, and that it affects 32 more fbdev drivers[1] .
The patch was applied regardless.
3. When the patch was suggested for backporting, I pointed out the
same[2].
The patch was backported regardless.
[1]
https://lore.kernel.org/r/alpine.deb.2.22.394.201036530.379...@ramsan.of.borg/
[2]
https://lore.kernel.org/r/camuhmdxrgam2zahpegcw8+76xm-0ao-ci9-ymva5jptkvhp...@mail.gmail.com/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v5, 15/15] media: mtk-vcodec: support stateless VP9 decoding
Il 19/01/22 12:28, AngeloGioacchino Del Regno ha scritto:
Il 17/01/22 10:40, Yunfei Dong ha scritto:
Add support for VP9 decoding using the stateless API,
as supported by MT8192. And the drivers is lat and core architecture.
Signed-off-by: Yunfei Dong
Signed-off-by: George Sun
---
drivers/media/platform/mtk-vcodec/Makefile | 1 +
.../mtk-vcodec/mtk_vcodec_dec_stateless.c | 26 +-
.../platform/mtk-vcodec/mtk_vcodec_drv.h | 1 +
.../mtk-vcodec/vdec/vdec_vp9_req_lat_if.c | 1973 +
.../media/platform/mtk-vcodec/vdec_drv_if.c | 4 +
.../media/platform/mtk-vcodec/vdec_drv_if.h | 1 +
6 files changed, 2003 insertions(+), 3 deletions(-)
create mode 100644
drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_req_lat_if.c
Hello Yunfei,
this driver is based on an older version of the VP9 stateless decoder uAPI,
hence this is not applicable upstream.
The latest linux-next tag (as of today) already contains the new and
accepted code; can you please rebase over that one?
Thanks,
Angelo
While finishing a rebase, I had time to do a fast port of this patch; in hopes
to spare you some time, I'm giving you my (fast) take at this.
Feel free to use it as you wish!
From 5f329ad271c94bf82d2dd12075372159466c28f9 Mon Sep 17 00:00:00 2001
From: AngeloGioacchino Del Regno
Date: Wed, 19 Jan 2022 12:45:18 +0100
Subject: [PATCH] media: mtk-vcodec: Port VP9 stateless driver to upstream uAPI
This driver was written based on an old VP9 uAPI, but that code
changed over time: port this over the newest, and upstream accepted,
VP9 uAPI.
Signed-off-by: AngeloGioacchino Del Regno
---
.../mtk-vcodec/mtk_vcodec_dec_stateless.c | 2 +-
.../mtk-vcodec/vdec/vdec_vp9_req_lat_if.c | 29 +++
2 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c
b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c
index 26fd97d867e9..7f4baa39bf6c 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c
@@ -94,7 +94,7 @@ static const struct mtk_stateless_control
mtk_stateless_controls[] = {
},
{
.cfg = {
- .id = V4L2_CID_MPEG_VIDEO_VP9_FRAME_DECODE_PARAMS,
+ .id = V4L2_CID_STATELESS_VP9_FRAME,
},
.codec_type = V4L2_PIX_FMT_VP9_FRAME,
},
diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_req_lat_if.c
b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_req_lat_if.c
index 92cd39f00840..8caf4f28db29 100644
--- a/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_req_lat_if.c
+++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_req_lat_if.c
@@ -711,7 +711,7 @@ static int vdec_vp9_slice_setup_lat_from_src_buf(struct
vdec_vp9_slice_instance
static void vdec_vp9_slice_setup_hdr(struct vdec_vp9_slice_instance *instance,
struct vdec_vp9_slice_uncompressed_header
*uh,
-struct v4l2_ctrl_vp9_frame_decode_params
*hdr)
+struct v4l2_ctrl_vp9_frame *hdr)
{
int i;
@@ -749,13 +749,13 @@ static void vdec_vp9_slice_setup_hdr(struct
vdec_vp9_slice_instance *instance,
* - LAST_FRAME = 1,
* - GOLDEN_FRAME = 2,
* - ALTREF_FRAME = 3,
-* ref_frame_sign_biases[INTRA_FRAME] is always 0
+* ref_frame_sign_bias[INTRA_FRAME] is always 0
* and VDA only passes another 3 directions
*/
uh->ref_frame_sign_bias[0] = 0;
for (i = 0; i < 3; i++)
uh->ref_frame_sign_bias[i + 1] =
- !!(hdr->ref_frame_sign_biases & (1 << i));
+ !!(hdr->ref_frame_sign_bias & (1 << i));
uh->allow_high_precision_mv = HDR_FLAG(ALLOW_HIGH_PREC_MV);
uh->interpolation_filter = hdr->interpolation_filter;
uh->refresh_frame_context = HDR_FLAG(REFRESH_FRAME_CTX);
@@ -772,7 +772,7 @@ static void vdec_vp9_slice_setup_hdr(struct
vdec_vp9_slice_instance *instance,
static void vdec_vp9_slice_setup_frame_ctx(struct vdec_vp9_slice_instance
*instance,
struct
vdec_vp9_slice_uncompressed_header *uh,
- struct
v4l2_ctrl_vp9_frame_decode_params *hdr)
+ struct v4l2_ctrl_vp9_frame *hdr)
{
int error_resilient_mode;
int reset_frame_context;
@@ -857,7 +857,7 @@ static void vdec_vp9_slice_setup_segmentation(struct
vdec_vp9_slice_uncompressed
}
static int vdec_vp9_slice_setup_tile(struct vdec_vp9_slice_vsi *vsi,
-struct v4l2_ctrl_vp9_frame_decode_params
*hdr)
+struct v4l2_ctrl_vp9_frame *hdr
Re: [PATCH v2] phy: dphy: Correct clk_pre parameter
Hi,
On Wed, Jan 19, 2022 at 10:37:14AM +0800, Liu Ying wrote:
> The D-PHY specification (v1.2) explicitly mentions that the T-CLK-PRE
> parameter's unit is Unit Interval(UI) and the minimum value is 8. Also,
> kernel doc of the 'clk_pre' member of struct phy_configure_opts_mipi_dphy
> mentions that it should be in UI. However, the dphy core driver wrongly
> sets 'clk_pre' to 8000, which seems to hint that it's in picoseconds.
> And, the kernel doc of the 'clk_pre' member wrongly says the minimum value
> is '8 UI', instead of 8.
>
> So, let's fix both the dphy core driver and the kernel doc of the 'clk_pre'
> member to correctly reflect the T-CLK-PRE parameter's unit and the minimum
> value according to the D-PHY specification.
>
> I'm assuming that all impacted custom drivers shall program values in
> TxByteClkHS cycles into hardware for the T-CLK-PRE parameter. The D-PHY
> specification mentions that the frequency of TxByteClkHS is exactly 1/8
> the High-Speed(HS) bit rate(each HS bit consumes one UI). So, relevant
> custom driver code is changed to program those values as
> DIV_ROUND_UP(cfg->clk_pre, BITS_PER_BYTE), then.
>
> Note that I've only tested the patch with RM67191 DSI panel on i.MX8mq EVK.
> Help is needed to test with other i.MX8mq, Meson and Rockchip platforms,
> as I don't have the hardwares.
>
> Fixes: 2ed869990e14 ("phy: Add MIPI D-PHY configuration options")
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Kishon Vijay Abraham I
> Cc: Vinod Koul
> Cc: Kevin Hilman
> Cc: Jerome Brunet
> Cc: Martin Blumenstingl
> Cc: Heiko Stuebner
> Cc: Maxime Ripard
> Cc: Guido Günther
> Tested-by: Liu Ying # RM67191 DSI panel on i.MX8mq EVK
> Signed-off-by: Liu Ying
> ---
> v1->v2:
> * Use BITS_PER_BYTE macro. (Andrzej)
> * Drop dsi argument from ui2bc() in nwl-dsi.c.
>
> drivers/gpu/drm/bridge/nwl-dsi.c | 12 +---
> drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c| 3 ++-
> drivers/phy/phy-core-mipi-dphy.c | 4 ++--
> drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c | 3 ++-
> include/linux/phy/phy-mipi-dphy.h| 2 +-
> 5 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c
> b/drivers/gpu/drm/bridge/nwl-dsi.c
> index a7389a0facfb..af07eeb47ca0 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -196,12 +197,9 @@ static u32 ps2bc(struct nwl_dsi *dsi, unsigned long long
> ps)
> /*
> * ui2bc - UI time periods to byte clock cycles
> */
> -static u32 ui2bc(struct nwl_dsi *dsi, unsigned long long ui)
> +static u32 ui2bc(unsigned int ui)
> {
> - u32 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> -
> - return DIV64_U64_ROUND_UP(ui * dsi->lanes,
> - dsi->mode.clock * 1000 * bpp);
> + return DIV_ROUND_UP(ui, BITS_PER_BYTE);
> }
>
> /*
> @@ -232,12 +230,12 @@ static int nwl_dsi_config_host(struct nwl_dsi *dsi)
> }
>
> /* values in byte clock cycles */
> - cycles = ui2bc(dsi, cfg->clk_pre);
> + cycles = ui2bc(cfg->clk_pre);
> DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_t_pre: 0x%x\n", cycles);
> nwl_dsi_write(dsi, NWL_DSI_CFG_T_PRE, cycles);
> cycles = ps2bc(dsi, cfg->lpx + cfg->clk_prepare + cfg->clk_zero);
> DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap (pre): 0x%x\n", cycles);
> - cycles += ui2bc(dsi, cfg->clk_pre);
> + cycles += ui2bc(cfg->clk_pre);
> DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_t_post: 0x%x\n", cycles);
> nwl_dsi_write(dsi, NWL_DSI_CFG_T_POST, cycles);
> cycles = ps2bc(dsi, cfg->hs_exit);
> diff --git a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> index cd2332bf0e31..fdbd64c03e12 100644
> --- a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> +++ b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c
> @@ -9,6 +9,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -250,7 +251,7 @@ static int phy_meson_axg_mipi_dphy_power_on(struct phy
> *phy)
>(DIV_ROUND_UP(priv->config.clk_zero, temp) << 16) |
>(DIV_ROUND_UP(priv->config.clk_prepare, temp) << 24));
> regmap_write(priv->regmap, MIPI_DSI_CLK_TIM1,
> - DIV_ROUND_UP(priv->config.clk_pre, temp));
> + DIV_ROUND_UP(priv->config.clk_pre, BITS_PER_BYTE));
>
> regmap_write(priv->regmap, MIPI_DSI_HS_TIM,
>DIV_ROUND_UP(priv->config.hs_exit, temp) |
> diff --git a/drivers/phy/phy-core-mipi-dphy.c
> b/drivers/phy/phy-core-mipi-dphy.c
> index 288c9c67aa74..ccb4045685cd 100644
> --- a/drivers/phy/phy-core-mipi-dphy.c
> +++ b/drivers/phy/phy-core-mipi-dphy.c
> @@ -36,7 +36,7
Re: [PATCH] dt-bindings: Improve phandle-array schemas
On 19.01.22 3:50, Rob Herring wrote: The 'phandle-array' type is a bit ambiguous. It can be either just an array of phandles or an array of phandles plus args. Many schemas for phandle-array properties aren't clear in the schema which case applies though the description usually describes it. The array of phandles case boils down to needing: items: maxItems: 1 The phandle plus args cases should typically take this form: items: - items: - description: A phandle - description: 1st arg cell - description: 2nd arg cell With this change, some examples need updating so that the bracketing of property values matches the schema. [..] .../bindings/interconnect/qcom,rpmh.yaml | 2 + Acked-by: Georgi Djakov
[Bug 215499] AMDGPU: Tahiti flagged as "[drm] Unsupported asic. Remove me when IP discovery init is in place."
https://bugzilla.kernel.org/show_bug.cgi?id=215499 Artem S. Tashkinov (a...@gmx.com) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |MOVED --- Comment #2 from Artem S. Tashkinov (a...@gmx.com) --- https://gitlab.freedesktop.org/drm/amd/-/issues/1860 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v2] mgag200 fix memmapsl configuration in GCTL6 register
Hi, Am 19.01.22 um 11:29 schrieb Jocelyn Falempe: On some servers with MGA G200_SE_A (rev 42), booting with Legacy BIOS, the hardware hangs when using kdump and kexec into the kdump kernel. This happens when the uncompress code tries to write "Decompressing Linux" to the VGA Console. It can be reproduced by writing to the VGA console (0xB8000) after booting to graphic mode, it generates the following error: kernel:NMI: PCI system error (SERR) for reason a0 on CPU 0. kernel:Dazed and confused, but trying to continue The root cause is the configuration of the MGA GCTL6 register According to the GCTL6 register documentation: bit 0 is gcgrmode: 0: Enables alpha mode, and the character generator addressing system is activated. 1: Enables graphics mode, and the character addressing system is not used. bit 1 is chainodd even: 0: The A0 signal of the memory address bus is used during system memory addressing. 1: Allows A0 to be replaced by either the A16 signal of the system address (ifmemmapsl is ‘00’), or by the hpgoddev (MISC<5>, odd/even page select) field, described on page 3-294). bit 3-2 are memmapsl: Memory map select bits 1 and 0. VGA. These bits select where the video memory is mapped, as shown below: 00 => Ah - Bh 01 => Ah - Ah 10 => Bh - B7FFFh 11 => B8000h - Bh bit 7-4 are reserved. Current code set it to 0x05 => memmapsl to b01 => 0xa (graphic mode) But on x86, the VGA console is at 0xb8000 (text mode) In arch/x86/boot/compressed/misc.c debug strings are written to 0xb8000 As the driver doesn't use this mapping at 0xa, it is safe to set it to 0xb8000 instead, to avoid kernel hang on G200_SE_A rev42, with kexec/kdump. Thus changing the value 0x05 to 0x0d Signed-off-by: Jocelyn Falempe Reviewed-by: Javier Martinez Canillas Acked-by: Lyude Paul Cc: sta...@vger.kernel.org --- v2: Add clear statement that it's not the right configuration, but it prevents an annoying bug with kexec/kdump. drivers/gpu/drm/mgag200/mgag200_mode.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index b983541a4c53..cd9ba13ad5fc 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -529,7 +529,10 @@ static void mgag200_set_format_regs(struct mga_device *mdev, WREG_GFX(3, 0x00); WREG_GFX(4, 0x00); WREG_GFX(5, 0x40); - WREG_GFX(6, 0x05); + /* GCTL6 should be 0x05, but we configure memmapsl to 0xb8000 (text mode), +* so that it doesn't hang when running kexec/kdump on G200_SE rev42. +*/ + WREG_GFX(6, 0x0d); Appears to be working on my test machine. But please rune scripts/checkpatch.pl on the patch before sending it. I get several errors WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #98: 0: Enables alpha mode, and the character generator addressing system is ERROR: trailing whitespace #149: FILE: drivers/gpu/drm/mgag200/mgag200_mode.c:532: +^I/* GCTL6 should be 0x05, but we configure memmapsl to 0xb8000 (text mode),^M$ ERROR: trailing whitespace #150: FILE: drivers/gpu/drm/mgag200/mgag200_mode.c:533: +^I * so that it doesn't hang when running kexec/kdump on G200_SE rev42.^M$ Best regards Thomas WREG_GFX(7, 0x0f); WREG_GFX(8, 0x0f); -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[PATCH v2] drm/bridge: dw-hdmi: use safe format when first in bridge chain
When the dw-hdmi bridge is in first place of the bridge chain, this
means there is no way to select an input format of the dw-hdmi HW
component.
Since introduction of display-connector, negotiation was broken since
the dw-hdmi negotiation code only worked when the dw-hdmi bridge was
in last position of the bridge chain or behind another bridge also
supporting input & output format negotiation.
Commit 0656d1285b79 ("drm/bridge: display-connector: implement bus fmts
callbacks")
was introduced to make negotiation work again by making display-connector
act as a pass-through concerning input & output format negotiation.
But in the case where the dw-hdmi is single in the bridge chain, for
example on Renesas SoCs, with the display-connector bridge the dw-hdmi
is no more single, breaking output format.
Reported-by: Biju Das
Bisected-by: Kieran Bingham
Tested-by: Kieran Bingham
Fixes: 0656d1285b79 ("drm/bridge: display-connector: implement bus fmts
callbacks").
Signed-off-by: Neil Armstrong
---
Changes since v1:
- Remove bad fix in dw_hdmi_bridge_atomic_get_input_bus_fmts
- Fix typos in commit message
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 54d8fdad395f..97cdc61b57f6 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2551,8 +2551,9 @@ static u32
*dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
if (!output_fmts)
return NULL;
- /* If dw-hdmi is the only bridge, avoid negociating with ourselves */
- if (list_is_singular(&bridge->encoder->bridge_chain)) {
+ /* If dw-hdmi is the first or only bridge, avoid negociating with
ourselves */
+ if (list_is_singular(&bridge->encoder->bridge_chain) ||
+ list_is_first(&bridge->chain_node, &bridge->encoder->bridge_chain))
{
*num_output_fmts = 1;
output_fmts[0] = MEDIA_BUS_FMT_FIXED;
--
2.25.1
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
Hello Greg,
On 1/19/22 12:47, Geert Uytterhoeven wrote:
> On Wed, Jan 19, 2022 at 12:28 PM Greg Kroah-Hartman
> wrote:
>> On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote:
>>> On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote:
This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
Revert this patch. This patch started to introduce the regression that
all hardware acceleration of more than 35 existing fbdev drivers were
bypassed and thus fbcon console output for those was dramatically slowed
down by factor of 10 and more.
Reverting this commit has no impact on DRM, since none of the DRM drivers
are
tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
FBINFO_HWACCEL_FILLRECT or others.
Signed-off-by: Helge Deller
>
>>> As for "why", I think there was a number of private bugs that were
>>> reported in this code, which is why it was removed. I do not think it
>>> can be safely added back in without addressing them first. Let me go
>>> dig through my email to see if I can find them...
>>
>> Ah, no, that was just the soft scrollback code I was thinking of, which
Right.
That was commit 973c096f6a85 and it was about vgacon, not fbcon.
I did mentioned it in my cover letter, together with my analysis of
the reported bugs.
Maybe I should have put all the information from the cover letter into
the patch here as well. If you haven't read the cover letter yet, please do.
Helge
> So the bugs argument is moot.
>
>> was a different revert and is still gone, thankfully :)
>
> FTR, not everyone else was thankful about that one...
>
>> This one was just removed because Daniel noticed that only 3 drivers
>> used this (nouveau, omapdrm, and gma600), so this shouldn't have caused
>> any regressions in any other drivers like you are reporting here.
>>
>> So perhaps this regression is caused by something else?
>
> 1. Daniel's patch was not CCed to linux-fbdev,
> 2. When I discovered the patch, I pointed out that the premise of 3
>drivers was not true, and that it affects 32 more fbdev drivers[1] .
>The patch was applied regardless.
> 3. When the patch was suggested for backporting, I pointed out the
>same[2].
>The patch was backported regardless.
>
> [1]
> https://lore.kernel.org/r/alpine.deb.2.22.394.201036530.379...@ramsan.of.borg/
> [2]
> https://lore.kernel.org/r/camuhmdxrgam2zahpegcw8+76xm-0ao-ci9-ymva5jptkvhp...@mail.gmail.com/
>
> Gr{oetje,eeting}s,
>
> Geert
[PATCH 1/2] drm/bridge: dw-hdmi: filter safe formats when first in bridge chain
If the dw-bridge is in the first position in the bridge chain, this
means there is no way to set the encoder output bus format.
In this case, this makes sure we only return the default format as return
of the get_input_bus_fmts() callback, limiting possible output formats
of dw-hdmi to what the dw-hdmi can convert from the default RGB24 input
format.
Fixes: 6c3c719936da ("drm/bridge: synopsys: dw-hdmi: add bus format
negociation")
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 97cdc61b57f6..56021f20d396 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2674,6 +2674,25 @@ static u32
*dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
if (!input_fmts)
return NULL;
+ /* If dw-hdmi is the first bridge make sure it only takes RGB24 as
input */
+ if (list_is_first(&bridge->chain_node, &bridge->encoder->bridge_chain))
{
+ switch (output_fmt) {
+ case MEDIA_BUS_FMT_FIXED:
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ case MEDIA_BUS_FMT_YUV8_1X24:
+ case MEDIA_BUS_FMT_UYVY8_1X16:
+ input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+ break;
+ default:
+ kfree(input_fmts);
+ input_fmts = NULL;
+ }
+
+ *num_input_fmts = i;
+
+ return input_fmts;
+ }
+
switch (output_fmt) {
/* If MEDIA_BUS_FMT_FIXED is tested, return default bus format */
case MEDIA_BUS_FMT_FIXED:
--
2.25.1
[PATCH 2/2] drm/bridge: dw-hdmi: filter out YUV output formats when DVI
When the display is not an HDMI sink, only the RGB output format is
valid. Thus stop returning YUV output formats when sink is not HDMI.
Fixes: 6c3c719936da ("drm/bridge: synopsys: dw-hdmi: add bus format
negociation")
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 56021f20d396..03057d335158 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2538,6 +2538,7 @@ static u32
*dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
struct drm_connector *conn = conn_state->connector;
struct drm_display_info *info = &conn->display_info;
struct drm_display_mode *mode = &crtc_state->mode;
+ struct dw_hdmi *hdmi = bridge->driver_private;
u8 max_bpc = conn_state->max_requested_bpc;
bool is_hdmi2_sink = info->hdmi.scdc.supported ||
(info->color_formats & DRM_COLOR_FORMAT_YCRCB420);
@@ -2564,7 +2565,7 @@ static u32
*dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
* If the current mode enforces 4:2:0, force the output but format
* to 4:2:0 and do not add the YUV422/444/RGB formats
*/
- if (conn->ycbcr_420_allowed &&
+ if (hdmi->sink_is_hdmi && conn->ycbcr_420_allowed &&
(drm_mode_is_420_only(info, mode) ||
(is_hdmi2_sink && drm_mode_is_420_also(info, mode {
@@ -2595,36 +2596,36 @@ static u32
*dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
*/
if (max_bpc >= 16 && info->bpc == 16) {
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
+ if (hdmi->sink_is_hdmi && info->color_formats &
DRM_COLOR_FORMAT_YCRCB444)
output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
output_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
}
if (max_bpc >= 12 && info->bpc >= 12) {
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
+ if (hdmi->sink_is_hdmi && info->color_formats &
DRM_COLOR_FORMAT_YCRCB422)
output_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
+ if (hdmi->sink_is_hdmi && info->color_formats &
DRM_COLOR_FORMAT_YCRCB444)
output_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
output_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
}
if (max_bpc >= 10 && info->bpc >= 10) {
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
+ if (hdmi->sink_is_hdmi && info->color_formats &
DRM_COLOR_FORMAT_YCRCB422)
output_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
+ if (hdmi->sink_is_hdmi && info->color_formats &
DRM_COLOR_FORMAT_YCRCB444)
output_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
}
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
+ if (hdmi->sink_is_hdmi && info->color_formats &
DRM_COLOR_FORMAT_YCRCB422)
output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
- if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
+ if (hdmi->sink_is_hdmi && info->color_formats &
DRM_COLOR_FORMAT_YCRCB444)
output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
/* Default 8bit RGB fallback */
--
2.25.1
Re: [PATCH 0/2] Fix regression introduced by disabling accelerated scrolling in fbcon
Hi Helge,
On Wed, Jan 19, 2022 at 12:10 PM Helge Deller wrote:
> This series reverts two patches which disabled scrolling acceleration in
> fbcon/fbdev. Those patches introduced a regression for fbdev-supported graphic
> cards because of the performance penalty by doing screen scrolling by software
> instead of using hardware acceleration.
>
> Console scrolling acceleration was disabled by dropping code which checked at
> runtime the driver hardware possibilities for the BINFO_HWACCEL_COPYAREA or
> FBINFO_HWACCEL_FILLRECT flags and if set, it enabled scrollmode SCROLL_MOVE
> which uses hardware acceleration to move screen contents. After dropping
> those
> checks scrollmode was hard-wired to SCROLL_REDRAW instead, which forces all
> graphic cards to redraw every character at the new screen position when
> scrolling.
>
> This change effectively disabled all hardware-based scrolling acceleration for
> ALL drivers, because now all kind of 2D hardware acceleration (bitblt,
> fillrect) in the drivers isn't used any longer.
>
> The original commit message mentions that only 3 DRM drivers (nouveau, omapdrm
> and gma500) used hardware acceleration in the past and thus code for checking
> and using scrolling acceleration is obsolete.
>
> This statement is NOT TRUE, because beside the DRM drivers there are around 35
> other fbdev drivers which depend on fbdev/fbcon and still provide hardware
> acceleration for fbdev/fbcon.
>
> The original commit message also states that syzbot found lots of bugs in
> fbcon
> and thus it's "often the solution to just delete code and remove features".
> This is true, and the bugs - which actually affected all users of fbcon,
> including DRM - were fixed, or code was dropped like e.g. the support for
> software scrollback in vgacon (commit 973c096f6a85).
>
> So to further analyze which bugs were found by syzbot, I've looked through all
> patches in drivers/video which were tagged with syzbot or syzkaller back to
> year 2005. The vast majority fixed the reported issues on a higher level, e.g.
> when screen is to be resized, or when font size is to be changed. The few ones
> which touched driver code fixed a real driver bug, e.g. by adding a check.
>
> But NONE of those patches touched code of either the SCROLL_MOVE or the
> SCROLL_REDRAW case.
>
> That means, there was no real reason why SCROLL_MOVE had to be ripped-out and
> just SCROLL_REDRAW had to be used instead. The only reason I can imagine so
> far
> was that SCROLL_MOVE wasn't used by DRM and as such it was assumed that it
> could go away. That argument completely missed the fact that SCROLL_MOVE is
> still heavily used by fbdev (non-DRM) drivers.
>
> Some people mention that using memcpy() instead of the hardware acceleration
> is
> pretty much the same speed. But that's not true, at least not for older
> graphic
> cards and machines where we see speed decreases by factor 10 and more and thus
> this change leads to console responsiveness way worse than before.
>
> That's why I propose to revert those patches, re-introduce hardware-based
> scrolling acceleration and fix the performance-regression for fbdev drivers.
> There isn't any impact on DRM when reverting those patches.
>
> Helge Deller (2):
> Revert "fbdev: Garbage collect fbdev scrolling acceleration, part 1
> (from TODO list)"
> Revert "fbcon: Disable accelerated scrolling"
Thank you for this series, and the prior analysis!
Acked-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
Hi Greg, Greg Kroah-Hartman writes: > On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote: >> On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote: >> > This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151. >> > >> > Revert this patch. This patch started to introduce the regression that >> > all hardware acceleration of more than 35 existing fbdev drivers were >> > bypassed and thus fbcon console output for those was dramatically slowed >> > down by factor of 10 and more. >> > >> > Reverting this commit has no impact on DRM, since none of the DRM drivers >> > are >> > tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA, >> > FBINFO_HWACCEL_FILLRECT or others. >> > >> > Signed-off-by: Helge Deller >> > Cc: sta...@vger.kernel.org # v5.16 >> >> Why just 5.16? This commit came in on 5.11 and was backported to >> 5.10.5. >> >> As for "why", I think there was a number of private bugs that were >> reported in this code, which is why it was removed. I do not think it >> can be safely added back in without addressing them first. Let me go >> dig through my email to see if I can find them... > > Ah, no, that was just the soft scrollback code I was thinking of, which > was a different revert and is still gone, thankfully :) > > This one was just removed because Daniel noticed that only 3 drivers > used this (nouveau, omapdrm, and gma600), so this shouldn't have caused > any regressions in any other drivers like you are reporting here. I'm counting more than 3 drivers using this. I think one of the reasons why it was reverted was that no one is actively maintaining fbdev. With Helge now volunteering i don't see a reason why it should stay reverted. If there are issues coming up i'm pretty sure Helge would care, and i would probably also take a look. /Sven
Re: [PATCH 0/2] Fix regression introduced by disabling accelerated scrolling in fbcon
Helge Deller writes: > This series reverts two patches which disabled scrolling acceleration in > fbcon/fbdev. Those patches introduced a regression for fbdev-supported graphic > cards because of the performance penalty by doing screen scrolling by software > instead of using hardware acceleration. > > Console scrolling acceleration was disabled by dropping code which checked at > runtime the driver hardware possibilities for the BINFO_HWACCEL_COPYAREA or > FBINFO_HWACCEL_FILLRECT flags and if set, it enabled scrollmode SCROLL_MOVE > which uses hardware acceleration to move screen contents. After dropping > those > checks scrollmode was hard-wired to SCROLL_REDRAW instead, which forces all > graphic cards to redraw every character at the new screen position when > scrolling. > > This change effectively disabled all hardware-based scrolling acceleration for > ALL drivers, because now all kind of 2D hardware acceleration (bitblt, > fillrect) in the drivers isn't used any longer. > > The original commit message mentions that only 3 DRM drivers (nouveau, omapdrm > and gma500) used hardware acceleration in the past and thus code for checking > and using scrolling acceleration is obsolete. > > This statement is NOT TRUE, because beside the DRM drivers there are around 35 > other fbdev drivers which depend on fbdev/fbcon and still provide hardware > acceleration for fbdev/fbcon. > > The original commit message also states that syzbot found lots of bugs in > fbcon > and thus it's "often the solution to just delete code and remove features". > This is true, and the bugs - which actually affected all users of fbcon, > including DRM - were fixed, or code was dropped like e.g. the support for > software scrollback in vgacon (commit 973c096f6a85). > > So to further analyze which bugs were found by syzbot, I've looked through all > patches in drivers/video which were tagged with syzbot or syzkaller back to > year 2005. The vast majority fixed the reported issues on a higher level, e.g. > when screen is to be resized, or when font size is to be changed. The few ones > which touched driver code fixed a real driver bug, e.g. by adding a check. > > But NONE of those patches touched code of either the SCROLL_MOVE or the > SCROLL_REDRAW case. > > That means, there was no real reason why SCROLL_MOVE had to be ripped-out and > just SCROLL_REDRAW had to be used instead. The only reason I can imagine so > far > was that SCROLL_MOVE wasn't used by DRM and as such it was assumed that it > could go away. That argument completely missed the fact that SCROLL_MOVE is > still heavily used by fbdev (non-DRM) drivers. > > Some people mention that using memcpy() instead of the hardware acceleration > is > pretty much the same speed. But that's not true, at least not for older > graphic > cards and machines where we see speed decreases by factor 10 and more and thus > this change leads to console responsiveness way worse than before. > > That's why I propose to revert those patches, re-introduce hardware-based > scrolling acceleration and fix the performance-regression for fbdev drivers. > There isn't any impact on DRM when reverting those patches. > > Helge Deller (2): > Revert "fbdev: Garbage collect fbdev scrolling acceleration, part 1 > (from TODO list)" > Revert "fbcon: Disable accelerated scrolling" > > Documentation/gpu/todo.rst | 24 -- > drivers/video/fbdev/core/bitblit.c | 16 + > drivers/video/fbdev/core/fbcon.c| 540 +++- > drivers/video/fbdev/core/fbcon.h| 59 +++ > drivers/video/fbdev/core/fbcon_ccw.c| 28 +- > drivers/video/fbdev/core/fbcon_cw.c | 28 +- > drivers/video/fbdev/core/fbcon_rotate.h | 9 + > drivers/video/fbdev/core/fbcon_ud.c | 37 +- > drivers/video/fbdev/core/tileblit.c | 16 + > drivers/video/fbdev/skeletonfb.c| 12 +- > include/linux/fb.h | 2 +- > 11 files changed, 703 insertions(+), 68 deletions(-) Thanks Helge! Feel free to add my: Acked-by: Sven Schnelle
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
On Wed, Jan 19, 2022 at 02:01:44PM +0100, Sven Schnelle wrote: > Hi Greg, > > Greg Kroah-Hartman writes: > > > On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote: > >> On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote: > >> > This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151. > >> > > >> > Revert this patch. This patch started to introduce the regression that > >> > all hardware acceleration of more than 35 existing fbdev drivers were > >> > bypassed and thus fbcon console output for those was dramatically slowed > >> > down by factor of 10 and more. > >> > > >> > Reverting this commit has no impact on DRM, since none of the DRM > >> > drivers are > >> > tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA, > >> > FBINFO_HWACCEL_FILLRECT or others. > >> > > >> > Signed-off-by: Helge Deller > >> > Cc: sta...@vger.kernel.org # v5.16 > >> > >> Why just 5.16? This commit came in on 5.11 and was backported to > >> 5.10.5. > >> > >> As for "why", I think there was a number of private bugs that were > >> reported in this code, which is why it was removed. I do not think it > >> can be safely added back in without addressing them first. Let me go > >> dig through my email to see if I can find them... > > > > Ah, no, that was just the soft scrollback code I was thinking of, which > > was a different revert and is still gone, thankfully :) > > > > This one was just removed because Daniel noticed that only 3 drivers > > used this (nouveau, omapdrm, and gma600), so this shouldn't have caused > > any regressions in any other drivers like you are reporting here. > > I'm counting more than 3 drivers using this. I think one of the reasons > why it was reverted was that no one is actively maintaining fbdev. With > Helge now volunteering i don't see a reason why it should stay reverted. > If there are issues coming up i'm pretty sure Helge would care, and i > would probably also take a look. Ok, no objection from me, but I think Daniel should weigh in as it is his commit that is being reverted here. thanks, greg k-h
[PATCH 2/4] dma-buf: warn about dma_fence_array container rules
It's not allowed to nest another dma_fence container into a dma_fence_array or otherwise we can run into recursion. Warn about that when we create a dma_fence_array. Signed-off-by: Christian König --- drivers/dma-buf/dma-fence-array.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index 3e07f961e2f3..4bfbcb885bbc 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -176,6 +176,19 @@ struct dma_fence_array *dma_fence_array_create(int num_fences, array->base.error = PENDING_ERROR; + /* dma_fence_array objects should never contain any other fence +* containers or otherwise we run into recursion and potential kernel +* stack overflow on operations on the dma_fence_array. +* +* The correct way of handling this is to flatten out the array by the +* caller instead. +* +* Enforce this here by checking that we don't create a dma_fence_array +* with any container inside. +*/ + while (seqno--) + WARN_ON(dma_fence_is_container(fences[seqno])); + return array; } EXPORT_SYMBOL(dma_fence_array_create); -- 2.25.1
[PATCH 1/4] dma-buf: consolidate dma_fence subclass checking
Consolidate the wrapper functions to check for dma_fence
subclasses in the dma_fence header.
This makes it easier to document and also check the different
requirements for fence containers in the subclasses.
Signed-off-by: Christian König
---
include/linux/dma-fence-array.h | 15 +
include/linux/dma-fence-chain.h | 3 +--
include/linux/dma-fence.h | 38 +
3 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
index 303dd712220f..fec374f69e12 100644
--- a/include/linux/dma-fence-array.h
+++ b/include/linux/dma-fence-array.h
@@ -45,19 +45,6 @@ struct dma_fence_array {
struct irq_work work;
};
-extern const struct dma_fence_ops dma_fence_array_ops;
-
-/**
- * dma_fence_is_array - check if a fence is from the array subsclass
- * @fence: fence to test
- *
- * Return true if it is a dma_fence_array and false otherwise.
- */
-static inline bool dma_fence_is_array(struct dma_fence *fence)
-{
- return fence->ops == &dma_fence_array_ops;
-}
-
/**
* to_dma_fence_array - cast a fence to a dma_fence_array
* @fence: fence to cast to a dma_fence_array
@@ -68,7 +55,7 @@ static inline bool dma_fence_is_array(struct dma_fence *fence)
static inline struct dma_fence_array *
to_dma_fence_array(struct dma_fence *fence)
{
- if (fence->ops != &dma_fence_array_ops)
+ if (!fence || !dma_fence_is_array(fence))
return NULL;
return container_of(fence, struct dma_fence_array, base);
diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h
index 54fe3443fd2c..ee906b659694 100644
--- a/include/linux/dma-fence-chain.h
+++ b/include/linux/dma-fence-chain.h
@@ -49,7 +49,6 @@ struct dma_fence_chain {
spinlock_t lock;
};
-extern const struct dma_fence_ops dma_fence_chain_ops;
/**
* to_dma_fence_chain - cast a fence to a dma_fence_chain
@@ -61,7 +60,7 @@ extern const struct dma_fence_ops dma_fence_chain_ops;
static inline struct dma_fence_chain *
to_dma_fence_chain(struct dma_fence *fence)
{
- if (!fence || fence->ops != &dma_fence_chain_ops)
+ if (!fence || !dma_fence_is_chain(fence))
return NULL;
return container_of(fence, struct dma_fence_chain, base);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 1ea691753bd3..775cdc0b4f24 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -587,4 +587,42 @@ struct dma_fence *dma_fence_get_stub(void);
struct dma_fence *dma_fence_allocate_private_stub(void);
u64 dma_fence_context_alloc(unsigned num);
+extern const struct dma_fence_ops dma_fence_array_ops;
+extern const struct dma_fence_ops dma_fence_chain_ops;
+
+/**
+ * dma_fence_is_array - check if a fence is from the array subclass
+ * @fence: the fence to test
+ *
+ * Return true if it is a dma_fence_array and false otherwise.
+ */
+static inline bool dma_fence_is_array(struct dma_fence *fence)
+{
+ return fence->ops == &dma_fence_array_ops;
+}
+
+/**
+ * dma_fence_is_chain - check if a fence is from the chain subclass
+ * @fence: the fence to test
+ *
+ * Return true if it is a dma_fence_chain and false otherwise.
+ */
+static inline bool dma_fence_is_chain(struct dma_fence *fence)
+{
+ return fence->ops == &dma_fence_chain_ops;
+}
+
+/**
+ * dma_fence_is_container - check if a fence is a container for other fences
+ * @fence: the fence to test
+ *
+ * Return true if this fence is a container for other fences, false otherwise.
+ * This is important since we can't build up large fence structure or otherwise
+ * we run into recursion during operation on those fences.
+ */
+static inline bool dma_fence_is_container(struct dma_fence *fence)
+{
+ return dma_fence_is_array(fence) || dma_fence_is_chain(fence);
+}
+
#endif /* __LINUX_DMA_FENCE_H */
--
2.25.1
[PATCH 4/4] dma-buf: warn about containers in dma_resv object
Drivers should not add containers as shared fences to the dma_resv object, instead each fence should be added individually. Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 6dd9a40b55d4..e8a0c1d51da2 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -256,6 +256,11 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) dma_resv_assert_held(obj); + /* Drivers should not add containers here, instead add each fence +* individually. +*/ + WARN_ON(dma_fence_is_container(fence)); + fobj = dma_resv_shared_list(obj); count = fobj->shared_count; -- 2.25.1
[PATCH 3/4] dma-buf: Warn about dma_fence_chain container rules
Chaining of dma_fence_chain objects is only allowed through the prev fence and not through the contained fence. Warn about that when we create a dma_fence_chain. Signed-off-by: Christian König --- drivers/dma-buf/dma-fence-chain.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c index 1b4cb3e5cec9..fa33f6b7f77b 100644 --- a/drivers/dma-buf/dma-fence-chain.c +++ b/drivers/dma-buf/dma-fence-chain.c @@ -254,5 +254,13 @@ void dma_fence_chain_init(struct dma_fence_chain *chain, dma_fence_init(&chain->base, &dma_fence_chain_ops, &chain->lock, context, seqno); + + /* Chaining dma_fence_chain container together is only allowed through +* the prev fence and not through the contained fence. +* +* The correct way of handling this is to flatten out the fence +* structure into a dma_fence_array by the caller instead. +*/ + WARN_ON(dma_fence_is_chain(fence)); } EXPORT_SYMBOL(dma_fence_chain_init); -- 2.25.1
heads-up for fbdev pull request
This is just a heads-up, that I'm planning to send a pull request for fbdev fixes to Linus later today. Nothing really important - just fixes for various fbdev drivers. It has been a few days in for-next and no DRM parts are touched. Changelog is below, and it can be pulled from: http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-5.17-2 Helge Chunyang Zhong (1): video: ocfb: add const to of_device_id Colin Ian King (2): fbdev: aty128fb: make some arrays static const video: fbdev: mb862xx: remove redundant assignment to pointer ptr Greg Kroah-Hartman (1): omapfb: use default_groups in kobj_type Jiasheng Jiang (1): video: fbdev: Check for null res pointer Luca Weiss (2): backlight: qcom-wled: Add PM6150L compatible dt-bindings: simple-framebuffer: allow standalone compatible Michael Kelley (1): video: hyperv_fb: Fix validation of screen resolution Minghao Chi (1): drivers/video: remove redundant res variable Xu Wang (2): backlight: lm3630a_bl: Remove redundant 'flush_workqueue()' calls fbdev: omap2: omapfb: Remove redundant 'flush_workqueue()' calls Yang Guang (1): video: fbdev: use swap() to make code cleaner YueHaibing (1): video: fbdev: controlfb: Fix COMPILE_TEST build Z. Liu (1): matroxfb: set maxvram of vbG200eW to the same as vbG200 to avoid black screen .../devicetree/bindings/display/simple-framebuffer.yaml | 12 +++- drivers/video/backlight/lm3630a_bl.c | 1 - drivers/video/backlight/qcom-wled.c | 1 + drivers/video/fbdev/aty/aty128fb.c | 10 ++ drivers/video/fbdev/aty/mach64_ct.c | 4 +--- drivers/video/fbdev/controlfb.c | 2 ++ drivers/video/fbdev/hyperv_fb.c | 16 +++- drivers/video/fbdev/imxfb.c | 2 ++ drivers/video/fbdev/matrox/matroxfb_base.c | 2 +- drivers/video/fbdev/mb862xx/mb862xxfb_accel.c| 2 +- drivers/video/fbdev/ocfb.c | 2 +- drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c | 3 ++- drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c | 3 ++- drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c | 3 ++- drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 1 - drivers/video/fbdev/sis/sis_main.c | 2 +- 16 files changed, 32 insertions(+), 34 deletions(-)
Re: [PATCH] drm/vmwgfx: Stop requesting the pci regions
Hi Zack
Am 19.01.22 um 10:13 schrieb Thomas Zimmermann:
Hi
Am 19.01.22 um 03:15 schrieb Zack Rusin:
On Tue, 2022-01-18 at 20:00 +0100, Javier Martinez Canillas wrote:
Hello Zack,
On 1/17/22 19:03, Zack Rusin wrote:
From: Zack Rusin
When sysfb_simple is enabled loading vmwgfx fails because the regions
are held by the platform. In that case
remove_conflicting*_framebuffers
only removes the simplefb but not the regions held by sysfb.
Indeed, that's an issue. I wonder if we should drop the IORESOURCE_BUSY
flag from the memory resource added to the "simple-framebuffer" device
?
I think this is one of those cases where it depends on what we plan to
do after that. Sementically it makes sense to have it in there - the
framebuffer memory is claimed by the "simple-framebuffer" and it's
busy, it's just that it creates issues for drivers after unloading. I
think removing it, while making things easier for drivers, would be
confusing for people reading the code later, unless there's some kind
of cleanup that would happen with it (e.g. removing IORESOURCE_BUSY
altogether and making the drm drivers properly request their
resources). At least by itself it doesn't seem to be much better
solution than having the drm drivers not call pci_request_region[s],
which apart from hyperv and cirrus (iirc bochs does it for resources
other than fb which wouldn't have been claimed by "simple-framebuffer")
is already the case.
I do think we should do one of them to make the codebase coherent:
either remove IORESOURCE_BUSY from "simple-framebuffer" or remove
pci_request_region[s] from hyperv and cirrus.
I just discussed this a bit with Javier. It's a problem with the
simple-framebuffer code, rather then vmwgfx.
IMHO the best solution is to drop IORESOURCE_BUSY from sysfb and have
drivers register/release the range with _BUSY. That would signal the
memory belongs to the sysfb device but is not busy unless a driver has
been bound. After simplefb released the range, it should be 'non-busy'
again and available for vmwgfx. Simpledrm does a hot-unplug of the sysfb
device, so the memory range gets released entirely. If you want, I'll
prepare some patches for this scenario.
Attached is a patch that implements this. Doing
cat /proc/iomem
...
e000-efff : :00:02.0
e000-e07e8fff : BOOTFB
e000-e07e8fff : simplefb
...
shows the memory. 'BOOTFB' is the simple-framebuffer device and
'simplefb' is the driver. Only the latter uses _BUSY. Same for
simpledrm. Once the drivers have been unloaded, the BUSY flag is gone
and the memory canbe acquired by vmwgfx.
Zack, please test this patch. If it works, I'll send out the real patchset.
Best regards
Thomas
If this doesn't work, pushing all request/release pairs into drivers
would be my next option.
If none of this is feasible, we can still remove pci_request_region()
from vmwgfx.
Best regards
Thomas
Like the other drm drivers we need to stop requesting all the pci
regions
to let the driver load with platform code enabled.
This allows vmwgfx to load correctly on systems with sysfb_simple
enabled.
I read this very interesting thread from two years ago:
https://lkml.org/lkml/2020/11/5/248
Maybe is worth mentioning in the commit message what Daniel said there,
that is that only a few DRM drivers request explicitly the PCI regions
and the only reliable approach is for bus drivers to claim these.
Ah, great point. I'll update the commit log with that.
Signed-off-by: Zack Rusin
Fixes: 523375c943e5 ("drm/vmwgfx: Port vmwgfx to arm64")
Cc: dri-devel@lists.freedesktop.org
Cc:
Reviewed-by: Martin Krastev
---
The patch looks good to me, thanks a lot for fixing this:
Reviewed-by: Javier Martinez Canillas
Thanks for taking a look at this, I appreciate it!
z
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
From f5da496296f7dcaf8754e1b0446660b100539a73 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann
Date: Wed, 19 Jan 2022 10:23:07 +0100
Subject: [PATCH] [RFC] drop IORESOURCE_BUSY from sysfb code and request memory
in drivers
Drop the IORESOURCE_BUSY flag when creating a simple-framebuffer
device. Instead acquire the memory in drivers.
---
drivers/firmware/sysfb_simplefb.c | 2 +-
drivers/gpu/drm/tiny/simpledrm.c | 17 ++---
drivers/video/fbdev/simplefb.c| 57 ++-
3 files changed, 53 insertions(+), 23 deletions(-)
diff --git a/drivers/firmware/sysfb_simplefb.c b/drivers/firmware/sysfb_simplefb.c
index b86761904949..179e9d0ef3e9 100644
--- a/drivers/firmware/sysfb_simplefb.c
+++ b/drivers/firmware/sysfb_simplefb.c
@@ -99,7 +99,7 @@ __init int sysfb_create_simplefb(const struct screen_info *si,
/* setup IORESOURCE_MEM as framebuffer memory */
memset(&res, 0, sizeof(res));
- res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+ res.flags = IORESOURCE_ME
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
On Wed, Jan 19, 2022 at 2:29 PM Helge Deller wrote:
>
> >>
> >> Ah, no, that was just the soft scrollback code I was thinking of, which
>
> Right.
> That was commit 973c096f6a85 and it was about vgacon, not fbcon.
No, fbcon had some bug too, although I've paged out the details. See
commit 50145474f6ef ("fbcon: remove soft scrollback code").
If I remember correctly (and it's entirely possible that I don't), the
whole "softback_lines" logic had serious problems with resizing the
console (or maybe changing the font size).
There may have been some other bad interaction with
foreground/background consoles too, I forget.
Linus
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
On 1/19/22 15:01, Linus Torvalds wrote:
> On Wed, Jan 19, 2022 at 2:29 PM Helge Deller wrote:
>>
Ah, no, that was just the soft scrollback code I was thinking of, which
>>
>> Right.
>> That was commit 973c096f6a85 and it was about vgacon, not fbcon.
>
> No, fbcon had some bug too, although I've paged out the details. See
> commit 50145474f6ef ("fbcon: remove soft scrollback code").
Yes, but those bugs didn't affected
> If I remember correctly (and it's entirely possible that I don't), the
> whole "softback_lines" logic had serious problems with resizing the
> console (or maybe changing the font size).
Right.
I'm not asking to revert any soft scrollback functions.
It's about if you allow to use the fbdev-drivers hardware acceleration
to move parts of the screen or if you are stuck to software memcpy() and
repainting instead.
None of the bug reports touched that part.
> There may have been some other bad interaction with
> foreground/background consoles too, I forget.
I think that is independend if you use hardware acceleration or not.
Helge
Re: [PATCH 0/3] lib/string_helpers: Add a few string helpers
On Wed, 19 Jan 2022, Petr Mladek wrote: > On Tue 2022-01-18 23:24:47, Lucas De Marchi wrote: >> Add some helpers under lib/string_helpers.h so they can be used >> throughout the kernel. When I started doing this there were 2 other >> previous attempts I know of, not counting the iterations each of them >> had: >> >> 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nik...@intel.com/ >> 2) >> https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevche...@linux.intel.com/#t >> >> Going through the comments I tried to find some common ground and >> justification for what is in here, addressing some of the concerns >> raised. >> >> d. This doesn't bring onoff() helper as there are some places in the >>kernel with onoff as variable - another name is probably needed for >>this function in order not to shadow the variable, or those variables >>could be renamed. Or if people wanting >>try to find a short one > > I would call it str_on_off(). > > And I would actually suggest to use the same style also for > the other helpers. > > The "str_" prefix would make it clear that it is something with > string. There are other _on_off() that affect some > functionality, e.g. mute_led_on_off(), e1000_vlan_filter_on_off(). > > The dash '_' would significantly help to parse the name. yesno() and > onoff() are nicely short and kind of acceptable. But "enabledisable()" > is a puzzle. > > IMHO, str_yes_no(), str_on_off(), str_enable_disable() are a good > compromise. > > The main motivation should be code readability. You write the > code once. But many people will read it many times. Open coding > is sometimes better than misleading macro names. > > That said, I do not want to block this patchset. If others like > it... ;-) I don't mind the names either way. Adding the prefix and dashes is helpful in that it's possible to add the functions first and convert users at leisure, though with a bunch of churn, while using names that collide with existing ones requires the changes to happen in one go. What I do mind is grinding this series to a halt once again. I sent a handful of versions of this three years ago, with inconclusive bikeshedding back and forth, eventually threw my hands up in disgust, and walked away. > > >> e. One alternative to all of this suggested by Christian König >>(43456ba7-c372-84cc-4949-dcb817188...@amd.com) would be to add a >>printk format. But besides the comment, he also seemed to like >>the common function. This brought the argument from others that the >>simple yesno()/enabledisable() already used in the code is easier to >>remember and use than e.g. %py[DOY] > > Thanks for not going this way :-) > >> Last patch also has some additional conversion of open coded cases. I >> preferred starting with drm/ since this is "closer to home". >> >> I hope this is a good summary of the previous attempts and a way we can >> move forward. >> >> Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my >> proposal is to take first 2 patches either through mm tree or maybe >> vsprintf. Last patch can be taken later through drm. > > I agree with Andy that it should go via drm tree. It would make it > easier to handle potential conflicts. > > Just in case, you decide to go with str_yes_no() or something similar. > Mass changes are typically done at the end on the merge window. > The best solution is when it can be done by a script. > > Best Regards, > Petr -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm/vmwgfx: Stop requesting the pci regions
On Wed, 2022-01-19 at 10:13 +0100, Thomas Zimmermann wrote: > Hi > > Am 19.01.22 um 03:15 schrieb Zack Rusin: > > On Tue, 2022-01-18 at 20:00 +0100, Javier Martinez Canillas wrote: > > > Hello Zack, > > > > > > On 1/17/22 19:03, Zack Rusin wrote: > > > > From: Zack Rusin > > > > > > > > When sysfb_simple is enabled loading vmwgfx fails because the > > > > regions > > > > are held by the platform. In that case > > > > remove_conflicting*_framebuffers > > > > only removes the simplefb but not the regions held by sysfb. > > > > > > > > > > Indeed, that's an issue. I wonder if we should drop the > > > IORESOURCE_BUSY > > > flag from the memory resource added to the "simple-framebuffer" > > > device > > > ? > > > > I think this is one of those cases where it depends on what we plan > > to > > do after that. Sementically it makes sense to have it in there - > > the > > framebuffer memory is claimed by the "simple-framebuffer" and it's > > busy, it's just that it creates issues for drivers after unloading. > > I > > think removing it, while making things easier for drivers, would be > > confusing for people reading the code later, unless there's some > > kind > > of cleanup that would happen with it (e.g. removing IORESOURCE_BUSY > > altogether and making the drm drivers properly request their > > resources). At least by itself it doesn't seem to be much better > > solution than having the drm drivers not call > > pci_request_region[s], > > which apart from hyperv and cirrus (iirc bochs does it for > > resources > > other than fb which wouldn't have been claimed by "simple- > > framebuffer") > > is already the case. > > > > I do think we should do one of them to make the codebase coherent: > > either remove IORESOURCE_BUSY from "simple-framebuffer" or remove > > pci_request_region[s] from hyperv and cirrus. > > I just discussed this a bit with Javier. It's a problem with the > simple-framebuffer code, rather then vmwgfx. > > IMHO the best solution is to drop IORESOURCE_BUSY from sysfb and have > drivers register/release the range with _BUSY. That would signal the > memory belongs to the sysfb device but is not busy unless a driver > has > been bound. After simplefb released the range, it should be 'non- > busy' > again and available for vmwgfx. Simpledrm does a hot-unplug of the > sysfb > device, so the memory range gets released entirely. If you want, I'll > prepare some patches for this scenario. > > If this doesn't work, pushing all request/release pairs into drivers > would be my next option. > > If none of this is feasible, we can still remove pci_request_region() > from vmwgfx. I think that's orthogonal to the fix because having pci_request_region makes vmwgfx behave differently from majority of DRM drivers, e.g. on systems with sysfb enabled with 5.15 vmwgfx fails to boot and leaves the system broken without any fb driver (because while we have *remove_conflicting*_framebuffers we don't have drm_restore_system_fb or such to load back the boot fb after drm driver load fails) but since it's one of the few drivers which does request regions it took a bit for us to notice. So in this case I'd much rather be like the other drivers rather than correct because it lowers the odds of vmwgfx breaking in the future. z
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds
wrote:
>
> On Wed, Jan 19, 2022 at 2:29 PM Helge Deller wrote:
> >
> > >>
> > >> Ah, no, that was just the soft scrollback code I was thinking of, which
> >
> > Right.
> > That was commit 973c096f6a85 and it was about vgacon, not fbcon.
>
> No, fbcon had some bug too, although I've paged out the details. See
> commit 50145474f6ef ("fbcon: remove soft scrollback code").
tbh I've paged it all out too.
> If I remember correctly (and it's entirely possible that I don't), the
> whole "softback_lines" logic had serious problems with resizing the
> console (or maybe changing the font size).
Yeah that pile of reverts was my motiviation to look into this and see
what else we could rip out most likely and still have an fbcon that
works as well as it does right now for almost all users (which is not
so great, but oh well).
> There may have been some other bad interaction with
> foreground/background consoles too, I forget.
Irrespective of this code being buggy or not buggy I think the bigger
pictures, and really the reason I want to see as much code ditched
from the fbdev/fbcon stack as we possible can, are very clear:
- it's full of bugs
- there's no test coverage/CI to speak of
- it's very arcane code which is damn hard to understand and fix issues within
- the locking is busted (largely thanks to console_lock, and the
effort to make that reasonable from -rt folks has been slowly creeping
forward for years).
Iow this subsystem is firmly stuck in the 90s, and I think it's best
to just leave it there. There's also not been anyone actually capable
and willing to put in the work to change this (pretty much all actual
changes/fixes have been done by drm folks anyway, like me having a
small pet project to make the fbdev vs fbcon locking slightly less
busted).
The other side is that being a maintainer is about collaboration, and
this entire fbdev maintainership takeover has been a demonstration of
anything but that. MAINTAINERS entry was a bit confusing since defacto
drm has been maintaining it for years, but for the above reasons we've
done that by just aggressively deleting stuff that isn't absolutely
needed - hence why I figured "orphaned" is a reasonable description of
the state of things. This entire affair of rushing in a maintainer
change over the w/e and then being greeted by a lot of wtf mails next
Monday does leave a rather sour aftertaste. Plus that thread shows a
lot of misunderstandings of what's all been going on and what drm can
and cannot do by Helge, which doesn't improve the entire "we need
fbdev back" argument.
But if the overall consensus is that that fbdev needs to be brought
back to it's full 90s glory then I think we need a copy of that code
for drm drivers (should work out if we intercept fb_open() and put our
own file_ops in there, maybe some more fun with fbcon), so that at
least for anything modern using drm driver we can keep on maintaining
that compat support code.
And with maintaining here I don't mean build a museum around it, but
actually try to keep/move the thing towards a state where we can still
tell distros that enabling it is an ok thing to do and not just a CVE
subscription (well it is that too right now, but at least we can fix a
lot of them by just deleting code).
I think until that mess is sorted out resurrecting code that's not
strictly needed is just not a bright idea.
Also wrt the issue at hand of "fbcon scrolling": The way to actually
do that with some speed is to render into a fully cached shadow buffer
and upload changed areas with a timer. Not with hw accelerated
scrolling, at least not if we just don't have full scale development
teams for each driver because creating 2d accel that doesn't suck is
really hard. drm fbdev compat helpers give you that shadow buffer for
free (well you got to set some options).
Unfortunately just ditching fbdev/fbcon compat is not an option for
many distros still, althought things are very slowly moving towards
that. Until we've arrived there I can't just pretend to not care about
what's going on in drivers/video.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH] drm/vmwgfx: Stop requesting the pci regions
Hi Am 19.01.22 um 15:24 schrieb Zack Rusin: On Wed, 2022-01-19 at 10:13 +0100, Thomas Zimmermann wrote: Hi Am 19.01.22 um 03:15 schrieb Zack Rusin: On Tue, 2022-01-18 at 20:00 +0100, Javier Martinez Canillas wrote: Hello Zack, On 1/17/22 19:03, Zack Rusin wrote: From: Zack Rusin When sysfb_simple is enabled loading vmwgfx fails because the regions are held by the platform. In that case remove_conflicting*_framebuffers only removes the simplefb but not the regions held by sysfb. Indeed, that's an issue. I wonder if we should drop the IORESOURCE_BUSY flag from the memory resource added to the "simple-framebuffer" device ? I think this is one of those cases where it depends on what we plan to do after that. Sementically it makes sense to have it in there - the framebuffer memory is claimed by the "simple-framebuffer" and it's busy, it's just that it creates issues for drivers after unloading. I think removing it, while making things easier for drivers, would be confusing for people reading the code later, unless there's some kind of cleanup that would happen with it (e.g. removing IORESOURCE_BUSY altogether and making the drm drivers properly request their resources). At least by itself it doesn't seem to be much better solution than having the drm drivers not call pci_request_region[s], which apart from hyperv and cirrus (iirc bochs does it for resources other than fb which wouldn't have been claimed by "simple- framebuffer") is already the case. I do think we should do one of them to make the codebase coherent: either remove IORESOURCE_BUSY from "simple-framebuffer" or remove pci_request_region[s] from hyperv and cirrus. I just discussed this a bit with Javier. It's a problem with the simple-framebuffer code, rather then vmwgfx. IMHO the best solution is to drop IORESOURCE_BUSY from sysfb and have drivers register/release the range with _BUSY. That would signal the memory belongs to the sysfb device but is not busy unless a driver has been bound. After simplefb released the range, it should be 'non- busy' again and available for vmwgfx. Simpledrm does a hot-unplug of the sysfb device, so the memory range gets released entirely. If you want, I'll prepare some patches for this scenario. If this doesn't work, pushing all request/release pairs into drivers would be my next option. If none of this is feasible, we can still remove pci_request_region() from vmwgfx. I think that's orthogonal to the fix because having pci_request_region makes vmwgfx behave differently from majority of DRM drivers, e.g. on systems with sysfb enabled with 5.15 vmwgfx fails to boot and leaves the system broken without any fb driver (because while we have *remove_conflicting*_framebuffers we don't have drm_restore_system_fb or such to load back the boot fb after drm driver load fails) but since it's one of the few drivers which does request regions it took a bit for us to notice. So in this case I'd much rather be like the other drivers rather than correct because it lowers the odds of vmwgfx breaking in the future. Well, if you want to remove the calls then do so, of course. I'd rather add the request_memory() calls to all the other drivers that are missing them. Javier suggested to make this an official TODO item. Having the BUSY flag set when a driver is active, still is the correct thing to do. I'm not sure i understand your comment about drm_restore_system_fb. There is no way of atomically switching drivers and that's always been a problem. Failing at pci_request_memroy() is just one of many possible reasons for the switch to fail. The best you could do is to rearrange the code to do 'remove_conflicting*()' at the latest point possible. Best regards Thomas z -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v2] mgag200 fix memmapsl configuration in GCTL6 register
Hi Am 19.01.22 um 13:21 schrieb Thomas Zimmermann: Appears to be working on my test machine. But please rune scripts/checkpatch.pl on the patch before sending it. I get several errors WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #98: 0: Enables alpha mode, and the character generator addressing system is ERROR: trailing whitespace #149: FILE: drivers/gpu/drm/mgag200/mgag200_mode.c:532: +^I/* GCTL6 should be 0x05, but we configure memmapsl to 0xb8000 (text mode),^M$ ERROR: trailing whitespace #150: FILE: drivers/gpu/drm/mgag200/mgag200_mode.c:533: +^I * so that it doesn't hang when running kexec/kdump on G200_SE rev42.^M$ Thanks a lot, the patch has been merge now. These problems might have been caused by my email client. Best regards Thomas Best regards Thomas WREG_GFX(7, 0x0f); WREG_GFX(8, 0x0f); -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 1/3] lib/string_helpers: Consolidate yesno() implementation
On Wed, 19 Jan 2022 11:15:08 +0200
Andy Shevchenko wrote:
> > +static inline const char *yesno(bool v) { return v ? "yes" : "no"; }
>
>
>
> Perhaps keep it on 4 lines? Yes, yes/no is short, but if we add others
> (enable/disable) it will not be possible to keep on one line. And hence
> style will be broken among similar functions.
Agreed. Functions should always be of the normal format:
type func(params)
{
body;
}
Unless it is a stub function.
type func(params) { return 0; }
-- Steve
Re: [PATCH] dma-buf: cma_heap: Fix mutex locking section
On Tue, Jan 04, 2022 at 03:35:45PM +0800, Weizhao Ouyang wrote:
> Fix cma_heap_buffer mutex locking critical section to protect vmap_cnt
> and vaddr.
>
> Fixes: a5d2d29e24be ("dma-buf: heaps: Move heap-helper logic into the
> cma_heap implementation")
> Signed-off-by: Weizhao Ouyang
> ---
> drivers/dma-buf/heaps/cma_heap.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma-buf/heaps/cma_heap.c
> b/drivers/dma-buf/heaps/cma_heap.c
> index 0c05b79870f9..83f02bd51dda 100644
> --- a/drivers/dma-buf/heaps/cma_heap.c
> +++ b/drivers/dma-buf/heaps/cma_heap.c
> @@ -124,10 +124,11 @@ static int cma_heap_dma_buf_begin_cpu_access(struct
> dma_buf *dmabuf,
> struct cma_heap_buffer *buffer = dmabuf->priv;
> struct dma_heap_attachment *a;
>
> + mutex_lock(&buffer->lock);
> +
> if (buffer->vmap_cnt)
> invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
Since this creates nesting with mm/, but optionally I think it'd be good
to prime lockdep so it knows about this. See e.g. dma_resv_lockdep() in
dma-resv.c, except I don't know offhand what the right lock for
invalidate_kernel_vmap_range is.
-Daniel
>
> - mutex_lock(&buffer->lock);
> list_for_each_entry(a, &buffer->attachments, list) {
> if (!a->mapped)
> continue;
> @@ -144,10 +145,11 @@ static int cma_heap_dma_buf_end_cpu_access(struct
> dma_buf *dmabuf,
> struct cma_heap_buffer *buffer = dmabuf->priv;
> struct dma_heap_attachment *a;
>
> + mutex_lock(&buffer->lock);
> +
> if (buffer->vmap_cnt)
> flush_kernel_vmap_range(buffer->vaddr, buffer->len);
>
> - mutex_lock(&buffer->lock);
> list_for_each_entry(a, &buffer->attachments, list) {
> if (!a->mapped)
> continue;
> --
> 2.32.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
Daniel Vetter writes: > On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds > wrote: > Irrespective of this code being buggy or not buggy I think the bigger > pictures, and really the reason I want to see as much code ditched > from the fbdev/fbcon stack as we possible can, are very clear: > > - it's full of bugs > - there's no test coverage/CI to speak of > - it's very arcane code which is damn hard to understand and fix issues within > - the locking is busted (largely thanks to console_lock, and the > effort to make that reasonable from -rt folks has been slowly creeping > forward for years). > > Iow this subsystem is firmly stuck in the 90s, and I think it's best > to just leave it there. There's also not been anyone actually capable > and willing to put in the work to change this (pretty much all actual > changes/fixes have been done by drm folks anyway, like me having a > small pet project to make the fbdev vs fbcon locking slightly less > busted). Saying it's stuck in the 90ies, and actively trying to prevent Helge from taking over maintainership at the same time looks odd. I think Helge should at least get a chance to fix the issues. If the state is still the same in a year or so it should be discussed again. > The other side is that being a maintainer is about collaboration, and > this entire fbdev maintainership takeover has been a demonstration of > anything but that. MAINTAINERS entry was a bit confusing since defacto > drm has been maintaining it for years. It was marked as 'Orphaned'. Anyone is free to send a Patch/PR to take over maintainership. If you have strong opinions about that code (And you obviously have reading your mail, set it to 'maintained' and care about it. Everything else is just wrong in my opinion. /Sven
Re: [PATCH 1/3] lib/string_helpers: Consolidate yesno() implementation
On Wed, 19 Jan 2022 11:18:59 +0200 Sakari Ailus wrote: > On Tue, Jan 18, 2022 at 11:24:48PM -0800, Lucas De Marchi wrote: > > @@ -1354,8 +1345,7 @@ static bool tomoyo_print_condition(struct > > tomoyo_io_buffer *head, > > case 3: > > if (cond->grant_log != TOMOYO_GRANTLOG_AUTO) > > tomoyo_io_printf(head, " grant_log=%s", > > -tomoyo_yesno(cond->grant_log == > > - TOMOYO_GRANTLOG_YES)); > > +yesno(cond->grant_log == > > TOMOYO_GRANTLOG_YES)); > > This would be better split on two lines. Really? Yuck! I thought the "max line size" guideline was going to grow to a 100, but I still see it as 80. But anyway... cond->grant_log == TOMOYO_GRANTLOG_YES is not readable at all. Not compared to cond->grant_log == TOMOYO_GRANTLOG_YES I say keep it one line! Reviewed-by: Steven Rostedt (Google) -- Steve > > Then, > > Reviewed-by: Sakari Ailus
Re: [PATCH v4 3/4] drm/bridge: anx7625: Support reading edid through aux channel
On Tue, 18 Jan 2022 at 10:20, Hsin-Yi Wang wrote:
>
> Support reading edid through aux channel if panel is connected to aux
> bus. Extend anx7625_aux_dpcd_trans() to implement aux transfer function:
>
> 1. panel is populated in devm_of_dp_aux_populate_ep_devices(), so move
>anx7625_parse_dt() after.
> 2. Use pm runtime autosuspend since aux transfer function is called
>multiple times when reading edid.
> 3. No-op if aux transfer length is 0.
>
> Signed-off-by: Hsin-Yi Wang
> Reviewed-by: Xin Ji
> ---
> v3->v4:
> rebase to latest drm-misc-next
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 120 ++
> drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
> 2 files changed, 103 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index b7e3373994b480..50b9c98277f0d7 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
drivers/gpu/drm/bridge/analogix/anx7625.c:27:10: fatal error:
drm/drm_dp_aux_bus.h: No such file or directory
27 | #include
drm/dp/rm_dp_aux_bus.h is probably the correct path.
> #include
> #include
> #include
> @@ -231,19 +232,23 @@ static int wait_aux_op_finish(struct anx7625_data *ctx)
> return 0;
> }
>
> -static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx, u8 op,
> - u32 address, u8 len, u8 *buf)
> +static int anx7625_aux_trans(struct anx7625_data *ctx, u8 op, u32 address,
> +u8 len, u8 *buf)
> {
> struct device *dev = &ctx->client->dev;
> int ret;
> u8 addrh, addrm, addrl;
> u8 cmd;
> + bool is_write = !(op & DP_AUX_I2C_READ);
>
> - if (len > MAX_DPCD_BUFFER_SIZE) {
> + if (len > DP_AUX_MAX_PAYLOAD_BYTES) {
> dev_err(dev, "exceed aux buffer len.\n");
> return -EINVAL;
> }
>
> + if (!len)
> + return len;
> +
> addrl = address & 0xFF;
> addrm = (address >> 8) & 0xFF;
> addrh = (address >> 16) & 0xFF;
> @@ -262,7 +267,7 @@ static int anx7625_aux_dpcd_trans(struct anx7625_data
> *ctx, u8 op,
> ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> AP_AUX_ADDR_19_16, addrh);
>
> - if (op == DP_AUX_NATIVE_WRITE)
> + if (is_write)
> ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p0_client,
>AP_AUX_BUFF_START, len, buf);
> /* Enable aux access */
> @@ -275,14 +280,14 @@ static int anx7625_aux_dpcd_trans(struct anx7625_data
> *ctx, u8 op,
> }
>
> ret = wait_aux_op_finish(ctx);
> - if (ret) {
> + if (ret < 0) {
> dev_err(dev, "aux IO error: wait aux op finish.\n");
> return ret;
> }
>
> /* Write done */
> - if (op == DP_AUX_NATIVE_WRITE)
> - return 0;
> + if (is_write)
> + return len;
>
> /* Read done, read out dpcd data */
> ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
> @@ -292,7 +297,7 @@ static int anx7625_aux_dpcd_trans(struct anx7625_data
> *ctx, u8 op,
> return -EIO;
> }
>
> - return 0;
> + return len;
> }
>
> static int anx7625_video_mute_control(struct anx7625_data *ctx,
> @@ -867,7 +872,7 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx)
> }
>
> /* Read downstream capability */
> - anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, &bcap);
> + anx7625_aux_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, &bcap);
> if (!(bcap & 0x01)) {
> pr_warn("downstream not support HDCP 1.4, cap(%x).\n", bcap);
> return 0;
> @@ -956,7 +961,7 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
> dev_dbg(dev, "notify downstream enter into standby\n");
> /* Downstream monitor enter into standby mode */
> data = 2;
> - ret |= anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_WRITE, 0x000600, 1,
> &data);
> + ret |= anx7625_aux_trans(ctx, DP_AUX_NATIVE_WRITE, 0x000600, 1,
> &data);
> if (ret < 0)
> DRM_DEV_ERROR(dev, "IO error : mute video fail\n");
>
> @@ -1655,11 +1660,56 @@ static int anx7625_parse_dt(struct device *dev,
> return 0;
> }
>
> +static bool anx7625_of_panel_on_aux_bus(struct device *dev)
> +{
> + struct device_node *bus, *panel;
> +
> + bus = of_get_child_by_name(dev->of_node, "aux-bus");
> + if (!bus)
> + return false;
> +
> + panel = of_get_child_by_name(bus, "panel");
> + of_node_put(bus);
> + if (!panel)
> + return false;
> + of_node_put(panel);
> +
> + return true;
> +}
> +
> static inline struct anx7
Re: [PATCH 6/7] drm: Document fdinfo format specification
On Thu, Jan 06, 2022 at 04:55:35PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Proposal to standardise the fdinfo text format as optionally output by DRM > drivers. > > Idea is that a simple but, well defined, spec will enable generic > userspace tools to be written while at the same time avoiding a more heavy > handed approach of adding a mid-layer to DRM. > > i915 implements a subset of the spec, everything apart from the memory > stats currently, and a matching intel_gpu_top tool exists. > > Open is to see if AMD can migrate to using the proposed GPU utilisation > key-value pairs, or if they are not workable to see whether to go > vendor specific, or if a standardised alternative can be found which is > workable for both drivers. > > Same for the memory utilisation key-value pairs proposal. > > v2: > * Update for removal of name and pid. > > v3: > * 'Drm-driver' tag will be obtained from struct drm_driver.name. (Daniel) > > Signed-off-by: Tvrtko Ursulin > Cc: David M Nieto > Cc: Christian König > Cc: Daniel Vetter > Cc: Daniel Stone > Cc: Chris Healy > Acked-by: Christian König I'm assuming this ack here and later on is a "amdgpu plans to use this too" kind of affair. Especially also in the lights of eventually using matching semantics for cgroups and everything else tied to gpu execution resource management. If not I'm mildly worried that we're creating fake-standard stuff here which cannot actually be used by anything resembling driver-agnostic userspace. -Daniel > --- > Documentation/gpu/drm-usage-stats.rst | 97 +++ > Documentation/gpu/index.rst | 1 + > 2 files changed, 98 insertions(+) > create mode 100644 Documentation/gpu/drm-usage-stats.rst > > diff --git a/Documentation/gpu/drm-usage-stats.rst > b/Documentation/gpu/drm-usage-stats.rst > new file mode 100644 > index ..c669026be244 > --- /dev/null > +++ b/Documentation/gpu/drm-usage-stats.rst > @@ -0,0 +1,97 @@ > +.. _drm-client-usage-stats: > + > +== > +DRM client usage stats > +== > + > +DRM drivers can choose to export partly standardised text output via the > +`fops->show_fdinfo()` as part of the driver specific file operations > registered > +in the `struct drm_driver` object registered with the DRM core. > + > +One purpose of this output is to enable writing as generic as practicaly > +feasible `top(1)` like userspace monitoring tools. > + > +Given the differences between various DRM drivers the specification of the > +output is split between common and driver specific parts. Having said that, > +wherever possible effort should still be made to standardise as much as > +possible. > + > +File format specification > += > + > +- File shall contain one key value pair per one line of text. > +- Colon character (`:`) must be used to delimit keys and values. > +- All keys shall be prefixed with `drm-`. > +- Whitespace between the delimiter and first non-whitespace character shall > be > + ignored when parsing. > +- Neither keys or values are allowed to contain whitespace characters. > +- Numerical key value pairs can end with optional unit string. > +- Data type of the value is fixed as defined in the specification. > + > +Key types > +- > + > +1. Mandatory, fully standardised. > +2. Optional, fully standardised. > +3. Driver specific. > + > +Data types > +-- > + > +- - Unsigned integer without defining the maximum value. > +- - String excluding any above defined reserved characters or > whitespace. > + > +Mandatory fully standardised keys > +- > + > +- drm-driver: > + > +String shall contain the name this driver registered as via the respective > +`struct drm_driver` data structure. > + > +Optional fully standardised keys > + > + > +- drm-pdev: > + > +For PCI devices this should contain the PCI slot address of the device in > +question. > + > +- drm-client-id: > + > +Unique value relating to the open DRM file descriptor used to distinguish > +duplicated and shared file descriptors. Conceptually the value should map 1:1 > +to the in kernel representation of `struct drm_file` instances. > + > +Uniqueness of the value shall be either globally unique, or unique within the > +scope of each device, in which case `drm-pdev` shall be present as well. > + > +Userspace should make sure to not double account any usage statistics by > using > +the above described criteria in order to associate data to individual > clients. > + > +- drm-engine-: ns > + > +GPUs usually contain multiple execution engines. Each shall be given a stable > +and unique name (str), with possible values documented in the driver specific > +documentation. > + > +Value shall be in specified time units which the respective GPU engine spent > +busy executing workloads belonging to this client. > + > +Values are not required to be constantly monotonic
Re: [PATCH RESEND] drm/doc: Fix TTM acronym
On Fri, Jan 07, 2022 at 07:02:30PM +0100, José Expósito wrote: > The TTM acronym is defined for the first time in the documentation as > "Translation Table Maps". Afterwards, "Translation Table Manager" is > used as definition. > > Fix the first definition to avoid confusion. > > Signed-off-by: José Expósito Applied to drm-misc-next, thanks for the patch. -Daniel > --- > Documentation/gpu/drm-mm.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > index e0538083a2c0..198bcc1affa1 100644 > --- a/Documentation/gpu/drm-mm.rst > +++ b/Documentation/gpu/drm-mm.rst > @@ -8,7 +8,7 @@ the very dynamic nature of many of that data, managing > graphics memory > efficiently is thus crucial for the graphics stack and plays a central > role in the DRM infrastructure. > > -The DRM core includes two memory managers, namely Translation Table Maps > +The DRM core includes two memory managers, namely Translation Table Manager > (TTM) and Graphics Execution Manager (GEM). TTM was the first DRM memory > manager to be developed and tried to be a one-size-fits-them all > solution. It provides a single userspace API to accommodate the need of > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] drm/vmwgfx: Stop requesting the pci regions
On Wed, 2022-01-19 at 15:00 +0100, Thomas Zimmermann wrote: > Hi Zack > > Am 19.01.22 um 10:13 schrieb Thomas Zimmermann: > > Hi > > > > Am 19.01.22 um 03:15 schrieb Zack Rusin: > > > On Tue, 2022-01-18 at 20:00 +0100, Javier Martinez Canillas wrote: > > > > Hello Zack, > > > > > > > > On 1/17/22 19:03, Zack Rusin wrote: > > > > > From: Zack Rusin > > > > > > > > > > When sysfb_simple is enabled loading vmwgfx fails because the > > > > > regions > > > > > are held by the platform. In that case > > > > > remove_conflicting*_framebuffers > > > > > only removes the simplefb but not the regions held by sysfb. > > > > > > > > > > > > > Indeed, that's an issue. I wonder if we should drop the > > > > IORESOURCE_BUSY > > > > flag from the memory resource added to the "simple-framebuffer" > > > > device > > > > ? > > > > > > I think this is one of those cases where it depends on what we plan > > > to > > > do after that. Sementically it makes sense to have it in there - > > > the > > > framebuffer memory is claimed by the "simple-framebuffer" and it's > > > busy, it's just that it creates issues for drivers after unloading. > > > I > > > think removing it, while making things easier for drivers, would be > > > confusing for people reading the code later, unless there's some > > > kind > > > of cleanup that would happen with it (e.g. removing IORESOURCE_BUSY > > > altogether and making the drm drivers properly request their > > > resources). At least by itself it doesn't seem to be much better > > > solution than having the drm drivers not call > > > pci_request_region[s], > > > which apart from hyperv and cirrus (iirc bochs does it for > > > resources > > > other than fb which wouldn't have been claimed by "simple- > > > framebuffer") > > > is already the case. > > > > > > I do think we should do one of them to make the codebase coherent: > > > either remove IORESOURCE_BUSY from "simple-framebuffer" or remove > > > pci_request_region[s] from hyperv and cirrus. > > > > I just discussed this a bit with Javier. It's a problem with the > > simple-framebuffer code, rather then vmwgfx. > > > > IMHO the best solution is to drop IORESOURCE_BUSY from sysfb and have > > drivers register/release the range with _BUSY. That would signal the > > memory belongs to the sysfb device but is not busy unless a driver > > has > > been bound. After simplefb released the range, it should be 'non- > > busy' > > again and available for vmwgfx. Simpledrm does a hot-unplug of the > > sysfb > > device, so the memory range gets released entirely. If you want, I'll > > prepare some patches for this scenario. > > Attached is a patch that implements this. Doing > > cat /proc/iomem > ... > e000-efff : :00:02.0 > > e000-e07e8fff : BOOTFB > > e000-e07e8fff : simplefb > > ... > > shows the memory. 'BOOTFB' is the simple-framebuffer device and > 'simplefb' is the driver. Only the latter uses _BUSY. Same for > and the memory canbe acquired by vmwgfx. > > Zack, please test this patch. If it works, I'll send out the real > patchset. Hmm, the patch looks good but it doesn't work. After boot: /proc/iomem 5000-7fff : pcie@0x4000 7800-7fff : :00:0f.0 7800-782f : BOOTFB and vmwgfx fails on pci_request_regions: kernel: fb0: switching to vmwgfx from simple kernel: Console: switching to colour dummy device 80x25 kernel: vmwgfx :00:0f.0: BAR 2: can't reserve [mem 0x7800- 0x7fff 64bit pref] kernel: vmwgfx: probe of :00:0f.0 failed with error -16 leaving the system without a fb driver. z
Re: [PATCH v2] drm/selftests/test-drm_dp_mst_helper: Fix memory leak in sideband_msg_req_encode_decode
On Sat, Jan 08, 2022 at 05:58:12PM +0100, José Expósito wrote:
> Avoid leaking the "out" variable if it is not possible to allocate
> the "txmsg" variable.
>
> Fixes: 09234b88ef55 ("drm/selftests/test-drm_dp_mst_helper: Move
> 'sideband_msg_req_encode_decode' onto the heap")
> Addresses-Coverity-ID: 1475685 ("Resource leak")
> Signed-off-by: José Expósito
>
> ---
>
> v2: Improve commit message
Applied to drm-misc-next, thanks.
-Daniel
> ---
> drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> index 6b4759ed6bfd..c491429f1a02 100644
> --- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> +++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> @@ -131,8 +131,10 @@ sideband_msg_req_encode_decode(struct
> drm_dp_sideband_msg_req_body *in)
> return false;
>
> txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> - if (!txmsg)
> + if (!txmsg) {
> + kfree(out);
> return false;
> + }
>
> drm_dp_encode_sideband_req(in, txmsg);
> ret = drm_dp_decode_sideband_req(txmsg, out);
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v5 1/4] drm/bridge: anx7625: send DPCD command to downstream
From: Xin Ji
Send DPCD command to downstream before anx7625 power down,
let downstream monitor enter into standby mode.
Signed-off-by: Xin Ji
Signed-off-by: Hsin-Yi Wang
---
v3->v4:
Use common DP_AUX_NATIVE_READ/WRITE
Previously in:
https://patchwork.kernel.org/project/dri-devel/patch/1f36f8bf0a48fb2bba17bacec23700e58c1d407d.1641891874.git@analogixsemi.com/
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 42 +++
drivers/gpu/drm/bridge/analogix/anx7625.h | 2 --
2 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 76662fce4ce61d..17b23940549a42 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -129,6 +129,23 @@ static int anx7625_reg_write(struct anx7625_data *ctx,
return ret;
}
+static int anx7625_reg_block_write(struct anx7625_data *ctx,
+ struct i2c_client *client,
+ u8 reg_addr, u8 len, u8 *buf)
+{
+ int ret;
+ struct device *dev = &client->dev;
+
+ i2c_access_workaround(ctx, client);
+
+ ret = i2c_smbus_write_i2c_block_data(client, reg_addr, len, buf);
+ if (ret < 0)
+ dev_err(dev, "write i2c block failed id=%x\n:%x",
+ client->addr, reg_addr);
+
+ return ret;
+}
+
static int anx7625_write_or(struct anx7625_data *ctx,
struct i2c_client *client,
u8 offset, u8 mask)
@@ -214,8 +231,8 @@ static int wait_aux_op_finish(struct anx7625_data *ctx)
return 0;
}
-static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
-u32 address, u8 len, u8 *buf)
+static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx, u8 op,
+ u32 address, u8 len, u8 *buf)
{
struct device *dev = &ctx->client->dev;
int ret;
@@ -231,8 +248,7 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
addrm = (address >> 8) & 0xFF;
addrh = (address >> 16) & 0xFF;
- cmd = DPCD_CMD(len, DPCD_READ);
- cmd = ((len - 1) << 4) | 0x09;
+ cmd = DPCD_CMD(len, op);
/* Set command and length */
ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
@@ -246,6 +262,9 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
AP_AUX_ADDR_19_16, addrh);
+ if (op == DP_AUX_NATIVE_WRITE)
+ ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p0_client,
+ AP_AUX_BUFF_START, len, buf);
/* Enable aux access */
ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client,
AP_AUX_CTRL_STATUS, AP_AUX_CTRL_OP_EN);
@@ -255,14 +274,17 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
return -EIO;
}
- usleep_range(2000, 2100);
-
ret = wait_aux_op_finish(ctx);
if (ret) {
dev_err(dev, "aux IO error: wait aux op finish.\n");
return ret;
}
+ /* Write done */
+ if (op == DP_AUX_NATIVE_WRITE)
+ return 0;
+
+ /* Read done, read out dpcd data */
ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
AP_AUX_BUFF_START, len, buf);
if (ret < 0) {
@@ -845,7 +867,7 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx)
}
/* Read downstream capability */
- anx7625_aux_dpcd_read(ctx, 0x68028, 1, &bcap);
+ anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, &bcap);
if (!(bcap & 0x01)) {
pr_warn("downstream not support HDCP 1.4, cap(%x).\n", bcap);
return 0;
@@ -918,6 +940,7 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
{
struct device *dev = &ctx->client->dev;
int ret;
+ u8 data;
DRM_DEV_DEBUG_DRIVER(dev, "stop dp output\n");
@@ -929,6 +952,11 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
ret |= anx7625_write_and(ctx, ctx->i2c.tx_p2_client, 0x08, 0x7f);
ret |= anx7625_video_mute_control(ctx, 1);
+
+ dev_dbg(dev, "notify downstream enter into standby\n");
+ /* Downstream monitor enter into standby mode */
+ data = 2;
+ ret |= anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_WRITE, 0x000600, 1,
&data);
if (ret < 0)
DRM_DEV_ERROR(dev, "IO error : mute video fail\n");
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 56165f5b254c14..64a8ab56529404 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -242,8 +242,6 @@
#define AP_AUX_COMMAND 0x27 /* com+len */
#define
[PATCH v5 2/4] drm/bridge: anx7625: Convert to use devm_kzalloc
Use devm_kzalloc instead of kzalloc and drop kfree(). Let the memory
handled by driver detach.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Xin Ji
---
v2->v3: remove kfree() in anx7625_i2c_remove().
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 17b23940549a42..b7e3373994b480 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2515,7 +2515,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
return -ENODEV;
}
- platform = kzalloc(sizeof(*platform), GFP_KERNEL);
+ platform = devm_kzalloc(dev, sizeof(*platform), GFP_KERNEL);
if (!platform) {
DRM_DEV_ERROR(dev, "fail to allocate driver data\n");
return -ENOMEM;
@@ -2527,7 +2527,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (ret) {
if (ret != -EPROBE_DEFER)
DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
- goto free_platform;
+ return ret;
}
platform->client = client;
@@ -2552,7 +2552,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (!platform->hdcp_workqueue) {
dev_err(dev, "fail to create work queue\n");
ret = -ENOMEM;
- goto free_platform;
+ return ret;
}
platform->pdata.intp_irq = client->irq;
@@ -2637,9 +2637,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (platform->hdcp_workqueue)
destroy_workqueue(platform->hdcp_workqueue);
-free_platform:
- kfree(platform);
-
return ret;
}
@@ -2666,7 +2663,6 @@ static int anx7625_i2c_remove(struct i2c_client *client)
if (platform->pdata.audio_en)
anx7625_unregister_audio(platform);
- kfree(platform);
return 0;
}
--
2.34.1.703.g22d0c6ccf7-goog
[PATCH v5 3/4] drm/bridge: anx7625: Support reading edid through aux channel
Support reading edid through aux channel if panel is connected to aux
bus. Extend anx7625_aux_dpcd_trans() to implement aux transfer function:
1. panel is populated in devm_of_dp_aux_populate_ep_devices(), so move
anx7625_parse_dt() after.
2. Use pm runtime autosuspend since aux transfer function is called
multiple times when reading edid.
3. No-op if aux transfer length is 0.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Xin Ji
---
v4->v5:
fix header and indent.
v3->v4:
rebase to latest drm-misc-next
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 120 ++
drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
2 files changed, 103 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index b7e3373994b480..a59a4f4d2c5b10 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -231,19 +232,23 @@ static int wait_aux_op_finish(struct anx7625_data *ctx)
return 0;
}
-static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx, u8 op,
- u32 address, u8 len, u8 *buf)
+static int anx7625_aux_trans(struct anx7625_data *ctx, u8 op, u32 address,
+u8 len, u8 *buf)
{
struct device *dev = &ctx->client->dev;
int ret;
u8 addrh, addrm, addrl;
u8 cmd;
+ bool is_write = !(op & DP_AUX_I2C_READ);
- if (len > MAX_DPCD_BUFFER_SIZE) {
+ if (len > DP_AUX_MAX_PAYLOAD_BYTES) {
dev_err(dev, "exceed aux buffer len.\n");
return -EINVAL;
}
+ if (!len)
+ return len;
+
addrl = address & 0xFF;
addrm = (address >> 8) & 0xFF;
addrh = (address >> 16) & 0xFF;
@@ -262,7 +267,7 @@ static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx,
u8 op,
ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
AP_AUX_ADDR_19_16, addrh);
- if (op == DP_AUX_NATIVE_WRITE)
+ if (is_write)
ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p0_client,
AP_AUX_BUFF_START, len, buf);
/* Enable aux access */
@@ -275,14 +280,14 @@ static int anx7625_aux_dpcd_trans(struct anx7625_data
*ctx, u8 op,
}
ret = wait_aux_op_finish(ctx);
- if (ret) {
+ if (ret < 0) {
dev_err(dev, "aux IO error: wait aux op finish.\n");
return ret;
}
/* Write done */
- if (op == DP_AUX_NATIVE_WRITE)
- return 0;
+ if (is_write)
+ return len;
/* Read done, read out dpcd data */
ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
@@ -292,7 +297,7 @@ static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx,
u8 op,
return -EIO;
}
- return 0;
+ return len;
}
static int anx7625_video_mute_control(struct anx7625_data *ctx,
@@ -867,7 +872,7 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx)
}
/* Read downstream capability */
- anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, &bcap);
+ anx7625_aux_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, &bcap);
if (!(bcap & 0x01)) {
pr_warn("downstream not support HDCP 1.4, cap(%x).\n", bcap);
return 0;
@@ -956,7 +961,7 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
dev_dbg(dev, "notify downstream enter into standby\n");
/* Downstream monitor enter into standby mode */
data = 2;
- ret |= anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_WRITE, 0x000600, 1,
&data);
+ ret |= anx7625_aux_trans(ctx, DP_AUX_NATIVE_WRITE, 0x000600, 1, &data);
if (ret < 0)
DRM_DEV_ERROR(dev, "IO error : mute video fail\n");
@@ -1655,11 +1660,56 @@ static int anx7625_parse_dt(struct device *dev,
return 0;
}
+static bool anx7625_of_panel_on_aux_bus(struct device *dev)
+{
+ struct device_node *bus, *panel;
+
+ bus = of_get_child_by_name(dev->of_node, "aux-bus");
+ if (!bus)
+ return false;
+
+ panel = of_get_child_by_name(bus, "panel");
+ of_node_put(bus);
+ if (!panel)
+ return false;
+ of_node_put(panel);
+
+ return true;
+}
+
static inline struct anx7625_data *bridge_to_anx7625(struct drm_bridge *bridge)
{
return container_of(bridge, struct anx7625_data, bridge);
}
+static ssize_t anx7625_aux_transfer(struct drm_dp_aux *aux,
+ struct drm_dp_aux_msg *msg)
+{
+ struct anx7625_data *ctx = container_of(aux, struct anx7625_data, aux);
+ struct device *dev = &ctx->client->dev;
+ u8 request = msg->request & ~DP_AUX_I2C_MOT;
+ int ret = 0;
[PATCH v5 4/4] dt-bindings: drm/bridge: anx7625: Add aux-bus node
List panel under aux-bus node if it's connected to anx7625's aux bus.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Xin Ji
---
.../display/bridge/analogix,anx7625.yaml| 17 +
1 file changed, 17 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index 1d3e88daca041a..0d38d6fe39830f 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -83,6 +83,9 @@ properties:
type: boolean
description: let the driver enable audio HDMI codec function or not.
+ aux-bus:
+$ref: /schemas/display/dp-aux-bus.yaml#
+
ports:
$ref: /schemas/graph.yaml#/properties/ports
@@ -167,5 +170,19 @@ examples:
};
};
};
+
+aux-bus {
+panel {
+compatible = "innolux,n125hce-gn1";
+power-supply = <&pp3300_disp_x>;
+backlight = <&backlight_lcd0>;
+
+port {
+panel_in: endpoint {
+remote-endpoint = <&anx7625_out>;
+};
+};
+};
+};
};
};
--
2.34.1.703.g22d0c6ccf7-goog
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
On 1/19/22 15:34, Daniel Vetter wrote:
> On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds
> wrote:
>>
>> On Wed, Jan 19, 2022 at 2:29 PM Helge Deller wrote:
>>>
>
> Ah, no, that was just the soft scrollback code I was thinking of, which
>>>
>>> Right.
>>> That was commit 973c096f6a85 and it was about vgacon, not fbcon.
>>
>> No, fbcon had some bug too, although I've paged out the details. See
>> commit 50145474f6ef ("fbcon: remove soft scrollback code").
>
> tbh I've paged it all out too.
>
>> If I remember correctly (and it's entirely possible that I don't), the
>> whole "softback_lines" logic had serious problems with resizing the
>> console (or maybe changing the font size).
>
> Yeah that pile of reverts was my motiviation to look into this and see
> what else we could rip out most likely and still have an fbcon that
> works as well as it does right now for almost all users (which is not
> so great, but oh well).
>
>> There may have been some other bad interaction with
>> foreground/background consoles too, I forget.
>
> Irrespective of this code being buggy or not buggy I think the bigger
> pictures, and really the reason I want to see as much code ditched
> from the fbdev/fbcon stack as we possible can, are very clear:
>
> - it's full of bugs
I'm sure that there are bugs. Not just in fbcon/fbdev.
Other than that, if there are bugs I'm sure they are independend
of the question if you use hardware acceleration or not.
> - there's no test coverage/CI to speak of
> - it's very arcane code which is damn hard to understand and fix issues within
> - the locking is busted (largely thanks to console_lock, and the
> effort to make that reasonable from -rt folks has been slowly creeping
> forward for years).
>
> Iow this subsystem is firmly stuck in the 90s, and I think it's best
> to just leave it there. There's also not been anyone actually capable
> and willing to put in the work to change this (pretty much all actual
> changes/fixes have been done by drm folks anyway, like me having a
> small pet project to make the fbdev vs fbcon locking slightly less
> busted).
Yes, drm folks fixed a lot of bugs in the generic fbcon code.
I think everyone is thankful for this.
> The other side is that being a maintainer is about collaboration, and
> this entire fbdev maintainership takeover has been a demonstration of
> anything but that. MAINTAINERS entry was a bit confusing since defacto
> drm has been maintaining it for years, but for the above reasons we've
> done that by just aggressively deleting stuff that isn't absolutely
> needed - hence why I figured "orphaned" is a reasonable description of
> the state of things. This entire affair of rushing in a maintainer
> change over the w/e and then being greeted by a lot of wtf mails next
> Monday does leave a rather sour aftertaste. Plus that thread shows a
> lot of misunderstandings of what's all been going on and what drm can
> and cannot do by Helge, which doesn't improve the entire "we need
> fbdev back" argument.
I'm happy to *really* maintain fbdev code & drivers.
Up to now only those parts which were still needed by drm (like fbcon)
were fixed & "maintained" by drm folks.
Nearly all other patches sent to the fbdev list were ignored and even new
submissions for fbdev drivers were denied.
Now in next step really important infrastructure for fbdev-drivers was
ripped out of fbcon, like suddenly denying fbdev-drivers to use hardware
acceleration.
According to the docs the next step would have been to drop even more
code from the fbdev drivers.
This is not what "maintain" really is about.
> But if the overall consensus is that that fbdev needs to be brought
> back to it's full 90s glory then I think we need a copy of that code
> for drm drivers (should work out if we intercept fb_open() and put our
> own file_ops in there, maybe some more fun with fbcon), so that at
> least for anything modern using drm driver we can keep on maintaining
> that compat support code.
It's not about to keep something alive or to stop future developments.
It's about fairness and not actively breaking other parts of the kernel
for no good reason.
> And with maintaining here I don't mean build a museum around it, but
> actually try to keep/move the thing towards a state where we can still
> tell distros that enabling it is an ok thing to do and not just a CVE
> subscription (well it is that too right now, but at least we can fix a
> lot of them by just deleting code).
>
> I think until that mess is sorted out resurrecting code that's not
> strictly needed is just not a bright idea.
That's wrong.
It's strictly needed by more than 35 fbdev drivers and as such
you introduced a regression for those.
> Also wrt the issue at hand of "fbcon scrolling": The way to actually
> do that with some speed is to render into a fully cached shadow buffer
> and upload changed areas with a timer. Not with hw accelerated
> scrolling, at least not if we just don't have full scale development
> tea
[PATCH v2 1/4] drm/msm/adreno: Add support for Adreno 8c Gen 3
Add support for "Adreno 8c Gen 3" gpu along with the necessary speedbin
support.
Signed-off-by: Akhil P Oommen
---
Changes in v2:
- Fix a bug in adreno_cmp_rev()
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 21 ++
drivers/gpu/drm/msm/adreno/adreno_device.c | 34 +++---
drivers/gpu/drm/msm/adreno/adreno_gpu.h| 10 +++--
3 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 51b8377..9268ce3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -10,7 +10,6 @@
#include
#include
-#include
#include
#define GPU_PAS_ID 13
@@ -1734,6 +1733,18 @@ static u32 a618_get_speed_bin(u32 fuse)
return UINT_MAX;
}
+static u32 adreno_7c3_get_speed_bin(u32 fuse)
+{
+ if (fuse == 0)
+ return 0;
+ else if (fuse == 117)
+ return 0;
+ else if (fuse == 190)
+ return 1;
+
+ return UINT_MAX;
+}
+
static u32 fuse_to_supp_hw(struct device *dev, struct adreno_rev rev, u32 fuse)
{
u32 val = UINT_MAX;
@@ -1741,6 +1752,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
adreno_rev rev, u32 fuse)
if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev))
val = a618_get_speed_bin(fuse);
+ if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
+ val = adreno_7c3_get_speed_bin(fuse);
+
if (val == UINT_MAX) {
DRM_DEV_ERROR(dev,
"missing support for speed-bin: %u. Some OPPs may not
be supported by hardware",
@@ -1753,11 +1767,10 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
adreno_rev rev, u32 fuse)
static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev)
{
- u32 supp_hw = UINT_MAX;
- u32 speedbin;
+ u32 speedbin, supp_hw = UINT_MAX;
int ret;
- ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", &speedbin);
+ ret = adreno_read_speedbin(dev, &speedbin);
/*
* -ENOENT means that the platform doesn't support speedbin which is
* fine
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 9300583..946f505 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -6,6 +6,7 @@
* Copyright (c) 2014,2017 The Linux Foundation. All rights reserved.
*/
+#include
#include "adreno_gpu.h"
bool hang_debug = false;
@@ -317,6 +318,17 @@ static const struct adreno_info gpulist[] = {
.zapfw = "a660_zap.mdt",
.hwcg = a660_hwcg,
}, {
+ .rev = ADRENO_REV_SKU(6, 3, 5, ANY_ID, 190),
+ .name = "Adreno 8c Gen 3",
+ .fw = {
+ [ADRENO_FW_SQE] = "a660_sqe.fw",
+ [ADRENO_FW_GMU] = "a660_gmu.bin",
+ },
+ .gmem = SZ_512K,
+ .inactive_period = DRM_MSM_INACTIVE_PERIOD,
+ .init = a6xx_gpu_init,
+ .hwcg = a660_hwcg,
+ }, {
.rev = ADRENO_REV(6, 3, 5, ANY_ID),
.name = "Adreno 7c Gen 3",
.fw = {
@@ -365,13 +377,19 @@ static inline bool _rev_match(uint8_t entry, uint8_t id)
return (entry == ANY_ID) || (entry == id);
}
+static inline bool _rev_match_sku(uint16_t entry, uint16_t id)
+{
+ return (entry == ANY_SKU) || (entry == id);
+}
+
bool adreno_cmp_rev(struct adreno_rev rev1, struct adreno_rev rev2)
{
return _rev_match(rev1.core, rev2.core) &&
_rev_match(rev1.major, rev2.major) &&
_rev_match(rev1.minor, rev2.minor) &&
- _rev_match(rev1.patchid, rev2.patchid);
+ _rev_match(rev1.patchid, rev2.patchid) &&
+ _rev_match_sku(rev1.sku, rev2.sku);
}
const struct adreno_info *adreno_info(struct adreno_rev rev)
@@ -445,12 +463,17 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
return gpu;
}
+int adreno_read_speedbin(struct device *dev, u32 *speedbin)
+{
+ return nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
+}
+
static int find_chipid(struct device *dev, struct adreno_rev *rev)
{
struct device_node *node = dev->of_node;
const char *compat;
int ret;
- u32 chipid;
+ u32 chipid, speedbin;
/* first search the compat strings for qcom,adreno-XYZ.W: */
ret = of_property_read_string_index(node, "compatible", 0, &compat);
@@ -466,7 +489,7 @@ static int find_chipid(struct device *dev, struct
adreno_rev *rev)
rev->minor = r;
rev->patchid = patch;
- return 0;
+ goto done;
}
}
@@ -486,6 +509,11 @@ static int find_chipid(struct device *dev, struct
[PATCH v2 2/4] arm64: dts: qcom: sc7280: Support gpu speedbin
Add the speedbin fuse and the required opps to support gpu sku.
Signed-off-by: Akhil P Oommen
---
(no changes since v1)
arch/arm64/boot/dts/qcom/sc7280.dtsi | 46
1 file changed, 46 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 365a2e0..f8fc8b8 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -605,6 +605,11 @@
power-domains = <&rpmhpd SC7280_MX>;
#address-cells = <1>;
#size-cells = <1>;
+
+ gpu_speed_bin: gpu_speed_bin@1e9 {
+ reg = <0x1e9 0x2>;
+ bits = <5 8>;
+ };
};
sdhc_1: sdhci@7c4000 {
@@ -1762,6 +1767,9 @@
interconnect-names = "gfx-mem";
#cooling-cells = <2>;
+ nvmem-cells = <&gpu_speed_bin>;
+ nvmem-cell-names = "speed_bin";
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
@@ -1769,18 +1777,56 @@
opp-hz = /bits/ 64 <31500>;
opp-level =
;
opp-peak-kBps = <1804000>;
+ opp-supported-hw = <0x03>;
};
opp-45000 {
opp-hz = /bits/ 64 <45000>;
opp-level = ;
opp-peak-kBps = <4068000>;
+ opp-supported-hw = <0x03>;
};
opp-55000 {
opp-hz = /bits/ 64 <55000>;
opp-level =
;
opp-peak-kBps = <6832000>;
+ opp-supported-hw = <0x03>;
+ };
+
+ opp-60800 {
+ opp-hz = /bits/ 64 <60800>;
+ opp-level =
;
+ opp-peak-kBps = <8368000>;
+ opp-supported-hw = <0x02>;
+ };
+
+ opp-7 {
+ opp-hz = /bits/ 64 <7>;
+ opp-level = ;
+ opp-peak-kBps = <8532000>;
+ opp-supported-hw = <0x02>;
+ };
+
+ opp-81200 {
+ opp-hz = /bits/ 64 <81200>;
+ opp-level =
;
+ opp-peak-kBps = <8532000>;
+ opp-supported-hw = <0x02>;
+ };
+
+ opp-84000 {
+ opp-hz = /bits/ 64 <84000>;
+ opp-level =
;
+ opp-peak-kBps = <8532000>;
+ opp-supported-hw = <0x02>;
+ };
+
+ opp-9 {
+ opp-hz = /bits/ 64 <9>;
+ opp-level =
;
+ opp-peak-kBps = <8532000>;
+ opp-supported-hw = <0x02>;
};
};
};
--
2.7.4
[PATCH v2 3/4] drm/msm/adreno: Expose speedbin to userspace
Expose speedbin through MSM_PARAM_CHIP_ID parameter to help userspace identify the sku. Signed-off-by: Akhil P Oommen --- Changes in v2: - Use SKU in chipid PARAM only in new targets (Rob) drivers/gpu/drm/msm/adreno/adreno_gpu.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index f33cfa4..807d9ff 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -242,10 +242,12 @@ int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value) *value = !adreno_is_a650_family(adreno_gpu) ? 0x10 : 0; return 0; case MSM_PARAM_CHIP_ID: - *value = adreno_gpu->rev.patchid | - (adreno_gpu->rev.minor << 8) | - (adreno_gpu->rev.major << 16) | - (adreno_gpu->rev.core << 24); + *value = (uint64_t) adreno_gpu->rev.patchid | + (uint64_t) (adreno_gpu->rev.minor << 8) | + (uint64_t) (adreno_gpu->rev.major << 16) | + (uint64_t) (adreno_gpu->rev.core << 24); + if (!adreno_gpu->info->revn) + *value |= ((uint64_t) adreno_gpu->rev.sku) << 32; return 0; case MSM_PARAM_MAX_FREQ: *value = adreno_gpu->base.fast_rate; -- 2.7.4
[PATCH v2 4/4] drm/msm/adreno: Update the name of 7c3 gpu
Update the name in the gpulist for 7c3 gpu as per the latest
recommendation.
Signed-off-by: Akhil P Oommen
---
(no changes since v1)
drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 946f505..bd4d6a1 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -330,7 +330,7 @@ static const struct adreno_info gpulist[] = {
.hwcg = a660_hwcg,
}, {
.rev = ADRENO_REV(6, 3, 5, ANY_ID),
- .name = "Adreno 7c Gen 3",
+ .name = "Adreno 7c+ Gen 3",
.fw = {
[ADRENO_FW_SQE] = "a660_sqe.fw",
[ADRENO_FW_GMU] = "a660_gmu.bin",
--
2.7.4
Re: [PATCH] dt-bindings: Improve phandle-array schemas
Hello Rob, On 1/19/22 2:50 AM, Rob Herring wrote: > The 'phandle-array' type is a bit ambiguous. It can be either just an > array of phandles or an array of phandles plus args. Many schemas for > phandle-array properties aren't clear in the schema which case applies > though the description usually describes it. > > The array of phandles case boils down to needing: > > items: > maxItems: 1 > > The phandle plus args cases should typically take this form: > > items: > - items: > - description: A phandle > - description: 1st arg cell > - description: 2nd arg cell > > With this change, some examples need updating so that the bracketing of > property values matches the schema. > > Cc: Damien Le Moal > Cc: Herbert Xu > Cc: "David S. Miller" > Cc: Chun-Kuang Hu > Cc: Philipp Zabel > Cc: Laurent Pinchart > Cc: Kieran Bingham > Cc: Vinod Koul > Cc: Georgi Djakov > Cc: Thomas Gleixner > Cc: Marc Zyngier > Cc: Joerg Roedel > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han > Cc: Pavel Machek > Cc: Mauro Carvalho Chehab > Cc: Krzysztof Kozlowski > Cc: Jakub Kicinski > Cc: Wolfgang Grandegger > Cc: Marc Kleine-Budde > Cc: Andrew Lunn > Cc: Vivien Didelot > Cc: Florian Fainelli > Cc: Vladimir Oltean > Cc: Kalle Valo > Cc: Viresh Kumar > Cc: Stephen Boyd > Cc: Kishon Vijay Abraham I > Cc: Linus Walleij > Cc: "Rafael J. Wysocki" > Cc: Kevin Hilman > Cc: Ulf Hansson > Cc: Sebastian Reichel > Cc: Mark Brown > Cc: Mathieu Poirier > Cc: Daniel Lezcano > Cc: Zhang Rui > Cc: Greg Kroah-Hartman > Cc: Thierry Reding > Cc: Jonathan Hunter > Cc: Sudeep Holla > Cc: Geert Uytterhoeven > Cc: linux-...@vger.kernel.org > Cc: linux-cry...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: dmaeng...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: io...@lists.linux-foundation.org > Cc: linux-l...@vger.kernel.org > Cc: linux-me...@vger.kernel.org > Cc: net...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: linux-wirel...@vger.kernel.org > Cc: linux-...@lists.infradead.org > Cc: linux-g...@vger.kernel.org > Cc: linux-ri...@lists.infradead.org > Cc: linux-remotep...@vger.kernel.org > Cc: alsa-de...@alsa-project.org > Cc: linux-...@vger.kernel.org > Signed-off-by: Rob Herring > --- [...] > .../bindings/remoteproc/st,stm32-rproc.yaml | 33 ++-- [...] > diff --git a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml > b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml > index b587c97c282b..be3d9b0e876b 100644 > --- a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml > +++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml > @@ -29,17 +29,22 @@ properties: > >st,syscfg-holdboot: > description: remote processor reset hold boot > - - Phandle of syscon block. > - - The offset of the hold boot setting register. > - - The field mask of the hold boot. > $ref: "/schemas/types.yaml#/definitions/phandle-array" > -maxItems: 1 > +items: > + - items: > + - description: Phandle of syscon block > + - description: The offset of the hold boot setting register > + - description: The field mask of the hold boot > >st,syscfg-tz: > description: >Reference to the system configuration which holds the RCC trust zone > mode > $ref: "/schemas/types.yaml#/definitions/phandle-array" > -maxItems: 1 > +items: > + - items: > + - description: Phandle of syscon block > + - description: FIXME > + - description: FIXME - description: The offset of the trust zone setting register - description: The field mask of the trust zone state > >interrupts: > description: Should contain the WWDG1 watchdog reset interrupt > @@ -93,20 +98,32 @@ properties: > $ref: "/schemas/types.yaml#/definitions/phandle-array" > description: | >Reference to the system configuration which holds the remote > -maxItems: 1 > +items: > + - items: > + - description: Phandle of syscon block > + - description: FIXME > + - description: FIXME - description: The offset of the power setting register - description: The field mask of the PDDS selection > >st,syscfg-m4-state: > $ref: "/schemas/types.yaml#/definitions/phandle-array" > description: | >Reference to the tamp register which exposes the Cortex-M4 state. > -maxItems: 1 > +items: > + - items: > + - description: Phandle of syscon block with the tamp register > + - description: FIXME > + - description: FIXME - description: The offset of the tamp register - description: The field mask of the Cortex-M4 state > >st,syscfg-rsc-tbl: > $ref: "/schemas/types.yaml#/definitions/phandle-array" > description: | >Reference to the tamp register which references the C
Re: [PATCH] dt-bindings: Improve phandle-array schemas
On Wed, Jan 19, 2022 at 4:35 AM Vladimir Oltean wrote: > > On Tue, Jan 18, 2022 at 07:50:38PM -0600, Rob Herring wrote: > > The 'phandle-array' type is a bit ambiguous. It can be either just an > > array of phandles or an array of phandles plus args. Many schemas for > > phandle-array properties aren't clear in the schema which case applies > > though the description usually describes it. > > > > The array of phandles case boils down to needing: > > > > items: > > maxItems: 1 > > > > The phandle plus args cases should typically take this form: > > > > items: > > - items: > > - description: A phandle > > - description: 1st arg cell > > - description: 2nd arg cell > > > > With this change, some examples need updating so that the bracketing of > > property values matches the schema. > > --- > (...) > > diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml > > b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml > > index 702df848a71d..c504feeec6db 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml > > +++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml > > @@ -34,6 +34,8 @@ properties: > >full routing information must be given, not just the one hop > >routes to neighbouring switches > > $ref: /schemas/types.yaml#/definitions/phandle-array > > +items: > > + maxItems: 1 > > > >ethernet: > > description: > > For better or worse, the mainline cases of this property all take the > form of: > > arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts > link = <&switch1port9 &switch2port9>; > link = <&switch1port10 &switch0port10>; > arch/arm/boot/dts/vf610-zii-dev-rev-b.dts > link = <&switch1port6 > &switch2port9>; > link = <&switch1port5 > &switch0port5>; > arch/arm/boot/dts/vf610-zii-scu4-aib.dts > link = <&switch1port10 > &switch3port10 > &switch2port10>; > link = <&switch3port10 > &switch2port10>; > link = <&switch1port9 > &switch0port10>; > > So not really an array of phandles. Either form is an array. The DT yaml encoding maintains the bracketing, so how the schema is defined matters. To some extent the tools will process the schema to support both forms of bracketing, but this has turned out to be fragile and just doesn't work for phandle arrays. I'm working on further changes that will get rid of the yaml encoded DT format and validate DTB files directly. These obviously have no bracketing and needing the DTS source files to change goes away. However, to be able to construct the internal format for validation, I do need the schemas to have more information on what exactly the phandle-array contains. Rob
Re: [PATCH] dt-bindings: Improve phandle-array schemas
On Wed, Jan 19, 2022 at 9:22 AM Arnaud POULIQUEN wrote: > > Hello Rob, > > On 1/19/22 2:50 AM, Rob Herring wrote: > > The 'phandle-array' type is a bit ambiguous. It can be either just an > > array of phandles or an array of phandles plus args. Many schemas for > > phandle-array properties aren't clear in the schema which case applies > > though the description usually describes it. > > > > The array of phandles case boils down to needing: > > > > items: > > maxItems: 1 > > > > The phandle plus args cases should typically take this form: > > > > items: > > - items: > > - description: A phandle > > - description: 1st arg cell > > - description: 2nd arg cell > > > > With this change, some examples need updating so that the bracketing of > > property values matches the schema. > > > > Cc: Damien Le Moal > > Cc: Herbert Xu > > Cc: "David S. Miller" > > Cc: Chun-Kuang Hu > > Cc: Philipp Zabel > > Cc: Laurent Pinchart > > Cc: Kieran Bingham > > Cc: Vinod Koul > > Cc: Georgi Djakov > > Cc: Thomas Gleixner > > Cc: Marc Zyngier > > Cc: Joerg Roedel > > Cc: Lee Jones > > Cc: Daniel Thompson > > Cc: Jingoo Han > > Cc: Pavel Machek > > Cc: Mauro Carvalho Chehab > > Cc: Krzysztof Kozlowski > > Cc: Jakub Kicinski > > Cc: Wolfgang Grandegger > > Cc: Marc Kleine-Budde > > Cc: Andrew Lunn > > Cc: Vivien Didelot > > Cc: Florian Fainelli > > Cc: Vladimir Oltean > > Cc: Kalle Valo > > Cc: Viresh Kumar > > Cc: Stephen Boyd > > Cc: Kishon Vijay Abraham I > > Cc: Linus Walleij > > Cc: "Rafael J. Wysocki" > > Cc: Kevin Hilman > > Cc: Ulf Hansson > > Cc: Sebastian Reichel > > Cc: Mark Brown > > Cc: Mathieu Poirier > > Cc: Daniel Lezcano > > Cc: Zhang Rui > > Cc: Greg Kroah-Hartman > > Cc: Thierry Reding > > Cc: Jonathan Hunter > > Cc: Sudeep Holla > > Cc: Geert Uytterhoeven > > Cc: linux-...@vger.kernel.org > > Cc: linux-cry...@vger.kernel.org > > Cc: dri-devel@lists.freedesktop.org > > Cc: dmaeng...@vger.kernel.org > > Cc: linux...@vger.kernel.org > > Cc: io...@lists.linux-foundation.org > > Cc: linux-l...@vger.kernel.org > > Cc: linux-me...@vger.kernel.org > > Cc: net...@vger.kernel.org > > Cc: linux-...@vger.kernel.org > > Cc: linux-wirel...@vger.kernel.org > > Cc: linux-...@lists.infradead.org > > Cc: linux-g...@vger.kernel.org > > Cc: linux-ri...@lists.infradead.org > > Cc: linux-remotep...@vger.kernel.org > > Cc: alsa-de...@alsa-project.org > > Cc: linux-...@vger.kernel.org > > Signed-off-by: Rob Herring > > --- > > [...] > > > .../bindings/remoteproc/st,stm32-rproc.yaml | 33 ++-- > > [...] > > > diff --git > > a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml > > b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml > > index b587c97c282b..be3d9b0e876b 100644 > > --- a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml > > +++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml > > @@ -29,17 +29,22 @@ properties: > > > >st,syscfg-holdboot: > > description: remote processor reset hold boot > > - - Phandle of syscon block. > > - - The offset of the hold boot setting register. > > - - The field mask of the hold boot. > > $ref: "/schemas/types.yaml#/definitions/phandle-array" > > -maxItems: 1 > > +items: > > + - items: > > + - description: Phandle of syscon block > > + - description: The offset of the hold boot setting register > > + - description: The field mask of the hold boot > > > >st,syscfg-tz: > > description: > >Reference to the system configuration which holds the RCC trust zone > > mode > > $ref: "/schemas/types.yaml#/definitions/phandle-array" > > -maxItems: 1 > > +items: > > + - items: > > + - description: Phandle of syscon block > > + - description: FIXME > > + - description: FIXME > > - description: The offset of the trust zone setting register > - description: The field mask of the trust zone state > > > > >interrupts: > > description: Should contain the WWDG1 watchdog reset interrupt > > @@ -93,20 +98,32 @@ properties: > > $ref: "/schemas/types.yaml#/definitions/phandle-array" > > description: | > >Reference to the system configuration which holds the remote > > -maxItems: 1 > > +items: > > + - items: > > + - description: Phandle of syscon block > > + - description: FIXME > > + - description: FIXME > > - description: The offset of the power setting register > - description: The field mask of the PDDS selection > > > > >st,syscfg-m4-state: > > $ref: "/schemas/types.yaml#/definitions/phandle-array" > > description: | > >Reference to the tamp register which exposes the Cortex-M4 state. > > -maxItems: 1 > > +items: > > + - items: > > + - description: Phandle of syscon block with the tamp register > > +
Re: [RESEND] drm: panel-orientation-quirks: Add quirk for the 1Netbook OneXPlayer
On Thu, Jan 13, 2022 at 08:06:20AM +0800, Raymond Jay Golo wrote:
> The 1Netbook OneXPlayer uses a panel which has been mounted
> 90 degrees rotated. Add a quirk for this.
>
> Signed-off-by: Raymond Jay Golo
Applied to drm-misc-next-fixes, should show pu in the merge window still
for -rc1.
-Daniel
> ---
> drivers/gpu/drm/drm_panel_orientation_quirks.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c
> b/drivers/gpu/drm/drm_panel_orientation_quirks.c
> index 042bb80383c9..b910978d3e48 100644
> --- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
> +++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
> @@ -115,6 +115,12 @@ static const struct drm_dmi_panel_orientation_data
> lcd1280x1920_rightside_up = {
> .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> };
>
> +static const struct drm_dmi_panel_orientation_data lcd1600x2560_leftside_up
> = {
> + .width = 1600,
> + .height = 2560,
> + .orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP,
> +};
> +
> static const struct dmi_system_id orientation_data[] = {
> { /* Acer One 10 (S1003) */
> .matches = {
> @@ -275,6 +281,12 @@ static const struct dmi_system_id orientation_data[] = {
> DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "Default string"),
> },
> .driver_data = (void *)&onegx1_pro,
> + }, {/* OneXPlayer */
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ONE-NETBOOK TECHNOLOGY CO.,
> LTD."),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "ONE XPLAYER"),
> + },
> + .driver_data = (void *)&lcd1600x2560_leftside_up,
> }, {/* Samsung GalaxyBook 10.6 */
> .matches = {
> DMI_EXACT_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO.,
> LTD."),
> --
> 2.34.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v5 4/4] dt-bindings: drm/bridge: anx7625: Add aux-bus node
Hey Hsin-Yi,
While I can review this patch, I don't have the authority to merge it
since it is outside the scope of my maintainership. Rob Herring,
Daniel Vetter or David Airlie would have to Ack this patch.
On Wed, 19 Jan 2022 at 16:18, Hsin-Yi Wang wrote:
>
> List panel under aux-bus node if it's connected to anx7625's aux bus.
>
> Signed-off-by: Hsin-Yi Wang
> Reviewed-by: Xin Ji
> ---
> .../display/bridge/analogix,anx7625.yaml| 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index 1d3e88daca041a..0d38d6fe39830f 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -83,6 +83,9 @@ properties:
> type: boolean
> description: let the driver enable audio HDMI codec function or not.
>
> + aux-bus:
> +$ref: /schemas/display/dp-aux-bus.yaml#
> +
>ports:
> $ref: /schemas/graph.yaml#/properties/ports
>
> @@ -167,5 +170,19 @@ examples:
> };
> };
> };
> +
> +aux-bus {
> +panel {
> +compatible = "innolux,n125hce-gn1";
> +power-supply = <&pp3300_disp_x>;
> +backlight = <&backlight_lcd0>;
> +
> +port {
> +panel_in: endpoint {
> +remote-endpoint = <&anx7625_out>;
> +};
> +};
> +};
> +};
> };
> };
> --
> 2.34.1.703.g22d0c6ccf7-goog
>
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
Hi Am 19.01.22 um 16:05 schrieb Sven Schnelle: Daniel Vetter writes: On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds wrote: Irrespective of this code being buggy or not buggy I think the bigger pictures, and really the reason I want to see as much code ditched from the fbdev/fbcon stack as we possible can, are very clear: - it's full of bugs - there's no test coverage/CI to speak of - it's very arcane code which is damn hard to understand and fix issues within - the locking is busted (largely thanks to console_lock, and the effort to make that reasonable from -rt folks has been slowly creeping forward for years). Iow this subsystem is firmly stuck in the 90s, and I think it's best to just leave it there. There's also not been anyone actually capable and willing to put in the work to change this (pretty much all actual changes/fixes have been done by drm folks anyway, like me having a small pet project to make the fbdev vs fbcon locking slightly less busted). Saying it's stuck in the 90ies, and actively trying to prevent Helge from taking over maintainership at the same time looks odd. The issues are in the design itself. It's impossible to model today's hardware and constraints with fbdev. It's impossible to change configuration in a reliable way (i.e., what DRM calls atomic). Fbdev mmaps plain video ram to userspace, which is one of the reasons why DRM's fbdev support is hard to improve. I think Helge should at least get a chance to fix the issues. If the state is still the same in a year or so it should be discussed again. You cannot fix that in 10yrs. The other side is that being a maintainer is about collaboration, and this entire fbdev maintainership takeover has been a demonstration of anything but that. MAINTAINERS entry was a bit confusing since defacto drm has been maintaining it for years. It was marked as 'Orphaned'. Anyone is free to send a Patch/PR to take over maintainership. If you have strong opinions about that code (And you obviously have reading your mail, set it to 'maintained' and care about it. Everything else is just wrong in my opinion. No, it's not wrong. Helge takes fbdev over the weekend, without noteworthy experience, and ignores advice from the people that have kept it alive over the past years. This isn't going to work in the long term. Best regards Thomas /Sven -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v5 1/4] drm/bridge: anx7625: send DPCD command to downstream
On Wed, 19 Jan 2022 at 16:17, Hsin-Yi Wang wrote:
>
> From: Xin Ji
>
> Send DPCD command to downstream before anx7625 power down,
> let downstream monitor enter into standby mode.
>
> Signed-off-by: Xin Ji
> Signed-off-by: Hsin-Yi Wang
Hsin-Yi: Can you supply a r-b tag to this patch if it looks good to you?
> ---
> v3->v4:
> Use common DP_AUX_NATIVE_READ/WRITE
>
> Previously in:
> https://patchwork.kernel.org/project/dri-devel/patch/1f36f8bf0a48fb2bba17bacec23700e58c1d407d.1641891874.git@analogixsemi.com/
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 42 +++
> drivers/gpu/drm/bridge/analogix/anx7625.h | 2 --
> 2 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 76662fce4ce61d..17b23940549a42 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -129,6 +129,23 @@ static int anx7625_reg_write(struct anx7625_data *ctx,
> return ret;
> }
>
> +static int anx7625_reg_block_write(struct anx7625_data *ctx,
> + struct i2c_client *client,
> + u8 reg_addr, u8 len, u8 *buf)
> +{
> + int ret;
> + struct device *dev = &client->dev;
> +
> + i2c_access_workaround(ctx, client);
> +
> + ret = i2c_smbus_write_i2c_block_data(client, reg_addr, len, buf);
> + if (ret < 0)
> + dev_err(dev, "write i2c block failed id=%x\n:%x",
> + client->addr, reg_addr);
> +
> + return ret;
> +}
> +
> static int anx7625_write_or(struct anx7625_data *ctx,
> struct i2c_client *client,
> u8 offset, u8 mask)
> @@ -214,8 +231,8 @@ static int wait_aux_op_finish(struct anx7625_data *ctx)
> return 0;
> }
>
> -static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
> -u32 address, u8 len, u8 *buf)
> +static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx, u8 op,
> + u32 address, u8 len, u8 *buf)
> {
> struct device *dev = &ctx->client->dev;
> int ret;
> @@ -231,8 +248,7 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
> addrm = (address >> 8) & 0xFF;
> addrh = (address >> 16) & 0xFF;
>
> - cmd = DPCD_CMD(len, DPCD_READ);
> - cmd = ((len - 1) << 4) | 0x09;
> + cmd = DPCD_CMD(len, op);
>
> /* Set command and length */
> ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> @@ -246,6 +262,9 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
> ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> AP_AUX_ADDR_19_16, addrh);
>
> + if (op == DP_AUX_NATIVE_WRITE)
> + ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p0_client,
> + AP_AUX_BUFF_START, len, buf);
> /* Enable aux access */
> ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client,
> AP_AUX_CTRL_STATUS, AP_AUX_CTRL_OP_EN);
> @@ -255,14 +274,17 @@ static int anx7625_aux_dpcd_read(struct anx7625_data
> *ctx,
> return -EIO;
> }
>
> - usleep_range(2000, 2100);
> -
> ret = wait_aux_op_finish(ctx);
> if (ret) {
> dev_err(dev, "aux IO error: wait aux op finish.\n");
> return ret;
> }
>
> + /* Write done */
> + if (op == DP_AUX_NATIVE_WRITE)
> + return 0;
> +
> + /* Read done, read out dpcd data */
> ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
> AP_AUX_BUFF_START, len, buf);
> if (ret < 0) {
> @@ -845,7 +867,7 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx)
> }
>
> /* Read downstream capability */
> - anx7625_aux_dpcd_read(ctx, 0x68028, 1, &bcap);
> + anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, &bcap);
> if (!(bcap & 0x01)) {
> pr_warn("downstream not support HDCP 1.4, cap(%x).\n", bcap);
> return 0;
> @@ -918,6 +940,7 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
> {
> struct device *dev = &ctx->client->dev;
> int ret;
> + u8 data;
>
> DRM_DEV_DEBUG_DRIVER(dev, "stop dp output\n");
>
> @@ -929,6 +952,11 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
> ret |= anx7625_write_and(ctx, ctx->i2c.tx_p2_client, 0x08, 0x7f);
>
> ret |= anx7625_video_mute_control(ctx, 1);
> +
> + dev_dbg(dev, "notify downstream enter into standby\n");
> + /* Downstream monitor enter into standby mode */
> + data = 2;
> + ret |= anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_WRITE, 0x000600, 1,
> &data);
> if (ret < 0)
> DRM_DEV_ER
Re: fbdev: Garbage collect fbdev scrolling acceleration
On Thu, Jan 13, 2022 at 10:46:03PM +0100, Sven Schnelle wrote:
> Helge Deller writes:
>
> > I may have missed some discussions, but I'm objecting against this patch:
> >
> > b3ec8cdf457e5 ("fbdev: Garbage collect fbdev scrolling acceleration,
> > part 1 (from TODO list)")
> >
> > Can we please (partly) revert it and restore the scrolling behaviour,
> > where fbcon uses fb_copyarea() to copy the screen contents instead of
> > redrawing the whole screen?
> >
> > I'm fine with dropping the ypan-functionality.
> >
> > Maybe on fast new x86 boxes the performance difference isn't huge,
> > but for all old systems, or when emulated in qemu, this makes
> > a big difference.
> >
> > Helge
>
> I second that. For most people, the framebuffer isn't important as
> they're mostly interested in getting to X11/wayland as fast as possible.
> But for systems like servers without X11 it's nice to have a fast
> console.
Fast console howto:
- shadow buffer in cached memory
- timer based upload of changed areas to the real framebuffer
This one is actually fast, instead of trying to use hw bltcopy and having
the most terrible fallback path if that's gone. Yes drm fbdev helpers has
this (but not enabled on most drivers because very, very few people care).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/bridge: Add missing pm_runtime_put_sync
pm_runtime_get_sync() will increase the rumtime PM counter
even it returns an error. Thus a pairing decrement is needed
to prevent refcount leak. Fix this by replacing this API with
pm_runtime_resume_and_get(), which will not change the runtime
PM counter on error. Besides, a matching decrement is needed
on the error handling path to keep the counter balanced.
Signed-off-by: Yongzhi Liu
---
drivers/gpu/drm/bridge/nwl-dsi.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
index 9282e61..e7dce5a 100644
--- a/drivers/gpu/drm/bridge/nwl-dsi.c
+++ b/drivers/gpu/drm/bridge/nwl-dsi.c
@@ -862,18 +862,19 @@ nwl_dsi_bridge_mode_set(struct drm_bridge *bridge,
memcpy(&dsi->mode, adjusted_mode, sizeof(dsi->mode));
drm_mode_debug_printmodeline(adjusted_mode);
- pm_runtime_get_sync(dev);
+ if (pm_runtime_resume_and_get(dev) < 0)
+ return;
if (clk_prepare_enable(dsi->lcdif_clk) < 0)
- return;
+ goto runtime_put;
if (clk_prepare_enable(dsi->core_clk) < 0)
- return;
+ goto runtime_put;
/* Step 1 from DSI reset-out instructions */
ret = reset_control_deassert(dsi->rst_pclk);
if (ret < 0) {
DRM_DEV_ERROR(dev, "Failed to deassert PCLK: %d\n", ret);
- return;
+ goto runtime_put;
}
/* Step 2 from DSI reset-out instructions */
@@ -883,13 +884,17 @@ nwl_dsi_bridge_mode_set(struct drm_bridge *bridge,
ret = reset_control_deassert(dsi->rst_esc);
if (ret < 0) {
DRM_DEV_ERROR(dev, "Failed to deassert ESC: %d\n", ret);
- return;
+ goto runtime_put;
}
ret = reset_control_deassert(dsi->rst_byte);
if (ret < 0) {
DRM_DEV_ERROR(dev, "Failed to deassert BYTE: %d\n", ret);
- return;
+ goto runtime_put;
}
+
+runtime_put:
+ pm_runtime_put_sync(dev);
+ return;
}
static void
--
2.7.4
Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"
On Wed, Jan 19, 2022 at 4:06 PM Sven Schnelle wrote: > > Daniel Vetter writes: > > > On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds > > wrote: > > Irrespective of this code being buggy or not buggy I think the bigger > > pictures, and really the reason I want to see as much code ditched > > from the fbdev/fbcon stack as we possible can, are very clear: > > > > - it's full of bugs > > - there's no test coverage/CI to speak of > > - it's very arcane code which is damn hard to understand and fix issues > > within > > - the locking is busted (largely thanks to console_lock, and the > > effort to make that reasonable from -rt folks has been slowly creeping > > forward for years). > > > > Iow this subsystem is firmly stuck in the 90s, and I think it's best > > to just leave it there. There's also not been anyone actually capable > > and willing to put in the work to change this (pretty much all actual > > changes/fixes have been done by drm folks anyway, like me having a > > small pet project to make the fbdev vs fbcon locking slightly less > > busted). > > Saying it's stuck in the 90ies, and actively trying to prevent > Helge from taking over maintainership at the same time looks odd. > I think Helge should at least get a chance to fix the issues. If the > state is still the same in a year or so it should be discussed again. You don't need maintainership to fix issues. You need to submit patches. If otoh you get the maintainership first to be able to cram in reverts without discussions, then it's very backwards. > > The other side is that being a maintainer is about collaboration, and > > this entire fbdev maintainership takeover has been a demonstration of > > anything but that. MAINTAINERS entry was a bit confusing since defacto > > drm has been maintaining it for years. > > It was marked as 'Orphaned'. Anyone is free to send a Patch/PR to take > over maintainership. If you have strong opinions about that code (And you > obviously have reading your mail, set it to 'maintained' and care about > it. Everything else is just wrong in my opinion. I already added dri-devel so anything we drastically change can be discussed first. If that's indeed not strong enough then yes I can whack in full maintainer entry with a bugfix-only status. But really I try to not create facts with just editing MAINTAINERS first and ask questions later, that's just not a great way to collaborate. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
