Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
On Mon, 2019-09-09 at 13:25 +0300, Ville Syrjälä wrote: > On Sat, Sep 07, 2019 at 11:19:55PM +, Mun, Gwan-gyeong wrote: > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote: > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä > > > wrote: > > > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote: > > > > > > -Original Message- > > > > > > From: Ilia Mirkin > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM > > > > > > To: Mun, Gwan-gyeong > > > > > > Cc: Intel Graphics Development < > > > > > > intel-...@lists.freedesktop.org > > > > > > > ; Shankar, Uma > > > > > > ; dri-devel < > > > > > > dri-devel@lists.freedesktop.org> > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace > > > > > > property > > > > > > > > > > > > So how would this work with a DP++ connector? Should it > > > > > > list > > > > > > the HDMI or DP > > > > > > properties? Or do we need a custom property checker which > > > > > > is > > > > > > aware of what is > > > > > > currently plugged in to validate the values? > > > > > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving > > > > > DP > > > > > or HDMI (with a passive dongle). > > > > > Based on the type of sink detected, we should expose DP or > > > > > HDMI > > > > > colorspaces to userspace. > > > > > > > > For i915 DP connector always drives DP mode, HDMI connector > > > > always > > > > drives > > > > HDMI mode, even when the physical connector is DP++. > > > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and > > > amdgpu > > > create 1 connector (not sure about other drivers) for a single > > > physical DP++ socket. Since we supply the list of valid values at > > > the > > > time of creating the connector, we can't know at that point > > > whether > > > in > > > the future a HDMI or DP will be plugged into it. > > > > > > -ilia > > Ilia, does it mean that the drm_connector type is > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode? > > > > And Ville and Uma, when we are useing dp active dongle (DP to HDMI > > dongle and DP branch device is HDMI) should we expose HDMI > > colorspace? > > We still set it up via DP MSA/VSC no? In that case it should follow > the > DP spec I think. LSPCON is probably different because we manually Yes, I agree too. - G.G. > generate > the AVI infoframe for it. But I'm not sure how we're going to > reconcile > that with the DP stuff we also set up for it. > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
On Sat, 2019-09-07 at 21:43 -0400, Ilia Mirkin wrote: > On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong > wrote: > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote: > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä > > > wrote: > > > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote: > > > > > > -Original Message- > > > > > > From: Ilia Mirkin > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM > > > > > > To: Mun, Gwan-gyeong > > > > > > Cc: Intel Graphics Development < > > > > > > intel-...@lists.freedesktop.org > > > > > > > ; Shankar, Uma > > > > > > ; dri-devel < > > > > > > dri-devel@lists.freedesktop.org> > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace > > > > > > property > > > > > > > > > > > > So how would this work with a DP++ connector? Should it > > > > > > list > > > > > > the HDMI or DP > > > > > > properties? Or do we need a custom property checker which > > > > > > is > > > > > > aware of what is > > > > > > currently plugged in to validate the values? > > > > > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving > > > > > DP > > > > > or HDMI (with a passive dongle). > > > > > Based on the type of sink detected, we should expose DP or > > > > > HDMI > > > > > colorspaces to userspace. > > > > > > > > For i915 DP connector always drives DP mode, HDMI connector > > > > always > > > > drives > > > > HDMI mode, even when the physical connector is DP++. > > > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and > > > amdgpu > > > create 1 connector (not sure about other drivers) for a single > > > physical DP++ socket. Since we supply the list of valid values at > > > the > > > time of creating the connector, we can't know at that point > > > whether > > > in > > > the future a HDMI or DP will be plugged into it. > > > > > > -ilia > > Ilia, does it mean that the drm_connector type is > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode? > > That is correct. The connector type is "DisplayPort" in such a case. > > Cheers, > > -ilia For now drm_mode_create_colorspace_property() is only used for i915. IMHO, when other drivers ( nouveau, radeon, and amdgpu ) are ready for using of drm_mode_create_colorspace_property(), what about do we add a variable which can identify DP++ and DP to drm_connector? And when the drivers (nouveau, radeon, and amdgpu) detect the current protocol, the drivers will set the variable. Br, - G.G. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
On Tue, Sep 10, 2019 at 07:34:31AM +, Mun, Gwan-gyeong wrote: > On Sat, 2019-09-07 at 21:43 -0400, Ilia Mirkin wrote: > > On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong > > wrote: > > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote: > > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä > > > > wrote: > > > > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote: > > > > > > > -Original Message- > > > > > > > From: Ilia Mirkin > > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM > > > > > > > To: Mun, Gwan-gyeong > > > > > > > Cc: Intel Graphics Development < > > > > > > > intel-...@lists.freedesktop.org > > > > > > > > ; Shankar, Uma > > > > > > > ; dri-devel < > > > > > > > dri-devel@lists.freedesktop.org> > > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace > > > > > > > property > > > > > > > > > > > > > > So how would this work with a DP++ connector? Should it > > > > > > > list > > > > > > > the HDMI or DP > > > > > > > properties? Or do we need a custom property checker which > > > > > > > is > > > > > > > aware of what is > > > > > > > currently plugged in to validate the values? > > > > > > > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving > > > > > > DP > > > > > > or HDMI (with a passive dongle). > > > > > > Based on the type of sink detected, we should expose DP or > > > > > > HDMI > > > > > > colorspaces to userspace. > > > > > > > > > > For i915 DP connector always drives DP mode, HDMI connector > > > > > always > > > > > drives > > > > > HDMI mode, even when the physical connector is DP++. > > > > > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and > > > > amdgpu > > > > create 1 connector (not sure about other drivers) for a single > > > > physical DP++ socket. Since we supply the list of valid values at > > > > the > > > > time of creating the connector, we can't know at that point > > > > whether > > > > in > > > > the future a HDMI or DP will be plugged into it. > > > > > > > > -ilia > > > Ilia, does it mean that the drm_connector type is > > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode? > > > > That is correct. The connector type is "DisplayPort" in such a case. > > > > Cheers, > > > > -ilia > > For now drm_mode_create_colorspace_property() is only used for i915. > IMHO, when other drivers ( nouveau, radeon, and amdgpu ) are ready for > using of drm_mode_create_colorspace_property(), > what about do we add a variable which can identify DP++ and DP to > drm_connector? > And when the drivers (nouveau, radeon, and amdgpu) detect the current > protocol, the drivers will set the variable. IMO better to just have two functions in that case: one for DP, another for HDMI. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: HDCP Content Type Interface
On Mon, 09 Sep 2019, "Lakha, Bhawanpreet" wrote: > Hi all, > > This is regarding the recent hdcp content type patch that was merged > into > drm-misc. > (https://patchwork.freedesktop.org/patch/320958/?series=57233&rev=11) > > There are displays on the market that advertise HDCP 2.2 support and > will pass authentication and encryption but will then show a > corrupted/blue/black screen (the driver cannot detect this). These > displays work with HDCP 1.4 without any issues. Due to the large > number of HDCP-supporting devices on the market we might not be able > to catch them with a blacklist. Has this been confirmed with some other HDCP source than the recently merged HDCP 2.2 support? I.e. has it been confirmed the issue is really in the sinks rather than in the source support? BR, Jani. > > From the user modes perspective, HDCP1.4 and HDCP2.2 Type0 are the > same thing. Meaning that this interface doesn't allow us to force the > hdcp version. Due to the problems mentioned above we might want to > expose the ability for a user to force an HDCP downgrade to a certain > level (e.g. 1.4) in case they experience problems. > > What are your thoughts? and what would be a good way to deal with it? > > > Thanks, > > Bhawan > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108718] Raven Ridge: ring sdma0 timeout on heavy CSS website with Firefox WebRender
https://bugs.freedesktop.org/show_bug.cgi?id=108718 --- Comment #4 from Pierre-Eric Pelloux-Prayer --- (In reply to Pierre-Eric Pelloux-Prayer from comment #2) > Can you still reproduce this issue? > > It seems to work fine here with a recent kernel + mesa configuration. Correction: I retested and can reproduce the problem partially. After zooming and scrolling for a while, there are logs like this in dmesg: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_vm_validate_pt_bos() failed. [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission! [TTM] Failed to find memory space for buffer 0xde5c5282 eviction [TTM] No space for de5c5282 (2224 pages, 8896K, 8M) [TTM]placement[0]=0x00060002 (1) [TTM] has_type: 1 [TTM] use_type: 1 [TTM] flags: 0x000A [TTM] gpu_offset: 0x [TTM] size: 786432 [TTM] available_caching: 0x0007 [TTM] default_caching: 0x0001 [TTM] 0x0400-0x0401: 1: used [TTM] 0x0401-0x0443: 66: used [TTM] 0x0443-0x0445: 2: used [TTM] 0x0445-0x0447: 2: used [TTM] 0x0447-0x0449: 2: used [TTM] 0x0449-0x044b: 2: used [TTM] 0x044b-0x044d: 2: used [TTM] 0x044d-0x044f: 2: used [TTM] 0x044f-0x0451: 2: used [TTM] 0x0451-0x0453: 2: used [TTM] 0x0453-0x0455: 2: used [TTM] 0x0455-0x0456: 1: used [TTM] 0x0456-0x0556: 256: used [TTM] 0x0556-0x0557: 1: used [TTM] 0x0557-0x0558: 1: used [TTM] 0x0558-0x0559: 1: used [TTM] 0x0559-0x055a: 1: used [TTM] 0x055a-0x055b: 1: used [TTM] 0x055b-0x055c: 1: used [TTM] 0x055c-0x055d: 1: used [TTM] 0x055d-0x055e: 1: used [TTM] 0x055e-0x0560: 2: used [TTM] 0x0560-0x0561: 1: used [TTM] 0x0561-0x0562: 1: used [TTM] 0x0562-0x0563: 1: used [TTM] 0x0563-0x0565: 2: used [TTM] 0x0565-0x0665: 256: used [TTM] 0x0665-0x075c: 247: used [TTM] 0x075c-0x0800: 164: free [TTM] 0x0800-0x0900: 256: used [TTM] 0x0900-0x0cca: 970: used [TTM] 0x0cca-0x0004: 258870: free [TTM] total: 261120, used 2086 free 259034 The computer doesn't hang or crash though and if I close Firefox's window everything goes back to normal. (using: Mesa master, LLVM 10, kernel 865b4ca43816 from amd-staging-drm-next) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108718] Raven Ridge: ring sdma0 timeout on heavy CSS website with Firefox WebRender
https://bugs.freedesktop.org/show_bug.cgi?id=108718 --- Comment #5 from Pierre-Eric Pelloux-Prayer --- (In reply to Haxk20 from comment #1) > I have the same exact bug. As a workaround i used earlier build of mesa but > indeed the latest one is borked and unusable in Firefox. > And without webrenderer firefox is kind of unusable as the color gradient > look horrible. What is the version of this "earlier build of mesa" that doesn't have this problem? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] FBTFT: fb_agm1264k: usleep_range is preferred over udelay
Hi Sreeram,
On Tue, Sep 10, 2019 at 2:25 AM Sreeram Veluthakkal wrote:
> On Mon, Sep 09, 2019 at 10:56:25AM +0100, Greg KH wrote:
> > On Sun, Sep 08, 2019 at 08:26:05PM -0500, Sreeram Veluthakkal wrote:
> > > This patch fixes the issue:
> > > FILE: drivers/staging/fbtft/fb_agm1264k-fl.c:88:
> > > CHECK: usleep_range is preferred over udelay; see
> > > Documentation/timers/timers-howto.rst
> > > + udelay(20);
> > >
> > > Signed-off-by: Sreeram Veluthakkal
Thanks for your patch!
> > > --- a/drivers/staging/fbtft/fb_agm1264k-fl.c
> > > +++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
> > > @@ -85,7 +85,7 @@ static void reset(struct fbtft_par *par)
> > > dev_dbg(par->info->device, "%s()\n", __func__);
> > >
> > > gpiod_set_value(par->gpio.reset, 0);
> > > - udelay(20);
> > > + usleep_range(20, 40);
> >
> > Is it "safe" to wait 40? This kind of change you can only do if you
> > know this is correct. Have you tested this with hardware?
> >
> > thanks,
> >
> > greg k-h
>
> Hi Greg, No I haven't tested it, I don't have the hw. I dug depeer in to the
> usleep_range
>
> https://github.com/torvalds/linux/blob/master/kernel/time/timer.c#L1993
> u64 delta = (u64)(max - min) * NSEC_PER_USEC;
>
> * The @delta argument gives the kernel the freedom to schedule the
> * actual wakeup to a time that is both power and performance friendly.
> * The kernel give the normal best effort behavior for "@expires+@delta",
> * but may decide to fire the timer earlier, but no earlier than @expires.
>
> My understanding is that keeping delta 0 (min=max=20) would be equivalent.
> I can revise the patch to usleep_range(20, 20) or usleep_range(20, 21) for a
> 1 usec delta.
> What do you suggest?
Please read the comment above the line you're referring to:
* In non-atomic context where the exact wakeup time is flexible, use
* usleep_range() instead of udelay(). The sleep improves responsiveness
* by avoiding the CPU-hogging busy-wait of udelay(), and the range reduces
* power usage by allowing hrtimers to take advantage of an already-
* scheduled interrupt instead of scheduling a new one just for this sleep.
Is this function always called in non-atomic context?
If it may be called in atomic context, replacing the udelay() call by a
usleep*() call will break the driver.
See also "the first and most important question" in
Documentation/timers/timers-howto.rst, as referred to by the checkpatch.pl
message.
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [EXT] Re: [v4 2/2] drm/arm/mali-dp: Add display QoS interface configuration for Mali DP500
> -Original Message-
> From: Liviu Dudau
> Sent: 2019年9月6日 22:18
> To: Wen He
> Cc: linux-de...@linux.nxdi.nxp.com; Brian Starkey ;
> David Airlie ; Daniel Vetter ; Rob Herring
> ; Mark Rutland ;
> dri-devel@lists.freedesktop.org; devicet...@vger.kernel.org;
> linux-ker...@vger.kernel.org; Leo Li
> Subject: [EXT] Re: [v4 2/2] drm/arm/mali-dp: Add display QoS interface
> configuration for Mali DP500
>
> Caution: EXT Email
>
> Hi Wen,
>
> On Thu, Aug 22, 2019 at 10:11:35AM +0800, Wen He wrote:
> > Configure the display Quality of service (QoS) levels priority if the
> > optional property node "arm,malidp-aqros-value" is defined in DTS file.
> >
> > QoS signaling using AQROS and AWQOS AXI interface signals, the AQROS
> > is driven from the "RQOS" register, so needed to program the RQOS
> > register to avoid the high resolutions flicker issue on the LS1028A
> > platform.
> >
> > Signed-off-by: Wen He
> > ---
> > drivers/gpu/drm/arm/malidp_drv.c | 6 ++
> > drivers/gpu/drm/arm/malidp_hw.c | 13 +
> > drivers/gpu/drm/arm/malidp_hw.h | 3 +++
> > drivers/gpu/drm/arm/malidp_regs.h | 10 ++
> > 4 files changed, 32 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c
> > b/drivers/gpu/drm/arm/malidp_drv.c
> > index c27ff456eddc..80e8d15760ac 100644
> > --- a/drivers/gpu/drm/arm/malidp_drv.c
> > +++ b/drivers/gpu/drm/arm/malidp_drv.c
> > @@ -815,6 +815,12 @@ static int malidp_bind(struct device *dev)
> >
> > malidp->core_id = version;
> >
> > + ret = of_property_read_u32(dev->of_node,
> > + "arm,malidp-arqos-value",
> > + &hwdev->arqos_value);
> > + if (ret)
> > + hwdev->arqos_value = 0x0;
> > +
> > /* set the number of lines used for output of RGB data */
> > ret = of_property_read_u8_array(dev->of_node,
> > "arm,malidp-output-port-lines",
> > diff --git a/drivers/gpu/drm/arm/malidp_hw.c
> > b/drivers/gpu/drm/arm/malidp_hw.c index 380be66d4c6e..f90a367a5bc9
> > 100644
> > --- a/drivers/gpu/drm/arm/malidp_hw.c
> > +++ b/drivers/gpu/drm/arm/malidp_hw.c
> > @@ -374,6 +374,19 @@ static void malidp500_modeset(struct
> malidp_hw_device *hwdev, struct videomode *
> > malidp_hw_setbits(hwdev, MALIDP_DISP_FUNC_ILACED,
> MALIDP_DE_DISPLAY_FUNC);
> > else
> > malidp_hw_clearbits(hwdev, MALIDP_DISP_FUNC_ILACED,
> > MALIDP_DE_DISPLAY_FUNC);
> > +
> > + /*
> > + * Program the RQoS register to avoid high resolutions flicker
> > + * issue on the LS1028A.
> > + */
> > + if (hwdev->arqos_value) {
> > + val = hwdev->arqos_value;
> > +
> > + if (mode->pixelclock > 14850)
> > + malidp_hw_setbits(hwdev, val,
> MALIDP500_RQOS_QUALITY);
> > + else
> > + malidp_hw_clearbits(hwdev, val,
> MALIDP500_RQOS_QUALITY);
> > + }
>
> This application of the arqos_value based on a pixel clock value bothers me,
> because it has two problems:
>
> 1. Some other user of the Mali DP driver can't apply a system QoS value that
> they can now specify in the DT, unless the requested pixel clock is bigger
> than
> 145MHz. :(
>
> 2. (A guess) The flickering issue shows up on a combination of pixelclock and
> resolution (i.e. it is a bandwidth problem), but you only address one of the
> variables. Haven't tested on the LS1028A yet, but do you know if
> (theoretically)
> it would have a flicker problem doing 640x480@200MHz without the QoS
> value?
>
> How about this instead?
>
> --8<-
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c
> b/drivers/gpu/drm/arm/malidp_hw.c index 380be66d4c6eb..e2f96dce13850
> 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -374,6 +374,22 @@ static void malidp500_modeset(struct
> malidp_hw_device *hwdev, struct videomode *
> malidp_hw_setbits(hwdev, MALIDP_DISP_FUNC_ILACED,
> MALIDP_DE_DISPLAY_FUNC);
> else
> malidp_hw_clearbits(hwdev, MALIDP_DISP_FUNC_ILACED,
> MALIDP_DE_DISPLAY_FUNC);
> +
> + /*
> +* Program the RQoS register. LS1028A has an issue where screen
> will
> +* flicker on pixelclocks higher than 148.5MHz but otherwise doesn't
> +* want an RQoS value, so special case it for them.
> +*/
> + if (hwdev->arqos_value) {
> + val = hwdev->arqos_value;
> +
> +#ifdef MALIDP_LS1028A
> + if (mode->pixelclock <= 14850)
> + malidp_hw_clearbits(hwdev, val,
> MALIDP500_RQOS_QUALITY);
> + else
> +#endif
> + malidp_hw_setbits(hwdev, val,
> MALIDP500_RQOS_QUALITY);
> + }
> }
>
> int malidp_format_get_bpp(u32 fmt)
> --8<-
>
> And then
[v5 1/2] dt/bindings: display: Add optional property node define for Mali DP500
Add optional property node 'arm,malidp-arqos-value' for the Mali DP500.
This property describe the ARQoS levels of DP500's QoS signaling.
Signed-off-by: Wen He
Reviewed-by: Rob Herring
---
Documentation/devicetree/bindings/display/arm,malidp.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/arm,malidp.txt
b/Documentation/devicetree/bindings/display/arm,malidp.txt
index 2f7870983ef1..7a97a2b48c2a 100644
--- a/Documentation/devicetree/bindings/display/arm,malidp.txt
+++ b/Documentation/devicetree/bindings/display/arm,malidp.txt
@@ -37,6 +37,8 @@ Optional properties:
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt)
to be used for the framebuffer; if not present, the framebuffer may
be located anywhere in memory.
+ - arm,malidp-arqos-high-level: integer of u32 value describing the ARQoS
+levels of DP500's QoS signaling.
Example:
@@ -54,6 +56,7 @@ Example:
clocks = <&oscclk2>, <&fpgaosc0>, <&fpgaosc1>, <&fpgaosc1>;
clock-names = "pxlclk", "mclk", "aclk", "pclk";
arm,malidp-output-port-lines = /bits/ 8 <8 8 8>;
+ arm,malidp-arqos-high-level = <0xd000d000>;
port {
dp0_output: endpoint {
remote-endpoint = <&tda998x_2_input>;
--
2.17.1
[v5 2/2] drm/arm/mali-dp: Add display QoS interface configuration for Mali DP500
Configure the display Quality of service (QoS) levels priority if the
optional property node "arm,malidp-aqros-value" is defined in DTS file.
QoS signaling using AQROS and AWQOS AXI interface signals, the AQROS is
driven from the "RQOS" register, so needed to program the RQOS register
to avoid the high resolutions flicker issue on the LS1028A platform.
Signed-off-by: Wen He
---
drivers/gpu/drm/arm/malidp_drv.c | 6 ++
drivers/gpu/drm/arm/malidp_hw.c | 9 +
drivers/gpu/drm/arm/malidp_hw.h | 3 +++
drivers/gpu/drm/arm/malidp_regs.h | 10 ++
4 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 333b88a5efb0..8a76315aaa0f 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -817,6 +817,12 @@ static int malidp_bind(struct device *dev)
malidp->core_id = version;
+ ret = of_property_read_u32(dev->of_node,
+ "arm,malidp-arqos-value",
+ &hwdev->arqos_value);
+ if (ret)
+ hwdev->arqos_value = 0x0;
+
/* set the number of lines used for output of RGB data */
ret = of_property_read_u8_array(dev->of_node,
"arm,malidp-output-port-lines",
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index bd8265f02e0b..ca570b135478 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -379,6 +379,15 @@ static void malidp500_modeset(struct malidp_hw_device
*hwdev, struct videomode *
malidp_hw_setbits(hwdev, MALIDP_DISP_FUNC_ILACED,
MALIDP_DE_DISPLAY_FUNC);
else
malidp_hw_clearbits(hwdev, MALIDP_DISP_FUNC_ILACED,
MALIDP_DE_DISPLAY_FUNC);
+
+ /*
+* Program the RQoS register to avoid high resolutions flicker
+* issue on the LS1028A.
+*/
+ if (hwdev->arqos_value) {
+ val = hwdev->arqos_value;
+ malidp_hw_setbits(hwdev, val, MALIDP500_RQOS_QUALITY);
+ }
}
int malidp_format_get_bpp(u32 fmt)
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 968a65eed371..e4c36bc90bda 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -251,6 +251,9 @@ struct malidp_hw_device {
/* size of memory used for rotating layers, up to two banks available */
u32 rotation_memory[2];
+
+ /* priority level of RQOS register used for driven the ARQOS signal */
+ u32 arqos_value;
};
static inline u32 malidp_hw_read(struct malidp_hw_device *hwdev, u32 reg)
diff --git a/drivers/gpu/drm/arm/malidp_regs.h
b/drivers/gpu/drm/arm/malidp_regs.h
index 993031542fa1..514c50dcb74d 100644
--- a/drivers/gpu/drm/arm/malidp_regs.h
+++ b/drivers/gpu/drm/arm/malidp_regs.h
@@ -210,6 +210,16 @@
#define MALIDP500_CONFIG_VALID 0x00f00
#define MALIDP500_CONFIG_ID0x00fd4
+/*
+ * The quality of service (QoS) register on the DP500. RQOS register values
+ * are driven by the ARQOS signal, using AXI transacations, dependent on the
+ * FIFO input level.
+ * The RQOS register can also set QoS levels for:
+ *- RED_ARQOS @ A 4-bit signal value for close to underflow conditions
+ *- GREEN_ARQOS @ A 4-bit signal value for normal conditions
+ */
+#define MALIDP500_RQOS_QUALITY 0x00500
+
/* register offsets and bits specific to DP550/DP650 */
#define MALIDP550_ADDR_SPACE_SIZE 0x1
#define MALIDP550_DE_CONTROL 0x00010
--
2.17.1
Re: [PATCH v2 1/2] iommu: add support for drivers that manage iommu explicitly
On Fri, Sep 06, 2019 at 02:44:01PM -0700, Rob Clark wrote: > @@ -674,7 +674,7 @@ int iommu_group_add_device(struct iommu_group *group, > struct device *dev) > > mutex_lock(&group->mutex); > list_add_tail(&device->list, &group->devices); > - if (group->domain) > + if (group->domain && !(dev->driver && > dev->driver->driver_manages_iommu)) Hmm, this code usually runs at enumeration time when no driver is attached to the device. Actually it would be pretty dangerous when this code runs while a driver is attached to the device. How does that change make things work for you? Regards, Joerg
Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
On 2019-09-07 10:32 p.m., Hans de Goede wrote: > Bail from the pci_driver probe function instead of from the drm_driver > load function. > > This avoid /dev/dri/card0 temporarily getting registered and then > unregistered again, sending unwanted add / remove udev events to > userspace. > > Specifically this avoids triggering the (userspace) bug fixed by this > plymouth merge-request: > https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59 > > Note that despite that being an userspace bug, not sending unnecessary > udev events is a good idea in general. > > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490 > Signed-off-by: Hans de Goede Reviewed-by: Michel Dänzer amdgpu should be changed correspondingly as well. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 07/27] drm/dp_mst: Add sideband down request tracing + selftests
On Tue, 03 Sep 2019, Lyude Paul wrote:
> Unfortunately the DP MST helpers do not have much in the way of
> debugging utilities. So, let's add some!
>
> This adds basic debugging output for down sideband requests that we send
> from the driver, so that we can actually discern what's happening when
> sideband requests timeout.
>
> Since there wasn't really a good way of testing that any of this worked,
> I ended up writing simple selftests that lightly test sideband message
> encoding and decoding as well. Enjoy!
>
> Changes since v1:
> * Clean up DO_TEST() and sideband_msg_req_encode_decode() - danvet
> * Get rid of pr_fmt(), just define a prefix string instead and use
> drm_printf()
> * Check highest bit of VCPI in drm_dp_decode_sideband_req() - danvet
> * Make the switch case order between drm_dp_decode_sideband_req() and
> drm_dp_encode_sideband_req() the same - danvet
> * Only check DRM_UT_DP - danvet
> * Clean up sideband_msg_req_equal() from selftests a bit, and add
> comments explaining why we can't just use memcmp - danvet
>
> Cc: Juston Li
> Cc: Imre Deak
> Cc: Ville Syrjälä
> Cc: Harry Wentland
> Reviewed-by: Daniel Vetter
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 309 +-
> .../gpu/drm/drm_dp_mst_topology_internal.h| 24 ++
> .../gpu/drm/selftests/drm_modeset_selftests.h | 1 +
> .../drm/selftests/test-drm_dp_mst_helper.c| 204
> .../drm/selftests/test-drm_modeset_common.h | 1 +
> include/drm/drm_dp_mst_helper.h | 2 +-
> 6 files changed, 536 insertions(+), 5 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_dp_mst_topology_internal.h
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1c862749cb63..f5f1d8b50fb6 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -37,6 +37,7 @@
> #include
>
> #include "drm_crtc_helper_internal.h"
> +#include "drm_dp_mst_topology_internal.h"
>
> /**
> * DOC: dp mst helper
> @@ -73,6 +74,8 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux
> *aux);
> static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux);
> static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
>
> +#define DBG_PREFIX "[dp_mst]"
> +
> #define DP_STR(x) [DP_ ## x] = #x
>
> static const char *drm_dp_mst_req_type_str(u8 req_type)
> @@ -129,6 +132,43 @@ static const char *drm_dp_mst_nak_reason_str(u8
> nak_reason)
> }
>
> #undef DP_STR
> +#define DP_STR(x) [DRM_DP_SIDEBAND_TX_ ## x] = #x
> +
> +static const char *drm_dp_mst_sideband_tx_state_str(int state)
> +{
> + static const char * const sideband_reason_str[] = {
> + DP_STR(QUEUED),
> + DP_STR(START_SEND),
> + DP_STR(SENT),
> + DP_STR(RX),
> + DP_STR(TIMEOUT),
> + };
> +
> + if (state >= ARRAY_SIZE(sideband_reason_str) ||
> + !sideband_reason_str[state])
> + return "unknown";
> +
> + return sideband_reason_str[state];
> +}
> +
> +static int
> +drm_dp_mst_rad_to_str(const u8 rad[8], u8 lct, char *out, size_t len)
> +{
> + int i;
> + u8 unpacked_rad[16];
> +
> + for (i = 0; i < lct; i++) {
> + if (i % 2)
> + unpacked_rad[i] = rad[i / 2] >> 4;
> + else
> + unpacked_rad[i] = rad[i / 2] & BIT_MASK(4);
> + }
> +
> + /* TODO: Eventually add something to printk so we can format the rad
> + * like this: 1.2.3
> + */
> + return snprintf(out, len, "%*phC", lct, unpacked_rad);
> +}
>
> /* sideband msg handling */
> static u8 drm_dp_msg_header_crc4(const uint8_t *data, size_t num_nibbles)
> @@ -261,8 +301,9 @@ static bool drm_dp_decode_sideband_msg_hdr(struct
> drm_dp_sideband_msg_hdr *hdr,
> return true;
> }
>
> -static void drm_dp_encode_sideband_req(struct drm_dp_sideband_msg_req_body
> *req,
> -struct drm_dp_sideband_msg_tx *raw)
> +void
> +drm_dp_encode_sideband_req(const struct drm_dp_sideband_msg_req_body *req,
> +struct drm_dp_sideband_msg_tx *raw)
> {
> int idx = 0;
> int i;
> @@ -363,6 +404,251 @@ static void drm_dp_encode_sideband_req(struct
> drm_dp_sideband_msg_req_body *req,
> }
> raw->cur_len = idx;
> }
> +EXPORT_SYMBOL_FOR_TESTS_ONLY(drm_dp_encode_sideband_req);
> +
> +/* Decode a sideband request we've encoded, mainly used for debugging */
> +int
> +drm_dp_decode_sideband_req(const struct drm_dp_sideband_msg_tx *raw,
> +struct drm_dp_sideband_msg_req_body *req)
> +{
> + const u8 *buf = raw->msg;
> + int i, idx = 0;
> +
> + req->req_type = buf[idx++] & 0x7f;
> + switch (req->req_type) {
> + case DP_ENUM_PATH_RESOURCES:
> + case DP_POWER_DOWN_PHY:
> + case DP_POWER_UP_PHY:
> + req->u.port_num.port_number = (buf[idx
[PATCH 2/9] drm/amdgpu: split the VM entity into direct and delayed
For page fault handling we need to use a direct update which can't be
blocked by ongoing user CS.
Signed-off-by: Christian König
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 21 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 5 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 5 +++--
4 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
index cd15540c5622..dfe155566571 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
@@ -282,7 +282,7 @@ static int amdgpu_vmid_grab_reserved(struct amdgpu_vm *vm,
!dma_fence_is_later(updates, (*id)->flushed_updates))
updates = NULL;
- if ((*id)->owner != vm->entity.fence_context ||
+ if ((*id)->owner != vm->direct.fence_context ||
job->vm_pd_addr != (*id)->pd_gpu_addr ||
updates || !(*id)->last_flush ||
((*id)->last_flush->context != fence_context &&
@@ -349,7 +349,7 @@ static int amdgpu_vmid_grab_used(struct amdgpu_vm *vm,
struct dma_fence *flushed;
/* Check all the prerequisites to using this VMID */
- if ((*id)->owner != vm->entity.fence_context)
+ if ((*id)->owner != vm->direct.fence_context)
continue;
if ((*id)->pd_gpu_addr != job->vm_pd_addr)
@@ -449,7 +449,7 @@ int amdgpu_vmid_grab(struct amdgpu_vm *vm, struct
amdgpu_ring *ring,
}
id->pd_gpu_addr = job->vm_pd_addr;
- id->owner = vm->entity.fence_context;
+ id->owner = vm->direct.fence_context;
if (job->vm_needs_flush) {
dma_fence_put(id->last_flush);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e11764164cbf..8a9ba66c8383 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2670,12 +2670,17 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
spin_lock_init(&vm->invalidated_lock);
INIT_LIST_HEAD(&vm->freed);
- /* create scheduler entity for page table updates */
- r = drm_sched_entity_init(&vm->entity, adev->vm_manager.vm_pte_rqs,
+ /* create scheduler entities for page table updates */
+ r = drm_sched_entity_init(&vm->direct, adev->vm_manager.vm_pte_rqs,
adev->vm_manager.vm_pte_num_rqs, NULL);
if (r)
return r;
+ r = drm_sched_entity_init(&vm->delayed, adev->vm_manager.vm_pte_rqs,
+ adev->vm_manager.vm_pte_num_rqs, NULL);
+ if (r)
+ goto error_free_direct;
+
vm->pte_support_ats = false;
if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE) {
@@ -2704,7 +2709,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
bp.flags &= ~AMDGPU_GEM_CREATE_SHADOW;
r = amdgpu_bo_create(adev, &bp, &root);
if (r)
- goto error_free_sched_entity;
+ goto error_free_delayed;
r = amdgpu_bo_reserve(root, true);
if (r)
@@ -2747,8 +2752,11 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
amdgpu_bo_unref(&vm->root.base.bo);
vm->root.base.bo = NULL;
-error_free_sched_entity:
- drm_sched_entity_destroy(&vm->entity);
+error_free_delayed:
+ drm_sched_entity_destroy(&vm->delayed);
+
+error_free_direct:
+ drm_sched_entity_destroy(&vm->direct);
return r;
}
@@ -2937,7 +2945,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
amdgpu_vm *vm)
spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
}
- drm_sched_entity_destroy(&vm->entity);
+ drm_sched_entity_destroy(&vm->direct);
+ drm_sched_entity_destroy(&vm->delayed);
if (!RB_EMPTY_ROOT(&vm->va.rb_root)) {
dev_err(adev->dev, "still active bo inside vm\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 3352a87b822e..7138722ee55f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -257,8 +257,9 @@ struct amdgpu_vm {
struct amdgpu_vm_pt root;
struct dma_fence*last_update;
- /* Scheduler entity for page table updates */
- struct drm_sched_entity entity;
+ /* Scheduler entities for page table updates */
+ struct drm_sched_entity direct;
+ struct drm_sched_entity delayed;
unsigned intpasid;
/* dedicated to vm */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index ddd181f5ed37..d087d6650d79 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_
[PATCH 9/9] drm/amdgpu: add graceful VM fault handling v3
Next step towards HMM support. For now just silence the retry fault and
optionally redirect the request to the dummy page.
v2: make sure the VM is not destroyed while we handle the fault.
v3: fix VM destroy check, cleanup comments
Signed-off-by: Christian König
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 73 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 4 ++
3 files changed, 79 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index db13ddd70a8d..48349e4f0701 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -3125,3 +3125,76 @@ void amdgpu_vm_set_task_info(struct amdgpu_vm *vm)
}
}
}
+
+/**
+ * amdgpu_vm_handle_fault - graceful handling of VM faults.
+ * @adev: amdgpu device pointer
+ * @pasid: PASID of the VM
+ * @addr: Address of the fault
+ *
+ * Try to gracefully handle a VM fault. Return true if the fault was handled
and
+ * shouldn't be reported any more.
+ */
+bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, unsigned int pasid,
+ uint64_t addr)
+{
+ struct amdgpu_bo *root;
+ uint64_t value, flags;
+ struct amdgpu_vm *vm;
+ long r;
+
+ spin_lock(&adev->vm_manager.pasid_lock);
+ vm = idr_find(&adev->vm_manager.pasid_idr, pasid);
+ if (vm)
+ root = amdgpu_bo_ref(vm->root.base.bo);
+ else
+ root = NULL;
+ spin_unlock(&adev->vm_manager.pasid_lock);
+
+ if (!root)
+ return false;
+
+ r = amdgpu_bo_reserve(root, true);
+ if (r)
+ goto error_unref;
+
+ /* Double check that the VM still exists */
+ spin_lock(&adev->vm_manager.pasid_lock);
+ vm = idr_find(&adev->vm_manager.pasid_idr, pasid);
+ if (vm && vm->root.base.bo != root)
+ vm = NULL;
+ spin_unlock(&adev->vm_manager.pasid_lock);
+ if (!vm)
+ goto error_unlock;
+
+ addr /= AMDGPU_GPU_PAGE_SIZE;
+ flags = AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED |
+ AMDGPU_PTE_SYSTEM;
+
+ if (amdgpu_vm_fault_stop == AMDGPU_VM_FAULT_STOP_NEVER) {
+ /* Redirect the access to the dummy page */
+ value = adev->dummy_page_addr;
+ flags |= AMDGPU_PTE_EXECUTABLE | AMDGPU_PTE_READABLE |
+ AMDGPU_PTE_WRITEABLE;
+ } else {
+ /* Let the hw retry silently on the PTE */
+ value = 0;
+ }
+
+ r = amdgpu_vm_bo_update_mapping(adev, vm, true, NULL, addr, addr + 1,
+ flags, value, NULL, NULL);
+ if (r)
+ goto error_unlock;
+
+ r = amdgpu_vm_update_pdes(adev, vm, true);
+
+error_unlock:
+ amdgpu_bo_unreserve(root);
+ if (r < 0)
+ DRM_ERROR("Can't handle page fault (%ld)\n", r);
+
+error_unref:
+ amdgpu_bo_unref(&root);
+
+ return false;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 0a97dc839f3b..4dbbe1b6b413 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -413,6 +413,8 @@ void amdgpu_vm_check_compute_bug(struct amdgpu_device
*adev);
void amdgpu_vm_get_task_info(struct amdgpu_device *adev, unsigned int pasid,
struct amdgpu_task_info *task_info);
+bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, unsigned int pasid,
+ uint64_t addr);
void amdgpu_vm_set_task_info(struct amdgpu_vm *vm);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 0d06c7901d26..ad19c8194a73 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -375,6 +375,10 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device
*adev,
}
/* If it's the first fault for this address, process it normally */
+ if (retry_fault && !in_interrupt() &&
+ amdgpu_vm_handle_fault(adev, entry->pasid, addr))
+ return 1; /* This also prevents sending it to KFD */
+
if (!amdgpu_sriov_vf(adev)) {
/*
* Issue a dummy read to wait for the status register to
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/9] drm/amdgpu: allow direct submission in the VM backends v2
This allows us to update page tables directly while in a page fault.
v2: use direct/delayed entities and still wait for moves
Signed-off-by: Christian König
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 5 +
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c | 16 ++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 25 +++--
3 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 7138722ee55f..54dcd0bcce1a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -201,6 +201,11 @@ struct amdgpu_vm_update_params {
*/
struct amdgpu_vm *vm;
+ /**
+* @direct: if changes should be made directly
+*/
+ bool direct;
+
/**
* @pages_addr:
*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
index 5222d165abfc..a2daeadd770f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
@@ -49,13 +49,6 @@ static int amdgpu_vm_cpu_prepare(struct
amdgpu_vm_update_params *p, void *owner,
{
int r;
- /* Wait for PT BOs to be idle. PTs share the same resv. object
-* as the root PD BO
-*/
- r = amdgpu_bo_sync_wait(p->vm->root.base.bo, owner, true);
- if (unlikely(r))
- return r;
-
/* Wait for any BO move to be completed */
if (exclusive) {
r = dma_fence_wait(exclusive, true);
@@ -63,7 +56,14 @@ static int amdgpu_vm_cpu_prepare(struct
amdgpu_vm_update_params *p, void *owner,
return r;
}
- return 0;
+ /* Don't wait for submissions during page fault */
+ if (p->direct)
+ return 0;
+
+ /* Wait for PT BOs to be idle. PTs share the same resv. object
+* as the root PD BO
+*/
+ return amdgpu_bo_sync_wait(p->vm->root.base.bo, owner, true);
}
/**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index d087d6650d79..38c966cedc26 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -68,17 +68,19 @@ static int amdgpu_vm_sdma_prepare(struct
amdgpu_vm_update_params *p,
if (r)
return r;
+ p->num_dw_left = ndw;
+
+ /* Wait for moves to be completed */
r = amdgpu_sync_fence(p->adev, &p->job->sync, exclusive, false);
if (r)
return r;
- r = amdgpu_sync_resv(p->adev, &p->job->sync, root->tbo.resv,
-owner, false);
- if (r)
- return r;
+ /* Don't wait for any submissions during page fault handling */
+ if (p->direct)
+ return 0;
- p->num_dw_left = ndw;
- return 0;
+ return amdgpu_sync_resv(p->adev, &p->job->sync, root->tbo.resv,
+ owner, false);
}
/**
@@ -95,23 +97,23 @@ static int amdgpu_vm_sdma_commit(struct
amdgpu_vm_update_params *p,
{
struct amdgpu_bo *root = p->vm->root.base.bo;
struct amdgpu_ib *ib = p->job->ibs;
+ struct drm_sched_entity *entity;
struct amdgpu_ring *ring;
struct dma_fence *f;
int r;
- ring = container_of(p->vm->delayed.rq->sched, struct amdgpu_ring,
- sched);
+ entity = p->direct ? &p->vm->direct : &p->vm->delayed;
+ ring = container_of(entity->rq->sched, struct amdgpu_ring, sched);
WARN_ON(ib->length_dw == 0);
amdgpu_ring_pad_ib(ring, ib);
WARN_ON(ib->length_dw > p->num_dw_left);
- r = amdgpu_job_submit(p->job, &p->vm->delayed,
- AMDGPU_FENCE_OWNER_VM, &f);
+ r = amdgpu_job_submit(p->job, entity, AMDGPU_FENCE_OWNER_VM, &f);
if (r)
goto error;
amdgpu_bo_fence(root, f, true);
- if (fence)
+ if (fence && !p->direct)
swap(*fence, f);
dma_fence_put(f);
return 0;
@@ -121,7 +123,6 @@ static int amdgpu_vm_sdma_commit(struct
amdgpu_vm_update_params *p,
return r;
}
-
/**
* amdgpu_vm_sdma_copy_ptes - copy the PTEs from mapping
*
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/9] drm/ttm: return -EBUSY on pipelining with no_gpu_wait
Setting the no_gpu_wait flag means that the allocate BO must be available
immediately and we can't wait for any GPU operation to finish.
Signed-off-by: Christian König
Acked-by: Felix Kuehling
---
drivers/gpu/drm/ttm/ttm_bo.c | 43 +---
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2070e8a57ed8..2899702139fb 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -924,7 +924,8 @@ EXPORT_SYMBOL(ttm_bo_mem_put);
*/
static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo,
struct ttm_mem_type_manager *man,
-struct ttm_mem_reg *mem)
+struct ttm_mem_reg *mem,
+bool no_wait_gpu)
{
struct dma_fence *fence;
int ret;
@@ -933,19 +934,22 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object
*bo,
fence = dma_fence_get(man->move);
spin_unlock(&man->move_lock);
- if (fence) {
- reservation_object_add_shared_fence(bo->resv, fence);
+ if (!fence)
+ return 0;
- ret = reservation_object_reserve_shared(bo->resv, 1);
- if (unlikely(ret)) {
- dma_fence_put(fence);
- return ret;
- }
+ if (no_wait_gpu)
+ return -EBUSY;
+
+ reservation_object_add_shared_fence(bo->resv, fence);
- dma_fence_put(bo->moving);
- bo->moving = fence;
+ ret = reservation_object_reserve_shared(bo->resv, 1);
+ if (unlikely(ret)) {
+ dma_fence_put(fence);
+ return ret;
}
+ dma_fence_put(bo->moving);
+ bo->moving = fence;
return 0;
}
@@ -974,7 +978,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object
*bo,
return ret;
} while (1);
- return ttm_bo_add_move_fence(bo, man, mem);
+ return ttm_bo_add_move_fence(bo, man, mem, ctx->no_wait_gpu);
}
static uint32_t ttm_bo_select_caching(struct ttm_mem_type_manager *man,
@@ -1116,13 +1120,16 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
if (unlikely(ret))
goto error;
- if (mem->mm_node) {
- ret = ttm_bo_add_move_fence(bo, man, mem);
- if (unlikely(ret)) {
- (*man->func->put_node)(man, mem);
- goto error;
- }
- return 0;
+ if (!mem->mm_node)
+ continue;
+
+ ret = ttm_bo_add_move_fence(bo, man, mem, ctx->no_wait_gpu);
+ if (unlikely(ret)) {
+ (*man->func->put_node)(man, mem);
+ if (ret == -EBUSY)
+ continue;
+
+ goto error;
}
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 8/9] drm/amdgpu: reserve the root PD while freeing PASIDs
Free the pasid only while the root PD is reserved. This prevents use after
free in the page fault handling.
Signed-off-by: Christian König
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3b954b53a450..db13ddd70a8d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2944,18 +2944,26 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
amdgpu_vm *vm)
struct amdgpu_bo_va_mapping *mapping, *tmp;
bool prt_fini_needed = !!adev->gmc.gmc_funcs->set_prt;
struct amdgpu_bo *root;
- int i, r;
+ int i;
amdgpu_amdkfd_gpuvm_destroy_cb(adev, vm);
+ root = amdgpu_bo_ref(vm->root.base.bo);
+ amdgpu_bo_reserve(root, true);
if (vm->pasid) {
unsigned long flags;
spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
idr_remove(&adev->vm_manager.pasid_idr, vm->pasid);
spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
+ vm->pasid = 0;
}
+ amdgpu_vm_free_pts(adev, vm, NULL);
+ amdgpu_bo_unreserve(root);
+ amdgpu_bo_unref(&root);
+ WARN_ON(vm->root.base.bo);
+
drm_sched_entity_destroy(&vm->direct);
drm_sched_entity_destroy(&vm->delayed);
@@ -2980,16 +2988,6 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
amdgpu_vm *vm)
amdgpu_vm_free_mapping(adev, vm, mapping, NULL);
}
- root = amdgpu_bo_ref(vm->root.base.bo);
- r = amdgpu_bo_reserve(root, true);
- if (r) {
- dev_err(adev->dev, "Leaking page tables because BO reservation
failed\n");
- } else {
- amdgpu_vm_free_pts(adev, vm, NULL);
- amdgpu_bo_unreserve(root);
- }
- amdgpu_bo_unref(&root);
- WARN_ON(vm->root.base.bo);
dma_fence_put(vm->last_update);
for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
amdgpu_vmid_free_reserved(adev, vm, i);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/9] drm/amdgpu: allow direct submission of PTE updates
For handling PTE updates directly in the fault handler.
Signed-off-by: Christian König
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index fff553a591b6..aed68fa88f16 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1492,13 +1492,14 @@ static int amdgpu_vm_update_ptes(struct
amdgpu_vm_update_params *params,
* amdgpu_vm_bo_update_mapping - update a mapping in the vm page table
*
* @adev: amdgpu_device pointer
- * @exclusive: fence we need to sync to
- * @pages_addr: DMA addresses to use for mapping
* @vm: requested vm
+ * @direct: direct submission in a page fault
+ * @exclusive: fence we need to sync to
* @start: start of mapped range
* @last: last mapped entry
* @flags: flags for the entries
* @addr: addr to set the area to
+ * @pages_addr: DMA addresses to use for mapping
* @fence: optional resulting fence
*
* Fill in the page table entries between @start and @last.
@@ -1507,11 +1508,11 @@ static int amdgpu_vm_update_ptes(struct
amdgpu_vm_update_params *params,
* 0 for success, -EINVAL for failure.
*/
static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
+ struct amdgpu_vm *vm, bool direct,
struct dma_fence *exclusive,
- dma_addr_t *pages_addr,
- struct amdgpu_vm *vm,
uint64_t start, uint64_t last,
uint64_t flags, uint64_t addr,
+ dma_addr_t *pages_addr,
struct dma_fence **fence)
{
struct amdgpu_vm_update_params params;
@@ -1521,6 +1522,7 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
memset(¶ms, 0, sizeof(params));
params.adev = adev;
params.vm = vm;
+ params.direct = direct;
params.pages_addr = pages_addr;
/* sync to everything except eviction fences on unmapping */
@@ -1633,9 +1635,9 @@ static int amdgpu_vm_bo_split_mapping(struct
amdgpu_device *adev,
}
last = min((uint64_t)mapping->last, start + max_entries - 1);
- r = amdgpu_vm_bo_update_mapping(adev, exclusive, dma_addr, vm,
+ r = amdgpu_vm_bo_update_mapping(adev, vm, false, exclusive,
start, last, flags, addr,
- fence);
+ dma_addr, fence);
if (r)
return r;
@@ -1929,9 +1931,9 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
mapping->start < AMDGPU_GMC_HOLE_START)
init_pte_value = AMDGPU_PTE_DEFAULT_ATC;
- r = amdgpu_vm_bo_update_mapping(adev, NULL, NULL, vm,
+ r = amdgpu_vm_bo_update_mapping(adev, vm, false, NULL,
mapping->start, mapping->last,
- init_pte_value, 0, &f);
+ init_pte_value, 0, NULL, &f);
amdgpu_vm_free_mapping(adev, vm, mapping, f);
if (r) {
dma_fence_put(f);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/9] drm/amdgpu: allow direct submission of clears
For handling PD/PT clears directly in the fault handler.
Signed-off-by: Christian König
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index aed68fa88f16..6c94c5a562cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -695,6 +695,7 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)
* @adev: amdgpu_device pointer
* @vm: VM to clear BO from
* @bo: BO to clear
+ * @direct: use a direct update
*
* Root PD needs to be reserved when calling this.
*
@@ -703,7 +704,8 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)
*/
static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
- struct amdgpu_bo *bo)
+ struct amdgpu_bo *bo,
+ bool direct)
{
struct ttm_operation_ctx ctx = { true, false };
unsigned level = adev->vm_manager.root_level;
@@ -762,6 +764,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
memset(¶ms, 0, sizeof(params));
params.adev = adev;
params.vm = vm;
+ params.direct = direct;
r = vm->update_funcs->prepare(¶ms, AMDGPU_FENCE_OWNER_KFD, NULL);
if (r)
@@ -852,7 +855,8 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
*/
static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
- struct amdgpu_vm_pt_cursor *cursor)
+ struct amdgpu_vm_pt_cursor *cursor,
+ bool direct)
{
struct amdgpu_vm_pt *entry = cursor->entry;
struct amdgpu_bo_param bp;
@@ -885,7 +889,7 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
pt->parent = amdgpu_bo_ref(cursor->parent->base.bo);
amdgpu_vm_bo_base_init(&entry->base, vm, pt);
- r = amdgpu_vm_clear_bo(adev, vm, pt);
+ r = amdgpu_vm_clear_bo(adev, vm, pt, direct);
if (r)
goto error_free_pt;
@@ -1401,7 +1405,8 @@ static int amdgpu_vm_update_ptes(struct
amdgpu_vm_update_params *params,
uint64_t incr, entry_end, pe_start;
struct amdgpu_bo *pt;
- r = amdgpu_vm_alloc_pts(params->adev, params->vm, &cursor);
+ r = amdgpu_vm_alloc_pts(params->adev, params->vm, &cursor,
+ params->direct);
if (r)
return r;
@@ -2725,7 +2730,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
- r = amdgpu_vm_clear_bo(adev, vm, root);
+ r = amdgpu_vm_clear_bo(adev, vm, root, false);
if (r)
goto error_unreserve;
@@ -2848,7 +2853,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev,
struct amdgpu_vm *vm, uns
*/
if (pte_support_ats != vm->pte_support_ats) {
vm->pte_support_ats = pte_support_ats;
- r = amdgpu_vm_clear_bo(adev, vm, vm->root.base.bo);
+ r = amdgpu_vm_clear_bo(adev, vm, vm->root.base.bo, false);
if (r)
goto free_idr;
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/9] drm/amdgpu: allow direct submission of PDE updates v2
For handling PDE updates directly in the fault handler.
v2: fix typo in comment
Signed-off-by: Christian König
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++--
5 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index d90b647985d7..51ff89cb2f16 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -343,7 +343,7 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct
amdgpu_sync *sync)
struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev);
int ret;
- ret = amdgpu_vm_update_directories(adev, vm);
+ ret = amdgpu_vm_update_pdes(adev, vm, false);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 51f3db08b8eb..bd6b88827447 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -845,7 +845,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
if (r)
return r;
- r = amdgpu_vm_update_directories(adev, vm);
+ r = amdgpu_vm_update_pdes(adev, vm, false);
if (r)
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 22eab74d9998..d4725f98643e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -521,7 +521,7 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device
*adev,
goto error;
}
- r = amdgpu_vm_update_directories(adev, vm);
+ r = amdgpu_vm_update_pdes(adev, vm, false);
error:
if (r && r != -ERESTARTSYS)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8a9ba66c8383..fff553a591b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1227,18 +1227,19 @@ static void amdgpu_vm_invalidate_pds(struct
amdgpu_device *adev,
}
/*
- * amdgpu_vm_update_directories - make sure that all directories are valid
+ * amdgpu_vm_update_pdes - make sure that all directories are valid
*
* @adev: amdgpu_device pointer
* @vm: requested vm
+ * @direct: submit directly to the paging queue
*
* Makes sure all directories are up to date.
*
* Returns:
* 0 for success, error for failure.
*/
-int amdgpu_vm_update_directories(struct amdgpu_device *adev,
-struct amdgpu_vm *vm)
+int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
+ struct amdgpu_vm *vm, bool direct)
{
struct amdgpu_vm_update_params params;
int r;
@@ -1249,6 +1250,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device
*adev,
memset(¶ms, 0, sizeof(params));
params.adev = adev;
params.vm = vm;
+ params.direct = direct;
r = vm->update_funcs->prepare(¶ms, AMDGPU_FENCE_OWNER_VM, NULL);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 54dcd0bcce1a..0a97dc839f3b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -366,8 +366,8 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
int (*callback)(void *p, struct amdgpu_bo *bo),
void *param);
int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job, bool
need_pipe_sync);
-int amdgpu_vm_update_directories(struct amdgpu_device *adev,
-struct amdgpu_vm *vm);
+int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
+ struct amdgpu_vm *vm, bool direct);
int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
struct dma_fence **fence);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/9] drm/amdgpu: allocate PDs/PTs with no_gpu_wait in a page fault
While handling a page fault we can't wait for other ongoing GPU
operations or we can potentially run into deadlocks.
Signed-off-by: Christian König
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +---
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 04767905f004..510d04fd6e5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -451,7 +451,7 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
{
struct ttm_operation_ctx ctx = {
.interruptible = (bp->type != ttm_bo_type_kernel),
- .no_wait_gpu = false,
+ .no_wait_gpu = bp->no_wait_gpu,
.resv = bp->resv,
.flags = TTM_OPT_FLAG_ALLOW_RES_EVICT
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 5a3c1779e200..e6ddd048a984 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -41,6 +41,7 @@ struct amdgpu_bo_param {
u32 preferred_domain;
u64 flags;
enum ttm_bo_typetype;
+ boolno_wait_gpu;
struct reservation_object *resv;
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 6c94c5a562cf..3b954b53a450 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -821,7 +821,8 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
* @bp: resulting BO allocation parameters
*/
static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm
*vm,
- int level, struct amdgpu_bo_param *bp)
+ int level, bool direct,
+ struct amdgpu_bo_param *bp)
{
memset(bp, 0, sizeof(*bp));
@@ -836,6 +837,7 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
else if (!vm->root.base.bo || vm->root.base.bo->shadow)
bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
bp->type = ttm_bo_type_kernel;
+ bp->no_wait_gpu = direct;
if (vm->root.base.bo)
bp->resv = vm->root.base.bo->tbo.resv;
}
@@ -877,7 +879,7 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
if (entry->base.bo)
return 0;
- amdgpu_vm_bo_param(adev, vm, cursor->level, &bp);
+ amdgpu_vm_bo_param(adev, vm, cursor->level, direct, &bp);
r = amdgpu_bo_create(adev, &bp, &pt);
if (r)
@@ -2713,7 +2715,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
vm->update_funcs = &amdgpu_vm_sdma_funcs;
vm->last_update = NULL;
- amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, &bp);
+ amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, false, &bp);
if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE)
bp.flags &= ~AMDGPU_GEM_CREATE_SHADOW;
r = amdgpu_bo_create(adev, &bp, &root);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
Hi, On 9/10/19 9:50 AM, Michel Dänzer wrote: On 2019-09-07 10:32 p.m., Hans de Goede wrote: Bail from the pci_driver probe function instead of from the drm_driver load function. This avoid /dev/dri/card0 temporarily getting registered and then unregistered again, sending unwanted add / remove udev events to userspace. Specifically this avoids triggering the (userspace) bug fixed by this plymouth merge-request: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59 Note that despite that being an userspace bug, not sending unnecessary udev events is a good idea in general. BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490 Signed-off-by: Hans de Goede Reviewed-by: Michel Dänzer Thank you for the review. I've drm push rights, but I guess that radeon/amd GPU patches are collected by someone @AMD, so I do not need to / should not push this myself, right? amdgpu should be changed correspondingly as well. Good point. I'm currently travelling (@plumbers) I can do this when I'm back home, but feel free to beat me to it (if you do please Cc me to avoid double work). Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RFC: IOCTL to label BO in DRM Core
Hi I'm investigating a way to label BO's in panfrost, similar to how VC4 does it and realised that this could be something that's might be useful to all drivers. With that in mind, would anyone be opposed to add a DRM_IOCTL_BO_SET_LABEL in DRM core that can be utilised by all drivers to label BO's? Cheers Rohan Garg signature.asc Description: This is a digitally signed message part. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/edid: Have cea_db_offsets() zero start/end when the data block collection isn't found
On Tue, Sep 10, 2019 at 11:46:20AM +0200, Jean Delvare wrote:
> Hi Ville,
>
> On Mon, 2 Sep 2019 16:15:46 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > Let's make cea_db_offsets() a bit more convenient to use by
> > setting the start/end offsets to zero whenever the data block
> > collection isn't present. This makes it safe for the caller
> > to blindly iterate the data blocks even if there are none.
> >
> > Cc: Jean Delvare
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/drm_edid.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 7b3072fc550b..e5905dc764c1 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3690,6 +3690,9 @@ cea_revision(const u8 *cea)
> > static int
> > cea_db_offsets(const u8 *cea, int *start, int *end)
> > {
> > + *start = 0;
> > + *end = 0;
> > +
> > if (cea_revision(cea) < 3)
> > return -ENOTSUPP;
> >
> > @@ -4116,10 +4119,7 @@ static void drm_edid_to_eld(struct drm_connector
> > *connector, struct edid *edid)
> > if (cea_revision(cea) >= 3) {
> > int i, start, end;
> >
> > - if (cea_db_offsets(cea, &start, &end)) {
> > - start = 0;
> > - end = 0;
> > - }
> > + cea_db_offsets(cea, &start, &end);
> >
> > for_each_cea_db(cea, i, start, end) {
> > db = &cea[i];
>
> Not sure if that's really needed. As it stands there's only one
> function which wants to continue after cea_db_offsets() fails, all
> others just bail out at that point. Now that cea_db_offsets() checks
> for revision >= 3, the construct above could simply become:
>
> int i, start, end;
>
> if (cea_db_offsets(cea, &start, &end) == 0) {
> for_each_cea_db(cea, i, start, end) {
> db = &cea[i];
>
> which is IMHO more elegant and does not require zeroing start and end
> in cea_db_offsets().
I dislike indentation. Also could perhaps even move the cea_db_offsets()
into the for_each_cea_db() macro so that the caller doesn't have to care
about these mundane details at all.
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: two planes with the same zpos have undefined ordering
Currently the property docs don't specify whether it's okay for two planes to have the same zpos value and what user-space should expect in this case. The rule mentionned in the past was to disambiguate with object IDs. However some drivers break this rule (that's why the ordering is documented as unspecified in case the zpos property is missing). Additionally it doesn't really make sense for a driver to user identical zpos values if it knows their relative position: the driver can just pick different values instead. So two solutions would make sense: either disallow completely identical zpos values for two different planes, either make the ordering unspecified. To allow drivers that don't know the relative ordering between two planes to still expose the zpos property, choose the latter solution. Signed-off-by: Simon Ser --- Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake. drivers/gpu/drm/drm_blend.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@ * planes. Without this property the primary plane is always below the cursor * plane, and ordering between all other planes is undefined. The positive * Z axis points towards the user, i.e. planes with lower Z position values - * are underneath planes with higher Z position values. Note that the Z - * position value can also be immutable, to inform userspace about the - * hard-coded stacking of overlay planes, see - * drm_plane_create_zpos_immutable_property(). + * are underneath planes with higher Z position values. Two planes with the + * same Z position value have undefined ordering. Note that the Z position + * value can also be immutable, to inform userspace about the hard-coded + * stacking of overlay planes, see drm_plane_create_zpos_immutable_property(). * * pixel blend mode: * Pixel blend mode is set up with drm_plane_create_blend_mode_property(). -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] backlight: lm3630a: add an enable gpio for the HWEN pin
On Mon, Sep 09, 2019 at 10:13:49PM +0200, Andreas Kemnade wrote:
> On Mon, 9 Sep 2019 11:57:29 +0100
> Daniel Thompson wrote:
>
> > On Sun, Sep 08, 2019 at 10:37:03PM +0200, Andreas Kemnade wrote:
> > > For now just enable it in the probe function to allow i2c
> > > access and disable it on remove. Disabling also means resetting
> > > the register values to default.
> > >
> > > Tested on Kobo Clara HD.
> > >
> > > Signed-off-by: Andreas Kemnade
> > > ---
> > > drivers/video/backlight/lm3630a_bl.c | 18 ++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/video/backlight/lm3630a_bl.c
> > > b/drivers/video/backlight/lm3630a_bl.c
> > > index b04b35d007a2..3b45a1733198 100644
> > > --- a/drivers/video/backlight/lm3630a_bl.c
> > > +++ b/drivers/video/backlight/lm3630a_bl.c
> > > @@ -12,6 +12,8 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > +#include
> > > #include
> > > #include
> > >
> > > @@ -48,6 +50,7 @@ struct lm3630a_chip {
> > > struct lm3630a_platform_data *pdata;
> > > struct backlight_device *bleda;
> > > struct backlight_device *bledb;
> > > + struct gpio_desc *enable_gpio;
> > > struct regmap *regmap;
> > > struct pwm_device *pwmd;
> > > };
> > > @@ -506,6 +509,14 @@ static int lm3630a_probe(struct i2c_client *client,
> > > return -ENOMEM;
> > > pchip->dev = &client->dev;
> > >
> > > + pchip->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
> > > + GPIOD_ASIS);
> >
> > Initializing GPIOD_ASIS doesn't look right to me.
> >
> > If you initialize ASIS then the driver must configure the pin as an
> > output... far easier just to set GPIOD_OUT_HIGH during the get.
> >
> > Note also that the call to this function should also be moved *below*
> > the calls parse the DT.
> >
> oops, must have forgotten that, and had good luck here.
> >
> > > + if (IS_ERR(pchip->enable_gpio)) {
> > > + rval = PTR_ERR(pchip->enable_gpio);
> > > + return rval;
> > > + }
> > > +
> > > +
> > > pchip->regmap = devm_regmap_init_i2c(client, &lm3630a_regmap);
> > > if (IS_ERR(pchip->regmap)) {
> > > rval = PTR_ERR(pchip->regmap);
> > > @@ -535,6 +546,10 @@ static int lm3630a_probe(struct i2c_client *client,
> > > }
> > > pchip->pdata = pdata;
> > >
> > > + if (pchip->enable_gpio) {
> > > + gpiod_set_value_cansleep(pchip->enable_gpio, 1);
> >
> > Not needed, use GPIOD_OUT_HIGH instead.
> >
> >
> > > + usleep_range(1000, 2000);
> >
> > Not needed, this sleep is already part of lm3630a_chip_init().
> >
> you are right.
> >
> > > + }
> > > /* chip initialize */
> > > rval = lm3630a_chip_init(pchip);
> > > if (rval < 0) {
> > > @@ -586,6 +601,9 @@ static int lm3630a_remove(struct i2c_client *client)
> > > if (rval < 0)
> > > dev_err(pchip->dev, "i2c failed to access register\n");
> > >
> > > + if (pchip->enable_gpio)
> > > + gpiod_set_value_cansleep(pchip->enable_gpio, 0);
> > > +
> >
> > Is this needed?
> >
> > This is a remove path, not a power management path, and we have no idea
> > what the original status of the pin was anyway?
> >
>
> Looking at Ishdn on page 5 of the datasheet, switching it off everytime
> possible seems not needed. We would need to call chip_init() everytime
> we enable the gpio or live with default values.
> Therefore I did decide to not put it into any power management path.
> But switching it on and not switching it off feels so unbalanced.
Either the power consumed by the controller when strings aren't lit up
matters, in which case the driver should implement proper power
management or it doesn't matter and changing the pin state isn't needed.
I'm happy with either of the above but this looks like a third way,
where eager users could hack in a bit of extra power management by
forcing drivers to unbind.
Daniel.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111630] Generate a list of tags for the machine that runs the testsuite
https://bugs.freedesktop.org/show_bug.cgi?id=111630 Bug ID: 111630 Summary: Generate a list of tags for the machine that runs the testsuite Product: DRI Version: XOrg git Hardware: Other OS: All Status: NEW Severity: not set Priority: not set Component: IGT Assignee: dri-devel@lists.freedesktop.org Reporter: martin.pe...@free.fr Bug filing is done based on machine tags. They can be indicating a platform (TGL, SKL, ...), or a configuration (HDMI, PSR, CHAMELIUM, ...). This tagging is currently done manually, but it often is not in sync with the current platform state, which makes bug filing difficult and leads to random noise in CI until the proper tag is set. Since we would like individual developers and CI machines to have a consistent way of creating these tags (so as individual developers can filter out the known issues from their run locally), having an IGT test/utility to generate these tags would be ideal. I'll let Arek explain how it could be done :) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: two planes with the same zpos have undefined ordering
On Tue, 10 Sep 2019 10:09:55 + Simon Ser wrote: > Currently the property docs don't specify whether it's okay for two planes to > have the same zpos value and what user-space should expect in this case. > > The rule mentionned in the past was to disambiguate with object IDs. However > some drivers break this rule (that's why the ordering is documented as > unspecified in case the zpos property is missing). Additionally it doesn't > really make sense for a driver to user identical zpos values if it knows their > relative position: the driver can just pick different values instead. > > So two solutions would make sense: either disallow completely identical zpos > values for two different planes, either make the ordering unspecified. To > allow > drivers that don't know the relative ordering between two planes to still > expose the zpos property, choose the latter solution. > > Signed-off-by: Simon Ser > --- > > Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake. > > drivers/gpu/drm/drm_blend.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c > index d02709dd2d4a..51bd5454e50a 100644 > --- a/drivers/gpu/drm/drm_blend.c > +++ b/drivers/gpu/drm/drm_blend.c > @@ -132,10 +132,10 @@ > * planes. Without this property the primary plane is always below the > cursor > * plane, and ordering between all other planes is undefined. The positive > * Z axis points towards the user, i.e. planes with lower Z position values > - * are underneath planes with higher Z position values. Note that the Z > - * position value can also be immutable, to inform userspace about the > - * hard-coded stacking of overlay planes, see > - * drm_plane_create_zpos_immutable_property(). > + * are underneath planes with higher Z position values. Two planes with the > + * same Z position value have undefined ordering. Note that the Z position > + * value can also be immutable, to inform userspace about the hard-coded > + * stacking of overlay planes, see > drm_plane_create_zpos_immutable_property(). > * > * pixel blend mode: > * Pixel blend mode is set up with drm_plane_create_blend_mode_property(). Hi, this seems to contradict what the docs say in another place: zpos Priority of the given plane on crtc (optional). Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to compare the plane object IDs; the plane with a higher ID must be stacked on top of a plane with a lower ID. See drm_plane_create_zpos_property() and drm_plane_create_zpos_immutable_property() for more details. from https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html#plane-functions-reference Thanks, pq pgpJB1hnbm2nr.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111630] Generate a list of tags for the machine that runs the testsuite
https://bugs.freedesktop.org/show_bug.cgi?id=111630
Arek Hiler changed:
What|Removed |Added
CC||petri.latv...@intel.com
--- Comment #1 from Arek Hiler ---
Martin first suggested to have something like
igt_require_feature(TWO_DISPLAYS), with a set of different defines/function
pointers, or whatever - so we have a single way of checking for a number of
connected displays and it's simple to tie it with a cibuglog tag.
I find that hard both hard to use (using names instead of numbers, changing
almost all of test to use this new framework) and implement (centralizing a lot
of knowledge, how to do the display checks without passing igt_display, fd,
etc). But it would be easy to assure consistency in the skip messages.
What I would to propose instead is a special igt test that would do the
discovery:
/* has to be before KMS/DISPALY checks to enable all outputs */
igt_subtest("CHAMELIUM") {
chamelium = chamelium_init(fd);
igt_require(chamelium);
/* deinit && plug all */
}
igt_subtest_group {
volatile igt_display display;
igt_fixture {
igt_require_display(&display, fd);
}
igt_subtest("DISPLAY")
{ /* intentionaly left blank */ }
igt_subtest("TWO_OUTPUTS") {
igt_require_n_outputs(&display, 2);
}
igt_feature("THREE_OUTPUTS") {
igt_require_n_outputs(&display, 3);
}
}
Then you can run it like that: `igt_runner -m -t feature_detection ...` which
would get you a nice parsable, piglit-style json as well as sample skip
messages.
It will still need a lot of test changes to assure consistency, but arguably
fewer than the igt_require_feature(XYZ). Since it's also a piglit-format json
we can ingest it easily and check for flip-floppers on those tags :-)
--
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] Merge VRAM MM and GEM VRAM source files
On Mon, Sep 09, 2019 at 03:04:49PM +0200, Thomas Zimmermann wrote: > VRAM MM and GEM VRAM are only used with each other. This patch set > moves VRAM MM into GEM VRAM source files and cleans up the helper's > public interface. > > Thomas Zimmermann (4): > drm/vram: Move VRAM memory manager to GEM VRAM implementation > drm/vram: Have VRAM MM call GEM VRAM functions directly > drm/vram: Unexport internal functions of VRAM MM > drm/vram: Unconditonally set BO call-back functions Looks all sane. Acked-by: Gerd Hoffmann cheers, Gerd
Re: [PATCH] drm: two planes with the same zpos have undefined ordering
On Tuesday, September 10, 2019 1:38 PM, Pekka Paalanen wrote: > On Tue, 10 Sep 2019 10:09:55 + > Simon Ser cont...@emersion.fr wrote: > > > Currently the property docs don't specify whether it's okay for two planes > > to > > have the same zpos value and what user-space should expect in this case. > > The rule mentionned in the past was to disambiguate with object IDs. However > > some drivers break this rule (that's why the ordering is documented as > > unspecified in case the zpos property is missing). Additionally it doesn't > > really make sense for a driver to user identical zpos values if it knows > > their > > relative position: the driver can just pick different values instead. > > So two solutions would make sense: either disallow completely identical zpos > > values for two different planes, either make the ordering unspecified. To > > allow > > drivers that don't know the relative ordering between two planes to still > > expose the zpos property, choose the latter solution. > > > > Signed-off-by: Simon Ser cont...@emersion.fr > > > > - > > > > Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake. > > drivers/gpu/drm/drm_blend.c | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c > > index d02709dd2d4a..51bd5454e50a 100644 > > --- a/drivers/gpu/drm/drm_blend.c > > +++ b/drivers/gpu/drm/drm_blend.c > > @@ -132,10 +132,10 @@ > > > > - planes. Without this property the primary plane is always below the > > cursor > > - plane, and ordering between all other planes is undefined. The positive > > - Z axis points towards the user, i.e. planes with lower Z position values > > > > - - are underneath planes with higher Z position values. Note that the Z > > - - position value can also be immutable, to inform userspace about the > > - - hard-coded stacking of overlay planes, see > > - - drm_plane_create_zpos_immutable_property(). > > > > - - are underneath planes with higher Z position values. Two planes > > with the > > - - same Z position value have undefined ordering. Note that the Z > > position > > - - value can also be immutable, to inform userspace about the > > hard-coded > > - - stacking of overlay planes, see > > drm_plane_create_zpos_immutable_property(). > > - > > - pixel blend mode: > > - Pixel blend mode is set up with > > drm_plane_create_blend_mode_property(). > > Hi, > > this seems to contradict what the docs say in another place: Except this comment is about drm_plane_state.zpos, an internal DRM property. This is not about the zpos property itself. The comment was introduced in v2 of [1], although the motivation for the change isn't documented. [1]: https://patchwork.freedesktop.org/series/13528/#rev2 > zpos > > Priority of the given plane on crtc (optional). > > Note that multiple active planes on the same crtc can have an > identical zpos value. The rule to solving the conflict is to > compare the plane object IDs; the plane with a higher ID must be > stacked on top of a plane with a lower ID. > > See drm_plane_create_zpos_property() and > drm_plane_create_zpos_immutable_property() for more details. > > from > https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html#plane-functions-reference > > Thanks, > pq ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110117] Waking from Suspend causes screen to appear with grey static (like a TV with no signal)
https://bugs.freedesktop.org/show_bug.cgi?id=110117 --- Comment #15 from leo60228 --- This issue also occurs on Windows. AMD support says that it will be fixed in the next Windows update; could they possibly contribute that same fix to Linux? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/mgag200: Add vblank support
On Mon, Sep 09, 2019 at 04:06:33PM +0200, Thomas Zimmermann wrote:
> Support for vblank requires VSYNC to signal an interrupt, which is broken
> on Matrox chipsets. The workaround that is used here and in other free
> Matrox drivers is to program to the value of and
> enable the VLINE interrupt. This triggers an interrupt at the same time
> when VSYNC begins.
>
> VLINE uses separate registers for enabling and clearing pending interrupts.
> No extra syncronization between irq handler and the rest of the driver is
> required.
Looks good overall, some minor nits ...
> +irqreturn_t mgag200_irq_handler(int irq, void *arg)
> +{
> + struct drm_device *dev = arg;
> + struct mga_device *mdev = dev->dev_private;
> + struct drm_crtc *crtc;
> + u32 status, iclear;
> +
> + status = RREG32(0x1e14);
> +
> + if (status & 0x0020) { /* test */
> + drm_for_each_crtc(crtc, dev) {
> + drm_crtc_handle_vblank(crtc);
> + }
> + iclear = RREG32(0x1e18);
> + iclear |= 0x0020; /* set */
#define for this would be good (you also don't need the comment then).
> + /* The VSYNC interrupt is broken on Matrox chipsets. We use
Codestyle. "/*" should be on a separate line.
> +static void mga_crtc_disable_vblank(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct mga_device *mdev = dev->dev_private;
> + u32 ien;
> +
> + ien = RREG32(0x1e1c);
> + ien &= 0xffdf; /* clear */
That is typically written as value &= ~(bit);
cheers,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/fb-helper: Synchronize dirty worker with vblank
On Mon, Sep 09, 2019 at 04:06:32PM +0200, Thomas Zimmermann wrote:
> Before updating the display from the console's shadow buffer, the dirty
> worker now waits for vblank. This allows several screen updates to pile
> up and acts as a rate limiter.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_fb_helper.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index a7ba5b4902d6..017e2f6bd1b9 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -402,8 +402,20 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
> dirty_work);
> struct drm_clip_rect *clip = &helper->dirty_clip;
> struct drm_clip_rect clip_copy;
> + struct drm_crtc *crtc;
> unsigned long flags;
> void *vaddr;
> + int ret;
> +
> + /* rate-limit update frequency */
> + mutex_lock(&helper->lock);
> + crtc = helper->client.modesets[0].crtc;
> + ret = drm_crtc_vblank_get(crtc);
> + if (!ret) {
> + drm_crtc_wait_one_vblank(crtc);
> + drm_crtc_vblank_put(crtc);
> + }
> + mutex_unlock(&helper->lock);
Hmm, not sure it is the best plan to sleep for a while in the worker,
especially while holding the lock.
What does the lock protect against here? Accessing
helper->client.modesets? If so then you can unlock before going to
sleep in drm_crtc_wait_one_vblank() I think.
cheers,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH/RFC 1/5] drm/edid: Reorganise the DisplayID parsing code
On Wed, Aug 21, 2019 at 09:50:01PM +0300, Laurent Pinchart wrote:
> This change started as an attempt to remove the forward declaration of
> validate_displayid(), and ended up reorganising the DisplayID parsing
> code to fix serial intertwined issues.
>
> The drm_parse_display_id() function, which parses a DisplayID block, is
> made aware of whether the DisplayID comes from an EDID extension block
> or is direct DisplayID data. This is a layering violation, this should
> be handled in the caller. Similarly, the validate_displayid() function
> should not take an offset in the data buffer, it should also receive a
> pointer to the DisplayID data.
>
> In order to simplify the callers of drm_find_displayid_extension(), the
> function is modified to return a pointer to the DisplayID data, not to
> the EDID extension block. The pointer can then be used directly for
> validation and parsing, without the need to add an offset.
>
> For consistency reasons the validate_displayid() function is renamed to
> drm_displayid_is_valid() and modified to return a bool, and the
> drm_parse_display_id() renamed to drm_parse_displayid().
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/gpu/drm/drm_edid.c | 104 ++---
> 1 file changed, 52 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 82a4ceed3fcf..7c6bc5183b60 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1342,7 +1342,6 @@ MODULE_PARM_DESC(edid_fixup,
>
> static void drm_get_displayid(struct drm_connector *connector,
> struct edid *edid);
> -static int validate_displayid(u8 *displayid, int length, int idx);
>
> static int drm_edid_block_checksum(const u8 *raw_edid)
> {
> @@ -2927,17 +2926,49 @@ static u8 *drm_find_edid_extension(const struct edid
> *edid, int ext_id)
> return edid_ext;
> }
>
> +static bool drm_displayid_is_valid(u8 *displayid, unsigned int length)
const* would be nice.
same in many other places in the series.
> +{
> + struct displayid_hdr *base;
> + u8 csum = 0;
> + int i;
> +
> + base = (struct displayid_hdr *)displayid;
Can be done when declaring the variable.
> +
> + DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
> + base->rev, base->bytes, base->prod_id, base->ext_count);
> +
> + if (base->bytes + 5 > length)
> + return false;
> +
> + for (i = 0; i <= base->bytes + 5; i++)
> + csum += displayid[i];
> +
> + if (csum) {
> + DRM_NOTE("DisplayID checksum invalid, remainder is %d\n", csum);
> + return false;
> + }
> +
> + return true;
> +}
>
> static u8 *drm_find_displayid_extension(const struct edid *edid)
> {
> - return drm_find_edid_extension(edid, DISPLAYID_EXT);
> + u8 *ext = drm_find_edid_extension(edid, DISPLAYID_EXT);
> +
> + if (!ext)
> + return NULL;
> +
> + /*
> + * Skip the EDID extension block tag number to return the DisplayID
> + * data.
> + */
> + return ext + 1;
> }
>
> static u8 *drm_find_cea_extension(const struct edid *edid)
> {
> - int ret;
> - int idx = 1;
> - int length = EDID_LENGTH;
> + int idx;
> + int length = EDID_LENGTH - 1;
> struct displayid_block *block;
> u8 *cea;
> u8 *displayid;
> @@ -2952,11 +2983,10 @@ static u8 *drm_find_cea_extension(const struct edid
> *edid)
> if (!displayid)
> return NULL;
>
> - ret = validate_displayid(displayid, length, idx);
> - if (ret)
> + if (!drm_displayid_is_valid(displayid, length))
> return NULL;
>
> - idx += sizeof(struct displayid_hdr);
> + idx = sizeof(struct displayid_hdr);
It's a bit silly/fragile that everyone has to do this. I'd rather
eliminate this idx initialzation from the callers of
for_each_displayid_db() entirely.
> for_each_displayid_db(displayid, block, idx, length) {
> if (block->tag == DATA_BLOCK_CTA) {
> cea = (u8 *)block;
This here is also borked. We should validate the size of the
CEA ext block somewhere.
> @@ -4711,29 +4741,6 @@ u32 drm_add_display_info(struct drm_connector
> *connector, const struct edid *edi
> return quirks;
> }
>
> -static int validate_displayid(u8 *displayid, int length, int idx)
> -{
> - int i;
> - u8 csum = 0;
> - struct displayid_hdr *base;
> -
> - base = (struct displayid_hdr *)&displayid[idx];
> -
> - DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
> - base->rev, base->bytes, base->prod_id, base->ext_count);
> -
> - if (base->bytes + 5 > length - idx)
> - return -EINVAL;
> - for (i = idx; i <= base->bytes + 5; i++) {
> - csum += displayid[i];
> - }
> - if (csum) {
> - DRM_NOTE("DisplayID checksum invalid, remainder is %d\n", csum);
> -
Re: [PATCH RFC v4 00/16] new cgroup controller for gpu/drm subsystem
On Fri 06-09-19 08:45:39, Tejun Heo wrote: > Hello, Daniel. > > On Fri, Sep 06, 2019 at 05:34:16PM +0200, Daniel Vetter wrote: > > > Hmm... what'd be the fundamental difference from slab or socket memory > > > which are handled through memcg? Is system memory used by GPUs have > > > further global restrictions in addition to the amount of physical > > > memory used? > > > > Sometimes, but that would be specific resources (kinda like vram), > > e.g. CMA regions used by a gpu. But probably not something you'll run > > in a datacenter and want cgroups for ... > > > > I guess we could try to integrate with the memcg group controller. One > > trouble is that aside from i915 most gpu drivers do not really have a > > full shrinker, so not sure how that would all integrate. > > So, while it'd great to have shrinkers in the longer term, it's not a > strict requirement to be accounted in memcg. It already accounts a > lot of memory which isn't reclaimable (a lot of slabs and socket > buffer). Yeah, having a shrinker is preferred but the memory should better be reclaimable in some form. If not by any other means then at least bound to a user process context so that it goes away with a task being killed by the OOM killer. If that is not the case then we cannot really charge it because then the memcg controller is of no use. We can tolerate it to some degree if the amount of memory charged like that is negligible to the overall size. But from the discussion it seems that these buffers are really large. -- Michal Hocko SUSE Labs ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] staging: android: ion: Replace strncpy() for stracpy()
On Sun, Sep 08, 2019 at 02:34:50PM +1000, Adam Zerella wrote:
> Using strncpy() does not always terminate the destination string.
> stracpy() is a alternative function that does, by using this new
> function we will no longer need to insert a null separator.
>
> Signed-off-by: Adam Zerella
> ---
> drivers/staging/android/ion/ion.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index e6b1ca141b93..17901bd626be 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -433,8 +433,7 @@ static int ion_query_heaps(struct ion_heap_query *query)
> max_cnt = query->cnt;
>
> plist_for_each_entry(heap, &dev->heaps, node) {
> - strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
> - hdata.name[sizeof(hdata.name) - 1] = '\0';
> + stracpy(hdata.name, heap->name, MAX_HEAP_NAME);
stracpy() only takes two arguments. This doesn't compile.
regards,
dan carpenter
[Bug 111232] 3200 Memory Crash My System
https://bugs.freedesktop.org/show_bug.cgi?id=111232 --- Comment #7 from bibitocarlos --- Created attachment 145321 --> https://bugs.freedesktop.org/attachment.cgi?id=145321&action=edit dmesg Tried this "amdgpu.ppfeaturemask=0x3fff" but iit doesnt work (from here: https://bugzilla.kernel.org/show_bug.cgi?id=204689). Add new dmesg to check firmware version -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111236] VA-API radeonsi SIGSEGV __memmove_avx_unaligned
https://bugs.freedesktop.org/show_bug.cgi?id=111236 --- Comment #14 from Víctor Jáquez --- (In reply to Michel Dänzer from comment #13) > Note that my patch is just a proof of concept pointing at where the issue > lies. I hope it helps someone else come up with a proper fix. :( Who could review/improve this patch? Or perhaps just be bold and open a MR in gitlab or send it to mesa's mailing list? O:) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/fb-helper: Synchronize dirty worker with vblank
Hi
Am 10.09.19 um 13:52 schrieb Gerd Hoffmann:
> On Mon, Sep 09, 2019 at 04:06:32PM +0200, Thomas Zimmermann wrote:
>> Before updating the display from the console's shadow buffer, the dirty
>> worker now waits for vblank. This allows several screen updates to pile
>> up and acts as a rate limiter.
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/drm_fb_helper.c | 12
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index a7ba5b4902d6..017e2f6bd1b9 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -402,8 +402,20 @@ static void drm_fb_helper_dirty_work(struct work_struct
>> *work)
>> dirty_work);
>> struct drm_clip_rect *clip = &helper->dirty_clip;
>> struct drm_clip_rect clip_copy;
>> +struct drm_crtc *crtc;
>> unsigned long flags;
>> void *vaddr;
>> +int ret;
>> +
>> +/* rate-limit update frequency */
>> +mutex_lock(&helper->lock);
>> +crtc = helper->client.modesets[0].crtc;
>> +ret = drm_crtc_vblank_get(crtc);
>> +if (!ret) {
>> +drm_crtc_wait_one_vblank(crtc);
>> +drm_crtc_vblank_put(crtc);
>> +}
>> +mutex_unlock(&helper->lock);
>
> Hmm, not sure it is the best plan to sleep for a while in the worker,
> especially while holding the lock.
>
> What does the lock protect against here? Accessing
This lock is hold by the fbdev code during mode-setting operations (but
not drawing operations). So *crtc might be gone if we don't hold it here.
> helper->client.modesets? If so then you can unlock before going to
> sleep in drm_crtc_wait_one_vblank() I think.
I looked, but I cannot find any code that protects crtc while vblank is
active. I'd rather not change the current code until someone can clarify.
Best regards
Thomas
>
> cheers,
> Gerd
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] meson: Fix sys/mkdev.h detection on Solaris
On Monday, 2019-09-09 16:51:16 -0700, Alan Coopersmith wrote:
> On Solaris, sys/sysmacros.h has long-deprecated copies of major() & minor()
> but not makedev(). sys/mkdev.h has all three and is the preferred choice.
>
> So we check for sys/mkdev.h first, as autoconf's AC_HEADER_MAJOR does.
Reviewed-by: Eric Engestrom
Alternatively, how about this?
---8<---
diff --git a/meson.build b/meson.build
index bc5cfc588d0c621a9725..263f691ab2b9107f5be1 100644
--- a/meson.build
+++ b/meson.build
@@ -183,9 +183,14 @@ foreach header : ['sys/sysctl.h', 'sys/select.h',
'alloca.h']
config.set('HAVE_' + header.underscorify().to_upper(),
cc.compiles('#include <@0@>'.format(header), name : '@0@
works'.format(header)))
endforeach
-if cc.has_header_symbol('sys/sysmacros.h', 'major')
+if (cc.has_header_symbol('sys/sysmacros.h', 'major') and
+ cc.has_header_symbol('sys/sysmacros.h', 'minor') and
+ cc.has_header_symbol('sys/sysmacros.h', 'makedev'))
config.set10('MAJOR_IN_SYSMACROS', true)
-elif cc.has_header_symbol('sys/mkdev.h', 'major')
+endif
+if (cc.has_header_symbol('sys/mkdev.h', 'major') and
+ cc.has_header_symbol('sys/mkdev.h', 'minor') and
+ cc.has_header_symbol('sys/mkdev.h', 'makedev'))
config.set10('MAJOR_IN_MKDEV', true)
endif
config.set10('HAVE_OPEN_MEMSTREAM', cc.has_function('open_memstream'))
--->8---
Makes both checks independent and represent the reality of what's wanted
more accurately (despite the historical name of the macro).
>
> Fixes build failure with error:
> ../xf86drm.c: In function ‘drmOpenMinor’:
> ../xf86drm.c:454:30: error: implicit declaration of function ‘makedev’
> [-Werror=implicit-function-declaration]
> 454 | return drmOpenDevice(makedev(DRM_MAJOR, minor), minor, type);
> | ^~~
>
> Signed-off-by: Alan Coopersmith
> ---
> meson.build | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index bc5cfc58..a3363c32 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -183,10 +183,10 @@ foreach header : ['sys/sysctl.h', 'sys/select.h',
> 'alloca.h']
>config.set('HAVE_' + header.underscorify().to_upper(),
> cc.compiles('#include <@0@>'.format(header), name : '@0@
> works'.format(header)))
> endforeach
> -if cc.has_header_symbol('sys/sysmacros.h', 'major')
> - config.set10('MAJOR_IN_SYSMACROS', true)
> -elif cc.has_header_symbol('sys/mkdev.h', 'major')
> +if cc.has_header_symbol('sys/mkdev.h', 'major')
>config.set10('MAJOR_IN_MKDEV', true)
> +elif cc.has_header_symbol('sys/sysmacros.h', 'major')
> + config.set10('MAJOR_IN_SYSMACROS', true)
> endif
> config.set10('HAVE_OPEN_MEMSTREAM', cc.has_function('open_memstream'))
>
> --
> 2.15.2
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config
+Manasi, Gaurav
On 2019-09-09 9:52 a.m., Benjamin Gaignard wrote:
> Change scale_increment_interval and nfl_bpg_offset fields to
> u32 to avoid W=1 warnings because we are testing them against
> 65535.
>
> Signed-off-by: Benjamin Gaignard
> ---
> include/drm/drm_dsc.h | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> index 887954cbfc60..e495024e901c 100644
> --- a/include/drm/drm_dsc.h
> +++ b/include/drm/drm_dsc.h
> @@ -207,11 +207,13 @@ struct drm_dsc_config {
>* Number of group times between incrementing the scale factor value
>* used at the beginning of a slice.
>*/
> - u16 scale_increment_interval;
> + u32 scale_increment_interval;
The DSC spec defines both as u16. I think the check in drm_dsc.c is
useless and should be dropped.
Harry
> +
> /**
>* @nfl_bpg_offset: Non first line BPG offset to be used
>*/
> - u16 nfl_bpg_offset;
> +
> + u32 nfl_bpg_offset;
> /**
>* @slice_bpg_offset: BPG offset used to enforce slice bit
>*/
>
Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
On Tue, Sep 10, 2019 at 3:34 AM Mun, Gwan-gyeong wrote: > > On Sat, 2019-09-07 at 21:43 -0400, Ilia Mirkin wrote: > > On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong > > wrote: > > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote: > > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä > > > > wrote: > > > > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote: > > > > > > > -Original Message- > > > > > > > From: Ilia Mirkin > > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM > > > > > > > To: Mun, Gwan-gyeong > > > > > > > Cc: Intel Graphics Development < > > > > > > > intel-...@lists.freedesktop.org > > > > > > > > ; Shankar, Uma > > > > > > > ; dri-devel < > > > > > > > dri-devel@lists.freedesktop.org> > > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace > > > > > > > property > > > > > > > > > > > > > > So how would this work with a DP++ connector? Should it > > > > > > > list > > > > > > > the HDMI or DP > > > > > > > properties? Or do we need a custom property checker which > > > > > > > is > > > > > > > aware of what is > > > > > > > currently plugged in to validate the values? > > > > > > > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving > > > > > > DP > > > > > > or HDMI (with a passive dongle). > > > > > > Based on the type of sink detected, we should expose DP or > > > > > > HDMI > > > > > > colorspaces to userspace. > > > > > > > > > > For i915 DP connector always drives DP mode, HDMI connector > > > > > always > > > > > drives > > > > > HDMI mode, even when the physical connector is DP++. > > > > > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and > > > > amdgpu > > > > create 1 connector (not sure about other drivers) for a single > > > > physical DP++ socket. Since we supply the list of valid values at > > > > the > > > > time of creating the connector, we can't know at that point > > > > whether > > > > in > > > > the future a HDMI or DP will be plugged into it. > > > > > > > > -ilia > > > Ilia, does it mean that the drm_connector type is > > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode? > > > > That is correct. The connector type is "DisplayPort" in such a case. > > > > Cheers, > > > > -ilia > > For now drm_mode_create_colorspace_property() is only used for i915. > IMHO, when other drivers ( nouveau, radeon, and amdgpu ) are ready for > using of drm_mode_create_colorspace_property(), > what about do we add a variable which can identify DP++ and DP to > drm_connector? > And when the drivers (nouveau, radeon, and amdgpu) detect the current > protocol, the drivers will set the variable. I've been working on adding this to nouveau. Can/should such properties be added/removed at "runtime", rather than at connector creation time? Either way, the function drm_mode_create_colorspace_property as proposed would not be reusable, since it looks at the connector type, which will always be "DisplayPort" in such cases. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 2/4] leds: Add managed API to get a LED from a device driver
If the LED is acquired by a consumer device with devm_led_get(), it is
automatically released when the device is detached.
Signed-off-by: Jean-Jacques Hiblot
Acked-by: Pavel Machek
---
drivers/leds/led-class.c | 42
include/linux/leds.h | 2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index d4d22f4b5052..664c81e2513c 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -263,6 +263,48 @@ void led_put(struct led_classdev *led_cdev)
}
EXPORT_SYMBOL_GPL(led_put);
+static void devm_led_release(struct device *dev, void *res)
+{
+ struct led_classdev **p = res;
+
+ led_put(*p);
+}
+
+/**
+ * devm_led_get - Resource-managed request of a LED device
+ * @dev: LED consumer
+ * @idx: index of the LED to obtain in the consumer
+ *
+ * The device node of the device is parse to find the request LED device.
+ * The LED device returned from this function is automatically released
+ * on driver detach.
+ *
+ * @return a pointer to a LED device or ERR_PTR(errno) on failure.
+ */
+struct led_classdev *__must_check devm_led_get(struct device *dev,
+ int index)
+{
+ struct led_classdev *led;
+ struct led_classdev **dr;
+
+ led = of_led_get(dev->of_node, index);
+ if (IS_ERR(led))
+ return led;
+
+ dr = devres_alloc(devm_led_release, sizeof(struct led_classdev *),
+ GFP_KERNEL);
+ if (!dr) {
+ led_put(led);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ *dr = led;
+ devres_add(dev, dr);
+
+ return led;
+}
+EXPORT_SYMBOL_GPL(devm_led_get);
+
static int match_name(struct device *dev, const void *data)
{
if (!dev_name(dev))
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 0a71c7cdd191..7fcec566d774 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -148,6 +148,8 @@ extern void led_classdev_resume(struct led_classdev
*led_cdev);
extern struct led_classdev *of_led_get(struct device_node *np, int index);
extern void led_put(struct led_classdev *led_cdev);
+struct led_classdev *__must_check devm_led_get(struct device *dev,
+ int index);
/**
* led_blink_set - set blinking with software fallback
--
2.17.1
[PATCH v6 4/4] backlight: add led-backlight driver
From: Tomi Valkeinen
This patch adds a led-backlight driver (led_bl), which is similar to
pwm_bl except the driver uses a LED class driver to adjust the
brightness in the HW. Multiple LEDs can be used for a single backlight.
Signed-off-by: Tomi Valkeinen
Signed-off-by: Jean-Jacques Hiblot
Acked-by: Pavel Machek
---
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/led_bl.c | 260 +++
3 files changed, 268 insertions(+)
create mode 100644 drivers/video/backlight/led_bl.c
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8b081d61773e..585a1787618c 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -458,6 +458,13 @@ config BACKLIGHT_RAVE_SP
help
Support for backlight control on RAVE SP device.
+config BACKLIGHT_LED
+ tristate "Generic LED based Backlight Driver"
+ depends on LEDS_CLASS && OF
+ help
+ If you have a LCD backlight adjustable by LED class driver, say Y
+ to enable this driver.
+
endif # BACKLIGHT_CLASS_DEVICE
endmenu
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 63c507c07437..2a67642966a5 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -57,3 +57,4 @@ obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
obj-$(CONFIG_BACKLIGHT_ARCXCNN)+= arcxcnn_bl.o
obj-$(CONFIG_BACKLIGHT_RAVE_SP)+= rave-sp-backlight.o
+obj-$(CONFIG_BACKLIGHT_LED)+= led_bl.o
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
new file mode 100644
index ..6044390d8b88
--- /dev/null
+++ b/drivers/video/backlight/led_bl.c
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2015-2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Tomi Valkeinen
+ *
+ * Based on pwm_bl.c
+ */
+
+#include
+#include
+#include
+#include
+
+struct led_bl_data {
+ struct device *dev;
+ struct backlight_device *bl_dev;
+ struct led_classdev **leds;
+ boolenabled;
+ int nb_leds;
+ unsigned int*levels;
+ unsigned intdefault_brightness;
+ unsigned intmax_brightness;
+};
+
+static void led_bl_set_brightness(struct led_bl_data *priv, int level)
+{
+ int i;
+ int bkl_brightness;
+
+ if (priv->levels)
+ bkl_brightness = priv->levels[level];
+ else
+ bkl_brightness = level;
+
+ for (i = 0; i < priv->nb_leds; i++)
+ led_set_brightness(priv->leds[i], bkl_brightness);
+
+ priv->enabled = true;
+}
+
+static void led_bl_power_off(struct led_bl_data *priv)
+{
+ int i;
+
+ if (!priv->enabled)
+ return;
+
+ for (i = 0; i < priv->nb_leds; i++)
+ led_set_brightness(priv->leds[i], LED_OFF);
+
+ priv->enabled = false;
+}
+
+static int led_bl_update_status(struct backlight_device *bl)
+{
+ struct led_bl_data *priv = bl_get_data(bl);
+ int brightness = bl->props.brightness;
+
+ if (bl->props.power != FB_BLANK_UNBLANK ||
+ bl->props.fb_blank != FB_BLANK_UNBLANK ||
+ bl->props.state & BL_CORE_FBBLANK)
+ brightness = 0;
+
+ if (brightness > 0)
+ led_bl_set_brightness(priv, brightness);
+ else
+ led_bl_power_off(priv);
+
+ return 0;
+}
+
+static const struct backlight_ops led_bl_ops = {
+ .update_status = led_bl_update_status,
+};
+
+static int led_bl_get_leds(struct device *dev,
+ struct led_bl_data *priv)
+{
+ int i, nb_leds, ret;
+ struct device_node *node = dev->of_node;
+ struct led_classdev **leds;
+ unsigned int max_brightness;
+ unsigned int default_brightness;
+
+ ret = of_count_phandle_with_args(node, "leds", NULL);
+ if (ret < 0) {
+ dev_err(dev, "Unable to get led count\n");
+ return -EINVAL;
+ }
+
+ nb_leds = ret;
+ if (nb_leds < 1) {
+ dev_err(dev, "At least one LED must be specified!\n");
+ return -EINVAL;
+ }
+
+ leds = devm_kzalloc(dev, sizeof(struct led_classdev *) * nb_leds,
+ GFP_KERNEL);
+ if (!leds)
+ return -ENOMEM;
+
+ for (i = 0; i < nb_leds; i++) {
+ leds[i] = devm_led_get(dev, i);
+ if (IS_ERR(leds[i]))
+ return PTR_ERR(leds[i]);
+ }
+
+ /* check that the LEDs all have the same brightness range */
+ max_brightness = leds[0]->max_brightness;
+ for (i = 1; i < nb_leds; i++) {
+ if (max_brightness != leds[i]->max_brightness) {
+
[PATCH v6 0/4] Add a generic driver for LED-based backlight
This series aims to add a led-backlight driver, similar to pwm-backlight, but using a LED class device underneath. A few years ago (2015), Tomi Valkeinen posted a series implementing a backlight driver on top of a LED device: https://patchwork.kernel.org/patch/7293991/ https://patchwork.kernel.org/patch/7294001/ https://patchwork.kernel.org/patch/7293981/ The discussion stopped because Tomi lacked the time to work on it. changes in v6: - trim the list of included headers - remove useless definition of BKL_FULL_BRIGHTNESS changes in v5: - removed LED brightness scaling - disable sysfs the interface of the LEDs when used by the backlight device changes in v4: - fix dev_err() messages and commit logs following the advices of Pavel - cosmetic changes (indents, getting rid of "? 1 : 0" in led_match_led_node()) changes in v3: - dt binding: don't limit the brightness range to 0-255. Use the range of the underlying LEDs. as a side-effect, all LEDs must now have the same range - driver: Adapt to dt binding update. - driver: rework probe() for clarity and remove the remaining goto. changes in v2: - handle more than one LED. - don't make the backlight device a child of the LED controller. - make brightness-levels and default-brightness-level optional - removed the option to use a GPIO enable. - removed the option to use a regulator. It should be handled by the LED core - don't make any change to the LED core (not needed anymore) Jean-Jacques Hiblot (2): leds: Add managed API to get a LED from a device driver dt-bindings: backlight: Add led-backlight binding Tomi Valkeinen (2): leds: Add of_led_get() and led_put() backlight: add led-backlight driver .../bindings/leds/backlight/led-backlight.txt | 28 ++ drivers/leds/led-class.c | 92 +++ drivers/video/backlight/Kconfig | 7 + drivers/video/backlight/Makefile | 1 + drivers/video/backlight/led_bl.c | 260 ++ include/linux/leds.h | 6 + 6 files changed, 394 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt create mode 100644 drivers/video/backlight/led_bl.c -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 3/4] dt-bindings: backlight: Add led-backlight binding
Add DT binding for led-backlight.
Signed-off-by: Jean-Jacques Hiblot
Reviewed-by: Daniel Thompson
---
.../bindings/leds/backlight/led-backlight.txt | 28 +++
1 file changed, 28 insertions(+)
create mode 100644
Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
new file mode 100644
index ..4c7dfbe7f67a
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
@@ -0,0 +1,28 @@
+led-backlight bindings
+
+This binding is used to describe a basic backlight device made of LEDs.
+It can also be used to describe a backlight device controlled by the output of
+a LED driver.
+
+Required properties:
+ - compatible: "led-backlight"
+ - leds: a list of LEDs
+
+Optional properties:
+ - brightness-levels: Array of distinct brightness levels. The levels must be
+ in the range accepted by the underlying LED devices.
+ This is used to translate a backlight brightness level
+ into a LED brightness level. If it is not provided, the
+ identity mapping is used.
+
+ - default-brightness-level: The default brightness level.
+
+Example:
+
+ backlight {
+ compatible = "led-backlight";
+
+ leds = <&led1>, <&led2>;
+ brightness-levels = <0 4 8 16 32 64 128 255>;
+ default-brightness-level = <6>;
+ };
--
2.17.1
[PATCH v6 1/4] leds: Add of_led_get() and led_put()
From: Tomi Valkeinen
This patch adds basic support for a kernel driver to get a LED device.
This will be used by the led-backlight driver.
Only OF version is implemented for now, and the behavior is similar to
PWM's of_pwm_get() and pwm_put().
Signed-off-by: Tomi Valkeinen
Signed-off-by: Jean-Jacques Hiblot
Acked-by: Pavel Machek
---
drivers/leds/led-class.c | 50
include/linux/leds.h | 4
2 files changed, 54 insertions(+)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 159c6caa374b..d4d22f4b5052 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include "leds.h"
static struct class *leds_class;
@@ -213,6 +214,55 @@ static int led_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume);
+static int led_match_led_node(struct device *led_dev, const void *data)
+{
+ return led_dev->of_node == data;
+}
+
+/**
+ * of_led_get() - request a LED device via the LED framework
+ * @np: device node to get the LED device from
+ * @index: the index of the LED
+ *
+ * Returns the LED device parsed from the phandle specified in the "leds"
+ * property of a device tree node or a negative error-code on failure.
+ */
+struct led_classdev *of_led_get(struct device_node *np, int index)
+{
+ struct device *led_dev;
+ struct led_classdev *led_cdev;
+ struct device_node *led_node;
+
+ led_node = of_parse_phandle(np, "leds", index);
+ if (!led_node)
+ return ERR_PTR(-ENOENT);
+
+ led_dev = class_find_device(leds_class, NULL, led_node,
+ led_match_led_node);
+ of_node_put(led_node);
+
+ if (!led_dev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ led_cdev = dev_get_drvdata(led_dev);
+
+ if (!try_module_get(led_cdev->dev->parent->driver->owner))
+ return ERR_PTR(-ENODEV);
+
+ return led_cdev;
+}
+EXPORT_SYMBOL_GPL(of_led_get);
+
+/**
+ * led_put() - release a LED device
+ * @led_cdev: LED device
+ */
+void led_put(struct led_classdev *led_cdev)
+{
+ module_put(led_cdev->dev->parent->driver->owner);
+}
+EXPORT_SYMBOL_GPL(led_put);
+
static int match_name(struct device *dev, const void *data)
{
if (!dev_name(dev))
diff --git a/include/linux/leds.h b/include/linux/leds.h
index bee8e3f8..0a71c7cdd191 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -19,6 +19,7 @@
struct device;
struct led_pattern;
+struct device_node;
/*
* LED Core
*/
@@ -145,6 +146,9 @@ extern void devm_led_classdev_unregister(struct device
*parent,
extern void led_classdev_suspend(struct led_classdev *led_cdev);
extern void led_classdev_resume(struct led_classdev *led_cdev);
+extern struct led_classdev *of_led_get(struct device_node *np, int index);
+extern void led_put(struct led_classdev *led_cdev);
+
/**
* led_blink_set - set blinking with software fallback
* @led_cdev: the LED to start blinking
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 4/4] backlight: add led-backlight driver
On Tue, Sep 10, 2019 at 03:27:34PM +0200, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen
>
> This patch adds a led-backlight driver (led_bl), which is similar to
> pwm_bl except the driver uses a LED class driver to adjust the
> brightness in the HW. Multiple LEDs can be used for a single backlight.
>
> Signed-off-by: Tomi Valkeinen
> Signed-off-by: Jean-Jacques Hiblot
> Acked-by: Pavel Machek
Reviewed-by: Daniel Thompson
> ---
> drivers/video/backlight/Kconfig | 7 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/led_bl.c | 260 +++
> 3 files changed, 268 insertions(+)
> create mode 100644 drivers/video/backlight/led_bl.c
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 8b081d61773e..585a1787618c 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -458,6 +458,13 @@ config BACKLIGHT_RAVE_SP
> help
> Support for backlight control on RAVE SP device.
>
> +config BACKLIGHT_LED
> + tristate "Generic LED based Backlight Driver"
> + depends on LEDS_CLASS && OF
> + help
> + If you have a LCD backlight adjustable by LED class driver, say Y
> + to enable this driver.
> +
> endif # BACKLIGHT_CLASS_DEVICE
>
> endmenu
> diff --git a/drivers/video/backlight/Makefile
> b/drivers/video/backlight/Makefile
> index 63c507c07437..2a67642966a5 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -57,3 +57,4 @@ obj-$(CONFIG_BACKLIGHT_TPS65217)+= tps65217_bl.o
> obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
> obj-$(CONFIG_BACKLIGHT_ARCXCNN) += arcxcnn_bl.o
> obj-$(CONFIG_BACKLIGHT_RAVE_SP) += rave-sp-backlight.o
> +obj-$(CONFIG_BACKLIGHT_LED) += led_bl.o
> diff --git a/drivers/video/backlight/led_bl.c
> b/drivers/video/backlight/led_bl.c
> new file mode 100644
> index ..6044390d8b88
> --- /dev/null
> +++ b/drivers/video/backlight/led_bl.c
> @@ -0,0 +1,260 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2015-2019 Texas Instruments Incorporated -
> http://www.ti.com/
> + * Author: Tomi Valkeinen
> + *
> + * Based on pwm_bl.c
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +struct led_bl_data {
> + struct device *dev;
> + struct backlight_device *bl_dev;
> + struct led_classdev **leds;
> + boolenabled;
> + int nb_leds;
> + unsigned int*levels;
> + unsigned intdefault_brightness;
> + unsigned intmax_brightness;
> +};
> +
> +static void led_bl_set_brightness(struct led_bl_data *priv, int level)
> +{
> + int i;
> + int bkl_brightness;
> +
> + if (priv->levels)
> + bkl_brightness = priv->levels[level];
> + else
> + bkl_brightness = level;
> +
> + for (i = 0; i < priv->nb_leds; i++)
> + led_set_brightness(priv->leds[i], bkl_brightness);
> +
> + priv->enabled = true;
> +}
> +
> +static void led_bl_power_off(struct led_bl_data *priv)
> +{
> + int i;
> +
> + if (!priv->enabled)
> + return;
> +
> + for (i = 0; i < priv->nb_leds; i++)
> + led_set_brightness(priv->leds[i], LED_OFF);
> +
> + priv->enabled = false;
> +}
> +
> +static int led_bl_update_status(struct backlight_device *bl)
> +{
> + struct led_bl_data *priv = bl_get_data(bl);
> + int brightness = bl->props.brightness;
> +
> + if (bl->props.power != FB_BLANK_UNBLANK ||
> + bl->props.fb_blank != FB_BLANK_UNBLANK ||
> + bl->props.state & BL_CORE_FBBLANK)
> + brightness = 0;
> +
> + if (brightness > 0)
> + led_bl_set_brightness(priv, brightness);
> + else
> + led_bl_power_off(priv);
> +
> + return 0;
> +}
> +
> +static const struct backlight_ops led_bl_ops = {
> + .update_status = led_bl_update_status,
> +};
> +
> +static int led_bl_get_leds(struct device *dev,
> +struct led_bl_data *priv)
> +{
> + int i, nb_leds, ret;
> + struct device_node *node = dev->of_node;
> + struct led_classdev **leds;
> + unsigned int max_brightness;
> + unsigned int default_brightness;
> +
> + ret = of_count_phandle_with_args(node, "leds", NULL);
> + if (ret < 0) {
> + dev_err(dev, "Unable to get led count\n");
> + return -EINVAL;
> + }
> +
> + nb_leds = ret;
> + if (nb_leds < 1) {
> + dev_err(dev, "At least one LED must be specified!\n");
> + return -EINVAL;
> + }
> +
> + leds = devm_kzalloc(dev, sizeof(struct led_classdev *) * nb_leds,
> + GFP_KERNEL);
> + if (!leds)
> + return -ENOMEM;
> +
> + for (i = 0; i < nb_leds; i++) {
> + leds[i] = devm_led_get(dev, i);
> + if (IS_ERR(leds[i]))
> +
Re: [PATCH v2] drm: panel-lvds: Potential Oops in probe error handling
On Sun, Sep 08, 2019 at 06:04:28PM +0200, Sam Ravnborg wrote:
> Hi Dan.
>
> On Wed, Sep 04, 2019 at 09:55:07PM +0300, Dan Carpenter wrote:
> > The "lvds->backlight" pointer could be NULL in situations were
> > of_parse_phandle() returns NULL. Also it's slightly cleaner to use
> > backlight_put() which already has a check for NULL built in.
> >
> > Fixes: 7c9dff5bd643 ("drm: panels: Add LVDS panel driver")
> > Signed-off-by: Dan Carpenter
>
> A much better fix would be to introduce use of devm_of_find_backlight().
> Then you do not have to worry about put().
>
> Care to respin a v3 that does this?
Sure. That does look nice. I will resend tomorrow.
regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/fb-helper: Synchronize dirty worker with vblank
Den 10.09.2019 14.48, skrev Thomas Zimmermann:
> Hi
>
> Am 10.09.19 um 13:52 schrieb Gerd Hoffmann:
>> On Mon, Sep 09, 2019 at 04:06:32PM +0200, Thomas Zimmermann wrote:
>>> Before updating the display from the console's shadow buffer, the dirty
>>> worker now waits for vblank. This allows several screen updates to pile
>>> up and acts as a rate limiter.
>>>
>>> Signed-off-by: Thomas Zimmermann
>>> ---
>>> drivers/gpu/drm/drm_fb_helper.c | 12
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>>> b/drivers/gpu/drm/drm_fb_helper.c
>>> index a7ba5b4902d6..017e2f6bd1b9 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -402,8 +402,20 @@ static void drm_fb_helper_dirty_work(struct
>>> work_struct *work)
>>> dirty_work);
>>> struct drm_clip_rect *clip = &helper->dirty_clip;
>>> struct drm_clip_rect clip_copy;
>>> + struct drm_crtc *crtc;
>>> unsigned long flags;
>>> void *vaddr;
>>> + int ret;
>>> +
>>> + /* rate-limit update frequency */
>>> + mutex_lock(&helper->lock);
>>> + crtc = helper->client.modesets[0].crtc;
>>> + ret = drm_crtc_vblank_get(crtc);
>>> + if (!ret) {
>>> + drm_crtc_wait_one_vblank(crtc);
>>> + drm_crtc_vblank_put(crtc);
>>> + }
>>> + mutex_unlock(&helper->lock);
>>
>> Hmm, not sure it is the best plan to sleep for a while in the worker,
>> especially while holding the lock.
>>
>> What does the lock protect against here? Accessing
>
> This lock is hold by the fbdev code during mode-setting operations (but
> not drawing operations). So *crtc might be gone if we don't hold it here.
>
>> helper->client.modesets? If so then you can unlock before going to
>> sleep in drm_crtc_wait_one_vblank() I think.
>
> I looked, but I cannot find any code that protects crtc while vblank is
> active. I'd rather not change the current code until someone can clarify.
>
The client->modesets array and the crtc struct member are invariant over
the lifetime of client (drm_client_modeset_create()). No need to take a
lock for access. See drm_client_modeset_release() for the things that
_can_ change and needs protection (protected by client->modeset_mutex).
I don't see a problem with sleeping in the worker though, but I might
miss out on something. AFAICS changes will just pile up in >dirty_clip
and the worker will be scheduled for a new run to happen when it's done
with the current update.
Noralf.
> Best regards
> Thomas
>
>>
>> cheers,
>> Gerd
>>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/fb-helper: Synchronize dirty worker with vblank
Hi
Am 10.09.19 um 15:34 schrieb Noralf Trønnes:
>
>
> Den 10.09.2019 14.48, skrev Thomas Zimmermann:
>> Hi
>>
>> Am 10.09.19 um 13:52 schrieb Gerd Hoffmann:
>>> On Mon, Sep 09, 2019 at 04:06:32PM +0200, Thomas Zimmermann wrote:
Before updating the display from the console's shadow buffer, the dirty
worker now waits for vblank. This allows several screen updates to pile
up and acts as a rate limiter.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_fb_helper.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c
b/drivers/gpu/drm/drm_fb_helper.c
index a7ba5b4902d6..017e2f6bd1b9 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -402,8 +402,20 @@ static void drm_fb_helper_dirty_work(struct
work_struct *work)
dirty_work);
struct drm_clip_rect *clip = &helper->dirty_clip;
struct drm_clip_rect clip_copy;
+ struct drm_crtc *crtc;
unsigned long flags;
void *vaddr;
+ int ret;
+
+ /* rate-limit update frequency */
+ mutex_lock(&helper->lock);
+ crtc = helper->client.modesets[0].crtc;
+ ret = drm_crtc_vblank_get(crtc);
+ if (!ret) {
+ drm_crtc_wait_one_vblank(crtc);
+ drm_crtc_vblank_put(crtc);
+ }
+ mutex_unlock(&helper->lock);
>>>
>>> Hmm, not sure it is the best plan to sleep for a while in the worker,
>>> especially while holding the lock.
>>>
>>> What does the lock protect against here? Accessing
>>
>> This lock is hold by the fbdev code during mode-setting operations (but
>> not drawing operations). So *crtc might be gone if we don't hold it here.
>>
>>> helper->client.modesets? If so then you can unlock before going to
>>> sleep in drm_crtc_wait_one_vblank() I think.
>>
>> I looked, but I cannot find any code that protects crtc while vblank is
>> active. I'd rather not change the current code until someone can clarify.
>>
>
> The client->modesets array and the crtc struct member are invariant over
> the lifetime of client (drm_client_modeset_create()). No need to take a
> lock for access. See drm_client_modeset_release() for the things that
> _can_ change and needs protection (protected by client->modeset_mutex).
Thanks for the reply. So we don't need the lock? But why does
drm_fb_helper_ioctl() take it? ioctl exclusiveness?
> I don't see a problem with sleeping in the worker though, but I might
> miss out on something. AFAICS changes will just pile up in >dirty_clip
> and the worker will be scheduled for a new run to happen when it's done
> with the current update.
Yes, that's the intention of the patch. We hope to reduce the number of
memcpys by handling several of them at once.
Best regards
Thomas
>
> Noralf.
>
>> Best regards
>> Thomas
>>
>>>
>>> cheers,
>>> Gerd
>>>
>>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/mgag200: Add vblank support
On Mon, Sep 09, 2019 at 04:06:33PM +0200, Thomas Zimmermann wrote:
> Support for vblank requires VSYNC to signal an interrupt, which is broken
> on Matrox chipsets.
I don't remember there being anything wrong with the vsync interrupt.
What makes you think it's broken?
> The workaround that is used here and in other free
> Matrox drivers is to program to the value of and
> enable the VLINE interrupt. This triggers an interrupt at the same time
> when VSYNC begins.
You're programming it to fire at start of vblank, not start of vsync.
>
> VLINE uses separate registers for enabling and clearing pending interrupts.
> No extra syncronization between irq handler and the rest of the driver is
> required.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/mgag200/mgag200_drv.c | 1 +
> drivers/gpu/drm/mgag200/mgag200_drv.h | 1 +
> drivers/gpu/drm/mgag200/mgag200_main.c | 33
> drivers/gpu/drm/mgag200/mgag200_mode.c | 42 +++---
> 4 files changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c
> b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index 4f9df3b93598..cff265973154 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -67,6 +67,7 @@ static struct drm_driver driver = {
> .driver_features = DRIVER_GEM | DRIVER_MODESET,
> .load = mgag200_driver_load,
> .unload = mgag200_driver_unload,
> + .irq_handler = mgag200_irq_handler,
> .fops = &mgag200_driver_fops,
> .name = DRIVER_NAME,
> .desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 1c93f8dc08c7..88cf256d135f 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -195,6 +195,7 @@ void mgag200_modeset_fini(struct mga_device *mdev);
> /* mgag200_main.c */
> int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
> void mgag200_driver_unload(struct drm_device *dev);
> +irqreturn_t mgag200_irq_handler(int irq, void *arg);
>
> /* mgag200_i2c.c */
> struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c
> b/drivers/gpu/drm/mgag200/mgag200_main.c
> index a9773334dedf..5941607796e8 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
> @@ -10,7 +10,9 @@
>
> #include
> #include
> +#include
> #include
> +#include
>
> #include "mgag200_drv.h"
>
> @@ -186,10 +188,18 @@ int mgag200_driver_load(struct drm_device *dev,
> unsigned long flags)
> }
> mdev->cursor.pixels_current = NULL;
>
> + r = drm_vblank_init(dev, 1);
> + if (r)
> + goto err_modeset;
> +
> r = drm_fbdev_generic_setup(mdev->dev, 0);
> if (r)
> goto err_modeset;
>
> + r = drm_irq_install(dev, dev->pdev->irq);
> + if (r)
> + goto err_modeset;
> +
> return 0;
>
> err_modeset:
> @@ -207,8 +217,31 @@ void mgag200_driver_unload(struct drm_device *dev)
>
> if (mdev == NULL)
> return;
> + drm_irq_uninstall(dev);
> mgag200_modeset_fini(mdev);
> drm_mode_config_cleanup(dev);
> mgag200_mm_fini(mdev);
> dev->dev_private = NULL;
> }
> +
> +irqreturn_t mgag200_irq_handler(int irq, void *arg)
> +{
> + struct drm_device *dev = arg;
> + struct mga_device *mdev = dev->dev_private;
> + struct drm_crtc *crtc;
> + u32 status, iclear;
> +
> + status = RREG32(0x1e14);
> +
> + if (status & 0x0020) { /* test */
> + drm_for_each_crtc(crtc, dev) {
> + drm_crtc_handle_vblank(crtc);
> + }
A bit odd way to write that but as long this driver doesn't support
crtc2 it should be fine.
> + iclear = RREG32(0x1e18);
> + iclear |= 0x0020; /* set */
> + WREG32(0x1e18, iclear);
> + return IRQ_HANDLED;
> + }
> +
> + return IRQ_NONE;
> +};
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 5e778b5f1a10..ffe5f15d0a7d 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -905,6 +905,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
> const struct drm_framebuffer *fb = crtc->primary->fb;
> int hdisplay, hsyncstart, hsyncend, htotal;
> int vdisplay, vsyncstart, vsyncend, vtotal;
> + int linecomp;
> int pitch;
> int option = 0, option2 = 0;
> int i;
> @@ -1042,6 +1043,13 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
> vsyncend = mode->vsync_end - 1;
> vtotal = mode->vtotal - 2;
>
> + /* The VSYNC interrupt is broken on Matrox chipsets. We use
> + * the VLINE interrupt instead. It triggers
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481
--- Comment #32 from Sebastian Meyer ---
Having the same issues with my new Powercolor RX 5700 XT on Arch Linux.
System freezes after a couple of seconds when I try to run games like RotTR.
Other games I've tested, like Dota 2 for example, are unreliable and make the
system freeze after a few of minutes or after an hour or so.
The dmesg output when SSHing into my system:
[65070.475185] [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for
fences timed out or interrupted!
[65070.475259] [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for
fences timed out or interrupted!
[65075.595093] [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for
fences timed out or interrupted!
[65075.595180] [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for
fences timed out or interrupted!
[65075.595260] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx_0.0.0
timeout, signaled seq=6662176, emitted seq=6662178
[65075.595322] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information:
process RiseOfTheTombRa pid 56804 thread RiseOfTheT:cs0 pid 56811
[65075.595324] [drm] GPU recovery disabled.
I've also had a couple of sdma0/sdma1 related freezes after opening
resource-heavy websites in Chromium. Unfortunately though, I'm unable to
reproduce it now. If the system freezes again, I will provide logs and umr
output, as requested. The website which caused most of the freezes was
izurvive.com (interactive DayZ map) and it froze while toggling map markers on
and off.
Sep 08 17:49:52 basti-pc kernel: [drm:amdgpu_dm_atomic_commit_tail [amdgpu]]
*ERROR* Waiting for fences timed out or interrupted!
Sep 08 17:49:57 basti-pc kernel: [drm:amdgpu_dm_atomic_commit_tail [amdgpu]]
*ERROR* Waiting for fences timed out or interrupted!
Sep 08 17:49:57 basti-pc kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR*
ring sdma1 timeout, signaled seq=2372, emitted seq=2375
Sep 08 17:49:57 basti-pc kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR*
Process information: process chromium pid 1271 thread chromium:cs0 pid 1331
$ pacman -Q linux-mainline linux-firmware-agd5f-radeon-navi10
{,lib32-}{mesa-git,vulkan-radeon-git,llvm-git,libdrm-git}
linux-mainline 5.3rc8-1
linux-firmware-agd5f-radeon-navi10 2019.08.26.14.36-1
mesa-git 1:19.3.0_devel.115190.f83f9d7daa0-1
lib32-mesa-git 1:19.3.0_devel.115190.f83f9d7daa0-1
vulkan-radeon-git 1:19.3.0_devel.115190.f83f9d7daa0-1
lib32-vulkan-radeon-git 1:19.3.0_devel.115190.f83f9d7daa0-1
llvm-git 10.0.0_r326348.d7d8bb937ad-1
lib32-llvm-git 10.0.0_r326355.d065c811649-1
libdrm-git 2.4.99.r17.g10cd9c3d-1
lib32-libdrm-git 2.4.99.r17.g10cd9c3d-1
--
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BACKPORT 4.14.y 03/18] drm/omap: panel-dsi-cm: fix driver
On Thu, Sep 05, 2019 at 10:17:44AM -0600, Mathieu Poirier wrote: > From: Tony Lindgren > > commit e128310ddd379b0fdd21dc41d176c3b3505a0832 upstream > > This adds support for get_timings() and check_timings() > to get the driver working and properly initializes the > timing information from DT. > > Signed-off-by: Tony Lindgren > Signed-off-by: Sebastian Reichel > Signed-off-by: Tomi Valkeinen > Signed-off-by: Mathieu Poirier > --- > .../gpu/drm/omapdrm/displays/panel-dsi-cm.c | 56 +-- > 1 file changed, 51 insertions(+), 5 deletions(-) THis looks like a "new feature", right? Why is this a stable patch? thanks, greg k-h
Re: Adreno crash on i.MX53 running 5.3-rc6
On 05/09/2019 20:05, Rob Clark wrote:
On Thu, Sep 5, 2019 at 10:03 AM Rob Clark wrote:
On Wed, Sep 4, 2019 at 11:06 AM Robin Murphy wrote:
On 04/09/2019 01:12, Rob Clark wrote:
On Tue, Sep 3, 2019 at 12:31 PM Fabio Estevam wrote:
Hi Jonathan,
On Tue, Sep 3, 2019 at 4:25 PM Jonathan Marek wrote:
Hi,
I tried this and it works with patches 4+5 from Rob's series and
changing gpummu to use sg_phys(sg) instead of sg->dma_address
(dma_address isn't set now that dma_map_sg isn't used).
Thanks for testing it. I haven't had a chance to test it yet.
Rob,
I assume your series is targeted to 5.4, correct?
maybe, although Christoph Hellwig didn't seem like a big fan of
exposing cache ops, and would rather add a new allocation API for
uncached pages.. so I'm not entirely sure what the way forward will
be.
TBH, the use of map/unmap looked reasonable in the context of
"start/stop using these pages for stuff which may include DMA", so even
if it was cheekily ignoring sg->dma_address I'm not sure I'd really
consider it "abuse" - in comparison, using sync without a prior map
unquestionably violates the API, and means that CONFIG_DMA_API_DEBUG
will be rendered useless with false positives if this driver is active
while trying to debug something else.
The warning referenced in 0036bc73ccbe represents something being
unmapped which didn't match a corresponding map - from what I can make
of get_pages()/put_pages() it looks like that would need msm_obj->flags
or msm_obj->sgt to change during the lifetime of the object, neither of
which sounds like a thing that should legitimately happen. Are you sure
this isn't all just hiding a subtle bug elsewhere? After all, if what
was being unmapped wasn't right, who says that what's now being synced is?
Correct, msm_obj->flags/sgt should not change.
I reverted the various patches, and went back to the original setup
that used dma_{map,unmap}_sg() to reproduce the original issue that
prompted the change in the first place. It is a pretty massive flood
of splats, which pretty quickly overflowed the dmesg ring buffer, so I
might be missing some things, but I'll poke around some more.
The one thing I wonder about, what would happen if the buffer is
allocated and dma_map_sg() called before drm/msm attaches it's own
iommu_domains, and then dma_unmap_sg() afterwards. We aren't actually
ever using the iommu domain that DMA API is creating for the device,
so all the extra iommu_map/unmap (and tlb flush) is at best
unnecessary. But I'm not sure if it could be having some unintended
side effects that cause this sort of problem.
Right, one of the semi-intentional side-effects of 43c5bf11a610 is that
iommu-dma no longer interferes with unmanaged domains - it will still go
and make its own redundant mappings in the unattached default domain,
but as long as the DMA API usage is fundamentally sound then it
shouldn't actually get in the way.
it seems like every time (or at least every time we splat), we end up
w/ iova=f000 .. which doesn't sound likely to be right.
Although from just looking at the dma-iommu.c code, I'm not sure how
this happens. And adding some printk's results in enough traces that
I can't boot for some reason..
Yeah, that's a bogus IOVA for sure, so regardless of how we actually
make Adreno happy it would still be interesting to figure out how it
came about. Do you see any WARNs from io-pgtable-arm before the one from
__iommu_dma_unmap()?
Robin.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BACKPORT 4.14.y 04/18] usb: dwc3: Allow disabling of metastability workaround
On Thu, Sep 05, 2019 at 10:17:45AM -0600, Mathieu Poirier wrote: > From: Roger Quadros > > commit 42bf02ec6e420e541af9a47437d0bdf961ca2972 upstream > > Some platforms (e.g. TI's DRA7 USB2 instance) have more trouble > with the metastability workaround as it supports only > a High-Speed PHY and the PHY can enter into an Erratic state [1] > when the controller is set in SuperSpeed mode as part of > the metastability workaround. > > This causes upto 2 seconds delay in enumeration on DRA7's USB2 > instance in gadget mode. > > If these platforms can be better off without the workaround, > provide a device tree property to suggest that so the workaround > is avoided. > > [1] Device mode enumeration trace showing PHY Erratic Error. > irq/90-dwc3-969 [000] d...52.323145: dwc3_event: event (0901): > Erratic Error [U0] > irq/90-dwc3-969 [000] d...52.560646: dwc3_event: event (0901): > Erratic Error [U0] > irq/90-dwc3-969 [000] d...52.798144: dwc3_event: event (0901): > Erratic Error [U0] Does the DT also need to get updated with this new id for this? Is that a separate patch somewhere? thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/mgag200: Add vblank support
Hi
thanks for the feedback.
Am 10.09.19 um 16:01 schrieb Ville Syrjälä:
> On Mon, Sep 09, 2019 at 04:06:33PM +0200, Thomas Zimmermann wrote:
>> Support for vblank requires VSYNC to signal an interrupt, which is broken
>> on Matrox chipsets.
>
> I don't remember there being anything wrong with the vsync interrupt.
> What makes you think it's broken?
I tested vsync with the same code as in this patch, with extra spin
locks to protect against concurrent register access (vsync uses crtc11h).
A few things constantly went wrong: I never received it when setting
linecomp to vdisplay. However it fired when linecomp was ~0. Then I
regularly missed the first interrupt (but only the first), and
occasionally my test system crashed when I entered commands on the
serial terminal. The vsync irq is maybe not entirely broken, but appears
to be much harder to get right.
>> The workaround that is used here and in other free
>> Matrox drivers is to program to the value of and
>> enable the VLINE interrupt. This triggers an interrupt at the same time
>> when VSYNC begins.
>
> You're programming it to fire at start of vblank, not start of vsync.
Ah, sorry for messing up terminology; will be fixed. No matter what's
wrong with the vsync irq: there's no vblank irq, so using linecomp
should be the way to go in any case.
>
>>
>> VLINE uses separate registers for enabling and clearing pending interrupts.
>> No extra syncronization between irq handler and the rest of the driver is
>> required.
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/mgag200/mgag200_drv.c | 1 +
>> drivers/gpu/drm/mgag200/mgag200_drv.h | 1 +
>> drivers/gpu/drm/mgag200/mgag200_main.c | 33
>> drivers/gpu/drm/mgag200/mgag200_mode.c | 42 +++---
>> 4 files changed, 73 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c
>> b/drivers/gpu/drm/mgag200/mgag200_drv.c
>> index 4f9df3b93598..cff265973154 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
>> @@ -67,6 +67,7 @@ static struct drm_driver driver = {
>> .driver_features = DRIVER_GEM | DRIVER_MODESET,
>> .load = mgag200_driver_load,
>> .unload = mgag200_driver_unload,
>> +.irq_handler = mgag200_irq_handler,
>> .fops = &mgag200_driver_fops,
>> .name = DRIVER_NAME,
>> .desc = DRIVER_DESC,
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
>> b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> index 1c93f8dc08c7..88cf256d135f 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> @@ -195,6 +195,7 @@ void mgag200_modeset_fini(struct mga_device *mdev);
>> /* mgag200_main.c */
>> int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
>> void mgag200_driver_unload(struct drm_device *dev);
>> +irqreturn_t mgag200_irq_handler(int irq, void *arg);
>>
>> /* mgag200_i2c.c */
>> struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c
>> b/drivers/gpu/drm/mgag200/mgag200_main.c
>> index a9773334dedf..5941607796e8 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
>> @@ -10,7 +10,9 @@
>>
>> #include
>> #include
>> +#include
>> #include
>> +#include
>>
>> #include "mgag200_drv.h"
>>
>> @@ -186,10 +188,18 @@ int mgag200_driver_load(struct drm_device *dev,
>> unsigned long flags)
>> }
>> mdev->cursor.pixels_current = NULL;
>>
>> +r = drm_vblank_init(dev, 1);
>> +if (r)
>> +goto err_modeset;
>> +
>> r = drm_fbdev_generic_setup(mdev->dev, 0);
>> if (r)
>> goto err_modeset;
>>
>> +r = drm_irq_install(dev, dev->pdev->irq);
>> +if (r)
>> +goto err_modeset;
>> +
>> return 0;
>>
>> err_modeset:
>> @@ -207,8 +217,31 @@ void mgag200_driver_unload(struct drm_device *dev)
>>
>> if (mdev == NULL)
>> return;
>> +drm_irq_uninstall(dev);
>> mgag200_modeset_fini(mdev);
>> drm_mode_config_cleanup(dev);
>> mgag200_mm_fini(mdev);
>> dev->dev_private = NULL;
>> }
>> +
>> +irqreturn_t mgag200_irq_handler(int irq, void *arg)
>> +{
>> +struct drm_device *dev = arg;
>> +struct mga_device *mdev = dev->dev_private;
>> +struct drm_crtc *crtc;
>> +u32 status, iclear;
>> +
>> +status = RREG32(0x1e14);
>> +
>> +if (status & 0x0020) { /* test */
>> +drm_for_each_crtc(crtc, dev) {
>> +drm_crtc_handle_vblank(crtc);
>> +}
>
> A bit odd way to write that but as long this driver doesn't support
> crtc2 it should be fine.
>
>> +iclear = RREG32(0x1e18);
>> +iclear |= 0x0020; /* set */
>> +WREG32(0x1e18, iclear);
>> +return IRQ_HANDLED;
>> +}
>> +
>> +return IRQ_NO
Re: [PATCH 1/2] drm/fb-helper: Synchronize dirty worker with vblank
Den 10.09.2019 15.51, skrev Thomas Zimmermann:
> Hi
>
> Am 10.09.19 um 15:34 schrieb Noralf Trønnes:
>>
>>
>> Den 10.09.2019 14.48, skrev Thomas Zimmermann:
>>> Hi
>>>
>>> Am 10.09.19 um 13:52 schrieb Gerd Hoffmann:
On Mon, Sep 09, 2019 at 04:06:32PM +0200, Thomas Zimmermann wrote:
> Before updating the display from the console's shadow buffer, the dirty
> worker now waits for vblank. This allows several screen updates to pile
> up and acts as a rate limiter.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_fb_helper.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c
> index a7ba5b4902d6..017e2f6bd1b9 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -402,8 +402,20 @@ static void drm_fb_helper_dirty_work(struct
> work_struct *work)
> dirty_work);
> struct drm_clip_rect *clip = &helper->dirty_clip;
> struct drm_clip_rect clip_copy;
> + struct drm_crtc *crtc;
> unsigned long flags;
> void *vaddr;
> + int ret;
> +
> + /* rate-limit update frequency */
> + mutex_lock(&helper->lock);
> + crtc = helper->client.modesets[0].crtc;
> + ret = drm_crtc_vblank_get(crtc);
> + if (!ret) {
> + drm_crtc_wait_one_vblank(crtc);
> + drm_crtc_vblank_put(crtc);
> + }
> + mutex_unlock(&helper->lock);
Hmm, not sure it is the best plan to sleep for a while in the worker,
especially while holding the lock.
What does the lock protect against here? Accessing
>>>
>>> This lock is hold by the fbdev code during mode-setting operations (but
>>> not drawing operations). So *crtc might be gone if we don't hold it here.
>>>
helper->client.modesets? If so then you can unlock before going to
sleep in drm_crtc_wait_one_vblank() I think.
>>>
>>> I looked, but I cannot find any code that protects crtc while vblank is
>>> active. I'd rather not change the current code until someone can clarify.
>>>
>>
>> The client->modesets array and the crtc struct member are invariant over
>> the lifetime of client (drm_client_modeset_create()). No need to take a
>> lock for access. See drm_client_modeset_release() for the things that
>> _can_ change and needs protection (protected by client->modeset_mutex).
>
> Thanks for the reply. So we don't need the lock? But why does
> drm_fb_helper_ioctl() take it? ioctl exclusiveness?
>
Because of drm_master_internal_acquire() it's necessary to take the lock
first. That's the locking rules of drm_fb_helper. First take the fb
helper lock, then the dev->master_mutex. We stay away if there's a
userspace master and if there's none, we prevent userspace from becoming
master while we do stuff.
But looking at drm_fb_helper_ioctl() now, I see that it's not strictly
necessary to do this since all this function can do is vblank wait and
that's fine even if userspace is master. The locking was necessary
before I refactored and moved stuff to drm_client, because at that time
the modeset array was deleted and recreated when probing connectors.
But it doesn't hurt either in case more functionality is added to the
ioctl. One wouldn't think that would ever happen, since fbdev is going
away soon, but still we keep polishing it ;)
Noralf.
>> I don't see a problem with sleeping in the worker though, but I might
>> miss out on something. AFAICS changes will just pile up in >dirty_clip
>> and the worker will be scheduled for a new run to happen when it's done
>> with the current update.
>
> Yes, that's the intention of the patch. We hope to reduce the number of
> memcpys by handling several of them at once.
>
> Best regards
> Thomas
>
>>
>> Noralf.
>>
>>> Best regards
>>> Thomas
>>>
cheers,
Gerd
>>>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/mgag200: Add vblank support
On Mon, Sep 09, 2019 at 04:06:33PM +0200, Thomas Zimmermann wrote:
> Support for vblank requires VSYNC to signal an interrupt, which is broken
> on Matrox chipsets. The workaround that is used here and in other free
> Matrox drivers is to program to the value of and
> enable the VLINE interrupt. This triggers an interrupt at the same time
> when VSYNC begins.
>
> VLINE uses separate registers for enabling and clearing pending interrupts.
> No extra syncronization between irq handler and the rest of the driver is
> required.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/mgag200/mgag200_drv.c | 1 +
> drivers/gpu/drm/mgag200/mgag200_drv.h | 1 +
> drivers/gpu/drm/mgag200/mgag200_main.c | 33
> drivers/gpu/drm/mgag200/mgag200_mode.c | 42 +++---
> 4 files changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c
> b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index 4f9df3b93598..cff265973154 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -67,6 +67,7 @@ static struct drm_driver driver = {
> .driver_features = DRIVER_GEM | DRIVER_MODESET,
> .load = mgag200_driver_load,
> .unload = mgag200_driver_unload,
> + .irq_handler = mgag200_irq_handler,
> .fops = &mgag200_driver_fops,
> .name = DRIVER_NAME,
> .desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 1c93f8dc08c7..88cf256d135f 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -195,6 +195,7 @@ void mgag200_modeset_fini(struct mga_device *mdev);
> /* mgag200_main.c */
> int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
> void mgag200_driver_unload(struct drm_device *dev);
> +irqreturn_t mgag200_irq_handler(int irq, void *arg);
>
> /* mgag200_i2c.c */
> struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c
> b/drivers/gpu/drm/mgag200/mgag200_main.c
> index a9773334dedf..5941607796e8 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
> @@ -10,7 +10,9 @@
>
> #include
> #include
> +#include
> #include
> +#include
>
> #include "mgag200_drv.h"
>
> @@ -186,10 +188,18 @@ int mgag200_driver_load(struct drm_device *dev,
> unsigned long flags)
> }
> mdev->cursor.pixels_current = NULL;
>
> + r = drm_vblank_init(dev, 1);
> + if (r)
> + goto err_modeset;
> +
> r = drm_fbdev_generic_setup(mdev->dev, 0);
> if (r)
> goto err_modeset;
>
> + r = drm_irq_install(dev, dev->pdev->irq);
> + if (r)
> + goto err_modeset;
> +
> return 0;
>
> err_modeset:
> @@ -207,8 +217,31 @@ void mgag200_driver_unload(struct drm_device *dev)
>
> if (mdev == NULL)
> return;
> + drm_irq_uninstall(dev);
> mgag200_modeset_fini(mdev);
> drm_mode_config_cleanup(dev);
> mgag200_mm_fini(mdev);
> dev->dev_private = NULL;
> }
> +
> +irqreturn_t mgag200_irq_handler(int irq, void *arg)
> +{
> + struct drm_device *dev = arg;
> + struct mga_device *mdev = dev->dev_private;
> + struct drm_crtc *crtc;
> + u32 status, iclear;
> +
> + status = RREG32(0x1e14);
> +
> + if (status & 0x0020) { /* test */
On further inspection this looks a bit iffy. IIRC the
status bits aren't masked out by IEN, so I would do
something like this:
irq_handler() {
status = read(STATUS) & read(IEN):
if (status == 0)
return IRQ_NONE;
write(ICLEAR, status);
drm_handle_vblank();
return IRQ_HANDLED;
}
vblank_enable() {
write(ICLEAR, vline);
write(IEN, vline);
}
vblank_disable() {
write(IEN, 0);
}
Otherwise you maybe try to handle a stale interrupt when
enabling the vblank irq, and also could accidentally
handle bogus interrupts from other devices on the same irq
line.
And probably clear IEN on before registering the irq handler
for good measure.
> + drm_for_each_crtc(crtc, dev) {
> + drm_crtc_handle_vblank(crtc);
> + }
> + iclear = RREG32(0x1e18);
> + iclear |= 0x0020; /* set */
> + WREG32(0x1e18, iclear);
> + return IRQ_HANDLED;
> + }
> +
> + return IRQ_NONE;
> +};
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #33 from Pierre-Eric Pelloux-Prayer --- Created attachment 145323 --> https://bugs.freedesktop.org/attachment.cgi?id=145323&action=edit wip patch You can give a try to the attached kernel patch which hopefully could prevent some sdma timeouts. I'm still testing it but the more testers the better :) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111591] [radeonsi/Navi] The Bard's Tale IV causes a GPU hang
https://bugs.freedesktop.org/show_bug.cgi?id=111591 --- Comment #5 from Pierre-Eric Pelloux-Prayer --- (In reply to Shmerl from comment #4) > Uploaded the trace here (should be valid for 30 days): > https://ufile.io/kvf9t1eu > > Sorry for huge size, there is an unskippable cutscene in the beginning. > > Compressed with pixz, so should be decompressible using all CPU cores > (compatible with regular single threaded decompressing xz as well). The attached trace doesn't cause a GPU hang here. Does it hang on your machine? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111591] [radeonsi/Navi] The Bard's Tale IV causes a GPU hang
https://bugs.freedesktop.org/show_bug.cgi?id=111591 --- Comment #6 from Shmerl --- (In reply to Pierre-Eric Pelloux-Prayer from comment #5) > > > The attached trace doesn't cause a GPU hang here. > > Does it hang on your machine? I recorded it until the freeze happened, and then had to do Alt+SysRq+REISUB to reboot. So that's the resulting file. I'll try replaying the trace to see what happens. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109628] WARNING at dcn10_hw_sequencer.c:868 dcn10_verify_allow_pstate_change_high()
https://bugs.freedesktop.org/show_bug.cgi?id=109628 --- Comment #16 from Johannes Hirte --- Seems to be fixed now. Tested with v5.3-rc8-7-g3120b9a6a3f7 and resume from S3 Works without problems. Interestingly even v5.3-rc6-129-g265381004994 works now without additional patches. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] iommu: add support for drivers that manage iommu explicitly
On Tue, Sep 10, 2019 at 1:14 AM Joerg Roedel wrote: > > On Fri, Sep 06, 2019 at 02:44:01PM -0700, Rob Clark wrote: > > @@ -674,7 +674,7 @@ int iommu_group_add_device(struct iommu_group *group, > > struct device *dev) > > > > mutex_lock(&group->mutex); > > list_add_tail(&device->list, &group->devices); > > - if (group->domain) > > + if (group->domain && !(dev->driver && > > dev->driver->driver_manages_iommu)) > > Hmm, this code usually runs at enumeration time when no driver is > attached to the device. Actually it would be pretty dangerous when this > code runs while a driver is attached to the device. How does that change > make things work for you? > I was seeing this get called via the path driver_probe_device() -> platform_dma_configure() -> of_dma_configure() -> of_iommu_configure() -> iommu_probe_device() -> ... The only cases I was seeing where dev->driver is NULL where a few places that drivers call of_dma_configure() on their own sub-devices. But maybe there are some other paths that I did not notice? BR, -R
[PATCH 1/2] dt-bindings: display: Add xylon logicvc bindings documentation
The Xylon LogiCVC is a display controller implemented as programmable
logic in Xilinx FPGAs.
Signed-off-by: Paul Kocialkowski
---
.../bindings/display/xylon,logicvc.txt| 188 ++
1 file changed, 188 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/xylon,logicvc.txt
diff --git a/Documentation/devicetree/bindings/display/xylon,logicvc.txt
b/Documentation/devicetree/bindings/display/xylon,logicvc.txt
new file mode 100644
index ..eb4b1553888a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/xylon,logicvc.txt
@@ -0,0 +1,188 @@
+Xylon LogiCVC display controller
+
+The Xylon LogiCVC is a display controller that supports multiple layers.
+It is usually implemented as programmable logic and was optimized for use
+with Xilinx Zynq-7000 SoCs and Xilinx FPGAs.
+
+Because the controller is intended for use in a FPGA, most of the configuration
+of the controller takes place at logic configuration bitstream synthesis time.
+As a result, many of the device-tree bindings are meant to reflect the
+synthesis configuration. These do not allow configuring the controller
+differently than synthesis configuration.
+
+Layers are declared in the "layers" sub-node and have dedicated configuration.
+In version 3 of the controller, each layer has fixed memory offset and address
+starting from the video memory base address for its framebuffer. With version
4,
+framebuffers are configured with a direct memory address instead.
+
+Matching synthesis parameters are provided when applicable.
+
+Required properties:
+- compatible: Should be one of:
+ "xylon,logicvc-3.02.a-display"
+ "xylon,logicvc-4.01.a-display"
+- reg: Physical base address and size for the controller registers.
+- clocks: List of phandle and clock-specifier pairs, one for each entry
+ in 'clock-names'
+- clock-names: List of clock names that should at least contain:
+ - "vclk": The VCLK video clock input.
+- interrupts: The interrupt to use for VBLANK signaling.
+- xylon,display-interface: Display interface in use, should be one of:
+ - "lvds-4bits": 4-bit LVDS interface (C_DISPLAY_INTERFACE == 4).
+- xylon,display-colorspace: Display output colorspace in use, should be one of:
+ - "rgb": RGB colorspace (C_DISPLAY_COLOR_SPACE == 0).
+- xylon,display-depth: Display output depth in use (C_PIXEL_DATA_WIDTH).
+- xylon,row-stride: Fixed number of pixels in a framebuffer row (C_ROW_STRIDE).
+- xylon,layers-count: The number of available layers (C_NUM_OF_LAYERS).
+
+Optional properties:
+- memory-region: phandle to a node describing memory, as specified in:
+ Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+- clock-names: List of clock names that can optionally contain:
+ - "vclk2": The VCLK2 doubled-rate video clock input.
+ - "lvdsclk": The LVDS clock.
+ - "lvdsclkn": The LVDS clock inverted.
+- xylon,syscon: Syscon phandle representing the logicvc instance.
+- xylon,dithering: Dithering module is enabled (C_XCOLOR).
+- xylon,background-layer: The last layer is used to display a black background
+ (C_USE_BACKGROUND). It must still be registered.
+- xylon,layers-configurable: Configuration of layers' size, position and offset
+ is enabled (C_USE_SIZE_POSITION).
+
+Required sub-nodes:
+- layers: The description of the display controller layers, containing layer
+ sub-nodes that each describe a registered layer.
+- ports: The LogiCVC connection to an encoder input port. The connection
+ is modeled using the OF graph bindings, as specified in:
+ Documentation/devicetree/bindings/graph.txt
+
+Required layer properties:
+- reg: Layer index (from front to back, starting at 0).
+- xylon,layer-depth: Layer depth in use (C_LAYER_0_DATA_WIDTH).
+- xylon,layer-colorspace: Layer colorspace in use, should be one of:
+ - "rgb": RGB colorspace (C_LAYER_*_TYPE == 0).
+- xylon,layer-alpha-mode: Alpha mode for the layer, should be one of:
+ - "layer": Alpha is configured layer-wide (C_LAYER_*_ALPHA_MODE == 0).
+ - "pixel": Alpha is configured per-pixel (C_LAYER_*_ALPHA_MODE == 1).
+- xylon,layer-base-offset: offset in number of lines (C_LAYER_*_OFFSET)
starting
+ from the video RAM base (C_VMEM_BASEADDR), only for version 3.
+- xylon,layer-buffer-offset: offset in number of lines (C_BUFFER_*_OFFSET)
+ starting from the layer base offset for the second buffer used in
+ double-buffering.
+
+Optional layer properties:
+- xylon,layer-primary: Layer should be registered as a primary plane (exactly
+ one is required).
+
+Example:
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ logicvc_cma: cma@1f80 {
+ compatible = "shared-dma-pool";
+ size = <0x80>;
+ alloc-ranges = <0x1f80 0x80>;
+ reusable;
+ };
+ };
+
+ logicvc: logicvc@43c0 {
+ compatib
[PATCH 0/2] drm: LogiCVC display controller support
This series introduces support for the LogiCVC display controller. The controller is a bit unusual since it is usually loaded as programmable logic on Xilinx FPGAs or Zynq-7000 SoCs. More details are presented on the main commit for the driver. More information about the controller is available on the dedicated web page: https://www.logicbricks.com/Products/logiCVC-ML.aspx Cheers! Paul Kocialkowski (2): dt-bindings: display: Add xylon logicvc bindings documentation drm: Add support for the LogiCVC display controller .../bindings/display/xylon,logicvc.txt| 188 ++ drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/logicvc/Kconfig | 8 + drivers/gpu/drm/logicvc/Makefile | 4 + drivers/gpu/drm/logicvc/logicvc_crtc.c| 272 drivers/gpu/drm/logicvc/logicvc_crtc.h| 25 + drivers/gpu/drm/logicvc/logicvc_drm.c | 457 ++ drivers/gpu/drm/logicvc/logicvc_drm.h | 60 ++ drivers/gpu/drm/logicvc/logicvc_interface.c | 235 +++ drivers/gpu/drm/logicvc/logicvc_interface.h | 32 + drivers/gpu/drm/logicvc/logicvc_layer.c | 594 ++ drivers/gpu/drm/logicvc/logicvc_layer.h | 65 ++ drivers/gpu/drm/logicvc/logicvc_mode.c| 103 +++ drivers/gpu/drm/logicvc/logicvc_mode.h| 15 + drivers/gpu/drm/logicvc/logicvc_of.c | 204 ++ drivers/gpu/drm/logicvc/logicvc_of.h | 28 + drivers/gpu/drm/logicvc/logicvc_regs.h| 88 +++ 18 files changed, 2381 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/xylon,logicvc.txt create mode 100644 drivers/gpu/drm/logicvc/Kconfig create mode 100644 drivers/gpu/drm/logicvc/Makefile create mode 100644 drivers/gpu/drm/logicvc/logicvc_crtc.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_crtc.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_drm.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_drm.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_interface.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_interface.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_layer.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_layer.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_mode.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_mode.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_of.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_of.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_regs.h -- 2.23.0
[PATCH 2/2] drm: Add support for the LogiCVC display controller
Introduces a driver for the LogiCVC display controller, a programmable logic controller optimized for use in Xilinx Zynq-7000 SoCs and other Xilinx FPGAs. The controller is mostly configured at logic synthesis time so only a subset of configuration is left for the driver to handle. The following features are implemented and tested: - LVDS 4-bit interface; - RGB565 pixel formats; - Multiple layers and hardware composition; - Layer-wide alpha mode; The following features are implemented but untested: - Other RGB pixel formats; - Layer framebuffer configuration for version 4; - Lowest-layer used as background color; - Per-pixel alpha mode. The following features are not implemented: - YUV pixel formats; - DVI, LVDS 3-bit, ITU656 and camera link interfaces; - External parallel input for layer; - Color-keying; - LUT-based alpha modes. Additional implementation-specific notes: - Panels are only enabled after the first page flip to avoid flashing a white screen. - Depth used in context of the LogiCVC driver only counts color components to match the definition of the synthesis parameters. Support is implemented for both version 3 and 4 of the controller. With version 3, framebuffers are stored in a dedicated contiguous memory area, with a base address hardcoded for each layer. This requires using a dedicated CMA pool registered at the base address and tweaking a few offset-related registers to try to use any buffer allocated from the pool. This is done on a best-effort basis to have the hardware cope with the DRM framebuffer allocation model and there is no guarantee that each buffer allocated by GEM CMA can be used for any layer. In particular, buffers allocated below the base address for a layer are guaranteed not to be configurable for that layer. See the implementation of logicvc_layer_buffer_find_setup for specifics. Version 4 allows configuring each buffer address directly, which guarantees that any buffer can be configured. Signed-off-by: Paul Kocialkowski --- drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/logicvc/Kconfig | 8 + drivers/gpu/drm/logicvc/Makefile| 4 + drivers/gpu/drm/logicvc/logicvc_crtc.c | 272 + drivers/gpu/drm/logicvc/logicvc_crtc.h | 25 + drivers/gpu/drm/logicvc/logicvc_drm.c | 457 +++ drivers/gpu/drm/logicvc/logicvc_drm.h | 60 ++ drivers/gpu/drm/logicvc/logicvc_interface.c | 235 drivers/gpu/drm/logicvc/logicvc_interface.h | 32 ++ drivers/gpu/drm/logicvc/logicvc_layer.c | 594 drivers/gpu/drm/logicvc/logicvc_layer.h | 65 +++ drivers/gpu/drm/logicvc/logicvc_mode.c | 103 drivers/gpu/drm/logicvc/logicvc_mode.h | 15 + drivers/gpu/drm/logicvc/logicvc_of.c| 204 +++ drivers/gpu/drm/logicvc/logicvc_of.h| 28 + drivers/gpu/drm/logicvc/logicvc_regs.h | 88 +++ 17 files changed, 2193 insertions(+) create mode 100644 drivers/gpu/drm/logicvc/Kconfig create mode 100644 drivers/gpu/drm/logicvc/Makefile create mode 100644 drivers/gpu/drm/logicvc/logicvc_crtc.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_crtc.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_drm.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_drm.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_interface.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_interface.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_layer.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_layer.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_mode.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_mode.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_of.c create mode 100644 drivers/gpu/drm/logicvc/logicvc_of.h create mode 100644 drivers/gpu/drm/logicvc/logicvc_regs.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index cd11a3bde19c..f55fc59ebad6 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -338,6 +338,8 @@ source "drivers/gpu/drm/arc/Kconfig" source "drivers/gpu/drm/hisilicon/Kconfig" +source "drivers/gpu/drm/logicvc/Kconfig" + source "drivers/gpu/drm/mediatek/Kconfig" source "drivers/gpu/drm/zte/Kconfig" diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 545c61d6528b..2b5f21e20a26 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -104,6 +104,7 @@ obj-$(CONFIG_DRM_STM) += stm/ obj-$(CONFIG_DRM_STI) += sti/ obj-$(CONFIG_DRM_IMX) += imx/ obj-$(CONFIG_DRM_INGENIC) += ingenic/ +obj-$(CONFIG_DRM_LOGICVC) += logicvc/ obj-$(CONFIG_DRM_MEDIATEK) += mediatek/ obj-$(CONFIG_DRM_MESON)+= meson/ obj-y += i2c/ diff --git a/drivers/gpu/drm/logicvc/Kconfig b/drivers/gpu/drm/logicvc/Kconfig new file mode 100644 index ..34dacabbb49a --- /dev/null +++ b/drivers/gpu/drm/logicvc/Kconfig @@ -0,0 +1,8 @@ +config DRM_LOGICVC + tristate "LogiC
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #34 from Mathieu Belanger --- Patch applied Removed nodma from the /etc/environment Will reboot at lunch time, Usually my IDEs trigger the crash. Will see how it go. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204805] New: BUG: kernel NULL pointer dereference, address: 0000000000000200
https://bugzilla.kernel.org/show_bug.cgi?id=204805 Bug ID: 204805 Summary: BUG: kernel NULL pointer dereference, address: 0200 Product: Drivers Version: 2.5 Kernel Version: 5.2.12 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: ilkka.pr...@gmail.com Regression: No Created attachment 284909 --> https://bugzilla.kernel.org/attachment.cgi?id=284909&action=edit kern.log A bug that looks like caused by amdgpu_drm. Hard lockup of system as-in not responding to any inputs, after forced reset the attached bug information in kern.log -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
On Tue, Sep 10, 2019 at 9:21 AM Ilia Mirkin wrote: > > On Tue, Sep 10, 2019 at 3:34 AM Mun, Gwan-gyeong > wrote: > > > > On Sat, 2019-09-07 at 21:43 -0400, Ilia Mirkin wrote: > > > On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong > > > wrote: > > > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote: > > > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä > > > > > wrote: > > > > > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote: > > > > > > > > -Original Message- > > > > > > > > From: Ilia Mirkin > > > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM > > > > > > > > To: Mun, Gwan-gyeong > > > > > > > > Cc: Intel Graphics Development < > > > > > > > > intel-...@lists.freedesktop.org > > > > > > > > > ; Shankar, Uma > > > > > > > > ; dri-devel < > > > > > > > > dri-devel@lists.freedesktop.org> > > > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace > > > > > > > > property > > > > > > > > > > > > > > > > So how would this work with a DP++ connector? Should it > > > > > > > > list > > > > > > > > the HDMI or DP > > > > > > > > properties? Or do we need a custom property checker which > > > > > > > > is > > > > > > > > aware of what is > > > > > > > > currently plugged in to validate the values? > > > > > > > > > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving > > > > > > > DP > > > > > > > or HDMI (with a passive dongle). > > > > > > > Based on the type of sink detected, we should expose DP or > > > > > > > HDMI > > > > > > > colorspaces to userspace. > > > > > > > > > > > > For i915 DP connector always drives DP mode, HDMI connector > > > > > > always > > > > > > drives > > > > > > HDMI mode, even when the physical connector is DP++. > > > > > > > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and > > > > > amdgpu > > > > > create 1 connector (not sure about other drivers) for a single > > > > > physical DP++ socket. Since we supply the list of valid values at > > > > > the > > > > > time of creating the connector, we can't know at that point > > > > > whether > > > > > in > > > > > the future a HDMI or DP will be plugged into it. > > > > > > > > > > -ilia > > > > Ilia, does it mean that the drm_connector type is > > > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode? > > > > > > That is correct. The connector type is "DisplayPort" in such a case. > > > > > > Cheers, > > > > > > -ilia > > > > For now drm_mode_create_colorspace_property() is only used for i915. > > IMHO, when other drivers ( nouveau, radeon, and amdgpu ) are ready for > > using of drm_mode_create_colorspace_property(), > > what about do we add a variable which can identify DP++ and DP to > > drm_connector? > > And when the drivers (nouveau, radeon, and amdgpu) detect the current > > protocol, the drivers will set the variable. > > I've been working on adding this to nouveau. > > Can/should such properties be added/removed at "runtime", rather than > at connector creation time? Either way, the function > drm_mode_create_colorspace_property as proposed would not be reusable, > since it looks at the connector type, which will always be > "DisplayPort" in such cases. Summary of conversation Ville and I had on IRC: - DP++ connectors to provide a single combined list of options for colorspace - set_property hook will check against currently plugged in thing, and reject incorrect values - in the case where someone sets e.g. an HDMI value for an HDMI-plugged-in thing, and then unplugs, and then plugs in a DP screen, the modeset should continue to succeed but use a default colorspace value. I think there was a bit of contention on that last point. Open to opinions, but we should try to avoid putting undue burden on esp non-atomic userspace. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v4 00/16] new cgroup controller for gpu/drm subsystem
Hello, Michal. On Tue, Sep 10, 2019 at 01:54:48PM +0200, Michal Hocko wrote: > > So, while it'd great to have shrinkers in the longer term, it's not a > > strict requirement to be accounted in memcg. It already accounts a > > lot of memory which isn't reclaimable (a lot of slabs and socket > > buffer). > > Yeah, having a shrinker is preferred but the memory should better be > reclaimable in some form. If not by any other means then at least bound > to a user process context so that it goes away with a task being killed > by the OOM killer. If that is not the case then we cannot really charge > it because then the memcg controller is of no use. We can tolerate it to > some degree if the amount of memory charged like that is negligible to > the overall size. But from the discussion it seems that these buffers > are really large. Yeah, oom kills should be able to reduce the usage; however, please note that tmpfs, among other things, can already escape this restriction and we can have cgroups which are over max and empty. It's obviously not ideal but the system doesn't melt down from it either. Thanks. -- tejun ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/2] iommu: handle drivers that manage iommu directly
On 06/09/2019 22:44, Rob Clark wrote: From: Rob Clark One of the challenges we have to enable the aarch64 laptops upstream is dealing with the fact that the bootloader enables the display and takes the corresponding SMMU context-bank out of BYPASS. Unfortunately, currently, the IOMMU framework attaches a DMA (or potentially an IDENTITY) domain before the driver is probed and has a chance to intervene and shutdown scanout. Which makes things go horribly wrong. Nope, things already went horribly wrong in arm_smmu_device_reset() - sure, sometimes for some configurations it might *seem* like they didn't and that you can fudge the context bank state at arm's length from core code later, but the truth is that impl->cfg_probe is your last chance to guarantee that any necessary SMMU state is preserved. The remainder of the problem involves reworking default domain allocation such that we can converge on what iommu_request_dm_for_dev() currently does but without the momentary attachment to a translation domain to cause hiccups. That's starting here: https://lore.kernel.org/linux-iommu/cover.1566353521.git.sai.praneeth.prak...@intel.com/ But in this case, drm/msm is already directly managing it's IOMMUs directly, the DMA API attached iommu_domain simply gets in the way. This series adds a way that a driver can indicate to drivers/iommu that it does not wish to have an DMA managed iommu_domain attached. This way, drm/msm can shut down scanout cleanly before attaching it's own iommu_domain. NOTE that to get things working with arm-smmu on the aarch64 laptops, you also need a patchset[1] from Bjorn Andersson to inherit SMMU config at boot, when it is already enabled. [1] https://www.spinics.net/lists/arm-kernel/msg732246.html NOTE that in discussion of previous revisions, RMRR came up. This is not really a replacement for RMRR (nor does RMRR really provide any more information than we already get from EFI GOP, or DT in the simplefb case). I also don't see how RMRR could help w/ SMMU handover of CB/SMR config (Bjorn's patchset[1]) without defining new tables. The point of RMRR-like-things is that they identify not just the memory region but also the specific device accessing them, which means the IOMMU driver knows up-front which IDs etc. it must be careful not to disrupt. Obviously for SMMU that *would* be some new table (designed to encompass everything relevant) since literal RMRRs are specifically an Intel VT-d thing. This perhaps doesn't solve the more general case of bootloader enabled display for drivers that actually want to use DMA API managed IOMMU. But it does also happen to avoid a related problem with GPU, caused by the DMA domain claiming the context bank that the GPU firmware expects to use. Careful bringing that up again, or I really will rework the context bank allocator to avoid this default domain problem entirely... ;) Robin. And it avoids spurious TLB invalidation coming from the unused DMA domain. So IMHO this is a useful and necessary change. Rob Clark (2): iommu: add support for drivers that manage iommu explicitly drm/msm: mark devices where iommu is managed by driver drivers/gpu/drm/msm/adreno/adreno_device.c | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 1 + drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 1 + drivers/gpu/drm/msm/msm_drv.c | 1 + drivers/iommu/iommu.c | 2 +- drivers/iommu/of_iommu.c | 3 +++ include/linux/device.h | 3 ++- 7 files changed, 10 insertions(+), 2 deletions(-)
Re: [PATCH v2 1/2] iommu: add support for drivers that manage iommu explicitly
On 10/09/2019 16:34, Rob Clark wrote: On Tue, Sep 10, 2019 at 1:14 AM Joerg Roedel wrote: On Fri, Sep 06, 2019 at 02:44:01PM -0700, Rob Clark wrote: @@ -674,7 +674,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev) mutex_lock(&group->mutex); list_add_tail(&device->list, &group->devices); - if (group->domain) + if (group->domain && !(dev->driver && dev->driver->driver_manages_iommu)) Hmm, this code usually runs at enumeration time when no driver is attached to the device. Actually it would be pretty dangerous when this code runs while a driver is attached to the device. How does that change make things work for you? I was seeing this get called via the path driver_probe_device() -> platform_dma_configure() -> of_dma_configure() -> of_iommu_configure() -> iommu_probe_device() -> ... The only cases I was seeing where dev->driver is NULL where a few places that drivers call of_dma_configure() on their own sub-devices. But maybe there are some other paths that I did not notice? For the of_iommu flow, it very much depends on your DT layout and driver probe order as to whether you catch the "proper" call from iommu_bus_notifier()/iommu_bus_init() or the late "replay" from of_iommu_configure(). I wouldn't make any assumptions of consistency. Robin.
Re: [PATCH v3 0/2] iommu: handle drivers that manage iommu directly
On Tue, Sep 10, 2019 at 9:34 AM Robin Murphy wrote: > > On 06/09/2019 22:44, Rob Clark wrote: > > From: Rob Clark > > > > One of the challenges we have to enable the aarch64 laptops upstream > > is dealing with the fact that the bootloader enables the display and > > takes the corresponding SMMU context-bank out of BYPASS. Unfortunately, > > currently, the IOMMU framework attaches a DMA (or potentially an > > IDENTITY) domain before the driver is probed and has a chance to > > intervene and shutdown scanout. Which makes things go horribly wrong. > > Nope, things already went horribly wrong in arm_smmu_device_reset() - > sure, sometimes for some configurations it might *seem* like they didn't > and that you can fudge the context bank state at arm's length from core > code later, but the truth is that impl->cfg_probe is your last chance to > guarantee that any necessary SMMU state is preserved. cfg_probe is where Bjorn's patch is preserving the SMMU state. So I think that should ensure device_reset() preserves the configuration.. or at least if something is missing, that seems fixable. > The remainder of the problem involves reworking default domain > allocation such that we can converge on what iommu_request_dm_for_dev() > currently does but without the momentary attachment to a translation > domain to cause hiccups. That's starting here: > > https://lore.kernel.org/linux-iommu/cover.1566353521.git.sai.praneeth.prak...@intel.com/ I suppose if the stream-match state and bootloader chosen context bank is preserved, then keeping it direct-mapped would avoid things starting to fault before display driver is probed. That plus some solution for GPU default domain would narrow the scope of what I need to just avoiding getting iommu dma_ops installed. > > But in this case, drm/msm is already directly managing it's IOMMUs > > directly, the DMA API attached iommu_domain simply gets in the way. > > This series adds a way that a driver can indicate to drivers/iommu > > that it does not wish to have an DMA managed iommu_domain attached. > > This way, drm/msm can shut down scanout cleanly before attaching it's > > own iommu_domain. > > > > NOTE that to get things working with arm-smmu on the aarch64 laptops, > > you also need a patchset[1] from Bjorn Andersson to inherit SMMU config > > at boot, when it is already enabled. > > > > [1] https://www.spinics.net/lists/arm-kernel/msg732246.html > > > > NOTE that in discussion of previous revisions, RMRR came up. This is > > not really a replacement for RMRR (nor does RMRR really provide any > > more information than we already get from EFI GOP, or DT in the > > simplefb case). I also don't see how RMRR could help w/ SMMU handover > > of CB/SMR config (Bjorn's patchset[1]) without defining new tables. > > The point of RMRR-like-things is that they identify not just the memory > region but also the specific device accessing them, which means the > IOMMU driver knows up-front which IDs etc. it must be careful not to > disrupt. Obviously for SMMU that *would* be some new table (designed to > encompass everything relevant) since literal RMRRs are specifically an > Intel VT-d thing. Perhaps I'm not looking in the right place, but the extent of what I could find about RMRR tables was: https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/DmaRemappingReportingTable.h#L122 I couldn't really see how that specifies the device. But entirely possible that I'm not seeing the whole picture. I am a bit curious about how windows handles this, since they must have the same problem on these laptops. > > This perhaps doesn't solve the more general case of bootloader enabled > > display for drivers that actually want to use DMA API managed IOMMU. > > But it does also happen to avoid a related problem with GPU, caused by > > the DMA domain claiming the context bank that the GPU firmware expects > > to use. > > Careful bringing that up again, or I really will rework the context bank > allocator to avoid this default domain problem entirely... ;) That doesn't seem like a bad outcome ;-) BR, -R > Robin. > > > And it avoids spurious TLB invalidation coming from the unused > > DMA domain. So IMHO this is a useful and necessary change. > > > > Rob Clark (2): > >iommu: add support for drivers that manage iommu explicitly > >drm/msm: mark devices where iommu is managed by driver > > > > drivers/gpu/drm/msm/adreno/adreno_device.c | 1 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 1 + > > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 1 + > > drivers/gpu/drm/msm/msm_drv.c | 1 + > > drivers/iommu/iommu.c | 2 +- > > drivers/iommu/of_iommu.c | 3 +++ > > include/linux/device.h | 3 ++- > > 7 files changed, 10 insertions(+), 2 deletions(-) > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.
Re: [PATCH RFC v4 00/16] new cgroup controller for gpu/drm subsystem
On Tue 10-09-19 09:03:29, Tejun Heo wrote: > Hello, Michal. > > On Tue, Sep 10, 2019 at 01:54:48PM +0200, Michal Hocko wrote: > > > So, while it'd great to have shrinkers in the longer term, it's not a > > > strict requirement to be accounted in memcg. It already accounts a > > > lot of memory which isn't reclaimable (a lot of slabs and socket > > > buffer). > > > > Yeah, having a shrinker is preferred but the memory should better be > > reclaimable in some form. If not by any other means then at least bound > > to a user process context so that it goes away with a task being killed > > by the OOM killer. If that is not the case then we cannot really charge > > it because then the memcg controller is of no use. We can tolerate it to > > some degree if the amount of memory charged like that is negligible to > > the overall size. But from the discussion it seems that these buffers > > are really large. > > Yeah, oom kills should be able to reduce the usage; however, please > note that tmpfs, among other things, can already escape this > restriction and we can have cgroups which are over max and empty. > It's obviously not ideal but the system doesn't melt down from it > either. Right, and that is a reason why an access to tmpfs should be restricted when containing a workload by memcg. My understanding of this particular feature is that memcg should be the primary containment method and that's why I brought this up. -- Michal Hocko SUSE Labs ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: include: fix W=1 warnings in struct drm_dsc_config
On Tue, Sep 10, 2019 at 12:58:24PM +, Harry Wentland wrote:
> +Manasi, Gaurav
>
> On 2019-09-09 9:52 a.m., Benjamin Gaignard wrote:
> > Change scale_increment_interval and nfl_bpg_offset fields to
> > u32 to avoid W=1 warnings because we are testing them against
> > 65535.
> >
> > Signed-off-by: Benjamin Gaignard
> > ---
> > include/drm/drm_dsc.h | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> > index 887954cbfc60..e495024e901c 100644
> > --- a/include/drm/drm_dsc.h
> > +++ b/include/drm/drm_dsc.h
> > @@ -207,11 +207,13 @@ struct drm_dsc_config {
> > * Number of group times between incrementing the scale factor value
> > * used at the beginning of a slice.
> > */
> > - u16 scale_increment_interval;
> > + u32 scale_increment_interval;
>
> The DSC spec defines both as u16. I think the check in drm_dsc.c is
> useless and should be dropped.
>
I agree with Harry here, all these variables should match the number of bits
in the spec, increasing them to u32 allows more values which violates the
DSC spec.
It should stay u16
Manasi
> Harry
>
> > +
> > /**
> > * @nfl_bpg_offset: Non first line BPG offset to be used
> > */
> > - u16 nfl_bpg_offset;
> > +
> > + u32 nfl_bpg_offset;
> > /**
> > * @slice_bpg_offset: BPG offset used to enforce slice bit
> > */
> >
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #35 from Alexandr Kára --- Created attachment 145324 --> https://bugs.freedesktop.org/attachment.cgi?id=145324&action=edit UMR dump of registers on a GPU lockup Sending dmesg output + UMR registers dump of both sdma0 and sdma1 for a lockup in Rise of the Tomb Raider. [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out or interrupted! [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx_0.0.0 timeout, signaled seq=104586, emitted seq=104588 [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process RiseOfTheTombRa pid 8457 thread RiseOfTheT:cs0 pid 8463 [drm] GPU recovery disabled. The lockup is reproducible and only affects the GPU - it's still fine to ssh to the machine and it's otherwise working fine. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] backlight: lm3630a: fix module aliases
Devicetree aliases are missing, so that module autoloading
does not work properly.
Signed-off-by: Andreas Kemnade
---
drivers/video/backlight/lm3630a_bl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/lm3630a_bl.c
b/drivers/video/backlight/lm3630a_bl.c
index 3b45a1733198..9d67c07db2f2 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -617,12 +617,14 @@ static const struct i2c_device_id lm3630a_id[] = {
{}
};
+MODULE_DEVICE_TABLE(i2c, lm3630a_id);
+
static const struct of_device_id lm3630a_match_table[] = {
{ .compatible = "ti,lm3630a", },
{ },
};
-MODULE_DEVICE_TABLE(i2c, lm3630a_id);
+MODULE_DEVICE_TABLE(of, lm3630a_match_table);
static struct i2c_driver lm3630a_i2c_driver = {
.driver = {
--
2.11.0
Re: [PATCH v6 1/4] leds: Add of_led_get() and led_put()
Hi Jean,
Please rebase your patch set onto linux-leds.git [0] -
we've had some updates in the meantime and now your
set doesn't apply.
Best regards,
Jacek Anaszewski
On 9/10/19 3:27 PM, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen
>
> This patch adds basic support for a kernel driver to get a LED device.
> This will be used by the led-backlight driver.
>
> Only OF version is implemented for now, and the behavior is similar to
> PWM's of_pwm_get() and pwm_put().
>
> Signed-off-by: Tomi Valkeinen
> Signed-off-by: Jean-Jacques Hiblot
> Acked-by: Pavel Machek
> ---
> drivers/leds/led-class.c | 50
> include/linux/leds.h | 4
> 2 files changed, 54 insertions(+)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 159c6caa374b..d4d22f4b5052 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include "leds.h"
>
> static struct class *leds_class;
> @@ -213,6 +214,55 @@ static int led_resume(struct device *dev)
>
> static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume);
>
> +static int led_match_led_node(struct device *led_dev, const void *data)
> +{
> + return led_dev->of_node == data;
> +}
> +
> +/**
> + * of_led_get() - request a LED device via the LED framework
> + * @np: device node to get the LED device from
> + * @index: the index of the LED
> + *
> + * Returns the LED device parsed from the phandle specified in the "leds"
> + * property of a device tree node or a negative error-code on failure.
> + */
> +struct led_classdev *of_led_get(struct device_node *np, int index)
> +{
> + struct device *led_dev;
> + struct led_classdev *led_cdev;
> + struct device_node *led_node;
> +
> + led_node = of_parse_phandle(np, "leds", index);
> + if (!led_node)
> + return ERR_PTR(-ENOENT);
> +
> + led_dev = class_find_device(leds_class, NULL, led_node,
> + led_match_led_node);
> + of_node_put(led_node);
> +
> + if (!led_dev)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + led_cdev = dev_get_drvdata(led_dev);
> +
> + if (!try_module_get(led_cdev->dev->parent->driver->owner))
> + return ERR_PTR(-ENODEV);
> +
> + return led_cdev;
> +}
> +EXPORT_SYMBOL_GPL(of_led_get);
> +
> +/**
> + * led_put() - release a LED device
> + * @led_cdev: LED device
> + */
> +void led_put(struct led_classdev *led_cdev)
> +{
> + module_put(led_cdev->dev->parent->driver->owner);
> +}
> +EXPORT_SYMBOL_GPL(led_put);
> +
> static int match_name(struct device *dev, const void *data)
> {
> if (!dev_name(dev))
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index bee8e3f8..0a71c7cdd191 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -19,6 +19,7 @@
>
> struct device;
> struct led_pattern;
> +struct device_node;
> /*
> * LED Core
> */
> @@ -145,6 +146,9 @@ extern void devm_led_classdev_unregister(struct device
> *parent,
> extern void led_classdev_suspend(struct led_classdev *led_cdev);
> extern void led_classdev_resume(struct led_classdev *led_cdev);
>
> +extern struct led_classdev *of_led_get(struct device_node *np, int index);
> +extern void led_put(struct led_classdev *led_cdev);
> +
> /**
> * led_blink_set - set blinking with software fallback
> * @led_cdev: the LED to start blinking
>
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git/log/?h=for-next
[Bug 111633] amdgpu driver crash with kernel NULL pointer dereference
https://bugs.freedesktop.org/show_bug.cgi?id=111633 Bug ID: 111633 Summary: amdgpu driver crash with kernel NULL pointer dereference Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: not set Priority: not set Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: vakevk+freedesktopbugzi...@gmail.com I am running on arch linux: Linux arch 5.2.13-arch1-1-ARCH #1 SMP PREEMPT Fri Sep 6 17:52:33 UTC 2019 x86_64 GNU/Linux I am running wayland via sway. My gpu is a Radeon RX Vega 64. While in my sway session the image on my screen froze but audio from a video continued to play. I was able to ssh in from a different machine and found this message with dmesg: BUG: kernel NULL pointer dereference, address: 0360 #PF: supervisor write access in kernel mode #PF: error_code(0x0002) - not-present page PGD 0 P4D 0 Oops: 0002 [#1] PREEMPT SMP PTI CPU: 1 PID: 12766 Comm: kworker/u16:0 Not tainted 5.2.11-arch1-1-ARCH #1 Hardware name: ASUS All Series/Z87-PLUS, BIOS 2103 08/15/2014 Workqueue: events_unbound commit_work [drm_kms_helper] RIP: 0010:dc_stream_retain+0x5/0x20 [amdgpu] Call Trace: dc_resource_state_copy_construct+0xa0/0xf0 [amdgpu] dc_commit_updates_for_stream+0xa63/0xc20 [amdgpu] amdgpu_dm_atomic_commit_tail+0xabe/0x19a0 [amdgpu] ? commit_tail+0x3c/0x70 [drm_kms_helper] commit_tail+0x3c/0x70 [drm_kms_helper] process_one_work+0x1d1/0x3e0 worker_thread+0x4a/0x3d0 kthread+0xfb/0x130 ? process_one_work+0x3e0/0x3e0 ? kthread_park+0x80/0x80 ret_from_fork+0x35/0x40 Modules linked in: snd_seq_dummy snd_seq tun nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c nf_tables_set cfg80211 nf_tables nfnetlink 8021q garp mrp stp llc intel_rapl nls_iso8859_1 nls_cp437 vfat fat snd_hda_codec_realtek snd_hda_codec_generic fuse ledtrig_audio ofpart snd_hda_codec_hdmi cmdlinepart btusb intel_spi_platform snd_hda_intel btrtl x86_pkg_temp_thermal intel_spi btbcm intel_powerclamp spi_nor btintel eeepc_wmi snd_usb_audio coretemp snd_hda_codec uvcvideo asus_wmi bluetooth snd_usbmidi_lib iTCO_wdt kvm_intel snd_hda_core videobuf2_vmalloc mei_hdcp mtd iTCO_vendor_support mxm_wmi wmi_bmof sparse_keymap snd_hwdep snd_rawmidi snd_seq_device videobuf2_memops snd_pcm videobuf2_v4l2 snd_timer videobuf2_common snd videodev kvm irqbypass input_leds ecdh_generic intel_cstate mousedev rfkill intel_uncore mei_me joydev cdc_acm media ecc e1000e intel_rapl_perf mei soundcore pcc_cpufreq i2c_i801 lpc_ich pcspkr wmi evdev mac_hid ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 hid_generic usbhid hid dm_crypt dm_mod sd_mod crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel ahci libahci aesni_intel libata aes_x86_64 xhci_pci crypto_simd cryptd glue_helper xhci_hcd scsi_mod ehci_pci ehci_hcd amdgpu gpu_sched i2c_algo_bit ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm agpgart CR2: 0360 ---[ end trace 08eaa2e1d713ba4d ]--- At this point I tried killing the sway process but did not succeed even with `kill -9`. Not even `sudo reboot` completed despite killing the ssh session. I had to hard reset the machine. Potentially related is that since roughly a week I have been experiencing intermittent screen freezes from time to time that would resolve themselves after about 10 seconds with a message like this in dmesg: drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:47:crtc-0] flip_done timed out [drm:amdgpu_dm_atomic_check [amdgpu]] *ERROR* [CRTC:47:crtc-0] hw_done or flip_done timed out -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] backlight: lm3630a: add an enable gpio for the HWEN pin
On Tue, 10 Sep 2019 11:21:56 +0100 Daniel Thompson wrote: [...] > > > Is this needed? > > > > > > This is a remove path, not a power management path, and we have no idea > > > what the original status of the pin was anyway? > > > > > > > Looking at Ishdn on page 5 of the datasheet, switching it off everytime > > possible seems not needed. We would need to call chip_init() everytime > > we enable the gpio or live with default values. > > Therefore I did decide to not put it into any power management path. > > But switching it on and not switching it off feels so unbalanced. > > Either the power consumed by the controller when strings aren't lit up > matters, in which case the driver should implement proper power > management or it doesn't matter and changing the pin state isn't needed. > > I'm happy with either of the above but this looks like a third way, > where eager users could hack in a bit of extra power management by > forcing drivers to unbind. > I think I will take the simple way. I am quite sure that the power consumption with HWEN on and leds off does not matter. If someone later comes up and finds out that I misread the datasheet, things are prepared to be improved. At least the hardware can be properly described in the devicetree. Regards, Andreas pgpvJw6rr7Ej0.pgp Description: OpenPGP digital signature
[Bug 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #32 from rol...@rptd.ch --- Is there a way I can verify the rebuilding has picked up the patch file? I rebuild and started the application but the result is the same. Now I'm not sure if the patch is not working or not picked up properly. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111482] Sapphire Pulse RX 5700 XT power consumption
https://bugs.freedesktop.org/show_bug.cgi?id=111482 --- Comment #17 from Robert --- Thanks Andrew! I played around a little bit with the refresh rates. Between 40-60Hz there is no difference in idle power consumption. The mem clock stays at 875Mhz and can't be changed. The best refresh rate with 8W idle power consumption I could get was at 39Hz: cvt12 5120 1440 39 -b xrandr --output DisplayPort-2 --newmode "5120x1440_39.00_rb2" 297.51 5120 5128 5160 5200 1440 1453 1461 1467 +hsync -vsync xrandr --output DisplayPort-2 --addmode DisplayPort-2 "5120x1440_39.00_rb2" xrandr --output DisplayPort-2 --mode "5120x1440_39.00_rb2" This causes the mem clock to go up to 625Mhz at first but it can be switched back to 100Mhz with echo "0" > /sys/class/drm/card0/device/pp_dpm_mclk Regarding my statement when using 30Hz in the last comment: """ It basically works but moving windows a little bit faster or moving the mouse pointer around looks "interesting". """ For this "flickering" that I saw and which was quite annoying I found a workaround :-) It looked like something didn't refresh fast enough. So I thought playing around with some frequencies would be a good idea... And the mem clock was the obvious one to start with. So I was setting the mem clock to 500Mhz with echo "1" > /sys/class/drm/card0/device/pp_dpm_mclk Then the "flickering" went away :-) But of course that brought idle power consumption to 24W. So just for fun I switched back to 100Mhz with echo "0" > /sys/class/drm/card0/device/pp_dpm_mclk Funny enough the "flickering" stayed away :-))) So for now after I start KDE plasma I enter Konsole and execute echo "1" > /sys/class/drm/card0/device/pp_dpm_mclk echo "0" > /sys/class/drm/card0/device/pp_dpm_mclk and be happy :D One final observation: I tried out kernel 5.3-rc8. With that kernel there is no way to reduce idle power consumption. It stays at 34W regardless what you do. But with this tag https://cgit.freedesktop.org/~agd5f/linux/tag/?h=drm-next-5.4-2019-08-30 (which basically is kernel 5.3-rc3 with the Navi10 patches for kernel 5.4 - if I got it right ;-) ) idle power consumption is as expected. So my whole issue basically comes down to this: If you have a resolution of 5120x1440 and a refresh rate of > 39Hz your idle power consumption stays at max and there is (at least until now) nothing you can do about it. So if I had used a lower resolution or a smaller screen I wouldn't have had an issue at all ;-) S... happens :D But anyways: Thanks so much for your help and also to Ilia! I'm now happy with my setup so far. It would be very interesting if there is really some kind of a cap with 5120x1444@39Hz or if this this "only" a firmware problem, a driver problem, a config error or something completely different. Maybe we'll find out in our next lives :D -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
Factor function in preparation to generating scatterlist prior to locking.
Signed-off-by: David Riley
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 981ee16e3ee9..bf5a4a50b002 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -299,17 +299,6 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct
virtio_gpu_device *vgdev,
return ret;
}
-static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
-{
- int rc;
-
- spin_lock(&vgdev->ctrlq.qlock);
- rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
- spin_unlock(&vgdev->ctrlq.qlock);
- return rc;
-}
-
static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf,
struct virtio_gpu_ctrl_hdr *hdr,
@@ -335,13 +324,19 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
goto again;
}
- if (fence)
+ if (hdr && fence)
virtio_gpu_fence_emit(vgdev, hdr, fence);
rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
spin_unlock(&vgdev->ctrlq.qlock);
return rc;
}
+static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_vbuffer *vbuf)
+{
+ return virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL, NULL);
+}
+
static int virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf)
{
--
2.23.0.162.g0b9fbb3734-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/2] drm/virtio: Use vmalloc for command buffer allocations.
Userspace requested command buffer allocations could be too large
to make as a contiguous allocation. Use vmalloc if necessary to
satisfy those allocations.
Signed-off-by: David Riley
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_vq.c| 79 +++---
2 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index ac60be9b5c19..a8732a8af766 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -195,7 +195,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
*dev, void *data,
if (ret)
goto out_free;
- buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
+ buf = vmemdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
if (IS_ERR(buf)) {
ret = PTR_ERR(buf);
goto out_unresv;
@@ -230,7 +230,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
*dev, void *data,
return 0;
out_memdup:
- kfree(buf);
+ kvfree(buf);
out_unresv:
ttm_eu_backoff_reservation(&ticket, &validate_list);
out_free:
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index bf5a4a50b002..76cf2b9d5d1d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -154,7 +154,7 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
{
if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
kfree(vbuf->resp_buf);
- kfree(vbuf->data_buf);
+ kvfree(vbuf->data_buf);
kmem_cache_free(vgdev->vbufs, vbuf);
}
@@ -251,13 +251,54 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct
*work)
wake_up(&vgdev->cursorq.ack_queue);
}
+/* Create sg_table from a vmalloc'd buffer. */
+static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
+{
+ int ret, s, i;
+ struct sg_table *sgt;
+ struct scatterlist *sg;
+ struct page *pg;
+
+ if (WARN_ON(!PAGE_ALIGNED(data)))
+ return NULL;
+
+ sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
+ if (!sgt)
+ return NULL;
+
+ *sg_ents = DIV_ROUND_UP(size, PAGE_SIZE);
+ ret = sg_alloc_table(sgt, *sg_ents, GFP_KERNEL);
+ if (ret) {
+ kfree(sgt);
+ return NULL;
+ }
+
+ for_each_sg(sgt->sgl, sg, *sg_ents, i) {
+ pg = vmalloc_to_page(data);
+ if (!pg) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ return NULL;
+ }
+
+ s = min_t(int, PAGE_SIZE, size);
+ sg_set_page(sg, pg, s, 0);
+
+ size -= s;
+ data += s;
+ }
+
+ return sgt;
+}
+
static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
+ struct virtio_gpu_vbuffer *vbuf,
+ struct scatterlist *vout)
__releases(&vgdev->ctrlq.qlock)
__acquires(&vgdev->ctrlq.qlock)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
- struct scatterlist *sgs[3], vcmd, vout, vresp;
+ struct scatterlist *sgs[3], vcmd, vresp;
int outcnt = 0, incnt = 0;
int ret;
@@ -268,9 +309,8 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct
virtio_gpu_device *vgdev,
sgs[outcnt + incnt] = &vcmd;
outcnt++;
- if (vbuf->data_size) {
- sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
- sgs[outcnt + incnt] = &vout;
+ if (vout) {
+ sgs[outcnt + incnt] = vout;
outcnt++;
}
@@ -305,7 +345,24 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_fence *fence)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
+ struct scatterlist *vout = NULL, sg;
+ struct sg_table *sgt = NULL;
int rc;
+ int outcnt = 0;
+
+ if (vbuf->data_size) {
+ if (is_vmalloc_addr(vbuf->data_buf)) {
+ sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size,
+&outcnt);
+ if (!sgt)
+ return -ENOMEM;
+ vout = sgt->sgl;
+ } else {
+ sg_init_one(&sg, vbuf->data_buf, vbuf->data_size);
+ vout = &sg;
+ outcnt = 1;
+ }
+ }
again:
spin_lock(&vgdev->ctrlq.qlock);
@@ -318,7 +375,7 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
* to
[Bug 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #33 from Matt Turner --- It will show the patch being applied before configuration. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #36 from Sebastian Meyer --- Created attachment 145326 --> https://bugs.freedesktop.org/attachment.cgi?id=145326&action=edit umr output of sdma0/sdma1 after RotTR freeze Applied the provided WIP patch to linux-mainline 5.3-rc8 and started RotTR again in order to trigger a system freeze. This time I also got a ring sdma0 and sdma1 timeout: [ 632.175837] [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out or interrupted! [ 632.175973] [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out or interrupted! [ 637.299049] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx_0.0.0 timeout, signaled seq=313757, emitted seq=313759 [ 637.299110] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process RiseOfTheTombRa pid 2584 thread RiseOfTheT:cs0 pid 2590 [ 637.299111] [drm] GPU recovery disabled. [ 646.468871] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled seq=278259, emitted seq=278263 [ 646.468961] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma1 timeout, signaled seq=21116, emitted seq=21119 [ 646.469052] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process pid 0 thread pid 0 [ 646.469141] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process plasmashell pid 989 thread plasmashel:cs0 pid 1155 [ 646.469141] [drm] GPU recovery disabled. [ 646.469142] [drm] GPU recovery disabled. Stdout of `umr -R sdma0` and `umr -R sdma1` is attached to this post, however, I also got a couple of stderr messages like "[ERROR]: No valid mapping for 3@80023f00" which I didn't include in the output. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/2] backlight_lm3630a: add enable_gpios property
To be able to handle the HWEN pin of the lm3630a, add an enable gpio to the driver and a property. Tested on Kobo Clara HD. Changes in v2: simplification and reordering Andreas Kemnade (2): dt-bindings: backlight: lm3630a: add enable_gpios backlight: lm3630a: add an enable gpio for the HWEN pin .../bindings/leds/backlight/lm3630a-backlight.yaml | 5 + drivers/video/backlight/lm3630a_bl.c | 10 ++ 2 files changed, 15 insertions(+) -- 2.20.1
[PATCH v2 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
add enable-gpios to describe HWEN pin
Signed-off-by: Andreas Kemnade
---
changes in v2: add example
.../bindings/leds/backlight/lm3630a-backlight.yaml | 5 +
1 file changed, 5 insertions(+)
diff --git
a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
index dc129d9a329e..1fa83feffe16 100644
--- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
@@ -29,6 +29,10 @@ properties:
'#size-cells':
const: 0
+ enable-gpios:
+description: GPIO to use to enable/disable the backlight (HWEN pin).
+maxItems: 1
+
required:
- compatible
- reg
@@ -92,6 +96,7 @@ examples:
i2c {
#address-cells = <1>;
#size-cells = <0>;
+enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
led-controller@38 {
compatible = "ti,lm3630a";
--
2.20.1
[PATCH v2 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
For now just enable it in the probe function to allow i2c
access. Disabling also means resetting the register values
to default and according to the datasheet does not give
power savings
Tested on Kobo Clara HD.
Signed-off-by: Andreas Kemnade
---
changes in v2:
- simplification
- correct gpio direction initialisation
drivers/video/backlight/lm3630a_bl.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/video/backlight/lm3630a_bl.c
b/drivers/video/backlight/lm3630a_bl.c
index 8f84f3684f04..9d0639d4202d 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -12,6 +12,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -48,6 +50,7 @@ struct lm3630a_chip {
struct lm3630a_platform_data *pdata;
struct backlight_device *bleda;
struct backlight_device *bledb;
+ struct gpio_desc *enable_gpio;
struct regmap *regmap;
struct pwm_device *pwmd;
};
@@ -535,6 +538,13 @@ static int lm3630a_probe(struct i2c_client *client,
}
pchip->pdata = pdata;
+ pchip->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(pchip->enable_gpio)) {
+ rval = PTR_ERR(pchip->enable_gpio);
+ return rval;
+ }
+
/* chip initialize */
rval = lm3630a_chip_init(pchip);
if (rval < 0) {
--
2.20.1
Re: [Nouveau] [Intel-gfx] [PATCH v6 08/17] drm/ttm: use gem vma_node
On Sat, Sep 07, 2019 at 09:58:46PM -0400, Ilia Mirkin wrote:
> On Wed, Aug 21, 2019 at 7:55 AM Thierry Reding
> wrote:
> >
> > On Wed, Aug 21, 2019 at 04:33:58PM +1000, Ben Skeggs wrote:
> > > On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann wrote:
> > > >
> > > > Hi,
> > > >
> > > > > > Changing the order doesn't look hard. Patch attached (untested,
> > > > > > have no
> > > > > > test hardware). But maybe I missed some detail ...
> > > > >
> > > > > I came up with something very similar by splitting up nouveau_bo_new()
> > > > > into allocation and initialization steps, so that when necessary the
> > > > > GEM
> > > > > object can be initialized in between. I think that's slightly more
> > > > > flexible and easier to understand than a boolean flag.
> > > >
> > > > Yes, that should work too.
> > > >
> > > > Acked-by: Gerd Hoffmann
> > > Acked-by: Ben Skeggs
> >
> > Thanks guys, applied to drm-misc-next.
>
> Hi Thierry,
>
> Initial investigations suggest that this commit currently in drm-next
>
> commit 019cbd4a4feb3aa3a917d78e7110e3011bbff6d5
> Author: Thierry Reding
> Date: Wed Aug 14 11:00:48 2019 +0200
>
> drm/nouveau: Initialize GEM object before TTM object
>
> breaks nouveau userspace which tries to allocate GEM objects with a
> non-page-aligned size. Previously nouveau_gem_new would just call
> nouveau_bo_init which would call nouveau_bo_fixup_align before
> initializing the GEM object. With this change, it is done after. What
> do you think -- OK to just move that bit of logic into the new
> nouveau_bo_alloc() (and make size/align be pointers so that they can
> be fixed up?)
Hi Ilia,
sorry, got side-tracked earlier and forgot to send this out. I'll turn
this into a proper patch, but if you manage to find the time to test
this while I work out the userspace issues that are preventing me from
testing this more thoroughly, that'd be great.
Thierry
--- >8 ---
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index e918b437af17..7d5ede756711 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -186,8 +186,8 @@ nouveau_bo_fixup_align(struct nouveau_bo *nvbo, u32 flags,
}
struct nouveau_bo *
-nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode,
-u32 tile_flags)
+nouveau_bo_alloc(struct nouveau_cli *cli, u64 *size, int *align, u32 flags,
+u32 tile_mode, u32 tile_flags)
{
struct nouveau_drm *drm = cli->drm;
struct nouveau_bo *nvbo;
@@ -195,8 +195,8 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32
flags, u32 tile_mode,
struct nvif_vmm *vmm = cli->svm.cli ? &cli->svm.vmm : &cli->vmm.vmm;
int i, pi = -1;
- if (!size) {
- NV_WARN(drm, "skipped size %016llx\n", size);
+ if (!*size) {
+ NV_WARN(drm, "skipped size %016llx\n", *size);
return ERR_PTR(-EINVAL);
}
@@ -266,7 +266,7 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32
flags, u32 tile_mode,
pi = i;
/* Stop once the buffer is larger than the current page size. */
- if (size >= 1ULL << vmm->page[i].shift)
+ if (*size >= 1ULL << vmm->page[i].shift)
break;
}
@@ -281,6 +281,8 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32
flags, u32 tile_mode,
}
nvbo->page = vmm->page[pi].shift;
+ nouveau_bo_fixup_align(nvbo, flags, align, size);
+
return nvbo;
}
@@ -292,12 +294,11 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int
align, u32 flags,
size_t acc_size;
int ret;
- acc_size = ttm_bo_dma_acc_size(nvbo->bo.bdev, size, sizeof(*nvbo));
-
- nouveau_bo_fixup_align(nvbo, flags, &align, &size);
nvbo->bo.mem.num_pages = size >> PAGE_SHIFT;
nouveau_bo_placement_set(nvbo, flags, 0);
+ acc_size = ttm_bo_dma_acc_size(nvbo->bo.bdev, size, sizeof(*nvbo));
+
ret = ttm_bo_init(nvbo->bo.bdev, &nvbo->bo, size, type,
&nvbo->placement, align >> PAGE_SHIFT, false,
acc_size, sg, robj, nouveau_bo_del_ttm);
@@ -318,7 +319,8 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align,
struct nouveau_bo *nvbo;
int ret;
- nvbo = nouveau_bo_alloc(cli, size, flags, tile_mode, tile_flags);
+ nvbo = nouveau_bo_alloc(cli, &size, &align, flags, tile_mode,
+ tile_flags);
if (IS_ERR(nvbo))
return PTR_ERR(nvbo);
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h
b/drivers/gpu/drm/nouveau/nouveau_bo.h
index 62930d834fba..38f9d8350963 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
@@ -71,8 +71,8 @@ nouveau_bo_ref(struct nouveau_bo *ref, struct nouveau_bo
**pnvbo)
extern struct ttm_bo_driver nouveau_bo_driver;
void n
[Bug 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #34 from rol...@rptd.ch --- I do not see anything mentioned there. I've wget the file into /etc/portage/patches/media-libs/mesa/ directory, which did not exist. Anything else I need to do to get Gentoo to pick up the patch? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111591] [radeonsi/Navi] The Bard's Tale IV causes a GPU hang
https://bugs.freedesktop.org/show_bug.cgi?id=111591 --- Comment #7 from Shmerl --- Just replayed the trace - it ended before the buggy part. Something must have interrupted it, or may be it has a size cap? I'll try making it again. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111591] [radeonsi/Navi] The Bard's Tale IV causes a GPU hang
https://bugs.freedesktop.org/show_bug.cgi?id=111591 --- Comment #8 from Shmerl --- Here is a new trace: https://uploadfiles.io/9uykx7nh Now it's catching the hang moment. Replaying it doesn't hang the GPU though, just produces some errors in the trace output. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111634] HD4550 lockup when starting blender
https://bugs.freedesktop.org/show_bug.cgi?id=111634 Bug ID: 111634 Summary: HD4550 lockup when starting blender Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: not set Priority: not set Component: DRM/Radeon Assignee: dri-devel@lists.freedesktop.org Reporter: petrcve...@gmail.com Created attachment 145327 --> https://bugs.freedesktop.org/attachment.cgi?id=145327&action=edit dmesg log from loading driver to blender start Hello, Starting the blender 2.81 on x86-64, xorg 1.20.5 and kernel 5.3.0-rc6-next will cause a GPU lockup error (freeze, and after timeout GPU softreset). Similar observation was made on mipsel debian sid (default blender and xorg version, 5.3.0-rc6-next kernel) and mipsel buildroot (git snapshot blender, last stable xorg, 5.3.0-rc6-next kernel). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111635] HD4550 fails to complete tests (radeon.test=2)
https://bugs.freedesktop.org/show_bug.cgi?id=111635 Bug ID: 111635 Summary: HD4550 fails to complete tests (radeon.test=2) Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: not set Priority: not set Component: DRM/Radeon Assignee: dri-devel@lists.freedesktop.org Reporter: petrcve...@gmail.com Created attachment 145328 --> https://bugs.freedesktop.org/attachment.cgi?id=145328&action=edit dmesg for radeon.test=2 Booting with parameter radeon.test=2 will cause the radeon driver's tests to fail. Observed kernel versions: 5.3.0-rc6-next and 4.19.71 Observed on mipsel too, kernel 5.3.0-rc6-next There is a possibility this is related to bug 111634 (same card, same kernel ...) https://bugs.freedesktop.org/show_bug.cgi?id=111634 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110199] [amdgpu] Screen flickering when using a 75Hz monitor paired with an RX 480 GPU
https://bugs.freedesktop.org/show_bug.cgi?id=110199 --- Comment #14 from guimarcalsi...@gmail.com --- I'm also affected by this bug. I have an MSI Rx 570 and my monitor is an Acer SA230. I'm using the display through HDMI since it doesn't have DP. In Linux Manjaro with kernel 4.19 the problem persists no matter what I try, Freesync enabled or disabled through the monitor, but now I'm typing on a Live USB of OpenMandriva Linux and after login, I went to KDE display settings and changed my framerate from 75 to 60 and back to 75 again, and the problem went away. The kernel is 5.1.9-desktop-1omv4000. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #35 from Matt Turner --- (In reply to rol...@rptd.ch from comment #34) > I do not see anything mentioned there. I've wget the file into > /etc/portage/patches/media-libs/mesa/ directory, which did not exist. > Anything else I need to do to get Gentoo to pick up the patch? p50-ethernet ~ # mkdir -p /etc/portage/patches/media-libs/mesa/ p50-ethernet ~ # cd /etc/portage/patches/media-libs/mesa/ p50-ethernet /etc/portage/patches/media-libs/mesa # wget 'https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1852.patch' --2019-09-10 17:09:04-- https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1852.patch Resolving gitlab.freedesktop.org... 35.185.111.185 Connecting to gitlab.freedesktop.org|35.185.111.185|:443... connected. HTTP request sent, awaiting response... 200 OK Length: unspecified [text/plain] Saving to: ‘1852.patch’ 1852.patch [ <=> ] 13.33K --.-KB/sin 0.08s 2019-09-10 17:09:04 (163 KB/s) - ‘1852.patch’ saved [13646] p50-ethernet /etc/portage/patches/media-libs/mesa # ebuild /var/db/repos/gentoo/media-libs/mesa/mesa-19.2.0_rc2.ebuild prepare * mesa-19.2.0-rc2.tar.xz BLAKE2B SHA512 size ;-) ... [ ok ] >>> Unpacking source... >>> Unpacking mesa-19.2.0-rc2.tar.xz to >>> /var/tmp/portage/media-libs/mesa-19.2.0_rc2/work >>> Source unpacked in /var/tmp/portage/media-libs/mesa-19.2.0_rc2/work >>> Preparing source in >>> /var/tmp/portage/media-libs/mesa-19.2.0_rc2/work/mesa-19.2.0-rc2 ... * Applying 1852.patch ... [ ok ] * User patches applied. >>> Source prepared. "Applying 1852.patch" is what you should see. There's nothing else you need to do other than putting the patch into that directory. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
