Re: (subset) [PATCH] drm/bridge: parade-ps8640: switch to devm_drm_of_get_bridge
On Mon, 28 Feb 2022 19:31:31 +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/bridge: nxp-ptn3460: switch to devm_drm_of_get_bridge
On Mon, 28 Feb 2022 19:26:00 +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/bridge: tc358775: switch to devm_drm_of_get_bridge
On Mon, 28 Feb 2022 19:35:37 +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/bridge: ti-sn65dsi83: switch to devm_drm_of_get_bridge
On Mon, 28 Feb 2022 19:37:24 +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/bridge: parade-ps8622: switch to devm_drm_of_get_bridge
On Mon, 28 Feb 2022 19:29:04 +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/bridge: tc358762: switch to devm_drm_of_get_bridge
On Mon, 28 Feb 2022 19:33:42 +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/bridge: parade-ps8622: switch to devm_drm_of_get_bridge
Hi Maxime, On Tue, Mar 8, 2022 at 4:51 PM Maxime Ripard wrote: > > On Mon, 28 Feb 2022 19:29:04 +0100, José Expósito wrote: > > The function "drm_of_find_panel_or_bridge" has been deprecated in > > favor of "devm_drm_of_get_bridge". > > > > Switch to the new function and reduce boilerplate. > > > > > > Applied to drm/drm-misc (drm-misc-next). Not sure whether it was intentionally or accidentally applied? the same patch has sent before this date and has sent the v3 a few hours ago. https://patchwork.kernel.org/project/dri-devel/patch/20211210174819.2250178-3-ja...@amarulasolutions.com/ Thanks, Jagan.
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
Hi Marek,
On Tue, Mar 8, 2022 at 4:00 PM Marek Vasut wrote:
>
> On 3/8/22 11:07, Jagan Teki wrote:
> > On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
> >>
> >> On 3/8/22 09:03, Jagan Teki wrote:
> >>
> >> Hi,
> >>
> >> [...]
> >>
> @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs
> chipone_bridge_funcs = {
> static int chipone_parse_dt(struct chipone *icn)
> {
> struct device *dev = icn->dev;
> + struct device_node *endpoint;
> struct drm_panel *panel;
> + int dsi_lanes;
> int ret;
>
> icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
> @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone *icn)
> return PTR_ERR(icn->enable_gpio);
> }
>
> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
> + dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
> + icn->host_node = of_graph_get_remote_port_parent(endpoint);
> + of_node_put(endpoint);
> +
> + if (!icn->host_node)
> + return -ENODEV;
> >>>
> >>> The non-ports-based OF graph returns a -19 example on the Allwinner
> >>> Display pipeline in R16 [1].
> >>>
> >>> We need to have a helper to return host_node for non-ports as I have
> >>> done it for drm_of_find_bridge.
> >>>
> >>> [1] https://patchwork.amarulasolutions.com/patch/1805/
> >>
> >> The link points to a patch marked "DO NOT MERGE", maybe that patch is
> >> missing the DSI host port@0 OF graph link ? Both port@0 and port@1 are
> >> required, see:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
> >>
> >> What is "non-ports-based OF graph" ?
> >>
> >> I don't see drm_of_find_bridge() in linux-next , what is that ?
> >
> > port@0 is optional as some of the DSI host OF-graph represent the
> > bridge or panel as child nodes instead of ports. (i think dt-binding
> > has to fix it to make port@0 optional)
>
> The current upstream DT binding document says:
>
> required:
>- port@0
>- port@1
>
> So port@0 is mandatory.
>
> So I don't think any change is needed to this patch ?
>
> > Example OF-graph is on the commit message.
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/drm_of.c?id=80253168dbfd256bca97cf7f13312863c5a7f2e5
>
> It seems the current upstream DT bindings only support DSI hosts which
> do provide an endpoint, because port@0 is required per DT binding
> document. If you want to support the other options as listed in the
> aforementioned commit, then you would have to extend this driver and its
> bindings, but that is out of scope of this series.
Your comments seem to be valid, thanks.
Jagan.
[Bug 215669] New: [drm:gfx_v10_0_priv_reg_irq [amdgpu]] *ERROR* Illegal register access in command stream
https://bugzilla.kernel.org/show_bug.cgi?id=215669 Bug ID: 215669 Summary: [drm:gfx_v10_0_priv_reg_irq [amdgpu]] *ERROR* Illegal register access in command stream Product: Drivers Version: 2.5 Kernel Version: 5.10.103-1-MANJARO x86_64 GNU/Linux Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: andreas.polna...@hotmail.com Regression: No Hardware specs: CPU: Intel(R) Core(TM) i7-4770K Motherboard Z97-S02 (MS-7821) GPU: Radeon RX 5500/5500M / Pro 5500M When playing Elden ring on Linux I have gotten following issue today: mar 08 12:31:46 Manjaro-desktop kernel: [drm:gfx_v10_0_priv_reg_irq [amdgpu]] *ERROR* Illegal register access in command stream mar 08 12:31:46 Manjaro-desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx_0.0.0 timeout, signaled seq=2590353, emitted seq=2590355 mar 08 12:31:46 Manjaro-desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process eldenring.exe pid 3576 thread eldenring.exe pid 3687 mar 08 12:31:50 Manjaro-desktop kernel: amdgpu :03:00.0: [drm:amdgpu_ring_test_helper [amdgpu]] *ERROR* ring kiq_2.1.0 test failed (-110) mar 08 12:31:50 Manjaro-desktop kernel: [drm:gfx_v10_0_hw_fini [amdgpu]] *ERROR* KGQ disable failed mar 08 12:31:51 Manjaro-desktop kernel: amdgpu :03:00.0: [drm:amdgpu_ring_test_helper [amdgpu]] *ERROR* ring kiq_2.1.0 test failed (-110) mar 08 12:31:51 Manjaro-desktop kernel: [drm:gfx_v10_0_hw_fini [amdgpu]] *ERROR* KCQ disable failed mar 08 12:31:51 Manjaro-desktop kernel: [drm:gfx_v10_0_hw_fini [amdgpu]] *ERROR* failed to halt cp gfx mar 08 12:31:53 Manjaro-desktop kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! mar 08 12:31:53 Manjaro-desktop kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! mar 08 12:31:53 Manjaro-desktop kernel: [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* got no status for stream a096b549 on acrtc23c89ab0 mar 08 12:32:04 Manjaro-desktop kernel: [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:66:crtc-0] flip_done timed out mar 08 12:32:04 Manjaro-desktop kernel: [drm:amdgpu_dm_atomic_check [amdgpu]] *ERROR* [CRTC:66:crtc-0] hw_done or flip_done timed out mar 08 12:32:04 Manjaro-desktop kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! mar 08 12:32:14 Manjaro-desktop kernel: [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:69:crtc-1] flip_done timed out The screen goes haywire with the colors and im unable to recover from this as the computer freezes, only way to recover is by forcefully rebooting the machine. -- 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 06/10] drm/gma500: Move GTT resume logic out of psb_gtt_init()
On Tue, Mar 8, 2022 at 9:48 AM Thomas Zimmermann wrote:
>
> Hi Sam and Patrik
>
> Am 07.03.22 um 22:02 schrieb Patrik Jakobsson:
> > On Mon, Mar 7, 2022 at 8:07 PM Sam Ravnborg wrote:
> >>
> >> Hi Thomas,
> >>
> >> One comment below.
> >>
> >> On Sun, Mar 06, 2022 at 09:36:15PM +0100, Thomas Zimmermann wrote:
> >>> The current implementation of psb_gtt_init() also does resume
> >>> handling. Move the resume code into its own helper.
> >>>
> >>> Signed-off-by: Thomas Zimmermann
> >>> ---
> >>> drivers/gpu/drm/gma500/gtt.c | 122 ++-
> >>> drivers/gpu/drm/gma500/gtt.h | 2 +-
> >>> drivers/gpu/drm/gma500/psb_drv.c | 2 +-
> >>> 3 files changed, 104 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
> >>> index acd50ee26b03..43ad3ec38c80 100644
> >>> --- a/drivers/gpu/drm/gma500/gtt.c
> >>> +++ b/drivers/gpu/drm/gma500/gtt.c
> >>> @@ -209,7 +209,7 @@ static void psb_gtt_populate_resources(struct
> >>> drm_psb_private *pdev)
> >>>drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored,
> >>> total, (size / 1024));
> >>> }
> >>>
> >>> -int psb_gtt_init(struct drm_device *dev, int resume)
> >>> +int psb_gtt_init(struct drm_device *dev)
> >>> {
> >>>struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> >>>struct pci_dev *pdev = to_pci_dev(dev->dev);
> >>> @@ -218,10 +218,8 @@ int psb_gtt_init(struct drm_device *dev, int resume)
> >>>struct psb_gtt *pg;
> >>>int ret = 0;
> >>>
> >>> - if (!resume) {
> >>> - mutex_init(&dev_priv->gtt_mutex);
> >>> - mutex_init(&dev_priv->mmap_mutex);
> >>> - }
> >>> + mutex_init(&dev_priv->gtt_mutex);
> >>> + mutex_init(&dev_priv->mmap_mutex);
> >>>
> >>>pg = &dev_priv->gtt;
> >>>
> >>> @@ -290,13 +288,6 @@ int psb_gtt_init(struct drm_device *dev, int resume)
> >>>dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
> >>>dev_priv->stolen_base, vram_stolen_size / 1024);
> >>>
> >>> - if (resume && (gtt_pages != pg->gtt_pages) &&
> >>> - (stolen_size != pg->stolen_size)) {
> >>> - dev_err(dev->dev, "GTT resume error.\n");
> >>> - ret = -EINVAL;
> >>> - goto out_err;
> >>> - }
> >>> -
> >>>pg->gtt_pages = gtt_pages;
> >>>pg->stolen_size = stolen_size;
> >>>dev_priv->vram_stolen_size = vram_stolen_size;
> >>> @@ -304,19 +295,14 @@ int psb_gtt_init(struct drm_device *dev, int resume)
> >>>/*
> >>> * Map the GTT and the stolen memory area
> >>> */
> >>> - if (!resume)
> >>> - dev_priv->gtt_map = ioremap(pg->gtt_phys_start,
> >>> - gtt_pages << PAGE_SHIFT);
> >>> + dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages <<
> >>> PAGE_SHIFT);
> >>>if (!dev_priv->gtt_map) {
> >>>dev_err(dev->dev, "Failure to map gtt.\n");
> >>>ret = -ENOMEM;
> >>>goto out_err;
> >>>}
> >>>
> >>> - if (!resume)
> >>> - dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base,
> >>> - stolen_size);
> >>> -
> >>> + dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base,
> >>> stolen_size);
> >>>if (!dev_priv->vram_addr) {
> >>>dev_err(dev->dev, "Failure to map stolen base.\n");
> >>>ret = -ENOMEM;
> >>> @@ -333,11 +319,107 @@ int psb_gtt_init(struct drm_device *dev, int
> >>> resume)
> >>>return ret;
> >>> }
> >>>
> >> The below is a lot of duplicated complex code.
> >> Can you add one more helper for this?
> >
>
> That makes a lot of sense.
>
> > I was thinking the same but figured it would be an easy follow up
> > patch. But sure, why not fix it here already.
> >
> > While at it, the gtt enable/disable code could be put in helpers as well.
>
> Patrik, do you know why the code re-reads all these values during
> resume? Do the values change? The driver never seems to do anything
> with the updated values. For example, it doesn't update the GTT mapping
> if gtt_phys_start changes. Can we remove all that code from the resume
> function?
In theory these values can change if you hibernate, make changes in
the bios and then resume. I think I've seen bioses that can change the
stolen size and/or aperture size. Not sure if that would also change
the value in eg. pge_ctl or the size of the gtt. I would have to do
some testing on real hardware to figure this out.
But as you say, the above scenario is already broken. For the time
being, can we perhaps just fail gracefully?
Re: [PATCH v1 1/1] staging: fbtft: Consider type of init sequence values in fbtft_init_display()
On Tue, Mar 08, 2022 at 11:46:32AM +0100, Greg Kroah-Hartman wrote: > On Tue, Mar 08, 2022 at 12:18:25PM +0200, Andy Shevchenko wrote: > > +Cc: Helge > > > > Maybe you can pick this up? > > > > On Fri, Mar 04, 2022 at 09:34:14PM +0200, Andy Shevchenko wrote: > > You sent this less than a week ago! Please relax, staging driver > patches are way down my list of priorities at the moment. Wait at least > 2 weeks before trying to get someone else to take patches for this > subsystem. > > relax... Ah, okay. Sorry, I was browsing my long mailbox and haven't paid attention on the date I had sent this one on. -- With Best Regards, Andy Shevchenko
Re: [PATCH V3 01/13] dt-bindings: display: bridge: icn6211: Document DSI data-lanes property
On Fri, Mar 04, 2022 at 01:24:56AM +0100, Marek Vasut wrote: > It is necessary to specify the number of connected/used DSI data lanes when > using the DSI input port of this bridge. Document the 'data-lanes' property > of the DSI input port. > > Signed-off-by: Marek Vasut > Cc: Jagan Teki > Cc: Maxime Ripard > Cc: Rob Herring > Cc: Robert Foss > Cc: Sam Ravnborg > Cc: Thomas Zimmermann > Cc: devicet...@vger.kernel.org > To: dri-devel@lists.freedesktop.org Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH V3 04/13] drm: bridge: icn6211: Add HS/VS/DE polarity handling
On Fri, Mar 04, 2022 at 01:24:59AM +0100, Marek Vasut wrote: > The driver currently hard-codes HS/VS polarity to active-low and DE to > active-high, which is not correct for a lot of supported DPI panels. > Add the missing mode flag handling for HS/VS/DE polarity. > > Signed-off-by: Marek Vasut > Cc: Jagan Teki > Cc: Maxime Ripard > Cc: Robert Foss > Cc: Sam Ravnborg > Cc: Thomas Zimmermann > To: dri-devel@lists.freedesktop.org Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH RFC libdrm 1/2] intel: Improve checks for big-endian
On 2022-03-07 20:53, Geert Uytterhoeven wrote:
- sparc64-linux-gnu-gcc does not define __BIG_ENDIAN__ or SPARC, but
does define __sparc__, hence replace the check for SPARC by a check
for __sparc__,
- powerpc{,64,64}-linux-gnu-gcc does not define __ppc__ or __ppc64__,
but does define __BIG_ENDIAN__.
powerpc64le-linux-gnu-gcc does not define __ppc__ or __ppc64__, but
does define __LITTLE_ENDIAN__.
Hence remove the checks for __ppc__ and __ppc64__.
- mips{,64}-linux-gnu{,abi64}-gcc does not define __BIG_ENDIAN__, but
does define __MIPSEB__, so add a check for the latter,
- m68k-linux-gnu-gcc does not define __BIG_ENDIAN__, but does define
__mc68000__, so add a check for the latter,
- hppa{,64}-linux-gnu-gcc defines __BIG_ENDIAN__, and thus should work
out-of-the-box.
FWIW there's also __ARM_BIG_ENDIAN for arm-* and aarch64-* targets in BE
mode, if you want to flesh out the list even more. In principle there's
also "__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__" which should supposedly be
consistent across platforms, but I don't know if that's even more of a
specific GCC-ism.
Robin.
Signed-off-by: Geert Uytterhoeven
---
Untested due to lack of hardware.
---
intel/uthash.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/intel/uthash.h b/intel/uthash.h
index 45d1f9fc12a1d6f9..0f5480be6e8ca033 100644
--- a/intel/uthash.h
+++ b/intel/uthash.h
@@ -648,7 +648,7 @@ do {
#define MUR_PLUS2_ALIGNED(p) (((unsigned long)p & 3UL) == 2UL)
#define MUR_PLUS3_ALIGNED(p) (((unsigned long)p & 3UL) == 3UL)
#define WP(p) ((uint32_t*)((unsigned long)(p) & ~3UL))
-#if (defined(__BIG_ENDIAN__) || defined(SPARC) || defined(__ppc__) ||
defined(__ppc64__))
+#if (defined(__BIG_ENDIAN__) || defined(__sparc__) || defined(__mc68000__) ||
defined(__MIPSEB__))
#define MUR_THREE_ONE(p) *WP(p))&0x00ff) << 8) | (((*(WP(p)+1))&0xff00)
>> 24))
#define MUR_TWO_TWO(p) *WP(p))&0x) <<16) | (((*(WP(p)+1))&0x)
>> 16))
#define MUR_ONE_THREE(p) *WP(p))&0x00ff) <<24) | (((*(WP(p)+1))&0xff00)
>> 8))
Re: [PATCH] drm/doc: pull in drm_buddy.c
On 09/02/2022 07:32, Christian König wrote: Am 08.02.22 um 16:12 schrieb Matthew Auld: Make sure we pull in the kernel-doc for this. Reported-by: Daniel Vetter Signed-off-by: Matthew Auld Cc: Arunpravin Cc: Christian König Reviewed-by: Christian König Thanks. Could you also push this? --- Documentation/gpu/drm-mm.rst | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index 198bcc1affa1..f32ccce5722d 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -466,6 +466,15 @@ DRM MM Range Allocator Function References .. kernel-doc:: drivers/gpu/drm/drm_mm.c :export: +DRM Buddy Allocator +=== + +DRM Buddy Function References +- + +.. kernel-doc:: drivers/gpu/drm/drm_buddy.c + :export: + DRM Cache Handling and Fast WC memcpy() ===
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
> On 3/8/22 11:07, Jagan Teki wrote:
> > On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
> > >
> > > On 3/8/22 09:03, Jagan Teki wrote:
> > >
> > > Hi,
> > >
> > > [...]
> > >
> > > > > @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs
> > > > > chipone_bridge_funcs = {
> > > > >static int chipone_parse_dt(struct chipone *icn)
> > > > >{
> > > > > struct device *dev = icn->dev;
> > > > > + struct device_node *endpoint;
> > > > > struct drm_panel *panel;
> > > > > + int dsi_lanes;
> > > > > int ret;
> > > > >
> > > > > icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
> > > > > @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone *icn)
> > > > > return PTR_ERR(icn->enable_gpio);
> > > > > }
> > > > >
> > > > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
> > > > > + dsi_lanes = of_property_count_u32_elems(endpoint,
> > > > > "data-lanes");
> > > > > + icn->host_node = of_graph_get_remote_port_parent(endpoint);
> > > > > + of_node_put(endpoint);
> > > > > +
> > > > > + if (!icn->host_node)
> > > > > + return -ENODEV;
> > > >
> > > > The non-ports-based OF graph returns a -19 example on the Allwinner
> > > > Display pipeline in R16 [1].
> > > >
> > > > We need to have a helper to return host_node for non-ports as I have
> > > > done it for drm_of_find_bridge.
> > > >
> > > > [1] https://patchwork.amarulasolutions.com/patch/1805/
> > >
> > > The link points to a patch marked "DO NOT MERGE", maybe that patch is
> > > missing the DSI host port@0 OF graph link ? Both port@0 and port@1 are
> > > required, see:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
> > >
> > > What is "non-ports-based OF graph" ?
> > >
> > > I don't see drm_of_find_bridge() in linux-next , what is that ?
> >
> > port@0 is optional as some of the DSI host OF-graph represent the
> > bridge or panel as child nodes instead of ports. (i think dt-binding
> > has to fix it to make port@0 optional)
>
> The current upstream DT binding document says:
>
> required:
> - port@0
> - port@1
>
> So port@0 is mandatory.
In the binding, sure, but fundamentally the DT excerpt Jagan provided is
correct. If the bridge supports DCS, there's no reason to use the OF
graph in the first place: the bridge node will be a child node of the
MIPI-DSI controller (and there's no obligation to use the OF-graph for a
MIPI-DSI controller).
I believe port@0 should be made optional (or downright removed if
MIPI-DCS in the only control bus).
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v1 02/10] drm/msm/a6xx: Send NMI to gmu when it is hung
Hi Akhil,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next]
[also build test WARNING on drm-intel/for-linux-next drm-tip/drm-tip
drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.17-rc7 next-20220308]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Akhil-P-Oommen/Support-for-GMU-coredump-and-some-related-improvements/20220303-013028
base: git://anongit.freedesktop.org/drm/drm drm-next
config: riscv-randconfig-r042-20220307
(https://download.01.org/0day-ci/archive/20220308/202203082018.ici00nvs-...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project
d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
#
https://github.com/0day-ci/linux/commit/23953efc645803299a93f178e9a32f2ae97dae39
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Akhil-P-Oommen/Support-for-GMU-coredump-and-some-related-improvements/20220303-013028
git checkout 23953efc645803299a93f178e9a32f2ae97dae39
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpu/drm/msm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:967:6: warning: no previous
>> prototype for function 'a6xx_get_gmu_state' [-Wmissing-prototypes]
void a6xx_get_gmu_state(struct msm_gpu *gpu, struct a6xx_gpu_state
*a6xx_state)
^
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:967:1: note: declare 'static' if
the function is not intended to be used outside of this translation unit
void a6xx_get_gmu_state(struct msm_gpu *gpu, struct a6xx_gpu_state
*a6xx_state)
^
static
1 warning generated.
vim +/a6xx_get_gmu_state +967 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
966
> 967 void a6xx_get_gmu_state(struct msm_gpu *gpu, struct a6xx_gpu_state
> *a6xx_state)
968 {
969 struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
970 struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
971 struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
972
973 if (gmu->hung)
974 a6xx_gmu_send_nmi(gmu);
975
976 a6xx_get_gmu_registers(gpu, a6xx_state);
977 }
978
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[PATCH v2 0/4] Ingenic DRM bridge_atomic_enable proposal
Hello, this is the v2 for my set of patches : - use dev_err_probe() instead of dev_err() in the newvision panel driver probe function - add bindings documentation for the Leadtek ltk035c5444t Cheers - Christophe Hello, this is a set of patches to allow the upstreaming of the NV3052C panel found in the Anbernic RG350M mips gaming handheld. It was never upstreamed so far due to a longstanding graphical bug, which I propose to solve by introducing ingenic_drm_bridge_atomic_enable in the drm driver so the CRTC can be enabled after the panel itself slept out, and not before as it used to. After the drm change, 2 of the existing panels have to be modified accordingly to introduce missing .enable and .disable in their code. Christophe Branchereau (4): drm/ingenic : add ingenic_drm_bridge_atomic_enable drm/panel: Add panel driver for NewVision NV3052C based LCDs drm/panel : innolux-ej030na and abt-y030xx067a : add .enable and .disable dt-bindings: display/panel: Add Leadtek ltk035c5444t .../panel/leadtek,ltk035c5444t-spi.yaml | 59 +++ drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 19 +- drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-abt-y030xx067a.c | 23 +- drivers/gpu/drm/panel/panel-innolux-ej030na.c | 31 +- .../gpu/drm/panel/panel-newvision-nv3052c.c | 497 ++ 7 files changed, 627 insertions(+), 12 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t-spi.yaml create mode 100644 drivers/gpu/drm/panel/panel-newvision-nv3052c.c -- 2.34.1
[PATCH v2 1/4] drm/ingenic : add ingenic_drm_bridge_atomic_enable
This allows the CRTC to be enabled after panels have slept out,
and before their display is turned on, solving a graphical bug
on the newvision nv3502c
Signed-off-by: Christophe Branchereau
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index dcf44cb00821..51512f41263e 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -226,6 +226,18 @@ static int ingenic_drm_update_pixclk(struct notifier_block
*nb,
}
}
+static void ingenic_drm_bridge_atomic_enable(struct drm_bridge *bridge,
+struct drm_bridge_state
*old_bridge_state)
+{
+ struct ingenic_drm *priv = drm_device_get_priv(bridge->dev);
+
+ regmap_write(priv->map, JZ_REG_LCD_STATE, 0);
+
+ regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
+ JZ_LCD_CTRL_ENABLE | JZ_LCD_CTRL_DISABLE,
+ JZ_LCD_CTRL_ENABLE);
+}
+
static void ingenic_drm_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
@@ -237,17 +249,11 @@ static void ingenic_drm_crtc_atomic_enable(struct
drm_crtc *crtc,
if (WARN_ON(IS_ERR(priv_state)))
return;
- regmap_write(priv->map, JZ_REG_LCD_STATE, 0);
-
/* Set addresses of our DMA descriptor chains */
next_id = priv_state->use_palette ? HWDESC_PALETTE : 0;
regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_addr(priv, next_id));
regmap_write(priv->map, JZ_REG_LCD_DA1, dma_hwdesc_addr(priv, 1));
- regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
- JZ_LCD_CTRL_ENABLE | JZ_LCD_CTRL_DISABLE,
- JZ_LCD_CTRL_ENABLE);
-
drm_crtc_vblank_on(crtc);
}
@@ -968,6 +974,7 @@ static const struct drm_encoder_helper_funcs
ingenic_drm_encoder_helper_funcs =
static const struct drm_bridge_funcs ingenic_drm_bridge_funcs = {
.attach = ingenic_drm_bridge_attach,
+ .atomic_enable = ingenic_drm_bridge_atomic_enable,
.atomic_check = ingenic_drm_bridge_atomic_check,
.atomic_reset = drm_atomic_helper_bridge_reset,
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
--
2.34.1
[PATCH v2 2/4] drm/panel: Add panel driver for NewVision NV3052C based LCDs
This driver supports the NewVision NV3052C based LCDs. Right now, it
only supports the LeadTek LTK035C5444T 2.4" 640x480 TFT LCD panel, which
can be found in the Anbernic RG-350M handheld console.
Signed-off-by: Christophe Branchereau
---
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile| 1 +
.../gpu/drm/panel/panel-newvision-nv3052c.c | 497 ++
3 files changed, 507 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-newvision-nv3052c.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index bb2e47229c68..40084f709789 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -283,6 +283,15 @@ config DRM_PANEL_NEC_NL8048HL11
panel (found on the Zoom2/3/3630 SDP boards). To compile this driver
as a module, choose M here.
+config DRM_PANEL_NEWVISION_NV3052C
+ tristate "NewVision NV3052C RGB/SPI panel"
+ depends on OF && SPI
+ depends on BACKLIGHT_CLASS_DEVICE
+ select DRM_MIPI_DBI
+ help
+ Say Y here if you want to enable support for the panels built
+ around the NewVision NV3052C display controller.
+
config DRM_PANEL_NOVATEK_NT35510
tristate "Novatek NT35510 RGB panel driver"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 5740911f637c..42a7ab54234b 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) +=
panel-leadtek-ltk500hd1829.o
obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
+obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35560) += panel-novatek-nt35560.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35950) += panel-novatek-nt35950.o
diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
new file mode 100644
index ..30ab7a9fc828
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -0,0 +1,497 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * NevVision NV3052C IPS LCD panel driver
+ *
+ * Copyright (C) 2020, Paul Cercueil
+ * Copyright (C) 2022, Christophe Branchereau
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+
+struct nv3052c_panel_info {
+ const struct drm_display_mode *display_modes;
+ unsigned int num_modes;
+ u16 width_mm, height_mm;
+ u32 bus_format, bus_flags;
+};
+
+struct nv3052c {
+ struct device *dev;
+ struct drm_panel panel;
+ struct mipi_dbi dbi;
+
+ const struct nv3052c_panel_info *panel_info;
+
+ struct regulator *supply;
+ struct gpio_desc *reset_gpio;
+};
+
+struct nv3052c_reg {
+ u8 cmd;
+ u8 val;
+};
+
+static const struct nv3052c_reg nv3052c_panel_regs[] = {
+ { 0xff, 0x30 },
+ { 0xff, 0x52 },
+ { 0xff, 0x01 },
+ { 0xe3, 0x00 },
+ { 0x40, 0x00 },
+ { 0x03, 0x40 },
+ { 0x04, 0x00 },
+ { 0x05, 0x03 },
+ { 0x08, 0x00 },
+ { 0x09, 0x07 },
+ { 0x0a, 0x01 },
+ { 0x0b, 0x32 },
+ { 0x0c, 0x32 },
+ { 0x0d, 0x0b },
+ { 0x0e, 0x00 },
+ { 0x23, 0xa0 },
+
+ { 0x24, 0x0c },
+ { 0x25, 0x06 },
+ { 0x26, 0x14 },
+ { 0x27, 0x14 },
+
+ { 0x38, 0xcc },
+ { 0x39, 0xd7 },
+ { 0x3a, 0x4a },
+
+ { 0x28, 0x40 },
+ { 0x29, 0x01 },
+ { 0x2a, 0xdf },
+ { 0x49, 0x3c },
+ { 0x91, 0x77 },
+ { 0x92, 0x77 },
+ { 0xa0, 0x55 },
+ { 0xa1, 0x50 },
+ { 0xa4, 0x9c },
+ { 0xa7, 0x02 },
+ { 0xa8, 0x01 },
+ { 0xa9, 0x01 },
+ { 0xaa, 0xfc },
+ { 0xab, 0x28 },
+ { 0xac, 0x06 },
+ { 0xad, 0x06 },
+ { 0xae, 0x06 },
+ { 0xaf, 0x03 },
+ { 0xb0, 0x08 },
+ { 0xb1, 0x26 },
+ { 0xb2, 0x28 },
+ { 0xb3, 0x28 },
+ { 0xb4, 0x33 },
+ { 0xb5, 0x08 },
+ { 0xb6, 0x26 },
+ { 0xb7, 0x08 },
+ { 0xb8, 0x26 },
+ { 0xf0, 0x00 },
+ { 0xf6, 0xc0 },
+
+ { 0xff, 0x30 },
+ { 0xff, 0x52 },
+ { 0xff, 0x02 },
+ { 0xb0, 0x0b },
+ { 0xb1, 0x16 },
+ { 0xb2, 0x17 },
+ { 0xb3, 0x2c },
+ { 0xb4, 0x32 },
+ { 0xb5, 0x3b },
+ { 0xb6, 0x29 },
+ { 0xb7, 0x40 },
+ { 0xb8, 0x0d },
+ { 0xb9, 0x05 },
+ { 0xba, 0x12 },
+ { 0xbb, 0x10 },
+ { 0xbc, 0x12 },
+ { 0xbd, 0x15 },
+ { 0xbe, 0x19 },
+ { 0xbf, 0x0e },
+ { 0xc0, 0x16 },
+ { 0xc1, 0x0a },
+ { 0xd0, 0x0c },
+ { 0xd1, 0x17 }
[PATCH v2 3/4] drm/panel : innolux-ej030na and abt-y030xx067a : add .enable and .disable
Following the introduction of bridge_atomic_enable in the ingenic
drm driver, the crtc is enabled between .prepare and .enable, if
it exists.
Add it so the backlight is only enabled after the crtc is, to avoid
graphical issues.
Signed-off-by: Christophe Branchereau
---
drivers/gpu/drm/panel/panel-abt-y030xx067a.c | 23 --
drivers/gpu/drm/panel/panel-innolux-ej030na.c | 31 ---
2 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-abt-y030xx067a.c
b/drivers/gpu/drm/panel/panel-abt-y030xx067a.c
index f043b484055b..b5736344e3ec 100644
--- a/drivers/gpu/drm/panel/panel-abt-y030xx067a.c
+++ b/drivers/gpu/drm/panel/panel-abt-y030xx067a.c
@@ -183,8 +183,6 @@ static int y030xx067a_prepare(struct drm_panel *panel)
goto err_disable_regulator;
}
- msleep(120);
-
return 0;
err_disable_regulator:
@@ -202,6 +200,25 @@ static int y030xx067a_unprepare(struct drm_panel *panel)
return 0;
}
+static int y030xx067a_enable(struct drm_panel *panel)
+{
+ if (panel->backlight) {
+ /* Wait for the picture to be ready before enabling backlight */
+ msleep(120);
+ }
+
+ return 0;
+}
+
+static int y030xx067a_disable(struct drm_panel *panel)
+{
+ struct y030xx067a *priv = to_y030xx067a(panel);
+
+ regmap_clear_bits(priv->map, 0x06, REG06_XPSAVE);
+
+ return 0;
+}
+
static int y030xx067a_get_modes(struct drm_panel *panel,
struct drm_connector *connector)
{
@@ -239,6 +256,8 @@ static int y030xx067a_get_modes(struct drm_panel *panel,
static const struct drm_panel_funcs y030xx067a_funcs = {
.prepare= y030xx067a_prepare,
.unprepare = y030xx067a_unprepare,
+ .enable = y030xx067a_enable,
+ .disable= y030xx067a_disable,
.get_modes = y030xx067a_get_modes,
};
diff --git a/drivers/gpu/drm/panel/panel-innolux-ej030na.c
b/drivers/gpu/drm/panel/panel-innolux-ej030na.c
index c558de3f99be..6de7370185cd 100644
--- a/drivers/gpu/drm/panel/panel-innolux-ej030na.c
+++ b/drivers/gpu/drm/panel/panel-innolux-ej030na.c
@@ -80,8 +80,6 @@ static const struct reg_sequence ej030na_init_sequence[] = {
{ 0x47, 0x08 },
{ 0x48, 0x0f },
{ 0x49, 0x0f },
-
- { 0x2b, 0x01 },
};
static int ej030na_prepare(struct drm_panel *panel)
@@ -109,8 +107,6 @@ static int ej030na_prepare(struct drm_panel *panel)
goto err_disable_regulator;
}
- msleep(120);
-
return 0;
err_disable_regulator:
@@ -128,6 +124,31 @@ static int ej030na_unprepare(struct drm_panel *panel)
return 0;
}
+static int ej030na_enable(struct drm_panel *panel)
+{
+ struct ej030na *priv = to_ej030na(panel);
+
+ /* standby off */
+ regmap_write(priv->map, 0x2b, 0x01);
+
+ if (panel->backlight) {
+ /* Wait for the picture to be ready before enabling backlight */
+ msleep(120);
+ }
+
+ return 0;
+}
+
+static int ej030na_disable(struct drm_panel *panel)
+{
+ struct ej030na *priv = to_ej030na(panel);
+
+ /* standby on */
+ regmap_write(priv->map, 0x2b, 0x00);
+
+ return 0;
+}
+
static int ej030na_get_modes(struct drm_panel *panel,
struct drm_connector *connector)
{
@@ -165,6 +186,8 @@ static int ej030na_get_modes(struct drm_panel *panel,
static const struct drm_panel_funcs ej030na_funcs = {
.prepare= ej030na_prepare,
.unprepare = ej030na_unprepare,
+ .enable = ej030na_enable,
+ .disable= ej030na_disable,
.get_modes = ej030na_get_modes,
};
--
2.34.1
[PATCH v2 4/4] dt-bindings: display/panel: Add Leadtek ltk035c5444t
Add binding for the leadtek ltk035c5444t, which is a 640x480
mipi-dbi over spi / 24-bit RGB panel based on the newvision
NV03052C chipset.
It is found in the Anbernic RG350M mips handheld.
Signed-off-by: Christophe Branchereau
---
.../panel/leadtek,ltk035c5444t-spi.yaml | 59 +++
1 file changed, 59 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t-spi.yaml
diff --git
a/Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t-spi.yaml
b/Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t-spi.yaml
new file mode 100644
index ..9b6f1810adab
--- /dev/null
+++
b/Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t-spi.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/leadtek,ltk035c5444t-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Leadtek ltk035c5444t 3.5" (640x480 pixels) 24-bit IPS LCD panel
+
+maintainers:
+ - Paul Cercueil
+ - Christophe Branchereau
+
+allOf:
+ - $ref: panel-common.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+const: leadtek,ltk035c5444t-spi
+
+ backlight: true
+ port: true
+ power-supply: true
+ reg: true
+ reset-gpios: true
+
+required:
+ - compatible
+ - power-supply
+ - reset-gpios
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+panel@0 {
+compatible = "leadtek,ltk035c5444t-spi";
+reg = <0>;
+
+spi-3wire;
+spi-max-frequency = <3125000>;
+
+reset-gpios = <&gpe 2 GPIO_ACTIVE_LOW>;
+
+backlight = <&backlight>;
+power-supply = <&vcc>;
+
+port {
+panel_input: endpoint {
+remote-endpoint = <&panel_output>;
+};
+};
+};
+};
--
2.34.1
Re: [PATCH] drm/doc: pull in drm_buddy.c
Am 08.03.22 um 13:51 schrieb Matthew Auld: On 09/02/2022 07:32, Christian König wrote: Am 08.02.22 um 16:12 schrieb Matthew Auld: Make sure we pull in the kernel-doc for this. Reported-by: Daniel Vetter Signed-off-by: Matthew Auld Cc: Arunpravin Cc: Christian König Reviewed-by: Christian König Thanks. Could you also push this? Done. --- Documentation/gpu/drm-mm.rst | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index 198bcc1affa1..f32ccce5722d 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -466,6 +466,15 @@ DRM MM Range Allocator Function References .. kernel-doc:: drivers/gpu/drm/drm_mm.c :export: +DRM Buddy Allocator +=== + +DRM Buddy Function References +- + +.. kernel-doc:: drivers/gpu/drm/drm_buddy.c + :export: + DRM Cache Handling and Fast WC memcpy() ===
[PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
Hello, This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. During OOM, the shrinker will release BOs that are marked as "not needed" by userspace using the new madvise IOCTL. The userspace in this case is the Mesa VirGL driver, it will mark the cached BOs as "not needed", allowing kernel driver to release memory of the cached shmem BOs on lowmem situations, preventing OOM kills. This patchset includes couple fixes for problems I found while was working on the shrinker, it also includes prerequisite DMA API usage improvement needed by the shrinker. The Mesa and IGT patches will be kept on hold until this kernel series will be approved and applied. This patchset was tested using Qemu and crosvm, including both cases of IOMMU off/on. Mesa: https://gitlab.freedesktop.org/digetx/mesa/-/commits/virgl-madvise IGT: https://gitlab.freedesktop.org/digetx/igt-gpu-tools/-/tree/virtio-madvise Dmitry Osipenko (5): drm/virtio: Correct drm_gem_shmem_get_sg_table() error handling drm/virtio: Check whether transferred 2D BO is shmem drm/virtio: Unlock GEM reservations in error code path drm/virtio: Improve DMA API usage for shmem BOs drm/virtio: Add memory shrinker drivers/gpu/drm/virtio/Makefile | 3 +- drivers/gpu/drm/virtio/virtgpu_drv.c | 22 +++- drivers/gpu/drm/virtio/virtgpu_drv.h | 31 - drivers/gpu/drm/virtio/virtgpu_gem.c | 84 drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c | 124 ++ drivers/gpu/drm/virtio/virtgpu_ioctl.c| 37 ++ drivers/gpu/drm/virtio/virtgpu_kms.c | 17 ++- drivers/gpu/drm/virtio/virtgpu_object.c | 63 +++-- drivers/gpu/drm/virtio/virtgpu_plane.c| 17 ++- drivers/gpu/drm/virtio/virtgpu_vq.c | 30 +++-- include/uapi/drm/virtgpu_drm.h| 14 ++ 11 files changed, 373 insertions(+), 69 deletions(-) create mode 100644 drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c -- 2.35.1
[PATCH v1 1/5] drm/virtio: Correct drm_gem_shmem_get_sg_table() error handling
drm_gem_shmem_get_sg_table() never ever returned NULL on error. Correct
the error handling to avoid crash on OOM.
Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_object.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index f293e6ad52da..bea7806a3ae3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -168,9 +168,11 @@ static int virtio_gpu_object_shmem_init(struct
virtio_gpu_device *vgdev,
* since virtio_gpu doesn't support dma-buf import from other devices.
*/
shmem->pages = drm_gem_shmem_get_sg_table(&bo->base);
- if (!shmem->pages) {
+ ret = PTR_ERR(shmem->pages);
+ if (ret) {
drm_gem_shmem_unpin(&bo->base);
- return -EINVAL;
+ shmem->pages = NULL;
+ return ret;
}
if (use_dma_api) {
--
2.35.1
[PATCH v1 2/5] drm/virtio: Check whether transferred 2D BO is shmem
Transferred 2D BO always must be a shmem BO. Add check for that to prevent NULL dereference if userspace passes a VRAM BO. Signed-off-by: Dmitry Osipenko --- drivers/gpu/drm/virtio/virtgpu_vq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 7c052efe8836..2edf31806b74 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -595,7 +595,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, bool use_dma_api = !virtio_has_dma_quirk(vgdev->vdev); struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo); - if (use_dma_api) + if (virtio_gpu_is_shmem(bo) && use_dma_api) dma_sync_sgtable_for_device(vgdev->vdev->dev.parent, shmem->pages, DMA_TO_DEVICE); -- 2.35.1
[PATCH v1 3/5] drm/virtio: Unlock GEM reservations in error code path
Unlock reservations in the error code path of virtio_gpu_object_create()
to silence debug warning splat produced by ww_mutex_destroy(&obj->lock)
when GEM is released with the held lock.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_object.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index bea7806a3ae3..0b8cbb87f8d8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -250,6 +250,8 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
if (ret != 0) {
+ if (fence)
+ virtio_gpu_array_unlock_resv(objs);
virtio_gpu_array_put_free(objs);
virtio_gpu_free_object(&shmem_obj->base);
return ret;
--
2.35.1
[PATCH v1 5/5] drm/virtio: Add memory shrinker
Add memory shrinker and new madvise IOCTL to the VirtIO-GPU driver.
Userspace (BO cache manager of Mesa driver) will mark BOs as "don't need"
using the new IOCTL to let shrinker purge the marked BOs on OOM, thus
shrinker will lower memory pressure and prevent OOM kills.
Signed-off-by: Daniel Almeida
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/Makefile | 3 +-
drivers/gpu/drm/virtio/virtgpu_drv.h | 26 +++-
drivers/gpu/drm/virtio/virtgpu_gem.c | 84
drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c | 124 ++
drivers/gpu/drm/virtio/virtgpu_ioctl.c| 37 ++
drivers/gpu/drm/virtio/virtgpu_kms.c | 10 ++
drivers/gpu/drm/virtio/virtgpu_object.c | 7 +
drivers/gpu/drm/virtio/virtgpu_plane.c| 17 ++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 15 +++
include/uapi/drm/virtgpu_drm.h| 14 ++
10 files changed, 333 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c
diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile
index b99fa4a73b68..e1715ae02a2d 100644
--- a/drivers/gpu/drm/virtio/Makefile
+++ b/drivers/gpu/drm/virtio/Makefile
@@ -6,6 +6,7 @@
virtio-gpu-y := virtgpu_drv.o virtgpu_kms.o virtgpu_gem.o virtgpu_vram.o \
virtgpu_display.o virtgpu_vq.o \
virtgpu_fence.o virtgpu_object.o virtgpu_debugfs.o virtgpu_plane.o \
- virtgpu_ioctl.o virtgpu_prime.o virtgpu_trace_points.o
+ virtgpu_ioctl.o virtgpu_prime.o virtgpu_trace_points.o \
+ virtgpu_gem_shrinker.o
obj-$(CONFIG_DRM_VIRTIO_GPU) += virtio-gpu.o
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index b2d93cb12ebf..f907fcd81a24 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -94,6 +94,8 @@ struct virtio_gpu_object {
int uuid_state;
uuid_t uuid;
+
+ refcount_t pin_count;
};
#define gem_to_virtio_gpu_obj(gobj) \
container_of((gobj), struct virtio_gpu_object, base.base)
@@ -211,6 +213,11 @@ struct virtio_gpu_drv_cap_cache {
atomic_t is_valid;
};
+struct virtio_gpu_shrinker {
+ struct shrinker shrinker;
+ struct list_head list;
+};
+
struct virtio_gpu_device {
struct drm_device *ddev;
@@ -261,6 +268,11 @@ struct virtio_gpu_device {
spinlock_t resource_export_lock;
/* protects map state and host_visible_mm */
spinlock_t host_visible_lock;
+
+ struct virtio_gpu_shrinker vgshrinker;
+
+ /* protects all memory management operations */
+ struct mutex mm_lock;
};
struct virtio_gpu_fpriv {
@@ -274,7 +286,7 @@ struct virtio_gpu_fpriv {
};
/* virtgpu_ioctl.c */
-#define DRM_VIRTIO_NUM_IOCTLS 12
+#define DRM_VIRTIO_NUM_IOCTLS 13
extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
void virtio_gpu_create_context(struct drm_device *dev, struct drm_file *file);
@@ -310,6 +322,12 @@ void virtio_gpu_array_put_free(struct
virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_work(struct work_struct *work);
+int virtio_gpu_array_validate(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object_array *objs);
+bool virtio_gpu_gem_madvise(struct virtio_gpu_object *obj, int madv);
+int virtio_gpu_gem_host_mem_release(struct virtio_gpu_object *bo);
+int virtio_gpu_gem_pin(struct virtio_gpu_object *bo);
+void virtio_gpu_gem_unpin(struct virtio_gpu_object *bo);
/* virtgpu_vq.c */
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
@@ -321,6 +339,8 @@ void virtio_gpu_cmd_create_resource(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_fence *fence);
void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo);
+int virtio_gpu_cmd_release_resource(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *bo);
void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
uint64_t offset,
uint32_t width, uint32_t height,
@@ -483,4 +503,8 @@ void virtio_gpu_vram_unmap_dma_buf(struct device *dev,
struct sg_table *sgt,
enum dma_data_direction dir);
+/* virtgpu_gem_shrinker.c */
+int virtio_gpu_gem_shrinker_init(struct virtio_gpu_device *vgdev);
+void virtio_gpu_gem_shrinker_fini(struct virtio_gpu_device *vgdev);
+
#endif
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c
b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 48d3c9955f0d..dbe5b8fa8285 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virti
[PATCH v1 4/5] drm/virtio: Improve DMA API usage for shmem BOs
DRM API requires the DRM's driver to be backed with the device that can
be used for generic DMA operations. The VirtIO-GPU device can't perform
DMA operations if it uses PCI transport because PCI device driver creates
a virtual VirtIO-GPU device that isn't associated with the PCI. Use PCI's
GPU device for the DRM's device instead of the VirtIO-GPU device and drop
DMA-related hacks from the VirtIO-GPU driver.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_drv.c| 22 +++---
drivers/gpu/drm/virtio/virtgpu_drv.h| 5 +--
drivers/gpu/drm/virtio/virtgpu_kms.c| 7 ++--
drivers/gpu/drm/virtio/virtgpu_object.c | 56 +
drivers/gpu/drm/virtio/virtgpu_vq.c | 13 +++---
5 files changed, 37 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c
b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 5f25a8d15464..8449dad3e65c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -46,9 +46,9 @@ static int virtio_gpu_modeset = -1;
MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
module_param_named(modeset, virtio_gpu_modeset, int, 0400);
-static int virtio_gpu_pci_quirk(struct drm_device *dev, struct virtio_device
*vdev)
+static int virtio_gpu_pci_quirk(struct drm_device *dev)
{
- struct pci_dev *pdev = to_pci_dev(vdev->dev.parent);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
const char *pname = dev_name(&pdev->dev);
bool vga = (pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA;
char unique[20];
@@ -101,6 +101,7 @@ static int virtio_gpu_pci_quirk(struct drm_device *dev,
struct virtio_device *vd
static int virtio_gpu_probe(struct virtio_device *vdev)
{
struct drm_device *dev;
+ struct device *dma_dev;
int ret;
if (drm_firmware_drivers_only() && virtio_gpu_modeset == -1)
@@ -109,18 +110,29 @@ static int virtio_gpu_probe(struct virtio_device *vdev)
if (virtio_gpu_modeset == 0)
return -EINVAL;
- dev = drm_dev_alloc(&driver, &vdev->dev);
+ /*
+* If GPU's parent is a PCI device, then we will use this PCI device
+* for the DRM's driver device because GPU won't have PCI's IOMMU DMA
+* ops in this case since GPU device is sitting on a separate (from PCI)
+* virtio-bus.
+*/
+ if (!strcmp(vdev->dev.parent->bus->name, "pci"))
+ dma_dev = vdev->dev.parent;
+ else
+ dma_dev = &vdev->dev;
+
+ dev = drm_dev_alloc(&driver, dma_dev);
if (IS_ERR(dev))
return PTR_ERR(dev);
vdev->priv = dev;
if (!strcmp(vdev->dev.parent->bus->name, "pci")) {
- ret = virtio_gpu_pci_quirk(dev, vdev);
+ ret = virtio_gpu_pci_quirk(dev);
if (ret)
goto err_free;
}
- ret = virtio_gpu_init(dev);
+ ret = virtio_gpu_init(vdev, dev);
if (ret)
goto err_free;
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 0a194aaad419..b2d93cb12ebf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -100,8 +100,6 @@ struct virtio_gpu_object {
struct virtio_gpu_object_shmem {
struct virtio_gpu_object base;
- struct sg_table *pages;
- uint32_t mapped;
};
struct virtio_gpu_object_vram {
@@ -214,7 +212,6 @@ struct virtio_gpu_drv_cap_cache {
};
struct virtio_gpu_device {
- struct device *dev;
struct drm_device *ddev;
struct virtio_device *vdev;
@@ -282,7 +279,7 @@ extern struct drm_ioctl_desc
virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
void virtio_gpu_create_context(struct drm_device *dev, struct drm_file *file);
/* virtgpu_kms.c */
-int virtio_gpu_init(struct drm_device *dev);
+int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev);
void virtio_gpu_deinit(struct drm_device *dev);
void virtio_gpu_release(struct drm_device *dev);
int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 3313b92db531..0d1e3eb61bee 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -110,7 +110,7 @@ static void virtio_gpu_get_capsets(struct virtio_gpu_device
*vgdev,
vgdev->num_capsets = num_capsets;
}
-int virtio_gpu_init(struct drm_device *dev)
+int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
{
static vq_callback_t *callbacks[] = {
virtio_gpu_ctrl_ack, virtio_gpu_cursor_ack
@@ -123,7 +123,7 @@ int virtio_gpu_init(struct drm_device *dev)
u32 num_scanouts, num_capsets;
int ret = 0;
- if (!virtio_has_feature(dev_to_virtio(dev->dev), VIRTIO_F_VERSION_1))
+ if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
ret
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On 3/8/22 13:51, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
On 3/8/22 11:07, Jagan Teki wrote:
On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
On 3/8/22 09:03, Jagan Teki wrote:
Hi,
[...]
@@ -314,7 +321,9 @@ static const struct drm_bridge_funcs chipone_bridge_funcs =
{
static int chipone_parse_dt(struct chipone *icn)
{
struct device *dev = icn->dev;
+ struct device_node *endpoint;
struct drm_panel *panel;
+ int dsi_lanes;
int ret;
icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
@@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone *icn)
return PTR_ERR(icn->enable_gpio);
}
+ endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
+ dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
+ icn->host_node = of_graph_get_remote_port_parent(endpoint);
+ of_node_put(endpoint);
+
+ if (!icn->host_node)
+ return -ENODEV;
The non-ports-based OF graph returns a -19 example on the Allwinner
Display pipeline in R16 [1].
We need to have a helper to return host_node for non-ports as I have
done it for drm_of_find_bridge.
[1] https://patchwork.amarulasolutions.com/patch/1805/
The link points to a patch marked "DO NOT MERGE", maybe that patch is
missing the DSI host port@0 OF graph link ? Both port@0 and port@1 are
required, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
What is "non-ports-based OF graph" ?
I don't see drm_of_find_bridge() in linux-next , what is that ?
port@0 is optional as some of the DSI host OF-graph represent the
bridge or panel as child nodes instead of ports. (i think dt-binding
has to fix it to make port@0 optional)
The current upstream DT binding document says:
required:
- port@0
- port@1
So port@0 is mandatory.
In the binding, sure, but fundamentally the DT excerpt Jagan provided is
correct. If the bridge supports DCS, there's no reason to use the OF
graph in the first place: the bridge node will be a child node of the
MIPI-DSI controller (and there's no obligation to use the OF-graph for a
MIPI-DSI controller).
I believe port@0 should be made optional (or downright removed if
MIPI-DCS in the only control bus).
That's out of scope of this series anyway, so Jagan can implement
patches for that mode if needed.
[PATCH v1] drm/shmem-helper: Correct doc-comment of drm_gem_shmem_get_sg_table()
drm_gem_shmem_get_sg_table() never returns NULL on error, but a ERR_PTR.
Correct the doc comment which says that it returns NULL on error.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 8ad0e02991ca..37009418cd28 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -662,7 +662,7 @@ EXPORT_SYMBOL(drm_gem_shmem_print_info);
* drm_gem_shmem_get_pages_sgt() instead.
*
* Returns:
- * A pointer to the scatter/gather table of pinned pages or NULL on failure.
+ * A pointer to the scatter/gather table of pinned pages or errno on failure.
*/
struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_shmem_object *shmem)
{
--
2.35.1
Re: [RESEND v13 07/22] soc: mediatek: mmsys: specify 64BIT dependency for MTK_MMSYS
Il 08/03/22 10:30, Nancy.Lin ha scritto: Because mtk-mutex change to use unsigned long mutex module type, it should depend 64BIT. This is a preparation for adding support for mt8195 vdosys1 mutex. Signed-off-by: Nancy.Lin --- drivers/soc/mediatek/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig index fdd8bc08569e..24f792c46444 100644 --- a/drivers/soc/mediatek/Kconfig +++ b/drivers/soc/mediatek/Kconfig @@ -68,6 +68,7 @@ config MTK_SCPSYS_PM_DOMAINS config MTK_MMSYS bool "MediaTek MMSYS Support" default ARCH_MEDIATEK + depends on 64BIT depends on HAS_IOMEM help Say yes here to add support for the MediaTek Multimedia Breaking old platforms is forbidden. MT2701 and MT7623N are 32-bit ARM SoCs and: - mt2701 needs mmsys only; but - mt7623n needs mmsys and mutex. Besides, this is an easy fix: just change your unsigned long to a fixed size u64.
Re: [RESEND v13 22/22] arm64: dts: mt8195: add display node for vdosys1
Il 08/03/22 10:30, Nancy.Lin ha scritto: Add display node for vdosys1. Signed-off-by: Nancy.Lin Reviewed-by: AngeloGioacchino Del Regno --- arch/arm64/boot/dts/mediatek/mt8195.dtsi | 223 +++ 1 file changed, 223 insertions(+)
Re: [RESEND v13 01/22] dt-bindings: mediatek: add vdosys1 RDMA definition for mt8195
Il 08/03/22 10:29, Nancy.Lin ha scritto:
Add vdosys1 RDMA definition.
Signed-off-by: Nancy.Lin
Reviewed-by: AngeloGioacchino Del Regno
---
.../arm/mediatek/mediatek,mdp-rdma.yaml | 86 +++
1 file changed, 86 insertions(+)
create mode 100644
Documentation/devicetree/bindings/arm/mediatek/mediatek,mdp-rdma.yaml
diff --git
a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mdp-rdma.yaml
b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mdp-rdma.yaml
new file mode 100644
index ..6ab773569462
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mdp-rdma.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/mediatek/mediatek,mdp-rdma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek MDP RDMA
+
+maintainers:
+ - Matthias Brugger
+
+description: |
+ The mediatek MDP RDMA stands for Read Direct Memory Access.
+ It provides real time data to the back-end panel driver, such as DSI,
+ DPI and DP_INTF.
+ It contains one line buffer to store the sufficient pixel data.
+ RDMA device node must be siblings to the central MMSYS_CONFIG node.
+ For a description of the MMSYS_CONFIG binding, see
+ Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml for
details.
+
+properties:
+ compatible:
+oneOf:
+ - items:
+ - const: mediatek,mt8195-vdo1-rdma
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ power-domains:
+description: A phandle and PM domain specifier as defined by bindings of
+ the power controller specified by phandle. See
+ Documentation/devicetree/bindings/power/power-domain.yaml for details.
+
+ clocks:
+items:
+ - description: RDMA Clock
+
+ iommus:
+description:
+ This property should point to the respective IOMMU block with master
port as argument,
+ see Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for
details.
+
+ mediatek,gce-client-reg:
+description:
+ The register of display function block to be set by gce. There are 4
arguments,
+ such as gce node, subsys id, offset and register size. The subsys id
that is
+ mapping to the register of display function blocks is defined in the gce
header
+ include/include/dt-bindings/gce/-gce.h of each chips.
+$ref: /schemas/types.yaml#/definitions/phandle-array
+maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - power-domains
+ - clocks
+ - iommus
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+#include
+#include
+#include
+#include
+
+soc {
+#address-cells = <2>;
+#size-cells = <2>;
+
+vdo1_rdma0: mdp-rdma@1c104000 {
+compatible = "mediatek,mt8195-vdo1-rdma";
+reg = <0 0x1c104000 0 0x1000>;
+interrupts = ;
+clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
+power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>;
+iommus = <&iommu_vdo M4U_PORT_L2_MDP_RDMA0>;
+mediatek,gce-client-reg = <&gce0 SUBSYS_1c10 0x4000 0x1000>;
+};
+};
Re: [PATCH] drm: remove min_order BUG_ON check
On 07/03/22 10:11 pm, Matthew Auld wrote:
> On 07/03/2022 14:37, Arunpravin wrote:
>> place BUG_ON(order < min_order) outside do..while
>> loop as it fails Unigine Heaven benchmark.
>>
>> Unigine Heaven has buffer allocation requests for
>> example required pages are 161 and alignment request
>> is 128. To allocate the remaining 33 pages, continues
>> the iteration to find the order value which is 5 and
>> when it compares with min_order = 7, enables the
>> BUG_ON(). To avoid this problem, placed the BUG_ON
>> check outside of do..while loop.
>>
>> Signed-off-by: Arunpravin
>> ---
>> drivers/gpu/drm/drm_buddy.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
>> index 72f52f293249..ed94c56b720f 100644
>> --- a/drivers/gpu/drm/drm_buddy.c
>> +++ b/drivers/gpu/drm/drm_buddy.c
>> @@ -669,10 +669,11 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
>> order = fls(pages) - 1;
>> min_order = ilog2(min_page_size) - ilog2(mm->chunk_size);
>>
>> +BUG_ON(order < min_order);
>
> Isn't the issue that we are allowing a size that is not aligned to the
> requested min_page_size? Should we not fix the caller(and throw a normal
> error here), or perhaps add the round_up() here instead?
>
CASE 1:
when size is not aligned to the requested min_page_size, for instance,
required size = 161 pages, min_page_size = 128 pages, here we have 3
possible options,
a. AFAIK,This kind of situation is common in any workload,the first
allocation (i.e) 128 pages is aligned to min_page_size, Should we just
allocate the left over 33 pages (2 pow 5, 2 pow 0) since the caller does
know the left over pages are not in min_page_size alignment?
b. There are many such instances in unigine heaven workload (there would
be many such workloads), throwing a normal error would lower the FPS? is
it possible to fix at caller application?
c. adding the round_up() is possible, but in every such instances we end
up allocating extra unused memory. For example, if required pages = 1028
and min_page_size = 1024 pages, we end up round up of left over 4 pages
to the min_page_size, so the total size would be 2048 pages.
> i.e if someone does:
>
> alloc_blocks(mm, 0, end, 4096, 1<<16, &blocks, flags);
CASE 2:
I think this case should be detected (i.e) when min_page_size > size,
should we return -EINVAL?
>
> This will still trigger the BUG_ON() even if we move it out of the loop,
> AFAICT.
>
Should we just allow the CASE 1 proceed for the allocation and return
-EINVAL for the CASE 2?
>> +
>> do {
>> order = min(order, (unsigned int)fls(pages) - 1);
>> BUG_ON(order > mm->max_order);
>> -BUG_ON(order < min_order);
>>
>> do {
>> if (flags & DRM_BUDDY_RANGE_ALLOCATION)
>>
>> base-commit: 8025c79350b90e5a8029234d433578f12abbae2b
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On Tue, Mar 08, 2022 at 02:27:40PM +0100, Marek Vasut wrote:
> On 3/8/22 13:51, Maxime Ripard wrote:
> > On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
> > > On 3/8/22 11:07, Jagan Teki wrote:
> > > > On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
> > > > >
> > > > > On 3/8/22 09:03, Jagan Teki wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > [...]
> > > > >
> > > > > > > @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs
> > > > > > > chipone_bridge_funcs = {
> > > > > > > static int chipone_parse_dt(struct chipone *icn)
> > > > > > > {
> > > > > > >struct device *dev = icn->dev;
> > > > > > > + struct device_node *endpoint;
> > > > > > >struct drm_panel *panel;
> > > > > > > + int dsi_lanes;
> > > > > > >int ret;
> > > > > > >
> > > > > > >icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
> > > > > > > @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone
> > > > > > > *icn)
> > > > > > >return PTR_ERR(icn->enable_gpio);
> > > > > > >}
> > > > > > >
> > > > > > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0,
> > > > > > > 0);
> > > > > > > + dsi_lanes = of_property_count_u32_elems(endpoint,
> > > > > > > "data-lanes");
> > > > > > > + icn->host_node =
> > > > > > > of_graph_get_remote_port_parent(endpoint);
> > > > > > > + of_node_put(endpoint);
> > > > > > > +
> > > > > > > + if (!icn->host_node)
> > > > > > > + return -ENODEV;
> > > > > >
> > > > > > The non-ports-based OF graph returns a -19 example on the Allwinner
> > > > > > Display pipeline in R16 [1].
> > > > > >
> > > > > > We need to have a helper to return host_node for non-ports as I have
> > > > > > done it for drm_of_find_bridge.
> > > > > >
> > > > > > [1] https://patchwork.amarulasolutions.com/patch/1805/
> > > > >
> > > > > The link points to a patch marked "DO NOT MERGE", maybe that patch is
> > > > > missing the DSI host port@0 OF graph link ? Both port@0 and port@1 are
> > > > > required, see:
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
> > > > >
> > > > > What is "non-ports-based OF graph" ?
> > > > >
> > > > > I don't see drm_of_find_bridge() in linux-next , what is that ?
> > > >
> > > > port@0 is optional as some of the DSI host OF-graph represent the
> > > > bridge or panel as child nodes instead of ports. (i think dt-binding
> > > > has to fix it to make port@0 optional)
> > >
> > > The current upstream DT binding document says:
> > >
> > > required:
> > >- port@0
> > >- port@1
> > >
> > > So port@0 is mandatory.
> >
> > In the binding, sure, but fundamentally the DT excerpt Jagan provided is
> > correct. If the bridge supports DCS, there's no reason to use the OF
> > graph in the first place: the bridge node will be a child node of the
> > MIPI-DSI controller (and there's no obligation to use the OF-graph for a
> > MIPI-DSI controller).
> >
> > I believe port@0 should be made optional (or downright removed if
> > MIPI-DCS in the only control bus).
>
> That's out of scope of this series anyway, so Jagan can implement patches
> for that mode if needed.
Not really? You can't count on the port@0 being there generally
speaking, so you can't count on data-lanes being there either, which
exactly what you're doing in this patch.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v2 1/2] dt-bindings: drm/bridge: anx7625: Revert DPI support
On Tue, Mar 8, 2022 at 3:49 AM Robert Foss wrote: > > Revert DPI support from binding. > > DPI support relies on the bus-type enum which does not yet support > Mipi DPI, since no v4l2_fwnode_bus_type has been defined for this > bus type. > > When DPI for anx7625 was initially added, it assumed that > V4L2_FWNODE_BUS_TYPE_PARALLEL was the correct bus type for > representing DPI, which it is not. > > In order to prevent adding this mis-usage to the ABI, let's revert > the support. > > Signed-off-by: Robert Foss > Reviewed-by: Laurent Pinchart > --- > > Changes since v1: > - Rob: Instead of reverting the entire commit introducing this, >do a partial revert of only the relevant parts. > > .../display/bridge/analogix,anx7625.yaml | 19 +-- > 1 file changed, 1 insertion(+), 18 deletions(-) Reviewed-by: Rob Herring
Re: [PATCH 2/9] fbdev: Put mmap for deferred I/O into drivers
On 3/3/22 21:58, Thomas Zimmermann wrote: > The fbdev mmap function fb_mmap() unconditionally overrides the > driver's implementation if deferred I/O has been activated. This > makes it hard to implement mmap with anything but a vmalloc()'ed > software buffer. That is specifically a problem for DRM, where > video memory is maintained by a memory manager. > > Leave the mmap handling to drivers and expect them to call the > helper for deferred I/O by thmeselves. > > Signed-off-by: Thomas Zimmermann > --- [snip] > > + /* > + * FB deferred I/O want you to handle mmap in your drivers. At a > + * minimum, point struct fb_ops.fb_mmap to fb_deferred_io_mmap(). > + */ > + if (WARN_ON_ONCE(info->fbdefio)) > + return -ENODEV; > + Maybe part of that comment could be printed as a WARN_ON_ONCE() message ? Regardless, the patch looks good to me: Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v7 22/24] drm: rockchip: Add VOP2 driver
On Tue, 8 Mar 2022 at 08:42, Andy Yan wrote: > On 3/7/22 21:09, Daniel Stone wrote: > > On Mon, 7 Mar 2022 at 12:18, Andy Yan wrote: > >> When run a weston 10.0.0: > >> > >># export XDG_RUNTIME_DIR=/tmp > >># weston --backend=drm-backend.so --use-pixma --tty=2 > >> --continue=without-input > >> > >> I got the following error: > >> > >> drm_atomic_check_only [PLANE:31:Smart0-win0] CRTC set but no FB > > Can you please start Weston with --logger-scopes=log,drm-backend and > > attach the output? > > Please see the weston ouput here[0] Are you running with musl perhaps? Either way, please make sure your libdrm build includes commit 79fa377c8bdc84fde99c6a6ac17e554971c617be. Cheers, Daniel
Re: [PATCH v2 2/4] drm/bridge: ti-sn65dsi86: Make connector creation optional
Hi Doug,
Quoting Doug Anderson (2022-03-07 23:21:28)
> Hi,
>
> On Mon, Mar 7, 2022 at 10:00 AM Kieran Bingham
> wrote:
> >
> > From: Laurent Pinchart
> >
> > Now that the driver supports the connector-related bridge operations,
> > make the connector creation optional. This enables usage of the
> > sn65dsi86 with the DRM bridge connector helper.
> >
> > Signed-off-by: Laurent Pinchart
> > Signed-off-by: Kieran Bingham
> >
> > ---
> > Changes since v1:
> > - None
> >
> > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 17 +++--
> > 1 file changed, 7 insertions(+), 10 deletions(-)
>
> Can you please CC me on this series next time you send it out? I was
> pretty involved in previous reviews here. Luckily I got CCed on one of
> the patches so I knew to look, but since that one is (probably)
> getting dropped it'd be good to make sure I was on the others. It's
> also pretty important to include +Sam Ravnborg since there's a lot of
> overlap with his series [1]:
Absolutely - I was assuming you were the main target for reviews. I'm
sorry - I also assumed get_maintainer.pl had / would add you - and I
didn't check getting the patches out last night.
I'll be sure to manually add you.
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index ffb6c04f6c46..29f5f7123ed9 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -745,11 +745,6 @@ static int (struct drm_bridge *bridge,
> > struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
> > int ret;
> >
> > - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> > - DRM_ERROR("Fix bridge driver to make connector optional!");
> > - return -EINVAL;
> > - }
>
> That won't work without some other fixes, sorry!
Ok ;-)
To be clear, my goal here has been to get the IRQ HPD working - so
my main focus is there. I guess I now have to handle custodianship of
these dependencies somehow too though.
> The problem is that you'll break ti_sn_bridge_get_bpp(). That
> absolutely relies on having our own connector so you need to fix that
> _before_ making the connector optional.
>
> Rob Clark made an attempt on this [2] but Laurent didn't like the
> solution he came up with. Sam's series that I mentioned [1] also made
> an attempt at this, specifically in patch 5 ("drm/bridge:
> ti-sn65dsi86: Fetch bpc via drm_bridge_state") of his series.
> Unfortunately, it didn't work because the "output_bus_cfg.format"
> wasn't set to anything. Personally I wouldn't mind Rob's solution as a
> stopgap if Laurent removes his NAK. Then we're not stuck while someone
> tries to find time to fix this correctly.
Ok - all of that is going to take some time for me to review and
process.
> One last problem here is that your patch doesn't actually apply to
> drm-misc/drm-misc-next, which is likely where it would land. I believe
> it conflicts with my recent commit e283820cbf80 ("drm/bridge:
> ti-sn65dsi86: Use drm_bridge_connector"). It looks like you based your
> series on mainline, but you should really be targeting drm-misc-next.
Ah sorry - I thought I had merged in drm-misc-next, but it was a week
ago or so so I guess I'm already outdated.
I'll cleanup for a new base now.
> -Doug
>
> [1] https://lore.kernel.org/r/20220206154405.124-1-...@ravnborg.org/
> [2] https://lore.kernel.org/all/20210920225801.227211-4-robdcl...@gmail.com/
Re: Writeback Assumptions for XRGB
Hi! On Tue, Mar 08, 2022 at 10:26:24AM +0200, Pekka Paalanen wrote: > On Fri, 4 Mar 2022 15:46:07 +0100 > Maxime Ripard wrote: > > > Indeed, while the input buffer uses 0xff for the X component, we'll get > > back 0x00 from the hardware, and thus the hashes are not identical. We > > can force the hardware to always set it to 0xff, but that doesn't seem > > right. It would make more sense to ignore the X component entirely in > > that case. > > Hi, > > I tried hard to send a slightly longer response, but gmail refuses to > deliver to anyone without explanation. For reference (and archives), this was your original message: > > if a pixel component is marked X, then its value must not have any > > observable effect when passed over an interface to another > > component. To me there is no doubt about that. > > > > OTOH, if both output and writeback FBs had ARGB instead of XRGB, > > things would be more complicated. Quite likely the CRTC background > > color is opaque (or maybe HDMI and DP cannot transmit alpha meaning > > that writeback with alpha < 1.0 makes no sense), which means that no > > matter what A values goes in, writeback A will always come out as > > 0xff (on 8 bpc). One might be able to argue otherwise on this. > > > > I would actually recommend IGT to put pseudo-random garbage on X > > channels to catch drivers and hardware that unexpectedly uses the X > > values. I've used this trick with weston-simple-shm: > > https://ppaalanen.blogspot.com/2012/04/improved-appearance-for-simplest.html > > > > Pixel component values 0x00 and 0xff are weak for testing blending > > and composition. > > So here's a summary of my opinion: > > - KMS must ignore X channel contents on read > - IGT must ignore X channel contents when comparing results > - it would be nice if IGT filled image X channels with garbage to > verify the first two points That works for me :) I'll work on a series of patches addressing this then Thanks! Maxime signature.asc Description: PGP signature
Re: [PATCH] drm: remove min_order BUG_ON check
On 07/03/22 9:23 pm, Christian König wrote:
> Am 07.03.22 um 15:37 schrieb Arunpravin:
>> place BUG_ON(order < min_order) outside do..while
>> loop as it fails Unigine Heaven benchmark.
>>
>> Unigine Heaven has buffer allocation requests for
>> example required pages are 161 and alignment request
>> is 128. To allocate the remaining 33 pages, continues
>> the iteration to find the order value which is 5 and
>> when it compares with min_order = 7, enables the
>> BUG_ON(). To avoid this problem, placed the BUG_ON
>> check outside of do..while loop.
>
> Well using BUG_ON sounds like the wrong approach in the first place.
>
> A BUG_ON() is only justified if you prevent further data corruption,
> e.g. when you detect for example a reference count overflow or similar.
>
> In all other cases you should trigger a WARN_ON() and abort the
> operation with -EINVAL if possible.
>
> Regards,
> Christian.
>
ok, in this case, I think it is acceptable to use WARN_ON and abort
using -EINVAL
Regards,
Arun
>>
>> Signed-off-by: Arunpravin
>> ---
>> drivers/gpu/drm/drm_buddy.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
>> index 72f52f293249..ed94c56b720f 100644
>> --- a/drivers/gpu/drm/drm_buddy.c
>> +++ b/drivers/gpu/drm/drm_buddy.c
>> @@ -669,10 +669,11 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
>> order = fls(pages) - 1;
>> min_order = ilog2(min_page_size) - ilog2(mm->chunk_size);
>>
>> +BUG_ON(order < min_order);
>> +
>> do {
>> order = min(order, (unsigned int)fls(pages) - 1);
>> BUG_ON(order > mm->max_order);
>> -BUG_ON(order < min_order);
>>
>> do {
>> if (flags & DRM_BUDDY_RANGE_ALLOCATION)
>>
>> base-commit: 8025c79350b90e5a8029234d433578f12abbae2b
>
Re: [PATCH v2 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint"
On 08/03/2022 10:49, Robert Foss wrote:
This reverts commit 32568ae37596b529628ac09b875f4874e614f63f.
Signed-off-by: Robert Foss
Reviewed-by: Chen-Yu Tsai
Reviewed-by: Laurent Pinchart
Acked-by: Matthias Brugger
---
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
index e8f133dc96b95..8f7bf33f607da 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
@@ -171,8 +171,6 @@ port@0 {
anx7625_in: endpoint {
remote-endpoint = <&dsi_out>;
- bus-type = <5>;
- data-lanes = <0 1 2 3>;
};
};
[Bug 215669] [drm:gfx_v10_0_priv_reg_irq [amdgpu]] *ERROR* Illegal register access in command stream
https://bugzilla.kernel.org/show_bug.cgi?id=215669 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- This is most likely a mesa bug. I'd suggest moving it to here: https://gitlab.freedesktop.org/groups/mesa/-/issues -- 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: [Intel-gfx] [PATCH] drm: remove min_order BUG_ON check
On 07/03/22 8:15 pm, Jani Nikula wrote:
> On Mon, 07 Mar 2022, Arunpravin wrote:
>> place BUG_ON(order < min_order) outside do..while
>> loop as it fails Unigine Heaven benchmark.
>>
>> Unigine Heaven has buffer allocation requests for
>> example required pages are 161 and alignment request
>> is 128. To allocate the remaining 33 pages, continues
>> the iteration to find the order value which is 5 and
>> when it compares with min_order = 7, enables the
>> BUG_ON(). To avoid this problem, placed the BUG_ON
>> check outside of do..while loop.
>
> How about turning these BUG_ON()s to WARN_ON()s with an error return?
> What's the point in oopsing?
>
yes, we will use WARN_ON with an error return
Thanks,
Arun
> BR,
> Jani.
>
>
>>
>> Signed-off-by: Arunpravin
>> ---
>> drivers/gpu/drm/drm_buddy.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
>> index 72f52f293249..ed94c56b720f 100644
>> --- a/drivers/gpu/drm/drm_buddy.c
>> +++ b/drivers/gpu/drm/drm_buddy.c
>> @@ -669,10 +669,11 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
>> order = fls(pages) - 1;
>> min_order = ilog2(min_page_size) - ilog2(mm->chunk_size);
>>
>> +BUG_ON(order < min_order);
>> +
>> do {
>> order = min(order, (unsigned int)fls(pages) - 1);
>> BUG_ON(order > mm->max_order);
>> -BUG_ON(order < min_order);
>>
>> do {
>> if (flags & DRM_BUDDY_RANGE_ALLOCATION)
>>
>> base-commit: 8025c79350b90e5a8029234d433578f12abbae2b
>
RE: [PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Hi Abhinav, > > Hi, > >>> Hi, > > Hi Abhinav, > >> Hi Laurent > >> > >> Ok sure, I can take this up but I need to understand the proposal > >> a little bit more before proceeding on this. So we will discuss > >> this in another email where we specifically talk about the > >> connector changes. > >> > >> Also, I am willing to take this up once the encoder part is done > >> and merged so that atleast we keep moving on that as MSM WB > >> implementation can proceed with that first. > >> > >> Hi Jani and Suraj > >> > >> Any concerns with splitting up the series into encoder and > >> connector OR re- arranging them? > >> > >> Let me know if you can do this OR I can also split this up > >> keeping Suraj's name in the Co-developed by tag. > > I was actually thinking of floating a new patch series with both > > encoder and connector pointer changes but with a change in > > initialization functions wherein we allocate a connector and > > encoder incase the driver doesn’t have its own this should > > minimize the effect on other drivers already using current drm > > writeback framework and also > allow i915 to create its own connector. > > > I was proposing to split up the encoder and connector because the > comments from Laurent meant we were in agreement about the encoder > but not about the connector. > > I am afraid another attempt with the similar idea is only going to stall the > series again and hence i gave this option. Seems like this patch series currently won't be able to move forward with the connector pointer change so I guess you can split the series to encoder pointer change where we optionally create encoder if required ,keeping my name in co-developed tag so that msm can move forward with its implementation and then we can work to see how the connector issue can be bypassed. Best Regards, Suraj Kandpal > Eventually its upto Laurent and the other maintainers to guide us forward on > this as this series has been stalled for weeks now. > > I thought that Laurent was quite strict about the connector. So I'd > suggest targeting drm_writeback_connector with an optionally > created encoder and the builtin connector > >>> In that case even if we target that i915 will not be able to move > >>> forward with its implementation of writeback as builtin connector > >>> does not work for us right now as explained earlier, optionally > >>> creating encoder and connector will help everyone move forward and > >>> once done > >> we > >>> can actually sit and work on how to side step this issue using > >>> Laurent's suggestion > >> > >> This is up to Laurent & other DRM maintainers to decide whether this > >> approach would be accepted or not. > >> So far unfortunately I have mostly seen the pushback and a strong > >> suggestion to stop treating each drm_connector as i915_connector. > >> I haven't checked the exact details, but IMO adding a branch if the > >> connector's type is DRM_CONNECTOR_VIRTUAL should be doable. > > Bringing in the change where we don’t treat each drm_connector as an > > intel_connector or even adding a branch which checks if drm_connector > > is DRM_CONNECTOR_VIRTUAL and conditionally cast it to intel_connector > > is quite a lot of work for i915. > > That's why I was suggesting if for now we could move forward with > > optionally creating both encoder and connector minimizing affects on > > other drivers as well as allowing us to move forward. > > What do you think, Launrent? > > > >> > > > We can work on Laurent's suggestion but that would first require > > the initial RFC patches and then a rework from both of our drivers > > side to see if they gel well with our current code which will take > > a considerable amount of time. We can for now however float the > > patch series up which minimally affects the current drivers and > > also allows > > i915 and msm to move forward with its writeback implementation off > > course with a proper documentation stating new drivers shouldn't > > try to > make their own connectors and encoders and that drm_writeback will > do that for them. > > Once that's done and merged we can work with Laurent on his > > proposal to improve the drm writeback framework so that this issue > > can be side > stepped entirely in future. > > For now I would like to keep the encoder and connector pointer > > changes > together. > >>> > >>
Re: [PATCH 3/9] fbdev: Track deferred-I/O pages in pageref struct
On 3/3/22 21:58, Thomas Zimmermann wrote:
> Store the per-page state for fbdev's deferred I/O in struct
> fb_deferred_io_pageref. Maintain a list of pagerefs for the pages
> that have to be flushed out to video memory. Update all affected
> drivers.
>
> Like with pages before, fbdev acquires a pageref when an mmaped page
> of the framebuffer is being written to. It holds the pageref in a
> list of all currently written pagerefs until it flushes the written
> pages to video memory. Writeback occurs periodically. After writeback
> fbdev releases all pagerefs and builds up a new dirty list until the
> next writeback occurs.
>
> Using pagerefs has a number of benefits.
>
> For pages of the framebuffer, the deferred I/O code used struct
> page.lru as an entry into the list of dirty pages. The lru field is
> owned by the page cache, which makes deferred I/O incompatible with
> some memory pages (e.g., most notably DRM's GEM SHMEM allocator).
> struct fb_deferred_io_pageref now provides an entry into a list of
> dirty framebuffer pages, free'ing lru for use with the page cache.
>
> Drivers also assumed that struct page.index is the page offset into
> the framebuffer. This is not true for DRM buffers, which are located
> at various offset within a mapped area. struct fb_deferred_io_pageref
> explicitly stores an offset into the framebuffer. struct page.index
> is now only the page offset into the mapped area.
>
> These changes will allow DRM to use fbdev deferred I/O without an
> intermediate shadow buffer.
>
As mentioned, this is a very nice cleanup.
> Signed-off-by: Thomas Zimmermann
> ---
[snip]
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> index 33f355907fbb..a35f695727c9 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> @@ -322,12 +322,13 @@ static void vmw_deferred_io(struct fb_info *info,
> struct vmw_fb_par *par = info->par;
> unsigned long start, end, min, max;
> unsigned long flags;
> - struct page *page;
> + struct fb_deferred_io_pageref *pageref;
> int y1, y2;
>
> min = ULONG_MAX;
> max = 0;
> - list_for_each_entry(page, pagelist, lru) {
> + list_for_each_entry(pageref, pagelist, list) {
> + struct page *page = pageref->page;
> start = page->index << PAGE_SHIFT;
Do you think that may be worth it to also decouple the struct page.index and
store the index as a struct fb_deferred_io_pageref.index field ?
It's unlikely that this would change but it feels as if the layers are more
separated that way, since no driver will access struct page fields directly.
The mapping would be 1:1 anyways just like it's the case for the page offset.
In fact, that could allow to even avoid declaring a struct page *page here.
[snip]
> @@ -340,7 +340,8 @@ static void fbtft_deferred_io(struct fb_info *info,
> struct list_head *pagelist)
> spin_unlock(&par->dirty_lock);
>
> /* Mark display lines as dirty */
> - list_for_each_entry(page, pagelist, lru) {
> + list_for_each_entry(pageref, pagelist, list) {
> + struct page *page = pageref->page;
Same here and the other drivers. You only need the page for the index AFAICT.
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On 3/8/22 14:49, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 02:27:40PM +0100, Marek Vasut wrote:
On 3/8/22 13:51, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
On 3/8/22 11:07, Jagan Teki wrote:
On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
On 3/8/22 09:03, Jagan Teki wrote:
Hi,
[...]
@@ -314,7 +321,9 @@ static const struct drm_bridge_funcs chipone_bridge_funcs =
{
static int chipone_parse_dt(struct chipone *icn)
{
struct device *dev = icn->dev;
+ struct device_node *endpoint;
struct drm_panel *panel;
+ int dsi_lanes;
int ret;
icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
@@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone *icn)
return PTR_ERR(icn->enable_gpio);
}
+ endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
+ dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
+ icn->host_node = of_graph_get_remote_port_parent(endpoint);
+ of_node_put(endpoint);
+
+ if (!icn->host_node)
+ return -ENODEV;
The non-ports-based OF graph returns a -19 example on the Allwinner
Display pipeline in R16 [1].
We need to have a helper to return host_node for non-ports as I have
done it for drm_of_find_bridge.
[1] https://patchwork.amarulasolutions.com/patch/1805/
The link points to a patch marked "DO NOT MERGE", maybe that patch is
missing the DSI host port@0 OF graph link ? Both port@0 and port@1 are
required, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
What is "non-ports-based OF graph" ?
I don't see drm_of_find_bridge() in linux-next , what is that ?
port@0 is optional as some of the DSI host OF-graph represent the
bridge or panel as child nodes instead of ports. (i think dt-binding
has to fix it to make port@0 optional)
The current upstream DT binding document says:
required:
- port@0
- port@1
So port@0 is mandatory.
In the binding, sure, but fundamentally the DT excerpt Jagan provided is
correct. If the bridge supports DCS, there's no reason to use the OF
graph in the first place: the bridge node will be a child node of the
MIPI-DSI controller (and there's no obligation to use the OF-graph for a
MIPI-DSI controller).
I believe port@0 should be made optional (or downright removed if
MIPI-DCS in the only control bus).
That's out of scope of this series anyway, so Jagan can implement patches
for that mode if needed.
Not really? You can't count on the port@0 being there generally
speaking, so you can't count on data-lanes being there either, which
exactly what you're doing in this patch.
I can because the upstream DT bindings currently say port@0 must be
present, see above. If that requirement should be relaxed, sure, but
that's a separate series.
[PATCH 2/2] drm/amdkfd: CRIU Refactor restore BO function
Refactor CRIU restore BO to reduce identation before adding support for
IPC.
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 271 +++
1 file changed, 129 insertions(+), 142 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 789bdfbd3f9b..2c7d76e67ddb 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2094,6 +2094,132 @@ static int criu_restore_devices(struct kfd_process *p,
return ret;
}
+static int criu_restore_memory_of_gpu(struct kfd_process_device *pdd,
+ struct kfd_criu_bo_bucket *bo_bucket,
+ struct kfd_criu_bo_priv_data *bo_priv,
+ struct kgd_mem **kgd_mem)
+{
+ int idr_handle;
+ int ret;
+ const bool criu_resume = true;
+ u64 offset;
+
+ if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL) {
+ if (bo_bucket->size != kfd_doorbell_process_slice(pdd->dev))
+ return -EINVAL;
+
+ offset = kfd_get_process_doorbells(pdd);
+ } else if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)
{
+ /* MMIO BOs need remapped bus address */
+ if (bo_bucket->size != PAGE_SIZE) {
+ pr_err("Invalid page size\n");
+ return -EINVAL;
+ }
+ offset = pdd->dev->adev->rmmio_remap.bus_addr;
+ if (!offset) {
+ pr_err("amdgpu_amdkfd_get_mmio_remap_phys_addr
failed\n");
+ return -ENOMEM;
+ }
+ } else if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
+ offset = bo_priv->user_addr;
+ }
+ /* Create the BO */
+ ret = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(pdd->dev->adev,
bo_bucket->addr,
+ bo_bucket->size,
pdd->drm_priv, kgd_mem,
+ &offset,
bo_bucket->alloc_flags, criu_resume);
+ if (ret) {
+ pr_err("Could not create the BO\n");
+ return ret;
+ }
+ pr_debug("New BO created: size:0x%llx addr:0x%llx offset:0x%llx\n",
+bo_bucket->size, bo_bucket->addr, offset);
+
+ /* Restore previous IDR handle */
+ pr_debug("Restoring old IDR handle for the BO");
+ idr_handle = idr_alloc(&pdd->alloc_idr, *kgd_mem, bo_priv->idr_handle,
+ bo_priv->idr_handle + 1, GFP_KERNEL);
+
+ if (idr_handle < 0) {
+ pr_err("Could not allocate idr\n");
+ amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev,
*kgd_mem, pdd->drm_priv,
+ NULL);
+ return -ENOMEM;
+ }
+
+ if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
+ bo_bucket->restored_offset = KFD_MMAP_TYPE_DOORBELL |
KFD_MMAP_GPU_ID(pdd->dev->id);
+ if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP) {
+ bo_bucket->restored_offset = KFD_MMAP_TYPE_MMIO |
KFD_MMAP_GPU_ID(pdd->dev->id);
+ } else if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_GTT) {
+ bo_bucket->restored_offset = offset;
+ } else if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ bo_bucket->restored_offset = offset;
+ /* Update the VRAM usage count */
+ WRITE_ONCE(pdd->vram_usage, pdd->vram_usage + bo_bucket->size);
+ }
+ return 0;
+}
+
+static int criu_restore_bo(struct kfd_process *p,
+ struct kfd_criu_bo_bucket *bo_bucket,
+ struct kfd_criu_bo_priv_data *bo_priv)
+{
+ struct kfd_process_device *pdd;
+ struct kgd_mem *kgd_mem;
+ int ret;
+ int j;
+
+ pr_debug("Restoring BO size:0x%llx addr:0x%llx gpu_id:0x%x flags:0x%x
idr_handle:0x%x\n",
+bo_bucket->size, bo_bucket->addr, bo_bucket->gpu_id,
bo_bucket->alloc_flags,
+bo_priv->idr_handle);
+
+ pdd = kfd_process_device_data_by_id(p, bo_bucket->gpu_id);
+ if (!pdd) {
+ pr_err("Failed to get pdd\n");
+ return -ENODEV;
+ }
+
+ ret = criu_restore_memory_of_gpu(pdd, bo_bucket, bo_priv, &kgd_mem);
+ if (ret)
+ return ret;
+
+ /* now map these BOs to GPU/s */
+ for (j = 0; j < p->n_pdds; j++) {
+ struct kfd_dev *peer;
+ struct kfd_process_device *peer_pdd;
+
+ if (!bo_priv->mapped_gpuids[j])
+ break;
+
+ peer_pdd = kfd_process_device_data_by_id(p,
bo_priv->mapped_gpuids[j]);
+ if (!peer_pdd)
+ return -EINVAL;
[PATCH 1/2] drm/amdkfd: CRIU remove sync and TLB flush on restore
When the process is getting restored, the queues are not mapped yet, so
there is no VMID assigned for this process and no TLBs to flush.
Signed-off-by: David Yat Sin
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 30 +---
1 file changed, 1 insertion(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 59d3fe269e7c..789bdfbd3f9b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2102,7 +2102,6 @@ static int criu_restore_bos(struct kfd_process *p,
struct kfd_criu_bo_bucket *bo_buckets = NULL;
struct kfd_criu_bo_priv_data *bo_privs = NULL;
const bool criu_resume = true;
- bool flush_tlbs = false;
int ret = 0, j = 0;
uint32_t i = 0;
@@ -2248,7 +2247,6 @@ static int criu_restore_bos(struct kfd_process *p,
for (j = 0; j < p->n_pdds; j++) {
struct kfd_dev *peer;
struct kfd_process_device *peer_pdd;
- bool table_freed = false;
if (!bo_priv->mapped_gpuids[j])
break;
@@ -2268,20 +2266,11 @@ static int criu_restore_bos(struct kfd_process *p,
pr_debug("map mem in restore ioctl -> 0x%llx\n",
((struct kgd_mem *)mem)->va);
ret = amdgpu_amdkfd_gpuvm_map_memory_to_gpu(peer->adev,
- (struct kgd_mem *)mem, peer_pdd->drm_priv,
&table_freed);
+ (struct kgd_mem *)mem, peer_pdd->drm_priv,
NULL);
if (ret) {
pr_err("Failed to map to gpu %d/%d\n", j,
p->n_pdds);
goto exit;
}
- if (table_freed)
- flush_tlbs = true;
- }
-
- ret = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
- (struct kgd_mem *) mem,
true);
- if (ret) {
- pr_debug("Sync memory failed, wait interrupted by user
signal\n");
- goto exit;
}
pr_debug("map memory was successful for the BO\n");
@@ -2296,23 +2285,6 @@ static int criu_restore_bos(struct kfd_process *p,
}
} /* done */
- if (flush_tlbs) {
- /* Flush TLBs after waiting for the page table updates to
complete */
- for (j = 0; j < p->n_pdds; j++) {
- struct kfd_dev *peer;
- struct kfd_process_device *pdd = p->pdds[j];
- struct kfd_process_device *peer_pdd;
-
- peer = kfd_device_by_id(pdd->dev->id);
- if (WARN_ON_ONCE(!peer))
- continue;
- peer_pdd = kfd_get_process_device_data(peer, p);
- if (WARN_ON_ONCE(!peer_pdd))
- continue;
- kfd_flush_tlb(peer_pdd, TLB_FLUSH_LEGACY);
- }
- }
-
/* Copy only the buckets back so user can read
bo_buckets[N].restored_offset */
ret = copy_to_user((void __user *)args->bos,
bo_buckets,
--
2.17.1
Re: [PATCH 2/2] drm/amdkfd: CRIU Refactor restore BO function
Am 2022-03-08 um 10:28 schrieb David Yat Sin:
Refactor CRIU restore BO to reduce identation before adding support for
IPC.
Update the commit message. There is no IPC support on the public branch.
The refactoring is still welcome to improve the readability and
maintainability of the code.
With that fixed, the series is
Reviewed-by: Felix Kuehling
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 271 +++
1 file changed, 129 insertions(+), 142 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 789bdfbd3f9b..2c7d76e67ddb 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2094,6 +2094,132 @@ static int criu_restore_devices(struct kfd_process *p,
return ret;
}
+static int criu_restore_memory_of_gpu(struct kfd_process_device *pdd,
+ struct kfd_criu_bo_bucket *bo_bucket,
+ struct kfd_criu_bo_priv_data *bo_priv,
+ struct kgd_mem **kgd_mem)
+{
+ int idr_handle;
+ int ret;
+ const bool criu_resume = true;
+ u64 offset;
+
+ if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL) {
+ if (bo_bucket->size != kfd_doorbell_process_slice(pdd->dev))
+ return -EINVAL;
+
+ offset = kfd_get_process_doorbells(pdd);
+ } else if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)
{
+ /* MMIO BOs need remapped bus address */
+ if (bo_bucket->size != PAGE_SIZE) {
+ pr_err("Invalid page size\n");
+ return -EINVAL;
+ }
+ offset = pdd->dev->adev->rmmio_remap.bus_addr;
+ if (!offset) {
+ pr_err("amdgpu_amdkfd_get_mmio_remap_phys_addr
failed\n");
+ return -ENOMEM;
+ }
+ } else if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
+ offset = bo_priv->user_addr;
+ }
+ /* Create the BO */
+ ret = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(pdd->dev->adev,
bo_bucket->addr,
+ bo_bucket->size,
pdd->drm_priv, kgd_mem,
+ &offset,
bo_bucket->alloc_flags, criu_resume);
+ if (ret) {
+ pr_err("Could not create the BO\n");
+ return ret;
+ }
+ pr_debug("New BO created: size:0x%llx addr:0x%llx offset:0x%llx\n",
+bo_bucket->size, bo_bucket->addr, offset);
+
+ /* Restore previous IDR handle */
+ pr_debug("Restoring old IDR handle for the BO");
+ idr_handle = idr_alloc(&pdd->alloc_idr, *kgd_mem, bo_priv->idr_handle,
+ bo_priv->idr_handle + 1, GFP_KERNEL);
+
+ if (idr_handle < 0) {
+ pr_err("Could not allocate idr\n");
+ amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, *kgd_mem,
pdd->drm_priv,
+ NULL);
+ return -ENOMEM;
+ }
+
+ if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
+ bo_bucket->restored_offset = KFD_MMAP_TYPE_DOORBELL |
KFD_MMAP_GPU_ID(pdd->dev->id);
+ if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP) {
+ bo_bucket->restored_offset = KFD_MMAP_TYPE_MMIO |
KFD_MMAP_GPU_ID(pdd->dev->id);
+ } else if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_GTT) {
+ bo_bucket->restored_offset = offset;
+ } else if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ bo_bucket->restored_offset = offset;
+ /* Update the VRAM usage count */
+ WRITE_ONCE(pdd->vram_usage, pdd->vram_usage + bo_bucket->size);
+ }
+ return 0;
+}
+
+static int criu_restore_bo(struct kfd_process *p,
+ struct kfd_criu_bo_bucket *bo_bucket,
+ struct kfd_criu_bo_priv_data *bo_priv)
+{
+ struct kfd_process_device *pdd;
+ struct kgd_mem *kgd_mem;
+ int ret;
+ int j;
+
+ pr_debug("Restoring BO size:0x%llx addr:0x%llx gpu_id:0x%x flags:0x%x
idr_handle:0x%x\n",
+bo_bucket->size, bo_bucket->addr, bo_bucket->gpu_id,
bo_bucket->alloc_flags,
+bo_priv->idr_handle);
+
+ pdd = kfd_process_device_data_by_id(p, bo_bucket->gpu_id);
+ if (!pdd) {
+ pr_err("Failed to get pdd\n");
+ return -ENODEV;
+ }
+
+ ret = criu_restore_memory_of_gpu(pdd, bo_bucket, bo_priv, &kgd_mem);
+ if (ret)
+ return ret;
+
+ /* now map these BOs to GPU/s */
+ for (j = 0; j < p->n_pdds; j++) {
+ struct kfd_dev *peer;
+
Re: Mandatory Test Suite for KMS Drivers?
Hi Rodrigo, On Thu, Mar 03, 2022 at 05:52:57AM -0500, Rodrigo Vivi wrote: > On Thu, Mar 03, 2022 at 10:05:07AM +0100, Maxime Ripard wrote: > > Hi, > > > > Back at XDC we floated the idea of creating a test suite for IGT that we > > expect any KMS driver to pass, similar to what v4l2-compliance and > > cec-compliance provide for v4l2 and CEC respectively. > > > > I was looking at the list of tests, and it's fairly massive, so it's not > > clear to me what tests we could start this suite with. I can only assume > > all the KMS (but the chamelium ones) and fbdev related ones would be a > > good start? > > > > What do you think? > > I believe we should start with the group of the tests that we know that > are reliably passing today on most of the platforms and then increase > the list as the tests and drivers become more reliable. I can see why that would be an objective too, but I'm not sure it would cover mine. What I'd like this series to be is something we can ask upfront to new drivers being submitted to make sure that they are sane. Whether or not old drivers pass that bar is a bit irrelevant to that objective (and this would actually create tasks for newcomers that are looking for something to work on). So, yeah, I don't mind having failing tests on older drivers, I kind of even expect them to fail somehow. It would essentially be a bar to show what any driver should strive for, not the lowest common denominator. Does that make sense? > For instance, many of these would be candidate to be filtered out for now > https://intel-gfx-ci.01.org/tree/drm-next/index.html?testfilter=kms > > compared to the whole view of kms tests: > https://intel-gfx-ci.01.org/tree/drm-next/shards-all.html?testfilter=kms So the set of tests that are always run would be the latter, right? Maxime signature.asc Description: PGP signature
Re: [igt-dev] Mandatory Test Suite for KMS Drivers?
Hi Daniel, On Fri, Mar 04, 2022 at 08:45:07AM +, Daniel Stone wrote: > On Thu, 3 Mar 2022 at 10:53, Rodrigo Vivi wrote: > > On Thu, Mar 03, 2022 at 10:05:07AM +0100, Maxime Ripard wrote: > > > Back at XDC we floated the idea of creating a test suite for IGT that we > > > expect any KMS driver to pass, similar to what v4l2-compliance and > > > cec-compliance provide for v4l2 and CEC respectively. > > > > > > I was looking at the list of tests, and it's fairly massive, so it's not > > > clear to me what tests we could start this suite with. I can only assume > > > all the KMS (but the chamelium ones) and fbdev related ones would be a > > > good start? > > > > > > What do you think? > > > > I believe we should start with the group of the tests that we know that > > are reliably passing today on most of the platforms and then increase > > the list as the tests and drivers become more reliable. > > > > For instance, many of these would be candidate to be filtered out for now > > https://intel-gfx-ci.01.org/tree/drm-next/index.html?testfilter=kms > > > > compared to the whole view of kms tests: > > https://intel-gfx-ci.01.org/tree/drm-next/shards-all.html?testfilter=kms > > We are running some of IGT on Panfrost + amdgpu + i915 as part of > KernelCI as well: go to https://linux.kernelci.org/test/ and search > for 'igt-gpu'. This gets run for mainline, next, and whatever other > kernel trees push into i915. I was mainly interested in KMS, but I saw that there's an igt-kms test as well, thanks! > There is a Grafana-based dashboard that the KernelCI team have been > working on to visualise test runs, but it's currently having some > backend issues so I can't show you a link for that. I did raise a > suggestion in their design discussion for a proper testing dashboard > for making it easier to see the status, so feel free to pile in there: > https://github.com/kernelci/kernelci-project/discussions/28#discussioncomment-2293696 I'll have a look, thanks Maxime signature.asc Description: PGP signature
Re: [PATCH v1 5/5] drm/virtio: Add memory shrinker
On 3/8/22 16:17, Dmitry Osipenko wrote: > @@ -246,20 +246,28 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane > *plane, > struct virtio_gpu_device *vgdev = dev->dev_private; > struct virtio_gpu_framebuffer *vgfb; > struct virtio_gpu_object *bo; > + int err; > > if (!new_state->fb) > return 0; > > vgfb = to_virtio_gpu_framebuffer(new_state->fb); > bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); > - if (!bo || (plane->type == DRM_PLANE_TYPE_PRIMARY && !bo->guest_blob)) > + > + err = virtio_gpu_gem_pin(bo); > + if (err) > + return err; I just noticed that this produces a refcount debug warning because I missed to initialize the refcount when BO is created. That warning splat was hidden by a huge lockdep splat produced by drm_aperture_remove_conflicting_pci_framebuffers(), which probably should be fixed. I'll correct it in v2.
[PATCH 1/2] drm: ssd130x: Fix COM scan direction register mask
From: Chen-Yu Tsai
The SSD130x's command to toggle COM scan direction uses bit 3 and only
bit 3 to set the direction of the scanout. The driver has an incorrect
GENMASK(3, 2), causing the setting to be set on bit 2, rendering it
ineffective.
Fix the mask to only bit 3, so that the requested setting is applied
correctly.
Fixes: a61732e80867 ("drm: Add driver for Solomon SSD130x OLED displays")
Signed-off-by: Chen-Yu Tsai
---
drivers/gpu/drm/solomon/ssd130x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x.c
b/drivers/gpu/drm/solomon/ssd130x.c
index ce4dc20412e0..ccd378135589 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -61,7 +61,7 @@
#define SSD130X_SET_COM_PINS_CONFIG0xda
#define SSD130X_SET_VCOMH 0xdb
-#define SSD130X_SET_COM_SCAN_DIR_MASK GENMASK(3, 2)
+#define SSD130X_SET_COM_SCAN_DIR_MASK GENMASK(3, 3)
#define SSD130X_SET_COM_SCAN_DIR_SET(val)
FIELD_PREP(SSD130X_SET_COM_SCAN_DIR_MASK, (val))
#define SSD130X_SET_CLOCK_DIV_MASK GENMASK(3, 0)
#define SSD130X_SET_CLOCK_DIV_SET(val)
FIELD_PREP(SSD130X_SET_CLOCK_DIV_MASK, (val))
--
2.34.1
[PATCH 2/2] drm: ssd130x: Always apply segment remap setting
From: Chen-Yu Tsai
Currently the ssd130x driver only sets the segment remap setting when
the device tree requests it; it however does not clear the setting if
it is not requested. This leads to the setting incorrectly persisting
if the hardware is always on and has no reset GPIO wired. This might
happen when a developer is trying to find the correct settings for an
unknown module, and cause the developer to get confused because the
settings from the device tree are not consistently applied.
Make the driver apply the segment remap setting consistently, setting
the value correctly based on the device tree setting. This also makes
this setting's behavior consistent with the other settings, which are
always applied.
Fixes: a61732e80867 ("drm: Add driver for Solomon SSD130x OLED displays")
Signed-off-by: Chen-Yu Tsai
---
drivers/gpu/drm/solomon/ssd130x.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x.c
b/drivers/gpu/drm/solomon/ssd130x.c
index ccd378135589..d08d86ef07bc 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -48,7 +48,7 @@
#define SSD130X_CONTRAST 0x81
#define SSD130X_SET_LOOKUP_TABLE 0x91
#define SSD130X_CHARGE_PUMP0x8d
-#define SSD130X_SEG_REMAP_ON 0xa1
+#define SSD130X_SET_SEG_REMAP 0xa0
#define SSD130X_DISPLAY_OFF0xae
#define SSD130X_SET_MULTIPLEX_RATIO0xa8
#define SSD130X_DISPLAY_ON 0xaf
@@ -61,6 +61,8 @@
#define SSD130X_SET_COM_PINS_CONFIG0xda
#define SSD130X_SET_VCOMH 0xdb
+#define SSD130X_SET_SEG_REMAP_MASK GENMASK(0, 0)
+#define SSD130X_SET_SEG_REMAP_SET(val)
FIELD_PREP(SSD130X_SET_SEG_REMAP_MASK, (val))
#define SSD130X_SET_COM_SCAN_DIR_MASK GENMASK(3, 3)
#define SSD130X_SET_COM_SCAN_DIR_SET(val)
FIELD_PREP(SSD130X_SET_COM_SCAN_DIR_MASK, (val))
#define SSD130X_SET_CLOCK_DIV_MASK GENMASK(3, 0)
@@ -235,7 +237,7 @@ static void ssd130x_power_off(struct ssd130x_device
*ssd130x)
static int ssd130x_init(struct ssd130x_device *ssd130x)
{
- u32 precharge, dclk, com_invdir, compins, chargepump;
+ u32 precharge, dclk, com_invdir, compins, chargepump, seg_remap;
int ret;
/* Set initial contrast */
@@ -244,11 +246,11 @@ static int ssd130x_init(struct ssd130x_device *ssd130x)
return ret;
/* Set segment re-map */
- if (ssd130x->seg_remap) {
- ret = ssd130x_write_cmd(ssd130x, 1, SSD130X_SEG_REMAP_ON);
- if (ret < 0)
- return ret;
- }
+ seg_remap = (SSD130X_SET_SEG_REMAP |
+SSD130X_SET_SEG_REMAP_SET(ssd130x->seg_remap));
+ ret = ssd130x_write_cmd(ssd130x, 1, seg_remap);
+ if (ret < 0)
+ return ret;
/* Set COM direction */
com_invdir = (SSD130X_SET_COM_SCAN_DIR |
--
2.34.1
Re: [PATCH 0/3] Fix MediaTek display dt-bindings issues
On Fri, Mar 4, 2022 at 5:49 PM Chun-Kuang Hu wrote: > > Hi, Rob: > > Would you like to take this series into your tree, or I take this > series into my tree? I can't. I don't have the broken commits in my tree. Rob
Re: [PATCH v6 2/6] drm/format-helper: Add drm_fb_xrgb8888_to_mono_reversed()
Hi Javier,
On Mon, Feb 14, 2022 at 2:37 PM Javier Martinez Canillas
wrote:
> Add support to convert from XR24 to reversed monochrome for drivers that
> control monochromatic display panels, that only have 1 bit per pixel.
>
> The function does a line-by-line conversion doing an intermediate step
> first from XR24 to 8-bit grayscale and then to reversed monochrome.
>
> The drm_fb_gray8_to_mono_reversed_line() helper was based on code from
> drivers/gpu/drm/tiny/repaper.c driver.
>
> Signed-off-by: Javier Martinez Canillas
> Reviewed-by: Thomas Zimmermann
> Reviewed-by: Andy Shevchenko
Thanks for your patch, which is now commit bcf8b616deb87941
("drm/format-helper: Add drm_fb_xrgb_to_mono_reversed()")
in drm/drm-next.
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -12,9 +12,11 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> +#include
> #include
>
> static unsigned int clip_offset(const struct drm_rect *clip, unsigned int
> pitch, unsigned int cpp)
> @@ -591,3 +593,111 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int
> dst_pitch, uint32_t dst_for
> return -EINVAL;
> }
> EXPORT_SYMBOL(drm_fb_blit_toio);
> +
> +static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src,
> unsigned int pixels,
"pixels" is not the number of pixels to process, but the number of
bytes in the monochrome destination buffer.
> + unsigned int start_offset,
> unsigned int end_len)
> +{
> + unsigned int xb, i;
> +
> + for (xb = 0; xb < pixels; xb++) {
> + unsigned int start = 0, end = 8;
> + u8 byte = 0x00;
> +
> + if (xb == 0 && start_offset)
> + start = start_offset;
> +
> + if (xb == pixels - 1 && end_len)
> + end = end_len;
> +
> + for (i = start; i < end; i++) {
> + unsigned int x = xb * 8 + i;
> +
> + byte >>= 1;
> + if (src[x] >> 7)
> + byte |= BIT(7);
> + }
> + *dst++ = byte;
> + }
The above is IMHO very hard to read.
I think it can be made simpler by passing the total number of pixels
to process instead of "pixels" (which is bytes) and "end_len".
> +}
> +
> +/**
> + * drm_fb_xrgb_to_mono_reversed - Convert XRGB to reversed monochrome
> + * @dst: reversed monochrome destination buffer
> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
> + * @src: XRGB source buffer
> + * @fb: DRM framebuffer
> + * @clip: Clip rectangle area to copy
> + *
> + * DRM doesn't have native monochrome support.
> + * Such drivers can announce the commonly supported XR24 format to userspace
> + * and use this function to convert to the native format.
> + *
> + * This function uses drm_fb_xrgb_to_gray8() to convert to grayscale and
> + * then the result is converted from grayscale to reversed monohrome.
> + */
> +void drm_fb_xrgb_to_mono_reversed(void *dst, unsigned int dst_pitch,
> const void *vaddr,
> + const struct drm_framebuffer *fb, const
> struct drm_rect *clip)
> +{
> + unsigned int linepixels = drm_rect_width(clip);
> + unsigned int lines = clip->y2 - clip->y1;
drm_rect_height(clip)?
> + unsigned int cpp = fb->format->cpp[0];
> + unsigned int len_src32 = linepixels * cpp;
> + struct drm_device *dev = fb->dev;
> + unsigned int start_offset, end_len;
> + unsigned int y;
> + u8 *mono = dst, *gray8;
> + u32 *src32;
> +
> + if (drm_WARN_ON(dev, fb->format->format != DRM_FORMAT_XRGB))
> + return;
> +
> + /*
> +* The reversed mono destination buffer contains 1 bit per pixel
> +* and destination scanlines have to be in multiple of 8 pixels.
> +*/
> + if (!dst_pitch)
> + dst_pitch = DIV_ROUND_UP(linepixels, 8);
This is not correct when clip->x1 is not a multiple of 8 pixels.
Should be:
DIV_ROUND_UP(linepixels + clip->x1 % 8, 8);
> +
> + drm_WARN_ONCE(dev, dst_pitch % 8 != 0, "dst_pitch is not a multiple
> of 8\n");
This triggers for me: dst_pitch = 1.
Which is perfectly fine, when flashing an 8-pixel wide cursor ;-)
> +
> + /*
> +* The cma memory is write-combined so reads are uncached.
> +* Speed up by fetching one line at a time.
> +*
> +* Also, format conversion from XR24 to reversed monochrome
> +* are done line-by-line but are converted to 8-bit grayscale
> +* as an intermediate step.
> +*
> +* Allocate a buffer to be used for both copying from the cma
> +* memory and to store the intermediate grayscale line pixels.
> +*/
> + src32 = kmalloc(len_src32 + linepixels, GFP_KERNEL);
> + if (!src32)
> +
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On Tue, Mar 08, 2022 at 03:47:22PM +0100, Marek Vasut wrote:
> On 3/8/22 14:49, Maxime Ripard wrote:
> > On Tue, Mar 08, 2022 at 02:27:40PM +0100, Marek Vasut wrote:
> > > On 3/8/22 13:51, Maxime Ripard wrote:
> > > > On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
> > > > > On 3/8/22 11:07, Jagan Teki wrote:
> > > > > > On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
> > > > > > >
> > > > > > > On 3/8/22 09:03, Jagan Teki wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > [...]
> > > > > > >
> > > > > > > > > @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs
> > > > > > > > > chipone_bridge_funcs = {
> > > > > > > > > static int chipone_parse_dt(struct chipone *icn)
> > > > > > > > > {
> > > > > > > > > struct device *dev = icn->dev;
> > > > > > > > > + struct device_node *endpoint;
> > > > > > > > > struct drm_panel *panel;
> > > > > > > > > + int dsi_lanes;
> > > > > > > > > int ret;
> > > > > > > > >
> > > > > > > > > icn->vdd1 = devm_regulator_get_optional(dev,
> > > > > > > > > "vdd1");
> > > > > > > > > @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct
> > > > > > > > > chipone *icn)
> > > > > > > > > return PTR_ERR(icn->enable_gpio);
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > + endpoint =
> > > > > > > > > of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
> > > > > > > > > + dsi_lanes = of_property_count_u32_elems(endpoint,
> > > > > > > > > "data-lanes");
> > > > > > > > > + icn->host_node =
> > > > > > > > > of_graph_get_remote_port_parent(endpoint);
> > > > > > > > > + of_node_put(endpoint);
> > > > > > > > > +
> > > > > > > > > + if (!icn->host_node)
> > > > > > > > > + return -ENODEV;
> > > > > > > >
> > > > > > > > The non-ports-based OF graph returns a -19 example on the
> > > > > > > > Allwinner
> > > > > > > > Display pipeline in R16 [1].
> > > > > > > >
> > > > > > > > We need to have a helper to return host_node for non-ports as I
> > > > > > > > have
> > > > > > > > done it for drm_of_find_bridge.
> > > > > > > >
> > > > > > > > [1] https://patchwork.amarulasolutions.com/patch/1805/
> > > > > > >
> > > > > > > The link points to a patch marked "DO NOT MERGE", maybe that
> > > > > > > patch is
> > > > > > > missing the DSI host port@0 OF graph link ? Both port@0 and
> > > > > > > port@1 are
> > > > > > > required, see:
> > > > > > >
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
> > > > > > >
> > > > > > > What is "non-ports-based OF graph" ?
> > > > > > >
> > > > > > > I don't see drm_of_find_bridge() in linux-next , what is that ?
> > > > > >
> > > > > > port@0 is optional as some of the DSI host OF-graph represent the
> > > > > > bridge or panel as child nodes instead of ports. (i think dt-binding
> > > > > > has to fix it to make port@0 optional)
> > > > >
> > > > > The current upstream DT binding document says:
> > > > >
> > > > > required:
> > > > > - port@0
> > > > > - port@1
> > > > >
> > > > > So port@0 is mandatory.
> > > >
> > > > In the binding, sure, but fundamentally the DT excerpt Jagan provided is
> > > > correct. If the bridge supports DCS, there's no reason to use the OF
> > > > graph in the first place: the bridge node will be a child node of the
> > > > MIPI-DSI controller (and there's no obligation to use the OF-graph for a
> > > > MIPI-DSI controller).
> > > >
> > > > I believe port@0 should be made optional (or downright removed if
> > > > MIPI-DCS in the only control bus).
> > >
> > > That's out of scope of this series anyway, so Jagan can implement patches
> > > for that mode if needed.
> >
> > Not really? You can't count on the port@0 being there generally
> > speaking, so you can't count on data-lanes being there either, which
> > exactly what you're doing in this patch.
>
> I can because the upstream DT bindings currently say port@0 must be present,
> see above. If that requirement should be relaxed, sure, but that's a
> separate series.
And another upstream DT bindings say that you don't need them at all.
Yes, there's a conflict. Yes, it's unfortunate. But the generic DSI
binding is far more relevant than a single bridge driver.
So figuring it out is very much a prerequisite to that series,
especially since those patches effectively make the OF-graph mandatory
in some situations, while it was purely aesthetics before.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On Tue, Mar 8, 2022 at 5:17 AM Dmitry Osipenko wrote: > > Hello, > > This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. > During OOM, the shrinker will release BOs that are marked as "not needed" > by userspace using the new madvise IOCTL. The userspace in this case is > the Mesa VirGL driver, it will mark the cached BOs as "not needed", > allowing kernel driver to release memory of the cached shmem BOs on lowmem > situations, preventing OOM kills. Will host memory pressure already trigger shrinker in guest? This is something I'm quite interested in for "virtgpu native contexts" (ie. native guest driver with new context type sitting on top of virtgpu), since that isn't using host storage BR, -R > This patchset includes couple fixes for problems I found while was working > on the shrinker, it also includes prerequisite DMA API usage improvement > needed by the shrinker. > > The Mesa and IGT patches will be kept on hold until this kernel series > will be approved and applied. > > This patchset was tested using Qemu and crosvm, including both cases of > IOMMU off/on. > > Mesa: https://gitlab.freedesktop.org/digetx/mesa/-/commits/virgl-madvise > IGT: > https://gitlab.freedesktop.org/digetx/igt-gpu-tools/-/tree/virtio-madvise > > Dmitry Osipenko (5): > drm/virtio: Correct drm_gem_shmem_get_sg_table() error handling > drm/virtio: Check whether transferred 2D BO is shmem > drm/virtio: Unlock GEM reservations in error code path > drm/virtio: Improve DMA API usage for shmem BOs > drm/virtio: Add memory shrinker > > drivers/gpu/drm/virtio/Makefile | 3 +- > drivers/gpu/drm/virtio/virtgpu_drv.c | 22 +++- > drivers/gpu/drm/virtio/virtgpu_drv.h | 31 - > drivers/gpu/drm/virtio/virtgpu_gem.c | 84 > drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c | 124 ++ > drivers/gpu/drm/virtio/virtgpu_ioctl.c| 37 ++ > drivers/gpu/drm/virtio/virtgpu_kms.c | 17 ++- > drivers/gpu/drm/virtio/virtgpu_object.c | 63 +++-- > drivers/gpu/drm/virtio/virtgpu_plane.c| 17 ++- > drivers/gpu/drm/virtio/virtgpu_vq.c | 30 +++-- > include/uapi/drm/virtgpu_drm.h| 14 ++ > 11 files changed, 373 insertions(+), 69 deletions(-) > create mode 100644 drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c > > -- > 2.35.1 >
Re: [PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
Hi Javier,
On Mon, Feb 14, 2022 at 2:37 PM Javier Martinez Canillas
wrote:
> This adds a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon
> OLED display controllers.
>
> It's only the core part of the driver and a bus specific driver is needed
> for each transport interface supported by the display controllers.
>
> Signed-off-by: Javier Martinez Canillas
Thanks for your patch, which is now commit a61732e808672cfa ("drm:
Add driver for Solomon SSD130x OLED displays") in drm/drm-next
Sorry for the delay, but finally I gave it a try on my Adafruit
FeatherWing 128x32 OLED.
Some of the weird issues (cursor disappears after printing some text,
more text also doesn't appear until I clear the display) are still there.
Unfortunately a regression was introduced since your v3: printed
text is mirrored upside-down. I.e. "E" is rendered correctly, but "L"
turns into "Γ" (Greek Gamma).
I suspect something went wrong with the display initialization
sequence.
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
[PATCH] drm/i915/guc: Use iosys_map interface to update lrc_desc
This patch is continuation of the effort to move all pointers in i915,
which at any point may be pointing to device memory or system memory, to
iosys_map interface.
More details about the need of this change is explained in the patch
series which initiated this task
https://patchwork.freedesktop.org/series/99711/
This patch converts all access to the lrc_desc through iosys_map
interfaces.
Cc: Lucas De Marchi
Cc: John Harrison
Cc: Matthew Brost
Cc: Umesh Nerlige Ramappa
Signed-off-by: Balasubramani Vivekanandan
---
drivers/gpu/drm/i915/gt/uc/intel_guc.h| 2 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 68 ---
2 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index e439e6c1ac8b..cbbc24dbaf0f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -168,7 +168,7 @@ struct intel_guc {
/** @lrc_desc_pool: object allocated to hold the GuC LRC descriptor
pool */
struct i915_vma *lrc_desc_pool;
/** @lrc_desc_pool_vaddr: contents of the GuC LRC descriptor pool */
- void *lrc_desc_pool_vaddr;
+ struct iosys_map lrc_desc_pool_vaddr;
/**
* @context_lookup: used to resolve intel_context from guc_id, if a
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 9ec03234d2c2..84b17ded886a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -467,13 +467,14 @@ static u32 *get_wq_pointer(struct guc_process_desc *desc,
return &__get_parent_scratch(ce)->wq[ce->parallel.guc.wqi_tail /
sizeof(u32)];
}
-static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index)
+static void __write_lrc_desc(struct intel_guc *guc, u32 index,
+struct guc_lrc_desc *desc)
{
- struct guc_lrc_desc *base = guc->lrc_desc_pool_vaddr;
+ unsigned int size = sizeof(struct guc_lrc_desc);
GEM_BUG_ON(index >= GUC_MAX_CONTEXT_ID);
- return &base[index];
+ iosys_map_memcpy_to(&guc->lrc_desc_pool_vaddr, index * size, desc,
size);
}
static inline struct intel_context *__get_context(struct intel_guc *guc, u32
id)
@@ -489,20 +490,28 @@ static int guc_lrc_desc_pool_create(struct intel_guc *guc)
{
u32 size;
int ret;
+ void *addr;
size = PAGE_ALIGN(sizeof(struct guc_lrc_desc) *
GUC_MAX_CONTEXT_ID);
ret = intel_guc_allocate_and_map_vma(guc, size, &guc->lrc_desc_pool,
-(void
**)&guc->lrc_desc_pool_vaddr);
+&addr);
+
if (ret)
return ret;
+ if (i915_gem_object_is_lmem(guc->lrc_desc_pool->obj))
+ iosys_map_set_vaddr_iomem(&guc->lrc_desc_pool_vaddr,
+ (void __iomem *)addr);
+ else
+ iosys_map_set_vaddr(&guc->lrc_desc_pool_vaddr, addr);
+
return 0;
}
static void guc_lrc_desc_pool_destroy(struct intel_guc *guc)
{
- guc->lrc_desc_pool_vaddr = NULL;
+ iosys_map_clear(&guc->lrc_desc_pool_vaddr);
i915_vma_unpin_and_release(&guc->lrc_desc_pool, I915_VMA_RELEASE_MAP);
}
@@ -513,9 +522,11 @@ static inline bool guc_submission_initialized(struct
intel_guc *guc)
static inline void _reset_lrc_desc(struct intel_guc *guc, u32 id)
{
- struct guc_lrc_desc *desc = __get_lrc_desc(guc, id);
+ unsigned int size = sizeof(struct guc_lrc_desc);
- memset(desc, 0, sizeof(*desc));
+ GEM_BUG_ON(id >= GUC_MAX_CONTEXT_ID);
+
+ iosys_map_memset(&guc->lrc_desc_pool_vaddr, id * size, 0, size);
}
static inline bool ctx_id_mapped(struct intel_guc *guc, u32 id)
@@ -2233,7 +2244,7 @@ static void prepare_context_registration_info(struct
intel_context *ce)
struct intel_engine_cs *engine = ce->engine;
struct intel_guc *guc = &engine->gt->uc.guc;
u32 ctx_id = ce->guc_id.id;
- struct guc_lrc_desc *desc;
+ struct guc_lrc_desc desc;
struct intel_context *child;
GEM_BUG_ON(!engine->mask);
@@ -2245,13 +2256,13 @@ static void prepare_context_registration_info(struct
intel_context *ce)
GEM_BUG_ON(i915_gem_object_is_lmem(guc->ct.vma->obj) !=
i915_gem_object_is_lmem(ce->ring->vma->obj));
- desc = __get_lrc_desc(guc, ctx_id);
- desc->engine_class = engine_class_to_guc_class(engine->class);
- desc->engine_submit_mask = engine->logical_mask;
- desc->hw_context_desc = ce->lrc.lrca;
- desc->priority = ce->guc_state.prio;
- desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
- guc_context_policy_init(engine, desc);
+ memset(&desc, 0, sizeof(desc));
+ desc.engine_class = engine_class_to_guc_class(eng
[PATCH v5 1/5] arm64/dts/qcom/sc7280: remove assigned-clock-rate property for mdp clk
Kernel clock driver assumes that initial rate is the
max rate for that clock and was not allowing it to scale
beyond the assigned clock value.
Drop the assigned clock rate property and vote on the mdp clock as per
calculated value during the usecase.
Changes in v2:
- Remove assigned-clock-rate property and set mdp clk during resume sequence.
- Add fixes tag.
Changes in v3:
- Remove extra line after fixes tag.(Stephen Boyd)
Fixes: 62fbdce91("arm64: dts: qcom: sc7280: add display dt nodes")
Signed-off-by: Vinod Polimera
Reviewed-by: Stephen Boyd
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index baf1653..408cf6c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2856,9 +2856,6 @@
"ahb",
"core";
- assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
- assigned-clock-rates = <3>;
-
interrupts = ;
interrupt-controller;
#interrupt-cells = <1>;
@@ -2892,11 +2889,9 @@
"lut",
"core",
"vsync";
- assigned-clocks = <&dispcc
DISP_CC_MDSS_MDP_CLK>,
- <&dispcc
DISP_CC_MDSS_VSYNC_CLK>,
+ assigned-clocks = <&dispcc
DISP_CC_MDSS_VSYNC_CLK>,
<&dispcc DISP_CC_MDSS_AHB_CLK>;
- assigned-clock-rates = <3>,
- <1920>,
+ assigned-clock-rates = <1920>,
<1920>;
operating-points-v2 = <&mdp_opp_table>;
power-domains = <&rpmhpd SC7280_CX>;
--
2.7.4
[PATCH v5 5/5] drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table during probe
use max clock during probe/bind sequence from the opp table. The clock will be scaled down when framework sends an update. Signed-off-by: Vinod Polimera --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index d550f90..d9922b9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1221,6 +1221,7 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) struct dpu_kms *dpu_kms; struct dss_module_power *mp; int ret = 0; + unsigned long max_freq = ULONG_MAX; dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL); if (!dpu_kms) @@ -1243,6 +1244,8 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) return ret; } + dev_pm_opp_find_freq_floor(dev, &max_freq); + dev_pm_opp_set_rate(dev, max_freq); platform_set_drvdata(pdev, dpu_kms); ret = msm_kms_init(&dpu_kms->base, &kms_funcs); -- 2.7.4
[PATCH v5 4/5] arm64/dts/qcom/sm8250: remove assigned-clock-rate property for mdp clk
Kernel clock driver assumes that initial rate is the
max rate for that clock and was not allowing it to scale
beyond the assigned clock value.
Drop the assigned clock rate property and vote on the mdp clock as per
calculated value during the usecase.
Fixes: 7c1dffd471("arm64: dts: qcom: sm8250.dtsi: add display system nodes")
Signed-off-by: Vinod Polimera
Reviewed-by: Stephen Boyd
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index fdaf303..2105eb7 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3164,9 +3164,6 @@
<&dispcc DISP_CC_MDSS_MDP_CLK>;
clock-names = "iface", "bus", "nrt_bus", "core";
- assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
- assigned-clock-rates = <46000>;
-
interrupts = ;
interrupt-controller;
#interrupt-cells = <1>;
@@ -3191,10 +3188,8 @@
<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
clock-names = "iface", "bus", "core", "vsync";
- assigned-clocks = <&dispcc
DISP_CC_MDSS_MDP_CLK>,
- <&dispcc
DISP_CC_MDSS_VSYNC_CLK>;
- assigned-clock-rates = <46000>,
- <1920>;
+ assigned-clocks = <&dispcc
DISP_CC_MDSS_VSYNC_CLK>;
+ assigned-clock-rates = <1920>;
operating-points-v2 = <&mdp_opp_table>;
power-domains = <&rpmhpd SM8250_MMCX>;
--
2.7.4
[PATCH v5 3/5] arm64/dts/qcom/sdm845: remove assigned-clock-rate property for mdp clk
Kernel clock driver assumes that initial rate is the
max rate for that clock and was not allowing it to scale
beyond the assigned clock value.
Drop the assigned clock rate property and vote on the mdp clock as per
calculated value during the usecase.
Fixes: 08c2a076d1("arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file")
Signed-off-by: Vinod Polimera
Reviewed-by: Stephen Boyd
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0d6286d..80dc486 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -4181,9 +4181,6 @@
<&dispcc DISP_CC_MDSS_MDP_CLK>;
clock-names = "iface", "core";
- assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
- assigned-clock-rates = <3>;
-
interrupts = ;
interrupt-controller;
#interrupt-cells = <1>;
@@ -4214,10 +4211,8 @@
<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
clock-names = "gcc-bus", "iface", "bus",
"core", "vsync";
- assigned-clocks = <&dispcc
DISP_CC_MDSS_MDP_CLK>,
- <&dispcc
DISP_CC_MDSS_VSYNC_CLK>;
- assigned-clock-rates = <3>,
- <1920>;
+ assigned-clocks = <&dispcc
DISP_CC_MDSS_VSYNC_CLK>;
+ assigned-clock-rates = <1920>;
operating-points-v2 = <&mdp_opp_table>;
power-domains = <&rpmhpd SDM845_CX>;
--
2.7.4
[PATCH v5 2/5] arm64/dts/qcom/sc7180: remove assigned-clock-rate property for mdp clk
Kernel clock driver assumes that initial rate is the
max rate for that clock and was not allowing it to scale
beyond the assigned clock value.
Drop the assigned clock rate property and vote on the mdp clock as per
calculated value during the usecase.
Fixes: a3db7ad1af("arm64: dts: qcom: sc7180: add display dt nodes")
Signed-off-by: Vinod Polimera
Reviewed-by: Stephen Boyd
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index e1c46b8..eaab746 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2900,9 +2900,6 @@
<&dispcc DISP_CC_MDSS_MDP_CLK>;
clock-names = "iface", "ahb", "core";
- assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
- assigned-clock-rates = <3>;
-
interrupts = ;
interrupt-controller;
#interrupt-cells = <1>;
@@ -2932,12 +2929,10 @@
<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
clock-names = "bus", "iface", "rot", "lut",
"core",
"vsync";
- assigned-clocks = <&dispcc
DISP_CC_MDSS_MDP_CLK>,
- <&dispcc
DISP_CC_MDSS_VSYNC_CLK>,
+ assigned-clocks = <&dispcc
DISP_CC_MDSS_VSYNC_CLK>,
<&dispcc
DISP_CC_MDSS_ROT_CLK>,
<&dispcc
DISP_CC_MDSS_AHB_CLK>;
- assigned-clock-rates = <3>,
- <1920>,
+ assigned-clock-rates = <1920>,
<1920>,
<1920>;
operating-points-v2 = <&mdp_opp_table>;
--
2.7.4
[PATCH v5 0/5] Update mdp clk to max supported value to support higher refresh rates
Kernel clock driver assumes that initial rate is the max rate for that clock and was not allowing it to scale beyond the assigned clock value. Drop the assigned clock rate property and vote on the mdp clock as per calculated value during the usecase. Changes in v2: - Remove assigned-clock-rate property and set mdp clk during resume sequence. - Add fixes tag. Changes in v3: - Remove extra line after fixes tag.(Stephen Boyd) - Add similar changes for sc7180, sdm845 which uses opp table for voting mdp clk.(Stephen Boyd) - Drop patch: "drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table" Changes in v4: - Add similar change for sm8250.(Dmitry) Changes in v5: - Add change to set mdp clk to max frequency in opp table during mdp probe/bind. Vinod Polimera (5): arm64/dts/qcom/sc7280: remove assigned-clock-rate property for mdp clk arm64/dts/qcom/sc7180: remove assigned-clock-rate property for mdp clk arm64/dts/qcom/sdm845: remove assigned-clock-rate property for mdp clk arm64/dts/qcom/sm8250: remove assigned-clock-rate property for mdp clk drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table during probe arch/arm64/boot/dts/qcom/sc7180.dtsi| 9 ++--- arch/arm64/boot/dts/qcom/sc7280.dtsi| 9 ++--- arch/arm64/boot/dts/qcom/sdm845.dtsi| 9 ++--- arch/arm64/boot/dts/qcom/sm8250.dtsi| 9 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ 5 files changed, 11 insertions(+), 28 deletions(-) -- 2.7.4
Re: [PATCH] drm: remove min_order BUG_ON check
On 08/03/2022 13:59, Arunpravin wrote:
On 07/03/22 10:11 pm, Matthew Auld wrote:
On 07/03/2022 14:37, Arunpravin wrote:
place BUG_ON(order < min_order) outside do..while
loop as it fails Unigine Heaven benchmark.
Unigine Heaven has buffer allocation requests for
example required pages are 161 and alignment request
is 128. To allocate the remaining 33 pages, continues
the iteration to find the order value which is 5 and
when it compares with min_order = 7, enables the
BUG_ON(). To avoid this problem, placed the BUG_ON
check outside of do..while loop.
Signed-off-by: Arunpravin
---
drivers/gpu/drm/drm_buddy.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 72f52f293249..ed94c56b720f 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -669,10 +669,11 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
order = fls(pages) - 1;
min_order = ilog2(min_page_size) - ilog2(mm->chunk_size);
+ BUG_ON(order < min_order);
Isn't the issue that we are allowing a size that is not aligned to the
requested min_page_size? Should we not fix the caller(and throw a normal
error here), or perhaps add the round_up() here instead?
CASE 1:
when size is not aligned to the requested min_page_size, for instance,
required size = 161 pages, min_page_size = 128 pages, here we have 3
possible options,
a. AFAIK,This kind of situation is common in any workload,the first
allocation (i.e) 128 pages is aligned to min_page_size, Should we just
allocate the left over 33 pages (2 pow 5, 2 pow 0) since the caller does
know the left over pages are not in min_page_size alignment?
So IIUC looking at amdgpu_gem_create_ioctl(), userspace can specify some
arbitrary physical alignment for an object? Is that not meant to apply
to every page/chunk? The above example would only have the correct
physical alignment guaranteed for the first chunk, or so, is this the
expected ABI behaviour?
Also looking at this some more, the other related bug here is the
order-- == min_order check, since it now won't bail when order == 0,
leading to order = -1, if we are unlucky...
Originally, if asking for min_page_size > chunk_size, then the
allocation was meant to fail if it can't fill the resource request with
pages of at least that size(and also alignment). Or at least that was
the original meaning in i915 IIRC.
b. There are many such instances in unigine heaven workload (there would
be many such workloads), throwing a normal error would lower the FPS? is
it possible to fix at caller application?
c. adding the round_up() is possible, but in every such instances we end
up allocating extra unused memory. For example, if required pages = 1028
and min_page_size = 1024 pages, we end up round up of left over 4 pages
to the min_page_size, so the total size would be 2048 pages.
i.e if someone does:
alloc_blocks(mm, 0, end, 4096, 1<<16, &blocks, flags);
CASE 2:
I think this case should be detected (i.e) when min_page_size > size,
should we return -EINVAL?
This will still trigger the BUG_ON() even if we move it out of the loop,
AFAICT.
Should we just allow the CASE 1 proceed for the allocation and return
-EINVAL for the CASE 2?
+
do {
order = min(order, (unsigned int)fls(pages) - 1);
BUG_ON(order > mm->max_order);
- BUG_ON(order < min_order);
do {
if (flags & DRM_BUDDY_RANGE_ALLOCATION)
base-commit: 8025c79350b90e5a8029234d433578f12abbae2b
Re: [PATCH v5 5/5] drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table during probe
On Tue, 8 Mar 2022 at 19:55, Vinod Polimera wrote: > > use max clock during probe/bind sequence from the opp table. > The clock will be scaled down when framework sends an update. > > Signed-off-by: Vinod Polimera > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index d550f90..d9922b9 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -1221,6 +1221,7 @@ static int dpu_bind(struct device *dev, struct device > *master, void *data) > struct dpu_kms *dpu_kms; > struct dss_module_power *mp; > int ret = 0; > + unsigned long max_freq = ULONG_MAX; > > dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL); > if (!dpu_kms) > @@ -1243,6 +1244,8 @@ static int dpu_bind(struct device *dev, struct device > *master, void *data) > return ret; > } > > + dev_pm_opp_find_freq_floor(dev, &max_freq); You leak a reference to the opp here. The function returns a value, which should be dev_pm_opp_put(). Moreover judging from the dev_pm_opp_set_rate() code I think you don't have to find an exact frequency, as it will call clk_round_rate()/_find_freq_ceil() anyway. Could you please check that it works? > + dev_pm_opp_set_rate(dev, max_freq); > platform_set_drvdata(pdev, dpu_kms); > > ret = msm_kms_init(&dpu_kms->base, &kms_funcs); > -- > 2.7.4 > -- With best wishes Dmitry
[PATCH] drm/mgag200: Fix PLL setup for g200wb and g200ew
commit f86c3ed55920ca1d874758cc290890902a6cffc4 ("drm/mgag200: Split PLL
setup into compute and update functions") introduced a regression for
g200wb and g200ew.
The PLLs are not set up properly, and VGA screen stays
black, or displays "out of range" message.
MGA1064_WB_PIX_PLLC_N/M/P was mistakenly replaced with
MGA1064_PIX_PLLC_N/M/P which have different addresses.
Patch tested on a Dell T310 with g200wb
Fixes: f86c3ed55920ca1d874758cc290890902a6cffc4
Cc: sta...@vger.kernel.org
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/mgag200/mgag200_pll.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_pll.c
b/drivers/gpu/drm/mgag200/mgag200_pll.c
index e9ae22b4f813..52be08b744ad 100644
--- a/drivers/gpu/drm/mgag200/mgag200_pll.c
+++ b/drivers/gpu/drm/mgag200/mgag200_pll.c
@@ -404,9 +404,9 @@ mgag200_pixpll_update_g200wb(struct mgag200_pll *pixpll,
const struct mgag200_pl
udelay(50);
/* program pixel pll register */
- WREG_DAC(MGA1064_PIX_PLLC_N, xpixpllcn);
- WREG_DAC(MGA1064_PIX_PLLC_M, xpixpllcm);
- WREG_DAC(MGA1064_PIX_PLLC_P, xpixpllcp);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_N, xpixpllcn);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_M, xpixpllcm);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_P, xpixpllcp);
udelay(50);
--
2.35.1
Re: [PATCH 4/9] fbdev: Export helper for implementing page_mkwrite
On 3/3/22 21:58, Thomas Zimmermann wrote: > Refactor the page-write handler and export it as helper function > fb_deferred_io_page_mkwrite(). Drivers that implement struct > vm_operations_struct.page_mkwrite for deferred I/O should use the > function to let fbdev track written pages of mmap'ed framebuffer > memory. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 5/9] drm/fb-helper: Separate deferred I/O from shadow buffers
On 3/3/22 21:58, Thomas Zimmermann wrote: > DRM drivers will be able to handle deferred I/O by themselves. So > a driver will be able to use deferred I/O without an intermediate > shadow buffer. > > Prepare fbdev emulation by separating shadow buffers and deferred > I/O from each other. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH] drm/mgag200: Fix PLL setup for g200wb and g200ew
On Tue, Mar 08, 2022 at 06:11:11PM +0100, Jocelyn Falempe wrote:
> commit f86c3ed55920ca1d874758cc290890902a6cffc4 ("drm/mgag200: Split PLL
> setup into compute and update functions") introduced a regression for
> g200wb and g200ew.
No need for all those digits in the sha1, see below:
> The PLLs are not set up properly, and VGA screen stays
> black, or displays "out of range" message.
>
> MGA1064_WB_PIX_PLLC_N/M/P was mistakenly replaced with
> MGA1064_PIX_PLLC_N/M/P which have different addresses.
>
> Patch tested on a Dell T310 with g200wb
>
> Fixes: f86c3ed55920ca1d874758cc290890902a6cffc4
As per the documentation that line should read:
Fixes: f86c3ed55920 ("drm/mgag200: Split PLL setup into compute and update
functions")
thanks,
greg k-h
Re: [PATCH] drm/mgag200: Fix PLL setup for g200wb and g200ew
On 08/03/2022 18:31, Greg KH wrote:
On Tue, Mar 08, 2022 at 06:11:11PM +0100, Jocelyn Falempe wrote:
commit f86c3ed55920ca1d874758cc290890902a6cffc4 ("drm/mgag200: Split PLL
setup into compute and update functions") introduced a regression for
g200wb and g200ew.
No need for all those digits in the sha1, see below:
The PLLs are not set up properly, and VGA screen stays
black, or displays "out of range" message.
MGA1064_WB_PIX_PLLC_N/M/P was mistakenly replaced with
MGA1064_PIX_PLLC_N/M/P which have different addresses.
Patch tested on a Dell T310 with g200wb
Fixes: f86c3ed55920ca1d874758cc290890902a6cffc4
As per the documentation that line should read:
Fixes: f86c3ed55920 ("drm/mgag200: Split PLL setup into compute and update
functions")
Sorry, I will send a v2 shortly.
thanks,
greg k-h
[PATCH v2] drm/mgag200: Fix PLL setup for g200wb and g200ew
commit f86c3ed55920 ("drm/mgag200: Split PLL setup into compute and
update functions") introduced a regression for g200wb and g200ew.
The PLLs are not set up properly, and VGA screen stays
black, or displays "out of range" message.
MGA1064_WB_PIX_PLLC_N/M/P was mistakenly replaced with
MGA1064_PIX_PLLC_N/M/P which have different addresses.
Patch tested on a Dell T310 with g200wb
Fixes: f86c3ed55920 ("drm/mgag200: Split PLL setup into compute and update
functions")
Cc: sta...@vger.kernel.org
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/mgag200/mgag200_pll.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_pll.c
b/drivers/gpu/drm/mgag200/mgag200_pll.c
index e9ae22b4f813..52be08b744ad 100644
--- a/drivers/gpu/drm/mgag200/mgag200_pll.c
+++ b/drivers/gpu/drm/mgag200/mgag200_pll.c
@@ -404,9 +404,9 @@ mgag200_pixpll_update_g200wb(struct mgag200_pll *pixpll,
const struct mgag200_pl
udelay(50);
/* program pixel pll register */
- WREG_DAC(MGA1064_PIX_PLLC_N, xpixpllcn);
- WREG_DAC(MGA1064_PIX_PLLC_M, xpixpllcm);
- WREG_DAC(MGA1064_PIX_PLLC_P, xpixpllcp);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_N, xpixpllcn);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_M, xpixpllcm);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_P, xpixpllcp);
udelay(50);
--
2.35.1
Re: [PATCH 6/9] drm/fb-helper: Provide callback to create fbdev dumb buffers
On 3/3/22 21:58, Thomas Zimmermann wrote:
> Provide struct drm_driver.dumb_create_fbdev, a callback hook for
> fbdev dumb buffers. Wire up fbdev and client helpers to use the new
> interface, if present.
>
> This acknowledges the fact that fbdev buffers are different. The most
> significant difference to regular GEM BOs is in mmap semantics. Fbdev
> userspace treats the pages as video memory, which makes it impossible
> to ever move the mmap'ed buffer. Hence, drivers ussually have to pin
> the BO permanently or install an intermediate shadow buffer for mmap.
>
> So far, fbdev memory came from dumb buffers and DRM drivers had no
> means of detecting this without reimplementing a good part of the fbdev
> code. With the new callback, drivers can perma-pin fbdev buffer objects
> if needed.
>
> Several drivers also require damage handling, which fbdev implements
> with its deferred I/O helpers. The new callback allows a driver's memory
> manager to set up a suitable mmap.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_client.c| 14 +++
> drivers/gpu/drm/drm_crtc_internal.h | 3 +++
> drivers/gpu/drm/drm_dumb_buffers.c | 36 +
> drivers/gpu/drm/drm_fb_helper.c | 26 +
> include/drm/drm_client.h| 3 ++-
> include/drm/drm_drv.h | 17 ++
> 6 files changed, 84 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index af3b7395bf69..c964064651d5 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -247,7 +247,8 @@ static void drm_client_buffer_delete(struct
> drm_client_buffer *buffer)
> }
>
> static struct drm_client_buffer *
> -drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32
> height, u32 format)
> +drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32
> height, u32 format,
> + bool fbdev)
> {
> const struct drm_format_info *info = drm_format_info(format);
> struct drm_mode_create_dumb dumb_args = { };
> @@ -265,7 +266,10 @@ drm_client_buffer_create(struct drm_client_dev *client,
> u32 width, u32 height, u
> dumb_args.width = width;
> dumb_args.height = height;
> dumb_args.bpp = info->cpp[0] * 8;
> - ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
> + if (fbdev)
Maybe if (defined(CONFIG_DRM_FBDEV_EMULATION) && fbdev) ?
> + ret = drm_mode_create_dumb_fbdev(dev, &dumb_args, client->file);
And drm_mode_create_dumb_fbdev() could just be made a stub if
CONFIG_DRM_FBDEV_EMULATION isn't enabled.
I believe the only usage of the DRM client API currently is the fbdev
emulation layer anyways? But still makes sense I think to condtionally
compile that since drm_client.o is built in the drm.ko module and the
drm_fb_helper.o only included if fbdev emulation has been enabled.
> + else
> + ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
> if (ret)
> goto err_delete;
>
> @@ -402,6 +406,7 @@ static int drm_client_buffer_addfb(struct
> drm_client_buffer *buffer,
> * @width: Framebuffer width
> * @height: Framebuffer height
> * @format: Buffer format
> + * @fbdev: True if the client provides an fbdev device, or false otherwise
> *
An emulated fbdev device ?
Other than those small nit,
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/gem: Remove logic for wbinvd_on_all_cpus
On Tue, Feb 22, 2022 at 08:24:31PM +0100, Thomas Hellström (Intel) wrote:
Hi, Michael,
On 2/22/22 18:26, Michael Cheng wrote:
This patch removes logic for wbinvd_on_all_cpus and brings in
drm_cache.h. This header has the logic that outputs a warning
when wbinvd_on_all_cpus when its being used on a non-x86 platform.
Signed-off-by: Michael Cheng
Linus has been pretty clear that he won't accept patches that add
macros that works on one arch and warns on others anymore in i915 and
I figure even less so in drm code.
So we shouldn't try to move this out to drm. Instead we should
restrict the wbinvd() inside our driver to integrated and X86 only.
For discrete on all architectures we should be coherent and hence not
be needing wbinvd().
the warn is there to guarantee we don't forget a code path. However
simply adding the warning is the real issue: we should rather guarantee
we can't take that code path. I.e., as you said refactor the code to
guarantee it works on discrete without that logic.
$ git grep wbinvd_on_all_cpus -- drivers/gpu/drm/
drivers/gpu/drm/drm_cache.c:if (wbinvd_on_all_cpus())
drivers/gpu/drm/drm_cache.c:if (wbinvd_on_all_cpus())
drivers/gpu/drm/drm_cache.c:if (wbinvd_on_all_cpus())
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c: * Currently we just do
a heavy handed wbinvd_on_all_cpus() here since
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c:
wbinvd_on_all_cpus();
It looks like we actually go through this on other discrete graphics. Is
this missing an update like s/IS_DG1/IS_DGFX/? Or should we be doing
something else?
drivers/gpu/drm/i915/gem/i915_gem_pm.c:#define wbinvd_on_all_cpus() \
drivers/gpu/drm/i915/gem/i915_gem_pm.c: wbinvd_on_all_cpus();
Those are for suspend. Revert ac05a22cd07a ("drm/i915/gem: Almagamate clflushes on
suspend")
or extract that part to a helper function and implement it differently
for arches != x86?
drivers/gpu/drm/i915/gem/i915_gem_pm.c: wbinvd_on_all_cpus();
Probably take a similar approach to the suspend case?
drivers/gpu/drm/i915/gt/intel_ggtt.c: wbinvd_on_all_cpus();
This one comes from 64b95df91f44 ("drm/i915: Assume exclusive access to objects
inside resume")
Shouldn't that be doing the invalidate if the write domain is
I915_GEM_DOMAIN_CPU
In the end I think the warning would be ok if it was the cherry on top,
to guarantee we don't take those paths. We should probably have a
warn_once() to avoid spamming the console. But we also have to rework
the code to guarantee we are the only ones who may eventually get that
warning, and not the end user.
Lucas De Marchi
Thanks,
/Thomas
Re: [Intel-gfx] [PATCH] drm/i915/xehp: Drop aux table invalidation on FlatCCS platforms
On Mon, Feb 28, 2022 at 09:29:52PM -0800, Matt Roper wrote:
Platforms with FlatCCS do not use auxiliary planes for compression
control data and thus do not need traditional aux table invalidation
(and the registers no longer even exist).
Original-author: CQ Tang
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 28
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 1f8cf4f790b2..13bbbf5d9737 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -231,7 +231,7 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
if (mode & EMIT_INVALIDATE) {
u32 flags = 0;
- u32 *cs;
+ u32 *cs, count;
flags |= PIPE_CONTROL_COMMAND_CACHE_INVALIDATE;
flags |= PIPE_CONTROL_TLB_INVALIDATE;
@@ -246,7 +246,12 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
flags |= PIPE_CONTROL_CS_STALL;
- cs = intel_ring_begin(rq, 8 + 4);
+ if (!HAS_FLAT_CCS(rq->engine->i915))
+ count = 8 + 4;
+ else
+ count = 8;
u32 count = 8;
...
if (!HAS_FLAT_CCS(rq->engine->i915))
count += 4;
would probably be shorter, or even
cs = intel_ring_begin(rq, HAS_FLAT_CCS(...) ? 12 : 8)
but doesn't really matter
Reviewed-by: Lucas De Marchi
Lucas De Marchi
[PATCH] drm/msm/gpu: Fix crash on devices without devfreq support (v2)
From: Rob Clark
Avoid going down devfreq paths on devices where devfreq is not
initialized.
v2: Change has_devfreq() logic [Dmitry]
Reported-by: Linux Kernel Functional Testing
Reported-by: Anders Roxell
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 30 ++-
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index 9bf319be11f6..12641616acd3 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -83,6 +83,12 @@ static struct devfreq_dev_profile msm_devfreq_profile = {
static void msm_devfreq_boost_work(struct kthread_work *work);
static void msm_devfreq_idle_work(struct kthread_work *work);
+static bool has_devfreq(struct msm_gpu *gpu)
+{
+ struct msm_gpu_devfreq *df = &gpu->devfreq;
+ return !!df->devfreq;
+}
+
void msm_devfreq_init(struct msm_gpu *gpu)
{
struct msm_gpu_devfreq *df = &gpu->devfreq;
@@ -149,6 +155,9 @@ void msm_devfreq_cleanup(struct msm_gpu *gpu)
{
struct msm_gpu_devfreq *df = &gpu->devfreq;
+ if (!has_devfreq(gpu))
+ return;
+
devfreq_cooling_unregister(gpu->cooling);
dev_pm_qos_remove_request(&df->boost_freq);
dev_pm_qos_remove_request(&df->idle_freq);
@@ -156,16 +165,24 @@ void msm_devfreq_cleanup(struct msm_gpu *gpu)
void msm_devfreq_resume(struct msm_gpu *gpu)
{
- gpu->devfreq.busy_cycles = 0;
- gpu->devfreq.time = ktime_get();
+ struct msm_gpu_devfreq *df = &gpu->devfreq;
- devfreq_resume_device(gpu->devfreq.devfreq);
+ if (!has_devfreq(gpu))
+ return;
+
+ df->busy_cycles = 0;
+ df->time = ktime_get();
+
+ devfreq_resume_device(df->devfreq);
}
void msm_devfreq_suspend(struct msm_gpu *gpu)
{
struct msm_gpu_devfreq *df = &gpu->devfreq;
+ if (!has_devfreq(gpu))
+ return;
+
devfreq_suspend_device(df->devfreq);
cancel_idle_work(df);
@@ -185,6 +202,9 @@ void msm_devfreq_boost(struct msm_gpu *gpu, unsigned factor)
struct msm_gpu_devfreq *df = &gpu->devfreq;
uint64_t freq;
+ if (!has_devfreq(gpu))
+ return;
+
freq = get_freq(gpu);
freq *= factor;
@@ -207,7 +227,7 @@ void msm_devfreq_active(struct msm_gpu *gpu)
struct devfreq_dev_status status;
unsigned int idle_time;
- if (!df->devfreq)
+ if (!has_devfreq(gpu))
return;
/*
@@ -253,7 +273,7 @@ void msm_devfreq_idle(struct msm_gpu *gpu)
{
struct msm_gpu_devfreq *df = &gpu->devfreq;
- if (!df->devfreq)
+ if (!has_devfreq(gpu))
return;
msm_hrtimer_queue_work(&df->idle_work, ms_to_ktime(1),
--
2.35.1
Re: [PATCH] drm/msm/gpu: Fix crash on devices without devfreq support (v2)
On Tue, Mar 8, 2022 at 3:48 PM Rob Clark wrote: > > From: Rob Clark > > Avoid going down devfreq paths on devices where devfreq is not > initialized. > > v2: Change has_devfreq() logic [Dmitry] > > Reported-by: Linux Kernel Functional Testing > Reported-by: Anders Roxell > Signed-off-by: Rob Clark Does this need a Fixes tag?
RE: [RFC 18/28] drm: rcar-du: Add RZ/G2L LCDC Support
Hi Laurent,
Thanks for the feedback.
> Subject: Re: [RFC 18/28] drm: rcar-du: Add RZ/G2L LCDC Support
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Wed, Jan 12, 2022 at 05:46:02PM +, Biju Das wrote:
> > The LCD controller is composed of Frame Compression Processor (FCPVD),
> > Video Signal Processor (VSPD), and Display Unit (DU).
> >
> > It has DPI/DSI interfaces and supports a maximum resolution of 1080p
> > along with 2 rpf's to support blending of two picture layers and
> > raster operations (ROPs).
> >
> > A feature bit for RZ/G2L SoC is introduced to support RZ/G2L with the
> > rest of the SoC supported by this driver.
>
> The RZ/G2L DU seems to be a completely different IP core than the DU in R-
> Car and other RZ SoCs. I think it should be supported by a separate
> driver, or at least with a different (and modularized) CRTC
> implementation. This patch has too many RCAR_DU_FEATURE_RZG2L checks.
Agreed, May be will create separate RZ/G2L specific DU driver based on RCar-DU
driver removing the plane, Group, CMM, LVDS, HDMI and CRTC adaptation for
RZ/G2L
and will send next version for feedback.
Regards,
Biju
>
> > Signed-off-by: Biju Das
> > ---
> > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 148 ++--
> > drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 +
> > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 23
> > drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 +
> > drivers/gpu/drm/rcar-du/rcar_du_group.c | 5 +
> > drivers/gpu/drm/rcar-du/rcar_du_regs.h | 52 +
> > 6 files changed, 195 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > index 521446890d3d..aea9178f3e7d 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > @@ -10,6 +10,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include
> > @@ -219,6 +220,42 @@ static void rcar_du_crtc_set_display_timing(struct
> rcar_du_crtc *rcrtc)
> > u32 dsmr;
> > u32 escr;
> >
> > + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + u32 ditr0, ditr1, ditr2, ditr3, ditr4, ditr5, pbcr0;
> > +
> > + clk_set_rate(rcrtc->extclock, mode_clock);
> > +
> > + ditr0 = (DU_DITR0_DEMD_HIGH
> > + | ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DU_DITR0_VSPOL : 0)
> > + | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DU_DITR0_HSPOL :
> 0));
> > +
> > + ditr1 = DU_DITR1_VSA(mode->vsync_end - mode->vsync_start)
> > + | DU_DITR1_VACTIVE(mode->vdisplay);
> > +
> > + ditr2 = DU_DITR2_VBP(mode->vtotal - mode->vsync_end)
> > + | DU_DITR2_VFP(mode->vsync_start - mode->vdisplay);
> > +
> > + ditr3 = DU_DITR3_HSA(mode->hsync_end - mode->hsync_start)
> > + | DU_DITR3_HACTIVE(mode->hdisplay);
> > +
> > + ditr4 = DU_DITR4_HBP(mode->htotal - mode->hsync_end)
> > + | DU_DITR4_HFP(mode->hsync_start - mode->hdisplay);
> > +
> > + ditr5 = DU_DITR5_VSFT(0) | DU_DITR5_HSFT(0);
> > +
> > + pbcr0 = DU_PBCR0_PB_DEP(0x1F);
> > +
> > + rcar_du_write(rcdu, DU_DITR0, ditr0);
> > + rcar_du_write(rcdu, DU_DITR1, ditr1);
> > + rcar_du_write(rcdu, DU_DITR2, ditr2);
> > + rcar_du_write(rcdu, DU_DITR3, ditr3);
> > + rcar_du_write(rcdu, DU_DITR4, ditr4);
> > + rcar_du_write(rcdu, DU_DITR5, ditr5);
> > + rcar_du_write(rcdu, DU_PBCR0, pbcr0);
> > +
> > + return;
> > + }
> > +
> > if (rcdu->info->dpll_mask & (1 << rcrtc->index)) {
> > unsigned long target = mode_clock;
> > struct dpll_info dpll = { 0 };
> > @@ -531,16 +568,23 @@ static void rcar_du_cmm_setup(struct drm_crtc
> > *crtc)
> >
> > static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) {
> > - /* Set display off and background to black */
> > - rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0));
> > - rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0));
> > + struct rcar_du_device *rcdu = rcrtc->dev;
> >
> > - /* Configure display timings and output routing */
> > - rcar_du_crtc_set_display_timing(rcrtc);
> > - rcar_du_group_set_routing(rcrtc->group);
> > + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + /* Set display off and background to black */
> > + rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0));
> > + rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0));
> >
> > - /* Start with all planes disabled. */
> > - rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR,
> 0);
> > + /* Configure display timings and output routing */
> > + rcar_du_crtc_set_display_timing(rcrtc);
> > + rcar_du_group_set_routing(rcrtc->group);
> > +
> > + /* Start with all planes disabled. */
> > + rcar_du_group_write(rcrtc->group, rcr
Re: [PATCH 7/9] drm/fb-helper: Provide fbdev deferred-I/O helpers
On 3/3/22 21:58, Thomas Zimmermann wrote: > Add drm_fb_helper_vm_page_mkwrite(), a helper to track pages written > by fbdev userspace. DRM drivers should use this function to implement > fbdev deferred I/O. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH] drm/msm/gpu: Fix crash on devices without devfreq support (v2)
On Tue, Mar 8, 2022 at 10:53 AM Fabio Estevam wrote:
>
> On Tue, Mar 8, 2022 at 3:48 PM Rob Clark wrote:
> >
> > From: Rob Clark
> >
> > Avoid going down devfreq paths on devices where devfreq is not
> > initialized.
> >
> > v2: Change has_devfreq() logic [Dmitry]
> >
> > Reported-by: Linux Kernel Functional Testing
> > Reported-by: Anders Roxell
Fixes: 6aa89ae1fb04 ("drm/msm/gpu: Cancel idle/boost work on suspend")
> > Signed-off-by: Rob Clark
>
> Does this need a Fixes tag?
Yes, sorry, patchwork had picked up the fixes tag from previous
version but I'd forgot to add it locally
BR,
-R
[PATCH 1/3] drm/bridge: Add MAINTAINERS entry for DRM drivers for bridge chip bindings
The bindings for bridge chips should also get the same maintainers entry so the right people get notified about bindings changes. Signed-off-by: Douglas Anderson --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0216d2ffe728..a73179d55d00 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6403,6 +6403,7 @@ R:Jonas Karlman R: Jernej Skrabec S: Maintained T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/bridge/ F: drivers/gpu/drm/bridge/ DRM DRIVERS FOR EXYNOS -- 2.35.1.616.g0bdcbb4464-goog
[PATCH 2/3] drm/bridge: Add myself as a reviewer for the TI SN65DSI86 bridge chip
I've spent quite a bit of time poking at this driver and it's used on several Chromebooks I'm involved with. I'd like to get notified about patches. Add myself as a reviewer. It's expected that changes will still be landed through drm-misc as they always have been. Signed-off-by: Douglas Anderson --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a73179d55d00..7d25d0b4dccc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6284,6 +6284,11 @@ DRM DRIVER FOR TDFX VIDEO CARDS S: Orphan / Obsolete F: drivers/gpu/drm/tdfx/ +DRM DRIVER FOR TI SN65DSI86 BRIDGE CHIP +R: Douglas Anderson +F: Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml +F: drivers/gpu/drm/bridge/ti-sn65dsi86.c + DRM DRIVER FOR TPO TPG110 PANELS M: Linus Walleij S: Maintained -- 2.35.1.616.g0bdcbb4464-goog
[PATCH 3/3] drm/bridge: Add myself as a reviewer for the Parade PS8640 bridge chip
Though the parade bridge chip is a little bit of a black box, I'm at least interested in hearing about changes to the driver since this bridge chip is used on some Chromebooks that I'm involved with. Signed-off-by: Douglas Anderson --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 7d25d0b4dccc..db7fe53643c2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6171,6 +6171,11 @@ S: Maintained F: Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino.yaml F: drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c +DRM DRIVER FOR PARADE PS8640 BRIDGE CHIP +R: Douglas Anderson +F: Documentation/devicetree/bindings/display/bridge/ps8640.yaml +F: drivers/gpu/drm/bridge/parade-ps8640.c + DRM DRIVER FOR PERVASIVE DISPLAYS REPAPER PANELS M: Noralf Trønnes S: Maintained -- 2.35.1.616.g0bdcbb4464-goog
[PATCH] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
Export dmabuf handles for GTT BOs so that their contents can be accessed
using SDMA during checkpoint/restore.
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 2c7d76e67ddb..e1e2362841f8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1759,7 +1759,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
goto exit;
}
}
- if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret =
criu_get_prime_handle(&dumper_bo->tbo.base,
bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? DRM_RDWR : 0,
@@ -1812,7 +1813,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
exit:
while (ret && bo_index--) {
- if (bo_buckets[bo_index].alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[bo_index].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[bo_index].dmabuf_fd);
}
@@ -2211,7 +2213,8 @@ static int criu_restore_bo(struct kfd_process *p,
pr_debug("map memory was successful for the BO\n");
/* create the dmabuf object and export the bo */
- if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM | KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret = criu_get_prime_handle(&kgd_mem->bo->tbo.base, DRM_RDWR,
&bo_bucket->dmabuf_fd);
if (ret)
@@ -2281,7 +2284,8 @@ static int criu_restore_bos(struct kfd_process *p,
exit:
while (ret && i--) {
- if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[i].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[i].dmabuf_fd);
}
kvfree(bo_buckets);
--
2.17.1
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On 3/8/22 19:29, Rob Clark wrote: > On Tue, Mar 8, 2022 at 5:17 AM Dmitry Osipenko > wrote: >> >> Hello, >> >> This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. >> During OOM, the shrinker will release BOs that are marked as "not needed" >> by userspace using the new madvise IOCTL. The userspace in this case is >> the Mesa VirGL driver, it will mark the cached BOs as "not needed", >> allowing kernel driver to release memory of the cached shmem BOs on lowmem >> situations, preventing OOM kills. > > Will host memory pressure already trigger shrinker in guest? The host memory pressure won't trigger shrinker in guest here. This series will help only with the memory pressure within the guest using a usual "virgl context". Having a host shrinker in a case of "virgl contexts" should be a difficult problem to solve. > This is > something I'm quite interested in for "virtgpu native contexts" (ie. > native guest driver with new context type sitting on top of virtgpu), In a case of "native contexts" it should be doable, at least I can't see any obvious problems. The madvise invocations could be passed to the host using a new virtio-gpu command by the guest's madvise IOCTL handler, instead-of/in-addition-to handling madvise in the guest's kernel, and that's it. > since that isn't using host storage s/host/guest ?
Re: [PATCH 8/9] drm/gem-shmem: Implement fbdev dumb buffer and mmap helpers
On 3/3/22 21:58, Thomas Zimmermann wrote:
> Implement struct drm_driver.dumb_create_fbdev for GEM SHMEM. The
> created buffer object returned by this function implements deferred
> I/O for its mmap operation.
>
> Add this feature to a number of drivers that use GEM SHMEM helpers
> as shadow planes over their regular video memory. The new macro
> DRM_GEM_SHMEM_DRIVER_OPS_WITH_SHADOW_PLANES sets the regular GEM
> functions and dumb_create_fbdev in struct drm_driver. Fbdev emulation
> on these drivers will now mmap the GEM SHMEM pages directly with
> deferred I/O without an intermediate shadow buffer.
>
> Signed-off-by: Thomas Zimmermann
> ---
[snip]
> @@ -49,8 +50,20 @@ static const struct drm_gem_object_funcs
> drm_gem_shmem_funcs = {
> .vm_ops = &drm_gem_shmem_vm_ops,
> };
>
> +static const struct drm_gem_object_funcs drm_gem_shmem_funcs_fbdev = {
> + .free = drm_gem_shmem_object_free,
> + .print_info = drm_gem_shmem_object_print_info,
> + .pin = drm_gem_shmem_object_pin,
> + .unpin = drm_gem_shmem_object_unpin,
> + .get_sg_table = drm_gem_shmem_object_get_sg_table,
> + .vmap = drm_gem_shmem_object_vmap,
> + .vunmap = drm_gem_shmem_object_vunmap,
> + .mmap = drm_gem_shmem_object_mmap_fbdev,
> + .vm_ops = &drm_gem_shmem_vm_ops_fbdev,
The drm_gem_shmem_funcs_fbdev is the same than drm_gem_shmem_funcs but
.mmap and .vm_ops callbacks. Maybe adding a macro to declare these two
struct drm_gem_object_funcs could make easier to maintain it long term ?
> +};
> +
> static struct drm_gem_shmem_object *
> -__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)
> +__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
> bool fbdev)
> {
> struct drm_gem_shmem_object *shmem;
> struct drm_gem_object *obj;
> @@ -70,8 +83,12 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t
> size, bool private)
> obj = &shmem->base;
> }
>
> - if (!obj->funcs)
> - obj->funcs = &drm_gem_shmem_funcs;
> + if (!obj->funcs) {
> + if (fbdev)
Same question than in patch #6, maybe
if (defined(CONFIG_DRM_FBDEV_EMULATION) && fbdev) ?
[snip]
> + */
> +int drm_gem_shmem_dumb_create_fbdev(struct drm_file *file, struct drm_device
> *dev,
> + struct drm_mode_create_dumb *args)
> +{
> +#if defined(CONFIG_DRM_FBDEV_EMULATION)
> + return __drm_gem_shmem_dumb_create(file, dev, true, args);
> +#else
> + return -ENOSYS;
> +#endif
> +}
I believe the correct errno code here should be -ENOTSUPP.
[snip]
> + /* We don't use vmf->pgoff since that has the fake offset */
> + page_offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
I see this (vmf->address - vma->vm_start) calculation in many places of
this series. Maybe we could add a macro to calculate the offset instead ?
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [RESEND PATCH] dt-bindings: display/msm: add missing brace in dpu-qcm2290.yaml
On Tue, Mar 1, 2022 at 6:14 PM Dmitry Baryshkov wrote: > > Add missing brace in dpu-qcm2290.yaml. While we are at it, also fix > indentation for another brace, so it matches the corresponding line. > > Reported-by: Rob Herring > Cc: Loic Poulain > Reviewed-by: Bjorn Andersson > Signed-off-by: Dmitry Baryshkov > --- > Didn't include freedreno@ in the first email, so resending. > --- > Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Now that the example actually builds, we get just schema warnings: /builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.example.dt.yaml: mdss@5e0: compatible: ['qcom,qcm2290-mdss', 'qcom,mdss'] is too long >From schema: >/builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml /builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.example.dt.yaml: mdss@5e0: 'mdp@5e01000' does not match any of the regexes: '^display-controller@[0-9a-f]+$', 'pinctrl-[0-9]+' >From schema: >/builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml I would have assumed upon reporting errors with 'make dt_binding_check' that the fixes would be tested with 'make dt_binding_check'... Rob
Re: [PATCH 9/9] drm/virtio: Implement dumb_create_fbdev with GEM SHMEM helpers
On 3/3/22 21:58, Thomas Zimmermann wrote:
> Implement struct drm_driver.dumb_create_fbdev with the helpers
> provided by GEM SHMEM. Fbdev deferred I/O will now work without
> an intermediate shadow buffer for mmap.
>
> As the virtio driver replaces several of the regular GEM SHMEM
> functions with its own implementation, some additional code is
> necessary make fbdev optimization work. Especially, the driver
> has to provide buffer-object functions for fbdev. Add the hook
> struct drm_driver.gem_create_object_fbdev, which is similar to
> struct drm_driver.gem_create_object and allows for the creation
> of dedicated fbdev buffer objects. Wire things up within GEM
> SHMEM.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 7 +++-
> drivers/gpu/drm/virtio/virtgpu_drv.c| 2 +
> drivers/gpu/drm/virtio/virtgpu_drv.h| 2 +
> drivers/gpu/drm/virtio/virtgpu_object.c | 54 +++--
> include/drm/drm_drv.h | 10 +
> 5 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index ab7cb7d896c3..225aa17895bd 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -71,7 +71,12 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t
> size, bool private, bool f
>
> size = PAGE_ALIGN(size);
>
> - if (dev->driver->gem_create_object) {
> + if (dev->driver->gem_create_object_fbdev && fbdev) {
Same comment here to check if fbdev emulation is enabled or maybe is not
worht it ? But I think this hints the compiler to optimize the if branch.
[snip]
> +}
> +#else
> +struct drm_gem_object *virtio_gpu_create_object_fbdev(struct drm_device *dev,
> + size_t size)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
As mentioned, I believe this should be ERR_PTR(-ENOTSUPP) instead.
Feel free to ignore all this nits if you consider that are not applicable.
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH 06/10] drm/gma500: Move GTT resume logic out of psb_gtt_init()
Hi
Am 08.03.22 um 13:07 schrieb Patrik Jakobsson:
On Tue, Mar 8, 2022 at 9:48 AM Thomas Zimmermann wrote:
Hi Sam and Patrik
Am 07.03.22 um 22:02 schrieb Patrik Jakobsson:
On Mon, Mar 7, 2022 at 8:07 PM Sam Ravnborg wrote:
Hi Thomas,
One comment below.
On Sun, Mar 06, 2022 at 09:36:15PM +0100, Thomas Zimmermann wrote:
The current implementation of psb_gtt_init() also does resume
handling. Move the resume code into its own helper.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/gtt.c | 122 ++-
drivers/gpu/drm/gma500/gtt.h | 2 +-
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
3 files changed, 104 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index acd50ee26b03..43ad3ec38c80 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -209,7 +209,7 @@ static void psb_gtt_populate_resources(struct
drm_psb_private *pdev)
drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total,
(size / 1024));
}
-int psb_gtt_init(struct drm_device *dev, int resume)
+int psb_gtt_init(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -218,10 +218,8 @@ int psb_gtt_init(struct drm_device *dev, int resume)
struct psb_gtt *pg;
int ret = 0;
- if (!resume) {
- mutex_init(&dev_priv->gtt_mutex);
- mutex_init(&dev_priv->mmap_mutex);
- }
+ mutex_init(&dev_priv->gtt_mutex);
+ mutex_init(&dev_priv->mmap_mutex);
pg = &dev_priv->gtt;
@@ -290,13 +288,6 @@ int psb_gtt_init(struct drm_device *dev, int resume)
dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
dev_priv->stolen_base, vram_stolen_size / 1024);
- if (resume && (gtt_pages != pg->gtt_pages) &&
- (stolen_size != pg->stolen_size)) {
- dev_err(dev->dev, "GTT resume error.\n");
- ret = -EINVAL;
- goto out_err;
- }
-
pg->gtt_pages = gtt_pages;
pg->stolen_size = stolen_size;
dev_priv->vram_stolen_size = vram_stolen_size;
@@ -304,19 +295,14 @@ int psb_gtt_init(struct drm_device *dev, int resume)
/*
* Map the GTT and the stolen memory area
*/
- if (!resume)
- dev_priv->gtt_map = ioremap(pg->gtt_phys_start,
- gtt_pages << PAGE_SHIFT);
+ dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages << PAGE_SHIFT);
if (!dev_priv->gtt_map) {
dev_err(dev->dev, "Failure to map gtt.\n");
ret = -ENOMEM;
goto out_err;
}
- if (!resume)
- dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base,
- stolen_size);
-
+ dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
if (!dev_priv->vram_addr) {
dev_err(dev->dev, "Failure to map stolen base.\n");
ret = -ENOMEM;
@@ -333,11 +319,107 @@ int psb_gtt_init(struct drm_device *dev, int resume)
return ret;
}
The below is a lot of duplicated complex code.
Can you add one more helper for this?
That makes a lot of sense.
I was thinking the same but figured it would be an easy follow up
patch. But sure, why not fix it here already.
While at it, the gtt enable/disable code could be put in helpers as well.
Patrik, do you know why the code re-reads all these values during
resume? Do the values change? The driver never seems to do anything
with the updated values. For example, it doesn't update the GTT mapping
if gtt_phys_start changes. Can we remove all that code from the resume
function?
In theory these values can change if you hibernate, make changes in
the bios and then resume. I think I've seen bioses that can change the
stolen size and/or aperture size. Not sure if that would also change
the value in eg. pge_ctl or the size of the gtt. I would have to do
some testing on real hardware to figure this out.
But as you say, the above scenario is already broken. For the time
being, can we perhaps just fail gracefully?
Ah, thanks for the explanation. I'll leave the current logic as-is.
Best regards
Thomas
--
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 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Hi Suraj On 3/8/2022 6:30 AM, Kandpal, Suraj wrote: Hi Abhinav, Hi, Hi, Hi Abhinav, Hi Laurent Ok sure, I can take this up but I need to understand the proposal a little bit more before proceeding on this. So we will discuss this in another email where we specifically talk about the connector changes. Also, I am willing to take this up once the encoder part is done and merged so that atleast we keep moving on that as MSM WB implementation can proceed with that first. Hi Jani and Suraj Any concerns with splitting up the series into encoder and connector OR re- arranging them? Let me know if you can do this OR I can also split this up keeping Suraj's name in the Co-developed by tag. I was actually thinking of floating a new patch series with both encoder and connector pointer changes but with a change in initialization functions wherein we allocate a connector and encoder incase the driver doesn’t have its own this should minimize the effect on other drivers already using current drm writeback framework and also allow i915 to create its own connector. I was proposing to split up the encoder and connector because the comments from Laurent meant we were in agreement about the encoder but not about the connector. I am afraid another attempt with the similar idea is only going to stall the series again and hence i gave this option. Seems like this patch series currently won't be able to move forward with the connector pointer change so I guess you can split the series to encoder pointer change where we optionally create encoder if required ,keeping my name in co-developed tag so that msm can move forward with its implementation and then we can work to see how the connector issue can be bypassed. Best Regards, Suraj Kandpal Thanks, let me re-spin this and will keep your name as co-developer. Should be able to get it on the list in a week. Thanks Abhinav Eventually its upto Laurent and the other maintainers to guide us forward on this as this series has been stalled for weeks now. I thought that Laurent was quite strict about the connector. So I'd suggest targeting drm_writeback_connector with an optionally created encoder and the builtin connector In that case even if we target that i915 will not be able to move forward with its implementation of writeback as builtin connector does not work for us right now as explained earlier, optionally creating encoder and connector will help everyone move forward and once done we can actually sit and work on how to side step this issue using Laurent's suggestion This is up to Laurent & other DRM maintainers to decide whether this approach would be accepted or not. So far unfortunately I have mostly seen the pushback and a strong suggestion to stop treating each drm_connector as i915_connector. I haven't checked the exact details, but IMO adding a branch if the connector's type is DRM_CONNECTOR_VIRTUAL should be doable. Bringing in the change where we don’t treat each drm_connector as an intel_connector or even adding a branch which checks if drm_connector is DRM_CONNECTOR_VIRTUAL and conditionally cast it to intel_connector is quite a lot of work for i915. That's why I was suggesting if for now we could move forward with optionally creating both encoder and connector minimizing affects on other drivers as well as allowing us to move forward. What do you think, Launrent? We can work on Laurent's suggestion but that would first require the initial RFC patches and then a rework from both of our drivers side to see if they gel well with our current code which will take a considerable amount of time. We can for now however float the patch series up which minimally affects the current drivers and also allows i915 and msm to move forward with its writeback implementation off course with a proper documentation stating new drivers shouldn't try to make their own connectors and encoders and that drm_writeback will do that for them. Once that's done and merged we can work with Laurent on his proposal to improve the drm writeback framework so that this issue can be side stepped entirely in future. For now I would like to keep the encoder and connector pointer changes together.
[PATCH v2 00/12] drm/gma500: Various cleanups to GEM code
Refactor and simplify various parts of the memory management. This
includes locking, initialization and finalizer functions, and code
organization.
Tested on Atom N2800 hardware.
v2:
* put common code in psb_gtt_{init,fini,resume}() into
helpers (Sam, Patrik)
Thomas Zimmermann (12):
drm/gma500: Remove struct psb_gem_object.npage
drm/gma500: Acquire reservation lock for GEM objects
drm/gma500: Move GTT locking into GTT helpers
drm/gma500: Remove struct psb_gtt.sem sempahore
drm/gma500: Move GTT setup and restoration into helper funtions
drm/gma500: Move GTT resume logic out of psb_gtt_init()
drm/gma500: Cleanup GTT uninit and error handling
drm/gma500: Split GTT init/resume/fini into GTT and GEM functions
drm/gma500: Inline psb_gtt_restore()
drm/gma500: Move GEM memory management functions to gem.c
drm/gma500: Move GTT enable and disable code into helpers
drm/gma500: Move GTT memory-range setup into helper
drivers/gpu/drm/gma500/gem.c | 161 ++-
drivers/gpu/drm/gma500/gem.h | 13 +-
drivers/gpu/drm/gma500/gma_display.c | 8 +-
drivers/gpu/drm/gma500/gtt.c | 295 +--
drivers/gpu/drm/gma500/gtt.h | 8 +-
drivers/gpu/drm/gma500/power.c | 5 +-
drivers/gpu/drm/gma500/psb_drv.c | 13 +-
drivers/gpu/drm/gma500/psb_drv.h | 1 -
8 files changed, 317 insertions(+), 187 deletions(-)
base-commit: 710a019ad85e96e66f7d75ee7f4733cdd8a2b0d0
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: 6e1032c6302461624f33194c8b8f37103a3fa6ef
--
2.35.1
[PATCH v2 05/12] drm/gma500: Move GTT setup and restoration into helper funtions
The GTT init and restore functions contain logic to populate the
GTT entries. Move the code into helper functions.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gtt.c | 115 +--
1 file changed, 68 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index c7b7cb1f2d13..acd50ee26b03 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -144,18 +144,79 @@ void psb_gtt_takedown(struct drm_device *dev)
iounmap(dev_priv->gtt_map);
}
+/* Clear GTT. Use a scratch page to avoid accidents or scribbles. */
+static void psb_gtt_clear(struct drm_psb_private *pdev)
+{
+ resource_size_t pfn_base;
+ unsigned long i;
+ uint32_t pte;
+
+ pfn_base = page_to_pfn(pdev->scratch_page);
+ pte = psb_gtt_mask_pte(pfn_base, PSB_MMU_CACHED_MEMORY);
+
+ for (i = 0; i < pdev->gtt.gtt_pages; ++i)
+ iowrite32(pte, pdev->gtt_map + i);
+
+ (void)ioread32(pdev->gtt_map + i - 1);
+}
+
+/* Insert vram stolen pages into the GTT. */
+static void psb_gtt_populate_stolen(struct drm_psb_private *pdev)
+{
+ struct drm_device *dev = &pdev->dev;
+ unsigned int pfn_base;
+ unsigned int i, num_pages;
+ uint32_t pte;
+
+ pfn_base = pdev->stolen_base >> PAGE_SHIFT;
+ num_pages = pdev->vram_stolen_size >> PAGE_SHIFT;
+
+ drm_dbg(dev, "Set up %u stolen pages starting at 0x%08x, GTT offset
%dK\n",
+ num_pages, pfn_base << PAGE_SHIFT, 0);
+
+ for (i = 0; i < num_pages; ++i) {
+ pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
+ iowrite32(pte, pdev->gtt_map + i);
+ }
+
+ (void)ioread32(pdev->gtt_map + i - 1);
+}
+
+/* Re-insert all pinned GEM objects into GTT. */
+static void psb_gtt_populate_resources(struct drm_psb_private *pdev)
+{
+ unsigned int restored = 0, total = 0, size = 0;
+ struct resource *r = pdev->gtt_mem->child;
+ struct drm_device *dev = &pdev->dev;
+ struct psb_gem_object *pobj;
+
+ while (r) {
+ /*
+* TODO: GTT restoration needs a refactoring, so that we don't
have to touch
+* struct psb_gem_object here. The type represents a GEM
object and is
+* not related to the GTT itself.
+*/
+ pobj = container_of(r, struct psb_gem_object, resource);
+ if (pobj->pages) {
+ psb_gtt_insert_pages(pdev, &pobj->resource,
pobj->pages);
+ size += resource_size(&pobj->resource);
+ ++restored;
+ }
+ r = r->sibling;
+ ++total;
+ }
+
+ drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total,
(size / 1024));
+}
+
int psb_gtt_init(struct drm_device *dev, int resume)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
unsigned gtt_pages;
unsigned long stolen_size, vram_stolen_size;
- unsigned i, num_pages;
- unsigned pfn_base;
struct psb_gtt *pg;
-
int ret = 0;
- uint32_t pte;
if (!resume) {
mutex_init(&dev_priv->gtt_mutex);
@@ -262,29 +323,9 @@ int psb_gtt_init(struct drm_device *dev, int resume)
goto out_err;
}
- /*
-* Insert vram stolen pages into the GTT
-*/
-
- pfn_base = dev_priv->stolen_base >> PAGE_SHIFT;
- num_pages = vram_stolen_size >> PAGE_SHIFT;
- dev_dbg(dev->dev, "Set up %d stolen pages starting at 0x%08x, GTT
offset %dK\n",
- num_pages, pfn_base << PAGE_SHIFT, 0);
- for (i = 0; i < num_pages; ++i) {
- pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
- iowrite32(pte, dev_priv->gtt_map + i);
- }
-
- /*
-* Init rest of GTT to the scratch page to avoid accidents or scribbles
-*/
-
- pfn_base = page_to_pfn(dev_priv->scratch_page);
- pte = psb_gtt_mask_pte(pfn_base, PSB_MMU_CACHED_MEMORY);
- for (; i < gtt_pages; ++i)
- iowrite32(pte, dev_priv->gtt_map + i);
+ psb_gtt_clear(dev_priv);
+ psb_gtt_populate_stolen(dev_priv);
- (void) ioread32(dev_priv->gtt_map + i - 1);
return 0;
out_err:
@@ -295,30 +336,10 @@ int psb_gtt_init(struct drm_device *dev, int resume)
int psb_gtt_restore(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
- struct resource *r = dev_priv->gtt_mem->child;
- struct psb_gem_object *pobj;
- unsigned int restored = 0, total = 0, size = 0;
psb_gtt_init(dev, 1);
- while (r != NULL) {
- /*
-* TODO: GTT restoration needs a refactoring, so that we don't
ha
[PATCH v2 06/12] drm/gma500: Move GTT resume logic out of psb_gtt_init()
The current implementation of psb_gtt_init() also does resume
handling. Move the resume code into its own helper.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gtt.c | 122 ++-
drivers/gpu/drm/gma500/gtt.h | 2 +-
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
3 files changed, 104 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index acd50ee26b03..43ad3ec38c80 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -209,7 +209,7 @@ static void psb_gtt_populate_resources(struct
drm_psb_private *pdev)
drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total,
(size / 1024));
}
-int psb_gtt_init(struct drm_device *dev, int resume)
+int psb_gtt_init(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -218,10 +218,8 @@ int psb_gtt_init(struct drm_device *dev, int resume)
struct psb_gtt *pg;
int ret = 0;
- if (!resume) {
- mutex_init(&dev_priv->gtt_mutex);
- mutex_init(&dev_priv->mmap_mutex);
- }
+ mutex_init(&dev_priv->gtt_mutex);
+ mutex_init(&dev_priv->mmap_mutex);
pg = &dev_priv->gtt;
@@ -290,13 +288,6 @@ int psb_gtt_init(struct drm_device *dev, int resume)
dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
dev_priv->stolen_base, vram_stolen_size / 1024);
- if (resume && (gtt_pages != pg->gtt_pages) &&
- (stolen_size != pg->stolen_size)) {
- dev_err(dev->dev, "GTT resume error.\n");
- ret = -EINVAL;
- goto out_err;
- }
-
pg->gtt_pages = gtt_pages;
pg->stolen_size = stolen_size;
dev_priv->vram_stolen_size = vram_stolen_size;
@@ -304,19 +295,14 @@ int psb_gtt_init(struct drm_device *dev, int resume)
/*
* Map the GTT and the stolen memory area
*/
- if (!resume)
- dev_priv->gtt_map = ioremap(pg->gtt_phys_start,
- gtt_pages << PAGE_SHIFT);
+ dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages <<
PAGE_SHIFT);
if (!dev_priv->gtt_map) {
dev_err(dev->dev, "Failure to map gtt.\n");
ret = -ENOMEM;
goto out_err;
}
- if (!resume)
- dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base,
-stolen_size);
-
+ dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
if (!dev_priv->vram_addr) {
dev_err(dev->dev, "Failure to map stolen base.\n");
ret = -ENOMEM;
@@ -333,11 +319,107 @@ int psb_gtt_init(struct drm_device *dev, int resume)
return ret;
}
+static int psb_gtt_resume(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
+ unsigned int gtt_pages;
+ unsigned long stolen_size, vram_stolen_size;
+ struct psb_gtt *pg;
+ int ret = 0;
+
+ pg = &dev_priv->gtt;
+
+ /* Enable the GTT */
+ pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
+ pci_write_config_word(pdev, PSB_GMCH_CTRL,
+ dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
+
+ dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
+ PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
+ (void) PSB_RVDC32(PSB_PGETBL_CTL);
+
+ /* The root resource we allocate address space from */
+ dev_priv->gtt_initialized = 1;
+
+ pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
+
+ /*
+* The video mmu has a hw bug when accessing 0x0D000.
+* Make gatt start at 0x0e000,. This doesn't actually
+* matter for us but may do if the video acceleration ever
+* gets opened up.
+*/
+ pg->mmu_gatt_start = 0xE000;
+
+ pg->gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
+ gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE) >> PAGE_SHIFT;
+ /* CDV doesn't report this. In which case the system has 64 gtt pages */
+ if (pg->gtt_start == 0 || gtt_pages == 0) {
+ dev_dbg(dev->dev, "GTT PCI BAR not initialized.\n");
+ gtt_pages = 64;
+ pg->gtt_start = dev_priv->pge_ctl;
+ }
+
+ pg->gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
+ pg->gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE) >>
PAGE_SHIFT;
+ dev_priv->gtt_mem = &pdev->resource[PSB_GATT_RESOURCE];
+
+ if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
+ static struct resource fudge; /* Preferably peppermint */
+
[PATCH v2 10/12] drm/gma500: Move GEM memory management functions to gem.c
Move GEM functions from gtt.c to gem.c. Adapt some names. No
functional changes.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gem.c | 133 +++
drivers/gpu/drm/gma500/gem.h | 12 +++
drivers/gpu/drm/gma500/gtt.c | 127 +
drivers/gpu/drm/gma500/gtt.h | 5 +-
drivers/gpu/drm/gma500/power.c | 1 +
drivers/gpu/drm/gma500/psb_drv.c | 1 +
6 files changed, 149 insertions(+), 130 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 2f44535f58e7..dffe37490206 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -21,6 +21,10 @@
#include "gem.h"
#include "psb_drv.h"
+/*
+ * PSB GEM object
+ */
+
int psb_gem_pin(struct psb_gem_object *pobj)
{
struct drm_gem_object *obj = &pobj->base;
@@ -296,3 +300,132 @@ static vm_fault_t psb_gem_fault(struct vm_fault *vmf)
return ret;
}
+
+/*
+ * Memory management
+ */
+
+/* Insert vram stolen pages into the GTT. */
+static void psb_gem_mm_populate_stolen(struct drm_psb_private *pdev)
+{
+ struct drm_device *dev = &pdev->dev;
+ unsigned int pfn_base;
+ unsigned int i, num_pages;
+ uint32_t pte;
+
+ pfn_base = pdev->stolen_base >> PAGE_SHIFT;
+ num_pages = pdev->vram_stolen_size >> PAGE_SHIFT;
+
+ drm_dbg(dev, "Set up %u stolen pages starting at 0x%08x, GTT offset
%dK\n",
+ num_pages, pfn_base << PAGE_SHIFT, 0);
+
+ for (i = 0; i < num_pages; ++i) {
+ pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
+ iowrite32(pte, pdev->gtt_map + i);
+ }
+
+ (void)ioread32(pdev->gtt_map + i - 1);
+}
+
+int psb_gem_mm_init(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
+ unsigned long stolen_size, vram_stolen_size;
+ struct psb_gtt *pg;
+ int ret;
+
+ mutex_init(&dev_priv->mmap_mutex);
+
+ pg = &dev_priv->gtt;
+
+ pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
+ vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base -
PAGE_SIZE;
+
+ stolen_size = vram_stolen_size;
+
+ dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
+ dev_priv->stolen_base, vram_stolen_size / 1024);
+
+ pg->stolen_size = stolen_size;
+ dev_priv->vram_stolen_size = vram_stolen_size;
+
+ dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
+ if (!dev_priv->vram_addr) {
+ dev_err(dev->dev, "Failure to map stolen base.\n");
+ ret = -ENOMEM;
+ goto err_mutex_destroy;
+ }
+
+ psb_gem_mm_populate_stolen(dev_priv);
+
+ return 0;
+
+err_mutex_destroy:
+ mutex_destroy(&dev_priv->mmap_mutex);
+ return ret;
+}
+
+void psb_gem_mm_fini(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+
+ iounmap(dev_priv->vram_addr);
+
+ mutex_destroy(&dev_priv->mmap_mutex);
+}
+
+/* Re-insert all pinned GEM objects into GTT. */
+static void psb_gem_mm_populate_resources(struct drm_psb_private *pdev)
+{
+ unsigned int restored = 0, total = 0, size = 0;
+ struct resource *r = pdev->gtt_mem->child;
+ struct drm_device *dev = &pdev->dev;
+ struct psb_gem_object *pobj;
+
+ while (r) {
+ /*
+* TODO: GTT restoration needs a refactoring, so that we don't
have to touch
+* struct psb_gem_object here. The type represents a GEM
object and is
+* not related to the GTT itself.
+*/
+ pobj = container_of(r, struct psb_gem_object, resource);
+ if (pobj->pages) {
+ psb_gtt_insert_pages(pdev, &pobj->resource,
pobj->pages);
+ size += resource_size(&pobj->resource);
+ ++restored;
+ }
+ r = r->sibling;
+ ++total;
+ }
+
+ drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total,
(size / 1024));
+}
+
+int psb_gem_mm_resume(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
+ unsigned long stolen_size, vram_stolen_size;
+ struct psb_gtt *pg;
+
+ pg = &dev_priv->gtt;
+
+ pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
+ vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base -
PAGE_SIZE;
+
+ stolen_size = vram_stolen_size;
+
+ dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
dev_priv->stolen_base,
+ vram_stolen_size / 1024);
+
+ if (stolen_size != pg->stolen_size) {
+ dev_err(dev->dev, "GTT resume error.\n");
+
[PATCH v2 07/12] drm/gma500: Cleanup GTT uninit and error handling
Replace psb_gtt_takedown() with finalizer function that is only called
for unloading the driver. Use roll-back pattern for error handling in
psb_gtt_init() and _resume(). Also fixes a bug where vmap_addr was never
unmapped.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gtt.c | 49
drivers/gpu/drm/gma500/gtt.h | 2 +-
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
drivers/gpu/drm/gma500/psb_drv.h | 1 -
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 43ad3ec38c80..99c644a5c5cb 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,23 +125,20 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev,
const struct resource *r
mutex_unlock(&pdev->gtt_mutex);
}
-void psb_gtt_takedown(struct drm_device *dev)
+void psb_gtt_fini(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
- if (dev_priv->gtt_map) {
- iounmap(dev_priv->gtt_map);
- dev_priv->gtt_map = NULL;
- }
- if (dev_priv->gtt_initialized) {
- pci_write_config_word(pdev, PSB_GMCH_CTRL,
- dev_priv->gmch_ctrl);
- PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
- (void) PSB_RVDC32(PSB_PGETBL_CTL);
- }
- if (dev_priv->vram_addr)
- iounmap(dev_priv->gtt_map);
+ iounmap(dev_priv->vram_addr);
+ iounmap(dev_priv->gtt_map);
+
+ pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+ PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+ (void)PSB_RVDC32(PSB_PGETBL_CTL);
+
+ mutex_destroy(&dev_priv->mmap_mutex);
+ mutex_destroy(&dev_priv->gtt_mutex);
}
/* Clear GTT. Use a scratch page to avoid accidents or scribbles. */
@@ -233,8 +230,6 @@ int psb_gtt_init(struct drm_device *dev)
(void) PSB_RVDC32(PSB_PGETBL_CTL);
/* The root resource we allocate address space from */
- dev_priv->gtt_initialized = 1;
-
pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
/*
@@ -299,14 +294,14 @@ int psb_gtt_init(struct drm_device *dev)
if (!dev_priv->gtt_map) {
dev_err(dev->dev, "Failure to map gtt.\n");
ret = -ENOMEM;
- goto out_err;
+ goto err_gtt_disable;
}
dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
if (!dev_priv->vram_addr) {
dev_err(dev->dev, "Failure to map stolen base.\n");
ret = -ENOMEM;
- goto out_err;
+ goto err_iounmap;
}
psb_gtt_clear(dev_priv);
@@ -314,8 +309,14 @@ int psb_gtt_init(struct drm_device *dev)
return 0;
-out_err:
- psb_gtt_takedown(dev);
+err_iounmap:
+ iounmap(dev_priv->gtt_map);
+err_gtt_disable:
+ pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+ PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+ (void)PSB_RVDC32(PSB_PGETBL_CTL);
+ mutex_destroy(&dev_priv->mmap_mutex);
+ mutex_destroy(&dev_priv->gtt_mutex);
return ret;
}
@@ -340,8 +341,6 @@ static int psb_gtt_resume(struct drm_device *dev)
(void) PSB_RVDC32(PSB_PGETBL_CTL);
/* The root resource we allocate address space from */
- dev_priv->gtt_initialized = 1;
-
pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
/*
@@ -398,7 +397,7 @@ static int psb_gtt_resume(struct drm_device *dev)
if ((gtt_pages != pg->gtt_pages) && (stolen_size != pg->stolen_size)) {
dev_err(dev->dev, "GTT resume error.\n");
ret = -EINVAL;
- goto out_err;
+ goto err_gtt_disable;
}
pg->gtt_pages = gtt_pages;
@@ -410,8 +409,10 @@ static int psb_gtt_resume(struct drm_device *dev)
return 0;
-out_err:
- psb_gtt_takedown(dev);
+err_gtt_disable:
+ pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+ PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+ (void)PSB_RVDC32(PSB_PGETBL_CTL);
return ret;
}
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index cb270ea40794..5839d5d06adc 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -26,7 +26,7 @@ struct psb_gtt {
/* Exported functions */
int psb_gtt_init(struct drm_device *dev);
-extern void psb_gtt_takedown(struct drm_device *dev);
+void psb_gtt_fini(struct drm_device *dev);
extern int psb_gtt_restore(struct drm_device *dev);
int psb_gtt_allocate_resource(struct drm_psb_private *pdev, struct resource
*res,
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 2891a3dc8d2e..41be6e1ac7f9 100644
--- a/drivers
[PATCH v2 11/12] drm/gma500: Move GTT enable and disable code into helpers
Move the code for enabling and disabling the GTT into helpers and call
the functions in psb_gtt_init(), psb_gtt_fini() and psb_gtt_resume().
Removes code duplication.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/gtt.c | 81
1 file changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index b03feec64f01..83d9a9f7c73c 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,17 +125,44 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev,
const struct resource *r
mutex_unlock(&pdev->gtt_mutex);
}
-void psb_gtt_fini(struct drm_device *dev)
+static int psb_gtt_enable(struct drm_psb_private *dev_priv)
{
- struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct drm_device *dev = &dev_priv->dev;
struct pci_dev *pdev = to_pci_dev(dev->dev);
+ int ret;
- iounmap(dev_priv->gtt_map);
+ ret = pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
+ if (ret)
+ return pcibios_err_to_errno(ret);
+ ret = pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl |
_PSB_GMCH_ENABLED);
+ if (ret)
+ return pcibios_err_to_errno(ret);
+
+ dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
+ PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
+
+ (void)PSB_RVDC32(PSB_PGETBL_CTL);
+
+ return 0;
+}
+
+static void psb_gtt_disable(struct drm_psb_private *dev_priv)
+{
+ struct drm_device *dev = &dev_priv->dev;
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+
(void)PSB_RVDC32(PSB_PGETBL_CTL);
+}
+void psb_gtt_fini(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+
+ iounmap(dev_priv->gtt_map);
+ psb_gtt_disable(dev_priv);
mutex_destroy(&dev_priv->gtt_mutex);
}
@@ -159,22 +186,15 @@ int psb_gtt_init(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
+ struct psb_gtt *pg = &dev_priv->gtt;
unsigned gtt_pages;
- struct psb_gtt *pg;
- int ret = 0;
+ int ret;
mutex_init(&dev_priv->gtt_mutex);
- pg = &dev_priv->gtt;
-
- /* Enable the GTT */
- pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
- pci_write_config_word(pdev, PSB_GMCH_CTRL,
- dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
-
- dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
- PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
- (void) PSB_RVDC32(PSB_PGETBL_CTL);
+ ret = psb_gtt_enable(dev_priv);
+ if (ret)
+ goto err_mutex_destroy;
/* The root resource we allocate address space from */
pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
@@ -227,17 +247,16 @@ int psb_gtt_init(struct drm_device *dev)
if (!dev_priv->gtt_map) {
dev_err(dev->dev, "Failure to map gtt.\n");
ret = -ENOMEM;
- goto err_gtt_disable;
+ goto err_psb_gtt_disable;
}
psb_gtt_clear(dev_priv);
return 0;
-err_gtt_disable:
- pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
- PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
- (void)PSB_RVDC32(PSB_PGETBL_CTL);
+err_psb_gtt_disable:
+ psb_gtt_disable(dev_priv);
+err_mutex_destroy:
mutex_destroy(&dev_priv->gtt_mutex);
return ret;
}
@@ -246,20 +265,14 @@ int psb_gtt_resume(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
+ struct psb_gtt *pg = &dev_priv->gtt;
unsigned int gtt_pages;
- struct psb_gtt *pg;
int ret;
- pg = &dev_priv->gtt;
-
/* Enable the GTT */
- pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
- pci_write_config_word(pdev, PSB_GMCH_CTRL,
- dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
-
- dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
- PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
- (void) PSB_RVDC32(PSB_PGETBL_CTL);
+ ret = psb_gtt_enable(dev_priv);
+ if (ret)
+ return ret;
/* The root resource we allocate address space from */
pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
@@ -311,16 +324,14 @@ int psb_gtt_resume(struct drm_device *dev)
if (gtt_pages != pg->gtt_pages) {
dev_err(dev->dev, "GTT resume error.\n");
ret = -EINVAL;
- goto err_gtt_disable;
+ g
