Drop use of DRM_WAIT_ON() [Was: drm/drm_vblank: Change EINVAL by the correct errno]
Hi Rodrigo.
On Wed, Jun 12, 2019 at 11:10:54PM -0300, Rodrigo Siqueira wrote:
> For historical reason, the function drm_wait_vblank_ioctl always return
> -EINVAL if something gets wrong. This scenario limits the flexibility
> for the userspace make detailed verification of the problem and take
> some action. In particular, the validation of “if (!dev->irq_enabled)”
> in the drm_wait_vblank_ioctl is responsible for checking if the driver
> support vblank or not. If the driver does not support VBlank, the
> function drm_wait_vblank_ioctl returns EINVAL which does not represent
> the real issue; this patch changes this behavior by return EOPNOTSUPP.
> Additionally, some operations are unsupported by this function, and
> returns EINVAL; this patch also changes the return value to EOPNOTSUPP
> in this case. Lastly, the function drm_wait_vblank_ioctl is invoked by
> libdrm, which is used by many compositors; because of this, it is
> important to check if this change breaks any compositor. In this sense,
> the following projects were examined:
>
> * Drm-hwcomposer
> * Kwin
> * Sway
> * Wlroots
> * Wayland-core
> * Weston
> * Xorg (67 different drivers)
>
> For each repository the verification happened in three steps:
>
> * Update the main branch
> * Look for any occurrence "drmWaitVBlank" with the command:
> git grep -n "drmWaitVBlank"
> * Look in the git history of the project with the command:
> git log -SdrmWaitVBlank
>
> Finally, none of the above projects validate the use of EINVAL which
> make safe, at least for these projects, to change the return values.
>
> Change since V2:
> Daniel Vetter and Chris Wilson
> - Replace ENOTTY by EOPNOTSUPP
> - Return EINVAL if the parameters are wrong
>
> Signed-off-by: Rodrigo Siqueira
> ---
> Update:
> Now IGT has a way to validate if a driver has vblank support or not.
> See:
> https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/2d244aed69165753f3adbbd6468db073dc1acf9A
>
> drivers/gpu/drm/drm_vblank.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 0d704bddb1a6..d76a783a7d4b 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1578,10 +1578,10 @@ int drm_wait_vblank_ioctl(struct drm_device *dev,
> void *data,
> unsigned int flags, pipe, high_pipe;
>
> if (!dev->irq_enabled)
> - return -EINVAL;
> + return -EOPNOTSUPP;
>
> if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
> - return -EINVAL;
> + return -EOPNOTSUPP;
>
> if (vblwait->request.type &
> ~(_DRM_VBLANK_TYPES_MASK | _DRM_VBLANK_FLAGS_MASK |
When touching this function, could I ask you to take a look at
eliminating the use of DRM_WAIT_ON()?
It comes from the deprecated drm_os_linux.h header, and it is only of
the few remaining users of DRM_WAIT_ON().
Below you can find my untested first try - where I did an attempt not to
change behaviour.
Sam
commit 17b119b02467356198b57bca9949b146082bcaa1
Author: Sam Ravnborg
Date: Thu May 30 09:38:47 2019 +0200
drm/vblank: drop use of DRM_WAIT_ON()
DRM_WAIT_ON() is from the deprecated drm_os_linux header and
the modern replacement is the wait_event_*.
The return values differ, so a conversion is needed to
keep the original interface towards userspace.
Introduced a switch/case to make code obvious and to allow
different debug prints depending on the result.
The timeout value of 3 * HZ was translated to 30 msec
Signed-off-by: Sam Ravnborg
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 0d704bddb1a6..51fc6b106333 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -31,7 +31,6 @@
#include
#include
#include
-#include
#include
#include "drm_internal.h"
@@ -1668,18 +1667,27 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
*data,
if (req_seq != seq) {
DRM_DEBUG("waiting on vblank count %llu, crtc %u\n",
req_seq, pipe);
- DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
- vblank_passed(drm_vblank_count(dev, pipe),
- req_seq) ||
- !READ_ONCE(vblank->enabled));
+ ret = wait_event_interruptible_timeout(vblank->queue,
+ vblank_passed(drm_vblank_count(dev, pipe), req_seq) ||
+ !READ_ONCE(vblank->enabled),
+ msecs_to_jiffies(30));
}
- if (ret != -EINTR) {
+ switch (ret) {
+ case 1:
+ ret = 0;
drm_wait_vblank_reply(dev, pipe, >reply);
-
DRM_DEBUG("crtc %d returning %u to
Re: [PATCH] drm/amd/display: fix compilation error
On Wed, Jun 12, 2019 at 10:34 PM Hariprasad Kelam wrote: > > this patch fixes below compilation error > > drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c: In > function ‘dcn10_apply_ctx_for_surface’: > drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2378:3: > error: implicit declaration of function ‘udelay’ > [-Werror=implicit-function-declaration] >udelay(underflow_check_delay_us); > > Signed-off-by: Hariprasad Kelam What branch is this patch based on? Alex > --- > drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c > b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c > index d2352949..1ac9a4f 100644 > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c > @@ -23,6 +23,7 @@ > * > */ > > +#include > #include "dm_services.h" > #include "core_types.h" > #include "resource.h" > -- > 2.7.4 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #11 from Ilia Mirkin --- (In reply to Christian König from comment #10) > (In reply to Gert Wollny from comment #9) > > Indeed, currently the code only tests whether compute shaders are supported, > > and DIV and TEX_LZ don't have any caps yet. I guess I'll take in on me to > > add these caps. > > Well the key point is probably rather that we should not use compute shaders > on that hw generation in the first place. > > How did that got enabled? It just checks for PIPE_CAP_COMPUTE (which *is* supported by r600). The change was targeted at radeonsi. I had to disable PIPE_CAP_COMPUTE on nv50 to work around this as well, but there it wasn't _really_ supported very well so it was OK. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] amdgpu drm-fixes-5.2
Hi Dave, Daniel,
Fixes for 5.2:
- Extend previous vce fix for resume to uvd and vcn
- Fix bounds checking in ras debugfs interface
- Fix a regression on SI using amdgpu
The following changes since commit 671e2ee5ee2127179ca884b439ab6001a623edd6:
Merge branch 'linux-5.2' of git://github.com/skeggsb/linux into drm-fixes
(2019-06-07 17:16:00 +1000)
are available in the Git repository at:
git://people.freedesktop.org/~agd5f/linux drm-fixes-5.2
for you to fetch changes up to f3a5231c8f14acd42845e9e60f506b4e948f0e68:
drm/amdgpu: return 0 by default in amdgpu_pm_load_smu_firmware (2019-06-12
20:39:49 -0500)
Alex Deucher (1):
drm/amdgpu: return 0 by default in amdgpu_pm_load_smu_firmware
Dan Carpenter (1):
drm/amdgpu: Fix bounds checking in amdgpu_ras_is_supported()
Shirish S (1):
drm/amdgpu/{uvd,vcn}: fetch ring's read_ptr after alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 +++-
drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 5 -
drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 5 -
5 files changed, 15 insertions(+), 5 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RESEND PATCH V3] drm/drm_vblank: Change EINVAL by the correct errno
For historical reason, the function drm_wait_vblank_ioctl always return -EINVAL if something gets wrong. This scenario limits the flexibility for the userspace make detailed verification of the problem and take some action. In particular, the validation of “if (!dev->irq_enabled)” in the drm_wait_vblank_ioctl is responsible for checking if the driver support vblank or not. If the driver does not support VBlank, the function drm_wait_vblank_ioctl returns EINVAL which does not represent the real issue; this patch changes this behavior by return EOPNOTSUPP. Additionally, some operations are unsupported by this function, and returns EINVAL; this patch also changes the return value to EOPNOTSUPP in this case. Lastly, the function drm_wait_vblank_ioctl is invoked by libdrm, which is used by many compositors; because of this, it is important to check if this change breaks any compositor. In this sense, the following projects were examined: * Drm-hwcomposer * Kwin * Sway * Wlroots * Wayland-core * Weston * Xorg (67 different drivers) For each repository the verification happened in three steps: * Update the main branch * Look for any occurrence "drmWaitVBlank" with the command: git grep -n "drmWaitVBlank" * Look in the git history of the project with the command: git log -SdrmWaitVBlank Finally, none of the above projects validate the use of EINVAL which make safe, at least for these projects, to change the return values. Change since V2: Daniel Vetter and Chris Wilson - Replace ENOTTY by EOPNOTSUPP - Return EINVAL if the parameters are wrong Signed-off-by: Rodrigo Siqueira --- Update: Now IGT has a way to validate if a driver has vblank support or not. See: https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/2d244aed69165753f3adbbd6468db073dc1acf9A drivers/gpu/drm/drm_vblank.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 0d704bddb1a6..d76a783a7d4b 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -1578,10 +1578,10 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data, unsigned int flags, pipe, high_pipe; if (!dev->irq_enabled) - return -EINVAL; + return -EOPNOTSUPP; if (vblwait->request.type & _DRM_VBLANK_SIGNAL) - return -EINVAL; + return -EOPNOTSUPP; if (vblwait->request.type & ~(_DRM_VBLANK_TYPES_MASK | _DRM_VBLANK_FLAGS_MASK | -- 2.21.0 signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110907] Xorg 1.19.6 segfaults at address 0x6d with mesa 19.1
https://bugs.freedesktop.org/show_bug.cgi?id=110907 Bug ID: 110907 Summary: Xorg 1.19.6 segfaults at address 0x6d with mesa 19.1 Product: Mesa Version: 19.1 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: hvba...@gmail.com QA Contact: dri-devel@lists.freedesktop.org Created attachment 144525 --> https://bugs.freedesktop.org/attachment.cgi?id=144525=edit Xorg log with backtrace After compiling the latest mesa 19.1.0 version on Ubuntu 16.04 with libdrm 2.4.97 (or 2.4.98) and LLVM 8.0.1 Xorg segfaults at address 0x6d, using the radeonsi driver on AMD Radeon (TM) Pro WX 7100 Graphics (POLARIS10). The Xorg.0.log is attached. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 --- Comment #16 from Rolf --- Btw, it would be very nice to list these PPAs on the driver page. Where are they documented? It's not like I can build the driver for myself without source. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 Rolf changed: What|Removed |Added Resolution|INVALID |WONTFIX --- Comment #15 from Rolf --- I have a Linux workstation and it's a development workstation. Your driver page (https://www.amd.com/en/support/graphics/amd-radeon-2nd-generation-vega/amd-radeon-2nd-generation-vega/amd-radeon-vii) is where people go for up-to-date drivers on Windows. It clearly has a Linux sections as well, so this is very confusing. I still have no idea if I should be using the pro or normal drivers. I am very appreciative to Alex for providing a script that gives me choice on Ubuntu 19.04. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 203879] hard freeze on high single threaded load when Xorg is active (AMD Ryzen 7 2700X CPU, AMD Radeon RX 580 GPU)
https://bugzilla.kernel.org/show_bug.cgi?id=203879 --- Comment #1 from Claude Heiland-Allen (cla...@mathr.co.uk) --- My conjecture that inactive Xorg prevents freeze is false: got a system freeze with virtual terminal active, Xorg running on inactive VT. No kernel messages were printed :( Now running a test without Xorg running at all. -- 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 hmm 00/11] Various revisions from a locking/code review
Rebase to https://github.com/jgunthorpe/linux.git hmm branch, need some
changes because of interface hmm_range_register change. Then run a quick
amdgpu_test. Test is finished, result is ok. But there is below kernel
BUG message, seems hmm_free_rcu calls down_write.
[ 1171.919921] BUG: sleeping function called from invalid context at
/home/yangp/git/compute_staging/kernel/kernel/locking/rwsem.c:65
[ 1171.919933] in_atomic(): 1, irqs_disabled(): 0, pid: 53, name:
kworker/1:1
[ 1171.919938] 2 locks held by kworker/1:1/53:
[ 1171.919940] #0: 1c7c19d4 ((wq_completion)rcu_gp){+.+.}, at:
process_one_work+0x20e/0x630
[ 1171.919951] #1: 923f2cfa
((work_completion)(>work)){+.+.}, at: process_one_work+0x20e/0x630
[ 1171.919959] CPU: 1 PID: 53 Comm: kworker/1:1 Tainted: GW
5.2.0-rc1-kfd-yangp #196
[ 1171.919961] Hardware name: ASUS All Series/Z97-PRO(Wi-Fi ac)/USB 3.1,
BIOS 9001 03/07/2016
[ 1171.919965] Workqueue: rcu_gp srcu_invoke_callbacks
[ 1171.919968] Call Trace:
[ 1171.919974] dump_stack+0x67/0x9b
[ 1171.919980] ___might_sleep+0x149/0x230
[ 1171.919985] down_write+0x1c/0x70
[ 1171.919989] hmm_free_rcu+0x24/0x80
[ 1171.919993] srcu_invoke_callbacks+0xc9/0x150
[ 1171.92] process_one_work+0x28e/0x630
[ 1171.920008] worker_thread+0x39/0x3f0
[ 1171.920014] ? process_one_work+0x630/0x630
[ 1171.920017] kthread+0x11c/0x140
[ 1171.920021] ? kthread_park+0x90/0x90
[ 1171.920026] ret_from_fork+0x24/0x30
Philip
On 2019-06-12 1:54 p.m., Kuehling, Felix wrote:
> [+Philip]
>
> Hi Jason,
>
> I'm out of the office this week.
>
> Hi Philip, can you give this a go? Not sure how much you've been
> following this patch series review. Message or call me on Skype to
> discuss any questions.
>
> Thanks,
> Felix
>
> On 2019-06-11 12:48, Jason Gunthorpe wrote:
>> On Thu, Jun 06, 2019 at 03:44:27PM -0300, Jason Gunthorpe wrote:
>>> From: Jason Gunthorpe
>>>
>>> For hmm.git:
>>>
>>> This patch series arised out of discussions with Jerome when looking at the
>>> ODP changes, particularly informed by use after free races we have already
>>> found and fixed in the ODP code (thanks to syzkaller) working with mmu
>>> notifiers, and the discussion with Ralph on how to resolve the lifetime
>>> model.
>>>
>>> Overall this brings in a simplified locking scheme and easy to explain
>>> lifetime model:
>>>
>>>If a hmm_range is valid, then the hmm is valid, if a hmm is valid then
>>> the mm
>>>is allocated memory.
>>>
>>>If the mm needs to still be alive (ie to lock the mmap_sem, find a vma,
>>> etc)
>>>then the mmget must be obtained via mmget_not_zero().
>>>
>>> Locking of mm->hmm is shifted to use the mmap_sem consistently for all
>>> read/write and unlocked accesses are removed.
>>>
>>> The use unlocked reads on 'hmm->dead' are also eliminated in favour of using
>>> standard mmget() locking to prevent the mm from being released. Many of the
>>> debugging checks of !range->hmm and !hmm->mm are dropped in favour of
>>> poison -
>>> which is much clearer as to the lifetime intent.
>>>
>>> The trailing patches are just some random cleanups I noticed when reviewing
>>> this code.
>>>
>>> This v2 incorporates alot of the good off list changes & feedback Jerome
>>> had,
>>> and all the on-list comments too. However, now that we have the shared git I
>>> have kept the one line change to nouveau_svm.c rather than the compat
>>> funtions.
>>>
>>> I believe we can resolve this merge in the DRM tree now and keep the core
>>> mm/hmm.c clean. DRM maintainers, please correct me if I'm wrong.
>>>
>>> It is on top of hmm.git, and I have a git tree of this series to ease
>>> testing
>>> here:
>>>
>>> https://github.com/jgunthorpe/linux/tree/hmm
>>>
>>> There are still some open locking issues, as I think this remains
>>> unaddressed:
>>>
>>> https://lore.kernel.org/linux-mm/20190527195829.gb18...@mellanox.com/
>>>
>>> I'm looking for some more acks, reviews and tests so this can move ahead to
>>> hmm.git.
>> AMD Folks, this is looking pretty good now, can you please give at
>> least a Tested-by for the new driver code using this that I see in
>> linux-next?
>>
>> Thanks,
>> Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/panel: Add helper for reading DT rotation
Hi Derek.
On Mon, Jun 10, 2019 at 09:03:46PM -0700, Derek Basehore wrote:
> This adds a helper function for reading the rotation (panel
> orientation) from the device tree.
>
> Signed-off-by: Derek Basehore
> ---
> drivers/gpu/drm/drm_panel.c | 41 +
> include/drm/drm_panel.h | 7 +++
> 2 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index dbd5b873e8f2..3b689ce4a51a 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -172,6 +172,47 @@ struct drm_panel *of_drm_find_panel(const struct
> device_node *np)
> return ERR_PTR(-EPROBE_DEFER);
> }
> EXPORT_SYMBOL(of_drm_find_panel);
> +
> +/**
> + * of_drm_get_panel_orientation - look up the rotation of the panel using a
> + * device tree node
> + * @np: device tree node of the panel
> + * @orientation: orientation enum to be filled in
> + *
> + * Looks up the rotation of a panel in the device tree. The rotation in the
> + * device tree is counter clockwise.
> + *
> + * Return: 0 when a valid rotation value (0, 90, 180, or 270) is read or the
> + * rotation property doesn't exist. -EERROR otherwise.
> + */
> +int of_drm_get_panel_orientation(const struct device_node *np, int
> *orientation)
> +{
> + int rotation, ret;
> +
> + ret = of_property_read_u32(np, "rotation", );
I just noticed that everywhere this code talks about orientation,
but the property that it reads are rotation.
To my best understanding these are not the same.
Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/panel: Add helper for reading DT rotation
Hi Derek.
On Mon, Jun 10, 2019 at 09:03:46PM -0700, Derek Basehore wrote:
> This adds a helper function for reading the rotation (panel
> orientation) from the device tree.
>
> Signed-off-by: Derek Basehore
> ---
> drivers/gpu/drm/drm_panel.c | 41 +
> include/drm/drm_panel.h | 7 +++
> 2 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index dbd5b873e8f2..3b689ce4a51a 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -172,6 +172,47 @@ struct drm_panel *of_drm_find_panel(const struct
> device_node *np)
> return ERR_PTR(-EPROBE_DEFER);
> }
> EXPORT_SYMBOL(of_drm_find_panel);
> +
> +/**
> + * of_drm_get_panel_orientation - look up the rotation of the panel using a
> + * device tree node
> + * @np: device tree node of the panel
> + * @orientation: orientation enum to be filled in
The comment says "enum" but the type used is an int.
Why not use enum drm_panel_orientation?
> + *
> + * Looks up the rotation of a panel in the device tree. The rotation in the
> + * device tree is counter clockwise.
> + *
> + * Return: 0 when a valid rotation value (0, 90, 180, or 270) is read or the
> + * rotation property doesn't exist. -EERROR otherwise.
> + */
Initially I read -EEROOR as a specific error code.
But I gues the semantic is to say that a negative error code is returned
if something was wrong.
As we do not use the "-EERROR" syntax anywhere else in drm, please
reword like we do in other places.
Also - it is worth to mention that the rotation returned is
DRM_MODE_PANEL_ORIENTATION_UNKNOWN if the property is not specified.
I wonder if this is correct, as no property could also been
interpretated as DRM_MODE_PANEL_ORIENTATION_NORMAL.
And in most cases the roation property is optional, so one could
assume that no property equals 0 degree.
Sam
> +int of_drm_get_panel_orientation(const struct device_node *np, int
> *orientation)
> +{
> + int rotation, ret;
> +
> + ret = of_property_read_u32(np, "rotation", );
> + if (ret == -EINVAL) {
> + /* Don't return an error if there's no rotation property. */
> + *orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> + return 0;
> + }
> +
> + if (ret < 0)
> + return ret;
> +
> + if (rotation == 0)
> + *orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> + else if (rotation == 90)
> + *orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
> + else if (rotation == 180)
> + *orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> + else if (rotation == 270)
> + *orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(of_drm_get_panel_orientation);
> #endif
>
> MODULE_AUTHOR("Thierry Reding ");
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 8c738c0e6e9f..13631b2efbaa 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -197,11 +197,18 @@ int drm_panel_detach(struct drm_panel *panel);
>
> #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
> struct drm_panel *of_drm_find_panel(const struct device_node *np);
> +int of_drm_get_panel_orientation(const struct device_node *np,
> + int *orientation);
> #else
> static inline struct drm_panel *of_drm_find_panel(const struct device_node
> *np)
> {
> return ERR_PTR(-ENODEV);
> }
> +int of_drm_get_panel_orientation(const struct device_node *np,
> + int *orientation)
> +{
> + return -ENODEV;
> +}
> #endif
>
> #endif
> --
> 2.22.0.rc2.383.gf4fbbf30c2-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #14 from cosiek...@o2.pl --- Created attachment 144524 --> https://bugs.freedesktop.org/attachment.cgi?id=144524=edit bigger glxgears window >Is HyperZ just good without any changes to stock mesa? yes, mesa is from manjaro repo, and I think that it's the same build as in arch repo. >Your card seems to be also reported as RC410 like mine, but you have much-much >more FPS for some reason. It's because window size of glxgears ;) We could test that if you want. But we need to agree on window size ;) >Is this also a laptop? Yes. I also tried to resize window to try to get some artifacts. Apart from some flickering during resizing, image was good. Flickering was also present without HyperZ. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #13 from cosiek...@o2.pl --- 01:05.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] RC410M [Mobility Radeon Xpress 200M] (prog-if 00 [VGA controller]) Subsystem: Packard Bell B.V. RC410M [Mobility Radeon Xpress 200M] Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: Tree for Jun 12 (amdgpu: dcn10_hw_sequencer)
On 6/12/19 12:00 AM, Stephen Rothwell wrote: > Hi all, > > Changes since 20190611: > on x86_64: ../drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c: In function ‘dcn10_apply_ctx_for_surface’: ../drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2378:3: error: implicit declaration of function ‘udelay’ [-Werror=implicit-function-declaration] udelay(underflow_check_delay_us); ^ -- ~Randy
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #12 from Richard Thier --- Hi Cosiek! What card is that (lspci output maybe)? Is HyperZ just good without any changes to stock mesa? Your card seems to be also reported as RC410 like mine, but you have much-much more FPS for some reason. Is this also a laptop? > I just want to warn you that there may be some more bugs in r300 driver. I also know about one more that affects specifically the "Total War" games. Both Rome: Total War 1 and Medieval: Total War 2. I see very bad "tiling" but from the size of the bad tiles I kind of think it is likely maybe some kind of texture format or something similar that goes wrong. I haven't even started the analysis because better do one thing at a time... It is good that you point me towards this issue too as who knows if this is maybe not the one affecting the game maybe? Also I have found out only now how "easy" is to gdb the userland driver! All I need to do is to know proper function names and I can breakpoint them easily after answering yes to gdb when it asks me that it does not find the function yet but if it should set breakpoint when the shared object is loaded... Before this I just used printfs and stuff... Both have use cases of course. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/edid: parse CEA blocks embedded in DisplayID
DisplayID blocks allow embedding of CEA blocks. The payloads are
identical to traditional top level CEA extension blocks, but the header
is slightly different.
This change allows the CEA parser to find a CEA block inside a DisplayID
block. Additionally, it adds support for parsing the embedded CTA
header. No further changes are necessary due to payload parity.
This change enables audio support for the Valve Index HMD.
Signed-off-by: Andres Rodriguez
---
drivers/gpu/drm/drm_edid.c | 79 +
include/drm/drm_displayid.h | 1 +
2 files changed, 72 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 649cfd8b4200..3e71c6ae48d4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1339,6 +1339,8 @@ MODULE_PARM_DESC(edid_fixup,
static void drm_get_displayid(struct drm_connector *connector,
struct edid *edid);
+static u8 *drm_find_displayid_extension(const struct edid *edid);
+static int validate_displayid(u8 *displayid, int length, int idx);
static int drm_edid_block_checksum(const u8 *raw_edid)
{
@@ -2885,7 +2887,40 @@ static u8 *drm_find_edid_extension(const struct edid
*edid, int ext_id)
static u8 *drm_find_cea_extension(const struct edid *edid)
{
- return drm_find_edid_extension(edid, CEA_EXT);
+ int ret;
+ int idx = 1;
+ int length = EDID_LENGTH;
+ struct displayid_block *block;
+ u8 *cea = NULL;
+ u8 *displayid = NULL;
+
+ cea = drm_find_edid_extension(edid, CEA_EXT);
+
+ /* CEA blocks can also be found embedded in a DisplayID block */
+ if (!cea) {
+ displayid = drm_find_displayid_extension(edid);
+ if (!displayid)
+ return NULL;
+
+ ret = validate_displayid(displayid, length, idx);
+ if (ret)
+ return NULL;
+
+ idx += sizeof(struct displayid_hdr);
+ while (block = (struct displayid_block *)[idx],
+ idx + sizeof(struct displayid_block) <= length &&
+ idx + sizeof(struct displayid_block) + block->num_bytes
<= length &&
+ block->num_bytes > 0) {
+ idx += block->num_bytes + sizeof(struct
displayid_block);
+ switch (block->tag) {
+ case DATA_BLOCK_CTA:
+ cea = (u8 *)block;
+ break;
+ }
+ }
+ }
+
+ return cea;
}
static u8 *drm_find_displayid_extension(const struct edid *edid)
@@ -3616,13 +3651,38 @@ cea_revision(const u8 *cea)
static int
cea_db_offsets(const u8 *cea, int *start, int *end)
{
- /* Data block offset in CEA extension block */
- *start = 4;
- *end = cea[2];
- if (*end == 0)
- *end = 127;
- if (*end < 4 || *end > 127)
- return -ERANGE;
+
+ /* DisplayID CTA extension blocks and top-level CEA EDID
+* blocks headers differ in two byte definitions:
+* 1) Byte 2 of the header specifies length differently,
+* 2) Byte 3 is only present in the CEA top level block.
+*
+* The different definitions for byte 2 follow.
+*
+* DisplayID CTA extension block defines byte 2 as:
+* Number of payload bytes
+*
+* CEA EDID block defines byte 2 as:
+* Byte number (decimal) within this block where the 18-byte
+* DTDs begin. If no non-DTD data is present in this extension
+* block, the value should be set to 04h (the byte after next).
+* If set to 00h, there are no DTDs present in this block and
+* no non-DTD data.
+*/
+ if (cea[0] == DATA_BLOCK_CTA) {
+ *start = 3;
+ *end = *start + cea[2];
+ } else if (cea[0] == CEA_EXT) {
+ *start = 4;
+ *end = cea[2];
+ /* Data block offset in CEA extension block */
+ if (*end == 0)
+ *end = 127;
+ if (*end < 4 || *end > 127)
+ return -ERANGE;
+ } else
+ return -ENOTSUPP;
+
return 0;
}
@@ -5240,6 +5300,9 @@ static int drm_parse_display_id(struct drm_connector
*connector,
case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
/* handled in mode gathering code. */
break;
+ case DATA_BLOCK_CTA:
+ /* handled in the cea parser code. */
+ break;
default:
DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n",
block->tag);
break;
diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h
index c0d4df6a606f..c7af857f4764 100644
---
[Bug 203879] New: hard freeze on high single threaded load when Xorg is active (AMD Ryzen 7 2700X CPU, AMD Radeon RX 580 GPU)
https://bugzilla.kernel.org/show_bug.cgi?id=203879 Bug ID: 203879 Summary: hard freeze on high single threaded load when Xorg is active (AMD Ryzen 7 2700X CPU, AMD Radeon RX 580 GPU) Product: Drivers Version: 2.5 Kernel Version: 4.19.37-3 (Debian 4.19.0-5-amd64) and others (including mainline versions) Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: cla...@mathr.co.uk Regression: No Created attachment 283233 --> https://bugzilla.kernel.org/attachment.cgi?id=283233=edit dmesg from 4.19.0-5-amd64 with amdgpu.dc=1 (no freeze yet) I am developing a CPU-based program to render fractals, which I usually run with "nice -n 20". The main calculations are multi-threaded, using 16 threads on AMD Ryzen 7 2700X Eight-Core Processor. However, final image PNG saving is single-threaded. During the single-threaded workload only (as observed by htop and program status prints), it can happen that the system freezes hard (no ssh, stuck mouse pointer, no NumLock LED toggle, no magic SysRq, only physical power button for hard power-off works). This freeze only happens when Xorg is running on the active virtual terminal: I tried to see if some kernel log messages would be displayed before freeze by switching to a console with Ctrl-Alt-F1 after launching my program, but with the terminal active it doesn't seem to freeze. The freeze does not always occur, but usually happens before a dozen images are saved (sequential process is full-threaded workload, followed by single-threaded workload, repeated). This can take a few hours. With the virtual terminal active instead of Xorg, I have rendered 100+ images in a row without any issues. Of course, I can't use other X applications at the same time, so this is an annoying workaround. I mostly run the regular Debian Buster kernel but I have had this freeze occur with other self-compiled kernels of various versions (newer than the Debian kernel, without Debian patches). I also had the freeze with both amdgpu.dc=1 (default) and amdgpu.dc=0 options. $ uname -a Linux eiskaffee 4.19.0-5-amd64 #1 SMP Debian 4.19.37-3 (2019-05-15) x86_64 GNU/Linux $ apt-cache policy linux-image-4.19.0-5-amd64 linux-image-4.19.0-5-amd64: Installed: 4.19.37-3 Candidate: 4.19.37-3 Version table: *** 4.19.37-3 990 990 http://ftp.uk.debian.org/debian buster/main amd64 Packages 500 http://ftp.uk.debian.org/debian unstable/main amd64 Packages 100 /var/lib/dpkg/status -- 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 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
On Wed, Jun 12, 2019 at 7:40 PM Mauro Carvalho Chehab wrote: > > Em Tue, 11 Jun 2019 09:37:01 -0600 > Jonathan Corbet escreveu: > > > On Tue, 11 Jun 2019 06:02:15 -0300 > > Mauro Carvalho Chehab wrote: > > > > > Jon, please correct me if I' wrong, bu I guess the plan is to place them > > > somewhere under Documentation/admin-guide/. > > > > That makes sense to me. > > > > > If so, perhaps creating a Documentation/admin-guide/drm dir there and > > > place docs like EDID/HOWTO.txt, svga.txt, etc would work. > > > > Maybe "graphics" or "display" rather than "drm", which may not entirely > > applicable to all of those docs or as familiar to all admins? > > It is up to Daniel/David to decide. Personally, I agree with you that > either "graphics" or "display" would be better at the admin guide. We use Documentation/gpu already for the developer guide, I think going with "gpu" on the admin guide for consistency would be good. I do personally think that splitting out the admin guide makes sense, we could also put some recommendations about access rights for drm device nodes and stuff like that in there. > > > Btw, that's one of the reasons[1] why I opted to keep the files where they > > > are: properly organizing the converted documents call for such kind > > > of discussions. On my experience, discussing names and directory locations > > > can generate warm discussions and take a lot of time to reach consensus. > > > > Moving docs is a pain; my life would certainly be easier if I were happy > > to just let everything lie where it fell :) But it's far from the hardest > > problem we solve in kernel development, I assume we can figure it out. > > Yeah, it is doable. I'm happy to write the rename patches and even try > to split some documents at the places I'm more familiar with, but, IMHO, > we should do some discussions before some of such renames. > > For example, Daniel said that: > > > > > Yeah atm we're doing a bad job of keeping the kapi and uapi parts > > > > separate. But the plan at least is to move all the gpu related uapi > > > > stuff > > > > into Documentation/gpu/drm-uapi.rst. Not sure there's value in moving > > > > that > > > > out of the gpu folder ... > > From the conversions I've made so far, almost all driver subsystems > put everything under Documentation/ driver-specific technical info. > > It should be doable to place kAPI and uAPI on different books, but there > will be lots of cross-reference links between them, on properly-written > docs. I'm not sure it makes sense to split out the kapi and uapi sides of the docs complete. For the admin guide I think one overall book covering all subsystems is good. But someone creating a drm/kms compositor is not going to be interested much into some special options for networking protocol I think. For those I think focusing more on the specific subsystem makes more sense (and easier to share common concepts/diagrams between uapi and kapi of a given subsystem). I do think for a given subsystem the uapi side should be clearly split out (otherwise it's impossible to find for non-kernel people). And currently drm falls short really badly on this. So maybe a good argument for a uapi kernel directory would be to force that, but not sure that's good enough of a reason. -Daniel > However, other admin-guide stuff under drivers are usually in the middle > of the documents. For example, on media, we have some at the uAPI guide, > like the Device Naming item: > > > https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/open.html#device-naming > > But splitting it from uAPI guide is not an easy task. > > At the driver's specific documentation is even messier. > > Ok, splitting is doable, but require lots of dedication, and I'm not > convinced if it would make much difference in practice. > > Thanks, > Mauro -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #10 from Christian König --- (In reply to Gert Wollny from comment #9) > Indeed, currently the code only tests whether compute shaders are supported, > and DIV and TEX_LZ don't have any caps yet. I guess I'll take in on me to > add these caps. Well the key point is probably rather that we should not use compute shaders on that hw generation in the first place. How did that got enabled? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #9 from Gert Wollny --- Indeed, currently the code only tests whether compute shaders are supported, and DIV and TEX_LZ don't have any caps yet. I guess I'll take in on me to add these caps. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 06/31] docs: console.txt: convert docs to ReST and rename to *.rst
Convert this small file to ReST in preparation for adding it to
the driver-api book.
While this is not part of the driver-api book, mark it as
:orphan:, in order to avoid build warnings.
Signed-off-by: Mauro Carvalho Chehab
---
.../console/{console.txt => console.rst} | 63 ++-
Documentation/fb/fbcon.rst| 4 +-
drivers/tty/Kconfig | 2 +-
3 files changed, 38 insertions(+), 31 deletions(-)
rename Documentation/console/{console.txt => console.rst} (80%)
diff --git a/Documentation/console/console.txt
b/Documentation/console/console.rst
similarity index 80%
rename from Documentation/console/console.txt
rename to Documentation/console/console.rst
index d73c2ab4beda..b374141b027e 100644
--- a/Documentation/console/console.txt
+++ b/Documentation/console/console.rst
@@ -1,3 +1,6 @@
+:orphan:
+
+===
Console Drivers
===
@@ -17,25 +20,26 @@ of driver occupying the consoles.) They can only take over
the console that is
occupied by the system driver. In the same token, if the modular driver is
released by the console, the system driver will take over.
-Modular drivers, from the programmer's point of view, have to call:
+Modular drivers, from the programmer's point of view, have to call::
do_take_over_console() - load and bind driver to console layer
give_up_console() - unload driver; it will only work if driver
is fully unbound
-In newer kernels, the following are also available:
+In newer kernels, the following are also available::
do_register_con_driver()
do_unregister_con_driver()
If sysfs is enabled, the contents of /sys/class/vtconsole can be
examined. This shows the console backends currently registered by the
-system which are named vtcon where is an integer from 0 to 15. Thus:
+system which are named vtcon where is an integer from 0 to 15.
+Thus::
ls /sys/class/vtconsole
. .. vtcon0 vtcon1
-Each directory in /sys/class/vtconsole has 3 files:
+Each directory in /sys/class/vtconsole has 3 files::
ls /sys/class/vtconsole/vtcon0
. .. bind name uevent
@@ -46,27 +50,29 @@ What do these files signify?
read, or acts to bind or unbind the driver to the virtual consoles
when written to. The possible values are:
- 0 - means the driver is not bound and if echo'ed, commands the driver
+ 0
+ - means the driver is not bound and if echo'ed, commands the driver
to unbind
-1 - means the driver is bound and if echo'ed, commands the driver to
+1
+ - means the driver is bound and if echo'ed, commands the driver to
bind
- 2. name - read-only file. Shows the name of the driver in this format:
+ 2. name - read-only file. Shows the name of the driver in this format::
- cat /sys/class/vtconsole/vtcon0/name
- (S) VGA+
+ cat /sys/class/vtconsole/vtcon0/name
+ (S) VGA+
- '(S)' stands for a (S)ystem driver, i.e., it cannot be directly
- commanded to bind or unbind
+ '(S)' stands for a (S)ystem driver, i.e., it cannot be directly
+ commanded to bind or unbind
- 'VGA+' is the name of the driver
+ 'VGA+' is the name of the driver
- cat /sys/class/vtconsole/vtcon1/name
- (M) frame buffer device
+ cat /sys/class/vtconsole/vtcon1/name
+ (M) frame buffer device
- In this case, '(M)' stands for a (M)odular driver, one that can be
- directly commanded to bind or unbind.
+ In this case, '(M)' stands for a (M)odular driver, one that can be
+ directly commanded to bind or unbind.
3. uevent - ignore this file
@@ -75,14 +81,17 @@ driver takes over the consoles vacated by the driver.
Binding, on the other
hand, will bind the driver to the consoles that are currently occupied by a
system driver.
-NOTE1: Binding and unbinding must be selected in Kconfig. It's under:
+NOTE1:
+ Binding and unbinding must be selected in Kconfig. It's under::
-Device Drivers -> Character devices -> Support for binding and unbinding
-console drivers
+Device Drivers ->
+ Character devices ->
+ Support for binding and unbinding console drivers
-NOTE2: If any of the virtual consoles are in KD_GRAPHICS mode, then binding or
-unbinding will not succeed. An example of an application that sets the console
-to KD_GRAPHICS is X.
+NOTE2:
+ If any of the virtual consoles are in KD_GRAPHICS mode, then binding or
+ unbinding will not succeed. An example of an application that sets the
+ console to KD_GRAPHICS is X.
How useful is this feature? This is very useful for console driver
developers. By unbinding the driver from the console layer, one can unload the
@@ -92,10 +101,10 @@ framebuffer console to VGA console and vice versa, this
feature
Re: [PATCH] drm/i915/gvt: remove duplicate entry of trace
On Wed, Jun 12, 2019 at 11:22:36AM +0800, Zhenyu Wang wrote: > On 2019.05.26 13:26:33 +0530, Hariprasad Kelam wrote: > > Remove duplicate include of trace.h > > > > Issue identified by includecheck > > > > Signed-off-by: Hariprasad Kelam > > --- > > drivers/gpu/drm/i915/gvt/trace_points.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gvt/trace_points.c > > b/drivers/gpu/drm/i915/gvt/trace_points.c > > index a3deed69..569f5e3 100644 > > --- a/drivers/gpu/drm/i915/gvt/trace_points.c > > +++ b/drivers/gpu/drm/i915/gvt/trace_points.c > > @@ -32,5 +32,4 @@ > > > > #ifndef __CHECKER__ > > #define CREATE_TRACE_POINTS > > -#include "trace.h" > > #endif > > -- > > This actually caused build issue like > ERROR: "__tracepoint_gma_index" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_render_mmio" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_gvt_command" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_spt_guest_change" [drivers/gpu/drm/i915/i915.ko] > undefined! > ERROR: "__tracepoint_gma_translate" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_spt_alloc" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_spt_change" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_oos_sync" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_write_ir" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_propagate_event" [drivers/gpu/drm/i915/i915.ko] > undefined! > ERROR: "__tracepoint_inject_msi" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_spt_refcount" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_spt_free" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_oos_change" [drivers/gpu/drm/i915/i915.ko] undefined! > scripts/Makefile.modpost:91: recipe for target '__modpost' failed > > Looks we need fix like below. > > Subject: [PATCH] drm/i915/gvt: remove duplicate include of trace.h > > This removes duplicate include of trace.h. Found by Hariprasad Kelam > with includecheck. > > Reported-by: Hariprasad Kelam > Signed-off-by: Zhenyu Wang > --- > drivers/gpu/drm/i915/gvt/trace_points.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/trace_points.c > b/drivers/gpu/drm/i915/gvt/trace_points.c > index a3deed692b9c..fe552e877e09 100644 > --- a/drivers/gpu/drm/i915/gvt/trace_points.c > +++ b/drivers/gpu/drm/i915/gvt/trace_points.c > @@ -28,8 +28,6 @@ > * > */ > > -#include "trace.h" > - > #ifndef __CHECKER__ > #define CREATE_TRACE_POINTS > #include "trace.h" > -- > 2.20.1 > > -- > Open Source Technology Center, Intel ltd. > > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 Hi Zhenyu Wang, Thanks for correcting the patch. It seems we should first define CREATE_TRACE_POINTS and include trace.h as per documentation. Thanks, Hariprasad k
[Bug 110822] [Bisected]Booting with kernel version 5.1.0 or higher on RX 580 hangs
https://bugs.freedesktop.org/show_bug.cgi?id=110822 --- Comment #19 from Alex Deucher --- (In reply to Gobinda Joy from comment #18) > > What I don't get is why they are using 2 calls to get the bandwidth reading. > Since both function walking the PCIe tree what's the point. Also it seems > like the call to pcie_bandwidth_available() function is casing the > freeze/hangs in my system. So that's counts for something. > Can you try a drm-next kernel? This code was ultimately cleaned in this patch: https://cgit.freedesktop.org/drm/drm/commit/?id=dbaa922b5706b1aff4572c280e15bbea2d04afe6 I don't know why pcie_bandwidth_available() is causing problems for you, it's just standard PCIE stuff. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] backlight: pwm_bl: Fix heuristic to determine number of brightness levels
With commit 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of
LED linearly to human eye") the number of set bits (aka hweight())
in the PWM period is used in the heuristic to determine the number
of brightness levels, when the brightness table isn't specified in
the DT. The number of set bits doesn't provide a reliable clue about
the length of the period, instead change the heuristic to:
nlevels = period / fls(period)
Also limit the maximum number of brightness levels to 4096 to avoid
excessively large tables.
With this the number of levels increases monotonically with the PWM
period, until the maximum of 4096 levels is reached:
period (ns)# levels
10016
50062
1000 111
5000 416
1 769
5
10 4096
Fixes: 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED linearly to
human eye")
Signed-off-by: Matthias Kaehlcke
---
drivers/video/backlight/pwm_bl.c | 24 ++--
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index fb45f866b923..0b7152fa24f7 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -194,29 +194,17 @@ int pwm_backlight_brightness_default(struct device *dev,
struct platform_pwm_backlight_data *data,
unsigned int period)
{
- unsigned int counter = 0;
- unsigned int i, n;
+ unsigned int i;
u64 retval;
/*
-* Count the number of bits needed to represent the period number. The
-* number of bits is used to calculate the number of levels used for the
-* brightness-levels table, the purpose of this calculation is have a
-* pre-computed table with enough levels to get linear brightness
-* perception. The period is divided by the number of bits so for a
-* 8-bit PWM we have 255 / 8 = 32 brightness levels or for a 16-bit PWM
-* we have 65535 / 16 = 4096 brightness levels.
-*
-* Note that this method is based on empirical testing on different
-* devices with PWM of 8 and 16 bits of resolution.
+* Once we have 4096 levels there's little point going much higher...
+* neither interactive sliders nor animation benefits from having
+* more values in the table.
*/
- n = period;
- while (n) {
- counter += n % 2;
- n >>= 1;
- }
+ data->max_brightness =
+ min((int)DIV_ROUND_UP(period, fls(period)), 4096);
- data->max_brightness = DIV_ROUND_UP(period, counter);
data->levels = devm_kcalloc(dev, data->max_brightness,
sizeof(*data->levels), GFP_KERNEL);
if (!data->levels)
--
2.22.0.rc2.383.gf4fbbf30c2-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 00/11] Various revisions from a locking/code review
[+Philip] Hi Jason, I'm out of the office this week. Hi Philip, can you give this a go? Not sure how much you've been following this patch series review. Message or call me on Skype to discuss any questions. Thanks, Felix On 2019-06-11 12:48, Jason Gunthorpe wrote: > On Thu, Jun 06, 2019 at 03:44:27PM -0300, Jason Gunthorpe wrote: >> From: Jason Gunthorpe >> >> For hmm.git: >> >> This patch series arised out of discussions with Jerome when looking at the >> ODP changes, particularly informed by use after free races we have already >> found and fixed in the ODP code (thanks to syzkaller) working with mmu >> notifiers, and the discussion with Ralph on how to resolve the lifetime >> model. >> >> Overall this brings in a simplified locking scheme and easy to explain >> lifetime model: >> >> If a hmm_range is valid, then the hmm is valid, if a hmm is valid then the >> mm >> is allocated memory. >> >> If the mm needs to still be alive (ie to lock the mmap_sem, find a vma, >> etc) >> then the mmget must be obtained via mmget_not_zero(). >> >> Locking of mm->hmm is shifted to use the mmap_sem consistently for all >> read/write and unlocked accesses are removed. >> >> The use unlocked reads on 'hmm->dead' are also eliminated in favour of using >> standard mmget() locking to prevent the mm from being released. Many of the >> debugging checks of !range->hmm and !hmm->mm are dropped in favour of poison >> - >> which is much clearer as to the lifetime intent. >> >> The trailing patches are just some random cleanups I noticed when reviewing >> this code. >> >> This v2 incorporates alot of the good off list changes & feedback Jerome had, >> and all the on-list comments too. However, now that we have the shared git I >> have kept the one line change to nouveau_svm.c rather than the compat >> funtions. >> >> I believe we can resolve this merge in the DRM tree now and keep the core >> mm/hmm.c clean. DRM maintainers, please correct me if I'm wrong. >> >> It is on top of hmm.git, and I have a git tree of this series to ease testing >> here: >> >> https://github.com/jgunthorpe/linux/tree/hmm >> >> There are still some open locking issues, as I think this remains >> unaddressed: >> >> https://lore.kernel.org/linux-mm/20190527195829.gb18...@mellanox.com/ >> >> I'm looking for some more acks, reviews and tests so this can move ahead to >> hmm.git. > AMD Folks, this is looking pretty good now, can you please give at > least a Tested-by for the new driver code using this that I see in > linux-next? > > Thanks, > Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 14/28] docs: locking: convert docs to ReST and rename to *.rst
Convert the locking documents to ReST and add them to the
kernel development book where it belongs.
Most of the stuff here is just to make Sphinx to properly
parse the text file, as they're already in good shape,
not requiring massive changes in order to be parsed.
The conversion is actually:
- add blank lines and identation in order to identify paragraphs;
- fix tables markups;
- add some lists markups;
- mark literal blocks;
- adjust title markups.
At its new index.rst, let's add a :orphan: while this is not linked to
the main index.rst file, in order to avoid build warnings.
Signed-off-by: Mauro Carvalho Chehab
Acked-by: Federico Vaga
---
Documentation/kernel-hacking/locking.rst | 2 +-
Documentation/locking/index.rst | 24 +++
...{lockdep-design.txt => lockdep-design.rst} | 51 +++--
Documentation/locking/lockstat.rst| 204 ++
Documentation/locking/lockstat.txt| 183
.../{locktorture.txt => locktorture.rst} | 105 +
.../{mutex-design.txt => mutex-design.rst}| 26 ++-
...t-mutex-design.txt => rt-mutex-design.rst} | 139 ++--
.../locking/{rt-mutex.txt => rt-mutex.rst}| 30 +--
.../locking/{spinlocks.txt => spinlocks.rst} | 32 ++-
...w-mutex-design.txt => ww-mutex-design.rst} | 82 +++
Documentation/pi-futex.txt| 2 +-
.../it_IT/kernel-hacking/locking.rst | 2 +-
drivers/gpu/drm/drm_modeset_lock.c| 2 +-
include/linux/lockdep.h | 2 +-
include/linux/mutex.h | 2 +-
include/linux/rwsem.h | 2 +-
kernel/locking/mutex.c| 2 +-
kernel/locking/rtmutex.c | 2 +-
lib/Kconfig.debug | 4 +-
20 files changed, 511 insertions(+), 387 deletions(-)
create mode 100644 Documentation/locking/index.rst
rename Documentation/locking/{lockdep-design.txt => lockdep-design.rst} (93%)
create mode 100644 Documentation/locking/lockstat.rst
delete mode 100644 Documentation/locking/lockstat.txt
rename Documentation/locking/{locktorture.txt => locktorture.rst} (57%)
rename Documentation/locking/{mutex-design.txt => mutex-design.rst} (94%)
rename Documentation/locking/{rt-mutex-design.txt => rt-mutex-design.rst} (91%)
rename Documentation/locking/{rt-mutex.txt => rt-mutex.rst} (71%)
rename Documentation/locking/{spinlocks.txt => spinlocks.rst} (89%)
rename Documentation/locking/{ww-mutex-design.txt => ww-mutex-design.rst} (93%)
diff --git a/Documentation/kernel-hacking/locking.rst
b/Documentation/kernel-hacking/locking.rst
index 519673df0e82..71a843464ec2 100644
--- a/Documentation/kernel-hacking/locking.rst
+++ b/Documentation/kernel-hacking/locking.rst
@@ -1364,7 +1364,7 @@ Futex API reference
Further reading
===
-- ``Documentation/locking/spinlocks.txt``: Linus Torvalds' spinlocking
+- ``Documentation/locking/spinlocks.rst``: Linus Torvalds' spinlocking
tutorial in the kernel sources.
- Unix Systems for Modern Architectures: Symmetric Multiprocessing and
diff --git a/Documentation/locking/index.rst b/Documentation/locking/index.rst
new file mode 100644
index ..ef5da7fe9aac
--- /dev/null
+++ b/Documentation/locking/index.rst
@@ -0,0 +1,24 @@
+:orphan:
+
+===
+locking
+===
+
+.. toctree::
+:maxdepth: 1
+
+lockdep-design
+lockstat
+locktorture
+mutex-design
+rt-mutex-design
+rt-mutex
+spinlocks
+ww-mutex-design
+
+.. only:: subproject and html
+
+ Indices
+ ===
+
+ * :ref:`genindex`
diff --git a/Documentation/locking/lockdep-design.txt
b/Documentation/locking/lockdep-design.rst
similarity index 93%
rename from Documentation/locking/lockdep-design.txt
rename to Documentation/locking/lockdep-design.rst
index f189d130e543..23fcbc4d3fc0 100644
--- a/Documentation/locking/lockdep-design.txt
+++ b/Documentation/locking/lockdep-design.rst
@@ -2,6 +2,7 @@ Runtime locking correctness validator
=
started by Ingo Molnar
+
additions by Arjan van de Ven
Lock-class
@@ -56,7 +57,7 @@ where the last 1 category is:
When locking rules are violated, these usage bits are presented in the
locking error messages, inside curlies, with a total of 2 * n STATEs bits.
-A contrived example:
+A contrived example::
modprobe/2287 is trying to acquire lock:
(_locks[i].lock){-.-.}, at: [] mutex_lock+0x21/0x24
@@ -70,12 +71,14 @@ of the lock and readlock (if exists), for each of the n
STATEs listed
above respectively, and the character displayed at each bit position
indicates:
+ === ===
'.' acquired while irqs disabled and not in irq context
'-' acquired in irq context
'+' acquired with irqs enabled
'?' acquired in irq context with irqs enabled.
+ ===
[PATCH v4 28/28] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
Sphinx need to know when a paragraph ends. So, do some adjustments
at the file for it to be properly parsed.
At its new index.rst, let's add a :orphan: while this is not linked to
the main index.rst file, in order to avoid build warnings.
that's said, I believe that this file should be moved to the
GPU/DRM documentation.
Signed-off-by: Mauro Carvalho Chehab
---
Documentation/EDID/{HOWTO.txt => howto.rst} | 31 ---
.../admin-guide/kernel-parameters.txt | 2 +-
drivers/gpu/drm/Kconfig | 2 +-
3 files changed, 22 insertions(+), 13 deletions(-)
rename Documentation/EDID/{HOWTO.txt => howto.rst} (83%)
diff --git a/Documentation/EDID/HOWTO.txt b/Documentation/EDID/howto.rst
similarity index 83%
rename from Documentation/EDID/HOWTO.txt
rename to Documentation/EDID/howto.rst
index 539871c3b785..725fd49a88ca 100644
--- a/Documentation/EDID/HOWTO.txt
+++ b/Documentation/EDID/howto.rst
@@ -1,3 +1,9 @@
+:orphan:
+
+
+EDID
+
+
In the good old days when graphics parameters were configured explicitly
in a file called xorg.conf, even broken hardware could be managed.
@@ -34,16 +40,19 @@ Makefile. Please note that the EDID data structure expects
the timing
values in a different way as compared to the standard X11 format.
X11:
-HTimings: hdisp hsyncstart hsyncend htotal
-VTimings: vdisp vsyncstart vsyncend vtotal
+ HTimings:
+hdisp hsyncstart hsyncend htotal
+ VTimings:
+vdisp vsyncstart vsyncend vtotal
-EDID:
-#define XPIX hdisp
-#define XBLANK htotal-hdisp
-#define XOFFSET hsyncstart-hdisp
-#define XPULSE hsyncend-hsyncstart
+EDID::
-#define YPIX vdisp
-#define YBLANK vtotal-vdisp
-#define YOFFSET vsyncstart-vdisp
-#define YPULSE vsyncend-vsyncstart
+ #define XPIX hdisp
+ #define XBLANK htotal-hdisp
+ #define XOFFSET hsyncstart-hdisp
+ #define XPULSE hsyncend-hsyncstart
+
+ #define YPIX vdisp
+ #define YBLANK vtotal-vdisp
+ #define YOFFSET vsyncstart-vdisp
+ #define YPULSE vsyncend-vsyncstart
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 3d072ca532bb..3faf37b8b001 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -930,7 +930,7 @@
edid/1680x1050.bin, or edid/1920x1080.bin is given
and no file with the same name exists. Details and
instructions how to build your own EDID data are
- available in Documentation/EDID/HOWTO.txt. An EDID
+ available in Documentation/EDID/howto.rst. An EDID
data set will only be used for a particular connector,
if its name and a colon are prepended to the EDID
name. Each connector may use a unique EDID data
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 6b34949416b1..c3a6dd284c91 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -141,7 +141,7 @@ config DRM_LOAD_EDID_FIRMWARE
monitor are unable to provide appropriate EDID data. Since this
feature is provided as a workaround for broken hardware, the
default case is N. Details and instructions how to build your own
- EDID data are given in Documentation/EDID/HOWTO.txt.
+ EDID data are given in Documentation/EDID/howto.rst.
config DRM_DP_CEC
bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #11 from cosiek...@o2.pl --- Created attachment 144523 --> https://bugs.freedesktop.org/attachment.cgi?id=144523=edit good HyperZ glxgears Extended renderer info (GLX_MESA_query_renderer): Vendor: X.Org R300 Project (0x1002) Device: ATI RC410 (0x5a62) Version: 19.0.6 Accelerated: yes Video memory: 128MB Unified memory: no Preferred profile: compat (0x2) Max core profile version: 0.0 Max compat profile version: 2.1 Max GLES1 profile version: 1.1 Max GLES[23] profile version: 2.0 OpenGL vendor string: X.Org R300 Project OpenGL renderer string: ATI RC410 OpenGL version string: 2.1 Mesa 19.0.6 OpenGL shading language version string: 1.20 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99970] DRI3 steam login window is empty
https://bugs.freedesktop.org/show_bug.cgi?id=99970 --- Comment #7 from cosiek...@o2.pl --- >You can try glamor, but I'm not sure it can work with your GPU. Yea I think it can't /var/log/Xorg.0.log >glamor requires at least 128 instructions (64 reported) Is is worth to try to fix DRI3 and EXA for this driver or is it better to stick to DRI2? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
Em Tue, 11 Jun 2019 09:37:01 -0600 Jonathan Corbet escreveu: > On Tue, 11 Jun 2019 06:02:15 -0300 > Mauro Carvalho Chehab wrote: > > > Jon, please correct me if I' wrong, bu I guess the plan is to place them > > somewhere under Documentation/admin-guide/. > > That makes sense to me. > > > If so, perhaps creating a Documentation/admin-guide/drm dir there and > > place docs like EDID/HOWTO.txt, svga.txt, etc would work. > > Maybe "graphics" or "display" rather than "drm", which may not entirely > applicable to all of those docs or as familiar to all admins? It is up to Daniel/David to decide. Personally, I agree with you that either "graphics" or "display" would be better at the admin guide. > > > Btw, that's one of the reasons[1] why I opted to keep the files where they > > are: properly organizing the converted documents call for such kind > > of discussions. On my experience, discussing names and directory locations > > can generate warm discussions and take a lot of time to reach consensus. > > Moving docs is a pain; my life would certainly be easier if I were happy > to just let everything lie where it fell :) But it's far from the hardest > problem we solve in kernel development, I assume we can figure it out. Yeah, it is doable. I'm happy to write the rename patches and even try to split some documents at the places I'm more familiar with, but, IMHO, we should do some discussions before some of such renames. For example, Daniel said that: > > > Yeah atm we're doing a bad job of keeping the kapi and uapi parts > > > separate. But the plan at least is to move all the gpu related uapi stuff > > > into Documentation/gpu/drm-uapi.rst. Not sure there's value in moving that > > > out of the gpu folder ... From the conversions I've made so far, almost all driver subsystems put everything under Documentation/https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/open.html#device-naming But splitting it from uAPI guide is not an easy task. At the driver's specific documentation is even messier. Ok, splitting is doable, but require lots of dedication, and I'm not convinced if it would make much difference in practice. Thanks, Mauro
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #10 from cosiek...@o2.pl --- >Was it looking similar? Was it solved for your case? I didn't report that bug. Someone just wrote in that thread that HyperZ was not enabled due to lack of testing, so I ran some piglid tests. :) I just want to warn you that there may be some more bugs in r300 driver. For example: https://bugs.freedesktop.org/show_bug.cgi?id=98869 There was workaround put in place to resolve this bug (So no proper fix for underlying issue. See this mailing list conversation: https://lists.freedesktop.org/archives/mesa-dev/2017-February/143980.html). But later I found out that after some upgrades game started crashing. But never really figured out why. And didn't test it after that as I already finished that game :) https://bugs.freedesktop.org/show_bug.cgi?id=101382 I'm glad for your r300 work! :) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #8 from Ilia Mirkin --- (In reply to Gert Wollny from comment #7) > This is a very deep rabbit hole: Not only does r600 not support DIV, it also > doesn't support TEX_LZ that is used by these compute shaders and Evergreen > class hardware doesn't support more then one target swizzle for the > destinations with RCP so that the shader is even more broken. I think the > best option now will be to disable this shader for now for this hardware. The state tracker has to respect PIPE_CAP's. If the driver doesn't say it has DIV or TEX_LZ, then those shouldn't be used. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #7 from Gert Wollny --- This is a very deep rabbit hole: Not only does r600 not support DIV, it also doesn't support TEX_LZ that is used by these compute shaders and Evergreen class hardware doesn't support more then one target swizzle for the destinations with RCP so that the shader is even more broken. I think the best option now will be to disable this shader for now for this hardware. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110883] [Regression linux 5.2-rc4][bisected] hang on boot
https://bugs.freedesktop.org/show_bug.cgi?id=110883 --- Comment #7 from Sibren Vasse --- > Does https://patchwork.freedesktop.org/patch/309712/ work? Yes, it does. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio
On Wed, Jun 12, 2019 at 12:37:57PM -0400, Sven Van Asbroeck wrote: > On Wed, Jun 12, 2019 at 12:28 PM Russell King - ARM Linux admin > wrote: > > > > The platform data path has never supported the HDMI codec way of doing > > things, so that really isn't a concern here. The platform data way > > was sufficient to allow SPDIF streams to work with a static setup of > > the TDA998x, and has never been intended to support anything beyond > > that. > > Thank you, I am not using the platform data path, so I had no idea. > > Wouldn't the current code always fail on the platform data path though? > > create() calls tda998x_set_config() in the platform data case. > If the audio_params.format is used (i.e. the platform data configures > audio), the function then returns the return value of tda998x_derive_cts_n(), > which is a positive divider (can be 0 if /1). I think you're confusing tda998x_derive_cts_n() and tda998x_get_adiv(). tda998x_derive_cts_n() only returns 0 or -EINVAL. > > Then in create(): if (ret) goto fail; > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/13] drm/i2c: tda998x: move audio routing configuration
On Wed, Jun 12, 2019 at 11:36:59AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:02 AM Russell King
> wrote:
> >
> > Move the mux and clocking selection out of tda998x_configure_audio()
> > into the parent functions, so we can validate this when parameters
> > are set outside of the audio mutex.
> >
> > Signed-off-by: Russell King
> > ---
>
> +/* Configure the TDA998x audio data and clock routing. */
> +static int tda998x_derive_routing(struct tda998x_priv *priv,
> + struct tda998x_audio_settings *s,
> + unsigned int route)
> +{
> + s->route = _audio_route[route];
> + s->ena_ap = priv->audio_port_enable[route];
> + if (s->ena_ap == 0) {
> + dev_err(>hdmi->dev, "no audio configuration found\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
>
> Nit: priv is nearly unused in this function.
> Maybe delegate the error log to the caller, in that case we could just pass
> route and const audio_port_enable to the function. Instead of passing in the
> 'kitchen sink' priv ?
I don't think that's worth doing. This way, compilers are free to
emit code to bounds-check the audio_port_enable access since they
know that it is a defined size. Passing in a const pointer ca
mean that check has to be avoided.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio
On Wed, Jun 12, 2019 at 11:27:16AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:02 AM Russell King
> wrote:
> >
> > The TDA998x derives the CTS value using the supplied I2S bit clock
> > (ACLK, in TDA998x parlence) rather than 128·fs. TDA998x uses two
> > constants named m and k in the CTS generator such that we have this
> > relationship between the I2S source ACLK and the sink fs:
> >
> > 128·fs_sink = ACLK·m / k
> >
> > Where ACLK = aclk_ratio·fs_source.
> >
> > When audio support was originally added, we supported a fixed ratio
> > of 64·fs, intending to support the Kirkwood I2S on Dove. However,
> > when hdmi-codec support was added, this was changed to scale the
> > ratio with the sample width, which would've broken its use with
> > Kirkwood I2S.
> >
> > We are now starting to see other users whose I2S blocks send at 64·fs
> > for 16-bit samples, so we need to reinstate the support for the fixed
> > ratio I2S bit clock.
> >
> > This commit takes a step towards supporting these configurations by
> > selecting the CTS_N register m and k values based on the bit clock
> > ratio. However, as the driver is not given the bit clock ratio from
> > ALSA, continue deriving this from the sample width. This will be
> > addressed in a later commit.
> >
> > Signed-off-by: Russell King
> > ---
>
> @@ -1657,9 +1701,17 @@ static void tda998x_set_config(struct tda998x_priv
> *priv,
> (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
>
> if (p->audio_params.format != AFMT_UNUSED) {
> + unsigned int ratio;
> + bool spdif = p->audio_params.format == AFMT_SPDIF;
> +
> priv->audio.params = p->audio_params;
> priv->audio.i2s_format = I2S_FORMAT_PHILIPS;
> +
> + ratio = spdif ? 64 : p->audio_params.sample_width * 2;
> + return tda998x_derive_cts_n(priv, >audio, ratio);
>
> Won't this make the platform_data path fail all the time?
> Also, the platform_data path doesn't appear to instantiate the HDMI_CODEC,
> so tda audio support would be completely missing in this case?
The platform data path has never supported the HDMI codec way of doing
things, so that really isn't a concern here. The platform data way
was sufficient to allow SPDIF streams to work with a static setup of
the TDA998x, and has never been intended to support anything beyond
that.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108514] heavy screen flickering with Mobility Radeon X1600 and kernel version 3.15rc2 onward
https://bugs.freedesktop.org/show_bug.cgi?id=108514 Paul Dufresne changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #36 from Paul Dufresne --- The patch is included in: Linux 4.14.125 Linux 4.19.50 Linux 5.1.9 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [GIT,PULL] mediatek drm fixes for 5.2
On Wed, Jun 12, 2019 at 03:51:08PM +0800, CK Hu wrote: > Hi Dave, Daniel: > > This include unbind error fix, clock control flow refinement, and PRIME > mmap with page offset. > > Regards, > CK > > The following changes since commit > a188339ca5a396acc588e5851ed7e19f66b0ebd9: > > Linux 5.2-rc1 (2019-05-19 15:47:09 -0700) > > are available in the Git repository at: > > https://github.com/ckhu-mediatek/linux.git-tags.git > mediatek-drm-fixes-5.2 For next time around, please make this a signed annotated tag. annotated because then it's less typing for me, and signed because it's on a server we don't necessarily trust. > > for you to fetch changes up to 2458d9d6d94be982b917e93c61a89b4426f32e31: > > drm/mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable() > (2019-06-04 09:54:42 +0800) > > > Hsin-Yi Wang (5): > drm/mediatek: fix unbind functions > drm/mediatek: unbind components in mtk_drm_unbind() > drm/mediatek: call drm_atomic_helper_shutdown() when unbinding > driver > drm/mediatek: clear num_pipes when unbind driver > drm/mediatek: call mtk_dsi_stop() after > mtk_drm_crtc_atomic_disable() > > Yongqiang Niu (2): > drm/mediatek: adjust ddp clock control flow > drm/mediatek: respect page offset for PRIME mmap calls You might want to look into all the recently added helpers for mmap, I don't think mtk does anything special here. > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 30 > ++ > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 8 +++- > drivers/gpu/drm/mediatek/mtk_drm_gem.c | 7 ++- > drivers/gpu/drm/mediatek/mtk_dsi.c | 12 +++- > 4 files changed, 26 insertions(+), 31 deletions(-) Pulled, thanks. -Daniel > > ___ > 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 03/13] drm/i2c: tda998x: improve programming of audio divisor
On Wed, Jun 12, 2019 at 11:25:59AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:02 AM Russell King
> wrote:
> >
> > Improve the selection of the audio clock divisor so that more modes
> > and sample rates work.
> >
> > Signed-off-by: Russell King
> > ---
>
> +static u8 tda998x_get_adiv(struct tda998x_priv *priv, unsigned int fs)
> +{
> + unsigned long min_audio_clk = fs * 128;
> + unsigned long ser_clk = priv->tmds_clock * 1000;
> + u8 adiv;
> +
> + for (adiv = AUDIO_DIV_SERCLK_32; adiv != AUDIO_DIV_SERCLK_1; adiv--)
> + if (ser_clk > min_audio_clk << adiv)
> + break;
> +
> + dev_dbg(>hdmi->dev,
> + "ser_clk=%luHz fs=%uHz min_aclk=%luHz adiv=%d\n",
> + ser_clk, fs, min_audio_clk, adiv);
> +
> + return adiv;
>
> Doesn't this function need an error return in case min_audio_clk > ser_clk ?
> Or is that a situation that can never arise?
It's possible it could arise. For example, if we have a 192kHz sample
rate, and a tmds clock slower than 24.576MHz.
In such a case, we will select AUDIO_DIV_SERCLK_1 as the divisor, which
is a legal value. The result _may_ be audio not working (which is what
already happens today when we get this setting wrong.)
If we were to return an error, there's no way to handle that error, so
what's the point of returning an error?
The results of this function match what the Philips firmware uses for
this register for all standard TMDS clocks and sample rates, so it's
not a problem that I'm particularly concerned about at this point.
The only way around this would be to increase the TMDS clock, which
means using pixel doubling tricks, and therefore a mode set. I don't
think any HDMI driver has the capability to deal with that yet.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110906] [Regression 5.2-rc4] Frozen screen with `Memory manager not clean during takedown.`
https://bugs.freedesktop.org/show_bug.cgi?id=110906 --- Comment #1 from Michel Dänzer --- Looks like bug 110883. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110883] [Regression linux 5.2-rc4][bisected] hang on boot
https://bugs.freedesktop.org/show_bug.cgi?id=110883 --- Comment #6 from Michel Dänzer --- Does https://patchwork.freedesktop.org/patch/309712/ work? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 10/15] drm/bridge: tc358767: Add support for address-only I2C transfers
On Wed, Jun 12, 2019 at 5:48 AM Tomi Valkeinen wrote: > > Hi, > > On 12/06/2019 11:32, Andrey Smirnov wrote: > > Transfer size of zero means a request to do an address-only > > transfer. Since the HW support this, we probably shouldn't be just > > ignoring such requests. While at it allow DP_AUX_I2C_MOT flag to pass > > through, since it is supported by the HW as well. > > I bisected the EDID read issue to this patch... > I don't think I've had any problems on my end with this. I'll double check. It might be the case that yours is the only setup where the problem can be repro'd, though. We can drop this patch if you don't have time/would rather not dig into this. Thanks, Andrey Smirnov
Re: [PATCH v17 14/15] vfio/type1, arm64: untag user pointers in vaddr_get_pfn
Hi Andrey,
On 6/12/19 1:43 PM, Andrey Konovalov wrote:
> This patch is a part of a series that extends arm64 kernel ABI to allow to
> pass tagged user pointers (with the top byte set to something else other
> than 0x00) as syscall arguments.
>
> vaddr_get_pfn() uses provided user pointers for vma lookups, which can
> only by done with untagged pointers.
>
> Untag user pointers in this function.
>
> Reviewed-by: Catalin Marinas
> Reviewed-by: Kees Cook
> Signed-off-by: Andrey Konovalov
Reviewed-by: Eric Auger
Thanks
Eric
> ---
> drivers/vfio/vfio_iommu_type1.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 3ddc375e7063..528e39a1c2dd 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -384,6 +384,8 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> long vaddr,
>
> down_read(>mmap_sem);
>
> + vaddr = untagged_addr(vaddr);
> +
> vma = find_vma_intersection(mm, vaddr, vaddr + 1);
>
> if (vma && vma->vm_flags & VM_PFNMAP) {
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
etnaviv: Possible circular lockingon i.MX6QP
Hi,
On a imx6qp-wandboard I get the warning below about a possible
circular locking dependency running 5.1.9 built from
imx_v6_v7_defconfig.
Such warning does not happen on the imx6q or imx6solo variants of
wandboard though.
Any ideas?
Thanks,
Fabio Estevam
** (matchbox-panel:708): WARNING **: Failed to load applet "battery"
(/usr/lib/matchbox-panel/libbattery.so: cannot open shared object
file: No such file or directory).
matchbox-wm: X error warning (0xe3): BadWindow (invalid Window
parameter) (opcode: 12)
etnaviv-gpu 134000.gpu: MMU fault status 0x0001
etnaviv-gpu 134000.gpu: MMU 0 fault addr 0x0805ffc0
==
WARNING: possible circular locking dependency detected
5.1.9 #58 Not tainted
--
kworker/0:1/29 is trying to acquire lock:
(ptrval) (&(>fence_spinlock)->rlock){-...}, at:
dma_fence_remove_callback+0x14/0x50
but task is already holding lock:
(ptrval) (&(>job_list_lock)->rlock){-...}, at: drm_sched_stop+0x1c/0x124
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&(>job_list_lock)->rlock){-...}:
drm_sched_process_job+0x5c/0x1c8
dma_fence_signal+0xdc/0x1d4
irq_handler+0xd0/0x1e0
__handle_irq_event_percpu+0x48/0x360
handle_irq_event_percpu+0x28/0x7c
handle_irq_event+0x38/0x5c
handle_fasteoi_irq+0xc0/0x17c
generic_handle_irq+0x20/0x34
__handle_domain_irq+0x64/0xe0
gic_handle_irq+0x4c/0xa8
__irq_svc+0x70/0x98
cpuidle_enter_state+0x168/0x5a4
cpuidle_enter_state+0x168/0x5a4
do_idle+0x220/0x2c0
cpu_startup_entry+0x18/0x20
start_kernel+0x3e4/0x498
-> #0 (&(>fence_spinlock)->rlock){-...}:
_raw_spin_lock_irqsave+0x38/0x4c
dma_fence_remove_callback+0x14/0x50
drm_sched_stop+0x98/0x124
etnaviv_sched_timedout_job+0x7c/0xb4
drm_sched_job_timedout+0x34/0x5c
process_one_work+0x2ac/0x704
worker_thread+0x2c/0x574
kthread+0x134/0x148
ret_from_fork+0x14/0x20
(null)
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0CPU1
lock(&(>job_list_lock)->rlock);
lock(&(>fence_spinlock)->rlock);
lock(&(>job_list_lock)->rlock);
lock(&(>fence_spinlock)->rlock);
*** DEADLOCK ***
3 locks held by kworker/0:1/29:
#0: (ptrval) ((wq_completion)events){+.+.}, at: process_one_work+0x1f4/0x704
#1: (ptrval) ((work_completion)(&(>work_tdr)->work)){+.+.},
at: process_one_work+0x1f4/0x704
#2: (ptrval) (&(>job_list_lock)->rlock){-...}, at:
drm_sched_stop+0x1c/0x124
stack backtrace:
CPU: 0 PID: 29 Comm: kworker/0:1 Not tainted 5.1.9 #58
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Workqueue: events drm_sched_job_timedout
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0xd8/0x110)
[] (dump_stack) from []
(print_circular_bug.constprop.19+0x1bc/0x2f0)
[] (print_circular_bug.constprop.19) from []
(__lock_acquire+0x1778/0x1f38)
[] (__lock_acquire) from [] (lock_acquire+0xcc/0x1e8)
[] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x38/0x4c)
[] (_raw_spin_lock_irqsave) from []
(dma_fence_remove_callback+0x14/0x50)
[] (dma_fence_remove_callback) from []
(drm_sched_stop+0x98/0x124)
[] (drm_sched_stop) from []
(etnaviv_sched_timedout_job+0x7c/0xb4)
[] (etnaviv_sched_timedout_job) from []
(drm_sched_job_timedout+0x34/0x5c)
[] (drm_sched_job_timedout) from []
(process_one_work+0x2ac/0x704)
[] (process_one_work) from [] (worker_thread+0x2c/0x574)
[] (worker_thread) from [] (kthread+0x134/0x148)
[] (kthread) from [] (ret_from_fork+0x14/0x20)
Exception stack(0xe81f7fb0 to 0xe81f7ff8)
7fa0:
7fc0:
7fe0: 0013
etnaviv-gpu 134000.gpu: recover hung GPU!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Tweak drm_encoder_helper_funcs.enable kerneldoc
On Wed, Jun 12, 2019 at 05:09:27PM +0200, Sam Ravnborg wrote:
> On Wed, Jun 12, 2019 at 11:00:34AM -0400, Sean Paul wrote:
> > From: Sean Paul
> >
> > I copied the kerneldoc for encoder_funcs.atomic_enable from
> > encoder_funcs.enable
> > in a recent patch [1]. Sam rightly pointed out in the review that "for
> > symmetry
> > with" text is awkward [2]. So here's a patch to fix up the source of the
> > awkward
> > language.
>
> Looks good.
> >
> > [1]
> > https://patchwork.freedesktop.org/patch/msgid/20190611160844.257498-2-s...@poorly.run
> > [2]
> > https://patchwork.freedesktop.org/patch/msgid/20190611185352.ga16...@ravnborg.org
> >
> > Suggested-by: Sam Ravnborg
> > Signed-off-by: Sean Paul
> Reviewed-by: Sam Ravnborg
Applied to drm-misc-next with your R-b, thanks!
Sean
>
> > ---
> > include/drm/drm_modeset_helper_vtables.h | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/drm/drm_modeset_helper_vtables.h
> > b/include/drm/drm_modeset_helper_vtables.h
> > index f9c94c2a13646..df80131bb10fc 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -719,11 +719,11 @@ struct drm_encoder_helper_funcs {
> > * hooks and call them from CRTC's callback by looping over all encoders
> > * connected to it using for_each_encoder_on_crtc().
> > *
> > -* This hook is used only by atomic helpers, for symmetry with @disable.
> > -* Atomic drivers don't need to implement it if there's no need to
> > -* enable anything at the encoder level. To ensure that runtime PM
> > handling
> > -* (using either DPMS or the new "ACTIVE" property) works
> > -* @enable must be the inverse of @disable for atomic drivers.
> > +* This hook is only used by atomic helpers, it is the opposite of
> > +* @disable. Atomic drivers don't need to implement it if there's no
> > +* need to enable anything at the encoder level. To ensure that
> > +* runtime PM handling (using either DPMS or the new "ACTIVE" property)
> > +* works @enable must be the inverse of @disable for atomic drivers.
> > */
> > void (*enable)(struct drm_encoder *encoder);
> >
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110381] Failed to updateMST allocation table forpipe idx:0
https://bugs.freedesktop.org/show_bug.cgi?id=110381 --- Comment #8 from Paul Menzel --- Why can’t it be said for certain, if the issue is fixed? The commit message should reference this report, but `git log --grep 110381` does not show anything. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110906] [Regression 5.2-rc4] Frozen screen with `Memory manager not clean during takedown.`
https://bugs.freedesktop.org/show_bug.cgi?id=110906 Bug ID: 110906 Summary: [Regression 5.2-rc4] Frozen screen with `Memory manager not clean during takedown.` Product: DRI Version: XOrg git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: pmenzel+bugs.freedesktop@molgen.mpg.de Created attachment 144521 --> https://bugs.freedesktop.org/attachment.cgi?id=144521=edit Linux kernel 5.2-rc4 messages There is a regression between Linux 5.2-rc3 and 5.2-rc4. The screen just freezes with start-up messages (mainly systemd notifications), and the login screen is not shown. Logging in remotely using SSH is possible. The X server cannot start as no screens are found. Linux logs the messages below. [ 11.452246] [ cut here ] [ 11.452697] Memory manager not clean during takedown. [ 11.453131] WARNING: CPU: 3 PID: 180 at drivers/gpu/drm/drm_mm.c:939 drm_mm_takedown+0x20/0x30 [drm] [ 11.453917] Modules linked in: amdgpu(+) input_leds led_class gpu_sched ttm drm_kms_helper x86_pkg_temp_thermal snd_hda_codec_realtek snd_hda_codec_hdmi snd_hda_codec_generic kvm_intel snd_hda_intel snd_hda_codec kvm snd_hda_core drm snd_pcm snd_timer wmi_bmof snd fb_sys_fops wmi syscopyarea irqbypass soundcore sysfillrect sysimgblt video crc32c_intel nfsd auth_rpcgss oid_registry nfs_acl lockd grace sunrpc ip_tables x_tables unix ipv6 autofs4 [ 11.457343] CPU: 3 PID: 180 Comm: systemd-udevd Not tainted 5.2.0-rc4.mx64.270 #1 [ 11.458004] Hardware name: Dell Inc. OptiPlex 5040/0R790T, BIOS 1.2.7 01/15/2016 [ 11.458653] RIP: 0010:drm_mm_takedown+0x20/0x30 [drm] [ 11.459081] Code: ff ff e8 a3 1d e3 e0 0f 1f 00 0f 1f 44 00 00 48 8b 47 38 48 83 c7 38 48 39 c7 75 02 f3 c3 48 c7 c7 00 1f 27 a0 e8 00 1b e3 e0 <0f> 0b c3 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 [ 11.460680] RSP: 0018:c9000127fa48 EFLAGS: 00010282 [ 11.461110] RAX: RBX: 8882515c4030 RCX: 0006 [ 11.461735] RDX: 0007 RSI: 0096 RDI: 8882543964c0 [ 11.462319] RBP: 88824db4fa00 R08: 02e6 R09: 0038 [ 11.462943] R10: R11: c9000127f8e8 R12: 8882515c41c0 [ 11.463527] R13: R14: 0170 R15: 8882405a7500 [ 11.464151] FS: 7fa1d302e840() GS:88825438() knlGS: [ 11.464844] CS: 0010 DS: ES: CR0: 80050033 [ 11.465316] CR2: 7fa1d04b9860 CR3: 00024a60c001 CR4: 003606e0 [ 11.465941] DR0: DR1: DR2: [ 11.466526] DR3: DR6: fffe0ff0 DR7: 0400 [ 11.467151] Call Trace: [ 11.467410] amdgpu_vram_mgr_fini+0x27/0xa0 [amdgpu] [ 11.467864] ttm_bo_clean_mm+0xa5/0xe0 [ttm] [ 11.468248] amdgpu_ttm_fini+0x72/0xd0 [amdgpu] [ 11.468690] amdgpu_bo_fini+0xe/0x30 [amdgpu] [ 11.469083] gmc_v6_0_sw_fini+0x26/0x50 [amdgpu] [ 11.469490] amdgpu_device_fini+0x368/0x4b0 [amdgpu] [ 11.469966] amdgpu_driver_unload_kms+0xb4/0x150 [amdgpu] [ 11.470435] amdgpu_driver_load_kms+0x169/0x310 [amdgpu] [ 11.470917] drm_dev_register+0x118/0x1b0 [drm] [ 11.471316] amdgpu_pci_probe+0xcb/0x160 [amdgpu] [ 11.471748] local_pci_probe+0x42/0x90 [ 11.472062] pci_device_probe+0x143/0x1b0 [ 11.472396] really_probe+0x1b6/0x2b0 [ 11.472744] driver_probe_device+0x50/0xf0 [ 11.473085] device_driver_attach+0x4f/0x60 [ 11.473433] __driver_attach+0x6c/0xb0 [ 11.473786] ? device_driver_attach+0x60/0x60 [ 11.474148] bus_for_each_dev+0x76/0xc0 [ 11.474468] bus_add_driver+0x176/0x1f0 [ 11.474829] ? 0xa07ae000 [ 11.475107] driver_register+0x5b/0xe0 [ 11.475420] ? 0xa07ae000 [ 11.475740] do_one_initcall+0x4f/0x200 [ 11.476061] ? _cond_resched+0x15/0x40 [ 11.476374] ? kmem_cache_alloc_trace+0xdf/0x1f0 [ 11.476798] do_init_module+0x5b/0x214 [ 11.477111] load_module+0x1d98/0x2170 [ 11.477425] __se_sys_finit_module+0xb1/0xd0 [ 11.477819] do_syscall_64+0x48/0x130 [ 11.478126] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 11.478544] RIP: 0033:0x7fa1d1e45d19 [ 11.478884] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 3f 21 2c 00 f7 d8 64 89 01 48 [ 11.480441] RSP: 002b:7fffb682ee38 EFLAGS: 0246 ORIG_RAX: 0139 [ 11.481091] RAX: ffda RBX: 00681350 RCX: 7fa1d1e45d19 [ 11.481715] RDX: RSI: 7fa1d279154b RDI: 0015 [ 11.482299] RBP: 7fa1d279154b R08: R09: [ 11.482914] R10: 0015 R11:
Re: [PATCH] drm/ttm: move cpu_writers handling into vmwgfx
Hi, Christian,
This looks OK, although there are a couple of minor alterations needed
in the vmwgfx driver:
- We should operate on vmw_buffer_objects rather than on
user_buffer_objects.
- vmw_user_bo_verify_synccpu should move to the validate code.
I can take care of that if it's ok with you.
Thanks,
Thomas
On Fri, 2019-06-07 at 16:47 +0200, Christian König wrote:
> This feature is only used by vmwgfx and superflous for everybody
> else.
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 27 --
> drivers/gpu/drm/ttm/ttm_bo_util.c| 1 -
> drivers/gpu/drm/ttm/ttm_execbuf_util.c | 7 +
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 35
>
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 ++
> drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 8 ++
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +++
> include/drm/ttm/ttm_bo_api.h | 31 -
> 8 files changed, 45 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> b/drivers/gpu/drm/ttm/ttm_bo.c
> index c7de667d482a..4ec055ffd6a7 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -153,7 +153,6 @@ static void ttm_bo_release_list(struct kref
> *list_kref)
>
> BUG_ON(kref_read(>list_kref));
> BUG_ON(kref_read(>kref));
> - BUG_ON(atomic_read(>cpu_writers));
> BUG_ON(bo->mem.mm_node != NULL);
> BUG_ON(!list_empty(>lru));
> BUG_ON(!list_empty(>ddestroy));
> @@ -1308,7 +1307,6 @@ int ttm_bo_init_reserved(struct ttm_bo_device
> *bdev,
>
> kref_init(>kref);
> kref_init(>list_kref);
> - atomic_set(>cpu_writers, 0);
> INIT_LIST_HEAD(>lru);
> INIT_LIST_HEAD(>ddestroy);
> INIT_LIST_HEAD(>swap);
> @@ -1814,31 +1812,6 @@ int ttm_bo_wait(struct ttm_buffer_object *bo,
> }
> EXPORT_SYMBOL(ttm_bo_wait);
>
> -int ttm_bo_synccpu_write_grab(struct ttm_buffer_object *bo, bool
> no_wait)
> -{
> - int ret = 0;
> -
> - /*
> - * Using ttm_bo_reserve makes sure the lru lists are updated.
> - */
> -
> - ret = ttm_bo_reserve(bo, true, no_wait, NULL);
> - if (unlikely(ret != 0))
> - return ret;
> - ret = ttm_bo_wait(bo, true, no_wait);
> - if (likely(ret == 0))
> - atomic_inc(>cpu_writers);
> - ttm_bo_unreserve(bo);
> - return ret;
> -}
> -EXPORT_SYMBOL(ttm_bo_synccpu_write_grab);
> -
> -void ttm_bo_synccpu_write_release(struct ttm_buffer_object *bo)
> -{
> - atomic_dec(>cpu_writers);
> -}
> -EXPORT_SYMBOL(ttm_bo_synccpu_write_release);
> -
> /**
> * A buffer object shrink method that tries to swap out the first
> * buffer object on the bo_global::swap_lru list.
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 895d77d799e4..6f43f1f0de7c 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -511,7 +511,6 @@ static int ttm_buffer_object_transfer(struct
> ttm_buffer_object *bo,
> mutex_init(>base.wu_mutex);
> fbo->base.moving = NULL;
> drm_vma_node_reset(>base.vma_node);
> - atomic_set(>base.cpu_writers, 0);
>
> kref_init(>base.list_kref);
> kref_init(>base.kref);
> diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> index 957ec375a4ba..80fa52b36d5c 100644
> --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> @@ -113,12 +113,7 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx
> *ticket,
> struct ttm_buffer_object *bo = entry->bo;
>
> ret = __ttm_bo_reserve(bo, intr, (ticket == NULL),
> ticket);
> - if (!ret && unlikely(atomic_read(>cpu_writers) >
> 0)) {
> - reservation_object_unlock(bo->resv);
> -
> - ret = -EBUSY;
> -
> - } else if (ret == -EALREADY && dups) {
> + if (ret == -EALREADY && dups) {
> struct ttm_validate_buffer *safe = entry;
> entry = list_prev_entry(entry, head);
> list_del(>head);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 5d5c2bce01f3..457861c5047f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -565,7 +565,7 @@ static void vmw_user_bo_ref_obj_release(struct
> ttm_base_object *base,
>
> switch (ref_type) {
> case TTM_REF_SYNCCPU_WRITE:
> - ttm_bo_synccpu_write_release(_bo->vbo.base);
> + atomic_dec(_bo->vbo.cpu_writers);
> break;
> default:
> WARN_ONCE(true, "Undefined buffer object reference
> release.\n");
> @@ -681,12 +681,12 @@ static int vmw_user_bo_synccpu_grab(struct
> vmw_user_buffer_object *user_bo,
> struct ttm_object_file *tfile,
>
Re: [PATCH] video: backlight: Replace old GPIO APIs with GPIO Consumer APIs for sky81542-backlight driver
On Wed, Jun 12, 2019 at 11:26:15AM +0100, Daniel Thompson wrote:
> Hi Shobhit
>
> Thanks for the patch. Feedback below...
Hi Daneil,
You provided some valuable feedback. Thank you for your time and
effort.
>
>
> On Tue, Jun 11, 2019 at 09:32:32PM -0700, Shobhit Kukreti wrote:
> > Port the sky81452-backlight driver to adhere to new gpio descriptor based
> > APIs. Modified the file sky81452-backlight.c and sky81452-backlight.h.
> > The gpio descriptor property in device tree should be "sky81452-en-gpios"
>
> That is contradicted by the device tree bindings. The property should
> remain "gpios" as it was before this conversion.
You are correct.
>
>
> > Removed unnecessary header files "linux/gpio.h" and "linux/of_gpio.h".
> >
> > Signed-off-by: Shobhit Kukreti
>
> What level of testing have you done? Is this a fix for hardware you own
> or a cleanup after searching the sources?
>
I did not the test on actual hardware. I was trying to do a clean up and boot
on
QEMU/rock960 board. I will drop the patch until I can find the sk81452
hardware to test it.
>
> > ---
> > drivers/video/backlight/sky81452-backlight.c | 24
> >
> > include/linux/platform_data/sky81452-backlight.h | 4 +++-
> > 2 files changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/video/backlight/sky81452-backlight.c
> > b/drivers/video/backlight/sky81452-backlight.c
> > index d414c7a..12ef628 100644
> > --- a/drivers/video/backlight/sky81452-backlight.c
> > +++ b/drivers/video/backlight/sky81452-backlight.c
> > @@ -19,12 +19,10 @@
> >
> > #include
> > #include
> > -#include
> > #include
> > #include
> > #include
> > #include
> > -#include
> > #include
> > #include
> > #include
> > @@ -193,7 +191,6 @@ static struct sky81452_bl_platform_data
> > *sky81452_bl_parse_dt(
> > pdata->ignore_pwm = of_property_read_bool(np, "skyworks,ignore-pwm");
> > pdata->dpwm_mode = of_property_read_bool(np, "skyworks,dpwm-mode");
> > pdata->phase_shift = of_property_read_bool(np, "skyworks,phase-shift");
> > - pdata->gpio_enable = of_get_gpio(np, 0);
> >
> > ret = of_property_count_u32_elems(np, "led-sources");
> > if (ret < 0) {
> > @@ -274,13 +271,17 @@ static int sky81452_bl_probe(struct platform_device
> > *pdev)
> > if (IS_ERR(pdata))
> > return PTR_ERR(pdata);
> > }
> > -
> > - if (gpio_is_valid(pdata->gpio_enable)) {
> > - ret = devm_gpio_request_one(dev, pdata->gpio_enable,
> > - GPIOF_OUT_INIT_HIGH, "sky81452-en");
> > - if (ret < 0) {
> > - dev_err(dev, "failed to request GPIO. err=%d\n", ret);
> > - return ret;
> > + pdata->gpiod_enable = devm_gpiod_get(dev, "sk81452-en", GPIOD_OUT_HIGH);
>
> As above... I think the second argument here needs to be NULL in order
> to preserve the current DT bindings.
You are correct. I was testing this driver with a custom dtb in rock960 board
and named
the property as above in the dts. It should be NULL.
>
> > + if (IS_ERR(pdata->gpiod_enable)) {
> > + long ret = PTR_ERR(pdata->gpiod_enable);
> > +
> > + /**
>
> Nitpicking... but no second star here. That's a trigger symbold for
> documentation processors.
That is a great feedback. I will keep that in mind.
>
> > +* gpiod_enable is optional in device tree.
> > +* Return error only if gpio was assigned in device tree
>
> Also nitpicking but I had to read this a few times because
> gpiod_enable is not in device tree, gpios is.
You are correct. I will make necessary changes.
>
> This is a common pattern so the comment can be very short. Something
> like:
>
> This DT property is optional so no need to propagate ENOENT
Noted.
>
>
> > +*/
> > + if (ret != -ENOENT) {
> > + dev_err(dev, "failed to request GPIO. err=%ld\n", ret);
> > + return PTR_ERR(pdata->gpiod_enable);
> > }
> > }
> >
> > @@ -323,8 +324,7 @@ static int sky81452_bl_remove(struct platform_device
> > *pdev)
> > bd->props.brightness = 0;
> > backlight_update_status(bd);
> >
> > - if (gpio_is_valid(pdata->gpio_enable))
> > - gpio_set_value_cansleep(pdata->gpio_enable, 0);
> > + gpiod_set_value_cansleep(pdata->gpiod_enable, 0);
> >
> > return 0;
> > }
> > diff --git a/include/linux/platform_data/sky81452-backlight.h
> > b/include/linux/platform_data/sky81452-backlight.h
> > index 1231e9b..dc4cb85 100644
> > --- a/include/linux/platform_data/sky81452-backlight.h
> > +++ b/include/linux/platform_data/sky81452-backlight.h
> > @@ -20,6 +20,8 @@
> > #ifndef _SKY81452_BACKLIGHT_H
> > #define _SKY81452_BACKLIGHT_H
> >
> > +#include
> > +
>
> This heaer file should be included from the C file... it is not required
> to parse the header.
I
[PATCH v6 1/9] mm: Allow the [page|pfn]_mkwrite callbacks to drop the mmap_sem
From: Thomas Hellstrom
Driver fault callbacks are allowed to drop the mmap_sem when expecting
long hardware waits to avoid blocking other mm users. Allow the mkwrite
callbacks to do the same by returning early on VM_FAULT_RETRY.
In particular we want to be able to drop the mmap_sem when waiting for
a reservation object lock on a GPU buffer object. These locks may be
held while waiting for the GPU.
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Will Deacon
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Minchan Kim
Cc: Michal Hocko
Cc: Huang Ying
Cc: Souptick Joarder
Cc: "Jérôme Glisse"
Cc: linux...@kvack.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Thomas Hellstrom
Reviewed-by: Ralph Campbell
---
mm/memory.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index ddf20bd0c317..168f546af1ad 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2238,7 +2238,7 @@ static vm_fault_t do_page_mkwrite(struct vm_fault *vmf)
ret = vmf->vma->vm_ops->page_mkwrite(vmf);
/* Restore original flags so that caller is not surprised */
vmf->flags = old_flags;
- if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE)))
+ if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_RETRY)))
return ret;
if (unlikely(!(ret & VM_FAULT_LOCKED))) {
lock_page(page);
@@ -2515,7 +2515,7 @@ static vm_fault_t wp_pfn_shared(struct vm_fault *vmf)
pte_unmap_unlock(vmf->pte, vmf->ptl);
vmf->flags |= FAULT_FLAG_MKWRITE;
ret = vma->vm_ops->pfn_mkwrite(vmf);
- if (ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE))
+ if (ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_RETRY))
return ret;
return finish_mkwrite_fault(vmf);
}
@@ -2536,7 +2536,8 @@ static vm_fault_t wp_page_shared(struct vm_fault *vmf)
pte_unmap_unlock(vmf->pte, vmf->ptl);
tmp = do_page_mkwrite(vmf);
if (unlikely(!tmp || (tmp &
- (VM_FAULT_ERROR | VM_FAULT_NOPAGE {
+ (VM_FAULT_ERROR | VM_FAULT_NOPAGE |
+ VM_FAULT_RETRY {
put_page(vmf->page);
return tmp;
}
@@ -3601,7 +3602,8 @@ static vm_fault_t do_shared_fault(struct vm_fault *vmf)
unlock_page(vmf->page);
tmp = do_page_mkwrite(vmf);
if (unlikely(!tmp ||
- (tmp & (VM_FAULT_ERROR | VM_FAULT_NOPAGE {
+ (tmp & (VM_FAULT_ERROR | VM_FAULT_NOPAGE |
+ VM_FAULT_RETRY {
put_page(vmf->page);
return tmp;
}
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 3/9] mm: Add write-protect and clean utilities for address space ranges
From: Thomas Hellstrom
Add two utilities to a) write-protect and b) clean all ptes pointing into
a range of an address space.
The utilities are intended to aid in tracking dirty pages (either
driver-allocated system memory or pci device memory).
The write-protect utility should be used in conjunction with
page_mkwrite() and pfn_mkwrite() to trigger write page-faults on page
accesses. Typically one would want to use this on sparse accesses into
large memory regions. The clean utility should be used to utilize
hardware dirtying functionality and avoid the overhead of page-faults,
typically on large accesses into small memory regions.
The added file "as_dirty_helpers.c" is initially listed as maintained by
VMware under our DRM driver. If somebody would like it elsewhere,
that's of course no problem.
Notable changes since RFC:
- Added comments to help avoid the usage of these function for VMAs
it's not intended for. We also do advisory checks on the vm_flags and
warn on illegal usage.
- Perform the pte modifications the same way softdirty does.
- Add mmu_notifier range invalidation calls.
- Add a config option so that this code is not unconditionally included.
- Tell the mmu_gather code about pending tlb flushes.
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Will Deacon
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Minchan Kim
Cc: Michal Hocko
Cc: Huang Ying
Cc: Souptick Joarder
Cc: "Jérôme Glisse"
Cc: linux...@kvack.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Thomas Hellstrom
Reviewed-by: Ralph Campbell #v1
---
MAINTAINERS | 1 +
include/linux/mm.h| 9 +-
mm/Kconfig| 3 +
mm/Makefile | 1 +
mm/as_dirty_helpers.c | 300 ++
5 files changed, 313 insertions(+), 1 deletion(-)
create mode 100644 mm/as_dirty_helpers.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2f487ea49a..a55d4ef91b0b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5179,6 +5179,7 @@ T:git git://people.freedesktop.org/~thomash/linux
S: Supported
F: drivers/gpu/drm/vmwgfx/
F: include/uapi/drm/vmwgfx_drm.h
+F: mm/as_dirty_helpers.c
DRM DRIVERS
M: David Airlie
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 3d06ce2a64af..a0bc2a82917e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2685,7 +2685,14 @@ struct pfn_range_apply {
};
extern int apply_to_pfn_range(struct pfn_range_apply *closure,
unsigned long address, unsigned long size);
-
+unsigned long apply_as_wrprotect(struct address_space *mapping,
+pgoff_t first_index, pgoff_t nr);
+unsigned long apply_as_clean(struct address_space *mapping,
+pgoff_t first_index, pgoff_t nr,
+pgoff_t bitmap_pgoff,
+unsigned long *bitmap,
+pgoff_t *start,
+pgoff_t *end);
#ifdef CONFIG_PAGE_POISONING
extern bool page_poisoning_enabled(void);
extern void kernel_poison_pages(struct page *page, int numpages, int enable);
diff --git a/mm/Kconfig b/mm/Kconfig
index f0c76ba47695..5006d0e6a5c7 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -765,4 +765,7 @@ config GUP_BENCHMARK
config ARCH_HAS_PTE_SPECIAL
bool
+config AS_DIRTY_HELPERS
+bool
+
endmenu
diff --git a/mm/Makefile b/mm/Makefile
index ac5e5ba78874..f5d412bbc2f7 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -104,3 +104,4 @@ obj-$(CONFIG_HARDENED_USERCOPY) += usercopy.o
obj-$(CONFIG_PERCPU_STATS) += percpu-stats.o
obj-$(CONFIG_HMM) += hmm.o
obj-$(CONFIG_MEMFD_CREATE) += memfd.o
+obj-$(CONFIG_AS_DIRTY_HELPERS) += as_dirty_helpers.o
diff --git a/mm/as_dirty_helpers.c b/mm/as_dirty_helpers.c
new file mode 100644
index ..f600e31534fb
--- /dev/null
+++ b/mm/as_dirty_helpers.c
@@ -0,0 +1,300 @@
+// SPDX-License-Identifier: GPL-2.0
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * struct apply_as - Closure structure for apply_as_range
+ * @base: struct pfn_range_apply we derive from
+ * @start: Address of first modified pte
+ * @end: Address of last modified pte + 1
+ * @total: Total number of modified ptes
+ * @vma: Pointer to the struct vm_area_struct we're currently operating on
+ */
+struct apply_as {
+ struct pfn_range_apply base;
+ unsigned long start;
+ unsigned long end;
+ unsigned long total;
+ struct vm_area_struct *vma;
+};
+
+/**
+ * apply_pt_wrprotect - Leaf pte callback to write-protect a pte
+ * @pte: Pointer to the pte
+ * @token: Page table token, see apply_to_pfn_range()
+ * @addr: The virtual page address
+ * @closure: Pointer to a struct pfn_range_apply embedded in a
+ * struct apply_as
+ *
+ * The function write-protects a pte and records the range in
+ * virtual address space of touched ptes for efficient range TLB flushes.
+ *
+ * Return: Always zero.
+ */
+static int
[PATCH v6 2/9] mm: Add an apply_to_pfn_range interface
From: Thomas Hellstrom
This is basically apply_to_page_range with added functionality:
Allocating missing parts of the page table becomes optional, which
means that the function can be guaranteed not to error if allocation
is disabled. Also passing of the closure struct and callback function
becomes different and more in line with how things are done elsewhere.
Finally we keep apply_to_page_range as a wrapper around apply_to_pfn_range
The reason for not using the page-walk code is that we want to perform
the page-walk on vmas pointing to an address space without requiring the
mmap_sem to be held rather than on vmas belonging to a process with the
mmap_sem held.
Notable changes since RFC:
Don't export apply_to_pfn range.
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Will Deacon
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Minchan Kim
Cc: Michal Hocko
Cc: Huang Ying
Cc: Souptick Joarder
Cc: "Jérôme Glisse"
Cc: linux...@kvack.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Thomas Hellstrom
Reviewed-by: Ralph Campbell #v1
---
include/linux/mm.h | 10
mm/memory.c| 135 ++---
2 files changed, 113 insertions(+), 32 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0e8834ac32b7..3d06ce2a64af 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2675,6 +2675,16 @@ typedef int (*pte_fn_t)(pte_t *pte, pgtable_t token,
unsigned long addr,
extern int apply_to_page_range(struct mm_struct *mm, unsigned long address,
unsigned long size, pte_fn_t fn, void *data);
+struct pfn_range_apply;
+typedef int (*pter_fn_t)(pte_t *pte, pgtable_t token, unsigned long addr,
+struct pfn_range_apply *closure);
+struct pfn_range_apply {
+ struct mm_struct *mm;
+ pter_fn_t ptefn;
+ unsigned int alloc;
+};
+extern int apply_to_pfn_range(struct pfn_range_apply *closure,
+ unsigned long address, unsigned long size);
#ifdef CONFIG_PAGE_POISONING
extern bool page_poisoning_enabled(void);
diff --git a/mm/memory.c b/mm/memory.c
index 168f546af1ad..462aa47f8878 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2032,18 +2032,17 @@ int vm_iomap_memory(struct vm_area_struct *vma,
phys_addr_t start, unsigned long
}
EXPORT_SYMBOL(vm_iomap_memory);
-static int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd,
-unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data)
+static int apply_to_pte_range(struct pfn_range_apply *closure, pmd_t *pmd,
+ unsigned long addr, unsigned long end)
{
pte_t *pte;
int err;
pgtable_t token;
spinlock_t *uninitialized_var(ptl);
- pte = (mm == _mm) ?
+ pte = (closure->mm == _mm) ?
pte_alloc_kernel(pmd, addr) :
- pte_alloc_map_lock(mm, pmd, addr, );
+ pte_alloc_map_lock(closure->mm, pmd, addr, );
if (!pte)
return -ENOMEM;
@@ -2054,86 +2053,109 @@ static int apply_to_pte_range(struct mm_struct *mm,
pmd_t *pmd,
token = pmd_pgtable(*pmd);
do {
- err = fn(pte++, token, addr, data);
+ err = closure->ptefn(pte++, token, addr, closure);
if (err)
break;
} while (addr += PAGE_SIZE, addr != end);
arch_leave_lazy_mmu_mode();
- if (mm != _mm)
+ if (closure->mm != _mm)
pte_unmap_unlock(pte-1, ptl);
return err;
}
-static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
-unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data)
+static int apply_to_pmd_range(struct pfn_range_apply *closure, pud_t *pud,
+ unsigned long addr, unsigned long end)
{
pmd_t *pmd;
unsigned long next;
- int err;
+ int err = 0;
BUG_ON(pud_huge(*pud));
- pmd = pmd_alloc(mm, pud, addr);
+ pmd = pmd_alloc(closure->mm, pud, addr);
if (!pmd)
return -ENOMEM;
+
do {
next = pmd_addr_end(addr, end);
- err = apply_to_pte_range(mm, pmd, addr, next, fn, data);
+ if (!closure->alloc && pmd_none_or_clear_bad(pmd))
+ continue;
+ err = apply_to_pte_range(closure, pmd, addr, next);
if (err)
break;
} while (pmd++, addr = next, addr != end);
return err;
}
-static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d,
-unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data)
+static int apply_to_pud_range(struct pfn_range_apply *closure, p4d_t *p4d,
+ unsigned long addr,
[PATCH v6 9/9] drm/vmwgfx: Add surface dirty-tracking callbacks
From: Thomas Hellstrom
Add the callbacks necessary to implement emulated coherent memory for
surfaces. Add a flag to the gb_surface_create ioctl to indicate that
surface memory should be coherent.
Also bump the drm minor version to signal the availability of coherent
surfaces.
Signed-off-by: Thomas Hellstrom
Reviewed-by: Deepak Rawat
---
.../device_include/svga3d_surfacedefs.h | 233 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 395 +-
include/uapi/drm/vmwgfx_drm.h | 4 +-
4 files changed, 629 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/device_include/svga3d_surfacedefs.h
b/drivers/gpu/drm/vmwgfx/device_include/svga3d_surfacedefs.h
index f2bfd3d80598..61414f105c67 100644
--- a/drivers/gpu/drm/vmwgfx/device_include/svga3d_surfacedefs.h
+++ b/drivers/gpu/drm/vmwgfx/device_include/svga3d_surfacedefs.h
@@ -1280,7 +1280,6 @@ svga3dsurface_get_pixel_offset(SVGA3dSurfaceFormat format,
return offset;
}
-
static inline u32
svga3dsurface_get_image_offset(SVGA3dSurfaceFormat format,
surf_size_struct baseLevelSize,
@@ -1375,4 +1374,236 @@
svga3dsurface_is_screen_target_format(SVGA3dSurfaceFormat format)
return svga3dsurface_is_dx_screen_target_format(format);
}
+/**
+ * struct svga3dsurface_mip - Mimpmap level information
+ * @bytes: Bytes required in the backing store of this mipmap level.
+ * @img_stride: Byte stride per image.
+ * @row_stride: Byte stride per block row.
+ * @size: The size of the mipmap.
+ */
+struct svga3dsurface_mip {
+ size_t bytes;
+ size_t img_stride;
+ size_t row_stride;
+ struct drm_vmw_size size;
+
+};
+
+/**
+ * struct svga3dsurface_cache - Cached surface information
+ * @desc: Pointer to the surface descriptor
+ * @mip: Array of mipmap level information. Valid size is @num_mip_levels.
+ * @mip_chain_bytes: Bytes required in the backing store for the whole chain
+ * of mip levels.
+ * @sheet_bytes: Bytes required in the backing store for a sheet
+ * representing a single sample.
+ * @num_mip_levels: Valid size of the @mip array. Number of mipmap levels in
+ * a chain.
+ * @num_layers: Number of slices in an array texture or number of faces in
+ * a cubemap texture.
+ */
+struct svga3dsurface_cache {
+ const struct svga3d_surface_desc *desc;
+ struct svga3dsurface_mip mip[DRM_VMW_MAX_MIP_LEVELS];
+ size_t mip_chain_bytes;
+ size_t sheet_bytes;
+ u32 num_mip_levels;
+ u32 num_layers;
+};
+
+/**
+ * struct svga3dsurface_loc - Surface location
+ * @sub_resource: Surface subresource. Defined as layer * num_mip_levels +
+ * mip_level.
+ * @x: X coordinate.
+ * @y: Y coordinate.
+ * @z: Z coordinate.
+ */
+struct svga3dsurface_loc {
+ u32 sub_resource;
+ u32 x, y, z;
+};
+
+/**
+ * svga3dsurface_subres - Compute the subresource from layer and mipmap.
+ * @cache: Surface layout data.
+ * @mip_level: The mipmap level.
+ * @layer: The surface layer (face or array slice).
+ *
+ * Return: The subresource.
+ */
+static inline u32 svga3dsurface_subres(const struct svga3dsurface_cache *cache,
+ u32 mip_level, u32 layer)
+{
+ return cache->num_mip_levels * layer + mip_level;
+}
+
+/**
+ * svga3dsurface_setup_cache - Build a surface cache entry
+ * @size: The surface base level dimensions.
+ * @format: The surface format.
+ * @num_mip_levels: Number of mipmap levels.
+ * @num_layers: Number of layers.
+ * @cache: Pointer to a struct svga3dsurface_cach object to be filled in.
+ *
+ * Return: Zero on success, -EINVAL on invalid surface layout.
+ */
+static inline int svga3dsurface_setup_cache(const struct drm_vmw_size *size,
+ SVGA3dSurfaceFormat format,
+ u32 num_mip_levels,
+ u32 num_layers,
+ u32 num_samples,
+ struct svga3dsurface_cache *cache)
+{
+ const struct svga3d_surface_desc *desc;
+ u32 i;
+
+ memset(cache, 0, sizeof(*cache));
+ cache->desc = desc = svga3dsurface_get_desc(format);
+ cache->num_mip_levels = num_mip_levels;
+ cache->num_layers = num_layers;
+ for (i = 0; i < cache->num_mip_levels; i++) {
+ struct svga3dsurface_mip *mip = >mip[i];
+
+ mip->size = svga3dsurface_get_mip_size(*size, i);
+ mip->bytes = svga3dsurface_get_image_buffer_size
+ (desc, >size, 0);
+ mip->row_stride =
+ __KERNEL_DIV_ROUND_UP(mip->size.width,
+ desc->block_size.width) *
+ desc->bytes_per_block * num_samples;
+ if (!mip->row_stride)
+ goto invalid_dim;
+
+
[PATCH v6 0/9] Emulated coherent graphics memory
Planning to merge this through the drm/vmwgfx tree soon, so if there are any objections, please speak up. Graphics APIs like OpenGL 4.4 and Vulkan require the graphics driver to provide coherent graphics memory, meaning that the GPU sees any content written to the coherent memory on the next GPU operation that touches that memory, and the CPU sees any content written by the GPU to that memory immediately after any fence object trailing the GPU operation has signaled. Paravirtual drivers that otherwise require explicit synchronization needs to do this by hooking up dirty tracking to pagefault handlers and buffer object validation. This is a first attempt to do that for the vmwgfx driver. The mm patches has been out for RFC. I think I have addressed all the feedback I got, except a possible softdirty breakage. But although the dirty-tracking and softdirty may write-protect PTEs both care about, that shouldn't really cause any operation interference. In particular since we use the hardware dirty PTE bits and softdirty uses other PTE bits. For the TTM changes they are hopefully in line with the long-term strategy of making helpers out of what's left of TTM. The code has been tested and exercised by a tailored version of mesa where we disable all explicit synchronization and assume graphics memory is coherent. The performance loss varies of course; a typical number is around 5%. Changes v1-v2: - Addressed a number of typos and formatting issues. - Added a usage warning for apply_to_pfn_range() and apply_to_page_range() - Re-evaluated the decision to use apply_to_pfn_range() rather than modifying the pagewalk.c. It still looks like generically handling the transparent huge page cases requires the mmap_sem to be held at least in read mode, so sticking with apply_to_pfn_range() for now. - The TTM page-fault helper vma copy argument was scratched in favour of a pageprot_t argument. Changes v3: - Adapted to upstream API changes. Changes v4: - Adapted to upstream mmu_notifier changes. (Jerome?) - Fixed a couple of warnings on 32-bit x86 - Fixed image offset computation on multisample images. Changes v5: - Updated usage warning in patch 3/9 after review comments from Nadav Amit. Changes v6: - Updated exports of new functionality in patch 3/9 to EXPORT_SYMBOL_GPL after review comments from Christoph Hellwig. Cc: Andrew Morton Cc: Matthew Wilcox Cc: Will Deacon Cc: Peter Zijlstra Cc: Rik van Riel Cc: Minchan Kim Cc: Michal Hocko Cc: Huang Ying Cc: Souptick Joarder Cc: "Jérôme Glisse" Cc: "Christian König" Cc: linux...@kvack.org ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 4/9] drm/ttm: Allow the driver to provide the ttm struct vm_operations_struct
From: Thomas Hellstrom
Add a pointer to the struct vm_operations_struct in the bo_device, and
assign that pointer to the default value currently used.
The driver can then optionally modify that pointer and the new value
can be used for each new vma created.
Cc: "Christian König"
Signed-off-by: Thomas Hellstrom
Reviewed-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c| 1 +
drivers/gpu/drm/ttm/ttm_bo_vm.c | 6 +++---
include/drm/ttm/ttm_bo_driver.h | 6 ++
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c7de667d482a..6953dd264172 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1739,6 +1739,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
mutex_lock(_global_mutex);
list_add_tail(>device_list, >device_list);
mutex_unlock(_global_mutex);
+ bdev->vm_ops = _bo_vm_ops;
return 0;
out_no_sys:
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 6dacff49c1cc..196e13a0adad 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -395,7 +395,7 @@ static int ttm_bo_vm_access(struct vm_area_struct *vma,
unsigned long addr,
return ret;
}
-static const struct vm_operations_struct ttm_bo_vm_ops = {
+const struct vm_operations_struct ttm_bo_vm_ops = {
.fault = ttm_bo_vm_fault,
.open = ttm_bo_vm_open,
.close = ttm_bo_vm_close,
@@ -448,7 +448,7 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct
*vma,
if (unlikely(ret != 0))
goto out_unref;
- vma->vm_ops = _bo_vm_ops;
+ vma->vm_ops = bdev->vm_ops;
/*
* Note: We're transferring the bo reference to
@@ -480,7 +480,7 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct
ttm_buffer_object *bo)
ttm_bo_get(bo);
- vma->vm_ops = _bo_vm_ops;
+ vma->vm_ops = bo->bdev->vm_ops;
vma->vm_private_data = bo;
vma->vm_flags |= VM_MIXEDMAP;
vma->vm_flags |= VM_IO | VM_DONTEXPAND;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index c9b8ba492f24..a2d810a2504d 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -442,6 +442,9 @@ extern struct ttm_bo_global {
* @driver: Pointer to a struct ttm_bo_driver struct setup by the driver.
* @man: An array of mem_type_managers.
* @vma_manager: Address space manager
+ * @vm_ops: Pointer to the struct vm_operations_struct used for this
+ * device's VM operations. The driver may override this before the first
+ * mmap() call.
* lru_lock: Spinlock that protects the buffer+device lru lists and
* ddestroy lists.
* @dev_mapping: A pointer to the struct address_space representing the
@@ -460,6 +463,7 @@ struct ttm_bo_device {
struct ttm_bo_global *glob;
struct ttm_bo_driver *driver;
struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES];
+ const struct vm_operations_struct *vm_ops;
/*
* Protected by internal locks.
@@ -488,6 +492,8 @@ struct ttm_bo_device {
bool no_retry;
};
+extern const struct vm_operations_struct ttm_bo_vm_ops;
+
/**
* struct ttm_lru_bulk_move_pos
*
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 7/9] drm/vmwgfx: Use an RBtree instead of linked list for MOB resources
From: Thomas Hellstrom
With emulated coherent memory we need to be able to quickly look up
a resource from the MOB offset. Instead of traversing a linked list with
O(n) worst case, use an RBtree with O(log n) worst case complexity.
Signed-off-by: Thomas Hellstrom
Reviewed-by: Deepak Rawat
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 5 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 10 +++
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 33 +---
3 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 90ca866640fe..e8bc7a7ac031 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -464,6 +464,7 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo)
struct vmw_buffer_object *vmw_bo = vmw_buffer_object(bo);
WARN_ON(vmw_bo->dirty);
+ WARN_ON(!RB_EMPTY_ROOT(_bo->res_tree));
vmw_bo_unmap(vmw_bo);
kfree(vmw_bo);
}
@@ -480,6 +481,7 @@ static void vmw_user_bo_destroy(struct ttm_buffer_object
*bo)
struct vmw_buffer_object *vbo = _user_bo->vbo;
WARN_ON(vbo->dirty);
+ WARN_ON(!RB_EMPTY_ROOT(>res_tree));
vmw_bo_unmap(vbo);
ttm_prime_object_kfree(vmw_user_bo, prime);
}
@@ -515,8 +517,7 @@ int vmw_bo_init(struct vmw_private *dev_priv,
memset(vmw_bo, 0, sizeof(*vmw_bo));
BUILD_BUG_ON(TTM_MAX_BO_PRIORITY <= 3);
vmw_bo->base.priority = 3;
-
- INIT_LIST_HEAD(_bo->res_list);
+ vmw_bo->res_tree = RB_ROOT;
ret = ttm_bo_init(bdev, _bo->base, size,
ttm_bo_type_device, placement,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 27c259395790..9b347923196f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -89,7 +89,7 @@ struct vmw_fpriv {
/**
* struct vmw_buffer_object - TTM buffer object with vmwgfx additions
* @base: The TTM buffer object
- * @res_list: List of resources using this buffer object as a backing MOB
+ * @res_tree: RB tree of resources using this buffer object as a backing MOB
* @pin_count: pin depth
* @dx_query_ctx: DX context if this buffer object is used as a DX query MOB
* @map: Kmap object for semi-persistent mappings
@@ -98,7 +98,7 @@ struct vmw_fpriv {
*/
struct vmw_buffer_object {
struct ttm_buffer_object base;
- struct list_head res_list;
+ struct rb_root res_tree;
s32 pin_count;
/* Not ref-counted. Protected by binding_mutex */
struct vmw_resource *dx_query_ctx;
@@ -146,8 +146,8 @@ struct vmw_res_func;
* pin-count greater than zero. It is not on the resource LRU lists and its
* backup buffer is pinned. Hence it can't be evicted.
* @func: Method vtable for this resource. Immutable.
+ * @mob_node; Node for the MOB backup rbtree. Protected by @backup reserved.
* @lru_head: List head for the LRU list. Protected by
@dev_priv::resource_lock.
- * @mob_head: List head for the MOB backup list. Protected by @backup reserved.
* @binding_head: List head for the context binding list. Protected by
* the @dev_priv::binding_mutex
* @res_free: The resource destructor.
@@ -168,8 +168,8 @@ struct vmw_resource {
unsigned long backup_offset;
unsigned long pin_count;
const struct vmw_res_func *func;
+ struct rb_node mob_node;
struct list_head lru_head;
- struct list_head mob_head;
struct list_head binding_head;
struct vmw_resource_dirty *dirty;
void (*res_free) (struct vmw_resource *res);
@@ -742,7 +742,7 @@ void vmw_resource_dirty_update(struct vmw_resource *res,
pgoff_t start,
*/
static inline bool vmw_resource_mob_attached(const struct vmw_resource *res)
{
- return !list_empty(>mob_head);
+ return !RB_EMPTY_NODE(>mob_node);
}
/**
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index da9afa83fb4f..b84dd5953886 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -41,11 +41,24 @@
void vmw_resource_mob_attach(struct vmw_resource *res)
{
struct vmw_buffer_object *backup = res->backup;
+ struct rb_node **new = >res_tree.rb_node, *parent = NULL;
lockdep_assert_held(>base.resv->lock.base);
res->used_prio = (res->res_dirty) ? res->func->dirty_prio :
res->func->prio;
- list_add_tail(>mob_head, >res_list);
+
+ while (*new) {
+ struct vmw_resource *this =
+ container_of(*new, struct vmw_resource, mob_node);
+
+ parent = *new;
+ new = (res->backup_offset < this->backup_offset) ?
+ &((*new)->rb_left) : &((*new)->rb_right);
+ }
+
+ rb_link_node(>mob_node, parent, new);
+ rb_insert_color(>mob_node, >res_tree);
+
[PATCH v6 6/9] drm/vmwgfx: Implement an infrastructure for write-coherent resources
From: Thomas Hellstrom
This infrastructure will, for coherent resources, make sure that
from the user-space point of view, data written by the CPU is immediately
automatically available to the GPU at resource validation time.
Signed-off-by: Thomas Hellstrom
Reviewed-by: Deepak Rawat
---
drivers/gpu/drm/vmwgfx/Kconfig| 1 +
drivers/gpu/drm/vmwgfx/Makefile | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c| 5 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 26 +-
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 1 -
drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c| 409 ++
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 57 +++
drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h | 11 +
drivers/gpu/drm/vmwgfx/vmwgfx_validation.c| 71 +++
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h| 16 +-
11 files changed, 584 insertions(+), 20 deletions(-)
create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
diff --git a/drivers/gpu/drm/vmwgfx/Kconfig b/drivers/gpu/drm/vmwgfx/Kconfig
index 6b28a326f8bb..d5fd81a521f6 100644
--- a/drivers/gpu/drm/vmwgfx/Kconfig
+++ b/drivers/gpu/drm/vmwgfx/Kconfig
@@ -8,6 +8,7 @@ config DRM_VMWGFX
select FB_CFB_IMAGEBLIT
select DRM_TTM
select FB
+ select AS_DIRTY_HELPERS
# Only needed for the transitional use of drm_crtc_init - can be removed
# again once vmwgfx sets up the primary plane itself.
select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
index 8841bd30e1e5..c877a21a0739 100644
--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile
@@ -8,7 +8,7 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o
vmwgfx_drv.o \
vmwgfx_cmdbuf_res.o vmwgfx_cmdbuf.o vmwgfx_stdu.o \
vmwgfx_cotable.o vmwgfx_so.o vmwgfx_binding.o vmwgfx_msg.o \
vmwgfx_simple_resource.o vmwgfx_va.o vmwgfx_blit.o \
- vmwgfx_validation.o \
+ vmwgfx_validation.o vmwgfx_page_dirty.o \
ttm_object.o ttm_lock.o
obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index c0829d50eecc..90ca866640fe 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -463,6 +463,7 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo)
{
struct vmw_buffer_object *vmw_bo = vmw_buffer_object(bo);
+ WARN_ON(vmw_bo->dirty);
vmw_bo_unmap(vmw_bo);
kfree(vmw_bo);
}
@@ -476,8 +477,10 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo)
static void vmw_user_bo_destroy(struct ttm_buffer_object *bo)
{
struct vmw_user_buffer_object *vmw_user_bo = vmw_user_buffer_object(bo);
+ struct vmw_buffer_object *vbo = _user_bo->vbo;
- vmw_bo_unmap(_user_bo->vbo);
+ WARN_ON(vbo->dirty);
+ vmw_bo_unmap(vbo);
ttm_prime_object_kfree(vmw_user_bo, prime);
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 4ff11a0077e1..d59c474be38e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -833,6 +833,11 @@ static int vmw_driver_load(struct drm_device *dev,
unsigned long chipset)
DRM_ERROR("Failed initializing TTM buffer object driver.\n");
goto out_no_bdev;
}
+ dev_priv->vm_ops = *dev_priv->bdev.vm_ops;
+ dev_priv->vm_ops.fault = vmw_bo_vm_fault;
+ dev_priv->vm_ops.pfn_mkwrite = vmw_bo_vm_mkwrite;
+ dev_priv->vm_ops.page_mkwrite = vmw_bo_vm_mkwrite;
+ dev_priv->bdev.vm_ops = _priv->vm_ops;
/*
* Enable VRAM, but initially don't use it until SVGA is enabled and
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 7c935c72d368..27c259395790 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -94,6 +94,7 @@ struct vmw_fpriv {
* @dx_query_ctx: DX context if this buffer object is used as a DX query MOB
* @map: Kmap object for semi-persistent mappings
* @res_prios: Eviction priority counts for attached resources
+ * @dirty: structure for user-space dirty-tracking
*/
struct vmw_buffer_object {
struct ttm_buffer_object base;
@@ -104,6 +105,7 @@ struct vmw_buffer_object {
/* Protected by reservation */
struct ttm_bo_kmap_obj map;
u32 res_prios[TTM_MAX_BO_PRIORITY];
+ struct vmw_bo_dirty *dirty;
};
/**
@@ -134,7 +136,8 @@ struct vmw_res_func;
* @res_dirty: Resource contains data not yet in the backup buffer. Protected
* by resource reserved.
* @backup_dirty: Backup buffer contains data not yet in the HW resource.
- * Protecte by resource reserved.
+ * Protected by resource reserved.
+ * @coherent: Emulate coherency by tracking vm accesses.
* @backup: The backup buffer
[PATCH v6 8/9] drm/vmwgfx: Implement an infrastructure for read-coherent resources
From: Thomas Hellstrom
Similar to write-coherent resources, make sure that from the user-space
point of view, GPU rendered contents is automatically available for
reading by the CPU.
Signed-off-by: Thomas Hellstrom
Reviewed-by: Deepak Rawat
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 7 +-
drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c| 73 -
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 103 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h | 2 +
drivers/gpu/drm/vmwgfx/vmwgfx_validation.c| 3 +-
5 files changed, 177 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 9b347923196f..dae3a39bf402 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -689,7 +689,8 @@ extern void vmw_resource_unreference(struct vmw_resource
**p_res);
extern struct vmw_resource *vmw_resource_reference(struct vmw_resource *res);
extern struct vmw_resource *
vmw_resource_reference_unless_doomed(struct vmw_resource *res);
-extern int vmw_resource_validate(struct vmw_resource *res, bool intr);
+extern int vmw_resource_validate(struct vmw_resource *res, bool intr,
+bool dirtying);
extern int vmw_resource_reserve(struct vmw_resource *res, bool interruptible,
bool no_backup);
extern bool vmw_resource_needs_backup(const struct vmw_resource *res);
@@ -733,6 +734,8 @@ void vmw_resource_mob_attach(struct vmw_resource *res);
void vmw_resource_mob_detach(struct vmw_resource *res);
void vmw_resource_dirty_update(struct vmw_resource *res, pgoff_t start,
pgoff_t end);
+int vmw_resources_clean(struct vmw_buffer_object *vbo, pgoff_t start,
+ pgoff_t end, pgoff_t *num_prefault);
/**
* vmw_resource_mob_attached - Whether a resource currently has a mob attached
@@ -1427,6 +1430,8 @@ int vmw_bo_dirty_add(struct vmw_buffer_object *vbo);
void vmw_bo_dirty_transfer_to_res(struct vmw_resource *res);
void vmw_bo_dirty_clear_res(struct vmw_resource *res);
void vmw_bo_dirty_release(struct vmw_buffer_object *vbo);
+void vmw_bo_dirty_unmap(struct vmw_buffer_object *vbo,
+ pgoff_t start, pgoff_t end);
vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf);
vm_fault_t vmw_bo_vm_mkwrite(struct vm_fault *vmf);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
index 8d154f90bdc0..730c51e397dd 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
@@ -153,7 +153,6 @@ static void vmw_bo_dirty_scan_mkwrite(struct
vmw_buffer_object *vbo)
}
}
-
/**
* vmw_bo_dirty_scan - Scan for dirty pages and add them to the dirty
* tracking structure
@@ -171,6 +170,51 @@ void vmw_bo_dirty_scan(struct vmw_buffer_object *vbo)
vmw_bo_dirty_scan_mkwrite(vbo);
}
+/**
+ * vmw_bo_dirty_pre_unmap - write-protect and pick up dirty pages before
+ * an unmap_mapping_range operation.
+ * @vbo: The buffer object,
+ * @start: First page of the range within the buffer object.
+ * @end: Last page of the range within the buffer object + 1.
+ *
+ * If we're using the _PAGETABLE scan method, we may leak dirty pages
+ * when calling unmap_mapping_range(). This function makes sure we pick
+ * up all dirty pages.
+ */
+static void vmw_bo_dirty_pre_unmap(struct vmw_buffer_object *vbo,
+ pgoff_t start, pgoff_t end)
+{
+ struct vmw_bo_dirty *dirty = vbo->dirty;
+ unsigned long offset = drm_vma_node_start(>base.vma_node);
+ struct address_space *mapping = vbo->base.bdev->dev_mapping;
+
+ if (dirty->method != VMW_BO_DIRTY_PAGETABLE || start >= end)
+ return;
+
+ apply_as_wrprotect(mapping, start + offset, end - start);
+ apply_as_clean(mapping, start + offset, end - start, offset,
+ >bitmap[0], >start, >end);
+}
+
+/**
+ * vmw_bo_dirty_unmap - Clear all ptes pointing to a range within a bo
+ * @vbo: The buffer object,
+ * @start: First page of the range within the buffer object.
+ * @end: Last page of the range within the buffer object + 1.
+ *
+ * This is similar to ttm_bo_unmap_virtual_locked() except it takes a subrange.
+ */
+void vmw_bo_dirty_unmap(struct vmw_buffer_object *vbo,
+ pgoff_t start, pgoff_t end)
+{
+ unsigned long offset = drm_vma_node_start(>base.vma_node);
+ struct address_space *mapping = vbo->base.bdev->dev_mapping;
+
+ vmw_bo_dirty_pre_unmap(vbo, start, end);
+ unmap_shared_mapping_range(mapping, (offset + start) << PAGE_SHIFT,
+ (loff_t) (end - start) << PAGE_SHIFT);
+}
+
/**
* vmw_bo_dirty_add - Add a dirty-tracking user to a buffer object
* @vbo: The buffer object
@@ -389,21 +433,40 @@ vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf)
if
[PATCH v6 5/9] drm/ttm: TTM fault handler helpers
From: Thomas Hellstrom
With the vmwgfx dirty tracking, the default TTM fault handler is not
completely sufficient (vmwgfx need to modify the vma->vm_flags member,
and also needs to restrict the number of prefaults).
We also want to replicate the new ttm_bo_vm_reserve() functionality
So start turning the TTM vm code into helpers: ttm_bo_vm_fault_reserved()
and ttm_bo_vm_reserve(), and provide a default TTM fault handler for other
drivers to use.
Cc: "Christian König"
Signed-off-by: Thomas Hellstrom
Reviewed-by: "Christian König" #v1
---
drivers/gpu/drm/ttm/ttm_bo_vm.c | 175 +++-
include/drm/ttm/ttm_bo_api.h| 10 ++
2 files changed, 113 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 196e13a0adad..2d9862fcf6fd 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -42,8 +42,6 @@
#include
#include
-#define TTM_BO_VM_NUM_PREFAULT 16
-
static vm_fault_t ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
struct vm_fault *vmf)
{
@@ -106,31 +104,30 @@ static unsigned long ttm_bo_io_mem_pfn(struct
ttm_buffer_object *bo,
+ page_offset;
}
-static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
+/**
+ * ttm_bo_vm_reserve - Reserve a buffer object in a retryable vm callback
+ * @bo: The buffer object
+ * @vmf: The fault structure handed to the callback
+ *
+ * vm callbacks like fault() and *_mkwrite() allow for the mm_sem to be dropped
+ * during long waits, and after the wait the callback will be restarted. This
+ * is to allow other threads using the same virtual memory space concurrent
+ * access to map(), unmap() completely unrelated buffer objects. TTM buffer
+ * object reservations sometimes wait for GPU and should therefore be
+ * considered long waits. This function reserves the buffer object
interruptibly
+ * taking this into account. Starvation is avoided by the vm system not
+ * allowing too many repeated restarts.
+ * This function is intended to be used in customized fault() and _mkwrite()
+ * handlers.
+ *
+ * Return:
+ *0 on success and the bo was reserved.
+ *VM_FAULT_RETRY if blocking wait.
+ *VM_FAULT_NOPAGE if blocking wait and retrying was not allowed.
+ */
+vm_fault_t ttm_bo_vm_reserve(struct ttm_buffer_object *bo,
+struct vm_fault *vmf)
{
- struct vm_area_struct *vma = vmf->vma;
- struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
- vma->vm_private_data;
- struct ttm_bo_device *bdev = bo->bdev;
- unsigned long page_offset;
- unsigned long page_last;
- unsigned long pfn;
- struct ttm_tt *ttm = NULL;
- struct page *page;
- int err;
- int i;
- vm_fault_t ret = VM_FAULT_NOPAGE;
- unsigned long address = vmf->address;
- struct ttm_mem_type_manager *man =
- >man[bo->mem.mem_type];
- struct vm_area_struct cvma;
-
- /*
-* Work around locking order reversal in fault / nopfn
-* between mmap_sem and bo_reserve: Perform a trylock operation
-* for reserve, and if it fails, retry the fault after waiting
-* for the buffer to become unreserved.
-*/
if (unlikely(!reservation_object_trylock(bo->resv))) {
if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
if (!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) {
@@ -151,14 +148,55 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
return VM_FAULT_NOPAGE;
}
+ return 0;
+}
+EXPORT_SYMBOL(ttm_bo_vm_reserve);
+
+/**
+ * ttm_bo_vm_fault_reserved - TTM fault helper
+ * @vmf: The struct vm_fault given as argument to the fault callback
+ * @prot: The page protection to be used for this memory area.
+ * @num_prefault: Maximum number of prefault pages. The caller may want to
+ * specify this based on madvice settings and the size of the GPU object
+ * backed by the memory.
+ *
+ * This function inserts one or more page table entries pointing to the
+ * memory backing the buffer object, and then returns a return code
+ * instructing the caller to retry the page access.
+ *
+ * Return:
+ * VM_FAULT_NOPAGE on success or pending signal
+ * VM_FAULT_SIGBUS on unspecified error
+ * VM_FAULT_OOM on out-of-memory
+ * VM_FAULT_RETRY if retryable wait
+ */
+vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
+ pgprot_t prot,
+ pgoff_t num_prefault)
+{
+ struct vm_area_struct *vma = vmf->vma;
+ struct vm_area_struct cvma = *vma;
+ struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
+ vma->vm_private_data;
+ struct ttm_bo_device *bdev = bo->bdev;
+ unsigned long page_offset;
+ unsigned long page_last;
+ unsigned long pfn;
+ struct ttm_tt *ttm = NULL;
+ struct page
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #6 from Gert Wollny --- The commit that added TGSI shaders with DIV were introduced with f6ac0b5d7187 gallium/auxiliary/vl: Add compute shader to support video compositor render and the use of the shaders was enabled with 9364d66cb7f7 gallium/auxiliary/vl: Add video compositor compute shader render The simplest approach is probably to add the lowering to RCP + MUL in the GLSL-TO-TGSI stage. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: backlight: Replace old GPIO APIs with GPIO Consumer APIs for sky81542-backlight driver
Hi Shobhit
> - if (gpio_is_valid(pdata->gpio_enable)) {
> - ret = devm_gpio_request_one(dev, pdata->gpio_enable,
> - GPIOF_OUT_INIT_HIGH, "sky81452-en");
> + pdata->gpiod_enable = devm_gpiod_get(dev, "sk81452-en", GPIOD_OUT_HIGH);
> + if (IS_ERR(pdata->gpiod_enable)) {
> + long ret = PTR_ERR(pdata->gpiod_enable);
In the old code the property was named "sky81452-en".
In the new code the property is named "sk81452-en".
Most likely a bug.
Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Tweak drm_encoder_helper_funcs.enable kerneldoc
On Wed, Jun 12, 2019 at 11:00:34AM -0400, Sean Paul wrote:
> From: Sean Paul
>
> I copied the kerneldoc for encoder_funcs.atomic_enable from
> encoder_funcs.enable
> in a recent patch [1]. Sam rightly pointed out in the review that "for
> symmetry
> with" text is awkward [2]. So here's a patch to fix up the source of the
> awkward
> language.
Looks good.
>
> [1]
> https://patchwork.freedesktop.org/patch/msgid/20190611160844.257498-2-s...@poorly.run
> [2]
> https://patchwork.freedesktop.org/patch/msgid/20190611185352.ga16...@ravnborg.org
>
> Suggested-by: Sam Ravnborg
> Signed-off-by: Sean Paul
Reviewed-by: Sam Ravnborg
> ---
> include/drm/drm_modeset_helper_vtables.h | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/drm/drm_modeset_helper_vtables.h
> b/include/drm/drm_modeset_helper_vtables.h
> index f9c94c2a13646..df80131bb10fc 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -719,11 +719,11 @@ struct drm_encoder_helper_funcs {
>* hooks and call them from CRTC's callback by looping over all encoders
>* connected to it using for_each_encoder_on_crtc().
>*
> - * This hook is used only by atomic helpers, for symmetry with @disable.
> - * Atomic drivers don't need to implement it if there's no need to
> - * enable anything at the encoder level. To ensure that runtime PM
> handling
> - * (using either DPMS or the new "ACTIVE" property) works
> - * @enable must be the inverse of @disable for atomic drivers.
> + * This hook is only used by atomic helpers, it is the opposite of
> + * @disable. Atomic drivers don't need to implement it if there's no
> + * need to enable anything at the encoder level. To ensure that
> + * runtime PM handling (using either DPMS or the new "ACTIVE" property)
> + * works @enable must be the inverse of @disable for atomic drivers.
>*/
> void (*enable)(struct drm_encoder *encoder);
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/panfrost: Add support for mapping BOs on GPU page faults
Quoting Rob Herring (2019-06-10 18:04:40) > The midgard/bifrost GPUs need to allocate GPU memory which is allocated > on GPU page faults and not pinned in memory. The vendor driver calls > this functionality GROW_ON_GPF. > > This implementation assumes that BOs allocated with the > PANFROST_BO_NOMAP flag are never mmapped or exported. Both of those may > actually work, but I'm unsure if there's some interaction there. It > would cause the whole object to be pinned in memory which would defeat > the point of this. > > Issues/questions/thoughts: > > What's the difference between i_mapping and f_mapping? > > What kind of clean-up on close is needed? Based on vgem faults, there > doesn't seem to be any refcounting. Assume userspace is responsible for > not freeing the BO while a page fault can occur? See drm_gem_vm_open and drm_gem_vm_close. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #5 from Gert Wollny --- No this doesn't fix the bug, there are other instances where a DIV is introduced. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Tweak drm_encoder_helper_funcs.enable kerneldoc
From: Sean Paul
I copied the kerneldoc for encoder_funcs.atomic_enable from encoder_funcs.enable
in a recent patch [1]. Sam rightly pointed out in the review that "for symmetry
with" text is awkward [2]. So here's a patch to fix up the source of the awkward
language.
[1]
https://patchwork.freedesktop.org/patch/msgid/20190611160844.257498-2-s...@poorly.run
[2]
https://patchwork.freedesktop.org/patch/msgid/20190611185352.ga16...@ravnborg.org
Suggested-by: Sam Ravnborg
Signed-off-by: Sean Paul
---
include/drm/drm_modeset_helper_vtables.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/drm/drm_modeset_helper_vtables.h
b/include/drm/drm_modeset_helper_vtables.h
index f9c94c2a13646..df80131bb10fc 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -719,11 +719,11 @@ struct drm_encoder_helper_funcs {
* hooks and call them from CRTC's callback by looping over all encoders
* connected to it using for_each_encoder_on_crtc().
*
-* This hook is used only by atomic helpers, for symmetry with @disable.
-* Atomic drivers don't need to implement it if there's no need to
-* enable anything at the encoder level. To ensure that runtime PM
handling
-* (using either DPMS or the new "ACTIVE" property) works
-* @enable must be the inverse of @disable for atomic drivers.
+* This hook is only used by atomic helpers, it is the opposite of
+* @disable. Atomic drivers don't need to implement it if there's no
+* need to enable anything at the encoder level. To ensure that
+* runtime PM handling (using either DPMS or the new "ACTIVE" property)
+* works @enable must be the inverse of @disable for atomic drivers.
*/
void (*enable)(struct drm_encoder *encoder);
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/panfrost: Add support for mapping BOs on GPU page faults
On Wed, Jun 12, 2019 at 02:54:56PM +0200, Tomeu Vizoso wrote:
> On Mon, 10 Jun 2019 at 19:06, Rob Herring wrote:
> >
> > The midgard/bifrost GPUs need to allocate GPU memory which is allocated
> > on GPU page faults and not pinned in memory. The vendor driver calls
> > this functionality GROW_ON_GPF.
> >
> > This implementation assumes that BOs allocated with the
> > PANFROST_BO_NOMAP flag are never mmapped or exported. Both of those may
> > actually work, but I'm unsure if there's some interaction there. It
> > would cause the whole object to be pinned in memory which would defeat
> > the point of this.
> >
> > Issues/questions/thoughts:
> >
> > What's the difference between i_mapping and f_mapping?
> >
> > What kind of clean-up on close is needed? Based on vgem faults, there
> > doesn't seem to be any refcounting. Assume userspace is responsible for
> > not freeing the BO while a page fault can occur?
This really shouldn't ever be possible, "rely on userspace to not oops the
kernel" is how dri1 worked :-)
>
> Aren't we taking a reference on all BOs that a job relates to and
> unreferencing them once the job is done? I would think that that's
> enough, or am I missing something?
Yup, this is how this is supposed to work.
-Daniel
>
> > What about evictions? Need to call mapping_set_unevictable()? Maybe we
> > want these pages to be swappable, but then we need some notification to
> > unmap them.
>
> I'm not sure there's much point in swapping out pages with lifetimes
> of a few milliseconds.
>
> > No need for runtime PM, because faults should only occur during job
> > submit?
>
> Makes sense to me.
>
> > Need to fault in 2MB sections when possible. It seems this has to be
> > done by getting an array of struct pages and then converting to a
> > scatterlist. Not sure if there is a better way to do that. There's also
> > some errata on T604 needing to fault in at least 8 pages at a time which
> > isn't handled.
>
> The old GPUs will be the most interesting to support...
>
> Will give this patch some testing tomorrow.
>
> Thanks,
>
> Tomeu
>
> > Cc: Robin Murphy
> > Cc: Steven Price
> > Cc: Alyssa Rosenzweig
> > Cc: Tomeu Vizoso
> > Signed-off-by: Rob Herring
> > ---
> >
> > drivers/gpu/drm/panfrost/TODO | 2 -
> > drivers/gpu/drm/panfrost/panfrost_drv.c | 22 +--
> > drivers/gpu/drm/panfrost/panfrost_gem.c | 3 +-
> > drivers/gpu/drm/panfrost/panfrost_gem.h | 8 +++
> > drivers/gpu/drm/panfrost/panfrost_mmu.c | 76 +++--
> > include/uapi/drm/panfrost_drm.h | 2 +
> > 6 files changed, 100 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/TODO b/drivers/gpu/drm/panfrost/TODO
> > index c2e44add37d8..64129bf73933 100644
> > --- a/drivers/gpu/drm/panfrost/TODO
> > +++ b/drivers/gpu/drm/panfrost/TODO
> > @@ -14,8 +14,6 @@
> >The hard part is handling when more address spaces are needed than what
> >the h/w provides.
> >
> > -- Support pinning pages on demand (GPU page faults).
> > -
> > - Support userspace controlled GPU virtual addresses. Needed for Vulkan.
> > (Tomeu)
> >
> > - Support for madvise and a shrinker.
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index d11e2281dde6..f08d41d5186a 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -44,9 +44,11 @@ static int panfrost_ioctl_create_bo(struct drm_device
> > *dev, void *data,
> > {
> > int ret;
> > struct drm_gem_shmem_object *shmem;
> > + struct panfrost_gem_object *bo;
> > struct drm_panfrost_create_bo *args = data;
> >
> > - if (!args->size || args->flags || args->pad)
> > + if (!args->size || args->pad ||
> > + (args->flags & ~PANFROST_BO_NOMAP))
> > return -EINVAL;
> >
> > shmem = drm_gem_shmem_create_with_handle(file, dev, args->size,
> > @@ -54,11 +56,16 @@ static int panfrost_ioctl_create_bo(struct drm_device
> > *dev, void *data,
> > if (IS_ERR(shmem))
> > return PTR_ERR(shmem);
> >
> > - ret = panfrost_mmu_map(to_panfrost_bo(>base));
> > - if (ret)
> > - goto err_free;
> > + bo = to_panfrost_bo(>base);
> >
> > - args->offset = to_panfrost_bo(>base)->node.start <<
> > PAGE_SHIFT;
> > + if (!(args->flags & PANFROST_BO_NOMAP)) {
> > + ret = panfrost_mmu_map(bo);
> > + if (ret)
> > + goto err_free;
> > + } else
> > + bo->no_map = 1;
> > +
> > + args->offset = bo->node.start << PAGE_SHIFT;
> >
> > return 0;
> >
> > @@ -269,6 +276,11 @@ static int panfrost_ioctl_mmap_bo(struct drm_device
> > *dev, void *data,
> > return -ENOENT;
> > }
> >
> > + if (to_panfrost_bo(gem_obj)->no_map) {
> > + DRM_DEBUG("Can't mmap 'no_map' GEM BO %d\n", args->handle);
> > +
Re: [PATCH v2 6/7] dt-bindings: Add ANX6345 DP/eDP transmitter binding
On Wed, Jun 12, 2019 at 10:16:37AM +0200, Andrzej Hajda wrote: > > +The ANX6345 is an ultra-low power Full-HD eDP transmitter designed for > > +portable devices. > > + > > +Required properties: > > + > > + - compatible : "analogix,anx6345" > > + - reg : I2C address of the device > > + - reset-gpios : Which GPIO to use for reset > > > You have not specified it's active state, since in driver's code you > named it RESETN I guess it should be active low. Yes. The chip's reset is active low. > > > + - dvdd12-supply : Regulator for 1.2V digital core power. > > + - dvdd25-supply : Regulator for 2.5V digital core power. > > + - Video port for LVTTL input, using the DT bindings defined in [1]. > > > Please assign port number for input (I guess 0). True. > > > + > > +Optional properties: > > + > > + - Video port for eDP output (panel or connector) using the DT bindings > > + defined in [1]. > > > Shouldn't it be also required? See previous discussion. Surely there should be _something_ connected to the output side, but that something might not be relevant for the software side, so it might be omitted from the device tree. In fact, I'll submit v3 with the SPDX changes and without exactly this output port spec which had caused the heated discussion. Torsten
Re: [PATCH 02/10] drm/vkms: Use spin_lock_irq in process context
On Wed, Jun 12, 2019 at 10:34:55AM -0300, Rodrigo Siqueira wrote:
> On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
> >
> > The worker is always in process context, no need for the _irqsafe
> > version. Same for the set_source callback, that's only called from the
> > debugfs handler in a syscall.
> >
> > Cc: Shayenne Moura
> > Cc: Rodrigo Siqueira
> > Signed-off-by: Daniel Vetter
> > Cc: Haneen Mohammed
> > Cc: Daniel Vetter
> > ---
> > drivers/gpu/drm/vkms/vkms_crc.c | 10 --
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_crc.c
> > b/drivers/gpu/drm/vkms/vkms_crc.c
> > index 66603da634fe..883e36fe7b6e 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> > @@ -167,16 +167,15 @@ void vkms_crc_work_handle(struct work_struct *work)
> > u32 crc32 = 0;
> > u64 frame_start, frame_end;
> > bool crc_pending;
> > - unsigned long flags;
> >
> > - spin_lock_irqsave(>state_lock, flags);
> > + spin_lock_irq(>state_lock);
> > frame_start = crtc_state->frame_start;
> > frame_end = crtc_state->frame_end;
> > crc_pending = crtc_state->crc_pending;
> > crtc_state->frame_start = 0;
> > crtc_state->frame_end = 0;
> > crtc_state->crc_pending = false;
> > - spin_unlock_irqrestore(>state_lock, flags);
> > + spin_unlock_irq(>state_lock);
> >
> > /*
> > * We raced with the vblank hrtimer and previous work already
> > computed
> > @@ -246,7 +245,6 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const
> > char *src_name)
> > {
> > struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> > bool enabled = false;
> > - unsigned long flags;
> > int ret = 0;
> >
> > ret = vkms_crc_parse_source(src_name, );
> > @@ -254,9 +252,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const
> > char *src_name)
> > /* make sure nothing is scheduled on crtc workq */
> > flush_workqueue(out->crc_workq);
> >
> > - spin_lock_irqsave(>lock, flags);
> > + spin_lock_irq(>lock);
> > out->crc_enabled = enabled;
> > - spin_unlock_irqrestore(>lock, flags);
>
> I was wondering if we could use atomic_t for crc_enabled and avoid
> this sort of lock. I did not try to change the data type; this is just
> an idea.
tldr; atomic_t does not do what you think it does. rule of thumb is you
can use it for reference counting and statistics book-keeping, but nothing
else.
The long explanation is that atomic_t in the linux kernel does not have
barriers (unlike atomic values in almost all other language), they are
weakly ordered. If you want to use them for logic (like here with this
bool) you need to think very carefully about barriers, document those
barriers, proof you got it all right, all for maybe a tiny speed-up
(spinlocks are extremely well optimized). In almost all cases that's not
worth it, and fairly often you end up with more atomic operations and so
overall slower code.
btw for this reasons atomic_t is wrapped as refcount_t for those cases
where it's safe to use (plus the code is even more optimized for the
refcount use-case). Except for statistics (like how many crc did we
compute) you shouldn't ever use atomic_t, at least as a good rule of
thumb.
-Daniel
>
> Reviewed-by: Rodrigo Siqueira
> Tested-by: Rodrigo Siqueira
>
> > + spin_unlock_irq(>lock);
> >
> > return ret;
> > }
> > --
> > 2.20.1
> >
>
>
> --
>
> Rodrigo Siqueira
> https://siqueira.tech
--
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 v6] drm: Add helpers to kick off self refresh mode in drivers
From: Sean Paul This patch adds a new drm helper library to help drivers implement self refresh. Drivers choosing to use it will register crtcs and will receive callbacks when it's time to enter or exit self refresh mode. In its current form, it has a timer which will trigger after a driver-specified amount of inactivity. When the timer triggers, the helpers will submit a new atomic commit to shut the refreshing pipe off. On the next atomic commit, the drm core will revert the self refresh state and bring everything back up to be actively driven. From the driver's perspective, this works like a regular disable/enable cycle. The driver need only check the 'self_refresh_active' state in crtc_state. It should initiate self refresh mode on the panel and enter an off or low-power state. Changes in v2: - s/psr/self_refresh/ (Daniel) - integrated the psr exit into the commit that wakes it up (Jose/Daniel) - made the psr state per-crtc (Jose/Daniel) Changes in v3: - Remove the self_refresh_(active|changed) from connector state (Daniel) - Simplify loop in drm_self_refresh_helper_alter_state (Daniel) - Improve self_refresh_aware comment (Daniel) - s/self_refresh_state/self_refresh_data/ (Daniel) Changes in v4: - Move docbook location below panel (Daniel) - Improve docbook with references and more detailed explanation (Daniel) - Instead of register/unregister, use init/cleanup (Daniel) Changes in v5: - Resolved conflict in drm_atomic_helper.c #include block - Resolved conflict in rst with HDCP helper docs Changes in v6: - Fix include ordering, clean up forward declarations (Sam) Link to v1: https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-2-s...@poorly.run Link to v2: https://patchwork.freedesktop.org/patch/msgid/20190326204509.96515-1-s...@poorly.run Link to v3: https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-6-s...@poorly.run Link to v4: https://patchwork.freedesktop.org/patch/msgid/20190508160920.144739-6-s...@poorly.run Link to v5: https://patchwork.freedesktop.org/patch/msgid/20190611160844.257498-6-s...@poorly.run Cc: Daniel Vetter Cc: Jose Souza Cc: Zain Wang Cc: Tomasz Figa Cc: Ville Syrjälä Cc: Sam Ravnborg Tested-by: Heiko Stuebner Reviewed-by: Daniel Vetter Signed-off-by: Sean Paul --- Documentation/gpu/drm-kms-helpers.rst | 9 + drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_atomic.c | 2 + drivers/gpu/drm/drm_atomic_helper.c | 35 +++- drivers/gpu/drm/drm_atomic_state_helper.c | 4 + drivers/gpu/drm/drm_atomic_uapi.c | 7 +- drivers/gpu/drm/drm_self_refresh_helper.c | 216 ++ include/drm/drm_atomic.h | 15 ++ include/drm/drm_connector.h | 14 ++ include/drm/drm_crtc.h| 19 ++ include/drm/drm_self_refresh_helper.h | 20 ++ 11 files changed, 338 insertions(+), 5 deletions(-) create mode 100644 drivers/gpu/drm/drm_self_refresh_helper.c create mode 100644 include/drm/drm_self_refresh_helper.h diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 0fe726a6ee678..b327bbc111821 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -181,6 +181,15 @@ Panel Helper Reference .. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c :export: +Panel Self Refresh Helper Reference +=== + +.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c + :doc: overview + +.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c + :export: + HDCP Helper Functions Reference === diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index d36feb4a62330..9d630a28a7880 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -43,7 +43,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o drm_probe_helper drm_simple_kms_helper.o drm_modeset_helper.o \ drm_scdc_helper.o drm_gem_framebuffer_helper.o \ drm_atomic_state_helper.o drm_damage_helper.o \ - drm_format_helper.o + drm_format_helper.o drm_self_refresh_helper.o drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index a4e779deab0fb..419381abbdd16 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -384,6 +384,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p, drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name); drm_printf(p, "\tenable=%d\n", state->enable); drm_printf(p, "\tactive=%d\n", state->active); + drm_printf(p, "\tself_refresh_active=%d\n", state->self_refresh_active); drm_printf(p, "\tplanes_changed=%d\n", state->planes_changed);
Re: [PATCH 01/10] drm/vkms: Fix crc worker races
On Wed, Jun 12, 2019 at 10:33:11AM -0300, Rodrigo Siqueira wrote:
> On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
> >
> > The issue we have is that the crc worker might fall behind. We've
> > tried to handle this by tracking both the earliest frame for which it
> > still needs to compute a crc, and the last one. Plus when the
> > crtc_state changes, we have a new work item, which are all run in
> > order due to the ordered workqueue we allocate for each vkms crtc.
> >
> > Trouble is there's been a few small issues in the current code:
> > - we need to capture frame_end in the vblank hrtimer, not in the
> > worker. The worker might run much later, and then we generate a lot
> > of crc for which there's already a different worker queued up.
> > - frame number might be 0, so create a new crc_pending boolean to
> > track this without confusion.
> > - we need to atomically grab frame_start/end and clear it, so do that
> > all in one go. This is not going to create a new race, because if we
> > race with the hrtimer then our work will be re-run.
> > - only race that can happen is the following:
> > 1. worker starts
> > 2. hrtimer runs and updates frame_end
> > 3. worker grabs frame_start/end, already reading the new frame_end,
> > and clears crc_pending
> > 4. hrtimer calls queue_work()
> > 5. worker completes
> > 6. worker gets re-run, crc_pending is false
> > Explain this case a bit better by rewording the comment.
> >
> > v2: Demote warning level output to debug when we fail to requeue, this
> > is expected under high load when the crc worker can't quite keep up.
> >
> > Cc: Shayenne Moura
> > Cc: Rodrigo Siqueira
> > Signed-off-by: Daniel Vetter
> > Cc: Haneen Mohammed
> > Cc: Daniel Vetter
> > ---
> > drivers/gpu/drm/vkms/vkms_crc.c | 27 +--
> > drivers/gpu/drm/vkms/vkms_crtc.c | 9 +++--
> > drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
> > 3 files changed, 22 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_crc.c
> > b/drivers/gpu/drm/vkms/vkms_crc.c
> > index d7b409a3c0f8..66603da634fe 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> > @@ -166,16 +166,24 @@ void vkms_crc_work_handle(struct work_struct *work)
> > struct drm_plane *plane;
> > u32 crc32 = 0;
> > u64 frame_start, frame_end;
> > + bool crc_pending;
> > unsigned long flags;
> >
> > spin_lock_irqsave(>state_lock, flags);
> > frame_start = crtc_state->frame_start;
> > frame_end = crtc_state->frame_end;
> > + crc_pending = crtc_state->crc_pending;
> > + crtc_state->frame_start = 0;
> > + crtc_state->frame_end = 0;
> > + crtc_state->crc_pending = false;
> > spin_unlock_irqrestore(>state_lock, flags);
> >
> > - /* _vblank_handle() hasn't updated frame_start yet */
> > - if (!frame_start || frame_start == frame_end)
> > - goto out;
> > + /*
> > +* We raced with the vblank hrtimer and previous work already
> > computed
> > +* the crc, nothing to do.
> > +*/
> > + if (!crc_pending)
> > + return;
>
> I think this condition is not reachable because crc_pending will be
> filled with true in `vkms_vblank_simulate()` which in turn schedule
> the function `vkms_crc_work_handle()`. Just for checking, I tried to
> reach this condition by running kms_flip, kms_pipe_crc_basic, and
> kms_cursor_crc with two different VM setups[1], but I couldn't reach
> it. What do you think?
thread Athread B
1. run vblank hrtimer
2. starts running crc work (from previous
vblank)
3. spin_lock() -> gets stalled on the spin_lock() because
thread A has it already
4. update frame_end (only in
later patches, atm this is
impossible). crc_pending is set
already.
5. schedule_work: since the work
is running already, this means it
is scheduled to run once more.
6. spin_unlock
7. compute crc, clear crc_pending
8. work finishes
9. work gets run again
8. crc_pending=false
Since the spin_lock critical section is _very_ short (less than 1 usec I
bet), this race is very hard to hit.
Exercise: Figure out why schedule_work _must_ schedule the work item to
re-run if it's running already. If it doesn't do that there's another
race.
>
> [1] Qemu parameters
> VM1: -m 1G -smp cores=2,cpus=2
> VM2: -enable-kvm -m 2G -smp cores=4,cpus=4
>
> > drm_for_each_plane(plane, >drm) {
> > struct vkms_plane_state *vplane_state;
> > @@ -196,20 +204,11 @@ void vkms_crc_work_handle(struct work_struct *work)
> > if (primary_crc)
> > crc32 = _vkms_get_crc(primary_crc, cursor_crc);
> >
> >
Re: [PATCH 3/3] media: vsp1: drm: Remove vsp1_du_setup_lif()
Hi Kieran,
Thank you for the patch.
On Fri, May 17, 2019 at 11:31:43PM +0100, Kieran Bingham wrote:
> The vsp1_du_setup_lif() function is deprecated, and the users have been
> removed. Remove the implementation and the associated configuration
> structure.
>
> Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/vsp1/vsp1_drm.c | 47 --
> include/media/vsp1.h | 22
> 2 files changed, 69 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index ce5c0780680f..12f76344bdec 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -817,53 +817,6 @@ int vsp1_du_atomic_disable(struct device *dev, unsigned
> int pipe_index)
> }
> EXPORT_SYMBOL_GPL(vsp1_du_atomic_disable);
>
> -/**
> - * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
> - * @dev: the VSP device
> - * @pipe_index: the DRM pipeline index
> - * @cfg: the LIF configuration
> - *
> - * Configure the output part of VSP DRM pipeline for the given frame
> @cfg.width
> - * and @cfg.height. This sets up formats on the BRx source pad, the WPF sink
> and
> - * source pads, and the LIF sink pad.
> - *
> - * The @pipe_index argument selects which DRM pipeline to setup. The number
> of
> - * available pipelines depend on the VSP instance.
> - *
> - * As the media bus code on the blend unit source pad is conditioned by the
> - * configuration of its sink 0 pad, we also set up the formats on all blend
> unit
> - * sinks, even if the configuration will be overwritten later by
> - * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is
> set to
> - * a well defined state.
> - *
> - * Return 0 on success or a negative error code on failure.
> - */
> -int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> - const struct vsp1_du_lif_config *cfg)
> -{
> - int ret;
> -
> - struct vsp1_du_modeset_config modes = {
> - .width = cfg->width,
> - .height = cfg->height,
> - .interlaced = cfg->interlaced,
> - };
> - struct vsp1_du_enable_config enable = {
> - .callback = cfg->callback,
> - .callback_data = cfg->callback_data,
> - };
> -
> - if (!cfg)
> - return vsp1_du_atomic_disable(dev, pipe_index);
> -
> - ret = vsp1_du_atomic_modeset(dev, pipe_index, );
> - if (ret)
> - return ret;
> -
> - return vsp1_du_atomic_enable(dev, pipe_index, );
> -}
> -EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
> -
> /**
> * vsp1_du_atomic_begin - Prepare for an atomic update
> * @dev: the VSP device
> diff --git a/include/media/vsp1.h b/include/media/vsp1.h
> index 13f5a1c4d45a..bc0a26d33d9a 100644
> --- a/include/media/vsp1.h
> +++ b/include/media/vsp1.h
> @@ -20,28 +20,6 @@ int vsp1_du_init(struct device *dev);
> #define VSP1_DU_STATUS_COMPLETE BIT(0)
> #define VSP1_DU_STATUS_WRITEBACK BIT(1)
>
> -/**
> - * struct vsp1_du_lif_config - VSP LIF configuration - Deprecated
> - * @width: output frame width
> - * @height: output frame height
> - * @interlaced: true for interlaced pipelines
> - * @callback: frame completion callback function (optional). When a callback
> - * is provided, the VSP driver guarantees that it will be called once
> - * and only once for each vsp1_du_atomic_flush() call.
> - * @callback_data: data to be passed to the frame completion callback
> - */
> -struct vsp1_du_lif_config {
> - unsigned int width;
> - unsigned int height;
> - bool interlaced;
> -
> - void (*callback)(void *data, unsigned int status, u32 crc);
> - void *callback_data;
> -};
> -
> -int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> - const struct vsp1_du_lif_config *cfg);
> -
> /**
> * struct vsp1_du_modeset_config - VSP LIF Mode configuration
> * @width: output frame width
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm: rcar-du: Convert to the new VSP atomic API
Hi Kieran,
Thank you for the patch.
On Fri, May 17, 2019 at 11:31:42PM +0100, Kieran Bingham wrote:
> The configuration API between the VSP and the DU has been updated to
> provide finer grain control over modesetting, and enablement.
>
> Split rcar_du_vsp_enable() into rcar_du_vsp_modeset() and
> rcar_du_vsp_enable() accordingly, and update each function to use the
> new VSP API.
>
> There are no further users of the deprecated vsp1_du_setup_lif() which
> can now be removed.
>
> Signed-off-by: Kieran Bingham
> ---
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 4 +++-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 21 +++--
> drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 2 ++
> 3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 2da46e3dc4ae..cccd6fe85749 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -492,8 +492,10 @@ static void rcar_du_crtc_setup(struct rcar_du_crtc
> *rcrtc)
> rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
>
> /* Enable the VSP compositor. */
> - if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
> + if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) {
> + rcar_du_vsp_modeset(rcrtc);
> rcar_du_vsp_enable(rcrtc);
> + }
>
> /* Turn vertical blanking interrupt reporting on. */
> drm_crtc_vblank_on(>crtc);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index 5e4faf258c31..c170427fcad9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -44,16 +44,14 @@ static void rcar_du_vsp_complete(void *private, unsigned
> int status, u32 crc)
> drm_crtc_add_crc_entry(>crtc, false, 0, );
> }
>
> -void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
> +void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc)
> {
> const struct drm_display_mode *mode = >crtc.state->adjusted_mode;
> struct rcar_du_device *rcdu = crtc->dev;
> - struct vsp1_du_lif_config cfg = {
> + struct vsp1_du_modeset_config cfg = {
> .width = mode->hdisplay,
> .height = mode->vdisplay,
> .interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE,
> - .callback = rcar_du_vsp_complete,
> - .callback_data = crtc,
> };
> struct rcar_du_plane_state state = {
> .state = {
> @@ -90,12 +88,23 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
>*/
> crtc->group->need_restart = true;
>
> - vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, );
> +
Extra blank line.
Apart from that,
Reviewed-by: Laurent Pinchart
> + vsp1_du_atomic_modeset(crtc->vsp->vsp, crtc->vsp_pipe, );
> +}
> +
> +void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
> +{
> + struct vsp1_du_enable_config cfg = {
> + .callback = rcar_du_vsp_complete,
> + .callback_data = crtc,
> + };
> +
> + vsp1_du_atomic_enable(crtc->vsp->vsp, crtc->vsp_pipe, );
> }
>
> void rcar_du_vsp_disable(struct rcar_du_crtc *crtc)
> {
> - vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, NULL);
> + vsp1_du_atomic_disable(crtc->vsp->vsp, crtc->vsp_pipe);
> }
>
> void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> index 9b4724159378..a6f6bb4690f2 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> @@ -58,6 +58,7 @@ to_rcar_vsp_plane_state(struct drm_plane_state *state)
> #ifdef CONFIG_DRM_RCAR_VSP
> int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
>unsigned int crtcs);
> +void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc);
> void rcar_du_vsp_enable(struct rcar_du_crtc *crtc);
> void rcar_du_vsp_disable(struct rcar_du_crtc *crtc);
> void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
> @@ -73,6 +74,7 @@ static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp,
> {
> return -ENXIO;
> }
> +static inlinc void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc) { };
> static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
> static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
> static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };
--
Regards,
Laurent Pinchart
Re: [RESEND PATCH v3 2/3] dma-buf: add DMA_BUF_{GET,SET}_NAME ioctls
Hello Chenbo,
Thanks very much for your patches. Other than a couple tiny nits
below, I think these look good, and I will merge them before the end
of this week.
On Tue, 11 Jun 2019 at 05:32, Chenbo Feng wrote:
>
> From: Greg Hackmann
>
> This patch adds complimentary DMA_BUF_SET_NAME and DMA_BUF_GET_NAME
> ioctls, which lets userspace processes attach a free-form name to each
> buffer.
This should remove the _GET_NAME bit since it's not there anymore.
>
> This information can be extremely helpful for tracking and accounting
> shared buffers. For example, on Android, we know what each buffer will
> be used for at allocation time: GL, multimedia, camera, etc. The
> userspace allocator can use DMA_BUF_SET_NAME to associate that
> information with the buffer, so we can later give developers a
> breakdown of how much memory they're allocating for graphics, camera,
> etc.
>
> Signed-off-by: Greg Hackmann
> Signed-off-by: Chenbo Feng
> ---
> drivers/dma-buf/dma-buf.c| 49 +---
> include/linux/dma-buf.h | 5 +++-
> include/uapi/linux/dma-buf.h | 3 +++
> 3 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index ffd5a2ad7d6f..c1da5f9ce44d 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -48,8 +48,24 @@ struct dma_buf_list {
>
> static struct dma_buf_list db_list;
>
> +static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen)
> +{
> + struct dma_buf *dmabuf;
> + char name[DMA_BUF_NAME_LEN];
> + size_t ret = 0;
> +
> + dmabuf = dentry->d_fsdata;
> + mutex_lock(>lock);
> + if (dmabuf->name)
> + ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN);
> + mutex_unlock(>lock);
> +
> + return dynamic_dname(dentry, buffer, buflen, "/%s:%s",
> +dentry->d_name.name, ret > 0 ? name : "");
> +}
> +
> static const struct dentry_operations dma_buf_dentry_ops = {
> - .d_dname = simple_dname,
> + .d_dname = dmabuffs_dname,
> };
>
> static struct vfsmount *dma_buf_mnt;
> @@ -297,6 +313,27 @@ static __poll_t dma_buf_poll(struct file *file,
> poll_table *poll)
> return events;
> }
>
> +static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> +{
> + char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> + long ret = 0;
> +
> + if (IS_ERR(name))
> + return PTR_ERR(name);
> +
> + mutex_lock(>lock);
> + if (!list_empty(>attachments)) {
> + ret = -EBUSY;
> + goto out_unlock;
> + }
We might also want to document this better - that name change for a
buffer is still allowed if it doesn't have any attached devices after
its usage is done but before it is destroyed? (theoritically it could
be reused with a different name?)
> + kfree(dmabuf->name);
> + dmabuf->name = name;
> +
> +out_unlock:
> + mutex_unlock(>lock);
> + return ret;
> +}
> +
> static long dma_buf_ioctl(struct file *file,
> unsigned int cmd, unsigned long arg)
> {
> @@ -335,6 +372,10 @@ static long dma_buf_ioctl(struct file *file,
> ret = dma_buf_begin_cpu_access(dmabuf, direction);
>
> return ret;
> +
> + case DMA_BUF_SET_NAME:
> + return dma_buf_set_name(dmabuf, (const char __user *)arg);
> +
> default:
> return -ENOTTY;
> }
> @@ -376,6 +417,7 @@ static struct file *dma_buf_getfile(struct dma_buf
> *dmabuf, int flags)
> goto err_alloc_file;
> file->f_flags = flags & (O_ACCMODE | O_NONBLOCK);
> file->private_data = dmabuf;
> + file->f_path.dentry->d_fsdata = dmabuf;
>
> return file;
>
> @@ -1082,12 +1124,13 @@ static int dma_buf_debug_show(struct seq_file *s,
> void *unused)
> continue;
> }
>
> - seq_printf(s, "%08zu\t%08x\t%08x\t%08ld\t%s\t%08lu\n",
> + seq_printf(s, "%08zu\t%08x\t%08x\t%08ld\t%s\t%08lu\t%s\n",
> buf_obj->size,
> buf_obj->file->f_flags, buf_obj->file->f_mode,
> file_count(buf_obj->file),
> buf_obj->exp_name,
> - file_inode(buf_obj->file)->i_ino);
> + file_inode(buf_obj->file)->i_ino,
> + buf_obj->name ?: "");
>
> robj = buf_obj->resv;
> while (true) {
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 58725f890b5b..582998e19df6 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -255,10 +255,12 @@ struct dma_buf_ops {
> * @file: file pointer used for sharing buffers across, and for refcounting.
> * @attachments: list of
Re: [PATCH 1/3] media: vsp1: drm: Split vsp1_du_setup_lif()
Hi Kieran,
Thank you for the patch.
On Fri, May 17, 2019 at 11:31:41PM +0100, Kieran Bingham wrote:
> Break vsp1_du_setup_lif() into components more suited to the DRM Atomic
> API. The existing vsp1_du_setup_lif() API call is maintained as it is
> still used from the DU.
>
> Signed-off-by: Kieran Bingham
> ---
> drivers/media/platform/vsp1/vsp1_drm.c | 233 ++---
> include/media/vsp1.h | 32 +++-
> 2 files changed, 199 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index a4a45d68a6ef..ce5c0780680f 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -616,18 +616,15 @@ int vsp1_du_init(struct device *dev)
> EXPORT_SYMBOL_GPL(vsp1_du_init);
>
> /**
> - * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
> + * vsp1_du_atomic_modeset - Configure the mode as part of an atomic update
> * @dev: the VSP device
> * @pipe_index: the DRM pipeline index
> - * @cfg: the LIF configuration
> + * @cfg: the mode configuration
> *
> * Configure the output part of VSP DRM pipeline for the given frame
> @cfg.width
> * and @cfg.height. This sets up formats on the BRx source pad, the WPF sink
> and
> * source pads, and the LIF sink pad.
> *
> - * The @pipe_index argument selects which DRM pipeline to setup. The number
> of
> - * available pipelines depend on the VSP instance.
> - *
Shouldn't this paragraph be preserved, as the function keeps its
pipe_index argument ?
> * As the media bus code on the blend unit source pad is conditioned by the
> * configuration of its sink 0 pad, we also set up the formats on all blend
> unit
> * sinks, even if the configuration will be overwritten later by
> @@ -635,15 +632,14 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
> * a well defined state.
> *
> * Return 0 on success or a negative error code on failure.
> + *
Not needed.
> */
> -int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> - const struct vsp1_du_lif_config *cfg)
> +int vsp1_du_atomic_modeset(struct device *dev, unsigned int pipe_index,
> +const struct vsp1_du_modeset_config *cfg)
> {
> struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> struct vsp1_drm_pipeline *drm_pipe;
> struct vsp1_pipeline *pipe;
> - unsigned long flags;
> - unsigned int i;
> int ret;
>
> if (pipe_index >= vsp1->info->lif_count)
> @@ -652,60 +648,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int
> pipe_index,
> drm_pipe = >drm->pipe[pipe_index];
> pipe = _pipe->pipe;
>
> - if (!cfg) {
> - struct vsp1_brx *brx;
> -
> - mutex_lock(>drm->lock);
> -
> - brx = to_brx(>brx->subdev);
> -
> - /*
> - * NULL configuration means the CRTC is being disabled, stop
> - * the pipeline and turn the light off.
> - */
> - ret = vsp1_pipeline_stop(pipe);
> - if (ret == -ETIMEDOUT)
> - dev_err(vsp1->dev, "DRM pipeline stop timeout\n");
> -
> - for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) {
> - struct vsp1_rwpf *rpf = pipe->inputs[i];
> -
> - if (!rpf)
> - continue;
> -
> - /*
> - * Remove the RPF from the pipe and the list of BRx
> - * inputs.
> - */
> - WARN_ON(!rpf->entity.pipe);
> - rpf->entity.pipe = NULL;
> - list_del(>entity.list_pipe);
> - pipe->inputs[i] = NULL;
> -
> - brx->inputs[rpf->brx_input].rpf = NULL;
> - }
> -
> - drm_pipe->du_complete = NULL;
> - pipe->num_inputs = 0;
> -
> - dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
> - __func__, pipe->lif->index,
> - BRX_NAME(pipe->brx));
> -
> - list_del(>brx->list_pipe);
> - pipe->brx->pipe = NULL;
> - pipe->brx = NULL;
> -
> - mutex_unlock(>drm->lock);
> -
> - vsp1_dlm_reset(pipe->output->dlm);
> - vsp1_device_put(vsp1);
> -
> - dev_dbg(vsp1->dev, "%s: pipeline disabled\n", __func__);
> -
> - return 0;
> - }
> -
> drm_pipe->width = cfg->width;
> drm_pipe->height = cfg->height;
> pipe->interlaced = cfg->interlaced;
> @@ -722,8 +664,43 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int
> pipe_index,
> goto unlock;
>
> ret = vsp1_du_pipeline_setup_output(vsp1, pipe);
> - if (ret < 0)
> - goto unlock;
> +
> +unlock:
> + mutex_unlock(>drm->lock);
> +
> + return ret;
> +}
> +
> +/**
> + * vsp1_du_atomic_enable - Enable and start
Re: [PATCH 00/10] drm/vkms: rework crc worker
On Wed, Jun 12, 2019 at 10:28:41AM -0300, Rodrigo Siqueira wrote: > Hi Daniel, > > First of all, thank you very much for your patchset. > > I tried to make a detailed review of your series, and you can see my > comments in each patch. You’ll notice that I asked many things related > to the DRM subsystem with the hope that I could learn a little bit > more about DRM from your comments. > > Before you go through the review, I would like to start a discussion > about the vkms race conditions. First, I have to admit that I did not > understand the race conditions that you described before because I > couldn’t reproduce them. Now, I'm suspecting that I could not > experience the problem because I'm using QEMU with KVM; with this idea > in mind, I suppose that we have two scenarios for using vkms in a > virtual machine: > > * Scenario 1: The user has hardware virtualization support; in this > case, it is a little bit harder to experience race conditions with > vkms. > > * Scenario 2: Without hardware virtualization support, it is much > easier to experience race conditions. > > With these two scenarios in mind, I conducted a bunch of experiments > for trying to shed light on this issue. I did: > > 1. Enabled lockdep > > 2. Defined two different environments for testing both using QEMU with > and without kvm. See below the QEMU hardware options: > > * env_kvm: -enable-kvm -daemonize -m 1G -smp cores=2,cpus=2 > * env_no_kvm: -daemonize -m 2G -smp cores=4,cpus=4 > > 3. My test protocol: I) turn on the vm, II) clean /proc/lock_stat, > III) execute kms_cursor_crc, III) save the lock_stat file, and IV) > turn off the vm. > > 4. From the lockdep_stat, I just highlighted the row related to vkms > and the columns holdtime-total and holdtime-avg > > I would like to highlight that the following data does not have any > statistical approach, and its intention is solely to assist our > discussion. See below the summary of the collected data: > > Summary of the experiment results: > > ++++ > || env_kvm| env_no_kvm | > ++++ > | Test | Before | After | Before | After | > +++---++---+ > | kms_cursor_crc | S1 | S2 | M1 | M2 | > +++---++---+ > > * Before: before apply this patchset > * After: after apply this patchset > > -+--+--- > S1: Without this patchset and with kvm | holdtime-total | holdtime-avg > -++- > &(_out->lock)->rlock: | 21983.52 | 6.21 > (work_completion)(_state->crc_wo: | 20.47 | 20.47 > (wq_completion)vkms_crc_workq: | 3999507.87| 3352.48 > &(_out->state_lock)->rlock: | 378.47| 0.30 > (work_completion)(_state->crc_#2: | 3999066.30| 2848.34 > -++- > S2: With this patchset and with kvm || > -++- > &(_out->lock)->rlock: | 23262.83 | 6.34 > (work_completion)(_state->crc_wo: | 8.98 | 8.98 > &(_out->crc_lock)->rlock: | 307.28| 0.32 > (wq_completion)vkms_crc_workq: | 6567727.05| 12345.35 > (work_completion)(_state->crc_#2: | 6567135.15| 4488.81 > -++- > M1: Without this patchset and without kvm|| > -++- > &(_out->state_lock)->rlock: | 4994.72 | 1.61 > &(_out->lock)->rlock: | 247190.04 | 39.39 > (work_completion)(_state->crc_wo: | 31.32 | 31.32 > (wq_completion)vkms_crc_workq: | 20991073.78 | 13525.18 > (work_completion)(_state->crc_#2: | 20988347.18 | 11904.90 > -++- > M2: With this patchset and without kvm || > -++- > (wq_completion)vkms_crc_workq: | 42819161.68 | 36597.57 > &(_out->lock)->rlock: | 251257.06 | 35.80 > (work_completion)(_state->crc_wo: | 69.37 | 69.37 > &(_out->crc_lock)->rlock: | 3620.92 | 1.54 > (work_completion)(_state->crc_#2: | 42803419.59 | 24306.31 > > First, I analyzed the scenarios with KVM (S1 and S2); more > specifically, I focused on these two classes: > > 1. (work_completion)(_state->crc_wo > 2. (work_completion)(_state->crc_#2 > > After taking a look at the data, it looks like that this patchset > greatly reduces the hold time average for crc_wo. On the other hand, > it
Re: [PATCH 6/8] drm/mgag200: Rewrite cursor handling
On Wed, Jun 12, 2019 at 09:27:21AM +0200, Thomas Zimmermann wrote: > Hi > > Am 11.06.19 um 17:33 schrieb Daniel Vetter: > > On Tue, Jun 11, 2019 at 2:32 PM Thomas Zimmermann > > wrote: > >> > >> Hi > >> > >> Am 05.06.19 um 11:58 schrieb Gerd Hoffmann: > >>> On Tue, Jun 04, 2019 at 05:41:59PM +0200, Thomas Zimmermann wrote: > The cursor handling in mgag200 is complicated to understand. It touches a > number of different BOs, but doesn't really use all of them. > > Rewriting the cursor update reduces the amount of cursor state. There are > two BOs for double-buffered HW updates. The source BO updates the one > that > is currently not displayed and then switches buffers. Explicit BO locking > has been removed from the code. BOs are simply pinned and unpinned in > video > RAM. > >>> > >>> Cursors are not that big after all, so maybe pin the two BOs for > >>> double-buffering permanently in vram to simplify things further? > >>> > >>> Also factoring out the code which updates the two BOs to a separate > >>> function should help making the code more readable. > >> > >> The cursor handling in the ast driver is similar, but uses one single BO > >> to hold both cursor buffers. I'm thinking about how to turn both > >> implementations into a generic helper for legacy cursors (i.e., low > >> number of colors or palette). This would also be helpful for my work on > >> fbdev porting. > >> > >> One idea is to adapt deferred I/O. DRM would expose an ARGB shadow > >> buffer to userspace and let the mmap implementation update the HW > >> buffers (including dithering, palette setup, etc.). > > > > No mmap games needed with kms, we expect userspace to give us an ioctl > > call in all cases. fbdev is the legacy horrors that works differently. > > Thanks for clarifying this. Conversion should be much easier this way. I > saw the dirty callback and the DIRTYFB ioctl, but I don't saw anything > in Weston that calls it. So I assumed that it's obsolete or optional. It's not optional nor obsolete, but you only need to call dirtyfb if you do frontbuffer rendering. Which weston doesnt do. As long as you pageflip, the pageflip will make sure the buffer contents gets updated. -Daniel > > Best regards > Thomas > > > So for cursors, assuming you have universal cursors, you just need to > > update the shadowed copy in the prepare_fb plane hook. For > > non-universal plane drivers you need to do that somewhere in your > > set/move_cursor hooks (I think both of them). Aside: For non-cursors > > there's also the dirtyfb ioctl, which serves the same purpose. > > > > Cheers, Daniel > > > >> > >> Best regards > >> Thomas > >> > >>> But even as-is the patch is a step into the right direction. > >>> > >>> Acked-by: Gerd Hoffmann > >>> > >>> cheers, > >>> Gerd > >>> > >> > >> -- > >> Thomas Zimmermann > >> Graphics Driver Developer > >> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany > >> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah > >> HRB 21284 (AG Nürnberg) > >> > > > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah > HRB 21284 (AG Nürnberg) > -- 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: ✗ Fi.CI.SPARSE: warning for dma-fence/reservation: Markup rcu protected access for DEBUG_MUTEXES
Quoting Patchwork (2019-06-12 15:07:50) > == Series Details == > > Series: dma-fence/reservation: Markup rcu protected access for DEBUG_MUTEXES > URL : https://patchwork.freedesktop.org/series/61963/ > State : warning > > == Summary == > > $ dim sparse origin/drm-tip > Sparse version: v0.5.2 > Commit: dma-fence/reservation: Markup rcu protected access for DEBUG_MUTEXES > -./include/linux/reservation.h:220:20: warning: dereference of noderef > expression [snip] > -./include/linux/reservation.h:220:45: warning: dereference of noderef > expression > -./include/linux/reservation That'll cheer up some of the warnings CI periodically sends. Thanks for the quick review, -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #4 from Gert Wollny --- @AngryPenguin A closer look shows that the bicubic filter in gallium/auxiliar/vl issues TGSI code that contains a DIV operation. Could you try this tree: https://gitlab.freedesktop.org/gerddie/mesa/tree/vl-fix-DIV You can also just apply the last commit, thanks. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 05/11] drm: Add helpers to kick off self refresh mode in drivers
On Tue, Jun 11, 2019 at 10:56:45PM +0200, Sam Ravnborg wrote:
> Hi Sean.
>
> Small things here and there. Did not stare at this long enough to
> understand the code, but added some feedback anyway.
Thanks for the comments, Sam, I'll send a revision shortly
>
> Sam
> >
/snip
> > +static void drm_self_refresh_helper_entry_work(struct work_struct *work)
> > +{
> > + struct drm_self_refresh_data *sr_data = container_of(
> > + to_delayed_work(work),
> > + struct drm_self_refresh_data, entry_work);
> > + struct drm_crtc *crtc = sr_data->crtc;
> > + struct drm_device *dev = crtc->dev;
> > + struct drm_modeset_acquire_ctx ctx;
> > + struct drm_atomic_state *state;
> > + struct drm_connector *conn;
> > + struct drm_connector_state *conn_state;
> > + struct drm_crtc_state *crtc_state;
> > + int i, ret;
> This function is called from a workqueue.
> Just wondering if this require any locking?
Yes, it does. The locks are acquired in the various drm_atomic_get_*_state()
function calls and dropped below in the out label (drm_modeset_drop_locks).
> (Maybe I missed it, browsed the code without a detailed review)
>
> > +
> > + drm_modeset_acquire_init(, 0);
> > +
> > + state = drm_atomic_state_alloc(dev);
> > + if (!state) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > +retry:
> > + state->acquire_ctx =
> > +
> > + crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > + if (IS_ERR(crtc_state)) {
> > + ret = PTR_ERR(crtc_state);
> > + goto out;
> > + }
> > +
> > + if (!crtc_state->enable)
> > + goto out;
> > +
> > + ret = drm_atomic_add_affected_connectors(state, crtc);
> > + if (ret)
> > + goto out;
> > +
> > + for_each_new_connector_in_state(state, conn, conn_state, i) {
> > + if (!conn_state->self_refresh_aware)
> > + goto out;
> > + }
> > +
> > + crtc_state->active = false;
> > + crtc_state->self_refresh_active = true;
> > +
> > + ret = drm_atomic_commit(state);
> > + if (ret)
> > + goto out;
> > +
> > +out:
> > + if (ret == -EDEADLK) {
> > + drm_atomic_state_clear(state);
> > + ret = drm_modeset_backoff();
> > + if (!ret)
> > + goto retry;
> > + }
> > +
> > + drm_atomic_state_put(state);
> > + drm_modeset_drop_locks();
> > + drm_modeset_acquire_fini();
> > +}
> > +
/snip
--
Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH][next] video: fbdev: atmel_lcdfb: remove redundant initialization to variable ret
On Wed, Jun 12, 2019 at 09:55:30AM +0200, Nicolas Ferre - M43238 wrote:
> On 11/06/2019 at 19:09, Colin King wrote:
> > External E-Mail
> >
> >
> > From: Colin Ian King
> >
> > Currently variable ret is being initialized with -ENOENT however that
> > value is never read and ret is being re-assigned later on. Hence this
> > assignment is redundant and can be removed.
> >
> > Addresses-Coverity: ("Unused value")
> > Signed-off-by: Colin Ian King
>
> Indeed:
> Acked-by: Nicolas Ferre
Acked-by: Ludovic Desroches
Thanks
>
> Thanks, best regards,
>Nicolas
>
>
> > ---
> > drivers/video/fbdev/atmel_lcdfb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/atmel_lcdfb.c
> > b/drivers/video/fbdev/atmel_lcdfb.c
> > index fb117ccbeab3..930cc3f92e01 100644
> > --- a/drivers/video/fbdev/atmel_lcdfb.c
> > +++ b/drivers/video/fbdev/atmel_lcdfb.c
> > @@ -950,7 +950,7 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info
> > *sinfo)
> > struct fb_videomode fb_vm;
> > struct gpio_desc *gpiod;
> > struct videomode vm;
> > - int ret = -ENOENT;
> > + int ret;
> > int i;
> >
> > sinfo->config = (struct atmel_lcdfb_config*)
> >
>
>
> --
> Nicolas Ferre
Re: [PATCH 02/34] gpu: drm: bridge: sii9234: simplify getting the adapter of a client
On 08.06.2019 13:40, Laurent Pinchart wrote:
> Hi Wolfram,
>
> Thank you for the patch.
>
> On Sat, Jun 08, 2019 at 12:55:41PM +0200, Wolfram Sang wrote:
>> We have a dedicated pointer for that, so use it. Much easier to read and
>> less computation involved.
>>
>> Signed-off-by: Wolfram Sang
> Reviewed-by: Laurent Pinchart
Queued to drm-misc-next.
Laurent, for unknown reason patchwork does not collect yours tags, and
it is not the 1st time, added manually.
Reviewed-by: Andrzej Hajda
--
Regards
Andrzej
>
>> ---
>>
>> Please apply to your subsystem tree.
>>
>> drivers/gpu/drm/bridge/sii9234.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/sii9234.c
>> b/drivers/gpu/drm/bridge/sii9234.c
>> index b36bbafb0e43..25d4ad8c7ad6 100644
>> --- a/drivers/gpu/drm/bridge/sii9234.c
>> +++ b/drivers/gpu/drm/bridge/sii9234.c
>> @@ -815,7 +815,7 @@ static irqreturn_t sii9234_irq_thread(int irq, void
>> *data)
>> static int sii9234_init_resources(struct sii9234 *ctx,
>>struct i2c_client *client)
>> {
>> -struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
>> +struct i2c_adapter *adapter = client->adapter;
>> int ret;
>>
>> if (!ctx->dev->of_node) {
>> @@ -897,7 +897,7 @@ static const struct drm_bridge_funcs
>> sii9234_bridge_funcs = {
>> static int sii9234_probe(struct i2c_client *client,
>> const struct i2c_device_id *id)
>> {
>> -struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
>> +struct i2c_adapter *adapter = client->adapter;
>> struct sii9234 *ctx;
>> struct device *dev = >dev;
>> int ret;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/vkms: Add support for writeback
On Mon, Jun 10, 2019 at 12:39 PM Liviu Dudau wrote:
>
> On Fri, Jun 07, 2019 at 11:58:04AM -0300, Rodrigo Siqueira wrote:
> > On Fri, Jun 7, 2019 at 4:48 AM Daniel Vetter wrote:
> > >
> > > On Thu, Jun 06, 2019 at 07:41:01PM -0300, Rodrigo Siqueira wrote:
> > > > This patch implements the necessary functions to add writeback support
> > > > for vkms. This feature is useful for testing compositors if you don’t
> > > > have hardware with writeback support.
> > > >
> > > > Signed-off-by: Rodrigo Siqueira
> > > > ---
> > > > drivers/gpu/drm/vkms/Makefile | 9 +-
> > > > drivers/gpu/drm/vkms/vkms_crtc.c | 5 +
> > > > drivers/gpu/drm/vkms/vkms_drv.c | 10 ++
> > > > drivers/gpu/drm/vkms/vkms_drv.h | 12 ++
> > > > drivers/gpu/drm/vkms/vkms_output.c| 6 +
> > > > drivers/gpu/drm/vkms/vkms_writeback.c | 165 ++
> > > > 6 files changed, 206 insertions(+), 1 deletion(-)
> > > > create mode 100644 drivers/gpu/drm/vkms/vkms_writeback.c
> > > >
> > > > diff --git a/drivers/gpu/drm/vkms/Makefile
> > > > b/drivers/gpu/drm/vkms/Makefile
> > > > index 89f09bec7b23..90eb7acd618d 100644
> > > > --- a/drivers/gpu/drm/vkms/Makefile
> > > > +++ b/drivers/gpu/drm/vkms/Makefile
> > > > @@ -1,4 +1,11 @@
> > > > # SPDX-License-Identifier: GPL-2.0-only
> > > > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> > > > vkms_crc.o
> > > > +vkms-y := \
> > > > + vkms_drv.o \
> > > > + vkms_plane.o \
> > > > + vkms_output.o \
> > > > + vkms_crtc.o \
> > > > + vkms_gem.o \
> > > > + vkms_crc.o \
> > > > + vkms_writeback.o
> > > >
> > > > obj-$(CONFIG_DRM_VKMS) += vkms.o
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > index 1bbe099b7db8..ce797e265b1b 100644
> > > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > @@ -23,6 +23,11 @@ static enum hrtimer_restart
> > > > vkms_vblank_simulate(struct hrtimer *timer)
> > > > if (!ret)
> > > > DRM_ERROR("vkms failure on handling vblank");
> > > >
> > > > + if (output->writeback_status == WB_START) {
> > > > + drm_writeback_signal_completion(>wb_connector, 0);
> > > > + output->writeback_status = WB_STOP;
> > > > + }
> > > > +
> > > > if (state && output->crc_enabled) {
> > > > u64 frame = drm_crtc_accurate_vblank_count(crtc);
> > > >
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c
> > > > b/drivers/gpu/drm/vkms/vkms_drv.c
> > > > index 92296bd8f623..d5917d5a45e3 100644
> > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > > > @@ -29,6 +29,10 @@ bool enable_cursor;
> > > > module_param_named(enable_cursor, enable_cursor, bool, 0444);
> > > > MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
> > > >
> > > > +int enable_writeback;
> > > > +module_param_named(enable_writeback, enable_writeback, int, 0444);
> > > > +MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback
> > > > connector");
> > > > +
> > > > static const struct file_operations vkms_driver_fops = {
> > > > .owner = THIS_MODULE,
> > > > .open = drm_open,
> > > > @@ -123,6 +127,12 @@ static int __init vkms_init(void)
> > > > goto out_fini;
> > > > }
> > > >
> > > > + vkms_device->output.writeback_status = WB_DISABLED;
> > > > + if (enable_writeback) {
> > > > + vkms_device->output.writeback_status = WB_STOP;
> > > > + DRM_INFO("Writeback connector enabled");
> > > > + }
> > > > +
> > > > ret = vkms_modeset_init(vkms_device);
> > > > if (ret)
> > > > goto out_fini;
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> > > > b/drivers/gpu/drm/vkms/vkms_drv.h
> > > > index e81073dea154..ca1f9ee63ec8 100644
> > > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > > > @@ -7,6 +7,7 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > >
> > > > #define XRES_MIN20
> > > > @@ -60,14 +61,22 @@ struct vkms_crtc_state {
> > > > u64 frame_end;
> > > > };
> > > >
> > > > +enum wb_status {
> > > > + WB_DISABLED,
> > > > + WB_START,
> > > > + WB_STOP,
> > > > +};
> > > > +
> > > > struct vkms_output {
> > > > struct drm_crtc crtc;
> > > > struct drm_encoder encoder;
> > > > struct drm_connector connector;
> > > > + struct drm_writeback_connector wb_connector;
> > > > struct hrtimer vblank_hrtimer;
> > > > ktime_t period_ns;
> > > > struct drm_pending_vblank_event *event;
> > > > bool crc_enabled;
> > > > + enum wb_status writeback_status;
> > > > /* ordered wq for crc_work */
> > > > struct workqueue_struct *crc_workq;
> > > > /* protects concurrent access to crc_data */
> > > > @@ -141,4 +150,7
[PATCH v3] dt-bindings: display: renesas: du: Document optional reset properties
Document the optional properties for describing module resets, to
support resetting display channels on R-Car Gen2 and Gen3.
Signed-off-by: Geert Uytterhoeven
Acked-by: Laurent Pinchart
Acked-by: Rob Herring
---
v3:
- Add Acked-by,
- Drop LVDS resets, as LVDS is now covered by a separate binding,
- Update the example.
v2:
- s/phandles/phandle/.
---
.../devicetree/bindings/display/renesas,du.txt | 10 ++
1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt
b/Documentation/devicetree/bindings/display/renesas,du.txt
index aedb22b4d1613d1f..87e5fb2ce75e103f 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/Documentation/devicetree/bindings/display/renesas,du.txt
@@ -44,6 +44,14 @@ Required Properties:
instance that serves the DU channel, and the channel index identifies the
LIF instance in that VSP.
+Optional properties:
+ - resets: A list of phandle + reset-specifier pairs, one for each entry in
+the reset-names property.
+ - reset-names: Names of the resets. This property is model-dependent.
+- R8A779[0123456] use one reset for a group of one or more successive
+ channels. The resets must be named "du.x" with "x" being the numerical
+ index of the lowest channel in the group.
+
Required nodes:
The connections to the DU output video ports are modeled using the OF graph
@@ -88,6 +96,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
< CPG_MOD 722>,
< CPG_MOD 721>;
clock-names = "du.0", "du.1", "du.2", "du.3";
+ resets = < 724>, < 722>;
+ reset-names = "du.0", "du.2";
vsps = < 0>, < 0>, < 0>, < 1>;
ports {
--
2.17.1
Re: [PATCH 10/10] drm/vkms: No need for ->pages_lock in crc work anymore
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> We're now guaranteed to no longer race against prepare_fb/cleanup_fb,
> which means we can access ->vaddr without having to hold a lock.
>
> Before the previous patches it was fairly easy to observe the cursor
> ->vaddr being invalid, but that's now gone, so we can upgrade to a
> full WARN_ON.
>
> Signed-off-by: Daniel Vetter
> Cc: Rodrigo Siqueira
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crc.c | 13 ++---
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 0d31cfc32042..4b3146d83265 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -97,16 +97,10 @@ static void compose_cursor(struct vkms_crc_data
> *cursor_crc,
> cursor_obj = drm_gem_fb_get_obj(_crc->fb, 0);
> cursor_vkms_obj = drm_gem_to_vkms_gem(cursor_obj);
>
> - mutex_lock(_vkms_obj->pages_lock);
> - if (!cursor_vkms_obj->vaddr) {
> - DRM_WARN("cursor plane vaddr is NULL");
> - goto out;
> - }
> + if (WARN_ON(!cursor_vkms_obj->vaddr))
> + return;
>
> blend(vaddr_out, cursor_vkms_obj->vaddr, primary_crc, cursor_crc);
> -
> -out:
> - mutex_unlock(_vkms_obj->pages_lock);
> }
>
> static uint32_t _vkms_get_crc(struct vkms_crc_data *primary_crc,
> @@ -123,15 +117,12 @@ static uint32_t _vkms_get_crc(struct vkms_crc_data
> *primary_crc,
> return 0;
> }
>
> - mutex_lock(_obj->pages_lock);
> if (WARN_ON(!vkms_obj->vaddr)) {
> - mutex_unlock(_obj->pages_lock);
> kfree(vaddr_out);
> return crc;
> }
>
> memcpy(vaddr_out, vkms_obj->vaddr, vkms_obj->gem.size);
> - mutex_unlock(_obj->pages_lock);
>
> if (cursor_crc)
> compose_cursor(cursor_crc, primary_crc, vaddr_out);
> --
> 2.20.1
>
Reviewed-by: Rodrigo Siqueira
Tested-by: Rodrigo Siqueira
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 09/10] drm/vkms: totally reworked crc data tracking
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> The crc computation worker needs to be able to get at some data
> structures and framebuffer mappings, while potentially more atomic
> updates are going on. The solution thus far is to copy relevant bits
> around, but that's very tedious.
>
> Here's a new approach, which tries to be more clever, but relies on a
> few not-so-obvious things:
> - crtc_state is always updated when a plane_state changes. Therefore
> we can just stuff plane_state pointers into a crtc_state. That
> solves the problem of easily getting at the needed plane_states.
Just for curiosity, where exactly the crct_state is updated? If
possible, could you elaborate a little bit about this?
> - with the flushing changes from previous patches the above also holds
> without races due to the next atomic update being a bit eager with
> cleaning up pending work - we always wait for all crc work items to
> complete before unmapping framebuffers.
> - we also need to make sure that the hrtimer fires off the right
> worker. Keep a new distinct crc_state pointer, under the
> vkms_output->lock protection for this. Note that crtc->state is
> updated very early in the atomic commit, way before we arm the
> vblank event - the vblank event should always match the buffers we
> use to compute the crc. This also solves an issue in the hrtimer,
> where we've accessed drm_crtc->state without holding the right locks
> (we held none - oops).
> - in the worker itself we can then just access the plane states we
> need, again solving a bunch of ordering and locking issues.
> Accessing plane->state requires locks, accessing the private
> vkms_crtc_state->active_planes pointer only requires that the memory
> doesn't get freed too early.
>
> The idea behind vkms_crtc_state->active_planes is that this would
> contain all visible planes, in z-order, as a first step towards a more
> generic blending implementation.
>
> Note that this patch also fixes races between prepare_fb/cleanup_fb
> and the crc worker accessing ->vaddr.
>
> Signed-off-by: Daniel Vetter
> Cc: Rodrigo Siqueira
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crc.c | 21 +++
> drivers/gpu/drm/vkms/vkms_crtc.c | 60 +---
> drivers/gpu/drm/vkms/vkms_drv.h | 9 -
> 3 files changed, 67 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 9d15e5e85830..0d31cfc32042 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -159,11 +159,8 @@ void vkms_crc_work_handle(struct work_struct *work)
> crc_work);
> struct drm_crtc *crtc = crtc_state->base.crtc;
> struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> - struct vkms_device *vdev = container_of(out, struct vkms_device,
> - output);
> struct vkms_crc_data *primary_crc = NULL;
> struct vkms_crc_data *cursor_crc = NULL;
> - struct drm_plane *plane;
> u32 crc32 = 0;
> u64 frame_start, frame_end;
> bool crc_pending;
> @@ -184,21 +181,11 @@ void vkms_crc_work_handle(struct work_struct *work)
> if (!crc_pending)
> return;
>
> - drm_for_each_plane(plane, >drm) {
> - struct vkms_plane_state *vplane_state;
> - struct vkms_crc_data *crc_data;
> + if (crtc_state->num_active_planes >= 1)
> + primary_crc = crtc_state->active_planes[0]->crc_data;
>
> - vplane_state = to_vkms_plane_state(plane->state);
> - crc_data = vplane_state->crc_data;
> -
> - if (drm_framebuffer_read_refcount(_data->fb) == 0)
> - continue;
> -
> - if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> - primary_crc = crc_data;
> - else
> - cursor_crc = crc_data;
> - }
> + if (crtc_state->num_active_planes == 2)
> + cursor_crc = crtc_state->active_planes[1]->crc_data;
>
> if (primary_crc)
> crc32 = _vkms_get_crc(primary_crc, cursor_crc);
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 48a793ba4030..14156ed70415 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0+
>
> #include "vkms_drv.h"
> +#include
> #include
> #include
>
> @@ -9,7 +10,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> hrtimer *timer)
> struct vkms_output *output = container_of(timer, struct vkms_output,
> vblank_hrtimer);
> struct drm_crtc *crtc = >crtc;
> - struct vkms_crtc_state *state = to_vkms_crtc_state(crtc->state);
> +
Re: [PATCH 08/10] drm/vkms: No _irqsave within spin_lock_irq needed
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> irqs are already off.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crtc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index b6987d90805f..48a793ba4030 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -183,17 +183,16 @@ static void vkms_crtc_atomic_flush(struct drm_crtc
> *crtc,
>struct drm_crtc_state *old_crtc_state)
> {
> struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
> - unsigned long flags;
>
> if (crtc->state->event) {
> - spin_lock_irqsave(>dev->event_lock, flags);
> + spin_lock(>dev->event_lock);
>
> if (drm_crtc_vblank_get(crtc) != 0)
> drm_crtc_send_vblank_event(crtc, crtc->state->event);
> else
> drm_crtc_arm_vblank_event(crtc, crtc->state->event);
>
> - spin_unlock_irqrestore(>dev->event_lock, flags);
> + spin_unlock(>dev->event_lock);
>
> crtc->state->event = NULL;
> }
> --
> 2.20.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Reviewed-by: Rodrigo Siqueira
Tested-by: Rodrigo Siqueira
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mcde: Fix uninitialized variable
On Wed, Jun 12, 2019 at 03:30:38PM +0200, Linus Walleij wrote:
> We need to handle the case when of_drm_find_bridge() returns
> NULL.
>
> Reported-by: Dan Carpenter
> Cc: Dan Carpenter
> Signed-off-by: Linus Walleij
> ---
> drivers/gpu/drm/mcde/mcde_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
> index baf63fb6850a..bc11c446e594 100644
> --- a/drivers/gpu/drm/mcde/mcde_drv.c
> +++ b/drivers/gpu/drm/mcde/mcde_drv.c
> @@ -319,7 +319,7 @@ static int mcde_probe(struct platform_device *pdev)
> struct device *dev = >dev;
> struct drm_device *drm;
> struct mcde *mcde;
> - struct component_match *match;
> + struct component_match *match = NULL;
> struct resource *res;
> u32 pid;
> u32 val;
> @@ -485,7 +485,7 @@ static int mcde_probe(struct platform_device *pdev)
> }
> put_device(p);
> }
> - if (IS_ERR(match)) {
> + if (IS_ERR_OR_NULL(match)) {
> dev_err(dev, "could not create component match\n");
> ret = PTR_ERR(match);
This doesn't work. If "match" is NULL then "ret" is zero which is
success.
> goto clk_disable;
regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/10] drm/vkms: flush crc workers earlier in commit flow
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> Currently we flush pending crc workers very late in the commit flow,
> when we destry all the old crtc states. Unfortunately at that point
destry -> destroy
> the framebuffers are already unpinned (and our vaddr possible gone),
> so this isn't good. Also, the plane_states we need might also already
> be cleaned up, since cleanup order of state structures isn't well
> defined.
>
> Fix this by waiting for all crc workers of the old state to complete
> before we start any of the cleanup work.
>
> Note that this is not yet race-free, because the hrtimer and crc
> worker look at the wrong state pointers, but that will be fixed in
> subsequent patches.
>
> Signed-off-by: Daniel Vetter
> Cc: Rodrigo Siqueira
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crtc.c | 2 +-
> drivers/gpu/drm/vkms/vkms_drv.c | 10 ++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 55b16d545fe7..b6987d90805f 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -125,7 +125,7 @@ static void vkms_atomic_crtc_destroy_state(struct
> drm_crtc *crtc,
> __drm_atomic_helper_crtc_destroy_state(state);
>
> if (vkms_state) {
> - flush_work(_state->crc_work);
> + WARN_ON(work_pending(_state->crc_work));
> kfree(vkms_state);
> }
> }
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index f677ab1d0094..cc53ef88a331 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -62,6 +62,9 @@ static void vkms_release(struct drm_device *dev)
> static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
> {
> struct drm_device *dev = old_state->dev;
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *old_crtc_state;
> + int i;
>
> drm_atomic_helper_commit_modeset_disables(dev, old_state);
>
> @@ -75,6 +78,13 @@ static void vkms_atomic_commit_tail(struct
> drm_atomic_state *old_state)
>
> drm_atomic_helper_wait_for_vblanks(dev, old_state);
>
> + for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> + struct vkms_crtc_state *vkms_state =
> + to_vkms_crtc_state(old_crtc_state);
> +
> + flush_work(_state->crc_work);
> + }
> +
> drm_atomic_helper_cleanup_planes(dev, old_state);
> }
why not use drm_atomic_helper_commit_tail() here? I mean:
for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
…
}
drm_atomic_helper_commit_tail(old_state);
After looking at drm_atomic_helper_cleanup_planes() it sounds safe for
me to use the above code; I just test it with two tests from
crc_cursor. Maybe I missed something, could you help me here?
Finally, IMHO, I think that Patch 05, 06 and 07 could be squashed in a
single patch to make it easier to understand the change.
> --
> 2.20.1
>
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/vkms: Move format arrays to vkms_plane.c
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> No need to have them multiple times.
>
> Signed-off-by: Daniel Vetter
> Cc: Rodrigo Siqueira
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_drv.h | 8
> drivers/gpu/drm/vkms/vkms_plane.c | 8
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 43d3f98289fe..2a35299bfb89 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -20,14 +20,6 @@
>
> extern bool enable_cursor;
>
> -static const u32 vkms_formats[] = {
> - DRM_FORMAT_XRGB,
> -};
> -
> -static const u32 vkms_cursor_formats[] = {
> - DRM_FORMAT_ARGB,
> -};
> -
> struct vkms_crc_data {
> struct drm_framebuffer fb;
> struct drm_rect src, dst;
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c
> b/drivers/gpu/drm/vkms/vkms_plane.c
> index 0e67d2d42f0c..0fceb6258422 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -6,6 +6,14 @@
> #include
> #include
>
> +static const u32 vkms_formats[] = {
> + DRM_FORMAT_XRGB,
> +};
> +
> +static const u32 vkms_cursor_formats[] = {
> + DRM_FORMAT_ARGB,
> +};
> +
> static struct drm_plane_state *
> vkms_plane_duplicate_state(struct drm_plane *plane)
> {
> --
> 2.20.1
>
Reviewed-by: Rodrigo Siqueira
Tested-by: Rodrigo Siqueira
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/10] drm/vkms: Rename vkms_output.state_lock to crc_lock
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> Plus add a comment about what it actually protects. It's very little.
>
> Signed-off-by: Daniel Vetter
> Cc: Rodrigo Siqueira
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crc.c | 4 ++--
> drivers/gpu/drm/vkms/vkms_crtc.c | 6 +++---
> drivers/gpu/drm/vkms/vkms_drv.h | 5 +++--
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 883e36fe7b6e..96806cd35ad4 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -168,14 +168,14 @@ void vkms_crc_work_handle(struct work_struct *work)
> u64 frame_start, frame_end;
> bool crc_pending;
>
> - spin_lock_irq(>state_lock);
> + spin_lock_irq(>crc_lock);
> frame_start = crtc_state->frame_start;
> frame_end = crtc_state->frame_end;
> crc_pending = crtc_state->crc_pending;
> crtc_state->frame_start = 0;
> crtc_state->frame_end = 0;
> crtc_state->crc_pending = false;
> - spin_unlock_irq(>state_lock);
> + spin_unlock_irq(>crc_lock);
>
> /*
> * We raced with the vblank hrtimer and previous work already computed
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index c727d8486e97..55b16d545fe7 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -29,7 +29,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> hrtimer *timer)
> /* update frame_start only if a queued vkms_crc_work_handle()
> * has read the data
> */
> - spin_lock(>state_lock);
> + spin_lock(>crc_lock);
> if (!state->crc_pending)
> state->frame_start = frame;
> else
> @@ -37,7 +37,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> hrtimer *timer)
> state->frame_start, frame);
> state->frame_end = frame;
> state->crc_pending = true;
> - spin_unlock(>state_lock);
> + spin_unlock(>crc_lock);
>
> ret = queue_work(output->crc_workq, >crc_work);
> if (!ret)
> @@ -224,7 +224,7 @@ int vkms_crtc_init(struct drm_device *dev, struct
> drm_crtc *crtc,
> drm_crtc_helper_add(crtc, _crtc_helper_funcs);
>
> spin_lock_init(_out->lock);
> - spin_lock_init(_out->state_lock);
> + spin_lock_init(_out->crc_lock);
>
> vkms_out->crc_workq = alloc_ordered_workqueue("vkms_crc_workq", 0);
> if (!vkms_out->crc_workq)
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 3c7e06b19efd..43d3f98289fe 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -57,6 +57,7 @@ struct vkms_crtc_state {
> struct drm_crtc_state base;
> struct work_struct crc_work;
>
> + /* below three are protected by vkms_output.crc_lock */
> bool crc_pending;
> u64 frame_start;
> u64 frame_end;
> @@ -74,8 +75,8 @@ struct vkms_output {
> struct workqueue_struct *crc_workq;
> /* protects concurrent access to crc_data */
> spinlock_t lock;
It's not really specific to this patch, but after reviewing it, I was
thinking about the use of the 'lock' field in the struct vkms_output.
Do we really need it? It looks like that crc_lock just replaced it.
Additionally, going a little bit deeper in the lock field at struct
vkms_output, I was also asking myself: what critical area do we want
to protect with this lock? After inspecting the use of this lock, I
noticed two different places that use it:
1. In the function vkms_vblank_simulate()
Note that we cover a vast area with ‘output->lock’. IMHO we just need
to protect the state variable (line “state = output->crc_state;”) and
we can also use crc_lock for this case. Make sense?
2. In the function vkms_crtc_atomic_begin()
We also hold output->lock in the vkms_crtc_atomic_begin() and just
release it at vkms_crtc_atomic_flush(). Do we still need it?
> - /* protects concurrent access to crtc_state */
> - spinlock_t state_lock;
> +
> + spinlock_t crc_lock;
> };
Maybe add a kernel doc on top of crc_lock?
>
> struct vkms_device {
> --
> 2.20.1
>
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/10] drm/vkms: Use spin_lock_irq in process context
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> The worker is always in process context, no need for the _irqsafe
> version. Same for the set_source callback, that's only called from the
> debugfs handler in a syscall.
>
> Cc: Shayenne Moura
> Cc: Rodrigo Siqueira
> Signed-off-by: Daniel Vetter
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crc.c | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 66603da634fe..883e36fe7b6e 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -167,16 +167,15 @@ void vkms_crc_work_handle(struct work_struct *work)
> u32 crc32 = 0;
> u64 frame_start, frame_end;
> bool crc_pending;
> - unsigned long flags;
>
> - spin_lock_irqsave(>state_lock, flags);
> + spin_lock_irq(>state_lock);
> frame_start = crtc_state->frame_start;
> frame_end = crtc_state->frame_end;
> crc_pending = crtc_state->crc_pending;
> crtc_state->frame_start = 0;
> crtc_state->frame_end = 0;
> crtc_state->crc_pending = false;
> - spin_unlock_irqrestore(>state_lock, flags);
> + spin_unlock_irq(>state_lock);
>
> /*
> * We raced with the vblank hrtimer and previous work already computed
> @@ -246,7 +245,6 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
> *src_name)
> {
> struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> bool enabled = false;
> - unsigned long flags;
> int ret = 0;
>
> ret = vkms_crc_parse_source(src_name, );
> @@ -254,9 +252,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
> *src_name)
> /* make sure nothing is scheduled on crtc workq */
> flush_workqueue(out->crc_workq);
>
> - spin_lock_irqsave(>lock, flags);
> + spin_lock_irq(>lock);
> out->crc_enabled = enabled;
> - spin_unlock_irqrestore(>lock, flags);
I was wondering if we could use atomic_t for crc_enabled and avoid
this sort of lock. I did not try to change the data type; this is just
an idea.
Reviewed-by: Rodrigo Siqueira
Tested-by: Rodrigo Siqueira
> + spin_unlock_irq(>lock);
>
> return ret;
> }
> --
> 2.20.1
>
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/10] drm/vkms: Fix crc worker races
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> The issue we have is that the crc worker might fall behind. We've
> tried to handle this by tracking both the earliest frame for which it
> still needs to compute a crc, and the last one. Plus when the
> crtc_state changes, we have a new work item, which are all run in
> order due to the ordered workqueue we allocate for each vkms crtc.
>
> Trouble is there's been a few small issues in the current code:
> - we need to capture frame_end in the vblank hrtimer, not in the
> worker. The worker might run much later, and then we generate a lot
> of crc for which there's already a different worker queued up.
> - frame number might be 0, so create a new crc_pending boolean to
> track this without confusion.
> - we need to atomically grab frame_start/end and clear it, so do that
> all in one go. This is not going to create a new race, because if we
> race with the hrtimer then our work will be re-run.
> - only race that can happen is the following:
> 1. worker starts
> 2. hrtimer runs and updates frame_end
> 3. worker grabs frame_start/end, already reading the new frame_end,
> and clears crc_pending
> 4. hrtimer calls queue_work()
> 5. worker completes
> 6. worker gets re-run, crc_pending is false
> Explain this case a bit better by rewording the comment.
>
> v2: Demote warning level output to debug when we fail to requeue, this
> is expected under high load when the crc worker can't quite keep up.
>
> Cc: Shayenne Moura
> Cc: Rodrigo Siqueira
> Signed-off-by: Daniel Vetter
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crc.c | 27 +--
> drivers/gpu/drm/vkms/vkms_crtc.c | 9 +++--
> drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
> 3 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index d7b409a3c0f8..66603da634fe 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -166,16 +166,24 @@ void vkms_crc_work_handle(struct work_struct *work)
> struct drm_plane *plane;
> u32 crc32 = 0;
> u64 frame_start, frame_end;
> + bool crc_pending;
> unsigned long flags;
>
> spin_lock_irqsave(>state_lock, flags);
> frame_start = crtc_state->frame_start;
> frame_end = crtc_state->frame_end;
> + crc_pending = crtc_state->crc_pending;
> + crtc_state->frame_start = 0;
> + crtc_state->frame_end = 0;
> + crtc_state->crc_pending = false;
> spin_unlock_irqrestore(>state_lock, flags);
>
> - /* _vblank_handle() hasn't updated frame_start yet */
> - if (!frame_start || frame_start == frame_end)
> - goto out;
> + /*
> +* We raced with the vblank hrtimer and previous work already computed
> +* the crc, nothing to do.
> +*/
> + if (!crc_pending)
> + return;
I think this condition is not reachable because crc_pending will be
filled with true in `vkms_vblank_simulate()` which in turn schedule
the function `vkms_crc_work_handle()`. Just for checking, I tried to
reach this condition by running kms_flip, kms_pipe_crc_basic, and
kms_cursor_crc with two different VM setups[1], but I couldn't reach
it. What do you think?
[1] Qemu parameters
VM1: -m 1G -smp cores=2,cpus=2
VM2: -enable-kvm -m 2G -smp cores=4,cpus=4
> drm_for_each_plane(plane, >drm) {
> struct vkms_plane_state *vplane_state;
> @@ -196,20 +204,11 @@ void vkms_crc_work_handle(struct work_struct *work)
> if (primary_crc)
> crc32 = _vkms_get_crc(primary_crc, cursor_crc);
>
> - frame_end = drm_crtc_accurate_vblank_count(crtc);
> -
> - /* queue_work can fail to schedule crc_work; add crc for
> -* missing frames
> + /*
> +* The worker can fall behind the vblank hrtimer, make sure we catch
> up.
> */
> while (frame_start <= frame_end)
> drm_crtc_add_crc_entry(crtc, true, frame_start++, );
I want to take this opportunity to ask about this while; It's not
really specific to this patch.
I have to admit that I never fully got the idea behind this 'while';
it looks like that we just fill out the missed frames with a repeated
value. FWIU, `drm_crtc_add_crc_entry()` will add an entry with the CRC
information for a frame, but in this case, we are adding the same CRC
for a different set of frames. I agree that near frame has a similar
CRC value, but could we rely on this all the time? What could happen
if we have a great difference from the frame_start and frame_end?
> -
> -out:
> - /* to avoid using the same value for frame number again */
> - spin_lock_irqsave(>state_lock, flags);
> - crtc_state->frame_end = frame_end;
> - crtc_state->frame_start = 0;
> - spin_unlock_irqrestore(>state_lock, flags);
> }
>
> static int
[PATCH] drm/mcde: Fix uninitialized variable
We need to handle the case when of_drm_find_bridge() returns
NULL.
Reported-by: Dan Carpenter
Cc: Dan Carpenter
Signed-off-by: Linus Walleij
---
drivers/gpu/drm/mcde/mcde_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
index baf63fb6850a..bc11c446e594 100644
--- a/drivers/gpu/drm/mcde/mcde_drv.c
+++ b/drivers/gpu/drm/mcde/mcde_drv.c
@@ -319,7 +319,7 @@ static int mcde_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct drm_device *drm;
struct mcde *mcde;
- struct component_match *match;
+ struct component_match *match = NULL;
struct resource *res;
u32 pid;
u32 val;
@@ -485,7 +485,7 @@ static int mcde_probe(struct platform_device *pdev)
}
put_device(p);
}
- if (IS_ERR(match)) {
+ if (IS_ERR_OR_NULL(match)) {
dev_err(dev, "could not create component match\n");
ret = PTR_ERR(match);
goto clk_disable;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-fence/reservation: Markup rcu protected access for DEBUG_MUTEXES
Am 12.06.19 um 15:28 schrieb Chris Wilson:
Mark the access to reservation_object.fence as being protected to
silence sparse.
Signed-off-by: Chris Wilson
Reviewed-by: Christian König
---
include/linux/reservation.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index ee750765cc94..644a22dbe53b 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -216,8 +216,12 @@ reservation_object_unlock(struct reservation_object *obj)
{
#ifdef CONFIG_DEBUG_MUTEXES
/* Test shared fence slot reservation */
- if (obj->fence)
- obj->fence->shared_max = obj->fence->shared_count;
+ if (rcu_access_pointer(obj->fence)) {
+ struct reservation_object_list *fence =
+ reservation_object_get_list(obj);
+
+ fence->shared_max = fence->shared_count;
+ }
#endif
ww_mutex_unlock(>lock);
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/10] drm/vkms: rework crc worker
Hi Daniel, First of all, thank you very much for your patchset. I tried to make a detailed review of your series, and you can see my comments in each patch. You’ll notice that I asked many things related to the DRM subsystem with the hope that I could learn a little bit more about DRM from your comments. Before you go through the review, I would like to start a discussion about the vkms race conditions. First, I have to admit that I did not understand the race conditions that you described before because I couldn’t reproduce them. Now, I'm suspecting that I could not experience the problem because I'm using QEMU with KVM; with this idea in mind, I suppose that we have two scenarios for using vkms in a virtual machine: * Scenario 1: The user has hardware virtualization support; in this case, it is a little bit harder to experience race conditions with vkms. * Scenario 2: Without hardware virtualization support, it is much easier to experience race conditions. With these two scenarios in mind, I conducted a bunch of experiments for trying to shed light on this issue. I did: 1. Enabled lockdep 2. Defined two different environments for testing both using QEMU with and without kvm. See below the QEMU hardware options: * env_kvm: -enable-kvm -daemonize -m 1G -smp cores=2,cpus=2 * env_no_kvm: -daemonize -m 2G -smp cores=4,cpus=4 3. My test protocol: I) turn on the vm, II) clean /proc/lock_stat, III) execute kms_cursor_crc, III) save the lock_stat file, and IV) turn off the vm. 4. From the lockdep_stat, I just highlighted the row related to vkms and the columns holdtime-total and holdtime-avg I would like to highlight that the following data does not have any statistical approach, and its intention is solely to assist our discussion. See below the summary of the collected data: Summary of the experiment results: ++++ || env_kvm| env_no_kvm | ++++ | Test | Before | After | Before | After | +++---++---+ | kms_cursor_crc | S1 | S2 | M1 | M2 | +++---++---+ * Before: before apply this patchset * After: after apply this patchset -+--+--- S1: Without this patchset and with kvm | holdtime-total | holdtime-avg -++- &(_out->lock)->rlock: | 21983.52 | 6.21 (work_completion)(_state->crc_wo: | 20.47 | 20.47 (wq_completion)vkms_crc_workq: | 3999507.87| 3352.48 &(_out->state_lock)->rlock: | 378.47| 0.30 (work_completion)(_state->crc_#2: | 3999066.30| 2848.34 -++- S2: With this patchset and with kvm || -++- &(_out->lock)->rlock: | 23262.83 | 6.34 (work_completion)(_state->crc_wo: | 8.98 | 8.98 &(_out->crc_lock)->rlock: | 307.28| 0.32 (wq_completion)vkms_crc_workq: | 6567727.05| 12345.35 (work_completion)(_state->crc_#2: | 6567135.15| 4488.81 -++- M1: Without this patchset and without kvm|| -++- &(_out->state_lock)->rlock: | 4994.72 | 1.61 &(_out->lock)->rlock: | 247190.04 | 39.39 (work_completion)(_state->crc_wo: | 31.32 | 31.32 (wq_completion)vkms_crc_workq: | 20991073.78 | 13525.18 (work_completion)(_state->crc_#2: | 20988347.18 | 11904.90 -++- M2: With this patchset and without kvm || -++- (wq_completion)vkms_crc_workq: | 42819161.68 | 36597.57 &(_out->lock)->rlock: | 251257.06 | 35.80 (work_completion)(_state->crc_wo: | 69.37 | 69.37 &(_out->crc_lock)->rlock: | 3620.92 | 1.54 (work_completion)(_state->crc_#2: | 42803419.59 | 24306.31 First, I analyzed the scenarios with KVM (S1 and S2); more specifically, I focused on these two classes: 1. (work_completion)(_state->crc_wo 2. (work_completion)(_state->crc_#2 After taking a look at the data, it looks like that this patchset greatly reduces the hold time average for crc_wo. On the other hand, it increases the hold time average for crc_#2. I didn’t understand well the reason for the difference. Could you help me here? When I looked for the second set of scenarios (M1 and M2, both without KVM), the results look much more distant; basically, this patchset increased
[PATCH] dma-fence/reservation: Markup rcu protected access for DEBUG_MUTEXES
Mark the access to reservation_object.fence as being protected to
silence sparse.
Signed-off-by: Chris Wilson
---
include/linux/reservation.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index ee750765cc94..644a22dbe53b 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -216,8 +216,12 @@ reservation_object_unlock(struct reservation_object *obj)
{
#ifdef CONFIG_DEBUG_MUTEXES
/* Test shared fence slot reservation */
- if (obj->fence)
- obj->fence->shared_max = obj->fence->shared_count;
+ if (rcu_access_pointer(obj->fence)) {
+ struct reservation_object_list *fence =
+ reservation_object_get_list(obj);
+
+ fence->shared_max = fence->shared_count;
+ }
#endif
ww_mutex_unlock(>lock);
}
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1] backlight: Don't build support by default
b20c5249aa6a ("backlight: Fix compile error if CONFIG_FB is unset")
added 'default m' for BACKLIGHT_CLASS_DEVICE and LCD_CLASS_DEVICE.
Let's go back to not building support by default.
Signed-off-by: Marc Gonzalez
---
drivers/video/backlight/Kconfig | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8b081d61773e..40676be2e46a 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -10,7 +10,6 @@ menu "Backlight & LCD device support"
#
config LCD_CLASS_DEVICE
tristate "Lowlevel LCD controls"
- default m
help
This framework adds support for low-level control of LCD.
Some framebuffer devices connect to platform-specific LCD modules
@@ -143,7 +142,6 @@ endif # LCD_CLASS_DEVICE
#
config BACKLIGHT_CLASS_DEVICE
tristate "Lowlevel Backlight controls"
- default m
help
This framework adds support for low-level control of the LCD
backlight. This includes support for brightness and power.
--
2.17.1
Re: [PATCH v3 3/6] drm/modes: Allow to specify rotation and reflection on the commandline
Den 11.06.2019 15.20, skrev Maxime Ripard:
> Hi Noralf,
>
> On Fri, Apr 19, 2019 at 10:53:28AM +0200, Noralf Trønnes wrote:
>> Den 18.04.2019 18.40, skrev Noralf Trønnes:
>>>
>>>
>>> Den 18.04.2019 14.41, skrev Maxime Ripard:
Rotations and reflections setup are needed in some scenarios to initialise
properly the initial framebuffer. Some drivers already had a bunch of
quirks to deal with this, such as either a private kernel command line
parameter (omapdss) or on the device tree (various panels).
In order to accomodate this, let's create a video mode parameter to deal
with the rotation and reflexion.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_client_modeset.c | 10 +++-
drivers/gpu/drm/drm_modes.c | 110 ++--
include/drm/drm_connector.h | 9 ++-
3 files changed, 109 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index f2869c82510c..15145d2c86d5 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -823,6 +823,7 @@ EXPORT_SYMBOL(drm_client_modeset_probe);
bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int
*rotation)
{
struct drm_connector *connector = modeset->connectors[0];
+ struct drm_cmdline_mode *cmdline;
struct drm_plane *plane = modeset->crtc->primary;
u64 valid_mask = 0;
unsigned int i;
@@ -844,6 +845,15 @@ bool drm_client_panel_rotation(struct drm_mode_set
*modeset, unsigned int *rotat
*rotation = DRM_MODE_ROTATE_0;
}
+ /**
+ * We want the rotation on the command line to overwrite
+ * whatever comes from the panel.
+ */
+ cmdline = >cmdline_mode;
+ if (cmdline->specified &&
+ cmdline->rotation != DRM_MODE_ROTATE_0)
>>>
>>> I believe you need to drop that second check, otherwise rotate=0 will
>>> not overwrite panel rotation.
>>>
+ *rotation = cmdline->rotation;
>>
>> I remembered that you wanted this to propagate to DRM userspace. That's
>> not happening here.
>
> It's propated to the userspace through the plane's rotation property,
> I just checked.
>
Oh, so the rotation property stores its value in plane_state->rotation.
And this is set in: drm_client_modeset_commit_atomic() ->
drm_client_panel_rotation(): plane_state->rotation = rotation;
>> The only way I see for that to happen, is to set
>> ->panel_orientation. And to repeat myself, imo that makes
>> 'orientation' a better name for this video= option.
>
> orientation and rotation are two different things to me. The
> orientation of a panel for example is absolute, while the rotation is
> a transformation. In this particular case, I think that both the
> orientation and the rotation should be taken into account, with the
> orientation being the default state, and the hardware / panel will
> tell us that, while the rotation would be a transformation from that
> default to whatever the user wants.
>
> More importantly, the orientation is a property of the hardware (ie,
> how the display has been assembled), while the rotation is a software
> construct.
>
> And if the property being used to expose that is the rotation, I guess
> it would make sense to just use the same name and remain consistent.
>
Ok, I see. Based on this, I would assume that rotation would be relative
to the orientation, but I see that in this patch rotation doesn't take
orintation into account, it just overwrites it. I don't how userspace
deals with rotation on top of orientation.
Noralf.
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
