Re: [RFC v3 2/3] virtio: Introduce Vdmabuf driver
Hi Vivek, Am 10.02.21 um 05:47 schrieb Kasireddy, Vivek: Hi Gerd, -Original Message- From: Gerd Hoffmann Sent: Tuesday, February 09, 2021 12:45 AM To: Kasireddy, Vivek Cc: Daniel Vetter ; virtualizat...@lists.linux-foundation.org; dri- de...@lists.freedesktop.org; Vetter, Daniel ; daniel.vet...@ffwll.ch; Kim, Dongwon ; sumit.sem...@linaro.org; christian.koe...@amd.com; linux-me...@vger.kernel.org Subject: Re: [RFC v3 2/3] virtio: Introduce Vdmabuf driver Hi, Nack, this doesn't work on dma-buf. And it'll blow up at runtime when you enable the very recently merged CONFIG_DMABUF_DEBUG (would be good to test with that, just to make sure). [Kasireddy, Vivek] Although, I have not tested it yet but it looks like this will throw a wrench in our solution as we use sg_next to iterate over all the struct page * and get their PFNs. I wonder if there is any other clean way to get the PFNs of all the pages associated with a dmabuf. Well, there is no guarantee that dma-buf backing storage actually has struct page ... [Kasireddy, Vivek] What if I do mmap() on the fd followed by mlock() or mmap() followed by get_user_pages()? If it still fails, would ioremapping the device memory and poking at the backing storage be an option? Or, if I bind the passthrough'd GPU device to vfio-pci and tap into the memory region associated with the device memory, can it be made to work? get_user_pages() is not allowed on mmaped DMA-bufs in the first place. Daniel is currently adding code to make sure that this is never ever used. And, I noticed that for PFNs that do not have valid struct page associated with it, KVM does a memremap() to access/map them. Is this an option? No, even for system memory which has a valid struct page touching it when it is part of a DMA-buf is illegal since the reference count and mapping fields in struct page might be used for something different. Keep in mind that struct page is a heavily overloaded structure for different use cases. You can't just use it for a different use case than what the owner of the page has intended it. Regards, Christian. [Kasireddy, Vivek] To exclude such cases, would it not be OK to limit the scope of this solution (Vdmabuf) to make it clear that the dma-buf has to live in Guest RAM? Or, are there any ways to pin the dma-buf pages in Guest RAM to make this solution work? At that point it becomes (i915) driver-specific. If you go that route it doesn't look that useful to use dma-bufs in the first place ... [Kasireddy, Vivek] I prefer not to make this driver specific if possible. IIUC, Virtio GPU is used to present a virtual GPU to the Guest and all the rendering commands are captured and forwarded to the Host GPU via Virtio. You don't have to use the rendering pipeline. You can let the i915 gpu render into a dma-buf shared with virtio-gpu, then use virtio-gpu only for buffer sharing with the host. [Kasireddy, Vivek] Is this the most viable path forward? I am not sure how complex or feasible it would be but I'll look into it. Also, not using the rendering capabilities of virtio-gpu and turning it into a sharing only device means there would be a giant mode switch with a lot of if() conditions sprinkled across. Are you OK with that? Thanks, Vivek take care, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: use getter/setter functions
On Tue, 09 Feb 2021, Julia Lawall wrote:
> Use getter and setter functions, for platform_device structures and a
> spi_device structure.
>
> Signed-off-by: Julia Lawall
>
> ---
> drivers/video/backlight/qcom-wled.c |2
> +-
This patch is fine.
Could you please split it out and submit it separately though please.
> drivers/video/fbdev/amifb.c |4
> ++--
> drivers/video/fbdev/da8xx-fb.c |4
> ++--
> drivers/video/fbdev/imxfb.c |2
> +-
> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c |6
> +++---
> drivers/video/fbdev/omap2/omapfb/dss/dpi.c |4
> ++--
> drivers/video/fbdev/omap2/omapfb/dss/dsi.c |4
> ++--
> drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c |2
> +-
> drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c |2
> +-
> drivers/video/fbdev/xilinxfb.c |2
> +-
> 10 files changed, 16 insertions(+), 16 deletions(-)
...]
> diff --git a/drivers/video/backlight/qcom-wled.c
> b/drivers/video/backlight/qcom-wled.c
> index 3bc7800eb0a9..091f07e7c145 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
> @@ -1692,7 +1692,7 @@ static int wled_probe(struct platform_device *pdev)
>
> static int wled_remove(struct platform_device *pdev)
> {
> - struct wled *wled = dev_get_drvdata(&pdev->dev);
> + struct wled *wled = platform_get_drvdata(pdev);
>
> mutex_destroy(&wled->lock);
> cancel_delayed_work_sync(&wled->ovp_work);
For my own reference (apply this as-is to your sign-off block):
Acked-for-Backlight-by: Lee Jones
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 1/2] drm/dp: Make number of AUX retries configurable by display drivers.
The number of AUX retries specified in the DP specs is 7. Currently, to make
Dell 4k monitors happier, the number of retries are 32.
i915 also retries 5 times (intel_dp_aux_xfer) which means in the case of AUX
timeout we actually retries 32 * 5 = 160 times.
So making the number of aux retires a variable to allow for fine tuning and
optimization of aux timing.
Signed-off-by: Khaled Almahallawy
---
drivers/gpu/drm/drm_dp_helper.c | 10 +++---
include/drm/drm_dp_helper.h | 1 +
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index eedbb48815b7..8fdf57b4a06c 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -249,13 +249,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
request,
mutex_lock(&aux->hw_mutex);
- /*
-* The specification doesn't give any recommendation on how often to
-* retry native transactions. We used to retry 7 times like for
-* aux i2c transactions but real world devices this wasn't
-* sufficient, bump to 32 which makes Dell 4k monitors happier.
-*/
- for (retry = 0; retry < 32; retry++) {
+ for (retry = 0; retry < aux->num_retries; retry++) {
if (ret != 0 && ret != -ETIMEDOUT) {
usleep_range(AUX_RETRY_INTERVAL,
AUX_RETRY_INTERVAL + 100);
@@ -1744,6 +1738,8 @@ void drm_dp_aux_init(struct drm_dp_aux *aux)
aux->ddc.retries = 3;
aux->ddc.lock_ops = &drm_dp_i2c_lock_ops;
+ /*Still making the Dell 4k monitors happier*/
+ aux->num_retries = 32;
}
EXPORT_SYMBOL(drm_dp_aux_init);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index edffd1dcca3e..16cbfc8f5e66 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1876,6 +1876,7 @@ struct drm_dp_aux {
struct mutex hw_mutex;
struct work_struct crc_work;
u8 crc_count;
+ int num_retries;
ssize_t (*transfer)(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg);
/**
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 2/2] drm/i915/dp: Retry AUX requests 7 times.
Given that intel_dp_aux_xfer retries 5 times, so configure drm_dpcd_access to retry only 7 times, which means the max number of retries for i915 = 7 * 5 = 35 times. Signed-off-by: Khaled Almahallawy --- drivers/gpu/drm/i915/display/intel_dp_aux.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c index eaebf123310a..73d711a94000 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c @@ -688,5 +688,7 @@ void intel_dp_aux_init(struct intel_dp *intel_dp) encoder->base.name); intel_dp->aux.transfer = intel_dp_aux_transfer; + /* Follow DP specs*/ + intel_dp->aux.num_retries = 7; cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE); } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] drm/dp: Make number of AUX retries configurable by display drivers.
Hi
Am 10.02.21 um 09:33 schrieb Khaled Almahallawy:
The number of AUX retries specified in the DP specs is 7. Currently, to make
Dell 4k monitors happier, the number of retries are 32.
i915 also retries 5 times (intel_dp_aux_xfer) which means in the case of AUX
timeout we actually retries 32 * 5 = 160 times.
So making the number of aux retires a variable to allow for fine tuning and
optimization of aux timing.
Signed-off-by: Khaled Almahallawy
---
drivers/gpu/drm/drm_dp_helper.c | 10 +++---
include/drm/drm_dp_helper.h | 1 +
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index eedbb48815b7..8fdf57b4a06c 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -249,13 +249,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
request,
mutex_lock(&aux->hw_mutex);
- /*
-* The specification doesn't give any recommendation on how often to
-* retry native transactions. We used to retry 7 times like for
-* aux i2c transactions but real world devices this wasn't
-* sufficient, bump to 32 which makes Dell 4k monitors happier.
-*/
- for (retry = 0; retry < 32; retry++) {
+ for (retry = 0; retry < aux->num_retries; retry++) {
if (ret != 0 && ret != -ETIMEDOUT) {
usleep_range(AUX_RETRY_INTERVAL,
AUX_RETRY_INTERVAL + 100);
@@ -1744,6 +1738,8 @@ void drm_dp_aux_init(struct drm_dp_aux *aux)
aux->ddc.retries = 3;
aux->ddc.lock_ops = &drm_dp_i2c_lock_ops;
+ /*Still making the Dell 4k monitors happier*/
The original comment was helpful; this one isn't.
Besides that, what problem does this patchset address? Too much probing?
If I connect a Dell monitor to an Intel card, how often does it have to
probe?
Best regards
Thomas
+ aux->num_retries = 32;
}
EXPORT_SYMBOL(drm_dp_aux_init);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index edffd1dcca3e..16cbfc8f5e66 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1876,6 +1876,7 @@ struct drm_dp_aux {
struct mutex hw_mutex;
struct work_struct crc_work;
u8 crc_count;
+ int num_retries;
ssize_t (*transfer)(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg);
/**
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/16] drm/exynos: implement a drm bridge
On 04.02.21 18:46, Daniel Vetter wrote: On Thu, Feb 4, 2021 at 6:26 PM Laurent Pinchart wrote: Hi Daniel, On Thu, Feb 04, 2021 at 06:19:22PM +0100, Daniel Vetter wrote: On Thu, Feb 4, 2021 at 5:28 PM Andrzej Hajda wrote: W dniu 04.02.2021 o 17:05, Daniel Vetter pisze: On Thu, Feb 04, 2021 at 11:56:32AM +0100, Michael Tretter wrote: On Thu, 04 Feb 2021 11:17:49 +0100, Daniel Vetter wrote: On Wed, Feb 3, 2021 at 9:32 PM Michael Tretter wrote: On Mon, 01 Feb 2021 17:33:14 +0100, Michael Tretter wrote: On Tue, 15 Sep 2020 21:40:40 +0200, Andrzej Hajda wrote: W dniu 14.09.2020 o 23:19, Andrzej Hajda pisze: On 14.09.2020 22:01, Michael Tretter wrote: On Mon, 14 Sep 2020 14:31:19 +0200, Marek Szyprowski wrote: On 14.09.2020 10:29, Marek Szyprowski wrote: On 11.09.2020 15:54, Michael Tretter wrote: Make the exynos_dsi driver a full drm bridge that can be found and used from other drivers. Other drivers can only attach to the bridge, if a mipi dsi device already attached to the bridge. This allows to defer the probe of the display pipe until the downstream bridges are available, too. Signed-off-by: Michael Tretter This one (and the whole series applied) still fails on Exynos boards: [drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops) OF: graph: no port node found in /soc/dsi@11c8 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 0084 pgd = (ptrval) [0084] *pgd= Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc4-next-20200911-00010-g417dc70d70ec #1608 Hardware name: Samsung Exynos (Flattened Device Tree) PC is at drm_bridge_attach+0x18/0x164 LR is at exynos_dsi_bind+0x88/0xa8 pc : []lr : []psr: 2013 sp : ef0dfca8 ip : 0002 fp : c13190e0 r10: r9 : ee46d580 r8 : c13190e0 r7 : ee438800 r6 : 0018 r5 : ef253810 r4 : ef39e840 r3 : r2 : 0018 r1 : ef39e888 r0 : ef39e840 Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000404a DAC: 0051 Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) Stack: (0xef0dfca8 to 0xef0e) ... [] (drm_bridge_attach) from [] (exynos_dsi_bind+0x88/0xa8) [] (exynos_dsi_bind) from [] (component_bind_all+0xfc/0x290) [] (component_bind_all) from [] (exynos_drm_bind+0xe4/0x19c) [] (exynos_drm_bind) from [] (try_to_bring_up_master+0x1e4/0x2c4) [] (try_to_bring_up_master) from [] (component_master_add_with_match+0xd4/0x108) [] (component_master_add_with_match) from [] (exynos_drm_platform_probe+0xe4/0x110) [] (exynos_drm_platform_probe) from [] (platform_drv_probe+0x6c/0xa4) [] (platform_drv_probe) from [] (really_probe+0x200/0x4fc) [] (really_probe) from [] (driver_probe_device+0x78/0x1fc) [] (driver_probe_device) from [] (device_driver_attach+0x58/0x60) [] (device_driver_attach) from [] (__driver_attach+0xdc/0x174) [] (__driver_attach) from [] (bus_for_each_dev+0x68/0xb4) [] (bus_for_each_dev) from [] (bus_add_driver+0x158/0x214) [] (bus_add_driver) from [] (driver_register+0x78/0x110) [] (driver_register) from [] (exynos_drm_init+0xe4/0x118) [] (exynos_drm_init) from [] (do_one_initcall+0x8c/0x42c) [] (do_one_initcall) from [] (kernel_init_freeable+0x190/0x1dc) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x118) [] (kernel_init) from [] (ret_from_fork+0x14/0x20) Exception stack(0xef0dffb0 to 0xef0dfff8) ... ---[ end trace ee27f313f9ed9da1 ]--- # arm-linux-gnueabi-addr2line -e vmlinux c0628c08 drivers/gpu/drm/drm_bridge.c:184 (discriminator 1) I will try to debug it a bit more today. The above crash has been caused by lack of in_bridge initialization to NULL in exynos_dsi_bind() in this patch. However, fixing it reveals another issue: [drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops) OF: graph: no port node found in /soc/dsi@11c8 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 0280 pgd = (ptrval) [0280] *pgd= Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc4-next-20200911-00010-g417dc70d70ec-dirty #1613 Hardware name: Samsung Exynos (Flattened Device Tree) PC is at __mutex_lock+0x54/0xb18 LR is at lock_is_held_type+0x80/0x138 pc : []lr : []psr: 6013 sp : ef0dfd30 ip : 33937b74 fp : c13193c8 r10: c1208eec r9 : r8 : ee45f808 r7 : c19561a4 r6 : r5 : r4 : 024c r3 : r2 : 00204140 r1 : c124f13c r0 : Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000404a DAC: 0051 Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) Stack: (0xef0dfd30 to 0xef0e) ... [] (__mutex_lock) from [] (mutex_lock_nested+0x1c/0x24) [] (mutex_lock_nested) from [
Re: [RFC v3 2/3] virtio: Introduce Vdmabuf driver
> > You don't have to use the rendering pipeline. You can let the i915 gpu > > render into a dma-buf shared with virtio-gpu, then use virtio-gpu only for > > buffer sharing with the host. > [Kasireddy, Vivek] Is this the most viable path forward? I am not sure how > complex or > feasible it would be but I'll look into it. > Also, not using the rendering capabilities of virtio-gpu and turning it into > a sharing only > device means there would be a giant mode switch with a lot of if() conditions > sprinkled > across. Are you OK with that? Hmm, why a big mode switch? You should be able to do that without modifying the virtio-gpu guest driver. On the host side qemu needs some work to support the most recent virtio-gpu features like the buffer uuids (assuming you use qemu userspace), right now those are only supported by crosvm. It might be useful to add support for display-less virtio-gpu, i.e. "qemu -device virtio-gpu-pci,max_outputs=0". Right now the linux driver throws an error in case no output (crtc) is present. Should be fixable without too much effort though, effectively the sanity check would have to be moved from driver initialization to commands like SET_SCANOUT which manage the outputs. take care, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] drm/dp: Make number of AUX retries configurable by display drivers.
On Wed, 2021-02-10 at 09:55 +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 10.02.21 um 09:33 schrieb Khaled Almahallawy:
> > The number of AUX retries specified in the DP specs is 7.
> > Currently, to make Dell 4k monitors happier, the number of retries
> > are 32.
> > i915 also retries 5 times (intel_dp_aux_xfer) which means in the
> > case of AUX timeout we actually retries 32 * 5 = 160 times.
> >
> > So making the number of aux retires a variable to allow for fine
> > tuning and optimization of aux timing.
> >
> > Signed-off-by: Khaled Almahallawy
> > ---
> > drivers/gpu/drm/drm_dp_helper.c | 10 +++---
> > include/drm/drm_dp_helper.h | 1 +
> > 2 files changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index eedbb48815b7..8fdf57b4a06c 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -249,13 +249,7 @@ static int drm_dp_dpcd_access(struct
> > drm_dp_aux *aux, u8 request,
> >
> > mutex_lock(&aux->hw_mutex);
> >
> > - /*
> > -* The specification doesn't give any recommendation on how
> > often to
> > -* retry native transactions. We used to retry 7 times like for
> > -* aux i2c transactions but real world devices this wasn't
> > -* sufficient, bump to 32 which makes Dell 4k monitors happier.
> > -*/
> > - for (retry = 0; retry < 32; retry++) {
> > + for (retry = 0; retry < aux->num_retries; retry++) {
> > if (ret != 0 && ret != -ETIMEDOUT) {
> > usleep_range(AUX_RETRY_INTERVAL,
> > AUX_RETRY_INTERVAL + 100);
> > @@ -1744,6 +1738,8 @@ void drm_dp_aux_init(struct drm_dp_aux *aux)
> > aux->ddc.retries = 3;
> >
> > aux->ddc.lock_ops = &drm_dp_i2c_lock_ops;
> > + /*Still making the Dell 4k monitors happier*/
>
> The original comment was helpful; this one isn't.
Noted and I apologize for the comment. Was just copy/past original
comment.
>
> Besides that, what problem does this patchset address? Too much
> probing?
The problem mainly with disconnect. When disconnecting, AUX read will
fail because of timing out. The 32 retries cause the disconnect flow
especially for Type-C/TBT Docks with MST hubs taking a long time.
Just trying to reduce this disconnect time. In addition as I noted,
i915 does retry in addition to retries by this function.
Currently this function retry for 4 AUX situations:
AUX_NAK, AUX_DEFER, I/O error reported by driver and AUX timeout.
> If I connect a Dell monitor to an Intel card, how often does it have
> to
> probe?
I guess it depends on how you connect (direct, behind MST hub, behind DOCK with
MST). But I believe the most time consuming is the disconnect flow.
Thanks
Khaled
>
> Best regards
> Thomas
>
> > + aux->num_retries = 32;
> > }
> > EXPORT_SYMBOL(drm_dp_aux_init);
> >
> > diff --git a/include/drm/drm_dp_helper.h
> > b/include/drm/drm_dp_helper.h
> > index edffd1dcca3e..16cbfc8f5e66 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -1876,6 +1876,7 @@ struct drm_dp_aux {
> > struct mutex hw_mutex;
> > struct work_struct crc_work;
> > u8 crc_count;
> > + int num_retries;
> > ssize_t (*transfer)(struct drm_dp_aux *aux,
> > struct drm_dp_aux_msg *msg);
> > /**
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 0/2] Implement DE2.0 and DE3.0 per-plane alpha support
On Thu, Jan 28, 2021 at 01:39:38PM +0200, Roman Stratiienko wrote: > > Please review/merge. > > v2: > Initial patch > > v3: > - Skip adding & applying alpha property if VI count > 1 (v3s case) > > v4: > Resend (author's email changed) > > v5: > Resend Applied, thanks Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/5] clk: sunxi-ng: mp: fix parent rate change flag check
Hi Mike, Stephen,
On Tue, Feb 09, 2021 at 06:58:56PM +0100, Jernej Skrabec wrote:
> CLK_SET_RATE_PARENT flag is checked on parent clock instead of current
> one. Fix that.
>
> Fixes: 3f790433c3cb ("clk: sunxi-ng: Adjust MP clock parent rate when
> allowed")
> Reviewed-by: Chen-Yu Tsai
> Tested-by: Andre Heider
> Signed-off-by: Jernej Skrabec
This is a last minute fix for us, can you merge it into clk-fixes directly?
Acked-by: Maxime Ripard
Thanks!
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/5] sunxi: fix H6 HDMI related issues
On Tue, Feb 09, 2021 at 06:58:55PM +0100, Jernej Skrabec wrote: > Over the year I got plenty of reports of troubles with H6 HDMI signal. > Sometimes monitor flickers, sometimes there was no image at all and > sometimes it didn't play well with AVR. > > It turns out there are multiple issues. Patch 1 fixes clock issue, > which didn't adjust parent rate, even if it is allowed to do so. Patch 2 > adds polarity config in tcon1. This is seemingly not needed for pre-HDMI2 > controllers, although BSP drivers set it accordingly every time. It > turns out that HDMI2 controllers often don't work with monitors if > polarity is not set correctly. Patch 3 always set clock rate for HDMI > controller. Patch 4 fixes H6 HDMI PHY settings. Patch 5 fixes comment and > clock rate limit (wrong reasoning). > > Please take a look. > > Best regards, > Jernej > > Changes from v2: > - use clk_hw_can_set_rate_parent() directly instead of checking flags > Changes from v1: > - collected Chen-Yu tags (except on replaced patch 4) > - Added some comments in patch 2 > - Replaced patch 4 (see commit log for explanation) Applied patches 2-5, thanks Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/i915/gem: Add a check for object size for corner cases
Quoting Anand Moon (2021-02-10 07:59:29) > Add check for object size to return appropriate error -E2BIG or -EINVAL > to avoid WARM_ON and sucessfull return for some testcase. No. You miss the point of having those warnings. We need to inspect the code to remove the last remaining "int pagenum", and then we can remove the GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX). These are not emitted for users, only for us to motivate us into finally fixing the code. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/6] Support second Image Signal Processor on rk3399
The rk3399 has two ISPs and right now only the first one is usable. The second ISP is connected to the TXRX dphy on the soc. The phy of ISP1 is only accessible through the DSI controller's io-memory, so this series adds support for simply using the dsi controller is a phy if needed. That solution is needed at least on rk3399 and rk3288 but no-one has looked at camera support on rk3288 at all, so right now only implement the rk3399 specifics. changes in v2: - enable grf-clock also for init callback to not break if for example hdmi is connected on boot and disabled the grf clock during its probe - add Sebastian's Tested-by - add Rob's Ack for the phy-cells property Heiko Stuebner (6): drm/rockchip: dsi: add own additional pclk handling dt-bindings: display: rockchip-dsi: add optional #phy-cells property drm/rockchip: dsi: add ability to work as a phy instead of full dsi arm64: dts: rockchip: add #phy-cells to mipi-dsi1 arm64: dts: rockchip: add cif clk-control pinctrl for rk3399 arm64: dts: rockchip: add isp1 node on rk3399 .../display/rockchip/dw_mipi_dsi_rockchip.txt | 1 + arch/arm64/boot/dts/rockchip/rk3399.dtsi | 39 ++ drivers/gpu/drm/rockchip/Kconfig | 2 + .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 349 ++ 4 files changed, 391 insertions(+) -- 2.29.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/6] arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
From: Heiko Stuebner
This enables variant a of the clkout signal for camera applications
and also the cifclkin pinctrl setting.
Signed-off-by: Heiko Stuebner
Tested-by: Sebastian Fricke
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 5d2178cb3e38..7c661d84df25 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2102,6 +2102,18 @@ clk_32k: clk-32k {
};
};
+ cif {
+ cif_clkin: cif-clkin {
+ rockchip,pins =
+ <2 RK_PB2 3 &pcfg_pull_none>;
+ };
+
+ cif_clkouta: cif-clkouta {
+ rockchip,pins =
+ <2 RK_PB3 3 &pcfg_pull_none>;
+ };
+ };
+
edp {
edp_hpd: edp-hpd {
rockchip,pins =
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/6] drm/rockchip: dsi: add own additional pclk handling
From: Heiko Stuebner
In a followup patch, we'll need to access the pclk ourself to enable some
functionality, so get and store it in the rockchip dw-dsi variant as well.
Clocks are refcounted, so possible cascading enablements are
no problem.
Signed-off-by: Heiko Stuebner
Tested-by: Sebastian Fricke
---
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
index 24a71091759c..18e112e30f6e 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
@@ -223,6 +223,7 @@ struct dw_mipi_dsi_rockchip {
void __iomem *base;
struct regmap *grf_regmap;
+ struct clk *pclk;
struct clk *pllref_clk;
struct clk *grf_clk;
struct clk *phy_cfg_clk;
@@ -1051,6 +1052,13 @@ static int dw_mipi_dsi_rockchip_probe(struct
platform_device *pdev)
return ret;
}
+ dsi->pclk = devm_clk_get(dev, "pclk");
+ if (IS_ERR(dsi->pclk)) {
+ ret = PTR_ERR(dsi->pclk);
+ DRM_DEV_ERROR(dev, "Unable to get pclk: %d\n", ret);
+ return ret;
+ }
+
dsi->pllref_clk = devm_clk_get(dev, "ref");
if (IS_ERR(dsi->pllref_clk)) {
if (dsi->phy) {
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/6] drm/rockchip: dsi: add ability to work as a phy instead of full dsi
From: Heiko Stuebner
SoCs like the rk3288 and rk3399 have 3 mipi dphys on them. One is TX-
only, one is RX-only and one can be configured to do either TX or RX.
The RX phy is statically connected to the first Image Signal Processor,
the TX phy is statically connected to the first DSI controller and
the TXRX phy is connected to both the second DSI controller as well
as the second ISP.
The RX dphy is controlled externally through registers in the "General
Register Files", while the other two are controlled through the
"Configuration and Test Interface" inside their DSI controller's
io-memory area.
The Rockchip dw-dsi controller already controls these dphys for the
TX case in the driver, but when we want to also allow configuration
for RX to the ISP from the media subsystem we need to expose phy-
functionality instead.
So add a bit of infrastructure to allow the dsi driver to work as a
phy and make sure it can be only one or the other at a time.
Similarly as the dsi-controller will be part of the drm-graph when
active, add an empty component to the drm-graph when in phy-mode
to make the rest of the drm-graph not wait for it.
Signed-off-by: Heiko Stuebner
Tested-by: Sebastian Fricke
---
drivers/gpu/drm/rockchip/Kconfig | 2 +
.../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 341 ++
2 files changed, 343 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index cb25c0e8fc9b..3094d4533ad6 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -9,6 +9,8 @@ config DRM_ROCKCHIP
select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
+ select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
+ select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI
select DRM_RGB if ROCKCHIP_RGB
select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
help
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
index 18e112e30f6e..e322749a5279 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -125,7 +126,9 @@
#define BANDGAP_AND_BIAS_CONTROL 0x20
#define TERMINATION_RESISTER_CONTROL 0x21
#define AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY0x22
+#define HS_RX_CONTROL_OF_LANE_CLK 0x34
#define HS_RX_CONTROL_OF_LANE_00x44
+#define HS_RX_CONTROL_OF_LANE_10x54
#define HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL0x60
#define HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL0x61
#define HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL0x62
@@ -137,6 +140,9 @@
#define HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL 0x72
#define HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL 0x73
#define HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL0x74
+#define HS_RX_DATA_LANE_THS_SETTLE_CONTROL 0x75
+#define HS_RX_CONTROL_OF_LANE_20x84
+#define HS_RX_CONTROL_OF_LANE_30x94
#define DW_MIPI_NEEDS_PHY_CFG_CLK BIT(0)
#define DW_MIPI_NEEDS_GRF_CLK BIT(1)
@@ -171,11 +177,19 @@
#define RK3399_TXRX_MASTERSLAVEZ BIT(7)
#define RK3399_TXRX_ENABLECLK BIT(6)
#define RK3399_TXRX_BASEDIRBIT(5)
+#define RK3399_TXRX_SRC_SEL_ISP0 BIT(4)
+#define RK3399_TXRX_TURNREQUESTGENMASK(3, 0)
#define HIWORD_UPDATE(val, mask) (val | (mask) << 16)
#define to_dsi(nm) container_of(nm, struct dw_mipi_dsi_rockchip, nm)
+enum {
+ DW_DSI_USAGE_IDLE,
+ DW_DSI_USAGE_DSI,
+ DW_DSI_USAGE_PHY,
+};
+
enum {
BANDGAP_97_07,
BANDGAP_98_05,
@@ -213,6 +227,10 @@ struct rockchip_dw_dsi_chip_data {
u32 lanecfg2_grf_reg;
u32 lanecfg2;
+ int (*dphy_rx_init)(struct phy *phy);
+ int (*dphy_rx_power_on)(struct phy *phy);
+ int (*dphy_rx_power_off)(struct phy *phy);
+
unsigned int flags;
unsigned int max_data_lanes;
};
@@ -236,6 +254,12 @@ struct dw_mipi_dsi_rockchip {
struct phy *phy;
union phy_configure_opts phy_opts;
+ /* being a phy for other mipi hosts */
+ unsigned int usage_mode;
+ struct mutex usage_mutex;
+ struct phy *dphy;
+ struct phy_configure_opts_mipi_dphy dphy_config;
+
unsigned int lane_mbps; /* per lane */
u16 input_div;
u16 feedback_div;
@@ -965,6 +989,17 @@ static int dw_mipi_dsi_rockchip_host_attach(void
*priv_data,
struct device *second;
int ret;
+ mutex_lock(&dsi->usage_mutex);
+
+ if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
+ DRM_DEV_ERRO
[PATCH 4/6] arm64: dts: rockchip: add #phy-cells to mipi-dsi1
From: Heiko Stuebner
The dsi controller includes access to the dphy which might be used
not only for dsi output but also for csi input on dsi1, so add the
necessary #phy-cells to allow it to be used as phy.
Signed-off-by: Heiko Stuebner
Tested-by: Sebastian Fricke
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index edbbf35fe19e..5d2178cb3e38 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1865,6 +1865,7 @@ mipi_dsi1: mipi@ff968000 {
rockchip,grf = <&grf>;
#address-cells = <1>;
#size-cells = <0>;
+ #phy-cells = <0>;
status = "disabled";
ports {
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6] arm64: dts: rockchip: add isp1 node on rk3399
From: Heiko Stuebner
ISP1 is supplied by the tx1rx1 dphy, that is controlled from
inside the dsi1 controller, so include the necessary phy-link
for it.
Signed-off-by: Heiko Stuebner
Tested-by: Sebastian Fricke
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 26
1 file changed, 26 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 7c661d84df25..98cec9387300 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1756,6 +1756,32 @@ isp0_mmu: iommu@ff914000 {
rockchip,disable-mmu-reset;
};
+ isp1: isp1@ff92 {
+ compatible = "rockchip,rk3399-cif-isp";
+ reg = <0x0 0xff92 0x0 0x4000>;
+ interrupts = ;
+ clocks = <&cru SCLK_ISP1>,
+<&cru ACLK_ISP1_WRAPPER>,
+<&cru HCLK_ISP1_WRAPPER>;
+ clock-names = "isp", "aclk", "hclk";
+ iommus = <&isp1_mmu>;
+ phys = <&mipi_dsi1>;
+ phy-names = "dphy";
+ power-domains = <&power RK3399_PD_ISP1>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+ };
+
isp1_mmu: iommu@ff924000 {
compatible = "rockchip,iommu";
reg = <0x0 0xff924000 0x0 0x100>, <0x0 0xff925000 0x0 0x100>;
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/6] dt-bindings: display: rockchip-dsi: add optional #phy-cells property
From: Heiko Stuebner The Rockchip DSI controller on some SoCs also controls a bidrectional dphy, which would be connected to an Image Signal Processor as a phy in the rx configuration. So allow a #phy-cells property for the dsi controller. Signed-off-by: Heiko Stuebner Acked-by: Rob Herring Tested-by: Sebastian Fricke --- .../bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt index 151be3bba06f..39792f051d2d 100644 --- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt @@ -23,6 +23,7 @@ Required properties: Optional properties: - phys: from general PHY binding: the phandle for the PHY device. - phy-names: Should be "dphy" if phys references an external phy. +- #phy-cells: Defined when used as ISP phy, should be 0. - power-domains: a phandle to mipi dsi power domain node. - resets: list of phandle + reset specifier pairs, as described in [3]. - reset-names: string reset name, must be "apb". -- 2.29.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] Support second Image Signal Processor on rk3399
Am Mittwoch, 10. Februar 2021, 12:10:14 CET schrieb Heiko Stuebner: > The rk3399 has two ISPs and right now only the first one is usable. > The second ISP is connected to the TXRX dphy on the soc. > > The phy of ISP1 is only accessible through the DSI controller's > io-memory, so this series adds support for simply using the dsi > controller is a phy if needed. > > That solution is needed at least on rk3399 and rk3288 but no-one > has looked at camera support on rk3288 at all, so right now > only implement the rk3399 specifics. > > changes in v2: > - enable grf-clock also for init callback > to not break if for example hdmi is connected on boot > and disabled the grf clock during its probe > - add Sebastian's Tested-by > - add Rob's Ack for the phy-cells property > > Heiko Stuebner (6): > drm/rockchip: dsi: add own additional pclk handling > dt-bindings: display: rockchip-dsi: add optional #phy-cells property > drm/rockchip: dsi: add ability to work as a phy instead of full dsi > arm64: dts: rockchip: add #phy-cells to mipi-dsi1 > arm64: dts: rockchip: add cif clk-control pinctrl for rk3399 > arm64: dts: rockchip: add isp1 node on rk3399 of course everything was meant to be v2 in the subject. Heiko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Support second Image Signal Processor on rk3399
Hi Sebastian, Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner: > Hi Sebastian, > > I did some tests myself today as well and can confirm your > hdmi related finding - at least when plugged in on boot. > > I tried some combinations of camera vs. hdmi and it seems > really only when hdmi is plugged in on boot as you can see in v2, it should work now even with hdmi connected on boot. My patch ignored the grf-clock when doing the grf-based init. All clocks are on during boot and I guess the hdmi-driver did disable it after its probe. The phy_power_on functions did handle it correctly already, so it was only happening with hdmi connected on boot. Btw. do you plan on submitting your ov13850 driver soonish? Heiko > > (1) > - boot > - camera > --> works > > (2) > - boot > - camera > - hdmi plugged in > - hdmi works > - camera > --> works > > (3) > - hdmi plugged in > - boot > - hdmi works > - camera > --> camera doesn't work > > (4) > - boot > - hdmi plugged in > - hdmi works > - camera > -> camera works > > > With a bit of brute-force [0] it seems the camera also works again even > with hdmi connected on boot. So conclusion would be that some clock > is misbehaving. > > Now we'll "only" need to find out which one that is. > > > Heiko > > > [0] > Don't disable any clock gates > > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > index 070dc47e95a1..8daf1fc3388c 100644 > --- a/drivers/clk/clk-gate.c > +++ b/drivers/clk/clk-gate.c > @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int > enable) > > set ^= enable; > > +if (!enable) > +return; > + > if (gate->lock) > spin_lock_irqsave(gate->lock, flags); > else > > > > Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner: > > Hi Sebastian, > > > > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke: > > > On 03.02.2021 20:54, Heiko Stübner wrote: > > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke: > > > >> I have tested your patch set on my nanoPC-T4, here is a complete log > > > >> with: > > > >> - relevant kernel log entries > > > >> - system information > > > >> - media ctl output > > > >> - sysfs entry information > > > >> > > > >> https://paste.debian.net/1183874/ > > > >> > > > >> Additionally, to your patchset I have applied the following patches: > > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup > > > >> > > > >> And just to not cause confusion the `media_dev` entries come from this > > > >> unmerged series: > > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269 > > > >> > > > >> I have actually been able to stream with both of my cameras at the same > > > >> time using the libcamera cam command. > > > >> I would like to thank you a lot for making this possible. > > > > > > > >Thanks for testing a dual camera setup. On my board I could only test > > > >the second ISP. And really glad it works for you tool :-) . > > > > > > > >Out of curiosity, do you also see that green tint in the images the > > > >cameras > > > >produce? > > > > > > Yes, I do. Actually, I currently have two forms of a green tint, on my > > > OV13850 everything is quite dark and greenish, which is caused by the > > > missing 3A algorithms. On my OV4689, I have big patches of the image > > > with bright green color and flickering, I investigated if this is > > > connected to the 2nd ISP instance, but that doesn't seem to be the case > > > as I have the same results when I switch the CSI ports of the cameras. > > > > > > I have found another issue, while testing I discovered following > > > issue: > > > When I start the system with an HDMI monitor connected, then the camera > > > on the 2nd port doesn't work. This is probably because the RX/TX is > > > reserved as a TX. > > > But it made me wonder because if the system has an RX, a TX, and > > > an RX/TX, why isn't the pure TX used by the monitor and the > > > cameras take RX and RX/TX? > > > Or do you think that this is maybe a malfunction of this patch? > > > > I don't think it is an issue with this specific series, but still puzzling. > > > > I.e. the DPHYs are actually only relevant to the DSI controllers, > > with TX0 being connected to DSI0 and TX1RX1 being connected > > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all. > > > > Out of curiosity what happens, when you boot without hdmi connected > > turn on the cameras, connect the hdmi after this, try the cameras again? > > > > > > Heiko > > > > > > > > > > > > >Thanks > > > >Heiko > > > > > > Greetings, > > > Sebastian > > > > > > > > > > > > > > >> If you like to you can add: > > > >> Tested-by: Sebastian Fricke > > > >> > > > >> On 02.02.2021 15:56, Heiko Stuebner wrote: > > > >> >The rk3399 has two ISPs and right now only the first one is usable. > > > >> >The second ISP is connected to the TXRX dphy on the soc. > > > >>
[Bug 210263] brightness device returns ENXIO (?) on brightness restore at boot, with bootoption "quiet"
https://bugzilla.kernel.org/show_bug.cgi?id=210263 --- Comment #2 from ninel...@protonmail.com --- I just found out that this Issue does NOT occur when you additionally add the fbcon=nodefer kernel parameter. -- You may reply to this email to add a comment. 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] drm/vblank: Document drm_crtc_vblank_restore constraints
On Tue, Feb 09, 2021 at 05:36:07PM +0200, Ville Syrjälä wrote:
> On Tue, Feb 09, 2021 at 11:15:23AM +0100, Daniel Vetter wrote:
> > I got real badly confused when trying to review a fix from Ville for
> > this. Let's try to document better what's required for this, and check
> > the minimal settings at runtime - we can't check ofc that there's
> > indeed no races in the driver callback.
> >
> > Also noticed that the drm_vblank_restore version is unused, so lets
> > unexport that while at it.
> >
> > Cc: Ville Syrjala
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_vblank.c | 25 ++---
> > include/drm/drm_vblank.h | 1 -
> > 2 files changed, 10 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index c914b14cfb43..05f4d4c078fd 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -1471,20 +1471,7 @@ void drm_crtc_vblank_on(struct drm_crtc *crtc)
> > }
> > EXPORT_SYMBOL(drm_crtc_vblank_on);
> >
> > -/**
> > - * drm_vblank_restore - estimate missed vblanks and update vblank count.
> > - * @dev: DRM device
> > - * @pipe: CRTC index
> > - *
> > - * Power manamement features can cause frame counter resets between vblank
> > - * disable and enable. Drivers can use this function in their
> > - * &drm_crtc_funcs.enable_vblank implementation to estimate missed vblanks
> > since
> > - * the last &drm_crtc_funcs.disable_vblank using timestamps and update the
> > - * vblank counter.
> > - *
> > - * This function is the legacy version of drm_crtc_vblank_restore().
> > - */
> > -void drm_vblank_restore(struct drm_device *dev, unsigned int pipe)
> > +static void drm_vblank_restore(struct drm_device *dev, unsigned int pipe)
> > {
> > ktime_t t_vblank;
> > struct drm_vblank_crtc *vblank;
> > @@ -1520,7 +1507,6 @@ void drm_vblank_restore(struct drm_device *dev,
> > unsigned int pipe)
> > diff, diff_ns, framedur_ns, cur_vblank - vblank->last);
> > store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
> > }
> > -EXPORT_SYMBOL(drm_vblank_restore);
> >
> > /**
> > * drm_crtc_vblank_restore - estimate missed vblanks and update vblank
> > count.
> > @@ -1531,9 +1517,18 @@ EXPORT_SYMBOL(drm_vblank_restore);
> > * &drm_crtc_funcs.enable_vblank implementation to estimate missed vblanks
> > since
> > * the last &drm_crtc_funcs.disable_vblank using timestamps and update the
> > * vblank counter.
> > + *
> > + * Note that drivers must have race-free high-precision timestamping
> > support,
> > + * i.e. &drm_crtc_funcs.get_vblank_timestamp must be hooked up and
> > + * &drm_driver.vblank_disable_immediate must be set to indicate the
> > + * time-stamping functions are race-free against vblank hardware counter
> > + * increments.
>
> Looks good. Might prevent someone from shooting themselves in
> the foot.
Yeah hopefully, maybe even me :-)
>
> Reviewed-by: Ville Syrjälä
Thanks for your review, I pushed to drm-misc-next.
-Daniel
>
> > */
> > void drm_crtc_vblank_restore(struct drm_crtc *crtc)
> > {
> > + WARN_ON_ONCE(!crtc->funcs->get_vblank_timestamp);
> > + WARN_ON_ONCE(!crtc->dev->vblank_disable_immediate);
> > +
> > drm_vblank_restore(crtc->dev, drm_crtc_index(crtc));
> > }
> > EXPORT_SYMBOL(drm_crtc_vblank_restore);
> > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > index dd125f8c766c..733a3e2d1d10 100644
> > --- a/include/drm/drm_vblank.h
> > +++ b/include/drm/drm_vblank.h
> > @@ -247,7 +247,6 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc);
> > void drm_crtc_vblank_reset(struct drm_crtc *crtc);
> > void drm_crtc_vblank_on(struct drm_crtc *crtc);
> > u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
> > -void drm_vblank_restore(struct drm_device *dev, unsigned int pipe);
> > void drm_crtc_vblank_restore(struct drm_crtc *crtc);
> >
> > void drm_calc_timestamping_constants(struct drm_crtc *crtc,
> > --
> > 2.30.0
>
> --
> Ville Syrjälä
> Intel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915/adl_s: Add gmbus pin mapping
Add table to map the GMBUS pin pairs to GPIO registers and port to DDC
mapping for ADL_S as per below Bspec.
Bspec:20124, 53597.
Cc: Aditya Swarup
Cc: Matt Roper
Cc: Lucas De Marchi
Signed-off-by: Anand Moon
---
drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c
b/drivers/gpu/drm/i915/display/intel_gmbus.c
index 0c952e1d720e..58b8e42d4f90 100644
--- a/drivers/gpu/drm/i915/display/intel_gmbus.c
+++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
@@ -52,6 +52,14 @@ static const struct gmbus_pin gmbus_pins[] = {
[GMBUS_PIN_DPD] = { "dpd", GPIOF },
};
+static const struct gmbus_pin gmbus_pins_adls[] = {
+ [GMBUS_PIN_1_BXT] = { "edp", GPIOA },
+ [GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOD },
+ [GMBUS_PIN_10_TC2_ICP] = { "tc2", GPIOE },
+ [GMBUS_PIN_11_TC3_ICP] = { "tc3", GPIOF },
+ [GMBUS_PIN_12_TC4_ICP] = { "tc4", GPIOG },
+};
+
static const struct gmbus_pin gmbus_pins_bdw[] = {
[GMBUS_PIN_VGADDC] = { "vga", GPIOA },
[GMBUS_PIN_DPC] = { "dpc", GPIOD },
@@ -101,7 +109,9 @@ static const struct gmbus_pin gmbus_pins_dg1[] = {
static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
unsigned int pin)
{
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
+ if (INTEL_PCH_TYPE(dev_priv) == PCH_ADP)
+ return &gmbus_pins_adls[pin];
+ else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
return &gmbus_pins_dg1[pin];
else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
return &gmbus_pins_icp[pin];
@@ -122,7 +132,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private
*dev_priv,
{
unsigned int size;
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
+ if (INTEL_PCH_TYPE(dev_priv) == PCH_ADP)
+ size = ARRAY_SIZE(gmbus_pins_adls);
+ else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
size = ARRAY_SIZE(gmbus_pins_dg1);
else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
size = ARRAY_SIZE(gmbus_pins_icp);
--
2.30.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4, 01/10] soc: mediatek: mmsys: create mmsys folder
On 09/02/2021 16:38, Enric Balletbo Serra wrote: > Hi Yongqiang Niu, > > Thank you for your patch. > > Missatge de Yongqiang Niu del dia dt., 5 > de gen. 2021 a les 4:07: >> >> the mmsys will more and more complicated after support >> more and more SoCs, add an independent folder will be >> more clear >> >> Signed-off-by: Yongqiang Niu >> --- >> drivers/soc/mediatek/Makefile | 2 +- > > It will not apply cleanly anymore after the below commit that is > already queued. Maybe you could rebase the patches and resend them > again? > Please don't do that, as I pointed out in [1] I don't like the approach of a new folder. If you disagree please let me know why. Otherwise please send a new version with the changes suggested by me :) Regards, Matthias [1] https://lore.kernel.org/linux-mediatek/4cadc9f0-0761-7609-abac-d2211b097...@gmail.com/ > commit e1e4f7fea37572f0ccf3887430e52c491e9accb6 > Author: CK Hu > Date: Tue Jul 21 15:46:06 2020 +0800 > > soc / drm: mediatek: Move mtk mutex driver to soc folder > > mtk mutex is used by DRM and MDP driver, and its function is SoC-specific, > so move it to soc folder. > > With that fixed, > > Reviewed-by: Enric Balletbo i Serra > > Thanks, > Enric > >> drivers/soc/mediatek/mmsys/Makefile| 2 + >> drivers/soc/mediatek/mmsys/mtk-mmsys.c | 373 >> + >> drivers/soc/mediatek/mtk-mmsys.c | 373 >> - >> 4 files changed, 376 insertions(+), 374 deletions(-) >> create mode 100644 drivers/soc/mediatek/mmsys/Makefile >> create mode 100644 drivers/soc/mediatek/mmsys/mtk-mmsys.c >> delete mode 100644 drivers/soc/mediatek/mtk-mmsys.c >> >> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile >> index b6908db..eca9774 100644 >> --- a/drivers/soc/mediatek/Makefile >> +++ b/drivers/soc/mediatek/Makefile >> @@ -5,4 +5,4 @@ obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o >> obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o >> obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o >> obj-$(CONFIG_MTK_SCPSYS_PM_DOMAINS) += mtk-pm-domains.o >> -obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o >> +obj-$(CONFIG_MTK_MMSYS) += mmsys/ >> diff --git a/drivers/soc/mediatek/mmsys/Makefile >> b/drivers/soc/mediatek/mmsys/Makefile >> new file mode 100644 >> index 000..f44eadc >> --- /dev/null >> +++ b/drivers/soc/mediatek/mmsys/Makefile >> @@ -0,0 +1,2 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> +obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o >> diff --git a/drivers/soc/mediatek/mmsys/mtk-mmsys.c >> b/drivers/soc/mediatek/mmsys/mtk-mmsys.c >> new file mode 100644 >> index 000..18f9397 >> --- /dev/null >> +++ b/drivers/soc/mediatek/mmsys/mtk-mmsys.c >> @@ -0,0 +1,373 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2014 MediaTek Inc. >> + * Author: James Liao >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define DISP_REG_CONFIG_DISP_OVL0_MOUT_EN 0x040 >> +#define DISP_REG_CONFIG_DISP_OVL1_MOUT_EN 0x044 >> +#define DISP_REG_CONFIG_DISP_OD_MOUT_EN0x048 >> +#define DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN 0x04c >> +#define DISP_REG_CONFIG_DISP_UFOE_MOUT_EN 0x050 >> +#define DISP_REG_CONFIG_DISP_COLOR0_SEL_IN 0x084 >> +#define DISP_REG_CONFIG_DISP_COLOR1_SEL_IN 0x088 >> +#define DISP_REG_CONFIG_DSIE_SEL_IN0x0a4 >> +#define DISP_REG_CONFIG_DSIO_SEL_IN0x0a8 >> +#define DISP_REG_CONFIG_DPI_SEL_IN 0x0ac >> +#define DISP_REG_CONFIG_DISP_RDMA2_SOUT0x0b8 >> +#define DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN 0x0c4 >> +#define DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN 0x0c8 >> +#define DISP_REG_CONFIG_MMSYS_CG_CON0 0x100 >> + >> +#define DISP_REG_CONFIG_DISP_OVL_MOUT_EN 0x030 >> +#define DISP_REG_CONFIG_OUT_SEL0x04c >> +#define DISP_REG_CONFIG_DSI_SEL0x050 >> +#define DISP_REG_CONFIG_DPI_SEL0x064 >> + >> +#define OVL0_MOUT_EN_COLOR00x1 >> +#define OD_MOUT_EN_RDMA0 0x1 >> +#define OD1_MOUT_EN_RDMA1 BIT(16) >> +#define UFOE_MOUT_EN_DSI0 0x1 >> +#define COLOR0_SEL_IN_OVL0 0x1 >> +#define OVL1_MOUT_EN_COLOR10x1 >> +#define GAMMA_MOUT_EN_RDMA10x1 >> +#define RDMA0_SOUT_DPI00x2 >> +#define RDMA0_SOUT_DPI10x3 >> +#define RDMA0_SOUT_DSI10x1 >> +#define RDMA0_SOUT_DSI20x4 >> +#define RDMA0_SOUT_DSI30x5 >> +#define RDMA1_SOUT_DPI00x2 >> +#define RDMA1_SOUT_DPI10x3 >> +#define RDMA1_SOUT_DSI10x1 >> +#define RDMA1_SOUT_DSI20x4 >> +#define
[PATCH][next] drm/amd/pm: fix spelling mistake in various messages "power_dpm_force_perfomance_level"
From: Colin Ian King
There are spelling mistakes in error and warning messages, the text
power_dpm_force_perfomance_level is missing a letter r and should be
power_dpm_force_performance_level. Fix them.
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c | 2 +-
drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 2 +-
drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
index ed05a30d1139..d1358a6dd2c8 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
@@ -1487,7 +1487,7 @@ static int smu10_set_fine_grain_clk_vol(struct pp_hwmgr
*hwmgr,
}
if (!smu10_data->fine_grain_enabled) {
- pr_err("pp_od_clk_voltage is not accessible if
power_dpm_force_perfomance_level is not in manual mode!\n");
+ pr_err("pp_od_clk_voltage is not accessible if
power_dpm_force_performance_level is not in manual mode!\n");
return -EINVAL;
}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
index 093b01159408..8abb25a28117 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
@@ -1462,7 +1462,7 @@ static int vangogh_od_edit_dpm_table(struct smu_context
*smu, enum PP_OD_DPM_TAB
if (!(smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)) {
dev_warn(smu->adev->dev,
- "pp_od_clk_voltage is not accessible if
power_dpm_force_perfomance_level is not in manual mode!\n");
+ "pp_od_clk_voltage is not accessible if
power_dpm_force_performance_level is not in manual mode!\n");
return -EINVAL;
}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
index 5faa509f0dba..b59156dfca19 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
@@ -351,7 +351,7 @@ static int renoir_od_edit_dpm_table(struct smu_context *smu,
if (!(smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)) {
dev_warn(smu->adev->dev,
- "pp_od_clk_voltage is not accessible if
power_dpm_force_perfomance_level is not in manual mode!\n");
+ "pp_od_clk_voltage is not accessible if
power_dpm_force_performance_level is not in manual mode!\n");
return -EINVAL;
}
--
2.30.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v13 7/8] soc: mediatek: add mtk mutex support for MT8183
On 09/02/2021 15:48, Enric Balletbo Serra wrote:
> Hi Hsin-Yi,
>
> Thank you for your patch.
>
> Missatge de Hsin-Yi Wang del dia dv., 29 de gen.
> 2021 a les 10:23:
>>
>> From: Yongqiang Niu
>>
>> Add mtk mutex support for MT8183 SoC.
>>
>> Signed-off-by: Yongqiang Niu
>> Signed-off-by: Hsin-Yi Wang
>> Reviewed-by: CK Hu
>
> Reviewed-by: Enric Balletbo i Serra
>
> FWIW this patch is required to have the display working on the
> Chromebook IdeaPad Duet, so
>
> Tested-by: Enric Balletbo i Serra
>
> Matthias, If I am not wrong, this patch is the only one that is not
> applied for this series. I know that is too late for 5.12, but If
> you're fine with it, could you pick this patch directly or do you
> prefer a resend of this patch alone once you will start to accept
> patches for the next release?
This patch is based on top of a patch that's in CK's branch.
Let's wait for v5.12-rc1 then I'll take it. If I forget just ping me here/IRC
Regards,
Matthias
>
> Thanks,
> Enric
>
>> ---
>> drivers/soc/mediatek/mtk-mutex.c | 50
>> 1 file changed, 50 insertions(+)
>>
>> diff --git a/drivers/soc/mediatek/mtk-mutex.c
>> b/drivers/soc/mediatek/mtk-mutex.c
>> index f531b119da7a9..718a41beb6afb 100644
>> --- a/drivers/soc/mediatek/mtk-mutex.c
>> +++ b/drivers/soc/mediatek/mtk-mutex.c
>> @@ -14,6 +14,8 @@
>>
>> #define MT2701_MUTEX0_MOD0 0x2c
>> #define MT2701_MUTEX0_SOF0 0x30
>> +#define MT8183_MUTEX0_MOD0 0x30
>> +#define MT8183_MUTEX0_SOF0 0x2c
>>
>> #define DISP_REG_MUTEX_EN(n) (0x20 + 0x20 * (n))
>> #define DISP_REG_MUTEX(n) (0x24 + 0x20 * (n))
>> @@ -37,6 +39,18 @@
>> #define MT8167_MUTEX_MOD_DISP_DITHER 15
>> #define MT8167_MUTEX_MOD_DISP_UFOE 16
>>
>> +#define MT8183_MUTEX_MOD_DISP_RDMA00
>> +#define MT8183_MUTEX_MOD_DISP_RDMA11
>> +#define MT8183_MUTEX_MOD_DISP_OVL0 9
>> +#define MT8183_MUTEX_MOD_DISP_OVL0_2L 10
>> +#define MT8183_MUTEX_MOD_DISP_OVL1_2L 11
>> +#define MT8183_MUTEX_MOD_DISP_WDMA012
>> +#define MT8183_MUTEX_MOD_DISP_COLOR0 13
>> +#define MT8183_MUTEX_MOD_DISP_CCORR0 14
>> +#define MT8183_MUTEX_MOD_DISP_AAL0 15
>> +#define MT8183_MUTEX_MOD_DISP_GAMMA0 16
>> +#define MT8183_MUTEX_MOD_DISP_DITHER0 17
>> +
>> #define MT8173_MUTEX_MOD_DISP_OVL0 11
>> #define MT8173_MUTEX_MOD_DISP_OVL1 12
>> #define MT8173_MUTEX_MOD_DISP_RDMA013
>> @@ -87,6 +101,11 @@
>> #define MT2712_MUTEX_SOF_DSI3 6
>> #define MT8167_MUTEX_SOF_DPI0 2
>> #define MT8167_MUTEX_SOF_DPI1 3
>> +#define MT8183_MUTEX_SOF_DSI0 1
>> +#define MT8183_MUTEX_SOF_DPI0 2
>> +
>> +#define MT8183_MUTEX_EOF_DSI0 (MT8183_MUTEX_SOF_DSI0 << 6)
>> +#define MT8183_MUTEX_EOF_DPI0 (MT8183_MUTEX_SOF_DPI0 << 6)
>>
>> struct mtk_mutex {
>> int id;
>> @@ -181,6 +200,20 @@ static const unsigned int
>> mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = {
>> [DDP_COMPONENT_WDMA1] = MT8173_MUTEX_MOD_DISP_WDMA1,
>> };
>>
>> +static const unsigned int mt8183_mutex_mod[DDP_COMPONENT_ID_MAX] = {
>> + [DDP_COMPONENT_AAL0] = MT8183_MUTEX_MOD_DISP_AAL0,
>> + [DDP_COMPONENT_CCORR] = MT8183_MUTEX_MOD_DISP_CCORR0,
>> + [DDP_COMPONENT_COLOR0] = MT8183_MUTEX_MOD_DISP_COLOR0,
>> + [DDP_COMPONENT_DITHER] = MT8183_MUTEX_MOD_DISP_DITHER0,
>> + [DDP_COMPONENT_GAMMA] = MT8183_MUTEX_MOD_DISP_GAMMA0,
>> + [DDP_COMPONENT_OVL0] = MT8183_MUTEX_MOD_DISP_OVL0,
>> + [DDP_COMPONENT_OVL_2L0] = MT8183_MUTEX_MOD_DISP_OVL0_2L,
>> + [DDP_COMPONENT_OVL_2L1] = MT8183_MUTEX_MOD_DISP_OVL1_2L,
>> + [DDP_COMPONENT_RDMA0] = MT8183_MUTEX_MOD_DISP_RDMA0,
>> + [DDP_COMPONENT_RDMA1] = MT8183_MUTEX_MOD_DISP_RDMA1,
>> + [DDP_COMPONENT_WDMA0] = MT8183_MUTEX_MOD_DISP_WDMA0,
>> +};
>> +
>> static const unsigned int mt2712_mutex_sof[MUTEX_SOF_DSI3 + 1] = {
>> [MUTEX_SOF_SINGLE_MODE] = MUTEX_SOF_SINGLE_MODE,
>> [MUTEX_SOF_DSI0] = MUTEX_SOF_DSI0,
>> @@ -198,6 +231,13 @@ static const unsigned int
>> mt8167_mutex_sof[MUTEX_SOF_DSI3 + 1] = {
>> [MUTEX_SOF_DPI1] = MT8167_MUTEX_SOF_DPI1,
>> };
>>
>> +/* Add EOF setting so overlay hardware can receive frame done irq */
>> +static const unsigned int mt8183_mutex_sof[MUTEX_SOF_DSI3 + 1] = {
>> + [MUTEX_SOF_SINGLE_MODE] = MUTEX_SOF_SINGLE_MODE,
>> + [MUTEX_SOF_DSI0] = MUTEX_SOF_DSI0 | MT8183_MUTEX_EOF_DSI0,
>> + [MUTEX_SOF_DPI0] = MT8183_MUTEX_SOF_DPI0 | MT8183_MUTEX_EOF_DPI0,
>> +};
>> +
>> static const struct mtk_mutex_data mt2701_mutex_driver_data = {
>> .mutex_mod = mt2701_mutex_mod,
>> .mutex_sof = mt2712_mutex_sof,
>> @@ -227,6 +267,14 @@ static const struct mtk_
Re: [Freedreno] [v2] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver
On 2021-02-01 00:46, Rob Clark wrote:
On Fri, Dec 18, 2020 at 2:27 AM Kalyan Thota
wrote:
Set the flag vblank_disable_immediate = true to turn off vblank irqs
immediately as soon as drm_vblank_put is requested so that there are
no irqs triggered during idle state. This will reduce cpu wakeups
and help in power saving.
To enable vblank_disable_immediate flag the underlying KMS driver
needs to support high precision vblank timestamping and also a
reliable way of providing vblank counter which is incrementing
at the leading edge of vblank.
This patch also brings in changes to support vblank_disable_immediate
requirement in dpu driver.
Changes in v1:
- Specify reason to add vblank timestamp support. (Rob)
- Add changes to provide vblank counter from dpu driver.
Signed-off-by: Kalyan Thota
This seems to be triggering:
[ +0.032668] [ cut here ]
[ +0.004759] msm ae0.mdss: drm_WARN_ON_ONCE(cur_vblank !=
vblank->last)
[ +0.24] WARNING: CPU: 0 PID: 362 at
drivers/gpu/drm/drm_vblank.c:354 drm_update_vblank_count+0x1e4/0x258
[ +0.017154] Modules linked in: joydev
[ +0.003784] CPU: 0 PID: 362 Comm: frecon Not tainted
5.11.0-rc5-00037-g33d3504871dd #2
[ +0.008135] Hardware name: Google Lazor (rev1 - 2) with LTE (DT)
[ +0.006167] pstate: 60400089 (nZCv daIf +PAN -UAO -TCO BTYPE=--)
[ +0.006169] pc : drm_update_vblank_count+0x1e4/0x258
[ +0.005105] lr : drm_update_vblank_count+0x1e4/0x258
[ +0.005106] sp : ffc010003b70
[ +0.003409] x29: ffc010003b70 x28: ff80855d9d98
[ +0.005466] x27: x26: 00fe502a
[ +0.005458] x25: 0001 x24: 0001
[ +0.005466] x23: 0001 x22: ff808561ce80
[ +0.005465] x21: x20:
[ +0.005468] x19: ff80850d6800 x18:
[ +0.005466] x17: x16:
[ +0.005465] x15: 000a x14: 263b
[ +0.005466] x13: 0006 x12:
[ +0.005465] x11: 0010 x10: ffc090003797
[ +0.005466] x9 : ffed200e2a8c x8 :
[ +0.005466] x7 : x6 : ffed213b2b51
[ +0.005465] x5 : c000dfff x4 : ffed21218048
[ +0.005465] x3 : x2 :
[ +0.005465] x1 : x0 :
[ +0.005466] Call trace:
[ +0.002520] drm_update_vblank_count+0x1e4/0x258
[ +0.004748] drm_handle_vblank+0xd0/0x35c
[ +0.004130] drm_crtc_handle_vblank+0x24/0x30
[ +0.004487] dpu_crtc_vblank_callback+0x3c/0xc4
[ +0.004662] dpu_encoder_vblank_callback+0x70/0xc4
[ +0.004931] dpu_encoder_phys_vid_vblank_irq+0x50/0x12c
[ +0.005378] dpu_core_irq_callback_handler+0xf4/0xfc
[ +0.005107] dpu_hw_intr_dispatch_irq+0x100/0x120
[ +0.004834] dpu_core_irq+0x44/0x5c
[ +0.003597] dpu_irq+0x1c/0x28
[ +0.003141] msm_irq+0x34/0x40
[ +0.003153] __handle_irq_event_percpu+0xfc/0x254
[ +0.004838] handle_irq_event_percpu+0x3c/0x94
[ +0.004574] handle_irq_event+0x54/0x98
[ +0.003944] handle_level_irq+0xa0/0xd0
[ +0.003943] generic_handle_irq+0x30/0x48
[ +0.004131] dpu_mdss_irq+0xe4/0x118
[ +0.003684] generic_handle_irq+0x30/0x48
[ +0.004127] __handle_domain_irq+0xa8/0xac
[ +0.004215] gic_handle_irq+0xdc/0x150
[ +0.003856] el1_irq+0xb4/0x180
[ +0.003237] dpu_encoder_vsync_time+0x78/0x230
[ +0.004574] dpu_encoder_kickoff+0x190/0x354
[ +0.004386] dpu_crtc_commit_kickoff+0x194/0x1a0
[ +0.004748] dpu_kms_flush_commit+0xf4/0x108
[ +0.004390] msm_atomic_commit_tail+0x2e8/0x384
[ +0.004661] commit_tail+0x80/0x108
[ +0.003588] drm_atomic_helper_commit+0x118/0x11c
[ +0.004834] drm_atomic_commit+0x58/0x68
[ +0.004033] drm_atomic_helper_set_config+0x70/0x9c
[ +0.005018] drm_mode_setcrtc+0x390/0x584
[ +0.004131] drm_ioctl_kernel+0xc8/0x11c
[ +0.004035] drm_ioctl+0x2f8/0x34c
[ +0.003500] drm_compat_ioctl+0x48/0xe8
[ +0.003945] __arm64_compat_sys_ioctl+0xe8/0x104
[ +0.004750] el0_svc_common.constprop.0+0x114/0x188
[ +0.005019] do_el0_svc_compat+0x28/0x38
[ +0.004031] el0_svc_compat+0x20/0x30
[ +0.003772] el0_sync_compat_handler+0x104/0x18c
[ +0.004749] el0_sync_compat+0x178/0x180
[ +0.004034] ---[ end trace 2959d178e74f2555 ]---
BR,
-R
Hi Rob,
on DPU HW, with prefetch enabled, the frame count increment and vsync
irq are not happening at same instance. This is causing the frame count
to mismatch.
Example:
|###--^--|###--^--|
for the above vsync cycle with prefetch enabled "^" --> marks a fetch
counter where in we are asking the hw to start fetching in the front
porch so that we will have more time to fetch data by first active line
of next frame.
In this case, the vsync irq will be triggered at fetch start marker
("^") so that double buffered updates are submitted to HW and the frame
count update will happen at the end of front porch ("|")
to handle this, can we fallback on the SW vblank counter
(drm_vblank_n
Re: [PATCH 5.10 078/120] drm/dp/mst: Export drm_dp_get_vc_payload_bw()
Hi! > commit 83404d581471775f37f85e5261ec0d09407d8bed upstream. > > This function will be needed by the next patch where the driver > calculates the BW based on driver specific parameters, so export it. > > At the same time sanitize the function params, passing the more natural > link rate instead of the encoding of the same rate. > Cc: This adds exports, but there's no user of the export, neither in 5.10-stable nor in linux-next. What is the plan here? Best regards, Pavel -- http://www.livejournal.com/~pavelmachek signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 8/9] drm/gem: Associate GEM objects with drm cgroup
On Wed, Feb 10, 2021 at 08:52:29AM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 09.02.21 um 11:54 schrieb Daniel Vetter:
> > *: vmwgfx is the only non-gem driver, but there's plans to move at least
> > vmwgfx internals (maybe not the uapi, we'll see) over to gem. Once that's
> > done it's truly all gpu memory.
>
> Do you have a URL to the discussion?
>
> While I recent worked on GEM, I thought that vmwgfx could easily switch to
> the GEM internals without adopting the interface.
Zack Rusin pinged me on irc I think, not sure there's anything on
dri-devel. zackr on freenode. I think he's working on this already.
> Personally, I think we should consider renaming struct drm_gem_object et al.
> It's not strictly GEM UAPI, but nowadays anything memory-related. Maybe
> drm_mem_object would fit.
I think abbrevations we've created that have been around for long enough
can stay. Otherwise we'd need to rename drm into something less confusing
too :-)
So gem just becomes the buffer based memory management thing in drm, which
is the accelerator and display framework in upstream. And ttm is our
memory manager for discrete gpus - ttm means translation table manager,
and that part just got moved out as a helper so drivers call we needed :-)
I mean we haven't even managed to rename the cma helpers to dma helpers.
The one thing we did manage is rename struct fence to struct dma_fence,
because no prefix was just really bad naming accident.
Cheers, Daniel
>
> Best regards
> Thomas
>
> > > ---
> > > drivers/gpu/drm/drm_gem.c | 89 +++
> > > include/drm/drm_gem.h | 17
> > > 2 files changed, 106 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > > index c2ce78c4edc3..a12da41eaafe 100644
> > > --- a/drivers/gpu/drm/drm_gem.c
> > > +++ b/drivers/gpu/drm/drm_gem.c
> > > @@ -29,6 +29,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -112,6 +113,89 @@ drm_gem_init(struct drm_device *dev)
> > > return drmm_add_action(dev, drm_gem_init_release, NULL);
> > > }
> > > +/**
> > > + * drm_gem_object_set_cgroup - associate GEM object with a cgroup
> > > + * @obj: GEM object which is being associated with a cgroup
> > > + * @task: task associated with process control group to use
> > > + *
> > > + * This will acquire a reference on cgroup and use for charging GEM
> > > + * memory allocations.
> > > + * This helper could be extended in future to migrate charges to another
> > > + * cgroup, print warning if this usage occurs.
> > > + */
> > > +void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
> > > +struct task_struct *task)
> > > +{
> > > + /* if object has existing cgroup, we migrate the charge... */
> > > + if (obj->drmcg) {
> > > + pr_warn("DRM: need to migrate cgroup charge of %lld\n",
> > > + atomic64_read(&obj->drmcg_bytes_charged));
> > > + }
> > > + obj->drmcg = drmcg_get(task);
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_object_set_cgroup);
> > > +
> > > +/**
> > > + * drm_gem_object_unset_cgroup - clear GEM object's associated cgroup
> > > + * @obj: GEM object
> > > + *
> > > + * This will release a reference on cgroup.
> > > + */
> > > +void drm_gem_object_unset_cgroup(struct drm_gem_object *obj)
> > > +{
> > > + WARN_ON(atomic64_read(&obj->drmcg_bytes_charged));
> > > + drmcg_put(obj->drmcg);
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_object_unset_cgroup);
> > > +
> > > +/**
> > > + * drm_gem_object_charge_mem - try charging size bytes to DRM cgroup
> > > + * @obj: GEM object which is being charged
> > > + * @size: number of bytes to charge
> > > + *
> > > + * Try to charge @size bytes to GEM object's associated DRM cgroup. This
> > > + * will fail if a successful charge would cause the current device memory
> > > + * usage to go above the cgroup's GPU memory maximum limit.
> > > + *
> > > + * Returns 0 on success. Otherwise, an error code is returned.
> > > + */
> > > +int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size)
> > > +{
> > > + int ret;
> > > +
> > > + ret = drm_cgroup_try_charge(obj->drmcg, obj->dev,
> > > + DRMCG_TYPE_MEM_CURRENT, size);
> > > + if (!ret)
> > > + atomic64_add(size, &obj->drmcg_bytes_charged);
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_object_charge_mem);
> > > +
> > > +/**
> > > + * drm_gem_object_uncharge_mem - uncharge size bytes from DRM cgroup
> > > + * @obj: GEM object which is being uncharged
> > > + * @size: number of bytes to uncharge
> > > + *
> > > + * Uncharge @size bytes from the DRM cgroup associated with specified
> > > + * GEM object.
> > > + *
> > > + * Returns 0 on success. Otherwise, an error code is returned.
> > > + */
> > > +void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size)
> > > +{
> > > + u64 charged = atomic64_read(&obj->drmcg_bytes_cha
Re: [PATCH 5.10 078/120] drm/dp/mst: Export drm_dp_get_vc_payload_bw()
Hi, On Wed, Feb 10, 2021 at 01:25:17PM +0100, Pavel Machek wrote: > Hi! > > > commit 83404d581471775f37f85e5261ec0d09407d8bed upstream. > > > > This function will be needed by the next patch where the driver > > calculates the BW based on driver specific parameters, so export it. > > > > At the same time sanitize the function params, passing the more natural > > link rate instead of the encoding of the same rate. > > > Cc: > > This adds exports, but there's no user of the export, neither in > 5.10-stable nor in linux-next. What is the plan here? the export is used by the upstream commit 882554042d138dbc6fb1a43017d0b9c3b38ee5f5 Author: Imre Deak Date: Mon Jan 25 19:36:36 2021 +0200 drm/i915: Fix the MST PBN divider calculation which I can also see now applied to 5.10.15: commit 0fe98e455784a6c11e0dd48612acd343f4a46fce Author: Imre Deak AuthorDate: Mon Jan 25 19:36:36 2021 +0200 Commit: Greg Kroah-Hartman CommitDate: Wed Feb 10 09:29:18 2021 +0100 drm/i915: Fix the MST PBN divider calculation commit 882554042d138dbc6fb1a43017d0b9c3b38ee5f5 upstream. --Imre ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/9] mm/migrate.c: Always allow device private pages to migrate
On Wed, Feb 10, 2021 at 02:40:10PM +1100, Alistair Popple wrote: > On Wednesday, 10 February 2021 12:39:32 AM AEDT Jason Gunthorpe wrote: > > On Tue, Feb 09, 2021 at 12:07:14PM +1100, Alistair Popple wrote: > > > Device private pages are used to represent device memory that is not > > > directly accessible from the CPU. Extra references to a device private > > > page are only used to ensure the struct page itself remains valid whilst > > > waiting for migration entries. Therefore extra references should not > > > prevent device private page migration as this can lead to failures to > > > migrate pages back to the CPU which are fatal to the user process. > > > > This should identify the extra references in expected_count, just > > disabling this protection seems unsafe, ZONE_DEVICE is not so special > > that the refcount means nothing > > This is similar to what migarte_vma_check_page() does now. The issue is that > a > migration wait takes a reference on the device private page so you can end up > with one thread stuck waiting for migration whilst the other can't migrate > due > to the extra refcount. > > Given device private pages can't undergo GUP and that it's not possible to > differentiate the migration wait refcount from any other refcount we assume > any possible extra reference must be from migration wait. GUP is not the only thing that elevates the refcount, I think this is an unsafe assumption Why is migration holding an extra refcount anyhow? Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] Add support for SVM atomics in Nouveau
On Tue, Feb 09, 2021 at 12:53:27PM -0800, John Hubbard wrote:
> On 2/9/21 5:37 AM, Daniel Vetter wrote:
> > On Tue, Feb 9, 2021 at 1:57 PM Alistair Popple wrote:
> > >
> > > On Tuesday, 9 February 2021 9:27:05 PM AEDT Daniel Vetter wrote:
> > > > >
> > > > > Recent changes to pin_user_pages() prevent the creation of pinned
> > > > > pages in
> > > > > ZONE_MOVABLE. This series allows pinned pages to be created in
> > > ZONE_MOVABLE
> > > > > as attempts to migrate may fail which would be fatal to userspace.
> > > > >
> > > > > In this case migration of the pinned page is unnecessary as the page
> > > > > can
> > > be
> > > > > unpinned at anytime by having the driver revoke atomic permission as
> > > > > it
> > > > > does for the migrate_to_ram() callback. However a method of calling
> > > > > this
> > > > > when memory needs to be moved has yet to be resolved so any
> > > > > discussion is
> > > > > welcome.
> > > >
> > > > Why do we need to pin for gpu atomics? You still have the callback for
> > > > cpu faults, so you
> > > > can move the page as needed, and hence a long-term pin sounds like the
> > > > wrong approach.
> > >
> > > Technically a real long term unmoveable pin isn't required, because as
> > > you say
> > > the page can be moved as needed at any time. However I needed some way of
> > > stopping the CPU page from being freed once the userspace mappings for it
> > > had
> > > been removed. Obviously I could have just used get_page() but from the
> > > perspective of page migration the result is much the same as a pin - a
> > > page
> > > which can't be moved because of the extra refcount.
> >
> > long term pin vs short term page reference aren't fully fleshed out.
> > But the rule more or less is:
> > - short term page reference: _must_ get released in finite time for
> > migration and other things, either because you have a callback, or
> > because it's just for direct I/O, which will complete. This means
> > short term pins will delay migration, but not foul it complete
>
>
> GPU atomic operations to sysmem are hard to categorize, because because
> application
> programmers could easily write programs that do a long series of atomic
> operations.
> Such a program would be a little weird, but it's hard to rule out.
Yeah, but we can forcefully break this whenever we feel like by revoking
the page, moving it, and then reinstating the gpu pte again and let it
continue.
If that's no possible then what we need here instead is an mlock() type of
thing I think.
> > - long term pin: the page cannot be moved, all migration must fail.
> > Also this will have an impact on COW behaviour for fork (but not sure
> > where those patches are, John Hubbard will know).
>
>
> That would be Jason's commit 57efa1fe59576 ("mm/gup: prevent gup_fast from
> racing
> with COW during fork"), which is in linux-next 20201216.
Nice, thanks for the pointer.
>
>
> >
> > So I think for your use case here you want a) short term page
> > reference to make sure it doesn't disappear plus b) callback to make
> > sure migrate isn't blocked.
> >
> > Breaking ZONE_MOVEABLE with either allowing long term pins or failing
> > migrations because you don't release your short term page reference
> > isn't good.
> >
> > > The normal solution of registering an MMU notifier to unpin the page when
> > > it
> > > needs to be moved also doesn't work as the CPU page tables now point to
> > > the
> > > device-private page and hence the migration code won't call any invalidate
> > > notifiers for the CPU page.
> >
> > Yeah you need some other callback for migration on the page directly.
> > it's a bit awkward since there is one already for struct
> > address_space, but that's own by the address_space/page cache, not
> > HMM. So I think we need something else, maybe something for each
> > ZONE_DEVICE?
> >
>
> This direction sounds at least...possible. Using MMU notifiers instead of pins
> is definitely appealing. I'm not quite clear on the callback idea above, but
> overall it seems like taking advantage of the ZONE_DEVICE tracking of pages
> (without having to put anything additional in each struct page), could work.
>
> Additional notes or ideas here are definitely welcome.
Well I don't have ideas for the details tbh, just from the little I
learned about how this all fits together pretending to be a pin while
pretending to not be a pin just gets us back to the mess we're trying to
solve with gup vs pup cleanup. And given that the pin_user_pages rollout
hasn't even completed yet it's maybe way to early to already toss it out
again.
I think overall we should try really hard to not mix up things between
memory that's pinned and memory that can be moved. Because retroactively
trying to fix things up just because it was easier to get a feature going
that way is very, very hard. And I think we've demonstrated countless
times that "ah we just fix this with pinning, it doesn't happen often, no
one will notice" alway
Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation
On Tue, Feb 09, 2021 at 12:16:51PM -0800, Suren Baghdasaryan wrote:
> On Tue, Feb 9, 2021 at 12:03 PM Daniel Vetter wrote:
> >
> > On Tue, Feb 9, 2021 at 6:46 PM Christian König
> > wrote:
> > >
> > >
> > >
> > > Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
> > > > On Tue, Feb 9, 2021 at 4:57 AM Christian König
> > > > wrote:
> > > >> Am 09.02.21 um 13:11 schrieb Christian König:
> > > >>> [SNIP]
> > > >> +void drm_page_pool_add(struct drm_page_pool *pool, struct page
> > > >> *page)
> > > >> +{
> > > >> + spin_lock(&pool->lock);
> > > >> + list_add_tail(&page->lru, &pool->items);
> > > >> + pool->count++;
> > > >> + atomic_long_add(1 << pool->order, &total_pages);
> > > >> + spin_unlock(&pool->lock);
> > > >> +
> > > >> + mod_node_page_state(page_pgdat(page),
> > > >> NR_KERNEL_MISC_RECLAIMABLE,
> > > >> + 1 << pool->order);
> > > > Hui what? What should that be good for?
> > > This is a carryover from the ION page pool implementation:
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cdff8edcd4d147a5b08d8cd20cff2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637484888114923580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9%2BIBC0tezSV6Ci4S3kWfW%2BQvJm4mdunn3dF6C0kyfCw%3D&reserved=0
> > >
> > >
> > > My sense is it helps with the vmstat/meminfo accounting so folks can
> > > see the cached pages are shrinkable/freeable. This maybe falls under
> > > other dmabuf accounting/stats discussions, so I'm happy to remove it
> > > for now, or let the drivers using the shared page pool logic handle
> > > the accounting themselves?
> > > >> Intentionally separated the discussion for that here.
> > > >>
> > > >> As far as I can see this is just bluntly incorrect.
> > > >>
> > > >> Either the page is reclaimable or it is part of our pool and freeable
> > > >> through the shrinker, but never ever both.
> > > > IIRC the original motivation for counting ION pooled pages as
> > > > reclaimable was to include them into /proc/meminfo's MemAvailable
> > > > calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
> > > > non-slab kernel pages" seems like a good place to account for them but
> > > > I might be wrong.
> > >
> > > Yeah, that's what I see here as well. But exactly that is utterly
> > > nonsense.
> > >
> > > Those pages are not "free" in the sense that get_free_page could return
> > > them directly.
> >
> > Well on Android that is kinda true, because Android has it's
> > oom-killer (way back it was just a shrinker callback, not sure how it
> > works now), which just shot down all the background apps. So at least
> > some of that (everything used by background apps) is indeed
> > reclaimable on Android.
> >
> > But that doesn't hold on Linux in general, so we can't really do this
> > for common code.
> >
> > Also I had a long meeting with Suren, John and other googles
> > yesterday, and the aim is now to try and support all the Android gpu
> > memory accounting needs with cgroups. That should work, and it will
> > allow Android to handle all the Android-ism in a clean way in upstream
> > code. Or that's at least the plan.
> >
> > I think the only thing we identified that Android still needs on top
> > is the dma-buf sysfs stuff, so that shared buffers (which on Android
> > are always dma-buf, and always stay around as dma-buf fd throughout
> > their lifetime) can be listed/analyzed with full detail.
> >
> > But aside from this the plan for all the per-process or per-heap
> > account, oom-killer integration and everything else is planned to be
> > done with cgroups.
>
> Until cgroups are ready we probably will need to add a sysfs node to
> report the total dmabuf pool size and I think that would cover our
> current accounting need here.
> As I mentioned, not including dmabuf pools into MemAvailable would
> affect that stat and I'm wondering if pools should be considered as
> part of MemAvailable or not. Since MemAvailable includes SReclaimable
> I think it makes sense to include them but maybe there are other
> considerations that I'm missing?
On Android, yes, on upstream, not so much. Because upstream doesn't have
the android low memory killer cleanup up all the apps, so effectively we
can't reclaim that memory, and we shouldn't report it as such.
-Daniel
>
> > Android (for now) only needs to account overall gpu
> > memory since none of it is swappable on android drivers anyway, plus
> > no vram, so not much needed.
> >
> > Cheers, Daniel
> >
> > >
> > > Regards,
> > > Christian.
> > >
> > > >
> > > >> In the best case this just messes up the accounting, in the worst case
> >
Re: [PATCH] drm/gem: Move drm_gem_fb_prepare_fb() to GEM atomic helpers
On Tue, Feb 09, 2021 at 11:29:13AM +0100, Thomas Zimmermann wrote:
> The function drm_gem_fb_prepare_fb() is a helper for atomic modesetting,
> but currently located next to framebuffer helpers. Move it to GEM atomic
> helpers, rename it slightly and adopt the drivers. Same for the rsp
> simple-pipe helper.
>
> Compile-tested with x86-64, aarch64 and arm. The patch is fairly large,
> but there are no functional changes.
>
> Signed-off-by: Thomas Zimmermann
If we bikeshed this, I think should also throw in the _helper_ somewhere?
But really I don't have an opinion on this.
-Daniel
> ---
> drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 4 +-
> drivers/gpu/drm/drm_gem_atomic_helper.c | 69 +++-
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 63 --
> drivers/gpu/drm/drm_gem_vram_helper.c| 4 +-
> drivers/gpu/drm/imx/dcss/dcss-plane.c| 4 +-
> drivers/gpu/drm/imx/ipuv3-plane.c| 4 +-
> drivers/gpu/drm/ingenic/ingenic-drm-drv.c| 3 +-
> drivers/gpu/drm/ingenic/ingenic-ipu.c| 4 +-
> drivers/gpu/drm/mcde/mcde_display.c | 4 +-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 6 +-
> drivers/gpu/drm/meson/meson_overlay.c| 8 +--
> drivers/gpu/drm/meson/meson_plane.c | 4 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c| 4 +-
> drivers/gpu/drm/msm/msm_atomic.c | 4 +-
> drivers/gpu/drm/mxsfb/mxsfb_kms.c| 6 +-
> drivers/gpu/drm/pl111/pl111_display.c| 4 +-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c| 4 +-
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 +-
> drivers/gpu/drm/stm/ltdc.c | 4 +-
> drivers/gpu/drm/sun4i/sun4i_layer.c | 4 +-
> drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 4 +-
> drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 4 +-
> drivers/gpu/drm/tegra/plane.c| 4 +-
> drivers/gpu/drm/tidss/tidss_plane.c | 4 +-
> drivers/gpu/drm/tiny/hx8357d.c | 4 +-
> drivers/gpu/drm/tiny/ili9225.c | 4 +-
> drivers/gpu/drm/tiny/ili9341.c | 4 +-
> drivers/gpu/drm/tiny/ili9486.c | 4 +-
> drivers/gpu/drm/tiny/mi0283qt.c | 4 +-
> drivers/gpu/drm/tiny/repaper.c | 3 +-
> drivers/gpu/drm/tiny/st7586.c| 4 +-
> drivers/gpu/drm/tiny/st7735r.c | 4 +-
> drivers/gpu/drm/tve200/tve200_display.c | 4 +-
> drivers/gpu/drm/vc4/vc4_plane.c | 4 +-
> drivers/gpu/drm/vkms/vkms_plane.c| 3 +-
> drivers/gpu/drm/xen/xen_drm_front_kms.c | 3 +-
> include/drm/drm_gem_atomic_helper.h | 8 +++
> include/drm/drm_gem_framebuffer_helper.h | 6 +-
> include/drm/drm_modeset_helper_vtables.h | 2 +-
> include/drm/drm_plane.h | 4 +-
> include/drm/drm_simple_kms_helper.h | 2 +-
> 41 files changed, 152 insertions(+), 141 deletions(-)
>
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> index e54686c31a90..d8f214e0be82 100644
> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> @@ -9,8 +9,8 @@
> #include
> #include
> #include
> +#include
> #include
> -#include
> #include
> #include
> #include
> @@ -219,7 +219,7 @@ static const struct drm_simple_display_pipe_funcs
> aspeed_gfx_funcs = {
> .enable = aspeed_gfx_pipe_enable,
> .disable= aspeed_gfx_pipe_disable,
> .update = aspeed_gfx_pipe_update,
> - .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> + .prepare_fb = drm_gem_simple_display_pipe_prepare_fb,
> .enable_vblank = aspeed_gfx_enable_vblank,
> .disable_vblank = aspeed_gfx_disable_vblank,
> };
> diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c
> b/drivers/gpu/drm/drm_gem_atomic_helper.c
> index e27762cef360..c656b40656bf 100644
> --- a/drivers/gpu/drm/drm_gem_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
> @@ -1,6 +1,10 @@
> // SPDX-License-Identifier: GPL-2.0-or-later
>
> +#include
> +
> #include
> +#include
> +#include
> #include
> #include
> #include
> @@ -12,10 +16,69 @@
> *
> * The GEM atomic helpers library implements generic atomic-commit
> * functions for drivers that use GEM objects. Currently, it provides
> - * plane state and framebuffer BO mappings for planes with shadow
> - * buffers.
> + * synchronization helpers, and plane state and framebuffer BO mappings
> + * for planes with shadow buffers.
> + */
> +
> +/*
> + * Plane Helpers
> */
>
> +/**
> + * drm_gem_prepare_fb() - Prepare a GEM backed framebuffer
> + * @plane: Plane
> + * @state: Plane state the fence will be attached to
> + *
> + * This function extracts the exclusive fence from &drm_gem_object.resv and
> + * attaches it to plane state for the atomic helper to wait on. This is
> + * necessar
Re: [PATCH] drm: use getter/setter functions
On Tue, Feb 09, 2021 at 10:13:04PM +0100, Julia Lawall wrote:
> Use getter and setter functions, for platform_device structures and a
> mipi_dsi_device structure.
>
> Signed-off-by: Julia Lawall
Applied to drm-misc-next, thanks for the patch.
-Daniel
>
> ---
> drivers/gpu/drm/aspeed/aspeed_gfx_drv.c |2 +-
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c |2 +-
> drivers/gpu/drm/panel/panel-lvds.c |2 +-
> drivers/gpu/drm/panel/panel-seiko-43wvf1g.c |4 ++--
> drivers/gpu/drm/panel/panel-simple.c|2 +-
> drivers/gpu/drm/rockchip/rockchip_lvds.c|2 +-
> 6 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 4e2dad314c79..9858079f9e14 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -4800,7 +4800,7 @@ static int panel_simple_dsi_probe(struct
> mipi_dsi_device *dsi)
>
> err = mipi_dsi_attach(dsi);
> if (err) {
> - struct panel_simple *panel = dev_get_drvdata(&dsi->dev);
> + struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
>
> drm_panel_remove(&panel->base);
> }
> diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> index 0ee508576231..3939b25e 100644
> --- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> +++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> @@ -267,7 +267,7 @@ static int seiko_panel_probe(struct device *dev,
>
> static int seiko_panel_remove(struct platform_device *pdev)
> {
> - struct seiko_panel *panel = dev_get_drvdata(&pdev->dev);
> + struct seiko_panel *panel = platform_get_drvdata(pdev);
>
> drm_panel_remove(&panel->base);
> drm_panel_disable(&panel->base);
> @@ -277,7 +277,7 @@ static int seiko_panel_remove(struct platform_device
> *pdev)
>
> static void seiko_panel_shutdown(struct platform_device *pdev)
> {
> - struct seiko_panel *panel = dev_get_drvdata(&pdev->dev);
> + struct seiko_panel *panel = platform_get_drvdata(pdev);
>
> drm_panel_disable(&panel->base);
> }
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 654bc52d9ff3..bd5ba10822c2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> @@ -725,7 +725,7 @@ static int rockchip_lvds_probe(struct platform_device
> *pdev)
>
> static int rockchip_lvds_remove(struct platform_device *pdev)
> {
> - struct rockchip_lvds *lvds = dev_get_drvdata(&pdev->dev);
> + struct rockchip_lvds *lvds = platform_get_drvdata(pdev);
>
> component_del(&pdev->dev, &rockchip_lvds_component_ops);
> clk_unprepare(lvds->pclk);
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> index 457ec04950f7..c7707338bfdb 100644
> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> @@ -284,7 +284,7 @@ static int aspeed_gfx_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - dev_set_drvdata(&pdev->dev, priv);
> + platform_set_drvdata(pdev, priv);
>
> ret = sysfs_create_group(&pdev->dev.kobj, &aspeed_sysfs_attr_group);
> if (ret)
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index d0c65610ebb5..989a05bc8197 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -2457,7 +2457,7 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
>
> static int cdns_mhdp_remove(struct platform_device *pdev)
> {
> - struct cdns_mhdp_device *mhdp = dev_get_drvdata(&pdev->dev);
> + struct cdns_mhdp_device *mhdp = platform_get_drvdata(pdev);
> unsigned long timeout = msecs_to_jiffies(100);
> bool stop_fw = false;
> int ret;
> diff --git a/drivers/gpu/drm/panel/panel-lvds.c
> b/drivers/gpu/drm/panel/panel-lvds.c
> index 66c7d765b8f7..59a8d99e777d 100644
> --- a/drivers/gpu/drm/panel/panel-lvds.c
> +++ b/drivers/gpu/drm/panel/panel-lvds.c
> @@ -244,7 +244,7 @@ static int panel_lvds_probe(struct platform_device *pdev)
>
> static int panel_lvds_remove(struct platform_device *pdev)
> {
> - struct panel_lvds *lvds = dev_get_drvdata(&pdev->dev);
> + struct panel_lvds *lvds = platform_get_drvdata(pdev);
>
> drm_panel_remove(&lvds->panel);
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: use getter/setter functions
On Wed, Feb 10, 2021 at 08:23:41AM +, Lee Jones wrote:
> On Tue, 09 Feb 2021, Julia Lawall wrote:
>
> > Use getter and setter functions, for platform_device structures and a
> > spi_device structure.
> >
> > Signed-off-by: Julia Lawall
> >
> > ---
> > drivers/video/backlight/qcom-wled.c |
> > 2 +-
>
> This patch is fine.
>
> Could you please split it out and submit it separately though please.
Or just apply the entire patch through backlight tree, there's nothing
going on in fbdev anyway I think.
Acked-by: Daniel Vetter
>
> > drivers/video/fbdev/amifb.c |
> > 4 ++--
> > drivers/video/fbdev/da8xx-fb.c |
> > 4 ++--
> > drivers/video/fbdev/imxfb.c |
> > 2 +-
> > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c |
> > 6 +++---
> > drivers/video/fbdev/omap2/omapfb/dss/dpi.c |
> > 4 ++--
> > drivers/video/fbdev/omap2/omapfb/dss/dsi.c |
> > 4 ++--
> > drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c |
> > 2 +-
> > drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c |
> > 2 +-
> > drivers/video/fbdev/xilinxfb.c |
> > 2 +-
> > 10 files changed, 16 insertions(+), 16 deletions(-)
>
> ...]
>
> > diff --git a/drivers/video/backlight/qcom-wled.c
> > b/drivers/video/backlight/qcom-wled.c
> > index 3bc7800eb0a9..091f07e7c145 100644
> > --- a/drivers/video/backlight/qcom-wled.c
> > +++ b/drivers/video/backlight/qcom-wled.c
> > @@ -1692,7 +1692,7 @@ static int wled_probe(struct platform_device *pdev)
> >
> > static int wled_remove(struct platform_device *pdev)
> > {
> > - struct wled *wled = dev_get_drvdata(&pdev->dev);
> > + struct wled *wled = platform_get_drvdata(pdev);
> >
> > mutex_destroy(&wled->lock);
> > cancel_delayed_work_sync(&wled->ovp_work);
>
> For my own reference (apply this as-is to your sign-off block):
>
> Acked-for-Backlight-by: Lee Jones
>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm/ttm: constify static vm_operations_structs
On Wed, Feb 10, 2021 at 08:45:56AM +0100, Christian König wrote: > Reviewed-by: Christian König for the series. Smash it into -misc? -Daniel > > Am 10.02.21 um 00:48 schrieb Rikard Falkeborn: > > Constify a few static vm_operations_struct that are never modified. Their > > only usage is to assign their address to the vm_ops field in the > > vm_area_struct, which is a pointer to const vm_operations_struct. Make them > > const to allow the compiler to put them in read-only memory. > > > > With this series applied, all static struct vm_operations_struct in the > > kernel tree are const. > > > > Rikard Falkeborn (3): > >drm/amdgpu/ttm: constify static vm_operations_struct > >drm/radeon/ttm: constify static vm_operations_struct > >drm/nouveau/ttm: constify static vm_operations_struct > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- > > drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 +- > > drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- > > 3 files changed, 3 insertions(+), 3 deletions(-) > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: use getter/setter functions
On Wed, 10 Feb 2021, Daniel Vetter wrote:
> On Wed, Feb 10, 2021 at 08:23:41AM +, Lee Jones wrote:
> > On Tue, 09 Feb 2021, Julia Lawall wrote:
> >
> > > Use getter and setter functions, for platform_device structures and a
> > > spi_device structure.
> > >
> > > Signed-off-by: Julia Lawall
> > >
> > > ---
> > > drivers/video/backlight/qcom-wled.c |
> > > 2 +-
> >
> > This patch is fine.
> >
> > Could you please split it out and submit it separately though please.
>
> Or just apply the entire patch through backlight tree, there's nothing
> going on in fbdev anyway I think.
I was indeed not sure how much to split this up. If it is desired to
split it more, I can do that.
julia
>
> Acked-by: Daniel Vetter
>
> >
> > > drivers/video/fbdev/amifb.c |
> > > 4 ++--
> > > drivers/video/fbdev/da8xx-fb.c |
> > > 4 ++--
> > > drivers/video/fbdev/imxfb.c |
> > > 2 +-
> > > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c |
> > > 6 +++---
> > > drivers/video/fbdev/omap2/omapfb/dss/dpi.c |
> > > 4 ++--
> > > drivers/video/fbdev/omap2/omapfb/dss/dsi.c |
> > > 4 ++--
> > > drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c |
> > > 2 +-
> > > drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c |
> > > 2 +-
> > > drivers/video/fbdev/xilinxfb.c |
> > > 2 +-
> > > 10 files changed, 16 insertions(+), 16 deletions(-)
> >
> > ...]
> >
> > > diff --git a/drivers/video/backlight/qcom-wled.c
> > > b/drivers/video/backlight/qcom-wled.c
> > > index 3bc7800eb0a9..091f07e7c145 100644
> > > --- a/drivers/video/backlight/qcom-wled.c
> > > +++ b/drivers/video/backlight/qcom-wled.c
> > > @@ -1692,7 +1692,7 @@ static int wled_probe(struct platform_device *pdev)
> > >
> > > static int wled_remove(struct platform_device *pdev)
> > > {
> > > - struct wled *wled = dev_get_drvdata(&pdev->dev);
> > > + struct wled *wled = platform_get_drvdata(pdev);
> > >
> > > mutex_destroy(&wled->lock);
> > > cancel_delayed_work_sync(&wled->ovp_work);
> >
> > For my own reference (apply this as-is to your sign-off block):
> >
> > Acked-for-Backlight-by: Lee Jones
> >
> > --
> > Lee Jones [李琼斯]
> > Senior Technical Lead - Developer Services
> > Linaro.org │ Open source software for Arm SoCs
> > Follow Linaro: Facebook | Twitter | Blog
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY"
On Tue, Feb 09, 2021 at 04:14:01PM -0800, Manasi Navare wrote:
> This reverts commit fb6473a48b635c55d04eb94e579eede52ef39550.
>
> These additional checks added to avoid EBUSY give unnecessary WARN_ON
> in case of big joiner used in i915 in which case even if the modeset
> is requested on a single pipe, internally another consecutive
> pipe is stolen and used to drive half of the transcoder timings.
> So in this case it is expected that requested crtc and affected crtcs
> do not match. Hence the added WARN ON becomes irrelevant.
> But there is no easy solution to get the bigjoiner information
> here at drm level. So for now revert this until we work out
> a better solution.
>
> Cc: Ville Syrjala
> Cc: Daniel Vetter
> Signed-off-by: Manasi Navare
Nope. We can maybe rework this so that i915 can do stuff under the hood,
but wrt uapi this was the thing we discussed with compositors. Without
such a guarantee atomic is defacto broken from a compositor pov.
This WARN_ON is not unecessary, compositor people really do not want the
kernel to throw around spurious EBUSY they have no visibility into.
Please also cc all the compositor people from my original patch if you
change anything in this area.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 29 -
> 1 file changed, 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index b1efa9322be2..48b2262d69f6 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -320,10 +320,6 @@ EXPORT_SYMBOL(__drm_atomic_state_free);
> * needed. It will also grab the relevant CRTC lock to make sure that the
> state
> * is consistent.
> *
> - * WARNING: Drivers may only add new CRTC states to a @state if
> - * drm_atomic_state.allow_modeset is set, or if it's a driver-internal commit
> - * not created by userspace through an IOCTL call.
> - *
> * Returns:
> *
> * Either the allocated state or the error code encoded into the pointer.
> When
> @@ -1306,15 +1302,10 @@ int drm_atomic_check_only(struct drm_atomic_state
> *state)
> struct drm_crtc_state *new_crtc_state;
> struct drm_connector *conn;
> struct drm_connector_state *conn_state;
> - unsigned requested_crtc = 0;
> - unsigned affected_crtc = 0;
> int i, ret = 0;
>
> DRM_DEBUG_ATOMIC("checking %p\n", state);
>
> - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
> - requested_crtc |= drm_crtc_mask(crtc);
> -
> for_each_oldnew_plane_in_state(state, plane, old_plane_state,
> new_plane_state, i) {
> ret = drm_atomic_plane_check(old_plane_state, new_plane_state);
> if (ret) {
> @@ -1362,26 +1353,6 @@ int drm_atomic_check_only(struct drm_atomic_state
> *state)
> }
> }
>
> - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
> - affected_crtc |= drm_crtc_mask(crtc);
> -
> - /*
> - * For commits that allow modesets drivers can add other CRTCs to the
> - * atomic commit, e.g. when they need to reallocate global resources.
> - * This can cause spurious EBUSY, which robs compositors of a very
> - * effective sanity check for their drawing loop. Therefor only allow
> - * drivers to add unrelated CRTC states for modeset commits.
> - *
> - * FIXME: Should add affected_crtc mask to the ATOMIC IOCTL as an output
> - * so compositors know what's going on.
> - */
> - if (affected_crtc != requested_crtc) {
> - DRM_DEBUG_ATOMIC("driver added CRTC to commit: requested 0x%x,
> affected 0x%0x\n",
> - requested_crtc, affected_crtc);
> - WARN(!state->allow_modeset, "adding CRTC not allowed without
> modesets: requested 0x%x, affected 0x%0x\n",
> - requested_crtc, affected_crtc);
> - }
> -
> return 0;
> }
> EXPORT_SYMBOL(drm_atomic_check_only);
> --
> 2.19.1
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY"
On Wednesday, February 10th, 2021 at 2:16 PM, Daniel Vetter wrote: > On Tue, Feb 09, 2021 at 04:14:01PM -0800, Manasi Navare wrote: > > > These additional checks added to avoid EBUSY give unnecessary WARN_ON > > in case of big joiner used in i915 in which case even if the modeset > > is requested on a single pipe, internally another consecutive > > pipe is stolen and used to drive half of the transcoder timings. > > So in this case it is expected that requested crtc and affected crtcs > > do not match. Hence the added WARN ON becomes irrelevant. The WARN_ON only happens if allow_modeset == false. If allow_modeset == true, then the driver is allowed to steal an unrelated pipe. Maybe i915 is stealing a pipe without allow_modeset? > Nope. We can maybe rework this so that i915 can do stuff under the hood, > but wrt uapi this was the thing we discussed with compositors. Without > such a guarantee atomic is defacto broken from a compositor pov. Agreed. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-fixes
Hi Dave and Daniel, here's this week's PR for drm-misc-fixes. There's a buffer overflow in vc4 and a memory leak in xlnx. The rest appear to be mere bug fixes. Best regards Thomas drm-misc-fixes-2021-02-10: * dp_mst: Don't report un-attached ports as connected * sun4i: tcon1 sync polarity fix; Always set HDMI clock rate; Fix H6 HDMI PHY config; Fix H6 max frequency * vc4: Fix buffer overflow * xlnx: Fix memory leak The following changes since commit 2b1b3e544f65f40df5eef99753e460a127910479: drm/ttm: Use __GFP_NOWARN for huge pages in ttm_pool_alloc_page (2021-01-28 13:01:52 +0100) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2021-02-10 for you to fetch changes up to 1926a0508d8947cf081280d85ff035300dc71da7: drm/sun4i: dw-hdmi: Fix max. frequency for H6 (2021-02-10 11:20:38 +0100) * dp_mst: Don't report un-attached ports as connected * sun4i: tcon1 sync polarity fix; Always set HDMI clock rate; Fix H6 HDMI PHY config; Fix H6 max frequency * vc4: Fix buffer overflow * xlnx: Fix memory leak Imre Deak (1): drm/dp_mst: Don't report ports connected if nothing is attached to them Jernej Skrabec (4): drm/sun4i: tcon: set sync polarity for tcon1 channel drm/sun4i: dw-hdmi: always set clock rate drm/sun4i: Fix H6 HDMI PHY configuration drm/sun4i: dw-hdmi: Fix max. frequency for H6 Maxime Ripard (1): drm/vc4: hvs: Fix buffer overflow with the dlist handling Quanyang Wang (1): drm/xlnx: fix kmemleak by sending vblank_event in atomic_disable drivers/gpu/drm/drm_dp_mst_topology.c | 1 + drivers/gpu/drm/sun4i/sun4i_tcon.c | 25 + drivers/gpu/drm/sun4i/sun4i_tcon.h | 6 ++ drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 10 +++--- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 - drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 26 +- drivers/gpu/drm/vc4/vc4_plane.c| 18 ++ drivers/gpu/drm/xlnx/zynqmp_disp.c | 15 +++ 8 files changed, 65 insertions(+), 37 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/gem: Move drm_gem_fb_prepare_fb() to GEM atomic helpers
Hi
Am 10.02.21 um 14:10 schrieb Daniel Vetter:
On Tue, Feb 09, 2021 at 11:29:13AM +0100, Thomas Zimmermann wrote:
The function drm_gem_fb_prepare_fb() is a helper for atomic modesetting,
but currently located next to framebuffer helpers. Move it to GEM atomic
helpers, rename it slightly and adopt the drivers. Same for the rsp
simple-pipe helper.
Compile-tested with x86-64, aarch64 and arm. The patch is fairly large,
but there are no functional changes.
Signed-off-by: Thomas Zimmermann
If we bikeshed this, I think should also throw in the _helper_ somewhere?
Sure. How about drm_gem_plane_helper_prepare_fb()?
Best regards
Thomas
But really I don't have an opinion on this.
-Daniel
---
drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 4 +-
drivers/gpu/drm/drm_gem_atomic_helper.c | 69 +++-
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 63 --
drivers/gpu/drm/drm_gem_vram_helper.c| 4 +-
drivers/gpu/drm/imx/dcss/dcss-plane.c| 4 +-
drivers/gpu/drm/imx/ipuv3-plane.c| 4 +-
drivers/gpu/drm/ingenic/ingenic-drm-drv.c| 3 +-
drivers/gpu/drm/ingenic/ingenic-ipu.c| 4 +-
drivers/gpu/drm/mcde/mcde_display.c | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 6 +-
drivers/gpu/drm/meson/meson_overlay.c| 8 +--
drivers/gpu/drm/meson/meson_plane.c | 4 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c| 4 +-
drivers/gpu/drm/msm/msm_atomic.c | 4 +-
drivers/gpu/drm/mxsfb/mxsfb_kms.c| 6 +-
drivers/gpu/drm/pl111/pl111_display.c| 4 +-
drivers/gpu/drm/rcar-du/rcar_du_vsp.c| 4 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 +-
drivers/gpu/drm/stm/ltdc.c | 4 +-
drivers/gpu/drm/sun4i/sun4i_layer.c | 4 +-
drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 4 +-
drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 4 +-
drivers/gpu/drm/tegra/plane.c| 4 +-
drivers/gpu/drm/tidss/tidss_plane.c | 4 +-
drivers/gpu/drm/tiny/hx8357d.c | 4 +-
drivers/gpu/drm/tiny/ili9225.c | 4 +-
drivers/gpu/drm/tiny/ili9341.c | 4 +-
drivers/gpu/drm/tiny/ili9486.c | 4 +-
drivers/gpu/drm/tiny/mi0283qt.c | 4 +-
drivers/gpu/drm/tiny/repaper.c | 3 +-
drivers/gpu/drm/tiny/st7586.c| 4 +-
drivers/gpu/drm/tiny/st7735r.c | 4 +-
drivers/gpu/drm/tve200/tve200_display.c | 4 +-
drivers/gpu/drm/vc4/vc4_plane.c | 4 +-
drivers/gpu/drm/vkms/vkms_plane.c| 3 +-
drivers/gpu/drm/xen/xen_drm_front_kms.c | 3 +-
include/drm/drm_gem_atomic_helper.h | 8 +++
include/drm/drm_gem_framebuffer_helper.h | 6 +-
include/drm/drm_modeset_helper_vtables.h | 2 +-
include/drm/drm_plane.h | 4 +-
include/drm/drm_simple_kms_helper.h | 2 +-
41 files changed, 152 insertions(+), 141 deletions(-)
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
index e54686c31a90..d8f214e0be82 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
@@ -9,8 +9,8 @@
#include
#include
#include
+#include
#include
-#include
#include
#include
#include
@@ -219,7 +219,7 @@ static const struct drm_simple_display_pipe_funcs
aspeed_gfx_funcs = {
.enable = aspeed_gfx_pipe_enable,
.disable= aspeed_gfx_pipe_disable,
.update = aspeed_gfx_pipe_update,
- .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+ .prepare_fb = drm_gem_simple_display_pipe_prepare_fb,
.enable_vblank = aspeed_gfx_enable_vblank,
.disable_vblank = aspeed_gfx_disable_vblank,
};
diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c
b/drivers/gpu/drm/drm_gem_atomic_helper.c
index e27762cef360..c656b40656bf 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -1,6 +1,10 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#include
+
#include
+#include
+#include
#include
#include
#include
@@ -12,10 +16,69 @@
*
* The GEM atomic helpers library implements generic atomic-commit
* functions for drivers that use GEM objects. Currently, it provides
- * plane state and framebuffer BO mappings for planes with shadow
- * buffers.
+ * synchronization helpers, and plane state and framebuffer BO mappings
+ * for planes with shadow buffers.
+ */
+
+/*
+ * Plane Helpers
*/
+/**
+ * drm_gem_prepare_fb() - Prepare a GEM backed framebuffer
+ * @plane: Plane
+ * @state: Plane state the fence will be attached to
+ *
+ * This function extracts the exclusive fence from &drm_gem_object.resv and
+ * attaches it to plane state for the atomic helper to wait on. This is
+ * necessary to correctly implement i
Re: linux-next: build failure after merge of the drm-misc tree
Op 2021-02-10 om 04:11 schreef Stephen Rothwell: > Hi all, > > After merging the drm-misc tree, today's linux-next build (x86_64 > allmodconfig) failed like this: > > drivers/gpu/drm/v3d/v3d_sched.c:263:1: error: return type is an incomplete > type > 263 | v3d_gpu_reset_for_timeout(struct v3d_dev *v3d, struct drm_sched_job > *sched_job) > | ^ > drivers/gpu/drm/v3d/v3d_sched.c: In function 'v3d_gpu_reset_for_timeout': > drivers/gpu/drm/v3d/v3d_sched.c:289:9: error: 'return' with a value, in > function returning void [-Werror=return-type] > 289 | return DRM_GPU_SCHED_STAT_NOMINAL; > | ^~ > drivers/gpu/drm/v3d/v3d_sched.c:263:1: note: declared here > 263 | v3d_gpu_reset_for_timeout(struct v3d_dev *v3d, struct drm_sched_job > *sched_job) > | ^ > drivers/gpu/drm/v3d/v3d_sched.c: At top level: > drivers/gpu/drm/v3d/v3d_sched.c:298:1: error: return type is an incomplete > type > 298 | v3d_cl_job_timedout(struct drm_sched_job *sched_job, enum v3d_queue q, > | ^~~ > drivers/gpu/drm/v3d/v3d_sched.c: In function 'v3d_cl_job_timedout': > drivers/gpu/drm/v3d/v3d_sched.c:309:10: error: 'return' with a value, in > function returning void [-Werror=return-type] > 309 | return DRM_GPU_SCHED_STAT_NOMINAL; > | ^~ > drivers/gpu/drm/v3d/v3d_sched.c:298:1: note: declared here > 298 | v3d_cl_job_timedout(struct drm_sched_job *sched_job, enum v3d_queue q, > | ^~~ > drivers/gpu/drm/v3d/v3d_sched.c: At top level: > drivers/gpu/drm/v3d/v3d_sched.c:316:1: error: return type is an incomplete > type > 316 | v3d_bin_job_timedout(struct drm_sched_job *sched_job) > | ^~~~ > drivers/gpu/drm/v3d/v3d_sched.c:325:1: error: return type is an incomplete > type > 325 | v3d_render_job_timedout(struct drm_sched_job *sched_job) > | ^~~ > drivers/gpu/drm/v3d/v3d_sched.c:334:1: error: return type is an incomplete > type > 334 | v3d_generic_job_timedout(struct drm_sched_job *sched_job) > | ^~~~ > drivers/gpu/drm/v3d/v3d_sched.c:342:1: error: return type is an incomplete > type > 342 | v3d_csd_job_timedout(struct drm_sched_job *sched_job) > | ^~~~ > drivers/gpu/drm/v3d/v3d_sched.c: In function 'v3d_csd_job_timedout': > drivers/gpu/drm/v3d/v3d_sched.c:353:10: error: 'return' with a value, in > function returning void [-Werror=return-type] > 353 | return DRM_GPU_SCHED_STAT_NOMINAL; > | ^~ > drivers/gpu/drm/v3d/v3d_sched.c:342:1: note: declared here > 342 | v3d_csd_job_timedout(struct drm_sched_job *sched_job) > | ^~~~ > drivers/gpu/drm/v3d/v3d_sched.c: At top level: > drivers/gpu/drm/v3d/v3d_sched.c:362:18: error: initialization of 'enum > drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer > type 'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types] > 362 | .timedout_job = v3d_bin_job_timedout, > | ^~~~ > drivers/gpu/drm/v3d/v3d_sched.c:362:18: note: (near initialization for > 'v3d_bin_sched_ops.timedout_job') > drivers/gpu/drm/v3d/v3d_sched.c:369:18: error: initialization of 'enum > drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer > type 'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types] > 369 | .timedout_job = v3d_render_job_timedout, > | ^~~ > drivers/gpu/drm/v3d/v3d_sched.c:369:18: note: (near initialization for > 'v3d_render_sched_ops.timedout_job') > drivers/gpu/drm/v3d/v3d_sched.c:376:18: error: initialization of 'enum > drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer > type 'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types] > 376 | .timedout_job = v3d_generic_job_timedout, > | ^~~~ > drivers/gpu/drm/v3d/v3d_sched.c:376:18: note: (near initialization for > 'v3d_tfu_sched_ops.timedout_job') > drivers/gpu/drm/v3d/v3d_sched.c:383:18: error: initialization of 'enum > drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer > type 'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types] > 383 | .timedout_job = v3d_csd_job_timedout, > | ^~~~ > drivers/gpu/drm/v3d/v3d_sched.c:383:18: note: (near initialization for > 'v3d_csd_sched_ops.timedout_job') > drivers/gpu/drm/v3d/v3d_sched.c:390:18: error: initialization of 'enum > drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer > type 'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types] > 390 | .timedout_job = v3d_generic_job_timedout, > | ^~~~ > drivers/gpu/drm/v3d/v3d_sched.c:390:18: note: (near initia
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- Please attach your xorg log (if using X) and your dmesg output. -- You may reply to this email to add a comment. 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 v2 14/15] ARM: dts: bcm2711: Add the BSC interrupt controller
Hi Dave, On Tue, Feb 09, 2021 at 09:49:05AM +, Dave Stevenson wrote: > On Mon, 11 Jan 2021 at 14:23, Maxime Ripard wrote: > > > > The BSC controllers used for the HDMI DDC have an interrupt controller > > shared between both instances. Let's add it to avoid polling. > > This seems to have unintended side effects. > GIC interrupt 117 is shared between the standard I2C controllers > (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts. > > Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that > doesn't appear to be an option for l2-intc registering as an interrupt > controller. i2c-bcm2835 therefore loses out and fails to register for > the interrupt. > > Is there an equivalent flag that an interrupt controller can add to > say that the parent interrupt is shared? Is that even supported? Indeed, it looks like setting an equivalent to IRQF_SHARED would be the solution, but I couldn't find anything that would allow us to in the irqchip code. Marc, Thomas, is it something that is allowed? Thanks! Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) changed: What|Removed |Added CC||nicholas.kazlaus...@amd.com --- Comment #2 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) --- Please reproduce the blackscreen issue and attach the following logs/information: - dmesg.log (with drm.debug=6 set as part of your modeline) - Xorg.0.log if using X - The name of your desktop environment - eg. GNOME, KDE, etc. I'm unable to reproduce this locally on my end and it doesn't seem to have applied generically to DCN21/DCN30 per Dan's test report. -- You may reply to this email to add a comment. 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: [Intel-gfx] [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY"
On Wed, Feb 10, 2021 at 01:38:45PM +, Simon Ser wrote: > On Wednesday, February 10th, 2021 at 2:16 PM, Daniel Vetter > wrote: > > > On Tue, Feb 09, 2021 at 04:14:01PM -0800, Manasi Navare wrote: > > > > > These additional checks added to avoid EBUSY give unnecessary WARN_ON > > > in case of big joiner used in i915 in which case even if the modeset > > > is requested on a single pipe, internally another consecutive > > > pipe is stolen and used to drive half of the transcoder timings. > > > So in this case it is expected that requested crtc and affected crtcs > > > do not match. Hence the added WARN ON becomes irrelevant. > > The WARN_ON only happens if allow_modeset == false. If allow_modeset == true, > then the driver is allowed to steal an unrelated pipe. > > Maybe i915 is stealing a pipe without allow_modeset? No. All page flips etc. will have to get split up internally between multiple crtcs. So I think there's basically three options: a) massive rewrite of i915 to bypass even more of drm_atomic stuff b) allow i915 to silence that warning, which opens up the question whether the warn is doing any good if it can just be bypassed c) nuke the warning entirely a) is not going to happen, and it would any way allow i915 to do things any which way it wants without tripping the warn, rendering the warn entirely toothless. Hmm. Maybe there is a d) which would be to ignore all crtcs that are not logically enabled in the warn? Not sure if that could allow something to slit through that people want it to catch? -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/ttm: move swapout logic around
Move the iteration of the global lru into the new function
ttm_global_swapout() and use that instead in drivers.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c| 57 -
drivers/gpu/drm/ttm/ttm_device.c| 29 +++
drivers/gpu/drm/ttm/ttm_tt.c| 2 +-
drivers/gpu/drm/vmwgfx/ttm_memory.c | 3 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
include/drm/ttm/ttm_bo_api.h| 3 +-
include/drm/ttm/ttm_device.h| 2 +
7 files changed, 53 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index e38102282fd5..d33578a112b4 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1184,56 +1184,35 @@ int ttm_bo_wait(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_bo_wait);
-/*
- * A buffer object shrink method that tries to swap out the first
- * buffer object on the bo_global::swap_lru list.
- */
-int ttm_bo_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags)
+int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
+ gfp_t gfp_flags)
{
struct ttm_global *glob = &ttm_glob;
- struct ttm_buffer_object *bo;
- int ret = -EBUSY;
bool locked;
- unsigned i;
-
- spin_lock(&glob->lru_lock);
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(bo, &glob->swap_lru[i], swap) {
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked,
- NULL))
- continue;
-
- if (!ttm_bo_get_unless_zero(bo)) {
- if (locked)
- dma_resv_unlock(bo->base.resv);
- continue;
- }
+ int ret;
- ret = 0;
- break;
- }
- if (!ret)
- break;
- }
+ if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked, NULL))
+ return -EBUSY;
- if (ret) {
- spin_unlock(&glob->lru_lock);
- return ret;
+ if (!ttm_bo_get_unless_zero(bo)) {
+ if (locked)
+ dma_resv_unlock(bo->base.resv);
+ return -EBUSY;
}
if (bo->deleted) {
- ret = ttm_bo_cleanup_refs(bo, false, false, locked);
+ ttm_bo_cleanup_refs(bo, false, false, locked);
ttm_bo_put(bo);
- return ret;
+ return 0;
}
ttm_bo_del_from_lru(bo);
+ /* TODO: Cleanup the locking */
spin_unlock(&glob->lru_lock);
- /**
+ /*
* Move to system cached
*/
-
if (bo->mem.mem_type != TTM_PL_SYSTEM) {
struct ttm_operation_ctx ctx = { false, false };
struct ttm_resource evict_mem;
@@ -1253,29 +1232,26 @@ int ttm_bo_swapout(struct ttm_operation_ctx *ctx, gfp_t
gfp_flags)
}
}
- /**
+ /*
* Make sure BO is idle.
*/
-
ret = ttm_bo_wait(bo, false, false);
if (unlikely(ret != 0))
goto out;
ttm_bo_unmap_virtual(bo);
- /**
+ /*
* Swap out. Buffer will be swapped in again as soon as
* anyone tries to access a ttm page.
*/
-
if (bo->bdev->funcs->swap_notify)
bo->bdev->funcs->swap_notify(bo);
ret = ttm_tt_swapout(bo->bdev, bo->ttm, gfp_flags);
out:
- /**
-*
+ /*
* Unreserve without putting on LRU to avoid swapping out an
* already swapped buffer.
*/
@@ -1284,7 +1260,6 @@ int ttm_bo_swapout(struct ttm_operation_ctx *ctx, gfp_t
gfp_flags)
ttm_bo_put(bo);
return ret;
}
-EXPORT_SYMBOL(ttm_bo_swapout);
void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
{
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 95e1b7b1f2e6..dfc2a7e4e490 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -102,6 +102,35 @@ static int ttm_global_init(void)
return ret;
}
+/**
+ * A buffer object shrink method that tries to swap out the first
+ * buffer object on the global::swap_lru list.
+ */
+long ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags)
+{
+ struct ttm_global *glob = &ttm_glob;
+ struct ttm_buffer_object *bo;
+ unsigned i;
+ int ret;
+
+ spin_lock(&glob->lru_lock);
+ for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
+ list_for_each_entry(bo, &glob->swap_lru[i], swap) {
+ uint32_t num_pages = bo->ttm->num_pages;
+
+ ret = ttm_bo_swapout(bo, ctx, gfp_flags);
+ /* ttm_bo_swapout has dropped th
[PATCH 3/3] drm/ttm: switch to per device LRU lock
Instead of having a global lock.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++---
drivers/gpu/drm/qxl/qxl_release.c | 5 +--
drivers/gpu/drm/ttm/ttm_bo.c | 49 --
drivers/gpu/drm/ttm/ttm_device.c | 12 +++
drivers/gpu/drm/ttm/ttm_execbuf_util.c | 8 ++---
drivers/gpu/drm/ttm/ttm_resource.c | 9 +++--
include/drm/ttm/ttm_bo_driver.h| 4 +--
include/drm/ttm/ttm_device.h | 4 +--
8 files changed, 43 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 9d19078246c8..ae18c0e32347 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -638,15 +638,15 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device
*adev,
struct amdgpu_vm_bo_base *bo_base;
if (vm->bulk_moveable) {
- spin_lock(&ttm_glob.lru_lock);
+ spin_lock(&adev->mman.bdev.lru_lock);
ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move);
- spin_unlock(&ttm_glob.lru_lock);
+ spin_unlock(&adev->mman.bdev.lru_lock);
return;
}
memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move));
- spin_lock(&ttm_glob.lru_lock);
+ spin_lock(&adev->mman.bdev.lru_lock);
list_for_each_entry(bo_base, &vm->idle, vm_status) {
struct amdgpu_bo *bo = bo_base->bo;
@@ -660,7 +660,7 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
&bo->shadow->tbo.mem,
&vm->lru_bulk_move);
}
- spin_unlock(&ttm_glob.lru_lock);
+ spin_unlock(&adev->mman.bdev.lru_lock);
vm->bulk_moveable = true;
}
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 6ed673d75f9f..29ad72cb6b54 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -417,16 +417,13 @@ void qxl_release_fence_buffer_objects(struct qxl_release
*release)
release->id | 0xf000, release->base.seqno);
trace_dma_fence_emit(&release->base);
- spin_lock(&ttm_glob.lru_lock);
-
list_for_each_entry(entry, &release->bos, head) {
bo = entry->bo;
dma_resv_add_shared_fence(bo->base.resv, &release->base);
- ttm_bo_move_to_lru_tail(bo, &bo->mem, NULL);
+ ttm_bo_move_to_lru_tail_unlocked(bo);
dma_resv_unlock(bo->base.resv);
}
- spin_unlock(&ttm_glob.lru_lock);
ww_acquire_fini(&release->ticket);
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index a1be88be357b..a8103c8718a3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -242,9 +242,9 @@ static int ttm_bo_individualize_resv(struct
ttm_buffer_object *bo)
* reference it any more. The only tricky case is the trylock on
* the resv object while holding the lru_lock.
*/
- spin_lock(&ttm_glob.lru_lock);
+ spin_lock(&bo->bdev->lru_lock);
bo->base.resv = &bo->base._resv;
- spin_unlock(&ttm_glob.lru_lock);
+ spin_unlock(&bo->bdev->lru_lock);
}
return r;
@@ -303,7 +303,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
if (unlock_resv)
dma_resv_unlock(bo->base.resv);
- spin_unlock(&ttm_glob.lru_lock);
+ spin_unlock(&bo->bdev->lru_lock);
lret = dma_resv_wait_timeout_rcu(resv, true, interruptible,
30 * HZ);
@@ -313,7 +313,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
else if (lret == 0)
return -EBUSY;
- spin_lock(&ttm_glob.lru_lock);
+ spin_lock(&bo->bdev->lru_lock);
if (unlock_resv && !dma_resv_trylock(bo->base.resv)) {
/*
* We raced, and lost, someone else holds the
reservation now,
@@ -323,7 +323,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
* delayed destruction would succeed, so just return
success
* here.
*/
- spin_unlock(&ttm_glob.lru_lock);
+ spin_unlock(&bo->bdev->lru_lock);
return 0;
}
ret = 0;
@@ -332,13 +332,13 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object
*bo,
if (ret || unlikely(list_empty(&bo->ddestroy))) {
if (unlock_resv)
dma_resv_unlock(bo->base.resv);
- spin_unlock(&ttm_glob.lru_lock);
[PATCH 2/3] drm/ttm: remove swap LRU
Instead evict round robin from each devices SYSTEM and TT domain.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c| 31 ++-
drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
drivers/gpu/drm/ttm/ttm_device.c| 59 +
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
include/drm/ttm/ttm_bo_api.h| 1 -
include/drm/ttm/ttm_bo_driver.h | 1 -
include/drm/ttm/ttm_device.h| 7 +---
7 files changed, 51 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d33578a112b4..a1be88be357b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -73,7 +73,6 @@ static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
{
struct ttm_device *bdev = bo->bdev;
- list_del_init(&bo->swap);
list_del_init(&bo->lru);
if (bdev->funcs->del_from_lru_notify)
@@ -104,14 +103,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
man = ttm_manager_type(bdev, mem->mem_type);
list_move_tail(&bo->lru, &man->lru[bo->priority]);
- if (man->use_tt && bo->ttm &&
- !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
-TTM_PAGE_FLAG_SWAPPED))) {
- struct list_head *swap;
-
- swap = &ttm_glob.swap_lru[bo->priority];
- list_move_tail(&bo->swap, swap);
- }
if (bdev->funcs->del_from_lru_notify)
bdev->funcs->del_from_lru_notify(bo);
@@ -126,9 +117,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo);
break;
}
- if (bo->ttm && !(bo->ttm->page_flags &
-(TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED)))
- ttm_bo_bulk_move_set_pos(&bulk->swap[bo->priority], bo);
}
}
EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
@@ -166,20 +154,6 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move
*bulk)
list_bulk_move_tail(&man->lru[i], &pos->first->lru,
&pos->last->lru);
}
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->swap[i];
- struct list_head *lru;
-
- if (!pos->first)
- continue;
-
- dma_resv_assert_held(pos->first->base.resv);
- dma_resv_assert_held(pos->last->base.resv);
-
- lru = &ttm_glob.swap_lru[i];
- list_bulk_move_tail(lru, &pos->first->swap, &pos->last->swap);
- }
}
EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
@@ -1056,7 +1030,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
kref_init(&bo->kref);
INIT_LIST_HEAD(&bo->lru);
INIT_LIST_HEAD(&bo->ddestroy);
- INIT_LIST_HEAD(&bo->swap);
bo->bdev = bdev;
bo->type = type;
bo->mem.mem_type = TTM_PL_SYSTEM;
@@ -1191,6 +1164,10 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct
ttm_operation_ctx *ctx,
bool locked;
int ret;
+ if (!bo->ttm || bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
+ TTM_PAGE_FLAG_SWAPPED))
+ return false;
+
if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked, NULL))
return -EBUSY;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 031e5819fec4..a2a17c84ceb3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -303,7 +303,6 @@ static int ttm_buffer_object_transfer(struct
ttm_buffer_object *bo,
atomic_inc(&ttm_glob.bo_count);
INIT_LIST_HEAD(&fbo->base.ddestroy);
INIT_LIST_HEAD(&fbo->base.lru);
- INIT_LIST_HEAD(&fbo->base.swap);
fbo->base.moving = NULL;
drm_vma_node_reset(&fbo->base.base.vma_node);
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index dfc2a7e4e490..5064f7db78ab 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -67,7 +67,6 @@ static int ttm_global_init(void)
unsigned long num_pages;
struct sysinfo si;
int ret = 0;
- unsigned i;
mutex_lock(&ttm_global_mutex);
if (++ttm_glob_use_count > 1)
@@ -90,8 +89,6 @@ static int ttm_global_init(void)
goto out;
}
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
- INIT_LIST_HEAD(&glob->swap_lru[i]);
INIT_LIST_HEAD(&glob->device_list);
atomic_set(&glob->bo_count, 0);
@@ -109,27 +106,59 @@ static int ttm_global_init(void)
long ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags)
{
struct ttm_global *glob = &ttm_glob;
+ struct ttm_device *bdev;
+ int ret = -EBUSY;
+
+ mu
Re: [PATCH v2 14/15] ARM: dts: bcm2711: Add the BSC interrupt controller
Hi Maxime, On 2021-02-10 14:40, Maxime Ripard wrote: Hi Dave, On Tue, Feb 09, 2021 at 09:49:05AM +, Dave Stevenson wrote: On Mon, 11 Jan 2021 at 14:23, Maxime Ripard wrote: > > The BSC controllers used for the HDMI DDC have an interrupt controller > shared between both instances. Let's add it to avoid polling. This seems to have unintended side effects. GIC interrupt 117 is shared between the standard I2C controllers (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts. Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that doesn't appear to be an option for l2-intc registering as an interrupt controller. i2c-bcm2835 therefore loses out and fails to register for the interrupt. Is there an equivalent flag that an interrupt controller can add to say that the parent interrupt is shared? Is that even supported? Indeed, it looks like setting an equivalent to IRQF_SHARED would be the solution, but I couldn't find anything that would allow us to in the irqchip code. Marc, Thomas, is it something that is allowed? No, not really. That's because the chained handler is actually an interrupt flow, and not a normal handler. IRQF_SHARED acts at the wrong level for that. I can see two possibilities: - the l2-intc gets turned into a normal handler, and does the demux from there. Horrible stuff. - the i2c controller gets parented to the l2c-int as a fake interrupt, and gets called from there. Horrible stuff. Pick your poison... :-/ M. -- Jazz is not dead. It just smells funny... ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 14/15] ARM: dts: bcm2711: Add the BSC interrupt controller
Hi Marc. On Wed, 10 Feb 2021 at 15:30, Marc Zyngier wrote: > > Hi Maxime, > > On 2021-02-10 14:40, Maxime Ripard wrote: > > Hi Dave, > > > > On Tue, Feb 09, 2021 at 09:49:05AM +, Dave Stevenson wrote: > >> On Mon, 11 Jan 2021 at 14:23, Maxime Ripard wrote: > >> > > >> > The BSC controllers used for the HDMI DDC have an interrupt controller > >> > shared between both instances. Let's add it to avoid polling. > >> > >> This seems to have unintended side effects. > >> GIC interrupt 117 is shared between the standard I2C controllers > >> (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts. > >> > >> Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that > >> doesn't appear to be an option for l2-intc registering as an interrupt > >> controller. i2c-bcm2835 therefore loses out and fails to register for > >> the interrupt. > >> > >> Is there an equivalent flag that an interrupt controller can add to > >> say that the parent interrupt is shared? Is that even supported? > > > > Indeed, it looks like setting an equivalent to IRQF_SHARED would be the > > solution, but I couldn't find anything that would allow us to in the > > irqchip code. > > > > Marc, Thomas, is it something that is allowed? > > No, not really. That's because the chained handler is actually an > interrupt flow, and not a normal handler. IRQF_SHARED acts at the wrong > level for that. > > I can see two possibilities: > > - the l2-intc gets turned into a normal handler, and does the demux >from there. Horrible stuff. > > - the i2c controller gets parented to the l2c-int as a fake interrupt, >and gets called from there. Horrible stuff. > > Pick your poison... :-/ Thanks for the info. Option 3 - remove l2-intc and drop back to polling the i2c-brcmstb blocks (which the driver supports anyway). HDMI I2C generally isn't heavily used once displays are connected, so I'd be OK with that. (We can keep the l2-intc that handles CEC and HPD as that is on a unique GIC interrupt). Dave ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: make sure pool pages are cleared
The old implementation wasn't consistend on this.
But it looks like we depend on this so better bring it back.
Signed-off-by: Christian König
Reported-and-tested-by: Mike Galbraith
Fixes: d099fc8f540a ("drm/ttm: new TT backend allocation pool v3")
---
drivers/gpu/drm/ttm/ttm_pool.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 74bf1c84b637..6e27cb1bf48b 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -33,6 +33,7 @@
#include
#include
+#include
#ifdef CONFIG_X86
#include
@@ -218,6 +219,15 @@ static void ttm_pool_unmap(struct ttm_pool *pool,
dma_addr_t dma_addr,
/* Give pages into a specific pool_type */
static void ttm_pool_type_give(struct ttm_pool_type *pt, struct page *p)
{
+ unsigned int i, num_pages = 1 << pt->order;
+
+ for (i = 0; i < num_pages; ++i) {
+ if (PageHighMem(p))
+ clear_highpage(p + i);
+ else
+ clear_page(page_address(p + i));
+ }
+
spin_lock(&pt->lock);
list_add(&p->lru, &pt->pages);
spin_unlock(&pt->lock);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 210263] brightness device returns ENXIO (?) on brightness restore at boot, with bootoption "quiet"
https://bugzilla.kernel.org/show_bug.cgi?id=210263 --- Comment #3 from Fabian (plusf...@gmail.com) --- (In reply to ninelore from comment #2) > I just found out that this Issue does NOT occur when you additionally add > the fbcon=nodefer kernel parameter. This workaround works for me too. -- You may reply to this email to add a comment. 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] video: use getter/setter functions
On Wed, 10 Feb 2021, Daniel Vetter wrote:
> On Wed, Feb 10, 2021 at 08:23:41AM +, Lee Jones wrote:
> > On Tue, 09 Feb 2021, Julia Lawall wrote:
> >
> > > Use getter and setter functions, for platform_device structures and a
> > > spi_device structure.
> > >
> > > Signed-off-by: Julia Lawall
> > >
> > > ---
> > > drivers/video/backlight/qcom-wled.c |
> > > 2 +-
> >
> > This patch is fine.
> >
> > Could you please split it out and submit it separately though please.
>
> Or just apply the entire patch through backlight tree, there's nothing
> going on in fbdev anyway I think.
>
> Acked-by: Daniel Vetter
I can do that. Is that an fbdev Ack?
> > > drivers/video/fbdev/amifb.c |
> > > 4 ++--
> > > drivers/video/fbdev/da8xx-fb.c |
> > > 4 ++--
> > > drivers/video/fbdev/imxfb.c |
> > > 2 +-
> > > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c |
> > > 6 +++---
> > > drivers/video/fbdev/omap2/omapfb/dss/dpi.c |
> > > 4 ++--
> > > drivers/video/fbdev/omap2/omapfb/dss/dsi.c |
> > > 4 ++--
> > > drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c |
> > > 2 +-
> > > drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c |
> > > 2 +-
> > > drivers/video/fbdev/xilinxfb.c |
> > > 2 +-
> > > 10 files changed, 16 insertions(+), 16 deletions(-)
> >
> > ...]
> >
> > > diff --git a/drivers/video/backlight/qcom-wled.c
> > > b/drivers/video/backlight/qcom-wled.c
> > > index 3bc7800eb0a9..091f07e7c145 100644
> > > --- a/drivers/video/backlight/qcom-wled.c
> > > +++ b/drivers/video/backlight/qcom-wled.c
> > > @@ -1692,7 +1692,7 @@ static int wled_probe(struct platform_device *pdev)
> > >
> > > static int wled_remove(struct platform_device *pdev)
> > > {
> > > - struct wled *wled = dev_get_drvdata(&pdev->dev);
> > > + struct wled *wled = platform_get_drvdata(pdev);
> > >
> > > mutex_destroy(&wled->lock);
> > > cancel_delayed_work_sync(&wled->ovp_work);
> >
> > For my own reference (apply this as-is to your sign-off block):
> >
> > Acked-for-Backlight-by: Lee Jones
> >
>
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 --- Comment #3 from youling...@gmail.com --- replug hdmi, [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:67:crtc-0] flip_done timed out [ 252.377374] [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:67:crtc-0] flip_done timed out [ 262.617295] [drm:drm_atomic_helper_wait_for_dependencies [drm_kms_helper]] *ERROR* [CRTC:67:crtc-0] flip_done timed out [ 270.268771] [drm] amdgpu_dm_irq_schedule_work FAILED src 2 [ 272.857440] [drm:drm_atomic_helper_wait_for_dependencies [drm_kms_helper]] *ERROR* [PLANE:55:plane-3] flip_done timed out [ 272.906034] [ cut here ] [ 272.906037] WARNING: CPU: 0 PID: 15 at amdgpu_dm_atomic_commit_tail+0x2582/0x25f0 [amdgpu] [ 272.906404] Modules linked in: nct6775 hwmon_vid ccm binfmt_misc overlay exportfs kvm_amd kvm crc32c_intel crc32_pclmul snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_timer snd soundcore snd_intel_dspcfg amdgpu drm_kms_helper cec fb_sys_fops syscopyarea sysfillrect sysimgblt gpu_sched drm_ttm_helper ttm drm ccp rng_core hid_multitouch btusb iwlmvm btrtl btintel btbcm bluetooth ecdh_generic ecc mac80211 libarc4 iwlwifi cfg80211 igb i2c_algo_bit k10temp gpio_amdpt gpio_generic efi_pstore sdcardfs [ 272.906461] CPU: 0 PID: 15 Comm: kworker/0:1 Tainted: GW 5.11.0-rc5-android-x86_64+ #1 [ 272.906466] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./B450 Gaming-ITX/ac, BIOS P4.10 07/08/2020 [ 272.906468] Workqueue: events dm_irq_work_func [amdgpu] [ 272.906826] RIP: 0010:amdgpu_dm_atomic_commit_tail+0x2582/0x25f0 [amdgpu] [ 272.907181] Code: a0 fd ff ff 01 c7 85 9c fd ff ff 37 00 00 00 c7 85 a4 fd ff ff 20 00 00 00 e8 5a 30 13 00 e9 a0 fa ff ff 0f 0b e9 f0 f8 ff ff <0f> 0b e9 3f f9 ff ff 0f 0b 0f 0b e9 55 f9 ff ff 49 8b 06 41 0f b6 [ 272.907185] RSP: 0018:a0a8000a3ab8 EFLAGS: 00210002 [ 272.907189] RAX: 0002 RBX: 0003 RCX: 996d91211918 [ 272.907191] RDX: 0001 RSI: 00200297 RDI: 996d510a0178 [ 272.907194] RBP: a0a8000a3db8 R08: 0005 R09: [ 272.907196] R10: a0a8000a3a18 R11: a0a8000a3a1c R12: 00200287 [ 272.907198] R13: 996d6c35d800 R14: 996d91211800 R15: 996d6f4d3600 [ 272.907200] FS: () GS:996e56e0() knlGS: [ 272.907203] CS: 0010 DS: ES: CR0: 80050033 [ 272.907206] CR2: 144c2000 CR3: 00010e262000 CR4: 001506f0 [ 272.907209] Call Trace: [ 272.907219] commit_tail+0x94/0x130 [drm_kms_helper] [ 272.907251] drm_atomic_helper_commit+0x113/0x140 [drm_kms_helper] [ 272.907280] dm_restore_drm_connector_state+0xef/0x170 [amdgpu] [ 272.907635] handle_hpd_irq+0x118/0x150 [amdgpu] [ 272.907990] dm_irq_work_func+0x49/0x60 [amdgpu] [ 272.908346] process_one_work+0x1c1/0x3c0 [ 272.908353] worker_thread+0x4d/0x3d0 [ 272.908357] ? rescuer_thread+0x3b0/0x3b0 [ 272.908362] kthread+0x133/0x150 [ 272.908367] ? kthread_create_worker_on_cpu+0x70/0x70 [ 272.908373] ret_from_fork+0x22/0x30 [ 272.908379] ---[ end trace 9e7a0ad9c8574ed6 ]--- [ 272.908402] [ cut here ] [ 272.908403] WARNING: CPU: 0 PID: 15 at amdgpu_dm_atomic_commit_tail+0x258b/0x25f0 [amdgpu] [ 272.908757] Modules linked in: nct6775 hwmon_vid ccm binfmt_misc overlay exportfs kvm_amd kvm crc32c_intel crc32_pclmul snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_timer snd soundcore snd_intel_dspcfg amdgpu drm_kms_helper cec fb_sys_fops syscopyarea sysfillrect sysimgblt gpu_sched drm_ttm_helper ttm drm ccp rng_core hid_multitouch btusb iwlmvm btrtl btintel btbcm bluetooth ecdh_generic ecc mac80211 libarc4 iwlwifi cfg80211 igb i2c_algo_bit k10temp gpio_amdpt gpio_generic efi_pstore sdcardfs [ 272.908803] CPU: 0 PID: 15 Comm: kworker/0:1 Tainted: GW 5.11.0-rc5-android-x86_64+ #1 [ 272.908807] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./B450 Gaming-ITX/ac, BIOS P4.10 07/08/2020 [ 272.908809] Workqueue: events dm_irq_work_func [amdgpu] [ 272.909163] RIP: 0010:amdgpu_dm_atomic_commit_tail+0x258b/0x25f0 [amdgpu] [ 272.909517] Code: ff ff 37 00 00 00 c7 85 a4 fd ff ff 20 00 00 00 e8 5a 30 13 00 e9 a0 fa ff ff 0f 0b e9 f0 f8 ff ff 0f 0b e9 3f f9 ff ff 0f 0b <0f> 0b e9 55 f9 ff ff 49 8b 06 41 0f b6 8e 2d 01 00 00 48 c7 c6 38 [ 272.909521] RSP: 0018:a0a8000a3ab8 EFLAGS: 00210086 [ 272.909524] RAX: 0001 RBX: 0003 RCX: 996d91211918 [ 272.909526] RDX: 0001 RSI: 00200297 RDI: 996d510a0178 [ 272.909528] RBP: a0a8000a3db8 R08: 0005 R09: [ 272.909530] R10: a0a8000a3a18 R11: a0a8000a3a1c R12:
Re: [PATCH 3/9] drm: Add simplekms driver
Hi
Am 29.06.20 um 11:06 schrieb Daniel Vetter:
+ ARRAY_SIZE(simplekms_formats),
+ simplekms_format_modifiers,
+ connector);
+ if (ret)
+ return ret;
+
+ drm_mode_config_reset(dev);
This breaks fastboot. I think ideally we'd have the state represent
everything is on, and allocate an fb + buffer with the current contents of
the framebuffer. Since we can allocate an fb that matches this shouldn't
be a problem, just a raw memcpy_fromio should do the job.
Having a nice new simplekms drm driver and then losing fastboot feels like
slightly off tradeoff.
Maybe in a follow-up patch, but before fbcon setup? Since ideally fbcon
also takes over the already existing framebuffer we allocated, so that as
long as nothing clears the fb (i.e. fbcon is quiet) we'd preserve the
original framebuffer throughout the boot-up sequence.
I recently looked at how to implement this and it seems fairly complicated.
What we want it to adopt the current mode config into fbcon (and
probably other in-kernel clients). The kernel client code uses it's own
file instance to allocate the framebuffer objects against. So we cannot
read-out the framebuffer state here. We'd ideally do this in the fbdev code.
I read through the proposal for read-out helpers. i915 seems to have
lots of special cases. Can we adopt a simplified version that is just
good enough to get the initial state for fbdev?
Best regards
Thomas
+
+ return 0;
+}
+
+/*
+ * Init / Cleanup
+ */
+
+static void simplekms_device_cleanup(struct simplekms_device* sdev)
+{
+ struct drm_device *dev = &sdev->dev;
+
+ drm_dev_unregister(dev);
I'd inline this, I guess there was once more before you switched
everything over to devm_
+}
+
+static struct simplekms_device *
+simplekms_device_create(struct drm_driver *drv, struct platform_device *pdev)
+{
+ struct simplekms_device *sdev;
+ int ret;
+
+ sdev = devm_drm_dev_alloc(&pdev->dev, drv, struct simplekms_device,
+ dev);
+ if (IS_ERR(sdev))
+ return ERR_CAST(sdev);
+ sdev->pdev = pdev;
+
+ ret = simplekms_device_init_fb(sdev);
+ if (ret)
+ return ERR_PTR(ret);
+ ret = simplekms_device_init_mm(sdev);
+ if (ret)
+ return ERR_PTR(ret);
+ ret = simplekms_device_init_modeset(sdev);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return sdev;
+}
+
+/*
+ * DRM driver
+ */
+
+DEFINE_DRM_GEM_FOPS(simplekms_fops);
+
+static struct drm_driver simplekms_driver = {
+ DRM_GEM_SHMEM_DRIVER_OPS,
+ .name = DRIVER_NAME,
+ .desc = DRIVER_DESC,
+ .date = DRIVER_DATE,
+ .major = DRIVER_MAJOR,
+ .minor = DRIVER_MINOR,
+ .driver_features= DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
+ .fops = &simplekms_fops,
+};
+
+/*
+ * Platform driver
+ */
+
+static int simplekms_probe(struct platform_device *pdev)
+{
+ struct simplekms_device *sdev;
+ struct drm_device *dev;
+ int ret;
+
+ sdev = simplekms_device_create(&simplekms_driver, pdev);
+ if (IS_ERR(sdev))
+ return PTR_ERR(sdev);
+ dev = &sdev->dev;
+
+ ret = drm_dev_register(dev, 0);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int simplekms_remove(struct platform_device *pdev)
+{
+ struct simplekms_device *sdev = platform_get_drvdata(pdev);
+
+ simplekms_device_cleanup(sdev);
If you add the ->disable hook then a comment here that we don't want to
shut down to allow fastboot would be nice.
+
+ return 0;
+}
+
+static struct platform_driver simplekms_platform_driver = {
+ .driver = {
+ .name = "simple-framebuffer", /* connect to sysfb */
+ },
+ .probe = simplekms_probe,
+ .remove = simplekms_remove,
+};
+
+module_platform_driver(simplekms_platform_driver);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL v2");
--
2.27.0
Cheers, Daniel
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915/adl_s: Add gmbus pin mapping
On 2/10/21 3:54 AM, Anand Moon wrote:
> Add table to map the GMBUS pin pairs to GPIO registers and port to DDC
> mapping for ADL_S as per below Bspec.
Has this patch been tested on an ADLS system? Upstream CI AFAIK doesn't have
support for ADL-S. Also comments below..
>
> Bspec:20124, 53597.
>
> Cc: Aditya Swarup
> Cc: Matt Roper
> Cc: Lucas De Marchi
> Signed-off-by: Anand Moon
> ---
> drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c
> b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 0c952e1d720e..58b8e42d4f90 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -52,6 +52,14 @@ static const struct gmbus_pin gmbus_pins[] = {
> [GMBUS_PIN_DPD] = { "dpd", GPIOF },
> };
>
> +static const struct gmbus_pin gmbus_pins_adls[] = {
> + [GMBUS_PIN_1_BXT] = { "edp", GPIOA },
I am pretty sure that GMBUS_PIN_1_BXT should map to GPIOB(1) and not GPIOA(0)
like what we have for ICP.
> + [GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOD },
> + [GMBUS_PIN_10_TC2_ICP] = { "tc2", GPIOE },
> + [GMBUS_PIN_11_TC3_ICP] = { "tc3", GPIOF },
> + [GMBUS_PIN_12_TC4_ICP] = { "tc4", GPIOG },
> +};
> +
> static const struct gmbus_pin gmbus_pins_bdw[] = {
> [GMBUS_PIN_VGADDC] = { "vga", GPIOA },
> [GMBUS_PIN_DPC] = { "dpc", GPIOD },
> @@ -101,7 +109,9 @@ static const struct gmbus_pin gmbus_pins_dg1[] = {
> static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private
> *dev_priv,
>unsigned int pin)
> {
> - if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> + if (INTEL_PCH_TYPE(dev_priv) == PCH_ADP)
This check should be converted to IS_ALDERLAKE_S(dev_priv) instead of PCH check
for PCH_ADP.
Unfortunately, we are reusing PCH_ADP for ADLS and ADLP which have different
mappings but the same ADP PCH.
This will break ADLP.
The PCH_ADP check for ADLS in intel_bios.c will also be changed in the ADLP
patches
that are going to be submitted upstream.
So might as well do the correct thing now and change this to
IS_ALDERLAKE_S(dev_priv).
> + return &gmbus_pins_adls[pin];
> + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> return &gmbus_pins_dg1[pin];
> else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> return &gmbus_pins_icp[pin];
> @@ -122,7 +132,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private
> *dev_priv,
> {
> unsigned int size;
>
> - if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> + if (INTEL_PCH_TYPE(dev_priv) == PCH_ADP)
See comment above. Change to IS_ALDERLAKE_S(dev_priv)
Aditya Swarup
> + size = ARRAY_SIZE(gmbus_pins_adls);
> + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> size = ARRAY_SIZE(gmbus_pins_dg1);
> else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> size = ARRAY_SIZE(gmbus_pins_icp);
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] pinctrl/sunxi: adding input-debounce-ns property
On Allwinner SoC interrupt debounce can be controlled by two oscillator
(32KHz and 24MHz) and a prescale divider.
Oscillator and prescale divider are set through
device tree property "input-debounce" which have 1uS accuracy.
For acheive nS precision a new device tree poperty is made
named "input-debounce-ns".
"input-debounce-ns" is checked only if "input-debounce"
property is not defined.
Suggested-by: Maxime Ripard
Signed-off-by: Marjan Pascolo
---
---
.../pinctrl/allwinner,sun4i-a10-pinctrl.yaml | 9 +++
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 25 ---
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git
a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
index 5240487dfe50..346776de3a44 100644
---
a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
+++
b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
@@ -93,6 +93,15 @@ properties:
minItems: 1
maxItems: 5
+ input-debounce-ns:
+ description:
+ Debouncing periods in nanoseconds, one period per interrupt
+ bank found in the controller.
+ Only checked if input-debounce is not present
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 1
+ maxItems: 5
+
patternProperties:
# It's pretty scary, but the basic idea is that:
# - One node name can start with either s- or r- for PRCM nodes,
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index dc8d39ae045b..869b6d5743ba 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -1335,14 +1335,31 @@ static int sunxi_pinctrl_setup_debounce(struct
sunxi_pinctrl *pctl,
struct clk *hosc, *losc;
u8 div, src;
int i, ret;
+ /* Keeping for loop below clean */
+ const char* debounce_prop_name;
+ unsigned long debounce_dividend;
/* Deal with old DTs that didn't have the oscillators */
if (of_clk_get_parent_count(node) != 3)
return 0;
+ /*
+ * Distinguish between simple input-debounce
+ * and new input-debounce-ns
+ */
+
/* If we don't have any setup, bail out */
- if (!of_find_property(node, "input-debounce", NULL))
- return 0;
+ if (!of_find_property(node, "input-debounce", NULL)) {
+ if(!of_find_property(node, "input-debounce-ns", NULL)) {
+ return 0;
+ } else {
+ debounce_prop_name="input-debounce-ns";
+ debounce_dividend=NSEC_PER_SEC;
+ }
+ } else {
+ debounce_prop_name="input-debounce";
+ debounce_dividend=USEC_PER_SEC;
+ }
losc = devm_clk_get(pctl->dev, "losc");
if (IS_ERR(losc))
@@ -1356,7 +1373,7 @@ static int sunxi_pinctrl_setup_debounce(struct
sunxi_pinctrl *pctl,
unsigned long debounce_freq;
u32 debounce;
- ret = of_property_read_u32_index(node, "input-debounce",
+ ret = of_property_read_u32_index(node, debounce_prop_name,
i, &debounce);
if (ret)
return ret;
@@ -1364,7 +1381,7 @@ static int sunxi_pinctrl_setup_debounce(struct
sunxi_pinctrl *pctl,
if (!debounce)
continue;
- debounce_freq = DIV_ROUND_CLOSEST(USEC_PER_SEC, debounce);
+ debounce_freq = DIV_ROUND_CLOSEST(debounce_dividend, debounce);
losc_div = sunxi_pinctrl_get_debounce_div(losc,
debounce_freq,
&losc_diff);
--
2.22.0.windows.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 --- Comment #4 from youling...@gmail.com --- Created attachment 295189 --> https://bugzilla.kernel.org/attachment.cgi?id=295189&action=edit dmesg.txt echo 6 > /sys/module/drm/parameters/debug replug hdmi dmesg. -- You may reply to this email to add a comment. 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
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 --- Comment #5 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) --- It would be good to know what OS/Userspace this is, I don't think I've seen anything that has atomic check failures on nearly every flip, eg: [ 179.142898] [drm:amdgpu_dm_atomic_check [amdgpu]] Atomic check failed with err: -22 Black screen comes from the page-flip timeout, but it's hard to say what the root cause is. When the screen is black can you cat the DTN log and then attach the result to this ticket? cat /sys/kernel/debug/dri/0/amdgpu_dm_dtn_log Thanks! -- You may reply to this email to add a comment. 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: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation
On Wed, Feb 10, 2021 at 5:39 PM Suren Baghdasaryan wrote:
>
> On Wed, Feb 10, 2021 at 5:06 AM Daniel Vetter wrote:
> >
> > On Tue, Feb 09, 2021 at 12:16:51PM -0800, Suren Baghdasaryan wrote:
> > > On Tue, Feb 9, 2021 at 12:03 PM Daniel Vetter wrote:
> > > >
> > > > On Tue, Feb 9, 2021 at 6:46 PM Christian König
> > > > wrote:
> > > > >
> > > > >
> > > > >
> > > > > Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
> > > > > > On Tue, Feb 9, 2021 at 4:57 AM Christian König
> > > > > > wrote:
> > > > > >> Am 09.02.21 um 13:11 schrieb Christian König:
> > > > > >>> [SNIP]
> > > > > >> +void drm_page_pool_add(struct drm_page_pool *pool, struct
> > > > > >> page *page)
> > > > > >> +{
> > > > > >> + spin_lock(&pool->lock);
> > > > > >> + list_add_tail(&page->lru, &pool->items);
> > > > > >> + pool->count++;
> > > > > >> + atomic_long_add(1 << pool->order, &total_pages);
> > > > > >> + spin_unlock(&pool->lock);
> > > > > >> +
> > > > > >> + mod_node_page_state(page_pgdat(page),
> > > > > >> NR_KERNEL_MISC_RECLAIMABLE,
> > > > > >> + 1 << pool->order);
> > > > > > Hui what? What should that be good for?
> > > > > This is a carryover from the ION page pool implementation:
> > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cdff8edcd4d147a5b08d8cd20cff2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637484888114923580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9%2BIBC0tezSV6Ci4S3kWfW%2BQvJm4mdunn3dF6C0kyfCw%3D&reserved=0
> > > > >
> > > > >
> > > > > My sense is it helps with the vmstat/meminfo accounting so folks
> > > > > can
> > > > > see the cached pages are shrinkable/freeable. This maybe falls
> > > > > under
> > > > > other dmabuf accounting/stats discussions, so I'm happy to
> > > > > remove it
> > > > > for now, or let the drivers using the shared page pool logic
> > > > > handle
> > > > > the accounting themselves?
> > > > > >> Intentionally separated the discussion for that here.
> > > > > >>
> > > > > >> As far as I can see this is just bluntly incorrect.
> > > > > >>
> > > > > >> Either the page is reclaimable or it is part of our pool and
> > > > > >> freeable
> > > > > >> through the shrinker, but never ever both.
> > > > > > IIRC the original motivation for counting ION pooled pages as
> > > > > > reclaimable was to include them into /proc/meminfo's MemAvailable
> > > > > > calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
> > > > > > non-slab kernel pages" seems like a good place to account for them
> > > > > > but
> > > > > > I might be wrong.
> > > > >
> > > > > Yeah, that's what I see here as well. But exactly that is utterly
> > > > > nonsense.
> > > > >
> > > > > Those pages are not "free" in the sense that get_free_page could
> > > > > return
> > > > > them directly.
> > > >
> > > > Well on Android that is kinda true, because Android has it's
> > > > oom-killer (way back it was just a shrinker callback, not sure how it
> > > > works now), which just shot down all the background apps. So at least
> > > > some of that (everything used by background apps) is indeed
> > > > reclaimable on Android.
> > > >
> > > > But that doesn't hold on Linux in general, so we can't really do this
> > > > for common code.
> > > >
> > > > Also I had a long meeting with Suren, John and other googles
> > > > yesterday, and the aim is now to try and support all the Android gpu
> > > > memory accounting needs with cgroups. That should work, and it will
> > > > allow Android to handle all the Android-ism in a clean way in upstream
> > > > code. Or that's at least the plan.
> > > >
> > > > I think the only thing we identified that Android still needs on top
> > > > is the dma-buf sysfs stuff, so that shared buffers (which on Android
> > > > are always dma-buf, and always stay around as dma-buf fd throughout
> > > > their lifetime) can be listed/analyzed with full detail.
> > > >
> > > > But aside from this the plan for all the per-process or per-heap
> > > > account, oom-killer integration and everything else is planned to be
> > > > done with cgroups.
> > >
> > > Until cgroups are ready we probably will need to add a sysfs node to
> > > report the total dmabuf pool size and I think that would cover our
> > > current accounting need here.
> > > As I mentioned, not including dmabuf pools into MemAvailable would
> > > affect that stat and I'm wondering if pools should be considered as
> > > part of MemAvailable or not. Since MemAvailable includes SReclaimable
> > > I think it makes sense to include them but maybe there are other
> > > considerations tha
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 --- Comment #6 from youling...@gmail.com --- Created attachment 295193 --> https://bugzilla.kernel.org/attachment.cgi?id=295193&action=edit amdgpu_dm_dtn_log I using amd 3400g running on androidx86 7 with mesa21.0 cat /sys/kernel/debug/dri/0/amdgpu_dm_dtn_log -- You may reply to this email to add a comment. 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] PCI: Also set up legacy files only after sysfs init
Hi Bjorn,
Can you ack this for merging through my topic branch with the other
follow_pfn/iomem revoke fixes for 5.12?
If not, what's the plan for getting this (or equivalent functionality)
in for 5.13? I have more of these follow_pfn/iomem revoke patches on
top, so I'd like to get the first cut merged sooner than later if
possible. And the other prep work has been in -next since -rc1.
Thanks, Daniel
On Fri, Feb 5, 2021 at 2:36 PM Daniel Vetter wrote:
>
> We are already doing this for all the regular sysfs files on PCI
> devices, but not yet on the legacy io files on the PCI buses. Thus far
> no problem, but in the next patch I want to wire up iomem revoke
> support. That needs the vfs up and running already to make sure that
> iomem_get_mapping() works.
>
> Wire it up exactly like the existing code in
> pci_create_sysfs_dev_files(). Note that pci_remove_legacy_files()
> doesn't need a check since the one for pci_bus->legacy_io is
> sufficient.
>
> An alternative solution would be to implement a callback in sysfs to
> set up the address space from iomem_get_mapping() when userspace calls
> mmap(). This also works, but Greg didn't really like that just to work
> around an ordering issue when the kernel loads initially.
>
> v2: Improve commit message (Bjorn)
>
> Signed-off-by: Daniel Vetter
> Cc: Stephen Rothwell
> Cc: Jason Gunthorpe
> Cc: Kees Cook
> Cc: Dan Williams
> Cc: Andrew Morton
> Cc: John Hubbard
> Cc: Jérôme Glisse
> Cc: Jan Kara
> Cc: Dan Williams
> Cc: Greg Kroah-Hartman
> Cc: linux...@kvack.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: Bjorn Helgaas
> Cc: linux-...@vger.kernel.org
> ---
> drivers/pci/pci-sysfs.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index fb072f4b3176..0c45b4f7b214 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -927,6 +927,9 @@ void pci_create_legacy_files(struct pci_bus *b)
> {
> int error;
>
> + if (!sysfs_initialized)
> + return;
> +
> b->legacy_io = kcalloc(2, sizeof(struct bin_attribute),
>GFP_ATOMIC);
> if (!b->legacy_io)
> @@ -1448,6 +1451,7 @@ void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
> static int __init pci_sysfs_init(void)
> {
> struct pci_dev *pdev = NULL;
> + struct pci_bus *pbus = NULL;
> int retval;
>
> sysfs_initialized = 1;
> @@ -1459,6 +1463,9 @@ static int __init pci_sysfs_init(void)
> }
> }
>
> + while ((pbus = pci_find_next_bus(pbus)))
> + pci_create_legacy_files(pbus);
> +
> return 0;
> }
> late_initcall(pci_sysfs_init);
> --
> 2.30.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] Add support for SVM atomics in Nouveau
On Tue, Feb 09, 2021 at 04:17:38PM -0500, Jerome Glisse wrote: > > > Yes, I would like to avoid the long term pin constraints as well if > > > possible I > > > just haven't found a solution yet. Are you suggesting it might be > > > possible to > > > add a callback in the page migration logic to specially deal with moving > > > these > > > pages? > > > > How would migration even find the page? > > Migration can scan memory from physical address (isolate_migratepages_range()) > So the CPU mapping is not the only path to get to a page. I mean find out that the page is now owned by the GPU driver to tell it that it needs to migrate that reference. Normally that would go through the VMA to the mmu notifier, but with the page removed from the normal VMA it can't get to a mmu notifier or the GPU driver. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] Add support for SVM atomics in Nouveau
On Tue, Feb 09, 2021 at 12:53:27PM -0800, John Hubbard wrote: > This direction sounds at least...possible. Using MMU notifiers instead of pins > is definitely appealing. I'm not quite clear on the callback idea above, but > overall it seems like taking advantage of the ZONE_DEVICE tracking of pages > (without having to put anything additional in each struct page), could work. It isn't the ZONE_DEVICE page that needs to be tracked. Really what you want to do here is leave the CPU page in the VMA and the page tables where it started and deny CPU access to the page. Then all the proper machinery will continue to work. IMHO "migration" is the wrong idea if the data isn't actually moving. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] drm/dp: Make number of AUX retries configurable by display drivers.
On Wed, 2021-02-10 at 00:33 -0800, Khaled Almahallawy wrote:
> The number of AUX retries specified in the DP specs is 7. Currently, to make
> Dell 4k monitors happier, the number of retries are 32.
> i915 also retries 5 times (intel_dp_aux_xfer) which means in the case of AUX
> timeout we actually retries 32 * 5 = 160 times.
Is there any good reason for i915 to actually be doing retries itself? It seems
like an easier solution here might just to be to fix i915 so it doesn't retry,
and just rely on DRM to do the retries as appropriate.
That being said though, I'm open to this if there is a legitimate reason for
i915 to be handling retries on its own
>
> So making the number of aux retires a variable to allow for fine tuning and
> optimization of aux timing.
>
> Signed-off-by: Khaled Almahallawy
> ---
> drivers/gpu/drm/drm_dp_helper.c | 10 +++---
> include/drm/drm_dp_helper.h | 1 +
> 2 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index eedbb48815b7..8fdf57b4a06c 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -249,13 +249,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
> request,
>
> mutex_lock(&aux->hw_mutex);
>
> - /*
> - * The specification doesn't give any recommendation on how often to
> - * retry native transactions. We used to retry 7 times like for
> - * aux i2c transactions but real world devices this wasn't
> - * sufficient, bump to 32 which makes Dell 4k monitors happier.
> - */
> - for (retry = 0; retry < 32; retry++) {
> + for (retry = 0; retry < aux->num_retries; retry++) {
> if (ret != 0 && ret != -ETIMEDOUT) {
> usleep_range(AUX_RETRY_INTERVAL,
> AUX_RETRY_INTERVAL + 100);
> @@ -1744,6 +1738,8 @@ void drm_dp_aux_init(struct drm_dp_aux *aux)
> aux->ddc.retries = 3;
>
> aux->ddc.lock_ops = &drm_dp_i2c_lock_ops;
> + /*Still making the Dell 4k monitors happier*/
> + aux->num_retries = 32;
> }
> EXPORT_SYMBOL(drm_dp_aux_init);
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index edffd1dcca3e..16cbfc8f5e66 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1876,6 +1876,7 @@ struct drm_dp_aux {
> struct mutex hw_mutex;
> struct work_struct crc_work;
> u8 crc_count;
> + int num_retries;
> ssize_t (*transfer)(struct drm_dp_aux *aux,
> struct drm_dp_aux_msg *msg);
> /**
--
Sincerely,
Lyude Paul (she/her)
Software Engineer at Red Hat
Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
asked me a question, are waiting for a review/merge on a patch, etc. and I
haven't responded in a while, please feel free to send me another email to check
on my status. I don't bite!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: make sure pool pages are cleared
On Wed, Feb 10, 2021 at 5:05 PM Christian König
wrote:
>
> The old implementation wasn't consistend on this.
>
> But it looks like we depend on this so better bring it back.
>
> Signed-off-by: Christian König
> Reported-and-tested-by: Mike Galbraith
> Fixes: d099fc8f540a ("drm/ttm: new TT backend allocation pool v3")
Well I think in general there's a bit an issue in ttm with not
clearing stuff everywhere. So definitely in favour of clearing stuff.
Looking at the code this only fixes the clearing, at alloc time we're
still very optional with clearing. I think we should just set
__GFP_ZERO unconditionally there too.
With that: Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/ttm/ttm_pool.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 74bf1c84b637..6e27cb1bf48b 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -33,6 +33,7 @@
>
> #include
> #include
> +#include
>
> #ifdef CONFIG_X86
> #include
> @@ -218,6 +219,15 @@ static void ttm_pool_unmap(struct ttm_pool *pool,
> dma_addr_t dma_addr,
> /* Give pages into a specific pool_type */
> static void ttm_pool_type_give(struct ttm_pool_type *pt, struct page *p)
> {
> + unsigned int i, num_pages = 1 << pt->order;
> +
> + for (i = 0; i < num_pages; ++i) {
> + if (PageHighMem(p))
> + clear_highpage(p + i);
> + else
> + clear_page(page_address(p + i));
> + }
> +
> spin_lock(&pt->lock);
> list_add(&p->lru, &pt->pages);
> spin_unlock(&pt->lock);
> --
> 2.25.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/v3d: Add job timeout module param
On 5/2/21 13:28, Yukimasa Sugizaki wrote: On 05/02/2021, Eric Anholt wrote: On Thu, Feb 4, 2021 at 10:09 AM Chema Casanova wrote: On 3/9/20 18:48, Yukimasa Sugizaki wrote: From: Yukimasa Sugizaki The default timeout is 500 ms which is too short for some workloads including Piglit. Adding this parameter will help users to run heavier tasks. Signed-off-by: Yukimasa Sugizaki --- drivers/gpu/drm/v3d/v3d_sched.c | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c index feef0c749e68..983efb018560 100644 --- a/drivers/gpu/drm/v3d/v3d_sched.c +++ b/drivers/gpu/drm/v3d/v3d_sched.c @@ -19,11 +19,17 @@ */ #include +#include #include "v3d_drv.h" #include "v3d_regs.h" #include "v3d_trace.h" +static uint timeout = 500; +module_param(timeout, uint, 0444); +MODULE_PARM_DESC(timeout, + "Timeout for a job in ms (0 means infinity and default is 500 ms)"); + I think that parameter definition could be included at v3d_drv.c as other drivers do. Then we would have all future parameters together. In that case we would need also to include the extern definition at v3d_drv.h for the driver variable. The param name could be more descriptive like "sched_timeout_ms" in the lima driver. I wonder if it would make sense to have different timeouts parameters for different type of jobs we have. At least this series come from the need additional time to complete compute jobs. This is what amdgpu does, but we probably don't need something such complex. I think it would also make sense to increase our default value for the compute jobs. What do you think? In any case I think that having this general scheduling timeout parameter as other drivers do is a good idea. I agree with your observations. I'm going to add bin_timeout_ms, render_timeout_ms, tfu_timeout_ms, v3d_timeout_ms, and cache_clean_timeout_ms parameters to v3d_drv.c instead of the sole timeout parameter. Though not sophisticated, this should be enough. Having a param for being able to test if the job completes eventually is a good idea, but if tests are triggering timeouts, then you probably need to investigate what's going on in the driver -- it's not like you want .5second unpreemptible jobs to be easy to produce. Also, for CS, I wonder if we have another reg besides CSD_CURRENT_CFG4 we could be looking at for "we're making progress on the job". Half a second with no discernible progress sounds like a bug. If binning/render/TFU/cache_clean jobs keep running over 0.5 seconds, then it should be a bug because they normally complete processing within the period. However, a CSD job can do anything. For example, SaschaWillems's ray tracing example requires about 0.7 seconds to compose a frame with compute shader with Igalia's Vulkan driver. I've been checking the values of the registers with SaschaWillems computeraytracing on job submission, timeout evaluation and job destroy. The driver detects progress. CSD_CURRENT_CFG4 values are decreased as expected based in the number of pending batches. Next debug info starts with 262144 - 1, and we can see it ends with 0xf. So, I think that using CSD_CURRENT_CFG4 is working as expected. We could see also that CSD_CURRENT_ID0 and CSD_CURRENT_ID1 could also be used to track progress as we see how values are changing for X, Y and Z component of current workgroup ID. But comparisons on CSD_CURRENT_CFG4 are simpler. 10661.766782] [drm] CSD launched: timeout_batches (0x) | cfg4 (0x0003fff9) | id0 (0x0080) | id1 (0x) | status (0x0056) [10662.272345] [drm] CSD timeout check: timeout_batches (0x) -> cfg4 (0x0002dfaf) | id0 (0x0005) | id1 (0x0024) | status (0x0056) [10662.792324] [drm] CSD timeout check: timeout_batches (0x0002dfaf) -> cfg4 (0x00018c1f) | id0 (0x003e) | id1 (0x004e) | status (0x0056) [10663.302350] [drm] CSD timeout check: timeout_batches (0x00018c1f) -> cfg4 (0x724f) | id0 (0x005b) | id1 (0x0071) | status (0x0056) [10663.487139] [drm] CSD free: timeout_batches (0x724f) -> cfg4 (0x) | id0 (0x) | id1 (0x0001) | status (0x0060) Another example is our matrix computation library for QPU: https://github.com/Idein/qmkl6 It requires about 1.1 seconds to multiply two 2048x2048 matrices. Because in general we do not know what is the expected behavior of a QPU program and what is not, we also cannot detect a progress the program is making. This is why we want to have the timeout parameters to be able to be modified. In case of qmkl6, all jobs are launched with CSD_CURRENT_CFG4 is initialized as 7 (Number of batches minus 1) so when the job starts, all the batches are on execution. So, the HW cannot report any progress, and CFG4 reports that there are no pending batches queued with ~0. This is the trace of v3d_submit_csd_ioctl calls showing CFG4.
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649
--- Comment #7 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) ---
That makes more sense then, that's the behavior I'd expect on CrOS/Android.
Also explains why we haven't seen this on X/Wayland based compositors like
GNOME on Ubuntu.
One more log I'd like captured here, the trace log. Please record a log from
before the black screen occurs until after.
I've included a snippet below that shows how to capture this log:
#!/bin/bash
set +x
OUTPUT_DIR=$(pwd)
mount -o remount,rw /
echo 4 > /sys/module/drm/parameters/debug
echo 65536 > /sys/kernel/debug/tracing/buffer_size_kb
cd /sys/kernel/debug/tracing/
echo amdgpu_dm* > set_ftrace_filter
echo function > current_tracer
echo 1 > events/amdgpu_dm/enable
echo 1 > tracing_on
echo > trace
echo "Capturing a trace...\n"
read -n 1
cat trace > $OUTPUT_DIR/trace.txt
echo "Saved trace to ${OUTPUT_DIR}/trace.txt"
dmesg > $OUTPUT_DIR/dmesg.txt
echo "Saved dmesg to ${OUTPUT_DIR}/dmesg.txt"
--
You may reply to this email to add a comment.
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: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation
Am 10.02.21 um 17:39 schrieb Suren Baghdasaryan:
On Wed, Feb 10, 2021 at 5:06 AM Daniel Vetter wrote:
On Tue, Feb 09, 2021 at 12:16:51PM -0800, Suren Baghdasaryan wrote:
On Tue, Feb 9, 2021 at 12:03 PM Daniel Vetter wrote:
On Tue, Feb 9, 2021 at 6:46 PM Christian König wrote:
Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
On Tue, Feb 9, 2021 at 4:57 AM Christian König wrote:
Am 09.02.21 um 13:11 schrieb Christian König:
[SNIP]
+void drm_page_pool_add(struct drm_page_pool *pool, struct page *page)
+{
+ spin_lock(&pool->lock);
+ list_add_tail(&page->lru, &pool->items);
+ pool->count++;
+ atomic_long_add(1 << pool->order, &total_pages);
+ spin_unlock(&pool->lock);
+
+ mod_node_page_state(page_pgdat(page),
NR_KERNEL_MISC_RECLAIMABLE,
+ 1 << pool->order);
Hui what? What should that be good for?
This is a carryover from the ION page pool implementation:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cbb7155447ee149a49f3a08d8cde2685d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637485719618339413%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=IYsJoAd7SUo12V7tS3CCRqNVm569iy%2FtoXQqm2MdC1g%3D&reserved=0
My sense is it helps with the vmstat/meminfo accounting so folks can
see the cached pages are shrinkable/freeable. This maybe falls under
other dmabuf accounting/stats discussions, so I'm happy to remove it
for now, or let the drivers using the shared page pool logic handle
the accounting themselves?
Intentionally separated the discussion for that here.
As far as I can see this is just bluntly incorrect.
Either the page is reclaimable or it is part of our pool and freeable
through the shrinker, but never ever both.
IIRC the original motivation for counting ION pooled pages as
reclaimable was to include them into /proc/meminfo's MemAvailable
calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
non-slab kernel pages" seems like a good place to account for them but
I might be wrong.
Yeah, that's what I see here as well. But exactly that is utterly nonsense.
Those pages are not "free" in the sense that get_free_page could return
them directly.
Well on Android that is kinda true, because Android has it's
oom-killer (way back it was just a shrinker callback, not sure how it
works now), which just shot down all the background apps. So at least
some of that (everything used by background apps) is indeed
reclaimable on Android.
But that doesn't hold on Linux in general, so we can't really do this
for common code.
Also I had a long meeting with Suren, John and other googles
yesterday, and the aim is now to try and support all the Android gpu
memory accounting needs with cgroups. That should work, and it will
allow Android to handle all the Android-ism in a clean way in upstream
code. Or that's at least the plan.
I think the only thing we identified that Android still needs on top
is the dma-buf sysfs stuff, so that shared buffers (which on Android
are always dma-buf, and always stay around as dma-buf fd throughout
their lifetime) can be listed/analyzed with full detail.
But aside from this the plan for all the per-process or per-heap
account, oom-killer integration and everything else is planned to be
done with cgroups.
Until cgroups are ready we probably will need to add a sysfs node to
report the total dmabuf pool size and I think that would cover our
current accounting need here.
As I mentioned, not including dmabuf pools into MemAvailable would
affect that stat and I'm wondering if pools should be considered as
part of MemAvailable or not. Since MemAvailable includes SReclaimable
I think it makes sense to include them but maybe there are other
considerations that I'm missing?
On Android, yes, on upstream, not so much. Because upstream doesn't have
the android low memory killer cleanup up all the apps, so effectively we
can't reclaim that memory, and we shouldn't report it as such.
-Daniel
Hmm. Sorry, I fail to see why Android's low memory killer makes a
difference here. In my mind, the pages in the pools are not used but
kept there in case heaps need them (maybe that's the part I'm wrong?).
These pages can be freed by the shrinker if memory pressure rises.
And exactly that's the difference. They *can* be freed is not the same
thing as they *are* free.
In that sense I think it's very similar to reclaimable slabs which are
already accounted as part of MemAvailable. So it seems logical to me
to include unused pages in the pools here as well. What am I missing?
See the shrinkers are there because you need to do some action before
you can re-use the memory.
In the case of the TTM/DRM pool for example you need to change the
caching attributes which might
Re: [PATCH v6] dt-bindings: display: panel: one file of all simple LVDS panels with dual ports
On Tue, Feb 09, 2021 at 10:13:12PM +0800, Liu Ying wrote:
> To complement panel-simple.yaml, create panel-simple-lvds-dual-ports.yaml.
> panel-simple-lvds-dual-ports.yaml is for all simple LVDS panels that
> have dual LVDS ports and require only a single power-supply.
> The first port receives odd pixels, and the second port receives even pixels.
> Optionally, a backlight and an enable GPIO can be specified as properties.
>
> Panels with swapped pixel order, if any, need dedicated bindings.
>
> Migrate 'auo,g133han01', 'auo,g185han01', 'auo,g190ean01',
> 'koe,tx26d202vm0bwa' and 'nlt,nl192108ac18-02d' over to the new file.
>
> The objectives with one file for all the simple LVDS panels with dual ports
> are:
> - Make it simpler to add bindings for this kind of LVDS panels
> - Keep the number of bindings file lower
> - Keep the binding documentation for this kind of LVDS panels more consistent
> - Make it possible for drivers to get pixel order via
> drm_of_lvds_get_dual_link_pixel_order(), as the 'ports' property is required
>
> Suggested-by: Sam Ravnborg
> Cc: Thierry Reding
> Cc: Sam Ravnborg
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Rob Herring
> Cc: Lucas Stach
> Cc: Sebastian Reichel
> Signed-off-by: Liu Ying
> ---
> v5->v6:
> * Use OF graph schema.
> * Drop Rob's R-b tag, as review is needed.
>
> v4->v5:
> * Require the 'ports' property and update commit message accordingly. (Rob)
> * Add Rob's R-b tag.
>
> v3->v4:
> * Add type and descriptions for dual-lvds-{odd,even}-pixels properties.
> Also, update descriptions for port@0 and port@1 properties accordingly.
> (Rob)
>
> v2->v3:
> * Do not allow 'port' property. (Rob)
> * Define port number. (Rob)
> * Specify 'dual-lvds-odd-pixels' and 'dual-lvds-even-pixels' properties. (Rob)
>
> v1->v2:
> * Correct pixel order in example LVDS panel node.
>
> .../panel/panel-simple-lvds-dual-ports.yaml| 116
> +
> .../bindings/display/panel/panel-simple.yaml | 10 --
> 2 files changed, 116 insertions(+), 10 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/display/panel/panel-simple-lvds-dual-ports.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/panel-simple-lvds-dual-ports.yaml
>
> b/Documentation/devicetree/bindings/display/panel/panel-simple-lvds-dual-ports.yaml
> new file mode 100644
> index ..274e89b
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/panel/panel-simple-lvds-dual-ports.yaml
> @@ -0,0 +1,116 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id:
> http://devicetree.org/schemas/display/panel/panel-simple-lvds-dual-ports.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Simple LVDS panels with one power supply and dual LVDS ports
> +
> +maintainers:
> + - Liu Ying
> + - Thierry Reding
> + - Sam Ravnborg
> +
> +description: |
> + This binding file is a collection of the LVDS panels that
> + has dual LVDS ports and requires only a single power-supply.
> + The first port receives odd pixels, and the second port receives even
> pixels.
> + There are optionally a backlight and an enable GPIO.
> + The panel may use an OF graph binding for the association to the display,
> + or it may be a direct child node of the display.
> +
> + If the panel is more advanced a dedicated binding file is required.
> +
> +allOf:
> + - $ref: panel-common.yaml#
> +
> +properties:
> +
> + compatible:
> +enum:
> +# compatible must be listed in alphabetical order, ordered by compatible.
> +# The description in the comment is mandatory for each compatible.
> +
> +# AU Optronics Corporation 13.3" FHD (1920x1080) TFT LCD panel
> + - auo,g133han01
> +# AU Optronics Corporation 18.5" FHD (1920x1080) TFT LCD panel
> + - auo,g185han01
> +# AU Optronics Corporation 19.0" (1280x1024) TFT LCD panel
> + - auo,g190ean01
> +# Kaohsiung Opto-Electronics Inc. 10.1" WUXGA (1920 x 1200) LVDS TFT
> LCD panel
> + - koe,tx26d202vm0bwa
> +# NLT Technologies, Ltd. 15.6" FHD (1920x1080) LVDS TFT LCD panel
> + - nlt,nl192108ac18-02d
> +
> + ports:
> +$ref: /schemas/graph.yaml#/properties/ports
> +
> +properties:
> + port@0:
> +$ref: /schemas/graph.yaml#/properties/port
If you have any extra properties as you do, then you use
'#/$defs/port-base'.
> +description: The first sink port.
> +
> +properties:
> + dual-lvds-odd-pixels:
> +type: boolean
> +description: The first sink port for odd pixels.
> +
> +required:
> + - dual-lvds-odd-pixels
> +
> + port@1:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: The second sink port.
> +
> +properties:
> + dual-lvds-even-pixels:
> +type: boolean
> +description: The second sink port for even pixels.
> +
>
Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation
Am 10.02.21 um 20:12 schrieb Suren Baghdasaryan:
On Wed, Feb 10, 2021 at 10:32 AM Christian König
wrote:
Am 10.02.21 um 17:39 schrieb Suren Baghdasaryan:
On Wed, Feb 10, 2021 at 5:06 AM Daniel Vetter wrote:
On Tue, Feb 09, 2021 at 12:16:51PM -0800, Suren Baghdasaryan wrote:
On Tue, Feb 9, 2021 at 12:03 PM Daniel Vetter wrote:
On Tue, Feb 9, 2021 at 6:46 PM Christian König wrote:
Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
On Tue, Feb 9, 2021 at 4:57 AM Christian König wrote:
Am 09.02.21 um 13:11 schrieb Christian König:
[SNIP]
+void drm_page_pool_add(struct drm_page_pool *pool, struct page *page)
+{
+ spin_lock(&pool->lock);
+ list_add_tail(&page->lru, &pool->items);
+ pool->count++;
+ atomic_long_add(1 << pool->order, &total_pages);
+ spin_unlock(&pool->lock);
+
+ mod_node_page_state(page_pgdat(page),
NR_KERNEL_MISC_RECLAIMABLE,
+ 1 << pool->order);
Hui what? What should that be good for?
This is a carryover from the ION page pool implementation:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7C4c92877de38440ef18ee08d8cdf7c8ab%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637485811429326241%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Ghp48YBj6R3z4yKsj8ecjXxhZp8g5sJOL0GJRUtCFKY%3D&reserved=0
My sense is it helps with the vmstat/meminfo accounting so folks can
see the cached pages are shrinkable/freeable. This maybe falls under
other dmabuf accounting/stats discussions, so I'm happy to remove it
for now, or let the drivers using the shared page pool logic handle
the accounting themselves?
Intentionally separated the discussion for that here.
As far as I can see this is just bluntly incorrect.
Either the page is reclaimable or it is part of our pool and freeable
through the shrinker, but never ever both.
IIRC the original motivation for counting ION pooled pages as
reclaimable was to include them into /proc/meminfo's MemAvailable
calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
non-slab kernel pages" seems like a good place to account for them but
I might be wrong.
Yeah, that's what I see here as well. But exactly that is utterly nonsense.
Those pages are not "free" in the sense that get_free_page could return
them directly.
Well on Android that is kinda true, because Android has it's
oom-killer (way back it was just a shrinker callback, not sure how it
works now), which just shot down all the background apps. So at least
some of that (everything used by background apps) is indeed
reclaimable on Android.
But that doesn't hold on Linux in general, so we can't really do this
for common code.
Also I had a long meeting with Suren, John and other googles
yesterday, and the aim is now to try and support all the Android gpu
memory accounting needs with cgroups. That should work, and it will
allow Android to handle all the Android-ism in a clean way in upstream
code. Or that's at least the plan.
I think the only thing we identified that Android still needs on top
is the dma-buf sysfs stuff, so that shared buffers (which on Android
are always dma-buf, and always stay around as dma-buf fd throughout
their lifetime) can be listed/analyzed with full detail.
But aside from this the plan for all the per-process or per-heap
account, oom-killer integration and everything else is planned to be
done with cgroups.
Until cgroups are ready we probably will need to add a sysfs node to
report the total dmabuf pool size and I think that would cover our
current accounting need here.
As I mentioned, not including dmabuf pools into MemAvailable would
affect that stat and I'm wondering if pools should be considered as
part of MemAvailable or not. Since MemAvailable includes SReclaimable
I think it makes sense to include them but maybe there are other
considerations that I'm missing?
On Android, yes, on upstream, not so much. Because upstream doesn't have
the android low memory killer cleanup up all the apps, so effectively we
can't reclaim that memory, and we shouldn't report it as such.
-Daniel
Hmm. Sorry, I fail to see why Android's low memory killer makes a
difference here. In my mind, the pages in the pools are not used but
kept there in case heaps need them (maybe that's the part I'm wrong?).
These pages can be freed by the shrinker if memory pressure rises.
And exactly that's the difference. They *can* be freed is not the same
thing as they *are* free.
No argument there. That's why I think meminfo has two separate stats
for MemFree and MemAvailable. MemFree is self-explanatory. The
description of MemAvailable in
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2FDocumentation%2Ffilesystems%2Fproc.txt&data=04%7C01%7Cc
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649
--- Comment #8 from youling...@gmail.com ---
(In reply to Nicholas Kazlauskas from comment #7)
> That makes more sense then, that's the behavior I'd expect on CrOS/Android.
>
> Also explains why we haven't seen this on X/Wayland based compositors like
> GNOME on Ubuntu.
>
> One more log I'd like captured here, the trace log. Please record a log from
> before the black screen occurs until after.
>
> I've included a snippet below that shows how to capture this log:
>
> #!/bin/bash
>
> set +x
> OUTPUT_DIR=$(pwd)
>
> mount -o remount,rw /
> echo 4 > /sys/module/drm/parameters/debug
> echo 65536 > /sys/kernel/debug/tracing/buffer_size_kb
>
> cd /sys/kernel/debug/tracing/
> echo amdgpu_dm* > set_ftrace_filter
> echo function > current_tracer
> echo 1 > events/amdgpu_dm/enable
> echo 1 > tracing_on
> echo > trace
> echo "Capturing a trace...\n"
> read -n 1
> cat trace > $OUTPUT_DIR/trace.txt
> echo "Saved trace to ${OUTPUT_DIR}/trace.txt"
> dmesg > $OUTPUT_DIR/dmesg.txt
> echo "Saved dmesg to ${OUTPUT_DIR}/dmesg.txt"
run the script,only echo Capturing a trace...\n
failed output trace.txt and dmesg.txt.
--
You may reply to this email to add a comment.
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
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 --- Comment #9 from youling...@gmail.com --- Created attachment 295197 --> https://bugzilla.kernel.org/attachment.cgi?id=295197&action=edit dmesg.txt I delete "read -n 1", can output dmesg.txt and trace.txt. trace.txt, # tracer: function # # entries-in-buffer/entries-written: 0/0 #P:8 # #_-=> irqs-off # / _=> need-resched # | / _---=> hardirq/softirq # || / _--=> preempt-depth # ||| / delay # TASK-PID CPU# TIMESTAMP FUNCTION # | | | | | -- You may reply to this email to add a comment. 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
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 --- Comment #10 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) --- I don't expect the trace log to be completely empty. You'll need to leave the trace log running after enabling it as well (with tracing on/trace). You'll also need various config options set in your kernel: CONFIG_FUNCTION_TRACER CONFIG_FUNCTION_GRAPH_TRACER CONFIG_STACK_TRACER CONFIG_DYNAMIC_FTRACE Sorry if this is a little bit involved, but I'd like capture the exact sequence of atomic commits with the trace so I can reproduce the sequence on my end. Thanks! -- You may reply to this email to add a comment. 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
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 --- Comment #11 from youling...@gmail.com --- Created attachment 295201 --> https://bugzilla.kernel.org/attachment.cgi?id=295201&action=edit trace.txt cat /sys/kernel/debug/tracing/trace -- You may reply to this email to add a comment. 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
[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=211649 --- Comment #12 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) --- Thanks, this was what I'm looking for. -- You may reply to this email to add a comment. 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 v3 3/3] dt-bindings: mediatek,dpi: add mt8192 to mediatek,dpi
On Mon, 08 Feb 2021 09:42:21 +0800, Jitao Shi wrote: > Add compatible "mediatek,mt8192-dpi" for the mt8192 dpi. > > Signed-off-by: Jitao Shi > --- > .../devicetree/bindings/display/mediatek/mediatek,dpi.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [v2] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver
On Wed, Feb 10, 2021 at 3:41 AM wrote: > > On 2021-02-01 00:46, Rob Clark wrote: > > On Fri, Dec 18, 2020 at 2:27 AM Kalyan Thota > > wrote: > >> > >> Set the flag vblank_disable_immediate = true to turn off vblank irqs > >> immediately as soon as drm_vblank_put is requested so that there are > >> no irqs triggered during idle state. This will reduce cpu wakeups > >> and help in power saving. > >> > >> To enable vblank_disable_immediate flag the underlying KMS driver > >> needs to support high precision vblank timestamping and also a > >> reliable way of providing vblank counter which is incrementing > >> at the leading edge of vblank. > >> > >> This patch also brings in changes to support vblank_disable_immediate > >> requirement in dpu driver. > >> > >> Changes in v1: > >> - Specify reason to add vblank timestamp support. (Rob) > >> - Add changes to provide vblank counter from dpu driver. > >> > >> Signed-off-by: Kalyan Thota > > > > This seems to be triggering: > > > > [ +0.032668] [ cut here ] > > [ +0.004759] msm ae0.mdss: drm_WARN_ON_ONCE(cur_vblank != > > vblank->last) > > [ +0.24] WARNING: CPU: 0 PID: 362 at > > drivers/gpu/drm/drm_vblank.c:354 drm_update_vblank_count+0x1e4/0x258 > > [ +0.017154] Modules linked in: joydev > > [ +0.003784] CPU: 0 PID: 362 Comm: frecon Not tainted > > 5.11.0-rc5-00037-g33d3504871dd #2 > > [ +0.008135] Hardware name: Google Lazor (rev1 - 2) with LTE (DT) > > [ +0.006167] pstate: 60400089 (nZCv daIf +PAN -UAO -TCO BTYPE=--) > > [ +0.006169] pc : drm_update_vblank_count+0x1e4/0x258 > > [ +0.005105] lr : drm_update_vblank_count+0x1e4/0x258 > > [ +0.005106] sp : ffc010003b70 > > [ +0.003409] x29: ffc010003b70 x28: ff80855d9d98 > > [ +0.005466] x27: x26: 00fe502a > > [ +0.005458] x25: 0001 x24: 0001 > > [ +0.005466] x23: 0001 x22: ff808561ce80 > > [ +0.005465] x21: x20: > > [ +0.005468] x19: ff80850d6800 x18: > > [ +0.005466] x17: x16: > > [ +0.005465] x15: 000a x14: 263b > > [ +0.005466] x13: 0006 x12: > > [ +0.005465] x11: 0010 x10: ffc090003797 > > [ +0.005466] x9 : ffed200e2a8c x8 : > > [ +0.005466] x7 : x6 : ffed213b2b51 > > [ +0.005465] x5 : c000dfff x4 : ffed21218048 > > [ +0.005465] x3 : x2 : > > [ +0.005465] x1 : x0 : > > [ +0.005466] Call trace: > > [ +0.002520] drm_update_vblank_count+0x1e4/0x258 > > [ +0.004748] drm_handle_vblank+0xd0/0x35c > > [ +0.004130] drm_crtc_handle_vblank+0x24/0x30 > > [ +0.004487] dpu_crtc_vblank_callback+0x3c/0xc4 > > [ +0.004662] dpu_encoder_vblank_callback+0x70/0xc4 > > [ +0.004931] dpu_encoder_phys_vid_vblank_irq+0x50/0x12c > > [ +0.005378] dpu_core_irq_callback_handler+0xf4/0xfc > > [ +0.005107] dpu_hw_intr_dispatch_irq+0x100/0x120 > > [ +0.004834] dpu_core_irq+0x44/0x5c > > [ +0.003597] dpu_irq+0x1c/0x28 > > [ +0.003141] msm_irq+0x34/0x40 > > [ +0.003153] __handle_irq_event_percpu+0xfc/0x254 > > [ +0.004838] handle_irq_event_percpu+0x3c/0x94 > > [ +0.004574] handle_irq_event+0x54/0x98 > > [ +0.003944] handle_level_irq+0xa0/0xd0 > > [ +0.003943] generic_handle_irq+0x30/0x48 > > [ +0.004131] dpu_mdss_irq+0xe4/0x118 > > [ +0.003684] generic_handle_irq+0x30/0x48 > > [ +0.004127] __handle_domain_irq+0xa8/0xac > > [ +0.004215] gic_handle_irq+0xdc/0x150 > > [ +0.003856] el1_irq+0xb4/0x180 > > [ +0.003237] dpu_encoder_vsync_time+0x78/0x230 > > [ +0.004574] dpu_encoder_kickoff+0x190/0x354 > > [ +0.004386] dpu_crtc_commit_kickoff+0x194/0x1a0 > > [ +0.004748] dpu_kms_flush_commit+0xf4/0x108 > > [ +0.004390] msm_atomic_commit_tail+0x2e8/0x384 > > [ +0.004661] commit_tail+0x80/0x108 > > [ +0.003588] drm_atomic_helper_commit+0x118/0x11c > > [ +0.004834] drm_atomic_commit+0x58/0x68 > > [ +0.004033] drm_atomic_helper_set_config+0x70/0x9c > > [ +0.005018] drm_mode_setcrtc+0x390/0x584 > > [ +0.004131] drm_ioctl_kernel+0xc8/0x11c > > [ +0.004035] drm_ioctl+0x2f8/0x34c > > [ +0.003500] drm_compat_ioctl+0x48/0xe8 > > [ +0.003945] __arm64_compat_sys_ioctl+0xe8/0x104 > > [ +0.004750] el0_svc_common.constprop.0+0x114/0x188 > > [ +0.005019] do_el0_svc_compat+0x28/0x38 > > [ +0.004031] el0_svc_compat+0x20/0x30 > > [ +0.003772] el0_sync_compat_handler+0x104/0x18c > > [ +0.004749] el0_sync_compat+0x178/0x180 > > [ +0.004034] ---[ end trace 2959d178e74f2555 ]--- > > > > > > BR, > > -R > > > Hi Rob, > > on DPU HW, with prefetch enabled, the frame count increment and vsync > irq are not happening at same instance. This is causing the frame count > to mismatch. > > Example: > |###--^--|###--^--| > > for the above vsync cycle with prefetch enabled
Re: [PATCH] drm/ttm: make sure pool pages are cleared
Am 10.02.21 um 19:15 schrieb Daniel Vetter:
On Wed, Feb 10, 2021 at 5:05 PM Christian König
wrote:
The old implementation wasn't consistend on this.
But it looks like we depend on this so better bring it back.
Signed-off-by: Christian König
Reported-and-tested-by: Mike Galbraith
Fixes: d099fc8f540a ("drm/ttm: new TT backend allocation pool v3")
Well I think in general there's a bit an issue in ttm with not
clearing stuff everywhere. So definitely in favour of clearing stuff.
Looking at the code this only fixes the clearing, at alloc time we're
still very optional with clearing. I think we should just set
__GFP_ZERO unconditionally there too.
No, the alloc handling is actually correct.
We are clearing only when we allocate pages for userspace. Not for the
kernel nor for eviction when pages are overwritten anyway.
The key point is that the old page pool was ignoring the flag for this
in some code paths and I wasn't sure if that's still necessary or not.
Turned out it was necessary after all.
Thanks,
Christian.
With that: Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/ttm/ttm_pool.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 74bf1c84b637..6e27cb1bf48b 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -33,6 +33,7 @@
#include
#include
+#include
#ifdef CONFIG_X86
#include
@@ -218,6 +219,15 @@ static void ttm_pool_unmap(struct ttm_pool *pool,
dma_addr_t dma_addr,
/* Give pages into a specific pool_type */
static void ttm_pool_type_give(struct ttm_pool_type *pt, struct page *p)
{
+ unsigned int i, num_pages = 1 << pt->order;
+
+ for (i = 0; i < num_pages; ++i) {
+ if (PageHighMem(p))
+ clear_highpage(p + i);
+ else
+ clear_page(page_address(p + i));
+ }
+
spin_lock(&pt->lock);
list_add(&p->lru, &pt->pages);
spin_unlock(&pt->lock);
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation
On Wed, Feb 10, 2021 at 5:06 AM Daniel Vetter wrote:
>
> On Tue, Feb 09, 2021 at 12:16:51PM -0800, Suren Baghdasaryan wrote:
> > On Tue, Feb 9, 2021 at 12:03 PM Daniel Vetter wrote:
> > >
> > > On Tue, Feb 9, 2021 at 6:46 PM Christian König
> > > wrote:
> > > >
> > > >
> > > >
> > > > Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
> > > > > On Tue, Feb 9, 2021 at 4:57 AM Christian König
> > > > > wrote:
> > > > >> Am 09.02.21 um 13:11 schrieb Christian König:
> > > > >>> [SNIP]
> > > > >> +void drm_page_pool_add(struct drm_page_pool *pool, struct page
> > > > >> *page)
> > > > >> +{
> > > > >> + spin_lock(&pool->lock);
> > > > >> + list_add_tail(&page->lru, &pool->items);
> > > > >> + pool->count++;
> > > > >> + atomic_long_add(1 << pool->order, &total_pages);
> > > > >> + spin_unlock(&pool->lock);
> > > > >> +
> > > > >> + mod_node_page_state(page_pgdat(page),
> > > > >> NR_KERNEL_MISC_RECLAIMABLE,
> > > > >> + 1 << pool->order);
> > > > > Hui what? What should that be good for?
> > > > This is a carryover from the ION page pool implementation:
> > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cdff8edcd4d147a5b08d8cd20cff2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637484888114923580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9%2BIBC0tezSV6Ci4S3kWfW%2BQvJm4mdunn3dF6C0kyfCw%3D&reserved=0
> > > >
> > > >
> > > > My sense is it helps with the vmstat/meminfo accounting so folks
> > > > can
> > > > see the cached pages are shrinkable/freeable. This maybe falls
> > > > under
> > > > other dmabuf accounting/stats discussions, so I'm happy to remove
> > > > it
> > > > for now, or let the drivers using the shared page pool logic handle
> > > > the accounting themselves?
> > > > >> Intentionally separated the discussion for that here.
> > > > >>
> > > > >> As far as I can see this is just bluntly incorrect.
> > > > >>
> > > > >> Either the page is reclaimable or it is part of our pool and freeable
> > > > >> through the shrinker, but never ever both.
> > > > > IIRC the original motivation for counting ION pooled pages as
> > > > > reclaimable was to include them into /proc/meminfo's MemAvailable
> > > > > calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
> > > > > non-slab kernel pages" seems like a good place to account for them but
> > > > > I might be wrong.
> > > >
> > > > Yeah, that's what I see here as well. But exactly that is utterly
> > > > nonsense.
> > > >
> > > > Those pages are not "free" in the sense that get_free_page could return
> > > > them directly.
> > >
> > > Well on Android that is kinda true, because Android has it's
> > > oom-killer (way back it was just a shrinker callback, not sure how it
> > > works now), which just shot down all the background apps. So at least
> > > some of that (everything used by background apps) is indeed
> > > reclaimable on Android.
> > >
> > > But that doesn't hold on Linux in general, so we can't really do this
> > > for common code.
> > >
> > > Also I had a long meeting with Suren, John and other googles
> > > yesterday, and the aim is now to try and support all the Android gpu
> > > memory accounting needs with cgroups. That should work, and it will
> > > allow Android to handle all the Android-ism in a clean way in upstream
> > > code. Or that's at least the plan.
> > >
> > > I think the only thing we identified that Android still needs on top
> > > is the dma-buf sysfs stuff, so that shared buffers (which on Android
> > > are always dma-buf, and always stay around as dma-buf fd throughout
> > > their lifetime) can be listed/analyzed with full detail.
> > >
> > > But aside from this the plan for all the per-process or per-heap
> > > account, oom-killer integration and everything else is planned to be
> > > done with cgroups.
> >
> > Until cgroups are ready we probably will need to add a sysfs node to
> > report the total dmabuf pool size and I think that would cover our
> > current accounting need here.
> > As I mentioned, not including dmabuf pools into MemAvailable would
> > affect that stat and I'm wondering if pools should be considered as
> > part of MemAvailable or not. Since MemAvailable includes SReclaimable
> > I think it makes sense to include them but maybe there are other
> > considerations that I'm missing?
>
> On Android, yes, on upstream, not so much. Because upstream doesn't have
> the android low memory killer cleanup up all the apps, so effectively we
> can't reclaim that memory, and we shouldn't report it as such.
> -Daniel
Hmm. Sorry, I fail to see why Android's low memory k
Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
Ping! Anybody wanna review the most trivial, obviously correct, and non-intrusive patch ever to fix a real bug? The Kodi devs changed their broken constant and as if by magic, some LG TV now has working HDR support, so we now know the constant is actually expected to be correct by at least some real display hw out there. thanks, -mario On Mon, Jan 25, 2021 at 8:53 PM Mario Kleiner wrote: > > > On Mon, Jan 25, 2021 at 5:05 PM Simon Ser wrote: > >> ‐‐‐ Original Message ‐‐‐ >> >> On Monday, January 25th, 2021 at 5:00 PM, Mario Kleiner < >> mario.kleiner...@gmail.com> wrote: >> >> > On Mon, Jan 25, 2021 at 1:14 PM Simon Ser wrote: >> > >> > > > This is not an uapi defintion anyway so fixing should be fine. >> > > >> > > Oh, my bad, I thought this was in drm_mode.h, but it's not. Then yeah >> > > >> > > should be completely fine to fix it. >> > >> > Good! The beginning of the end of a sad story ;). So i guess i can >> > get your r-b's for it? >> >> Sorry, I haven't verified that this wouldn't break the world, so I'm >> not comfortable giving a R-b. >> >> > Breaking the world is pretty unlikely for an unused #define, but I > understand. > > I guess Ville will have access to the relevant spec to verify: It is the > CTA-861-G spec, table 44 in section 6.9 and also specifically section 6.9.1. > > >> > Will this fix propagate into igt and libdrm? Or are separate fixup >> patches needed? >> >> No, since this is not part of UAPI. >> > > Ok. I'll submit patches once this one landed in the kernel. > > >> >> > Simon, could you let the Kodi devs know in case you have a line to >> > them? I didn't know that there is even one more real-world HDR client >> > for Linux, apart from AMD's amdvlk Vulkan driver, which does things >> > right and doesn't need fixing. >> >> Seems like Kodi hardcodes the bad version: >> >> >> https://github.com/xbmc/xbmc/blob/aa5c2e79c069ba7d0ab1d8ad930e4294bf554680/xbmc/cores/VideoPlayer/Buffers/VideoBufferDRMPRIME.h#L24 >> >> > Thanks. I've filed an issue to them under: > > https://github.com/xbmc/xbmc/issues/19122 > > >> Maybe we should add the good version in UAPI header? >> > > I'm scared that future HDR definitions would be as carefully done and > reviewed as this one, given how much harder it would be to fix them :/ > But maybe that's just exhausted me who spent too many weeks dealing with > HDR bugs everywhere. > > -mario > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
Ping. Any bit of info appreciated wrt. the DCE-11.2+ situation.
-mario
On Mon, Jan 25, 2021 at 8:24 PM Mario Kleiner
wrote:
> Thanks Alex and Nicholas! Brings quite a bit of extra shiny to those older
> asics :)
>
> Nicholas, any thoughts on my cover-letter wrt. why a similar patch (that I
> wrote and tested to no good or bad effect) not seem to be needed on DCN,
> and probably not DCE-11.2+ either? Is what is left in DC for those asic's
> just dead code? My Atombios disassembly sort of pointed into that
> direction, but reading disassembly is not easy on the brain, and my brain
> was getting quite mushy towards the end of digging through all the code. So
> some official statement would add peace of mind on my side. Is there a
> certain DCE version at which your team starts validating output precision /
> HDR etc. on hw?
>
> Thanks,
> -mario
>
>
> On Mon, Jan 25, 2021 at 8:16 PM Kazlauskas, Nicholas <
> nicholas.kazlaus...@amd.com> wrote:
>
>> On 2021-01-25 12:57 p.m., Alex Deucher wrote:
>> > On Thu, Jan 21, 2021 at 1:17 AM Mario Kleiner
>> > wrote:
>> >>
>> >> This fixes corrupted display output in HDMI deep color
>> >> 10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
>> >>
>> >> It will hopefully also provide fixes for other DCE's up to
>> >> DCE-11, assuming those will need similar fixes, but i could
>> >> not test that for HDMI due to lack of suitable hw, so viewer
>> >> discretion is advised.
>> >>
>> >> dce110_stream_encoder_hdmi_set_stream_attribute() is used for
>> >> HDMI setup on all DCE's and is missing color_depth assignment.
>> >>
>> >> dce110_program_pix_clk() is used for pixel clock setup on HDMI
>> >> for DCE 6-11, and is missing color_depth assignment.
>> >>
>> >> Additionally some of the underlying Atombios specific encoder
>> >> and pixelclock setup functions are missing code which is in
>> >> the classic amdgpu kms modesetting path and the in the radeon
>> >> kms driver for DCE6/DCE8.
>> >>
>> >> encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
>> >> and radeon kms classic drivers. Added here, but untested due to
>> >> lack of suitable test hw.
>> >>
>> >> encoder_control_digx_v4() - Added missing setup code.
>> >> Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
>> >> output at 10 bpc and 12 bpc.
>> >>
>> >> Note that encoder_control_digx_v5() has proper setup code in place
>> >> and is used, e.g., by DCE-11.2, but this code wasn't used for deep
>> >> color setup due to the missing cntl.color_depth setup in the calling
>> >> function for HDMI.
>> >>
>> >> set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
>> >> kms. Added here, but untested due to lack of hw.
>> >>
>> >> set_pixel_clock_v6() - Missing setup code added. Successfully tested
>> >> on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
>> >> deep color output with 10 bpc or 12 bpc.
>> >>
>> >> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
>> >>
>> >> Signed-off-by: Mario Kleiner
>> >> Cc: Harry Wentland
>> >
>> > These make sense. I've applied the series. I'll let the display guys
>> > gauge the other points in your cover letter.
>> >
>> > Alex
>>
>> I don't have any concerns with this patch.
>>
>> Even though it's already applied feel free to have my:
>>
>> Reviewed-by: Nicholas Kazlauskas
>>
>> Regards,
>> Nicholas Kazlauskas
>>
>> >
>> >
>> >> ---
>> >> .../drm/amd/display/dc/bios/command_table.c | 61
>> +++
>> >> .../drm/amd/display/dc/dce/dce_clock_source.c | 14 +
>> >> .../amd/display/dc/dce/dce_stream_encoder.c | 1 +
>> >> 3 files changed, 76 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> >> index 070459e3e407..afc10b954ffa 100644
>> >> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> >> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> >> @@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
>> >> cntl->enable_dp_audio);
>> >> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>> >>
>> >> + switch (cntl->color_depth) {
>> >> + case COLOR_DEPTH_888:
>> >> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_101010:
>> >> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_121212:
>> >> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_161616:
>> >> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >> +
>> >> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
>> >> result = BP_RESULT_OK;
>> >>
>> >> @@ -274,6 +291,23 @@ static enum bp_result enc
Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
On Wednesday, February 10th, 2021 at 10:04 PM, Mario Kleiner wrote: > Ping! I now understand the problem better. Reviewed-by: Simon Ser I'll push to drm-misc-next in a few days if no-one complains. Ping me again if I forget. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/amd/display: Fix potential integer overflow
Fix potential integer overflow by casting actual_calculated_clock_100hz
to u64, in order to give the compiler complete information about the
proper arithmetic to use.
Notice that such variable is used in a context that expects
an expression of type u64 (64 bits, unsigned) and the following
expression is currently being evaluated using 32-bit arithmetic:
actual_calculated_clock_100hz * post_divider
Fixes: 7a03fdf628af ("drm/amd/display: fix 64bit division issue on 32bit OS")
Addresses-Coverity-ID: 1501691 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
index bc942725b9d8..dec58b3c42e4 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
@@ -240,7 +240,7 @@ static bool calc_fb_divider_checking_tolerance(
pll_settings->calculated_pix_clk_100hz =
actual_calculated_clock_100hz;
pll_settings->vco_freq =
- div_u64(actual_calculated_clock_100hz * post_divider,
10);
+ div_u64((u64)actual_calculated_clock_100hz *
post_divider, 10);
return true;
}
return false;
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation
On Wed, Feb 10, 2021 at 9:21 AM Daniel Vetter wrote:
>
> On Wed, Feb 10, 2021 at 5:39 PM Suren Baghdasaryan wrote:
> >
> > On Wed, Feb 10, 2021 at 5:06 AM Daniel Vetter wrote:
> > >
> > > On Tue, Feb 09, 2021 at 12:16:51PM -0800, Suren Baghdasaryan wrote:
> > > > On Tue, Feb 9, 2021 at 12:03 PM Daniel Vetter wrote:
> > > > >
> > > > > On Tue, Feb 9, 2021 at 6:46 PM Christian König
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
> > > > > > > On Tue, Feb 9, 2021 at 4:57 AM Christian König
> > > > > > > wrote:
> > > > > > >> Am 09.02.21 um 13:11 schrieb Christian König:
> > > > > > >>> [SNIP]
> > > > > > >> +void drm_page_pool_add(struct drm_page_pool *pool, struct
> > > > > > >> page *page)
> > > > > > >> +{
> > > > > > >> + spin_lock(&pool->lock);
> > > > > > >> + list_add_tail(&page->lru, &pool->items);
> > > > > > >> + pool->count++;
> > > > > > >> + atomic_long_add(1 << pool->order, &total_pages);
> > > > > > >> + spin_unlock(&pool->lock);
> > > > > > >> +
> > > > > > >> + mod_node_page_state(page_pgdat(page),
> > > > > > >> NR_KERNEL_MISC_RECLAIMABLE,
> > > > > > >> + 1 << pool->order);
> > > > > > > Hui what? What should that be good for?
> > > > > > This is a carryover from the ION page pool implementation:
> > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cdff8edcd4d147a5b08d8cd20cff2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637484888114923580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9%2BIBC0tezSV6Ci4S3kWfW%2BQvJm4mdunn3dF6C0kyfCw%3D&reserved=0
> > > > > >
> > > > > >
> > > > > > My sense is it helps with the vmstat/meminfo accounting so
> > > > > > folks can
> > > > > > see the cached pages are shrinkable/freeable. This maybe falls
> > > > > > under
> > > > > > other dmabuf accounting/stats discussions, so I'm happy to
> > > > > > remove it
> > > > > > for now, or let the drivers using the shared page pool logic
> > > > > > handle
> > > > > > the accounting themselves?
> > > > > > >> Intentionally separated the discussion for that here.
> > > > > > >>
> > > > > > >> As far as I can see this is just bluntly incorrect.
> > > > > > >>
> > > > > > >> Either the page is reclaimable or it is part of our pool and
> > > > > > >> freeable
> > > > > > >> through the shrinker, but never ever both.
> > > > > > > IIRC the original motivation for counting ION pooled pages as
> > > > > > > reclaimable was to include them into /proc/meminfo's MemAvailable
> > > > > > > calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
> > > > > > > non-slab kernel pages" seems like a good place to account for
> > > > > > > them but
> > > > > > > I might be wrong.
> > > > > >
> > > > > > Yeah, that's what I see here as well. But exactly that is utterly
> > > > > > nonsense.
> > > > > >
> > > > > > Those pages are not "free" in the sense that get_free_page could
> > > > > > return
> > > > > > them directly.
> > > > >
> > > > > Well on Android that is kinda true, because Android has it's
> > > > > oom-killer (way back it was just a shrinker callback, not sure how it
> > > > > works now), which just shot down all the background apps. So at least
> > > > > some of that (everything used by background apps) is indeed
> > > > > reclaimable on Android.
> > > > >
> > > > > But that doesn't hold on Linux in general, so we can't really do this
> > > > > for common code.
> > > > >
> > > > > Also I had a long meeting with Suren, John and other googles
> > > > > yesterday, and the aim is now to try and support all the Android gpu
> > > > > memory accounting needs with cgroups. That should work, and it will
> > > > > allow Android to handle all the Android-ism in a clean way in upstream
> > > > > code. Or that's at least the plan.
> > > > >
> > > > > I think the only thing we identified that Android still needs on top
> > > > > is the dma-buf sysfs stuff, so that shared buffers (which on Android
> > > > > are always dma-buf, and always stay around as dma-buf fd throughout
> > > > > their lifetime) can be listed/analyzed with full detail.
> > > > >
> > > > > But aside from this the plan for all the per-process or per-heap
> > > > > account, oom-killer integration and everything else is planned to be
> > > > > done with cgroups.
> > > >
> > > > Until cgroups are ready we probably will need to add a sysfs node to
> > > > report the total dmabuf pool size and I think that would cover our
> > > > current accounting need here.
> > > > As I mentioned, not including dmabuf pools into MemAvailable would
> > >
Re: [PATCH] PCI: Also set up legacy files only after sysfs init
On Fri, Feb 05, 2021 at 02:36:32PM +0100, Daniel Vetter wrote:
> We are already doing this for all the regular sysfs files on PCI
> devices, but not yet on the legacy io files on the PCI buses. Thus far
> no problem, but in the next patch I want to wire up iomem revoke
> support. That needs the vfs up and running already to make sure that
> iomem_get_mapping() works.
>
> Wire it up exactly like the existing code in
> pci_create_sysfs_dev_files(). Note that pci_remove_legacy_files()
> doesn't need a check since the one for pci_bus->legacy_io is
> sufficient.
>
> An alternative solution would be to implement a callback in sysfs to
> set up the address space from iomem_get_mapping() when userspace calls
> mmap(). This also works, but Greg didn't really like that just to work
> around an ordering issue when the kernel loads initially.
>
> v2: Improve commit message (Bjorn)
>
> Signed-off-by: Daniel Vetter
Acked-by: Bjorn Helgaas
I wish we weren't extending a known-racy mechanism to do this, but at
least we're not *adding* a brand new race.
> Cc: Stephen Rothwell
> Cc: Jason Gunthorpe
> Cc: Kees Cook
> Cc: Dan Williams
> Cc: Andrew Morton
> Cc: John Hubbard
> Cc: Jérôme Glisse
> Cc: Jan Kara
> Cc: Dan Williams
> Cc: Greg Kroah-Hartman
> Cc: linux...@kvack.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: Bjorn Helgaas
> Cc: linux-...@vger.kernel.org
> ---
> drivers/pci/pci-sysfs.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index fb072f4b3176..0c45b4f7b214 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -927,6 +927,9 @@ void pci_create_legacy_files(struct pci_bus *b)
> {
> int error;
>
> + if (!sysfs_initialized)
> + return;
> +
> b->legacy_io = kcalloc(2, sizeof(struct bin_attribute),
> GFP_ATOMIC);
> if (!b->legacy_io)
> @@ -1448,6 +1451,7 @@ void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
> static int __init pci_sysfs_init(void)
> {
> struct pci_dev *pdev = NULL;
> + struct pci_bus *pbus = NULL;
> int retval;
>
> sysfs_initialized = 1;
> @@ -1459,6 +1463,9 @@ static int __init pci_sysfs_init(void)
> }
> }
>
> + while ((pbus = pci_find_next_bus(pbus)))
> + pci_create_legacy_files(pbus);
> +
> return 0;
> }
> late_initcall(pci_sysfs_init);
> --
> 2.30.0
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 8/9] drm/gem: Associate GEM objects with drm cgroup
On 2/9/2021 2:54 AM, Daniel Vetter wrote: > On Tue, Jan 26, 2021 at 01:46:25PM -0800, Brian Welty wrote: >> This patch adds tracking of which cgroup to make charges against for a >> given GEM object. We associate the current task's cgroup with GEM objects >> as they are created. First user of this is for charging DRM cgroup for >> device memory allocations. The intended behavior is for device drivers to >> make the cgroup charging calls at the time that backing store is allocated >> or deallocated for the object. >> >> Exported functions are provided for charging memory allocations for a >> GEM object to DRM cgroup. To aid in debugging, we store how many bytes >> have been charged inside the GEM object. Add helpers for setting and >> clearing the object's associated cgroup which will check that charges are >> not being leaked. >> >> For shared objects, this may make the charge against a cgroup that is >> potentially not the same cgroup as the process using the memory. Based >> on the memory cgroup's discussion of "memory ownership", this seems >> acceptable [1]. >> >> [1] https://www.kernel.org/doc/Documentation/cgroup-v2.txt, "Memory >> Ownership" >> >> Signed-off-by: Brian Welty > > Since for now we only have the generic gpu/xpu/bikeshed.memory bucket that > counts everything, why don't we also charge in these gem functions? I'm not sure what you mean exactly. You want to merge/move the charging logic proposed in patch #5 (drm_cgroup_try_charge in kernel/cgroup/drm.c) into drm_gem_object_charge_mem() ? Or reading below, I think you are okay keeping the logic separated as is, but you want much of the code in kernel/cgroup/drm.c moved to drivers/gpu/cgroup ? Yes, I see that should allow to reduce number of exported functions. > > Also, that would remove the need for all these functions exported to > drivers. Plus the cgroups setup could also move fully into drm core code, > since all drivers (*) support it > That way this would really be a fully > generic cgroups controller, and we could land it. Patch #2 proposed to have a few setup functions called during drm device registration. You are suggesting to have this more tightly integrated? Okay, can see what that looks like. It's true most of the exported functions from kernel/cgroup/drm.c were taking a struct drm_device pointer, so seems it can be restructured as you suggest. But I guess we will always need some logic in kernel/cgroup/drm.c to encapsulate the allocation of the 'struct cgroup_subsys_state' for this new controller. But I'm not sure I see how this makes the controller 'fully generic' as you describe. > > The other things I'd do: > - drop gpu scheduling controller from the initial patch series. Yes we'll > need it, but we also need vram limits and all these things for full > featured controller. Having the minimal viable cgroup controller in > upstream would unblock all these other things, and we could discuss them > in separate patch series, instead of one big bikeshed that never reaches > full consensus. > > - the xpu thing is probably real, I just chatted with Android people for > their gpu memory accounting needs, and cgroups sounds like a solution > for them too. But unlike on desktop/server linux, on Android all shared > buffers are allocated from dma-buf heaps, so outside of drm, and hence a > cgroup controller that's tightly tied to drm isn't that useful. So I > think we should move the controller/charge functions up one level into > drivers/gpu/cgroups. Hmm, so for this, you are asking for the cgroup logic to not directly use DRM data structures? Okay, that's why you suggest drivers/gpu/cgroups and not drivers/gpu/drm/cgroups. So this is your angle to make it 'fully generic' then. > > On the naming bikeshed I think gpu is perfectly fine, just explain in > the docs that the G stands for "general" :-) Otherwise we might need to > rename drivers/gpu to drivers/xpu too, and that's maybe one bikeshed too > far. Plus, right now it really is the controller for gpu related memory, > even if we extend it to Android (where it would also include > video/camera allocatioons). Extending this cgroup controller to > accelerators in general is maybe a bit too much. > > - The other disambiguation is how we account dma-buf (well, buffer based) > gpu allocations vs HMM gpu memory allocations, that might be worth > clarifying in the docs. > > - Finally to accelerate this further, I think it'd be good to pull out the > cgroup spec for this more minimized series into patch 1, as a draft. > That way we could get all stakeholders to ack on that ack, so hopefully > we're building something that will work for everyone. That way we can > hopefully untangle the controller design discussions from the > implementation bikeshedding as much as possible. Okay, thanks for all the inputs. I agree the 'cgroup spec' should be in first patch. Can redo this way as well. As much of the
Re: [PATCH v4 11/14] drm/amdgpu: Guard against write accesses after device removal
On 2/9/21 10:40 AM, Christian König wrote:
Am 09.02.21 um 15:30 schrieb Andrey Grodzovsky:
[SNIP]
Question - Why can't we just set those PTEs to point to system memory
(another RO dummy page)
filled with 1s ?
Then writes are not discarded. E.g. the 1s would change to something else.
Christian.
I see but, what about marking the mappings as RO and discarding the write
access page faults continuously until the device is finalized ?
Regarding using an unused range behind the upper bridge as Daniel suggested,
I wonder will this interfere with
the upcoming feature to support BARs movement during hot plug -
https://www.spinics.net/lists/linux-pci/msg103195.html ?
In the picture in my head the address will never be used.
But it doesn't even needs to be an unused range of the root bridge. That one
is usually stuffed full by the BIOS.
For my BAR resize work I looked at quite a bunch of NB documentation. At least
for AMD CPUs we should always have an MMIO address which is never ever used.
That makes this platform/CPU dependent, but the code is just minimal.
The really really nice thing about this approach is that you could unit test
and audit the code for problems even without *real* hotplug hardware. E.g. we
can swap the kernel PTEs and see how the whole power and display code reacts
to that etc
Christian.
Tried to play with hacking mmio tracer as Daniel suggested but just hanged the
machine so...
Can you tell me how to dynamically obtain this kind of unused MMIO address ? I
think given such address, writing
to which is dropped and reading from return all 1s, I can then do something like
bellow, if that what u meant -
for (address = adev->rmmio; address < adev->rmmio_size; adress += PAGE_SIZE) {
old_pte = get_locked_pte(init_mm, address)
dummy_pte = pfn_pte(fake_mmio_address, 0);
set_pte(&old_pte, dummy_pte)
}
flush_tlb ???
P.S I hope to obtain thunderbolt eGPU adapter soon so even if this won't work I
still will be able to test how the driver handles all 1s.
Andrey
Andrey
Andrey
Christian.
It's a nifty idea indeed otherwise ...
-Daniel
Regards,
Christian.
But ugh ...
Otoh validating an entire driver like amdgpu without such a trick
against 0xff reads is practically impossible. So maybe you need to
add
this as one of the tasks here?
Or I could just for validation purposes return ~0 from all reg
reads in the code
and ignore writes if drm_dev_unplugged, this could already easily
validate a big
portion of the code flow under such scenario.
Hm yeah if your really wrap them all, that should work too. Since
iommappings have __iomem pointer type, as long as amdgpu is sparse
warning free, should be doable to guarantee this.
Problem is that ~0 is not always a valid register value.
You would need to audit every register read that it doesn't use the
returned
value blindly as index or similar. That is quite a bit of work.
Yeah that's the entire crux here :-/
-Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
On Wed, Feb 10, 2021 at 10:14 PM Simon Ser wrote: > On Wednesday, February 10th, 2021 at 10:04 PM, Mario Kleiner < > mario.kleiner...@gmail.com> wrote: > > > Ping! > > I now understand the problem better. > > Reviewed-by: Simon Ser > > I'll push to drm-misc-next in a few days if no-one complains. Ping me > again if I forget. > Thanks! I'll prepare patches with the same fix for libdrm and igt as well soon. -mario ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 211651] Dual screen not working with Nvidia GTX 1050ti in a notebook
https://bugzilla.kernel.org/show_bug.cgi?id=211651 Artem S. Tashkinov (a...@gmx.com) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |WILL_NOT_FIX --- Comment #3 from Artem S. Tashkinov (a...@gmx.com) --- NVIDIA drivers are outside of control of Linux kernel developers, so you're very unlikely to get any help here. Please try asking here: https://forums.developer.nvidia.com/c/gpu-unix-graphics/linux/148 -- You may reply to this email to add a comment. 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 2/2] PCI: Revoke mappings like devmem
I see I already acked this, but if you haven't merged it yet there are
a few typos in the commit log:
On Thu, Feb 04, 2021 at 05:58:31PM +0100, Daniel Vetter wrote:
> Since 3234ac664a87 ("/dev/mem: Revoke mappings when a driver claims
> the region") /dev/kmem zaps ptes when the kernel requests exclusive
> acccess to an iomem region. And with CONFIG_IO_STRICT_DEVMEM, this is
> the default for all driver uses.
s/ptes/PTEs/
> Except there's two more ways to access PCI BARs: sysfs and proc mmap
> support. Let's plug that hole.
s/there's two/there are two/
> For revoke_devmem() to work we need to link our vma into the same
> address_space, with consistent vma->vm_pgoff. ->pgoff is already
> adjusted, because that's how (io_)remap_pfn_range works, but for the
> mapping we need to adjust vma->vm_file->f_mapping. The cleanest way is
> to adjust this at at ->open time:
>
> - for sysfs this is easy, now that binary attributes support this. We
> just set bin_attr->mapping when mmap is supported
> - for procfs it's a bit more tricky, since procfs pci access has only
> one file per device, and access to a specific resources first needs
> to be set up with some ioctl calls. But mmap is only supported for
> the same resources as sysfs exposes with mmap support, and otherwise
> rejected, so we can set the mapping unconditionally at open time
> without harm.
s/pci access/PCI access/
s/a specific resources/a specific resource/
> A special consideration is for arch_can_pci_mmap_io() - we need to
> make sure that the ->f_mapping doesn't alias between ioport and iomem
> space. There's only 2 ways in-tree to support mmap of ioports: generic
> pci mmap (ARCH_GENERIC_PCI_MMAP_RESOURCE), and sparc as the single
> architecture hand-rolling. Both approach support ioport mmap through a
> special pfn range and not through magic pte attributes. Aliasing is
> therefore not a problem.
s/There's only 2/There are only two/
s/pci mmap/PCI mmap/
s/Both approach/Both approaches/
s/pfn/PFN/
s/pte/PTE/
> The only difference in access checks left is that sysfs PCI mmap does
> not check for CAP_RAWIO. I'm not really sure whether that should be
> added or not.
>
> Acked-by: Bjorn Helgaas
> Reviewed-by: Dan Williams
> Signed-off-by: Daniel Vetter
> Cc: Stephen Rothwell
> Cc: Jason Gunthorpe
> Cc: Kees Cook
> Cc: Dan Williams
> Cc: Andrew Morton
> Cc: John Hubbard
> Cc: Jérôme Glisse
> Cc: Jan Kara
> Cc: Dan Williams
> Cc: Greg Kroah-Hartman
> Cc: linux...@kvack.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: Bjorn Helgaas
> Cc: linux-...@vger.kernel.org
> ---
> drivers/pci/pci-sysfs.c | 4
> drivers/pci/proc.c | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 0c45b4f7b214..f8afd54ca3e1 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -942,6 +942,7 @@ void pci_create_legacy_files(struct pci_bus *b)
> b->legacy_io->read = pci_read_legacy_io;
> b->legacy_io->write = pci_write_legacy_io;
> b->legacy_io->mmap = pci_mmap_legacy_io;
> + b->legacy_io->mapping = iomem_get_mapping();
> pci_adjust_legacy_attr(b, pci_mmap_io);
> error = device_create_bin_file(&b->dev, b->legacy_io);
> if (error)
> @@ -954,6 +955,7 @@ void pci_create_legacy_files(struct pci_bus *b)
> b->legacy_mem->size = 1024*1024;
> b->legacy_mem->attr.mode = 0600;
> b->legacy_mem->mmap = pci_mmap_legacy_mem;
> + b->legacy_io->mapping = iomem_get_mapping();
> pci_adjust_legacy_attr(b, pci_mmap_mem);
> error = device_create_bin_file(&b->dev, b->legacy_mem);
> if (error)
> @@ -1169,6 +1171,8 @@ static int pci_create_attr(struct pci_dev *pdev, int
> num, int write_combine)
> res_attr->mmap = pci_mmap_resource_uc;
> }
> }
> + if (res_attr->mmap)
> + res_attr->mapping = iomem_get_mapping();
> res_attr->attr.name = res_attr_name;
> res_attr->attr.mode = 0600;
> res_attr->size = pci_resource_len(pdev, num);
> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index 3a2f90beb4cb..9bab07302bbf 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -298,6 +298,7 @@ static int proc_bus_pci_open(struct inode *inode, struct
> file *file)
> fpriv->write_combine = 0;
>
> file->private_data = fpriv;
> + file->f_mapping = iomem_get_mapping();
>
> return 0;
> }
> --
> 2.30.0
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] amdgpu drm-fixes-5.11
Hi Dave, Daniel, Just a single revert to fix a blank screen. The following changes since commit 92bf22614b21a2706f4993b278017e437f7785b3: Linux 5.11-rc7 (2021-02-07 13:57:38 -0800) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-fixes-5.11-2021-02-10 for you to fetch changes up to cf050f96e0970a557601953ed7269d07a7885078: Revert "drm/amd/display: Update NV1x SR latency values" (2021-02-09 23:23:18 -0500) amd-drm-fixes-5.11-2021-02-10: amdgpu: - Blank screen fix Alex Deucher (1): Revert "drm/amd/display: Update NV1x SR latency values" drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/amd/pm: Replace one-element array with flexible-array in struct SISLANDS_SMC_SWSTATE
There is a regular need in the kernel to provide a way to declare having
a dynamically sized set of trailing elements in a structure. Kernel code
should always use “flexible array members”[1] for these cases. The older
style of one-element or zero-length arrays should no longer be used[2].
Refactor the code according to the use of a flexible-array member in
struct SISLANDS_SMC_SWSTATE, instead of a one-element array, and use
the struct_size() helper to calculate the size for the allocation.
Also, this helps with the ongoing efforts to enable -Warray-bounds and
fix the following warnings:
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/si_dpm.c:2448:20: warning: array
subscript 1 is above array bounds of ‘SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’
{aka ‘struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’} [-Warray-bounds]
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/si_dpm.c:2449:20: warning: array
subscript 1 is above array bounds of ‘SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’
{aka ‘struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’} [-Warray-bounds]
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/si_dpm.c:2450:20: warning: array
subscript 1 is above array bounds of ‘SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’
{aka ‘struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’} [-Warray-bounds]
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/si_dpm.c:2451:20: warning: array
subscript 1 is above array bounds of ‘SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’
{aka ‘struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’} [-Warray-bounds]
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/si_dpm.c:2452:20: warning: array
subscript 1 is above array bounds of ‘SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’
{aka ‘struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’} [-Warray-bounds]
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/si_dpm.c:5570:20: warning: array
subscript 1 is above array bounds of ‘SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’
{aka ‘struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’} [-Warray-bounds]
[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2]
https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Build-tested-by: kernel test robot
Link: https://lore.kernel.org/lkml/6023be58.sk66l%2fv4vusji5mi%25...@intel.com/
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/pm/powerplay/si_dpm.c | 6 ++
drivers/gpu/drm/amd/pm/powerplay/sislands_smc.h | 10 +-
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
index afa1711c9620..62291358fb1c 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
@@ -5715,11 +5715,9 @@ static int si_upload_sw_state(struct amdgpu_device *adev,
int ret;
u32 address = si_pi->state_table_start +
offsetof(SISLANDS_SMC_STATETABLE, driverState);
- u32 state_size = sizeof(SISLANDS_SMC_SWSTATE) +
- ((new_state->performance_level_count - 1) *
-sizeof(SISLANDS_SMC_HW_PERFORMANCE_LEVEL));
SISLANDS_SMC_SWSTATE *smc_state = &si_pi->smc_statetable.driverState;
-
+ size_t state_size = struct_size(smc_state, levels,
+ new_state->performance_level_count);
memset(smc_state, 0, state_size);
ret = si_convert_power_state_to_smc(adev, amdgpu_new_state, smc_state);
diff --git a/drivers/gpu/drm/amd/pm/powerplay/sislands_smc.h
b/drivers/gpu/drm/amd/pm/powerplay/sislands_smc.h
index d2930eceaf3c..0f7554052c90 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/sislands_smc.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/sislands_smc.h
@@ -182,11 +182,11 @@ typedef struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL
SISLANDS_SMC_HW_PERFORMANCE_LEV
struct SISLANDS_SMC_SWSTATE
{
-uint8_t flags;
-uint8_t levelCount;
-uint8_t padding2;
-uint8_t padding3;
-SISLANDS_SMC_HW_PERFORMANCE_LEVEL levels[1];
+ uint8_t flags;
+ uint8_t levelCount;
+ uint8_t padding2;
+ uint8_t padding3;
+ SISLANDS_SMC_HW_PERFORMANCE_LEVEL levels[];
};
typedef struct SISLANDS_SMC_SWSTATE SISLANDS_SMC_SWSTATE;
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH v2 11/16] dt-bindings: gpu: v3d: Add BCM2711's compatible
On Tue, 09 Feb 2021 13:59:07 +0100, Nicolas Saenz Julienne wrote: > BCM2711, Raspberry Pi 4's SoC, contains a V3D core. So add its specific > compatible to the bindings. > > Signed-off-by: Nicolas Saenz Julienne > --- > Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation
On Wed, Feb 10, 2021 at 10:32 AM Christian König
wrote:
>
>
>
> Am 10.02.21 um 17:39 schrieb Suren Baghdasaryan:
> > On Wed, Feb 10, 2021 at 5:06 AM Daniel Vetter wrote:
> >> On Tue, Feb 09, 2021 at 12:16:51PM -0800, Suren Baghdasaryan wrote:
> >>> On Tue, Feb 9, 2021 at 12:03 PM Daniel Vetter wrote:
> On Tue, Feb 9, 2021 at 6:46 PM Christian König
> wrote:
> >
> >
> > Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
> >> On Tue, Feb 9, 2021 at 4:57 AM Christian König
> >> wrote:
> >>> Am 09.02.21 um 13:11 schrieb Christian König:
> [SNIP]
> >>> +void drm_page_pool_add(struct drm_page_pool *pool, struct page
> >>> *page)
> >>> +{
> >>> + spin_lock(&pool->lock);
> >>> + list_add_tail(&page->lru, &pool->items);
> >>> + pool->count++;
> >>> + atomic_long_add(1 << pool->order, &total_pages);
> >>> + spin_unlock(&pool->lock);
> >>> +
> >>> + mod_node_page_state(page_pgdat(page),
> >>> NR_KERNEL_MISC_RECLAIMABLE,
> >>> + 1 << pool->order);
> >> Hui what? What should that be good for?
> > This is a carryover from the ION page pool implementation:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cbb7155447ee149a49f3a08d8cde2685d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637485719618339413%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=IYsJoAd7SUo12V7tS3CCRqNVm569iy%2FtoXQqm2MdC1g%3D&reserved=0
> >
> >
> > My sense is it helps with the vmstat/meminfo accounting so folks can
> > see the cached pages are shrinkable/freeable. This maybe falls under
> > other dmabuf accounting/stats discussions, so I'm happy to remove it
> > for now, or let the drivers using the shared page pool logic handle
> > the accounting themselves?
> >>> Intentionally separated the discussion for that here.
> >>>
> >>> As far as I can see this is just bluntly incorrect.
> >>>
> >>> Either the page is reclaimable or it is part of our pool and freeable
> >>> through the shrinker, but never ever both.
> >> IIRC the original motivation for counting ION pooled pages as
> >> reclaimable was to include them into /proc/meminfo's MemAvailable
> >> calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
> >> non-slab kernel pages" seems like a good place to account for them but
> >> I might be wrong.
> > Yeah, that's what I see here as well. But exactly that is utterly
> > nonsense.
> >
> > Those pages are not "free" in the sense that get_free_page could return
> > them directly.
> Well on Android that is kinda true, because Android has it's
> oom-killer (way back it was just a shrinker callback, not sure how it
> works now), which just shot down all the background apps. So at least
> some of that (everything used by background apps) is indeed
> reclaimable on Android.
>
> But that doesn't hold on Linux in general, so we can't really do this
> for common code.
>
> Also I had a long meeting with Suren, John and other googles
> yesterday, and the aim is now to try and support all the Android gpu
> memory accounting needs with cgroups. That should work, and it will
> allow Android to handle all the Android-ism in a clean way in upstream
> code. Or that's at least the plan.
>
> I think the only thing we identified that Android still needs on top
> is the dma-buf sysfs stuff, so that shared buffers (which on Android
> are always dma-buf, and always stay around as dma-buf fd throughout
> their lifetime) can be listed/analyzed with full detail.
>
> But aside from this the plan for all the per-process or per-heap
> account, oom-killer integration and everything else is planned to be
> done with cgroups.
> >>> Until cgroups are ready we probably will need to add a sysfs node to
> >>> report the total dmabuf pool size and I think that would cover our
> >>> current accounting need here.
> >>> As I mentioned, not including dmabuf pools into MemAvailable would
> >>> affect that stat and I'm wondering if pools should be considered as
> >>> part of MemAvailable or not. Since MemAvailable includes SReclaimable
> >>> I think it makes sense to include them but maybe there are other
> >>> considerations that I'm missing?
> >> On Android, yes, on upstream, not so much. Because upstream doesn't have
> >> the android low memory killer cleanup up all the apps, so effectively we
> >> can't reclaim that memory, and we shouldn't report it as such.
> >> -D
Re: [Intel-gfx] [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY"
On Wed, Feb 10, 2021 at 05:07:03PM +0200, Ville Syrjälä wrote: > On Wed, Feb 10, 2021 at 01:38:45PM +, Simon Ser wrote: > > On Wednesday, February 10th, 2021 at 2:16 PM, Daniel Vetter > > wrote: > > > > > On Tue, Feb 09, 2021 at 04:14:01PM -0800, Manasi Navare wrote: > > > > > > > These additional checks added to avoid EBUSY give unnecessary WARN_ON > > > > in case of big joiner used in i915 in which case even if the modeset > > > > is requested on a single pipe, internally another consecutive > > > > pipe is stolen and used to drive half of the transcoder timings. > > > > So in this case it is expected that requested crtc and affected crtcs > > > > do not match. Hence the added WARN ON becomes irrelevant. > > > > The WARN_ON only happens if allow_modeset == false. If allow_modeset == > > true, > > then the driver is allowed to steal an unrelated pipe. > > > > Maybe i915 is stealing a pipe without allow_modeset? > > No. All page flips etc. will have to get split up internally > between multiple crtcs. > > So I think there's basically three options: > a) massive rewrite of i915 to bypass even more of drm_atomic stuff > b) allow i915 to silence that warning, which opens up the question >whether the warn is doing any good if it can just be bypassed > c) nuke the warning entirely > > a) is not going to happen, and it would any way allow i915 to > do things any which way it wants without tripping the warn, > rendering the warn entirely toothless. > > Hmm. Maybe there is a d) which would be to ignore all crtcs > that are not logically enabled in the warn? Not sure if that > could allow something to slit through that people want it to > catch? So as per the offline IRC discussions, - We can check for crtc_state->enable and only use the enabled crtcs in the affected crtc calculation. And this enable would only be set when modeset is done. So in case of bigjoiner no modeset on Pipe A, even if Pipe B is stolen, since no modeset and because that pipe doesnt get enabled the affected crtcs would still be 0x1. This should solve the problem. Ville, Danvet - I will make this change? Manasi > > -- > Ville Syrjälä > Intel > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/amd/pm: Replace one-element array with flexible-array in struct _ATOM_Vega10_GFXCLK_Dependency_Table
There is a regular need in the kernel to provide a way to declare having
a dynamically sized set of trailing elements in a structure. Kernel code
should always use “flexible array members”[1] for these cases. The older
style of one-element or zero-length arrays should no longer be used[2].
Use flexible-array member in struct _ATOM_Vega10_GFXCLK_Dependency_Table,
instead of one-element array.
Also, this helps with the ongoing efforts to enable -Warray-bounds and
fix the following warning:
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/vega10_hwmgr.c: In function
‘vega10_get_pp_table_entry_callback_func’:
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/vega10_hwmgr.c:3113:30:
warning: array subscript 4 is above array bounds of
‘ATOM_Vega10_GFXCLK_Dependency_Record[1]’ {aka ‘struct
_ATOM_Vega10_GFXCLK_Dependency_Record[1]’} [-Warray-bounds]
3113 | gfxclk_dep_table->entries[4].ulClk;
| ~^~~
[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2]
https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Build-tested-by: kernel test robot
Link: https://lore.kernel.org/lkml/6023ff3d.wy3ssckgrqpdplvo%25...@intel.com/
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_pptable.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_pptable.h
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_pptable.h
index c934e9612c1b..9c479bd9a786 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_pptable.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_pptable.h
@@ -161,9 +161,9 @@ typedef struct _ATOM_Vega10_MCLK_Dependency_Record {
} ATOM_Vega10_MCLK_Dependency_Record;
typedef struct _ATOM_Vega10_GFXCLK_Dependency_Table {
-UCHAR ucRevId;
-UCHAR ucNumEntries; /* Number of
entries. */
-ATOM_Vega10_GFXCLK_Dependency_Record entries[1];/* Dynamically
allocate entries. */
+ UCHAR ucRevId;
+ UCHAR ucNumEntries; /* Number of
entries. */
+ ATOM_Vega10_GFXCLK_Dependency_Record entries[]; /* Dynamically
allocate entries. */
} ATOM_Vega10_GFXCLK_Dependency_Table;
typedef struct _ATOM_Vega10_MCLK_Dependency_Table {
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
