Have you attended XDC 2021? Give us your feedback!
Hi, First of all, thanks organizers for such a great conference. It was smooth and, although there were some issues as it is usual in any conference, they were fixed promptly :-) I would like also to thank all of you for attending and participating either via submitting talks, watching them or joining the hallway track in IRC :-D Without you this conference won't make sense. Now it is time to gather feedback from you :-D * Do you have any comment on how was XDC 2021? - For example, have you tried the new streaming service (https://streaming.media.ccc.de/xdc2021)? Was it fine? * Do you think we can improve on something for future events? * What did you like most/least about the conference? You can reply privately to me if you wish. We will share all the gathered feedback among organizers and the X.Org Foundation board. Now that I got your attention... Did you like XDC 2021? What about organizing XDC 2023 (likely in Europe)? We know this is a decision that takes time (trigger internal discussion, looking for volunteers, budget, and a venue suitable for the event, etc). Therefore, we encourage potential interested parties to start the internal discussions now, so any question they have can be answered before we open the call for proposals for XDC 2023 next year. Please read [0] and feel free to contact me or the board for more info if needed. Thanks, Sam [0] https://www.x.org/wiki/Events/RFP/ signature.asc Description: This is a digitally signed message part
Re: [PATCH 2/2] MAINTAINERS: add Andrey as the DRM GPU scheduler maintainer
Am 17.09.21 um 18:15 schrieb Alex Deucher: Now that the scheduler is being used by more and more drivers, we need someone to maintain it. Andrey has stepped up to maintain the scheduler. Cc: Andrey Grodzovsky Cc: airl...@gmail.com Cc: daniel.vet...@ffwll.ch Signed-off-by: Alex Deucher Reviewed-by: Christian König --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 379092f34fff..9d567e66a65f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6369,6 +6369,14 @@ T: git git://anongit.freedesktop.org/drm/drm-misc F:drivers/gpu/drm/ttm/ F:include/drm/ttm/ +DRM GPU SCHEDULER +M: Andrey Grodzovsky +L: dri-devel@lists.freedesktop.org +S: Maintained +T: git git://anongit.freedesktop.org/drm/drm-misc +F: drivers/gpu/drm/scheduler/ +F: include/drm/gpu_scheduler.h + DSBR100 USB FM RADIO DRIVER M:Alexey Klimov L:linux-me...@vger.kernel.org
Re: [PATCH] drm/i915/dp: add a delay before setting panel brightness after power on
On Wed, Sep 15, 2021 at 1:47 AM Jani Nikula wrote:
>
> On Tue, 14 Sep 2021, Vasily Khoruzhick wrote:
> > On Tue, Sep 14, 2021 at 3:31 PM Jani Nikula
> > wrote:
> >>
> >> On Tue, 14 Sep 2021, Lyude Paul wrote:
> >> > On Tue, 2021-09-14 at 12:09 +0300, Jani Nikula wrote:
> >> >> On Mon, 13 Sep 2021, Vasily Khoruzhick wrote:
> >> >> > Panel in my Dell XPS 7590, that uses Intel's HDR backlight interface
> >> >> > to
> >> >> > control brightness, apparently needs a delay before setting brightness
> >> >> > after power on. Without this delay the panel does accept the setting
> >> >> > and may come up with some arbitrary brightness (sometimes it's too
> >> >> > dark,
> >> >> > sometimes it's too bright, I wasn't able to find a system).
> >> >> >
> >> >> > I don't have access to the spec, so I'm not sure if it's expected
> >> >> > behavior or a quirk for particular device.
> >> >> >
> >> >> > Delay was chosen by experiment: it works with 100ms, but fails with
> >> >> > anything lower than 75ms.
> >> >>
> >> >> Looks like we don't respect the panel delays for DPCD backlight. The
> >> >> values are used for setting up the panel power sequencer, and thus PWM
> >> >> based backlight, but we should probably use the delays in DPCD backlight
> >> >> code too.
> >> >
> >> > This makes sense to me, you're referring to the panel delays in the VBT
> >> > correct?
> >>
> >> Yes. See pps_init_delays() and intel_pps_backlight_on(). The delays
> >> aren't applied to DPCD backlight, I think it would make sense if they
> >> were.
> >
> > I guess it explains why it usually stops working after suspend.
> > Probably BIOS doesn't re-init the power sequencer on resume.
>
> The point is, the DPCD backlight isn't driven via the power sequencer,
> while the PWM pin would be.
>
> Please file a bug at [1], and attach /sys/kernel/debug/dri/0/i915_vbt as
> well as dmesg from boot with drm.debug=14 module parameter set.
Done, see https://gitlab.freedesktop.org/drm/intel/-/issues/4170
> Thanks,
> Jani.
>
>
> [1] https://gitlab.freedesktop.org/drm/intel/issues/new
>
>
>
> >
> >> BR,
> >> Jani.
> >>
> >> >
> >> >>
> >> >> BR,
> >> >> Jani.
> >> >>
> >> >>
> >> >> >
> >> >> > Signed-off-by: Vasily Khoruzhick
> >> >> > ---
> >> >> > drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 4
> >> >> > 1 file changed, 4 insertions(+)
> >> >> >
> >> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> >> >> > b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> >> >> > index 4f8337c7fd2e..c4f35e1b5870 100644
> >> >> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> >> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> >> >> > @@ -210,6 +210,10 @@ intel_dp_aux_hdr_enable_backlight(const struct
> >> >> > intel_crtc_state *crtc_state,
> >> >> >
> >> >> > ctrl = old_ctrl;
> >> >> > if (panel->backlight.edp.intel.sdr_uses_aux) {
> >> >> > + /* Wait 100ms to ensure that panel is ready otherwise
> >> >> > it
> >> >> > may not
> >> >> > +* set chosen backlight level
> >> >> > +*/
> >> >> > + msleep(100);
> >> >> > ctrl |= INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE;
> >> >> > intel_dp_aux_hdr_set_aux_backlight(conn_state, level);
> >> >> > } else {
> >> >>
> >>
> >> --
> >> Jani Nikula, Intel Open Source Graphics Center
>
> --
> Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH] drm/i915: fix blank screen booting crashes
On 18/09/2021 00:38, Matthew Brost wrote:
From: Hugh Dickins
5.15-rc1 crashes with blank screen when booting up on two ThinkPads
using i915. Bisections converge convincingly, but arrive at different
and surprising "culprits", none of them the actual culprit.
It is certainly surprising this patch crashed SNB and KBL.
How feasible would it be to make this code just not run when GuC is not
used? Given the field it adds is called ce->guc_blocked it sounds like a
natural and preferable thing to do... if possible.
netconsole (with init_netconsole() hacked to call i915_init() when
logging has started, instead of by module_init()) tells the story:
kernel BUG at drivers/gpu/drm/i915/i915_sw_fence.c:245!
with RSI: 814d408b pointing to sw_fence_dummy_notify().
I've been building with CONFIG_CC_OPTIMIZE_FOR_SIZE=y, and that
function needs to be 4-byte aligned.
v2:
(Jani Nikula)
- Change BUG_ON to WARN_ON
However in this case the code would then go on and call into a wrong
function offset which may be worse than a BUG_ON, no?
Fixes: 62eaf0ae217d ("drm/i915/guc: Support request cancellation")
Signed-off-by: Hugh Dickins
Signed-off-by: Matthew Brost
Reviewed-by: Matthew Brost
---
drivers/gpu/drm/i915/gt/intel_context.c | 1 +
drivers/gpu/drm/i915/i915_sw_fence.c| 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
b/drivers/gpu/drm/i915/gt/intel_context.c
index ff637147b1a9..f02c2202da9d 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -362,6 +362,7 @@ static int __intel_context_active(struct i915_active
*active)
return 0;
}
+__aligned(4) /* Respect the I915_SW_FENCE_MASK */
Hugh suggested __i915_sw_fence_call which I think would be the right
thing to do.
Regards,
Tvrtko
static int sw_fence_dummy_notify(struct i915_sw_fence *sf,
enum i915_sw_fence_notify state)
{
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c
b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..1217b124c1d0 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -14,8 +14,10 @@
#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
#define I915_SW_FENCE_BUG_ON(expr) BUG_ON(expr)
+#define I915_SW_FENCE_WARN_ON(expr) WARN_ON(expr)
#else
#define I915_SW_FENCE_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
+#define I915_SW_FENCE_WARN_ON(expr) BUILD_BUG_ON_INVALID(expr)
#endif
static DEFINE_SPINLOCK(i915_sw_fence_lock);
@@ -242,7 +244,7 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
const char *name,
struct lock_class_key *key)
{
- BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
+ I915_SW_FENCE_WARN_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
__init_waitqueue_head(>wait, name, key);
fence->flags = (unsigned long)fn;
Re: [PATCH 05/26] dma-buf: use new iterator in dma_resv_wait_timeout
Am 17.09.21 um 16:43 schrieb Daniel Vetter:
On Fri, Sep 17, 2021 at 02:34:52PM +0200, Christian König wrote:
This makes the function much simpler since the complex
retry logic is now handled elsewhere.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-resv.c | 68 ++
1 file changed, 10 insertions(+), 58 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 9b90bd9ac018..c7db553ab115 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -569,74 +569,26 @@ long dma_resv_wait_timeout(struct dma_resv *obj, bool
wait_all, bool intr,
unsigned long timeout)
{
long ret = timeout ? timeout : 1;
- unsigned int seq, shared_count;
+ struct dma_resv_iter cursor;
struct dma_fence *fence;
- int i;
-retry:
- shared_count = 0;
- seq = read_seqcount_begin(>seq);
rcu_read_lock();
I missed this in my previous conversion reviews, but pls move the
rcu_read_lock into the iterator. That should simplify the flow in all of
these quite a bit more, and since the iter_next_unlocked grabs a full
reference for the iteration body we really don't need that protected by
rcu.
I intentionally didn't do that because it makes it much more clear that
we are using RCU here and there is absolutely no guarantee that the
collection won't change.
But I'm fine if we go down that route instead if you think that's the
way to go.
Thanks,
Christian.
We can't toss rcu protection for dma_resv anytime soon (if ever), but we
can at least make it an implementation detail.
- i = -1;
-
- fence = dma_resv_excl_fence(obj);
- if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) {
- if (!dma_fence_get_rcu(fence))
- goto unlock_retry;
+ dma_resv_iter_begin(, obj, wait_all);
+ dma_resv_for_each_fence_unlocked(, fence) {
+ rcu_read_unlock();
- if (dma_fence_is_signaled(fence)) {
- dma_fence_put(fence);
- fence = NULL;
+ ret = dma_fence_wait_timeout(fence, intr, ret);
+ if (ret <= 0) {
+ dma_resv_iter_end();
+ return ret;
}
- } else {
- fence = NULL;
- }
-
- if (wait_all) {
- struct dma_resv_list *fobj = dma_resv_shared_list(obj);
-
- if (fobj)
- shared_count = fobj->shared_count;
-
- for (i = 0; !fence && i < shared_count; ++i) {
- struct dma_fence *lfence;
-
- lfence = rcu_dereference(fobj->shared[i]);
- if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
->flags))
- continue;
-
- if (!dma_fence_get_rcu(lfence))
- goto unlock_retry;
-
- if (dma_fence_is_signaled(lfence)) {
- dma_fence_put(lfence);
- continue;
- }
-
- fence = lfence;
- break;
- }
+ rcu_read_lock();
}
-
+ dma_resv_iter_end();
rcu_read_unlock();
- if (fence) {
- if (read_seqcount_retry(>seq, seq)) {
- dma_fence_put(fence);
- goto retry;
- }
- ret = dma_fence_wait_timeout(fence, intr, ret);
- dma_fence_put(fence);
- if (ret > 0 && wait_all && (i + 1 < shared_count))
- goto retry;
- }
return ret;
-
-unlock_retry:
- rcu_read_unlock();
- goto retry;
I think we still have the same semantics, and it's so much tidier.
With the rcu_read_unlock stuff into iterators (also applies to previous
two patches):
Reviewed-by: Daniel Vetter
}
EXPORT_SYMBOL_GPL(dma_resv_wait_timeout);
--
2.25.1
Re: [PATCH 21/26] drm: use new iterator in drm_gem_plane_helper_prepare_fb v2
Am 17.09.21 um 16:55 schrieb Daniel Vetter:
On Fri, Sep 17, 2021 at 02:35:08PM +0200, Christian König wrote:
Makes the handling a bit more complex, but avoids the use of
dma_resv_get_excl_unlocked().
v2: add missing rcu_read_lock()/unlock()
Signed-off-by: Christian König
---
drivers/gpu/drm/drm_gem_atomic_helper.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c
b/drivers/gpu/drm/drm_gem_atomic_helper.c
index e570398abd78..d8f9c6432544 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -143,6 +143,7 @@
*/
int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct
drm_plane_state *state)
{
+ struct dma_resv_iter cursor;
struct drm_gem_object *obj;
struct dma_fence *fence;
@@ -150,9 +151,18 @@ int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_st
return 0;
obj = drm_gem_fb_get_obj(state->fb, 0);
- fence = dma_resv_get_excl_unlocked(obj->resv);
- drm_atomic_set_fence_for_plane(state, fence);
+ rcu_read_lock();
+ dma_resv_iter_begin(, obj->resv, false);
+ dma_resv_for_each_fence_unlocked(, fence) {
+ rcu_read_unlock();
+ /* TODO: We only use the first write fence here */
+ drm_atomic_set_fence_for_plane(state, fence);
Yeah I wonder whether we should/need to collate them all together. But I
guesss whomever hits that first with their funny multi-plane yuv or
whatever gets to do that. Or I'm not clear on what exactly your TODO here
means?
Yeah, exactly that. Basically we have use cases where where we have more
than one fence to wait for.
The TODO is here because adding that to the atomic helper is just not my
construction site at the moment.
Regards,
Christian.
+ return 0;
+ }
+ dma_resv_iter_end();
+ rcu_read_unlock();
Imo we should do full dma_resv_lock here. atomic helpers are designed to
allow this, and it simplifies things. Also it really doesn't matter for
atomic, we should be able to do 60fps*a few planes easily :-)
-Daniel
+ drm_atomic_set_fence_for_plane(state, NULL);
return 0;
}
EXPORT_SYMBOL_GPL(drm_gem_plane_helper_prepare_fb);
--
2.25.1
Re: [Intel-gfx] [PATCH 14/19] drm/i915/oprom: Basic sanitization
On Fri, 17 Sep 2021, Lucas De Marchi wrote: > On Mon, May 17, 2021 at 02:57:33PM +0300, Jani Nikula wrote: >>On Mon, 12 Apr 2021, Matthew Auld wrote: >>> From: Anshuman Gupta >>> >>> Sanitize OPROM header, CPD signature and OPROM PCI version. >>> OPROM_HEADER, EXPANSION_ROM_HEADER and OPROM_MEU_BLOB structures >>> and PCI struct offsets are provided by GSC counterparts. >>> These are yet to be Documented in B.Spec. >>> After successful sanitization, extract VBT from opregion >>> image. >> >>So I don't understand what the point is with two consecutive patches >>where the latter rewrites a lot of the former. > > I actually wonder what's the point of this. Getting it from spi is > already the fallback and looks much more complex. Yes, it's pretty > detailed and document the format pretty well, but it still looks more > complex than the initial code. Do you see additional benefit in this > one? The commit message doesn't really explain much. Anshuman? BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm/i915: Make wa list per-gt
On 17/09/2021 18:08, Matt Roper wrote:
From: Venkata Sandeep Dhanalakota
Support for multiple GT's within a single i915 device will be arriving
soon. Since each GT may have its own fusing and require different
workarounds, we need to make the GT workaround functions and multicast
steering setup per-gt.
Cc: Tvrtko Ursulin
Cc: Daniele Ceraolo Spurio
Signed-off-by: Venkata Sandeep Dhanalakota
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/intel_gt.c| 3 +
drivers/gpu/drm/i915/gt/intel_gt_types.h | 2 +
drivers/gpu/drm/i915/gt/intel_workarounds.c | 143 +-
drivers/gpu/drm/i915/gt/intel_workarounds.h | 2 +-
.../gpu/drm/i915/gt/selftest_workarounds.c| 2 +-
drivers/gpu/drm/i915/i915_drv.c | 2 -
drivers/gpu/drm/i915/i915_drv.h | 2 -
drivers/gpu/drm/i915/i915_gem.c | 2 -
8 files changed, 81 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 55e87aff51d2..449ff6e83543 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -660,6 +660,8 @@ int intel_gt_init(struct intel_gt *gt)
if (err)
return err;
+ intel_gt_init_workarounds(gt);
+
/*
* This is just a security blanket to placate dragons.
* On some systems, we very sporadically observe that the first TLBs
@@ -767,6 +769,7 @@ void intel_gt_driver_release(struct intel_gt *gt)
if (vm) /* FIXME being called twice on error paths :( */
i915_vm_put(vm);
+ intel_wa_list_free(>wa_list);
intel_gt_pm_fini(gt);
intel_gt_fini_scratch(gt);
intel_gt_fini_buffer_pool(gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h
b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index 6fdcde64c180..ce127cae9e49 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -72,6 +72,8 @@ struct intel_gt {
struct intel_uc uc;
+ struct i915_wa_list wa_list;
+
struct intel_gt_timelines {
spinlock_t lock; /* protects active_list */
struct list_head active_list;
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index c314d4917b6b..1f0a54b383d9 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -804,7 +804,7 @@ int intel_engine_emit_ctx_wa(struct i915_request *rq)
}
static void
-gen4_gt_workarounds_init(struct drm_i915_private *i915,
+gen4_gt_workarounds_init(struct intel_gt *gt,
struct i915_wa_list *wal)
{
/* WaDisable_RenderCache_OperationalFlush:gen4,ilk */
@@ -812,29 +812,29 @@ gen4_gt_workarounds_init(struct drm_i915_private *i915,
}
static void
-g4x_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
*wal)
+g4x_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
{
- gen4_gt_workarounds_init(i915, wal);
+ gen4_gt_workarounds_init(gt, wal);
/* WaDisableRenderCachePipelinedFlush:g4x,ilk */
wa_masked_en(wal, CACHE_MODE_0, CM0_PIPELINED_RENDER_FLUSH_DISABLE);
}
static void
-ilk_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
*wal)
+ilk_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
{
- g4x_gt_workarounds_init(i915, wal);
+ g4x_gt_workarounds_init(gt, wal);
wa_masked_en(wal, _3D_CHICKEN2, _3D_CHICKEN2_WM_READ_PIPELINED);
}
static void
-snb_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
*wal)
+snb_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
{
}
static void
-ivb_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
*wal)
+ivb_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
{
/* Apply the WaDisableRHWOOptimizationForRenderHang:ivb workaround. */
wa_masked_dis(wal,
@@ -850,7 +850,7 @@ ivb_gt_workarounds_init(struct drm_i915_private *i915,
struct i915_wa_list *wal)
}
static void
-vlv_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
*wal)
+vlv_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
{
/* WaForceL3Serialization:vlv */
wa_write_clr(wal, GEN7_L3SQCREG4, L3SQ_URB_READ_CAM_MATCH_DISABLE);
@@ -863,7 +863,7 @@ vlv_gt_workarounds_init(struct drm_i915_private *i915,
struct i915_wa_list *wal)
}
static void
-hsw_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
*wal)
+hsw_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
{
/* L3 caching of data atomics doesn't work -- disable it. */
wa_write(wal, HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
@@ -878,15 +878,15 @@ hsw_gt_workarounds_init(struct drm_i915_private *i915,
struct
Re: [PATCH 0/5] lib: devres: Add managed helpers for write-combine setup
Hi Am 17.09.21 um 16:47 schrieb Hans de Goede: Hi, On 9/16/21 8:15 PM, Thomas Zimmermann wrote: Add devm_arch_phys_wc_add() and devm_arch_io_reserve_memtype_wc() for automatic cleanup of writecombine setup. Several DRM drivers use the non-managed functions for setting their framebuffer memory to write-combine access. Convert ast, mgag200 and vboxvideo. Remove rsp clean-up code form drivers. Tested on mgag200 hardware. Thomas Zimmermann (5): lib: devres: Add managed arch_phys_wc_add() lib: devres: Add managed arch_io_reserve_memtype_wc() drm/ast: Use managed interfaces for framebuffer write combining drm/mgag200: Use managed interfaces for framebuffer write combining drm/vboxvideo: Use managed interfaces for framebuffer write combining Thanks, the entire series looks good to me: Reviewed-by: Hans de Goede For the series. Thanks. Let me know if I can review some DRM code for you, as Daniel suggested. I'll wait a bit before merging the patches. Maybe devres devs want to comment. Best regards Thomas Regards, Hans -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] drm/v3d: Make use of the helper function devm_platform_ioremap_resource_byname()
On 09/01, Cai Huoqing wrote:
> Use the devm_platform_ioremap_resource_byname() helper instead of
> calling platform_get_resource_byname() and devm_ioremap_resource()
> separately
>
> Signed-off-by: Cai Huoqing
> ---
> drivers/gpu/drm/v3d/v3d_drv.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
> index 9403c3b36aca..c1deab2cf38d 100644
> --- a/drivers/gpu/drm/v3d/v3d_drv.c
> +++ b/drivers/gpu/drm/v3d/v3d_drv.c
> @@ -206,10 +206,7 @@ MODULE_DEVICE_TABLE(of, v3d_of_match);
> static int
> map_regs(struct v3d_dev *v3d, void __iomem **regs, const char *name)
> {
> - struct resource *res =
> - platform_get_resource_byname(v3d_to_pdev(v3d), IORESOURCE_MEM,
> name);
> -
> - *regs = devm_ioremap_resource(v3d->drm.dev, res);
> + *regs = devm_platform_ioremap_resource_byname(v3d_to_pdev(v3d), name);
> return PTR_ERR_OR_ZERO(*regs);
> }
lgtm.
Reviewed-by: Melissa Wen
>
> --
> 2.25.1
>
signature.asc
Description: PGP signature
Re: [PATCH] drm/amdgpu: use generic fb helpers instead of setting up AMD own's.
(cc'ing dri-devel)
Hi
Am 13.09.21 um 16:36 schrieb Alex Deucher:
On Thu, Sep 9, 2021 at 11:25 PM Evan Quan wrote:
With the shadow buffer support from generic framebuffer emulation, it's
possible now to have runpm kicked when no update for console.
Change-Id: I285472c9100ee6f649d3f3f3548f402b9cd34eaf
Signed-off-by: Evan Quan
Acked-by: Christian König
Reviewed-by: Alex Deucher
There was a long discussion about this change within radeon and the
result was that it cannot be done. [1] I don't remember the full
details, but semantics of the vmap/vunmap for dma-bufs were not
compatible IIRC. And the resolution was a redesign of the API.
If that has changed, I'd be happy to see this patch merged. Otherwise,
it should better not be taken.
Best regards
Thomas
[1] https://patchwork.freedesktop.org/patch/400054/?series=83765=1
--
v1->v2:
- rename amdgpu_align_pitch as amdgpu_gem_align_pitch to align with
other APIs from the same file (Alex)
---
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 11 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 13 +
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 388
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 30 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 20 -
7 files changed, 50 insertions(+), 426 deletions(-)
delete mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 8d0748184a14..73a2151ee43f 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -45,7 +45,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
amdgpu_atombios.o atombios_crtc.o amdgpu_connectors.o \
atom.o amdgpu_fence.o amdgpu_ttm.o amdgpu_object.o amdgpu_gart.o \
amdgpu_encoders.o amdgpu_display.o amdgpu_i2c.o \
- amdgpu_fb.o amdgpu_gem.o amdgpu_ring.o \
+ amdgpu_gem.o amdgpu_ring.o \
amdgpu_cs.o amdgpu_bios.o amdgpu_benchmark.o amdgpu_test.o \
atombios_dp.o amdgpu_afmt.o amdgpu_trace_points.o \
atombios_encoders.o amdgpu_sa.o atombios_i2c.o \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 682d459e992a..bcc308b7f826 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3695,8 +3695,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
/* Get a log2 for easy divisions. */
adev->mm_stats.log2_max_MBps = ilog2(max(1u, max_MBps));
- amdgpu_fbdev_init(adev);
-
r = amdgpu_pm_sysfs_init(adev);
if (r) {
adev->pm_sysfs_en = false;
@@ -3854,8 +3852,6 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
amdgpu_ucode_sysfs_fini(adev);
sysfs_remove_files(>dev->kobj, amdgpu_dev_attributes);
- amdgpu_fbdev_fini(adev);
-
amdgpu_irq_fini_hw(adev);
amdgpu_device_ip_fini_early(adev);
@@ -3931,7 +3927,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool
fbcon)
drm_kms_helper_poll_disable(dev);
if (fbcon)
- amdgpu_fbdev_set_suspend(adev, 1);
+
drm_fb_helper_set_suspend_unlocked(adev_to_drm(adev)->fb_helper, true);
cancel_delayed_work_sync(>delayed_init_work);
@@ -4009,7 +4005,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool
fbcon)
flush_delayed_work(>delayed_init_work);
if (fbcon)
- amdgpu_fbdev_set_suspend(adev, 0);
+
drm_fb_helper_set_suspend_unlocked(adev_to_drm(adev)->fb_helper, false);
drm_kms_helper_poll_enable(dev);
@@ -4638,7 +4634,7 @@ int amdgpu_do_asic_reset(struct list_head
*device_list_handle,
if (r)
goto out;
- amdgpu_fbdev_set_suspend(tmp_adev, 0);
+
drm_fb_helper_set_suspend_unlocked(adev_to_drm(tmp_adev)->fb_helper, false);
/*
* The GPU enters bad state once faulty pages
@@ -5025,7 +5021,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
*/
amdgpu_unregister_gpu_instance(tmp_adev);
- amdgpu_fbdev_set_suspend(tmp_adev, 1);
+
drm_fb_helper_set_suspend_unlocked(adev_to_drm(adev)->fb_helper, true);
/* disable ras on ALL IPs */
if (!need_emergency_restart &&
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 7a7316731911..58bfc7f00d76 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1572,13 +1572,10 @@ int amdgpu_display_suspend_helper(struct amdgpu_device
Re: [Intel-gfx] [PATCH 01/26] dma-buf: add dma_resv_for_each_fence_unlocked v2
On 17/09/2021 14:23, Daniel Vetter wrote:
On Fri, Sep 17, 2021 at 02:34:48PM +0200, Christian König wrote:
Abstract the complexity of iterating over all the fences
in a dma_resv object.
The new loop handles the whole RCU and retry dance and
returns only fences where we can be sure we grabbed the
right one.
v2: fix accessing the shared fences while they might be freed,
improve kerneldoc, rename _cursor to _iter, add
dma_resv_iter_is_exclusive, add dma_resv_iter_begin/end
Signed-off-by: Christian König
---
drivers/dma-buf/dma-resv.c | 61 +++
include/linux/dma-resv.h | 84 ++
2 files changed, 145 insertions(+)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 84fbe60629e3..3e77cad2c9d4 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -323,6 +323,67 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct
dma_fence *fence)
}
EXPORT_SYMBOL(dma_resv_add_excl_fence);
+/**
+ * dma_resv_iter_walk_unlocked - walk over fences in a dma_resv obj
+ * @cursor: cursor to record the current position
+ * @first: if we should start over
+ *
+ * Return all the fences in the dma_resv object which are not yet signaled.
+ * The returned fence has an extra local reference so will stay alive.
+ * If a concurrent modify is detected the whole iterration is started over
again.
+ */
+struct dma_fence *dma_resv_iter_walk_unlocked(struct dma_resv_iter *cursor,
Bit ocd, but I'd still just call that iter_next.
+ bool first)
Hm I'd put all the init code into iter_begin ...
@Christian:
Could you engineer something in here which would, at least in debug
builds, catch failures to call "iter begin" before using the iterator macro?
+{
+ struct dma_resv *obj = cursor->obj;
Aren't we missing rcu_read_lock() around the entire thing here?
+
+ first |= read_seqcount_retry(>seq, cursor->seq);
+ do {
+ /* Drop the reference from the previous round */
+ dma_fence_put(cursor->fence);
+
+ cursor->is_first = first;
+ if (first) {
+ cursor->seq = read_seqcount_begin(>seq);
+ cursor->index = -1;
+ cursor->fences = dma_resv_shared_list(obj);
And then also call iter_begin from here. That way we guarantee that
read_seqcount_begin is always called before _retry(). It's not a problem
with the seqcount implementation (I think at least), but it definitely
looks funny.
Calling iter_begin here also makes it clear that we're essentially
restarting.
+
+ cursor->fence = dma_resv_excl_fence(obj);
+ if (cursor->fence &&
+ test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
Please use the right dma_fence wrapper here for this and don't look at the
bits/flags outside of dma_fence.[hc] code. I just realized that we don't
have the right amount of barriers in there for the fastpath, i.e. if we
have:
x = 0; /* static initializer */
thread a
x = 1;
dma_fence_signal(fence);
thread b;
if (dma_fence_is_signalled(fence))
printk("%i\n", x);
Then you might actually be able to observe x == 0 in thread b. Which is
not what we want at all.
@Daniel:
What do you mean here - in terms of if 'x' is "external" (not part of
dma-fence), then are you suggesting dma-fence code should serialise it
by using barriers?
That would sound incorrect to me, or in other words, I think it's fine
if x == 0 is observed in your example thread B since that code is mixing
external data with dma-fence.
Hm also, there is that annoying bit where by using dma_fence_is_signaled
any code becomes a fence signaling critical path, which I never bought
into. There should be a way to test the signaled status without actually
doing the signaling. Or I am misunderstanding something so badly that is
really really has to be like this?
So no open-coding of dma_fence flag bits code outside of drm_fence.[hc]
please. And yes i915-gem code is unfortunately a disaster.
Don't even miss an opportunity for some good trashing no? :D
But yes, deconstructed dma_fence_signal I thought we were supposed to
add to core. Or at least propose, don't exactly remember how that went.
+>fence->flags))
+ cursor->fence = NULL;
+ } else {
+ cursor->fence = NULL;
+ }
+
+ if (cursor->fence) {
+ cursor->fence = dma_fence_get_rcu(cursor->fence);
+ } else if (cursor->all_fences && cursor->fences) {
+ struct dma_resv_list *fences = cursor->fences;
+
+ while (++cursor->index < fences->shared_count) {
+ cursor->fence = rcu_dereference(
+
Re: [PATCH] drm/amdgpu: use generic fb helpers instead of setting up AMD own's.
Hi
Am 20.09.21 um 10:41 schrieb Thomas Zimmermann:
(cc'ing dri-devel)
Hi
Am 13.09.21 um 16:36 schrieb Alex Deucher:
On Thu, Sep 9, 2021 at 11:25 PM Evan Quan wrote:
With the shadow buffer support from generic framebuffer emulation, it's
possible now to have runpm kicked when no update for console.
Change-Id: I285472c9100ee6f649d3f3f3548f402b9cd34eaf
Signed-off-by: Evan Quan
Acked-by: Christian König
Reviewed-by: Alex Deucher
There was a long discussion about this change within radeon and the
result was that it cannot be done. [1] I don't remember the full
details, but semantics of the vmap/vunmap for dma-bufs were not
compatible IIRC. And the resolution was a redesign of the API.
I posted a patchset with a new interface at [1].
Best regards
Thomas
[1]
https://lore.kernel.org/dri-devel/20201209142527.26415-1-tzimmerm...@suse.de/
If that has changed, I'd be happy to see this patch merged. Otherwise,
it should better not be taken.
Best regards
Thomas
[1] https://patchwork.freedesktop.org/patch/400054/?series=83765=1
--
v1->v2:
- rename amdgpu_align_pitch as amdgpu_gem_align_pitch to align with
other APIs from the same file (Alex)
---
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 11 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 13 +
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 388
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 30 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 20 -
7 files changed, 50 insertions(+), 426 deletions(-)
delete mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 8d0748184a14..73a2151ee43f 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -45,7 +45,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
amdgpu_atombios.o atombios_crtc.o amdgpu_connectors.o \
atom.o amdgpu_fence.o amdgpu_ttm.o amdgpu_object.o
amdgpu_gart.o \
amdgpu_encoders.o amdgpu_display.o amdgpu_i2c.o \
- amdgpu_fb.o amdgpu_gem.o amdgpu_ring.o \
+ amdgpu_gem.o amdgpu_ring.o \
amdgpu_cs.o amdgpu_bios.o amdgpu_benchmark.o amdgpu_test.o \
atombios_dp.o amdgpu_afmt.o amdgpu_trace_points.o \
atombios_encoders.o amdgpu_sa.o atombios_i2c.o \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 682d459e992a..bcc308b7f826 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3695,8 +3695,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
/* Get a log2 for easy divisions. */
adev->mm_stats.log2_max_MBps = ilog2(max(1u, max_MBps));
- amdgpu_fbdev_init(adev);
-
r = amdgpu_pm_sysfs_init(adev);
if (r) {
adev->pm_sysfs_en = false;
@@ -3854,8 +3852,6 @@ void amdgpu_device_fini_hw(struct amdgpu_device
*adev)
amdgpu_ucode_sysfs_fini(adev);
sysfs_remove_files(>dev->kobj, amdgpu_dev_attributes);
- amdgpu_fbdev_fini(adev);
-
amdgpu_irq_fini_hw(adev);
amdgpu_device_ip_fini_early(adev);
@@ -3931,7 +3927,7 @@ int amdgpu_device_suspend(struct drm_device
*dev, bool fbcon)
drm_kms_helper_poll_disable(dev);
if (fbcon)
- amdgpu_fbdev_set_suspend(adev, 1);
+
drm_fb_helper_set_suspend_unlocked(adev_to_drm(adev)->fb_helper, true);
cancel_delayed_work_sync(>delayed_init_work);
@@ -4009,7 +4005,7 @@ int amdgpu_device_resume(struct drm_device
*dev, bool fbcon)
flush_delayed_work(>delayed_init_work);
if (fbcon)
- amdgpu_fbdev_set_suspend(adev, 0);
+
drm_fb_helper_set_suspend_unlocked(adev_to_drm(adev)->fb_helper, false);
drm_kms_helper_poll_enable(dev);
@@ -4638,7 +4634,7 @@ int amdgpu_do_asic_reset(struct list_head
*device_list_handle,
if (r)
goto out;
- amdgpu_fbdev_set_suspend(tmp_adev, 0);
+
drm_fb_helper_set_suspend_unlocked(adev_to_drm(tmp_adev)->fb_helper,
false);
/*
* The GPU enters bad state once
faulty pages
@@ -5025,7 +5021,7 @@ int amdgpu_device_gpu_recover(struct
amdgpu_device *adev,
*/
amdgpu_unregister_gpu_instance(tmp_adev);
- amdgpu_fbdev_set_suspend(tmp_adev, 1);
+
drm_fb_helper_set_suspend_unlocked(adev_to_drm(adev)->fb_helper, true);
/* disable ras on ALL IPs */
if (!need_emergency_restart &&
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
[PATCH v2 2/6] dt-bindings: mediatek,dp: Add Display Port binding
This controller is present on several mediatek hardware. Currently
mt8195 and mt8395 have this controller without a functional difference,
so only one compatible field is added.
The controller can have two forms, as a normal display port and as an
embedded display port.
Signed-off-by: Markus Schneider-Pargmann
---
.../display/mediatek/mediatek,dp.yaml | 89 +++
1 file changed, 89 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
diff --git
a/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
new file mode 100644
index ..f7a35962c23b
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,dp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Display Port Controller
+
+maintainers:
+ - CK Hu
+ - Jitao shi
+
+description: |
+ Device tree bindings for the Mediatek (embedded) Display Port controller
+ present on some Mediatek SoCs.
+
+properties:
+ compatible:
+enum:
+ - mediatek,mt8195-edp_tx
+ - mediatek,mt8195-dp_tx
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+items:
+ - description: faxi clock
+
+ clock-names:
+items:
+ - const: faxi
+
+ power-domains:
+maxItems: 1
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description: Input endpoint of the controller, usually dp_intf
+
+ port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description: Output endpoint of the controller
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+#include
+dp_tx: edp_tx@1c50 {
+compatible = "mediatek,mt8195-edp_tx";
+reg = <0 0x1c50 0 0x8000>;
+interrupts = ;
+power-domains = < MT8195_POWER_DOMAIN_EPD_TX>;
+pinctrl-names = "default";
+pinctrl-0 = <_pin>;
+status = "okay";
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+edp_in: endpoint {
+remote-endpoint = <_intf0_out>;
+};
+};
+port@1 {
+reg = <1>;
+edp_out: endpoint {
+ remote-endpoint = <_in>;
+};
+};
+};
+};
--
2.33.0
[PATCH v2 0/6] drm/mediatek: Add mt8195 DisplayPort driver
Hi everyone, this series is built around the DisplayPort driver. The dpi/dpintf driver and the added helper functions are required for the DisplayPort driver to work. For v2 I rebased the series on top of v5.15-rc1. There still is a functional dependency on many different patches pulled in through the main two dependencies, vdosys0 and vdosys1, but I wasn't able to get a clean base with these patches yet so to continue improving this series in the meantime, I decided to go with v5.15-rc1 as a base. The main potential merge-conflict points of this series with other series are mtk_drm_drv.c and mtk_drm_ddp_comp.* etc. which should be easy to resolve later on. Note: This patch series is currently tested on v5.10 and I am still working on testing it on v5.15. Dependencies: - Add Mediatek Soc DRM (vdosys0) support for mt8195 https://lore.kernel.org/linux-mediatek/20210825144833.7757-1-jason-jh@mediatek.com/ - Add MediaTek SoC DRM (vdosys1) support for mt8195 https://lore.kernel.org/linux-mediatek/20210825100531.5653-1-nancy@mediatek.com/ Older revisions: RFC - https://lore.kernel.org/linux-mediatek/20210816192523.1739365-1-...@baylibre.com/ v1 - https://lore.kernel.org/linux-mediatek/20210906193529.718845-1-...@baylibre.com/ Thanks in advance for any feedback and comments. Best, Markus Markus Schneider-Pargmann (6): dt-bindings: mediatek,dpintf: Add DP_INTF binding dt-bindings: mediatek,dp: Add Display Port binding drm/edid: Add cea_sad helpers for freq/length video/hdmi: Add audio_infoframe packing for DP drm/mediatek: dpi: Add dpintf support drm/mediatek: Add mt8195 DisplayPort driver .../display/mediatek/mediatek,dp.yaml | 89 + .../display/mediatek/mediatek,dpintf.yaml | 78 + drivers/gpu/drm/drm_edid.c| 74 + drivers/gpu/drm/mediatek/Kconfig |7 + drivers/gpu/drm/mediatek/Makefile |2 + drivers/gpu/drm/mediatek/mtk_dp.c | 2855 + drivers/gpu/drm/mediatek/mtk_dp_reg.h | 498 +++ drivers/gpu/drm/mediatek/mtk_dpi.c| 248 +- drivers/gpu/drm/mediatek/mtk_dpi_regs.h | 12 + drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |4 + drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |1 + drivers/gpu/drm/mediatek/mtk_drm_drv.c|6 +- drivers/gpu/drm/mediatek/mtk_drm_drv.h|1 + drivers/phy/mediatek/Kconfig |8 + drivers/phy/mediatek/Makefile |1 + drivers/phy/mediatek/phy-mtk-dp.c | 218 ++ drivers/video/hdmi.c | 83 +- include/drm/drm_dp_helper.h |2 + include/drm/drm_edid.h| 18 +- include/linux/hdmi.h |7 +- include/linux/soc/mediatek/mtk-mmsys.h|2 + 21 files changed, 4140 insertions(+), 74 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dpintf.yaml create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h create mode 100644 drivers/phy/mediatek/phy-mtk-dp.c -- 2.33.0
[PATCH v2 1/6] dt-bindings: mediatek,dpintf: Add DP_INTF binding
DP_INTF is a similar functional block to mediatek,dpi but is different
in that it serves the DisplayPort controller on mediatek SoCs and uses
different clocks. Therefore this patch creates a new binding file for
this functional block.
Signed-off-by: Markus Schneider-Pargmann
---
Notes:
Changes v1 -> v2:
- Move the devicetree binding from mediatek,dpi into its own binding file.
.../display/mediatek/mediatek,dpintf.yaml | 78 +++
1 file changed, 78 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/mediatek/mediatek,dpintf.yaml
diff --git
a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpintf.yaml
b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpintf.yaml
new file mode 100644
index ..ac1fd93327e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpintf.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,dpintf.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek DP_INTF Controller Device Tree Bindings
+
+maintainers:
+ - CK Hu
+ - Jitao shi
+
+description: |
+ The Mediatek DP_INTF function block is a sink of the display subsystem
+ connected to the display port controller.
+
+properties:
+ compatible:
+enum:
+ - mediatek,mt8195-dpintf
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+items:
+ - description: hf_fmm Clock
+ - description: hf_fdp Clock
+ - description: Pixel Clock
+ - description: DP_INTF PLL
+
+ clock-names:
+items:
+ - const: hf_fmm
+ - const: hf_fdp
+ - const: pixel
+ - const: pll
+
+ port:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+ Output port node. This port should be connected to the input port of an
+ attached display port controller.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - port
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+#include
+#include
+
+ dp_intf1: dp_intf1@1c113000 {
+ compatible = "mediatek,mt8195-dpintf";
+ reg = <0 0x1c113000 0 0x1000>;
+ interrupts = ;
+ clocks = < CLK_VDO1_DP_INTF0_MM>,
+< CLK_VDO1_DPINTF>,
+< CLK_TOP_DP_SEL>,
+< CLK_TOP_TVDPLL2>;
+ clock-names = "hf_fmm",
+ "hf_fdp",
+ "pixel",
+ "pll";
+ };
+
+...
--
2.33.0
RE: [Intel-gfx] [PATCH 14/19] drm/i915/oprom: Basic sanitization
On Mon, 20 Sep 2021, "Gupta, Anshuman" wrote: >> -Original Message- >> From: Nikula, Jani >> Sent: Monday, September 20, 2021 1:12 PM >> To: De Marchi, Lucas >> Cc: Auld, Matthew ; intel-...@lists.freedesktop.org; >> dri-devel@lists.freedesktop.org; Gupta, Anshuman >> >> Subject: Re: [Intel-gfx] [PATCH 14/19] drm/i915/oprom: Basic sanitization >> >> On Fri, 17 Sep 2021, Lucas De Marchi wrote: >> > On Mon, May 17, 2021 at 02:57:33PM +0300, Jani Nikula wrote: >> >>On Mon, 12 Apr 2021, Matthew Auld wrote: >> >>> From: Anshuman Gupta >> >>> >> >>> Sanitize OPROM header, CPD signature and OPROM PCI version. >> >>> OPROM_HEADER, EXPANSION_ROM_HEADER and OPROM_MEU_BLOB >> structures and >> >>> PCI struct offsets are provided by GSC counterparts. >> >>> These are yet to be Documented in B.Spec. >> >>> After successful sanitization, extract VBT from opregion image. >> >> >> >>So I don't understand what the point is with two consecutive patches >> >>where the latter rewrites a lot of the former. >> > >> > I actually wonder what's the point of this. Getting it from spi is >> > already the fallback and looks much more complex. Yes, it's pretty >> > detailed and document the format pretty well, but it still looks more >> > complex than the initial code. Do you see additional benefit in this >> > one? > Getting opregion image from spi is needed to get the intel_opregion and its > mailboxes on discrete card. I mean what's the point of the "drm/i915/oprom: Basic sanitization" patch? And if that's needed, then why is it separate from "drm/i915/dg1: Read OPROM via SPI controller"? >> The commit message doesn't really explain much. Anshuman? > I will get rework of the patches and float it again. Lucas already sent something, please sync with him. BR, Jani. > Thanks, > Anshuman Gupta. >> >> BR, >> Jani. >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center
[PATCH v2 6/6] drm/mediatek: Add mt8195 DisplayPort driver
This patch adds a DisplayPort driver for the Mediatek mt8195 SoC and a
according phy driver mediatek-dp-phy.
It supports both functional units on the mt8195, the embedded
DisplayPort as well as the external DisplayPort units. It offers
hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
to 4 lanes.
The driver creates a child device for the phy. The child device will
never exist without the parent being active. As they are sharing a
register range, the parent passes a regmap pointer to the child so that
both can work with the same register range. The phy driver sets device
data that is read by the parent to get the phy device that can be used
to control the phy properties.
This driver is based on an initial version by
Jason-JH.Lin .
Signed-off-by: Markus Schneider-Pargmann
---
Notes:
Changes v1 -> v2:
- Fix checkpatch --strict suggestions
- General cleanups of the code.
- Remove all remaining non-atomic functions.
- Remove unused includes and sort them.
- Remove unused select GENERIC_PHY
- Rename phy registers DP_PHY -> MTK_DP_PHY
- Replace usage of delays with usleep_range.
- Split the phy register accesses into a separate phy driver.
- Use a lock to guard access to mtk_dp->edid as it can be
allocated/used/freed
in different threads
- use struct dp_sdp for sdp packets.
Changes RFC -> v1:
- Removed unused register definitions.
- Replaced workqueue with threaded irq.
- Removed connector code.
- Move to atomic_* drm functions.
- General cleanups of the code.
- Remove unused select GENERIC_PHY.
drivers/gpu/drm/mediatek/Kconfig |7 +
drivers/gpu/drm/mediatek/Makefile |2 +
drivers/gpu/drm/mediatek/mtk_dp.c | 2855
drivers/gpu/drm/mediatek/mtk_dp_reg.h | 498 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
drivers/phy/mediatek/Kconfig |8 +
drivers/phy/mediatek/Makefile |1 +
drivers/phy/mediatek/phy-mtk-dp.c | 218 ++
include/linux/soc/mediatek/mtk-mmsys.h |2 +
10 files changed, 3593 insertions(+)
create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
create mode 100644 drivers/phy/mediatek/phy-mtk-dp.c
diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index 2976d21e9a34..029b94c71613 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -28,3 +28,10 @@ config DRM_MEDIATEK_HDMI
select PHY_MTK_HDMI
help
DRM/KMS HDMI driver for Mediatek SoCs
+
+config MTK_DPTX_SUPPORT
+ tristate "DRM DPTX Support for Mediatek SoCs"
+ depends on DRM_MEDIATEK
+ select PHY_MTK_DP
+ help
+ DRM/KMS Display Port driver for Mediatek SoCs.
diff --git a/drivers/gpu/drm/mediatek/Makefile
b/drivers/gpu/drm/mediatek/Makefile
index 29098d7c8307..d86a6406055e 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -21,3 +21,5 @@ mediatek-drm-hdmi-objs := mtk_cec.o \
mtk_hdmi_ddc.o
obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
+
+obj-$(CONFIG_MTK_DPTX_SUPPORT) += mtk_dp.o
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
b/drivers/gpu/drm/mediatek/mtk_dp.c
new file mode 100644
index ..5d95ff68b0df
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -0,0 +1,2855 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 MediaTek Inc.
+ * Copyright (c) 2021 BayLibre
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "mtk_dp_reg.h"
+
+#define MTK_DP_AUX_WAIT_REPLY_COUNT2
+#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT3
+
+#define MTK_DP_MAX_LANES 4
+#define MTK_DP_MAX_LINK_RATE MTK_DP_LINKRATE_HBR3
+
+#define MTK_DP_TBC_BUF_READ_START_ADDR 0x08
+
+#define MTK_DP_TRAIN_RETRY_LIMIT 8
+#define MTK_DP_TRAIN_MAX_ITERATIONS5
+
+#define MTK_DP_AUX_WRITE_READ_WAIT_TIME_US 20
+
+#define MTK_DP_DP_VERSION_11 0x11
+
+enum mtk_dp_state {
+ MTK_DP_STATE_INITIAL,
+ MTK_DP_STATE_IDLE,
+ MTK_DP_STATE_PREPARE,
+ MTK_DP_STATE_NORMAL,
+};
+
+enum mtk_dp_train_state {
+ MTK_DP_TRAIN_STATE_STARTUP = 0,
+ MTK_DP_TRAIN_STATE_CHECKCAP,
+ MTK_DP_TRAIN_STATE_CHECKEDID,
+ MTK_DP_TRAIN_STATE_TRAINING_PRE,
+ MTK_DP_TRAIN_STATE_TRAINING,
+ MTK_DP_TRAIN_STATE_CHECKTIMING,
+ MTK_DP_TRAIN_STATE_NORMAL,
+ MTK_DP_TRAIN_STATE_POWERSAVE,
+ MTK_DP_TRAIN_STATE_DPIDLE,
+};
+
+struct mtk_dp_timings {
+ struct videomode vm;
+
+ u16 htotal;
+
[PATCH v2 3/6] drm/edid: Add cea_sad helpers for freq/length
This patch adds two helper functions that extract the frequency and word
length from a struct cea_sad.
For these helper functions new defines are added that help translate the
'freq' and 'byte2' fields into real numbers.
Signed-off-by: Markus Schneider-Pargmann
---
Notes:
Changes v1 -> v2:
- Use const struct pointers.
- Add a check whether the format is actually uncompressed or not.
drivers/gpu/drm/drm_edid.c | 74 ++
include/drm/drm_edid.h | 18 --
2 files changed, 90 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6325877c5fd6..28df422fbc03 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4666,6 +4666,80 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8
**sadb)
}
EXPORT_SYMBOL(drm_edid_to_speaker_allocation);
+/**
+ * drm_cea_sad_get_sample_rate - Extract the sample rate from cea_sad
+ * @sad: Pointer to the cea_sad struct
+ *
+ * Extracts the cea_sad frequency field and returns the sample rate in Hz.
+ *
+ * Return: Sample rate in Hz or a negative errno if parsing failed.
+ */
+int drm_cea_sad_get_sample_rate(const struct cea_sad *sad)
+{
+ switch (sad->freq) {
+ case CEA_SAD_FREQ_32KHZ:
+ return 32000;
+ case CEA_SAD_FREQ_44KHZ:
+ return 44100;
+ case CEA_SAD_FREQ_48KHZ:
+ return 48000;
+ case CEA_SAD_FREQ_88KHZ:
+ return 88200;
+ case CEA_SAD_FREQ_96KHZ:
+ return 96000;
+ case CEA_SAD_FREQ_176KHZ:
+ return 176400;
+ case CEA_SAD_FREQ_192KHZ:
+ return 192000;
+ default:
+ return -EINVAL;
+ }
+}
+EXPORT_SYMBOL(drm_cea_sad_get_sample_rate);
+
+static bool drm_cea_sad_is_uncompressed(const struct cea_sad *sad)
+{
+ switch (sad->format) {
+ case HDMI_AUDIO_CODING_TYPE_STREAM:
+ case HDMI_AUDIO_CODING_TYPE_PCM:
+ return true;
+ default:
+ return false;
+ }
+}
+
+/**
+ * drm_cea_sad_get_uncompressed_word_length - Extract word length
+ * @sad: Pointer to the cea_sad struct
+ *
+ * Extracts the cea_sad byte2 field and returns the word length for an
+ * uncompressed stream.
+ *
+ * Note: This function may only be called for uncompressed audio.
+ *
+ * Return: Word length in bits or a negative errno if parsing failed.
+ */
+int drm_cea_sad_get_uncompressed_word_length(const struct cea_sad *sad)
+{
+ if (!drm_cea_sad_is_uncompressed(sad)) {
+ DRM_WARN("Unable to get the uncompressed word length for a
compressed format: %u\n",
+sad->format);
+ return -EINVAL;
+ }
+
+ switch (sad->byte2) {
+ case CEA_SAD_UNCOMPRESSED_WORD_16BIT:
+ return 16;
+ case CEA_SAD_UNCOMPRESSED_WORD_20BIT:
+ return 20;
+ case CEA_SAD_UNCOMPRESSED_WORD_24BIT:
+ return 24;
+ default:
+ return -EINVAL;
+ }
+}
+EXPORT_SYMBOL(drm_cea_sad_get_uncompressed_word_length);
+
/**
* drm_av_sync_delay - compute the HDMI/DP sink audio-video sync delay
* @connector: connector associated with the HDMI/DP sink
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index deccfd39e6db..7b7d71a7154d 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -361,12 +361,24 @@ struct edid {
/* Short Audio Descriptor */
struct cea_sad {
- u8 format;
+ u8 format; /* See HDMI_AUDIO_CODING_TYPE_* */
u8 channels; /* max number of channels - 1 */
- u8 freq;
+ u8 freq; /* See CEA_SAD_FREQ_* */
u8 byte2; /* meaning depends on format */
};
+#define CEA_SAD_FREQ_32KHZ BIT(0)
+#define CEA_SAD_FREQ_44KHZ BIT(1)
+#define CEA_SAD_FREQ_48KHZ BIT(2)
+#define CEA_SAD_FREQ_88KHZ BIT(3)
+#define CEA_SAD_FREQ_96KHZ BIT(4)
+#define CEA_SAD_FREQ_176KHZ BIT(5)
+#define CEA_SAD_FREQ_192KHZ BIT(6)
+
+#define CEA_SAD_UNCOMPRESSED_WORD_16BIT BIT(0)
+#define CEA_SAD_UNCOMPRESSED_WORD_20BIT BIT(1)
+#define CEA_SAD_UNCOMPRESSED_WORD_24BIT BIT(2)
+
struct drm_encoder;
struct drm_connector;
struct drm_connector_state;
@@ -374,6 +386,8 @@ struct drm_display_mode;
int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb);
+int drm_cea_sad_get_sample_rate(const struct cea_sad *sad);
+int drm_cea_sad_get_uncompressed_word_length(const struct cea_sad *sad);
int drm_av_sync_delay(struct drm_connector *connector,
const struct drm_display_mode *mode);
--
2.33.0
[PATCH v2 4/6] video/hdmi: Add audio_infoframe packing for DP
Similar to HDMI, DP uses audio infoframes as well which are structured
very similar to the HDMI ones.
This patch adds a helper function to pack the HDMI audio infoframe for
DP, called hdmi_audio_infoframe_pack_for_dp().
hdmi_audio_infoframe_pack_only() is split into two parts. One of them
packs the payload only and can be used for HDMI and DP.
Signed-off-by: Markus Schneider-Pargmann
---
Notes:
Changes v1 -> v2:
- Create a define for HB2.
- Use struct dp_sdp to pass data in a better way.
drivers/video/hdmi.c| 83 -
include/drm/drm_dp_helper.h | 2 +
include/linux/hdmi.h| 7 +++-
3 files changed, 72 insertions(+), 20 deletions(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 947be761dfa4..63e74d9fd210 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -21,6 +21,7 @@
* DEALINGS IN THE SOFTWARE.
*/
+#include
#include
#include
#include
@@ -381,12 +382,34 @@ static int hdmi_audio_infoframe_check_only(const struct
hdmi_audio_infoframe *fr
*
* Returns 0 on success or a negative error code on failure.
*/
-int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame)
+int hdmi_audio_infoframe_check(const struct hdmi_audio_infoframe *frame)
{
return hdmi_audio_infoframe_check_only(frame);
}
EXPORT_SYMBOL(hdmi_audio_infoframe_check);
+static void
+hdmi_audio_infoframe_pack_payload(const struct hdmi_audio_infoframe *frame,
+ u8 *buffer)
+{
+ u8 channels;
+
+ if (frame->channels >= 2)
+ channels = frame->channels - 1;
+ else
+ channels = 0;
+
+ buffer[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
+ buffer[1] = ((frame->sample_frequency & 0x7) << 2) |
+(frame->sample_size & 0x3);
+ buffer[2] = frame->coding_type_ext & 0x1f;
+ buffer[3] = frame->channel_allocation;
+ buffer[4] = (frame->level_shift_value & 0xf) << 3;
+
+ if (frame->downmix_inhibit)
+ buffer[4] |= BIT(7);
+}
+
/**
* hdmi_audio_infoframe_pack_only() - write HDMI audio infoframe to binary
buffer
* @frame: HDMI audio infoframe
@@ -404,7 +427,6 @@ EXPORT_SYMBOL(hdmi_audio_infoframe_check);
ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe
*frame,
void *buffer, size_t size)
{
- unsigned char channels;
u8 *ptr = buffer;
size_t length;
int ret;
@@ -420,28 +442,13 @@ ssize_t hdmi_audio_infoframe_pack_only(const struct
hdmi_audio_infoframe *frame,
memset(buffer, 0, size);
- if (frame->channels >= 2)
- channels = frame->channels - 1;
- else
- channels = 0;
-
ptr[0] = frame->type;
ptr[1] = frame->version;
ptr[2] = frame->length;
ptr[3] = 0; /* checksum */
- /* start infoframe payload */
- ptr += HDMI_INFOFRAME_HEADER_SIZE;
-
- ptr[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
- ptr[1] = ((frame->sample_frequency & 0x7) << 2) |
-(frame->sample_size & 0x3);
- ptr[2] = frame->coding_type_ext & 0x1f;
- ptr[3] = frame->channel_allocation;
- ptr[4] = (frame->level_shift_value & 0xf) << 3;
-
- if (frame->downmix_inhibit)
- ptr[4] |= BIT(7);
+ hdmi_audio_infoframe_pack_payload(frame,
+ ptr + HDMI_INFOFRAME_HEADER_SIZE);
hdmi_infoframe_set_checksum(buffer, length);
@@ -479,6 +486,44 @@ ssize_t hdmi_audio_infoframe_pack(struct
hdmi_audio_infoframe *frame,
}
EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
+/**
+ * hdmi_audio_infoframe_pack_for_dp - Pack a HDMI Audio infoframe for
+ *displayport
+ *
+ * @frame HDMI Audio infoframe
+ * @sdp secondary data packet for display port. This is filled with the
+ * appropriate data
+ * @dp_version Display Port version to be encoded in the header
+ *
+ * Packs a HDMI Audio Infoframe to be sent over Display Port. This function
+ * fills the secondary data packet to be used for Display Port.
+ *
+ * Return: Number of total written bytes or a negative errno on failure.
+ */
+ssize_t
+hdmi_audio_infoframe_pack_for_dp(const struct hdmi_audio_infoframe *frame,
+struct dp_sdp *sdp, u8 dp_version)
+{
+ int ret;
+
+ ret = hdmi_audio_infoframe_check(frame);
+ if (ret)
+ return ret;
+
+ memset(sdp->db, 0, sizeof(sdp->db));
+
+ // Secondary-data packet header
+ sdp->sdp_header.HB0 = 0;
+ sdp->sdp_header.HB1 = frame->type;
+ sdp->sdp_header.HB2 = DP_SDP_AUDIO_INFOFRAME_HB2;
+ sdp->sdp_header.HB3 = (dp_version & 0x3f) << 2;
+
+ hdmi_audio_infoframe_pack_payload(frame, sdp->db);
+
+ return frame->length + 4;
+}
+EXPORT_SYMBOL(hdmi_audio_infoframe_pack_for_dp);
+
/**
*
[PATCH v2 5/6] drm/mediatek: dpi: Add dpintf support
dpintf is the displayport interface hardware unit. This unit is similar
to dpi and can reuse most of the code.
This patch adds support for mt8195-dpintf to this dpi driver. Main
differences are:
- Some features/functional components are not available for dpintf
which are now excluded from code execution once is_dpintf is set
- dpintf can and needs to choose between different clockdividers based
on the clockspeed. This is done by choosing a different clock parent.
- There are two additional clocks that need to be managed. These are
only set for dpintf and will be set to NULL if not supplied. The
clk_* calls handle these as normal clocks then.
- Some register contents differ slightly between the two components. To
work around this I added register bits/masks with a DPINTF_ prefix
and use them where different.
Based on a separate driver for dpintf created by
Jason-JH.Lin .
Signed-off-by: Markus Schneider-Pargmann
---
Notes:
Changes RFC -> v1:
- Remove setting parents and fully rely on the clock tree instead which
already
models a mux at the important place.
- Integrated mtk_dpi dpintf changes into the mediatek drm driver.
drivers/gpu/drm/mediatek/mtk_dpi.c | 248
drivers/gpu/drm/mediatek/mtk_dpi_regs.h | 12 +
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 4 +
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +-
5 files changed, 218 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 4554e2de1430..87961ebf5d35 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -71,6 +71,8 @@ struct mtk_dpi {
void __iomem *regs;
struct device *dev;
struct clk *engine_clk;
+ struct clk *hf_fmm_clk;
+ struct clk *hf_fdp_clk;
struct clk *pixel_clk;
struct clk *tvd_clk;
int irq;
@@ -125,6 +127,7 @@ struct mtk_dpi_conf {
bool edge_sel_en;
const u32 *output_fmts;
u32 num_output_fmts;
+ bool is_dpintf;
};
static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 mask)
@@ -153,30 +156,52 @@ static void mtk_dpi_disable(struct mtk_dpi *dpi)
static void mtk_dpi_config_hsync(struct mtk_dpi *dpi,
struct mtk_dpi_sync_param *sync)
{
- mtk_dpi_mask(dpi, DPI_TGEN_HWIDTH,
-sync->sync_width << HPW, HPW_MASK);
- mtk_dpi_mask(dpi, DPI_TGEN_HPORCH,
-sync->back_porch << HBP, HBP_MASK);
- mtk_dpi_mask(dpi, DPI_TGEN_HPORCH, sync->front_porch << HFP,
-HFP_MASK);
+ if (dpi->conf->is_dpintf) {
+ mtk_dpi_mask(dpi, DPI_TGEN_HWIDTH,
+sync->sync_width << HPW, DPINTF_HPW_MASK);
+ mtk_dpi_mask(dpi, DPI_TGEN_HPORCH,
+sync->back_porch << HBP, DPINTF_HBP_MASK);
+ mtk_dpi_mask(dpi, DPI_TGEN_HPORCH, sync->front_porch << HFP,
+DPINTF_HFP_MASK);
+ } else {
+ mtk_dpi_mask(dpi, DPI_TGEN_HWIDTH,
+sync->sync_width << HPW, HPW_MASK);
+ mtk_dpi_mask(dpi, DPI_TGEN_HPORCH,
+sync->back_porch << HBP, HBP_MASK);
+ mtk_dpi_mask(dpi, DPI_TGEN_HPORCH, sync->front_porch << HFP,
+HFP_MASK);
+ }
}
static void mtk_dpi_config_vsync(struct mtk_dpi *dpi,
struct mtk_dpi_sync_param *sync,
u32 width_addr, u32 porch_addr)
{
- mtk_dpi_mask(dpi, width_addr,
-sync->sync_width << VSYNC_WIDTH_SHIFT,
-VSYNC_WIDTH_MASK);
mtk_dpi_mask(dpi, width_addr,
sync->shift_half_line << VSYNC_HALF_LINE_SHIFT,
VSYNC_HALF_LINE_MASK);
- mtk_dpi_mask(dpi, porch_addr,
-sync->back_porch << VSYNC_BACK_PORCH_SHIFT,
-VSYNC_BACK_PORCH_MASK);
- mtk_dpi_mask(dpi, porch_addr,
-sync->front_porch << VSYNC_FRONT_PORCH_SHIFT,
-VSYNC_FRONT_PORCH_MASK);
+
+ if (dpi->conf->is_dpintf) {
+ mtk_dpi_mask(dpi, width_addr,
+sync->sync_width << VSYNC_WIDTH_SHIFT,
+DPINTF_VSYNC_WIDTH_MASK);
+ mtk_dpi_mask(dpi, porch_addr,
+sync->back_porch << VSYNC_BACK_PORCH_SHIFT,
+DPINTF_VSYNC_BACK_PORCH_MASK);
+ mtk_dpi_mask(dpi, porch_addr,
+sync->front_porch << VSYNC_FRONT_PORCH_SHIFT,
+DPINTF_VSYNC_FRONT_PORCH_MASK);
+ } else {
+ mtk_dpi_mask(dpi, width_addr,
+sync->sync_width << VSYNC_WIDTH_SHIFT,
+
Re: [Intel-gfx] [PATCH 13/26] drm/i915: use the new iterator in i915_gem_busy_ioctl
On 17/09/2021 13:35, Christian König wrote:
This makes the function much simpler since the complex
retry logic is now handled else where.
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/gem/i915_gem_busy.c | 32
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
index 6234e17259c1..b1cb7ba688da 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
@@ -82,8 +82,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_gem_busy *args = data;
struct drm_i915_gem_object *obj;
- struct dma_resv_list *list;
- unsigned int seq;
+ struct dma_resv_iter cursor;
+ struct dma_fence *fence;
int err;
err = -ENOENT;
@@ -109,27 +109,17 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
* to report the overall busyness. This is what the wait-ioctl does.
*
*/
-retry:
- seq = raw_read_seqcount(>base.resv->seq);
-
- /* Translate the exclusive fence to the READ *and* WRITE engine */
- args->busy = busy_check_writer(dma_resv_excl_fence(obj->base.resv));
-
- /* Translate shared fences to READ set of engines */
- list = dma_resv_shared_list(obj->base.resv);
- if (list) {
- unsigned int shared_count = list->shared_count, i;
-
- for (i = 0; i < shared_count; ++i) {
- struct dma_fence *fence =
- rcu_dereference(list->shared[i]);
-
+ args->busy = false;
+ dma_resv_iter_begin(, obj->base.resv, true);
+ dma_resv_for_each_fence_unlocked(, fence) {
You did not agree with my suggestion to reset args->busy on restart and
so preserve current behaviour?
Regards,
Tvrtko
+ if (dma_resv_iter_is_exclusive())
+ /* Translate the exclusive fence to the READ *and*
WRITE engine */
+ args->busy = busy_check_writer(fence);
+ else
+ /* Translate shared fences to READ set of engines */
args->busy |= busy_check_reader(fence);
- }
}
-
- if (args->busy && read_seqcount_retry(>base.resv->seq, seq))
- goto retry;
+ dma_resv_iter_end();
err = 0;
out:
Re: [Intel-gfx] [PATCH 14/26] drm/i915: use the new iterator in i915_sw_fence_await_reservation v3
On 17/09/2021 13:35, Christian König wrote:
Simplifying the code a bit.
v2: use dma_resv_for_each_fence instead, according to Tvrtko the lock is
held here anyway.
v3: back to using dma_resv_for_each_fence_unlocked.
It did not work out - what happened?
Regards,
Tvrtko
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/i915_sw_fence.c | 57
1 file changed, 15 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c
b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..7635b0478ea5 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -572,56 +572,29 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence
*fence,
unsigned long timeout,
gfp_t gfp)
{
- struct dma_fence *excl;
+ struct dma_resv_iter cursor;
+ struct dma_fence *f;
int ret = 0, pending;
debug_fence_assert(fence);
might_sleep_if(gfpflags_allow_blocking(gfp));
- if (write) {
- struct dma_fence **shared;
- unsigned int count, i;
-
- ret = dma_resv_get_fences(resv, , , );
- if (ret)
- return ret;
-
- for (i = 0; i < count; i++) {
- if (shared[i]->ops == exclude)
- continue;
-
- pending = i915_sw_fence_await_dma_fence(fence,
- shared[i],
- timeout,
- gfp);
- if (pending < 0) {
- ret = pending;
- break;
- }
-
- ret |= pending;
- }
-
- for (i = 0; i < count; i++)
- dma_fence_put(shared[i]);
- kfree(shared);
- } else {
- excl = dma_resv_get_excl_unlocked(resv);
- }
-
- if (ret >= 0 && excl && excl->ops != exclude) {
- pending = i915_sw_fence_await_dma_fence(fence,
- excl,
- timeout,
+ rcu_read_lock();
+ dma_resv_iter_begin(, resv, write);
+ dma_resv_for_each_fence_unlocked(, f) {
+ rcu_read_unlock();
+ pending = i915_sw_fence_await_dma_fence(fence, f, timeout,
gfp);
- if (pending < 0)
+ rcu_read_lock();
+ if (pending < 0) {
ret = pending;
- else
- ret |= pending;
- }
-
- dma_fence_put(excl);
+ break;
+ }
+ ret |= pending;
+ }
+ dma_resv_iter_end();
+ rcu_read_unlock();
return ret;
}
Re: [Intel-gfx] [PATCH 14/26] drm/i915: use the new iterator in i915_sw_fence_await_reservation v3
On 20/09/2021 09:45, Tvrtko Ursulin wrote:
On 17/09/2021 13:35, Christian König wrote:
Simplifying the code a bit.
v2: use dma_resv_for_each_fence instead, according to Tvrtko the lock is
held here anyway.
v3: back to using dma_resv_for_each_fence_unlocked.
It did not work out - what happened?
Wait, my suggestion to try the locked iterator was against
i915_request_await_object. I haven't looked at this one at the time or
even now.
Regards,
Tvrtko
Regards,
Tvrtko
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/i915_sw_fence.c | 57
1 file changed, 15 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c
b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..7635b0478ea5 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -572,56 +572,29 @@ int i915_sw_fence_await_reservation(struct
i915_sw_fence *fence,
unsigned long timeout,
gfp_t gfp)
{
- struct dma_fence *excl;
+ struct dma_resv_iter cursor;
+ struct dma_fence *f;
int ret = 0, pending;
debug_fence_assert(fence);
might_sleep_if(gfpflags_allow_blocking(gfp));
- if (write) {
- struct dma_fence **shared;
- unsigned int count, i;
-
- ret = dma_resv_get_fences(resv, , , );
- if (ret)
- return ret;
-
- for (i = 0; i < count; i++) {
- if (shared[i]->ops == exclude)
- continue;
-
- pending = i915_sw_fence_await_dma_fence(fence,
- shared[i],
- timeout,
- gfp);
- if (pending < 0) {
- ret = pending;
- break;
- }
-
- ret |= pending;
- }
-
- for (i = 0; i < count; i++)
- dma_fence_put(shared[i]);
- kfree(shared);
- } else {
- excl = dma_resv_get_excl_unlocked(resv);
- }
-
- if (ret >= 0 && excl && excl->ops != exclude) {
- pending = i915_sw_fence_await_dma_fence(fence,
- excl,
- timeout,
+ rcu_read_lock();
+ dma_resv_iter_begin(, resv, write);
+ dma_resv_for_each_fence_unlocked(, f) {
+ rcu_read_unlock();
+ pending = i915_sw_fence_await_dma_fence(fence, f, timeout,
gfp);
- if (pending < 0)
+ rcu_read_lock();
+ if (pending < 0) {
ret = pending;
- else
- ret |= pending;
- }
-
- dma_fence_put(excl);
+ break;
+ }
+ ret |= pending;
+ }
+ dma_resv_iter_end();
+ rcu_read_unlock();
return ret;
}
Re: [PATCH] drm/i915/guc, docs: Fix pdfdocs build error by removing nested grid
On Mon, 20 Sep 2021, Akira Yokosawa wrote:
> Nested grids in grid-table cells are not specified as proper ReST
> constructs.
> Commit 572f2a5cd974 ("drm/i915/guc: Update firmware to v62.0.0")
> added a couple of kerneldoc tables of the form:
>
> +---+---+--+
> | 1 | 31:0 | ++ |
> +---+---+ || |
> |...| | | Embedded `HXG Message`_ | |
> +---+---+ || |
> | n | 31:0 | ++ |
> +---+---+--+
>
> For "make htmldocs", they happen to work as one might expect,
> but they are incompatible with "make latexdocs" and "make pdfdocs",
> and cause the generated gpu.tex file to become incomplete and
> unbuildable by xelatex.
>
> Restore the compatibility by removing those nested grids in the tables.
>
> Size comparison of generated gpu.tex:
>
> Sphinx 2.4.4 Sphinx 4.2.0
> v5.14: 3238686 3841631
> v5.15-rc1:376270432729
> with this fix: 3377846 3998095
>
> Fixes: 572f2a5cd974 ("drm/i915/guc: Update firmware to v62.0.0")
> Cc: John Harrison
> Cc: Michal Wajdeczko
> Cc: Matthew Brost
> Cc: Daniele Ceraolo Spurio
> Cc: Matt Roper
> Cc: Jonathan Corbet
> Signed-off-by: Akira Yokosawa
> ---
> Hi all,
>
> I know there is little interest in building pdfdocs (or LaTeX) version
> of kernel-doc, and this issue does not matter most of you.
>
> But "make pdfdocs" is supposed to work, give or take those tables
> with squeezed columns, and at least it is expected to complete
> without fatal errors.
Absolutely!
> I have no idea who is responsible to those grid-tables, so added
> a lot of people in the To: and Cc: lists.
>
> Does removing those nested grids look reasonable to you?
>
> Any feedback is welcome!
>
> Note: This patch is against the docs-next branch of Jon's -doc tree
> (git://git.lwn.net/linux.git). It can be applied against v5.15-rc1
> and v5.15-rc2 as well.
I think this should go through drm-intel, and it'll find its way back to
some v5.15-rc.
BR,
Jani.
>
> Thanks, Akira
> --
> .../gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h | 10 +-
> .../drm/i915/gt/uc/abi/guc_communication_mmio_abi.h| 10 +-
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> index 99e1fad5ca20..c9086a600bce 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> @@ -102,11 +102,11 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64);
> * |
> +---+--+
> * | | 7:0 | NUM_DWORDS = length (in dwords) of the embedded HXG
> message |
> *
> +---+---+--+
> - * | 1 | 31:0 |
> ++ |
> - * +---+---+ |
> | |
> - * |...| | | Embedded `HXG Message`_
> | |
> - * +---+---+ |
> | |
> - * | n | 31:0 |
> ++ |
> + * | 1 | 31:0 |
> |
> + * +---+---+
> |
> + * |...| | [Embedded `HXG Message`_]
> |
> + * +---+---+
> |
> + * | n | 31:0 |
> |
> *
> +---+---+--+
> */
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
> index bbf1ddb77434..9baa3cb07d13 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
> @@ -38,11 +38,11 @@
> *
> +---+---+--+
> * | | Bits | Description
> |
> *
> +===+===+==+
> - * | 0 | 31:0 |
> ++ |
> - * +---+---+ |
> | |
> - * |...| | | Embedded `HXG
Re: [PATCH v2 09/12] lib: test_hmm add module param for zone device type
On Tuesday, 14 September 2021 2:16:01 AM AEST Alex Sierra wrote:
> In order to configure device public in test_hmm, two module parameters
> should be passed, which correspond to the SP start address of each
> device (2) spm_addr_dev0 & spm_addr_dev1. If no parameters are passed,
> private device type is configured.
It's a pity that testing this seems to require some amount of special setup to
test. Is there a way this could be made to work on a more standard setup
similar to how DEVICE_PRIVATE is tested?
> Signed-off-by: Alex Sierra
> ---
> v5:
> Remove devmem->pagemap.type = MEMORY_DEVICE_PRIVATE at
> dmirror_allocate_chunk that was forcing to configure pagemap.type
> to MEMORY_DEVICE_PRIVATE
>
> v6:
> Check for null pointers for resource and memremap references
> at dmirror_allocate_chunk
>
> v7:
> Due to patch dropped from these patch series "kernel: resource:
> lookup_resource as exported symbol", lookup_resource was not longer a
> callable function. This was used in public device configuration, to
> get start and end addresses, to create pgmap->range struct. This
> information is now taken directly from the spm_addr_devX parameters and
> the fixed size DEVMEM_CHUNK_SIZE.
> ---
> lib/test_hmm.c | 66 +++--
> lib/test_hmm_uapi.h | 1 +
> 2 files changed, 47 insertions(+), 20 deletions(-)
>
> diff --git a/lib/test_hmm.c b/lib/test_hmm.c
> index 3cd91ca31dd7..ef27e355738a 100644
> --- a/lib/test_hmm.c
> +++ b/lib/test_hmm.c
> @@ -33,6 +33,16 @@
> #define DEVMEM_CHUNK_SIZE(256 * 1024 * 1024U)
> #define DEVMEM_CHUNKS_RESERVE16
>
> +static unsigned long spm_addr_dev0;
> +module_param(spm_addr_dev0, long, 0644);
> +MODULE_PARM_DESC(spm_addr_dev0,
> + "Specify start address for SPM (special purpose memory) used
> for device 0. By setting this Generic device type will be used. Make sure
> spm_addr_dev1 is set too");
> +
> +static unsigned long spm_addr_dev1;
> +module_param(spm_addr_dev1, long, 0644);
> +MODULE_PARM_DESC(spm_addr_dev1,
> + "Specify start address for SPM (special purpose memory) used
> for device 1. By setting this Generic device type will be used. Make sure
> spm_addr_dev0 is set too");
> +
> static const struct dev_pagemap_ops dmirror_devmem_ops;
> static const struct mmu_interval_notifier_ops dmirror_min_ops;
> static dev_t dmirror_dev;
> @@ -450,11 +460,11 @@ static int dmirror_write(struct dmirror *dmirror,
> struct hmm_dmirror_cmd *cmd)
> return ret;
> }
>
> -static bool dmirror_allocate_chunk(struct dmirror_device *mdevice,
> +static int dmirror_allocate_chunk(struct dmirror_device *mdevice,
> struct page **ppage)
> {
> struct dmirror_chunk *devmem;
> - struct resource *res;
> + struct resource *res = NULL;
> unsigned long pfn;
> unsigned long pfn_first;
> unsigned long pfn_last;
> @@ -462,17 +472,29 @@ static bool dmirror_allocate_chunk(struct
> dmirror_device *mdevice,
>
> devmem = kzalloc(sizeof(*devmem), GFP_KERNEL);
> if (!devmem)
> - return false;
> + return -ENOMEM;
>
> - res = request_free_mem_region(_resource, DEVMEM_CHUNK_SIZE,
> - "hmm_dmirror");
> - if (IS_ERR(res))
> - goto err_devmem;
> + if (!spm_addr_dev0 && !spm_addr_dev1) {
> + res = request_free_mem_region(_resource,
> DEVMEM_CHUNK_SIZE,
> + "hmm_dmirror");
> + if (IS_ERR_OR_NULL(res))
> + goto err_devmem;
> + devmem->pagemap.range.start = res->start;
> + devmem->pagemap.range.end = res->end;
> + devmem->pagemap.type = MEMORY_DEVICE_PRIVATE;
> + mdevice->zone_device_type = HMM_DMIRROR_MEMORY_DEVICE_PRIVATE;
> + } else if (spm_addr_dev0 && spm_addr_dev1) {
> + devmem->pagemap.range.start = MINOR(mdevice->cdevice.dev) ?
> + spm_addr_dev0 :
> + spm_addr_dev1;
> + devmem->pagemap.range.end = devmem->pagemap.range.start +
> + DEVMEM_CHUNK_SIZE - 1;
> + devmem->pagemap.type = MEMORY_DEVICE_PUBLIC;
> + mdevice->zone_device_type = HMM_DMIRROR_MEMORY_DEVICE_PUBLIC;
> + } else {
> + pr_err("Both spm_addr_dev parameters should be set\n");
> + }
>
> - mdevice->zone_device_type = HMM_DMIRROR_MEMORY_DEVICE_PRIVATE;
> - devmem->pagemap.type = MEMORY_DEVICE_PRIVATE;
> - devmem->pagemap.range.start = res->start;
> - devmem->pagemap.range.end = res->end;
> devmem->pagemap.nr_range = 1;
> devmem->pagemap.ops = _devmem_ops;
> devmem->pagemap.owner = mdevice;
> @@ -493,10 +515,14 @@ static bool dmirror_allocate_chunk(struct
> dmirror_device *mdevice,
>
Re: [git pull] drm for 5.14-rc1
Hi, On Sat, Sep 18, 2021 at 11:18:33AM +0200, Michael Stapelberg wrote: > torvalds at linux-foundation.org (Linus Torvalds) writes: > > Did I fix it up correctly? Who knows. The code makes more sense to me > > now and seems valid. But I really *really* want to stress how locking > > is important. > > As far as I can tell, this merge conflict resolution made my Raspberry > Pi 3 hang on boot. You can find the full serial console output at: > > https://t.zekjur.net/linux-5.14-raspberry-pi-3-hang-vc4.txt > > The last few messages are from vc4, then the boot hangs. > > Using git-bisect, I tracked this down to > https://github.com/torvalds/linux/commit/e058a84bfddc42ba356a2316f2cf1141974625c9, > which is the merge you’re talking about here, AFAICT. > > I also tried the git://anongit.freedesktop.org/drm/drm, and that tree > boots as expected, suggesting that the problem really is with the > additional changes. > > The code seems to work on my Raspberry Pi 4, just not on the Raspberry > Pi 3. Any ideas why that might be, and how to fix it? I assume you run your Pi without anything connected on HDMI, and without hdmi_force_hotplug in your config.txt? If so, can you test that branch, and let me know if it works for you https://github.com/mripard/linux/tree/rpi/bug-fixes Maxime signature.asc Description: PGP signature
[PATCH] drm/i915/guc, docs: Fix pdfdocs build error by removing nested grid
Nested grids in grid-table cells are not specified as proper ReST
constructs.
Commit 572f2a5cd974 ("drm/i915/guc: Update firmware to v62.0.0")
added a couple of kerneldoc tables of the form:
+---+---+--+
| 1 | 31:0 | ++ |
+---+---+ || |
|...| | | Embedded `HXG Message`_ | |
+---+---+ || |
| n | 31:0 | ++ |
+---+---+--+
For "make htmldocs", they happen to work as one might expect,
but they are incompatible with "make latexdocs" and "make pdfdocs",
and cause the generated gpu.tex file to become incomplete and
unbuildable by xelatex.
Restore the compatibility by removing those nested grids in the tables.
Size comparison of generated gpu.tex:
Sphinx 2.4.4 Sphinx 4.2.0
v5.14: 3238686 3841631
v5.15-rc1:376270432729
with this fix: 3377846 3998095
Fixes: 572f2a5cd974 ("drm/i915/guc: Update firmware to v62.0.0")
Cc: John Harrison
Cc: Michal Wajdeczko
Cc: Matthew Brost
Cc: Daniele Ceraolo Spurio
Cc: Matt Roper
Cc: Jonathan Corbet
Signed-off-by: Akira Yokosawa
---
Hi all,
I know there is little interest in building pdfdocs (or LaTeX) version
of kernel-doc, and this issue does not matter most of you.
But "make pdfdocs" is supposed to work, give or take those tables
with squeezed columns, and at least it is expected to complete
without fatal errors.
I have no idea who is responsible to those grid-tables, so added
a lot of people in the To: and Cc: lists.
Does removing those nested grids look reasonable to you?
Any feedback is welcome!
Note: This patch is against the docs-next branch of Jon's -doc tree
(git://git.lwn.net/linux.git). It can be applied against v5.15-rc1
and v5.15-rc2 as well.
Thanks, Akira
--
.../gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h | 10 +-
.../drm/i915/gt/uc/abi/guc_communication_mmio_abi.h| 10 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
index 99e1fad5ca20..c9086a600bce 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
@@ -102,11 +102,11 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64);
* |
+---+--+
* | | 7:0 | NUM_DWORDS = length (in dwords) of the embedded HXG message
|
*
+---+---+--+
- * | 1 | 31:0 | ++
|
- * +---+---+ ||
|
- * |...| | | Embedded `HXG Message`_ |
|
- * +---+---+ ||
|
- * | n | 31:0 | ++
|
+ * | 1 | 31:0 |
|
+ * +---+---+
|
+ * |...| | [Embedded `HXG Message`_]
|
+ * +---+---+
|
+ * | n | 31:0 |
|
*
+---+---+--+
*/
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
index bbf1ddb77434..9baa3cb07d13 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
@@ -38,11 +38,11 @@
*
+---+---+--+
* | | Bits | Description
|
*
+===+===+==+
- * | 0 | 31:0 | ++
|
- * +---+---+ ||
|
- * |...| | | Embedded `HXG Message`_ |
|
- * +---+---+ ||
|
- * | n | 31:0 | ++
|
+ * | 0 | 31:0 |
|
+ * +---+---+
Re: [PATCH 03/26] dma-buf: use new iterator in dma_resv_copy_fences
Am 17.09.21 um 16:35 schrieb Daniel Vetter:
On Fri, Sep 17, 2021 at 02:34:50PM +0200, Christian König wrote:
This makes the function much simpler since the complex
retry logic is now handled else where.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-resv.c | 86 --
1 file changed, 35 insertions(+), 51 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index a3c79a99fb44..406150dea5e4 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -426,74 +426,58 @@ EXPORT_SYMBOL_GPL(dma_resv_iter_walk);
*/
int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
{
- struct dma_resv_list *src_list, *dst_list;
- struct dma_fence *old, *new;
- unsigned int i;
+ struct dma_resv_iter cursor;
+ struct dma_resv_list *list;
+ struct dma_fence *f, *excl;
dma_resv_assert_held(dst);
- rcu_read_lock();
- src_list = dma_resv_shared_list(src);
-
-retry:
- if (src_list) {
- unsigned int shared_count = src_list->shared_count;
-
- rcu_read_unlock();
+ list = NULL;
+ excl = NULL;
- dst_list = dma_resv_list_alloc(shared_count);
- if (!dst_list)
- return -ENOMEM;
+ rcu_read_lock();
+ dma_resv_iter_begin(, src, true);
+ dma_resv_for_each_fence_unlocked(, f) {
- rcu_read_lock();
- src_list = dma_resv_shared_list(src);
- if (!src_list || src_list->shared_count > shared_count) {
- kfree(dst_list);
- goto retry;
- }
+ if (cursor.is_first) {
Maybe have a wrapper for this, like dma_resv_iter_is_reset or is_first or
is_restart (my preference) with some nice docs that this returns true
everytime we had to restart the sequence?
Exactly that's what I wanted to avoid since the is_first (or whatever)
function should only be used inside of the dma_resv.c code.
On the other hand I can just make that static here and document that it
should never be exported.
Christian.
Otherwise I fully agree, this is so much better with all the hairy
restarting and get_rcu and test_bit shovelled away somewhere.
Either way (but I much prefer a wrapper for is_first):
Reviewed-by: Daniel Vetter
+ dma_resv_list_free(list);
+ dma_fence_put(excl);
- dst_list->shared_count = 0;
- for (i = 0; i < src_list->shared_count; ++i) {
- struct dma_fence __rcu **dst;
- struct dma_fence *fence;
+ if (cursor.fences) {
+ unsigned int cnt = cursor.fences->shared_count;
- fence = rcu_dereference(src_list->shared[i]);
- if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
->flags))
- continue;
+ rcu_read_unlock();
+ list = dma_resv_list_alloc(cnt);
+ if (!list) {
+ dma_resv_iter_end();
+ return -ENOMEM;
+ }
- if (!dma_fence_get_rcu(fence)) {
- dma_resv_list_free(dst_list);
- src_list = dma_resv_shared_list(src);
- goto retry;
- }
+ list->shared_count = 0;
+ rcu_read_lock();
- if (dma_fence_is_signaled(fence)) {
- dma_fence_put(fence);
- continue;
+ } else {
+ list = NULL;
}
-
- dst = _list->shared[dst_list->shared_count++];
- rcu_assign_pointer(*dst, fence);
+ excl = NULL;
}
- } else {
- dst_list = NULL;
- }
- new = dma_fence_get_rcu_safe(>fence_excl);
+ dma_fence_get(f);
+ if (dma_resv_iter_is_exclusive())
+ excl = f;
+ else
+ RCU_INIT_POINTER(list->shared[list->shared_count++], f);
+ }
+ dma_resv_iter_end();
rcu_read_unlock();
- src_list = dma_resv_shared_list(dst);
- old = dma_resv_excl_fence(dst);
-
write_seqcount_begin(>seq);
- /* write_seqcount_begin provides the necessary memory barrier */
- RCU_INIT_POINTER(dst->fence_excl, new);
- RCU_INIT_POINTER(dst->fence, dst_list);
+ excl = rcu_replace_pointer(dst->fence_excl, excl, dma_resv_held(dst));
+ list = rcu_replace_pointer(dst->fence, list, dma_resv_held(dst));
write_seqcount_end(>seq);
- dma_resv_list_free(src_list);
-
[Bug 214413] Kernel oops on boot for amdgpu (in si_dpm_set_power_state)
https://bugzilla.kernel.org/show_bug.cgi?id=214413 --- Comment #2 from Marco Piazza (mpia...@gmail.com) --- It is a regression, without apparent problems. In fact the laptop start and is working as usual. I've found a similar bug described here: https://gitlab.freedesktop.org/drm/amd/-/issues/1698 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 20/26] drm: use new iterator in drm_gem_fence_array_add_implicit v2
Am 17.09.21 um 16:53 schrieb Daniel Vetter:
On Fri, Sep 17, 2021 at 02:35:07PM +0200, Christian König wrote:
Simplifying the code a bit.
v2: add missing rcu_read_lock()/unlock()
Signed-off-by: Christian König
This will be gone as soon as I can land the last conversion patches. Plus
it's always called with dma_resv_lock held.
Yeah, already thought so as well. I will just keep that around to get
rid of dma_resv_get_excl_unlocked() for now until your patch lands.
Regards,
Christian.
I wouldn't bother tbh.
-Daniel
---
drivers/gpu/drm/drm_gem.c | 34 --
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 09c820045859..c2c41b668f40 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1340,31 +1340,21 @@ int drm_gem_fence_array_add_implicit(struct xarray
*fence_array,
struct drm_gem_object *obj,
bool write)
{
- int ret;
- struct dma_fence **fences;
- unsigned int i, fence_count;
-
- if (!write) {
- struct dma_fence *fence =
- dma_resv_get_excl_unlocked(obj->resv);
-
- return drm_gem_fence_array_add(fence_array, fence);
- }
-
- ret = dma_resv_get_fences(obj->resv, NULL,
- _count, );
- if (ret || !fence_count)
- return ret;
-
- for (i = 0; i < fence_count; i++) {
- ret = drm_gem_fence_array_add(fence_array, fences[i]);
+ struct dma_resv_iter cursor;
+ struct dma_fence *fence;
+ int ret = 0;
+
+ rcu_read_lock();
+ dma_resv_iter_begin(, obj->resv, write);
+ dma_resv_for_each_fence_unlocked(, fence) {
+ rcu_read_unlock();
+ ret = drm_gem_fence_array_add(fence_array, fence);
+ rcu_read_lock();
if (ret)
break;
}
-
- for (; i < fence_count; i++)
- dma_fence_put(fences[i]);
- kfree(fences);
+ dma_resv_iter_end();
+ rcu_read_unlock();
return ret;
}
EXPORT_SYMBOL(drm_gem_fence_array_add_implicit);
--
2.25.1
Re: [PATCH] drm/radeon: fix uninitialized bool variable
Am 18.09.21 um 11:41 schrieb Zhiwei Yang:
The bool variable detected_hpd_without_ddc in struct radeon_connector
is uninitialized when first used, that may cause unnecessary ddc ops.
Make it as false when a new connector is alloced.
Signed-off-by: Zhiwei Yang
---
drivers/gpu/drm/radeon/radeon_connectors.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index fe12d9d91d7a..c1987a66373f 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1866,6 +1866,7 @@ radeon_add_atom_connector(struct drm_device *dev,
bool shared_ddc = false;
bool is_dp_bridge = false;
bool has_aux = false;
+ bool detected_hpd_without_ddc = false;
In general good catch, but I don't see the need for a local variable here.
Just initialize the member directly or even better change the allocation
of the connector into a kzalloc().
Christian.
if (connector_type == DRM_MODE_CONNECTOR_Unknown)
return;
@@ -1923,6 +1924,7 @@ radeon_add_atom_connector(struct drm_device *dev,
radeon_connector->shared_ddc = shared_ddc;
radeon_connector->connector_object_id = connector_object_id;
radeon_connector->hpd = *hpd;
+ radeon_connector->detected_hpd_without_ddc = detected_hpd_without_ddc;
radeon_connector->router = *router;
if (router->ddc_valid || router->cd_valid) {
@@ -2384,6 +2386,7 @@ radeon_add_legacy_connector(struct drm_device *dev,
struct radeon_connector *radeon_connector;
struct i2c_adapter *ddc = NULL;
uint32_t subpixel_order = SubPixelNone;
+ bool detected_hpd_without_ddc = false;
if (connector_type == DRM_MODE_CONNECTOR_Unknown)
return;
@@ -2414,6 +2417,7 @@ radeon_add_legacy_connector(struct drm_device *dev,
radeon_connector->devices = supported_device;
radeon_connector->connector_object_id = connector_object_id;
radeon_connector->hpd = *hpd;
+ radeon_connector->detected_hpd_without_ddc = detected_hpd_without_ddc;
switch (connector_type) {
case DRM_MODE_CONNECTOR_VGA:
RE: [Intel-gfx] [PATCH 14/19] drm/i915/oprom: Basic sanitization
> -Original Message- > From: Nikula, Jani > Sent: Monday, September 20, 2021 1:12 PM > To: De Marchi, Lucas > Cc: Auld, Matthew ; intel-...@lists.freedesktop.org; > dri-devel@lists.freedesktop.org; Gupta, Anshuman > > Subject: Re: [Intel-gfx] [PATCH 14/19] drm/i915/oprom: Basic sanitization > > On Fri, 17 Sep 2021, Lucas De Marchi wrote: > > On Mon, May 17, 2021 at 02:57:33PM +0300, Jani Nikula wrote: > >>On Mon, 12 Apr 2021, Matthew Auld wrote: > >>> From: Anshuman Gupta > >>> > >>> Sanitize OPROM header, CPD signature and OPROM PCI version. > >>> OPROM_HEADER, EXPANSION_ROM_HEADER and OPROM_MEU_BLOB > structures and > >>> PCI struct offsets are provided by GSC counterparts. > >>> These are yet to be Documented in B.Spec. > >>> After successful sanitization, extract VBT from opregion image. > >> > >>So I don't understand what the point is with two consecutive patches > >>where the latter rewrites a lot of the former. > > > > I actually wonder what's the point of this. Getting it from spi is > > already the fallback and looks much more complex. Yes, it's pretty > > detailed and document the format pretty well, but it still looks more > > complex than the initial code. Do you see additional benefit in this > > one? Getting opregion image from spi is needed to get the intel_opregion and its mailboxes on discrete card. > > The commit message doesn't really explain much. Anshuman? I will get rework of the patches and float it again. Thanks, Anshuman Gupta. > > BR, > Jani. > > > -- > Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 0/5] lib: devres: Add managed helpers for write-combine setup
Hi, On 9/20/21 10:01 AM, Thomas Zimmermann wrote: > Hi > > Am 17.09.21 um 16:47 schrieb Hans de Goede: >> Hi, >> >> On 9/16/21 8:15 PM, Thomas Zimmermann wrote: >>> Add devm_arch_phys_wc_add() and devm_arch_io_reserve_memtype_wc() for >>> automatic cleanup of writecombine setup. >>> >>> Several DRM drivers use the non-managed functions for setting their >>> framebuffer memory to write-combine access. Convert ast, mgag200 and >>> vboxvideo. Remove rsp clean-up code form drivers. >>> >>> Tested on mgag200 hardware. >>> >>> Thomas Zimmermann (5): >>> lib: devres: Add managed arch_phys_wc_add() >>> lib: devres: Add managed arch_io_reserve_memtype_wc() >>> drm/ast: Use managed interfaces for framebuffer write combining >>> drm/mgag200: Use managed interfaces for framebuffer write combining >>> drm/vboxvideo: Use managed interfaces for framebuffer write combining >> >> Thanks, the entire series looks good to me: >> >> Reviewed-by: Hans de Goede >> >> For the series. > > Thanks. Let me know if I can review some DRM code for you, as Daniel > suggested. Thank you for the offer, ATM I don't have anything queued up in need of review, but I will probably make use of your offer in the future :) Regards, Hans
Re: [PATCH] drm/ttm: Don't delete the system manager before the delayed delete
Am 17.09.21 um 19:53 schrieb Zack Rusin:
On some hardware, in particular in virtualized environments, the
system memory can be shared with the "hardware". In those cases
the BO's allocated through the ttm system manager might be
busy during ttm_bo_put which results in them being scheduled
for a delayed deletion.
While the patch itself is probably fine the reasoning here is a clear NAK.
Buffers in the system domain are not GPU accessible by definition, even
in a shared environment and so *must* be idle.
Otherwise you break quite a number of assumptions in the code.
Regards,
Christian.
The problem is that that the ttm system manager is disabled
before the final delayed deletion is ran in ttm_device_fini.
This results in crashes during freeing of the BO resources
because they're trying to remove themselves from a no longer
existent ttm_resource_manager (e.g. in IGT's core_hotunplug
on vmwgfx)
In general reloading any driver that could share system mem
resources with "hardware" could hit it because nothing
prevents the system mem resources from being scheduled
for delayed deletion (apart from them not being busy probably
anywhere apart from virtualized environments).
Signed-off-by: Zack Rusin
Cc: Christian Koenig
Cc: Huang Rui
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/ttm/ttm_device.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 9eb8f54b66fc..4ef19cafc755 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -225,10 +225,6 @@ void ttm_device_fini(struct ttm_device *bdev)
struct ttm_resource_manager *man;
unsigned i;
- man = ttm_manager_type(bdev, TTM_PL_SYSTEM);
- ttm_resource_manager_set_used(man, false);
- ttm_set_driver_manager(bdev, TTM_PL_SYSTEM, NULL);
-
mutex_lock(_global_mutex);
list_del(>device_list);
mutex_unlock(_global_mutex);
@@ -238,6 +234,10 @@ void ttm_device_fini(struct ttm_device *bdev)
if (ttm_bo_delayed_delete(bdev, true))
pr_debug("Delayed destroy list was clean\n");
+ man = ttm_manager_type(bdev, TTM_PL_SYSTEM);
+ ttm_resource_manager_set_used(man, false);
+ ttm_set_driver_manager(bdev, TTM_PL_SYSTEM, NULL);
+
spin_lock(>lru_lock);
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
if (list_empty(>lru[0]))
Re: [Intel-gfx] [PATCH] drm/i915: fix blank screen booting crashes
On Mon, 20 Sep 2021, Tvrtko Ursulin wrote:
> On 18/09/2021 00:38, Matthew Brost wrote:
>> From: Hugh Dickins
>>
>> 5.15-rc1 crashes with blank screen when booting up on two ThinkPads
>> using i915. Bisections converge convincingly, but arrive at different
>> and surprising "culprits", none of them the actual culprit.
>
> It is certainly surprising this patch crashed SNB and KBL.
>
> How feasible would it be to make this code just not run when GuC is not
> used? Given the field it adds is called ce->guc_blocked it sounds like a
> natural and preferable thing to do... if possible.
>
>> netconsole (with init_netconsole() hacked to call i915_init() when
>> logging has started, instead of by module_init()) tells the story:
>>
>> kernel BUG at drivers/gpu/drm/i915/i915_sw_fence.c:245!
>> with RSI: 814d408b pointing to sw_fence_dummy_notify().
>> I've been building with CONFIG_CC_OPTIMIZE_FOR_SIZE=y, and that
>> function needs to be 4-byte aligned.
>>
>> v2:
>> (Jani Nikula)
>>- Change BUG_ON to WARN_ON
>
> However in this case the code would then go on and call into a wrong
> function offset which may be worse than a BUG_ON, no?
So how about just
if (WARN_ON(...))
return;
or whatever is needed to give both the user and the CI a better
opportunity to see the error.
BR,
Jani
>
>>
>> Fixes: 62eaf0ae217d ("drm/i915/guc: Support request cancellation")
>> Signed-off-by: Hugh Dickins
>> Signed-off-by: Matthew Brost
>> Reviewed-by: Matthew Brost
>> ---
>> drivers/gpu/drm/i915/gt/intel_context.c | 1 +
>> drivers/gpu/drm/i915/i915_sw_fence.c| 4 +++-
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
>> b/drivers/gpu/drm/i915/gt/intel_context.c
>> index ff637147b1a9..f02c2202da9d 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_context.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
>> @@ -362,6 +362,7 @@ static int __intel_context_active(struct i915_active
>> *active)
>> return 0;
>> }
>>
>> +__aligned(4)/* Respect the I915_SW_FENCE_MASK */
>
> Hugh suggested __i915_sw_fence_call which I think would be the right
> thing to do.
>
> Regards,
>
> Tvrtko
>
>> static int sw_fence_dummy_notify(struct i915_sw_fence *sf,
>> enum i915_sw_fence_notify state)
>> {
>> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c
>> b/drivers/gpu/drm/i915/i915_sw_fence.c
>> index c589a681da77..1217b124c1d0 100644
>> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
>> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
>> @@ -14,8 +14,10 @@
>>
>> #if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
>> #define I915_SW_FENCE_BUG_ON(expr) BUG_ON(expr)
>> +#define I915_SW_FENCE_WARN_ON(expr) WARN_ON(expr)
>> #else
>> #define I915_SW_FENCE_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
>> +#define I915_SW_FENCE_WARN_ON(expr) BUILD_BUG_ON_INVALID(expr)
>> #endif
>>
>> static DEFINE_SPINLOCK(i915_sw_fence_lock);
>> @@ -242,7 +244,7 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
>>const char *name,
>>struct lock_class_key *key)
>> {
>> -BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
>> +I915_SW_FENCE_WARN_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
>>
>> __init_waitqueue_head(>wait, name, key);
>> fence->flags = (unsigned long)fn;
>>
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH] drm/i915: fix blank screen booting crashes
On 20/09/2021 08:38, Jani Nikula wrote:
On Mon, 20 Sep 2021, Tvrtko Ursulin wrote:
On 18/09/2021 00:38, Matthew Brost wrote:
From: Hugh Dickins
5.15-rc1 crashes with blank screen when booting up on two ThinkPads
using i915. Bisections converge convincingly, but arrive at different
and surprising "culprits", none of them the actual culprit.
It is certainly surprising this patch crashed SNB and KBL.
How feasible would it be to make this code just not run when GuC is not
used? Given the field it adds is called ce->guc_blocked it sounds like a
natural and preferable thing to do... if possible.
netconsole (with init_netconsole() hacked to call i915_init() when
logging has started, instead of by module_init()) tells the story:
kernel BUG at drivers/gpu/drm/i915/i915_sw_fence.c:245!
with RSI: 814d408b pointing to sw_fence_dummy_notify().
I've been building with CONFIG_CC_OPTIMIZE_FOR_SIZE=y, and that
function needs to be 4-byte aligned.
v2:
(Jani Nikula)
- Change BUG_ON to WARN_ON
However in this case the code would then go on and call into a wrong
function offset which may be worse than a BUG_ON, no?
So how about just
if (WARN_ON(...))
return;
or whatever is needed to give both the user and the CI a better
opportunity to see the error.
Sounds good to me.
Regards,
Tvrtko
BR,
Jani
Fixes: 62eaf0ae217d ("drm/i915/guc: Support request cancellation")
Signed-off-by: Hugh Dickins
Signed-off-by: Matthew Brost
Reviewed-by: Matthew Brost
---
drivers/gpu/drm/i915/gt/intel_context.c | 1 +
drivers/gpu/drm/i915/i915_sw_fence.c| 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
b/drivers/gpu/drm/i915/gt/intel_context.c
index ff637147b1a9..f02c2202da9d 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -362,6 +362,7 @@ static int __intel_context_active(struct i915_active
*active)
return 0;
}
+__aligned(4) /* Respect the I915_SW_FENCE_MASK */
Hugh suggested __i915_sw_fence_call which I think would be the right
thing to do.
Regards,
Tvrtko
static int sw_fence_dummy_notify(struct i915_sw_fence *sf,
enum i915_sw_fence_notify state)
{
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c
b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..1217b124c1d0 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -14,8 +14,10 @@
#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
#define I915_SW_FENCE_BUG_ON(expr) BUG_ON(expr)
+#define I915_SW_FENCE_WARN_ON(expr) WARN_ON(expr)
#else
#define I915_SW_FENCE_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
+#define I915_SW_FENCE_WARN_ON(expr) BUILD_BUG_ON_INVALID(expr)
#endif
static DEFINE_SPINLOCK(i915_sw_fence_lock);
@@ -242,7 +244,7 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
const char *name,
struct lock_class_key *key)
{
- BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
+ I915_SW_FENCE_WARN_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
__init_waitqueue_head(>wait, name, key);
fence->flags = (unsigned long)fn;
Re: [PATCH] drm/bridge: Move devm_drm_of_get_bridge to bridge/panel.c
Am 17.09.21 um 20:09 schrieb Maxime Ripard:
By depending on devm_drm_panel_bridge_add(), devm_drm_of_get_bridge()
introduces a circular dependency between the modules drm (where
devm_drm_of_get_bridge() ends up) and drm_kms_helper (where
devm_drm_panel_bridge_add() is).
Fix this by moving devm_drm_of_get_bridge() to bridge/panel.c and thus
drm_kms_helper.
Fixes: 87ea95808d53 ("drm/bridge: Add a function to abstract away panels")
Reported-by: Stephen Rothwell
Signed-off-by: Maxime Ripard
Acked-by: Thomas Zimmermann
---
Hi Stephen,
I think it fixes the issue, but couldn't reproduce it here with my
config for some reason.
Let me know if it works for you.
Maxime
---
drivers/gpu/drm/bridge/panel.c | 36 ++
drivers/gpu/drm/drm_bridge.c | 34
2 files changed, 36 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index c916f4b8907e..285a079cdef5 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -332,3 +332,39 @@ struct drm_connector *drm_panel_bridge_connector(struct
drm_bridge *bridge)
return _bridge->connector;
}
EXPORT_SYMBOL(drm_panel_bridge_connector);
+
+#ifdef CONFIG_OF
+/**
+ * devm_drm_of_get_bridge - Return next bridge in the chain
+ * @dev: device to tie the bridge lifetime to
+ * @np: device tree node containing encoder output ports
+ * @port: port in the device tree node
+ * @endpoint: endpoint in the device tree node
+ *
+ * Given a DT node's port and endpoint number, finds the connected node
+ * and returns the associated bridge if any, or creates and returns a
+ * drm panel bridge instance if a panel is connected.
+ *
+ * Returns a pointer to the bridge if successful, or an error pointer
+ * otherwise.
+ */
+struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
+ struct device_node *np,
+ u32 port, u32 endpoint)
+{
+ struct drm_bridge *bridge;
+ struct drm_panel *panel;
+ int ret;
+
+ ret = drm_of_find_panel_or_bridge(np, port, endpoint,
+ , );
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (panel)
+ bridge = devm_drm_panel_bridge_add(dev, panel);
+
+ return bridge;
+}
+EXPORT_SYMBOL(devm_drm_of_get_bridge);
+#endif
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 4c68733fa660..7ee29f073857 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1232,40 +1232,6 @@ struct drm_bridge *of_drm_find_bridge(struct device_node
*np)
return NULL;
}
EXPORT_SYMBOL(of_drm_find_bridge);
-
-/**
- * devm_drm_of_get_bridge - Return next bridge in the chain
- * @dev: device to tie the bridge lifetime to
- * @np: device tree node containing encoder output ports
- * @port: port in the device tree node
- * @endpoint: endpoint in the device tree node
- *
- * Given a DT node's port and endpoint number, finds the connected node
- * and returns the associated bridge if any, or creates and returns a
- * drm panel bridge instance if a panel is connected.
- *
- * Returns a pointer to the bridge if successful, or an error pointer
- * otherwise.
- */
-struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
- struct device_node *np,
- u32 port, u32 endpoint)
-{
- struct drm_bridge *bridge;
- struct drm_panel *panel;
- int ret;
-
- ret = drm_of_find_panel_or_bridge(np, port, endpoint,
- , );
- if (ret)
- return ERR_PTR(ret);
-
- if (panel)
- bridge = devm_drm_panel_bridge_add(dev, panel);
-
- return bridge;
-}
-EXPORT_SYMBOL(devm_drm_of_get_bridge);
#endif
MODULE_AUTHOR("Ajay Kumar ");
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH] drm/ast: Atomic CR/SR reg R/W
Hi
Am 17.09.21 um 09:22 schrieb KuoHsiang Chou:
1. Avoid IO-index racing
2. IO-index racing happened on resolustion switching
and mouse moving at the same time
3. System hung while IO-index racing occurred.
I'd say that there's something else going one here. Mode setting and
cursor movement should be protected against each other by DRM locking.
Changing these low-level functions would not solve the issues. I'll try
to reproduce the problem ASAP.
Best regards
Thomas
Signed-off-by: KuoHsiang Chou
---
drivers/gpu/drm/ast/ast_main.c | 48 +-
1 file changed, 36 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 79a361867..1d8fa70c5 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -41,28 +41,52 @@ void ast_set_index_reg_mask(struct ast_private *ast,
uint32_t base, uint8_t index,
uint8_t mask, uint8_t val)
{
- u8 tmp;
- ast_io_write8(ast, base, index);
- tmp = (ast_io_read8(ast, base + 1) & mask) | val;
- ast_set_index_reg(ast, base, index, tmp);
+ uint16_t volatile usData;
+ uint8_t volatile jData;
+
+ do {
+ ast_io_write8(ast, base, index);
+ usData = ast_io_read16(ast, base);
+ } while ((uint8_t)(usData) != index);
+
+ jData = (uint8_t)(usData >> 8);
+ jData &= mask;
+ jData |= val;
+ usData = ((uint16_t) jData << 8) | (uint16_t) index;
+ ast_io_write16(ast, base, usData);
}
uint8_t ast_get_index_reg(struct ast_private *ast,
uint32_t base, uint8_t index)
{
- uint8_t ret;
- ast_io_write8(ast, base, index);
- ret = ast_io_read8(ast, base + 1);
- return ret;
+ uint16_t volatile usData;
+ uint8_t volatile jData;
+
+ do {
+ ast_io_write8(ast, base, index);
+ usData = ast_io_read16(ast, base);
+ } while ((uint8_t)(usData) != index);
+
+ jData = (uint8_t)(usData >> 8);
+
+ return jData;
}
uint8_t ast_get_index_reg_mask(struct ast_private *ast,
uint32_t base, uint8_t index, uint8_t mask)
{
- uint8_t ret;
- ast_io_write8(ast, base, index);
- ret = ast_io_read8(ast, base + 1) & mask;
- return ret;
+ uint16_t volatile usData;
+ uint8_t volatile jData;
+
+ do {
+ ast_io_write8(ast, base, index);
+ usData = ast_io_read16(ast, base);
+ } while ((uint8_t)(usData) != index);
+
+ jData = (uint8_t)(usData >> 8);
+ jData &= mask;
+
+ return jData;
}
static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
--
2.18.4
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH] drm: bridge: it66121: Fix return value it66121_probe
Hey Alex, Thanks for submitting this. Applied to drm-misc-next.
Re: [PATCH] drm: bridge: it66121: Added it66121 chip external screen status judgment.
Hey Yunlongli,
Thanks for submitting this fix.
On Sat, 18 Sept 2021 at 05:51, Yunlongli wrote:
The formatting of this commit message is a bit unusual, let's try to
change it to the normal formatting.
Remove the dot from the commit title:
"drm: bridge: it66121: Added it66121 chip external screen status
judgment." -> "drm: bridge: it66121: Added it66121 chip external
screen status judgment"
>
> fix: Add further confirm if external screens are involved.
The "fix:" tag is not needed. However if this commit fixes a bug
introduced in an earlier commit a machine readable tag like the the
one below could be added after the commit message.
Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")
>
> log: In the actual tests, the IT66121 chip sometimes misjudged whether
> it had an external screen, so, reference the it66121_user_guid.pdf
> about Audio/Video data is stable or not A typical initialization
> of HDMI link should be based on interrupt signal and appropriate
> register probing. Recommended flow is detailed in IT66121
> Programming Guide. Simply put, the microcontroller should monitor
> the HPD status first. Upon valid HPD event, move on to check
> RxSENDetect register to see if the receiver chip is ready for
> further handshaking. When RxSENDetect is asserted, start reading EDID
> data through DDC channels and carry on the rest of the handshaking
> subsequently.If the micro-controller makes no use of the interrupt
> signal as well as the above-mentioned status registers, the link
> establishment might fail. Please do follow the suggested
> initialization flow recommended in IT66121 Programming Guide.
> So, I add the IT66121_SYS_STATUS_SENDECTECT register status detection.
>
The "log:" prefix is not needed, and neither is the indentation of the text.
Secondly maybe it would be nice to format the above chunk of text into
paragraphs just to make it easier to read.
> Signed-off-by: Yunlongli
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c
> b/drivers/gpu/drm/bridge/ite-it66121.c
> index 2f2a09adb4bc..9ed4fa298d11 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -523,7 +523,7 @@ static bool it66121_is_hpd_detect(struct it66121_ctx *ctx)
> if (regmap_read(ctx->regmap, IT66121_SYS_STATUS_REG, ))
> return false;
>
> - return val & IT66121_SYS_STATUS_HPDETECT;
> + return ((val & IT66121_SYS_STATUS_HPDETECT) && (val &
> IT66121_SYS_STATUS_SENDECTECT));
> }
>
> static int it66121_bridge_attach(struct drm_bridge *bridge,
> --
> 2.20.1
>
>
>
With the above suggestions fixed, feel free to add my r-b and submit a
v2 of this patch.
Reviewed-by: Robert Foss
Re: [PATCH v3 4/6] drm/i915/gt: Register the migrate contexts with their engines
On 14/09/2021 20:31, Thomas Hellström wrote: Pinned contexts, like the migrate contexts need reset after resume since their context image may have been lost. Also the GuC needs to register pinned contexts. Add a list to struct intel_engine_cs where we add all pinned contexts on creation, and traverse that list at resume time to reset the pinned contexts. This fixes the kms_pipe_crc_basic@suspend-read-crc-pipe-a selftest for now, but proper LMEM backup / restore is needed for full suspend functionality. However, note that even with full LMEM backup / restore it may be desirable to keep the reset since backing up the migrate context images must happen using memcpy() after the migrate context has become inactive, and for performance- and other reasons we want to avoid memcpy() from LMEM. Also traverse the list at guc_init_lrc_mapping() calling guc_kernel_context_pin() for the pinned contexts, like is already done for the kernel context. v2: - Don't reset the contexts on each __engine_unpark() but rather at resume time (Chris Wilson). v3: - Reset contexts in the engine sanitize callback. (Chris Wilson) Cc: Tvrtko Ursulin Cc: Matthew Auld Cc: Maarten Lankhorst Cc: Brost Matthew Cc: Chris Wilson Signed-off-by: Thomas Hellström Reviewed-by: Matthew Auld
[Bug 214413] Kernel oops on boot for amdgpu (in si_dpm_set_power_state)
https://bugzilla.kernel.org/show_bug.cgi?id=214413 --- Comment #3 from Marco Piazza (mpia...@gmail.com) --- Created attachment 298885 --> https://bugzilla.kernel.org/attachment.cgi?id=298885=edit Patch to revert ATPX/ATCS global structures -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 214413] Kernel oops on boot for amdgpu (in si_dpm_set_power_state)
https://bugzilla.kernel.org/show_bug.cgi?id=214413 --- Comment #4 from Marco Piazza (mpia...@gmail.com) --- I confirm that using the above patch make the oops disappear. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v3 5/6] drm/i915: Don't back up pinned LMEM context images and rings during suspend
On 14/09/2021 20:31, Thomas Hellström wrote: Pinned context images are now reset during resume. Don't back them up, and assuming that rings can be assumed empty at suspend, don't back them up either. Introduce a new object flag, I915_BO_ALLOC_PM_VOLATILE meaning that an object is allowed to lose its content on suspend. v3: - Slight documentation clarification (Matthew Auld) Signed-off-by: Thomas Hellström Reviewed-by: Matthew Auld
Re: [Intel-gfx] [PATCH 16/26] drm/i915: use new iterator in i915_gem_object_wait_reservation v2
On 17/09/2021 13:35, Christian König wrote:
Simplifying the code a bit.
v2: add missing rcu read unlock.
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/gem/i915_gem_wait.c | 57 ++--
1 file changed, 14 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
index f909aaa09d9c..e416cf528635 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
@@ -37,55 +37,26 @@ i915_gem_object_wait_reservation(struct dma_resv *resv,
unsigned int flags,
long timeout)
{
- struct dma_fence *excl;
- bool prune_fences = false;
-
- if (flags & I915_WAIT_ALL) {
- struct dma_fence **shared;
- unsigned int count, i;
- int ret;
-
- ret = dma_resv_get_fences(resv, , , );
- if (ret)
- return ret;
-
- for (i = 0; i < count; i++) {
- timeout = i915_gem_object_wait_fence(shared[i],
-flags, timeout);
- if (timeout < 0)
- break;
-
- dma_fence_put(shared[i]);
- }
-
- for (; i < count; i++)
- dma_fence_put(shared[i]);
- kfree(shared);
-
- /*
-* If both shared fences and an exclusive fence exist,
-* then by construction the shared fences must be later
-* than the exclusive fence. If we successfully wait for
-* all the shared fences, we know that the exclusive fence
-* must all be signaled. If all the shared fences are
-* signaled, we can prune the array and recover the
-* floating references on the fences/requests.
-*/
- prune_fences = count && timeout >= 0;
- } else {
- excl = dma_resv_get_excl_unlocked(resv);
+ struct dma_resv_iter cursor;
+ struct dma_fence *fence;
+
+ rcu_read_lock();
+ dma_resv_iter_begin(, resv, flags & I915_WAIT_ALL);
+ dma_resv_for_each_fence_unlocked(, fence) {
+ rcu_read_unlock();
+ timeout = i915_gem_object_wait_fence(fence, flags, timeout);
Converting this one could be problematic. It's the wait ioctl which used
to grab an atomic snapshot and wait for that rendering to complete. With
this change I think it has the potential to run forever keeps catching
new activity against the same object.
I am not sure whether or not the difference is relevant for how
userspace uses it but I think needs discussion.
Hm actually there are internal callers as well, and at least some of
those have the object locked. Would a wider refactoring to separate
those into buckets (locked vs unlocked) make sense?
Regards,
Tvrtko
+ rcu_read_lock();
+ if (timeout < 0)
+ break;
}
-
- if (excl && timeout >= 0)
- timeout = i915_gem_object_wait_fence(excl, flags, timeout);
-
- dma_fence_put(excl);
+ dma_resv_iter_end();
+ rcu_read_unlock();
/*
* Opportunistically prune the fences iff we know they have *all* been
* signaled.
*/
- if (prune_fences)
+ if (timeout > 0)
dma_resv_prune(resv);
return timeout;
Re: [Intel-gfx] [PATCH 01/26] dma-buf: add dma_resv_for_each_fence_unlocked v2
Am 20.09.21 um 10:43 schrieb Tvrtko Ursulin:
On 17/09/2021 14:23, Daniel Vetter wrote:
On Fri, Sep 17, 2021 at 02:34:48PM +0200, Christian König wrote:
Abstract the complexity of iterating over all the fences
in a dma_resv object.
The new loop handles the whole RCU and retry dance and
returns only fences where we can be sure we grabbed the
right one.
v2: fix accessing the shared fences while they might be freed,
improve kerneldoc, rename _cursor to _iter, add
dma_resv_iter_is_exclusive, add dma_resv_iter_begin/end
Signed-off-by: Christian König
---
drivers/dma-buf/dma-resv.c | 61 +++
include/linux/dma-resv.h | 84
++
2 files changed, 145 insertions(+)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 84fbe60629e3..3e77cad2c9d4 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -323,6 +323,67 @@ void dma_resv_add_excl_fence(struct dma_resv
*obj, struct dma_fence *fence)
}
EXPORT_SYMBOL(dma_resv_add_excl_fence);
+/**
+ * dma_resv_iter_walk_unlocked - walk over fences in a dma_resv obj
+ * @cursor: cursor to record the current position
+ * @first: if we should start over
+ *
+ * Return all the fences in the dma_resv object which are not yet
signaled.
+ * The returned fence has an extra local reference so will stay alive.
+ * If a concurrent modify is detected the whole iterration is
started over again.
+ */
+struct dma_fence *dma_resv_iter_walk_unlocked(struct dma_resv_iter
*cursor,
Bit ocd, but I'd still just call that iter_next.
+ bool first)
Hm I'd put all the init code into iter_begin ...
@Christian:
Could you engineer something in here which would, at least in debug
builds, catch failures to call "iter begin" before using the iterator
macro?
Yeah, I've already played with the thought of somehow teaching lockdep
that. But then abandoned this as abusive of lockdep.
+{
+ struct dma_resv *obj = cursor->obj;
Aren't we missing rcu_read_lock() around the entire thing here?
+
+ first |= read_seqcount_retry(>seq, cursor->seq);
+ do {
+ /* Drop the reference from the previous round */
+ dma_fence_put(cursor->fence);
+
+ cursor->is_first = first;
+ if (first) {
+ cursor->seq = read_seqcount_begin(>seq);
+ cursor->index = -1;
+ cursor->fences = dma_resv_shared_list(obj);
And then also call iter_begin from here. That way we guarantee that
read_seqcount_begin is always called before _retry(). It's not a problem
with the seqcount implementation (I think at least), but it definitely
looks funny.
Calling iter_begin here also makes it clear that we're essentially
restarting.
+
+ cursor->fence = dma_resv_excl_fence(obj);
+ if (cursor->fence &&
+ test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
Please use the right dma_fence wrapper here for this and don't look
at the
bits/flags outside of dma_fence.[hc] code. I just realized that we don't
have the right amount of barriers in there for the fastpath, i.e. if we
have:
x = 0; /* static initializer */
thread a
x = 1;
dma_fence_signal(fence);
thread b;
if (dma_fence_is_signalled(fence))
printk("%i\n", x);
Then you might actually be able to observe x == 0 in thread b. Which is
not what we want at all.
@Daniel:
What do you mean here - in terms of if 'x' is "external" (not part of
dma-fence), then are you suggesting dma-fence code should serialise it
by using barriers?
That would sound incorrect to me, or in other words, I think it's fine
if x == 0 is observed in your example thread B since that code is
mixing external data with dma-fence.
No, Daniel is right. The problem is that on architectures other than x86
barriers are per memory address (or rather cache line in practice).
So you need to be really careful that you see the fully consistent state
and not just one variable but others in the old state.
But this was buggy before as well. I'm just pulling the existing test
into the new iterator.
Hm also, there is that annoying bit where by using
dma_fence_is_signaled any code becomes a fence signaling critical
path, which I never bought into. There should be a way to test the
signaled status without actually doing the signaling. Or I am
misunderstanding something so badly that is really really has to be
like this?
You are mixing things up. Testing is unproblematic, signaling is the
problematic one.
So no open-coding of dma_fence flag bits code outside of drm_fence.[hc]
please. And yes i915-gem code is unfortunately a disaster.
Don't even miss an opportunity for some good trashing no? :D
But yes, deconstructed dma_fence_signal I thought we were supposed to
add to core. Or at least propose, don't exactly remember how that went.
The problem is that you need to grab a reference to call
dma_fence_signal while testing the
[PATCH] doc: gpu: drm-internals: Create reference to DRM mm
This short sentence references nothing for details about memory manager. Replace it with the documentation file for DRM memory management. Cc: Jani Nikula Signed-off-by: Markus Schneider-Pargmann --- Documentation/gpu/drm-internals.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index 06af044c882f..bdcdfc4ede04 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -126,8 +126,8 @@ Memory Manager Initialization Every DRM driver requires a memory manager which must be initialized at load time. DRM currently contains two memory managers, the Translation Table Manager (TTM) and the Graphics Execution Manager (GEM). This -document describes the use of the GEM memory manager only. See ? for -details. +document describes the use of the GEM memory manager only. See +Documentation/gpu/drm-mm.rst for details. Miscellaneous Device Configuration ~~ -- 2.33.0
Re: [Intel-gfx] [PATCH 13/26] drm/i915: use the new iterator in i915_gem_busy_ioctl
Am 20.09.21 um 10:45 schrieb Tvrtko Ursulin:
On 17/09/2021 13:35, Christian König wrote:
This makes the function much simpler since the complex
retry logic is now handled else where.
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/gem/i915_gem_busy.c | 32
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
index 6234e17259c1..b1cb7ba688da 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
@@ -82,8 +82,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void
*data,
{
struct drm_i915_gem_busy *args = data;
struct drm_i915_gem_object *obj;
- struct dma_resv_list *list;
- unsigned int seq;
+ struct dma_resv_iter cursor;
+ struct dma_fence *fence;
int err;
err = -ENOENT;
@@ -109,27 +109,17 @@ i915_gem_busy_ioctl(struct drm_device *dev,
void *data,
* to report the overall busyness. This is what the wait-ioctl
does.
*
*/
-retry:
- seq = raw_read_seqcount(>base.resv->seq);
-
- /* Translate the exclusive fence to the READ *and* WRITE engine */
- args->busy =
busy_check_writer(dma_resv_excl_fence(obj->base.resv));
-
- /* Translate shared fences to READ set of engines */
- list = dma_resv_shared_list(obj->base.resv);
- if (list) {
- unsigned int shared_count = list->shared_count, i;
-
- for (i = 0; i < shared_count; ++i) {
- struct dma_fence *fence =
- rcu_dereference(list->shared[i]);
-
+ args->busy = false;
+ dma_resv_iter_begin(, obj->base.resv, true);
+ dma_resv_for_each_fence_unlocked(, fence) {
You did not agree with my suggestion to reset args->busy on restart
and so preserve current behaviour?
No, I want to keep the restart behavior internally to the dma_resv
object and as far as I can see it should not make a difference here.
Regards,
Christian.
Regards,
Tvrtko
+ if (dma_resv_iter_is_exclusive())
+ /* Translate the exclusive fence to the READ *and* WRITE
engine */
+ args->busy = busy_check_writer(fence);
+ else
+ /* Translate shared fences to READ set of engines */
args->busy |= busy_check_reader(fence);
- }
}
-
- if (args->busy && read_seqcount_retry(>base.resv->seq, seq))
- goto retry;
+ dma_resv_iter_end();
err = 0;
out:
Re: [Intel-gfx] [PATCH 14/26] drm/i915: use the new iterator in i915_sw_fence_await_reservation v3
Am 20.09.21 um 10:47 schrieb Tvrtko Ursulin:
On 20/09/2021 09:45, Tvrtko Ursulin wrote:
On 17/09/2021 13:35, Christian König wrote:
Simplifying the code a bit.
v2: use dma_resv_for_each_fence instead, according to Tvrtko the
lock is
held here anyway.
v3: back to using dma_resv_for_each_fence_unlocked.
It did not work out - what happened?
Wait, my suggestion to try the locked iterator was against
i915_request_await_object. I haven't looked at this one at the time or
even now.
Exactly! I've mixed the two up and this one here is really called
without holding a lock.
Regards,
Christian.
Regards,
Tvrtko
Regards,
Tvrtko
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/i915_sw_fence.c | 57
1 file changed, 15 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c
b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..7635b0478ea5 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -572,56 +572,29 @@ int i915_sw_fence_await_reservation(struct
i915_sw_fence *fence,
unsigned long timeout,
gfp_t gfp)
{
- struct dma_fence *excl;
+ struct dma_resv_iter cursor;
+ struct dma_fence *f;
int ret = 0, pending;
debug_fence_assert(fence);
might_sleep_if(gfpflags_allow_blocking(gfp));
- if (write) {
- struct dma_fence **shared;
- unsigned int count, i;
-
- ret = dma_resv_get_fences(resv, , , );
- if (ret)
- return ret;
-
- for (i = 0; i < count; i++) {
- if (shared[i]->ops == exclude)
- continue;
-
- pending = i915_sw_fence_await_dma_fence(fence,
- shared[i],
- timeout,
- gfp);
- if (pending < 0) {
- ret = pending;
- break;
- }
-
- ret |= pending;
- }
-
- for (i = 0; i < count; i++)
- dma_fence_put(shared[i]);
- kfree(shared);
- } else {
- excl = dma_resv_get_excl_unlocked(resv);
- }
-
- if (ret >= 0 && excl && excl->ops != exclude) {
- pending = i915_sw_fence_await_dma_fence(fence,
- excl,
- timeout,
+ rcu_read_lock();
+ dma_resv_iter_begin(, resv, write);
+ dma_resv_for_each_fence_unlocked(, f) {
+ rcu_read_unlock();
+ pending = i915_sw_fence_await_dma_fence(fence, f, timeout,
gfp);
- if (pending < 0)
+ rcu_read_lock();
+ if (pending < 0) {
ret = pending;
- else
- ret |= pending;
- }
-
- dma_fence_put(excl);
+ break;
+ }
+ ret |= pending;
+ }
+ dma_resv_iter_end();
+ rcu_read_unlock();
return ret;
}
Re: [Intel-gfx] [PATCH 01/26] dma-buf: add dma_resv_for_each_fence_unlocked v2
On 20/09/2021 11:09, Christian König wrote:
Am 20.09.21 um 10:43 schrieb Tvrtko Ursulin:
On 17/09/2021 14:23, Daniel Vetter wrote:
On Fri, Sep 17, 2021 at 02:34:48PM +0200, Christian König wrote:
Abstract the complexity of iterating over all the fences
in a dma_resv object.
The new loop handles the whole RCU and retry dance and
returns only fences where we can be sure we grabbed the
right one.
v2: fix accessing the shared fences while they might be freed,
improve kerneldoc, rename _cursor to _iter, add
dma_resv_iter_is_exclusive, add dma_resv_iter_begin/end
Signed-off-by: Christian König
---
drivers/dma-buf/dma-resv.c | 61 +++
include/linux/dma-resv.h | 84
++
2 files changed, 145 insertions(+)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 84fbe60629e3..3e77cad2c9d4 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -323,6 +323,67 @@ void dma_resv_add_excl_fence(struct dma_resv
*obj, struct dma_fence *fence)
}
EXPORT_SYMBOL(dma_resv_add_excl_fence);
+/**
+ * dma_resv_iter_walk_unlocked - walk over fences in a dma_resv obj
+ * @cursor: cursor to record the current position
+ * @first: if we should start over
+ *
+ * Return all the fences in the dma_resv object which are not yet
signaled.
+ * The returned fence has an extra local reference so will stay alive.
+ * If a concurrent modify is detected the whole iterration is
started over again.
+ */
+struct dma_fence *dma_resv_iter_walk_unlocked(struct dma_resv_iter
*cursor,
Bit ocd, but I'd still just call that iter_next.
+ bool first)
Hm I'd put all the init code into iter_begin ...
@Christian:
Could you engineer something in here which would, at least in debug
builds, catch failures to call "iter begin" before using the iterator
macro?
Yeah, I've already played with the thought of somehow teaching lockdep
that. But then abandoned this as abusive of lockdep.
Yes probably not lockdep but would need to be a separate build time
option akin to DEBUG_WW_MUTEXES and similar.
+{
+ struct dma_resv *obj = cursor->obj;
Aren't we missing rcu_read_lock() around the entire thing here?
+
+ first |= read_seqcount_retry(>seq, cursor->seq);
+ do {
+ /* Drop the reference from the previous round */
+ dma_fence_put(cursor->fence);
+
+ cursor->is_first = first;
+ if (first) {
+ cursor->seq = read_seqcount_begin(>seq);
+ cursor->index = -1;
+ cursor->fences = dma_resv_shared_list(obj);
And then also call iter_begin from here. That way we guarantee that
read_seqcount_begin is always called before _retry(). It's not a problem
with the seqcount implementation (I think at least), but it definitely
looks funny.
Calling iter_begin here also makes it clear that we're essentially
restarting.
+
+ cursor->fence = dma_resv_excl_fence(obj);
+ if (cursor->fence &&
+ test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
Please use the right dma_fence wrapper here for this and don't look
at the
bits/flags outside of dma_fence.[hc] code. I just realized that we don't
have the right amount of barriers in there for the fastpath, i.e. if we
have:
x = 0; /* static initializer */
thread a
x = 1;
dma_fence_signal(fence);
thread b;
if (dma_fence_is_signalled(fence))
printk("%i\n", x);
Then you might actually be able to observe x == 0 in thread b. Which is
not what we want at all.
@Daniel:
What do you mean here - in terms of if 'x' is "external" (not part of
dma-fence), then are you suggesting dma-fence code should serialise it
by using barriers?
That would sound incorrect to me, or in other words, I think it's fine
if x == 0 is observed in your example thread B since that code is
mixing external data with dma-fence.
No, Daniel is right. The problem is that on architectures other than x86
barriers are per memory address (or rather cache line in practice).
So you need to be really careful that you see the fully consistent state
and not just one variable but others in the old state.
I don't see it yet - what are the variables we are talking about here?
Ordering relating to the iterator code in here or something truly external?
Iterator can obviously race and "return" and already signaled fence
(transitioned from unsignaled to signaled between iterator checking and
deciding to walk it). But that I don't think you can, or plan to, fix.
But this was buggy before as well. I'm just pulling the existing test
into the new iterator.
Okay.
Hm also, there is that annoying bit where by using
dma_fence_is_signaled any code becomes a fence signaling critical
path, which I never bought into. There should be a way to test the
signaled status without actually doing the signaling. Or I am
misunderstanding something so badly that is really really has to be
Re: [Intel-gfx] [PATCH 13/26] drm/i915: use the new iterator in i915_gem_busy_ioctl
On 20/09/2021 11:13, Christian König wrote:
Am 20.09.21 um 10:45 schrieb Tvrtko Ursulin:
On 17/09/2021 13:35, Christian König wrote:
This makes the function much simpler since the complex
retry logic is now handled else where.
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/gem/i915_gem_busy.c | 32
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
index 6234e17259c1..b1cb7ba688da 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
@@ -82,8 +82,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void
*data,
{
struct drm_i915_gem_busy *args = data;
struct drm_i915_gem_object *obj;
- struct dma_resv_list *list;
- unsigned int seq;
+ struct dma_resv_iter cursor;
+ struct dma_fence *fence;
int err;
err = -ENOENT;
@@ -109,27 +109,17 @@ i915_gem_busy_ioctl(struct drm_device *dev,
void *data,
* to report the overall busyness. This is what the wait-ioctl
does.
*
*/
-retry:
- seq = raw_read_seqcount(>base.resv->seq);
-
- /* Translate the exclusive fence to the READ *and* WRITE engine */
- args->busy =
busy_check_writer(dma_resv_excl_fence(obj->base.resv));
-
- /* Translate shared fences to READ set of engines */
- list = dma_resv_shared_list(obj->base.resv);
- if (list) {
- unsigned int shared_count = list->shared_count, i;
-
- for (i = 0; i < shared_count; ++i) {
- struct dma_fence *fence =
- rcu_dereference(list->shared[i]);
-
+ args->busy = false;
+ dma_resv_iter_begin(, obj->base.resv, true);
+ dma_resv_for_each_fence_unlocked(, fence) {
You did not agree with my suggestion to reset args->busy on restart
and so preserve current behaviour?
No, I want to keep the restart behavior internally to the dma_resv
object and as far as I can see it should not make a difference here.
To be clear, on paper difference between old and new implementation is
if the restart happens while processing the shared fences.
Old implementation unconditionally goes to "args->busy =
>>> busy_check_writer(dma_resv_excl_fence(obj->base.resv));" and so
overwrites the set of flags returned to userspace.
New implementation can merge new read flags to the old set of flags and
so return a composition of past and current fences.
Maybe it does not matter hugely in this case, depends if userspace
typically just restarts until flags are clear. But I am not sure.
On the higher level - what do you mean with wanting to keep the restart
behaviour internal? Not providing iterators users means of detecting it?
I think it has to be provided.
Regards,
Tvrtko
Regards,
Christian.
Regards,
Tvrtko
+ if (dma_resv_iter_is_exclusive())
+ /* Translate the exclusive fence to the READ *and* WRITE
engine */
+ args->busy = busy_check_writer(fence);
+ else
+ /* Translate shared fences to READ set of engines */
args->busy |= busy_check_reader(fence);
- }
}
-
- if (args->busy && read_seqcount_retry(>base.resv->seq, seq))
- goto retry;
+ dma_resv_iter_end();
err = 0;
out:
Re: [PATCH v2 4/9] vfio/ccw: Make the FSM complete and synchronize it to the mdev
On Mon, Sep 20, 2021 at 02:19:18PM +0200, Cornelia Huck wrote: > On Thu, Sep 09 2021, Jason Gunthorpe wrote: > > > The subchannel should be left in a quiescent state unless the VFIO device > > FD is opened. When the FD is opened bring the chanel to active and allow > > the VFIO device to operate. When the device FD is closed then quiesce the > > channel. > > > > To make this work the FSM needs to handle the transitions to/from open and > > closed so everything is sequenced. Rename state NOT_OPER to BROKEN and use > > it wheneven the driver has malfunctioned. STANDBY becomes CLOSED. The > > normal case FSM looks like: > > CLOSED -> IDLE -> PROCESS/PENDING* -> IDLE -> CLOSED > > > > With a possible branch off to BROKEN from any state. Once the device is in > > BROKEN it cannot be recovered other than be reloading the driver. > > Hm, not sure whether it is a good idea to conflate "something went > wrong" and "device is not operational". Yes, but that is exactly what this FSM is currently doing, NO_OPER is a dumping ground for all kinds of wonky stuff, and what exactly it is supposed to mean or do is unclear. > while the former case could mean all kind of > things, but the subchannel will likely stay around. I think NOT_OPER > was always meant to be a transitional state. Then these sorts of failures should recover the device and FSM back to an appropriate operational state and keep going - but I'm not going to attempt to guess when each of the conditions are recoverable or not. > > Delete the triply redundant calls to > > vfio_ccw_sch_quiesce(). vfio_ccw_mdev_close_device() always leaves the > > subchannel quiescent. vfio_ccw_mdev_remove() cannot return until > > vfio_ccw_mdev_close_device() completes and vfio_ccw_sch_remove() cannot > > return until vfio_ccw_mdev_remove() completes. Have the FSM code take care > > of calling cp_free() when appropriate. > > I remember some serialization issues wrt cp_free() etc. coming up every > now and than; that might need extra care (I'm taking a look.) I'm not too surprised, things like NOT_OPER just exiting the usual FSM logic mean stuff couldn't be properly sequenced. The new logic puts a cp_free in each of arcs entering the terminal states broken/closed and all the flows that would get us to vfio_ccw_mdev_remove() will enter one of those states. It is quite possible this patch needs someone who actually understand this HW to polish it up - the point was to show how ccw should be cleanly structured. I'd like to go ahead with the other patches and leave this for the ccw maintainers if it is needs significant work. The other patches are what are blocking the core code cleanups. Jason
[Bug 214413] Kernel oops on boot for amdgpu (in si_dpm_set_power_state)
https://bugzilla.kernel.org/show_bug.cgi?id=214413 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #5 from Alex Deucher (alexdeuc...@gmail.com) --- Can provide the info requested in: https://gitlab.freedesktop.org/drm/amd/-/issues/1698#note_1066944 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [Intel-gfx] [PATCH] drm/i915: Make wa list per-gt
On Fri, Sep 17, 2021 at 10:08:45AM -0700, Matt Roper wrote:
> From: Venkata Sandeep Dhanalakota
>
> Support for multiple GT's within a single i915 device will be arriving
> soon. Since each GT may have its own fusing and require different
> workarounds, we need to make the GT workaround functions and multicast
> steering setup per-gt.
>
> Cc: Tvrtko Ursulin
> Cc: Daniele Ceraolo Spurio
> Signed-off-by: Venkata Sandeep Dhanalakota
> Signed-off-by: Matt Roper
Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c| 3 +
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 2 +
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 143 +-
> drivers/gpu/drm/i915/gt/intel_workarounds.h | 2 +-
> .../gpu/drm/i915/gt/selftest_workarounds.c| 2 +-
> drivers/gpu/drm/i915/i915_drv.c | 2 -
> drivers/gpu/drm/i915/i915_drv.h | 2 -
> drivers/gpu/drm/i915/i915_gem.c | 2 -
> 8 files changed, 81 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 55e87aff51d2..449ff6e83543 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -660,6 +660,8 @@ int intel_gt_init(struct intel_gt *gt)
> if (err)
> return err;
>
> + intel_gt_init_workarounds(gt);
> +
> /*
>* This is just a security blanket to placate dragons.
>* On some systems, we very sporadically observe that the first TLBs
> @@ -767,6 +769,7 @@ void intel_gt_driver_release(struct intel_gt *gt)
> if (vm) /* FIXME being called twice on error paths :( */
> i915_vm_put(vm);
>
> + intel_wa_list_free(>wa_list);
> intel_gt_pm_fini(gt);
> intel_gt_fini_scratch(gt);
> intel_gt_fini_buffer_pool(gt);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index 6fdcde64c180..ce127cae9e49 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -72,6 +72,8 @@ struct intel_gt {
>
> struct intel_uc uc;
>
> + struct i915_wa_list wa_list;
> +
> struct intel_gt_timelines {
> spinlock_t lock; /* protects active_list */
> struct list_head active_list;
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index c314d4917b6b..1f0a54b383d9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -804,7 +804,7 @@ int intel_engine_emit_ctx_wa(struct i915_request *rq)
> }
>
> static void
> -gen4_gt_workarounds_init(struct drm_i915_private *i915,
> +gen4_gt_workarounds_init(struct intel_gt *gt,
>struct i915_wa_list *wal)
> {
> /* WaDisable_RenderCache_OperationalFlush:gen4,ilk */
> @@ -812,29 +812,29 @@ gen4_gt_workarounds_init(struct drm_i915_private *i915,
> }
>
> static void
> -g4x_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
> *wal)
> +g4x_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> {
> - gen4_gt_workarounds_init(i915, wal);
> + gen4_gt_workarounds_init(gt, wal);
>
> /* WaDisableRenderCachePipelinedFlush:g4x,ilk */
> wa_masked_en(wal, CACHE_MODE_0, CM0_PIPELINED_RENDER_FLUSH_DISABLE);
> }
>
> static void
> -ilk_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
> *wal)
> +ilk_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> {
> - g4x_gt_workarounds_init(i915, wal);
> + g4x_gt_workarounds_init(gt, wal);
>
> wa_masked_en(wal, _3D_CHICKEN2, _3D_CHICKEN2_WM_READ_PIPELINED);
> }
>
> static void
> -snb_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
> *wal)
> +snb_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> {
> }
>
> static void
> -ivb_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
> *wal)
> +ivb_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> {
> /* Apply the WaDisableRHWOOptimizationForRenderHang:ivb workaround. */
> wa_masked_dis(wal,
> @@ -850,7 +850,7 @@ ivb_gt_workarounds_init(struct drm_i915_private *i915,
> struct i915_wa_list *wal)
> }
>
> static void
> -vlv_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
> *wal)
> +vlv_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> {
> /* WaForceL3Serialization:vlv */
> wa_write_clr(wal, GEN7_L3SQCREG4, L3SQ_URB_READ_CAM_MATCH_DISABLE);
> @@ -863,7 +863,7 @@ vlv_gt_workarounds_init(struct drm_i915_private *i915,
> struct i915_wa_list *wal)
> }
>
> static void
> -hsw_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list
> *wal)
> +hsw_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> {
Re: Regression with mainline kernel on rpi4
On Mon, Sep 20, 2021 at 05:43:33PM +0200, Maxime Ripard wrote: > On Mon, Sep 20, 2021 at 04:47:31PM +0200, Maxime Ripard wrote: > > On Sat, Sep 04, 2021 at 10:40:29AM +0100, Sudip Mukherjee wrote: > > > Hi Maxime, > > > > > > On Sat, Sep 4, 2021 at 10:10 AM Maxime Ripard wrote: > > > > > > > > On Fri, Sep 03, 2021 at 09:09:50PM +0100, Sudip Mukherjee wrote: > > > > > Hi Maxime, > > > > > > > > > > On Fri, Sep 3, 2021 at 5:03 PM Maxime Ripard > > > > > wrote: > > > > > > > > > > > > Hi Sudip, > > > > > > > > > > > > On Thu, Sep 02, 2021 at 10:08:19AM +0100, Sudip Mukherjee wrote: > > > > > > > Hi All, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You can see the complete dmesg at > > > > > > > https://openqa.qa.codethink.co.uk/tests/76#step/dmesg/8 > > > > > > > > > > > > What test were you running? > > > > > > > > > > Nothing particular, its just a boot test that we do every night after > > > > > pulling from torvalds/linux.git > > > > > > > > What are you booting to then? > > > > > > I am not sure I understood the question. > > > Its an Ubuntu image. The desktop environment is gnome. And as > > > mentioned earlier, we use the HEAD of linus tree every night to boot > > > the rpi4 and test that we can login via desktop environment and that > > > there is no regression in dmesg. > > > > Looking at the CI, this isn't from a RPi but from qemu? > > > > What defconfig are you using? How did you generate the Ubuntu image? > > Through debootstrap? Any additional package? > > So qemu (at least on Fedora 34) doesn't seem to have an RPI4 target, nor > upstream: > https://github.com/qemu/qemu/blob/079b1252e9de384385c9da910262312ec2e574c8/hw/arm/raspi.c#L367 > > I've tested an Ubuntu 21.04 arm64 build (since it seems like it's what > you've been using), built using debootstrap + ubuntu-desktop, both with > and without a monitor attached, and up to the desktop once logged in. > > I don't see any crash. That was with arm64's defconfig. Maxime signature.asc Description: PGP signature
[Bug 211277] sometimes crash at s2ram-wake (Ryzen 3500U): amdgpu, drm, commit_tail, amdgpu_dm_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=211277 --- Comment #61 from James Zhu (jam...@amd.com) --- (In reply to youling257 from comment #60) > Created attachment 298889 [details] > dmesg5.15.txt > > (In reply to James Zhu from comment #59) > > (In reply to youling257 from comment #58) > > > drm/amdgpu: move iommu_resume before ip init/resume cause suspend to disk > > > resume failed on my amdgpu 3400g. > > > > Can you share whole demsg log? Regards! James > > when resume failed have to force shutdown, how to output dmesg? > only has boot log dmesg. after reboot, you can find under /var/log/kern.log and /var/log/syslog based on timestamp. you can just attach kern.log -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH] staging: fbtft: add docs for fbtft_write_spi()
On Mon, Sep 20, 2021 at 08:26:03AM -0700, Zachary Mayhew wrote:
> Subject: [PATCH] staging: fbtft: add docs for fbtft_write_spi()
Odd, this shouldn't be in the body of the email :(
>
> This patch adds documentation for fbtft_write_spi() to make its
> calling context clear and explain what it does.
>
> Signed-off-by: Zachary Mayhew
> ---
> drivers/staging/fbtft/fbtft-io.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/staging/fbtft/fbtft-io.c
> b/drivers/staging/fbtft/fbtft-io.c
> index de1904a443c2..985d7cf8c774 100644
> --- a/drivers/staging/fbtft/fbtft-io.c
> +++ b/drivers/staging/fbtft/fbtft-io.c
> @@ -5,6 +5,19 @@
> #include
> #include "fbtft.h"
>
> +/**
> + * fbtft_write_spi() - write data to current spi
> + * @par: Driver data including driver spi_device
> + * @buf: Buffer to write to spi
> + * @len: Length of the buffer
> + * Context: can sleep
> + *
> + * Builds an spi_transfer and spi_message object based on the
> + * given @buf and @len. These are then used in a call to spi_sync() which
> will
> + * write to the spi.
> + *
> + * Return: zero on success or else a negative error code
> + */
> int fbtft_write_spi(struct fbtft_par *par, void *buf, size_t len)
> {
> struct spi_transfer t = {
> --
> 2.33.0
>
>
Is this file being imported into the kernel doc tools? If so, great, if
not, this isn't going to help out all that much, right?
thanks,
greg k-h
Re: [PATCH] drm/amd/display: fix empty debug macros
On Mon, Sep 20, 2021 at 8:16 AM Arnd Bergmann wrote:
>
> From: Arnd Bergmann
>
> Using an empty macro expansion as a conditional expression
> produces a W=1 warning:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c: In function
> 'dce_aux_transfer_with_retries':
> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:775:156: error:
> suggest braces around empty body in an 'if' statement [-Werror=empty-body]
> 775 |
> "dce_aux_transfer_with_retries: AUX_RET_SUCCESS:
> AUX_TRANSACTION_REPLY_I2C_OVER_AUX_DEFER");
> |
>
>^
> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:783:155: error:
> suggest braces around empty body in an 'if' statement [-Werror=empty-body]
> 783 |
> "dce_aux_transfer_with_retries: AUX_RET_SUCCESS:
> AUX_TRANSACTION_REPLY_I2C_OVER_AUX_NACK");
> |
>
> ^
>
> Expand it to "do { } while (0)" instead to make the expression
> more robust and avoid the warning.
>
> Fixes: 56aca2309301 ("drm/amd/display: Add AUX I2C tracing.")
> Signed-off-by: Arnd Bergmann
Applied. Thanks!
Alex
> ---
> drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> index e14f99b4b0c3..3c3347341103 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> @@ -42,7 +42,7 @@
> #define DC_LOGGER \
> engine->ctx->logger
>
> -#define DC_TRACE_LEVEL_MESSAGE(...) /* do nothing */
> +#define DC_TRACE_LEVEL_MESSAGE(...) do { } while (0)
> #define IS_DC_I2CAUX_LOGGING_ENABLED() (false)
> #define LOG_FLAG_Error_I2cAux LOG_ERROR
> #define LOG_FLAG_I2cAux_DceAux LOG_I2C_AUX
> @@ -76,7 +76,7 @@ enum {
> #define DEFAULT_AUX_ENGINE_MULT 0
> #define DEFAULT_AUX_ENGINE_LENGTH 69
>
> -#define DC_TRACE_LEVEL_MESSAGE(...) /* do nothing */
> +#define DC_TRACE_LEVEL_MESSAGE(...) do { } while (0)
>
> static void release_engine(
> struct dce_aux *engine)
> --
> 2.29.2
>
[PATCH 4/9] drm/msm: simplify getting .driver_data
We should get 'driver_data' from 'struct device' directly. Going via
platform_device is an unneeded step back and forth.
Signed-off-by: Wolfram Sang
---
Build tested only. buildbot is happy.
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 13 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 ++
drivers/gpu/drm/msm/dp/dp_display.c | 6 ++
drivers/gpu/drm/msm/dsi/dsi_host.c | 6 ++
drivers/gpu/drm/msm/msm_drv.c| 3 +--
5 files changed, 12 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index ae48f41821cf..32410bd299e7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1185,16 +1185,15 @@ static int dpu_bind(struct device *dev, struct device
*master, void *data)
static void dpu_unbind(struct device *dev, struct device *master, void *data)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
+ struct dpu_kms *dpu_kms = dev_get_drvdata(dev);
struct dss_module_power *mp = _kms->mp;
msm_dss_put_clk(mp->clk_config, mp->num_clk);
- devm_kfree(>dev, mp->clk_config);
+ devm_kfree(dev, mp->clk_config);
mp->num_clk = 0;
if (dpu_kms->rpm_enabled)
- pm_runtime_disable(>dev);
+ pm_runtime_disable(dev);
}
static const struct component_ops dpu_ops = {
@@ -1216,8 +1215,7 @@ static int dpu_dev_remove(struct platform_device *pdev)
static int __maybe_unused dpu_runtime_suspend(struct device *dev)
{
int i, rc = -1;
- struct platform_device *pdev = to_platform_device(dev);
- struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
+ struct dpu_kms *dpu_kms = dev_get_drvdata(dev);
struct dss_module_power *mp = _kms->mp;
/* Drop the performance state vote */
@@ -1235,8 +1233,7 @@ static int __maybe_unused dpu_runtime_suspend(struct
device *dev)
static int __maybe_unused dpu_runtime_resume(struct device *dev)
{
int rc = -1;
- struct platform_device *pdev = to_platform_device(dev);
- struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
+ struct dpu_kms *dpu_kms = dev_get_drvdata(dev);
struct drm_encoder *encoder;
struct drm_device *ddev;
struct dss_module_power *mp = _kms->mp;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index b3b42672b2d4..3db9d1603dfe 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -1015,8 +1015,7 @@ static int mdp5_dev_remove(struct platform_device *pdev)
static __maybe_unused int mdp5_runtime_suspend(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct mdp5_kms *mdp5_kms = platform_get_drvdata(pdev);
+ struct mdp5_kms *mdp5_kms = dev_get_drvdata(dev);
DBG("");
@@ -1025,8 +1024,7 @@ static __maybe_unused int mdp5_runtime_suspend(struct
device *dev)
static __maybe_unused int mdp5_runtime_resume(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct mdp5_kms *mdp5_kms = platform_get_drvdata(pdev);
+ struct mdp5_kms *mdp5_kms = dev_get_drvdata(dev);
DBG("");
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index fbe4c2cd52a3..a58fccacc874 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1272,8 +1272,7 @@ static int dp_display_remove(struct platform_device *pdev)
static int dp_pm_resume(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct msm_dp *dp_display = platform_get_drvdata(pdev);
+ struct msm_dp *dp_display = dev_get_drvdata(dev);
struct dp_display_private *dp;
int sink_count = 0;
@@ -1329,8 +1328,7 @@ static int dp_pm_resume(struct device *dev)
static int dp_pm_suspend(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct msm_dp *dp_display = platform_get_drvdata(pdev);
+ struct msm_dp *dp_display = dev_get_drvdata(dev);
struct dp_display_private *dp;
dp = container_of(dp_display, struct dp_display_private, dp_display);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index e269df285136..d27db5777f2c 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -470,8 +470,7 @@ static void dsi_bus_clk_disable(struct msm_dsi_host
*msm_host)
int msm_dsi_runtime_suspend(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct msm_dsi *msm_dsi = platform_get_drvdata(pdev);
+ struct msm_dsi *msm_dsi = dev_get_drvdata(dev);
struct mipi_dsi_host *host = msm_dsi->host;
[PATCH 0/9] treewide: simplify getting .driver_data
I got tired of fixing this in Renesas drivers manually, so I took the big hammer. Remove this cumbersome code pattern which got copy-pasted too much already: - struct platform_device *pdev = to_platform_device(dev); - struct ep93xx_keypad *keypad = platform_get_drvdata(pdev); + struct ep93xx_keypad *keypad = dev_get_drvdata(dev); A branch, tested by buildbot, can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git coccinelle/get_drvdata I am open for other comments, suggestions, too, of course. Here is the cocci-script I created: @@ struct device* d; identifier pdev; expression *ptr; @@ ( - struct platform_device *pdev = to_platform_device(d); | - struct platform_device *pdev; ... - pdev = to_platform_device(d); ) <... when != pdev - >dev + d ...> ptr = - platform_get_drvdata(pdev) + dev_get_drvdata(d) <... when != pdev - >dev + d ...> Kind regards, Wolfram Wolfram Sang (9): dmaengine: stm32-dmamux: simplify getting .driver_data firmware: meson: simplify getting .driver_data gpio: xilinx: simplify getting .driver_data drm/msm: simplify getting .driver_data drm/panfrost: simplify getting .driver_data iio: common: cros_ec_sensors: simplify getting .driver_data net: mdio: mdio-bcm-iproc: simplify getting .driver_data platform: chrome: cros_ec_sensorhub: simplify getting .driver_data remoteproc: omap_remoteproc: simplify getting .driver_data drivers/dma/stm32-dmamux.c | 14 +- drivers/firmware/meson/meson_sm.c | 3 +-- drivers/gpio/gpio-xilinx.c | 6 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 13 + drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 ++ drivers/gpu/drm/msm/dp/dp_display.c| 6 ++ drivers/gpu/drm/msm/dsi/dsi_host.c | 6 ++ drivers/gpu/drm/msm/msm_drv.c | 3 +-- drivers/gpu/drm/panfrost/panfrost_device.c | 6 ++ .../common/cros_ec_sensors/cros_ec_sensors_core.c | 3 +-- drivers/net/mdio/mdio-bcm-iproc.c | 3 +-- drivers/platform/chrome/cros_ec_sensorhub.c| 6 ++ drivers/remoteproc/omap_remoteproc.c | 6 ++ 13 files changed, 28 insertions(+), 53 deletions(-) -- 2.30.2
[PATCH 5/9] drm/panfrost: simplify getting .driver_data
We should get 'driver_data' from 'struct device' directly. Going via
platform_device is an unneeded step back and forth.
Signed-off-by: Wolfram Sang
---
Build tested only. buildbot is happy.
drivers/gpu/drm/panfrost/panfrost_device.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
b/drivers/gpu/drm/panfrost/panfrost_device.c
index bd9b7be63b0f..fd4309209088 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -400,8 +400,7 @@ void panfrost_device_reset(struct panfrost_device *pfdev)
#ifdef CONFIG_PM
int panfrost_device_resume(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct panfrost_device *pfdev = platform_get_drvdata(pdev);
+ struct panfrost_device *pfdev = dev_get_drvdata(dev);
panfrost_device_reset(pfdev);
panfrost_devfreq_resume(pfdev);
@@ -411,8 +410,7 @@ int panfrost_device_resume(struct device *dev)
int panfrost_device_suspend(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct panfrost_device *pfdev = platform_get_drvdata(pdev);
+ struct panfrost_device *pfdev = dev_get_drvdata(dev);
if (!panfrost_job_is_idle(pfdev))
return -EBUSY;
--
2.30.2
Re: [git pull] drm for 5.14-rc1
On Sun, Sep 19, 2021 at 10:19:35AM -0700, Linus Torvalds wrote:
> On Sun, Sep 19, 2021 at 4:05 AM Sudip Mukherjee
> wrote:
> >
> > And indeed, reverting 27da370e0fb3 ("drm/vc4: hdmi: Remove
> > drm_encoder->crtc usage") on top of d4d016caa4b8 ("alpha: move
> > __udiv_qrnnd library function to arch/alpha/lib/")
> > has fixed the error.
>
> Ok, since your pulseaudio problem was reported already over two weeks
> ago with no apparent movement, I've done that revert in my tree.
>
> I reverted the two runtime PM changes that cause problems for Michael too.
I'm sorry, but I find that situation to be a bit ridiculous. In order to
be properly addressed, Michael's issue needs some patches that have been
unreviewed for 5 monthes now.
However, if one reports an issue to you, without cc'ing the author, on a
week-end, the revert is done in a single day.
Even that audio bug only got a proper report yesterday, after you asked
for it.
Do you really expect us to work during the week end too?
Maxime
signature.asc
Description: PGP signature
[PATCH] agp: define proper stubs for empty helpers
From: Arnd Bergmann
The empty unmap_page_from_agp() macro causes a warning when
building with 'make W=1' on a couple of architectures:
drivers/char/agp/generic.c: In function 'agp_generic_destroy_page':
drivers/char/agp/generic.c:1265:28: error: suggest braces around empty body in
an 'if' statement [-Werror=empty-body]
1265 | unmap_page_from_agp(page);
Change the definitions to a 'do { } while (0)' construct to
make these more reliable.
Signed-off-by: Arnd Bergmann
---
arch/parisc/include/asm/agp.h | 4 ++--
arch/powerpc/include/asm/agp.h | 4 ++--
arch/sparc/include/asm/agp.h | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/parisc/include/asm/agp.h b/arch/parisc/include/asm/agp.h
index cb04470e63d0..14ae54cfd368 100644
--- a/arch/parisc/include/asm/agp.h
+++ b/arch/parisc/include/asm/agp.h
@@ -8,8 +8,8 @@
*
*/
-#define map_page_into_agp(page)/* nothing */
-#define unmap_page_from_agp(page) /* nothing */
+#define map_page_into_agp(page)do { } while (0)
+#define unmap_page_from_agp(page) do { } while (0)
#define flush_agp_cache() mb()
/* GATT allocation. Returns/accepts GATT kernel virtual address. */
diff --git a/arch/powerpc/include/asm/agp.h b/arch/powerpc/include/asm/agp.h
index b29b1186f819..6b6485c988dd 100644
--- a/arch/powerpc/include/asm/agp.h
+++ b/arch/powerpc/include/asm/agp.h
@@ -5,8 +5,8 @@
#include
-#define map_page_into_agp(page)
-#define unmap_page_from_agp(page)
+#define map_page_into_agp(page) do {} while (0)
+#define unmap_page_from_agp(page) do {} while (0)
#define flush_agp_cache() mb()
/* GATT allocation. Returns/accepts GATT kernel virtual address. */
diff --git a/arch/sparc/include/asm/agp.h b/arch/sparc/include/asm/agp.h
index efe0d6a12e5a..2d0ff84cee3f 100644
--- a/arch/sparc/include/asm/agp.h
+++ b/arch/sparc/include/asm/agp.h
@@ -4,9 +4,9 @@
/* dummy for now */
-#define map_page_into_agp(page)
-#define unmap_page_from_agp(page)
-#define flush_agp_cache() mb()
+#define map_page_into_agp(page)do { } while (0)
+#define unmap_page_from_agp(page) do { } while (0)
+#define flush_agp_cache() mb()
/* GATT allocation. Returns/accepts GATT kernel virtual address. */
#define alloc_gatt_pages(order)\
--
2.29.2
Re: [git pull] drm for 5.14-rc1
On Mon, Sep 20, 2021 at 10:55:31AM +0200, Maxime Ripard wrote: > Hi, > > On Sat, Sep 18, 2021 at 11:18:33AM +0200, Michael Stapelberg wrote: > > torvalds at linux-foundation.org (Linus Torvalds) writes: > > > Did I fix it up correctly? Who knows. The code makes more sense to me > > > now and seems valid. But I really *really* want to stress how locking > > > is important. > > > > As far as I can tell, this merge conflict resolution made my Raspberry > > Pi 3 hang on boot. You can find the full serial console output at: > > > > https://t.zekjur.net/linux-5.14-raspberry-pi-3-hang-vc4.txt > > > > The last few messages are from vc4, then the boot hangs. > > > > Using git-bisect, I tracked this down to > > https://github.com/torvalds/linux/commit/e058a84bfddc42ba356a2316f2cf1141974625c9, > > which is the merge you’re talking about here, AFAICT. > > > > I also tried the git://anongit.freedesktop.org/drm/drm, and that tree > > boots as expected, suggesting that the problem really is with the > > additional changes. > > > > The code seems to work on my Raspberry Pi 4, just not on the Raspberry > > Pi 3. Any ideas why that might be, and how to fix it? > > I assume you run your Pi without anything connected on HDMI, and without > hdmi_force_hotplug in your config.txt? > > If so, can you test that branch, and let me know if it works for you > https://github.com/mripard/linux/tree/rpi/bug-fixes This breaks every one else, unfortunately. I'll try to come up with something. Maxime signature.asc Description: PGP signature
Re: [PATCH] doc: gpu: drm-internals: Create reference to DRM mm
Hi Jani, On Mon, Sep 20, 2021 at 02:01:57PM +0300, Jani Nikula wrote: > On Mon, 20 Sep 2021, Markus Schneider-Pargmann wrote: > > This short sentence references nothing for details about memory manager. > > Replace it with the documentation file for DRM memory management. > > > > Cc: Jani Nikula > > Signed-off-by: Markus Schneider-Pargmann > > --- > > Documentation/gpu/drm-internals.rst | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/gpu/drm-internals.rst > > b/Documentation/gpu/drm-internals.rst > > index 06af044c882f..bdcdfc4ede04 100644 > > --- a/Documentation/gpu/drm-internals.rst > > +++ b/Documentation/gpu/drm-internals.rst > > @@ -126,8 +126,8 @@ Memory Manager Initialization > > Every DRM driver requires a memory manager which must be initialized at > > load time. DRM currently contains two memory managers, the Translation > > Table Manager (TTM) and the Graphics Execution Manager (GEM). This > > -document describes the use of the GEM memory manager only. See ? for > > -details. > > +document describes the use of the GEM memory manager only. See > > +Documentation/gpu/drm-mm.rst for details. > > Please use rst references instead of a file reference. Thanks for your comment. Could you please explain it a bit more to me? I am new to the kernel sphinx documentation so I looked it up in Documentation/doc-guide/sphinx.rst 'Cross-referencing'. It is listed as the preferred way to reference other documents if I understand it correctly. Should the doc-guide be updated then if a rst reference is preferred? Best, Markus
[PATCH 1/5] drm/gma500: Replace references to dev_private with helper function
Replace most references to struct drm_device.dev_private with the new
helper function to_drm_psb_private(). The only references left are in
assignments and the helper itself.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/backlight.c | 12 -
drivers/gpu/drm/gma500/cdv_device.c| 22 +++
drivers/gpu/drm/gma500/cdv_intel_display.c | 10 +++
drivers/gpu/drm/gma500/cdv_intel_dp.c | 12 -
drivers/gpu/drm/gma500/cdv_intel_lvds.c| 22 +++
drivers/gpu/drm/gma500/framebuffer.c | 16 +--
drivers/gpu/drm/gma500/gem.c | 2 +-
drivers/gpu/drm/gma500/gma_device.c| 2 +-
drivers/gpu/drm/gma500/gma_display.c | 14 +-
drivers/gpu/drm/gma500/gtt.c | 18 ++---
drivers/gpu/drm/gma500/intel_bios.c| 4 +--
drivers/gpu/drm/gma500/intel_gmbus.c | 6 ++---
drivers/gpu/drm/gma500/mid_bios.c | 4 +--
drivers/gpu/drm/gma500/mmu.c | 12 -
drivers/gpu/drm/gma500/oaktrail_crtc.c | 8 +++---
drivers/gpu/drm/gma500/oaktrail_device.c | 20 +++---
drivers/gpu/drm/gma500/oaktrail_hdmi.c | 18 ++---
drivers/gpu/drm/gma500/oaktrail_lvds.c | 14 +-
drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c | 2 +-
drivers/gpu/drm/gma500/opregion.c | 12 -
drivers/gpu/drm/gma500/power.c | 20 +++---
drivers/gpu/drm/gma500/psb_device.c| 16 +--
drivers/gpu/drm/gma500/psb_drv.c | 6 ++---
drivers/gpu/drm/gma500/psb_drv.h | 21 +++
drivers/gpu/drm/gma500/psb_intel_display.c | 10 +++
drivers/gpu/drm/gma500/psb_intel_lvds.c| 31 ++
drivers/gpu/drm/gma500/psb_intel_sdvo.c| 10 +++
drivers/gpu/drm/gma500/psb_irq.c | 18 ++---
28 files changed, 178 insertions(+), 184 deletions(-)
diff --git a/drivers/gpu/drm/gma500/backlight.c
b/drivers/gpu/drm/gma500/backlight.c
index 9e90258541a4..46b9c0f13d6d 100644
--- a/drivers/gpu/drm/gma500/backlight.c
+++ b/drivers/gpu/drm/gma500/backlight.c
@@ -16,7 +16,7 @@
#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
static void do_gma_backlight_set(struct drm_device *dev)
{
- struct drm_psb_private *dev_priv = dev->dev_private;
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
backlight_update_status(dev_priv->backlight_device);
}
#endif
@@ -24,7 +24,7 @@ static void do_gma_backlight_set(struct drm_device *dev)
void gma_backlight_enable(struct drm_device *dev)
{
#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
- struct drm_psb_private *dev_priv = dev->dev_private;
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
dev_priv->backlight_enabled = true;
if (dev_priv->backlight_device) {
dev_priv->backlight_device->props.brightness =
dev_priv->backlight_level;
@@ -36,7 +36,7 @@ void gma_backlight_enable(struct drm_device *dev)
void gma_backlight_disable(struct drm_device *dev)
{
#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
- struct drm_psb_private *dev_priv = dev->dev_private;
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
dev_priv->backlight_enabled = false;
if (dev_priv->backlight_device) {
dev_priv->backlight_device->props.brightness = 0;
@@ -48,7 +48,7 @@ void gma_backlight_disable(struct drm_device *dev)
void gma_backlight_set(struct drm_device *dev, int v)
{
#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
- struct drm_psb_private *dev_priv = dev->dev_private;
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
dev_priv->backlight_level = v;
if (dev_priv->backlight_device && dev_priv->backlight_enabled) {
dev_priv->backlight_device->props.brightness = v;
@@ -60,7 +60,7 @@ void gma_backlight_set(struct drm_device *dev, int v)
int gma_backlight_init(struct drm_device *dev)
{
#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
- struct drm_psb_private *dev_priv = dev->dev_private;
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
dev_priv->backlight_enabled = true;
return dev_priv->ops->backlight_init(dev);
#else
@@ -71,7 +71,7 @@ int gma_backlight_init(struct drm_device *dev)
void gma_backlight_exit(struct drm_device *dev)
{
#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
- struct drm_psb_private *dev_priv = dev->dev_private;
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
if (dev_priv->backlight_device) {
dev_priv->backlight_device->props.brightness = 0;
backlight_update_status(dev_priv->backlight_device);
diff --git a/drivers/gpu/drm/gma500/cdv_device.c
b/drivers/gpu/drm/gma500/cdv_device.c
index 1342e7fb382f..ce8215e0b1c1 100644
--- a/drivers/gpu/drm/gma500/cdv_device.c
+++ b/drivers/gpu/drm/gma500/cdv_device.c
@@ -38,7 +38,7 @@ static void cdv_disable_vga(struct drm_device *dev)
static int
[PATCH 3/5] drm/gma500: Embed struct drm_device in struct drm_psb_private
Embed struct drm_device in struct drm_psb_private. Replace the use
of dev_private by an upcast operation. Switch to managed release of
struct drm_psb_private.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/cdv_device.c | 2 +-
drivers/gpu/drm/gma500/intel_bios.c | 6 +++---
drivers/gpu/drm/gma500/intel_gmbus.c | 6 +++---
drivers/gpu/drm/gma500/mid_bios.c| 7 +++
drivers/gpu/drm/gma500/opregion.c| 2 +-
drivers/gpu/drm/gma500/psb_drv.c | 23 +++
drivers/gpu/drm/gma500/psb_drv.h | 5 +++--
drivers/gpu/drm/gma500/psb_irq.c | 8
drivers/gpu/drm/gma500/psb_lid.c | 2 +-
9 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_device.c
b/drivers/gpu/drm/gma500/cdv_device.c
index ce8215e0b1c1..d7c6cca23e94 100644
--- a/drivers/gpu/drm/gma500/cdv_device.c
+++ b/drivers/gpu/drm/gma500/cdv_device.c
@@ -429,7 +429,7 @@ static void cdv_hotplug_work_func(struct work_struct *work)
{
struct drm_psb_private *dev_priv = container_of(work, struct
drm_psb_private,
hotplug_work);
-struct drm_device *dev = dev_priv->dev;
+ struct drm_device *dev = _priv->dev;
/* Just fire off a uevent and let userspace tell us what to do */
drm_helper_hpd_irq_event(dev);
diff --git a/drivers/gpu/drm/gma500/intel_bios.c
b/drivers/gpu/drm/gma500/intel_bios.c
index ebc7dd828302..d5ca5f241974 100644
--- a/drivers/gpu/drm/gma500/intel_bios.c
+++ b/drivers/gpu/drm/gma500/intel_bios.c
@@ -207,7 +207,7 @@ static void parse_backlight_data(struct drm_psb_private
*dev_priv,
lvds_bl = kmemdup(vbt_lvds_bl, sizeof(*vbt_lvds_bl), GFP_KERNEL);
if (!lvds_bl) {
- dev_err(dev_priv->dev->dev, "out of memory for backlight
data\n");
+ dev_err(dev_priv->dev.dev, "out of memory for backlight
data\n");
return;
}
dev_priv->lvds_bl = lvds_bl;
@@ -248,7 +248,7 @@ static void parse_lfp_panel_data(struct drm_psb_private
*dev_priv,
panel_fixed_mode = kzalloc(sizeof(*panel_fixed_mode),
GFP_KERNEL);
if (panel_fixed_mode == NULL) {
- dev_err(dev_priv->dev->dev, "out of memory for fixed panel
mode\n");
+ dev_err(dev_priv->dev.dev, "out of memory for fixed panel
mode\n");
return;
}
@@ -259,7 +259,7 @@ static void parse_lfp_panel_data(struct drm_psb_private
*dev_priv,
dev_priv->lfp_lvds_vbt_mode = panel_fixed_mode;
drm_mode_debug_printmodeline(panel_fixed_mode);
} else {
- dev_dbg(dev_priv->dev->dev, "ignoring invalid LVDS VBT\n");
+ dev_dbg(dev_priv->dev.dev, "ignoring invalid LVDS VBT\n");
dev_priv->lvds_vbt = 0;
kfree(panel_fixed_mode);
}
diff --git a/drivers/gpu/drm/gma500/intel_gmbus.c
b/drivers/gpu/drm/gma500/intel_gmbus.c
index 413a18cdd6d6..09cedabf4776 100644
--- a/drivers/gpu/drm/gma500/intel_gmbus.c
+++ b/drivers/gpu/drm/gma500/intel_gmbus.c
@@ -196,7 +196,7 @@ intel_gpio_create(struct drm_psb_private *dev_priv, u32 pin)
"gma500 GPIO%c", "?BACDE?F"[pin]);
gpio->adapter.owner = THIS_MODULE;
gpio->adapter.algo_data = >algo;
- gpio->adapter.dev.parent = dev_priv->dev->dev;
+ gpio->adapter.dev.parent = dev_priv->dev.dev;
gpio->algo.setsda = set_data;
gpio->algo.setscl = set_clock;
gpio->algo.getsda = get_data;
@@ -226,7 +226,7 @@ intel_i2c_quirk_xfer(struct drm_psb_private *dev_priv,
adapter);
int ret;
- gma_intel_i2c_reset(dev_priv->dev);
+ gma_intel_i2c_reset(_priv->dev);
intel_i2c_quirk_set(dev_priv, true);
set_data(gpio, 1);
@@ -432,7 +432,7 @@ int gma_intel_setup_gmbus(struct drm_device *dev)
bus->force_bit = intel_gpio_create(dev_priv, i);
}
- gma_intel_i2c_reset(dev_priv->dev);
+ gma_intel_i2c_reset(_priv->dev);
return 0;
diff --git a/drivers/gpu/drm/gma500/mid_bios.c
b/drivers/gpu/drm/gma500/mid_bios.c
index bdc57b9070ec..7e76790c6a81 100644
--- a/drivers/gpu/drm/gma500/mid_bios.c
+++ b/drivers/gpu/drm/gma500/mid_bios.c
@@ -94,7 +94,7 @@ static void mid_get_fuse_settings(struct drm_device *dev)
static void mid_get_pci_revID(struct drm_psb_private *dev_priv)
{
uint32_t platform_rev_id = 0;
- struct pci_dev *pdev = to_pci_dev(dev_priv->dev->dev);
+ struct pci_dev *pdev = to_pci_dev(dev_priv->dev.dev);
int domain = pci_domain_nr(pdev->bus);
struct pci_dev *pci_gfx_root =
pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(2, 0));
@@ -106,8 +106,7 @@ static void mid_get_pci_revID(struct drm_psb_private
*dev_priv)
pci_read_config_dword(pci_gfx_root, 0x08, _rev_id);
[PATCH 4/5] drm/gma500: Remove dev_priv branch from unload function
The value of dev_priv in psb_driver_unload() is always non-zero. Remove
the respective test.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/psb_drv.c | 93
1 file changed, 46 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 48967bbc4501..cef9fb6a06d2 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -167,57 +167,56 @@ static void psb_driver_unload(struct drm_device *dev)
/* TODO: Kill vblank etc here */
- if (dev_priv) {
- if (dev_priv->backlight_device)
- gma_backlight_exit(dev);
- psb_modeset_cleanup(dev);
+ if (dev_priv->backlight_device)
+ gma_backlight_exit(dev);
+ psb_modeset_cleanup(dev);
- if (dev_priv->ops->chip_teardown)
- dev_priv->ops->chip_teardown(dev);
+ if (dev_priv->ops->chip_teardown)
+ dev_priv->ops->chip_teardown(dev);
- psb_intel_opregion_fini(dev);
+ psb_intel_opregion_fini(dev);
- if (dev_priv->pf_pd) {
- psb_mmu_free_pagedir(dev_priv->pf_pd);
- dev_priv->pf_pd = NULL;
- }
- if (dev_priv->mmu) {
- struct psb_gtt *pg = _priv->gtt;
-
- down_read(>sem);
- psb_mmu_remove_pfn_sequence(
- psb_mmu_get_default_pd
- (dev_priv->mmu),
- pg->mmu_gatt_start,
- dev_priv->vram_stolen_size >> PAGE_SHIFT);
- up_read(>sem);
- psb_mmu_driver_takedown(dev_priv->mmu);
- dev_priv->mmu = NULL;
- }
- psb_gtt_takedown(dev);
- if (dev_priv->scratch_page) {
- set_pages_wb(dev_priv->scratch_page, 1);
- __free_page(dev_priv->scratch_page);
- dev_priv->scratch_page = NULL;
- }
- if (dev_priv->vdc_reg) {
- iounmap(dev_priv->vdc_reg);
- dev_priv->vdc_reg = NULL;
- }
- if (dev_priv->sgx_reg) {
- iounmap(dev_priv->sgx_reg);
- dev_priv->sgx_reg = NULL;
- }
- if (dev_priv->aux_reg) {
- iounmap(dev_priv->aux_reg);
- dev_priv->aux_reg = NULL;
- }
- pci_dev_put(dev_priv->aux_pdev);
- pci_dev_put(dev_priv->lpc_pdev);
-
- /* Destroy VBT data */
- psb_intel_destroy_bios(dev);
+ if (dev_priv->pf_pd) {
+ psb_mmu_free_pagedir(dev_priv->pf_pd);
+ dev_priv->pf_pd = NULL;
}
+ if (dev_priv->mmu) {
+ struct psb_gtt *pg = _priv->gtt;
+
+ down_read(>sem);
+ psb_mmu_remove_pfn_sequence(
+ psb_mmu_get_default_pd
+ (dev_priv->mmu),
+ pg->mmu_gatt_start,
+ dev_priv->vram_stolen_size >> PAGE_SHIFT);
+ up_read(>sem);
+ psb_mmu_driver_takedown(dev_priv->mmu);
+ dev_priv->mmu = NULL;
+ }
+ psb_gtt_takedown(dev);
+ if (dev_priv->scratch_page) {
+ set_pages_wb(dev_priv->scratch_page, 1);
+ __free_page(dev_priv->scratch_page);
+ dev_priv->scratch_page = NULL;
+ }
+ if (dev_priv->vdc_reg) {
+ iounmap(dev_priv->vdc_reg);
+ dev_priv->vdc_reg = NULL;
+ }
+ if (dev_priv->sgx_reg) {
+ iounmap(dev_priv->sgx_reg);
+ dev_priv->sgx_reg = NULL;
+ }
+ if (dev_priv->aux_reg) {
+ iounmap(dev_priv->aux_reg);
+ dev_priv->aux_reg = NULL;
+ }
+ pci_dev_put(dev_priv->aux_pdev);
+ pci_dev_put(dev_priv->lpc_pdev);
+
+ /* Destroy VBT data */
+ psb_intel_destroy_bios(dev);
+
gma_power_uninit(dev);
}
--
2.33.0
[PATCH 2/5] drm/gma500: Disable PCI device during shutdown
Use managed disablement of PCI devices via pcim_device_enable().
Disables the PCI device and simplifies error rollback in probe
function.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/psb_drv.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 05f42e66af86..80ef2f0562c3 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -448,15 +448,13 @@ static int psb_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
struct drm_device *dev;
int ret;
- ret = pci_enable_device(pdev);
+ ret = pcim_enable_device(pdev);
if (ret)
return ret;
dev = drm_dev_alloc(, >dev);
- if (IS_ERR(dev)) {
- ret = PTR_ERR(dev);
- goto err_pci_disable_device;
- }
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
pci_set_drvdata(pdev, dev);
@@ -474,8 +472,6 @@ static int psb_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
psb_driver_unload(dev);
err_drm_dev_put:
drm_dev_put(dev);
-err_pci_disable_device:
- pci_disable_device(pdev);
return ret;
}
--
2.33.0
[PATCH 5/5] drm/gma500: Managed device release
Set up a clean-up action to automatically release device resources
during driver shutdown. Remove manual release code.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/psb_drv.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index cef9fb6a06d2..3d036d2a3b29 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -220,6 +220,13 @@ static void psb_driver_unload(struct drm_device *dev)
gma_power_uninit(dev);
}
+static void psb_device_release(void *data)
+{
+ struct drm_device *dev = data;
+
+ psb_driver_unload(dev);
+}
+
static int psb_driver_load(struct drm_device *dev, unsigned long flags)
{
struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -400,8 +407,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned
long flags)
pm_runtime_enable(dev->dev);
pm_runtime_set_active(dev->dev);
#endif
- /* Intel drm driver load is done, continue doing pvr load */
- return 0;
+
+ return devm_add_action_or_reset(dev->dev, psb_device_release, dev);
+
out_err:
psb_driver_unload(dev);
return ret;
@@ -457,13 +465,9 @@ static int psb_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
ret = drm_dev_register(dev, ent->driver_data);
if (ret)
- goto err_psb_driver_unload;
+ return ret;
return 0;
-
-err_psb_driver_unload:
- psb_driver_unload(dev);
- return ret;
}
static void psb_pci_remove(struct pci_dev *pdev)
@@ -471,7 +475,6 @@ static void psb_pci_remove(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
drm_dev_unregister(dev);
- psb_driver_unload(dev);
}
static const struct dev_pm_ops psb_pm_ops = {
--
2.33.0
[PATCH 0/5] drm/gma500: Managed cleanup
Switch gma500 to managed cleanup and remove the manual cleanup code from the driver's PCI callbacks. Managed cleanup involves embedding the DRM device structure in the driver's structure. In preparation, patch 1 replaces references all references to dev_private with a helper function. Patch 2 adds managed cleanup for pci_enable_device(). Patches 3 and 4 embed struct drm_device in struct_drm_psb_private. The structure's memory is being automatically released. Patch 5 adds managed cleanup for the device resources. Instead of calling the large, monolithic function psb_driver_unload(), the release code could be split up split into smaller helpers and reuse exising functionality from devres. Future work: for a number of drivers, the PCI remove callback contains only a single call to drm_device_unregister(). In a later patchset, this could be implemented as another shared helper within DRM. Tested on Atom N2800 hardware. Thomas Zimmermann (5): drm/gma500: Replace references to dev_private with helper function drm/gma500: Disable PCI device during shutdown drm/gma500: Embed struct drm_device in struct drm_psb_private drm/gma500: Remove dev_priv branch from unload function drm/gma500: Managed device release drivers/gpu/drm/gma500/backlight.c | 12 +- drivers/gpu/drm/gma500/cdv_device.c| 24 ++-- drivers/gpu/drm/gma500/cdv_intel_display.c | 10 +- drivers/gpu/drm/gma500/cdv_intel_dp.c | 12 +- drivers/gpu/drm/gma500/cdv_intel_lvds.c| 22 +-- drivers/gpu/drm/gma500/framebuffer.c | 16 +-- drivers/gpu/drm/gma500/gem.c | 2 +- drivers/gpu/drm/gma500/gma_device.c| 2 +- drivers/gpu/drm/gma500/gma_display.c | 14 +- drivers/gpu/drm/gma500/gtt.c | 18 +-- drivers/gpu/drm/gma500/intel_bios.c| 10 +- drivers/gpu/drm/gma500/intel_gmbus.c | 12 +- drivers/gpu/drm/gma500/mid_bios.c | 11 +- drivers/gpu/drm/gma500/mmu.c | 12 +- drivers/gpu/drm/gma500/oaktrail_crtc.c | 8 +- drivers/gpu/drm/gma500/oaktrail_device.c | 20 +-- drivers/gpu/drm/gma500/oaktrail_hdmi.c | 18 +-- drivers/gpu/drm/gma500/oaktrail_lvds.c | 14 +- drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c | 2 +- drivers/gpu/drm/gma500/opregion.c | 14 +- drivers/gpu/drm/gma500/power.c | 20 +-- drivers/gpu/drm/gma500/psb_device.c| 16 +-- drivers/gpu/drm/gma500/psb_drv.c | 147 ++--- drivers/gpu/drm/gma500/psb_drv.h | 24 ++-- drivers/gpu/drm/gma500/psb_intel_display.c | 10 +- drivers/gpu/drm/gma500/psb_intel_lvds.c| 31 ++--- drivers/gpu/drm/gma500/psb_intel_sdvo.c| 10 +- drivers/gpu/drm/gma500/psb_irq.c | 26 ++-- drivers/gpu/drm/gma500/psb_lid.c | 2 +- 29 files changed, 261 insertions(+), 278 deletions(-) -- 2.33.0
Re: [PATCH v2] kernel/locking: Add context to ww_mutex_trylock.
Quoting Peter Zijlstra (2021-09-17 16:13:19) > On Thu, Sep 16, 2021 at 03:28:11PM +0200, Peter Zijlstra wrote: > > On Thu, Sep 16, 2021 at 03:00:39PM +0200, Maarten Lankhorst wrote: > > > > > > For merge logistics, can we pls have a stable branch? I expect that the > > > > i915 patches will be ready for 5.16. > > > > > > > > Or send it in for -rc2 so that the interface change doesn't cause > > > > needless > > > > conflicts, whatever you think is best. > > > > > Yeah, some central branch drm could pull from, would make upstreaming > > > patches that depends on it easier. :) > > > > I think I'll make tip/locking/wwmutex and include that in > > tip/locking/core, let me have a poke. > > This is now so. Enjoy! This is now merged to drm-intel-gt-next. Regards, Joonas
Re: [PATCH v2 01/12] virtio-gpu api: multiple context types with explicit initialization
Hi Gurchetan, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.15-rc2 next-20210920] [cannot apply to airlied/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Gurchetan-Singh/Context-types/20210917-091131 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-m021-20210920 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/0fb3eeca4360faf3cfc75a26b8ddf1df6b13b8ee git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Gurchetan-Singh/Context-types/20210917-091131 git checkout 0fb3eeca4360faf3cfc75a26b8ddf1df6b13b8ee # save the attached .config to linux build tree mkdir build_dir make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): In file included from :32: >> ./usr/include/linux/virtio_gpu.h:142:2: error: unknown type name 'u8' 142 | u8 ring_idx; | ^~ ./usr/include/linux/virtio_gpu.h:143:2: error: unknown type name 'u8' 143 | u8 padding[3]; | ^~ --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
[PATCH v4 4/6] drm/i915/gt: Register the migrate contexts with their engines
Pinned contexts, like the migrate contexts need reset after resume
since their context image may have been lost. Also the GuC needs to
register pinned contexts.
Add a list to struct intel_engine_cs where we add all pinned contexts on
creation, and traverse that list at resume time to reset the pinned
contexts.
This fixes the kms_pipe_crc_basic@suspend-read-crc-pipe-a selftest for now,
but proper LMEM backup / restore is needed for full suspend functionality.
However, note that even with full LMEM backup / restore it may be
desirable to keep the reset since backing up the migrate context images
must happen using memcpy() after the migrate context has become inactive,
and for performance- and other reasons we want to avoid memcpy() from
LMEM.
Also traverse the list at guc_init_lrc_mapping() calling
guc_kernel_context_pin() for the pinned contexts, like is already done
for the kernel context.
v2:
- Don't reset the contexts on each __engine_unpark() but rather at
resume time (Chris Wilson).
v3:
- Reset contexts in the engine sanitize callback. (Chris Wilson)
Cc: Tvrtko Ursulin
Cc: Matthew Auld
Cc: Maarten Lankhorst
Cc: Brost Matthew
Cc: Chris Wilson
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gt/intel_context_types.h | 8 +++
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 23 +++
drivers/gpu/drm/i915/gt/intel_engine_pm.h | 2 ++
drivers/gpu/drm/i915/gt/intel_engine_types.h | 7 ++
.../drm/i915/gt/intel_execlists_submission.c | 2 ++
.../gpu/drm/i915/gt/intel_ring_submission.c | 3 +++
drivers/gpu/drm/i915/gt/mock_engine.c | 2 ++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 12 +++---
9 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 930569a1a01f..12252c411159 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -153,6 +153,14 @@ struct intel_context {
/** sseu: Control eu/slice partitioning */
struct intel_sseu sseu;
+ /**
+* pinned_contexts_link: List link for the engine's pinned contexts.
+* This is only used if this is a perma-pinned kernel context and
+* the list is assumed to only be manipulated during driver load
+* or unload time so no mutex protection currently.
+*/
+ struct list_head pinned_contexts_link;
+
u8 wa_bb_page; /* if set, page num reserved for context workarounds */
struct {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 332efea696a5..c606a4714904 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -320,6 +320,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum
intel_engine_id id)
BUILD_BUG_ON(BITS_PER_TYPE(engine->mask) < I915_NUM_ENGINES);
+ INIT_LIST_HEAD(>pinned_contexts_list);
engine->id = id;
engine->legacy_idx = INVALID_ENGINE;
engine->mask = BIT(id);
@@ -875,6 +876,8 @@ intel_engine_create_pinned_context(struct intel_engine_cs
*engine,
return ERR_PTR(err);
}
+ list_add_tail(>pinned_contexts_link, >pinned_contexts_list);
+
/*
* Give our perma-pinned kernel timelines a separate lockdep class,
* so that we can use them from within the normal user timelines
@@ -897,6 +900,7 @@ void intel_engine_destroy_pinned_context(struct
intel_context *ce)
list_del(>timeline->engine_link);
mutex_unlock(>vm->mutex);
+ list_del(>pinned_contexts_link);
intel_context_unpin(ce);
intel_context_put(ce);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 1f07ac4e0672..dacd62773735 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -298,6 +298,29 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
intel_engine_init_heartbeat(engine);
}
+/**
+ * intel_engine_reset_pinned_contexts - Reset the pinned contexts of
+ * an engine.
+ * @engine: The engine whose pinned contexts we want to reset.
+ *
+ * Typically the pinned context LMEM images lose or get their content
+ * corrupted on suspend. This function resets their images.
+ */
+void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine)
+{
+ struct intel_context *ce;
+
+ list_for_each_entry(ce, >pinned_contexts_list,
+ pinned_contexts_link) {
+ /* kernel context gets reset at __engine_unpark() */
+ if (ce == engine->kernel_context)
+ continue;
+
+ dbg_poison_ce(ce);
+ ce->ops->reset(ce);
+ }
+}
+
#if
[PATCH v4 5/6] drm/i915: Don't back up pinned LMEM context images and rings during suspend
Pinned context images are now reset during resume. Don't back them up,
and assuming that rings can be assumed empty at suspend, don't back them
up either.
Introduce a new object flag, I915_BO_ALLOC_PM_VOLATILE meaning that an
object is allowed to lose its content on suspend.
v3:
- Slight documentation clarification (Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
.../gpu/drm/i915/gem/i915_gem_object_types.h| 17 ++---
drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c | 3 +++
drivers/gpu/drm/i915/gt/intel_lrc.c | 3 ++-
drivers/gpu/drm/i915/gt/intel_ring.c| 3 ++-
4 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 734cc8e16481..118691ce81d7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -288,16 +288,19 @@ struct drm_i915_gem_object {
I915_SELFTEST_DECLARE(struct list_head st_link);
unsigned long flags;
-#define I915_BO_ALLOC_CONTIGUOUS BIT(0)
-#define I915_BO_ALLOC_VOLATILE BIT(1)
-#define I915_BO_ALLOC_CPU_CLEAR BIT(2)
-#define I915_BO_ALLOC_USER BIT(3)
+#define I915_BO_ALLOC_CONTIGUOUS BIT(0)
+#define I915_BO_ALLOC_VOLATILEBIT(1)
+#define I915_BO_ALLOC_CPU_CLEAR BIT(2)
+#define I915_BO_ALLOC_USERBIT(3)
+/* Object is allowed to lose its contents on suspend / resume, even if pinned
*/
+#define I915_BO_ALLOC_PM_VOLATILE BIT(4)
#define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | \
I915_BO_ALLOC_VOLATILE | \
I915_BO_ALLOC_CPU_CLEAR | \
-I915_BO_ALLOC_USER)
-#define I915_BO_READONLY BIT(4)
-#define I915_TILING_QUIRK_BIT5 /* unknown swizzling; do not release! */
+I915_BO_ALLOC_USER | \
+I915_BO_ALLOC_PM_VOLATILE)
+#define I915_BO_READONLY BIT(5)
+#define I915_TILING_QUIRK_BIT 6 /* unknown swizzling; do not release! */
/**
* @mem_flags - Mutable placement-related flags
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
index cb1c46724f70..03a00d193f40 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
@@ -60,6 +60,9 @@ static int i915_ttm_backup(struct i915_gem_apply_to_region
*apply,
if (!pm_apply->backup_pinned)
return 0;
+ if (obj->flags & I915_BO_ALLOC_PM_VOLATILE)
+ return 0;
+
backup = i915_gem_object_create_shmem(i915, obj->base.size);
if (IS_ERR(backup))
return PTR_ERR(backup);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 6ba8daea2f56..3ef9eaf8c50e 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -942,7 +942,8 @@ __lrc_alloc_state(struct intel_context *ce, struct
intel_engine_cs *engine)
context_size += PAGE_SIZE;
}
- obj = i915_gem_object_create_lmem(engine->i915, context_size, 0);
+ obj = i915_gem_object_create_lmem(engine->i915, context_size,
+ I915_BO_ALLOC_PM_VOLATILE);
if (IS_ERR(obj))
obj = i915_gem_object_create_shmem(engine->i915, context_size);
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c
b/drivers/gpu/drm/i915/gt/intel_ring.c
index 7c4d5158e03b..2fdd52b62092 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -112,7 +112,8 @@ static struct i915_vma *create_ring_vma(struct i915_ggtt
*ggtt, int size)
struct drm_i915_gem_object *obj;
struct i915_vma *vma;
- obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_VOLATILE);
+ obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_VOLATILE |
+ I915_BO_ALLOC_PM_VOLATILE);
if (IS_ERR(obj) && i915_ggtt_has_aperture(ggtt))
obj = i915_gem_object_create_stolen(i915, size);
if (IS_ERR(obj))
--
2.31.1
[PATCH v4 3/6] drm/i915 Implement LMEM backup and restore for suspend / resume
Just evict unpinned objects to system. For pinned LMEM objects,
make a backup system object and blit the contents to that.
Backup is performed in three steps,
1: Opportunistically evict evictable objects using the gpu blitter.
2: After gt idle, evict evictable objects using the gpu blitter. This will
be modified in an upcoming patch to backup pinned objects that are not used
by the blitter itself.
3: Backup remaining pinned objects using memcpy.
Also move uC suspend to after 2) to make sure we have a functional GuC
during 2) if using GuC submission.
v2:
- Major refactor to make sure gem_exec_suspend@hang-SX subtests work, and
suspend / resume works with a slightly modified GuC submission enabling
patch series.
v3:
- Fix a potential use-after-free (Matthew Auld)
- Use i915_gem_object_create_shmem() instead of
i915_gem_object_create_region (Matthew Auld)
- Minor simplifications (Matthew Auld)
- Fix up kerneldoc for i195_ttm_restore_region().
- Final lmem_suspend() call moved to i915_gem_backup_suspend from
i915_gem_suspend_late, since the latter gets called at driver unload
and we don't unnecessarily want to run it at that time.
v4:
- Interface change of ttm- & lmem suspend / resume functions to use
flags rather than bools. (Matthew Auld)
- Completely drop the i915_gem_backup_suspend change (Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/Makefile | 1 +
.../gpu/drm/i915/gem/i915_gem_object_types.h | 1 +
drivers/gpu/drm/i915/gem/i915_gem_pm.c| 87
drivers/gpu/drm/i915/gem/i915_gem_pm.h| 1 +
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 30 ++-
drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 10 +
drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c| 202 ++
drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.h| 26 +++
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 4 +-
drivers/gpu/drm/i915/i915_drv.c | 4 +-
10 files changed, 353 insertions(+), 13 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 335a8c668848..5c8e022a7383 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -154,6 +154,7 @@ gem-y += \
gem/i915_gem_throttle.o \
gem/i915_gem_tiling.o \
gem/i915_gem_ttm.o \
+ gem/i915_gem_ttm_pm.o \
gem/i915_gem_userptr.o \
gem/i915_gem_wait.o \
gem/i915_gemfs.o
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 2471f36aaff3..734cc8e16481 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -534,6 +534,7 @@ struct drm_i915_gem_object {
struct {
struct sg_table *cached_io_st;
struct i915_gem_object_page_iter get_io_page;
+ struct drm_i915_gem_object *backup;
bool created:1;
} ttm;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 8b9d7d14c4bd..12b37b4c1192 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -5,6 +5,7 @@
*/
#include "gem/i915_gem_pm.h"
+#include "gem/i915_gem_ttm_pm.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
#include "gt/intel_gt_requests.h"
@@ -39,6 +40,84 @@ void i915_gem_suspend(struct drm_i915_private *i915)
i915_gem_drain_freed_objects(i915);
}
+static int lmem_restore(struct drm_i915_private *i915, u32 flags)
+{
+ struct intel_memory_region *mr;
+ int ret = 0, id;
+
+ for_each_memory_region(mr, i915, id) {
+ if (mr->type == INTEL_MEMORY_LOCAL) {
+ ret = i915_ttm_restore_region(mr, flags);
+ if (ret)
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static int lmem_suspend(struct drm_i915_private *i915, u32 flags)
+{
+ struct intel_memory_region *mr;
+ int ret = 0, id;
+
+ for_each_memory_region(mr, i915, id) {
+ if (mr->type == INTEL_MEMORY_LOCAL) {
+ ret = i915_ttm_backup_region(mr, flags);
+ if (ret)
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static void lmem_recover(struct drm_i915_private *i915)
+{
+ struct intel_memory_region *mr;
+ int id;
+
+ for_each_memory_region(mr, i915, id)
+ if (mr->type == INTEL_MEMORY_LOCAL)
+ i915_ttm_recover_region(mr);
+}
+
+int i915_gem_backup_suspend(struct drm_i915_private *i915)
+{
+ int ret;
+
+ /* Opportunistically try to evict unpinned objects */
+ ret = lmem_suspend(i915,
[PATCH v4 2/6] drm/i915/gem: Implement a function to process all gem objects of a region
An upcoming common pattern is to traverse the region object list and
perform certain actions on all objects in a region. It's a little tricky
to get the list locking right, in particular since a gem object may
change region unless it's pinned or the object lock is held.
Define a function that does this for us and that takes an argument that
defines the action to be performed on each object.
v3:
- Improve structure documentation a bit (Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_region.c | 70 ++
drivers/gpu/drm/i915/gem/i915_gem_region.h | 37
2 files changed, 107 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.c
b/drivers/gpu/drm/i915/gem/i915_gem_region.c
index 1f557b2178ed..a016ccec36f3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.c
@@ -80,3 +80,73 @@ i915_gem_object_create_region(struct intel_memory_region
*mem,
i915_gem_object_free(obj);
return ERR_PTR(err);
}
+
+/**
+ * i915_gem_process_region - Iterate over all objects of a region using ops
+ * to process and optionally skip objects
+ * @mr: The memory region
+ * @apply: ops and private data
+ *
+ * This function can be used to iterate over the regions object list,
+ * checking whether to skip objects, and, if not, lock the objects and
+ * process them using the supplied ops. Note that this function temporarily
+ * removes objects from the region list while iterating, so that if run
+ * concurrently with itself may not iterate over all objects.
+ *
+ * Return: 0 if successful, negative error code on failure.
+ */
+int i915_gem_process_region(struct intel_memory_region *mr,
+ struct i915_gem_apply_to_region *apply)
+{
+ const struct i915_gem_apply_to_region_ops *ops = apply->ops;
+ struct drm_i915_gem_object *obj;
+ struct list_head still_in_list;
+ int ret = 0;
+
+ /*
+* In the future, a non-NULL apply->ww could mean the caller is
+* already in a locking transaction and provides its own context.
+*/
+ GEM_WARN_ON(apply->ww);
+
+ INIT_LIST_HEAD(_in_list);
+ mutex_lock(>objects.lock);
+ for (;;) {
+ struct i915_gem_ww_ctx ww;
+
+ obj = list_first_entry_or_null(>objects.list, typeof(*obj),
+ mm.region_link);
+ if (!obj)
+ break;
+
+ list_move_tail(>mm.region_link, _in_list);
+ if (!kref_get_unless_zero(>base.refcount))
+ continue;
+
+ /*
+* Note: Someone else might be migrating the object at this
+* point. The object's region is not stable until we lock
+* the object.
+*/
+ mutex_unlock(>objects.lock);
+ apply->ww =
+ for_i915_gem_ww(, ret, apply->interruptible) {
+ ret = i915_gem_object_lock(obj, apply->ww);
+ if (ret)
+ continue;
+
+ if (obj->mm.region == mr)
+ ret = ops->process_obj(apply, obj);
+ /* Implicit object unlock */
+ }
+
+ i915_gem_object_put(obj);
+ mutex_lock(>objects.lock);
+ if (ret)
+ break;
+ }
+ list_splice_tail(_in_list, >objects.list);
+ mutex_unlock(>objects.lock);
+
+ return ret;
+}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.h
b/drivers/gpu/drm/i915/gem/i915_gem_region.h
index 1008e580a89a..fcaa12d657d4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.h
@@ -12,6 +12,41 @@ struct intel_memory_region;
struct drm_i915_gem_object;
struct sg_table;
+struct i915_gem_apply_to_region;
+
+/**
+ * struct i915_gem_apply_to_region_ops - ops to use when iterating over all
+ * region objects.
+ */
+struct i915_gem_apply_to_region_ops {
+ /**
+* process_obj - Process the current object
+* @apply: Embed this for private data.
+* @obj: The current object.
+*
+* Note that if this function is part of a ww transaction, and
+* if returns -EDEADLK for one of the objects, it may be
+* rerun for that same object in the same pass.
+*/
+ int (*process_obj)(struct i915_gem_apply_to_region *apply,
+ struct drm_i915_gem_object *obj);
+};
+
+/**
+ * struct i915_gem_apply_to_region - Argument to the struct
+ * i915_gem_apply_to_region_ops functions.
+ * @ops: The ops for the operation.
+ * @ww: Locking context used for the transaction.
+ * @interruptible: Whether to perform object locking interruptible.
+ *
+ * This structure is intended to be embedded in a private
[PATCH v4 1/6] drm/i915/ttm: Implement a function to copy the contents of two TTM-based objects
When backing up or restoring contents of pinned objects at suspend /
resume time we need to allocate a new object as the backup. Add a function
to facilitate copies between the two. Some data needs to be copied before
the migration context is ready for operation, so make sure we can
disable accelerated copies.
v2:
- Fix a missing return value check (Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 69 +
drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 4 ++
2 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 2f672f06b169..22d59510d0c3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -428,6 +428,7 @@ i915_ttm_resource_get_st(struct drm_i915_gem_object *obj,
static int i915_ttm_accel_move(struct ttm_buffer_object *bo,
bool clear,
struct ttm_resource *dst_mem,
+ struct ttm_tt *dst_ttm,
struct sg_table *dst_st)
{
struct drm_i915_private *i915 = container_of(bo->bdev, typeof(*i915),
@@ -437,14 +438,14 @@ static int i915_ttm_accel_move(struct ttm_buffer_object
*bo,
struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
struct sg_table *src_st;
struct i915_request *rq;
- struct ttm_tt *ttm = bo->ttm;
+ struct ttm_tt *src_ttm = bo->ttm;
enum i915_cache_level src_level, dst_level;
int ret;
if (!i915->gt.migrate.context)
return -EINVAL;
- dst_level = i915_ttm_cache_level(i915, dst_mem, ttm);
+ dst_level = i915_ttm_cache_level(i915, dst_mem, dst_ttm);
if (clear) {
if (bo->type == ttm_bo_type_kernel)
return -EINVAL;
@@ -461,10 +462,10 @@ static int i915_ttm_accel_move(struct ttm_buffer_object
*bo,
}
intel_engine_pm_put(i915->gt.migrate.context->engine);
} else {
- src_st = src_man->use_tt ? i915_ttm_tt_get_st(ttm) :
+ src_st = src_man->use_tt ? i915_ttm_tt_get_st(src_ttm) :
obj->ttm.cached_io_st;
- src_level = i915_ttm_cache_level(i915, bo->resource, ttm);
+ src_level = i915_ttm_cache_level(i915, bo->resource, src_ttm);
intel_engine_pm_get(i915->gt.migrate.context->engine);
ret = intel_context_migrate_copy(i915->gt.migrate.context,
NULL, src_st->sgl, src_level,
@@ -484,11 +485,14 @@ static int i915_ttm_accel_move(struct ttm_buffer_object
*bo,
static void __i915_ttm_move(struct ttm_buffer_object *bo, bool clear,
struct ttm_resource *dst_mem,
- struct sg_table *dst_st)
+ struct ttm_tt *dst_ttm,
+ struct sg_table *dst_st,
+ bool allow_accel)
{
- int ret;
+ int ret = -EINVAL;
- ret = i915_ttm_accel_move(bo, clear, dst_mem, dst_st);
+ if (allow_accel)
+ ret = i915_ttm_accel_move(bo, clear, dst_mem, dst_ttm, dst_st);
if (ret) {
struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
struct intel_memory_region *dst_reg, *src_reg;
@@ -503,7 +507,7 @@ static void __i915_ttm_move(struct ttm_buffer_object *bo,
bool clear,
GEM_BUG_ON(!dst_reg || !src_reg);
dst_iter = !cpu_maps_iomem(dst_mem) ?
- ttm_kmap_iter_tt_init(&_dst_iter.tt, bo->ttm) :
+ ttm_kmap_iter_tt_init(&_dst_iter.tt, dst_ttm) :
ttm_kmap_iter_iomap_init(&_dst_iter.io, _reg->iomap,
dst_st, dst_reg->region.start);
@@ -558,7 +562,7 @@ static int i915_ttm_move(struct ttm_buffer_object *bo, bool
evict,
clear = !cpu_maps_iomem(bo->resource) && (!ttm ||
!ttm_tt_is_populated(ttm));
if (!(clear && ttm && !(ttm->page_flags & TTM_PAGE_FLAG_ZERO_ALLOC)))
- __i915_ttm_move(bo, clear, dst_mem, dst_st);
+ __i915_ttm_move(bo, clear, dst_mem, bo->ttm, dst_st, true);
ttm_bo_move_sync_cleanup(bo, dst_mem);
i915_ttm_adjust_domains_after_move(obj);
@@ -973,3 +977,50 @@ i915_gem_ttm_system_setup(struct drm_i915_private *i915,
intel_memory_region_set_name(mr, "system-ttm");
return mr;
}
+
+/**
+ * i915_gem_obj_copy_ttm - Copy the contents of one ttm-based gem object to
+ * another
+ * @dst: The destination object
+ * @src: The source object
+ * @allow_accel: Allow using the blitter. Otherwise TTM memcpy is used.
+ * @intr: Whether to perform waits interruptible:
+ *
+ * Note: The caller is responsible for assuring that the underlying
+
[PATCH v4 0/6] drm/i915: Suspend / resume backup- and restore of LMEM.
Implement backup and restore of LMEM during suspend / resume. What complicates things a bit is handling of pinned LMEM memory during suspend and the fact that we might be dealing with unmappable LMEM in the future, which makes us want to restrict the number of pinned objects that need memcpy resume. The first two patches are prereq patches implementing object content copy and a generic means of iterating through all objects in a region. The third patch adds the backup / recover / restore functions and the two last patches deal with restricting the number of objects we need to use memcpy for. v2: - Some polishing of patch 4/6, see patch commit message for details (Chris Wilson) - Rework of patch 3/6. v3: - Comment changes in patch 2/6 (Matthew Auld) - A number of changes to patch 3/6, see commit message. - Slightly reword comment in patch 5/6. (Matthew Auld). v4: - Various cleanups, among other things reworking the ttm / lmem backup- and resume interfaces somewhat. Thomas Hellström (6): drm/i915/ttm: Implement a function to copy the contents of two TTM-based objects drm/i915/gem: Implement a function to process all gem objects of a region drm/i915 Implement LMEM backup and restore for suspend / resume drm/i915/gt: Register the migrate contexts with their engines drm/i915: Don't back up pinned LMEM context images and rings during suspend drm/i915: Reduce the number of objects subject to memcpy recover drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gem/i915_gem_context.c | 4 +- .../gpu/drm/i915/gem/i915_gem_object_types.h | 21 +- drivers/gpu/drm/i915/gem/i915_gem_pm.c| 91 drivers/gpu/drm/i915/gem/i915_gem_pm.h| 1 + drivers/gpu/drm/i915/gem/i915_gem_region.c| 70 ++ drivers/gpu/drm/i915/gem/i915_gem_region.h| 37 drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 99 +++-- drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 14 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c| 206 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.h| 26 +++ .../gpu/drm/i915/gem/selftests/huge_pages.c | 2 +- drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 2 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 5 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.h | 4 +- drivers/gpu/drm/i915/gt/intel_context_types.h | 8 + drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 + drivers/gpu/drm/i915/gt/intel_engine_pm.c | 23 ++ drivers/gpu/drm/i915/gt/intel_engine_pm.h | 2 + drivers/gpu/drm/i915/gt/intel_engine_types.h | 7 + .../drm/i915/gt/intel_execlists_submission.c | 2 + drivers/gpu/drm/i915/gt/intel_ggtt.c | 3 +- drivers/gpu/drm/i915/gt/intel_gt.c| 2 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 4 +- drivers/gpu/drm/i915/gt/intel_gtt.c | 3 +- drivers/gpu/drm/i915/gt/intel_gtt.h | 9 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 3 +- drivers/gpu/drm/i915/gt/intel_migrate.c | 2 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 13 +- drivers/gpu/drm/i915/gt/intel_ring.c | 3 +- .../gpu/drm/i915/gt/intel_ring_submission.c | 3 + drivers/gpu/drm/i915/gt/mock_engine.c | 2 + drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 12 +- drivers/gpu/drm/i915/gvt/scheduler.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 4 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 +- 37 files changed, 644 insertions(+), 56 deletions(-) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.h -- 2.31.1
[PATCH v4 6/6] drm/i915: Reduce the number of objects subject to memcpy recover
We really only need memcpy restore for objects that affect the
operability of the migrate context. That is, primarily the page-table
objects of the migrate VM.
Add an object flag, I915_BO_ALLOC_PM_EARLY for objects that need early
restores using memcpy and a way to assign LMEM page-table object flags
to be used by the vms.
Restore objects without this flag with the gpu blitter and only objects
carrying the flag using TTM memcpy.
Initially mark the migrate, gt, gtt and vgpu vms to use this flag, and
defer for a later audit which vms actually need it. Most importantly, user-
allocated vms with pinned page-table objects can be restored using the
blitter.
Performance-wise memcpy restore is probably as fast as gpu restore if not
faster, but using gpu restore will help tackling future restrictions in
mappable LMEM size.
v4:
- Don't mark the aliasing ppgtt page table flags for early resume, but
rather the ggtt page table flags as intended. (Matthew Auld)
- The check for user buffer objects during early resume is pointless, since
they are never marked I915_BO_ALLOC_PM_EARLY. (Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 4 ++--
drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++---
drivers/gpu/drm/i915/gem/i915_gem_pm.c | 6 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c | 5 +++--
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 2 +-
drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 2 +-
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 5 +++--
drivers/gpu/drm/i915/gt/gen8_ppgtt.h | 4 +++-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 3 ++-
drivers/gpu/drm/i915/gt/intel_gt.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gtt.c | 3 ++-
drivers/gpu/drm/i915/gt/intel_gtt.h | 9 +++--
drivers/gpu/drm/i915/gt/intel_migrate.c | 2 +-
drivers/gpu/drm/i915/gt/intel_ppgtt.c| 13 -
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +-
drivers/gpu/drm/i915/gvt/scheduler.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c| 4 ++--
17 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c2ab0e22db0a..8208fd5b72c3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1287,7 +1287,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
} else if (HAS_FULL_PPGTT(i915)) {
struct i915_ppgtt *ppgtt;
- ppgtt = i915_ppgtt_create(>gt);
+ ppgtt = i915_ppgtt_create(>gt, 0);
if (IS_ERR(ppgtt)) {
drm_dbg(>drm, "PPGTT setup failed (%ld)\n",
PTR_ERR(ppgtt));
@@ -1465,7 +1465,7 @@ int i915_gem_vm_create_ioctl(struct drm_device *dev, void
*data,
if (args->flags)
return -EINVAL;
- ppgtt = i915_ppgtt_create(>gt);
+ ppgtt = i915_ppgtt_create(>gt, 0);
if (IS_ERR(ppgtt))
return PTR_ERR(ppgtt);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 118691ce81d7..fa2ba9e2a4d0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -294,13 +294,16 @@ struct drm_i915_gem_object {
#define I915_BO_ALLOC_USERBIT(3)
/* Object is allowed to lose its contents on suspend / resume, even if pinned
*/
#define I915_BO_ALLOC_PM_VOLATILE BIT(4)
+/* Object needs to be restored early using memcpy during resume */
+#define I915_BO_ALLOC_PM_EARLYBIT(5)
#define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | \
I915_BO_ALLOC_VOLATILE | \
I915_BO_ALLOC_CPU_CLEAR | \
I915_BO_ALLOC_USER | \
-I915_BO_ALLOC_PM_VOLATILE)
-#define I915_BO_READONLY BIT(5)
-#define I915_TILING_QUIRK_BIT 6 /* unknown swizzling; do not release! */
+I915_BO_ALLOC_PM_VOLATILE | \
+I915_BO_ALLOC_PM_EARLY)
+#define I915_BO_READONLY BIT(6)
+#define I915_TILING_QUIRK_BIT 7 /* unknown swizzling; do not release! */
/**
* @mem_flags - Mutable placement-related flags
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 12b37b4c1192..726b40e1fbb0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -97,8 +97,12 @@ int i915_gem_backup_suspend(struct drm_i915_private *i915)
* More objects may have become unpinned as requests were
* retired. Now try to evict again. The gt may be wedged here
* in which case we automatically fall back
[Bug 211277] sometimes crash at s2ram-wake (Ryzen 3500U): amdgpu, drm, commit_tail, amdgpu_dm_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=211277 youling...@gmail.com changed: What|Removed |Added CC||youling...@gmail.com --- Comment #58 from youling...@gmail.com --- drm/amdgpu: move iommu_resume before ip init/resume cause suspend to disk resume failed on my amdgpu 3400g. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v3 6/6] drm/i915: Reduce the number of objects subject to memcpy recover
On 14/09/2021 20:31, Thomas Hellström wrote:
We really only need memcpy restore for objects that affect the
operability of the migrate context. That is, primarily the page-table
objects of the migrate VM.
Add an object flag, I915_BO_ALLOC_PM_EARLY for objects that need early
restores using memcpy and a way to assign LMEM page-table object flags
to be used by the vms.
Restore objects without this flag with the gpu blitter and only objects
carrying the flag using TTM memcpy.
Initially mark the migrate, gt, gtt and vgpu vms to use this flag, and
defer for a later audit which vms actually need it. Most importantly, user-
allocated vms with pinned page-table objects can be restored using the
blitter.
Performance-wise memcpy restore is probably as fast as gpu restore if not
faster, but using gpu restore will help tackling future restrictions in
mappable LMEM size.
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 4 ++--
drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++---
drivers/gpu/drm/i915/gem/i915_gem_pm.c | 5 -
drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c | 6 --
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 2 +-
drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 2 +-
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 5 +++--
drivers/gpu/drm/i915/gt/gen8_ppgtt.h | 4 +++-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gtt.c | 3 ++-
drivers/gpu/drm/i915/gt/intel_gtt.h | 9 +++--
drivers/gpu/drm/i915/gt/intel_migrate.c | 2 +-
drivers/gpu/drm/i915/gt/intel_ppgtt.c| 13 -
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +-
drivers/gpu/drm/i915/gvt/scheduler.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c| 4 ++--
17 files changed, 48 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c2ab0e22db0a..8208fd5b72c3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1287,7 +1287,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
} else if (HAS_FULL_PPGTT(i915)) {
struct i915_ppgtt *ppgtt;
- ppgtt = i915_ppgtt_create(>gt);
+ ppgtt = i915_ppgtt_create(>gt, 0);
if (IS_ERR(ppgtt)) {
drm_dbg(>drm, "PPGTT setup failed (%ld)\n",
PTR_ERR(ppgtt));
@@ -1465,7 +1465,7 @@ int i915_gem_vm_create_ioctl(struct drm_device *dev, void
*data,
if (args->flags)
return -EINVAL;
- ppgtt = i915_ppgtt_create(>gt);
+ ppgtt = i915_ppgtt_create(>gt, 0);
if (IS_ERR(ppgtt))
return PTR_ERR(ppgtt);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 118691ce81d7..fa2ba9e2a4d0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -294,13 +294,16 @@ struct drm_i915_gem_object {
#define I915_BO_ALLOC_USERBIT(3)
/* Object is allowed to lose its contents on suspend / resume, even if pinned
*/
#define I915_BO_ALLOC_PM_VOLATILE BIT(4)
+/* Object needs to be restored early using memcpy during resume */
+#define I915_BO_ALLOC_PM_EARLYBIT(5)
#define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | \
I915_BO_ALLOC_VOLATILE | \
I915_BO_ALLOC_CPU_CLEAR | \
I915_BO_ALLOC_USER | \
-I915_BO_ALLOC_PM_VOLATILE)
-#define I915_BO_READONLY BIT(5)
-#define I915_TILING_QUIRK_BIT 6 /* unknown swizzling; do not release! */
+I915_BO_ALLOC_PM_VOLATILE | \
+I915_BO_ALLOC_PM_EARLY)
+#define I915_BO_READONLY BIT(6)
+#define I915_TILING_QUIRK_BIT 7 /* unknown swizzling; do not release! */
/**
* @mem_flags - Mutable placement-related flags
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 8736ae1dfbb2..c4a75e1c12ee 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -98,8 +98,11 @@ int i915_gem_backup_suspend(struct drm_i915_private *i915)
* More objects may have become unpinned as requests were
* retired. Now try to evict again. The gt may be wedged here
* in which case we automatically fall back to memcpy.
+* We allow also backing up pinned objects that have not been
+* marked for early recover, and that may contain, for example,
+* page-tables for the migrate context.
*/
- ret = lmem_suspend(i915, true,
Re: [PATCH v3 3/6] drm/i915 Implement LMEM backup and restore for suspend / resume
On Mon, 2021-09-20 at 11:49 +0100, Matthew Auld wrote:
> On 14/09/2021 20:31, Thomas Hellström wrote:
> > Just evict unpinned objects to system. For pinned LMEM objects,
> > make a backup system object and blit the contents to that.
> >
> > Backup is performed in three steps,
> > 1: Opportunistically evict evictable objects using the gpu blitter.
> > 2: After gt idle, evict evictable objects using the gpu blitter.
> > This will
> > be modified in an upcoming patch to backup pinned objects that are
> > not used
> > by the blitter itself.
> > 3: Backup remaining pinned objects using memcpy.
> >
> > Also move uC suspend to after 2) to make sure we have a functional
> > GuC
> > during 2) if using GuC submission.
> >
> > v2:
> > - Major refactor to make sure gem_exec_suspend@hang-SX subtests
> > work, and
> > suspend / resume works with a slightly modified GuC submission
> > enabling
> > patch series.
> >
> > v3:
> > - Fix a potential use-after-free (Matthew Auld)
> > - Use i915_gem_object_create_shmem() instead of
> > i915_gem_object_create_region (Matthew Auld)
> > - Minor simplifications (Matthew Auld)
> > - Fix up kerneldoc for i195_ttm_restore_region().
> > - Final lmem_suspend() call moved to i915_gem_backup_suspend from
> > i915_gem_suspend_late, since the latter gets called at driver
> > unload
> > and we don't unnecessarily want to run it at that time.
> >
> > Signed-off-by: Thomas Hellström
>
>
>
> > +
> > +static int i915_ttm_restore(struct i915_gem_apply_to_region
> > *apply,
> > + struct drm_i915_gem_object *obj)
> > +{
> > + struct i915_gem_ttm_pm_apply *pm_apply =
> > + container_of(apply, typeof(*pm_apply), base);
> > + struct drm_i915_gem_object *backup = obj->ttm.backup;
> > + struct ttm_buffer_object *backup_bo =
> > i915_gem_to_ttm(backup);
> > + struct ttm_operation_ctx ctx = {};
> > + int err;
> > +
> > + if (!backup)
> > + return 0;
> > +
> > + if (!pm_apply->allow_gpu && (obj->flags &
> > I915_BO_ALLOC_USER))
> > + return 0;
>
> Hmm, do we ever hit this? I would presume anything that userspace
> directly allocated in lmem can be kicked out with
> ttm_bo_validate(sys)
> i.e backup == NULL?
At this point, (before patch 6/6) I think we might do. Typical
candidates are dma-buf objects that have become pinned on exporting,
and perhaps framebuffers that are pinned, not sure they are unpinned
before we back them up.
But it might be that we should remove this after patch 6/6 where we
require a special flag for early recovers using memcpy.
/Thomas
>
> > +
> > + err = i915_gem_object_lock(backup, apply->ww);
> > + if (err)
> > + return err;
> > +
> > + /* Content may have been swapped. */
> > + err = ttm_tt_populate(backup_bo->bdev, backup_bo->ttm,
> > );
> > + if (!err) {
> > + err = i915_gem_obj_copy_ttm(obj, backup, pm_apply-
> > >allow_gpu,
> > + false);
> > + GEM_WARN_ON(err);
> > +
> > + obj->ttm.backup = NULL;
> > + err = 0;
> > + }
> > +
> > + i915_gem_ww_unlock_single(backup);
> > +
> > + if (!err)
> > + i915_gem_object_put(backup);
> > +
> > + return err;
> > +}
> > +
[Bug 211277] sometimes crash at s2ram-wake (Ryzen 3500U): amdgpu, drm, commit_tail, amdgpu_dm_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=211277 --- Comment #60 from youling...@gmail.com --- Created attachment 298889 --> https://bugzilla.kernel.org/attachment.cgi?id=298889=edit dmesg5.15.txt (In reply to James Zhu from comment #59) > (In reply to youling257 from comment #58) > > drm/amdgpu: move iommu_resume before ip init/resume cause suspend to disk > > resume failed on my amdgpu 3400g. > > Can you share whole demsg log? Regards! James when resume failed have to force shutdown, how to output dmesg? only has boot log dmesg. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: Regression with mainline kernel on rpi4
On Sat, Sep 04, 2021 at 10:40:29AM +0100, Sudip Mukherjee wrote: > Hi Maxime, > > On Sat, Sep 4, 2021 at 10:10 AM Maxime Ripard wrote: > > > > On Fri, Sep 03, 2021 at 09:09:50PM +0100, Sudip Mukherjee wrote: > > > Hi Maxime, > > > > > > On Fri, Sep 3, 2021 at 5:03 PM Maxime Ripard wrote: > > > > > > > > Hi Sudip, > > > > > > > > On Thu, Sep 02, 2021 at 10:08:19AM +0100, Sudip Mukherjee wrote: > > > > > Hi All, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You can see the complete dmesg at > > > > > https://openqa.qa.codethink.co.uk/tests/76#step/dmesg/8 > > > > > > > > What test were you running? > > > > > > Nothing particular, its just a boot test that we do every night after > > > pulling from torvalds/linux.git > > > > What are you booting to then? > > I am not sure I understood the question. > Its an Ubuntu image. The desktop environment is gnome. And as > mentioned earlier, we use the HEAD of linus tree every night to boot > the rpi4 and test that we can login via desktop environment and that > there is no regression in dmesg. Looking at the CI, this isn't from a RPi but from qemu? What defconfig are you using? How did you generate the Ubuntu image? Through debootstrap? Any additional package? Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/ttm: Don't delete the system manager before the delayed delete
> On Sep 20, 2021, at 02:30, Christian König wrote: > > Am 17.09.21 um 19:53 schrieb Zack Rusin: >> On some hardware, in particular in virtualized environments, the >> system memory can be shared with the "hardware". In those cases >> the BO's allocated through the ttm system manager might be >> busy during ttm_bo_put which results in them being scheduled >> for a delayed deletion. > > While the patch itself is probably fine the reasoning here is a clear NAK. > > Buffers in the system domain are not GPU accessible by definition, even in a > shared environment and so *must* be idle. I’m assuming that means they are not allowed to be ever fenced then, yes? > Otherwise you break quite a number of assumptions in the code. Are there more assumptions like that or do you mean there’s more places that depend on the assumption that system domain bo’s are always idle? If there’s more assumptions like that in TTM that would be incredibly valuable to know. I haven’t been paying much attention to the kernel code in years and coming back now and looking at a few years old vmwgfx code it’s almost impossible to tell the difference between: “this assumption breaks the driver” and “this driver breaks this assumption”. z
Re: [igt-dev] [PATCH i-g-t v11 00/15] Introduce PXP Test
On Fri, Sep 17, 2021 at 09:54:21PM -0700, Alan Previn wrote: > This series adds gem_pxp tests for the new PXP subsystem currently > being reviewed at https://patchwork.freedesktop.org/series/90504/. > This series currently includes 4 groups of tests addressing the > features and restrictions described by Daniele's series : >1. test i915 interfaces for allocation of protected bo's > and contexts and enforcement of UAPI rule disallowing the > modification of parameters after it's been created. >2. verify PXP subsystem protected sessions generate encrypted > content on protected output buffers and decrypt protected > inputs buffers. >3. verify i915 PXP auto-teardown succeeds on suspend-resume > cycles and gem-exec of stale protected assets fail. Ensure > protected-contexts adhere to stricter invalidation > enforcement upon teardown event. >4. Ensure that display plane decryption works as expected with > protected buffers. > > NOTE: This series is on the tenth revision. All R-v-b's have been > received except UAPI patch which will be dropped at merge time (after > kernel patches gets merged and igt's drm UAPI gets sync'd). > > Changes from prior rev1 to now: >v11: > - When detecting hw support, retry pxp context creation > multiple times a timeout as per HW SLA. > - initialize bo or ctx handles to zero before calling > creation ioctl wrapper. Acked-by: Rodrigo Vivi >v10: > - In patch #2, reuse existing gem_create_ext wrapper. > - In patch #10, kernel side changed the debugfs file name > (but no difference in behavior / usage). > - Removed patch #14 from Rev9 as decision on kernel side > was to drop the usage of RESET_STATS IOCTL to track > invalidated pxp contexts. >v9: > - Remove patch #2 from rev7 as it was duplicating > an existing ioctl wrapper helper > - Fix the false-negative warnings when triggering > auto-suspend-resume (remove checking if we are > suspending after the system has already resumed). >v8: > - Nothing - mistaken detection from patchwork >v7: > - In prior rev, Patches #11->13 was testing expected results > from calling gem_execbuf with stale pxp-context, pxp-buffer > or combinations of them (including an opt-out usage). All > of them used a single suspend-resume power state cycles to > trigger the PXP teardown event. These patches have been > combined into patch #14 that continues to carry the prior rev > Rvb. > - In its place, the new patches of #11->#13 do the identical > set of tests as before (results from gem_execbuf with various > combinations of stale pxp context and buffer), but this time > using a debugfs file handle that triggers the same code path > taken when the HW triggers the pxp teardown. That said, the > code is nearly identical as v6 but I did not keep the Rvb's. > - In patch #15, RESET_STAT now reports invalidated / banned > pxp contexts via the existing batch_active's lost count. >v6: > - Addressed rev5 review comments for patch #1, #7, #14 > and #17. > - For #17, I'm using Rodrigo's Rv-b because offline > discussions concluded that we couldn't use those > test sequences with HDCP and so it was removed it. > - Added Rv-b into all patches that received it. > - Modified the test requirement from a list of device > ids to checking if runtime PXP interface succeeds > due to kernel's build config dependency. >v5: > - Addressed all rev4 review comments. No changes to > overall flow and logic compared to the last rev. >v4: > - Addressed all rev3 review comments. NOTE: that all > test cases and code logic are the same but a decent > amount of refactoring has occured due to address > v3 comments to break out subtests into separate > functions while combining certain checks into the same > function to reduce test time by minimizing number of > suspend-resume power cycles. >v3: > - Addressed all rev2 review comments. > - In line with one of the rev2 comments, a thorough fixup > of all line-breaks in function calls was made for a more > consistent styling. > - Rebased on igt upstream repo and updated to latest kernel > UAPI that added GEM_CREATE_EXT. >v2: > - Addressed all rev1 review comments except these: >1.Chris Wilson : "...have the caller do 1-3 once for its protected > context. Call it something like intel_bb_enable_pxp(), > intel_bb_set_pxp if it should be reversible.". > - This couldn't be implemented because [1] HW needs different > instruction sequences for enabling/disabling PXP depending >
Re: [PATCH] agp: define proper stubs for empty helpers
On Mon, Sep 20, 2021 at 02:17:19PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> The empty unmap_page_from_agp() macro causes a warning when
> building with 'make W=1' on a couple of architectures:
>
> drivers/char/agp/generic.c: In function 'agp_generic_destroy_page':
> drivers/char/agp/generic.c:1265:28: error: suggest braces around empty body
> in an 'if' statement [-Werror=empty-body]
> 1265 | unmap_page_from_agp(page);
>
> Change the definitions to a 'do { } while (0)' construct to
> make these more reliable.
>
> Signed-off-by: Arnd Bergmann
Looks good.
Acked-by: Sam Ravnborg
Sam
Re: [PATCH] doc: gpu: drm-internals: Create reference to DRM mm
On Mon, 20 Sep 2021, Markus Schneider-Pargmann wrote: > This short sentence references nothing for details about memory manager. > Replace it with the documentation file for DRM memory management. > > Cc: Jani Nikula > Signed-off-by: Markus Schneider-Pargmann > --- > Documentation/gpu/drm-internals.rst | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/gpu/drm-internals.rst > b/Documentation/gpu/drm-internals.rst > index 06af044c882f..bdcdfc4ede04 100644 > --- a/Documentation/gpu/drm-internals.rst > +++ b/Documentation/gpu/drm-internals.rst > @@ -126,8 +126,8 @@ Memory Manager Initialization > Every DRM driver requires a memory manager which must be initialized at > load time. DRM currently contains two memory managers, the Translation > Table Manager (TTM) and the Graphics Execution Manager (GEM). This > -document describes the use of the GEM memory manager only. See ? for > -details. > +document describes the use of the GEM memory manager only. See > +Documentation/gpu/drm-mm.rst for details. Please use rst references instead of a file reference. BR, Jani. > > Miscellaneous Device Configuration > ~~ -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v2 4/9] vfio/ccw: Make the FSM complete and synchronize it to the mdev
On Thu, Sep 09 2021, Jason Gunthorpe wrote: > The subchannel should be left in a quiescent state unless the VFIO device > FD is opened. When the FD is opened bring the chanel to active and allow > the VFIO device to operate. When the device FD is closed then quiesce the > channel. > > To make this work the FSM needs to handle the transitions to/from open and > closed so everything is sequenced. Rename state NOT_OPER to BROKEN and use > it wheneven the driver has malfunctioned. STANDBY becomes CLOSED. The > normal case FSM looks like: > CLOSED -> IDLE -> PROCESS/PENDING* -> IDLE -> CLOSED > > With a possible branch off to BROKEN from any state. Once the device is in > BROKEN it cannot be recovered other than be reloading the driver. Hm, not sure whether it is a good idea to conflate "something went wrong" and "device is not operational". In the latter case, we will eventually get a removal of the css device when the common I/O layer has processed the channel report for the subchannel; while the former case could mean all kind of things, but the subchannel will likely stay around. I think NOT_OPER was always meant to be a transitional state. > > Delete the triply redundant calls to > vfio_ccw_sch_quiesce(). vfio_ccw_mdev_close_device() always leaves the > subchannel quiescent. vfio_ccw_mdev_remove() cannot return until > vfio_ccw_mdev_close_device() completes and vfio_ccw_sch_remove() cannot > return until vfio_ccw_mdev_remove() completes. Have the FSM code take care > of calling cp_free() when appropriate. I remember some serialization issues wrt cp_free() etc. coming up every now and than; that might need extra care (I'm taking a look.) > > Device reset becomes a CLOSE/OPEN sequence which now properly handles the > situation if the device becomes BROKEN. > > Machine shutdown via vfio_ccw_sch_shutdown() now simply tries to close and > leaves the device BROKEN (though arguably the bus should take care to > quiet down the subchannel HW during shutdown, not the drivers) The problem is that there is not really a uniform thing that can be done for shutdown; e.g. we can quiesce and then disable I/O and EADM subchannels, but CHSC subchannels cannot be quiesced.
Re: [PATCH] staging: fbtft: add docs for fbtft_write_spi()
On Mon, Sep 20, 2021 at 09:07:13AM -0700, Zachary Mayhew wrote:
> On Mon, Sep 20, 2021 at 05:38:30PM +0200, Greg KH wrote:
> > On Mon, Sep 20, 2021 at 08:26:03AM -0700, Zachary Mayhew wrote:
> > > Subject: [PATCH] staging: fbtft: add docs for fbtft_write_spi()
> >
> > Odd, this shouldn't be in the body of the email :(
> >
> > >
> > > This patch adds documentation for fbtft_write_spi() to make its
> > > calling context clear and explain what it does.
> > >
> > > Signed-off-by: Zachary Mayhew
> > > ---
> > > drivers/staging/fbtft/fbtft-io.c | 13 +
> > > 1 file changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/staging/fbtft/fbtft-io.c
> > > b/drivers/staging/fbtft/fbtft-io.c
> > > index de1904a443c2..985d7cf8c774 100644
> > > --- a/drivers/staging/fbtft/fbtft-io.c
> > > +++ b/drivers/staging/fbtft/fbtft-io.c
> > > @@ -5,6 +5,19 @@
> > > #include
> > > #include "fbtft.h"
> > >
> > > +/**
> > > + * fbtft_write_spi() - write data to current spi
> > > + * @par: Driver data including driver spi_device
> > > + * @buf: Buffer to write to spi
> > > + * @len: Length of the buffer
> > > + * Context: can sleep
> > > + *
> > > + * Builds an spi_transfer and spi_message object based
> > > on the
> > > + * given @buf and @len. These are then used in a call to spi_sync()
> > > which will
> > > + * write to the spi.
> > > + *
> > > + * Return: zero on success or else a negative error code
> > > + */
> > > int fbtft_write_spi(struct fbtft_par *par, void *buf, size_t len)
> > > {
> > > struct spi_transfer t = {
> > > --
> > > 2.33.0
> > >
> > >
> >
> > Is this file being imported into the kernel doc tools? If so, great, if
> > not, this isn't going to help out all that much, right?
>
> It doesn't appear to be imported at this time, as such it may not be
> necessary.
So is this change needed?
thanks,
greg k-h
Re: [PATCH v3 3/6] drm/i915 Implement LMEM backup and restore for suspend / resume
On 14/09/2021 20:31, Thomas Hellström wrote:
Just evict unpinned objects to system. For pinned LMEM objects,
make a backup system object and blit the contents to that.
Backup is performed in three steps,
1: Opportunistically evict evictable objects using the gpu blitter.
2: After gt idle, evict evictable objects using the gpu blitter. This will
be modified in an upcoming patch to backup pinned objects that are not used
by the blitter itself.
3: Backup remaining pinned objects using memcpy.
Also move uC suspend to after 2) to make sure we have a functional GuC
during 2) if using GuC submission.
v2:
- Major refactor to make sure gem_exec_suspend@hang-SX subtests work, and
suspend / resume works with a slightly modified GuC submission enabling
patch series.
v3:
- Fix a potential use-after-free (Matthew Auld)
- Use i915_gem_object_create_shmem() instead of
i915_gem_object_create_region (Matthew Auld)
- Minor simplifications (Matthew Auld)
- Fix up kerneldoc for i195_ttm_restore_region().
- Final lmem_suspend() call moved to i915_gem_backup_suspend from
i915_gem_suspend_late, since the latter gets called at driver unload
and we don't unnecessarily want to run it at that time.
Signed-off-by: Thomas Hellström
+
+static int i915_ttm_restore(struct i915_gem_apply_to_region *apply,
+ struct drm_i915_gem_object *obj)
+{
+ struct i915_gem_ttm_pm_apply *pm_apply =
+ container_of(apply, typeof(*pm_apply), base);
+ struct drm_i915_gem_object *backup = obj->ttm.backup;
+ struct ttm_buffer_object *backup_bo = i915_gem_to_ttm(backup);
+ struct ttm_operation_ctx ctx = {};
+ int err;
+
+ if (!backup)
+ return 0;
+
+ if (!pm_apply->allow_gpu && (obj->flags & I915_BO_ALLOC_USER))
+ return 0;
Hmm, do we ever hit this? I would presume anything that userspace
directly allocated in lmem can be kicked out with ttm_bo_validate(sys)
i.e backup == NULL?
+
+ err = i915_gem_object_lock(backup, apply->ww);
+ if (err)
+ return err;
+
+ /* Content may have been swapped. */
+ err = ttm_tt_populate(backup_bo->bdev, backup_bo->ttm, );
+ if (!err) {
+ err = i915_gem_obj_copy_ttm(obj, backup, pm_apply->allow_gpu,
+ false);
+ GEM_WARN_ON(err);
+
+ obj->ttm.backup = NULL;
+ err = 0;
+ }
+
+ i915_gem_ww_unlock_single(backup);
+
+ if (!err)
+ i915_gem_object_put(backup);
+
+ return err;
+}
+
??????[PATCH] drm: bridge: it66121: Added it66121 chip external screen status judgment.
OK??I will modify the message. Thanks a lot. ------ ??: Robert Foss
Re: [PATCH 5/9] drm/panfrost: simplify getting .driver_data
Reviewed-by: Alyssa Rosenzweig
> index bd9b7be63b0f..fd4309209088 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -400,8 +400,7 @@ void panfrost_device_reset(struct panfrost_device *pfdev)
> #ifdef CONFIG_PM
> int panfrost_device_resume(struct device *dev)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct panfrost_device *pfdev = platform_get_drvdata(pdev);
> + struct panfrost_device *pfdev = dev_get_drvdata(dev);
>
> panfrost_device_reset(pfdev);
> panfrost_devfreq_resume(pfdev);
> @@ -411,8 +410,7 @@ int panfrost_device_resume(struct device *dev)
>
> int panfrost_device_suspend(struct device *dev)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct panfrost_device *pfdev = platform_get_drvdata(pdev);
> + struct panfrost_device *pfdev = dev_get_drvdata(dev);
>
> if (!panfrost_job_is_idle(pfdev))
> return -EBUSY;
> --
> 2.30.2
>
Re: [PATCH v2 2/9] vfio/ccw: Pass vfio_ccw_private not mdev_device to various functions
On Thu, Sep 09 2021, Jason Gunthorpe wrote: > mdev_device should only be used in functions assigned to ops callbacks, > interior functions should use the struct vfio_ccw_private instead of > repeatedly trying to get it from the mdev. > > Signed-off-by: Jason Gunthorpe > --- > drivers/s390/cio/vfio_ccw_ops.c | 37 + > 1 file changed, 15 insertions(+), 22 deletions(-) Reviewed-by: Cornelia Huck
Re: [PATCH 5/9] drm/panfrost: simplify getting .driver_data
On 20/09/2021 10:05, Wolfram Sang wrote:
> We should get 'driver_data' from 'struct device' directly. Going via
> platform_device is an unneeded step back and forth.
>
> Signed-off-by: Wolfram Sang
> ---
Reviewed-by: Steven Price
I'll push this to drm-misc-next.
Thanks,
Steve
>
> Build tested only. buildbot is happy.
>
> drivers/gpu/drm/panfrost/panfrost_device.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
> b/drivers/gpu/drm/panfrost/panfrost_device.c
> index bd9b7be63b0f..fd4309209088 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -400,8 +400,7 @@ void panfrost_device_reset(struct panfrost_device *pfdev)
> #ifdef CONFIG_PM
> int panfrost_device_resume(struct device *dev)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct panfrost_device *pfdev = platform_get_drvdata(pdev);
> + struct panfrost_device *pfdev = dev_get_drvdata(dev);
>
> panfrost_device_reset(pfdev);
> panfrost_devfreq_resume(pfdev);
> @@ -411,8 +410,7 @@ int panfrost_device_resume(struct device *dev)
>
> int panfrost_device_suspend(struct device *dev)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct panfrost_device *pfdev = platform_get_drvdata(pdev);
> + struct panfrost_device *pfdev = dev_get_drvdata(dev);
>
> if (!panfrost_job_is_idle(pfdev))
> return -EBUSY;
>
Re: [PATCH] panfrost: make mediatek_mt8183_supplies and mediatek_mt8183_pm_domains static
On 18/09/2021 10:13, Jiapeng Chong wrote:
> This symbol is not used outside of panfrost_drv.c, so marks it static.
>
> Fix the following sparse warning:
>
> drivers/gpu/drm/panfrost/panfrost_drv.c:641:12: warning: symbol
> 'mediatek_mt8183_supplies' was not declared. Should it be static?
>
> drivers/gpu/drm/panfrost/panfrost_drv.c:642:12: warning: symbol
> 'mediatek_mt8183_pm_domains' was not declared. Should it be static?
>
> Reported-by: Abaci Robot
> Signed-off-by: Jiapeng Chong
Reviewed-by: Steven Price
I'll push to drm-misc-next.
Thanks,
Steve
> ---
> drivers/gpu/drm/panfrost/panfrost_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 077cbbf..82ad9a6 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -638,8 +638,8 @@ static int panfrost_remove(struct platform_device *pdev)
> .vendor_quirk = panfrost_gpu_amlogic_quirk,
> };
>
> -const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
> -const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1",
> "core2" };
> +static const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
> +static const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1",
> "core2" };
> static const struct panfrost_compatible mediatek_mt8183_data = {
> .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
> .supply_names = mediatek_mt8183_supplies,
>
Re: Regression with mainline kernel on rpi4
On Mon, Sep 20, 2021 at 04:47:31PM +0200, Maxime Ripard wrote: > On Sat, Sep 04, 2021 at 10:40:29AM +0100, Sudip Mukherjee wrote: > > Hi Maxime, > > > > On Sat, Sep 4, 2021 at 10:10 AM Maxime Ripard wrote: > > > > > > On Fri, Sep 03, 2021 at 09:09:50PM +0100, Sudip Mukherjee wrote: > > > > Hi Maxime, > > > > > > > > On Fri, Sep 3, 2021 at 5:03 PM Maxime Ripard wrote: > > > > > > > > > > Hi Sudip, > > > > > > > > > > On Thu, Sep 02, 2021 at 10:08:19AM +0100, Sudip Mukherjee wrote: > > > > > > Hi All, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You can see the complete dmesg at > > > > > > https://openqa.qa.codethink.co.uk/tests/76#step/dmesg/8 > > > > > > > > > > What test were you running? > > > > > > > > Nothing particular, its just a boot test that we do every night after > > > > pulling from torvalds/linux.git > > > > > > What are you booting to then? > > > > I am not sure I understood the question. > > Its an Ubuntu image. The desktop environment is gnome. And as > > mentioned earlier, we use the HEAD of linus tree every night to boot > > the rpi4 and test that we can login via desktop environment and that > > there is no regression in dmesg. > > Looking at the CI, this isn't from a RPi but from qemu? > > What defconfig are you using? How did you generate the Ubuntu image? > Through debootstrap? Any additional package? So qemu (at least on Fedora 34) doesn't seem to have an RPI4 target, nor upstream: https://github.com/qemu/qemu/blob/079b1252e9de384385c9da910262312ec2e574c8/hw/arm/raspi.c#L367 I've tested an Ubuntu 21.04 arm64 build (since it seems like it's what you've been using), built using debootstrap + ubuntu-desktop, both with and without a monitor attached, and up to the desktop once logged in. I don't see any crash. Maxime signature.asc Description: PGP signature
Re: [PATCH v2 2/2] drm/lease: allow empty leases
Anyone up for a review on this one? Daniel Vetter has ack'ed but doesn't have time for a full review. CC Maarten Lankhorst, Maxime Ripard, Michel Dänzer Who else should I CC?
