Re: [PATCH 35/45] drm/vram-helper: move to invalidate callback.
Hi
Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/drm_gem_vram_helper.c | 16 ++--
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 5d4182f5c22f..9d4100071e1d 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -433,7 +433,7 @@ static void drm_gem_vram_kunmap_locked(struct
> drm_gem_vram_object *gbo)
>* Permanently mapping and unmapping buffers adds overhead from
>* updating the page tables and creates debugging output. Therefore,
>* we delay the actual unmap operation until the BO gets evicted
> - * from memory. See drm_gem_vram_bo_driver_move_notify().
> + * from memory. See drm_gem_vram_bo_driver_invalidate_notify().
>*/
> }
>
> @@ -585,9 +585,7 @@ static void drm_gem_vram_bo_driver_evict_flags(struct
> drm_gem_vram_object *gbo,
> *pl = gbo->placement;
> }
>
> -static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object
> *gbo,
> -bool evict,
> -struct ttm_resource *new_mem)
> +static void drm_gem_vram_bo_driver_invalidate_notify(struct
> drm_gem_vram_object *gbo)
> {
> struct ttm_bo_kmap_obj *kmap = &gbo->kmap;
>
> @@ -605,7 +603,7 @@ static int drm_gem_vram_bo_driver_move(struct
> drm_gem_vram_object *gbo,
> struct ttm_operation_ctx *ctx,
> struct ttm_resource *new_mem)
> {
> - drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
> + drm_gem_vram_bo_driver_invalidate_notify(gbo);
> return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
> }
I don't fully understand TTM's order of operation, so this might be a
dumb question: why is invalidate_notify() called from within the move()
callback? I'd expect that the invalidate_notify() callback is called by
TTM before moving the BO?
>
> @@ -956,9 +954,7 @@ static void bo_driver_evict_flags(struct
> ttm_buffer_object *bo,
> drm_gem_vram_bo_driver_evict_flags(gbo, placement);
> }
>
> -static void bo_driver_move_notify(struct ttm_buffer_object *bo,
> - bool evict,
> - struct ttm_resource *new_mem)
> +static void bo_driver_invalidate_notify(struct ttm_buffer_object *bo)
> {
> struct drm_gem_vram_object *gbo;
>
> @@ -968,7 +964,7 @@ static void bo_driver_move_notify(struct
> ttm_buffer_object *bo,
>
> gbo = drm_gem_vram_of_bo(bo);
>
> - drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
> + drm_gem_vram_bo_driver_invalidate_notify(gbo);
> }
>
> static int bo_driver_move(struct ttm_buffer_object *bo,
> @@ -1008,7 +1004,7 @@ static struct ttm_bo_driver bo_driver = {
> .eviction_valuable = ttm_bo_eviction_valuable,
> .evict_flags = bo_driver_evict_flags,
> .move = bo_driver_move,
> - .move_notify = bo_driver_move_notify,
> + .invalidate_notify = bo_driver_invalidate_notify,
> .io_mem_reserve = bo_driver_io_mem_reserve,
> };
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] MAINTAINERS: add Dan Murphy as TP LP8xxx drivers maintainer
On Wed, 23 Sep 2020 at 22:01, Jonathan Cameron wrote: > > On Wed, 23 Sep 2020 11:53:33 -0500 > Dan Murphy wrote: > > > Hello > > > > On 9/22/20 10:28 AM, Krzysztof Kozlowski wrote: > > > Milo Kim's email in TI bounces with permanent error (550: Invalid > > > recipient). Last email from him on LKML was in 2017. Move Milo Kim to > > > credits and add Dan Murphy from TI to look after: > > > - TI LP855x backlight driver, > > > - TI LP8727 charger driver, > > > - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers. > > > > > > Cc: Dan Murphy > > > Signed-off-by: Krzysztof Kozlowski > > > > Acked-by: Dan Murphy > > > Not sure who will pick this one up, but > Acked-by: Jonathan Cameron I guess whoever is first. :) This spans across systems but the common part is MFD, so maybe Lee - could you pick it up? Best regards, Krzysztof ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] dma-buf: Use struct dma_buf_map in dma_buf_vmap() interfaces
Hi
Am 23.09.20 um 17:39 schrieb Daniel Vetter:
> On Wed, Sep 23, 2020 at 02:32:04PM +0200, Thomas Zimmermann wrote:
>> This patch updates dma_buf_vmap() and dma-buf's vmap callback to use
>> struct dma_buf_map.
>>
>> The interfaces used to return a buffer address. This address now gets
>> stored in an instance of the structure that is given as an additional
>> argument. The functions return an errno code on errors.
>>
>> Users of the functions are updated accordingly. This is only an interface
>> change. It is currently expected that dma-buf memory can be accessed with
>> system memory load/store operations.
>>
>> v2:
>> * always clear map parameter in dma_buf_vmap() (Daniel)
>> * include dma-buf-heaps and i915 selftests (kernel test robot)
>>
>> Signed-off-by: Thomas Zimmermann
>> Acked-by: Sumit Semwal
>
> Too lazy to check for all possible conversion issues, but I think I've
> done a close look last time around. As long as this is build tested across
> all drivers and architectures we care about we should be fine.
I built for x86-64, i586, aarch64 and arm. Apparently the kernel test
robot keeps digging up new build errors, so I'll double check before
committing. In any case, the errors should be trivial to fix.
Best regards
Thomas
>
> Acked-by: Daniel Vetter
>> ---
>> drivers/dma-buf/dma-buf.c | 28 +++
>> drivers/dma-buf/heaps/heap-helpers.c | 8 --
>> drivers/gpu/drm/drm_gem_cma_helper.c | 13 +
>> drivers/gpu/drm/drm_gem_shmem_helper.c| 14 ++
>> drivers/gpu/drm/drm_prime.c | 8 --
>> drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 8 +-
>> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c| 11 ++--
>> .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 12 ++--
>> .../gpu/drm/i915/gem/selftests/mock_dmabuf.c | 10 +--
>> drivers/gpu/drm/tegra/gem.c | 18
>> .../common/videobuf2/videobuf2-dma-contig.c | 14 +++---
>> .../media/common/videobuf2/videobuf2-dma-sg.c | 16 +++
>> .../common/videobuf2/videobuf2-vmalloc.c | 15 +++---
>> include/drm/drm_prime.h | 3 +-
>> include/linux/dma-buf-map.h | 13 +
>> include/linux/dma-buf.h | 6 ++--
>> 16 files changed, 138 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 5e849ca241a0..61bd24d21b38 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -1186,46 +1186,50 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>> * dma_buf_vmap - Create virtual mapping for the buffer object into kernel
>> * address space. Same restrictions as for vmap and friends apply.
>> * @dmabuf: [in]buffer to vmap
>> + * @map:[out] returns the vmap pointer
>> *
>> * This call may fail due to lack of virtual mapping address space.
>> * These calls are optional in drivers. The intended use for them
>> * is for mapping objects linear in kernel space for high use objects.
>> * Please attempt to use kmap/kunmap before thinking about these interfaces.
>> *
>> - * Returns NULL on error.
>> + * Returns 0 on success, or a negative errno code otherwise.
>> */
>> -void *dma_buf_vmap(struct dma_buf *dmabuf)
>> +int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map)
>> {
>> -void *ptr;
>> +struct dma_buf_map ptr;
>> +int ret = 0;
>> +
>> +dma_buf_map_clear(map);
>>
>> if (WARN_ON(!dmabuf))
>> -return NULL;
>> +return -EINVAL;
>>
>> if (!dmabuf->ops->vmap)
>> -return NULL;
>> +return -EINVAL;
>>
>> mutex_lock(&dmabuf->lock);
>> if (dmabuf->vmapping_counter) {
>> dmabuf->vmapping_counter++;
>> BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr));
>> -ptr = dmabuf->vmap_ptr.vaddr;
>> +*map = dmabuf->vmap_ptr;
>> goto out_unlock;
>> }
>>
>> BUG_ON(dma_buf_map_is_set(&dmabuf->vmap_ptr));
>>
>> -ptr = dmabuf->ops->vmap(dmabuf);
>> -if (WARN_ON_ONCE(IS_ERR(ptr)))
>> -ptr = NULL;
>> -if (!ptr)
>> +ret = dmabuf->ops->vmap(dmabuf, &ptr);
>> +if (WARN_ON_ONCE(ret))
>> goto out_unlock;
>>
>> -dmabuf->vmap_ptr.vaddr = ptr;
>> +dmabuf->vmap_ptr = ptr;
>> dmabuf->vmapping_counter = 1;
>>
>> +*map = dmabuf->vmap_ptr;
>> +
>> out_unlock:
>> mutex_unlock(&dmabuf->lock);
>> -return ptr;
>> +return ret;
>> }
>> EXPORT_SYMBOL_GPL(dma_buf_vmap);
>>
>> diff --git a/drivers/dma-buf/heaps/heap-helpers.c
>> b/drivers/dma-buf/heaps/heap-helpers.c
>> index d0696cf937af..aeb9e100f339 100644
>> --- a/drivers/dma-buf/heaps/heap-helpers.c
>> +++ b/drivers/dma-buf/heaps/heap-helpers.c
>> @@ -235,7 +235,7 @@ static int dma_heap_dma_buf_end_cpu_access(struct
>> dma_buf *dmabuf,
>> return 0;
>> }
>>
>> -s
Re: [PATCH] drm/msm/dp: fix incorrect function prototype of dp_debug_get()
Quoting Abhinav Kumar (2020-09-23 16:24:48)
> Fix the incorrect function prototype for dp_debug_get()
> in the dp_debug module to address compilation warning.
>
> Fixes: f913454aae8e ("drm/msm/dp: move debugfs node to
> /sys/kernel/debug/dri/*/")
> Reported-by: kernel test robot
> Signed-off-by: Abhinav Kumar
> ---
Reviewed-by: Stephen Boyd
Will the compliance testing parts be moved out of debugfs at some point?
Just curious what the plan is there.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active
On Fri, Sep 18, 2020 at 04:43:20PM +0100, Dave Stevenson wrote:
> Hi Maxime
>
> Thanks for the patch.
>
> On Fri, 18 Sep 2020 at 15:59, Maxime Ripard wrote:
> >
> > The HVS has three FIFOs that can be assigned to a number of PixelValves
> > through a mux.
> >
> > However, changing that FIFO requires that we disable and then enable the
> > pixelvalve, so we want to assign FIFOs to all the enabled CRTCs, and not
> > just the active ones.
> >
> > Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel
> > automatically")
> > Signed-off-by: Maxime Ripard
>
> Tested-by: Dave Stevenson
> Reviewed-by: Dave Stevenson
Applied, thanks!
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Mon, Sep 21 2020 at 21:27, Thomas Gleixner wrote: > On Mon, Sep 21 2020 at 09:24, Linus Torvalds wrote: >> On Mon, Sep 21, 2020 at 12:39 AM Thomas Gleixner wrote: >> Maybe we really *could* call this new kmap functionality something >> like "kmap_percpu()" (or maybe "local" is good enough), and make it >> act like your RT code does for spinlocks - not disable preemption, but >> only disabling CPU migration. > > I"m all for it, but the scheduler people have opinions :) I just took the latest version of migrate disable patches https://lore.kernel.org/r/20200921163557.234036...@infradead.org removed the RT dependency on top of them and adopted the kmap stuff (addressing the various comments while at it and unbreaking ARM). I'm not going to post that until there is consensus about the general availability of migrate disable, but for those who want to play with it I've pushed the resulting combo out to: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git highmem For testing I've tweaked a few places to use the new _local() variants and it survived testing so far and I've verified that there is actual preemption which means zap/restore of the thread local kmaps. Thanks, tglx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/vc4: crtc: Rework a bit the CRTC state code
The current CRTC state reset hook in vc4 allocates a vc4_crtc_state
structure as a drm_crtc_state, and relies on the fact that vc4_crtc_state
embeds drm_crtc_state as its first member, and therefore can be safely
casted.
However, this is pretty fragile especially since there's no check for this
in place, and we're going to need to access vc4_crtc_state member at reset
so this looks like a good occasion to make it more robust.
Signed-off-by: Maxime Ripard
Tested-by: Dave Stevenson
---
Changes from v1:
- new patch
---
drivers/gpu/drm/vc4/vc4_crtc.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index a393f93390a2..7ef20adedee5 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -852,11 +852,18 @@ void vc4_crtc_destroy_state(struct drm_crtc *crtc,
void vc4_crtc_reset(struct drm_crtc *crtc)
{
+ struct vc4_crtc_state *vc4_crtc_state;
+
if (crtc->state)
vc4_crtc_destroy_state(crtc, crtc->state);
- crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
- if (crtc->state)
- __drm_atomic_helper_crtc_reset(crtc, crtc->state);
+
+ vc4_crtc_state = kzalloc(sizeof(*vc4_crtc_state), GFP_KERNEL);
+ if (!vc4_crtc_state) {
+ crtc->state = NULL;
+ return;
+ }
+
+ __drm_atomic_helper_crtc_reset(crtc, &vc4_crtc_state->base);
}
static const struct drm_crtc_funcs vc4_crtc_funcs = {
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm/vc4: crtc: Keep the previously assigned HVS FIFO
The HVS FIFOs are currently assigned each time we have an atomic_check
for all the enabled CRTCs.
However, if we are running multiple outputs in parallel and we happen to
disable the first (by index) CRTC, we end up changing the assigned FIFO
of the second CRTC without disabling and reenabling the pixelvalve which
ends up in a stall and eventually a VBLANK timeout.
In order to fix this, we can create a special value for our assigned
channel to mark it as disabled, and if our CRTC already had an assigned
channel in its previous state, we keep on using it.
Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
Signed-off-by: Maxime Ripard
Tested-by: Dave Stevenson
---
Changes from v1:
- Split away the crtc state reset refactoring
- Fixed the checkpatch warnings
---
drivers/gpu/drm/vc4/vc4_crtc.c | 1 +
drivers/gpu/drm/vc4/vc4_drv.h | 2 ++
drivers/gpu/drm/vc4/vc4_kms.c | 22 --
3 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 7ef20adedee5..482219fb4db2 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -863,6 +863,7 @@ void vc4_crtc_reset(struct drm_crtc *crtc)
return;
}
+ vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
__drm_atomic_helper_crtc_reset(crtc, &vc4_crtc_state->base);
}
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 8c8d96b6289f..90b911fd2a7f 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -532,6 +532,8 @@ struct vc4_crtc_state {
} margins;
};
+#define VC4_HVS_CHANNEL_DISABLED ((unsigned int)-1)
+
static inline struct vc4_crtc_state *
to_vc4_crtc_state(struct drm_crtc_state *crtc_state)
{
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 01fa60844695..149825ff5df8 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -616,7 +616,7 @@ static int
vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
{
unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0);
- struct drm_crtc_state *crtc_state;
+ struct drm_crtc_state *old_crtc_state, *new_crtc_state;
struct drm_crtc *crtc;
int i, ret;
@@ -629,6 +629,8 @@ vc4_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
* modified.
*/
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ struct drm_crtc_state *crtc_state;
+
if (!crtc->state->enable)
continue;
@@ -637,15 +639,23 @@ vc4_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
return PTR_ERR(crtc_state);
}
- for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
- struct vc4_crtc_state *vc4_crtc_state =
- to_vc4_crtc_state(crtc_state);
+ for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
+ struct vc4_crtc_state *new_vc4_crtc_state =
+ to_vc4_crtc_state(new_crtc_state);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
unsigned int matching_channels;
- if (!crtc_state->enable)
+ if (old_crtc_state->enable && !new_crtc_state->enable)
+ new_vc4_crtc_state->assigned_channel =
VC4_HVS_CHANNEL_DISABLED;
+
+ if (!new_crtc_state->enable)
continue;
+ if (new_vc4_crtc_state->assigned_channel !=
VC4_HVS_CHANNEL_DISABLED) {
+ unassigned_channels &=
~BIT(new_vc4_crtc_state->assigned_channel);
+ continue;
+ }
+
/*
* The problem we have to solve here is that we have
* up to 7 encoders, connected to up to 6 CRTCs.
@@ -674,7 +684,7 @@ vc4_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
if (matching_channels) {
unsigned int channel = ffs(matching_channels) - 1;
- vc4_crtc_state->assigned_channel = channel;
+ new_vc4_crtc_state->assigned_channel = channel;
unassigned_channels &= ~BIT(channel);
} else {
return -EINVAL;
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Wed, Sep 23 2020 at 17:12, Steven Rostedt wrote: > On Wed, 23 Sep 2020 22:55:54 +0200 > Then scratch the idea of having anonymous local_lock() and just bring > local_lock in directly? Then have a kmap local lock, which would only > block those that need to do a kmap. That's still going to end up in lock ordering nightmares and you lose the ability to use kmap_local from arbitrary contexts which was again one of the goals of this exercise. Aside of that you're imposing reentrancy protections on something which does not need it in the first place. > Now as for migration disabled nesting, at least now we would have > groupings of this, and perhaps the theorists can handle that. I mean, > how is this much different that having a bunch of tasks blocked on a > mutex with the owner is pinned on a CPU? > > migrate_disable() is a BKL of pinning affinity. No. That's just wrong. preempt disable is a concurrency control, i.e. protecting against reentrancy on a given CPU. But it's a cpu global protection which means that it's not protecting a specific code path. Contrary to preempt disable, migrate disable is not protecting against reentrancy on a given CPU. It's a temporary restriction to the scheduler on placement. The fact that disabling preemption implicitely disables migration does not make them semantically equivalent. > If we only have local_lock() available (even on !RT), then it makes > the blocking in groups. At least this way you could grep for all the > different local_locks in the system and plug that into the algorithm > for WCS, just like one would with a bunch of mutexes. You cannot do that on RT at all where migrate disable is substituting preempt disable in spin and rw locks. The result would be the same as with a !RT kernel just with horribly bad performance. That means the stacking problem has to be solved anyway. So why on earth do you want to create yet another special duct tape case for kamp_local() which proliferates inconsistency instead of aiming for consistency accross all preemption models? Thanks, tglx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Wed, Sep 23 2020 at 12:19, peterz wrote: > On Mon, Sep 21, 2020 at 09:27:57PM +0200, Thomas Gleixner wrote: >> Alternatively this could of course be solved with per CPU page tables >> which will come around some day anyway I fear. > > Previously (with PTI) we looked at making the entire kernel map per-CPU, > and that takes a 2K copy on switch_mm() (or more general, the user part > of whatever the top level directory is for architectures that have a > shared kernel/user page-table setup in the first place). > > The idea was having a fixed per-cpu kernel page-table, share a bunch of > (kernel) page-tables between all CPUs and then copy in the user part on > switch. > > I've forgotten what the plan was for ASID/PCID in that scheme. > > For x86_64 we've been fearing the performance of that 2k copy, but I > don't think we've ever actually bit the bullet and implemented it to see > how bad it really is. I actually did at some point and depending on the workload the overhead was clearly measurable. And yes, it fell apart with PCID and I could not come up with a scheme for it which did not suck horribly. So I burried the patches in the poison cabinet. Aside of that, we'd need to implement that for a eight other architectures as well... Thanks, tglx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Wed, Sep 23 2020 at 10:40, peterz wrote: > Right, so I'm concerned. migrate_disable() wrecks pretty much all > Real-Time scheduler theory we have, and PREEMPRT_RT bringing it in is > somewhat ironic. It's even more ironic that the approach of PREEMPT_RT has been 'pragmatic ignorance of theory' from the very beginning and despite violating all theories it still works. :) > Yes, it allows breaking up non-preemptible regions of non-deterministic > duration, and thereby both reduce and bound the scheduling latency, the > cost for doing that is that the theory on CPU utilization/bandwidth go > out the window. I agree, that the theory goes out of the window, but does it matter in practice? I've yet to see a report of migrate disable stacking being the culprit of a missed deadline and I surely have stared at lots of reports in the past 10+ years. > To easily see this consider an SMP system with a number of tasks equal > to the number of CPUs. On a regular (preempt_disable) kernel we can > always run each task, by virtue of always having an idle CPU to take the > task. > > However, with migrate_disable() we can have each task preempted in a > migrate_disable() region, worse we can stack them all on the _same_ CPU > (super ridiculous odds, sure). And then we end up only able to run one > task, with the rest of the CPUs picking their nose. > > The end result is that, like with unbounded latency, we will still miss > our deadline, simply because we got starved for CPU. I'm well aware of that. > Now, while we could (with a _lot_ of work) rework the kernel to not rely > on the implicit per-cpu ness of things like spinlock_t, the moment we > bring in basic primitives that rely on migrate_disable() we're stuck > with it. Right, but we are stuck with per CPUness and distangling that is just infeasible IMO. > The problem is; afaict there's been no research into this problem. There is no research on a lot of things the kernel does :) > There might be scheduling (read: balancing) schemes that can > mitigate/solve this problem, or it might prove to be a 'hard' problem, > I just don't know. In practive balancing surely can take the number of preempted tasks which are in a migrate disable section into account which would be just another measure to work around the fact that the kernel is not adhering to the theories. It never did that even w/o migrate disable. > But once we start down this road, it's going to be hell to get rid of > it. Like most of the other things the kernel came up with to deal with the oddities of modern hardware :) > That's why I've been arguing (for many years) to strictly limit this to > PREEMPT_RT and only as a gap-stop, not a fundamental primitive to build > on. I know, but short of rewriting the world, I'm not seing the faintest plan to remove the stop gap. :) As we discussed not long ago we have too many inconsistent preemption models already. RT is adding yet another one. And that's worse than introducing migrate disable as a general available facility. IMO, reaching a point of consistency where our different preemption models (NONE, VOLUNTARY, PREEMPT. RT) build on each other is far more important. For !RT migrate disable is far less of an danger than for RT kernels because the amount of code which will use it is rather limited compared to code which still will disable preemption implicit through spin and rw locks. On RT converting these locks to 'sleepable spinlocks' is just possible because RT forces almost everything into task context and migrate disable is just the obvious decomposition of preempt disable which implicitely disables migration. But that means that RT is by orders of magnitude more prone to run into the scheduling trainwreck you are worried about. It just refuses to do so at least with real world work loads. I'm surely in favour of having solid theories behind implementation, but at some point you just have to bite the bullet and chose pragmatism in order to make progress. Proliferating inconsistency is not real progress, as it is violating the most fundamental engineering principles. That's by far more dangerous than violating scheduling theories which are built on perfect models and therefore enforce violation by practical implementations anyway. Thanks, tglx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: skip probed failed device
Heiko Stübner 於 2020年9月23日 週三 下午7:16寫道：
>
> Am Mittwoch, 23. September 2020, 13:05:26 CEST schrieb Robin Murphy:
> > On 2020-09-23 07:59, Jian-Hong Pan wrote:
> > > The cdn-dp sub driver probes the device failed on PINEBOOK Pro.
> > >
> > > kernel: cdn-dp fec0.dp: [drm:cdn_dp_probe [rockchipdrm]] *ERROR*
> > > missing extcon or phy
> > > kernel: cdn-dp: probe of fec0.dp failed with error -22
> >
> > Wouldn't it make more sense to simply not enable the DisplayPort node in
> > the upstream DT, until the type-C phy work has been done to make it
> > usable at all?
>
> Or alternatively just disable the cdn-dp Rockchip driver in the kernel config,
> which results in it also not getting probed.
This may be the simplest way.
However, considering generic distro kernels have a policy to enable
all drivers, disabling the DisplayPort node in the upstream DT, until
the type-C phy work has been done may be a better solution for now.
I can prepare a patch for this.
Jian-Hong Pan
> > AIUI the "official" Manjaro kernel is carrying a bunch of
> > hacks to make type-C work via extcon, but they know that isn't an
> > upstreamable solution.
> >
> > Robin.
> >
> > > Then, the device halts all of the DRM related device jobs. For example,
> > > the operations: vop_component_ops, vop_component_ops and
> > > rockchip_dp_component_ops cannot be bound to corresponding devices. So,
> > > Xorg cannot find the correct DRM device.
> > >
> > > This patch skips the probing failed devices to fix this issue.
> > >
> > > Link:
> > > http://lists.infradead.org/pipermail/linux-rockchip/2020-September/022352.html
> > > Signed-off-by: Jian-Hong Pan
> > > ---
> > > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > > b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > > index 0f3eb392fe39..de13588602b4 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > > @@ -331,6 +331,12 @@ static struct component_match
> > > *rockchip_drm_match_add(struct device *dev)
> > >
> > > if (!d)
> > > break;
> > > + if (!d->driver) {
> > > + DRM_DEV_ERROR(d,
> > > + "%s did not probe successfully",
> > > + drv->driver.name);
> > > + continue;
> > > + }
> > >
> > > device_link_add(dev, d, DL_FLAG_STATELESS);
> > > component_match_add(dev, &match, compare_dev, d);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Wed, Sep 23 2020 at 11:52, Steven Rostedt wrote: > On Wed, 23 Sep 2020 10:40:32 +0200 > pet...@infradead.org wrote: > >> However, with migrate_disable() we can have each task preempted in a >> migrate_disable() region, worse we can stack them all on the _same_ CPU >> (super ridiculous odds, sure). And then we end up only able to run one >> task, with the rest of the CPUs picking their nose. > > What if we just made migrate_disable() a local_lock() available for !RT? > > I mean make it a priority inheritance PER CPU lock. > > That is, no two tasks could do a migrate_disable() on the same CPU? If > one task does a migrate_disable() and then gets preempted and the > preempting task does a migrate_disable() on the same CPU, it will block > and wait for the first task to do a migrate_enable(). > > No two tasks on the same CPU could enter the migrate_disable() section > simultaneously, just like no two tasks could enter a preempt_disable() > section. > > In essence, we just allow local_lock() to be used for both RT and !RT. > > Perhaps make migrate_disable() an anonymous local_lock()? > > This should lower the SHC in theory, if you can't have stacked migrate > disables on the same CPU. I'm pretty sure this ends up in locking hell pretty fast and aside of that it's not working for scenarios like: kmap_local(); migrate_disable(); ... copy_from_user() -> #PF -> schedule() which brought us into that discussion in the first place. You would stop any other migrate disable user from running until the page fault is resolved... Thanks, tglx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] linux-firmware: Update firmware for Cadence MHDP8546 DP bridge
Update firmware for Cadence MHDP8546 DP bridge to version 2.0.0.
The firmware source code now complies with MISRA2012 and HIS
rules and directives. Also, there are some improvements in AUX
channel communication handling.
Signed-off-by: Swapnil Jakhade
---
WHENCE | 4 ++--
cadence/mhdp8546.bin | Bin 131072 -> 131072 bytes
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/WHENCE b/WHENCE
index 7511677..d4bd8cf 100644
--- a/WHENCE
+++ b/WHENCE
@@ -5003,10 +5003,10 @@ Licence:
--
-Driver: cdns-mhdp - Cadence MHDP DP bridge
+Driver: cdns-mhdp - Cadence MHDP8546 DP bridge
File: cadence/mhdp8546.bin
-Version: 1.2.15
+Version: 2.0.0
Licence: Redistributable. See LICENCE.cadence for details
diff --git a/cadence/mhdp8546.bin b/cadence/mhdp8546.bin
index
40097e0ec908ab5c4b54803f4b99519ced7a5e3d..2f85f57c506e3347c1f4670b9a0b96e415d2eaaa
100755
GIT binary patch
literal 131072
zcmeFZd03NI_Bj47Nyu_lf=ZEqCV?m-b^_XJtrh|l*|b2Fy0rtrAmUQBbabqcHg*nGZ>=#f6y=tt-wzrmahLicB+gVF{5Eq?gSMTS-)H9Y&+qv@
z-{<-Gc)8oT+qvhSd+s^sjX;253Z+tsVOaYL3{yk={Xc|Z-%bvCS)DUko{cF6*z`d*
zdyqwBA(58rdyemgi^)mD=_J;X(J8uq-v#F
z{&$Xz4;I{lF239K3j9rg?*H}Q#U!jvMU=Kyl(k8;?Jbevps4IU5&f~K;naUW
z7rArt9tumx1T6cnbNio^Xt0DaXzJ#%(fom
z=$nvkox?`wLH-Mf*F$U%!!SGa-3npIpN+1W!bX1ta1oF`3h^~44}FS_Rzo=EkEuha
zveBCZFtsn_-H<;9aCQjSAbbm<1;Q=}JgDD14d5Uh0ezkh!PKXr?F9%NS@_B@bz(54
zZh^WoD04&nEd;EChZ!K=CWo;Q8=d9LM#~i#hC(om2Xp;G!bbb1Vb~N10T6;9$RUJ6
zsDXY~iP`7|So{1S2yz$?)~$v$q=o_>03W_k-U4Ooq2Hr(V0cNT^TVGTfoSOw(UAv8)b
z?B-7}b;)F)L1^>ZeE0hA>{m=Abg
z1$CRB#nik7KyR>@0T5?FP(%GBSkDN=VbDehp$6z(4d-Yx&}9J3$qjwlp}Y~wK7zBd
z82UR6VHe~LkmtdiK7#r(f{mVnv(fD-Y;+>@(F`zHDx5!VJxgG&8cbaQWBw7!7sq1i
zT@Wys`!S%CEZD;s=p#N2Vwg)0&{0JqroO<@2GBw&#EnpY2jX@ZBMs)Y0>UZ?>mX!7
zSPvl&!e$6}Aap`-L+FDr1c3*k0(g-dLIm_FfN^7B&hak-ZkA%|a3~9aF&D!4Da5D!6|279s#_69*-y&Uo{Vd^kQAA{HpVLq(!*bLxlQ=zF3?1K>S
z3T@^?P^V++GN`)-dl(OWV89o6BA}5;K#vd$Ap}A2g&>Cz3IV|$2Zg~Ng~A$U!5GU{LHaU;21wt8zE(j!2Kd7?5CHFR
zC1aQh!!7_!AHbZ3vA%$Q${=s}Dc}Oi7IFddHz9u!;!6+`p{y0gRRjN>55X7sQW(S-
z1Xw$^4#o+9xHJdm0PCIt>2Cp_sSsn(UJm7Z*29^CG!H^3(4HE?9vD9nVBFBg7s`i#
zw$=e{wE}Hn`4AxfB8iP&v&Ld+*e=2DK%ZeS4<*m=POW;%*mWMt0O22fG0t
zVTzGa>By*m2B#q)Xe{y()T+ILrRE_dOXFL4TzU4u==Q5~*$Xp*aULA!Mq98{c7VPF02F$KBDTA1chRU>|cKk&F;GlS_);>*N{58a*m
zpLJ(Bj>}f2r4{6U+QR(TsOl5^2+!Lcw}xtBrXPujV7HmxtHLo_&I@)2UD9FLz7dwI
zx683GsP(QF1-cN10-+KLpst>94YRR~0X>B>BdbP@RipB>0>jESW98_gXAZ2S+Y2&(
zDKdrVJr{@lS)solZ>HZ*kl4T0XXdmmGiPVbd~RWGQid(Wu;!HQ>#63HP*aG33Ngwm
zO*w|c70+pN^yd&}rb{h@EKL_cKl|KlM6zXo-8amlq~yOiCLAy?J7{Q;q6y9%!=+1(
z9Had=#iks6J)!fk`@Uh(bIE@*FF&yF5sPLgKQOH8KQTD6v=KmbM#lMeRjkf
zkVtTsg#q?`$4!I7B@(MZJ14f>dps}WveC$tQA9+M7Y+01s1Q90pyGmO$8q2sd}IPt
zDsvRdI76_T%fIkX`RGx^Q<4TPW52m0eaR?#YDb(QZEx9*M8l3mBS&3*8{X|6$AM}T
zMnV`i0
zSn*O>wEdv8`KeLGgi&*Z-V{-EK&f=jRyypmN^ehTUza&26qzsi`d=9{Z7y;~7|fgX
ztvB9qZiXczBHMzktJhDr}pIZ%AB?vjSk^~$xeOR>fIfeF|Te8v_Unr~7+#7)vW~|q1*Bk7~WfX6>d$-ho*XWT`6C~W)
z?FY)7iF-b)>h-Q>@V)n*>|G*J616BkiCA!ql$Gp{g8j?JW=r2ElN3w%{>O#ezZyy8
zHS#O*8UhjJUyPK9S4pnnL6V^fhW7ru)2;;@41N1W?N_Mw9?Ecwy8hJNSq7GR=ukYYNx#pvhSF_J_ma(L_XE8$X3NHq{GNGfb?j$Debe#!bzFG8XKp#HhLtI8A<=`
zh$BQ1t9oCl`k76Fx3hy~yJx{k8n=1zLnM+3LvN_}do=|Z4p^@fy
zM|R9!tew3iBVxwo2nvabW
zLFY)OV{w>HiIgrYI4qn{&u_#Z1Gp3bfO{u$BCcR47L6$RhS*TO(uqsBWi%10hqy6l#5RSXcbCad8FtdZ22S8R
zuKMmcKc(KsB_NiFt>LgP*ClVwJ7MEyotBMu|{T=9MP7%^AesHpd`eeHYjlw}a7o$~9_
zo1=hmUYDTW2RheHhN*wwTer8jdDbxL_<796Zt<>=^~AKu+mD7nE(iLRoxo2>ZRtE}
zHI`!4P4Zsp9M3fl3^T*Sl+Y0QVEB00bX&+0M+mDJ8m=XsjyrJa46{vyIb^Y}2cIhr
zb~B#geFI*;bMzw=fr3kvc_FrIV_EBI!(N)YHWvHUm^n;u3e_KZZqngJ!LesYs0$;X
zRovs4TB!>81-na#xkyrCm|N^EPzJjz$hoMD%Z}dx^@QbRY?-*8RK1K{;kCrKgmZ1r
zpCq})M^2Lnnl!=6P%s>LZc4BQ=1;?xYdCXdSmpiex{5O`!#eL@)m6Ow@-PxoNqX*_
z8nHrhiC_!QHkqgiX+I@Zt-zkcXKP~p3DZ-=&(mWnbNmbV$Y-^pQu-X;eoT5IvPK%F
zSfyErH>NKPX`Hi4RUt`xg;p=1uaK+wC1hG9b&FqBVo*}9XzUhI&ug4{g;Ygjt4SoR
zh?MQzwykeb<^I__-~9}R08J*|epZ^&(il>sY&b@%7Qi~11Qs`YK{+#A$Tij-hpVy)
z)$=ZdSU&Y$qud^)mkul58;(^i0dSrR5s{_CGs7n|5%uI1o{Io}A|Nm>r}`2$>1Naw
zUOj1jXV|G?J1*aGxmn{chWGQw@oU7g4-S{)BL2;Q(hn;_N;j6q*MuTbXMWJeeM0Q!
zr`z`qJ7(!eU;;>DOZd)m3r{x=XXZcE(8-^{H@g-uL;P3T>YCT5JlS6zBr)JJZ
z)9C%h`-y#G%@Q1<-mb2uFf^n
zZ@emYT>3@k=|0!jEc3!}6ZxuG)r{BTk&B0+J*l(fw-*n$#SJ%!Yw+vRhB|rXBcG-T
zHd#^q#5tTu;5vubD$pDza##haNc23T9=3drUyHn;;N)Enp2tKCBZ(^z%S?tIRSkI1
zSWpk$8s9u?_@c&{T68r!wDk}6dcG~S2sp1R3#Q(vsV8lcB2^YfuautyBcwRtOBTf9
z2>@QtPge|=NPA}tqv=-J@QK`xgzr2vil~61HSC%aDsvl^xxh$YA!{UuDet8Pe6xaALLLqX_G0)fYldwI6awJ^j&(0#jpPYnV8QOC
zCIJ#JBlnTm0uv93_u(L3H<6Y)Jo6-w5DL0TxTauDVsko_Tda8Inh~cIg9`@nYm^s4
z5l_}f@1^bU!gW`pyd$KK(QB3`3GYbUF@0&j)K=^8v>SH|fQvWrj?Az8a=nE6tWBS&#T%3@$
zPMfJ4J=e+t-hjKvTs80@xw#u!)+m8dSfp5u@~boOGF98NE{ZFOx$vey)1DJ^PYRB&
zBxOIBo<{elHS{5L5kH}*ja)(;w*|}NHO}S$bMusg)+>0>M9Wq->Wq8flY930QgxJl
zE_u8-SRMBx5cEskW&jx)D8AR~1*s1Cp*@;z$!8_=GCvus!7H*GMRli6fHq&7QVGwx
zC;W36q4XoFqY^`7qUflAW-JfbB|sE@Q0nrq=Jd&RpTNEWCwLF`WaL1Ubn?-88B)K-^b5*s#(IA1
zUugRo+R-xNyu~`_MytL99Dz`t`Ik{NtuebM5(yjX>|R!LnNj_W8_V)0UeDv!#^ECy
z)4cSES@x(_dU+8QPC3K%$|*K;_#Vd;D{ZD5#df7#qHJxUFCc@GK7Wcxcm><@sFt8Y
z^gE_lBQ=jW=n%ba
Re: [PATCH V4] drm/dp_mst: Retrieve extended DPCD caps for topology manager
Thanks a lot.
On Wed, Sep 23, 2020, 10:18 PM Lyude Paul wrote:
> On Wed, 2020-09-23 at 10:16 +0800, Koba Ko wrote:
> > Thanks for the review.
> > Sorry for that I thought the review tag should be appended by myself.
> > One thing to confirm with you, will you or I push this patch to drm-misc-
> > next ?
>
> I already pushed it with the change, so everything is all set
>
> > Thanks a lot.
> >
> > On Wed, Sep 23, 2020 at 2:01 AM Lyude Paul wrote:
> > > One last change I realized we should do is print the name of the AUX
> > > adapter
> > > in question. I don't mind just adding that myself before I push it
> though
> > > so
> > > you don't need to send a respin.
> > >
> > > Going to go push this to drm-misc-next, thanks!
> > >
> > > On Tue, 2020-09-22 at 14:53 +0800, Koba Ko wrote:
> > > > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1,read the
> DP_DP13_DPCD_REV
> > > > to
> > > > get the faster capability.
> > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0,read DP_DPCD_REV.
> > > >
> > > > Signed-off-by: Koba Ko
> > > > Reviewed-by: Lyude Paul
> > > > ---
> > > > drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > index e87542533640..63f8809b9aa4 100644
> > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > @@ -3686,9 +3686,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > > > drm_dp_mst_topology_mgr *mgr, bool ms
> > > > WARN_ON(mgr->mst_primary);
> > > >
> > > > /* get dpcd info */
> > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV,
> mgr->dpcd,
> > > > DP_RECEIVER_CAP_SIZE);
> > > > - if (ret != DP_RECEIVER_CAP_SIZE) {
> > > > - DRM_DEBUG_KMS("failed to read DPCD\n");
> > > > + ret = drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd);
> > > > + if (ret < 0) {
> > > > + drm_dbg_kms(mgr->dev, "failed to read DPCD, ret
> > > > %d\n",
> > > > ret);
> > > > goto out_unlock;
> > > > }
> > > >
> > > --
> > > Cheers,
> > > Lyude Paul (she/her)
> > > Software Engineer at Red Hat
> > >
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 11/23] device-dax: Kill dax_kmem_res
On 23.09.20 23:41, Dan Williams wrote:
> On Wed, Sep 23, 2020 at 1:04 AM David Hildenbrand wrote:
>>
>> On 08.09.20 17:33, Joao Martins wrote:
>>> [Sorry for the late response]
>>>
>>> On 8/21/20 11:06 AM, David Hildenbrand wrote:
On 03.08.20 07:03, Dan Williams wrote:
> @@ -37,109 +45,94 @@ int dev_dax_kmem_probe(struct device *dev)
> * could be mixed in a node with faster memory, causing
> * unavoidable performance issues.
> */
> - numa_node = dev_dax->target_node;
> if (numa_node < 0) {
> dev_warn(dev, "rejecting DAX region with invalid node: %d\n",
> numa_node);
> return -EINVAL;
> }
>
> - /* Hotplug starting at the beginning of the next block: */
> - kmem_start = ALIGN(range->start, memory_block_size_bytes());
> -
> - kmem_size = range_len(range);
> - /* Adjust the size down to compensate for moving up kmem_start: */
> - kmem_size -= kmem_start - range->start;
> - /* Align the size down to cover only complete blocks: */
> - kmem_size &= ~(memory_block_size_bytes() - 1);
> - kmem_end = kmem_start + kmem_size;
> -
> - new_res_name = kstrdup(dev_name(dev), GFP_KERNEL);
> - if (!new_res_name)
> + res_name = kstrdup(dev_name(dev), GFP_KERNEL);
> + if (!res_name)
> return -ENOMEM;
>
> - /* Region is permanently reserved if hotremove fails. */
> - new_res = request_mem_region(kmem_start, kmem_size, new_res_name);
> - if (!new_res) {
> - dev_warn(dev, "could not reserve region [%pa-%pa]\n",
> -&kmem_start, &kmem_end);
> - kfree(new_res_name);
> + res = request_mem_region(range.start, range_len(&range), res_name);
I think our range could be empty after aligning. I assume
request_mem_region() would check that, but maybe we could report a
better error/warning in that case.
>>> dax_kmem_range() already returns a memory-block-aligned @range but
>>> IIUC request_mem_region() isn't checking for that. Having said that
>>> the returned @res wouldn't be different from the passed range.start.
>>>
> /*
> * Ensure that future kexec'd kernels will not treat this as RAM
> * automatically.
> */
> - rc = add_memory_driver_managed(numa_node, new_res->start,
> - resource_size(new_res), kmem_name);
> + rc = add_memory_driver_managed(numa_node, res->start,
> + resource_size(res), kmem_name);
> +
> + res->flags |= IORESOURCE_BUSY;
Hm, I don't think that's correct. Any specific reason why to mark the
not-added, unaligned parts BUSY? E.g., walk_system_ram_range() could
suddenly stumble over it - and e.g., similarly kexec code when trying to
find memory for placing kexec images. I think we should leave this
!BUSY, just as it is right now.
>>> Agreed.
>>>
> if (rc) {
> - release_resource(new_res);
> - kfree(new_res);
> - kfree(new_res_name);
> + release_mem_region(range.start, range_len(&range));
> + kfree(res_name);
> return rc;
> }
> - dev_dax->dax_kmem_res = new_res;
> +
> + dev_set_drvdata(dev, res_name);
>
> return 0;
> }
>
> #ifdef CONFIG_MEMORY_HOTREMOVE
> -static int dev_dax_kmem_remove(struct device *dev)
> +static void dax_kmem_release(struct dev_dax *dev_dax)
> {
> - struct dev_dax *dev_dax = to_dev_dax(dev);
> - struct resource *res = dev_dax->dax_kmem_res;
> - resource_size_t kmem_start = res->start;
> - resource_size_t kmem_size = resource_size(res);
> - const char *res_name = res->name;
> int rc;
> + struct device *dev = &dev_dax->dev;
> + const char *res_name = dev_get_drvdata(dev);
> + struct range range = dax_kmem_range(dev_dax);
>
> /*
> * We have one shot for removing memory, if some memory blocks were
> not
> * offline prior to calling this function remove_memory() will fail,
> and
> * there is no way to hotremove this memory until reboot because
> device
> -* unbind will succeed even if we return failure.
> +* unbind will proceed regardless of the remove_memory result.
> */
> - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size);
> - if (rc) {
> - any_hotremove_failed = true;
> - dev_err(dev,
> - "DAX region %pR cannot be hotremoved until the next
> reboot\n",
> - res);
> - return rc;
> + rc = remove_memory(dev_dax->target_node, range.start,
> range_len(&range));
> + if (rc == 0) {
if (!rc) ?
>>> B
Re: [PATCH] drm/atomic: document and enforce rules around "spurious" EBUSY
On Wed, 23 Sep 2020 22:01:25 +0200
Daniel Vetter wrote:
> On Wed, Sep 23, 2020 at 9:17 PM Marius Vlad wrote:
> >
> > On Wed, Sep 23, 2020 at 05:18:52PM +0200, Daniel Vetter wrote:
> > > When doing an atomic modeset with ALLOW_MODESET drivers are allowed to
> > > pull in arbitrary other resources, including CRTCs (e.g. when
> > > reconfiguring global resources).
...
> > > @@ -1313,6 +1322,26 @@ int drm_atomic_check_only(struct drm_atomic_state
> > > *state)
> > > }
> > > }
> > >
> > > + for_each_new_crtc_in_state(state, crtc, old_crtc_state, i)
> > > + affected_crtc |= drm_crtc_mask(crtc);
> > > +
> > > + /*
> > > + * For commits that allow modesets drivers can add other CRTCs to
> > > the
> > > + * atomic commit, e.g. when they need to reallocate global
> > > resources.
> > > + * This can cause spurious EBUSY, which robs compositors of a very
> > > + * effective sanity check for their drawing loop. Therefor only
> > > allow
> > > + * drivers to add unrelated CRTC states for modeset commits.
> > > + *
> > > + * FIXME: Should add affected_crtc mask to the ATOMIC IOCTL as an
> > > output
> > > + * so compositors know what's going on.
> > > + */
> > > + if (affected_crtc != requested_crtc) {
> > > + DRM_DEBUG_ATOMIC("driver added CRTC to commit: requested
> > > 0x%x, affected 0x%0x\n",
> > > + requested_crtc, affected_crtc);
> > > + WARN(!state->allow_modeset, "adding CRTC not allowed
> > > without modesets: requested 0x%x, affected 0x%0x\n",
> > > + requested_crtc, affected_crtc);
> > Previous patch had the warn on state->allow_modeset now is
> > !state->allow_modeset. Is that correct?
>
> We need to fire a warning when allow_modeset is _not_ set. An earlier
> version got that wrong, and yes that would have caused a _ton_ of
> warnings on any fairly new intel platform.
>
> > I haven't followed the entire thread on this matter, but I guess the idea
> > is that somehow the kernel would pass to userspace a CRTC mask of
> > affected_crtc (somehow, we don't know how atm) and with it, userspace
> > can then issue a new commit (this commit blocking) with those?
>
> Either that, or just use that to track all the in-flight drm events.
> Userspace will get events for all the crtc, not just the one it asked
> to update.
Wait, does that happen already? Getting CRTC events for CRTCs userspace
didn't include in the atomic commit?
That could explain why Weston freaks out in
https://gitlab.freedesktop.org/wayland/weston/-/issues/435
Thanks,
pq
> If that's easier to do by re-issuing the commit with the
> full set of crtc, then I guess that's an option. But not required (I
> think at least, would need to test that to make sure).
> -Daniel
>
> > > + }
> > > +
> > > return 0;
> > > }
> > > EXPORT_SYMBOL(drm_atomic_check_only);
pgpexnHLXE7hN.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic: document and enforce rules around "spurious" EBUSY
On Thu, Sep 24, 2020 at 9:41 AM Pekka Paalanen wrote:
>
> On Wed, 23 Sep 2020 22:01:25 +0200
> Daniel Vetter wrote:
>
> > On Wed, Sep 23, 2020 at 9:17 PM Marius Vlad
> > wrote:
> > >
> > > On Wed, Sep 23, 2020 at 05:18:52PM +0200, Daniel Vetter wrote:
> > > > When doing an atomic modeset with ALLOW_MODESET drivers are allowed to
> > > > pull in arbitrary other resources, including CRTCs (e.g. when
> > > > reconfiguring global resources).
>
> ...
>
> > > > @@ -1313,6 +1322,26 @@ int drm_atomic_check_only(struct
> > > > drm_atomic_state *state)
> > > > }
> > > > }
> > > >
> > > > + for_each_new_crtc_in_state(state, crtc, old_crtc_state, i)
> > > > + affected_crtc |= drm_crtc_mask(crtc);
> > > > +
> > > > + /*
> > > > + * For commits that allow modesets drivers can add other CRTCs to
> > > > the
> > > > + * atomic commit, e.g. when they need to reallocate global
> > > > resources.
> > > > + * This can cause spurious EBUSY, which robs compositors of a very
> > > > + * effective sanity check for their drawing loop. Therefor only
> > > > allow
> > > > + * drivers to add unrelated CRTC states for modeset commits.
> > > > + *
> > > > + * FIXME: Should add affected_crtc mask to the ATOMIC IOCTL as an
> > > > output
> > > > + * so compositors know what's going on.
> > > > + */
> > > > + if (affected_crtc != requested_crtc) {
> > > > + DRM_DEBUG_ATOMIC("driver added CRTC to commit: requested
> > > > 0x%x, affected 0x%0x\n",
> > > > + requested_crtc, affected_crtc);
> > > > + WARN(!state->allow_modeset, "adding CRTC not allowed
> > > > without modesets: requested 0x%x, affected 0x%0x\n",
> > > > + requested_crtc, affected_crtc);
> > > Previous patch had the warn on state->allow_modeset now is
> > > !state->allow_modeset. Is that correct?
> >
> > We need to fire a warning when allow_modeset is _not_ set. An earlier
> > version got that wrong, and yes that would have caused a _ton_ of
> > warnings on any fairly new intel platform.
> >
> > > I haven't followed the entire thread on this matter, but I guess the idea
> > > is that somehow the kernel would pass to userspace a CRTC mask of
> > > affected_crtc (somehow, we don't know how atm) and with it, userspace
> > > can then issue a new commit (this commit blocking) with those?
> >
> > Either that, or just use that to track all the in-flight drm events.
> > Userspace will get events for all the crtc, not just the one it asked
> > to update.
>
> Wait, does that happen already? Getting CRTC events for CRTCs userspace
> didn't include in the atomic commit?
Yeah I'm pretty sure. With the affected_crtc mask you could update
your internal book-keeping to catch these, which should also prevent
all the spurious EBUSY. But I'm not entirely sure, I just read the
code, haven't tested.
> That could explain why Weston freaks out in
> https://gitlab.freedesktop.org/wayland/weston/-/issues/435
Hm it's strange that you first get an EBUSY, and only on the next
modeset get the spurious event. You should get one already on the
first modeset.
-Daniel
>
>
> Thanks,
> pq
>
>
> > If that's easier to do by re-issuing the commit with the
> > full set of crtc, then I guess that's an option. But not required (I
> > think at least, would need to test that to make sure).
> > -Daniel
> >
> > > > + }
> > > > +
> > > > return 0;
> > > > }
> > > > EXPORT_SYMBOL(drm_atomic_check_only);
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active
Hi Maxime,
On 9/18/20 11:59 PM, Maxime Ripard wrote:
> The HVS has three FIFOs that can be assigned to a number of PixelValves
> through a mux.
>
> However, changing that FIFO requires that we disable and then enable the
> pixelvalve, so we want to assign FIFOs to all the enabled CRTCs, and not
> just the active ones.
Thanks for the patch.
There is a problem when doing page flip.
After connecting 2 HDMIs without hotplug and booting.(Same thing when
using hotplug.)
When executing page flip for each of HDMI 0 and 1 in modetest
operation does not work normally. (crtc irq does not occur, so timeout
occurs.)
Sometimes both hdmi 0 or 1 work or not.
LOGs:
root:~> modetest -Mvc4 -s 32:1280x720 -v
setting mode 1280x720-60Hz@XR24 on connectors 32, crtc 64
failed to set gamma: Invalid argument
freq: 60.24Hz
freq: 60.00Hz
root:~> modetest -Mvc4 -s 38:1280x720 -v
setting mode 1280x720-60Hz@XR24 on connectors 38, crtc 69
failed to set gamma: Invalid argument
select timed out or error (ret 0)
select timed out or error (ret 0)
Could you please check it?
Best regards,
Hoegeun
>
> Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index af3ee3dcdab6..01fa60844695 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -643,7 +643,7 @@ vc4_atomic_check(struct drm_device *dev, struct
> drm_atomic_state *state)
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> unsigned int matching_channels;
>
> - if (!crtc_state->active)
> + if (!crtc_state->enable)
> continue;
>
> /*
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH rdma-next v3 1/2] lib/scatterlist: Add support in dynamic allocation of SG table from pages
On 22/09/2020 09:39, Leon Romanovsky wrote:
From: Maor Gottlieb
Extend __sg_alloc_table_from_pages to support dynamic allocation of
SG table from pages. It should be used by drivers that can't supply
all the pages at one time.
This function returns the last populated SGE in the table. Users should
pass it as an argument to the function from the second call and forward.
As before, nents will be equal to the number of populated SGEs (chunks).
So it's appending and growing the "list", did I get that right? Sounds
handy indeed. Some comments/questions below.
With this new extension, drivers can benefit the optimization of merging
contiguous pages without a need to allocate all pages in advance and
hold them in a large buffer.
E.g. with the Infiniband driver that allocates a single page for hold
the
pages. For 1TB memory registration, the temporary buffer would consume
only
4KB, instead of 2GB.
Signed-off-by: Maor Gottlieb
Signed-off-by: Leon Romanovsky
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 12 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 15 +-
include/linux/scatterlist.h | 43 +++---
lib/scatterlist.c | 158 +++-
lib/sg_pool.c | 3 +-
tools/testing/scatterlist/main.c| 9 +-
6 files changed, 163 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 12b30075134a..f2eaed6aca3d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -403,6 +403,7 @@ __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object
*obj,
unsigned int max_segment = i915_sg_segment_size();
struct sg_table *st;
unsigned int sg_page_sizes;
+ struct scatterlist *sg;
int ret;
st = kmalloc(sizeof(*st), GFP_KERNEL);
@@ -410,13 +411,12 @@ __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object
*obj,
return ERR_PTR(-ENOMEM);
alloc_table:
- ret = __sg_alloc_table_from_pages(st, pvec, num_pages,
- 0, num_pages << PAGE_SHIFT,
- max_segment,
- GFP_KERNEL);
- if (ret) {
+ sg = __sg_alloc_table_from_pages(st, pvec, num_pages, 0,
+num_pages << PAGE_SHIFT, max_segment,
+NULL, 0, GFP_KERNEL);
+ if (IS_ERR(sg)) {
kfree(st);
- return ERR_PTR(ret);
+ return ERR_CAST(sg);
}
ret = i915_gem_gtt_prepare_pages(obj, st);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index ab524ab3b0b4..f22acd398b1f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -419,6 +419,7 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
int ret = 0;
static size_t sgl_size;
static size_t sgt_size;
+ struct scatterlist *sg;
if (vmw_tt->mapped)
return 0;
@@ -441,13 +442,15 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
if (unlikely(ret != 0))
return ret;
- ret = __sg_alloc_table_from_pages
- (&vmw_tt->sgt, vsgt->pages, vsgt->num_pages, 0,
-(unsigned long) vsgt->num_pages << PAGE_SHIFT,
-dma_get_max_seg_size(dev_priv->dev->dev),
-GFP_KERNEL);
- if (unlikely(ret != 0))
+ sg = __sg_alloc_table_from_pages(&vmw_tt->sgt, vsgt->pages,
+ vsgt->num_pages, 0,
+ (unsigned long) vsgt->num_pages << PAGE_SHIFT,
+ dma_get_max_seg_size(dev_priv->dev->dev),
+ NULL, 0, GFP_KERNEL);
+ if (IS_ERR(sg)) {
+ ret = PTR_ERR(sg);
goto out_sg_alloc_fail;
+ }
if (vsgt->num_pages > vmw_tt->sgt.nents) {
uint64_t over_alloc =
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 45cf7b69d852..c24cc667b56b 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -165,6 +165,22 @@ static inline void sg_set_buf(struct scatterlist *sg,
const void *buf,
#define for_each_sgtable_dma_sg(sgt, sg, i) \
for_each_sg((sgt)->sgl, sg, (sgt)->nents, i)
+static inline void __sg_chain(struct scatterlist *chain_sg,
+ struct scatterlist *sgl)
+{
+ /*
+* offset and length are unused for chain entry. Clear them.
+*/
+ chain_sg->offset = 0;
+ chain_sg->length = 0;
+
+ /*
+* Set lowest bit to indi
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Wed, Sep 23, 2020 at 11:52:51AM -0400, Steven Rostedt wrote: > On Wed, 23 Sep 2020 10:40:32 +0200 > pet...@infradead.org wrote: > > > However, with migrate_disable() we can have each task preempted in a > > migrate_disable() region, worse we can stack them all on the _same_ CPU > > (super ridiculous odds, sure). And then we end up only able to run one > > task, with the rest of the CPUs picking their nose. > > What if we just made migrate_disable() a local_lock() available for !RT? Can't, neiter migrate_disable() nor migrate_enable() are allowed to block -- which is what makes their implementation so 'interesting'. > This should lower the SHC in theory, if you can't have stacked migrate > disables on the same CPU. See this email in that other thread (you're on Cc too IIRC): https://lkml.kernel.org/r/20200923170809.gy1362...@hirez.programming.kicks-ass.net I think that is we 'frob' the balance PULL, we'll end up with something similar. Whichever way around we turn this thing, the migrate_disable() runtime (we'll have to add a tracer for that), will be an interference term on the lower priority task, exactly like preempt_disable() would be. We'll just not exclude a higher priority task from running. AFAICT; the best case is a single migrate_disable() nesting, where a higher priority task preempts in a migrate_disable() section -- this is per design. When this preempted task becomes elegible to run under the ideal model (IOW it becomes one of the M highest priority tasks), it might still have to wait for the preemptee's migrate_disable() section to complete. Thereby suffering interference in the exact duration of migrate_disable() section. Per this argument, the change from preempt_disable() to migrate_disable() gets us: - higher priority tasks gain reduced wake-up latency - lower priority tasks are unchanged and are subject to the exact same interference term as if the higher priority task were using preempt_disable(). Since we've already established this term is unbounded, any task but the highest priority task is basically buggered. TL;DR, if we get balancing fixed and achieve (near) the optimal case above, migrate_disable() is an over-all win. But it's provably non-deterministic as long as the migrate_disable() sections are non-deterministic. The reason this all mostly works in practise is (I think) because: - People care most about the higher prio RT tasks and craft them to mostly avoid the migrate_disable() infected code. - The preemption scenario is most pronounced at higher utilization scenarios, and I suspect this is fairly rare to begin with. - And while many of these migrate_disable() regions are unbound in theory, in practise they're often fairly reasonable. So my current todo list is: - Change RT PULL - Change DL PULL - Add migrate_disable() tracer; exactly like preempt/irqoff, except measuring task-runtime instead of cpu-time. - Add a mode that measures actual interference. - Add a traceevent to detect preemption in migrate_disable(). And then I suppose I should twist Daniel's arm to update his model to include these scenarios and numbers. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/bridge: tc358764: restore connector support
This patch restores DRM connector registration in the TC358764 bridge
driver and restores usage of the old drm_panel_* API, thus allows dynamic
panel registration. This fixes panel operation on Exynos5250-based
Arndale board.
This is equivalent to the revert of the following commits:
1644127f83bc "drm/bridge: tc358764: add drm_panel_bridge support"
385ca38da29c "drm/bridge: tc358764: drop drm_connector_(un)register"
and removal of the calls to drm_panel_attach()/drm_panel_detach(), which
were no-ops and has been removed in meanwhile.
Signed-off-by: Marek Szyprowski
---
As I've reported and Andrzej Hajda pointed, the reverted patches break
operation of the panel on the Arndale board. Noone suggested how to fix
the regression, I've decided to send a revert until a new solution is
found.
The issues with tc358764 might be automatically resolved once the Exynos
DSI itself is converted to DRM bridge:
https://patchwork.kernel.org/cover/11770683/
but that approach has also its own issues so far.
Best regards,
Marek Szyprowski
---
drivers/gpu/drm/bridge/tc358764.c | 107 +-
1 file changed, 92 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358764.c
b/drivers/gpu/drm/bridge/tc358764.c
index d89394bc5aa4..c1e35bdf9232 100644
--- a/drivers/gpu/drm/bridge/tc358764.c
+++ b/drivers/gpu/drm/bridge/tc358764.c
@@ -153,9 +153,10 @@ static const char * const tc358764_supplies[] = {
struct tc358764 {
struct device *dev;
struct drm_bridge bridge;
+ struct drm_connector connector;
struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
struct gpio_desc *gpio_reset;
- struct drm_bridge *panel_bridge;
+ struct drm_panel *panel;
int error;
};
@@ -209,6 +210,12 @@ static inline struct tc358764 *bridge_to_tc358764(struct
drm_bridge *bridge)
return container_of(bridge, struct tc358764, bridge);
}
+static inline
+struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
+{
+ return container_of(connector, struct tc358764, connector);
+}
+
static int tc358764_init(struct tc358764 *ctx)
{
u32 v = 0;
@@ -271,11 +278,43 @@ static void tc358764_reset(struct tc358764 *ctx)
usleep_range(1000, 2000);
}
+static int tc358764_get_modes(struct drm_connector *connector)
+{
+ struct tc358764 *ctx = connector_to_tc358764(connector);
+
+ return drm_panel_get_modes(ctx->panel, connector);
+}
+
+static const
+struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
+ .get_modes = tc358764_get_modes,
+};
+
+static const struct drm_connector_funcs tc358764_connector_funcs = {
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .reset = drm_atomic_helper_connector_reset,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static void tc358764_disable(struct drm_bridge *bridge)
+{
+ struct tc358764 *ctx = bridge_to_tc358764(bridge);
+ int ret = drm_panel_disable(bridge_to_tc358764(bridge)->panel);
+
+ if (ret < 0)
+ dev_err(ctx->dev, "error disabling panel (%d)\n", ret);
+}
+
static void tc358764_post_disable(struct drm_bridge *bridge)
{
struct tc358764 *ctx = bridge_to_tc358764(bridge);
int ret;
+ ret = drm_panel_unprepare(ctx->panel);
+ if (ret < 0)
+ dev_err(ctx->dev, "error unpreparing panel (%d)\n", ret);
tc358764_reset(ctx);
usleep_range(1, 15000);
ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
@@ -296,28 +335,71 @@ static void tc358764_pre_enable(struct drm_bridge *bridge)
ret = tc358764_init(ctx);
if (ret < 0)
dev_err(ctx->dev, "error initializing bridge (%d)\n", ret);
+ ret = drm_panel_prepare(ctx->panel);
+ if (ret < 0)
+ dev_err(ctx->dev, "error preparing panel (%d)\n", ret);
+}
+
+static void tc358764_enable(struct drm_bridge *bridge)
+{
+ struct tc358764 *ctx = bridge_to_tc358764(bridge);
+ int ret = drm_panel_enable(ctx->panel);
+
+ if (ret < 0)
+ dev_err(ctx->dev, "error enabling panel (%d)\n", ret);
}
static int tc358764_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)
+{
+ struct tc358764 *ctx = bridge_to_tc358764(bridge);
+ struct drm_device *drm = bridge->dev;
+ int ret;
+
+ if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
+ DRM_ERROR("Fix bridge driver to make connector optional!");
+ return -EINVAL;
+ }
+
+ ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
+ ret = drm_connector_init(drm, &ctx->connector,
+&tc358764_connector_funcs,
+DRM_MODE_CONNECTOR_LVDS);
+
Re: [PATCH 1/2] drm/dp: add subheadings to DPCD address definitions
On Fri, 18 Sep 2020, "Navare, Manasi" wrote: > On Fri, Sep 18, 2020 at 02:40:16PM +0300, Jani Nikula wrote: >> Add the subheadings from the DP spec. No functional changes. >> >> Signed-off-by: Jani Nikula > > Looks good to me > > Reviewed-by: Manasi Navare Thanks, pushed both to drm-misc-next. BR, Jani. > > Manasi > >> --- >> include/drm/drm_dp_helper.h | 22 ++ >> 1 file changed, 18 insertions(+), 4 deletions(-) >> >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> index c9f2851904d0..388083b4716b 100644 >> --- a/include/drm/drm_dp_helper.h >> +++ b/include/drm/drm_dp_helper.h >> @@ -106,8 +106,9 @@ struct drm_device; >> #define DP_AUX_I2C_REPLY_DEFER (0x2 << 2) >> #define DP_AUX_I2C_REPLY_MASK (0x3 << 2) >> >> -/* AUX CH addresses */ >> -/* DPCD */ >> +/* DPCD Field Address Mapping */ >> + >> +/* Receiver Capability */ >> #define DP_DPCD_REV 0x000 >> # define DP_DPCD_REV_10 0x10 >> # define DP_DPCD_REV_11 0x11 >> @@ -426,7 +427,7 @@ struct drm_device; >> #define DP_DSC_BRANCH_OVERALL_THROUGHPUT_1 0x0a1 >> #define DP_DSC_BRANCH_MAX_LINE_WIDTH0x0a2 >> >> -/* link configuration */ >> +/* Link Configuration */ >> #define DP_LINK_BW_SET 0x100 >> # define DP_LINK_RATE_TABLE 0x00/* eDP 1.4 */ >> # define DP_LINK_BW_1_620x06 >> @@ -580,6 +581,7 @@ struct drm_device; >> #define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1 >> #define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2 >> >> +/* Link/Sink Device Status */ >> #define DP_SINK_COUNT 0x200 >> /* prior to 1.2 bit 7 was reserved mbz */ >> # define DP_GET_SINK_COUNT(x) x) & 0x80) >> 1) | ((x) >> & 0x3f)) >> @@ -779,20 +781,27 @@ struct drm_device; >> #define DP_VC_PAYLOAD_ID_SLOT_1 0x2c1 /* 1.2 MST */ >> /* up to ID_SLOT_63 at 0x2ff */ >> >> +/* Source Device-specific */ >> #define DP_SOURCE_OUI 0x300 >> + >> +/* Sink Device-specific */ >> #define DP_SINK_OUI 0x400 >> + >> +/* Branch Device-specific */ >> #define DP_BRANCH_OUI 0x500 >> #define DP_BRANCH_ID0x503 >> #define DP_BRANCH_REVISION_START0x509 >> #define DP_BRANCH_HW_REV0x509 >> #define DP_BRANCH_SW_REV0x50A >> >> +/* Link/Sink Device Power Control */ >> #define DP_SET_POWER0x600 >> # define DP_SET_POWER_D00x1 >> # define DP_SET_POWER_D30x2 >> # define DP_SET_POWER_MASK 0x3 >> # define DP_SET_POWER_D3_AUX_ON 0x5 >> >> +/* eDP-specific */ >> #define DP_EDP_DPCD_REV 0x700/* eDP 1.2 */ >> # define DP_EDP_11 0x00 >> # define DP_EDP_12 0x01 >> @@ -876,11 +885,13 @@ struct drm_device; >> #define DP_EDP_REGIONAL_BACKLIGHT_BASE 0x740/* eDP 1.4 */ >> #define DP_EDP_REGIONAL_BACKLIGHT_0 0x741/* eDP 1.4 */ >> >> +/* Sideband MSG Buffers */ >> #define DP_SIDEBAND_MSG_DOWN_REQ_BASE 0x1000 /* 1.2 MST */ >> #define DP_SIDEBAND_MSG_UP_REP_BASE 0x1200 /* 1.2 MST */ >> #define DP_SIDEBAND_MSG_DOWN_REP_BASE 0x1400 /* 1.2 MST */ >> #define DP_SIDEBAND_MSG_UP_REQ_BASE 0x1600 /* 1.2 MST */ >> >> +/* DPRX Event Status Indicator */ >> #define DP_SINK_COUNT_ESI 0x2002 /* 1.2 */ >> /* 0-5 sink count */ >> # define DP_SINK_COUNT_CP_READY (1 << 6) >> @@ -934,6 +945,7 @@ struct drm_device; >> #define DP_LANE_ALIGN_STATUS_UPDATED_ESI 0x200e /* status same as >> 0x204 */ >> #define DP_SINK_STATUS_ESI 0x200f /* status same as >> 0x205 */ >> >> +/* Extended Receiver Capability */ >> #define DP_DP13_DPCD_REV0x2200 >> #define DP_DP13_MAX_LINK_RATE 0x2201 >> >> @@ -947,6 +959,7 @@ struct drm_device; >> # define DP_VSC_EXT_CEA_SDP_SUPPORTED (1 << 6) /* DP >> 1.4 */ >> # define DP_VSC_EXT_CEA_SDP_CHAINING_SUPPORTED (1 << 7) /* DP >> 1.4 */ >> >> +/* Protocol Converter Extension */ >> /* HDMI CEC tunneling over AUX DP 1.3 section 5.3.3.3.1 DPCD 1.4+ */ >> #define DP_CEC_TUNNELING_CAPABILITY0x3000 >> # define DP_CEC_TUNNELING_CAPABLE (1 << 0) >> @@ -1013,6 +1026,7 @@ struct drm_device; >> #define DP_PROTOCOL_CONVERTER_CONTROL_2 0x3052 /* DP 1.3 */ >> # define DP_CONVERSION_TO_YCBCR422_ENABLE (1 << 0) /* DP 1.3 */ >> >> +/* HDCP 1.3 and HDCP 2.2 */ >> #define DP_AUX_HDCP_BKSV0x68000 >> #define DP_AUX_HDCP_RI_PRIME0x68005 >> #define DP_AUX_HDCP_AKSV0x68007 >> @@ -1058,7 +1072,7 @@ struct drm_device; >> #define DP_HDCP_2_2_REG_STREAM_TYPE_OF
Re: [PATCH 0/3] drm: commit_work scheduling
On Wed, Sep 23, 2020 at 07:33:17PM -0700, Rob Clark wrote:
> On Wed, Sep 23, 2020 at 8:25 AM Daniel Vetter wrote:
> >
> > On Tue, Sep 22, 2020 at 07:48:10AM -0700, Rob Clark wrote:
> > > On Mon, Sep 21, 2020 at 11:59 PM Daniel Vetter wrote:
> > > >
> > > > On Mon, Sep 21, 2020 at 5:16 PM Rob Clark wrote:
> > > > >
> > > > > On Mon, Sep 21, 2020 at 2:21 AM Daniel Vetter wrote:
> > > > > >
> > > > > > On Sat, Sep 19, 2020 at 12:37:23PM -0700, Rob Clark wrote:
> > > > > > > From: Rob Clark
> > > > > > >
> > > > > > > The android userspace treats the display pipeline as a realtime
> > > > > > > problem.
> > > > > > > And arguably, if your goal is to not miss frame deadlines (ie.
> > > > > > > vblank),
> > > > > > > it is. (See https://lwn.net/Articles/809545/ for the best
> > > > > > > explaination
> > > > > > > that I found.)
> > > > > > >
> > > > > > > But this presents a problem with using workqueues for non-blocking
> > > > > > > atomic commit_work(), because the SCHED_FIFO userspace thread(s)
> > > > > > > can
> > > > > > > preempt the worker. Which is not really the outcome you want..
> > > > > > > once
> > > > > > > the required fences are scheduled, you want to push the atomic
> > > > > > > commit
> > > > > > > down to hw ASAP.
> > > > > > >
> > > > > > > But the decision of whether commit_work should be RT or not really
> > > > > > > depends on what userspace is doing. For a pure CFS userspace
> > > > > > > display
> > > > > > > pipeline, commit_work() should remain SCHED_NORMAL.
> > > > > > >
> > > > > > > To handle this, convert non-blocking commit_work() to use per-CRTC
> > > > > > > kthread workers, instead of system_unbound_wq. Per-CRTC workers
> > > > > > > are
> > > > > > > used to avoid serializing commits when userspace is using a
> > > > > > > per-CRTC
> > > > > > > update loop.
> > > > > > >
> > > > > > > A client-cap is introduced so that userspace can opt-in to
> > > > > > > SCHED_FIFO
> > > > > > > priority commit work.
> > > > > > >
> > > > > > > A potential issue is that since 616d91b68cd ("sched: Remove
> > > > > > > sched_setscheduler*() EXPORTs") we have limited RT priority
> > > > > > > levels,
> > > > > > > meaning that commit_work() ends up running at the same priority
> > > > > > > level
> > > > > > > as vblank-work. This shouldn't be a big problem *yet*, due to
> > > > > > > limited
> > > > > > > use of vblank-work at this point. And if it could be arranged
> > > > > > > that
> > > > > > > vblank-work is scheduled before signaling out-fences and/or
> > > > > > > sending
> > > > > > > pageflip events, it could probably work ok to use a single
> > > > > > > priority
> > > > > > > level for both commit-work and vblank-work.
> > > > > >
> > > > > > The part I don't like about this is that it all feels rather hacked
> > > > > > together, and if we add more stuff (or there's some different thing
> > > > > > in the
> > > > > > system that also needs rt scheduling) then it doesn't compose.
> > > > >
> > > > > The ideal thing would be that userspace is in control of the
> > > > > priorities.. the setclientcap approach seemed like a reasonable way to
> > > > > give the drm-master a way to opt in.
> > > > >
> > > > > I suppose instead userspace could use sched_setscheduler().. but that
> > > > > would require userspace to be root, and would require some way to find
> > > > > the tid.
> > > >
> > > > Userspace already needs that for the SCHED_FIFO for surface-flinger.
> > > > Or is the problem that CAP_SYS_NICE is only good for your own
> > > > processes?
> > >
> > > tbh, I'm not completely sure offhand what gives surfaceflinger
> > > permission to set itself SCHED_FIFO
> > >
> > > (But on CrOS there are a few more pieces to the puzzle)
> > >
> > > > Other question I have for this is whether there's any recommendations
> > > > for naming the kthreads (since I guess that name is what becomes the
> > > > uapi for userspace to control this)?
> > > >
> > > > Otherwise I think "userspace calls sched_setscheduler on the right
> > > > kthreads" sounds like a good interface, since it lets userspace decide
> > > > how it all needs to fit together and compose. Anything we hard-code in
> > > > an ioctl is kinda lost cause. And we can choose the default values to
> > > > work reasonably well when the compositor runs at normal priority
> > > > (lowest niceness or something like that for the commit work).
> > >
> > > I don't really like the naming convention approach.. what is to stop
> > > some unrelated process to name it's thread the same thing to get a
> > > SCHED_FIFO boost..
> > >
> > > But we can stick with my idea to expose the thread id as a read-only
> > > CRTC property, for userspace to find the things to call
> > > sched_setscheduler() on. If for whatever reason the drm master is not
> > > privileged (or is running in a sandbox, etc), a small helper that has
> > > the necessary permissions could open the drm device to find the CRTC
> > > thread-ids and call sched
Re: [PATCH 01/11] drm/ttm: add ttm_bo_pin()/ttm_bo_unpin() v2
On Wed, Sep 23, 2020 at 01:01:14PM +1000, Dave Airlie wrote: > On Tue, 22 Sep 2020 at 23:32, Christian König > wrote: > > > > As an alternative to the placement flag add a > > pin count to the ttm buffer object. > > These all look good to me, nice cleanup. > > For the series: > Reviewed-by: Dave Airlie Yeah I like, but plenty of review already so I wont bother. I do wonder whether we should/could lift this one more level to drm_gem_object, since cma/shmem gem helpers have this too. But they have hand-rolled locking for it of dubious quality, and don't use dma_resv_lock for any of this unfortunately. And I guess that would need to be fixed first. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-fixes
Hi Dave & Daniel - Just a couple of simple fixes. With Daniel's irc ack I backmerged Linus' tree at an arbitrary commit due to a build failure in v5.9-rc6 that blocked CI. drm-intel-fixes-2020-09-24: drm/i915 fixes for v5.9-rc7: - Fix selftest reference to stack data out of scope - Fix GVT null pointer dereference - Backmerge from Linus' master to fix build BR, Jani. The following changes since commit 98477740630f270aecf648f1d6a9dbc6027d4ff1: Merge branch 'rcu/urgent' of git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu (2020-09-21 12:42:31 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2020-09-24 for you to fetch changes up to 16cce04cdb200ba905d1241b425ac48da5a9ace5: drm/i915/selftests: Push the fake iommu device from the stack to data (2020-09-23 10:15:46 +0300) drm/i915 fixes for v5.9-rc7: - Fix selftest reference to stack data out of scope - Fix GVT null pointer dereference - Backmerge from Linus' master to fix build Chris Wilson (1): drm/i915/selftests: Push the fake iommu device from the stack to data Jani Nikula (2): Merge remote-tracking branch 'origin/master' into drm-intel-fixes Merge tag 'gvt-fixes-2020-09-17' of https://github.com/intel/gvt-linux into drm-intel-fixes Zhenyu Wang (1): drm/i915/gvt: Fix port number for BDW on EDID region setup drivers/gpu/drm/i915/gvt/vgpu.c | 6 +- drivers/gpu/drm/i915/selftests/mock_gem_device.c | 12 +--- 2 files changed, 10 insertions(+), 8 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209373] New: Pixels
https://bugzilla.kernel.org/show_bug.cgi?id=209373 Bug ID: 209373 Summary: Pixels Product: Drivers Version: 2.5 Kernel Version: 5.8.0-1-amd64, but also 5.7 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: rapp.j...@gmail.com Regression: No Created attachment 292613 --> https://bugzilla.kernel.org/attachment.cgi?id=292613&action=edit screenshot of pixelated screen I'm running debian bullseye on a thinkpad e595 with ryzen 3700u (VEGA 10 GPU) >From time to time I get weird pixels spread on screen. Sometimes they look like distorted parts of my displayed windows (like text console). Weird thing is, when I drag a window, the spreaded pixels on it also move. Pixels seem to "re-spread" when the window gets redrawn. Sometimes this effect vanishes but only for some hours or days. I also tried a live linux system (manjaro) which gave the same error. Grub boot screen looks fine, simple text console mode, too. So I think it's some hardware rendering issue.. Since i don't run Windows I could not see if this gets those errors, too. All I tried was running Windows installer up to the time where it wants to format my drive- There haven't been any graphics issues but I'm not shure if Windows installer does hardware rendering ( some people told me it does ) I didn't find much in my logs.. but here they are. $ sudo dmesg -x -l crit,err,warn kern :warn : [ 0.378239] TSC synchronization [CPU#0 -> CPU#1]: kern :warn : [ 0.378239] Measured 6970522673 cycles TSC warp between CPUs, turning off TSC clock. kern :warn : [ 0.378334] #2 #3 #4 #5 #6 #7 kern :warn : [ 1.196110] pci :00:00.2: can't derive routing for PCI INT A kern :warn : [ 1.196111] pci :00:00.2: PCI INT A: not connected kern :warn : [ 1.199508] PPR NX GT IA GA PC GA_vAPIC kern :warn : [ 1.501357] Unstable clock detected, switching default tracing clock to "global" If you want to keep using the local clock, then add: "trace_clock=local" on the kernel command line kern :warn : [ 1.637520] acpi PNP0C14:01: duplicate WMI GUID 05901221-D566-11D1-B2F0-00A0C9062910 (first instance was on PNP0C14:00) kern :warn : [ 1.637590] acpi PNP0C14:02: duplicate WMI GUID 05901221-D566-11D1-B2F0-00A0C9062910 (first instance was on PNP0C14:00) kern :warn : [ 1.637647] acpi PNP0C14:03: duplicate WMI GUID 05901221-D566-11D1-B2F0-00A0C9062910 (first instance was on PNP0C14:00) kern :warn : [ 1.652129] r8169 :02:00.0: can't disable ASPM; OS doesn't have ASPM control kern :err : [ 2.381817] irq 7: nobody cared (try booting with the "irqpoll" option) kern :warn : [ 2.381846] CPU: 3 PID: 0 Comm: swapper/3 Not tainted 5.7.0-3-amd64 #1 Debian 5.7.17-1 kern :warn : [ 2.381846] Hardware name: LENOVO 20NFGE/20NFGE, BIOS R11ET32W (1.12 ) 12/23/2019 kern :warn : [ 2.381847] Call Trace: kern :warn : [ 2.381849] kern :warn : [ 2.381855] dump_stack+0x66/0x90 kern :warn : [ 2.381859] __report_bad_irq+0x38/0xad kern :warn : [ 2.381861] note_interrupt.cold+0xb/0x6e kern :warn : [ 2.381862] handle_irq_event_percpu+0x72/0x80 kern :warn : [ 2.381864] handle_irq_event+0x3c/0x5c kern :warn : [ 2.381865] handle_fasteoi_irq+0xa3/0x160 kern :warn : [ 2.381868] do_IRQ+0x53/0xe0 kern :warn : [ 2.381870] common_interrupt+0xf/0xf kern :warn : [ 2.381871] kern :warn : [ 2.381874] RIP: 0010:tick_nohz_idle_enter+0x43/0x50 kern :warn : [ 2.381875] Code: e5 4e 73 48 83 bb b0 00 00 00 00 75 1f 80 4b 4c 01 e8 c1 17 ff ff 80 4b 4c 04 48 89 43 78 e8 14 86 f9 ff fb 66 0f 1f 44 00 00 <5b> c3 0f 0b eb dd 0f 1f 80 00 00 00 00 0f 1f 44 00 00 53 48 c7 c3 kern :warn : [ 2.381876] RSP: 0018:c2fb8015fec8 EFLAGS: 0246 ORIG_RAX: ffde kern :warn : [ 2.381878] RAX: 8df61552 RBX: 9f18f0ae06c0 RCX: 8df5f8fc kern :warn : [ 2.381879] RDX: 8e020d58 RSI: 8df5f8fc RDI: fff3eba4 kern :warn : [ 2.381879] RBP: 008e R08: 8df61552 R09: kern :warn : [ 2.381880] R10: R11: 000c R12: 9f1647f8db80 kern :warn : [ 2.381881] R13: R14: R15: kern :warn : [ 2.381883] ? tick_nohz_idle_enter+0x3c/0x50 kern :warn : [ 2.381885] do_idle+0x3f/0x280 kern :warn : [ 2.381887] cpu_startup_entry+0x19/0x20 kern :warn : [ 2.381890] start_secondary+0x169/0x1c0 kern :warn : [ 2.381892] secondary_startup_64+0xa4/0xb0 kern :err : [ 2.381893] handlers: kern :err : [ 2.381905] [<595fde45>] amd_gpio_irq_handler daemon:warn : [ 2.782479] systemd[1]: /lib/systemd/system/plymouth-start.service:16: Unit configured to use KillMode=none. This is unsafe, as it disables systemd's process lifecycle management for the service. Please update your service to use a safer KillMode=, such as 'mi
Re: [PATCH 30/45] drm/ttm: add a new invalidate notify callback.
On Thu, Sep 24, 2020 at 03:18:30PM +1000, Dave Airlie wrote:
> From: Dave Airlie
>
> Signed-off-by: Dave Airlie
A bikeshed, but why not just call this ->invalidate? ->move_notify needed
the _notify because we already had the ->move callback, but there's not
invalidate that also needs a invalidate_notify. And a callback kinda
implies that the driver gets notified about some stuff, but doesn't really
add anything about what it should do now. Plus if we go with notify, I
guess it should be called ->delete_notify, and use that to do it's
invalidation.
Otherwise I think this entire series is a solid demidlayer of all the move
stuff here.
Cheers, Daniel
> ---
> drivers/gpu/drm/ttm/ttm_bo.c| 4 +++-
> include/drm/ttm/ttm_bo_driver.h | 7 +++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index a2a61a8d1394..ba69c682e53b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -282,7 +282,9 @@ static int ttm_bo_handle_move_mem(struct
> ttm_buffer_object *bo,
>
> static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
> {
> - if (bo->bdev->driver->move_notify)
> + if (bo->bdev->driver->invalidate_notify)
> + bo->bdev->driver->invalidate_notify(bo);
> + else if (bo->bdev->driver->move_notify)
> bo->bdev->driver->move_notify(bo, false, NULL);
>
> ttm_bo_tt_destroy(bo);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index cfb151dbb2d0..da4afe669664 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -165,6 +165,13 @@ struct ttm_bo_driver {
> void (*move_notify)(struct ttm_buffer_object *bo,
> bool evict,
> struct ttm_resource *new_mem);
> +
> + /**
> + * Hook to notify driver about a bo being torn down.
> + * can be used for invalidation instead of move_notify.
> + */
> + void (*invalidate_notify)(struct ttm_buffer_object *bo);
> +
> /* notify the driver we are taking a fault on this BO
>* and have reserved it */
> int (*fault_reserve_notify)(struct ttm_buffer_object *bo);
> --
> 2.27.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 09/11] drm/amdgpu: switch over to the new pin interface
Tested-by: Nirmoy Das
On 9/22/20 3:32 PM, Christian König wrote:
Stop using TTM_PL_FLAG_NO_EVICT.
Signed-off-by: Christian König
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 5 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 +---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 5 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 41 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.h| 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 2 +-
9 files changed, 24 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index b6b821500d30..64d4b5ff95d6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1479,7 +1479,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
}
}
- if (!amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) && !bo->pin_count)
+ if (!amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) && !bo->tbo.pin_count)
amdgpu_bo_fence(bo,
&avm->process_info->eviction_fence->base,
true);
@@ -1558,7 +1558,8 @@ int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
* required.
*/
if (mem->mapped_to_gpu_memory == 0 &&
- !amdgpu_ttm_tt_get_usermm(mem->bo->tbo.ttm) && !mem->bo->pin_count)
+ !amdgpu_ttm_tt_get_usermm(mem->bo->tbo.ttm) &&
+ !mem->bo->tbo.pin_count)
amdgpu_amdkfd_remove_eviction_fence(mem->bo,
process_info->eviction_fence);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 12598a4b5c78..d50b63a93d37 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -410,7 +410,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
uint32_t domain;
int r;
- if (bo->pin_count)
+ if (bo->tbo.pin_count)
return 0;
/* Don't move this buffer if we have depleted our allowance
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index c81206e6096f..4cba095b6c44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -132,10 +132,7 @@ static void amdgpu_display_unpin_work_func(struct
work_struct *__work)
/* unpin of the old buffer */
r = amdgpu_bo_reserve(work->old_abo, true);
if (likely(r == 0)) {
- r = amdgpu_bo_unpin(work->old_abo);
- if (unlikely(r != 0)) {
- DRM_ERROR("failed to unpin buffer after flip\n");
- }
+ amdgpu_bo_unpin(work->old_abo);
amdgpu_bo_unreserve(work->old_abo);
} else
DRM_ERROR("failed to reserve buffer after flip\n");
@@ -249,8 +246,7 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc
*crtc,
}
unpin:
if (!adev->enable_virtual_display)
- if (unlikely(amdgpu_bo_unpin(new_abo) != 0))
- DRM_ERROR("failed to unpin new abo in error path\n");
+ amdgpu_bo_unpin(new_abo);
unreserve:
amdgpu_bo_unreserve(new_abo);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 957934926b24..5b465ab774d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -281,7 +281,7 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
struct sg_table *sgt;
long r;
- if (!bo->pin_count) {
+ if (!bo->tbo.pin_count) {
/* move buffer into GTT or VRAM */
struct ttm_operation_ctx ctx = { false, false };
unsigned domains = AMDGPU_GEM_DOMAIN_GTT;
@@ -390,7 +390,8 @@ static int amdgpu_dma_buf_begin_cpu_access(struct dma_buf
*dma_buf,
if (unlikely(ret != 0))
return ret;
- if (!bo->pin_count && (bo->allowed_domains & AMDGPU_GEM_DOMAIN_GTT)) {
+ if (!bo->tbo.pin_count &&
+ (bo->allowed_domains & AMDGPU_GEM_DOMAIN_GTT)) {
amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index aa7f230c71bf..59b52804622d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -860,7 +860,7 @@ static int amdgpu_debugfs_gem_bo_info(int id, void *ptr,
void *data)
seq_printf(m, "\t0x%08x: %12ld byte
Re: [PATCH 07/10] drm/amdgpu/ttm: handle tt moves properly.
Am 24.09.20 um 02:46 schrieb Dave Airlie:
On Thu, 24 Sep 2020 at 00:46, Christian König wrote:
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie
The core move code currently handles use_tt moves, for amdgpu
this was being handled also in the driver, but not using the same
paths.
If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use
the core move function.
Eventually the core will be flipped over to calling the driver.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index db5f761f37ec..d3bd2fd448be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
bool evict,
ttm_bo_move_null(bo, new_mem);
return 0;
}
- if ((old_mem->mem_type == TTM_PL_TT &&
- new_mem->mem_type == TTM_PL_SYSTEM) ||
- (old_mem->mem_type == TTM_PL_SYSTEM &&
- new_mem->mem_type == TTM_PL_TT)) {
- /* bind is enough */
+ if (old_mem->mem_type == TTM_PL_SYSTEM &&
+ new_mem->mem_type == TTM_PL_TT) {
ttm_bo_move_null(bo, new_mem);
I would feel better if we nuke ttm_bo_move_null() and always use
ttm_bo_move_ttm().
Any reason? The code path in the current move code pretty much
is this.
The current move paths are
if (new_tt && old_tt)
if old is system
move null
else
move ttm
else
call driver move.
So I wanted to maintain that order. calling the move ttm will just
make a pointless caching, populate, bind step.
Well we want to get rid of the bind/unbind stuff in TTM.
And I'm seriously thinking about getting rid of all the caching stuff.
So all of this should just go away rather soon.
Christian.
But I'm going to think about it a bit more.
Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 0/6] drm/nouveau: Support sync FDs and sync objects
On Wed, Sep 23, 2020 at 05:21:24PM +0200, Daniel Vetter wrote: > On Wed, Sep 23, 2020 at 11:18:53AM +0200, Thierry Reding wrote: > > On Fri, Aug 28, 2020 at 12:40:10PM +0200, Thierry Reding wrote: > > > From: Thierry Reding > > > > > > Hi, > > > > > > This series implements a new IOCTL to submit push buffers that can > > > optionally return a sync FD or sync object to userspace. This is useful > > > in cases where userspace wants to synchronize operations between the GPU > > > and another driver (such as KMS for display). Among other things this > > > allows extensions such as eglDupNativeFenceFDANDROID to be implemented. > > > > > > Note that patch 4 modifies the ABI introduced in patch 3 by allowing DRM > > > sync objects to be passed rather than only sync FDs. It also allows any > > > number of sync FDs/objects to be passed in or emitted. I think those are > > > useful features, but I left them in a separate patch in case everybody > > > else thinks that this won't be needed. If we decide to merge the new ABI > > > then patch 4 should be squashed into patch 3. > > > > > > The corresponding userspace changes can be found here: > > > > > > libdrm: > > > https://gitlab.freedesktop.org/tagr/drm/-/commits/nouveau-sync-fd-v2/ > > > mesa: > > > https://gitlab.freedesktop.org/tagr/mesa/-/commits/nouveau-sync-fd/ > > > > > > I've verified that this works with kmscube's --atomic mode and Weston. > > > > Hi Ben, > > > > any thoughts on this series? I realize that this is somewhat suboptimal > > because we're effectively adding a duplicate of the existing IOCTL with > > only the "minor" extension of adding sync FDs/objects, but at the same > > time I don't have any good ideas on what else to add to make this more > > appealing or if you have any plans of your own to address this in the > > future. > > drm core automatically zero-extends ioctl structs both ways, so if all you > do is add more stuff to the top level ioctl struct at the bottom, there's > no need to duplicate any code. At least as long as you guarantee that 0 == > old behaviour for both in and out parameters. But that only works if the structure size remains fixed, right? In this case, however, we have to extend the structure with additional fields, so the size is going to change and therefore the IOCTL number will also change. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic: document and enforce rules around "spurious" EBUSY
On Thu, 24 Sep 2020 10:04:12 +0200
Daniel Vetter wrote:
> On Thu, Sep 24, 2020 at 9:41 AM Pekka Paalanen wrote:
> >
> > On Wed, 23 Sep 2020 22:01:25 +0200
> > Daniel Vetter wrote:
> >
> > > On Wed, Sep 23, 2020 at 9:17 PM Marius Vlad
> > > wrote:
> > > >
> > > > On Wed, Sep 23, 2020 at 05:18:52PM +0200, Daniel Vetter wrote:
> > > > > When doing an atomic modeset with ALLOW_MODESET drivers are allowed to
> > > > > pull in arbitrary other resources, including CRTCs (e.g. when
> > > > > reconfiguring global resources).
> >
> > ...
> >
> > > > > @@ -1313,6 +1322,26 @@ int drm_atomic_check_only(struct
> > > > > drm_atomic_state *state)
> > > > > }
> > > > > }
> > > > >
> > > > > + for_each_new_crtc_in_state(state, crtc, old_crtc_state, i)
> > > > > + affected_crtc |= drm_crtc_mask(crtc);
> > > > > +
> > > > > + /*
> > > > > + * For commits that allow modesets drivers can add other CRTCs
> > > > > to the
> > > > > + * atomic commit, e.g. when they need to reallocate global
> > > > > resources.
> > > > > + * This can cause spurious EBUSY, which robs compositors of a
> > > > > very
> > > > > + * effective sanity check for their drawing loop. Therefor only
> > > > > allow
> > > > > + * drivers to add unrelated CRTC states for modeset commits.
> > > > > + *
> > > > > + * FIXME: Should add affected_crtc mask to the ATOMIC IOCTL as
> > > > > an output
> > > > > + * so compositors know what's going on.
> > > > > + */
> > > > > + if (affected_crtc != requested_crtc) {
> > > > > + DRM_DEBUG_ATOMIC("driver added CRTC to commit:
> > > > > requested 0x%x, affected 0x%0x\n",
> > > > > + requested_crtc, affected_crtc);
> > > > > + WARN(!state->allow_modeset, "adding CRTC not allowed
> > > > > without modesets: requested 0x%x, affected 0x%0x\n",
> > > > > + requested_crtc, affected_crtc);
> > > > Previous patch had the warn on state->allow_modeset now is
> > > > !state->allow_modeset. Is that correct?
> > >
> > > We need to fire a warning when allow_modeset is _not_ set. An earlier
> > > version got that wrong, and yes that would have caused a _ton_ of
> > > warnings on any fairly new intel platform.
> > >
> > > > I haven't followed the entire thread on this matter, but I guess the
> > > > idea
> > > > is that somehow the kernel would pass to userspace a CRTC mask of
> > > > affected_crtc (somehow, we don't know how atm) and with it, userspace
> > > > can then issue a new commit (this commit blocking) with those?
> > >
> > > Either that, or just use that to track all the in-flight drm events.
> > > Userspace will get events for all the crtc, not just the one it asked
> > > to update.
> >
> > Wait, does that happen already? Getting CRTC events for CRTCs userspace
> > didn't include in the atomic commit?
>
> Yeah I'm pretty sure. With the affected_crtc mask you could update
> your internal book-keeping to catch these, which should also prevent
> all the spurious EBUSY. But I'm not entirely sure, I just read the
> code, haven't tested.
If that actually happens, how does userspace know whether the
userdata argument with the event is valid or not?
The kernel should expect the userdata argument to be one-shot, because
it may be a pointer to a malloc()'d struct that gets freed the moment
the originally expected event is handled, so re-using userdata is going
to break userspace (ISTR Mutter uses this style with legacy, Weston
passes somewhat more persistent pointers with both legacy and atomic).
Does the kernel reset it to zero? What if userspace doesn't use a
pointer but e.g. an index where 0 may be a legal value but also wrong?
Thanks,
pq
> > That could explain why Weston freaks out in
> > https://gitlab.freedesktop.org/wayland/weston/-/issues/435
>
> Hm it's strange that you first get an EBUSY, and only on the next
> modeset get the spurious event. You should get one already on the
> first modeset.
> -Daniel
>
> >
> >
> > Thanks,
> > pq
> >
> >
> > > If that's easier to do by re-issuing the commit with the
> > > full set of crtc, then I guess that's an option. But not required (I
> > > think at least, would need to test that to make sure).
> > > -Daniel
> > >
> > > > > + }
> > > > > +
> > > > > return 0;
> > > > > }
> > > > > EXPORT_SYMBOL(drm_atomic_check_only);
>
>
>
pgpVgkgnfcOuj.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209373] Pixels
https://bugzilla.kernel.org/show_bug.cgi?id=209373 --- Comment #1 from Michel Dänzer (mic...@daenzer.net) --- (In reply to Jens R. from comment #0) > kern :warn : [ 2.381846] Hardware name: LENOVO 20NFGE/20NFGE, BIOS > R11ET32W (1.12 ) 12/23/2019 I'm writing this on the exact same machine, but I've never seen such issues. I recommend updating the BIOS to the current version, that resolved https://gitlab.freedesktop.org/drm/amd/-/issues/1146 for me (BIOS version 1.16 currently). If the problem persists with that, a hardware issue does seem plausible. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/45] drm/qxl: drop unused code
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_ttm.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 2c35ca4270c6..5738be300078 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -100,17 +100,12 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
*/
struct qxl_ttm_tt {
struct ttm_tt ttm;
- struct qxl_device *qdev;
- u64 offset;
};
Any reason we keep the qxl_ttm_tt structure around when it only contains
the ttm_tt field?
Christian.
static int qxl_ttm_backend_bind(struct ttm_bo_device *bdev,
struct ttm_tt *ttm,
struct ttm_resource *bo_mem)
{
- struct qxl_ttm_tt *gtt = (void *)ttm;
-
- gtt->offset = (unsigned long)(bo_mem->start << PAGE_SHIFT);
if (!ttm->num_pages) {
WARN(1, "nothing to bind %lu pages for mreg %p back %p!\n",
ttm->num_pages, bo_mem, ttm);
@@ -138,14 +133,11 @@ static void qxl_ttm_backend_destroy(struct ttm_bo_device
*bdev,
static struct ttm_tt *qxl_ttm_tt_create(struct ttm_buffer_object *bo,
uint32_t page_flags)
{
- struct qxl_device *qdev;
struct qxl_ttm_tt *gtt;
- qdev = qxl_get_qdev(bo->bdev);
gtt = kzalloc(sizeof(struct qxl_ttm_tt), GFP_KERNEL);
if (gtt == NULL)
return NULL;
- gtt->qdev = qdev;
if (ttm_tt_init(>t->ttm, bo, page_flags)) {
kfree(gtt);
return NULL;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/45] drm/ttm: handle the SYSTEM->TT path in same place as others.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
This just consolidates the code making the flow easier to understand
and also helps when moving move to the driver side.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/ttm/ttm_bo.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c342bfc2b4c1..6d1520255fc1 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -265,20 +265,18 @@ static int ttm_bo_handle_move_mem(struct
ttm_buffer_object *bo,
if (ret)
goto out_err;
}
-
- if (bo->mem.mem_type == TTM_PL_SYSTEM) {
- if (bdev->driver->move_notify)
- bdev->driver->move_notify(bo, evict, mem);
- bo->mem = *mem;
- goto moved;
- }
}
if (bdev->driver->move_notify)
bdev->driver->move_notify(bo, evict, mem);
- if (old_man->use_tt && new_man->use_tt)
- ret = ttm_bo_move_ttm(bo, ctx, mem);
+ if (old_man->use_tt && new_man->use_tt) {
+ if (bo->mem.mem_type == TTM_PL_SYSTEM) {
+ ttm_bo_assign_mem(bo, mem);
+ ret = 0;
+ } else
+ ret = ttm_bo_move_ttm(bo, ctx, mem);
+ }
else if (bdev->driver->move)
This should then use "} else if (...) {", apart from that the patch is
Reviewed-by: Christian König .
Christian.
ret = bdev->driver->move(bo, evict, ctx, mem);
else
@@ -294,7 +292,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
goto out_err;
}
-moved:
ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
return 0;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ast: Reload gamma LUT after changing primary plane's color format
Hi
Am 23.09.20 um 17:30 schrieb Daniel Vetter:
> On Tue, Sep 22, 2020 at 04:46:55PM +0200, Thomas Zimmermann wrote:
>> The gamma LUT has to be reloaded after changing the primary plane's
>> color format. This used to be done implicitly by the CRTC atomic_enable()
>> helper after updating the primary plane. With the recent reordering of
>> the steps, the primary plane's setup was moved last and invalidated
>> the gamma LUT. Fix this by setting the LUT from within atomic_flush().
>>
>> Signed-off-by: Thomas Zimmermann
>> Fixes: 2f0ddd89fe32 ("drm/ast: Enable CRTC before planes")
>> Cc: Thomas Zimmermann
>> Cc: Daniel Vetter
>> Cc: Dave Airlie
>> Cc: dri-devel@lists.freedesktop.org
>
> Does what it says in the commit message, and makes sense.
>
> Maybe add a comment to the load_lut function or where it's called stating
> that this must be done after every plane color change, seems like an
> important piece of information to carry around in the code itself and not
> just in the commit message.
I'll do that before committing the patch.
>
> Either way: Reviewed-by: Daniel Vetter
Thanks
Best regards
Thomas
>
>> ---
>> drivers/gpu/drm/ast/ast_mode.c | 13 -
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index 834a156e3a75..ba3bf76e104d 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -742,7 +742,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int
>> mode)
>> case DRM_MODE_DPMS_SUSPEND:
>> if (ast->tx_chip_type == AST_TX_DP501)
>> ast_set_dp501_video_output(crtc->dev, 1);
>> -ast_crtc_load_lut(ast, crtc);
>> break;
>> case DRM_MODE_DPMS_OFF:
>> if (ast->tx_chip_type == AST_TX_DP501)
>> @@ -777,6 +776,17 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc
>> *crtc,
>> return 0;
>> }
>>
>> +static void
>> +ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state
>> *old_crtc_state)
>> +{
>> +struct ast_private *ast = to_ast_private(crtc->dev);
>> +struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc->state);
>> +struct ast_crtc_state *old_ast_crtc_state =
>> to_ast_crtc_state(old_crtc_state);
>> +
>> +if (old_ast_crtc_state->format != ast_crtc_state->format)
>> +ast_crtc_load_lut(ast, crtc);
>> +}
>> +
>> static void
>> ast_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>>struct drm_crtc_state *old_crtc_state)
>> @@ -830,6 +840,7 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
>>
>> static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>> .atomic_check = ast_crtc_helper_atomic_check,
>> +.atomic_flush = ast_crtc_helper_atomic_flush,
>> .atomic_enable = ast_crtc_helper_atomic_enable,
>> .atomic_disable = ast_crtc_helper_atomic_disable,
>> };
>> --
>> 2.28.0
>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: bridge: cdns-mhdp8546: fix compile warning
On 23/09/2020 11:30, Tomi Valkeinen wrote:
> On x64 we get:
>
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:751:10: warning:
> conversion from 'long unsigned int' to 'unsigned int' changes value from
> '18446744073709551613' to '4294967293' [-Woverflow]
>
> The registers are 32 bit, so fix by casting to u32.
>
> Fixes: fb43aa0acdfd ("drm: bridge: Add support for Cadence MHDP8546 DPI/DP
> bridge")
> Signed-off-by: Tomi Valkeinen
I forgot to add
Reported-by: Stephen Rothwell
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209373] Pixels
https://bugzilla.kernel.org/show_bug.cgi?id=209373 --- Comment #2 from Jens R. (rapp.j...@gmail.com) --- I followed this description https://www.cyberciti.biz/faq/update-lenovo-bios-from-linux-usb-stick-pen/ for making a bootable stick and it didn't work on three different sticks.. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-fixes
drm-misc-fixes-2020-09-24: drm-misc-fixes for v5.9: - Single null pointer deref fix for dma-buf. The following changes since commit 74ea06164cda81dc80e97790164ca533fd7e3087: drm/sun4i: mixer: Extend regmap max_register (2020-09-10 13:08:48 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2020-09-24 for you to fetch changes up to 19a508bd1ad8e444de86873bf2f2b2ab8edd6552: dmabuf: fix NULL pointer dereference in dma_buf_release() (2020-09-21 11:17:06 +0200) drm-misc-fixes for v5.9: - Single null pointer deref fix for dma-buf. Charan Teja Reddy (1): dmabuf: fix NULL pointer dereference in dma_buf_release() drivers/dma-buf/dma-buf.c | 2 ++ 1 file changed, 2 insertions(+) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic: document and enforce rules around "spurious" EBUSY
On Thu, Sep 24, 2020 at 01:10:56PM +0300, Pekka Paalanen wrote:
> On Thu, 24 Sep 2020 10:04:12 +0200
> Daniel Vetter wrote:
>
> > On Thu, Sep 24, 2020 at 9:41 AM Pekka Paalanen wrote:
> > >
> > > On Wed, 23 Sep 2020 22:01:25 +0200
> > > Daniel Vetter wrote:
> > >
> > > > On Wed, Sep 23, 2020 at 9:17 PM Marius Vlad
> > > > wrote:
> > > > >
> > > > > On Wed, Sep 23, 2020 at 05:18:52PM +0200, Daniel Vetter wrote:
> > > > > > When doing an atomic modeset with ALLOW_MODESET drivers are allowed
> > > > > > to
> > > > > > pull in arbitrary other resources, including CRTCs (e.g. when
> > > > > > reconfiguring global resources).
> > >
> > > ...
> > >
> > > > > > @@ -1313,6 +1322,26 @@ int drm_atomic_check_only(struct
> > > > > > drm_atomic_state *state)
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > + for_each_new_crtc_in_state(state, crtc, old_crtc_state, i)
> > > > > > + affected_crtc |= drm_crtc_mask(crtc);
> > > > > > +
> > > > > > + /*
> > > > > > + * For commits that allow modesets drivers can add other
> > > > > > CRTCs to the
> > > > > > + * atomic commit, e.g. when they need to reallocate global
> > > > > > resources.
> > > > > > + * This can cause spurious EBUSY, which robs compositors of a
> > > > > > very
> > > > > > + * effective sanity check for their drawing loop. Therefor
> > > > > > only allow
> > > > > > + * drivers to add unrelated CRTC states for modeset commits.
> > > > > > + *
> > > > > > + * FIXME: Should add affected_crtc mask to the ATOMIC IOCTL
> > > > > > as an output
> > > > > > + * so compositors know what's going on.
> > > > > > + */
> > > > > > + if (affected_crtc != requested_crtc) {
> > > > > > + DRM_DEBUG_ATOMIC("driver added CRTC to commit:
> > > > > > requested 0x%x, affected 0x%0x\n",
> > > > > > + requested_crtc, affected_crtc);
> > > > > > + WARN(!state->allow_modeset, "adding CRTC not allowed
> > > > > > without modesets: requested 0x%x, affected 0x%0x\n",
> > > > > > + requested_crtc, affected_crtc);
> > > > > Previous patch had the warn on state->allow_modeset now is
> > > > > !state->allow_modeset. Is that correct?
> > > >
> > > > We need to fire a warning when allow_modeset is _not_ set. An earlier
> > > > version got that wrong, and yes that would have caused a _ton_ of
> > > > warnings on any fairly new intel platform.
> > > >
> > > > > I haven't followed the entire thread on this matter, but I guess the
> > > > > idea
> > > > > is that somehow the kernel would pass to userspace a CRTC mask of
> > > > > affected_crtc (somehow, we don't know how atm) and with it, userspace
> > > > > can then issue a new commit (this commit blocking) with those?
> > > >
> > > > Either that, or just use that to track all the in-flight drm events.
> > > > Userspace will get events for all the crtc, not just the one it asked
> > > > to update.
> > >
> > > Wait, does that happen already? Getting CRTC events for CRTCs userspace
> > > didn't include in the atomic commit?
> >
> > Yeah I'm pretty sure. With the affected_crtc mask you could update
> > your internal book-keeping to catch these, which should also prevent
> > all the spurious EBUSY. But I'm not entirely sure, I just read the
> > code, haven't tested.
>
> If that actually happens, how does userspace know whether the
> userdata argument with the event is valid or not?
At some point I was worried about the kernel potentially sending spurious
events, but IIRC I managed to convince myself that it shouldn't happen.
I think I came to the conclusion the events were populated before the
core calls into the driver. But maybe I misanalyzed it, or something
has since broken?
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209373] Pixels
https://bugzilla.kernel.org/show_bug.cgi?id=209373 --- Comment #3 from Michel Dänzer (mic...@daenzer.net) --- Yeah, the BIOS seems very picky about which partitions it boots from. With GParted or a similar tool, check that the partition which contains the EFI directory has at least the boot & esp flags set. (I only realized this might be enough after manually extracting the BIOS image to a USB HD, and updating from that) -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/45] drm/amdgpu/ttm: handle tt moves properly.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
The core move code currently handles use_tt moves, for amdgpu
this was being handled also in the driver, but not using the same
paths.
If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use
the core move function.
Eventually the core will be flipped over to calling the driver.
Signed-off-by: Dave Airlie
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index db5f761f37ec..d3bd2fd448be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
bool evict,
ttm_bo_move_null(bo, new_mem);
return 0;
}
- if ((old_mem->mem_type == TTM_PL_TT &&
-new_mem->mem_type == TTM_PL_SYSTEM) ||
- (old_mem->mem_type == TTM_PL_SYSTEM &&
-new_mem->mem_type == TTM_PL_TT)) {
- /* bind is enough */
+ if (old_mem->mem_type == TTM_PL_SYSTEM &&
+ new_mem->mem_type == TTM_PL_TT) {
ttm_bo_move_null(bo, new_mem);
return 0;
}
+
+ if (old_mem->mem_type == TTM_PL_TT &&
+ new_mem->mem_type == TTM_PL_SYSTEM)
+ return ttm_bo_move_ttm(bo, ctx, new_mem);
+
if (old_mem->mem_type == AMDGPU_PL_GDS ||
old_mem->mem_type == AMDGPU_PL_GWS ||
old_mem->mem_type == AMDGPU_PL_OA ||
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/45] drm/radeon/ttm: handle ttm moves properly
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
The core move code currently handles use_tt moves, for radeon
this was being handled also in the driver, but not using the same
paths.
If moving between TT/SYSTEM (all the use_tt paths on radeon) use
the core move function.
Eventually the core will be flipped over to calling the driver.
Signed-off-by: Dave Airlie
Reviewed-by: Christian König
---
drivers/gpu/drm/radeon/radeon_ttm.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index ea9ffa6198da..df5cedb2b632 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -316,14 +316,16 @@ static int radeon_bo_move(struct ttm_buffer_object *bo,
bool evict,
ttm_bo_move_null(bo, new_mem);
return 0;
}
- if ((old_mem->mem_type == TTM_PL_TT &&
-new_mem->mem_type == TTM_PL_SYSTEM) ||
- (old_mem->mem_type == TTM_PL_SYSTEM &&
-new_mem->mem_type == TTM_PL_TT)) {
- /* bind is enough */
+ if (old_mem->mem_type == TTM_PL_SYSTEM &&
+ new_mem->mem_type == TTM_PL_TT) {
ttm_bo_move_null(bo, new_mem);
return 0;
}
+
+ if (old_mem->mem_type == TTM_PL_TT &&
+ new_mem->mem_type == TTM_PL_SYSTEM)
+ return ttm_bo_move_ttm(bo, ctx, new_mem);
+
if (!rdev->ring[radeon_copy_ring_index(rdev)].ready ||
rdev->asic->copy.copy == NULL) {
/* use memcpy */
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/45] drm/nouveau/ttm: handle ttm moves properly.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
The idea is to flip the core over to calling the driver always,
so add support for moves here.
Signed-off-by: Dave Airlie
Acked-by: Christian König
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 8d51cfca07c8..2c10a84b2cc0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1057,6 +1057,18 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
goto out;
}
+ if (old_reg->mem_type == TTM_PL_SYSTEM &&
+ new_reg->mem_type == TTM_PL_TT) {
+ ttm_bo_move_null(bo, new_reg);
+ goto out;
+ }
+
+ if (old_reg->mem_type == TTM_PL_TT &&
+ new_reg->mem_type == TTM_PL_SYSTEM) {
+ ret = ttm_bo_move_ttm(bo, ctx, new_reg);
+ goto out;
+ }
+
/* Hardware assisted copy. */
if (drm->ttm.move) {
if (new_reg->mem_type == TTM_PL_SYSTEM)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/45] drm/vmwgfx: move null mem checks outside move notifies
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
Both fns checked mem == NULL, just move the check outside.
Signed-off-by: Dave Airlie
Acked-by: Christian König
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 ---
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 ++
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index a1f675c5f471..b09f4f064ae4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -1191,9 +1191,6 @@ void vmw_bo_move_notify(struct ttm_buffer_object *bo,
{
struct vmw_buffer_object *vbo;
- if (mem == NULL)
- return;
-
/* Make sure @bo is embedded in a struct vmw_buffer_object? */
if (bo->destroy != vmw_bo_bo_free &&
bo->destroy != vmw_user_bo_destroy)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 5e922d9d5f2c..00b535831a7a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -867,7 +867,7 @@ void vmw_query_move_notify(struct ttm_buffer_object *bo,
mutex_lock(&dev_priv->binding_mutex);
dx_query_mob = container_of(bo, struct vmw_buffer_object, base);
- if (mem == NULL || !dx_query_mob || !dx_query_mob->dx_query_ctx) {
+ if (!dx_query_mob || !dx_query_mob->dx_query_ctx) {
mutex_unlock(&dev_priv->binding_mutex);
return;
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index fc68f54df46a..2f88d2d79f9a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -707,6 +707,8 @@ static void vmw_move_notify(struct ttm_buffer_object *bo,
bool evict,
struct ttm_resource *mem)
{
+ if (!mem)
+ return;
vmw_bo_move_notify(bo, mem);
vmw_query_move_notify(bo, mem);
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic: document and enforce rules around "spurious" EBUSY
On Thu, Sep 24, 2020 at 1:01 PM Ville Syrjälä
wrote:
>
> On Thu, Sep 24, 2020 at 01:10:56PM +0300, Pekka Paalanen wrote:
> > On Thu, 24 Sep 2020 10:04:12 +0200
> > Daniel Vetter wrote:
> >
> > > On Thu, Sep 24, 2020 at 9:41 AM Pekka Paalanen
> > > wrote:
> > > >
> > > > On Wed, 23 Sep 2020 22:01:25 +0200
> > > > Daniel Vetter wrote:
> > > >
> > > > > On Wed, Sep 23, 2020 at 9:17 PM Marius Vlad
> > > > > wrote:
> > > > > >
> > > > > > On Wed, Sep 23, 2020 at 05:18:52PM +0200, Daniel Vetter wrote:
> > > > > > > When doing an atomic modeset with ALLOW_MODESET drivers are
> > > > > > > allowed to
> > > > > > > pull in arbitrary other resources, including CRTCs (e.g. when
> > > > > > > reconfiguring global resources).
> > > >
> > > > ...
> > > >
> > > > > > > @@ -1313,6 +1322,26 @@ int drm_atomic_check_only(struct
> > > > > > > drm_atomic_state *state)
> > > > > > > }
> > > > > > > }
> > > > > > >
> > > > > > > + for_each_new_crtc_in_state(state, crtc, old_crtc_state, i)
> > > > > > > + affected_crtc |= drm_crtc_mask(crtc);
> > > > > > > +
> > > > > > > + /*
> > > > > > > + * For commits that allow modesets drivers can add other
> > > > > > > CRTCs to the
> > > > > > > + * atomic commit, e.g. when they need to reallocate global
> > > > > > > resources.
> > > > > > > + * This can cause spurious EBUSY, which robs compositors of
> > > > > > > a very
> > > > > > > + * effective sanity check for their drawing loop. Therefor
> > > > > > > only allow
> > > > > > > + * drivers to add unrelated CRTC states for modeset commits.
> > > > > > > + *
> > > > > > > + * FIXME: Should add affected_crtc mask to the ATOMIC IOCTL
> > > > > > > as an output
> > > > > > > + * so compositors know what's going on.
> > > > > > > + */
> > > > > > > + if (affected_crtc != requested_crtc) {
> > > > > > > + DRM_DEBUG_ATOMIC("driver added CRTC to commit:
> > > > > > > requested 0x%x, affected 0x%0x\n",
> > > > > > > + requested_crtc, affected_crtc);
> > > > > > > + WARN(!state->allow_modeset, "adding CRTC not
> > > > > > > allowed without modesets: requested 0x%x, affected 0x%0x\n",
> > > > > > > + requested_crtc, affected_crtc);
> > > > > > Previous patch had the warn on state->allow_modeset now is
> > > > > > !state->allow_modeset. Is that correct?
> > > > >
> > > > > We need to fire a warning when allow_modeset is _not_ set. An earlier
> > > > > version got that wrong, and yes that would have caused a _ton_ of
> > > > > warnings on any fairly new intel platform.
> > > > >
> > > > > > I haven't followed the entire thread on this matter, but I guess
> > > > > > the idea
> > > > > > is that somehow the kernel would pass to userspace a CRTC mask of
> > > > > > affected_crtc (somehow, we don't know how atm) and with it,
> > > > > > userspace
> > > > > > can then issue a new commit (this commit blocking) with those?
> > > > >
> > > > > Either that, or just use that to track all the in-flight drm events.
> > > > > Userspace will get events for all the crtc, not just the one it asked
> > > > > to update.
> > > >
> > > > Wait, does that happen already? Getting CRTC events for CRTCs userspace
> > > > didn't include in the atomic commit?
> > >
> > > Yeah I'm pretty sure. With the affected_crtc mask you could update
> > > your internal book-keeping to catch these, which should also prevent
> > > all the spurious EBUSY. But I'm not entirely sure, I just read the
> > > code, haven't tested.
> >
> > If that actually happens, how does userspace know whether the
> > userdata argument with the event is valid or not?
>
> At some point I was worried about the kernel potentially sending spurious
> events, but IIRC I managed to convince myself that it shouldn't happen.
> I think I came to the conclusion the events were populated before the
> core calls into the driver. But maybe I misanalyzed it, or something
> has since broken?
Hm right this shouldn't happen, I misread the code. So if userspace
wants events for all affected crtc, it needs to add them explicitly to
the atomic ioctl (just set an arbitrary property on each crtc to its
current value or something like that). That also means that the bug
Pekka posted shouldn't have been caused by this stuff here.
Note for code readers: There's a retry loop for ww-mutex backoff, but
we do completely clear all states, so crtc states shouldn't be able to
persist and then get events when userspace didn't ask for them.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/45] drm/vmwgfx: add a move callback.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
This just copies the fallback to vmwgfx, I'm going to iterate on this
a bit until it's not the same as the fallback path.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 2f88d2d79f9a..6e36fc932aeb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -725,6 +725,23 @@ static void vmw_swap_notify(struct ttm_buffer_object *bo)
(void) ttm_bo_wait(bo, false, false);
}
+static int vmw_move(struct ttm_buffer_object *bo,
+bool evict,
+struct ttm_operation_ctx *ctx,
+struct ttm_resource *new_mem)
+{
+ struct ttm_resource_manager *old_man = ttm_manager_type(bo->bdev,
bo->mem.mem_type);
+ struct ttm_resource_manager *new_man = ttm_manager_type(bo->bdev,
new_mem->mem_type);
+
+ if (old_man->use_tt && new_man->use_tt) {
+ if (bo->mem.mem_type == TTM_PL_SYSTEM) {
+ ttm_bo_assign_mem(bo, new_mem);
+ return 0;
+ }
+ return ttm_bo_move_ttm(bo, ctx, new_mem);
+ } else
+ return ttm_bo_move_memcpy(bo, ctx, new_mem);
This should be "} else {", apart from that Reviewed-by: Christian König
+}
struct ttm_bo_driver vmw_bo_driver = {
.ttm_tt_create = &vmw_ttm_tt_create,
@@ -735,7 +752,7 @@ struct ttm_bo_driver vmw_bo_driver = {
.ttm_tt_destroy = &vmw_ttm_destroy,
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = vmw_evict_flags,
- .move = NULL,
+ .move = vmw_move,
.verify_access = vmw_verify_access,
.move_notify = vmw_move_notify,
.swap_notify = vmw_swap_notify,
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/45] drm/vram_helper: implement a ttm move callback.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
This will always do memcpy moves.
Signed-off-by: Dave Airlie
Acked-by: Christian König
---
drivers/gpu/drm/drm_gem_vram_helper.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 171ea57b0311..9fd80a3643f6 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -600,6 +600,14 @@ static void drm_gem_vram_bo_driver_move_notify(struct
drm_gem_vram_object *gbo,
kmap->virtual = NULL;
}
+static int drm_gem_vram_bo_driver_move(struct drm_gem_vram_object *gbo,
+ bool evict,
+ struct ttm_operation_ctx *ctx,
+ struct ttm_resource *new_mem)
+{
+ return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
+}
+
/*
* Helpers for struct drm_gem_object_funcs
*/
@@ -962,6 +970,18 @@ static void bo_driver_move_notify(struct ttm_buffer_object
*bo,
drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
}
+static int bo_driver_move(struct ttm_buffer_object *bo,
+ bool evict,
+ struct ttm_operation_ctx *ctx,
+ struct ttm_resource *new_mem)
+{
+ struct drm_gem_vram_object *gbo;
+
+ gbo = drm_gem_vram_of_bo(bo);
+
+ return drm_gem_vram_bo_driver_move(gbo, evict, ctx, new_mem);
+}
+
static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
struct ttm_resource *mem)
{
@@ -986,6 +1006,7 @@ static struct ttm_bo_driver bo_driver = {
.ttm_tt_destroy = bo_driver_ttm_tt_destroy,
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = bo_driver_evict_flags,
+ .move = bo_driver_move,
.move_notify = bo_driver_move_notify,
.io_mem_reserve = bo_driver_io_mem_reserve,
};
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 09/45] drm/ttm: make move callback compulstory
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
All drivers should have a move callback now so make it compulsory.
Signed-off-by: Dave Airlie
Reviewed-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 6d1520255fc1..6a7f4c028801 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -270,18 +270,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
if (bdev->driver->move_notify)
bdev->driver->move_notify(bo, evict, mem);
- if (old_man->use_tt && new_man->use_tt) {
- if (bo->mem.mem_type == TTM_PL_SYSTEM) {
- ttm_bo_assign_mem(bo, mem);
- ret = 0;
- } else
- ret = ttm_bo_move_ttm(bo, ctx, mem);
- }
- else if (bdev->driver->move)
- ret = bdev->driver->move(bo, evict, ctx, mem);
- else
- ret = ttm_bo_move_memcpy(bo, ctx, mem);
-
+ ret = bdev->driver->move(bo, evict, ctx, mem);
if (ret) {
if (bdev->driver->move_notify) {
swap(*mem, bo->mem);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 0/6] drm/nouveau: Support sync FDs and sync objects
On Thu, Sep 24, 2020 at 12:05 PM Thierry Reding wrote: > > On Wed, Sep 23, 2020 at 05:21:24PM +0200, Daniel Vetter wrote: > > On Wed, Sep 23, 2020 at 11:18:53AM +0200, Thierry Reding wrote: > > > On Fri, Aug 28, 2020 at 12:40:10PM +0200, Thierry Reding wrote: > > > > From: Thierry Reding > > > > > > > > Hi, > > > > > > > > This series implements a new IOCTL to submit push buffers that can > > > > optionally return a sync FD or sync object to userspace. This is useful > > > > in cases where userspace wants to synchronize operations between the GPU > > > > and another driver (such as KMS for display). Among other things this > > > > allows extensions such as eglDupNativeFenceFDANDROID to be implemented. > > > > > > > > Note that patch 4 modifies the ABI introduced in patch 3 by allowing DRM > > > > sync objects to be passed rather than only sync FDs. It also allows any > > > > number of sync FDs/objects to be passed in or emitted. I think those are > > > > useful features, but I left them in a separate patch in case everybody > > > > else thinks that this won't be needed. If we decide to merge the new ABI > > > > then patch 4 should be squashed into patch 3. > > > > > > > > The corresponding userspace changes can be found here: > > > > > > > > libdrm: > > > > https://gitlab.freedesktop.org/tagr/drm/-/commits/nouveau-sync-fd-v2/ > > > > mesa: > > > > https://gitlab.freedesktop.org/tagr/mesa/-/commits/nouveau-sync-fd/ > > > > > > > > I've verified that this works with kmscube's --atomic mode and Weston. > > > > > > Hi Ben, > > > > > > any thoughts on this series? I realize that this is somewhat suboptimal > > > because we're effectively adding a duplicate of the existing IOCTL with > > > only the "minor" extension of adding sync FDs/objects, but at the same > > > time I don't have any good ideas on what else to add to make this more > > > appealing or if you have any plans of your own to address this in the > > > future. > > > > drm core automatically zero-extends ioctl structs both ways, so if all you > > do is add more stuff to the top level ioctl struct at the bottom, there's > > no need to duplicate any code. At least as long as you guarantee that 0 == > > old behaviour for both in and out parameters. > > But that only works if the structure size remains fixed, right? In this > case, however, we have to extend the structure with additional fields, > so the size is going to change and therefore the IOCTL number will also > change. Nope, drm_ioctl() is pretty much magic, and will zero-extend size mismatches in both ways. Which means you can run userspace compile against old kernels (so user_sz > kernel_sz) and you can run old userspace on new kernels (so user_sz < kernel_sz) and it will all work correctly. No need to allocate new ioctl numbers for this case, just extend at the bottom. We're doing this pretty much all the time. You might still want a getparam (or explicit flag, if all versions of that ioctl validated the flags correctly) since doing since a dummy pushbuf on an old kernel won't result in anything getting rejected. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/45] drm/ttm: refactor out common code to setup a new tt backed resource
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
This factors out the code to set the new caching and for non-system
tt populate and bind things.
The same code was used twice in the move paths.
Signed-off-by: Dave Airlie
Reviewed-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 12 +-
drivers/gpu/drm/ttm/ttm_bo_util.c | 40 +++
include/drm/ttm/ttm_bo_driver.h | 3 +++
3 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 6a7f4c028801..c8dffc8b40fc 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -252,19 +252,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
if (ret)
goto out_err;
- ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
+ ret = ttm_bo_move_to_new_tt_mem(bo, ctx, mem);
if (ret)
goto out_err;
-
- if (mem->mem_type != TTM_PL_SYSTEM) {
- ret = ttm_tt_populate(bdev, bo->ttm, ctx);
- if (ret)
- goto out_err;
-
- ret = ttm_bo_tt_bind(bo, mem);
- if (ret)
- goto out_err;
- }
}
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 bdee4df1f3f2..aecdb2d92a54 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -50,11 +50,33 @@ void ttm_bo_free_old_node(struct ttm_buffer_object *bo)
ttm_resource_free(bo, &bo->mem);
}
+int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
+ struct ttm_operation_ctx *ctx,
+ struct ttm_resource *new_mem)
+{
+ struct ttm_tt *ttm = bo->ttm;
+ int ret;
+
+ ret = ttm_tt_set_placement_caching(ttm, new_mem->placement);
+ if (unlikely(ret != 0))
+ return ret;
+
+ if (new_mem->mem_type != TTM_PL_SYSTEM) {
+ ret = ttm_tt_populate(bo->bdev, ttm, ctx);
+ if (unlikely(ret != 0))
+ return ret;
+
+ ret = ttm_bo_tt_bind(bo, new_mem);
+ if (unlikely(ret != 0))
+ return ret;
+ }
+ return 0;
+}
+
int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx,
struct ttm_resource *new_mem)
{
- struct ttm_tt *ttm = bo->ttm;
struct ttm_resource *old_mem = &bo->mem;
int ret;
@@ -72,21 +94,9 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
old_mem->mem_type = TTM_PL_SYSTEM;
}
- ret = ttm_tt_set_placement_caching(ttm, new_mem->placement);
- if (unlikely(ret != 0))
+ ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
+ if (ret)
return ret;
-
- if (new_mem->mem_type != TTM_PL_SYSTEM) {
-
- ret = ttm_tt_populate(bo->bdev, ttm, ctx);
- if (unlikely(ret != 0))
- return ret;
-
- ret = ttm_bo_tt_bind(bo, new_mem);
- if (unlikely(ret != 0))
- return ret;
- }
-
ttm_bo_assign_mem(bo, new_mem);
return 0;
}
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 864afa8f6f18..20e6839e9b73 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -605,6 +605,9 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx,
struct ttm_resource *new_mem);
+int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
+ struct ttm_operation_ctx *ctx,
+ struct ttm_resource *new_mem);
/**
* ttm_bo_move_memcpy
*
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] MAINTAINERS: add Dan Murphy as TP LP8xxx drivers maintainer
Hello On 9/22/20 10:28 AM, Krzysztof Kozlowski wrote: Milo Kim's email in TI bounces with permanent error (550: Invalid recipient). Last email from him on LKML was in 2017. Move Milo Kim to credits and add Dan Murphy from TI to look after: - TI LP855x backlight driver, - TI LP8727 charger driver, - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers. Cc: Dan Murphy Signed-off-by: Krzysztof Kozlowski Acked-by: Dan Murphy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/45] drm/ttm: split out the move to system from move ttm code
What should that be good for?
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 39 ---
1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index aecdb2d92a54..0ad02e27865d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -73,27 +73,38 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
return 0;
}
-int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
- struct ttm_operation_ctx *ctx,
- struct ttm_resource *new_mem)
+static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
+struct ttm_operation_ctx *ctx)
{
struct ttm_resource *old_mem = &bo->mem;
int ret;
- if (old_mem->mem_type != TTM_PL_SYSTEM) {
- ret = ttm_bo_wait_ctx(bo, ctx);
-
- if (unlikely(ret != 0)) {
- if (ret != -ERESTARTSYS)
- pr_err("Failed to expire sync object before
unbinding TTM\n");
- return ret;
- }
+ if (old_mem->mem_type == TTM_PL_SYSTEM)
+ return 0;
- ttm_bo_tt_unbind(bo);
- ttm_bo_free_old_node(bo);
- old_mem->mem_type = TTM_PL_SYSTEM;
+ ret = ttm_bo_wait_ctx(bo, ctx);
+ if (unlikely(ret != 0)) {
+ if (ret != -ERESTARTSYS)
+ pr_err("Failed to expire sync object before unbinding
TTM\n");
+ return ret;
}
+ ttm_bo_tt_unbind(bo);
+ ttm_bo_free_old_node(bo);
+ old_mem->mem_type = TTM_PL_SYSTEM;
+ return 0;
+}
+
+int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
+ struct ttm_operation_ctx *ctx,
+ struct ttm_resource *new_mem)
+{
+ int ret;
+
+ ret = ttm_bo_move_old_to_system(bo, ctx);
+ if (ret)
+ return ret;
+
ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
if (ret)
return ret;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 12/45] drm/ttm: drop free old node wrapper.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
This isn't really used anymore, if drivers needs it later,
just add back an inline wrapper.
Signed-off-by: Dave Airlie
Reviewed-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 11 +++
include/drm/ttm/ttm_bo_driver.h | 9 -
2 files changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 0ad02e27865d..daf9a91857f8 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -45,11 +45,6 @@ struct ttm_transfer_obj {
struct ttm_buffer_object *bo;
};
-void ttm_bo_free_old_node(struct ttm_buffer_object *bo)
-{
- ttm_resource_free(bo, &bo->mem);
-}
-
int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx,
struct ttm_resource *new_mem)
@@ -90,7 +85,7 @@ static int ttm_bo_move_old_to_system(struct ttm_buffer_object
*bo,
}
ttm_bo_tt_unbind(bo);
- ttm_bo_free_old_node(bo);
+ ttm_resource_free(bo, &bo->mem);
old_mem->mem_type = TTM_PL_SYSTEM;
return 0;
}
@@ -557,7 +552,7 @@ static int ttm_bo_wait_free_node(struct ttm_buffer_object
*bo,
if (!dst_use_tt)
ttm_bo_tt_destroy(bo);
- ttm_bo_free_old_node(bo);
+ ttm_resource_free(bo, &bo->mem);
return 0;
}
@@ -618,7 +613,7 @@ static void ttm_bo_move_pipeline_evict(struct ttm_buffer_object *bo,
}
spin_unlock(&from->move_lock);
- ttm_bo_free_old_node(bo);
+ ttm_resource_free(bo, &bo->mem);
dma_fence_put(bo->moving);
bo->moving = dma_fence_get(fence);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 20e6839e9b73..6690ec5d90ec 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -630,15 +630,6 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx,
struct ttm_resource *new_mem);
-/**
- * ttm_bo_free_old_node
- *
- * @bo: A pointer to a struct ttm_buffer_object.
- *
- * Utility function to free an old placement after a successful move.
- */
-void ttm_bo_free_old_node(struct ttm_buffer_object *bo);
-
/**
* ttm_bo_move_accel_cleanup.
*
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/45] drm/ttm: use new move interface for known system->ttm moves
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
In all 3 drivers there is a case where the driver knows the
bo is in SYSTEM so don't call the api that checks that.
Signed-off-by: Dave Airlie
In the long term I completely want to get rid of this nonsense because
it means that we call back into TTM which then calls us again.
But for now the patch is Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++--
drivers/gpu/drm/nouveau/nouveau_bo.c| 3 ++-
drivers/gpu/drm/radeon/radeon_ttm.c | 3 ++-
drivers/gpu/drm/ttm/ttm_bo_util.c | 1 +
4 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d3bd2fd448be..960a99d6793a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -607,11 +607,11 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object
*bo, bool evict,
}
/* move/bind old memory to GTT space */
- r = ttm_bo_move_ttm(bo, ctx, &tmp_mem);
+ r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
if (unlikely(r)) {
goto out_cleanup;
}
-
+ ttm_bo_assign_mem(bo, &tmp_mem);
/* copy to VRAM */
r = amdgpu_move_blit(bo, evict, new_mem, old_mem);
if (unlikely(r)) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 2c10a84b2cc0..2cb61eea9481 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -945,10 +945,11 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool
evict,
if (ret)
return ret;
- ret = ttm_bo_move_ttm(bo, ctx, &tmp_reg);
+ ret = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_reg);
if (ret)
goto out;
+ ttm_bo_assign_mem(bo, &tmp_reg);
ret = nouveau_bo_move_m2mf(bo, true, ctx, new_reg);
if (ret)
goto out;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index df5cedb2b632..7b778fc74f7b 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -280,10 +280,11 @@ static int radeon_move_ram_vram(struct ttm_buffer_object
*bo,
if (unlikely(r)) {
return r;
}
- r = ttm_bo_move_ttm(bo, ctx, &tmp_mem);
+ r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
if (unlikely(r)) {
goto out_cleanup;
}
+ ttm_bo_assign_mem(bo, &tmp_mem);
r = radeon_move_blit(bo, true, new_mem, old_mem);
if (unlikely(r)) {
goto out_cleanup;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index daf9a91857f8..e76883836e6e 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -67,6 +67,7 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
}
return 0;
}
+EXPORT_SYMBOL(ttm_bo_move_to_new_tt_mem);
static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] MAINTAINERS: mark FRAMEBUFFER LAYER as Orphan
It has been a fun ride since 2017 but unfortunately I don't have enough time to look after it properly anymore. Signed-off-by: Bartlomiej Zolnierkiewicz --- MAINTAINERS |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Index: b/MAINTAINERS === --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6894,10 +6894,9 @@ F: drivers/net/wan/dlci.c F: drivers/net/wan/sdla.c FRAMEBUFFER LAYER -M: Bartlomiej Zolnierkiewicz L: dri-devel@lists.freedesktop.org L: linux-fb...@vger.kernel.org -S: Maintained +S: Orphan Q: http://patchwork.kernel.org/project/linux-fbdev/list/ T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/fb/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic: document and enforce rules around "spurious" EBUSY
On Thu, Sep 24, 2020 at 01:13:17PM +0200, Daniel Vetter wrote:
> On Thu, Sep 24, 2020 at 1:01 PM Ville Syrjälä
> wrote:
> >
> > On Thu, Sep 24, 2020 at 01:10:56PM +0300, Pekka Paalanen wrote:
> > > On Thu, 24 Sep 2020 10:04:12 +0200
> > > Daniel Vetter wrote:
> > >
> > > > On Thu, Sep 24, 2020 at 9:41 AM Pekka Paalanen
> > > > wrote:
> > > > >
> > > > > On Wed, 23 Sep 2020 22:01:25 +0200
> > > > > Daniel Vetter wrote:
> > > > >
> > > > > > On Wed, Sep 23, 2020 at 9:17 PM Marius Vlad
> > > > > > wrote:
> > > > > > >
> > > > > > > On Wed, Sep 23, 2020 at 05:18:52PM +0200, Daniel Vetter wrote:
> > > > > > > > When doing an atomic modeset with ALLOW_MODESET drivers are
> > > > > > > > allowed to
> > > > > > > > pull in arbitrary other resources, including CRTCs (e.g. when
> > > > > > > > reconfiguring global resources).
> > > > >
> > > > > ...
> > > > >
> > > > > > > > @@ -1313,6 +1322,26 @@ int drm_atomic_check_only(struct
> > > > > > > > drm_atomic_state *state)
> > > > > > > > }
> > > > > > > > }
> > > > > > > >
> > > > > > > > + for_each_new_crtc_in_state(state, crtc, old_crtc_state, i)
> > > > > > > > + affected_crtc |= drm_crtc_mask(crtc);
> > > > > > > > +
> > > > > > > > + /*
> > > > > > > > + * For commits that allow modesets drivers can add other
> > > > > > > > CRTCs to the
> > > > > > > > + * atomic commit, e.g. when they need to reallocate
> > > > > > > > global resources.
> > > > > > > > + * This can cause spurious EBUSY, which robs compositors
> > > > > > > > of a very
> > > > > > > > + * effective sanity check for their drawing loop.
> > > > > > > > Therefor only allow
> > > > > > > > + * drivers to add unrelated CRTC states for modeset
> > > > > > > > commits.
> > > > > > > > + *
> > > > > > > > + * FIXME: Should add affected_crtc mask to the ATOMIC
> > > > > > > > IOCTL as an output
> > > > > > > > + * so compositors know what's going on.
> > > > > > > > + */
> > > > > > > > + if (affected_crtc != requested_crtc) {
> > > > > > > > + DRM_DEBUG_ATOMIC("driver added CRTC to commit:
> > > > > > > > requested 0x%x, affected 0x%0x\n",
> > > > > > > > + requested_crtc, affected_crtc);
> > > > > > > > + WARN(!state->allow_modeset, "adding CRTC not
> > > > > > > > allowed without modesets: requested 0x%x, affected 0x%0x\n",
> > > > > > > > + requested_crtc, affected_crtc);
> > > > > > > Previous patch had the warn on state->allow_modeset now is
> > > > > > > !state->allow_modeset. Is that correct?
> > > > > >
> > > > > > We need to fire a warning when allow_modeset is _not_ set. An
> > > > > > earlier
> > > > > > version got that wrong, and yes that would have caused a _ton_ of
> > > > > > warnings on any fairly new intel platform.
> > > > > >
> > > > > > > I haven't followed the entire thread on this matter, but I guess
> > > > > > > the idea
> > > > > > > is that somehow the kernel would pass to userspace a CRTC mask of
> > > > > > > affected_crtc (somehow, we don't know how atm) and with it,
> > > > > > > userspace
> > > > > > > can then issue a new commit (this commit blocking) with those?
> > > > > >
> > > > > > Either that, or just use that to track all the in-flight drm events.
> > > > > > Userspace will get events for all the crtc, not just the one it
> > > > > > asked
> > > > > > to update.
> > > > >
> > > > > Wait, does that happen already? Getting CRTC events for CRTCs
> > > > > userspace
> > > > > didn't include in the atomic commit?
> > > >
> > > > Yeah I'm pretty sure. With the affected_crtc mask you could update
> > > > your internal book-keeping to catch these, which should also prevent
> > > > all the spurious EBUSY. But I'm not entirely sure, I just read the
> > > > code, haven't tested.
> > >
> > > If that actually happens, how does userspace know whether the
> > > userdata argument with the event is valid or not?
> >
> > At some point I was worried about the kernel potentially sending spurious
> > events, but IIRC I managed to convince myself that it shouldn't happen.
> > I think I came to the conclusion the events were populated before the
> > core calls into the driver. But maybe I misanalyzed it, or something
> > has since broken?
>
> Hm right this shouldn't happen, I misread the code. So if userspace
> wants events for all affected crtc, it needs to add them explicitly to
> the atomic ioctl (just set an arbitrary property on each crtc to its
> current value or something like that).
Hmm. I thought we wouldn't even need the dummy prop. But looking at
the code we do in fact need it. The ioctl structure itself should
allow adding an object without any properties, but the code then
skips the get_crtc_state() and thus no event for that crtc. I guess
it's been like that forever so not much point in trying to change
that anymore.
--
Ville Syrjälä
Intel
___
dri-
[Bug 198551] amdgpu error on shutdown or gpu intense game
https://bugzilla.kernel.org/show_bug.cgi?id=198551 Gerry Binder (gerrybinde...@gmail.com) changed: What|Removed |Added CC||gerrybinde...@gmail.com --- Comment #4 from Gerry Binder (gerrybinde...@gmail.com) --- The top goal scorer of El Clasico rivalry, Lionel Messi could make return for Real Madrid vs Barcelona match after suffering from long time injury. For More Details visit: https://elclasiconews.com/ -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: bridge: cdns-mhdp8546: fix compile warning
Hi Tomi,
Thank you for the patch.
On Wed, Sep 23, 2020 at 11:30:57AM +0300, Tomi Valkeinen wrote:
> On x64 we get:
>
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:751:10: warning:
> conversion from 'long unsigned int' to 'unsigned int' changes value from
> '18446744073709551613' to '4294967293' [-Woverflow]
>
> The registers are 32 bit, so fix by casting to u32.
I wonder if we need a BIT32 macro... This is a common issue, it would be
nice to handle it in the macros that define register fields.
> Fixes: fb43aa0acdfd ("drm: bridge: Add support for Cadence MHDP8546 DPI/DP
> bridge")
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 621ebdbff8a3..d0c65610ebb5 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -748,7 +748,7 @@ static int cdns_mhdp_fw_activate(const struct firmware
> *fw,
>* bridge should already be detached.
>*/
> if (mhdp->bridge_attached)
> - writel(~CDNS_APB_INT_MASK_SW_EVENT_INT,
> + writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT,
> mhdp->regs + CDNS_APB_INT_MASK);
>
> spin_unlock(&mhdp->start_lock);
> @@ -1689,7 +1689,7 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
>
> /* Enable SW event interrupts */
> if (hw_ready)
> - writel(~CDNS_APB_INT_MASK_SW_EVENT_INT,
> + writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT,
> mhdp->regs + CDNS_APB_INT_MASK);
>
> return 0;
> @@ -2122,7 +2122,7 @@ static void cdns_mhdp_bridge_hpd_enable(struct
> drm_bridge *bridge)
>
> /* Enable SW event interrupts */
> if (mhdp->bridge_attached)
> - writel(~CDNS_APB_INT_MASK_SW_EVENT_INT,
> + writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT,
> mhdp->regs + CDNS_APB_INT_MASK);
> }
>
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2] dt-bindings: dp-connector: add binding for DisplayPort connector
On 23/09/2020 23:00, Rob Herring wrote: > On Wed, Sep 23, 2020 at 11:15 AM Tomi Valkeinen wrote: >> >> Hi Rob, >> >> On 23/09/2020 19:17, Rob Herring wrote: >> * No eDP. There's really no "eDP connector", as it's always a custom made connection between the DP and the DP panel. So possibly there is no need for edp-connector binding, but even if there is, I don't want to guess what it could look like, and could it be part of the dp-connector binding. >>> >>> I don't think that's true. Do an image search for 'edp pinout'. AFAICT, >>> there's 2 lane 30 pin and 4 lane 40 pin. One image says 'Table 5-3 in >>> eDP v1.2'. Of course, I'm sure there's custom ones too. From a binding >>> perspective, we probably don't care about the differences, but just need >>> to be able to describe HPD, backlight power, enable, and pwm, and LCD >>> power. >> >> That's true. The eDP spec lists 4 different standard pinouts (how >> strictly those are followed, I have no idea). But it does not define a >> connector or a cable. And afaik eDP is defined to be not user-detachable. > > Yes, but HPD is still used (or sometimes broken). We could just put > that all in panel nodes, but IIRC the last time this came up the issue > was handling devices with different panels stuffed by the > manufacturer. An eDP connector binding would be one way to handle that > as it is somewhat standardized while panel connections aren't at all. HPD in DisplayPort, and especially in eDP, is not strictly speaking "cable inserted or removed", but rather signaling that the monitor/panel is ready (e.g. after powering it up), or there has been a link issue, or there has been a major change in settings, or signaling DP interrupt, etc. And HPD (and EDID and some other things) are optional with eDP. I don't think those rule out an edp-connector, but just things to consider. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209373] Pixels
https://bugzilla.kernel.org/show_bug.cgi?id=209373 --- Comment #4 from Jens R. (rapp.j...@gmail.com) --- (In reply to Michel Dänzer from comment #3) > Yeah, the BIOS seems very picky about which partitions it boots from. With > GParted or a similar tool, check that the partition which contains the EFI > directory has at least the boot & esp flags set. (I only realized this might > be enough after manually extracting the BIOS image to a USB HD, and updating > from that) You're right... it's really picky.. You need to switch to insecure boot and after, legacy boot before uefi. Updating BIOS seems to work I'll watch behaviour for a while but I think that the issue is fixed by BIOS update. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/45] drm/ttm: add move old to system to drivers.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
Uninline ttm_bo_move_ttm. Eventually want to unhook the unbind out.
Signed-off-by: Dave Airlie
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 -
drivers/gpu/drm/nouveau/nouveau_bo.c| 9 -
drivers/gpu/drm/radeon/radeon_ttm.c | 10 +-
drivers/gpu/drm/ttm/ttm_bo_util.c | 5 +++--
include/drm/ttm/ttm_bo_driver.h | 2 ++
5 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 960a99d6793a..e20ce380f627 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -568,7 +568,14 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object
*bo, bool evict,
}
/* move BO (in tmp_mem) to new_mem */
- r = ttm_bo_move_ttm(bo, ctx, new_mem);
+ r = ttm_bo_move_old_to_system(bo, ctx);
+ if (unlikely(r))
+ goto out_cleanup;
+
+ r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
+ if (unlikely(r))
+ goto out_cleanup;
+ ttm_bo_assign_mem(bo, new_mem);
out_cleanup:
ttm_resource_free(bo, &tmp_mem);
return r;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 2cb61eea9481..a95d208c76a1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -915,7 +915,14 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool
evict,
if (ret)
goto out;
- ret = ttm_bo_move_ttm(bo, ctx, new_reg);
+ ret = ttm_bo_move_old_to_system(bo, ctx);
+ if (ret)
+ goto out;
+
+ ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
+ if (ret)
+ goto out;
+ ttm_bo_assign_mem(bo, new_reg);
out:
ttm_resource_free(bo, &tmp_reg);
return ret;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 7b778fc74f7b..89455f2d3bb6 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -249,7 +249,15 @@ static int radeon_move_vram_ram(struct ttm_buffer_object
*bo,
if (unlikely(r)) {
goto out_cleanup;
}
- r = ttm_bo_move_ttm(bo, ctx, new_mem);
+ r = ttm_bo_move_old_to_system(bo, ctx);
+ if (unlikely(r))
+ goto out_cleanup;
+
+ r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
+ if (unlikely(r))
+ goto out_cleanup;
+ ttm_bo_assign_mem(bo, new_mem);
+
out_cleanup:
ttm_resource_free(bo, &tmp_mem);
return r;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index e76883836e6e..1e701dd192d3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -69,8 +69,8 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_bo_move_to_new_tt_mem);
-static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
-struct ttm_operation_ctx *ctx)
+int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
+ struct ttm_operation_ctx *ctx)
{
struct ttm_resource *old_mem = &bo->mem;
int ret;
@@ -90,6 +90,7 @@ static int ttm_bo_move_old_to_system(struct ttm_buffer_object
*bo,
old_mem->mem_type = TTM_PL_SYSTEM;
return 0;
}
+EXPORT_SYMBOL(ttm_bo_move_old_to_system);
int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx,
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 6690ec5d90ec..65cf86b3ba0b 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -605,6 +605,8 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx,
struct ttm_resource *new_mem);
+int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
+ struct ttm_operation_ctx *ctx);
int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx,
struct ttm_resource *new_mem);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: mark FRAMEBUFFER LAYER as Orphan
On Thu, Sep 24, 2020 at 1:25 PM Bartlomiej Zolnierkiewicz wrote: > > It has been a fun ride since 2017 but unfortunately I don't have > enough time to look after it properly anymore. > > Signed-off-by: Bartlomiej Zolnierkiewicz Thanks a lot for all the work you put in in picking up patches and cleaning up drivers for compile testing! Very much appreciated. I think for now we can just leave fbdev in drm-misc, with people picking up patches ad-hoc (and maintainers serving as fallbacks). Reviewed-by: Daniel Vetter I guess you'll push this yourself as the last one, for closure and all :-) Also drm-misc commit rights will stay around for you (except if you want to hand that in) so if you have and display/gpu patches you can still push them through the usual flow. Cheers, Daniel > --- > MAINTAINERS |3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > Index: b/MAINTAINERS > === > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6894,10 +6894,9 @@ F: drivers/net/wan/dlci.c > F: drivers/net/wan/sdla.c > > FRAMEBUFFER LAYER > -M: Bartlomiej Zolnierkiewicz > L: dri-devel@lists.freedesktop.org > L: linux-fb...@vger.kernel.org > -S: Maintained > +S: Orphan > Q: http://patchwork.kernel.org/project/linux-fbdev/list/ > T: git git://anongit.freedesktop.org/drm/drm-misc > F: Documentation/fb/ > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 15/45] drm/ttm: push copy unbind into drivers.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
This uninlines ttm_bo_move_old_to_system into 3 places
Signed-off-by: Dave Airlie
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 ++-
drivers/gpu/drm/nouveau/nouveau_bo.c| 7 +--
drivers/gpu/drm/radeon/radeon_ttm.c | 7 ++-
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e20ce380f627..d165edacc347 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -66,6 +66,8 @@
static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
struct ttm_tt *ttm,
struct ttm_resource *bo_mem);
+static void amdgpu_ttm_backend_unbind(struct ttm_bo_device *bdev,
+ struct ttm_tt *ttm);
static int amdgpu_ttm_init_on_chip(struct amdgpu_device *adev,
unsigned int type,
@@ -568,10 +570,13 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object
*bo, bool evict,
}
/* move BO (in tmp_mem) to new_mem */
- r = ttm_bo_move_old_to_system(bo, ctx);
+ r = ttm_bo_wait_ctx(bo, ctx);
if (unlikely(r))
goto out_cleanup;
+ amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
+ ttm_resource_free(bo, &bo->mem);
+
r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
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 a95d208c76a1..1e6c2561d692 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -46,7 +46,7 @@
static int nouveau_ttm_tt_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm,
struct ttm_resource *reg);
-
+static void nouveau_ttm_tt_unbind(struct ttm_bo_device *bdev, struct ttm_tt
*ttm);
/*
* NV10-NV40 tiling helpers
*/
@@ -915,10 +915,13 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool
evict,
if (ret)
goto out;
- ret = ttm_bo_move_old_to_system(bo, ctx);
+ ret = ttm_bo_wait_ctx(bo, ctx);
if (ret)
goto out;
+ nouveau_ttm_tt_unbind(bo->bdev, bo->ttm);
+ ttm_resource_free(bo, &bo->mem);
+
ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
if (ret)
goto out;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 89455f2d3bb6..10d25d3b83f2 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -59,6 +59,8 @@ static void radeon_ttm_debugfs_fini(struct radeon_device
*rdev);
static int radeon_ttm_tt_bind(struct ttm_bo_device *bdev,
struct ttm_tt *ttm,
struct ttm_resource *bo_mem);
+static void radeon_ttm_tt_unbind(struct ttm_bo_device *bdev,
+struct ttm_tt *ttm);
struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
{
@@ -249,10 +251,13 @@ static int radeon_move_vram_ram(struct ttm_buffer_object
*bo,
if (unlikely(r)) {
goto out_cleanup;
}
- r = ttm_bo_move_old_to_system(bo, ctx);
+ r = ttm_bo_wait_ctx(bo, ctx);
if (unlikely(r))
goto out_cleanup;
+ radeon_ttm_tt_unbind(bo->bdev, bo->ttm);
+ ttm_resource_free(bo, &bo->mem);
+
r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
if (unlikely(r))
goto out_cleanup;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 4/6] drm/i915: use vmap in i915_gem_object_map
On 23/09/2020 15:44, Christoph Hellwig wrote: On Wed, Sep 23, 2020 at 02:58:43PM +0100, Tvrtko Ursulin wrote: Series did not get a CI run from our side because of a different base so I don't know if you would like to have a run there? If so you would need to rebase against git://anongit.freedesktop.org/drm-tip drm-tip and you could even send a series to intel-gfx-try...@lists.freedesktop.org, suppressing cc, to check it out without sending a copy to the real mailing list. It doesn't seem like I can post to any freedesktop list, as I always get rejection messages. But I'll happily prepare a branch if one of you an feed it into your CI. That's fine, just ping me and I will forward it for testing, thanks! git://git.infradead.org/users/hch/misc.git i915-vmap-wip Gitweb: http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/i915-vmap-wip note that this includes a new commit to clean up one of the recent commits in the code. CI says series looks good from the i915 perspective (*). I don't know how will you handle it logistically, but when you have final version I am happy to re-read and r-b the i915 patches. Regards, Tvrtko *) https://patchwork.freedesktop.org/series/82051/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] MAINTAINERS: add Dan Murphy as TP LP8xxx drivers maintainer
On Thu, 24 Sep 2020, Krzysztof Kozlowski wrote: > On Wed, 23 Sep 2020 at 22:01, Jonathan Cameron wrote: > > > > On Wed, 23 Sep 2020 11:53:33 -0500 > > Dan Murphy wrote: > > > > > Hello > > > > > > On 9/22/20 10:28 AM, Krzysztof Kozlowski wrote: > > > > Milo Kim's email in TI bounces with permanent error (550: Invalid > > > > recipient). Last email from him on LKML was in 2017. Move Milo Kim to > > > > credits and add Dan Murphy from TI to look after: > > > > - TI LP855x backlight driver, > > > > - TI LP8727 charger driver, > > > > - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers. > > > > > > > > Cc: Dan Murphy > > > > Signed-off-by: Krzysztof Kozlowski > > > > > > Acked-by: Dan Murphy > > > > > Not sure who will pick this one up, but > > Acked-by: Jonathan Cameron > > I guess whoever is first. :) > This spans across systems but the common part is MFD, so maybe Lee - > could you pick it up? Yes, I'll handle it. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 30/45] drm/ttm: add a new invalidate notify callback.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
Signed-off-by: Dave Airlie
NAK, completely unnecessary.
We should rather do the remaining accounting in the already existing
release_notify() callback.
That makes much more sense and if I'm not completely mistaken could
actually fix a bug in amdgpu.
Christian.
---
drivers/gpu/drm/ttm/ttm_bo.c| 4 +++-
include/drm/ttm/ttm_bo_driver.h | 7 +++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index a2a61a8d1394..ba69c682e53b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -282,7 +282,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
{
- if (bo->bdev->driver->move_notify)
+ if (bo->bdev->driver->invalidate_notify)
+ bo->bdev->driver->invalidate_notify(bo);
+ else if (bo->bdev->driver->move_notify)
bo->bdev->driver->move_notify(bo, false, NULL);
ttm_bo_tt_destroy(bo);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index cfb151dbb2d0..da4afe669664 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -165,6 +165,13 @@ struct ttm_bo_driver {
void (*move_notify)(struct ttm_buffer_object *bo,
bool evict,
struct ttm_resource *new_mem);
+
+ /**
+* Hook to notify driver about a bo being torn down.
+* can be used for invalidation instead of move_notify.
+*/
+ void (*invalidate_notify)(struct ttm_buffer_object *bo);
+
/* notify the driver we are taking a fault on this BO
* and have reserved it */
int (*fault_reserve_notify)(struct ttm_buffer_object *bo);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, 24 Sep 2020 08:57:52 +0200 Thomas Gleixner wrote: > > Now as for migration disabled nesting, at least now we would have > > groupings of this, and perhaps the theorists can handle that. I mean, > > how is this much different that having a bunch of tasks blocked on a > > mutex with the owner is pinned on a CPU? > > > > migrate_disable() is a BKL of pinning affinity. > > No. That's just wrong. preempt disable is a concurrency control, I think you totally misunderstood what I was saying. The above wasn't about comparing preempt_disable to migrate_disable. It was comparing migrate_disable to a chain of tasks blocked on mutexes where the top owner has preempt_disable set. You still have a bunch of tasks that can't move to other CPUs. > > If we only have local_lock() available (even on !RT), then it makes > > the blocking in groups. At least this way you could grep for all the > > different local_locks in the system and plug that into the algorithm > > for WCS, just like one would with a bunch of mutexes. > > You cannot do that on RT at all where migrate disable is substituting > preempt disable in spin and rw locks. The result would be the same as > with a !RT kernel just with horribly bad performance. Note, the spin and rwlocks already have a lock associated with them. Why would it be any different on RT? I wasn't suggesting adding another lock inside a spinlock. Why would I recommend THAT? I wasn't recommending blindly replacing migrate_disable() with local_lock(). I just meant expose local_lock() but not migrate_disable(). > > That means the stacking problem has to be solved anyway. > > So why on earth do you want to create yet another special duct tape case > for kamp_local() which proliferates inconsistency instead of aiming for > consistency accross all preemption models? The idea was to help with the scheduling issue. Anyway, instead of blocking. What about having a counter of number of migrate disabled tasks per cpu, and when taking a migrate_disable(), and there's already another task with migrate_disabled() set, and the current task has an affinity greater than 1, it tries to migrate to another CPU? This way migrate_disable() is less likely to have a bunch of tasks blocked on one CPU serialized by each task exiting the migrate_disable() section. Yes, there's more overhead, but it only happens if multiple tasks are in a migrate disable section on the same CPU. -- Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
All the accel moves do the same pattern here, provide a helper
And exactly that pattern I want to get away from.
See what happens if we (for example) have a VRAM -> SYSTEM move is the
following:
1. TTM allocates a new ttm_resource object in the SYSTEM domain.
2. We call the driver to move from VRAM to SYSTEM.
3. Driver finds that it can't do this and calls TTM to allocate GTT.
4. Since we are maybe out of GTT TTM evicts a different BO from GTT to
SYSTEM and call driver again.
This is a horrible ping/pong between driver/TTM/driver/TTM/driver and we
should stop that immediately.
My suggestion is that we rewrite how drivers call the ttm_bo_validate()
function so that we can guarantee that this never happens.
What do you think?
Thanks,
Christian.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/ttm/ttm_bo.c| 28
include/drm/ttm/ttm_bo_driver.h | 5 +
2 files changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index eb76002aa53d..358d1580dc16 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1541,3 +1541,31 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
ttm_tt_destroy(bo->bdev, bo->ttm);
bo->ttm = NULL;
}
+
+int ttm_bo_create_tt_tmp(struct ttm_buffer_object *bo,
+struct ttm_operation_ctx *ctx,
+struct ttm_resource *new_mem,
+struct ttm_resource *new_temp)
+{
+ struct ttm_place placement_memtype = {
+ .fpfn = 0,
+ .lpfn = 0,
+ .mem_type = TTM_PL_TT,
+ .flags = TTM_PL_MASK_CACHING
+ };
+ struct ttm_placement placement;
+ int ret;
+
+ placement.num_placement = placement.num_busy_placement = 1;
+ placement.placement = placement.busy_placement = &placement_memtype;
+
+ *new_temp = *new_mem;
+ new_temp->mm_node = NULL;
+
+ ret = ttm_bo_mem_space(bo, &placement, new_temp, ctx);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(ttm_bo_create_tt_tmp);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4a63fec24e90..a7507dfaa89d 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -558,6 +558,11 @@ void ttm_mem_io_free(struct ttm_bo_device *bdev,
int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx,
struct ttm_resource *new_mem);
+
+int ttm_bo_create_tt_tmp(struct ttm_buffer_object *bo,
+struct ttm_operation_ctx *ctx,
+struct ttm_resource *new_mem,
+struct ttm_resource *new_temp);
/**
* ttm_bo_move_memcpy
*
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, Sep 24, 2020 at 08:32:41AM -0400, Steven Rostedt wrote: > Anyway, instead of blocking. What about having a counter of number of > migrate disabled tasks per cpu, and when taking a migrate_disable(), and > there's > already another task with migrate_disabled() set, and the current task has > an affinity greater than 1, it tries to migrate to another CPU? That doesn't solve the problem. On wakeup we should already prefer an idle CPU over one running a (RT) task, but you can always wake more tasks than there's CPUs around and you'll _have_ to stack at some point. The trick is how to unstack them correctly. We need to detect when a migrate_disable() task _should_ start running again, and migrate away whoever is in the way at that point. It turns out, that getting selected for pull-balance is exactly that condition, and clearly a migrate_disable() task cannot be pulled, but we can use that signal to try and pull away the running task that's in the way. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 26/45] drm/ttm: remove bind/unbind callbacks.
Am 24.09.20 um 07:18 schrieb Dave Airlie:
From: Dave Airlie
If a driver wants to bind/unbind then it should implement a move
callback.
Signed-off-by: Dave Airlie
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 8 +++--
drivers/gpu/drm/nouveau/nouveau_bo.c | 8 ++---
drivers/gpu/drm/qxl/qxl_ttm.c | 20 ---
drivers/gpu/drm/radeon/radeon_ttm.c| 8 ++---
drivers/gpu/drm/ttm/ttm_bo.c | 17 --
drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 8 +++--
include/drm/ttm/ttm_bo_driver.h| 39 --
8 files changed, 16 insertions(+), 93 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 50362f56d2d0..a729bdcdd017 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -701,6 +701,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
bool evict,
ttm_bo_move_null(bo, new_mem);
return 0;
}
+
if (old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_TT) {
ttm_bo_move_null(bo, new_mem);
@@ -709,9 +710,12 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
bool evict,
if (old_mem->mem_type == TTM_PL_TT &&
new_mem->mem_type == TTM_PL_SYSTEM) {
- r = ttm_bo_move_old_to_system(bo, ctx);
+ r = ttm_bo_wait_ctx(bo, ctx);
if (r)
return r;
+ amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
+ ttm_resource_free(bo, &bo->mem);
+
r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
if (r)
return r;
@@ -1740,8 +1744,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
.ttm_tt_create = &amdgpu_ttm_tt_create,
.ttm_tt_populate = &amdgpu_ttm_tt_populate,
.ttm_tt_unpopulate = &amdgpu_ttm_tt_unpopulate,
- .ttm_tt_bind = &amdgpu_ttm_backend_bind,
- .ttm_tt_unbind = &amdgpu_ttm_backend_unbind,
.ttm_tt_destroy = &amdgpu_ttm_backend_destroy,
.eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
.evict_flags = &amdgpu_evict_flags,
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index fc0f9e9232db..cb2a0f1bf7ff 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1107,9 +1107,9 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
if (old_reg->mem_type == TTM_PL_TT &&
new_reg->mem_type == TTM_PL_SYSTEM) {
- ret = ttm_bo_move_old_to_system(bo, ctx);
- if (ret)
- goto out;
+ nouveau_ttm_tt_unbind(bo->bdev, bo->ttm);
+ ttm_resource_free(bo, &bo->mem);
+
ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
if (ret)
goto out;
@@ -1438,8 +1438,6 @@ struct ttm_bo_driver nouveau_bo_driver = {
.ttm_tt_create = &nouveau_ttm_tt_create,
.ttm_tt_populate = &nouveau_ttm_tt_populate,
.ttm_tt_unpopulate = &nouveau_ttm_tt_unpopulate,
- .ttm_tt_bind = &nouveau_ttm_tt_bind,
- .ttm_tt_unbind = &nouveau_ttm_tt_unbind,
.ttm_tt_destroy = &nouveau_ttm_tt_destroy,
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = nouveau_bo_evict_flags,
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 378b6827b7a3..3bca5f8d8ac5 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -102,24 +102,6 @@ struct qxl_ttm_tt {
struct ttm_tt ttm;
};
-static int qxl_ttm_backend_bind(struct ttm_bo_device *bdev,
- struct ttm_tt *ttm,
- struct ttm_resource *bo_mem)
-{
- if (!ttm->num_pages) {
- WARN(1, "nothing to bind %lu pages for mreg %p back %p!\n",
-ttm->num_pages, bo_mem, ttm);
- }
- /* Not implemented */
- return -1;
-}
-
-static void qxl_ttm_backend_unbind(struct ttm_bo_device *bdev,
- struct ttm_tt *ttm)
-{
- /* Not implemented */
-}
-
static void qxl_ttm_backend_destroy(struct ttm_bo_device *bdev,
struct ttm_tt *ttm)
{
@@ -186,9 +168,7 @@ static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
static struct ttm_bo_driver qxl_bo_driver = {
.ttm_tt_create = &qxl_ttm_tt_create,
- .ttm_tt_bind = &qxl_ttm_backend_bind,
.ttm_tt_destroy = &qxl_ttm_backend_destroy,
- .ttm_tt_unbind = &qxl_ttm_backend_unbind,
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = &qxl_evict_flags,
.move = &qxl_bo_m
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, 24 Sep 2020 14:42:41 +0200 Peter Zijlstra wrote: > On Thu, Sep 24, 2020 at 08:32:41AM -0400, Steven Rostedt wrote: > > Anyway, instead of blocking. What about having a counter of number of > > migrate disabled tasks per cpu, and when taking a migrate_disable(), and > > there's > > already another task with migrate_disabled() set, and the current task has > > an affinity greater than 1, it tries to migrate to another CPU? > > That doesn't solve the problem. On wakeup we should already prefer an > idle CPU over one running a (RT) task, but you can always wake more > tasks than there's CPUs around and you'll _have_ to stack at some point. Yes, understood. > > The trick is how to unstack them correctly. We need to detect when a > migrate_disable() task _should_ start running again, and migrate away > whoever is in the way at that point. > > It turns out, that getting selected for pull-balance is exactly that > condition, and clearly a migrate_disable() task cannot be pulled, but we > can use that signal to try and pull away the running task that's in the > way. Unless of course that running task is in a migrate disable section itself ;-) But I guess we will always have that SHC, and there will always be a scenario that you can't balance properly. But hopefully in practice we wont see that. How to handle kmap_local(), will migrate_disable() be used only for 32bit or, for consistency, will it also apply to 64bit? -- Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 11/23] device-dax: Kill dax_kmem_res
On Thu, Sep 24, 2020 at 12:26 AM David Hildenbrand wrote:
>
> On 23.09.20 23:41, Dan Williams wrote:
> > On Wed, Sep 23, 2020 at 1:04 AM David Hildenbrand wrote:
> >>
> >> On 08.09.20 17:33, Joao Martins wrote:
> >>> [Sorry for the late response]
> >>>
> >>> On 8/21/20 11:06 AM, David Hildenbrand wrote:
> On 03.08.20 07:03, Dan Williams wrote:
> > @@ -37,109 +45,94 @@ int dev_dax_kmem_probe(struct device *dev)
> > * could be mixed in a node with faster memory, causing
> > * unavoidable performance issues.
> > */
> > - numa_node = dev_dax->target_node;
> > if (numa_node < 0) {
> > dev_warn(dev, "rejecting DAX region with invalid node:
> > %d\n",
> > numa_node);
> > return -EINVAL;
> > }
> >
> > - /* Hotplug starting at the beginning of the next block: */
> > - kmem_start = ALIGN(range->start, memory_block_size_bytes());
> > -
> > - kmem_size = range_len(range);
> > - /* Adjust the size down to compensate for moving up kmem_start: */
> > - kmem_size -= kmem_start - range->start;
> > - /* Align the size down to cover only complete blocks: */
> > - kmem_size &= ~(memory_block_size_bytes() - 1);
> > - kmem_end = kmem_start + kmem_size;
> > -
> > - new_res_name = kstrdup(dev_name(dev), GFP_KERNEL);
> > - if (!new_res_name)
> > + res_name = kstrdup(dev_name(dev), GFP_KERNEL);
> > + if (!res_name)
> > return -ENOMEM;
> >
> > - /* Region is permanently reserved if hotremove fails. */
> > - new_res = request_mem_region(kmem_start, kmem_size, new_res_name);
> > - if (!new_res) {
> > - dev_warn(dev, "could not reserve region [%pa-%pa]\n",
> > -&kmem_start, &kmem_end);
> > - kfree(new_res_name);
> > + res = request_mem_region(range.start, range_len(&range), res_name);
>
> I think our range could be empty after aligning. I assume
> request_mem_region() would check that, but maybe we could report a
> better error/warning in that case.
>
> >>> dax_kmem_range() already returns a memory-block-aligned @range but
> >>> IIUC request_mem_region() isn't checking for that. Having said that
> >>> the returned @res wouldn't be different from the passed range.start.
> >>>
> > /*
> > * Ensure that future kexec'd kernels will not treat this as RAM
> > * automatically.
> > */
> > - rc = add_memory_driver_managed(numa_node, new_res->start,
> > - resource_size(new_res), kmem_name);
> > + rc = add_memory_driver_managed(numa_node, res->start,
> > + resource_size(res), kmem_name);
> > +
> > + res->flags |= IORESOURCE_BUSY;
>
> Hm, I don't think that's correct. Any specific reason why to mark the
> not-added, unaligned parts BUSY? E.g., walk_system_ram_range() could
> suddenly stumble over it - and e.g., similarly kexec code when trying to
> find memory for placing kexec images. I think we should leave this
> !BUSY, just as it is right now.
>
> >>> Agreed.
> >>>
> > if (rc) {
> > - release_resource(new_res);
> > - kfree(new_res);
> > - kfree(new_res_name);
> > + release_mem_region(range.start, range_len(&range));
> > + kfree(res_name);
> > return rc;
> > }
> > - dev_dax->dax_kmem_res = new_res;
> > +
> > + dev_set_drvdata(dev, res_name);
> >
> > return 0;
> > }
> >
> > #ifdef CONFIG_MEMORY_HOTREMOVE
> > -static int dev_dax_kmem_remove(struct device *dev)
> > +static void dax_kmem_release(struct dev_dax *dev_dax)
> > {
> > - struct dev_dax *dev_dax = to_dev_dax(dev);
> > - struct resource *res = dev_dax->dax_kmem_res;
> > - resource_size_t kmem_start = res->start;
> > - resource_size_t kmem_size = resource_size(res);
> > - const char *res_name = res->name;
> > int rc;
> > + struct device *dev = &dev_dax->dev;
> > + const char *res_name = dev_get_drvdata(dev);
> > + struct range range = dax_kmem_range(dev_dax);
> >
> > /*
> > * We have one shot for removing memory, if some memory blocks were
> > not
> > * offline prior to calling this function remove_memory() will
> > fail, and
> > * there is no way to hotremove this memory until reboot because
> > device
> > -* unbind will succeed even if we return failure.
> > +* unbind will proceed regardless of the remove_memory result.
> > */
> > - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size);
> > - if (rc) {
> > - any_hotremove_failed = true;
> > - dev_err(dev,
> >
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, Sep 24, 2020 at 09:51:38AM -0400, Steven Rostedt wrote: > > It turns out, that getting selected for pull-balance is exactly that > > condition, and clearly a migrate_disable() task cannot be pulled, but we > > can use that signal to try and pull away the running task that's in the > > way. > > Unless of course that running task is in a migrate disable section > itself ;-) See my ramblings here: https://lkml.kernel.org/r/20200924082717.ga1362...@hirez.programming.kicks-ass.net My plan was to have the migrate_enable() of the running task trigger the migration in that case. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers
On Thu, Sep 24, 2020 at 09:38:22AM -0400, Peilin Ye wrote:
> Hi all,
>
> syzbot has reported [1] a global out-of-bounds read issue in
> fbcon_get_font(). A malicious user may resize `vc_font.height` to a large
> value in vt_ioctl(), causing fbcon_get_font() to overflow our built-in
> font data buffers, declared in lib/fonts/font_*.c:
>
> (e.g. lib/fonts/font_8x8.c)
> #define FONTDATAMAX 2048
>
> static const unsigned char fontdata_8x8[FONTDATAMAX] = {
>
> /* 0 0x00 '^@' */
> 0x00, /* */
> 0x00, /* */
> 0x00, /* */
> 0x00, /* */
> 0x00, /* */
> 0x00, /* */
> 0x00, /* */
> 0x00, /* */
> [...]
>
> In order to perform a reliable range check, fbcon_get_font() needs to know
> `FONTDATAMAX` for each built-in font under lib/fonts/. Unfortunately, we
> do not keep that information in our font descriptor,
> `struct console_font`:
>
> (include/uapi/linux/kd.h)
> struct console_font {
> unsigned int width, height; /* font size */
> unsigned int charcount;
> unsigned char *data;/* font data with height fixed to 32 */
> };
>
> To make things worse, `struct console_font` is part of the UAPI, so we
> cannot add a new field to keep track of `FONTDATAMAX`.
>
> Fortunately, the framebuffer layer itself gives us a hint of how to
> resolve this issue without changing UAPI. When allocating a buffer for a
> user-provided font, fbcon_set_font() reserves four "extra words" at the
> beginning of the buffer:
>
> (drivers/video/fbdev/core/fbcon.c)
> new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size, GFP_USER);
> [...]
> new_data += FONT_EXTRA_WORDS * sizeof(int);
> FNTSIZE(new_data) = size;
> FNTCHARCNT(new_data) = charcount;
> REFCOUNT(new_data) = 0; /* usage counter */
> [...]
> FNTSUM(new_data) = csum;
>
> Later, to get the size of a data buffer, the framebuffer layer simply
> calls FNTSIZE() on it:
>
> (drivers/video/fbdev/core/fbcon.h)
> /* Font */
> #define REFCOUNT(fd)(((int *)(fd))[-1])
> #define FNTSIZE(fd) (((int *)(fd))[-2])
> #define FNTCHARCNT(fd) (((int *)(fd))[-3])
> #define FNTSUM(fd) (((int *)(fd))[-4])
> #define FONT_EXTRA_WORDS 4
>
> Currently, this is only done for user-provided fonts. Let us do the same
> thing for built-in fonts, prepend these "extra words" (including
> `FONTDATAMAX`) to their data buffers, so that other subsystems, like the
> framebuffer layer, can use these macros on all fonts, no matter built-in
> or user-provided. As an example, this series fixes the syzbot issue in
> fbcon_get_font():
>
> (drivers/video/fbdev/core/fbcon.c)
> if (font->width <= 8) {
> j = vc->vc_font.height;
> + if (font->charcount * j > FNTSIZE(fontdata))
> + return -EINVAL;
> [...]
>
> Similarly, newport_con also use these macros. It only uses three of them:
>
> (drivers/video/console/newport_con.c)
> /* borrowed from fbcon.c */
> #define REFCOUNT(fd)(((int *)(fd))[-1])
> #define FNTSIZE(fd) (((int *)(fd))[-2])
> #define FNTCHARCNT(fd) (((int *)(fd))[-3])
> #define FONT_EXTRA_WORDS 3
>
> To keep things simple, move all these macro definitions to ,
> use four words instead of three, and initialize the fourth word in
> newport_set_font() properly.
>
> Many thanks to Greg Kroah-Hartman , who
> reviewed and improved this series!
>
> [1]: KASAN: global-out-of-bounds Read in fbcon_get_font
>
> https://syzkaller.appspot.com/bug?id=08b8be45afea11888776f897895aef9ad1c3ecfd
>
> Peilin Ye (3):
> fbdev, newport_con: Move FONT_EXTRA_WORDS macros into linux/font.h
> Fonts: Support FONT_EXTRA_WORDS macros for built-in fonts
> fbcon: Fix global-out-of-bounds read in fbcon_get_font()
>
> drivers/video/console/newport_con.c | 7 +--
> drivers/video/fbdev/core/fbcon.c| 12
> drivers/video/fbdev/core/fbcon.h| 7 ---
> drivers/video/fbdev/core/fbcon_rotate.c | 1 +
> drivers/video/fbdev/core/tileblit.c | 1 +
> include/linux/font.h| 13 +
> lib/fonts/font_10x18.c | 9 -
> lib/fonts/font_6x10.c | 9 +
> lib/fonts/font_6x11.c | 9 -
> lib/fonts/font_7x14.c | 9 -
> lib/fonts/font_8x16.c | 9 -
> lib/fonts/font_8x8.c| 9 -
> lib/fonts/font_acorn_8x8.c | 9 ++---
> lib/fonts/font_mini_4x6.c | 8
> lib/fonts/font_pearl_8x8.c | 9 -
> lib/fonts/font_sun12x22.c | 9 -
> lib/fonts/font_sun8x16.c| 7 ---
> lib/fonts/font_ter16x32.c | 9 -
> 18 files changed, 79 insertions(+),
Re: [PATCH 0/3] drm: commit_work scheduling
On Thu, Sep 24, 2020 at 1:49 AM Daniel Vetter wrote:
>
> On Wed, Sep 23, 2020 at 07:33:17PM -0700, Rob Clark wrote:
> > On Wed, Sep 23, 2020 at 8:25 AM Daniel Vetter wrote:
> > >
> > > On Tue, Sep 22, 2020 at 07:48:10AM -0700, Rob Clark wrote:
> > > > On Mon, Sep 21, 2020 at 11:59 PM Daniel Vetter wrote:
> > > > >
> > > > > On Mon, Sep 21, 2020 at 5:16 PM Rob Clark wrote:
> > > > > >
> > > > > > On Mon, Sep 21, 2020 at 2:21 AM Daniel Vetter
> > > > > > wrote:
> > > > > > >
> > > > > > > On Sat, Sep 19, 2020 at 12:37:23PM -0700, Rob Clark wrote:
> > > > > > > > From: Rob Clark
> > > > > > > >
> > > > > > > > The android userspace treats the display pipeline as a realtime
> > > > > > > > problem.
> > > > > > > > And arguably, if your goal is to not miss frame deadlines (ie.
> > > > > > > > vblank),
> > > > > > > > it is. (See https://lwn.net/Articles/809545/ for the best
> > > > > > > > explaination
> > > > > > > > that I found.)
> > > > > > > >
> > > > > > > > But this presents a problem with using workqueues for
> > > > > > > > non-blocking
> > > > > > > > atomic commit_work(), because the SCHED_FIFO userspace
> > > > > > > > thread(s) can
> > > > > > > > preempt the worker. Which is not really the outcome you want..
> > > > > > > > once
> > > > > > > > the required fences are scheduled, you want to push the atomic
> > > > > > > > commit
> > > > > > > > down to hw ASAP.
> > > > > > > >
> > > > > > > > But the decision of whether commit_work should be RT or not
> > > > > > > > really
> > > > > > > > depends on what userspace is doing. For a pure CFS userspace
> > > > > > > > display
> > > > > > > > pipeline, commit_work() should remain SCHED_NORMAL.
> > > > > > > >
> > > > > > > > To handle this, convert non-blocking commit_work() to use
> > > > > > > > per-CRTC
> > > > > > > > kthread workers, instead of system_unbound_wq. Per-CRTC
> > > > > > > > workers are
> > > > > > > > used to avoid serializing commits when userspace is using a
> > > > > > > > per-CRTC
> > > > > > > > update loop.
> > > > > > > >
> > > > > > > > A client-cap is introduced so that userspace can opt-in to
> > > > > > > > SCHED_FIFO
> > > > > > > > priority commit work.
> > > > > > > >
> > > > > > > > A potential issue is that since 616d91b68cd ("sched: Remove
> > > > > > > > sched_setscheduler*() EXPORTs") we have limited RT priority
> > > > > > > > levels,
> > > > > > > > meaning that commit_work() ends up running at the same priority
> > > > > > > > level
> > > > > > > > as vblank-work. This shouldn't be a big problem *yet*, due to
> > > > > > > > limited
> > > > > > > > use of vblank-work at this point. And if it could be arranged
> > > > > > > > that
> > > > > > > > vblank-work is scheduled before signaling out-fences and/or
> > > > > > > > sending
> > > > > > > > pageflip events, it could probably work ok to use a single
> > > > > > > > priority
> > > > > > > > level for both commit-work and vblank-work.
> > > > > > >
> > > > > > > The part I don't like about this is that it all feels rather
> > > > > > > hacked
> > > > > > > together, and if we add more stuff (or there's some different
> > > > > > > thing in the
> > > > > > > system that also needs rt scheduling) then it doesn't compose.
> > > > > >
> > > > > > The ideal thing would be that userspace is in control of the
> > > > > > priorities.. the setclientcap approach seemed like a reasonable way
> > > > > > to
> > > > > > give the drm-master a way to opt in.
> > > > > >
> > > > > > I suppose instead userspace could use sched_setscheduler().. but
> > > > > > that
> > > > > > would require userspace to be root, and would require some way to
> > > > > > find
> > > > > > the tid.
> > > > >
> > > > > Userspace already needs that for the SCHED_FIFO for surface-flinger.
> > > > > Or is the problem that CAP_SYS_NICE is only good for your own
> > > > > processes?
> > > >
> > > > tbh, I'm not completely sure offhand what gives surfaceflinger
> > > > permission to set itself SCHED_FIFO
> > > >
> > > > (But on CrOS there are a few more pieces to the puzzle)
> > > >
> > > > > Other question I have for this is whether there's any recommendations
> > > > > for naming the kthreads (since I guess that name is what becomes the
> > > > > uapi for userspace to control this)?
> > > > >
> > > > > Otherwise I think "userspace calls sched_setscheduler on the right
> > > > > kthreads" sounds like a good interface, since it lets userspace decide
> > > > > how it all needs to fit together and compose. Anything we hard-code in
> > > > > an ioctl is kinda lost cause. And we can choose the default values to
> > > > > work reasonably well when the compositor runs at normal priority
> > > > > (lowest niceness or something like that for the commit work).
> > > >
> > > > I don't really like the naming convention approach.. what is to stop
> > > > some unrelated process to name it's thread the same thing to get a
> > > > SCHED_FIFO boost..
> > > >
> > > > But we can stick w
Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers
Smatch has a tool to show where struct members are set. `~/smatch/smatch_data/db/smdb.py where console_font height` It's not perfect and this output comes from allmodconfig on yesterday's linux-next. regards, dan carpenter drivers/video/console/vgacon.c | vgacon_init| (struct console_font)->height | 0-32 drivers/video/console/vgacon.c | vgacon_adjust_height | (struct console_font)->height | 1-32 drivers/video/fbdev/core/fbcon.c | fbcon_startup | (struct console_font)->height | 6,8,10-11,14,16,18,22,32 drivers/video/fbdev/core/fbcon.c | fbcon_init | (struct console_font)->height | 6,8,10-11,14,16,18,22,32 drivers/video/fbdev/core/fbcon.c | fbcon_do_set_font | (struct console_font)->height | 0-u32max drivers/video/fbdev/core/fbcon.c | fbcon_set_def_font | (struct console_font)->height | 6,8,10-11,14,16,18,22,32 drivers/usb/misc/sisusbvga/sisusb_con.c | sisusbcon_init | (struct console_font)->height | 0-u32max drivers/usb/misc/sisusbvga/sisusb_con.c | sisusbcon_do_font_op | (struct console_font)->height | 1-32 drivers/tty/vt/vt_ioctl.c | vt_k_ioctl | (struct console_font)->height | ignore drivers/tty/vt/vt_ioctl.c | vt_resizex | (struct console_font)->height | 0-u32max drivers/tty/vt/vt_ioctl.c | vt_ioctl | (struct console_font)->height | ignore drivers/tty/vt/vt_ioctl.c | vt_compat_ioctl| (struct console_font)->height | ignore drivers/tty/vt/vt.c| vc_allocate| (struct console_font)->height | 0 drivers/tty/vt/vt.c| vt_resize | (struct console_font)->height | ignore drivers/tty/vt/vt.c| do_con_write | (struct console_font)->height | ignore drivers/tty/vt/vt.c| con_unthrottle | (struct console_font)->height | ignore drivers/tty/vt/vt.c| con_flush_chars| (struct console_font)->height | ignore drivers/tty/vt/vt.c| con_shutdown | (struct console_font)->height | ignore drivers/tty/vt/vt.c| con_cleanup| (struct console_font)->height | ignore drivers/tty/vt/vt.c| con_init | (struct console_font)->height | 0 drivers/tty/vt/vt.c| con_font_set | (struct console_font)->height | 1-32 drivers/tty/vt/vt.c| con_font_default | (struct console_font)->height | 0-u32max drivers/tty/vt/selection.c | paste_selection| (struct console_font)->height | ignore ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/dp: fix incorrect function prototype of dp_debug_get()
Hi Stephen
Thanks for the review.
On 2020-09-23 23:32, Stephen Boyd wrote:
Quoting Abhinav Kumar (2020-09-23 16:24:48)
Fix the incorrect function prototype for dp_debug_get()
in the dp_debug module to address compilation warning.
Fixes: f913454aae8e ("drm/msm/dp: move debugfs node to
/sys/kernel/debug/dri/*/")
Reported-by: kernel test robot
Signed-off-by: Abhinav Kumar
---
Reviewed-by: Stephen Boyd
Will the compliance testing parts be moved out of debugfs at some
point?
Just curious what the plan is there.
No, the video pattern compliance testing parts will remain in debugfs as
they use
IGT as the userspace entity which is debugfs based for DP compliance
tests.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 11/23] device-dax: Kill dax_kmem_res
On 24.09.20 15:54, Dan Williams wrote:
> On Thu, Sep 24, 2020 at 12:26 AM David Hildenbrand wrote:
>>
>> On 23.09.20 23:41, Dan Williams wrote:
>>> On Wed, Sep 23, 2020 at 1:04 AM David Hildenbrand wrote:
On 08.09.20 17:33, Joao Martins wrote:
> [Sorry for the late response]
>
> On 8/21/20 11:06 AM, David Hildenbrand wrote:
>> On 03.08.20 07:03, Dan Williams wrote:
>>> @@ -37,109 +45,94 @@ int dev_dax_kmem_probe(struct device *dev)
>>> * could be mixed in a node with faster memory, causing
>>> * unavoidable performance issues.
>>> */
>>> - numa_node = dev_dax->target_node;
>>> if (numa_node < 0) {
>>> dev_warn(dev, "rejecting DAX region with invalid node:
>>> %d\n",
>>> numa_node);
>>> return -EINVAL;
>>> }
>>>
>>> - /* Hotplug starting at the beginning of the next block: */
>>> - kmem_start = ALIGN(range->start, memory_block_size_bytes());
>>> -
>>> - kmem_size = range_len(range);
>>> - /* Adjust the size down to compensate for moving up kmem_start: */
>>> - kmem_size -= kmem_start - range->start;
>>> - /* Align the size down to cover only complete blocks: */
>>> - kmem_size &= ~(memory_block_size_bytes() - 1);
>>> - kmem_end = kmem_start + kmem_size;
>>> -
>>> - new_res_name = kstrdup(dev_name(dev), GFP_KERNEL);
>>> - if (!new_res_name)
>>> + res_name = kstrdup(dev_name(dev), GFP_KERNEL);
>>> + if (!res_name)
>>> return -ENOMEM;
>>>
>>> - /* Region is permanently reserved if hotremove fails. */
>>> - new_res = request_mem_region(kmem_start, kmem_size, new_res_name);
>>> - if (!new_res) {
>>> - dev_warn(dev, "could not reserve region [%pa-%pa]\n",
>>> -&kmem_start, &kmem_end);
>>> - kfree(new_res_name);
>>> + res = request_mem_region(range.start, range_len(&range), res_name);
>>
>> I think our range could be empty after aligning. I assume
>> request_mem_region() would check that, but maybe we could report a
>> better error/warning in that case.
>>
> dax_kmem_range() already returns a memory-block-aligned @range but
> IIUC request_mem_region() isn't checking for that. Having said that
> the returned @res wouldn't be different from the passed range.start.
>
>>> /*
>>> * Ensure that future kexec'd kernels will not treat this as RAM
>>> * automatically.
>>> */
>>> - rc = add_memory_driver_managed(numa_node, new_res->start,
>>> - resource_size(new_res), kmem_name);
>>> + rc = add_memory_driver_managed(numa_node, res->start,
>>> + resource_size(res), kmem_name);
>>> +
>>> + res->flags |= IORESOURCE_BUSY;
>>
>> Hm, I don't think that's correct. Any specific reason why to mark the
>> not-added, unaligned parts BUSY? E.g., walk_system_ram_range() could
>> suddenly stumble over it - and e.g., similarly kexec code when trying to
>> find memory for placing kexec images. I think we should leave this
>> !BUSY, just as it is right now.
>>
> Agreed.
>
>>> if (rc) {
>>> - release_resource(new_res);
>>> - kfree(new_res);
>>> - kfree(new_res_name);
>>> + release_mem_region(range.start, range_len(&range));
>>> + kfree(res_name);
>>> return rc;
>>> }
>>> - dev_dax->dax_kmem_res = new_res;
>>> +
>>> + dev_set_drvdata(dev, res_name);
>>>
>>> return 0;
>>> }
>>>
>>> #ifdef CONFIG_MEMORY_HOTREMOVE
>>> -static int dev_dax_kmem_remove(struct device *dev)
>>> +static void dax_kmem_release(struct dev_dax *dev_dax)
>>> {
>>> - struct dev_dax *dev_dax = to_dev_dax(dev);
>>> - struct resource *res = dev_dax->dax_kmem_res;
>>> - resource_size_t kmem_start = res->start;
>>> - resource_size_t kmem_size = resource_size(res);
>>> - const char *res_name = res->name;
>>> int rc;
>>> + struct device *dev = &dev_dax->dev;
>>> + const char *res_name = dev_get_drvdata(dev);
>>> + struct range range = dax_kmem_range(dev_dax);
>>>
>>> /*
>>> * We have one shot for removing memory, if some memory blocks were
>>> not
>>> * offline prior to calling this function remove_memory() will
>>> fail, and
>>> * there is no way to hotremove this memory until reboot because
>>> device
>>> -* unbind will succeed even if we return failure.
>>> +* unbind will proceed regardless of the remove_memory result.
>>> */
>>> - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size);
>>> - if (rc) {
>>> - any_hotremove_failed = true
[PATCH v2 0/6] drm/i915: Add support for LTTPR non-transparent link training mode
This patchset is v2 of [1], addressing the comments from Ville and an issue in the last patch with the per-PHY capability readout (needs to be done after switching to non-transparent mode). [1] https://patchwork.freedesktop.org/series/81968/ Cc: dri-devel@lists.freedesktop.org Cc: Ville Syrjälä Imre Deak (6): drm/i915: Fix DP link training pattern mask drm/i915: Simplify the link training functions drm/i915: Factor out a helper to disable the DPCD training pattern drm/dp: Add LTTPR helpers drm/i915: Switch to LTTPR transparent mode link training drm/i915: Switch to LTTPR non-transparent mode link training drivers/gpu/drm/drm_dp_helper.c | 244 - drivers/gpu/drm/i915/display/intel_ddi.c | 3 +- .../drm/i915/display/intel_display_types.h| 2 + drivers/gpu/drm/i915/display/intel_dp.c | 46 +- drivers/gpu/drm/i915/display/intel_dp.h | 3 - .../drm/i915/display/intel_dp_link_training.c | 486 +++--- .../drm/i915/display/intel_dp_link_training.h | 13 +- include/drm/drm_dp_helper.h | 62 +++ 8 files changed, 750 insertions(+), 109 deletions(-) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/6] drm/dp: Add LTTPR helpers
Add the helpers and register definitions needed to read out the common
and per-PHY LTTPR capabilities and perform link training in the LTTPR
non-transparent mode.
v2:
- Add drm_dp_dpcd_read_phy_link_status() and DP_PHY_LTTPR() here instead
of adding these to i915. (Ville)
Cc: dri-devel@lists.freedesktop.org
Cc: Ville Syrjälä
Signed-off-by: Imre Deak
---
drivers/gpu/drm/drm_dp_helper.c | 244 +++-
include/drm/drm_dp_helper.h | 62
2 files changed, 302 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 478dd51f738d..6014c858b06b 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -150,11 +150,8 @@ void drm_dp_link_train_clock_recovery_delay(const u8
dpcd[DP_RECEIVER_CAP_SIZE])
}
EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
-void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+static void __drm_dp_link_train_channel_eq_delay(unsigned long rd_interval)
{
- unsigned long rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
-DP_TRAINING_AUX_RD_MASK;
-
if (rd_interval > 4)
DRM_DEBUG_KMS("AUX interval %lu, out of range (max 4)\n",
rd_interval);
@@ -166,8 +163,35 @@ void drm_dp_link_train_channel_eq_delay(const u8
dpcd[DP_RECEIVER_CAP_SIZE])
usleep_range(rd_interval, rd_interval * 2);
}
+
+void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+ __drm_dp_link_train_channel_eq_delay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
+DP_TRAINING_AUX_RD_MASK);
+}
EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
+void drm_dp_lttpr_link_train_clock_recovery_delay(void)
+{
+ usleep_range(100, 200);
+}
+EXPORT_SYMBOL(drm_dp_lttpr_link_train_clock_recovery_delay);
+
+static u8 dp_lttpr_phy_cap(const u8 phy_cap[DP_LTTPR_PHY_CAP_SIZE], int r)
+{
+ return phy_cap[r - DP_TRAINING_AUX_RD_INTERVAL_PHY_REPEATER1];
+}
+
+void drm_dp_lttpr_link_train_channel_eq_delay(const u8
phy_cap[DP_LTTPR_PHY_CAP_SIZE])
+{
+ u8 interval = dp_lttpr_phy_cap(phy_cap,
+
DP_TRAINING_AUX_RD_INTERVAL_PHY_REPEATER1) &
+ DP_TRAINING_AUX_RD_MASK;
+
+ __drm_dp_link_train_channel_eq_delay(interval);
+}
+EXPORT_SYMBOL(drm_dp_lttpr_link_train_channel_eq_delay);
+
u8 drm_dp_link_rate_to_bw_code(int link_rate)
{
/* Spec says link_bw = link_rate / 0.27Gbps */
@@ -363,6 +387,71 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
}
EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
+/**
+ * drm_dp_dpcd_read_phy_link_status - get the link status information for a DP
PHY
+ * @aux: DisplayPort AUX channel
+ * @dp_phy: the DP PHY to get the link status for
+ * @link_status: buffer to return the status in
+ *
+ * Fetch the AUX DPCD registers for the DPRX or an LTTPR PHY link status. The
+ * layout of the returned @link_status matches the DPCD register layout of the
+ * DPRX PHY link status.
+ *
+ * Returns 0 if the information was read successfully or a negative error code
+ * on failure.
+ */
+int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux *aux,
+enum drm_dp_phy dp_phy,
+u8 link_status[DP_LINK_STATUS_SIZE])
+{
+ u8 lttpr_status[DP_LINK_STATUS_SIZE - 1];
+ int ret;
+
+ if (dp_phy == DP_PHY_DPRX) {
+ ret = drm_dp_dpcd_read(aux,
+ DP_LANE0_1_STATUS,
+ link_status,
+ DP_LINK_STATUS_SIZE);
+
+ if (ret < 0)
+ return ret;
+
+ WARN_ON(ret != DP_LINK_STATUS_SIZE);
+
+ return 0;
+ }
+
+ ret = drm_dp_dpcd_read(aux,
+ DP_LANE0_1_STATUS_PHY_REPEATER(dp_phy),
+ lttpr_status,
+ sizeof(lttpr_status));
+
+ if (ret < 0)
+ return ret;
+
+ WARN_ON(ret != sizeof(lttpr_status));
+
+#define link_reg(reg) link_status[(reg) - DP_LANE0_1_STATUS]
+#define lttpr_reg(reg) lttpr_status[(reg) - DP_LANE0_1_STATUS_PHY_REPEATER1]
+
+ /* Convert the LTTPR to the sink PHY link status layout */
+ link_reg(DP_LANE0_1_STATUS) =
lttpr_reg(DP_LANE0_1_STATUS_PHY_REPEATER1);
+ link_reg(DP_LANE2_3_STATUS) =
lttpr_reg(DP_LANE2_3_STATUS_PHY_REPEATER1);
+ link_reg(DP_LANE_ALIGN_STATUS_UPDATED) =
+ lttpr_reg(DP_LANE_ALIGN_STATUS_UPDATED_PHY_REPEATER1);
+ link_reg(DP_SINK_STATUS) = 0;
+ link_reg(DP_ADJUST_REQUEST_LANE0_1) =
+ lttpr_reg(DP_ADJUST_REQUEST_LANE0_1_PHY_REPEATER1);
+ link_reg(DP_ADJUST_REQUEST_LANE2_3) =
+ lttpr_reg(DP_ADJUST_REQUEST_LANE2_3_PHY_REPEATER1);
+
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, 24 Sep 2020 19:55:10 +0200 Thomas Gleixner wrote: > On Thu, Sep 24 2020 at 08:32, Steven Rostedt wrote: > > On Thu, 24 Sep 2020 08:57:52 +0200 > > Thomas Gleixner wrote: > > > >> > Now as for migration disabled nesting, at least now we would have > >> > groupings of this, and perhaps the theorists can handle that. I mean, > >> > how is this much different that having a bunch of tasks blocked on a > >> > mutex with the owner is pinned on a CPU? > >> > > >> > migrate_disable() is a BKL of pinning affinity. > >> > >> No. That's just wrong. preempt disable is a concurrency control, > > > > I think you totally misunderstood what I was saying. The above wasn't about > > comparing preempt_disable to migrate_disable. It was comparing > > migrate_disable to a chain of tasks blocked on mutexes where the top owner > > has preempt_disable set. You still have a bunch of tasks that can't move to > > other CPUs. > > What? The top owner does not prevent any task from moving. The tasks > cannot move because they are blocked on the mutex, which means they are > not runnable and non runnable tasks are not migrated at all. And neither are migrated disabled tasks preempted by a high priority task. > > I really don't understand what you are trying to say. Don't worry about it. I was just making a high level comparison of how migrate disabled tasks blocked on a higher priority task is similar to that of tasks blocked on a mutex held by a pinned task that is preempted by a high priority task. But we can forget this analogy as it's not appropriate for the current conversation. > > >> > If we only have local_lock() available (even on !RT), then it makes > >> > the blocking in groups. At least this way you could grep for all the > >> > different local_locks in the system and plug that into the algorithm > >> > for WCS, just like one would with a bunch of mutexes. > >> > >> You cannot do that on RT at all where migrate disable is substituting > >> preempt disable in spin and rw locks. The result would be the same as > >> with a !RT kernel just with horribly bad performance. > > > > Note, the spin and rwlocks already have a lock associated with them. Why > > would it be any different on RT? I wasn't suggesting adding another lock > > inside a spinlock. Why would I recommend THAT? I wasn't recommending > > blindly replacing migrate_disable() with local_lock(). I just meant expose > > local_lock() but not migrate_disable(). > > We already exposed local_lock() to non RT and it's for places which do > preempt_disable() or local_irq_disable() without having a lock > associated. But both primitives are scope less and therefore behave like > CPU local BKLs. What local_lock() provides in these cases is: > > - Making the protection scope clear by associating a named local > lock which is coverred by lockdep. > > - It still maps to preempt_disable() or local_irq_disable() in !RT > kernels > > - The scope and the named lock allows RT kernels to substitute with > real (recursion aware) locking primitives which keep preemption and > interupts enabled, but provide the fine grained protection for the > scoped critical section. I'm very much aware of the above. > > So how would you substitute migrate_disable() with a local_lock()? You > can't. Again migrate_disable() is NOT a concurrency control and > therefore it cannot be substituted by any concurrency control primitive. When I was first writing my email, I was writing about a way to replace migrate_disable with a construct similar to local locks without actually mentioning local locks, but then rewrote it to state local locks, trying to simplify what I was writing. I shouldn't have done that, because it portrayed that I wanted to use local_lock() unmodified. I was actually thinking of a new construct that was similar but not exactly the same as local lock. But this will just make things more complex and we can forget about it. I'll wait to see what Peter produces. -- Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On 9/24/20 10:27 AM, pet...@infradead.org wrote: > So my current todo list is: > > - Change RT PULL > - Change DL PULL > - Add migrate_disable() tracer; exactly like preempt/irqoff, except >measuring task-runtime instead of cpu-time. > - Add a mode that measures actual interference. > - Add a traceevent to detect preemption in migrate_disable(). > > > And then I suppose I should twist Daniel's arm to update his model to > include these scenarios and numbers. Challenge accepted :-) -- Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
Hi Guido. On Mon, Sep 21, 2020 at 06:55:52PM +0200, Guido Günther wrote: > We need to reset both for the panel to show an image. > > Signed-off-by: Guido Günther > --- > .../bindings/display/panel/mantix,mlaf057we51-x.yaml | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml > b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml > index 937323cc9aaa..ba5a18fac9f9 100644 > --- > a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml > +++ > b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml > @@ -35,7 +35,9 @@ properties: >vddi-supply: > description: 1.8V I/O voltage supply > > - reset-gpios: true > + reset-gpios: > +minItems: 2 > +maxItems: 2 reset-gpios is, as you already wrote, defined in panel-common.yaml. Do not try to change it here. It would be much better, I think, to introduce a mantix,reset-gpios property. This would avoid that we had two different reset-gpios definitions. Sam > >backlight: true > > @@ -62,7 +64,8 @@ examples: > avdd-supply = <®_avdd>; > avee-supply = <®_avee>; > vddi-supply = <®_1v8_p>; > -reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>; > +reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>, > + <&gpio1 24 GPIO_ACTIVE_LOW>; > backlight = <&backlight>; > }; > }; > -- > 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/panel: simplify the return expression of td028ttec1_prepare
Hi Qinglang
On Mon, Sep 21, 2020 at 09:10:18PM +0800, Qinglang Miao wrote:
> Simplify the return expression.
>
> Signed-off-by: Qinglang Miao
Thanks, applied to drm-misc-next.
Sam
> ---
> drivers/gpu/drm/panel/panel-tpo-td028ttec1.c | 7 +--
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c
> b/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c
> index 037c14fd6..ba0c00d1a 100644
> --- a/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c
> +++ b/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c
> @@ -242,13 +242,8 @@ static int td028ttec1_prepare(struct drm_panel *panel)
> static int td028ttec1_enable(struct drm_panel *panel)
> {
> struct td028ttec1_panel *lcd = to_td028ttec1_device(panel);
> - int ret;
>
> - ret = jbt_ret_write_0(lcd, JBT_REG_DISPLAY_ON, NULL);
> - if (ret)
> - return ret;
> -
> - return 0;
> + return jbt_ret_write_0(lcd, JBT_REG_DISPLAY_ON, NULL);
> }
>
> static int td028ttec1_disable(struct drm_panel *panel)
> --
> 2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panel: rm68200: allow using non-continuous dsi clock
Hi Yannick On Fri, Sep 18, 2020 at 01:47:36PM +0200, Yannick Fertre wrote: > The panel is able to work when dsi clock is non-continuous, thus > the system power consumption can be reduced using such feature. > > Add MIPI_DSI_CLOCK_NON_CONTINUOUS to panel's mode_flags. > > Signed-off-by: Antonio Borneo > Signed-off-by: Yannick Fertre Applied to drm-misc-next. Sam > --- > drivers/gpu/drm/panel/panel-raydium-rm68200.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-raydium-rm68200.c > b/drivers/gpu/drm/panel/panel-raydium-rm68200.c > index f908eeafb1af..2b9e48b0a491 100644 > --- a/drivers/gpu/drm/panel/panel-raydium-rm68200.c > +++ b/drivers/gpu/drm/panel/panel-raydium-rm68200.c > @@ -391,7 +391,7 @@ static int rm68200_probe(struct mipi_dsi_device *dsi) > dsi->lanes = 2; > dsi->format = MIPI_DSI_FMT_RGB888; > dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | > - MIPI_DSI_MODE_LPM; > + MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS; > > drm_panel_init(&ctx->panel, dev, &rm68200_drm_funcs, > DRM_MODE_CONNECTOR_DSI); > -- > 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panel: otm8009a: remove hack to force commands in HS
Hi Yannick
On Fri, Sep 18, 2020 at 01:47:18PM +0200, Yannick Fertre wrote:
> From: Antonio Borneo
>
> The panel is able to receive commands in LP. The current hack to
> force backlight commands in HS was due to workaround an incorrect
> settings on DSI controller that prevents sending LP commands while
> video out was active.
>
> Remove the hack that forces HS commands.
>
> Signed-off-by: Antonio Borneo
As Daniel wrote this and the next patch is missing a Signed-off-by:
So not applied, but waits for v2 - or that you get commit right and can
do it yourself.
You can add my:
Acked-by: Sam Ravnborg
> ---
> .../gpu/drm/panel/panel-orisetech-otm8009a.c | 18 ++
> 1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> index 6ac1accade80..f80b44a8a700 100644
> --- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> +++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> @@ -99,20 +99,6 @@ static void otm8009a_dcs_write_buf(struct otm8009a *ctx,
> const void *data,
> dev_warn(ctx->dev, "mipi dsi dcs write buffer failed\n");
> }
>
> -static void otm8009a_dcs_write_buf_hs(struct otm8009a *ctx, const void *data,
> - size_t len)
> -{
> - struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> -
> - /* data will be sent in dsi hs mode (ie. no lpm) */
> - dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> -
> - otm8009a_dcs_write_buf(ctx, data, len);
> -
> - /* restore back the dsi lpm mode */
> - dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> -}
> -
> #define dcs_write_seq(ctx, seq...) \
> ({ \
> static const u8 d[] = { seq }; \
> @@ -400,7 +386,7 @@ static int otm8009a_backlight_update_status(struct
> backlight_device *bd)
>*/
> data[0] = MIPI_DCS_SET_DISPLAY_BRIGHTNESS;
> data[1] = bd->props.brightness;
> - otm8009a_dcs_write_buf_hs(ctx, data, ARRAY_SIZE(data));
> + otm8009a_dcs_write_buf(ctx, data, ARRAY_SIZE(data));
>
> /* set Brightness Control & Backlight on */
> data[1] = 0x24;
> @@ -412,7 +398,7 @@ static int otm8009a_backlight_update_status(struct
> backlight_device *bd)
>
> /* Update Brightness Control & Backlight */
> data[0] = MIPI_DCS_WRITE_CONTROL_DISPLAY;
> - otm8009a_dcs_write_buf_hs(ctx, data, ARRAY_SIZE(data));
> + otm8009a_dcs_write_buf(ctx, data, ARRAY_SIZE(data));
>
> return 0;
> }
> --
> 2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panel: otm8009a: allow using non-continuous dsi clock
On Fri, Sep 18, 2020 at 01:47:26PM +0200, Yannick Fertre wrote: > From: Antonio Borneo > > The panel is able to work when dsi clock is non-continuous, thus > the system power consumption can be reduced using such feature. > > Add MIPI_DSI_CLOCK_NON_CONTINUOUS to panel's mode_flags. > > Signed-off-by: Antonio Borneo Patch looks good - but I have no clue if the panel supports it or not. Anyway: Acked-by: Sam Ravnborg > --- > drivers/gpu/drm/panel/panel-orisetech-otm8009a.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c > b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c > index b6e377aa1131..6ac1accade80 100644 > --- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c > +++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c > @@ -452,7 +452,7 @@ static int otm8009a_probe(struct mipi_dsi_device *dsi) > dsi->lanes = 2; > dsi->format = MIPI_DSI_FMT_RGB888; > dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | > - MIPI_DSI_MODE_LPM; > + MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS; > > drm_panel_init(&ctx->panel, dev, &otm8009a_drm_funcs, > DRM_MODE_CONNECTOR_DSI); > -- > 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: mx3fb: remove unused variable 'irq'
On Thu, Sep 17, 2020 at 05:19:20PM +0800, Xiaofei Tan wrote: > Remove the variable 'irq' that is set but never used. > > Signed-off-by: Xiaofei Tan Thanks, applied to drm-misc-next. Sam > --- > drivers/video/fbdev/mx3fb.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/video/fbdev/mx3fb.c b/drivers/video/fbdev/mx3fb.c > index 603731a..894617d 100644 > --- a/drivers/video/fbdev/mx3fb.c > +++ b/drivers/video/fbdev/mx3fb.c > @@ -1428,7 +1428,6 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, > struct idmac_channel *ichan) > struct device *dev = mx3fb->dev; > struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev); > const char *name = mx3fb_pdata->name; > - unsigned int irq; > struct fb_info *fbi; > struct mx3fb_info *mx3fbi; > const struct fb_videomode *mode; > @@ -1441,7 +1440,6 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, > struct idmac_channel *ichan) > } > > ichan->client = mx3fb; > - irq = ichan->eof_irq; > > if (ichan->dma_chan.chan_id != IDMAC_SDC_0) > return -EINVAL; > -- > 2.8.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/ingenic: Add support for 30-bit modes
On Tue, Sep 15, 2020 at 02:38:16PM +0200, Paul Cercueil wrote:
> Starting from the JZ4760 SoC, the primary and overlay planes support
> 30-bit pixel modes (10 bits per color component). Add support for these
> in the ingenic-drm driver.
>
> Signed-off-by: Paul Cercueil
Acked-by: Sam Ravnborg
> ---
> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 41 +--
> drivers/gpu/drm/ingenic/ingenic-drm.h | 1 +
> 2 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 937d080f5d06..fb62869befdc 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -49,6 +49,8 @@ struct jz_soc_info {
> bool needs_dev_clk;
> bool has_osd;
> unsigned int max_width, max_height;
> + const u32 *formats;
> + unsigned int num_formats;
> };
>
> struct ingenic_drm {
> @@ -73,12 +75,6 @@ struct ingenic_drm {
> bool no_vblank;
> };
>
> -static const u32 ingenic_drm_primary_formats[] = {
> - DRM_FORMAT_XRGB1555,
> - DRM_FORMAT_RGB565,
> - DRM_FORMAT_XRGB,
> -};
> -
> static bool ingenic_drm_cached_gem_buf;
> module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, bool,
> 0400);
> MODULE_PARM_DESC(cached_gem_buffers,
> @@ -411,6 +407,9 @@ void ingenic_drm_plane_config(struct device *dev,
> case DRM_FORMAT_XRGB:
> ctrl |= JZ_LCD_OSDCTRL_BPP_18_24;
> break;
> + case DRM_FORMAT_XRGB2101010:
> + ctrl |= JZ_LCD_OSDCTRL_BPP_30;
> + break;
> }
>
> regmap_update_bits(priv->map, JZ_REG_LCD_OSDCTRL,
> @@ -426,6 +425,9 @@ void ingenic_drm_plane_config(struct device *dev,
> case DRM_FORMAT_XRGB:
> ctrl |= JZ_LCD_CTRL_BPP_18_24;
> break;
> + case DRM_FORMAT_XRGB2101010:
> + ctrl |= JZ_LCD_CTRL_BPP_30;
> + break;
> }
>
> regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
> @@ -894,8 +896,8 @@ static int ingenic_drm_bind(struct device *dev, bool
> has_components)
>
> ret = drm_universal_plane_init(drm, &priv->f1, 1,
> &ingenic_drm_primary_plane_funcs,
> -ingenic_drm_primary_formats,
> -ARRAY_SIZE(ingenic_drm_primary_formats),
> +priv->soc_info->formats,
> +priv->soc_info->num_formats,
> NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> dev_err(dev, "Failed to register plane: %i\n", ret);
> @@ -919,8 +921,8 @@ static int ingenic_drm_bind(struct device *dev, bool
> has_components)
>
> ret = drm_universal_plane_init(drm, &priv->f0, 1,
> &ingenic_drm_primary_plane_funcs,
> -ingenic_drm_primary_formats,
> -
> ARRAY_SIZE(ingenic_drm_primary_formats),
> +priv->soc_info->formats,
> +priv->soc_info->num_formats,
> NULL, DRM_PLANE_TYPE_OVERLAY,
> NULL);
> if (ret) {
> @@ -1121,11 +1123,26 @@ static int ingenic_drm_remove(struct platform_device
> *pdev)
> return 0;
> }
>
> +static const u32 jz4740_formats[] = {
> + DRM_FORMAT_XRGB1555,
> + DRM_FORMAT_RGB565,
> + DRM_FORMAT_XRGB,
> +};
> +
> +static const u32 jz4770_formats[] = {
> + DRM_FORMAT_XRGB1555,
> + DRM_FORMAT_RGB565,
> + DRM_FORMAT_XRGB,
> + DRM_FORMAT_XRGB2101010,
> +};
> +
> static const struct jz_soc_info jz4740_soc_info = {
> .needs_dev_clk = true,
> .has_osd = false,
> .max_width = 800,
> .max_height = 600,
> + .formats = jz4740_formats,
> + .num_formats = ARRAY_SIZE(jz4740_formats),
> };
>
> static const struct jz_soc_info jz4725b_soc_info = {
> @@ -1133,6 +1150,8 @@ static const struct jz_soc_info jz4725b_soc_info = {
> .has_osd = true,
> .max_width = 800,
> .max_height = 600,
> + .formats = jz4740_formats,
> + .num_formats = ARRAY_SIZE(jz4740_formats),
> };
>
> static const struct jz_soc_info jz4770_soc_info = {
> @@ -1140,6 +1159,8 @@ static const struct jz_soc_info jz4770_soc_info = {
> .has_osd = true,
> .max_width = 1280,
> .max_height = 720,
> + .formats = jz4770_formats,
> + .num_formats = ARRAY_SIZE(jz4770_formats),
> };
>
> static const struct of_device_id ingenic_drm_of_match[] = {
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h
> b/dr
Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
Hi Alexandre,
On Thu, Sep 24, 2020 at 5:16 PM Alexandru Gagniuc wrote:
> + ret = regulator_enable(sii902x->cvcc12);
> + if (ret < 0) {
> + dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
> + regulator_disable(sii902x->iovcc);
> + return PTR_ERR(sii902x->cvcc12);
return ret;
>
> ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
> regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
> regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>
> - if (client->irq > 0) {
> + if (sii902x->i2c->irq > 0) {
Unrelated change.
> regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
> SII902X_HOTPLUG_EVENT);
>
> - ret = devm_request_threaded_irq(dev, client->irq, NULL,
> + ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
Unrelated change.
sii902x_interrupt,
> IRQF_ONESHOT, dev_name(dev),
> sii902x);
> @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
>
> sii902x_audio_codec_init(sii902x, dev);
>
> - i2c_set_clientdata(client, sii902x);
> + i2c_set_clientdata(sii902x->i2c, sii902x);
Unrelated change.
> - sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> + sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
Unrelated change.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/ingenic: Reset pixclock rate when parent clock rate changes
Hi Paul.
On Tue, Sep 15, 2020 at 02:38:17PM +0200, Paul Cercueil wrote:
> Old Ingenic SoCs can overclock very well, up to +50% of their nominal
> clock rate, whithout requiring overvolting or anything like that, just
> by changing the rate of the main PLL. Unfortunately, all clocks on the
> system are derived from that PLL, and when the PLL rate is updated, so
> is our pixel clock.
>
> To counter that issue, we make sure that the panel is in VBLANK before
> the rate change happens, and we will then re-set the pixel clock rate
> afterwards, once the PLL has been changed, to be as close as possible to
> the pixel rate requested by the encoder.
>
> Signed-off-by: Paul Cercueil
> ---
> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 49 ++-
> 1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index fb62869befdc..aa32660033d2 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -73,6 +74,9 @@ struct ingenic_drm {
>
> bool panel_is_sharp;
> bool no_vblank;
> + bool update_clk_rate;
> + struct mutex clk_mutex;
Please add comment about what the mutex protects.
Especially since the mutex is locked and unlocked based on a
notification.
With the comment added:
Acked-by: Sam Ravnborg
> + struct notifier_block clock_nb;
> };
>
> static bool ingenic_drm_cached_gem_buf;
> @@ -115,6 +119,29 @@ static inline struct ingenic_drm
> *drm_crtc_get_priv(struct drm_crtc *crtc)
> return container_of(crtc, struct ingenic_drm, crtc);
> }
>
> +static inline struct ingenic_drm *drm_nb_get_priv(struct notifier_block *nb)
> +{
> + return container_of(nb, struct ingenic_drm, clock_nb);
> +}
> +
> +static int ingenic_drm_update_pixclk(struct notifier_block *nb,
> + unsigned long action,
> + void *data)
> +{
> + struct ingenic_drm *priv = drm_nb_get_priv(nb);
> +
> + switch (action) {
> + case PRE_RATE_CHANGE:
> + mutex_lock(&priv->clk_mutex);
> + priv->update_clk_rate = true;
> + drm_crtc_wait_one_vblank(&priv->crtc);
> + return NOTIFY_OK;
> + default:
> + mutex_unlock(&priv->clk_mutex);
Any risk the POST_RATE_CHANGE or ABORT_RATE_CHANGE may go missing so we
fail to unlock the mutex?
I think not but wanted to make sure you had thought about it.
> + return NOTIFY_OK;
> + }
> +}
> +
> static void ingenic_drm_crtc_atomic_enable(struct drm_crtc *crtc,
> struct drm_crtc_state *state)
> {
> @@ -280,8 +307,14 @@ static void ingenic_drm_crtc_atomic_flush(struct
> drm_crtc *crtc,
>
> if (drm_atomic_crtc_needs_modeset(state)) {
> ingenic_drm_crtc_update_timings(priv, &state->mode);
> + priv->update_clk_rate = true;
> + }
>
> + if (priv->update_clk_rate) {
> + mutex_lock(&priv->clk_mutex);
> clk_set_rate(priv->pix_clk, state->adjusted_mode.clock * 1000);
> + priv->update_clk_rate = false;
> + mutex_unlock(&priv->clk_mutex);
> }
>
> if (event) {
> @@ -1046,16 +1079,28 @@ static int ingenic_drm_bind(struct device *dev, bool
> has_components)
> if (soc_info->has_osd)
> regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
>
> + mutex_init(&priv->clk_mutex);
> + priv->clock_nb.notifier_call = ingenic_drm_update_pixclk;
> +
> + parent_clk = clk_get_parent(priv->pix_clk);
> + ret = clk_notifier_register(parent_clk, &priv->clock_nb);
> + if (ret) {
> + dev_err(dev, "Unable to register clock notifier\n");
> + goto err_devclk_disable;
> + }
> +
> ret = drm_dev_register(drm, 0);
> if (ret) {
> dev_err(dev, "Failed to register DRM driver\n");
> - goto err_devclk_disable;
> + goto err_clk_notifier_unregister;
> }
>
> drm_fbdev_generic_setup(drm, 32);
>
> return 0;
>
> +err_clk_notifier_unregister:
> + clk_notifier_unregister(parent_clk, &priv->clock_nb);
> err_devclk_disable:
> if (priv->lcd_clk)
> clk_disable_unprepare(priv->lcd_clk);
> @@ -1077,7 +1122,9 @@ static int compare_of(struct device *dev, void *data)
> static void ingenic_drm_unbind(struct device *dev)
> {
> struct ingenic_drm *priv = dev_get_drvdata(dev);
> + struct clk *parent_clk = clk_get_parent(priv->pix_clk);
>
> + clk_notifier_unregister(parent_clk, &priv->clock_nb);
> if (priv->lcd_clk)
> clk_disable_unprepare(priv->lcd_clk);
> clk_disable_unprepare(priv->pix_clk);
> --
> 2.28.0
___
dri-d
Re: [PATCH 3/3] drm/ingenic: Add support for reserved memory
On Tue, Sep 15, 2020 at 02:38:18PM +0200, Paul Cercueil wrote:
> Add support for static memory reserved from Device Tree. Since we're
> using GEM buffers backed by CMA, it is interesting to have an option to
> specify the CMA area where the GEM buffers will be allocated.
>
> Signed-off-by: Paul Cercueil
Acked-by: Sam Ravnborg
> ---
> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index aa32660033d2..44b0d029095e 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -14,6 +14,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -827,6 +828,11 @@ static void ingenic_drm_unbind_all(void *d)
> component_unbind_all(priv->dev, &priv->drm);
> }
>
> +static void __maybe_unused ingenic_drm_release_rmem(void *d)
> +{
> + of_reserved_mem_device_release(d);
> +}
> +
> static int ingenic_drm_bind(struct device *dev, bool has_components)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -848,6 +854,19 @@ static int ingenic_drm_bind(struct device *dev, bool
> has_components)
> return -EINVAL;
> }
>
> + if (IS_ENABLED(CONFIG_OF_RESERVED_MEM)) {
> + ret = of_reserved_mem_device_init(dev);
> +
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(dev, ret, "Failed to get reserved
> memory\n");
> +
> + if (ret != -ENODEV) {
> + ret = devm_add_action_or_reset(dev,
> ingenic_drm_release_rmem, dev);
> + if (ret)
> + return ret;
> + }
> + }
> +
> priv = devm_drm_dev_alloc(dev, &ingenic_drm_driver_data,
> struct ingenic_drm, drm);
> if (IS_ERR(priv))
> --
> 2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: mark FRAMEBUFFER LAYER as Orphan
Hi Bartlomiej On Thu, Sep 24, 2020 at 01:25:30PM +0200, Bartlomiej Zolnierkiewicz wrote: > It has been a fun ride since 2017 but unfortunately I don't have > enough time to look after it properly anymore. Thanks for all your work on fbdev, and other stuff. I hope you have fun in the current job with whatever you do or even better manage to find something else so we can get you back. I could see that Daniel thinks we shall keep fbdev in drm-misc but we shall no longer rely on you sweeping the mailing list for all the pending patches :-( So I will likely start breaking fbdev some more (read: applying a few more fbdev patches). > > Signed-off-by: Bartlomiej Zolnierkiewicz Acked-by: Sam Ravnborg Please consider a proper entry in CREDITS too! Sam > --- > MAINTAINERS |3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > Index: b/MAINTAINERS > === > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6894,10 +6894,9 @@ F: drivers/net/wan/dlci.c > F: drivers/net/wan/sdla.c > > FRAMEBUFFER LAYER > -M: Bartlomiej Zolnierkiewicz > L: dri-devel@lists.freedesktop.org > L: linux-fb...@vger.kernel.org > -S: Maintained > +S: Orphan > Q: http://patchwork.kernel.org/project/linux-fbdev/list/ > T: git git://anongit.freedesktop.org/drm/drm-misc > F: Documentation/fb/ > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/panel: otm8009a: allow using non-continuous dsi clock
On Tue, Sep 22, 2020 at 09:42:53AM +0200, Yannick Fertre wrote: > From: Antonio Borneo > > The panel is able to work when dsi clock is non-continuous, thus > the system power consumption can be reduced using such feature. > > Add MIPI_DSI_CLOCK_NON_CONTINUOUS to panel's mode_flags. > > Changes in v2: > - Added my signed-off > > Signed-off-by: Antonio Borneo > Signed-off-by: Yannick Fertre Applied to drm-misc-next. Sam > --- > drivers/gpu/drm/panel/panel-orisetech-otm8009a.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c > b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c > index b6e377aa1131..6ac1accade80 100644 > --- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c > +++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c > @@ -452,7 +452,7 @@ static int otm8009a_probe(struct mipi_dsi_device *dsi) > dsi->lanes = 2; > dsi->format = MIPI_DSI_FMT_RGB888; > dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | > - MIPI_DSI_MODE_LPM; > + MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS; > > drm_panel_init(&ctx->panel, dev, &otm8009a_drm_funcs, > DRM_MODE_CONNECTOR_DSI); > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/panel: otm8009a: remove hack to force commands in HS
On Tue, Sep 22, 2020 at 09:42:42AM +0200, Yannick Fertre wrote:
> From: Antonio Borneo
>
> The panel is able to receive commands in LP. The current hack to
> force backlight commands in HS was due to workaround an incorrect
> settings on DSI controller that prevents sending LP commands while
> video out was active.
>
> Remove the hack that forces HS commands.
>
> Changes in v2:
> - Added my signed-off
>
> Signed-off-by: Antonio Borneo
> Signed-off-by: Yannick Fertre
Thanks, applied to drm-misc-next.
Sam
> ---
> .../gpu/drm/panel/panel-orisetech-otm8009a.c | 18 ++
> 1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> index 6ac1accade80..f80b44a8a700 100644
> --- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> +++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> @@ -99,20 +99,6 @@ static void otm8009a_dcs_write_buf(struct otm8009a *ctx,
> const void *data,
> dev_warn(ctx->dev, "mipi dsi dcs write buffer failed\n");
> }
>
> -static void otm8009a_dcs_write_buf_hs(struct otm8009a *ctx, const void *data,
> - size_t len)
> -{
> - struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> -
> - /* data will be sent in dsi hs mode (ie. no lpm) */
> - dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> -
> - otm8009a_dcs_write_buf(ctx, data, len);
> -
> - /* restore back the dsi lpm mode */
> - dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> -}
> -
> #define dcs_write_seq(ctx, seq...) \
> ({ \
> static const u8 d[] = { seq }; \
> @@ -400,7 +386,7 @@ static int otm8009a_backlight_update_status(struct
> backlight_device *bd)
>*/
> data[0] = MIPI_DCS_SET_DISPLAY_BRIGHTNESS;
> data[1] = bd->props.brightness;
> - otm8009a_dcs_write_buf_hs(ctx, data, ARRAY_SIZE(data));
> + otm8009a_dcs_write_buf(ctx, data, ARRAY_SIZE(data));
>
> /* set Brightness Control & Backlight on */
> data[1] = 0x24;
> @@ -412,7 +398,7 @@ static int otm8009a_backlight_update_status(struct
> backlight_device *bd)
>
> /* Update Brightness Control & Backlight */
> data[0] = MIPI_DCS_WRITE_CONTROL_DISPLAY;
> - otm8009a_dcs_write_buf_hs(ctx, data, ARRAY_SIZE(data));
> + otm8009a_dcs_write_buf(ctx, data, ARRAY_SIZE(data));
>
> return 0;
> }
> --
> 2.17.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: dsi: Avoid printing errors for -EPROBE_DEFER
Hi Yannick
On Fri, Sep 18, 2020 at 01:46:24PM +0200, Yannick Fertre wrote:
> Don't print error when probe deferred error is returned.
>
> Signed-off-by: Yannick Fertre
> ---
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> index 2e1f2664495d..164f79ef6269 100644
> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> @@ -419,7 +419,8 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
> *pdev)
> dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> if (IS_ERR(dsi->dsi)) {
> ret = PTR_ERR(dsi->dsi);
> - DRM_ERROR("Failed to initialize mipi dsi host: %d\n", ret);
> + if (ret != -EPROBE_DEFER)
> + DRM_ERROR("Failed to initialize mipi dsi host: %d\n",
> ret);
> goto err_dsi_probe;
Please use dev_err_probe() here.
We will loose [drm] but gain more than we loose.
Sam
> }
>
> --
> 2.17.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] fbdev: stop using compat_alloc_user_space
Hi Daniel/Arnd. On Fri, Sep 18, 2020 at 02:48:08PM +0200, Daniel Vetter wrote: > On Fri, Sep 18, 2020 at 12:08:10PM +0200, Arnd Bergmann wrote: > > The fbdev code uses compat_alloc_user_space in a few of its > > compat_ioctl handlers, which tends to be a bit more complicated > > and error-prone than calling the underlying handlers directly, > > so I would like to remove it completely. > > > > This modifies two such functions in fbdev, and removes another > > one that is completely unused. > > > > Arnd > > > > Arnd Bergmann (3): > > fbdev: simplify fb_getput_cmap() > > fbdev: sbuslib: remove unused FBIOSCURSOR32 helper > > fbdev: sbuslib: remove compat_alloc_user_space usage > > Looks all good, but we're also kinda looking for a new volunteer for > handling fbdev patches ... drm-misc commit rights, still not interested? Hi Daniel - I read the above as an a-b. And Arnd did not take the bait it seems. Hi Arnd. checkpatch complained about some whitespace, which I fixed while applying. Will push to drm-misc-next tomorrow unless I hear anything else. Sam > -Daniel > > > > > drivers/video/fbdev/core/fbmem.c | 44 +-- > > drivers/video/fbdev/sbuslib.c| 124 ++- > > 2 files changed, 90 insertions(+), 78 deletions(-) > > > > -- > > 2.27.0 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: Add Thomas as reviewer for ast, mgag200 and udl
Hi Thomas. On Tue, Sep 15, 2020 at 09:17:08AM +0200, Thomas Zimmermann wrote: > I'm adding myself as reviewer for ast, mgag200 and udl. I've already > been keeping these drivers in shape for a while. In my mind you are maintainer of these and not just reviewer. I had expected you to take the lead maintainer role and then Dave Airlie could be the second/fallback - no? Sam > > While at it I'm also setting the list and tree for ast and mgag200, > and update each driver's status to Supported. Working on these drivers > is part of my job. > > Signed-off-by: Thomas Zimmermann > --- > MAINTAINERS | 13 ++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 58ea37042d22..2baaec07cde3 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5419,7 +5419,10 @@ F: drivers/gpu/drm/aspeed/ > > DRM DRIVER FOR AST SERVER GRAPHICS CHIPS > M: Dave Airlie > -S: Odd Fixes > +R: Thomas Zimmermann > +L: dri-devel@lists.freedesktop.org > +S: Supported > +T: git git://anongit.freedesktop.org/drm/drm-misc > F: drivers/gpu/drm/ast/ > > DRM DRIVER FOR BOCHS VIRTUAL GPU > @@ -5507,7 +5510,10 @@ F: include/uapi/drm/mga_drm.h > > DRM DRIVER FOR MGA G200 GRAPHICS CHIPS > M: Dave Airlie > -S: Odd Fixes > +R: Thomas Zimmermann > +L: dri-devel@lists.freedesktop.org > +S: Supported > +T: git git://anongit.freedesktop.org/drm/drm-misc > F: drivers/gpu/drm/mgag200/ > > DRM DRIVER FOR MI0283QT > @@ -5652,8 +5658,9 @@ F: drivers/gpu/drm/panel/panel-tpo-tpg110.c > DRM DRIVER FOR USB DISPLAYLINK VIDEO ADAPTERS > M: Dave Airlie > R: Sean Paul > +R: Thomas Zimmermann > L: dri-devel@lists.freedesktop.org > -S: Odd Fixes > +S: Supported > T: git git://anongit.freedesktop.org/drm/drm-misc > F: drivers/gpu/drm/udl/ > > -- > 2.28.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 11/23] device-dax: Kill dax_kmem_res
[..] > > I'm not suggesting to busy the whole "virtio" range, just the portion > > that's about to be passed to add_memory_driver_managed(). > > I'm afraid I don't get your point. For virtio-mem: > > Before: > > 1. Create virtio0 container resource > > 2. (somewhen in the future) add_memory_driver_managed() > - Create resource (System RAM (virtio_mem)), marking it busy/driver >managed > > After: > > 1. Create virtio0 container resource > > 2. (somewhen in the future) Create resource (System RAM (virtio_mem)), >marking it busy/driver managed > 3. add_memory_driver_managed() > > Not helpful or simpler IMHO. The concern I'm trying to address is the theoretical race window and layering violation in this sequence in the kmem driver: 1/ res = request_mem_region(...); 2/ res->flags = IORESOURCE_MEM; 3/ add_memory_driver_managed(); Between 2/ and 3/ something can race and think that it owns the region. Do I think it will happen in practice, no, but it's still a pattern that deserves come cleanup. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
