[PATCH 2/2] drm/ttm: Remove unused parameter evict from ttm_bo_move_memcpy
From: Michel Dänzer
Signed-off-by: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 +--
drivers/gpu/drm/nouveau/nouveau_bo.c| 2 +-
drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
drivers/gpu/drm/radeon/radeon_ttm.c | 3 +--
drivers/gpu/drm/ttm/ttm_bo.c| 3 +--
drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +--
include/drm/ttm/ttm_bo_driver.h | 4 +---
7 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b55310e..566c6dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -435,8 +435,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
if (r) {
memcpy:
- r = ttm_bo_move_memcpy(bo, evict, interruptible,
- no_wait_gpu, new_mem);
+ r = ttm_bo_move_memcpy(bo, interruptible, no_wait_gpu, new_mem);
if (r) {
return r;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 01460d7..8ab9ce5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1297,7 +1297,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
bool intr,
/* Fallback to software copy. */
ret = ttm_bo_wait(bo, intr, no_wait_gpu);
if (ret == 0)
- ret = ttm_bo_move_memcpy(bo, evict, intr, no_wait_gpu, new_mem);
+ ret = ttm_bo_move_memcpy(bo, intr, no_wait_gpu, new_mem);
out:
if (drm->device.info.family < NV_DEVICE_INFO_V0_TESLA) {
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index d50c967..6a22de0 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -361,8 +361,8 @@ static int qxl_bo_move(struct ttm_buffer_object *bo,
qxl_move_null(bo, new_mem);
return 0;
}
- return ttm_bo_move_memcpy(bo, evict, interruptible,
- no_wait_gpu, new_mem);
+ return ttm_bo_move_memcpy(bo, interruptible, no_wait_gpu,
+ new_mem);
}
static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index dd06047..bb4c02b 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -444,8 +444,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo,
if (r) {
memcpy:
- r = ttm_bo_move_memcpy(bo, evict, interruptible,
- no_wait_gpu, new_mem);
+ r = ttm_bo_move_memcpy(bo, interruptible, no_wait_gpu, new_mem);
if (r) {
return r;
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2016c08..f16fa4e 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -360,8 +360,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
ret = bdev->driver->move(bo, evict, interruptible,
no_wait_gpu, mem);
else
- ret = ttm_bo_move_memcpy(bo, evict, interruptible,
-no_wait_gpu, mem);
+ ret = ttm_bo_move_memcpy(bo, interruptible, no_wait_gpu, mem);
if (ret) {
if (bdev->driver->move_notify) {
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index f87162f..bf6e216 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -329,8 +329,7 @@ static int ttm_copy_ttm_io_page(struct ttm_tt *ttm, void
*dst,
}
int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
- bool evict, bool interruptible,
- bool no_wait_gpu,
+ bool interruptible, bool no_wait_gpu,
struct ttm_mem_reg *new_mem)
{
struct ttm_bo_device *bdev = bo->bdev;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 12d348f..c986fa7 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -983,7 +983,6 @@ extern int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
* ttm_bo_move_memcpy
*
* @bo: A pointer to a struct ttm_buffer_object.
- * @evict: 1: This is an eviction. Don't try to pipeline.
* @interruptible: Sleep interruptible if waiting.
* @no_wait_gpu: Return immediately if the GPU is busy.
* @new_mem: struct ttm_mem_reg indicating where to move.
@@ -999,8 +998,7 @@ extern int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
*/
extern int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
- bool evict, bool interruptible,
- bool no_wait_gpu,
[PATCH 0/2] Remove unused parameter evict from ttm_bo_move_{ttm, memcpy}
From: Michel Dänzer
I noticed that these parameters are unused.
These patches apply on top of 34b58355ad1d ("drm/ttm: Wait for a BO to
become idle before unbinding it from GTT"), which is being merged to 4.8
via Alex's tree.
Michel Dänzer (2):
drm/ttm: Remove unused parameter evict from ttm_bo_move_ttm
drm/ttm: Remove unused parameter evict from ttm_bo_move_memcpy
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 +++
drivers/gpu/drm/nouveau/nouveau_bo.c| 6 +++---
drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
drivers/gpu/drm/radeon/radeon_ttm.c | 7 +++
drivers/gpu/drm/ttm/ttm_bo.c| 6 ++
drivers/gpu/drm/ttm/ttm_bo_util.c | 7 +++
include/drm/ttm/ttm_bo_driver.h | 7 ++-
7 files changed, 18 insertions(+), 26 deletions(-)
--
2.8.1
[PATCH 1/2] drm/ttm: Remove unused parameter evict from ttm_bo_move_ttm
From: Michel Dänzer
Signed-off-by: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++--
drivers/gpu/drm/nouveau/nouveau_bo.c| 4 ++--
drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
drivers/gpu/drm/ttm/ttm_bo.c| 3 +--
drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++--
include/drm/ttm/ttm_bo_driver.h | 3 +--
6 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 9b61c8b..b55310e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -335,7 +335,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object
*bo,
if (unlikely(r)) {
goto out_cleanup;
}
- r = ttm_bo_move_ttm(bo, true, interruptible, no_wait_gpu, new_mem);
+ r = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, new_mem);
out_cleanup:
ttm_bo_mem_put(bo, &tmp_mem);
return r;
@@ -368,7 +368,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object
*bo,
if (unlikely(r)) {
return r;
}
- r = ttm_bo_move_ttm(bo, true, interruptible, no_wait_gpu, &tmp_mem);
+ r = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, &tmp_mem);
if (unlikely(r)) {
goto out_cleanup;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 6190035..01460d7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1151,7 +1151,7 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool
evict, bool intr,
if (ret)
goto out;
- ret = ttm_bo_move_ttm(bo, true, intr, no_wait_gpu, new_mem);
+ ret = ttm_bo_move_ttm(bo, intr, no_wait_gpu, new_mem);
out:
ttm_bo_mem_put(bo, &tmp_mem);
return ret;
@@ -1179,7 +1179,7 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool
evict, bool intr,
if (ret)
return ret;
- ret = ttm_bo_move_ttm(bo, true, intr, no_wait_gpu, &tmp_mem);
+ ret = ttm_bo_move_ttm(bo, intr, no_wait_gpu, &tmp_mem);
if (ret)
goto out;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 557ef0c..dd06047 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -346,7 +346,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object
*bo,
if (unlikely(r)) {
goto out_cleanup;
}
- r = ttm_bo_move_ttm(bo, true, interruptible, no_wait_gpu, new_mem);
+ r = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, new_mem);
out_cleanup:
ttm_bo_mem_put(bo, &tmp_mem);
return r;
@@ -379,7 +379,7 @@ static int radeon_move_ram_vram(struct ttm_buffer_object
*bo,
if (unlikely(r)) {
return r;
}
- r = ttm_bo_move_ttm(bo, true, interruptible, no_wait_gpu, &tmp_mem);
+ r = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, &tmp_mem);
if (unlikely(r)) {
goto out_cleanup;
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index f078038..2016c08 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -355,8 +355,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
- ret = ttm_bo_move_ttm(bo, evict, interruptible, no_wait_gpu,
- mem);
+ ret = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, mem);
else if (bdev->driver->move)
ret = bdev->driver->move(bo, evict, interruptible,
no_wait_gpu, mem);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index f157a9e..f87162f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -45,8 +45,8 @@ void ttm_bo_free_old_node(struct ttm_buffer_object *bo)
}
int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
- bool evict, bool interruptible,
- bool no_wait_gpu, struct ttm_mem_reg *new_mem)
+ bool interruptible, bool no_wait_gpu,
+ struct ttm_mem_reg *new_mem)
{
struct ttm_tt *ttm = bo->ttm;
struct ttm_mem_reg *old_mem = &bo->mem;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 99c6d01..12d348f 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -961,7 +961,6 @@ void ttm_mem_io_free(struct ttm_bo_device *bdev,
* ttm_bo_move_ttm
*
* @bo: A pointer to a struct ttm_buffer_object.
- * @evict: 1: This is an eviction. Don't try to pipeline.
* @interruptible: Sleep interruptible if waiting.
* @no_wait_gpu: Retur
[PATCH 1/6] drm: Add page_flip_target CRTC hook
On 05.08.2016 00:13, Alex Deucher wrote:
> On Wed, Aug 3, 2016 at 11:39 PM, Michel Dänzer wrote:
>>
>> @@ -5434,6 +5435,18 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>> if (!crtc)
>> return -ENOENT;
>>
>> + if (crtc->funcs->page_flip_target) {
>> + int r;
>> +
>> + r = drm_crtc_vblank_get(crtc);
>> + if (r)
>> + return r;
>> +
>> + target_vblank = drm_crtc_vblank_count(crtc) +
>> + !(page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC);
>> + } else if (crtc->funcs->page_flip == NULL)
>> + return -EINVAL;
>> +
>
> According to kernel coding style, this last block should have parens
> because the previous block did.
Right, I can never remember which project wants this which way. :)
> With that and Daniel's comments addressed:
> Reviewed-by: Alex Deucher
Thanks! I'll send out a v2 patch.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[PATCH 0/6] drm: Explicit target vblank seqno for page flips
On 04.08.2016 19:12, Daniel Stone wrote: > On 4 August 2016 at 11:01, Michel Dänzer wrote: >> On 04.08.2016 18:51, Daniel Stone wrote: >>> On 4 August 2016 at 04:39, Michel Dänzer wrote: Patch 6 extends the ioctl with new flags, which allow userspace to explicitly specify the target vblank seqno. This can also avoid delaying flips in some cases where we are already in the target vertical blank period when the ioctl is called. >>> >>> Is there open userspace for this? >> >> Sure, referenced in patch 6: >> >> https://cgit.freedesktop.org/~daenzer/xf86-video-ati/commit/?id=fc884a8af25345c32bd4104c864ecfeb9bb3db9b >> >> https://cgit.freedesktop.org/~daenzer/xf86-video-amdgpu/commit/?id=b8631a9ba49c0d0ebe5dcd1dbfb68fcfe907296f [...] >> The only change compared to the existing ioctl is that userspace can ask >> for a flip to take effect in the current vblank seqno. The code added by >> the patch checks for target vblank seqno > current vblank seqno + 1 and >> returns -EINVAL in that case. This is also documented in drm_mode.h. > > Is there any particular benefit to having split absolute/relative > modes in this case? Personally I'm struggling to see the use of > relative. See the userspace patches above. It allows userspace to set the target to the current/next vblank seqno without a DRM_IOCTL_WAIT_VBLANK round-trip (which could also result in the target getting delayed by one frame compared to using a relative target directly). >>> Is all this tested somewhere? >> >> Yes, I've been using it for a while on all my machines. > > I mean in a test suite. :) Not yet. Are you thinking of intel-gpu-tools, or something else? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
[PATCH 1/6] drm: Add page_flip_target CRTC hook
On 04.08.2016 20:01, Daniel Vetter wrote:
> On Thu, Aug 04, 2016 at 12:47:38PM +0200, Daniel Vetter wrote:
>> On Thu, Aug 04, 2016 at 12:39:36PM +0900, Michel Dänzer wrote:
>>>
>>> @@ -5434,6 +5435,18 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>>> if (!crtc)
>>> return -ENOENT;
>>>
>>> + if (crtc->funcs->page_flip_target) {
>>> + int r;
>>> +
>>> + r = drm_crtc_vblank_get(crtc);
>>
>> This leaks when e.g framebuffer_lookup fails.
Good catch.
>>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>>> index 9e6ab4a..ae1d9b6 100644
>>> --- a/include/drm/drm_crtc.h
>>> +++ b/include/drm/drm_crtc.h
>>> @@ -579,6 +579,20 @@ struct drm_crtc_funcs {
>>> uint32_t flags);
>>>
>>> /**
>>> +* @page_flip_target:
>>> +*
>>> +* Same as @page_flip but with an additional parameter specifying the
>>> +* target vertical blank period when the flip should take effect.
>
> *absolute target vertical blank period as reported by
> drm_crtc_vblank_count()
>
> would imo be a good addition here.
[...]
>>> +*
>>> +* Note that the core code calls drm_crtc_vblank_get before this entry
>>> +* point.
>>
>> I think you should add "Drivers must drop that reference again by calling
>> drm_crtc_vblank_put()."
Thanks for the suggestions.
> Also, who should drop the reference in case ->page_flip_target fails?
The core DRM code.
> With all issues addressed:
>
> Reviewed-by: Daniel Vetter
Thanks! I'll send out a v2 patch with your and Alex's feedback
addressed. Will it be okay to merge this via Alex's tree?
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[Bug 151341] AMDGPU Hawaii: screen freeze, Xorg blocked in fence_default_wait
https://bugzilla.kernel.org/show_bug.cgi?id=151341 --- Comment #5 from yshuiv7 at gmail.com --- I tried to reset the gpu by using /sys/kernel/debug/dri/0/amdgpu_gpu_reset, and the result is a NULL pointer dereference in the kernel. dmesg attached -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 151341] AMDGPU Hawaii: screen freeze, Xorg blocked in fence_default_wait
https://bugzilla.kernel.org/show_bug.cgi?id=151341 --- Comment #6 from yshuiv7 at gmail.com --- Created attachment 227901 --> https://bugzilla.kernel.org/attachment.cgi?id=227901&action=edit Kernel Oops when resetting the GPU -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 6/6] drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags
On 04.08.2016 19:56, Daniel Vetter wrote:
> On Thu, Aug 04, 2016 at 12:39:41PM +0900, Michel Dänzer wrote:
>> From: Michel Dänzer
>>
>> @@ -543,15 +549,25 @@ struct drm_color_lut {
>> * 'as soon as possible', meaning that it not delay waiting for vblank.
>> * This may cause tearing on the screen.
>> *
>> - * The reserved field must be zero until we figure out something
>> - * clever to use it for.
>> + * The sequence field must be zero unless either of the
>> + * DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags is specified. When
>> + * the ABSOLUTE flag is specified, the sequence field denotes the absolute
>> + * vblank sequence when the flip should take effect. When the RELATIVE
>> + * flag is specified, the sequence field denotes the relative (to the
>> + * current one when the ioctl is called) vblank sequence when the flip
>> + * should take effect. NOTE: DRM_IOCTL_WAIT_VBLANK must still be used to
>> + * make sure the vblank sequence before the target one has passed before
>> + * calling this ioctl. The purpose of the
>> + * DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags is merely to clarify
>> + * the target for when code dealing with a page flip runs during a
>> + * vertical blank period.
>> */
>>
>> struct drm_mode_crtc_page_flip {
>> __u32 crtc_id;
>> __u32 fb_id;
>> __u32 flags;
>> -__u32 reserved;
>> +__u32 sequence;
>
> Might break abi somewhere. I think it'd be better to create a
> struct drm_mode_crtc_page_flip2 with the renamed field.
It doesn't break ABI. I guess you mean there might be userspace code
referencing the reserved field? Such code would have to be open-coding
drmModePageFlip instead of using it. And it turns out you're right, e.g.
SNA actually does that and would fail to compile... Will be fixed in v2.
> Reviewed-by: Daniel Vetter
Thanks! Will it be okay to merge this via Alex's tree?
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[PATCH 6/6] drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags
On 04.08.2016 16:42, Ville Syrjälä wrote: > On Thu, Aug 04, 2016 at 12:39:41PM +0900, Michel Dänzer wrote: >> From: Michel Dänzer >> >> These flags allow userspace to explicitly specify the target vertical >> blank period when a flip should take effect. > > I like the idea. Atomic could use this too, although there we'd need to > potentially pass in a target vblank for each crtc taking part of the > operation, or I suppose you could pass it only for, say, a single crtc > in case you only want ot sync to that one. Yes, userspace tends to just synchronize to one CRTC and doesn't particularly care about any others performing the same flip. > One thing I've pondered in the past is the OML_sync_control modulo stuff. > Looks like what you have here won't free up userspace from doing the > wait_vblank+flip sequence if it wants to implement the modulo behaviour. > And of course it's still not guaranteed to honor the modulo if, for > instance, the flip ioctl itself gets blocked on a mutex for a wee bit too > long and misses the target. So should we just implement the modulo stuff > in the kernel instead? That might be nice, but I'm afraid it would be rather more complex than this patch for various reasons. E.g., in no particular order: The modulo can't be passed to the ioctl without extending the struct used by the ioctl, which could be tricky to get right in all corner cases. The current driver interface of the Present extension code in xserver wouldn't allow making use of it, the xserver code already handles the modulo before calling into the driver. The kernel driver interface would have to change significantly as well, it would be basically the driver's responsibility to handle the modulo. It would also be rather tricky to make it work reliably with our hardware, because we can't reliably determine in advance when a flip will take effect in the hardware. Overall, it seems like too much effort for too little gain with the legacy KMS API. Might be worth tackling for atomic though. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
[PATCH 1/6] drm: Add page_flip_target CRTC hook v2
From: Michel Dänzer
Mostly the same as the existing page_flip hook, but takes an additional
parameter specifying the target vertical blank period when the flip
should take effect.
v2:
* Add curly braces around else statement corresponding to an if block
with curly braces (Alex Deucher)
* Call drm_crtc_vblank_put in the error case (Daniel Vetter)
* Clarify entry point documentation comment (Daniel Vetter)
Reviewed-by: Alex Deucher
Reviewed-by: Daniel Vetter
Signed-off-by: Michel Dänzer
---
drivers/gpu/drm/drm_crtc.c | 26 ++
include/drm/drm_crtc.h | 18 ++
2 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9d3f80e..d6491ef 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5421,6 +5421,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
struct drm_crtc *crtc;
struct drm_framebuffer *fb = NULL;
struct drm_pending_vblank_event *e = NULL;
+ u32 target_vblank = 0;
int ret = -EINVAL;
if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS ||
@@ -5434,6 +5435,19 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
if (!crtc)
return -ENOENT;
+ if (crtc->funcs->page_flip_target) {
+ int r;
+
+ r = drm_crtc_vblank_get(crtc);
+ if (r)
+ return r;
+
+ target_vblank = drm_crtc_vblank_count(crtc) +
+ !(page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC);
+ } else if (crtc->funcs->page_flip == NULL) {
+ return -EINVAL;
+ }
+
drm_modeset_lock_crtc(crtc, crtc->primary);
if (crtc->primary->fb == NULL) {
/* The framebuffer is currently unbound, presumably
@@ -5444,9 +5458,6 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
goto out;
}
- if (crtc->funcs->page_flip == NULL)
- goto out;
-
fb = drm_framebuffer_lookup(dev, page_flip->fb_id);
if (!fb) {
ret = -ENOENT;
@@ -5487,7 +5498,12 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
}
crtc->primary->old_fb = crtc->primary->fb;
- ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
+ if (crtc->funcs->page_flip_target)
+ ret = crtc->funcs->page_flip_target(crtc, fb, e,
+ page_flip->flags,
+ target_vblank);
+ else
+ ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
if (ret) {
if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT)
drm_event_cancel_free(dev, &e->base);
@@ -5500,6 +5516,8 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
}
out:
+ if (ret)
+ drm_crtc_vblank_put(crtc);
if (fb)
drm_framebuffer_unreference(fb);
if (crtc->primary->old_fb)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 9e6ab4a..1eaf2e1 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -579,6 +579,24 @@ struct drm_crtc_funcs {
uint32_t flags);
/**
+* @page_flip_target:
+*
+* Same as @page_flip but with an additional parameter specifying the
+* absolute target vertical blank period (as reported by
+* drm_crtc_vblank_count()) when the flip should take effect.
+*
+* Note that the core code calls drm_crtc_vblank_get before this entry
+* point, and will call drm_crtc_vblank_put if this entry point returns
+* any non-0 error code. It's the driver's responsibility to call
+* drm_crtc_vblank_put after this entry point returns 0, typically when
+* the flip completes.
+*/
+ int (*page_flip_target)(struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ struct drm_pending_vblank_event *event,
+ uint32_t flags, uint32_t target);
+
+ /**
* @set_property:
*
* This is the legacy entry point to update a property attached to the
--
2.8.1
[PATCH 6/6] drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags v2
From: Michel Dänzer
These flags allow userspace to explicitly specify the target vertical
blank period when a flip should take effect.
v2:
* Add new struct drm_mode_crtc_page_flip_target instead of modifying
struct drm_mode_crtc_page_flip, to make sure all existing userspace
code keeps compiling (Daniel Vetter)
Reviewed-by: Daniel Vetter
Signed-off-by: Michel Dänzer
---
drivers/gpu/drm/drm_crtc.c | 48 ++---
drivers/gpu/drm/drm_ioctl.c | 8
include/uapi/drm/drm.h | 1 +
include/uapi/drm/drm_mode.h | 39 +---
4 files changed, 86 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d6491ef..3e1a63d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5417,15 +5417,23 @@ out:
int drm_mode_page_flip_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv)
{
- struct drm_mode_crtc_page_flip *page_flip = data;
+ struct drm_mode_crtc_page_flip_target *page_flip = data;
struct drm_crtc *crtc;
struct drm_framebuffer *fb = NULL;
struct drm_pending_vblank_event *e = NULL;
- u32 target_vblank = 0;
+ u32 target_vblank = page_flip->sequence;
int ret = -EINVAL;
- if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS ||
- page_flip->reserved != 0)
+ if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS)
+ return -EINVAL;
+
+ if (page_flip->sequence != 0 && !(page_flip->flags &
DRM_MODE_PAGE_FLIP_TARGET))
+ return -EINVAL;
+
+ /* Only one of the DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags
+* can be specified
+*/
+ if ((page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET) ==
DRM_MODE_PAGE_FLIP_TARGET)
return -EINVAL;
if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) &&
!dev->mode_config.async_page_flip)
@@ -5436,15 +5444,41 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
return -ENOENT;
if (crtc->funcs->page_flip_target) {
+ u32 current_vblank;
int r;
r = drm_crtc_vblank_get(crtc);
if (r)
return r;
- target_vblank = drm_crtc_vblank_count(crtc) +
- !(page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC);
- } else if (crtc->funcs->page_flip == NULL) {
+ current_vblank = drm_crtc_vblank_count(crtc);
+
+ switch (page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET) {
+ case DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE:
+ if ((int)(target_vblank - current_vblank) > 1) {
+ DRM_DEBUG("Invalid absolute flip target %u, "
+ "must be <= %u\n", target_vblank,
+ current_vblank + 1);
+ drm_crtc_vblank_put(crtc);
+ return -EINVAL;
+ }
+ break;
+ case DRM_MODE_PAGE_FLIP_TARGET_RELATIVE:
+ if (target_vblank != 0 && target_vblank != 1) {
+ DRM_DEBUG("Invalid relative flip target %u, "
+ "must be 0 or 1\n", target_vblank);
+ drm_crtc_vblank_put(crtc);
+ return -EINVAL;
+ }
+ target_vblank += current_vblank;
+ break;
+ default:
+ target_vblank = current_vblank +
+ !(page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC);
+ break;
+ }
+ } else if (crtc->funcs->page_flip == NULL ||
+ (page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET)) {
return -EINVAL;
}
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 33af4a5..0099d2a 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -228,6 +228,7 @@ static int drm_getstats(struct drm_device *dev, void *data,
static int drm_getcap(struct drm_device *dev, void *data, struct drm_file
*file_priv)
{
struct drm_get_cap *req = data;
+ struct drm_crtc *crtc;
req->value = 0;
switch (req->capability) {
@@ -254,6 +255,13 @@ static int drm_getcap(struct drm_device *dev, void *data,
struct drm_file *file_
case DRM_CAP_ASYNC_PAGE_FLIP:
req->value = dev->mode_config.async_page_flip;
break;
+ case DRM_CAP_PAGE_FLIP_TARGET:
+ req->value = 1;
+ drm_for_each_crtc(crtc, dev) {
+ if (!crtc->funcs->page_flip_target)
+ req->value = 0;
+ }
+ break;
[Bug 118701] x86_64 GMA500 reports /dev/dri/card0: failed to set DRM interface version 1.4: Inappropriate ioctl for device
https://bugzilla.kernel.org/show_bug.cgi?id=118701 Stuart Foster changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|CODE_FIX|--- --- Comment #6 from Stuart Foster --- Hi Patrik, I have checked both linux-4.7 and linux-4.8-rc1 and this fix has not made it through, when do you think it will be included ? Thanks. -- You are receiving this mail because: You are watching the assignee of the bug.
[Intel-gfx] [PATCH v2 3/9] drm/plane-helper: Add drm_plane_helper_check_state()
Hi Ville,
Two fixes inline...
On Wed, Jul 27, 2016 at 1:34 AM, wrote:
>
> From: Ville Syrjälä
>
> Add a version of drm_plane_helper_check_update() which takes a plane
> state instead of having the caller pass in everything.
>
> And to reduce code duplication, let's reimplement
> drm_plane_helper_check_update() in terms of the new function, by
> having a tempororary plane state on the stack.
>
> v2: Add a note that the functions modifies the state (Chris)
>
> Cc: Chris Wilson
> Signed-off-by: Ville Syrjälä
[snip...]
> +int drm_plane_helper_check_update(struct drm_plane *plane,
> + struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_rect *src,
> + struct drm_rect *dst,
> + const struct drm_rect *clip,
> + unsigned int rotation,
> + int min_scale,
> + int max_scale,
> + bool can_position,
> + bool can_update_disabled,
> + bool *visible)
> +{
> + struct drm_plane_state state = {
> + .plane = plane,
> + .crtc = crtc,
> + .fb = fb,
> + .src_x = src->x1,
> + .src_y = src->x2,
This should be:
src->y1
> + .src_w = drm_rect_width(src),
> + .src_h = drm_rect_height(src),
> + .crtc_x = dst->x1,
> + .crtc_y = dst->x2,
And this should be:
dst->y1
Thanks,
-Dan
[Intel-gfx] [PATCH 3/6] drm/i915: Check PSR setup time vs. vblank length
On Fri, Aug 05, 2016 at 03:10:51PM -0700, Rodrigo Vivi wrote:
> This patch is blocking PSR on panels that we know that our hardware support.
How do we know that?
> I wonder if:
> 1. This restrictions was for older platforms and spec is out dated
> 2. Or Spec is not documenting the restriction properly
I doubt it. AFAICS the only way that restriction could be lifted is by
adding support for the "Frame Capture Indication" bit in the PSR DPCD
register 0x170. That would cause the panel to wait one extra frame
between receiving the PSR entry indication and capturing the last
active frame. But I see no knob in Bspec that would allow us to tell
the source to send out that one extra active frame.
But maybe I've missed something. Art?
> 3. Or we have some issue with out setup time calculation.
I don't think so. Well, unless the panel is crap and reports a
totally bogus setup time.
I did notice that my SKL RVP stops trying to do PSR with this patch.
The EDID specifies two modes: 3200x1800 at 60Hz with 146us vblank,
and 3200x1800 at 48Hz with with ~2.5ms vblank. The setup time is
declared as 330us, so with the default mode we won't use PSR. We
could use the other timings I suppose, but I'm not sure everyone
would be happy with a 48Hz refresh rate. This is really a question
policy that shouldn't be handled in the kernel. What we could do is
expose both 60Hz and 48Hz modes, and let userspace choose the
refresh rate.
> On Tue, May 31, 2016 at 8:50 AM, wrote:
> > From: Ville Syrjälä
> >
> > Bspec says:
> > "Restriction : SRD must not be enabled when the PSR Setup time from DPCD
> > 00071h is greater than the time for vertical blank minus one line."
> >
> > Let's check for that and disallow PSR if we exceed the limit.
> >
> > Cc: Daniel Vetter
> > Reviewed-by: Daniel Vetter
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_drv.h| 2 ++
> > drivers/gpu/drm/i915/intel_psr.c| 19 ++-
> > drivers/gpu/drm/i915/intel_sprite.c | 6 +++---
> > 3 files changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 9b5f6634c558..56ae3b78e25e 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1672,6 +1672,8 @@ bool intel_sdvo_init(struct drm_device *dev,
> >
> >
> > /* intel_sprite.c */
> > +int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
> > +int usecs);
> > int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
> > int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
> > struct drm_file *file_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 29a09bf6bd18..aacd8d1767f2 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -327,6 +327,9 @@ static bool intel_psr_match_conditions(struct intel_dp
> > *intel_dp)
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct drm_crtc *crtc = dig_port->base.base.crtc;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + const struct drm_display_mode *adjusted_mode =
> > + &intel_crtc->config->base.adjusted_mode;
> > + int psr_setup_time;
> >
> > lockdep_assert_held(&dev_priv->psr.lock);
> > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> > @@ -365,11 +368,25 @@ static bool intel_psr_match_conditions(struct
> > intel_dp *intel_dp)
> > }
> >
> > if (IS_HASWELL(dev) &&
> > - intel_crtc->config->base.adjusted_mode.flags &
> > DRM_MODE_FLAG_INTERLACE) {
> > + adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> > DRM_DEBUG_KMS("PSR condition failed: Interlaced is
> > Enabled\n");
> > return false;
> > }
> >
> > + psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
> > + if (psr_setup_time < 0) {
> > + DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time
> > (0x%02x)\n",
> > + intel_dp->psr_dpcd[1]);
> > + return false;
> > + }
> > +
> > + if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
> > + adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
> > + DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us)
> > too long\n",
> > + psr_setup_time);
> > + return false;
> > + }
> > +
> > dev_priv->psr.source_ok = true;
> > return true;
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 324ccb06397d..293b48007006 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -53,8 +53,8 @@ for
[PATCH v8 2/6] drm/i915/gen9: Only copy WM results for changed pipes to skl_hw
Op 06-08-16 om 02:07 schreef Lyude:
> From: Matt Roper
>
> When we write watermark values to the hardware, those values are stored
> in dev_priv->wm.skl_hw. However with recent watermark changes, the
> results structure we're copying from only contains valid watermark and
> DDB values for the pipes that are actually changing; the values for
> other pipes remain 0. Thus a blind copy of the entire skl_wm_values
> structure will clobber the values for unchanged pipes...we need to be
> more selective and only copy over the values for the changing pipes.
>
> This mistake was hidden until recently due to another bug that caused us
> to erroneously re-calculate watermarks for all active pipes rather than
> changing pipes. Only when that bug was fixed was the impact of this bug
> discovered (e.g., modesets failing with "Requested display configuration
> exceeds system watermark limitations" messages and leaving watermarks
> non-functional, even ones initiated by intel_fbdev_restore_mode).
>
> Changes since v1:
> - Add a function for copying a pipe's wm values
>(skl_copy_wm_for_pipe()) so we can reuse this later
>
> Fixes: 734fa01f3a17 ("drm/i915/gen9: Calculate watermarks during atomic
> 'check' (v2)")
> Fixes: 9b6130227495 ("drm/i915/gen9: Re-allocate DDB only for changed pipes")
> Signed-off-by: Matt Roper
> Signed-off-by: Lyude
> Reviewed-by: Matt Roper
> Cc: stable at vger.kernel.org
> Cc: Maarten Lankhorst
> Cc: Ville Syrjälä
> Cc: Daniel Vetter
> Cc: Radhakrishna Sripada
> Cc: Hans de Goede
Testcase: kms_cursor_legacy
Reviewed-by: Maarten Lankhorst
[Intel-gfx] [PATCH v2 3/9] drm/plane-helper: Add drm_plane_helper_check_state()
On Mon, Aug 08, 2016 at 03:29:24PM +0800, Daniel Kurtz wrote:
> Hi Ville,
>
> Two fixes inline...
>
> On Wed, Jul 27, 2016 at 1:34 AM, wrote:
> >
> > From: Ville Syrjälä
> >
> > Add a version of drm_plane_helper_check_update() which takes a plane
> > state instead of having the caller pass in everything.
> >
> > And to reduce code duplication, let's reimplement
> > drm_plane_helper_check_update() in terms of the new function, by
> > having a tempororary plane state on the stack.
> >
> > v2: Add a note that the functions modifies the state (Chris)
> >
> > Cc: Chris Wilson
> > Signed-off-by: Ville Syrjälä
>
> [snip...]
>
> > +int drm_plane_helper_check_update(struct drm_plane *plane,
> > + struct drm_crtc *crtc,
> > + struct drm_framebuffer *fb,
> > + struct drm_rect *src,
> > + struct drm_rect *dst,
> > + const struct drm_rect *clip,
> > + unsigned int rotation,
> > + int min_scale,
> > + int max_scale,
> > + bool can_position,
> > + bool can_update_disabled,
> > + bool *visible)
> > +{
> > + struct drm_plane_state state = {
> > + .plane = plane,
> > + .crtc = crtc,
> > + .fb = fb,
> > + .src_x = src->x1,
> > + .src_y = src->x2,
>
> This should be:
> src->y1
>
> > + .src_w = drm_rect_width(src),
> > + .src_h = drm_rect_height(src),
> > + .crtc_x = dst->x1,
> > + .crtc_y = dst->x2,
>
> And this should be:
> dst->y1
Whoops. Copypaste fail, or something. Thanks for catching those.
--
Ville Syrjälä
Intel OTC
[PATCH v3 3/9] drm/plane-helper: Add drm_plane_helper_check_state()
From: Ville Syrjälä
Add a version of drm_plane_helper_check_update() which takes a plane
state instead of having the caller pass in everything.
And to reduce code duplication, let's reimplement
drm_plane_helper_check_update() in terms of the new function, by
having a tempororary plane state on the stack.
v2: Add a note that the functions modifies the state (Chris)
v3: Fix drm_plane_helper_check_update() y coordinates (Daniel Kurtz)
Cc: Daniel Kurtz
Cc: Chris Wilson
Signed-off-by: Ville Syrjälä
Reviewed-by: Sean Paul (v2)
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_plane_helper.c | 139 -
include/drm/drm_plane_helper.h | 5 ++
2 files changed, 112 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 16c4a7bd7465..0251aeec2bf3 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -108,14 +108,9 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
}
/**
- * drm_plane_helper_check_update() - Check plane update for validity
- * @plane: plane object to update
- * @crtc: owning CRTC of owning plane
- * @fb: framebuffer to flip onto plane
- * @src: source coordinates in 16.16 fixed point
- * @dest: integer destination coordinates
+ * drm_plane_helper_check_state() - Check plane state for validity
+ * @state: plane state to check
* @clip: integer clipping coordinates
- * @rotation: plane rotation
* @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
* @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
* @can_position: is it legal to position the plane such that it
@@ -123,10 +118,9 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
*only be false for primary planes.
* @can_update_disabled: can the plane be updated while the crtc
* is disabled?
- * @visible: output parameter indicating whether plane is still visible after
- * clipping
*
- * Checks that a desired plane update is valid. Drivers that provide
+ * Checks that a desired plane update is valid, and updates various
+ * bits of derived state (clipped coordinates etc.). Drivers that provide
* their own plane handling rather than helper-provided implementations may
* still wish to call this function to avoid duplication of error checking
* code.
@@ -134,29 +128,38 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
* RETURNS:
* Zero if update appears valid, error code on failure
*/
-int drm_plane_helper_check_update(struct drm_plane *plane,
- struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- struct drm_rect *src,
- struct drm_rect *dest,
- const struct drm_rect *clip,
- unsigned int rotation,
- int min_scale,
- int max_scale,
- bool can_position,
- bool can_update_disabled,
- bool *visible)
+int drm_plane_helper_check_state(struct drm_plane_state *state,
+const struct drm_rect *clip,
+int min_scale,
+int max_scale,
+bool can_position,
+bool can_update_disabled)
{
+ struct drm_crtc *crtc = state->crtc;
+ struct drm_framebuffer *fb = state->fb;
+ struct drm_rect *src = &state->src;
+ struct drm_rect *dst = &state->dst;
+ unsigned int rotation = state->rotation;
int hscale, vscale;
+ src->x1 = state->src_x;
+ src->y1 = state->src_y;
+ src->x2 = state->src_x + state->src_w;
+ src->y2 = state->src_y + state->src_h;
+
+ dst->x1 = state->crtc_x;
+ dst->y1 = state->crtc_y;
+ dst->x2 = state->crtc_x + state->crtc_w;
+ dst->y2 = state->crtc_y + state->crtc_h;
+
if (!fb) {
- *visible = false;
+ state->visible = false;
return 0;
}
/* crtc should only be NULL when disabling (i.e., !fb) */
if (WARN_ON(!crtc)) {
- *visible = false;
+ state->visible = false;
return 0;
}
@@ -168,20 +171,20 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
/* Check scaling */
- hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale);
- vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale);
+ hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
+ vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
if (hsc
[PATCH 6/6] drm/i915: Sanitize drm crtc vblank counter after DC reset frame count.
On Fri, Aug 05, 2016 at 02:49:44PM -0700, Rodrigo Vivi wrote:
> On Thu, Aug 4, 2016 at 1:26 AM, Daniel Vetter wrote:
> > On Wed, Aug 03, 2016 at 02:33:39PM -0700, Rodrigo Vivi wrote:
> >> DC state reset the frame counter that is a read-only register.
> >>
> >> So, besides blocking DC state on vblank let's restore the
> >> drm crtc vblank counter to a place we know it is reliable.
> >>
> >> Signed-off-by: Rodrigo Vivi
> >> ---
> >> drivers/gpu/drm/i915/i915_irq.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> >> b/drivers/gpu/drm/i915/i915_irq.c
> >> index 4efe20c..82d6896 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -2759,7 +2759,9 @@ static int gen8_enable_vblank(struct drm_device
> >> *dev, unsigned int pipe)
> >> static void gen9_prepare_vblank(struct drm_device *dev, unsigned int pipe)
> >> {
> >> struct drm_i915_private *dev_priv = to_i915(dev);
> >> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> >> intel_display_power_get(dev_priv, POWER_DOMAIN_VBLANK);
> >> + drm_crtc_vblank_sanitize_counter(crtc);
> >
> > I think this should be done within the platform power code. Otherwise
> > something else might keep the system out of dc states, but then we might
> > wreak havoc by calling this function here.
>
> This is safe here because DC is handled as a power_well... so as long
> as there is one domain holding its reference DC will be off.
>
> Also this needs to be in a place we are sure that vblanks are yet disabled.
>
> Now it comes back to that point on how to make sure that we only run 1
> prepare pre-enable...
>
> multiple prepare/unprepares will keep trying to resync it useless and
> if we have that WARN_ON we will get flooded
Yeah, my comment was under the assumption that there can be multiple
prepare/unprepare, and then it makes sense to reuse the refcounting we
already have. Still not sure it'll make sense to implement
prepare/unprepare refcounting in the vblank code, it'll be fairly tricky
in there. And for use useless, since our power well code already has
refcounting.
-Daniel
>
> > -Daniel
> >
> >> }
> >>
> >> static void gen9_unprepare_vblank(struct drm_device *dev, unsigned int
> >> pipe)
> >> --
> >> 2.4.3
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel at 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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 1/6] drm: Add page_flip_target CRTC hook
On Mon, Aug 08, 2016 at 12:54:45PM +0900, Michel Dänzer wrote:
> On 04.08.2016 20:01, Daniel Vetter wrote:
> > On Thu, Aug 04, 2016 at 12:47:38PM +0200, Daniel Vetter wrote:
> >> On Thu, Aug 04, 2016 at 12:39:36PM +0900, Michel Dänzer wrote:
> >>>
> >>> @@ -5434,6 +5435,18 @@ int drm_mode_page_flip_ioctl(struct drm_device
> >>> *dev,
> >>> if (!crtc)
> >>> return -ENOENT;
> >>>
> >>> + if (crtc->funcs->page_flip_target) {
> >>> + int r;
> >>> +
> >>> + r = drm_crtc_vblank_get(crtc);
> >>
> >> This leaks when e.g framebuffer_lookup fails.
>
> Good catch.
>
>
> >>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >>> index 9e6ab4a..ae1d9b6 100644
> >>> --- a/include/drm/drm_crtc.h
> >>> +++ b/include/drm/drm_crtc.h
> >>> @@ -579,6 +579,20 @@ struct drm_crtc_funcs {
> >>>uint32_t flags);
> >>>
> >>> /**
> >>> + * @page_flip_target:
> >>> + *
> >>> + * Same as @page_flip but with an additional parameter specifying the
> >>> + * target vertical blank period when the flip should take effect.
> >
> > *absolute target vertical blank period as reported by
> > drm_crtc_vblank_count()
> >
> > would imo be a good addition here.
>
> [...]
>
> >>> + *
> >>> + * Note that the core code calls drm_crtc_vblank_get before this entry
> >>> + * point.
> >>
> >> I think you should add "Drivers must drop that reference again by calling
> >> drm_crtc_vblank_put()."
>
> Thanks for the suggestions.
>
>
> > Also, who should drop the reference in case ->page_flip_target fails?
>
> The core DRM code.
Please add that to the kerneldoc, too, when the code is fixed.
> > With all issues addressed:
> >
> > Reviewed-by: Daniel Vetter
>
> Thanks! I'll send out a v2 patch with your and Alex's feedback
> addressed. Will it be okay to merge this via Alex's tree?
Sure, ack on merging through amdgpu. Would be good to then send an earlier
pull request with it to avoid conflicts when it's too long outside of
drm-next.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
VT Switching with Atomic Modeset
On Sun, Aug 07, 2016 at 06:41:11PM -0500, Scot Doyle wrote: > Hi all, > > I'm interested in discussing ways compositors could cooperate > without CONFIG_VT/VT_VACTIVATE/VT_SETMODE. > > Daniel wrote up a nice article "VT Switching with Atomic Modeset" [1] > inviting discussion on dri-devel (about much more than, but including > this), David posted recently about it on dri-devel [2] and Noralf also > posted the "DRM text mode" patch series. Which problem are you trying to solve by avoiding the system compositor part? Afaiui there's not just the drm console, but all the other resources assigned to a seat (input, audio, whatever) which also need to be switched. And doing that imo makes much more sense if it's all done in userspace. Other parts of your plan (like a sysrq for the drm_text console) makes sense, but those also don't need new ioctls. -Daniel > So, maybe I could continue the discussion by proposing some DRM > interface additions. > > The goals of the proposal are to > - not affect current CONFIG_VT operation > - enable compositor switching [1] > - without a system compositor or intermediary [1] > - without CONFIG_VT [2] > - be compatible with the in-kernel DRM text mode[3] > - provide manual device reset for aberrant states [1] > - don't consume kernel memory unnecessarily [1] > - provide DRM clients with access to device boot state [1] > - remain compatible with legacy DRM/KMS interface [1] > - remain compatible with future atomic properties [1] > - leave as many policy decisions to userspace as practical [1] > > The "boot state" is the device state after any hardware and firmware > initialization, but before the DRM device driver is loaded. Since > different DRM clients (compositors) may want access to this state, > and no intermediary userspace process should be required, this state > should be stored in the kernel. > > Daniel referred to the "reset" state as "FBDEV resets to defaults" [1]. > I understand it to be the state of a device after the driver has performed > some positive set of actions to re-initialize the device. It may be more > or less desirable from a user's or compositor's perspective than boot > state. > > Ideas 5-9 below are not necessary to switch compositors without an > intermediary, e.g. a compositor could obtain the current drm client > process id and send a pre-agreed signal. However, since the DRM interface > already includes the idea of exclusive access to modesetting > (SET_MASTER/DROP_MASTER), it seemed fitting to extend it using the > existing ioctl and event model. > > > Implementation overview > > 1. Create a SYSRQ that restores all DRM devices to reset state > > 2. Kernel saves boot state of atomic-capable DRM devices at driver load > > 3. Create new ioctl DRM_IOCTL_DISCARD_BOOT_STATE >- frees kernel memory used in #2, if boot state not needed > > 4. Provide (optional) mechanism to atomic modeset starting from either >boot or reset state >- Daniel discussed two possible mechanisms in [1], an extension to > the GET_PROPERTY ioctl or a new flag for the MODE_ATOMIC ioctl >- if boot state selected but unavailable (see #3), return error > > 5. Create new ioctl DRM_IOCTL_VOLUNTEER_MASTER >- indicating current master plans to cooperate with other clients >- reset in each successful DRM_IOCTL_SET_MASTER ioctl > > 6. Create new ioctl DRM_IOCTL_REQUEST_MASTER >- if no root permissions (like DRM_IOCTL_SET_MASTER), return error >- if no current master, then return error indicating such >- if current master didn't VOLUNTEER_MASTER, return error >- else send event DRM_EVENT_MASTER_REQUESTED to current master > > 7. Create new event DRM_EVENT_MASTER_REQUESTED >- sent via the DRM asynchronous read/poll interface >- recipient may ignore event (e.g. when using CONFIG_VT) >- recipient, or its agent, may drop master > > 8. Modify DRM_IOCTL_DROP_MASTER ioctl >- send event DRM_EVENT_MASTER_DROPPED >- to clients that requested master since the latter of > - the last time this event was sent > - the last successful DRM_IOCTL_SET_MASTER ioctl > > 9. Create new event DRM_EVENT_MASTER_DROPPED >- sent via the DRM asynchronous read/poll interface >- recipient issues DRM_IOCTL_SET_MASTER ioctl if still interested > > > I welcome all discussion, being new to this topic. > > Thoughts? > Scot > > [1] http://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html > [2] https://lists.freedesktop.org/archives/dri-devel/2016-August/114517.html > [3] https://lists.freedesktop.org/archives/dri-devel/2016-July/114208.html -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Intel-gfx] [PATCH 3/6] drm/i915: Check PSR setup time vs. vblank length
On Sat, 06 Aug 2016, Rodrigo Vivi wrote:
> This patch is blocking PSR on panels that we know that our hardware support.
And it also fixes a regression on Linus' laptop, and it's been merged
upstream...
BR,
Jani.
>
> I wonder if:
> 1. This restrictions was for older platforms and spec is out dated
> 2. Or Spec is not documenting the restriction properly
> 3. Or we have some issue with out setup time calculation.
>
>
> On Tue, May 31, 2016 at 8:50 AM, wrote:
>> From: Ville Syrjälä
>>
>> Bspec says:
>> "Restriction : SRD must not be enabled when the PSR Setup time from DPCD
>> 00071h is greater than the time for vertical blank minus one line."
>>
>> Let's check for that and disallow PSR if we exceed the limit.
>>
>> Cc: Daniel Vetter
>> Reviewed-by: Daniel Vetter
>> Signed-off-by: Ville Syrjälä
>> ---
>> drivers/gpu/drm/i915/intel_drv.h| 2 ++
>> drivers/gpu/drm/i915/intel_psr.c| 19 ++-
>> drivers/gpu/drm/i915/intel_sprite.c | 6 +++---
>> 3 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 9b5f6634c558..56ae3b78e25e 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1672,6 +1672,8 @@ bool intel_sdvo_init(struct drm_device *dev,
>>
>>
>> /* intel_sprite.c */
>> +int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>> +int usecs);
>> int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
>> int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>> struct drm_file *file_priv);
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c
>> b/drivers/gpu/drm/i915/intel_psr.c
>> index 29a09bf6bd18..aacd8d1767f2 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -327,6 +327,9 @@ static bool intel_psr_match_conditions(struct intel_dp
>> *intel_dp)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct drm_crtc *crtc = dig_port->base.base.crtc;
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> + const struct drm_display_mode *adjusted_mode =
>> + &intel_crtc->config->base.adjusted_mode;
>> + int psr_setup_time;
>>
>> lockdep_assert_held(&dev_priv->psr.lock);
>> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>> @@ -365,11 +368,25 @@ static bool intel_psr_match_conditions(struct intel_dp
>> *intel_dp)
>> }
>>
>> if (IS_HASWELL(dev) &&
>> - intel_crtc->config->base.adjusted_mode.flags &
>> DRM_MODE_FLAG_INTERLACE) {
>> + adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
>> DRM_DEBUG_KMS("PSR condition failed: Interlaced is
>> Enabled\n");
>> return false;
>> }
>>
>> + psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
>> + if (psr_setup_time < 0) {
>> + DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time
>> (0x%02x)\n",
>> + intel_dp->psr_dpcd[1]);
>> + return false;
>> + }
>> +
>> + if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
>> + adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
>> + DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us)
>> too long\n",
>> + psr_setup_time);
>> + return false;
>> + }
>> +
>> dev_priv->psr.source_ok = true;
>> return true;
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>> b/drivers/gpu/drm/i915/intel_sprite.c
>> index 324ccb06397d..293b48007006 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -53,8 +53,8 @@ format_is_yuv(uint32_t format)
>> }
>> }
>>
>> -static int usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>> - int usecs)
>> +int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>> +int usecs)
>> {
>> /* paranoia */
>> if (!adjusted_mode->crtc_htotal)
>> @@ -91,7 +91,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>> vblank_start = DIV_ROUND_UP(vblank_start, 2);
>>
>> /* FIXME needs to be calibrated sensibly */
>> - min = vblank_start - usecs_to_scanlines(adjusted_mode, 100);
>> + min = vblank_start - intel_usecs_to_scanlines(adjusted_mode, 100);
>> max = vblank_start - 1;
>>
>> local_irq_disable();
>> --
>> 2.7.4
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
[PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
On Sat, Aug 06, 2016 at 10:26:09PM -0700, Keith Packard wrote:
> Daniel Vetter writes:
>
> > Hm, I can't see v1 anywhere, but I think it'd be better. You can't store
> > any transient state related to the current update in struct drm_plane. In
> > this case the cleanup_buffers from a previous update might overlap (for
> > nonblocking atomic commits) with the prepare_planes for the next one.
> > Either we need special cleanup vs. error-path code, or some flag somewhere
> > in the drm_plane_state.
>
> Ok, here's pretty much the previous version, which works now that I've
> fixed the intel driver. Instead of just comparing the fb's, I'm using
> the framebuffer_changed function, which seems like a nice bit of
> documentation if nothing else.
>
> From a75251d5762b1c200ed2f3dca2a5b00cc85bea95 Mon Sep 17 00:00:00 2001
> From: Keith Packard
> Date: Sat, 4 Jun 2016 01:16:22 -0700
> Subject: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v3]
>
> When reconfiguring a plane position (as in moving the cursor), the
> frame buffer for the cursor isn't changing, so don't call the prepare
> or cleanup driver functions.
>
> This avoids making cursor position updates block on all pending rendering.
>
> v3: use drm_atomic_helper_framebuffer_changed in both prepare and
> cleanup phases instead of keeping state in the plane.
>
> cc: dri-devel at lists.freedesktop.org
> cc: David Airlie
> cc: Daniel Vetter
> Signed-off-by: Keith Packard
Rebased onto 4.8 while applying, which did simplify the patch a lot (4.8
is already using for_each_plane_in_state, but slightly differently).
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 25 +++--
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index ddfa0d1..72e50bc 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1246,18 +1246,19 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
> * Returns:
> * 0 on success, negative error code on failure.
> */
> +
> int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>struct drm_atomic_state *state)
> {
> - int nplanes = dev->mode_config.num_total_plane;
> + struct drm_plane *plane;
> + struct drm_plane_state *plane_state;
> int ret, i;
> + int max_prepared_i = 0;
>
> - for (i = 0; i < nplanes; i++) {
> + for_each_plane_in_state(state, plane, plane_state, i) {
> const struct drm_plane_helper_funcs *funcs;
> - struct drm_plane *plane = state->planes[i];
> - struct drm_plane_state *plane_state = state->plane_states[i];
>
> - if (!plane)
> + if (!drm_atomic_helper_framebuffer_changed(dev, state,
> plane_state->crtc))
> continue;
>
> funcs = plane->helper_private;
> @@ -1267,24 +1268,25 @@ int drm_atomic_helper_prepare_planes(struct
> drm_device *dev,
> if (ret)
> goto fail;
> }
> + max_prepared_i = i;
> }
>
> return 0;
>
> fail:
> - for (i--; i >= 0; i--) {
> + for_each_plane_in_state(state, plane, plane_state, i) {
> const struct drm_plane_helper_funcs *funcs;
> - struct drm_plane *plane = state->planes[i];
> - struct drm_plane_state *plane_state = state->plane_states[i];
>
> - if (!plane)
> + if (i > max_prepared_i)
> + break;
> +
> + if (!drm_atomic_helper_framebuffer_changed(dev, state,
> plane_state->crtc))
> continue;
>
> funcs = plane->helper_private;
>
> if (funcs->cleanup_fb)
> funcs->cleanup_fb(plane, plane_state);
> -
> }
>
> return ret;
> @@ -1527,6 +1529,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device
> *dev,
> for_each_plane_in_state(old_state, plane, plane_state, i) {
> const struct drm_plane_helper_funcs *funcs;
>
> + if (!drm_atomic_helper_framebuffer_changed(dev, old_state,
> plane_state->crtc))
> + continue;
> +
> funcs = plane->helper_private;
>
> if (funcs->cleanup_fb)
> --
> 2.8.1
>
>
> --
> -keith
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 0/2] Remove unused parameter evict from ttm_bo_move_{ttm, memcpy}
Am 08.08.2016 um 05:28 schrieb Michel Dänzer:
> From: Michel Dänzer
Reviewed-by: Christian König
>
> I noticed that these parameters are unused.
>
> These patches apply on top of 34b58355ad1d ("drm/ttm: Wait for a BO to
> become idle before unbinding it from GTT"), which is being merged to 4.8
> via Alex's tree.
>
> Michel Dänzer (2):
>drm/ttm: Remove unused parameter evict from ttm_bo_move_ttm
>drm/ttm: Remove unused parameter evict from ttm_bo_move_memcpy
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 +++
> drivers/gpu/drm/nouveau/nouveau_bo.c| 6 +++---
> drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
> drivers/gpu/drm/radeon/radeon_ttm.c | 7 +++
> drivers/gpu/drm/ttm/ttm_bo.c| 6 ++
> drivers/gpu/drm/ttm/ttm_bo_util.c | 7 +++
> include/drm/ttm/ttm_bo_driver.h | 7 ++-
> 7 files changed, 18 insertions(+), 26 deletions(-)
>
[PATCH v3 2/3] drm: Add API for capturing frame CRCs
On 6 August 2016 at 19:04, Daniel Stone wrote: > Hi Tomeu, > > On 22 July 2016 at 15:10, Tomeu Vizoso wrote: >> +/** >> + * DOC: CRC ABI >> + * >> + * DRM device drivers can provide to userspace CRC information of each >> frame as >> + * it reached a given hardware component (a "source"). >> + * >> + * Userspace can control generation of CRCs in a given CRTC by writing to >> the > > s/can/must/ > > Is it worth having 'auto' as a default source perhaps? Yup, it's the case in v4, so you just cat the data file and start getting CRCs. >> + * file dri/0/crtc-N/crc/control in debugfs, with N being the index of the >> CRTC. >> + * Accepted values are source names (which are driver-specific) and the >> "none" >> + * and "auto" keywords. "none" will disable CRC generation and "auto" will >> let >> + * the driver select a default source of frame CRCs for this CRTC. > > Is it also worth having 'connector-%s' (named as per sysfs, e.g. > connector-HDMI-A-0) as a standardised entry, for cloneable CRTCs which > have CRC control on the connector rather than the CRTC? My impression right now is that only "auto" makes sense as a standardised entry, as any explicit sources are pretty much hw-dependent so the tests will need knowledge about the hw anyway. The IGT tests already try each connector in each CRTC when looking for a setup that supports CRC capture (with the auto source). Regards, Tomeu
[PATCH 2/7] staging/android: display sync_pt name on debugfs
Op 20-06-16 om 17:53 schreef Gustavo Padovan:
> From: Gustavo Padovan
>
> When creating a sync_pt the name received wasn't used anywhere.
> Now we add it to the sync info debug output to make it easier to indetify
> the userspace name of that sync pt.
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/staging/android/sw_sync.c| 16
> drivers/staging/android/sync_debug.c | 5 +++--
> drivers/staging/android/sync_debug.h | 9 +
> 3 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/android/sw_sync.c
> b/drivers/staging/android/sw_sync.c
> index b4ae092..ea27512 100644
> --- a/drivers/staging/android/sw_sync.c
> +++ b/drivers/staging/android/sw_sync.c
> @@ -37,15 +37,6 @@ struct sw_sync_create_fence_data {
> struct sw_sync_create_fence_data)
> #define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1,
> __u32)
>
> -static const struct fence_ops timeline_fence_ops;
> -
> -static inline struct sync_pt *fence_to_sync_pt(struct fence *fence)
> -{
> - if (fence->ops != &timeline_fence_ops)
> - return NULL;
> - return container_of(fence, struct sync_pt, base);
> -}
> -
> struct sync_timeline *sync_timeline_create(const char *name)
> {
> struct sync_timeline *obj;
> @@ -108,7 +99,7 @@ static void sync_timeline_signal(struct sync_timeline
> *obj, unsigned int inc)
> }
>
> static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size,
> - unsigned int value)
> + unsigned int value, char *name)
> {
> unsigned long flags;
> struct sync_pt *pt;
> @@ -120,6 +111,7 @@ static struct sync_pt *sync_pt_create(struct
> sync_timeline *obj, int size,
> if (!pt)
> return NULL;
>
> + strlcpy(pt->name, name, sizeof(pt->name));
> spin_lock_irqsave(&obj->child_list_lock, flags);
> sync_timeline_get(obj);
> fence_init(&pt->base, &timeline_fence_ops, &obj->child_list_lock,
> @@ -191,7 +183,7 @@ static void timeline_fence_timeline_value_str(struct
> fence *fence,
> snprintf(str, size, "%d", parent->value);
> }
>
> -static const struct fence_ops timeline_fence_ops = {
> +const struct fence_ops timeline_fence_ops = {
> .get_driver_name = timeline_fence_get_driver_name,
> .get_timeline_name = timeline_fence_get_timeline_name,
> .enable_signaling = timeline_fence_enable_signaling,
> @@ -252,7 +244,7 @@ static long sw_sync_ioctl_create_fence(struct
> sync_timeline *obj,
> goto err;
> }
>
> - pt = sync_pt_create(obj, sizeof(*pt), data.value);
> + pt = sync_pt_create(obj, sizeof(*pt), data.value, data.name);
> if (!pt) {
> err = -ENOMEM;
> goto err;
> diff --git a/drivers/staging/android/sync_debug.c
> b/drivers/staging/android/sync_debug.c
> index 4c5a855..b732ea3 100644
> --- a/drivers/staging/android/sync_debug.c
> +++ b/drivers/staging/android/sync_debug.c
> @@ -75,13 +75,14 @@ static void sync_print_fence(struct seq_file *s, struct
> fence *fence, bool show)
> {
> int status = 1;
> struct sync_timeline *parent = fence_parent(fence);
> + struct sync_pt *pt = fence_to_sync_pt(fence);
>
> if (fence_is_signaled_locked(fence))
> status = fence->status;
>
> - seq_printf(s, " %s%sfence %s",
> + seq_printf(s, " %s%sfence %s %s",
> show ? parent->name : "",
> -show ? "_" : "",
> +show ? "_" : "", pt->name,
> sync_status_str(status));
>
NAK,
A fence in sync_print_fence can be of any type. If you want to print the name,
use the fence_value_str callback.
~Maarten
[Intel-gfx] include/drm/i915_drm.h:96: possible bad bitmask ?
On Mon, Aug 08, 2016 at 10:31:32AM +0100, David Binderman wrote: > Hello there, > > Recent versions of gcc say this: > > include/drm/i915_drm.h:96:34: warning: result of â65535 << 20â > requires 37 bits to represent, but âintâ only has 32 bits > [-Wshift-overflow=] > > Source code is > > #define INTEL_BSM_MASK (0x << 20) > > Maybe something like > > #define INTEL_BSM_MASK (0xUL<< 20) > > might be better. Yup. Care to bake this into a patch (with s-o-b and everything per Documentation/SubmittingPatches) so I can apply it? -Daniel > > > Regards > > David Binderman > ___ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Bug 97240] VCE encoding sometimes locks up since 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97240 Bug ID: 97240 Summary: VCE encoding sometimes locks up since 4.8-rc1 Product: DRI Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel at lists.freedesktop.org Reporter: haagch at frickel.club RX 480 with mesa git etc. Running this gstreamer pipeline gst-launch-1.0 -e filesrc location=big_buck_bunny_720p_1mb.mp4 ! qtdemux ! decodebin ! videoconvert ! omxh264enc ! h264parse ! matroskamux ! filesink location=/tmp/output.mkv works fine on 4.7. On 4.8-rc1 running it a couple of times randomly locks up the gst-launch process. Then while it hangs, running glxgears or so locks up everything. On the plus side, when it works, it works a lot quicker on 4.8. I'll look into bisecting unless someone else can reproduce and already knows why it happens. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/59a9e70c/attachment.html>
[PATCH] drm/bridge: analogix_dp: Remove duplicated code v2
+ Archit
Tomeu,
Thanks for the update :-)
But you have missed three tiny align problems, please see my inline
notes, wish you could fix them. After that this patch looks good to me, so:
Reviewed-by: Yakir Yang
I guess this patch should go through Archit's drm_bridge tree, so I
added him into the CC list.
- Yakir
On 08/05/2016 08:59 PM, Tomeu Vizoso wrote:
> Remove code for reading the EDID and DPCD fields and use the helpers
> instead.
>
> Besides the obvious code reduction, other helpers are being added to the
> core that could be used in this driver and will be good to be able to
> use them instead of duplicating them.
>
> Signed-off-by: Tomeu Vizoso
> Tested-by: Javier Martinez Canillas
> Tested-by: Sean Paul
> Cc: Javier Martinez Canillas
> Cc: Mika Kahola
> Cc: Yakir Yang
> Cc: Daniel Vetter
>
> v2:
> - A bunch of good fixes from Sean and Yakir
> - Moved the transfer function to analogix_dp_reg.c
> - Removed reference to the EDID from the dp struct
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 263
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 40 +-
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 451
> ++---
> 3 files changed, 204 insertions(+), 550 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 32715daf73cb..624fc4f44450 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -31,6 +31,7 @@
> #include
>
> #include "analogix_dp_core.h"
> +#include "analogix_dp_reg.h"
>
> #define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
>
> @@ -97,150 +98,21 @@ static int analogix_dp_detect_hpd(struct
> analogix_dp_device *dp)
> return 0;
> }
>
> -static unsigned char analogix_dp_calc_edid_check_sum(unsigned char
> *edid_data)
> -{
> - int i;
> - unsigned char sum = 0;
> -
> - for (i = 0; i < EDID_BLOCK_LENGTH; i++)
> - sum = sum + edid_data[i];
> -
> - return sum;
> -}
> -
> -static int analogix_dp_read_edid(struct analogix_dp_device *dp)
> -{
> - unsigned char *edid = dp->edid;
> - unsigned int extend_block = 0;
> - unsigned char sum;
> - unsigned char test_vector;
> - int retval;
> -
> - /*
> - * EDID device address is 0x50.
> - * However, if necessary, you must have set upper address
> - * into E-EDID in I2C device, 0x30.
> - */
> -
> - /* Read Extension Flag, Number of 128-byte EDID extension blocks */
> - retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
> - EDID_EXTENSION_FLAG,
> - &extend_block);
> - if (retval)
> - return retval;
> -
> - if (extend_block > 0) {
> - dev_dbg(dp->dev, "EDID data includes a single extension!\n");
> -
> - /* Read EDID data */
> - retval = analogix_dp_read_bytes_from_i2c(dp,
> - I2C_EDID_DEVICE_ADDR,
> - EDID_HEADER_PATTERN,
> - EDID_BLOCK_LENGTH,
> - &edid[EDID_HEADER_PATTERN]);
> - if (retval != 0) {
> - dev_err(dp->dev, "EDID Read failed!\n");
> - return -EIO;
> - }
> - sum = analogix_dp_calc_edid_check_sum(edid);
> - if (sum != 0) {
> - dev_err(dp->dev, "EDID bad checksum!\n");
> - return -EIO;
> - }
> -
> - /* Read additional EDID data */
> - retval = analogix_dp_read_bytes_from_i2c(dp,
> - I2C_EDID_DEVICE_ADDR,
> - EDID_BLOCK_LENGTH,
> - EDID_BLOCK_LENGTH,
> - &edid[EDID_BLOCK_LENGTH]);
> - if (retval != 0) {
> - dev_err(dp->dev, "EDID Read failed!\n");
> - return -EIO;
> - }
> - sum = analogix_dp_calc_edid_check_sum(&edid[EDID_BLOCK_LENGTH]);
> - if (sum != 0) {
> - dev_err(dp->dev, "EDID bad checksum!\n");
> - return -EIO;
> - }
> -
> - analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
> - &test_vector);
> - if (test_vector & DP_TEST_LINK_EDID_READ) {
> - analogix_dp_write_byte_to_dpcd(dp,
> - DP_TEST_EDID_CHECKSUM,
> - edid[EDID_BLOCK_LENGTH + EDID_CHECKSUM]);
> - analogix_dp_write_byte_to_dpcd(dp,
> - DP_TEST_RESPONSE,
> -
[PATCH] drm/etnaviv: take GPU lock later in the submit process
Both the fence and event alloc are safe to be done without holding the GPU
lock, as they either don't need any locking (fences) or are protected by
their own lock (events).
This solves a bad locking interaction between the submit path and the
recover worker. If userspace manages to exhaust all available events while
the GPU is hung, the submit will wait for events to become available
holding the GPU lock. The recover worker waits for this lock to become
available before trying to recover the GPU which frees up the allocated
events. Essentially both paths are deadlocked until the submit path
times out waiting for available events, failing the submit that could
otherwise be handled just fine if the recover worker had the chance to
bring the GPU back in a working state.
Signed-off-by: Lucas Stach
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 87ef34150d46..b382cf505262 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1333,8 +1333,6 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu,
if (ret < 0)
return ret;
- mutex_lock(&gpu->lock);
-
/*
* TODO
*
@@ -1348,16 +1346,18 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu,
if (unlikely(event == ~0U)) {
DRM_ERROR("no free event\n");
ret = -EBUSY;
- goto out_unlock;
+ goto out_pm_put;
}
fence = etnaviv_gpu_fence_alloc(gpu);
if (!fence) {
event_free(gpu, event);
ret = -ENOMEM;
- goto out_unlock;
+ goto out_pm_put;
}
+ mutex_lock(&gpu->lock);
+
gpu->event[event].fence = fence;
submit->fence = fence->seqno;
gpu->active_fence = submit->fence;
@@ -1395,9 +1395,9 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu,
hangcheck_timer_reset(gpu);
ret = 0;
-out_unlock:
mutex_unlock(&gpu->lock);
+out_pm_put:
etnaviv_gpu_pm_put(gpu);
return ret;
--
2.8.1
[PATCH] drm/bridge: analogix_dp: Remove duplicated code v2
On 8 August 2016 at 12:43, Yakir Yang wrote:
> + Archit
>
> Tomeu,
>
> Thanks for the update :-)
>
> But you have missed three tiny align problems, please see my inline notes,
> wish you could fix them. After that this patch looks good to me, so:
>
> Reviewed-by: Yakir Yang
Hi Yakir,
thanks for remembering the formatting issues and sorry for having
missed them before. I don't care much myself, but as the second line
isn't an argument to the function, I found it could be misleading if
it was aligned as if it was. A quick grep in the DRM sources showed
there isn't uniformity on this, so I'm happy to change it to whatever
the maintainer prefers.
> I guess this patch should go through Archit's drm_bridge tree, so I added
> him into the CC list.
Cool, thanks again. Archit, do you want me to submit a patch to
MAINTAINERS so the tools pick up your address for the right patches?
Thanks,
Tomeu
>
> - Yakir
>
>
>
> On 08/05/2016 08:59 PM, Tomeu Vizoso wrote:
>>
>> Remove code for reading the EDID and DPCD fields and use the helpers
>> instead.
>>
>> Besides the obvious code reduction, other helpers are being added to the
>> core that could be used in this driver and will be good to be able to
>> use them instead of duplicating them.
>>
>> Signed-off-by: Tomeu Vizoso
>> Tested-by: Javier Martinez Canillas
>> Tested-by: Sean Paul
>> Cc: Javier Martinez Canillas
>> Cc: Mika Kahola
>> Cc: Yakir Yang
>> Cc: Daniel Vetter
>>
>> v2:
>> - A bunch of good fixes from Sean and Yakir
>> - Moved the transfer function to analogix_dp_reg.c
>> - Removed reference to the EDID from the dp struct
>> ---
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 263
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 40 +-
>> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 451
>> ++---
>> 3 files changed, 204 insertions(+), 550 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 32715daf73cb..624fc4f44450 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -31,6 +31,7 @@
>> #include
>> #include "analogix_dp_core.h"
>> +#include "analogix_dp_reg.h"
>> #define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
>> @@ -97,150 +98,21 @@ static int analogix_dp_detect_hpd(struct
>> analogix_dp_device *dp)
>> return 0;
>> }
>> -static unsigned char analogix_dp_calc_edid_check_sum(unsigned char
>> *edid_data)
>> -{
>> - int i;
>> - unsigned char sum = 0;
>> -
>> - for (i = 0; i < EDID_BLOCK_LENGTH; i++)
>> - sum = sum + edid_data[i];
>> -
>> - return sum;
>> -}
>> -
>> -static int analogix_dp_read_edid(struct analogix_dp_device *dp)
>> -{
>> - unsigned char *edid = dp->edid;
>> - unsigned int extend_block = 0;
>> - unsigned char sum;
>> - unsigned char test_vector;
>> - int retval;
>> -
>> - /*
>> -* EDID device address is 0x50.
>> -* However, if necessary, you must have set upper address
>> -* into E-EDID in I2C device, 0x30.
>> -*/
>> -
>> - /* Read Extension Flag, Number of 128-byte EDID extension blocks
>> */
>> - retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
>> - EDID_EXTENSION_FLAG,
>> - &extend_block);
>> - if (retval)
>> - return retval;
>> -
>> - if (extend_block > 0) {
>> - dev_dbg(dp->dev, "EDID data includes a single
>> extension!\n");
>> -
>> - /* Read EDID data */
>> - retval = analogix_dp_read_bytes_from_i2c(dp,
>> - I2C_EDID_DEVICE_ADDR,
>> - EDID_HEADER_PATTERN,
>> - EDID_BLOCK_LENGTH,
>> -
>> &edid[EDID_HEADER_PATTERN]);
>> - if (retval != 0) {
>> - dev_err(dp->dev, "EDID Read failed!\n");
>> - return -EIO;
>> - }
>> - sum = analogix_dp_calc_edid_check_sum(edid);
>> - if (sum != 0) {
>> - dev_err(dp->dev, "EDID bad checksum!\n");
>> - return -EIO;
>> - }
>> -
>> - /* Read additional EDID data */
>> - retval = analogix_dp_read_bytes_from_i2c(dp,
>> - I2C_EDID_DEVICE_ADDR,
>> - EDID_BLOCK_LENGTH,
>> - EDID_BLOCK_LENGTH,
>> - &edid[EDID_BLOCK_LENGTH]);
>> - if (retval != 0) {
>> - dev_err(dp->dev, "EDID Read failed!\n");
>> - return -EIO;
>> - }
>> -
[PATCH 2/2] drm/i915: Drop reference to current state wait req as soon as it goes unused
There are two paths into intel_cleanup_plane_fb, the normal completion
path and the failure path.
In the failure case, intel_cleanup_plane_fb is called before
drm_atomic_helper_swap_state, so any wait_req reference made in
intel_prepare_plane_fb will be in old_intel_state->wait_req.
In the normal completion path, wait_req is not freed until
the next commit, which is no longer used after waiting.
Free it as soon as possible, so we don't hold on to it indefinitely.
Signed-off-by: Maarten Lankhorst
Cc: Keith Packard
Cc: Daniel Vetter
Cc: David Airlie
Cc: intel-gfx at lists.freedesktop.org
Cc: dri-devel at lists.freedesktop.org
Fixes: 849782575325 ("drm/i915: cleanup_plane_fb: also drop reference to
current state wait_req")
---
drivers/gpu/drm/i915/intel_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f56707289615..e72ad97998a4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13754,6 +13754,8 @@ static void intel_atomic_commit_tail(struct
drm_atomic_state *state)
/* EIO should be eaten, and we can't get interrupted in the
* worker, and blocking commits have waited already. */
WARN_ON(ret);
+
+ i915_gem_request_assign(&intel_plane_state->wait_req, NULL);
}
if (!state->legacy_cursor_update)
@@ -14190,7 +14192,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
const struct drm_plane_state *old_state)
{
struct drm_device *dev = plane->dev;
- struct intel_plane_state *intel_state =
to_intel_plane_state(plane->state);
struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
struct intel_plane_state *old_intel_state =
to_intel_plane_state(old_state);
@@ -14199,7 +14200,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
!INTEL_INFO(dev)->cursor_needs_physical))
intel_unpin_fb_obj(old_state->fb, old_state->rotation);
- i915_gem_request_assign(&intel_state->wait_req, NULL);
i915_gem_request_assign(&old_intel_state->wait_req, NULL);
}
--
2.7.4
[Bug 97240] VCE encoding sometimes locks up since 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97240 --- Comment #1 from Christoph Haag --- Created attachment 125591 --> https://bugs.freedesktop.org/attachment.cgi?id=125591&action=edit dmesg Almost forgot dmesg. I think these messages correspond with the hang: [ 64.406834] failed to send pre message 15b ret is 0 [ 106.006518] failed to send pre message 155 ret is 0 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/ef670b46/attachment.html>
[PATCH 2/3] drm/i2c: tda998x: Register ASoC hdmi-codec and add audio DT binding
On 08/06/16 01:19, Russell King - ARM Linux wrote: >> > It'll pick up that as the DT device to hang things off which I'd expect >> > to be the desired outcome given that this is a very similar situation to >> > the MFD situation. I've not been following the full thread so there is >> > probably context I'm missing here... > Okay, and that sounds to me like a very reasonable thing to want to > happen - so that the audio side can be clearly identified as being > coupled with the video side. > > So, I'm not seeing a problem with my suggestion... I'm actually more > reasons why it's a good thing to want. Ok, so getting back to this comment: >> +priv->audio_pdev = platform_device_register_data( >> > + dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO, >> > + &codec_data, sizeof(codec_data)); > I'd much prefer that this was a child of the I2C device, which will > show the relationship between this virtual platform device and the > device which it's associated with. That can be done via > platform_device_register_full(). The platform_device_register_data() sets the parent of the registered platform device according to the first parameter, the tda998x i2c-client in this case. Is there some reason why I should use platform_device_register_full() and should I set parent to something else than the tda998x i2c device? BR, Jyri
[PATCH v2 2/3] drm: simpledrm: add fbdev fallback support
Den 06.08.2016 00:38, skrev Paul Gortmaker: > On Fri, Aug 5, 2016 at 11:44 AM, Noralf Trønnes > wrote: >> Create a simple fbdev device during SimpleDRM setup so legacy user-space >> and fbcon can use it. >> >> Original work by David Herrmann. >> >> Cc: dh.herrmann at gmail.com >> Signed-off-by: Noralf Trønnes >> --- >> >> Changes from version 1: >>No changes >> >> Changes from previous version: >> - Remove the DRM_SIMPLEDRM_FBDEV kconfig option and use DRM_FBDEV_EMULATION >> - Suspend fbcon/fbdev when the pipeline is enabled, resume in lastclose >> - Add FBINFO_CAN_FORCE_OUTPUT flag so we get oops'es on the console >> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c >> b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c >> new file mode 100644 >> index 000..b83646b >> --- /dev/null >> +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c >> @@ -0,0 +1,160 @@ >> +/* >> + * SimpleDRM firmware framebuffer driver >> + * Copyright (c) 2012-2014 David Herrmann >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License as published by the >> Free >> + * Software Foundation; either version 2 of the License, or (at your option) >> + * any later version. >> + */ >> + >> +/* >> + * fbdev compatibility layer >> + * We provide a basic fbdev device for the same framebuffer that is used for >> + * the pseudo CRTC. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include > You should not need module.h here since this file is not doing the > module_init or module_exit or MODULE_ALIAS etc etc. > > An empty file with just module.h in it outputs about 750k of goo > from cpp, so it is best avoided wherever not strictly needed. I've never thought of superfluous includes in terms of compile time before, but that makes sense, especially on a large project like this. Thanks, Noralf. > Thanks, > Paul. > -- > >> +#include >> +#include >> +#include "simpledrm.h" >> +
[Bug 97240] VCE encoding sometimes locks up since 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97240 Christian König changed: What|Removed |Added CC||deathsimple at vodafone.de --- Comment #2 from Christian König --- Sounds like a power management problem to me. Could you bisect? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/285d5b57/attachment.html>
[PATCH v9] drm/i915/skl: Update DDB values atomically with wms/plane attrs
Now that we can hook into update_crtcs and control the order in which we
update CRTCs at each modeset, we can finish the final step of fixing
Skylake's watermark handling by performing DDB updates at the same time
as plane updates and watermark updates.
The first major change in this patch is skl_update_crtcs(), which
handles ensuring that we order each CRTC update in our atomic commits
properly so that they honor the DDB flush order.
The second major change in this patch is the order in which we flush the
pipes. While the previous order may have worked, it can't be used in
this approach since it no longer will do the right thing. For example,
using the old ddb flush order:
We have pipes A, B, and C enabled, and we're disabling C. Initial ddb
allocation looks like this:
| A | B |xxx|
Since we're performing the ddb updates after performing any CRTC
disablements in intel_atomic_commit_tail(), the space to the right of
pipe B is unallocated.
1. Flush pipes with new allocation contained into old space. None
apply, so we skip this
2. Flush pipes having their allocation reduced, but overlapping with a
previous allocation. None apply, so we also skip this
3. Flush pipes that got more space allocated. This applies to A and B,
giving us the following update order: A, B
This is wrong, since updating pipe A first will cause it to overlap with
B and potentially burst into flames. Our new order (see the code
comments for details) would update the pipes in the proper order: B, A.
As well, we calculate the order for each DDB update during the check
phase, and reference it later in the commit phase when we hit
skl_update_crtcs().
This long overdue patch fixes the rest of the underruns on Skylake.
Changes since v1:
- Add skl_ddb_entry_write() for cursor into skl_write_cursor_wm()
Changes since v2:
- Use the method for updating CRTCs that Ville suggested
- In skl_update_wm(), only copy the watermarks for the crtc that was
passed to us
Changes since v3:
- Small comment fix in skl_ddb_allocation_overlaps()
Fixes: 0e8fb7ba7ca5 ("drm/i915/skl: Flush the WM configuration")
Fixes: 8211bd5bdf5e ("drm/i915/skl: Program the DDB allocation")
[omitting CC for stable, since this patch will need to be changed for
such backports first]
Testcase: kms_cursor_legacy
Signed-off-by: Lyude
Reviewed-by: Maarten Lankhorst
Cc: Ville Syrjälä
Cc: Daniel Vetter
Cc: Radhakrishna Sripada
Cc: Hans de Goede
Cc: Matt Roper
---
drivers/gpu/drm/i915/intel_display.c | 100 +++--
drivers/gpu/drm/i915/intel_drv.h | 7 ++
drivers/gpu/drm/i915/intel_pm.c | 207 +--
3 files changed, 144 insertions(+), 170 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 980b6fd..ad5f6e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12903,16 +12903,23 @@ static void verify_wm_state(struct drm_crtc *crtc,
hw_entry->start, hw_entry->end);
}
- /* cursor */
- hw_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
- sw_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
-
- if (!skl_ddb_entry_equal(hw_entry, sw_entry)) {
- DRM_ERROR("mismatch in DDB state pipe %c cursor "
- "(expected (%u,%u), found (%u,%u))\n",
- pipe_name(pipe),
- sw_entry->start, sw_entry->end,
- hw_entry->start, hw_entry->end);
+ /*
+* cursor
+* If the cursor plane isn't active, we may not have updated it's ddb
+* allocation. In that case since the ddb allocation will be updated
+* once the plane becomes visible, we can skip this check
+*/
+ if (intel_crtc->cursor_addr) {
+ hw_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
+ sw_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
+
+ if (!skl_ddb_entry_equal(hw_entry, sw_entry)) {
+ DRM_ERROR("mismatch in DDB state pipe %c cursor "
+ "(expected (%u,%u), found (%u,%u))\n",
+ pipe_name(pipe),
+ sw_entry->start, sw_entry->end,
+ hw_entry->start, hw_entry->end);
+ }
}
}
@@ -13664,6 +13671,72 @@ static void intel_update_crtcs(struct drm_atomic_state
*state,
}
}
+static void skl_update_crtcs(struct drm_atomic_state *state,
+unsigned int *crtc_vblank_mask)
+{
+ struct drm_device *dev = state->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *old_crtc_state;
+ struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+ struct skl_ddb_allocation
[PATCH v9] drm/i915/skl: Update DDB values atomically with wms/plane attrs
Now that we can hook into update_crtcs and control the order in which we
update CRTCs at each modeset, we can finish the final step of fixing
Skylake's watermark handling by performing DDB updates at the same time
as plane updates and watermark updates.
The first major change in this patch is skl_update_crtcs(), which
handles ensuring that we order each CRTC update in our atomic commits
properly so that they honor the DDB flush order.
The second major change in this patch is the order in which we flush the
pipes. While the previous order may have worked, it can't be used in
this approach since it no longer will do the right thing. For example,
using the old ddb flush order:
We have pipes A, B, and C enabled, and we're disabling C. Initial ddb
allocation looks like this:
| A | B |xxx|
Since we're performing the ddb updates after performing any CRTC
disablements in intel_atomic_commit_tail(), the space to the right of
pipe B is unallocated.
1. Flush pipes with new allocation contained into old space. None
apply, so we skip this
2. Flush pipes having their allocation reduced, but overlapping with a
previous allocation. None apply, so we also skip this
3. Flush pipes that got more space allocated. This applies to A and B,
giving us the following update order: A, B
This is wrong, since updating pipe A first will cause it to overlap with
B and potentially burst into flames. Our new order (see the code
comments for details) would update the pipes in the proper order: B, A.
As well, we calculate the order for each DDB update during the check
phase, and reference it later in the commit phase when we hit
skl_update_crtcs().
This long overdue patch fixes the rest of the underruns on Skylake.
Changes since v1:
- Add skl_ddb_entry_write() for cursor into skl_write_cursor_wm()
Changes since v2:
- Use the method for updating CRTCs that Ville suggested
- In skl_update_wm(), only copy the watermarks for the crtc that was
passed to us
Changes since v3:
- Small comment fix in skl_ddb_allocation_overlaps()
Fixes: 0e8fb7ba7ca5 ("drm/i915/skl: Flush the WM configuration")
Fixes: 8211bd5bdf5e ("drm/i915/skl: Program the DDB allocation")
[omitting CC for stable, since this patch will need to be changed for
such backports first]
Testcase: kms_cursor_legacy
Signed-off-by: Lyude
Reviewed-by: Maarten Lankhorst
Cc: Ville Syrjälä
Cc: Daniel Vetter
Cc: Radhakrishna Sripada
Cc: Hans de Goede
Cc: Matt Roper
---
drivers/gpu/drm/i915/intel_display.c | 100 +++--
drivers/gpu/drm/i915/intel_drv.h | 7 ++
drivers/gpu/drm/i915/intel_pm.c | 207 +--
3 files changed, 144 insertions(+), 170 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 980b6fd..ad5f6e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12903,16 +12903,23 @@ static void verify_wm_state(struct drm_crtc *crtc,
hw_entry->start, hw_entry->end);
}
- /* cursor */
- hw_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
- sw_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
-
- if (!skl_ddb_entry_equal(hw_entry, sw_entry)) {
- DRM_ERROR("mismatch in DDB state pipe %c cursor "
- "(expected (%u,%u), found (%u,%u))\n",
- pipe_name(pipe),
- sw_entry->start, sw_entry->end,
- hw_entry->start, hw_entry->end);
+ /*
+* cursor
+* If the cursor plane isn't active, we may not have updated it's ddb
+* allocation. In that case since the ddb allocation will be updated
+* once the plane becomes visible, we can skip this check
+*/
+ if (intel_crtc->cursor_addr) {
+ hw_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
+ sw_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
+
+ if (!skl_ddb_entry_equal(hw_entry, sw_entry)) {
+ DRM_ERROR("mismatch in DDB state pipe %c cursor "
+ "(expected (%u,%u), found (%u,%u))\n",
+ pipe_name(pipe),
+ sw_entry->start, sw_entry->end,
+ hw_entry->start, hw_entry->end);
+ }
}
}
@@ -13664,6 +13671,72 @@ static void intel_update_crtcs(struct drm_atomic_state
*state,
}
}
+static void skl_update_crtcs(struct drm_atomic_state *state,
+unsigned int *crtc_vblank_mask)
+{
+ struct drm_device *dev = state->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *old_crtc_state;
+ struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+ struct skl_ddb_allocation
[PATCH 0/2] Remove unused parameter evict from ttm_bo_move_{ttm, memcpy}
On Mon, Aug 8, 2016 at 5:08 AM, Christian König
wrote:
> Am 08.08.2016 um 05:28 schrieb Michel Dänzer:
>>
>> From: Michel Dänzer
>
>
> Reviewed-by: Christian König
Applied. thanks!
Alex
>
>>
>> I noticed that these parameters are unused.
>>
>> These patches apply on top of 34b58355ad1d ("drm/ttm: Wait for a BO to
>> become idle before unbinding it from GTT"), which is being merged to 4.8
>> via Alex's tree.
>>
>> Michel Dänzer (2):
>>drm/ttm: Remove unused parameter evict from ttm_bo_move_ttm
>>drm/ttm: Remove unused parameter evict from ttm_bo_move_memcpy
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 +++
>> drivers/gpu/drm/nouveau/nouveau_bo.c| 6 +++---
>> drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
>> drivers/gpu/drm/radeon/radeon_ttm.c | 7 +++
>> drivers/gpu/drm/ttm/ttm_bo.c| 6 ++
>> drivers/gpu/drm/ttm/ttm_bo_util.c | 7 +++
>> include/drm/ttm/ttm_bo_driver.h | 7 ++-
>> 7 files changed, 18 insertions(+), 26 deletions(-)
>>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100871] radeon fails to initialize one DisplayPort monitor
https://bugzilla.kernel.org/show_bug.cgi?id=100871 --- Comment #10 from Charles R. Anderson --- Problem still exists in Linux 4.6.4 in Fedora 24 (kernel-4.6.4-301.fc24.x86_64). -- You are receiving this mail because: You are watching the assignee of the bug.
[Intel-gfx] [PATCH 2/2] drm/i915: Drop reference to current state wait req as soon as it goes unused
On Mon, Aug 8, 2016 at 1:23 PM, Maarten Lankhorst
wrote:
> There are two paths into intel_cleanup_plane_fb, the normal completion
> path and the failure path.
>
> In the failure case, intel_cleanup_plane_fb is called before
> drm_atomic_helper_swap_state, so any wait_req reference made in
> intel_prepare_plane_fb will be in old_intel_state->wait_req.
>
> In the normal completion path, wait_req is not freed until
> the next commit, which is no longer used after waiting.
>
> Free it as soon as possible, so we don't hold on to it indefinitely.
>
> Signed-off-by: Maarten Lankhorst
> Cc: Keith Packard
> Cc: Daniel Vetter
> Cc: David Airlie
> Cc: intel-gfx at lists.freedesktop.org
> Cc: dri-devel at lists.freedesktop.org
> Fixes: 849782575325 ("drm/i915: cleanup_plane_fb: also drop reference to
> current state wait_req")
We still need to clean up the reference in case of failure, which
means latest in intel_plane_destroy_state(). Also hanging onto a
request isn't that evil really, why can't we just only clean up in the
destroy function?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f56707289615..e72ad97998a4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13754,6 +13754,8 @@ static void intel_atomic_commit_tail(struct
> drm_atomic_state *state)
> /* EIO should be eaten, and we can't get interrupted in the
> * worker, and blocking commits have waited already. */
> WARN_ON(ret);
> +
> + i915_gem_request_assign(&intel_plane_state->wait_req, NULL);
> }
>
> if (!state->legacy_cursor_update)
> @@ -14190,7 +14192,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>const struct drm_plane_state *old_state)
> {
> struct drm_device *dev = plane->dev;
> - struct intel_plane_state *intel_state =
> to_intel_plane_state(plane->state);
> struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
> struct intel_plane_state *old_intel_state =
> to_intel_plane_state(old_state);
> @@ -14199,7 +14200,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
> !INTEL_INFO(dev)->cursor_needs_physical))
> intel_unpin_fb_obj(old_state->fb, old_state->rotation);
>
> - i915_gem_request_assign(&intel_state->wait_req, NULL);
> i915_gem_request_assign(&old_intel_state->wait_req, NULL);
> }
>
> --
> 2.7.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/i2c: tda998x: don't register the connector
Hi, On Mon, Jul 25, 2016 at 05:08:21PM +0200, Daniel Vetter wrote: >On Mon, Jul 25, 2016 at 01:54:06PM +0100, Brian Starkey wrote: >> Hi Russell, >> >> On Mon, Jul 25, 2016 at 01:25:04PM +0100, Russell King - ARM Linux wrote: >> > On Mon, Jul 25, 2016 at 11:55:48AM +0100, Brian Starkey wrote: >> > > The connector shouldn't be registered until the rest of the whole device >> > > is set up, so that consistent state is presented to userspace. >> > > >> > > As drm_dev_register() now registers all of the connectors anyway, >> > > there's no need to explicitly do it in individual drivers so remove >> > > the calls to drm_connector_register()/drm_connector_unregister(). >> > > >> > > This allows componentised drivers to use tda998x without having racy >> > > initialisation. >> > >> > Is there a corresponding patch for armada-drm so that the cubox doesn't >> > regress? Has it already been merged? >> > >> >> A patch for armada-drm to do what? >> >> I should perhaps have explicitly mentioned that this change depends >> on e28cd4d0a223: "drm: Automatically register/unregister all >> connectors", which is in drm-next. >> >> Like my commit message says - after the above commit, all connectors >> are automatically registered in drm_dev_register() - so I don't >> anticipate any regression, but I don't have a cubox to test. >> >> armada-drm seems to be doing effectively the same thing as arm/hdlcd, >> which works fine after this patch with no other changes. >> >> Let me know if I've missed something; or if you are able to test on >> cubox that would be great. > >Ack from my side on generally nuking drm_connector_register() from >everywhere except truely hotplugged connectors like dp mst. It should keep >working for everyone. Only exception is if there's a driver which calls >drm_dev_register too early (before all connectors are probed), which would >be a bug anyway. Right; the motivation for this change is to fix the init order in HDLCD and Mali-DP (move drm_dev_register to the end), which we can't do right now because tda998x expects the DRM device sysfs to be set up in bind. @Daniel: Can I take this as your Acked-by? Should this go in via Russell's tree? @Russell, are you happy with this change? Thanks, Brian >-Daniel >-- >Daniel Vetter >Software Engineer, Intel Corporation >http://blog.ffwll.ch >
[PATCH] drm: Make sure drm_vblank_no_hw_counter isn't abused
Shouldn't be possible since everyone kzallocs this, but better safe
than sorry. Random drive-by-idea really.
Cc: Rodrigo Vivi
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index f5a9f8cef360..10611a936059 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1795,6 +1795,7 @@ EXPORT_SYMBOL(drm_crtc_handle_vblank);
*/
u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe)
{
+ WARN_ON_ONCE(dev->max_vblank_count != 0);
return 0;
}
EXPORT_SYMBOL(drm_vblank_no_hw_counter);
--
2.8.1
[PATCH] drm: Make sure drm_vblank_no_hw_counter isn't abused
Reviewed-by: Rodrigo Vivi
On Mon, 2016-08-08 at 18:24 +0200, Daniel Vetter wrote:
> Shouldn't be possible since everyone kzallocs this, but better safe
> than sorry. Random drive-by-idea really.
>
> Cc: Rodrigo Vivi
> Signed-off-by: Daniel Vetter
> ---
> Â drivers/gpu/drm/drm_irq.c | 1 +
> Â 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index f5a9f8cef360..10611a936059 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1795,6 +1795,7 @@ EXPORT_SYMBOL(drm_crtc_handle_vblank);
> Â */
> Â u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int
> pipe)
> Â {
> + WARN_ON_ONCE(dev->max_vblank_count != 0);
> Â return 0;
> Â }
> Â EXPORT_SYMBOL(drm_vblank_no_hw_counter);
[PATCH 0/7] Minor DP aux transaction fixes
On Fri, Aug 5, 2016 at 8:30 PM, Lyude wrote: > While I was investigating an unrelated bug on the radeon driver, I noticed > that > it's become rather difficult to actually read through dmesg with drm.debug > turned on, on account of the huge number of messages we end up printing from > failed DP aux transactions that happen every time we reprobe each connector. > > Timed out transactions are relatively normal, and as well there's a lot of > places in radeon/amdgpu where we're printing redundant debugging information > dozens of times each time we attempt a DP aux transactions. > > Additionally, I've removed some of the retry loops in amdgpu/radeon. These > were > definitely useful at one point, but since we now retry any failed aux > transaction unconditionally in DRM's dp helpers they don't serve much purpose > other then to make failing aux transactions take a lot more time then they > need > to. I've applied the amdgpu and radeon patches. For the drm patches, I can either take them through my tree or via drm-misc. Alex > > Lyude (7): > drm/dp_helper: Print first error received on failure in > drm_dp_dpcd_access() > drm/radeon: Don't print error on aux transaction timeouts > drm/radeon: Don't retry 7 times in radeon_dp_dpcd() > drm/amdgpu: Don't print error on aux transaction timeouts > drm/amdgpu: Don't retry 7 times in amdgpu_atombios_dp_get_dpcd() > drm: Add ratelimited versions of the DRM_DEBUG* macros > drm/dp_helper: Rate limit timeout errors from drm_dp_i2c_do_msg() > > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 22 ++ > drivers/gpu/drm/drm_dp_helper.c | 14 -- > drivers/gpu/drm/radeon/atombios_dp.c | 21 ++--- > drivers/gpu/drm/radeon/radeon_dp_auxch.c | 1 - > include/drm/drmP.h | 30 ++ > 5 files changed, 62 insertions(+), 26 deletions(-) > > -- > 2.7.4 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
VT Switching with Atomic Modeset
On Mon, 8 Aug 2016, Daniel Vetter wrote: > On Sun, Aug 07, 2016 at 06:41:11PM -0500, Scot Doyle wrote: > > Hi all, > > > > I'm interested in discussing ways compositors could cooperate > > without CONFIG_VT/VT_VACTIVATE/VT_SETMODE. > > > > Daniel wrote up a nice article "VT Switching with Atomic Modeset" [1] > > inviting discussion on dri-devel (about much more than, but including > > this), David posted recently about it on dri-devel [2] and Noralf also > > posted the "DRM text mode" patch series. Hi Daniel, thanks for taking the time to respond. > Which problem are you trying to solve by avoiding the system compositor > part? Providing a transition path away from CONFIG_VT for those users who would otherwise prefer CONFIG_VT to using a system compositor. And as a bonus decreasing the usage of CONFIG_VT. > Afaiui there's not just the drm console, but all the other resources > assigned to a seat (input, audio, whatever) which also need to be > switched. And doing that imo makes much more sense if it's all done in > userspace. I agree that #5-#9 aren't necessary in the case of a system compositor. Would they would make sense as a config option? > Other parts of your plan (like a sysrq for the drm_text console) makes > sense, but those also don't need new ioctls. > -Daniel > > > So, maybe I could continue the discussion by proposing some DRM > > interface additions. > > > > The goals of the proposal are to > > - not affect current CONFIG_VT operation > > - enable compositor switching [1] > > - without a system compositor or intermediary [1] > > - without CONFIG_VT [2] > > - be compatible with the in-kernel DRM text mode[3] > > - provide manual device reset for aberrant states [1] > > - don't consume kernel memory unnecessarily [1] > > - provide DRM clients with access to device boot state [1] > > - remain compatible with legacy DRM/KMS interface [1] > > - remain compatible with future atomic properties [1] > > - leave as many policy decisions to userspace as practical [1] > > > > The "boot state" is the device state after any hardware and firmware > > initialization, but before the DRM device driver is loaded. Since > > different DRM clients (compositors) may want access to this state, > > and no intermediary userspace process should be required, this state > > should be stored in the kernel. > > > > Daniel referred to the "reset" state as "FBDEV resets to defaults" [1]. > > I understand it to be the state of a device after the driver has performed > > some positive set of actions to re-initialize the device. It may be more > > or less desirable from a user's or compositor's perspective than boot > > state. > > > > Ideas 5-9 below are not necessary to switch compositors without an > > intermediary, e.g. a compositor could obtain the current drm client > > process id and send a pre-agreed signal. However, since the DRM interface > > already includes the idea of exclusive access to modesetting > > (SET_MASTER/DROP_MASTER), it seemed fitting to extend it using the > > existing ioctl and event model. > > > > > > Implementation overview > > > > 1. Create a SYSRQ that restores all DRM devices to reset state > > > > 2. Kernel saves boot state of atomic-capable DRM devices at driver load > > > > 3. Create new ioctl DRM_IOCTL_DISCARD_BOOT_STATE > >- frees kernel memory used in #2, if boot state not needed > > > > 4. Provide (optional) mechanism to atomic modeset starting from either > >boot or reset state > >- Daniel discussed two possible mechanisms in [1], an extension to > > the GET_PROPERTY ioctl or a new flag for the MODE_ATOMIC ioctl > >- if boot state selected but unavailable (see #3), return error > > > > 5. Create new ioctl DRM_IOCTL_VOLUNTEER_MASTER > >- indicating current master plans to cooperate with other clients > >- reset in each successful DRM_IOCTL_SET_MASTER ioctl > > > > 6. Create new ioctl DRM_IOCTL_REQUEST_MASTER > >- if no root permissions (like DRM_IOCTL_SET_MASTER), return error > >- if no current master, then return error indicating such > >- if current master didn't VOLUNTEER_MASTER, return error > >- else send event DRM_EVENT_MASTER_REQUESTED to current master > > > > 7. Create new event DRM_EVENT_MASTER_REQUESTED > >- sent via the DRM asynchronous read/poll interface > >- recipient may ignore event (e.g. when using CONFIG_VT) > >- recipient, or its agent, may drop master > > > > 8. Modify DRM_IOCTL_DROP_MASTER ioctl > >- send event DRM_EVENT_MASTER_DROPPED > >- to clients that requested master since the latter of > > - the last time this event was sent > > - the last successful DRM_IOCTL_SET_MASTER ioctl > > > > 9. Create new event DRM_EVENT_MASTER_DROPPED > >- sent via the DRM asynchronous read/poll interface > >- recipie
[PATCH] drm: Make sure drm_vblank_no_hw_counter isn't abused
On Mon, Aug 8, 2016 at 12:27 PM, Vivi, Rodrigo
wrote:
> Reviewed-by: Rodrigo Vivi
>
> On Mon, 2016-08-08 at 18:24 +0200, Daniel Vetter wrote:
>> Shouldn't be possible since everyone kzallocs this, but better safe
>> than sorry. Random drive-by-idea really.
>>
>> Cc: Rodrigo Vivi
>> Signed-off-by: Daniel Vetter
Applied to drm-misc
>> ---
>> drivers/gpu/drm/drm_irq.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> index f5a9f8cef360..10611a936059 100644
>> --- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -1795,6 +1795,7 @@ EXPORT_SYMBOL(drm_crtc_handle_vblank);
>> */
>> u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int
>> pipe)
>> {
>> + WARN_ON_ONCE(dev->max_vblank_count != 0);
>> return 0;
>> }
>> EXPORT_SYMBOL(drm_vblank_no_hw_counter);
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/bridge: analogix_dp: Ensure the panel is properly prepared/unprepared
Instead of just preparing the panel on bind, actually prepare/unprepare
during modeset/disable. The panel must be prepared in order to read hpd
status and edid, so we need to keep state around the prepares in order
to ensure we don't accidentally turn the panel off at the wrong time.
Signed-off-by: Sean Paul
---
Changes in v2:
- Added panel_is_modeset state/lock to avoid racing detect with modeset
(marcheu)
- Added prepare/unprepare in .get_modes (yakir)
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 101 ++---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +
2 files changed, 93 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 32715da..47c449a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -923,11 +923,63 @@ static void analogix_dp_commit(struct analogix_dp_device
*dp)
analogix_dp_start_video(dp);
}
+/*
+ * This function is a bit of a catch-all for panel preparation, hopefully
+ * simplifying the logic of functions that need to prepare/unprepare the panel
+ * below.
+ *
+ * If @prepare is true, this function will prepare the panel. Conversely, if it
+ * is false, the panel will be unprepared.
+ *
+ * If @is_modeset_prepare is true, the function will disregard the current
state
+ * of the panel and either prepare/unprepare the panel based on @prepare. Once
+ * it finishes, it will update dp->panel_is_modeset to reflect the current
state
+ * of the panel.
+ */
+static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
+bool prepare, bool is_modeset_prepare)
+{
+ int ret = 0;
+
+ if (!dp->plat_data->panel)
+ return 0;
+
+ mutex_lock(&dp->panel_lock);
+
+ /*
+* Exit early if this is a temporary prepare/unprepare and we're already
+* modeset (since we neither want to prepare twice or unprepare early).
+*/
+ if (dp->panel_is_modeset && !is_modeset_prepare)
+ goto out;
+
+ if (prepare)
+ ret = drm_panel_prepare(dp->plat_data->panel);
+ else
+ ret = drm_panel_unprepare(dp->plat_data->panel);
+
+ if (ret)
+ goto out;
+
+ if (is_modeset_prepare)
+ dp->panel_is_modeset = prepare;
+
+out:
+ mutex_unlock(&dp->panel_lock);
+ return ret;
+}
+
int analogix_dp_get_modes(struct drm_connector *connector)
{
struct analogix_dp_device *dp = to_dp(connector);
struct edid *edid = (struct edid *)dp->edid;
- int num_modes = 0;
+ int ret, num_modes = 0;
+
+ ret = analogix_dp_prepare_panel(dp, true, false);
+ if (ret) {
+ DRM_ERROR("Failed to prepare panel (%d)\n", ret);
+ return 0;
+ }
if (analogix_dp_handle_edid(dp) == 0) {
drm_mode_connector_update_edid_property(&dp->connector, edid);
@@ -940,6 +992,10 @@ int analogix_dp_get_modes(struct drm_connector *connector)
if (dp->plat_data->get_modes)
num_modes += dp->plat_data->get_modes(dp->plat_data, connector);
+ ret = analogix_dp_prepare_panel(dp, false, false);
+ if (ret)
+ DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
+
return num_modes;
}
@@ -960,11 +1016,23 @@ enum drm_connector_status
analogix_dp_detect(struct drm_connector *connector, bool force)
{
struct analogix_dp_device *dp = to_dp(connector);
+ enum drm_connector_status status = connector_status_disconnected;
+ int ret;
- if (analogix_dp_detect_hpd(dp))
+ ret = analogix_dp_prepare_panel(dp, true, false);
+ if (ret) {
+ DRM_ERROR("Failed to prepare panel (%d)\n", ret);
return connector_status_disconnected;
+ }
+
+ if (!analogix_dp_detect_hpd(dp))
+ status = connector_status_connected;
- return connector_status_connected;
+ ret = analogix_dp_prepare_panel(dp, false, false);
+ if (ret)
+ DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
+
+ return status;
}
static void analogix_dp_connector_destroy(struct drm_connector *connector)
@@ -1035,6 +1103,16 @@ static int analogix_dp_bridge_attach(struct drm_bridge
*bridge)
return 0;
}
+static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge)
+{
+ struct analogix_dp_device *dp = bridge->driver_private;
+ int ret;
+
+ ret = analogix_dp_prepare_panel(dp, true, true);
+ if (ret)
+ DRM_ERROR("failed to setup the panel ret = %d\n", ret);
+}
+
static void analogix_dp_bridge_enable(struct drm_bridge *bridge)
{
struct analogix_dp_device *dp = bridge->driver_private;
@@ -1058,6 +1136,7 @@ static void analogix_dp_bridge_enable(struct drm_bridge
*bridge)
static void analo
[Bug 97248] [regression] [amdgpu] New regression in 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97248 Bug ID: 97248 Summary: [regression] [amdgpu] New regression in 4.8-rc1 Product: DRI Version: DRI git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel at lists.freedesktop.org Reporter: mike at fireburn.co.uk Created attachment 125605 --> https://bugs.freedesktop.org/attachment.cgi?id=125605&action=edit dmesg I'm seeing a new regression since 4.8-rc1 on Linus's tree, drm-next, agd5f's 4.9-next-wip since 4.8-rc1 was merged. I'm about to start bisecting [ 12.644973] amdgpu :01:00.0: Refused to change power state, currently in D3 [ 12.720886] amdgpu :01:00.0: Refused to change power state, currently in D3 [ 12.733891] amdgpu :01:00.0: Refused to change power state, currently in D3 [ 12.958093] amdgpu :01:00.0: Wait for MC idle timedout ! [ 13.071062] amdgpu :01:00.0: Wait for MC idle timedout ! [ 13.081607] [ powerplay ] Invalid VramInfo table.Failed to initialize MC reg table! [ 13.770954] [drm:gfx_v8_0_ring_test_ring] *ERROR* amdgpu: ring 0 test failed (scratch(0xC040)=0x) [ 13.772803] [drm:amdgpu_resume] *ERROR* resume of IP block failed -22 [ 13.774713] [drm:amdgpu_resume_kms] *ERROR* amdgpu_resume failed (-22). [ 13.776643] amdgpu :01:00.0: couldn't schedule ib [ 13.778602] [drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22) [ 13.805924] amdgpu :01:00.0: couldn't schedule ib [ 13.805928] [drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22) [ 18.934066] [drm:amdgpu_suspend] *ERROR* suspend of IP block failed -110 [ 18.943133] [drm:amdgpu_suspend] *ERROR* set_clockgating_state(ungate) of IP block failed -16 These are probably the most relevant messages -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/bfe36ff0/attachment.html>
[Bug 97248] [regression] [amdgpu] New regression in 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97248 --- Comment #1 from Alex Deucher --- does reverting c63695cc5e5f685e924e25a8f9555f6e846f1fc6 help? drm/amdgpu: work around lack of upstream ACPI support for D3cold Until Dave's patch to support the new hybrid gfx ACPI method goes upstream, we can fallback to the old ATPX method which seems to still work. Signed-off-by: Alex Deucher The d3cold patches went upstream in 4.8, so we should drop the workarounds. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/ccdbc239/attachment.html>
[PATCH] drm/rockchip: Properly adjust to a true clock in adjusted_mode
From: Douglas Anderson When fixing up the clock in vop_crtc_mode_fixup() we're not doing it quite correctly. Specifically if we've got the true clock 26667 Hz, we'll perform this calculation: 26667 / 1000 => 26 Later when we try to set the clock we'll do clk_set_rate(26 * 1000). The common clock framework won't actually pick the proper clock in this case since it always wants clocks <= the specified one. Let's solve this by using DIV_ROUND_UP. Signed-off-by: Douglas Anderson Signed-off-by: Sean Paul --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 31744fe..1bbffaf 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -891,7 +891,8 @@ static bool vop_crtc_mode_fixup(struct drm_crtc *crtc, struct vop *vop = to_vop(crtc); adjusted_mode->clock = - clk_round_rate(vop->dclk, mode->clock * 1000) / 1000; + DIV_ROUND_UP(clk_round_rate(vop->dclk, mode->clock * 1000), +1000); return true; } -- 2.8.0.rc3.226.g39d4020
[Bug 97248] [regression] [amdgpu] New regression in 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97248 Mike Lothian changed: What|Removed |Added CC||mike at fireburn.co.uk --- Comment #2 from Mike Lothian --- I'll revert that on the 4.9-next-wip branch and see if it makes a difference I'm bisecting on linus's tree at the moment to see where the regression crept in I've also been runnig with these local patches to use Dave's D3cold work (I reverted those before reporting) I'll attach them to the bug though -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/d1ea75d9/attachment.html>
[Bug 97248] [regression] [amdgpu] New regression in 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97248 --- Comment #3 from Mike Lothian --- Created attachment 125606 --> https://bugs.freedesktop.org/attachment.cgi?id=125606&action=edit acpi switcherooo -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/dd1f5c82/attachment.html>
[Bug 97248] [regression] [amdgpu] New regression in 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97248 --- Comment #4 from Mike Lothian --- Created attachment 125607 --> https://bugs.freedesktop.org/attachment.cgi?id=125607&action=edit acpi off -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/16709819/attachment.html>
[Bug 97248] [regression] [amdgpu] New regression in 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97248 --- Comment #5 from Mike Lothian --- Created attachment 125608 --> https://bugs.freedesktop.org/attachment.cgi?id=125608&action=edit amdgpu switcheroo -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/adcadcd4/attachment.html>
[PATCH 0/7] de-stage SW_SYNC validation frawework
On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote: > 2016-08-07 Pavel Machek : > > > On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote: > > > On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote: > > > > Hi, > > > > > > > > Do you think there is time to get this in for 4.8? > > > > > > No, it was too late on my end, due to travel and vacation, sorry. I'll > > > queue it up for 4.9-rc1. > > > > Could we get some documentation what this does? Is it visilble to > > userspace? > > This interface is only intended for testing and validation, there are > ioctls on the debugfs file that can be accessed by userspace but there > isn't any exported kernel header with this info. The tester should know > and add a internal header to be able to access it. We want to prevent > people from misusing this feature by not advertising it nor providing > documentation. You are playing dangerous game here. debugfs is not normally considered stable, but otoh... ioctls on debugfs? Anyway, please provide some documentation. Kernel hackers need to know what this does. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[Bug 97248] [regression] [amdgpu] New regression in 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97248 --- Comment #6 from Mike Lothian --- Yes reverting that patch seems to work -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/009c2ab5/attachment.html>
[Bug 97248] [regression] [amdgpu] New regression in 4.8-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=97248 --- Comment #7 from Alex Deucher --- (In reply to Mike Lothian from comment #2) > I'll revert that on the 4.9-next-wip branch and see if it makes a difference > > I'm bisecting on linus's tree at the moment to see where the regression > crept in It's probably due to the d3cold support that when upstream in 4.8. > > I've also been runnig with these local patches to use Dave's D3cold work (I > reverted those before reporting) I'll attach them to the bug though You shouldn't need those (or the workaround from my tree referenced in comment 1) now that the d3cold support has gone upstream via the pci tree. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/4ffb02ee/attachment-0001.html>
[PATCH 0/7] de-stage SW_SYNC validation frawework
2016-07-24 Pavel Machek : > On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote: > > 2016-08-07 Pavel Machek : > > > > > On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote: > > > > On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote: > > > > > Hi, > > > > > > > > > > Do you think there is time to get this in for 4.8? > > > > > > > > No, it was too late on my end, due to travel and vacation, sorry. I'll > > > > queue it up for 4.9-rc1. > > > > > > Could we get some documentation what this does? Is it visilble to > > > userspace? > > > > This interface is only intended for testing and validation, there are > > ioctls on the debugfs file that can be accessed by userspace but there > > isn't any exported kernel header with this info. The tester should know > > and add a internal header to be able to access it. We want to prevent > > people from misusing this feature by not advertising it nor providing > > documentation. > > You are playing dangerous game here. debugfs is not normally considered > stable, > but otoh... ioctls on debugfs? > > Anyway, please provide some documentation. Kernel hackers need to know what > this does. Okay, where do you think is the best place? Would documentation inside the .c file suffice for you?
[Bug 96278] [amdgpu][tonga]Kernel Hung when starting X while detecting outputs
https://bugs.freedesktop.org/show_bug.cgi?id=96278 JohnDoe changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #4 from JohnDoe --- Fixed with 4.8rc1 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/c3bea066/attachment.html>
[PATCH v10] drm/i915/skl: Add support for the SAGV, fix underrun hangs
Since the watermark calculations for Skylake are still broken, we're apt
to hitting underruns very easily under multi-monitor configurations.
While it would be lovely if this was fixed, it's not. Another problem
that's been coming from this however, is the mysterious issue of
underruns causing full system hangs. An easy way to reproduce this with
a skylake system:
- Get a laptop with a skylake GPU, and hook up two external monitors to
it
- Move the cursor from the built-in LCD to one of the external displays
as quickly as you can
- You'll get a few pipe underruns, and eventually the entire system will
just freeze.
After doing a lot of investigation and reading through the bspec, I
found the existence of the SAGV, which is responsible for adjusting the
system agent voltage and clock frequencies depending on how much power
we need. According to the bspec:
"The display engine access to system memory is blocked during the
adjustment time. SAGV defaults to enabled. Software must use the
GT-driver pcode mailbox to disable SAGV when the display engine is not
able to tolerate the blocking time."
The rest of the bspec goes on to explain that software can simply leave
the SAGV enabled, and disable it when we use interlaced pipes/have more
then one pipe active.
Sure enough, with this patchset the system hangs resulting from pipe
underruns on Skylake have completely vanished on my T460s. Additionally,
the bspec mentions turning off the SAGV with more then one pipe enabled
as a workaround for display underruns. While this patch doesn't entirely
fix that, it looks like it does improve the situation a little bit so
it's likely this is going to be required to make watermarks on Skylake
fully functional.
Changes since v9:
- Only enable/disable sagv on Skylake
Changes since v8:
- Add intel_state->modeset guard to the conditional for
skl_enable_sagv()
Changes since v7:
- Remove GEN9_SAGV_LOW_FREQ, replace with GEN9_SAGV_IS_ENABLED (that's
all we use it for anyway)
- Use GEN9_SAGV_IS_ENABLED instead of 0x1 for clarification
- Fix a styling error that snuck past me
Changes since v6:
- Protect skl_enable_sagv() with intel_state->modeset conditional in
intel_atomic_commit_tail()
Changes since v5:
- Don't use is_power_of_2. Makes things confusing
- Don't use the old state to figure out whether or not to
enable/disable the sagv, use the new one
- Split the loop in skl_disable_sagv into it's own function
- Move skl_sagv_enable/disable() calls into intel_atomic_commit_tail()
Changes since v4:
- Use is_power_of_2 against active_crtcs to check whether we have > 1
pipe enabled
- Fix skl_sagv_get_hw_state(): (temp & 0x1) indicates disabled, 0x0
enabled
- Call skl_sagv_enable/disable() from pre/post-plane updates
Changes since v3:
- Use time_before() to compare timeout to jiffies
Changes since v2:
- Really apply minor style nitpicks to patch this time
Changes since v1:
- Added comments about this probably being one of the requirements to
fixing Skylake's watermark issues
- Minor style nitpicks from Matt Roper
- Disable these functions on Broxton, since it doesn't have an SAGV
Reviewed-by: Matt Roper
Reviewed-by: Maarten Lankhorst
Signed-off-by: Lyude
Cc: Daniel Vetter
Cc: Ville Syrjälä
Cc: stable at vger.kernel.org
squash! drm/i915/skl: Add support for the SAGV, fix underrun hangs
squash! drm/i915/skl: Add support for the SAGV, fix underrun hangs
Signed-off-by: Lyude
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_reg.h | 4 ++
drivers/gpu/drm/i915/intel_display.c | 12
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_pm.c | 112 +++
5 files changed, 132 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index feec00f..eb449f6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1948,6 +1948,8 @@ struct drm_i915_private {
struct i915_suspend_saved_registers regfile;
struct vlv_s0ix_state vlv_s0ix_state;
+ bool skl_sagv_enabled;
+
struct {
/*
* Raw watermark latency values:
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f38a5e2..f7e0bc2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7170,6 +7170,10 @@ enum {
#define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
#define DISPLAY_IPS_CONTROL 0x19
#define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A
+#define GEN9_PCODE_SAGV_CONTROL 0x21
+#define GEN9_SAGV_DISABLE 0x0
+#define GEN9_SAGV_IS_DISABLED 0x1
+#define GEN9_SAGV_DYNAMIC_FREQ 0x3
#define GEN6_PCODE_DATA_MMIO(0x138128)
#define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8
#define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16
diff --git
[PATCH v10] drm/i915/skl: Add support for the SAGV, fix underrun hangs
Since the watermark calculations for Skylake are still broken, we're apt
to hitting underruns very easily under multi-monitor configurations.
While it would be lovely if this was fixed, it's not. Another problem
that's been coming from this however, is the mysterious issue of
underruns causing full system hangs. An easy way to reproduce this with
a skylake system:
- Get a laptop with a skylake GPU, and hook up two external monitors to
it
- Move the cursor from the built-in LCD to one of the external displays
as quickly as you can
- You'll get a few pipe underruns, and eventually the entire system will
just freeze.
After doing a lot of investigation and reading through the bspec, I
found the existence of the SAGV, which is responsible for adjusting the
system agent voltage and clock frequencies depending on how much power
we need. According to the bspec:
"The display engine access to system memory is blocked during the
adjustment time. SAGV defaults to enabled. Software must use the
GT-driver pcode mailbox to disable SAGV when the display engine is not
able to tolerate the blocking time."
The rest of the bspec goes on to explain that software can simply leave
the SAGV enabled, and disable it when we use interlaced pipes/have more
then one pipe active.
Sure enough, with this patchset the system hangs resulting from pipe
underruns on Skylake have completely vanished on my T460s. Additionally,
the bspec mentions turning off the SAGV with more then one pipe enabled
as a workaround for display underruns. While this patch doesn't entirely
fix that, it looks like it does improve the situation a little bit so
it's likely this is going to be required to make watermarks on Skylake
fully functional.
Changes since v9:
- Only enable/disable sagv on Skylake
Changes since v8:
- Add intel_state->modeset guard to the conditional for
skl_enable_sagv()
Changes since v7:
- Remove GEN9_SAGV_LOW_FREQ, replace with GEN9_SAGV_IS_ENABLED (that's
all we use it for anyway)
- Use GEN9_SAGV_IS_ENABLED instead of 0x1 for clarification
- Fix a styling error that snuck past me
Changes since v6:
- Protect skl_enable_sagv() with intel_state->modeset conditional in
intel_atomic_commit_tail()
Changes since v5:
- Don't use is_power_of_2. Makes things confusing
- Don't use the old state to figure out whether or not to
enable/disable the sagv, use the new one
- Split the loop in skl_disable_sagv into it's own function
- Move skl_sagv_enable/disable() calls into intel_atomic_commit_tail()
Changes since v4:
- Use is_power_of_2 against active_crtcs to check whether we have > 1
pipe enabled
- Fix skl_sagv_get_hw_state(): (temp & 0x1) indicates disabled, 0x0
enabled
- Call skl_sagv_enable/disable() from pre/post-plane updates
Changes since v3:
- Use time_before() to compare timeout to jiffies
Changes since v2:
- Really apply minor style nitpicks to patch this time
Changes since v1:
- Added comments about this probably being one of the requirements to
fixing Skylake's watermark issues
- Minor style nitpicks from Matt Roper
- Disable these functions on Broxton, since it doesn't have an SAGV
Reviewed-by: Matt Roper
Reviewed-by: Maarten Lankhorst
Signed-off-by: Lyude
Cc: Daniel Vetter
Cc: Ville Syrjälä
Cc: stable at vger.kernel.org
squash! drm/i915/skl: Add support for the SAGV, fix underrun hangs
squash! drm/i915/skl: Add support for the SAGV, fix underrun hangs
Signed-off-by: Lyude
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_reg.h | 4 ++
drivers/gpu/drm/i915/intel_display.c | 12
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_pm.c | 112 +++
5 files changed, 132 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index feec00f..eb449f6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1948,6 +1948,8 @@ struct drm_i915_private {
struct i915_suspend_saved_registers regfile;
struct vlv_s0ix_state vlv_s0ix_state;
+ bool skl_sagv_enabled;
+
struct {
/*
* Raw watermark latency values:
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f38a5e2..f7e0bc2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7170,6 +7170,10 @@ enum {
#define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
#define DISPLAY_IPS_CONTROL 0x19
#define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A
+#define GEN9_PCODE_SAGV_CONTROL 0x21
+#define GEN9_SAGV_DISABLE 0x0
+#define GEN9_SAGV_IS_DISABLED 0x1
+#define GEN9_SAGV_DYNAMIC_FREQ 0x3
#define GEN6_PCODE_DATA_MMIO(0x138128)
#define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8
#define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16
diff --git
Linux 4.8-rc1 Cirrus QEMU crashes on boot.
On Mon, Aug 8, 2016 at 1:24 PM, Eric W. Biederman
wrote:
>
> Booting up v4.8-rc1 in qemu today I ran I ran into this beautiful oops.
>
> I am just about to head out the door on vacation until the end of the
> month so hopefully I am tossing out enough information to the right
> people.
>
> I have confirmed that disabling CONFIG_DRM_CIRRUS_QEMU avoids this
> crash.
>
> [0.720167] BUG: unable to handle kernel NULL pointer dereference at
> 0018
> [0.721348] IP: [] drm_pick_crtcs+0xfc/0x270
> [0.722249] PGD 0
> [0.722659] Oops: [#1] SMP
> [0.723141] Modules linked in:
> [0.723643] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc1+ #116
> [0.724602] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
> [0.725450] task: 8800bbf28000 task.stack: 8800bbf3
> [0.726327] RIP: 0010:[] []
> drm_pick_crtcs+0xfc/0x270
> [0.727648] RSP: :8800bbf33978 EFLAGS: 00010217
> [0.728444] RAX: 8d623a20 RBX: RCX:
> 8800baf3f860
> [0.729498] RDX: RSI: 1000 RDI:
> 8800baf3f800
> [0.730626] RBP: 8800bbf339f8 R08: R09:
> 0001
> [0.731704] R10: 0001 R11: R12:
> 8800bbaf3af0
> [0.732827] R13: 8800bbaf3af8 R14: R15:
> 8800baf3fc00
> [0.733863] FS: () GS:8800bf80()
> knlGS:
> [0.735106] CS: 0010 DS: ES: CR0: 80050033
> [0.735964] CR2: 0018 CR3: 3b219000 CR4:
> 06f0
> [0.737021] Stack:
> [0.737342] 8800bbf33998 0246 8800bbaf3b18
> 8800bbaf3b00
> [0.738602] 1001 8800bbaf3b00
> 8800baf3f800
> [0.739807] 00031000 8800bbaf3b18 8800bbf339e8
> 8800baf3fc00
> [0.740993] Call Trace:
> [0.741393] [] drm_setup_crtcs+0x3bd/0xb50
> [0.742252] [] ? mark_held_locks+0x71/0x90
> [0.743176] [] ? __mutex_unlock_slowpath+0xeb/0x1c0
> [0.744138] [] drm_fb_helper_initial_config+0x81/0x3c0
> [0.745166] [] ? drm_modeset_unlock_all+0x54/0x80
> [0.746136] [] cirrus_fbdev_init+0xa0/0xb0
> [0.747033] [] cirrus_modeset_init+0x18b/0x1e0
> .. snip snip ..
> [0.771023] Code: e8 ba e1 ff ff 48 8b 55 b8 48 83 f8 01 83 5d c4 ff 48 8b
> 7d b8 48 8b 82 98 02 00 00 49 8b 57 08 48 8b 40 10 48 8b 92 00 0a 00 00 <48>
> 83 7a 18 00 74 09 48 85 c0 0f 84 53 01 00 00 ff d0 49 89 c3
This decodes to
0: e8 ba e1 ff ff callq ...
5: 48 8b 55 b8 mov-0x48(%rbp),%rdx
9: 48 83 f8 01 cmp$0x1,%rax
d: 83 5d c4 ff sbbl $0x,-0x3c(%rbp)
11: 48 8b 7d b8 mov-0x48(%rbp),%rdi
15: 48 8b 82 98 02 00 00 mov0x298(%rdx),%rax
1c: 49 8b 57 08 mov0x8(%r15),%rdx
20: 48 8b 40 10 mov0x10(%rax),%rax
24: 48 8b 92 00 0a 00 00 mov0xa00(%rdx),%rdx
2b:* 48 83 7a 18 00 cmpq $0x0,0x18(%rdx) <-- trapping instruction
30: 74 09 je 0x3b
32: 48 85 c0 test %rax,%rax
35: 0f 84 53 01 00 00 je 0x18e
3b: ff d0 callq *%rax
3d: 49 89 c3 mov%rax,%r11
and trying to find matching code I think the oops is from this:
if (fb_helper->dev->mode_config.funcs->atomic_commit &&
!connector_funcs->best_encoder)
encoder = drm_atomic_helper_best_encoder(connector);
else
encoder = connector_funcs->best_encoder(connector);
in particular, the trapping instruction is the load of "atomic_commit".
(The "callq *%rax" seems to be the "connector_funcs->best_encoder()" call)
So I *think* we have "fb_helper->dev->mode_config.funcs" being NULL.
But I might have gotten the code matching wrong. I don't have the same
compiler as Eric, so even when using his config my code generation
doesn't really match. But there is only one indirect call in that
function, which is that ->best_encoder() call, so it does seem to
match up.
Adding Boris Brezillon and Daniel Vetter to the cc, since assuming I'm
right, the main suspect is commit c61b93fe51b1 ("drm/atomic: Fix
remaining places where !funcs->best_encoder is valid").
Boris? Daniel?
Linus
[PATCH v2 0/6] de-stage SW_SYNC validation frawework
From: Gustavo Padovan Hi Greg, This is the last step in the Sync Framwork de-stage task. It de-stage the SW_SYNC validation framework and the sync_debug info debugfs file. The first 2 patches are clean up and improvements and the rest is preparation to de-stage and then finally the actual de-stage. v2: - add documentation about the SW_SYNC ioctl API (comments from Pavel Machek) - remove for now patch to add sync_pt name to debugfs Please review, Gustavo --- Gustavo Padovan (6): staging/android: remove doc from sw_sync staging/android: do not let userspace trigger WARN_ON staging/android: move trace/sync.h to sync_trace.h staging/android: prepare sw_sync files for de-staging staging/android: add Doc for SW_SYNC ioctl interface dma-buf/sw_sync: de-stage SW_SYNC drivers/dma-buf/Kconfig | 13 ++ drivers/dma-buf/Makefile | 1 + drivers/dma-buf/sw_sync.c| 349 +++ drivers/dma-buf/sync_debug.c | 230 +++ drivers/dma-buf/sync_debug.h | 69 +++ drivers/dma-buf/sync_trace.h | 32 drivers/staging/android/Kconfig | 13 -- drivers/staging/android/Makefile | 1 - drivers/staging/android/sw_sync.c| 344 -- drivers/staging/android/sync_debug.c | 230 --- drivers/staging/android/sync_debug.h | 84 - drivers/staging/android/trace/sync.h | 32 12 files changed, 694 insertions(+), 704 deletions(-) create mode 100644 drivers/dma-buf/sw_sync.c create mode 100644 drivers/dma-buf/sync_debug.c create mode 100644 drivers/dma-buf/sync_debug.h create mode 100644 drivers/dma-buf/sync_trace.h delete mode 100644 drivers/staging/android/sw_sync.c delete mode 100644 drivers/staging/android/sync_debug.c delete mode 100644 drivers/staging/android/sync_debug.h delete mode 100644 drivers/staging/android/trace/sync.h -- 2.5.5
[PATCH v2 1/6] staging/android: remove doc from sw_sync
From: Gustavo Padovan
SW_SYNC should never be used by other pieces of the kernel apart from
sync_debug as it is only a Sync File Validation Framework, so hide any
info to avoid confuse this with a standard kernel internal API.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sw_sync.c| 26 --
drivers/staging/android/sync_debug.h | 15 ---
2 files changed, 41 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c
b/drivers/staging/android/sw_sync.c
index 115c917..b4ae092 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -46,13 +46,6 @@ static inline struct sync_pt *fence_to_sync_pt(struct fence
*fence)
return container_of(fence, struct sync_pt, base);
}
-/**
- * sync_timeline_create() - creates a sync object
- * @name: sync_timeline name
- *
- * Creates a new sync_timeline. Returns the sync_timeline object or NULL in
- * case of error.
- */
struct sync_timeline *sync_timeline_create(const char *name)
{
struct sync_timeline *obj;
@@ -94,14 +87,6 @@ static void sync_timeline_put(struct sync_timeline *obj)
kref_put(&obj->kref, sync_timeline_free);
}
-/**
- * sync_timeline_signal() - signal a status change on a sync_timeline
- * @obj: sync_timeline to signal
- * @inc: num to increment on timeline->value
- *
- * A sync implementation should call this any time one of it's fences
- * has signaled or has an error condition.
- */
static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
{
unsigned long flags;
@@ -122,17 +107,6 @@ static void sync_timeline_signal(struct sync_timeline
*obj, unsigned int inc)
spin_unlock_irqrestore(&obj->child_list_lock, flags);
}
-/**
- * sync_pt_create() - creates a sync pt
- * @parent:fence's parent sync_timeline
- * @size: size to allocate for this pt
- * @inc: value of the fence
- *
- * Creates a new sync_pt as a child of @parent. @size bytes will be
- * allocated allowing for implementation specific data to be kept after
- * the generic sync_timeline struct. Returns the sync_pt object or
- * NULL in case of error.
- */
static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size,
unsigned int value)
{
diff --git a/drivers/staging/android/sync_debug.h
b/drivers/staging/android/sync_debug.h
index fab6639..5b82cf8 100644
--- a/drivers/staging/android/sync_debug.h
+++ b/drivers/staging/android/sync_debug.h
@@ -20,15 +20,6 @@
#include
#include
-/**
- * struct sync_timeline - sync object
- * @kref: reference count on fence.
- * @name: name of the sync_timeline. Useful for debugging
- * @child_list_head: list of children sync_pts for this sync_timeline
- * @child_list_lock: lock protecting @child_list_head and fence.status
- * @active_list_head: list of active (unsignaled/errored) sync_pts
- * @sync_timeline_list:membership in global sync_timeline_list
- */
struct sync_timeline {
struct kref kref;
charname[32];
@@ -51,12 +42,6 @@ static inline struct sync_timeline *fence_parent(struct
fence *fence)
child_list_lock);
}
-/**
- * struct sync_pt - sync_pt object
- * @base: base fence object
- * @child_list: sync timeline child's list
- * @active_list: sync timeline active child's list
- */
struct sync_pt {
struct fence base;
struct list_head child_list;
--
2.5.5
[PATCH v2 2/6] staging/android: do not let userspace trigger WARN_ON
From: Gustavo Padovan Closing the timeline without waiting all fences to signal is not a critical failure, it is just bad usage from userspace so avoid calling WARN_ON in this case. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index b4ae092..ad0bb1a 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -150,7 +150,7 @@ static void timeline_fence_release(struct fence *fence) spin_lock_irqsave(fence->lock, flags); list_del(&pt->child_list); - if (WARN_ON_ONCE(!list_empty(&pt->active_list))) + if (!list_empty(&pt->active_list)) list_del(&pt->active_list); spin_unlock_irqrestore(fence->lock, flags); -- 2.5.5
[PATCH v2 3/6] staging/android: move trace/sync.h to sync_trace.h
From: Gustavo Padovan
The common behaviour for trace headers is to have them in the same folder
they are used, instead of creating a special trace/ directory.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sw_sync.c| 2 +-
drivers/staging/android/sync_trace.h | 32
drivers/staging/android/trace/sync.h | 32
3 files changed, 33 insertions(+), 33 deletions(-)
create mode 100644 drivers/staging/android/sync_trace.h
delete mode 100644 drivers/staging/android/trace/sync.h
diff --git a/drivers/staging/android/sw_sync.c
b/drivers/staging/android/sw_sync.c
index ad0bb1a..745597b 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -23,7 +23,7 @@
#include "sync_debug.h"
#define CREATE_TRACE_POINTS
-#include "trace/sync.h"
+#include "sync_trace.h"
struct sw_sync_create_fence_data {
__u32 value;
diff --git a/drivers/staging/android/sync_trace.h
b/drivers/staging/android/sync_trace.h
new file mode 100644
index 000..ea485f7
--- /dev/null
+++ b/drivers/staging/android/sync_trace.h
@@ -0,0 +1,32 @@
+#undef TRACE_SYSTEM
+#define TRACE_INCLUDE_PATH ../../drivers/staging/android
+#define TRACE_SYSTEM sync_trace
+
+#if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SYNC_H
+
+#include "sync_debug.h"
+#include
+
+TRACE_EVENT(sync_timeline,
+ TP_PROTO(struct sync_timeline *timeline),
+
+ TP_ARGS(timeline),
+
+ TP_STRUCT__entry(
+ __string(name, timeline->name)
+ __field(u32, value)
+ ),
+
+ TP_fast_assign(
+ __assign_str(name, timeline->name);
+ __entry->value = timeline->value;
+ ),
+
+ TP_printk("name=%s value=%d", __get_str(name), __entry->value)
+);
+
+#endif /* if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ) */
+
+/* This part must be outside protection */
+#include
diff --git a/drivers/staging/android/trace/sync.h
b/drivers/staging/android/trace/sync.h
deleted file mode 100644
index 6b5ce96..000
--- a/drivers/staging/android/trace/sync.h
+++ /dev/null
@@ -1,32 +0,0 @@
-#undef TRACE_SYSTEM
-#define TRACE_INCLUDE_PATH ../../drivers/staging/android/trace
-#define TRACE_SYSTEM sync
-
-#if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ)
-#define _TRACE_SYNC_H
-
-#include "../sync_debug.h"
-#include
-
-TRACE_EVENT(sync_timeline,
- TP_PROTO(struct sync_timeline *timeline),
-
- TP_ARGS(timeline),
-
- TP_STRUCT__entry(
- __string(name, timeline->name)
- __field(u32, value)
- ),
-
- TP_fast_assign(
- __assign_str(name, timeline->name);
- __entry->value = timeline->value;
- ),
-
- TP_printk("name=%s value=%d", __get_str(name), __entry->value)
-);
-
-#endif /* if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ) */
-
-/* This part must be outside protection */
-#include
--
2.5.5
[PATCH v2 4/6] staging/android: prepare sw_sync files for de-staging
From: Gustavo Padovan remove file paths in the comments and add short description about each file. v2: remove file paths instead of just change them. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c| 2 +- drivers/staging/android/sync_debug.c | 2 +- drivers/staging/android/sync_debug.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 745597b..43491b6 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -1,5 +1,5 @@ /* - * drivers/dma-buf/sw_sync.c + * Sync File validation framework * * Copyright (C) 2012 Google, Inc. * diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 4c5a855..fab9520 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -1,5 +1,5 @@ /* - * drivers/base/sync.c + * Sync File validation framework and debug information * * Copyright (C) 2012 Google, Inc. * diff --git a/drivers/staging/android/sync_debug.h b/drivers/staging/android/sync_debug.h index 5b82cf8..ee3c27b 100644 --- a/drivers/staging/android/sync_debug.h +++ b/drivers/staging/android/sync_debug.h @@ -1,5 +1,5 @@ /* - * include/linux/sync.h + * Sync File validation framework * * Copyright (C) 2012 Google, Inc. * -- 2.5.5
[PATCH v2 5/6] staging/android: add Doc for SW_SYNC ioctl interface
From: Gustavo Padovan
This interface is hidden from kernel headers and it is intended for use
only for testing. So testers would have to add the ioctl information
internally. This is to prevent misuse of this feature.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sw_sync.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/drivers/staging/android/sw_sync.c
b/drivers/staging/android/sw_sync.c
index 43491b6..2ac5608 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -25,6 +25,36 @@
#define CREATE_TRACE_POINTS
#include "sync_trace.h"
+/*
+ * SW SYNC validation framework
+ *
+ * A sync object driver that uses a 32bit counter to coordinate
+ * synchronization. Useful when there is no hardware primitive backing
+ * the synchronization.
+ *
+ * To start the framework just open:
+ *
+ * /sync/sw_sync
+ *
+ * That will create a sync timeline, all fences created under this timeline
+ * file descriptor will belong to the this timeline.
+ *
+ * The 'sw_sync' file can be opened many times as to create different
+ * timelines.
+ *
+ * Fences can be created with SW_SYNC_IOC_CREATE_FENCE ioctl with struct
+ * sw_sync_ioctl_create_fence as parameter.
+ *
+ * To increment the timeline counter SW_SYNC_IOC_INC ioctl should be used
+ * with the increment as u32. This will update the last signaled value
+ * from the timeline and signal any fence that has seqno, smaller of equal
+ * it.
+ *
+ * struct sw_sync_ioctl_create_fence
+ * @value: the seqno to initiate the fence with
+ * @name: the name of the new sync point
+ * @fence: return the fd of the new sync_file with the created fence
+ */
struct sw_sync_create_fence_data {
__u32 value;
charname[32];
@@ -35,6 +65,7 @@ struct sw_sync_create_fence_data {
#define SW_SYNC_IOC_CREATE_FENCE _IOWR(SW_SYNC_IOC_MAGIC, 0,\
struct sw_sync_create_fence_data)
+
#define SW_SYNC_IOC_INC_IOW(SW_SYNC_IOC_MAGIC, 1,
__u32)
static const struct fence_ops timeline_fence_ops;
--
2.5.5
[PATCH v2 6/6] dma-buf/sw_sync: de-stage SW_SYNC
From: Gustavo Padovan
SW_SYNC allows to run tests on the sync_file framework via debugfs on
/sync/sw_sync
Opening and closing the file triggers creation and release of a sync
timeline. To create fences on this timeline the SW_SYNC_IOC_CREATE_FENCE
ioctl should be used. To increment the timeline value use SW_SYNC_IOC_INC.
Also it exports Sync information on
/sync/info
Signed-off-by: Gustavo Padovan
---
drivers/dma-buf/Kconfig | 13 ++
drivers/dma-buf/Makefile | 1 +
drivers/dma-buf/sw_sync.c| 349 +++
drivers/dma-buf/sync_debug.c | 230 +++
drivers/dma-buf/sync_debug.h | 69 +++
drivers/dma-buf/sync_trace.h | 32
drivers/staging/android/Kconfig | 13 --
drivers/staging/android/Makefile | 1 -
drivers/staging/android/sw_sync.c| 349 ---
drivers/staging/android/sync_debug.c | 230 ---
drivers/staging/android/sync_debug.h | 69 ---
drivers/staging/android/sync_trace.h | 32
12 files changed, 694 insertions(+), 694 deletions(-)
create mode 100644 drivers/dma-buf/sw_sync.c
create mode 100644 drivers/dma-buf/sync_debug.c
create mode 100644 drivers/dma-buf/sync_debug.h
create mode 100644 drivers/dma-buf/sync_trace.h
delete mode 100644 drivers/staging/android/sw_sync.c
delete mode 100644 drivers/staging/android/sync_debug.c
delete mode 100644 drivers/staging/android/sync_debug.h
delete mode 100644 drivers/staging/android/sync_trace.h
diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
index 25bcfa0..2585821 100644
--- a/drivers/dma-buf/Kconfig
+++ b/drivers/dma-buf/Kconfig
@@ -17,4 +17,17 @@ config SYNC_FILE
Files fds, to the DRM driver for example. More details at
Documentation/sync_file.txt.
+config SW_SYNC
+ bool "Sync File Validation Framework"
+ default n
+ depends on SYNC_FILE
+ depends on DEBUG_FS
+ ---help---
+ A sync object driver that uses a 32bit counter to coordinate
+ synchronization. Useful when there is no hardware primitive backing
+ the synchronization.
+
+ WARNING: improper use of this can result in deadlocking kernel
+ drivers from userspace. Intended for test and debug only.
+
endmenu
diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index f353db2..210a10b 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,2 +1,3 @@
obj-y := dma-buf.o fence.o reservation.o seqno-fence.o fence-array.o
obj-$(CONFIG_SYNC_FILE)+= sync_file.o
+obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
new file mode 100644
index 000..2ac5608
--- /dev/null
+++ b/drivers/dma-buf/sw_sync.c
@@ -0,0 +1,349 @@
+/*
+ * Sync File validation framework
+ *
+ * Copyright (C) 2012 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "sync_debug.h"
+
+#define CREATE_TRACE_POINTS
+#include "sync_trace.h"
+
+/*
+ * SW SYNC validation framework
+ *
+ * A sync object driver that uses a 32bit counter to coordinate
+ * synchronization. Useful when there is no hardware primitive backing
+ * the synchronization.
+ *
+ * To start the framework just open:
+ *
+ * /sync/sw_sync
+ *
+ * That will create a sync timeline, all fences created under this timeline
+ * file descriptor will belong to the this timeline.
+ *
+ * The 'sw_sync' file can be opened many times as to create different
+ * timelines.
+ *
+ * Fences can be created with SW_SYNC_IOC_CREATE_FENCE ioctl with struct
+ * sw_sync_ioctl_create_fence as parameter.
+ *
+ * To increment the timeline counter SW_SYNC_IOC_INC ioctl should be used
+ * with the increment as u32. This will update the last signaled value
+ * from the timeline and signal any fence that has seqno, smaller of equal
+ * it.
+ *
+ * struct sw_sync_ioctl_create_fence
+ * @value: the seqno to initiate the fence with
+ * @name: the name of the new sync point
+ * @fence: return the fd of the new sync_file with the created fence
+ */
+struct sw_sync_create_fence_data {
+ __u32 value;
+ charname[32];
+ __s32 fence; /* fd of new fence */
+};
+
+#define SW_SYNC_IOC_MAGIC 'W'
+
+#define SW_SYNC_IOC_CREATE_FENCE _IOWR(SW_SYNC_IOC_MAGIC, 0,\
+ struct sw_sync_create_fence_data)
+
+#define SW_SYNC_IOC_INC
[Bug 97249] R290x combined with Mg279q, Dp link fail in ubuntu, in multihead setup.
https://bugs.freedesktop.org/show_bug.cgi?id=97249 Bug ID: 97249 Summary: R290x combined with Mg279q, Dp link fail in ubuntu, in multihead setup. Product: Mesa Version: 11.2 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel at lists.freedesktop.org Reporter: ramsland at hotmail.com QA Contact: dri-devel at lists.freedesktop.org Epuigpment: CPU Amd fx8350, Mboard: Gigabyte GA-990FXA-UD3, Ram 16 gb. Sapphire R9 290x. Monitor setup: 2 monitors, 1 right side (DP-0) Asus MG279q, 1 left(HDMI-0)pb279. Both are 1440p monitors, with the Mq279q have high refreshrate 144hz. Using randr for multihead. OS: Ubuntu 16.04, kernel 4.4.0-31-generic PPA:Obiaf. Problem: The right screen is blank, as long as I try to use standard resolution, meaning 1440p. Can only get it to work in 1080p, at 60hz. It worked perfectly in the old fgrlx when running ubuntu 14.4, and with radeon in ubuntu 14.4, though it might have been both screens at 60hz with the radeon driver. When trying with livecd's, the fedora 24 is working at 1440p at 60 hz, but fails if i try to increase the refresh rate. Ubuntu live 16.04, fails the same way as my install. I think its releated to the radeon driver, but i cant be sure. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/b71f2526/attachment-0001.html>
[Bug 97249] R290x combined with Mg279q, Dp link fail in ubuntu, in multihead setup.
https://bugs.freedesktop.org/show_bug.cgi?id=97249 --- Comment #1 from ramsland at hotmail.com --- Created attachment 125612 --> https://bugs.freedesktop.org/attachment.cgi?id=125612&action=edit glxinfo -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/00f79f6f/attachment.html>
[Bug 97249] R290x combined with Mg279q, Dp link fail in ubuntu-multihead setup.
https://bugs.freedesktop.org/show_bug.cgi?id=97249 ramsland at hotmail.com changed: What|Removed |Added Summary|R290x combined with Mg279q, |R290x combined with Mg279q, |Dp link fail in ubuntu, in |Dp link fail in |multihead setup.|ubuntu-multihead setup. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/90770831/attachment.html>
[Bug 97249] R290x combined with Mg279q, Dp link fail in ubuntu-multihead setup.
https://bugs.freedesktop.org/show_bug.cgi?id=97249 --- Comment #2 from ramsland at hotmail.com --- Created attachment 125613 --> https://bugs.freedesktop.org/attachment.cgi?id=125613&action=edit Error msg in dmsg at startup. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/4b751b0d/attachment.html>
[Bug 97249] R290x combined with Mg279q, Dp link fail in ubuntu-multihead setup.
https://bugs.freedesktop.org/show_bug.cgi?id=97249 --- Comment #3 from ramsland at hotmail.com --- Created attachment 125614 --> https://bugs.freedesktop.org/attachment.cgi?id=125614&action=edit dmsg error when using xrandr to set resolution higher than 1080p. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/d070d311/attachment.html>
Linux 4.8-rc1 Cirrus QEMU crashes on boot.
- Original Message -
> From: "Linus Torvalds"
> To: "Eric W. Biederman" , "Boris Brezillon"
> , "Daniel
> Vetter"
> Cc: "Linux Kernel Mailing List" , "Dave
> Airlie" , "David Airlie"
> , "DRI"
> Sent: Tuesday, 9 August, 2016 7:19:23 AM
> Subject: Re: Linux 4.8-rc1 Cirrus QEMU crashes on boot.
>
> On Mon, Aug 8, 2016 at 1:24 PM, Eric W. Biederman
> wrote:
> >
> > Booting up v4.8-rc1 in qemu today I ran I ran into this beautiful oops.
> >
> > I am just about to head out the door on vacation until the end of the
> > month so hopefully I am tossing out enough information to the right
> > people.
> >
> > I have confirmed that disabling CONFIG_DRM_CIRRUS_QEMU avoids this
> > crash.
> >
> > [0.720167] BUG: unable to handle kernel NULL pointer dereference at
> > 0018
> > [0.721348] IP: [] drm_pick_crtcs+0xfc/0x270
> > [0.722249] PGD 0
> > [0.722659] Oops: [#1] SMP
> > [0.723141] Modules linked in:
> > [0.723643] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc1+ #116
> > [0.724602] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
> > [0.725450] task: 8800bbf28000 task.stack: 8800bbf3
> > [0.726327] RIP: 0010:[] []
> > drm_pick_crtcs+0xfc/0x270
> > [0.727648] RSP: :8800bbf33978 EFLAGS: 00010217
> > [0.728444] RAX: 8d623a20 RBX: RCX:
> > 8800baf3f860
> > [0.729498] RDX: RSI: 1000 RDI:
> > 8800baf3f800
> > [0.730626] RBP: 8800bbf339f8 R08: R09:
> > 0001
> > [0.731704] R10: 0001 R11: R12:
> > 8800bbaf3af0
> > [0.732827] R13: 8800bbaf3af8 R14: R15:
> > 8800baf3fc00
> > [0.733863] FS: () GS:8800bf80()
> > knlGS:
> > [0.735106] CS: 0010 DS: ES: CR0: 80050033
> > [0.735964] CR2: 0018 CR3: 3b219000 CR4:
> > 06f0
> > [0.737021] Stack:
> > [0.737342] 8800bbf33998 0246 8800bbaf3b18
> > 8800bbaf3b00
> > [0.738602] 1001 8800bbaf3b00
> > 8800baf3f800
> > [0.739807] 00031000 8800bbaf3b18 8800bbf339e8
> > 8800baf3fc00
> > [0.740993] Call Trace:
> > [0.741393] [] drm_setup_crtcs+0x3bd/0xb50
> > [0.742252] [] ? mark_held_locks+0x71/0x90
> > [0.743176] [] ? __mutex_unlock_slowpath+0xeb/0x1c0
> > [0.744138] []
> > drm_fb_helper_initial_config+0x81/0x3c0
> > [0.745166] [] ? drm_modeset_unlock_all+0x54/0x80
> > [0.746136] [] cirrus_fbdev_init+0xa0/0xb0
> > [0.747033] [] cirrus_modeset_init+0x18b/0x1e0
> > .. snip snip ..
> > [0.771023] Code: e8 ba e1 ff ff 48 8b 55 b8 48 83 f8 01 83 5d c4 ff 48
> > 8b 7d b8 48 8b 82 98 02 00 00 49 8b 57 08 48 8b 40 10 48 8b 92 00 0a 00 00
> > <48> 83 7a 18 00 74 09 48 85 c0 0f 84 53 01 00 00 ff d0 49 89 c3
>
> This decodes to
>
>0: e8 ba e1 ff ff callq ...
>5: 48 8b 55 b8 mov-0x48(%rbp),%rdx
>9: 48 83 f8 01 cmp$0x1,%rax
>d: 83 5d c4 ff sbbl $0x,-0x3c(%rbp)
> 11: 48 8b 7d b8 mov-0x48(%rbp),%rdi
> 15: 48 8b 82 98 02 00 00 mov0x298(%rdx),%rax
> 1c: 49 8b 57 08 mov0x8(%r15),%rdx
> 20: 48 8b 40 10 mov0x10(%rax),%rax
> 24: 48 8b 92 00 0a 00 00 mov0xa00(%rdx),%rdx
> 2b:* 48 83 7a 18 00 cmpq $0x0,0x18(%rdx) <-- trapping instruction
> 30: 74 09 je 0x3b
> 32: 48 85 c0 test %rax,%rax
> 35: 0f 84 53 01 00 00 je 0x18e
> 3b: ff d0 callq *%rax
> 3d: 49 89 c3 mov%rax,%r11
>
>
> and trying to find matching code I think the oops is from this:
>
> if (fb_helper->dev->mode_config.funcs->atomic_commit &&
> !connector_funcs->best_encoder)
> encoder = drm_atomic_helper_best_encoder(connector);
> else
> encoder = connector_funcs->best_encoder(connector);
>
> in particular, the trapping instruction is the load of "atomic_commit".
>
> (The "callq *%rax" seems to be the "connector_funcs->best_encoder()" call)
>
> So I *think* we have "fb_helper->dev->mode_config.funcs" being NULL.
>
> But I might have gotten the code matching wrong. I don't have the same
> compiler as Eric, so even when using his config my code generation
> doesn't really match. But there is only one indirect call in that
> function, which is that ->best_encoder() call, so it does seem to
> match up.
>
> Adding Boris Brezillon and Daniel Vetter to the cc, since assuming I'm
> right, the main suspect is commit c61b93fe51b1 ("drm/atomic: Fix
> remaining places where !funcs->best_encoder is valid").
>
> Boris? Daniel?
I figured this out earlier this morning on another thread,
cirrus sets the mode funcs later than other drivers, will send a fix later,
but I
[Bug 97249] R290x combined with Mg279q, Dp link fail in ubuntu-multihead setup.
https://bugs.freedesktop.org/show_bug.cgi?id=97249 --- Comment #4 from ramsland at hotmail.com --- Created attachment 125621 --> https://bugs.freedesktop.org/attachment.cgi?id=125621&action=edit glx info from fedora 24 live. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/c7343a61/attachment.html>
[Bug 97249] R290x combined with Mg279q, Dp link fail in ubuntu-multihead setup.
https://bugs.freedesktop.org/show_bug.cgi?id=97249 --- Comment #5 from ramsland at hotmail.com --- Created attachment 125622 --> https://bugs.freedesktop.org/attachment.cgi?id=125622&action=edit Xorg-log -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/b8ed0e84/attachment.html>
[PATCH v7 0/5] drm/i915: DP branch devices
Prep work for DP branch device handling This series of patches reads DPCD register 0x80h for receiver capabilities for DP branch devices. The branch device types are converters for the following standards - DP to VGA - DP to DVI - DP to HDMI - DP++ dual mode - Wireless WiGig DPCD register defines max pixel rate for VGA dongles. This check is carried out during mode validation. [1] git://github.com/mkahola/drm-intel-mika.git dp_branch_device v2: DPCD register read outs moved to drm (Ville, Daniel) v3: Max pixel rate computation moved to drm (Daniel) v4: Use of drm_dp_helper routines to collect data (Ville) v5: Remove duplicate code and unnecessary functions from drm_dp_helper (Ville) v6: Rebase and i915_debugfs cleanup v7: Structure cleanups and initial step to move DP debugging info to drm_dp_helpers Mika Kahola (5): drm: Read DP branch device HW revision drm: Read DP branch device SW revision drm/i915: Check pixel rate for DP to VGA dongle drm: Update bits per component for display info drm: Add DP branch device info on debugfs drivers/gpu/drm/drm_dp_helper.c | 158 drivers/gpu/drm/i915/i915_debugfs.c | 2 + drivers/gpu/drm/i915/intel_dp.c | 28 ++- drivers/gpu/drm/i915/intel_drv.h| 1 + include/drm/drm_dp_helper.h | 20 + 5 files changed, 208 insertions(+), 1 deletion(-) -- 1.9.1
[PATCH v7 4/5] drm: Update bits per component for display info
DisplayPort branch device may define max supported bits per
component. Update display info based on this value if bpc
is defined.
v2: cleanup to match the drm_dp_helper.c patches introduced
earlier in this series
v3: Fill bpc for connector's display info in separate
drm_dp_helper function (Daniel)
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/drm_dp_helper.c | 24
drivers/gpu/drm/i915/intel_dp.c | 3 +++
include/drm/drm_dp_helper.h | 4
3 files changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 1f36016..a2c46ed 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -514,6 +514,30 @@ int drm_dp_downstream_max_bpc(const u8
dpcd[DP_RECEIVER_CAP_SIZE],
EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
/**
+ * drm_dp_downstream_update_max_bpc() - extract branch device max
+ * bits per component and update
+ * connector max bpc
+ * @dpcd: DisplayPort configuration data
+ * @port_cap: port capabilities
+ * @connector: Connector info
+ * Returns current max bpc on success
+ */
+int drm_dp_downstream_update_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+const u8 port_cap[4],
+struct drm_connector *connector)
+{
+ if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) {
+ int bpc = drm_dp_downstream_max_bpc(dpcd, port_cap);
+
+ if (bpc > 0)
+ connector->display_info.bpc = bpc;
+ }
+
+ return connector->display_info.bpc;
+}
+EXPORT_SYMBOL(drm_dp_downstream_update_max_bpc);
+
+/**
* drm_dp_downstream_hw_rev() - read DP branch device HW revision
* @aux: DisplayPort AUX channel
*
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e990c8b..763e2f5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3985,6 +3985,9 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
uint8_t *dpcd = intel_dp->dpcd;
uint8_t type;
+ drm_dp_downstream_update_max_bpc(dpcd, intel_dp->downstream_ports,
+&intel_dp->attached_connector->base);
+
if (!intel_dp_get_dpcd(intel_dp))
return connector_status_disconnected;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 45366aa..5491a9b 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
/*
* Unless otherwise noted, all values are from the DP 1.1a spec. Note that
@@ -827,6 +828,9 @@ int drm_dp_downstream_max_clock(const u8
dpcd[DP_RECEIVER_CAP_SIZE],
const u8 port_cap[4]);
int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
const u8 port_cap[4]);
+int drm_dp_downstream_update_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+const u8 port_cap[4],
+struct drm_connector *connector);
int drm_dp_downstream_id(struct drm_dp_aux *aux, char id[6]);
int drm_dp_downstream_hw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
struct drm_dp_aux *aux, struct drm_dp_link *link);
--
1.9.1
HDMI 2.0 Support
Hi, Currently I am working on some HDMI 2.0 features using the DRM infrastructure. I am mainly working in parsing the newly introduced EDID blocks such as HDMI Forum Vendor Specific Data Block, YCBCR 420 Video Data Block and YCBCR Capability Map Data Block. The new blocks require some changes in the EDID parsing, the addition of new aspect ratios, the addition of new VIC's and also a particularly special configuration flow when in 420 mode. Before sending patches I would like to know: Is there anyone working on something similar? What is your opinion about the addition of this new features? Best regards, Jose Miguel Abreu
[Intel-gfx] [PATCH 2/2] drm/i915: Drop reference to current state wait req as soon as it goes unused
Daniel Vetter writes: > We still need to clean up the reference in case of failure, which > means latest in intel_plane_destroy_state(). Also hanging onto a > request isn't that evil really, why can't we just only clean up in the > destroy function? Sticking a cleanup there as well is good insurance, but it seems like a reasonable policy to drop references when values go 'out of scope' as they do in cleanup_plane_fb. But, you're right, we only *need* to drop the reference in the destroy function, it's just that the state hangs around until the next mode setting operation, which is likely to be days away. -- -keith -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 810 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/4af545e3/attachment-0001.sig>
[PATCH 0/7] de-stage SW_SYNC validation frawework
2016-08-07 Pavel Machek : > On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote: > > On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote: > > > Hi, > > > > > > Do you think there is time to get this in for 4.8? > > > > No, it was too late on my end, due to travel and vacation, sorry. I'll > > queue it up for 4.9-rc1. > > Could we get some documentation what this does? Is it visilble to > userspace? This interface is only intended for testing and validation, there are ioctls on the debugfs file that can be accessed by userspace but there isn't any exported kernel header with this info. The tester should know and add a internal header to be able to access it. We want to prevent people from misusing this feature by not advertising it nor providing documentation. There will be, though, kselftest for this interface, that I send out once SW_SYNC is de-staged.
[PATCH v2] exynos-drm: Fix unsupported GEM memory type error message to be clear
Fix unsupported GEM memory type error message to include the memory type
information.
Signed-off-by: Shuah Khan
---
Changes since v1:
-- Comment changed to read clearly
drivers/gpu/drm/exynos/exynos_drm_fb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c
b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index e016640..40ce841 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -55,11 +55,11 @@ static int check_fb_gem_memory_type(struct drm_device
*drm_dev,
flags = exynos_gem->flags;
/*
-* without iommu support, not support physically non-continuous memory
-* for framebuffer.
+* Physically non-contiguous memory type for framebuffer is not
+* supported without IOMMU.
*/
if (IS_NONCONTIG_BUFFER(flags)) {
- DRM_ERROR("cannot use this gem memory type for fb.\n");
+ DRM_ERROR("Non-contiguous GEM memory is not supported.\n");
return -EINVAL;
}
--
2.7.4
include/drm/i915_drm.h:96: possible bad bitmask ?
Hello there, Recent versions of gcc say this: include/drm/i915_drm.h:96:34: warning: result of â65535 << 20â requires 37 bits to represent, but âintâ only has 32 bits [-Wshift-overflow=] Source code is #define INTEL_BSM_MASK (0x << 20) Maybe something like #define INTEL_BSM_MASK (0xUL<< 20) might be better. Regards David Binderman
[Intel-gfx] include/drm/i915_drm.h:96: possible bad bitmask ?
Hello there, On 8 August 2016 at 10:40, Daniel Vetter wrote: > Care to bake this into a patch > No thanks, but I am happy for someone else to invent a patch. Regards David Binderman -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/d6389a0e/attachment-0001.html>
[PATCH v7 2/5] drm: Read DP branch device SW revision
SW revision is mandatory field for DisplayPort branch
devices. This is defined in DPCD register field 0x50A.
v2: move drm_dp_ds_revision structure to be part of
drm_dp_link structure (Daniel)
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/drm_dp_helper.c | 27 +++
include/drm/drm_dp_helper.h | 5 +
2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 5fecdc1..1f36016 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -541,6 +541,33 @@ int drm_dp_downstream_hw_rev(const u8
dpcd[DP_RECEIVER_CAP_SIZE],
EXPORT_SYMBOL(drm_dp_downstream_hw_rev);
/**
+ * drm_dp_downstream_sw_rev() - read DP branch device SW revision
+ * @aux: DisplayPort AUX channel
+ *
+ * Returns SW revision on success or negative error code on failure
+ */
+int drm_dp_downstream_sw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+struct drm_dp_aux *aux, struct drm_dp_link *link)
+{
+ uint8_t tmp[2];
+ int err;
+
+ if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
+ return -EINVAL;
+
+ err = drm_dp_dpcd_read(aux, DP_BRANCH_SW_REV, tmp, 2);
+
+ if (err < 0)
+ return err;
+
+ link->ds_sw_rev.major = tmp[0];
+ link->ds_sw_rev.minor = tmp[1];
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_downstream_sw_rev);
+
+/**
* drm_dp_downstream_id() - identify branch device
* @aux: DisplayPort AUX channel
*
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 1127948..45366aa 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -447,6 +447,7 @@
#define DP_BRANCH_OUI 0x500
#define DP_BRANCH_ID0x503
#define DP_BRANCH_HW_REV0x509
+#define DP_BRANCH_SW_REV0x50A
#define DP_SET_POWER0x600
# define DP_SET_POWER_D00x1
@@ -815,6 +816,7 @@ struct drm_dp_link {
unsigned int num_lanes;
unsigned long capabilities;
struct drm_dp_ds_revision ds_hw_rev;
+ struct drm_dp_ds_revision ds_sw_rev;
};
int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
@@ -829,6 +831,9 @@ int drm_dp_downstream_id(struct drm_dp_aux *aux, char
id[6]);
int drm_dp_downstream_hw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
struct drm_dp_aux *aux, struct drm_dp_link *link);
+int drm_dp_downstream_sw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+struct drm_dp_aux *aux, struct drm_dp_link *link);
+
void drm_dp_aux_init(struct drm_dp_aux *aux);
int drm_dp_aux_register(struct drm_dp_aux *aux);
void drm_dp_aux_unregister(struct drm_dp_aux *aux);
--
1.9.1
[PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle
Filter out a mode that exceeds the max pixel rate setting
for DP to VGA dongle. This is defined in DPCD register 0x81
if detailed cap info i.e. info field is 4 bytes long and
it is available for DP downstream port.
The register defines the pixel rate divided by 8 in MP/s.
v2: DPCD read outs and computation moved to drm (Ville, Daniel)
v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock()
function (Daniel)
v4: Use of drm_dp_helper.c routines to compute max pixel clock (Ville)
v5: Use of intel_dp->downstream_ports to read out port capabilities.
Code restructuring (Ville)
v6: Move DP branch device check to drm_dp_helper.c (Daniel)
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/i915/intel_dp.c | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 21b04c3..e990c8b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -190,6 +190,26 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
return (max_link_clock * max_lanes * 8) / 10;
}
+static int
+intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp, int dotclk)
+{
+ int ds_dotclk;
+ int type;
+
+ ds_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
+ intel_dp->downstream_ports);
+
+ if (ds_dotclk == 0)
+ return dotclk;
+
+ type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
+
+ if (type != DP_DS_PORT_TYPE_VGA)
+ return dotclk;
+
+ return min(dotclk, ds_dotclk);
+}
+
static enum drm_mode_status
intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
@@ -199,7 +219,10 @@ intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
int target_clock = mode->clock;
int max_rate, mode_rate, max_lanes, max_link_clock;
- int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
+ int max_dotclk;
+
+ max_dotclk = intel_dp_downstream_max_dotclock(intel_dp,
+
to_i915(connector->dev)->max_dotclk_freq);
if (is_edp(intel_dp) && fixed_mode) {
if (mode->hdisplay > fixed_mode->hdisplay)
--
1.9.1
[PATCH v7 5/5] drm: Add DP branch device info on debugfs
Read DisplayPort branch device info from through debugfs
interface.
v2: use drm_dp_helper routines to collect data
v3: cleanup to match the drm_dp_helper.c patches introduced
earlier in this series
v4: move DP branch device info to function 'intel_dp_branch_device_info()'
v5: initial step to move debugging info from intel_dp. to drm_dp_helper.c
(Daniel)
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/drm_dp_helper.c | 80 +
drivers/gpu/drm/i915/i915_debugfs.c | 2 +
include/drm/drm_dp_helper.h | 2 +
3 files changed, 84 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index a2c46ed..47bbb5e 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -603,6 +603,86 @@ int drm_dp_downstream_id(struct drm_dp_aux *aux, char
id[6])
}
EXPORT_SYMBOL(drm_dp_downstream_id);
+/**
+ * drm_dp_downstream_debug() - debug DP branch devices
+ * @m: pointer for debugfs file
+ * @dpcd: DisplayPort configuration data
+ * @port_cap: port capabilities
+ * @aux: DisplayPort AUX channel
+ *
+ */
+void drm_dp_downstream_debug(struct seq_file *m, const u8
dpcd[DP_RECEIVER_CAP_SIZE],
+const u8 port_cap[4], struct drm_dp_aux *aux)
+{
+ struct drm_dp_link link;
+ bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
+ DP_DETAILED_CAP_INFO_AVAILABLE;
+ int clk;
+ int bpc;
+ char id[6];
+ int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
+ bool branch_device = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
DP_DWN_STRM_PORT_PRESENT;
+
+ seq_printf(m, "\tDP branch device present: %s\n", branch_device ? "yes"
: "no");
+
+ if (!branch_device)
+ return;
+
+ switch (type) {
+ case DP_DS_PORT_TYPE_DP:
+ seq_printf(m, "\t\tType: DisplayPort\n");
+ break;
+ case DP_DS_PORT_TYPE_VGA:
+ seq_printf(m, "\t\tType: VGA\n");
+ break;
+ case DP_DS_PORT_TYPE_DVI:
+ seq_printf(m, "\t\tType: DVI\n");
+ break;
+ case DP_DS_PORT_TYPE_HDMI:
+ seq_printf(m, "\t\tType: HDMI\n");
+ break;
+ case DP_DS_PORT_TYPE_NON_EDID:
+ seq_printf(m, "\t\tType: others without EDID support\n");
+ break;
+ case DP_DS_PORT_TYPE_DP_DUALMODE:
+ seq_printf(m, "\t\tType: DP++\n");
+ break;
+ case DP_DS_PORT_TYPE_WIRELESS:
+ seq_printf(m, "\t\tType: Wireless\n");
+ break;
+ default:
+ seq_printf(m, "\t\tType: N/A\n");
+ }
+
+ drm_dp_downstream_id(aux, id);
+ seq_printf(m, "\t\tID: %s\n", id);
+
+ if (drm_dp_downstream_hw_rev(dpcd, aux, &link) == 0)
+ seq_printf(m, "\t\tHW revision: %.2d.%.2d\n",
link.ds_hw_rev.major,
+ link.ds_hw_rev.minor);
+
+ if (drm_dp_downstream_sw_rev(dpcd, aux, &link) == 0)
+ seq_printf(m, "\t\tSW revision: %.2d.%.2d\n",
link.ds_sw_rev.major,
+ link.ds_sw_rev.minor);
+
+ if (detailed_cap_info) {
+ clk = drm_dp_downstream_max_clock(dpcd, port_cap);
+
+ if (clk > 0) {
+ if (type == DP_DS_PORT_TYPE_VGA)
+ seq_printf(m, "\t\tMax dot clock: %d kHz\n",
clk);
+ else
+ seq_printf(m, "\t\tMax TMDS clock: %d kHz\n",
clk);
+ }
+
+ bpc = drm_dp_downstream_max_bpc(dpcd, port_cap);
+
+ if (bpc > 0)
+ seq_printf(m, "\t\tMax bpc: %d\n", bpc);
+ }
+}
+EXPORT_SYMBOL(drm_dp_downstream_debug);
+
/*
* I2C-over-AUX implementation
*/
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 531ca02..3747f67 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2959,6 +2959,8 @@ static void intel_dp_info(struct seq_file *m,
seq_printf(m, "\taudio support: %s\n", yesno(intel_dp->has_audio));
if (intel_connector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
intel_panel_info(m, &intel_connector->panel);
+ drm_dp_downstream_debug(m, intel_dp->dpcd, intel_dp->downstream_ports,
+ &intel_dp->aux);
}
static void intel_hdmi_info(struct seq_file *m,
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 5491a9b..85123ac 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -837,6 +837,8 @@ int drm_dp_downstream_hw_rev(const u8
dpcd[DP_RECEIVER_CAP_SIZE],
int drm_dp_downstream_sw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
struct drm_dp_aux *aux, struct drm_dp_link *link);
+void drm_dp_downstream_debug(struct seq_file *m, const u8
dpcd[DP_RECEIVER_CAP_SIZE],
+
[PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
Daniel Vetter writes: > Rebased onto 4.8 while applying, which did simplify the patch a lot (4.8 > is already using for_each_plane_in_state, but slightly differently). Your rebase looks great, thanks! -- -keith -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 810 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160808/5837fef5/attachment-0001.sig>
[PATCH 3/3 v3] drm: bridge/dw-hdmi: Move edid reading to .detect() callback
Hi,
On 05-08-2016 09:13, Chris Wilson wrote:
> On Fri, Aug 05, 2016 at 10:06:08AM +0200, Daniel Vetter wrote:
>> On Fri, Aug 05, 2016 at 12:01:25AM +0100, Russell King - ARM Linux wrote:
>>> On Thu, Aug 04, 2016 at 06:13:18PM +0100, Jose Abreu wrote:
Hi Russell,
So, I didn't use framebuffer console but used X instead and it is
working as it should. I think we can drop this patch. I am now
making interoperability with DVI and I am facing the following
scenario:
- I start the driver
- An EDID is sent which tells the driver that HDMI is NOT
supported;
- The driver configures itself to a DVI mode;
Until this point everything is working as it should. But:
- Now I send an EDID which tells the driver that HDMI is
supported;
- As the EDID has the same preferred mode the user will not
reconfigure the mode and there will be no change to HDMI mode.
>>> The EDID should still be read, but as you say, userspace doesn't take
>>> any action because it sees that the mode parameters are still the same,
>>> as you have identified.
>>>
The missing change to HDMI mode will cause the test to fail. The
workaround that I am using is to reconfigure to another video
mode and then configure to the preferred one but I think this
could be fixed in the driver, right?
>>> This one is extremely awkward - to fix it, we would need to check to
>>> see whether we reconfigure the hardware each time we read the EDID,
>>> and I don't think that's a particularly nice thing to do.
>>>
>>> I'd like to hear what other DRM developers think about this issue.
>> I pondored whether we should have a counter on each connector for probe
>> state, which helpers would increment whenever something changes, like:
>> - connector_status (as tracked by probe helpers)
>> - anything in the edid changes (when setting it
>> drm_mode_connector_update_edid_property)
>> - other changes (like sink state changes in dpcd or whatever)
>>
>> That way userspace would be able to reliably spot such changes and do a
>> new modeset.
> Yes, please. I have had similar wishes for state changes and overall
> modeset counters.
> -Chris
>
What about this: Assuming that the modes probed by EDID have the
picture aspect ratio field set we can check for one of this
probed modes when receiving a mode from userspace and then if the
modes match (except from the picture aspect field, which is not
set by userspace) then we can use the picture aspect value of the
probed mode. I am using something like this in my modeset callback:
/* 'mode' comes from userspace, 'pmode' comes from EDID */
if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_NONE) {
struct drm_display_mode *pmode;
list_for_each_entry(pmode, &connector.modes, head) {
if (drm_mode_equal(pmode, mode))
mode->picture_aspect_ratio =
pmode->picture_aspect_ratio;
}
}
Of course if the EDID has for example two exactly equal modes
that only differ in the picture aspect ratio then this will not
work unless user passes the picture aspect when setting the mode.
Best regards,
Jose Miguel Abreu
[PATCH] exynos-drm: Fix unsupported GEM memory type error message to be clear
On Fri, Aug 05, 2016 at 07:50:06PM -0600, Shuah Khan wrote:
> Fix unsupported GEM memory type error message to include the memory type
> information.
>
> Signed-off-by: Shuah Khan
> ---
> drivers/gpu/drm/exynos/exynos_drm_fb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index e016640..c9315df 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -55,11 +55,11 @@ static int check_fb_gem_memory_type(struct drm_device
> *drm_dev,
> flags = exynos_gem->flags;
>
> /*
> - * without iommu support, not support physically non-continuous memory
> + * without iommu support, not support physically non-contiguous memory
While at it, how about changing entire sentence to something in English? :)
Best regards,
Krzysztof
>* for framebuffer.
>*/
> if (IS_NONCONTIG_BUFFER(flags)) {
> - DRM_ERROR("cannot use this gem memory type for fb.\n");
> + DRM_ERROR("Non-continguous GEM memory is not supported.\n");
> return -EINVAL;
> }
>
> --
> 2.7.4
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[PATCH] exynos-drm: Fix unsupported GEM memory type error message to be clear
On 08/08/2016 10:56 AM, Krzysztof Kozlowski wrote:
> On Fri, Aug 05, 2016 at 07:50:06PM -0600, Shuah Khan wrote:
>> Fix unsupported GEM memory type error message to include the memory type
>> information.
>>
>> Signed-off-by: Shuah Khan
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_fb.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> index e016640..c9315df 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> @@ -55,11 +55,11 @@ static int check_fb_gem_memory_type(struct drm_device
>> *drm_dev,
>> flags = exynos_gem->flags;
>>
>> /*
>> - * without iommu support, not support physically non-continuous memory
>> + * without iommu support, not support physically non-contiguous memory
>
> While at it, how about changing entire sentence to something in English? :)
Yes I can do that. Will send v2 :)
>
> Best regards,
> Krzysztof
>
>> * for framebuffer.
>> */
>> if (IS_NONCONTIG_BUFFER(flags)) {
>> -DRM_ERROR("cannot use this gem memory type for fb.\n");
>> +DRM_ERROR("Non-continguous GEM memory is not supported.\n");
>> return -EINVAL;
>> }
>>
>> --
>> 2.7.4
>>
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[PATCH v7 1/5] drm: Read DP branch device HW revision
HW revision is mandatory field for DisplayPort branch
devices. This is defined in DPCD register field 0x509.
v2: move drm_dp_ds_revision structure to be part of
drm_dp_link structure (Daniel)
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/drm_dp_helper.c | 27 +++
drivers/gpu/drm/i915/intel_drv.h | 1 +
include/drm/drm_dp_helper.h | 9 +
3 files changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 75b2873..5fecdc1 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -514,6 +514,33 @@ int drm_dp_downstream_max_bpc(const u8
dpcd[DP_RECEIVER_CAP_SIZE],
EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
/**
+ * drm_dp_downstream_hw_rev() - read DP branch device HW revision
+ * @aux: DisplayPort AUX channel
+ *
+ * Returns 0 on succes or negative error code on failure
+ */
+int drm_dp_downstream_hw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+struct drm_dp_aux *aux, struct drm_dp_link *link)
+{
+ uint8_t tmp;
+ int err;
+
+ if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
+ return -EINVAL;
+
+ err = drm_dp_dpcd_read(aux, DP_BRANCH_HW_REV, &tmp, 1);
+
+ if (err < 0)
+ return err;
+
+ link->ds_hw_rev.major = (tmp & 0xf0) >> 4;
+ link->ds_hw_rev.minor = tmp & 0xf;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_downstream_hw_rev);
+
+/**
* drm_dp_downstream_id() - identify branch device
* @aux: DisplayPort AUX channel
*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e74d851..a6eccf5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -865,6 +865,7 @@ struct intel_dp {
uint8_t num_sink_rates;
int sink_rates[DP_MAX_SUPPORTED_RATES];
struct drm_dp_aux aux;
+ struct drm_dp_link link;
uint8_t train_set[4];
int panel_power_up_delay;
int panel_power_down_delay;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8e1fe58..1127948 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -446,6 +446,7 @@
#define DP_SINK_OUI0x400
#define DP_BRANCH_OUI 0x500
#define DP_BRANCH_ID0x503
+#define DP_BRANCH_HW_REV0x509
#define DP_SET_POWER0x600
# define DP_SET_POWER_D00x1
@@ -803,11 +804,17 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
*/
#define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)
+struct drm_dp_ds_revision {
+ int major;
+ int minor;
+};
+
struct drm_dp_link {
unsigned char revision;
unsigned int rate;
unsigned int num_lanes;
unsigned long capabilities;
+ struct drm_dp_ds_revision ds_hw_rev;
};
int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
@@ -819,6 +826,8 @@ int drm_dp_downstream_max_clock(const u8
dpcd[DP_RECEIVER_CAP_SIZE],
int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
const u8 port_cap[4]);
int drm_dp_downstream_id(struct drm_dp_aux *aux, char id[6]);
+int drm_dp_downstream_hw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+struct drm_dp_aux *aux, struct drm_dp_link *link);
void drm_dp_aux_init(struct drm_dp_aux *aux);
int drm_dp_aux_register(struct drm_dp_aux *aux);
--
1.9.1
[PATCH 2/7] staging/android: display sync_pt name on debugfs
2016-08-08 Maarten Lankhorst :
> Op 20-06-16 om 17:53 schreef Gustavo Padovan:
> > From: Gustavo Padovan
> >
> > When creating a sync_pt the name received wasn't used anywhere.
> > Now we add it to the sync info debug output to make it easier to indetify
> > the userspace name of that sync pt.
> >
> > Signed-off-by: Gustavo Padovan
> > ---
> > drivers/staging/android/sw_sync.c| 16
> > drivers/staging/android/sync_debug.c | 5 +++--
> > drivers/staging/android/sync_debug.h | 9 +
> > 3 files changed, 16 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/staging/android/sw_sync.c
> > b/drivers/staging/android/sw_sync.c
> > index b4ae092..ea27512 100644
> > --- a/drivers/staging/android/sw_sync.c
> > +++ b/drivers/staging/android/sw_sync.c
> > @@ -37,15 +37,6 @@ struct sw_sync_create_fence_data {
> > struct sw_sync_create_fence_data)
> > #define SW_SYNC_IOC_INC_IOW(SW_SYNC_IOC_MAGIC, 1,
> > __u32)
> >
> > -static const struct fence_ops timeline_fence_ops;
> > -
> > -static inline struct sync_pt *fence_to_sync_pt(struct fence *fence)
> > -{
> > - if (fence->ops != &timeline_fence_ops)
> > - return NULL;
> > - return container_of(fence, struct sync_pt, base);
> > -}
> > -
> > struct sync_timeline *sync_timeline_create(const char *name)
> > {
> > struct sync_timeline *obj;
> > @@ -108,7 +99,7 @@ static void sync_timeline_signal(struct sync_timeline
> > *obj, unsigned int inc)
> > }
> >
> > static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size,
> > -unsigned int value)
> > +unsigned int value, char *name)
> > {
> > unsigned long flags;
> > struct sync_pt *pt;
> > @@ -120,6 +111,7 @@ static struct sync_pt *sync_pt_create(struct
> > sync_timeline *obj, int size,
> > if (!pt)
> > return NULL;
> >
> > + strlcpy(pt->name, name, sizeof(pt->name));
> > spin_lock_irqsave(&obj->child_list_lock, flags);
> > sync_timeline_get(obj);
> > fence_init(&pt->base, &timeline_fence_ops, &obj->child_list_lock,
> > @@ -191,7 +183,7 @@ static void timeline_fence_timeline_value_str(struct
> > fence *fence,
> > snprintf(str, size, "%d", parent->value);
> > }
> >
> > -static const struct fence_ops timeline_fence_ops = {
> > +const struct fence_ops timeline_fence_ops = {
> > .get_driver_name = timeline_fence_get_driver_name,
> > .get_timeline_name = timeline_fence_get_timeline_name,
> > .enable_signaling = timeline_fence_enable_signaling,
> > @@ -252,7 +244,7 @@ static long sw_sync_ioctl_create_fence(struct
> > sync_timeline *obj,
> > goto err;
> > }
> >
> > - pt = sync_pt_create(obj, sizeof(*pt), data.value);
> > + pt = sync_pt_create(obj, sizeof(*pt), data.value, data.name);
> > if (!pt) {
> > err = -ENOMEM;
> > goto err;
> > diff --git a/drivers/staging/android/sync_debug.c
> > b/drivers/staging/android/sync_debug.c
> > index 4c5a855..b732ea3 100644
> > --- a/drivers/staging/android/sync_debug.c
> > +++ b/drivers/staging/android/sync_debug.c
> > @@ -75,13 +75,14 @@ static void sync_print_fence(struct seq_file *s, struct
> > fence *fence, bool show)
> > {
> > int status = 1;
> > struct sync_timeline *parent = fence_parent(fence);
> > + struct sync_pt *pt = fence_to_sync_pt(fence);
> >
> > if (fence_is_signaled_locked(fence))
> > status = fence->status;
> >
> > - seq_printf(s, " %s%sfence %s",
> > + seq_printf(s, " %s%sfence %s %s",
> >show ? parent->name : "",
> > - show ? "_" : "",
> > + show ? "_" : "", pt->name,
> >sync_status_str(status));
> >
> NAK,
> A fence in sync_print_fence can be of any type. If you want to print the
> name, use the fence_value_str callback.
Indeed. But fence_value_str doesn't return the sync_pt name, but the
seqno of that fence. I'll keep this change out for the de-staging and
then try to come with something that works better.
Gustavo
[PATCH] drm/cirrus: Fix NULL pointer dereference when registering the fbdev
Boris Brezillon writes:
> cirrus_modeset_init() is initializing/registering the emulated fbdev
> and, since commit c61b93fe51b1 ("drm/atomic: Fix remaining places where
> !funcs->best_encoder is valid"), DRM internals can access/test some of
> the fields in mode_config->funcs as part of the fbdev registration
> process.
> Make sure dev->mode_config.funcs is properly set to avoid dereferencing
> a NULL pointer.
That fixes the issues I am seeing.
Tested-by: "Eric W. Biederman"
> Signed-off-by: Boris Brezillon
> Fixes: c61b93fe51b1 ("drm/atomic: Fix remaining places where
> !funcs->best_encoder is valid")
> ---
> Hi Dave,
>
> As discussed on IRC, I'm sending this patch in a proper format. That's
> probably better to wait for Eric's feeback before applying it though.
It is weird I didn't see either of your email messages directly only
through lkml. Weird.
> Regards,
>
> Boris
> ---
> drivers/gpu/drm/cirrus/cirrus_main.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c
> b/drivers/gpu/drm/cirrus/cirrus_main.c
> index 80446e2d3ab6..76bcb43e7c06 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_main.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_main.c
> @@ -185,14 +185,23 @@ int cirrus_driver_load(struct drm_device *dev, unsigned
> long flags)
> goto out;
> }
>
> + /*
> + * cirrus_modeset_init() is initializing/registering the emulated fbdev
> + * and DRM internals can access/test some of the fields in
> + * mode_config->funcs as part of the fbdev registration process.
> + * Make sure dev->mode_config.funcs is properly set to avoid
> + * dereferencing a NULL pointer.
> + * FIXME: mode_config.funcs assignment should probably be done in
> + * cirrus_modeset_init() (that's a common pattern seen in other DRM
> + * drivers).
> + */
> + dev->mode_config.funcs = &cirrus_mode_funcs;
> r = cirrus_modeset_init(cdev);
> if (r) {
> dev_err(&dev->pdev->dev, "Fatal error during modeset init:
> %d\n", r);
> goto out;
> }
>
> - dev->mode_config.funcs = (void *)&cirrus_mode_funcs;
> -
> return 0;
> out:
> cirrus_driver_unload(dev);
