Re: [Intel-gfx] linux-next: build failure after merge of the drm-misc tree
On Thu, Oct 12, 2023 at 12:27:49PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> On Thu, 12 Oct 2023 12:22:09 +1100 Stephen Rothwell
> wrote:
> >
> > After merging the drm-misc tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > drivers/usb/typec/altmodes/displayport.c: In function 'dp_altmode_vdm':
> > drivers/usb/typec/altmodes/displayport.c:309:33: error: too few arguments
> > to function 'drm_connector_oob_hotplug_event'
> > 309 |
> > drm_connector_oob_hotplug_event(dp->connector_fwnode);
> > | ^~~
> > In file included from drivers/usb/typec/altmodes/displayport.c:17:
> > include/drm/drm_connector.h:1984:6: note: declared here
> > 1984 | void drm_connector_oob_hotplug_event(struct fwnode_handle
> > *connector_fwnode,
> > | ^~~
> >
> > Caused by commit
> >
> > fc93835bb0d7 ("drm: Add HPD state to drm_connector_oob_hotplug_event()")
> >
> > interacting with commit
> >
> > 89434b069e46 ("usb: typec: altmodes/displayport: Signal hpd low when
> > exiting mode")
> >
> > from the usb.current tree.
> >
> > I have applied the following merge fix patch.
> >
> > From: Stephen Rothwell
> > Date: Thu, 12 Oct 2023 12:17:31 +1100
> > Subject: [PATCH] fix up for "drm: Add HPD state to
> > drm_connector_oob_hotplug_event()"
> >
> > interacting with commit
> >
> > 89434b069e46 ("usb: typec: altmodes/displayport: Signal hpd low when
> > exiting mode")
> >
> > Signed-off-by: Stephen Rothwell
> > ---
> > drivers/usb/typec/altmodes/displayport.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/typec/altmodes/displayport.c
> > b/drivers/usb/typec/altmodes/displayport.c
> > index ddfb5b6ace4f..eb0bf08fc97a 100644
> > --- a/drivers/usb/typec/altmodes/displayport.c
> > +++ b/drivers/usb/typec/altmodes/displayport.c
> > @@ -306,7 +306,8 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
> > dp->data.status = 0;
> > dp->data.conf = 0;
> > if (dp->hpd) {
> > -
> > drm_connector_oob_hotplug_event(dp->connector_fwnode);
> > +
> > drm_connector_oob_hotplug_event(dp->connector_fwnode
>
> Pretend that there is a comma at the end of the above line :-)
Looks good to me, thanks!
greg k-h
[Intel-gfx] [PATCH 6.5 308/321] drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top
6.5-stable review patch. If anyone has any objections, please let me know.
--
From: Javier Pello
commit b7599d241778d0b10cdf7a5c755aa7db9b83250c upstream.
There is an assertion in ggtt_reserve_guc_top that the global GTT
is of size at least GUC_GGTT_TOP, which is not the case on a 32-bit
platform; see commit 562d55d991b39ce376c492df2f7890fd6a541ffc
("drm/i915/bdw: Only use 2g GGTT for 32b platforms"). If GEM_BUG_ON
is enabled, this triggers a BUG(); if GEM_BUG_ON is disabled, the
subsequent reservation fails and the driver fails to initialise
the device:
i915 :00:02.0: [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT
for GuC
i915 :00:02.0: Device initialization failed (-28)
i915 :00:02.0: Please file a bug on drm/i915; see
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs for
details.
i915: probe of :00:02.0 failed with error -28
Make the reservation at the top of the available space, whatever
that is, instead of assuming that the top will be GUC_GGTT_TOP.
Fixes: 911800765ef6 ("drm/i915/uc: Reserve upper range of GGTT")
Link: https://gitlab.freedesktop.org/drm/intel/-/issues/9080
Signed-off-by: Javier Pello
Reviewed-by: Daniele Ceraolo Spurio
Cc: Fernando Pacheco
Cc: Chris Wilson
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: sta...@vger.kernel.org # v5.3+
Signed-off-by: John Harrison
Link:
https://patchwork.freedesktop.org/patch/msgid/20230902171039.2229126186d697dbcf62d...@otheo.eu
(cherry picked from commit 0f3fa942d91165c2702577e9274d2ee1c7212afc)
Signed-off-by: Rodrigo Vivi
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -511,20 +511,31 @@ void intel_ggtt_unbind_vma(struct i915_a
vm->clear_range(vm, vma_res->start, vma_res->vma_size);
}
+/*
+ * Reserve the top of the GuC address space for firmware images. Addresses
+ * beyond GUC_GGTT_TOP in the GuC address space are inaccessible by GuC,
+ * which makes for a suitable range to hold GuC/HuC firmware images if the
+ * size of the GGTT is 4G. However, on a 32-bit platform the size of the GGTT
+ * is limited to 2G, which is less than GUC_GGTT_TOP, but we reserve a chunk
+ * of the same size anyway, which is far more than needed, to keep the logic
+ * in uc_fw_ggtt_offset() simple.
+ */
+#define GUC_TOP_RESERVE_SIZE (SZ_4G - GUC_GGTT_TOP)
+
static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
{
- u64 size;
+ u64 offset;
int ret;
if (!intel_uc_uses_guc(>vm.gt->uc))
return 0;
- GEM_BUG_ON(ggtt->vm.total <= GUC_GGTT_TOP);
- size = ggtt->vm.total - GUC_GGTT_TOP;
+ GEM_BUG_ON(ggtt->vm.total <= GUC_TOP_RESERVE_SIZE);
+ offset = ggtt->vm.total - GUC_TOP_RESERVE_SIZE;
- ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw, size,
- GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
- PIN_NOEVICT);
+ ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw,
+ GUC_TOP_RESERVE_SIZE, offset,
+ I915_COLOR_UNEVICTABLE, PIN_NOEVICT);
if (ret)
drm_dbg(>vm.i915->drm,
"Failed to reserve top of GGTT for GuC\n");
[Intel-gfx] [PATCH 6.1 251/259] drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top
6.1-stable review patch. If anyone has any objections, please let me know.
--
From: Javier Pello
commit b7599d241778d0b10cdf7a5c755aa7db9b83250c upstream.
There is an assertion in ggtt_reserve_guc_top that the global GTT
is of size at least GUC_GGTT_TOP, which is not the case on a 32-bit
platform; see commit 562d55d991b39ce376c492df2f7890fd6a541ffc
("drm/i915/bdw: Only use 2g GGTT for 32b platforms"). If GEM_BUG_ON
is enabled, this triggers a BUG(); if GEM_BUG_ON is disabled, the
subsequent reservation fails and the driver fails to initialise
the device:
i915 :00:02.0: [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT
for GuC
i915 :00:02.0: Device initialization failed (-28)
i915 :00:02.0: Please file a bug on drm/i915; see
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs for
details.
i915: probe of :00:02.0 failed with error -28
Make the reservation at the top of the available space, whatever
that is, instead of assuming that the top will be GUC_GGTT_TOP.
Fixes: 911800765ef6 ("drm/i915/uc: Reserve upper range of GGTT")
Link: https://gitlab.freedesktop.org/drm/intel/-/issues/9080
Signed-off-by: Javier Pello
Reviewed-by: Daniele Ceraolo Spurio
Cc: Fernando Pacheco
Cc: Chris Wilson
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: sta...@vger.kernel.org # v5.3+
Signed-off-by: John Harrison
Link:
https://patchwork.freedesktop.org/patch/msgid/20230902171039.2229126186d697dbcf62d...@otheo.eu
(cherry picked from commit 0f3fa942d91165c2702577e9274d2ee1c7212afc)
Signed-off-by: Rodrigo Vivi
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -500,20 +500,31 @@ void intel_ggtt_unbind_vma(struct i915_a
vm->clear_range(vm, vma_res->start, vma_res->vma_size);
}
+/*
+ * Reserve the top of the GuC address space for firmware images. Addresses
+ * beyond GUC_GGTT_TOP in the GuC address space are inaccessible by GuC,
+ * which makes for a suitable range to hold GuC/HuC firmware images if the
+ * size of the GGTT is 4G. However, on a 32-bit platform the size of the GGTT
+ * is limited to 2G, which is less than GUC_GGTT_TOP, but we reserve a chunk
+ * of the same size anyway, which is far more than needed, to keep the logic
+ * in uc_fw_ggtt_offset() simple.
+ */
+#define GUC_TOP_RESERVE_SIZE (SZ_4G - GUC_GGTT_TOP)
+
static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
{
- u64 size;
+ u64 offset;
int ret;
if (!intel_uc_uses_guc(>vm.gt->uc))
return 0;
- GEM_BUG_ON(ggtt->vm.total <= GUC_GGTT_TOP);
- size = ggtt->vm.total - GUC_GGTT_TOP;
+ GEM_BUG_ON(ggtt->vm.total <= GUC_TOP_RESERVE_SIZE);
+ offset = ggtt->vm.total - GUC_TOP_RESERVE_SIZE;
- ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw, size,
- GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
- PIN_NOEVICT);
+ ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw,
+ GUC_TOP_RESERVE_SIZE, offset,
+ I915_COLOR_UNEVICTABLE, PIN_NOEVICT);
if (ret)
drm_dbg(>vm.i915->drm,
"Failed to reserve top of GGTT for GuC\n");
Re: [Intel-gfx] [PATCH 04/15] tty: Remove now superfluous sentinel element from ctl_table array
On Mon, Oct 02, 2023 at 08:47:53AM +, Christophe Leroy wrote:
>
>
> Le 02/10/2023 à 10:17, Jiri Slaby a écrit :
> > On 28. 09. 23, 15:21, Joel Granados via B4 Relay wrote:
> >> From: Joel Granados
> >>
> >> This commit comes at the tail end of a greater effort to remove the
> >> empty elements at the end of the ctl_table arrays (sentinels) which
> >> will reduce the overall build time size of the kernel and run time
> >> memory bloat by ~64 bytes per sentinel (further information Link :
> >> https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
> >>
> >> Remove sentinel from tty_table
> >>
> >> Signed-off-by: Joel Granados
> >> ---
> >> drivers/tty/tty_io.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> >> index 8a94e5a43c6d..2f925dc54a20 100644
> >> --- a/drivers/tty/tty_io.c
> >> +++ b/drivers/tty/tty_io.c
> >> @@ -3607,8 +3607,7 @@ static struct ctl_table tty_table[] = {
> >> .proc_handler = proc_dointvec,
> >> .extra1 = SYSCTL_ZERO,
> >> .extra2 = SYSCTL_ONE,
> >> - },
> >> - { }
> >> + }
> >
> > Why to remove the comma? One would need to add one when adding a new entry?
>
> Does it make any difference at all ?
>
> In one case you have:
>
> @
> something old,
> },
> + {
> + something new,
> + },
> }
>
> In the other case you have:
>
> @
> something old,
> + },
> + {
> + something new,
> }
> }
Because that way it is obvious you are only touching the "something new"
lines and never have to touch the "something old" ones.
It's just a long-standing tradition in Linux, don't have an extra
character if you don't need it :)
thanks,
greg k-h
Re: [Intel-gfx] [PATCH 01/15] cdrom: Remove now superfluous sentinel element from ctl_table array
On Thu, Sep 28, 2023 at 03:21:26PM +0200, Joel Granados via B4 Relay wrote:
> From: Joel Granados
>
> This commit comes at the tail end of a greater effort to remove the
> empty elements at the end of the ctl_table arrays (sentinels) which
> will reduce the overall build time size of the kernel and run time
> memory bloat by ~64 bytes per sentinel (further information Link :
> https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
>
> Remove sentinel element from cdrom_table
>
> Signed-off-by: Joel Granados
> ---
> drivers/cdrom/cdrom.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index cc2839805983..451907ade389 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -3654,8 +3654,7 @@ static struct ctl_table cdrom_table[] = {
> .maxlen = sizeof(int),
> .mode = 0644,
> .proc_handler = cdrom_sysctl_handler
> - },
> - { }
> + }
You should have the final entry as "}," so as to make any future
additions to the list to only contain that entry, that's long been the
kernel style for lists like this.
So your patches will just remove one line, not 2 and add 1, making it a
smaller diff.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Move abs_diff() to math.h
On Mon, Jul 24, 2023 at 11:25:11AM +0300, Andy Shevchenko wrote: > abs_diff() belongs to math.h. Move it there. > This will allow others to use it. > > Signed-off-by: Andy Shevchenko > Reviewed-by: Jiri Slaby # tty/serial Acked-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH 0/1] v6.1 stable backport request
On Mon, Jul 10, 2023 at 05:13:58PM +0300, Imre Deak wrote: > Stable team, please apply patch 1/1 in this patchset along with its > dependencies to the v6.1 stable tree. The patch required a trivial > rebase adding a header include, hence resending it, while its 2 > dependencies listed at Cc: stable lines in the commit message can be > cherry-picked as-is. > > Thanks, > Imre > > Imre Deak (1): > drm/i915/tc: Fix system resume MST mode restore for DP-alt sinks > > .../drm/i915/display/intel_display_types.h| 1 + > drivers/gpu/drm/i915/display/intel_tc.c | 51 +-- > 2 files changed, 48 insertions(+), 4 deletions(-) All now queued up, thanks. greg k-h
[Intel-gfx] [PATCH 5.10 090/173] drm/i915/active: Fix missing debug object activation
From: Nirmoy Das
commit e92eb246feb9019b0b137706c934b8891cdfe3c2 upstream.
debug_active_activate() expected ref->count to be zero
which is not true anymore as __i915_active_activate() calls
debug_active_activate() after incrementing the count.
v2: No need to check for "ref->count == 1" as __i915_active_activate()
already make sure of that(Janusz).
Fixes: 04240e30ed06 ("drm/i915: Skip taking acquire mutex for no ref->active
callback")
Cc: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Thomas Hellström
Cc: Andi Shyti
Cc: intel-gfx@lists.freedesktop.org
Cc: Janusz Krzysztofik
Cc: # v5.10+
Signed-off-by: Nirmoy Das
Reviewed-by: Janusz Krzysztofik
Reviewed-by: Andrzej Hajda
Link:
https://patchwork.freedesktop.org/patch/msgid/20230313114613.9874-1-nirmoy@intel.com
(cherry picked from commit bfad380c542438a9b642f8190b7fd37bc77e2723)
Signed-off-by: Jani Nikula
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/i915_active.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -96,8 +96,7 @@ static void debug_active_init(struct i91
static void debug_active_activate(struct i915_active *ref)
{
lockdep_assert_held(>tree_lock);
- if (!atomic_read(>count)) /* before the first inc */
- debug_object_activate(ref, _debug_desc);
+ debug_object_activate(ref, _debug_desc);
}
static void debug_active_deactivate(struct i915_active *ref)
[Intel-gfx] [PATCH 5.15 132/146] drm/i915/active: Fix missing debug object activation
From: Nirmoy Das
commit e92eb246feb9019b0b137706c934b8891cdfe3c2 upstream.
debug_active_activate() expected ref->count to be zero
which is not true anymore as __i915_active_activate() calls
debug_active_activate() after incrementing the count.
v2: No need to check for "ref->count == 1" as __i915_active_activate()
already make sure of that(Janusz).
Fixes: 04240e30ed06 ("drm/i915: Skip taking acquire mutex for no ref->active
callback")
Cc: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Thomas Hellström
Cc: Andi Shyti
Cc: intel-gfx@lists.freedesktop.org
Cc: Janusz Krzysztofik
Cc: # v5.10+
Signed-off-by: Nirmoy Das
Reviewed-by: Janusz Krzysztofik
Reviewed-by: Andrzej Hajda
Link:
https://patchwork.freedesktop.org/patch/msgid/20230313114613.9874-1-nirmoy@intel.com
(cherry picked from commit bfad380c542438a9b642f8190b7fd37bc77e2723)
Signed-off-by: Jani Nikula
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/i915_active.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -92,8 +92,7 @@ static void debug_active_init(struct i91
static void debug_active_activate(struct i915_active *ref)
{
lockdep_assert_held(>tree_lock);
- if (!atomic_read(>count)) /* before the first inc */
- debug_object_activate(ref, _debug_desc);
+ debug_object_activate(ref, _debug_desc);
}
static void debug_active_deactivate(struct i915_active *ref)
[Intel-gfx] [PATCH 6.1 202/224] drm/i915/active: Fix missing debug object activation
From: Nirmoy Das
commit e92eb246feb9019b0b137706c934b8891cdfe3c2 upstream.
debug_active_activate() expected ref->count to be zero
which is not true anymore as __i915_active_activate() calls
debug_active_activate() after incrementing the count.
v2: No need to check for "ref->count == 1" as __i915_active_activate()
already make sure of that(Janusz).
Fixes: 04240e30ed06 ("drm/i915: Skip taking acquire mutex for no ref->active
callback")
Cc: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Thomas Hellström
Cc: Andi Shyti
Cc: intel-gfx@lists.freedesktop.org
Cc: Janusz Krzysztofik
Cc: # v5.10+
Signed-off-by: Nirmoy Das
Reviewed-by: Janusz Krzysztofik
Reviewed-by: Andrzej Hajda
Link:
https://patchwork.freedesktop.org/patch/msgid/20230313114613.9874-1-nirmoy@intel.com
(cherry picked from commit bfad380c542438a9b642f8190b7fd37bc77e2723)
Signed-off-by: Jani Nikula
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/i915_active.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -92,8 +92,7 @@ static void debug_active_init(struct i91
static void debug_active_activate(struct i915_active *ref)
{
lockdep_assert_held(>tree_lock);
- if (!atomic_read(>count)) /* before the first inc */
- debug_object_activate(ref, _debug_desc);
+ debug_object_activate(ref, _debug_desc);
}
static void debug_active_deactivate(struct i915_active *ref)
[Intel-gfx] [PATCH 6.2 221/240] drm/i915/active: Fix missing debug object activation
From: Nirmoy Das
commit e92eb246feb9019b0b137706c934b8891cdfe3c2 upstream.
debug_active_activate() expected ref->count to be zero
which is not true anymore as __i915_active_activate() calls
debug_active_activate() after incrementing the count.
v2: No need to check for "ref->count == 1" as __i915_active_activate()
already make sure of that(Janusz).
Fixes: 04240e30ed06 ("drm/i915: Skip taking acquire mutex for no ref->active
callback")
Cc: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Thomas Hellström
Cc: Andi Shyti
Cc: intel-gfx@lists.freedesktop.org
Cc: Janusz Krzysztofik
Cc: # v5.10+
Signed-off-by: Nirmoy Das
Reviewed-by: Janusz Krzysztofik
Reviewed-by: Andrzej Hajda
Link:
https://patchwork.freedesktop.org/patch/msgid/20230313114613.9874-1-nirmoy@intel.com
(cherry picked from commit bfad380c542438a9b642f8190b7fd37bc77e2723)
Signed-off-by: Jani Nikula
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/i915_active.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -92,8 +92,7 @@ static void debug_active_init(struct i91
static void debug_active_activate(struct i915_active *ref)
{
lockdep_assert_held(>tree_lock);
- if (!atomic_read(>count)) /* before the first inc */
- debug_object_activate(ref, _debug_desc);
+ debug_object_activate(ref, _debug_desc);
}
static void debug_active_deactivate(struct i915_active *ref)
Re: [Intel-gfx] [Regression] drm/i915: Don't use BAR mappings for ring buffers with LLC alone creates issues in stable kernels
On Tue, Mar 21, 2023 at 07:58:44AM +0700, Philip Müller wrote: > On 20.03.23 20:28, Greg Kroah-Hartman wrote: > > On Sun, Mar 19, 2023 at 10:01:01AM +0700, Philip Müller wrote: > > > Have to correct the affected kernels to these: 4.14.310, 4.19.278, > > > 5.4.237, > > > 5.10.175 > > > > Please don't top-post :( > > > > Anyway, should be fixed in the next round of releases in a few days, if > > not, please let us know. > > > > greg k-h > > Hi Greg, > > seems the RCs work out. 4.19.279-rc1 and 5.10.176-rc1 were tested by a user > who had the issue on i915. In 5.15 series the drm patch got reverted. I only > see "drm/i915: Don't use stolen memory for ring buffers with LLC" there as > "drm/i915: Don't use BAR mappings for ring buffers with LLC" was reverted > with 5.15.101. So is 5.15.y ok or not? Sorry, I could not parse your response here. thanks, greg k-h
[Intel-gfx] [PATCH 5.15 109/115] drm/i915: Dont use stolen memory for ring buffers with LLC
From: John Harrison
commit 690e0ec8e63da9a29b39fedc6ed5da09c7c82651 upstream.
Direction from hardware is that stolen memory should never be used for
ring buffer allocations on platforms with LLC. There are too many
caching pitfalls due to the way stolen memory accesses are routed. So
it is safest to just not use it.
Signed-off-by: John Harrison
Fixes: c58b735fc762 ("drm/i915: Allocate rings from stolen")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-2-john.c.harri...@intel.com
(cherry picked from commit f54c1f6c697c4297f7ed94283c184acc338a5cf8)
Signed-off-by: Jani Nikula
Signed-off-by: John Harrison
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ring.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -113,7 +113,7 @@ static struct i915_vma *create_ring_vma(
struct i915_vma *vma;
obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_VOLATILE);
- if (IS_ERR(obj) && i915_ggtt_has_aperture(ggtt))
+ if (IS_ERR(obj) && i915_ggtt_has_aperture(ggtt) && !HAS_LLC(i915))
obj = i915_gem_object_create_stolen(i915, size);
if (IS_ERR(obj))
obj = i915_gem_object_create_internal(i915, size);
[Intel-gfx] [PATCH 5.10 77/99] drm/i915: Dont use stolen memory for ring buffers with LLC
From: John Harrison
commit 690e0ec8e63da9a29b39fedc6ed5da09c7c82651 upstream.
Direction from hardware is that stolen memory should never be used for
ring buffer allocations on platforms with LLC. There are too many
caching pitfalls due to the way stolen memory accesses are routed. So
it is safest to just not use it.
Signed-off-by: John Harrison
Fixes: c58b735fc762 ("drm/i915: Allocate rings from stolen")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-2-john.c.harri...@intel.com
(cherry picked from commit f54c1f6c697c4297f7ed94283c184acc338a5cf8)
Signed-off-by: Jani Nikula
Signed-off-by: John Harrison
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ring.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -108,7 +108,7 @@ static struct i915_vma *create_ring_vma(
struct i915_vma *vma;
obj = ERR_PTR(-ENODEV);
- if (i915_ggtt_has_aperture(ggtt))
+ if (i915_ggtt_has_aperture(ggtt) && !HAS_LLC(i915))
obj = i915_gem_object_create_stolen(i915, size);
if (IS_ERR(obj))
obj = i915_gem_object_create_internal(i915, size);
[Intel-gfx] [PATCH 5.4 55/60] drm/i915: Dont use stolen memory for ring buffers with LLC
From: John Harrison
commit 690e0ec8e63da9a29b39fedc6ed5da09c7c82651 upstream.
Direction from hardware is that stolen memory should never be used for
ring buffer allocations on platforms with LLC. There are too many
caching pitfalls due to the way stolen memory accesses are routed. So
it is safest to just not use it.
Signed-off-by: John Harrison
Fixes: c58b735fc762 ("drm/i915: Allocate rings from stolen")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-2-john.c.harri...@intel.com
(cherry picked from commit f54c1f6c697c4297f7ed94283c184acc338a5cf8)
Signed-off-by: Jani Nikula
Signed-off-by: John Harrison
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ringbuffer.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -1268,10 +1268,11 @@ static struct i915_vma *create_ring_vma(
{
struct i915_address_space *vm = >vm;
struct drm_i915_private *i915 = vm->i915;
- struct drm_i915_gem_object *obj;
+ struct drm_i915_gem_object *obj = NULL;
struct i915_vma *vma;
- obj = i915_gem_object_create_stolen(i915, size);
+ if (!HAS_LLC(i915))
+ obj = i915_gem_object_create_stolen(i915, size);
if (!obj)
obj = i915_gem_object_create_internal(i915, size);
if (IS_ERR(obj))
[Intel-gfx] [PATCH 4.19 33/36] drm/i915: Dont use stolen memory for ring buffers with LLC
From: John Harrison
commit 690e0ec8e63da9a29b39fedc6ed5da09c7c82651 upstream.
Direction from hardware is that stolen memory should never be used for
ring buffer allocations on platforms with LLC. There are too many
caching pitfalls due to the way stolen memory accesses are routed. So
it is safest to just not use it.
Signed-off-by: John Harrison
Fixes: c58b735fc762 ("drm/i915: Allocate rings from stolen")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-2-john.c.harri...@intel.com
(cherry picked from commit f54c1f6c697c4297f7ed94283c184acc338a5cf8)
Signed-off-by: Jani Nikula
Signed-off-by: John Harrison
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/intel_ringbuffer.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1132,10 +1132,11 @@ static struct i915_vma *
intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
{
struct i915_address_space *vm = _priv->ggtt.vm;
- struct drm_i915_gem_object *obj;
+ struct drm_i915_gem_object *obj = NULL;
struct i915_vma *vma;
- obj = i915_gem_object_create_stolen(dev_priv, size);
+ if (!HAS_LLC(dev_priv))
+ obj = i915_gem_object_create_stolen(dev_priv, size);
if (!obj)
obj = i915_gem_object_create_internal(dev_priv, size);
if (IS_ERR(obj))
[Intel-gfx] [PATCH 4.14 27/30] drm/i915: Dont use stolen memory for ring buffers with LLC
From: John Harrison
commit 690e0ec8e63da9a29b39fedc6ed5da09c7c82651 upstream.
Direction from hardware is that stolen memory should never be used for
ring buffer allocations on platforms with LLC. There are too many
caching pitfalls due to the way stolen memory accesses are routed. So
it is safest to just not use it.
Signed-off-by: John Harrison
Fixes: c58b735fc762 ("drm/i915: Allocate rings from stolen")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-2-john.c.harri...@intel.com
(cherry picked from commit f54c1f6c697c4297f7ed94283c184acc338a5cf8)
Signed-off-by: Jani Nikula
Signed-off-by: John Harrison
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/intel_ringbuffer.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1359,10 +1359,11 @@ static struct i915_vma *
intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
{
struct i915_address_space *vm = _priv->ggtt.base;
- struct drm_i915_gem_object *obj;
+ struct drm_i915_gem_object *obj = NULL;
struct i915_vma *vma;
- obj = i915_gem_object_create_stolen(dev_priv, size);
+ if (!HAS_LLC(dev_priv))
+ obj = i915_gem_object_create_stolen(dev_priv, size);
if (!obj)
obj = i915_gem_object_create_internal(dev_priv, size);
if (IS_ERR(obj))
Re: [Intel-gfx] [Regression] drm/i915: Don't use BAR mappings for ring buffers with LLC alone creates issues in stable kernels
On Sun, Mar 19, 2023 at 10:01:01AM +0700, Philip Müller wrote: > Have to correct the affected kernels to these: 4.14.310, 4.19.278, 5.4.237, > 5.10.175 Please don't top-post :( Anyway, should be fixed in the next round of releases in a few days, if not, please let us know. greg k-h
[Intel-gfx] [PATCH 5.10 080/104] drm/i915: Dont use BAR mappings for ring buffers with LLC
From: John Harrison
commit 85636167e3206c3fbd52254fc432991cc4e90194 upstream.
Direction from hardware is that ring buffers should never be mapped
via the BAR on systems with LLC. There are too many caching pitfalls
due to the way BAR accesses are routed. So it is safest to just not
use it.
Signed-off-by: John Harrison
Fixes: 9d80841ea4c9 ("drm/i915: Allow ringbuffers to be bound anywhere")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-3-john.c.harri...@intel.com
(cherry picked from commit 65c08339db1ada87afd6cfe7db8e60bb4851d919)
Signed-off-by: Jani Nikula
Signed-off-by: John Harrison
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ring.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -49,7 +49,7 @@ int intel_ring_pin(struct intel_ring *ri
if (unlikely(ret))
goto err_unpin;
- if (i915_vma_is_map_and_fenceable(vma))
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
addr = (void __force *)i915_vma_pin_iomap(vma);
else
addr = i915_gem_object_pin_map(vma->obj,
@@ -91,7 +91,7 @@ void intel_ring_unpin(struct intel_ring
return;
i915_vma_unset_ggtt_write(vma);
- if (i915_vma_is_map_and_fenceable(vma))
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
i915_vma_unpin_iomap(vma);
else
i915_gem_object_unpin_map(vma->obj);
[Intel-gfx] [PATCH 5.4 60/68] drm/i915: Dont use BAR mappings for ring buffers with LLC
From: John Harrison
commit 85636167e3206c3fbd52254fc432991cc4e90194 upstream.
Direction from hardware is that ring buffers should never be mapped
via the BAR on systems with LLC. There are too many caching pitfalls
due to the way BAR accesses are routed. So it is safest to just not
use it.
Signed-off-by: John Harrison
Fixes: 9d80841ea4c9 ("drm/i915: Allow ringbuffers to be bound anywhere")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-3-john.c.harri...@intel.com
(cherry picked from commit 65c08339db1ada87afd6cfe7db8e60bb4851d919)
Signed-off-by: Jani Nikula
Signed-off-by: John Harrison
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ringbuffer.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -1208,7 +1208,7 @@ int intel_ring_pin(struct intel_ring *ri
if (unlikely(ret))
goto err_unpin;
- if (i915_vma_is_map_and_fenceable(vma))
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
addr = (void __force *)i915_vma_pin_iomap(vma);
else
addr = i915_gem_object_pin_map(vma->obj,
@@ -1252,7 +1252,7 @@ void intel_ring_unpin(struct intel_ring
intel_ring_reset(ring, ring->emit);
i915_vma_unset_ggtt_write(vma);
- if (i915_vma_is_map_and_fenceable(vma))
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
i915_vma_unpin_iomap(vma);
else
i915_gem_object_unpin_map(vma->obj);
[Intel-gfx] [PATCH 4.19 36/39] drm/i915: Dont use BAR mappings for ring buffers with LLC
From: John Harrison
commit 85636167e3206c3fbd52254fc432991cc4e90194 upstream.
Direction from hardware is that ring buffers should never be mapped
via the BAR on systems with LLC. There are too many caching pitfalls
due to the way BAR accesses are routed. So it is safest to just not
use it.
Signed-off-by: John Harrison
Fixes: 9d80841ea4c9 ("drm/i915: Allow ringbuffers to be bound anywhere")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-3-john.c.harri...@intel.com
(cherry picked from commit 65c08339db1ada87afd6cfe7db8e60bb4851d919)
Signed-off-by: Jani Nikula
Signed-off-by: John Harrison
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/intel_ringbuffer.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1083,7 +1083,7 @@ int intel_ring_pin(struct intel_ring *ri
if (unlikely(ret))
return ret;
- if (i915_vma_is_map_and_fenceable(vma))
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
addr = (void __force *)i915_vma_pin_iomap(vma);
else
addr = i915_gem_object_pin_map(vma->obj, map);
@@ -1118,7 +1118,7 @@ void intel_ring_unpin(struct intel_ring
/* Discard any unused bytes beyond that submitted to hw. */
intel_ring_reset(ring, ring->tail);
- if (i915_vma_is_map_and_fenceable(ring->vma))
+ if (i915_vma_is_map_and_fenceable(ring->vma) &&
!HAS_LLC(ring->vma->vm->i915))
i915_vma_unpin_iomap(ring->vma);
else
i915_gem_object_unpin_map(ring->vma->obj);
[Intel-gfx] [PATCH 4.14 20/21] drm/i915: Dont use BAR mappings for ring buffers with LLC
From: John Harrison
commit 85636167e3206c3fbd52254fc432991cc4e90194 upstream.
Direction from hardware is that ring buffers should never be mapped
via the BAR on systems with LLC. There are too many caching pitfalls
due to the way BAR accesses are routed. So it is safest to just not
use it.
Signed-off-by: John Harrison
Fixes: 9d80841ea4c9 ("drm/i915: Allow ringbuffers to be bound anywhere")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-3-john.c.harri...@intel.com
(cherry picked from commit 65c08339db1ada87afd6cfe7db8e60bb4851d919)
Signed-off-by: Jani Nikula
Signed-off-by: John Harrison
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/intel_ringbuffer.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1314,7 +1314,7 @@ int intel_ring_pin(struct intel_ring *ri
if (unlikely(ret))
return ret;
- if (i915_vma_is_map_and_fenceable(vma))
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
addr = (void __force *)i915_vma_pin_iomap(vma);
else
addr = i915_gem_object_pin_map(vma->obj, map);
@@ -1346,7 +1346,7 @@ void intel_ring_unpin(struct intel_ring
/* Discard any unused bytes beyond that submitted to hw. */
intel_ring_reset(ring, ring->tail);
- if (i915_vma_is_map_and_fenceable(ring->vma))
+ if (i915_vma_is_map_and_fenceable(ring->vma) &&
!HAS_LLC(ring->vma->vm->i915))
i915_vma_unpin_iomap(ring->vma);
else
i915_gem_object_unpin_map(ring->vma->obj);
[Intel-gfx] [PATCH 30/36] drm/i915/huc: use const struct bus_type pointers
The struct bus_type pointers in the functions
intel_huc_register_gsc_notifier() and
intel_huc_unregister_gsc_notifier() should be a const pointer, as the
structure is not modified anywhere in the functions, and the pointer
they are passed will be a const * in the near future.
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: David Airlie
Cc: Daniel Vetter
Cc: Daniele Ceraolo Spurio
Cc: Alan Previn
Cc: John Harrison
Cc: Greg Kroah-Hartman
Cc: Tony Ye
Cc: Vitaly Lubart
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Greg Kroah-Hartman
---
Note, this is a patch that is a prepatory cleanup as part of a larger
series of patches that is working on resolving some old driver core
design mistakes. It will build and apply cleanly on top of 6.3-rc2 on
its own, but I'd prefer if I could take it through my driver-core tree
so that the driver core changes can be taken through there for 6.4-rc1.
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 4 ++--
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 410905da8e97..8b453bd7c953 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -183,7 +183,7 @@ static int gsc_notifier(struct notifier_block *nb, unsigned
long action, void *d
return 0;
}
-void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus)
+void intel_huc_register_gsc_notifier(struct intel_huc *huc, const struct
bus_type *bus)
{
int ret;
@@ -200,7 +200,7 @@ void intel_huc_register_gsc_notifier(struct intel_huc *huc,
struct bus_type *bus
}
}
-void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus)
+void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, const struct
bus_type *bus)
{
if (!huc->delayed_load.nb.notifier_call)
return;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 52db03620c60..05d4832f8461 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -51,8 +51,8 @@ int intel_huc_check_status(struct intel_huc *huc);
void intel_huc_update_auth_status(struct intel_huc *huc);
bool intel_huc_is_authenticated(struct intel_huc *huc);
-void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus);
-void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus);
+void intel_huc_register_gsc_notifier(struct intel_huc *huc, const struct
bus_type *bus);
+void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, const struct
bus_type *bus);
static inline int intel_huc_sanitize(struct intel_huc *huc)
{
--
2.39.2
Re: [Intel-gfx] Stable Patch Don't use BAR mappings for ring buffers with LLC might create some issues with 5.15
On Sun, Mar 12, 2023 at 12:13:03PM +0100, Greg Kroah-Hartman wrote: > On Sun, Mar 12, 2023 at 04:41:32PM +0700, Philip Müller wrote: > > > > > > || > > > > Hi all, > > > > seems there is a report open on the "drm/i915: Don't use BAR mappings for > > ring buffers with LLC" patch, which was included into 5.15.99 lately. > > > > I saw this patch also on 6.1 and 6.2. Older LTS kernels I didn't found them > > yet, even it is tagged to be included from v4.9+ on. However I saw also the > > patch "drm/i915: Don't use stolen memory for ring buffers with LLC" applied > > when applied to other kernel series. > > > > Reverting the patch according to this fixes > > it:https://gitlab.freedesktop.org/drm/intel/-/issues/8284 > > > > Maybe double-check what is actually needed if this creates issues on some > > Intel i915 hardware. Thx. > > Also another report of this here: > https://lore.kernel.org/r/nqjqg8n--...@tuta.io > > I'll go revert this for now and do a new release. Should be fixed in 5.15.101. If not, please let us know. thanks, greg k-h
Re: [Intel-gfx] Stable Patch Don't use BAR mappings for ring buffers with LLC might create some issues with 5.15
On Sun, Mar 12, 2023 at 04:41:32PM +0700, Philip Müller wrote: > > > || > > Hi all, > > seems there is a report open on the "drm/i915: Don't use BAR mappings for > ring buffers with LLC" patch, which was included into 5.15.99 lately. > > I saw this patch also on 6.1 and 6.2. Older LTS kernels I didn't found them > yet, even it is tagged to be included from v4.9+ on. However I saw also the > patch "drm/i915: Don't use stolen memory for ring buffers with LLC" applied > when applied to other kernel series. > > Reverting the patch according to this fixes > it:https://gitlab.freedesktop.org/drm/intel/-/issues/8284 > > Maybe double-check what is actually needed if this creates issues on some > Intel i915 hardware. Thx. Also another report of this here: https://lore.kernel.org/r/nqjqg8n--...@tuta.io I'll go revert this for now and do a new release. thanks, greg k-h
[Intel-gfx] [PATCH 6.1 068/200] drm/i915: move a Kconfig symbol to unbreak the menu presentation
From: Randy Dunlap
[ Upstream commit 0b93efca3659f6d55ed31cff6722dca5f6e4d6e2 ]
Inserting a Kconfig symbol that does not have a dependency (DRM_I915_GVT)
into a list of other symbols that do have a dependency (on DRM_I915)
breaks the driver menu presentation in 'make *config'.
Relocate the DRM_I915_GVT symbol so that it does not cause this
problem.
Fixes: 8b750bf74418 ("drm/i915/gvt: move the gvt code into kvmgt.ko")
Signed-off-by: Randy Dunlap
Cc: Christoph Hellwig
Cc: Zhi Wang
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: Zhenyu Wang
Cc: intel-gfx@lists.freedesktop.org
Cc: intel-gvt-...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Reviewed-by: Christoph Hellwig
Acked-by: Zhenyu Wang
Signed-off-by: Zhenyu Wang
Link:
http://patchwork.freedesktop.org/patch/msgid/20230215044533.4847-1-rdun...@infradead.org
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/i915/Kconfig | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 3efce05d7b57c..3a6e176d77aa5 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -107,9 +107,6 @@ config DRM_I915_USERPTR
If in doubt, say "Y".
-config DRM_I915_GVT
- bool
-
config DRM_I915_GVT_KVMGT
tristate "Enable KVM host support Intel GVT-g graphics virtualization"
depends on DRM_I915
@@ -160,3 +157,6 @@ menu "drm/i915 Unstable Evolution"
depends on DRM_I915
source "drivers/gpu/drm/i915/Kconfig.unstable"
endmenu
+
+config DRM_I915_GVT
+ bool
--
2.39.2
[Intel-gfx] [PATCH 6.2 075/211] drm/i915: move a Kconfig symbol to unbreak the menu presentation
From: Randy Dunlap
[ Upstream commit 0b93efca3659f6d55ed31cff6722dca5f6e4d6e2 ]
Inserting a Kconfig symbol that does not have a dependency (DRM_I915_GVT)
into a list of other symbols that do have a dependency (on DRM_I915)
breaks the driver menu presentation in 'make *config'.
Relocate the DRM_I915_GVT symbol so that it does not cause this
problem.
Fixes: 8b750bf74418 ("drm/i915/gvt: move the gvt code into kvmgt.ko")
Signed-off-by: Randy Dunlap
Cc: Christoph Hellwig
Cc: Zhi Wang
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: Zhenyu Wang
Cc: intel-gfx@lists.freedesktop.org
Cc: intel-gvt-...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Reviewed-by: Christoph Hellwig
Acked-by: Zhenyu Wang
Signed-off-by: Zhenyu Wang
Link:
http://patchwork.freedesktop.org/patch/msgid/20230215044533.4847-1-rdun...@infradead.org
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/i915/Kconfig | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 3efce05d7b57c..3a6e176d77aa5 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -107,9 +107,6 @@ config DRM_I915_USERPTR
If in doubt, say "Y".
-config DRM_I915_GVT
- bool
-
config DRM_I915_GVT_KVMGT
tristate "Enable KVM host support Intel GVT-g graphics virtualization"
depends on DRM_I915
@@ -160,3 +157,6 @@ menu "drm/i915 Unstable Evolution"
depends on DRM_I915
source "drivers/gpu/drm/i915/Kconfig.unstable"
endmenu
+
+config DRM_I915_GVT
+ bool
--
2.39.2
[Intel-gfx] [PATCH 5.15 566/567] drm/i915: Dont use BAR mappings for ring buffers with LLC
From: John Harrison
commit 85636167e3206c3fbd52254fc432991cc4e90194 upstream.
Direction from hardware is that ring buffers should never be mapped
via the BAR on systems with LLC. There are too many caching pitfalls
due to the way BAR accesses are routed. So it is safest to just not
use it.
Signed-off-by: John Harrison
Fixes: 9d80841ea4c9 ("drm/i915: Allow ringbuffers to be bound anywhere")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-3-john.c.harri...@intel.com
(cherry picked from commit 65c08339db1ada87afd6cfe7db8e60bb4851d919)
Signed-off-by: Jani Nikula
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ring.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -51,7 +51,7 @@ int intel_ring_pin(struct intel_ring *ri
if (unlikely(ret))
goto err_unpin;
- if (i915_vma_is_map_and_fenceable(vma)) {
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915)) {
addr = (void __force *)i915_vma_pin_iomap(vma);
} else {
int type = i915_coherent_map_type(vma->vm->i915, vma->obj,
false);
@@ -96,7 +96,7 @@ void intel_ring_unpin(struct intel_ring
return;
i915_vma_unset_ggtt_write(vma);
- if (i915_vma_is_map_and_fenceable(vma))
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
i915_vma_unpin_iomap(vma);
else
i915_gem_object_unpin_map(vma->obj);
[Intel-gfx] [PATCH 6.1 882/885] drm/i915: Dont use BAR mappings for ring buffers with LLC
From: John Harrison
commit 85636167e3206c3fbd52254fc432991cc4e90194 upstream.
Direction from hardware is that ring buffers should never be mapped
via the BAR on systems with LLC. There are too many caching pitfalls
due to the way BAR accesses are routed. So it is safest to just not
use it.
Signed-off-by: John Harrison
Fixes: 9d80841ea4c9 ("drm/i915: Allow ringbuffers to be bound anywhere")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-3-john.c.harri...@intel.com
(cherry picked from commit 65c08339db1ada87afd6cfe7db8e60bb4851d919)
Signed-off-by: Jani Nikula
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ring.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -53,7 +53,7 @@ int intel_ring_pin(struct intel_ring *ri
if (unlikely(ret))
goto err_unpin;
- if (i915_vma_is_map_and_fenceable(vma)) {
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915)) {
addr = (void __force *)i915_vma_pin_iomap(vma);
} else {
int type = i915_coherent_map_type(vma->vm->i915, vma->obj,
false);
@@ -98,7 +98,7 @@ void intel_ring_unpin(struct intel_ring
return;
i915_vma_unset_ggtt_write(vma);
- if (i915_vma_is_map_and_fenceable(vma))
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
i915_vma_unpin_iomap(vma);
else
i915_gem_object_unpin_map(vma->obj);
[Intel-gfx] [PATCH 6.1 881/885] drm/i915: Dont use stolen memory for ring buffers with LLC
From: John Harrison
commit 690e0ec8e63da9a29b39fedc6ed5da09c7c82651 upstream.
Direction from hardware is that stolen memory should never be used for
ring buffer allocations on platforms with LLC. There are too many
caching pitfalls due to the way stolen memory accesses are routed. So
it is safest to just not use it.
Signed-off-by: John Harrison
Fixes: c58b735fc762 ("drm/i915: Allocate rings from stolen")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-2-john.c.harri...@intel.com
(cherry picked from commit f54c1f6c697c4297f7ed94283c184acc338a5cf8)
Signed-off-by: Jani Nikula
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ring.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -116,7 +116,7 @@ static struct i915_vma *create_ring_vma(
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))
+ if (IS_ERR(obj) && i915_ggtt_has_aperture(ggtt) && !HAS_LLC(i915))
obj = i915_gem_object_create_stolen(i915, size);
if (IS_ERR(obj))
obj = i915_gem_object_create_internal(i915, size);
[Intel-gfx] [PATCH 6.2 0998/1001] drm/i915: Dont use BAR mappings for ring buffers with LLC
From: John Harrison
commit 85636167e3206c3fbd52254fc432991cc4e90194 upstream.
Direction from hardware is that ring buffers should never be mapped
via the BAR on systems with LLC. There are too many caching pitfalls
due to the way BAR accesses are routed. So it is safest to just not
use it.
Signed-off-by: John Harrison
Fixes: 9d80841ea4c9 ("drm/i915: Allow ringbuffers to be bound anywhere")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-3-john.c.harri...@intel.com
(cherry picked from commit 65c08339db1ada87afd6cfe7db8e60bb4851d919)
Signed-off-by: Jani Nikula
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ring.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -53,7 +53,7 @@ int intel_ring_pin(struct intel_ring *ri
if (unlikely(ret))
goto err_unpin;
- if (i915_vma_is_map_and_fenceable(vma)) {
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915)) {
addr = (void __force *)i915_vma_pin_iomap(vma);
} else {
int type = i915_coherent_map_type(vma->vm->i915, vma->obj,
false);
@@ -98,7 +98,7 @@ void intel_ring_unpin(struct intel_ring
return;
i915_vma_unset_ggtt_write(vma);
- if (i915_vma_is_map_and_fenceable(vma))
+ if (i915_vma_is_map_and_fenceable(vma) && !HAS_LLC(vma->vm->i915))
i915_vma_unpin_iomap(vma);
else
i915_gem_object_unpin_map(vma->obj);
[Intel-gfx] [PATCH 6.2 0997/1001] drm/i915: Dont use stolen memory for ring buffers with LLC
From: John Harrison
commit 690e0ec8e63da9a29b39fedc6ed5da09c7c82651 upstream.
Direction from hardware is that stolen memory should never be used for
ring buffer allocations on platforms with LLC. There are too many
caching pitfalls due to the way stolen memory accesses are routed. So
it is safest to just not use it.
Signed-off-by: John Harrison
Fixes: c58b735fc762 ("drm/i915: Allocate rings from stolen")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: # v4.9+
Tested-by: Jouni Högander
Reviewed-by: Daniele Ceraolo Spurio
Link:
https://patchwork.freedesktop.org/patch/msgid/20230216011101.1909009-2-john.c.harri...@intel.com
(cherry picked from commit f54c1f6c697c4297f7ed94283c184acc338a5cf8)
Signed-off-by: Jani Nikula
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ring.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -116,7 +116,7 @@ static struct i915_vma *create_ring_vma(
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))
+ if (IS_ERR(obj) && i915_ggtt_has_aperture(ggtt) && !HAS_LLC(i915))
obj = i915_gem_object_create_stolen(i915, size);
if (IS_ERR(obj))
obj = i915_gem_object_create_internal(i915, size);
Re: [Intel-gfx] [char-misc-next v3 0/2] mei: gsc proxy component
On Wed, Feb 08, 2023 at 12:17:23PM -0500, Rodrigo Vivi wrote: > On Wed, Feb 08, 2023 at 04:23:56PM +0200, Tomas Winkler wrote: > > GSC Proxy component is used for communication between the > > Intel graphics driver and MEI driver. > > > > Daniele, please ack so that drm part can be merged via Greg's tree. > > Cc: Daniele, since he was missing on this submission. > > He raise concerns on getting this through another tree since he > has a pending series that uses this interface here. The propagation > at this point will take so long. > > Could we do the other way around and get Greg's and your, Thomas', > acks to merge this through our drm-intel trees? Acked-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH] drm: Disable dynamic debug as broken
On Tue, Feb 07, 2023 at 04:33:37PM +0200, Jani Nikula wrote:
> From: Ville Syrjälä
>
> CONFIG_DRM_USE_DYNAMIC_DEBUG breaks debug prints for (at least modular)
> drm drivers. The debug prints can be reinstated by manually frobbing
> /sys/module/drm/parameters/debug after the fact, but at that point the
> damage is done and all debugs from driver probe are lost. This makes
> drivers totally undebuggable.
>
> There's a more complete fix in progress [1], with further details, but
> we need this fixed in stable kernels. Mark the feature as broken and
> disable it by default, with hopes distros follow suit and disable it as
> well.
>
> [1] https://lore.kernel.org/r/20230125203743.564009-1-jim.cro...@gmail.com
>
> Fixes: 84ec67288c10 ("drm_print: wrap drm_*_dbg in dyndbg descriptor factory
> macro")
> Cc: Jim Cromie
> Cc: Greg Kroah-Hartman
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: dri-de...@lists.freedesktop.org
> Cc: # v6.1+
> Signed-off-by: Ville Syrjälä
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index f42d4c6a19f2..dc0f94f02a82 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -52,7 +52,8 @@ config DRM_DEBUG_MM
>
> config DRM_USE_DYNAMIC_DEBUG
> bool "use dynamic debug to implement drm.debug"
> - default y
> + default n
> + depends on BROKEN
> depends on DRM
> depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> depends on JUMP_LABEL
Acked-by: Greg Kroah-Hartman
[Intel-gfx] [PATCH] i915: fix memory leak with using debugfs_lookup()
When calling debugfs_lookup() the result must have dput() called on it, otherwise the memory will leak over time. To make things simpler, just call debugfs_lookup_and_remove() instead which handles all of the logic at once. Cc: Zhenyu Wang Cc: Zhi Wang Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: Tvrtko Ursulin Cc: David Airlie Cc: Daniel Vetter Cc: intel-gvt-...@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 8ae7039b3683..de675d799c7d 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -699,7 +699,7 @@ static void intel_vgpu_close_device(struct vfio_device *vfio_dev) clear_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status); - debugfs_remove(debugfs_lookup(KVMGT_DEBUGFS_FILENAME, vgpu->debugfs)); + debugfs_lookup_and_remove(KVMGT_DEBUGFS_FILENAME, vgpu->debugfs); kvm_page_track_unregister_notifier(vgpu->vfio_device.kvm, >track_node); -- 2.39.1
[Intel-gfx] [PATCH 5.15 118/204] drm/i915: Allow switching away via vga-switcheroo if uninitialized
From: Thomas Zimmermann
[ Upstream commit a273e95721e96885971a05f1b34cb6d093904d9d ]
Always allow switching away via vga-switcheroo if the display is
uninitalized. Instead prevent switching to i915 if the device has
not been initialized.
This issue was introduced by commit 5df7bd130818 ("drm/i915: skip
display initialization when there is no display") protected, which
protects code paths from being executed on uninitialized devices.
In the case of vga-switcheroo, we want to allow a switch away from
i915's device. So run vga_switcheroo_process_delayed_switch() and
test in the switcheroo callbacks if the i915 device is available.
Fixes: 5df7bd130818 ("drm/i915: skip display initialization when there is no
display")
Signed-off-by: Thomas Zimmermann
Reviewed-by: Alex Deucher
Cc: Radhakrishna Sripada
Cc: Lucas De Marchi
Cc: José Roberto de Souza
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: "Ville Syrjälä"
Cc: Manasi Navare
Cc: Stanislav Lisovskiy
Cc: Imre Deak
Cc: "Jouni Högander"
Cc: Uma Shankar
Cc: Ankit Nautiyal
Cc: "Jason A. Donenfeld"
Cc: Matt Roper
Cc: Ramalingam C
Cc: Thomas Zimmermann
Cc: Andi Shyti
Cc: Andrzej Hajda
Cc: "José Roberto de Souza"
Cc: Julia Lawall
Cc: intel-gfx@lists.freedesktop.org
Cc: # v5.14+
Link:
https://patchwork.freedesktop.org/patch/msgid/20230116115425.13484-2-tzimmerm...@suse.de
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/i915/i915_drv.c| 3 +--
drivers/gpu/drm/i915/i915_switcheroo.c | 6 +-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 59fb4c710c8c..20b9e58de155 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -990,8 +990,7 @@ static void i915_driver_lastclose(struct drm_device *dev)
intel_fbdev_restore_mode(dev);
- if (HAS_DISPLAY(i915))
- vga_switcheroo_process_delayed_switch();
+ vga_switcheroo_process_delayed_switch();
}
static void i915_driver_postclose(struct drm_device *dev, struct drm_file
*file)
diff --git a/drivers/gpu/drm/i915/i915_switcheroo.c
b/drivers/gpu/drm/i915/i915_switcheroo.c
index de0e224b56ce..f1ce9f591efa 100644
--- a/drivers/gpu/drm/i915/i915_switcheroo.c
+++ b/drivers/gpu/drm/i915/i915_switcheroo.c
@@ -18,6 +18,10 @@ static void i915_switcheroo_set_state(struct pci_dev *pdev,
dev_err(>dev, "DRM not initialized, aborting switch.\n");
return;
}
+ if (!HAS_DISPLAY(i915)) {
+ dev_err(>dev, "Device state not initialized, aborting
switch.\n");
+ return;
+ }
if (state == VGA_SWITCHEROO_ON) {
drm_info(>drm, "switched on\n");
@@ -43,7 +47,7 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
* locking inversion with the driver load path. And the access here is
* completely racy anyway. So don't bother with locking for now.
*/
- return i915 && atomic_read(>drm.open_count) == 0;
+ return i915 && HAS_DISPLAY(i915) && atomic_read(>drm.open_count)
== 0;
}
static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
--
2.39.0
Re: [Intel-gfx] [PATCH v7 3/6] mei: clean pending read with vtag on bus
On Wed, Jan 25, 2023 at 12:28:29PM -0500, Rodrigo Vivi wrote: > > Greg, ack on getting these 3 mei patches merged through intel-gfx? I only see 2 mei patches in this series, what am I missing? thanks, greg k-h
Re: [Intel-gfx] [PATCH v7 1/6] mei: mei-me: resume device in prepare
On Wed, Jan 25, 2023 at 12:26:32AM -0800, Alan Previn wrote:
> From: Alexander Usyskin
>
> Asynchronous runtime resume is not possible while the system
> is suspending.
> The power management subsystem resumes the device only in the
> suspend phase, not in the prepare phase.
> Force resume device in prepare to allow drivers on mei bus
> to communicate in their prepare callbacks.
>
> Signed-off-by: Alexander Usyskin
> Reviewed-by: Tomas Winkler
> Signed-off-by: Alan Previn
> ---
> drivers/misc/mei/pci-me.c | 20 +++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
> index 5bf0d50d55a0..676d566f38dd 100644
> --- a/drivers/misc/mei/pci-me.c
> +++ b/drivers/misc/mei/pci-me.c
> @@ -342,6 +342,12 @@ static void mei_me_remove(struct pci_dev *pdev)
> }
>
> #ifdef CONFIG_PM_SLEEP
> +static int mei_me_pci_prepare(struct device *device)
> +{
> + pm_runtime_resume(device);
> + return 0;
> +}
> +
> static int mei_me_pci_suspend(struct device *device)
> {
> struct pci_dev *pdev = to_pci_dev(device);
> @@ -398,7 +404,17 @@ static int mei_me_pci_resume(struct device *device)
>
> return 0;
> }
> -#endif /* CONFIG_PM_SLEEP */
> +
> +static void mei_me_pci_complete(struct device *device)
> +{
> + pm_runtime_suspend(device);
> +}
> +#else /* CONFIG_PM_SLEEP */
> +
> +#define mei_me_pci_prepare NULL
> +#define mei_me_pci_complete NULL
> +
> +#endif /* !CONFIG_PM_SLEEP */
>
> #ifdef CONFIG_PM
> static int mei_me_pm_runtime_idle(struct device *device)
> @@ -501,6 +517,8 @@ static inline void mei_me_unset_pm_domain(struct
> mei_device *dev)
> }
>
> static const struct dev_pm_ops mei_me_pm_ops = {
> + .prepare = mei_me_pci_prepare,
> + .complete = mei_me_pci_complete,
> SET_SYSTEM_SLEEP_PM_OPS(mei_me_pci_suspend,
> mei_me_pci_resume)
> SET_RUNTIME_PM_OPS(
> --
> 2.39.0
>
Acked-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v7 3/6] mei: clean pending read with vtag on bus
On Wed, Jan 25, 2023 at 12:26:34AM -0800, Alan Previn wrote:
> From: Alexander Usyskin
>
> Client on bus have only one vtag map slot and should disregard the vtag
> value when cleaning pending read flag.
> Fixes read flow control message unexpectedly generated when
> clent on bus send messages with different vtags.
>
> Signed-off-by: Alexander Usyskin
> Reviewed-by: Tomas Winkler
> Signed-off-by: Alan Previn
> ---
> drivers/misc/mei/client.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
> index 9ddb854b8155..5c19097266fe 100644
> --- a/drivers/misc/mei/client.c
> +++ b/drivers/misc/mei/client.c
> @@ -1343,7 +1343,9 @@ static void mei_cl_reset_read_by_vtag(const struct
> mei_cl *cl, u8 vtag)
> struct mei_cl_vtag *vtag_l;
>
> list_for_each_entry(vtag_l, >vtag_map, list) {
> - if (vtag_l->vtag == vtag) {
> + /* The client on bus has one fixed vtag map */
> + if ((cl->cldev && mei_cldev_enabled(cl->cldev)) ||
> + vtag_l->vtag == vtag) {
> vtag_l->pending_read = false;
> break;
> }
> --
> 2.39.0
>
Acked-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v7 3/6] mei: clean pending read with vtag on bus
Why are you top posting? On Wed, Jan 25, 2023 at 12:28:29PM -0500, Rodrigo Vivi wrote: > > Greg, ack on getting these 3 mei patches merged through intel-gfx? $ mdfrm -c ~/mail/todo/ 756 messages in /home/gregkh/mail/todo/ Give me a chance, what is the sudden rush? greg k-h
[Intel-gfx] [PATCH 6.1 139/193] drm/i915: Allow switching away via vga-switcheroo if uninitialized
From: Thomas Zimmermann
commit a273e95721e96885971a05f1b34cb6d093904d9d upstream.
Always allow switching away via vga-switcheroo if the display is
uninitalized. Instead prevent switching to i915 if the device has
not been initialized.
This issue was introduced by commit 5df7bd130818 ("drm/i915: skip
display initialization when there is no display") protected, which
protects code paths from being executed on uninitialized devices.
In the case of vga-switcheroo, we want to allow a switch away from
i915's device. So run vga_switcheroo_process_delayed_switch() and
test in the switcheroo callbacks if the i915 device is available.
Fixes: 5df7bd130818 ("drm/i915: skip display initialization when there is no
display")
Signed-off-by: Thomas Zimmermann
Reviewed-by: Alex Deucher
Cc: Radhakrishna Sripada
Cc: Lucas De Marchi
Cc: José Roberto de Souza
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: "Ville Syrjälä"
Cc: Manasi Navare
Cc: Stanislav Lisovskiy
Cc: Imre Deak
Cc: "Jouni Högander"
Cc: Uma Shankar
Cc: Ankit Nautiyal
Cc: "Jason A. Donenfeld"
Cc: Matt Roper
Cc: Ramalingam C
Cc: Thomas Zimmermann
Cc: Andi Shyti
Cc: Andrzej Hajda
Cc: "José Roberto de Souza"
Cc: Julia Lawall
Cc: intel-gfx@lists.freedesktop.org
Cc: # v5.14+
Link:
https://patchwork.freedesktop.org/patch/msgid/20230116115425.13484-2-tzimmerm...@suse.de
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/i915_driver.c |3 +--
drivers/gpu/drm/i915/i915_switcheroo.c |6 +-
2 files changed, 6 insertions(+), 3 deletions(-)
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1074,8 +1074,7 @@ static void i915_driver_lastclose(struct
intel_fbdev_restore_mode(dev);
- if (HAS_DISPLAY(i915))
- vga_switcheroo_process_delayed_switch();
+ vga_switcheroo_process_delayed_switch();
}
static void i915_driver_postclose(struct drm_device *dev, struct drm_file
*file)
--- a/drivers/gpu/drm/i915/i915_switcheroo.c
+++ b/drivers/gpu/drm/i915/i915_switcheroo.c
@@ -19,6 +19,10 @@ static void i915_switcheroo_set_state(st
dev_err(>dev, "DRM not initialized, aborting switch.\n");
return;
}
+ if (!HAS_DISPLAY(i915)) {
+ dev_err(>dev, "Device state not initialized, aborting
switch.\n");
+ return;
+ }
if (state == VGA_SWITCHEROO_ON) {
drm_info(>drm, "switched on\n");
@@ -44,7 +48,7 @@ static bool i915_switcheroo_can_switch(s
* locking inversion with the driver load path. And the access here is
* completely racy anyway. So don't bother with locking for now.
*/
- return i915 && atomic_read(>drm.open_count) == 0;
+ return i915 && HAS_DISPLAY(i915) && atomic_read(>drm.open_count)
== 0;
}
static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
Re: [Intel-gfx] [PATCH v5 1/4] i915: Move list_count() to list.h as list_count_nodes() for broader use
On Thu, Dec 08, 2022 at 02:54:45PM +0200, Andy Shevchenko wrote: > On Wed, Nov 30, 2022 at 03:48:35PM +0200, Andy Shevchenko wrote: > > Some of the existing users, and definitely will be new ones, want to > > count existing nodes in the list. Provide a generic API for that by > > moving code from i915 to list.h. > > Greg, I believe this one is ready to be taken. Or please tell me what I need > to do. Wait for me to get through the current backlog of patches that I have in my review queue. Odds are, it will have to wait until after 6.2-rc1 is out based on when 6.1 is going to be released. Don't worry, it's not lost. thanks, greg k-h
Re: [Intel-gfx] [PATCH v3 0/2] mei: add timeout to send
On Wed, Nov 30, 2022 at 09:20:28AM -0500, Rodrigo Vivi wrote:
> On Wed, Nov 16, 2022 at 02:47:33PM +0200, Alexander Usyskin wrote:
> > When driver wakes up the firmware from the low power state,
> > it is sending a memory ready message.
> > The send is done via synchronous/blocking function to ensure
> > that firmware is in ready state. However, in case of firmware
> > undergoing reset send might be block forever.
> > To address this issue a timeout is added to blocking
> > write command on the internal bus.
> >
> > Introduce the __mei_cl_send_timeout function to use instead of
> > __mei_cl_send in cases where timeout is required.
> > The mei_cl_write has only two callers and there is no need to split
> > it into two functions.
> >
> > V2: address review comments:
> > - split __mei_cl_send and __mei_cl_send_timeout
> > - add units to timeout KDoc
> > - use MAX_SCHEDULE_TIMEOUT to squash wait to one macro
> >
> > V3: - split the state fix into separate patch
> > - document define unit
> > - expand timeout KDoc
>
> These 2 patches looks good to me now.
>
> Greg, whenever you review it, please let me know if it is
> okay to me to push these through the drm-fixes, or if you
> prefer these to the mei branches.
These have been in my tree for over a week now, sorry. No one told me
not to take them...
{sigh}
greg k-h
Re: [Intel-gfx] [PATCH v2] mei: add timeout to send
On Tue, Nov 15, 2022 at 02:27:02PM +, Usyskin, Alexander wrote:
> > > - else
> > > + } else {
> > > dev_dbg(>dev, "memory ready command
> > sent\n");
> > > + cldev->bus->pxp_mode = MEI_DEV_PXP_SETUP;
> >
> > What does the mode change have to do with a timeout?
> With timeout the mei_gfx_memory_ready may now fail gracefully
> and we should not move state if message is not sent.
>
> Should I split this fix into another patch or document in this one?
Split it into a different patch please.
> > > +/**
> > > + * __mei_cl_send_timeout - internal client send (write)
> > > + *
> > > + * @cl: host client
> > > + * @buf: buffer to send
> > > + * @length: buffer length
> > > + * @vtag: virtual tag
> > > + * @mode: sending mode
> > > + * @timeout: send timeout in milliseconds for blocking writes
> >
> > What do you mean "for blocking writes"?
>
> The timeout has no effect for non-blocking writes (bit in mode parameter),
> as they are returning immediately and are not waiting at all.
That's not obvious, please say that the timeout is affected by the mode
and how it is affected.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v2] mei: add timeout to send
On Tue, Nov 15, 2022 at 01:14:38PM +0200, Alexander Usyskin wrote:
> When driver wakes up the firmware from the low power state, it is sending
> a memory ready message.
> The send is done via synchronous/blocking function to ensure that firmware
> is in ready state. However, in case of firmware undergoing reset send
> might be block forever.
> To address this issue a timeout is added to blocking write command on
> the internal bus.
Odd formatting of the text :(
>
> Introduce the __mei_cl_send_timeout function to use instead of
> __mei_cl_send in cases where timeout is required.
> The mei_cl_write has only two callers and there is no need to split
> it into two functions.
>
> Signed-off-by: Alexander Usyskin
> ---
> V2: address review comments:
> - split __mei_cl_send and __mei_cl_send_timeout
> - add units to timeout KDoc
> - use MAX_SCHEDULE_TIMEOUT to squash wait to one macro
>
> drivers/misc/mei/bus-fixup.c | 14 +-
> drivers/misc/mei/bus.c | 22 --
> drivers/misc/mei/client.c| 18 ++
> drivers/misc/mei/client.h| 2 +-
> drivers/misc/mei/main.c | 2 +-
> drivers/misc/mei/mei_dev.h | 2 ++
> 6 files changed, 47 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 71fbf0bc8453..9959b8e8e91d 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -188,17 +188,20 @@ static int mei_fwver(struct mei_cl_device *cldev)
> return ret;
> }
>
> +#define GFX_MEMORY_READY_TIMEOUT 200
units?
> +
> static int mei_gfx_memory_ready(struct mei_cl_device *cldev)
> {
> struct mkhi_gfx_mem_ready req = {0};
> - unsigned int mode = MEI_CL_IO_TX_INTERNAL;
> + unsigned int mode = MEI_CL_IO_TX_INTERNAL | MEI_CL_IO_TX_BLOCKING;
>
> req.hdr.group_id = MKHI_GROUP_ID_GFX;
> req.hdr.command = MKHI_GFX_MEMORY_READY_CMD_REQ;
> req.flags = MKHI_GFX_MEM_READY_PXP_ALLOWED;
>
> dev_dbg(>dev, "Sending memory ready command\n");
> - return __mei_cl_send(cldev->cl, (u8 *), sizeof(req), 0, mode);
> + return __mei_cl_send_timeout(cldev->cl, (u8 *), sizeof(req), 0,
> + mode, GFX_MEMORY_READY_TIMEOUT);
> }
>
> static void mei_mkhi_fix(struct mei_cl_device *cldev)
> @@ -263,12 +266,13 @@ static void mei_gsc_mkhi_fix_ver(struct mei_cl_device
> *cldev)
>
> if (cldev->bus->pxp_mode == MEI_DEV_PXP_INIT) {
> ret = mei_gfx_memory_ready(cldev);
> - if (ret < 0)
> + if (ret < 0) {
> dev_err(>dev, "memory ready command failed
> %d\n", ret);
> - else
> + } else {
> dev_dbg(>dev, "memory ready command sent\n");
> + cldev->bus->pxp_mode = MEI_DEV_PXP_SETUP;
What does the mode change have to do with a timeout?
> + }
> /* we go to reset after that */
> - cldev->bus->pxp_mode = MEI_DEV_PXP_SETUP;
> goto out;
> }
>
> diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> index 1fbe127ff633..63043e8df980 100644
> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -32,8 +32,26 @@
> *
> * Return: written size bytes or < 0 on error
> */
> -ssize_t __mei_cl_send(struct mei_cl *cl, const u8 *buf, size_t length, u8
> vtag,
> +inline ssize_t __mei_cl_send(struct mei_cl *cl, const u8 *buf, size_t
> length, u8 vtag,
> unsigned int mode)
Why inline? The compiler is smart enough.
> +{
> + return __mei_cl_send_timeout(cl, buf, length, vtag, mode,
> MAX_SCHEDULE_TIMEOUT);
So this will block for how long? Please document this.
> +}
> +
> +/**
> + * __mei_cl_send_timeout - internal client send (write)
> + *
> + * @cl: host client
> + * @buf: buffer to send
> + * @length: buffer length
> + * @vtag: virtual tag
> + * @mode: sending mode
> + * @timeout: send timeout in milliseconds for blocking writes
What do you mean "for blocking writes"?
And what do you use here to wait "for forever"?
> + *
> + * Return: written size bytes or < 0 on error
> + */
> +ssize_t __mei_cl_send_timeout(struct mei_cl *cl, const u8 *buf, size_t
> length, u8 vtag,
> + unsigned int mode, unsigned long timeout)
> {
> struct mei_device *bus;
> struct mei_cl_cb *cb;
> @@ -108,7 +126,7 @@ ssize_t __mei_cl_send(struct mei_cl *cl, const u8 *buf,
> size_t length, u8 vtag,
> cb->buf.size = 0;
> }
>
> - rets = mei_cl_write(cl, cb);
> + rets = mei_cl_write(cl, cb, timeout);
>
> if (mode & MEI_CL_IO_SGL && rets == 0)
> rets = length;
> diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
> index 6c8b71ae32c8..02c278202ad7 100644
> --- a/drivers/misc/mei/client.c
> +++ b/drivers/misc/mei/client.c
> @@ -1954,10 +1954,11 @@ int mei_cl_irq_write(struct mei_cl *cl, struct
> mei_cl_cb *cb,
>
Re: [Intel-gfx] [PATCH] mei: add timeout to send
On Thu, Nov 03, 2022 at 05:55:34PM +0200, Alexander Usyskin wrote: > When driver wakes up the firmware from the low power stand, > it is sending a memory ready message. > The send is done via synchronous/blocking function to ensure > that firmware is in ready state. However firmware might be > in unstable state and send might be block forever. > To address this issue a timeout is added to blocking write command on > the internal bus. > > Signed-off-by: Alexander Usyskin > --- > drivers/misc/mei/bus-fixup.c | 19 +++ > drivers/misc/mei/bus.c | 9 + > drivers/misc/mei/client.c| 21 + > drivers/misc/mei/client.h| 2 +- > drivers/misc/mei/main.c | 2 +- > drivers/misc/mei/mei_dev.h | 2 +- > 6 files changed, 36 insertions(+), 19 deletions(-) > > diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c > index 71fbf0bc8453..3174cad8a5cc 100644 > --- a/drivers/misc/mei/bus-fixup.c > +++ b/drivers/misc/mei/bus-fixup.c > @@ -128,7 +128,7 @@ static int mei_osver(struct mei_cl_device *cldev) > os_ver = (struct mei_os_ver *)fwcaps->data; > os_ver->os_type = OSTYPE_LINUX; > > - return __mei_cl_send(cldev->cl, buf, size, 0, mode); > + return __mei_cl_send(cldev->cl, buf, size, 0, mode, 0); Ick, adding another parameter to a function is not helpful here, what does 0 mean unless you look it up? > --- a/drivers/misc/mei/bus.c > +++ b/drivers/misc/mei/bus.c > @@ -29,11 +29,12 @@ > * @length: buffer length > * @vtag: virtual tag > * @mode: sending mode > + * @timeout: send timeout for blocking writes, 0 for infinite timeout What units are these in? Jiffies? seconds? hours? Don't make us guess :( Why not write a function that is mei_cl_send_timeout() that has the new option? That way it's obvious that the other ones are blocking, right? thanks, greg k-h
[Intel-gfx] [PATCH 6.0 189/862] drm/i915: Fix display problems after resume
From: Thomas Hellström
commit 6c482c62a635aa4f534d2439fbf8afa37452b986 upstream.
Commit 39a2bd34c933 ("drm/i915: Use the vma resource as argument for gtt
binding / unbinding") introduced a regression that due to the vma resource
tracking of the binding state, dpt ptes were not correctly repopulated.
Fix this by clearing the vma resource state before repopulating.
The state will subsequently be restored by the bind_vma operation.
Fixes: 39a2bd34c933 ("drm/i915: Use the vma resource as argument for gtt
binding / unbinding")
Signed-off-by: Thomas Hellström
Link:
https://patchwork.freedesktop.org/patch/msgid/20220912121957.31310-1-thomas.hellst...@linux.intel.com
Cc: Matthew Auld
Cc: intel-gfx@lists.freedesktop.org
Cc: # v5.18+
Reported-and-tested-by: Kevin Boulain
Tested-by: David de Sousa
Reviewed-by: Matthew Auld
Reviewed-by: Andrzej Hajda
Signed-off-by: Matthew Auld
Link:
https://patchwork.freedesktop.org/patch/msgid/20221005121159.340245-1-thomas.hellst...@linux.intel.com
(cherry picked from commit bc2472538c0d1cce334ffc9e97df0614cd2b1469)
Signed-off-by: Tvrtko Ursulin
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ggtt.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -1267,10 +1267,16 @@ bool i915_ggtt_resume_vm(struct i915_add
atomic_read(>flags) & I915_VMA_BIND_MASK;
GEM_BUG_ON(!was_bound);
- if (!retained_ptes)
+ if (!retained_ptes) {
+ /*
+* Clear the bound flags of the vma resource to allow
+* ptes to be repopulated.
+*/
+ vma->resource->bound_flags = 0;
vma->ops->bind_vma(vm, NULL, vma->resource,
obj ? obj->cache_level : 0,
was_bound);
+ }
if (obj) { /* only used during resume => exclusive access */
write_domain_objs |= fetch_and_zero(>write_domain);
obj->read_domains |= I915_GEM_DOMAIN_GTT;
Re: [Intel-gfx] v5.19 & v6.0 stable backport request
On Wed, Oct 19, 2022 at 08:55:55AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 18, 2022 at 02:02:08PM +0300, Jani Nikula wrote:
> >
> > Hello stable team, please backport these two commits to stable kernels
> > v5.19 and v6.0:
> >
> > 4e78d6023c15 ("drm/i915/bios: Validate fp_timing terminator presence")
>
> Does not apply to 5.19.y, can you provide a working backport?
>
> > d3a7051841f0 ("drm/i915/bios: Use hardcoded fp_timing size for generating
> > LFP data pointers")
>
> Queued up to both trees now, thanks.
No, wait, that breaks the build!
How did you test this? I'm dropping both of these now.
Please resubmit a set of tested patches if you wish to have them applied
to the tree. These were obviously not even attempted, which just wastes
all of our time :(
greg k-h
Re: [Intel-gfx] v5.19 & v6.0 stable backport request
On Tue, Oct 18, 2022 at 02:02:08PM +0300, Jani Nikula wrote:
>
> Hello stable team, please backport these two commits to stable kernels
> v5.19 and v6.0:
>
> 4e78d6023c15 ("drm/i915/bios: Validate fp_timing terminator presence")
Does not apply to 5.19.y, can you provide a working backport?
> d3a7051841f0 ("drm/i915/bios: Use hardcoded fp_timing size for generating LFP
> data pointers")
Queued up to both trees now, thanks.
greg k-h
Re: [Intel-gfx] Regression on 5.19.12, display flickering on Framework laptop
On Tue, Oct 04, 2022 at 04:44:35PM +0300, Ville Syrjälä wrote: > On Tue, Oct 04, 2022 at 03:40:55PM +0200, Greg Kroah-Hartman wrote: > > On Tue, Oct 04, 2022 at 06:46:10AM -0500, David Matthew Mattli wrote: > > > Thorsten Leemhuis writes: > > > > > > > On 03.10.22 19:48, Ville Syrjälä wrote: > > > >> On Mon, Oct 03, 2022 at 08:45:18PM +0300, Ville Syrjälä wrote: > > > >>> On Sat, Oct 01, 2022 at 12:07:39PM +0200, Thorsten Leemhuis wrote: > > > >>>> On 30.09.22 14:26, Jerry Ling wrote: > > > >>>>> > > > >>>>> looks like someone has done it: > > > >>>>> https://bbs.archlinux.org/viewtopic.php?pid=2059823#p2059823 > > > >>>>> > > > >>>>> and the bisect points to: > > > >>>>> > > > >>>>> |# first bad commit: [fc6aff984b1c63d6b9e54f5eff9cc5ac5840bc8c] > > > >>>>> drm/i915/bios: Split VBT data into per-panel vs. global parts Best, > > > >>>>> Jerry > > > | > > > >>>> > > > >>>> FWIW, that's 3cf050762534 in mainline. Adding Ville, its author to > > > >>>> the > > > >>>> list of recipients. > > > >>> > > > >>> I definitely had no plans to backport any of that stuff, > > > >>> but I guess the automagics did it anyway. > > > >>> > > > >>> Looks like stable is at least missing this pile of stuff: > > > >>> 50759c13735d drm/i915/pps: Keep VDD enabled during eDP probe > > > >>> 67090801489d drm/i915/pps: Reinit PPS delays after VBT has been fully > > > parsed > > > >>> 8e75e8f573e1 drm/i915/pps: Split PPS init+sanitize in two > > > >>> 586294c3c186 drm/i915/pps: Stash away original BIOS programmed PPS > > > >>> delays > > > >>> 89fcdf430599 drm/i915/pps: Don't apply quirks/etc. to the VBT PPS > > > >>> delays if they haven't been initialized > > > >>> 60b02a09598f drm/i915/pps: Introduce pps_delays_valid() > > > >>> > > > >>> But dunno if even that is enough. > > > > > > > > If you need testers: David (now CCed) apparently has a affected machine > > > > and offered to test patches in a different subthread of this thread. > > > > > > > > > > I cherry-picked the six commits Thorsten listed onto 5.19.12 and it > > > resolved the issue on my Framework laptop. > > > > Thanks for testing, but I'm just going to revert the offending commits > > as they probably shouldn't all be added to 5.19.y > > Yeah, revert seems the safer route. Thanks. 5.19.13 is now released with 8 reverts for this driver, hopefully that sould resolve this issue. thanks, greg k-h
Re: [Intel-gfx] Regression on 5.19.12, display flickering on Framework laptop
On Tue, Oct 04, 2022 at 06:46:10AM -0500, David Matthew Mattli wrote: > Thorsten Leemhuis writes: > > > On 03.10.22 19:48, Ville Syrjälä wrote: > >> On Mon, Oct 03, 2022 at 08:45:18PM +0300, Ville Syrjälä wrote: > >>> On Sat, Oct 01, 2022 at 12:07:39PM +0200, Thorsten Leemhuis wrote: > On 30.09.22 14:26, Jerry Ling wrote: > > > > looks like someone has done it: > > https://bbs.archlinux.org/viewtopic.php?pid=2059823#p2059823 > > > > and the bisect points to: > > > > |# first bad commit: [fc6aff984b1c63d6b9e54f5eff9cc5ac5840bc8c] > > drm/i915/bios: Split VBT data into per-panel vs. global parts Best, > > Jerry > | > > FWIW, that's 3cf050762534 in mainline. Adding Ville, its author to the > list of recipients. > >>> > >>> I definitely had no plans to backport any of that stuff, > >>> but I guess the automagics did it anyway. > >>> > >>> Looks like stable is at least missing this pile of stuff: > >>> 50759c13735d drm/i915/pps: Keep VDD enabled during eDP probe > >>> 67090801489d drm/i915/pps: Reinit PPS delays after VBT has been fully > parsed > >>> 8e75e8f573e1 drm/i915/pps: Split PPS init+sanitize in two > >>> 586294c3c186 drm/i915/pps: Stash away original BIOS programmed PPS delays > >>> 89fcdf430599 drm/i915/pps: Don't apply quirks/etc. to the VBT PPS > >>> delays if they haven't been initialized > >>> 60b02a09598f drm/i915/pps: Introduce pps_delays_valid() > >>> > >>> But dunno if even that is enough. > > > > If you need testers: David (now CCed) apparently has a affected machine > > and offered to test patches in a different subthread of this thread. > > > > I cherry-picked the six commits Thorsten listed onto 5.19.12 and it > resolved the issue on my Framework laptop. Thanks for testing, but I'm just going to revert the offending commits as they probably shouldn't all be added to 5.19.y thanks, greg k-h
Re: [Intel-gfx] Regression on 5.19.12, display flickering on Framework laptop
On Tue, Oct 04, 2022 at 03:35:44PM +0300, Ville Syrjälä wrote: > On Mon, Oct 03, 2022 at 08:28:50PM +0200, Thorsten Leemhuis wrote: > > > > > > On 03.10.22 19:48, Ville Syrjälä wrote: > > > On Mon, Oct 03, 2022 at 08:45:18PM +0300, Ville Syrjälä wrote: > > >> On Sat, Oct 01, 2022 at 12:07:39PM +0200, Thorsten Leemhuis wrote: > > >>> On 30.09.22 14:26, Jerry Ling wrote: > > > > looks like someone has done it: > > https://bbs.archlinux.org/viewtopic.php?pid=2059823#p2059823 > > > > and the bisect points to: > > > > |# first bad commit: [fc6aff984b1c63d6b9e54f5eff9cc5ac5840bc8c] > > drm/i915/bios: Split VBT data into per-panel vs. global parts Best, > > Jerry | > > >>> > > >>> FWIW, that's 3cf050762534 in mainline. Adding Ville, its author to the > > >>> list of recipients. > > >> > > >> I definitely had no plans to backport any of that stuff, > > >> but I guess the automagics did it anyway. > > >> > > >> Looks like stable is at least missing this pile of stuff: > > >> 50759c13735d drm/i915/pps: Keep VDD enabled during eDP probe > > >> 67090801489d drm/i915/pps: Reinit PPS delays after VBT has been fully > > >> parsed > > >> 8e75e8f573e1 drm/i915/pps: Split PPS init+sanitize in two > > >> 586294c3c186 drm/i915/pps: Stash away original BIOS programmed PPS delays > > >> 89fcdf430599 drm/i915/pps: Don't apply quirks/etc. to the VBT PPS delays > > >> if they haven't been initialized > > >> 60b02a09598f drm/i915/pps: Introduce pps_delays_valid() > > >> > > >> But dunno if even that is enough. > > > > If you need testers: David (now CCed) apparently has a affected machine > > and offered to test patches in a different subthread of this thread. > > > > >> This bug report is probably the same thing: > > >> https://gitlab.freedesktop.org/drm/intel/-/issues/7013 > > > > Sounds like it. > > > > > Also cc intel-gfx... > > > > Ahh, sorry, should have done that when I CCed you. > > After looking at some logs we do end up with potentially bogus > panel power sequencing delays, which may harm the LCD panel. > > > Greg, I recommend immediate revert of this stuff, and new stable > release ASAP. Plus a recommendation that no one using laptops > with Intel GPUs run 5.19.12. Ok, will do, I'll go do that right now, thanks and sorry for the problems. greg k-h
Re: [Intel-gfx] [PATCH v5 05/15] mei: pxp: add command streamer API to the PXP driver
On Mon, Sep 12, 2022 at 05:57:29PM -0700, Daniele Ceraolo Spurio wrote: > From: Vitaly Lubart > > The discrete graphics card with GSC firmware > using command streamer API hence it requires to enhance > pxp module with the new gsc_command() handler. > > The handler is implemented via mei_pxp_gsc_command() which is > just a thin wrapper around mei_cldev_send_gsc_command() > > Signed-off-by: Vitaly Lubart > Signed-off-by: Tomas Winkler > Signed-off-by: Daniele Ceraolo Spurio > Cc: Greg Kroah-Hartman > Reviewed-by: Alan Previn > --- Reviewed-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v5 04/15] mei: bus: extend bus API to support command streamer API
On Mon, Sep 12, 2022 at 05:57:28PM -0700, Daniele Ceraolo Spurio wrote: > From: Vitaly Lubart > > Add mei bus API for sending gsc commands: mei_cldev_send_gsc_command() > > The GSC commands are originated in the graphics stack > and are in form of SGL DMA buffers. > The GSC commands are synchronous, the response is received > in the same call on the out sg list buffers. > The function setups pointers for in and out sg lists in the > mei sgl extended header and sends it to the firmware. > > Signed-off-by: Vitaly Lubart > Signed-off-by: Tomas Winkler > Signed-off-by: Daniele Ceraolo Spurio > Cc: Greg Kroah-Hartman > --- Reviewed-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v5 06/15] mei: pxp: support matching with a gfx discrete card
On Mon, Sep 12, 2022 at 05:57:30PM -0700, Daniele Ceraolo Spurio wrote: > From: Tomas Winkler > > With on-boards graphics card, both i915 and MEI > are in the same device hierarchy with the same parent, > while for discrete gfx card the MEI is its child device. > Adjust the match function for that scenario > by matching MEI parent device with i915. > > Signed-off-by: Tomas Winkler > Signed-off-by: Daniele Ceraolo Spurio > Cc: Vitaly Lubart > Cc: Greg Kroah-Hartman > Reviewed-by: Alan Previn > --- Reviewed-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v5 03/15] mei: adjust extended header kdocs
On Mon, Sep 12, 2022 at 05:57:27PM -0700, Daniele Ceraolo Spurio wrote: > From: Tomas Winkler > > Fix kdoc for struct mei_ext_hdr and mei_ext_begin(). > > Signed-off-by: Tomas Winkler > Signed-off-by: Daniele Ceraolo Spurio > Cc: Greg Kroah-Hartman > --- Reviewed-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v5 02/15] mei: bus: enable sending gsc commands
On Mon, Sep 12, 2022 at 05:57:26PM -0700, Daniele Ceraolo Spurio wrote: > From: Tomas Winkler > > GSC command is and extended header containing a scatter gather > list and without a data buffer. Using MEI_CL_IO_SGL flag, > the caller send the GSC command as a data and the function internally > moves it to the extended header. > > Signed-off-by: Tomas Winkler > Signed-off-by: Daniele Ceraolo Spurio > Cc: Vitaly Lubart > Cc: Greg Kroah-Hartman > --- Reviewed-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v5 01/15] mei: add support to GSC extended header
On Mon, Sep 12, 2022 at 05:57:25PM -0700, Daniele Ceraolo Spurio wrote: > From: Tomas Winkler > > GSC extend header is of variable size and data > is provided in a sgl list inside the header > and not in the data buffers, need to enable the path. > > Signed-off-by: Tomas Winkler > Signed-off-by: Daniele Ceraolo Spurio > Cc: Vitaly Lubart > Cc: Greg Kroah-Hartman > --- Reviewed-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v5 00/15] drm/i915: HuC loading for DG2
On Wed, Sep 14, 2022 at 04:51:03PM +, Winkler, Tomas wrote: > > > > On DG2, HuC loading is performed by the GSC, via a PXP command. The load > > operation itself is relatively simple (just send a message to the GSC with > > the > > physical address of the HuC in LMEM), but there are timing changes that > > requires special attention. In particular, to send a PXP command we need to > > first export the GSC as an aux device and then wait for the mei-gsc and mei- > > pxp modules to start, which means that HuC load will complete after i915 > > load is complete. This means that there is a small window of time after > > i915 is > > registered and before HuC is loaded during which userspace could submit > > and/or check the HuC load status, although this is quite unlikely to happen > > (HuC is usually loaded before kernel init/resume completes). > > We've consulted with the media team in regards to how to handle this and > > they've asked us to stall all userspace VCS submission until HuC is loaded. > > Stalls are expected to be very rare (if any), due to the fact that HuC is > > usually > > loaded before kernel init/resume is completed. > > > > Timeouts are in place to ensure all submissions are unlocked in case > > something goes wrong. Since we need to monitor the status of the mei > > driver to know what's happening and when to time out, a notifier has been > > added so we get a callback when the status of the mei driver changes. > > > > Note that this series includes several mei patches that add support for > > sending the HuC loading command via mei-gsc. We plan to merge those > > patches through the drm tree because i915 is the sole user. > > > > v2: address review comments, Reporting HuC loading still in progress while > > we wait for mei-gsc init to complete, rebase on latest mei-gsc series. > > > > v3: fix cc list in mei patches. > > > > v4: update mei patches, fix includes, rebase on new FW fetch logic and > > merged mei-gsc support. > > > > v5: update mei patches > > Greg, I hope I've addressed most of your comments. > Can you please check if the mei patches are in acceptable state or anything > else can be improved with this series. Appreciated. These were sent 2 days ago, in the middle of a conference travel. Please relax, there's no special rush needed here, you know better. In the mean time, if you are just waiting for my review, please take the time to review other pending patches from other developers to help lighten the load on me, and other maintainers. thanks, greg k-h
Re: [Intel-gfx] [PATCH v4 05/15] mei: pxp: add command streamer API to the PXP driver
On Mon, Sep 12, 2022 at 09:59:45AM +, Winkler, Tomas wrote:
> >
> > On Fri, Sep 09, 2022 at 06:38:33AM +, Winkler, Tomas wrote:
> > > >
> > > > On Thu, Sep 08, 2022 at 05:16:02PM -0700, Daniele Ceraolo Spurio wrote:
> > > > > +static ssize_t mei_pxp_gsc_command(struct device *dev, u8
> > > > > +client_id,
> > > > u32 fence_id,
> > > > > +struct scatterlist *sg_in, size_t
> > > > > total_in_len,
> > > > > +struct scatterlist *sg_out) {
> > > > > + struct mei_cl_device *cldev;
> > > > > +
> > > > > + if (!dev || !sg_in || !sg_out)
> > > > > + return -EINVAL;
> > > >
> > > > How can these ever be NULL? Doesn't the core control this, so why
> > > > would that happen?
> > > This is any interface function between modules, I think it is not
> > > healthy to take assumptions here about how caller behaves, this is not an
> > inner functions. This is important even for catching programmatical
> > mistakes.
> >
> > It is a static function being called from a framework. Enforce this in the
> > framework, don't sprinkle this stuff everywhere, the kernel is NOT defensive
> > about internal users like this otherwise it will overwhelm every function
> > call.
>
> I'm not sure, this is the case here. The function is passed to i915 (other
> driver) driver via struct i915_pxp_component_op.
> This is outside of the component framework.
Then pass in the real mei structure please. Don't force someone else to
"find" it in the device tree like you are currently trying to do.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v4 06/15] mei: pxp: support matching with a gfx discrete card
On Fri, Sep 09, 2022 at 09:21:30AM +, Winkler, Tomas wrote:
> > >
> > > > -Original Message-----
> > > > From: Greg Kroah-Hartman
> > > > Sent: Friday, September 09, 2022 09:16
> > > > To: Ceraolo Spurio, Daniele
> > > > Cc: intel-gfx@lists.freedesktop.org;
> > > > dri-de...@lists.freedesktop.org; Winkler, Tomas
> > > > ; Lubart, Vitaly ;
> > > > Teres Alexis, Alan Previn
> > > > Subject: Re: [PATCH v4 06/15] mei: pxp: support matching with a gfx
> > > > discrete card
> > > >
> > > > On Thu, Sep 08, 2022 at 05:16:03PM -0700, Daniele Ceraolo Spurio wrote:
> > > > > From: Tomas Winkler
> > > > >
> > > > > With on-boards graphics card, both i915 and MEI are in the same
> > > > > device hierarchy with the same parent, while for discrete gfx card
> > > > > the MEI is its child device.
> > > > > Adjust the match function for that scenario by matching MEI parent
> > > > > device with i915.
> > > > >
> > > > > V2:
> > > > > 1. More detailed commit message
> > > > > 2. Check for dev is not null before it is accessed.
> > > > >
> > > > > Signed-off-by: Tomas Winkler
> > > > > Signed-off-by: Daniele Ceraolo Spurio
> > > > >
> > > > > Cc: Vitaly Lubart
> > > > > Cc: Greg Kroah-Hartman
> > > > > Reviewed-by: Alan Previn
> > > > > ---
> > > > > drivers/misc/mei/pxp/mei_pxp.c | 13 ++---
> > > > > 1 file changed, 10 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/misc/mei/pxp/mei_pxp.c
> > > > > b/drivers/misc/mei/pxp/mei_pxp.c index 17c5d201603f..afc047627800
> > > > > 100644
> > > > > --- a/drivers/misc/mei/pxp/mei_pxp.c
> > > > > +++ b/drivers/misc/mei/pxp/mei_pxp.c
> > > > > @@ -159,17 +159,24 @@ static int mei_pxp_component_match(struct
> > > > device
> > > > > *dev, int subcomponent, {
> > > > > struct device *base = data;
> > > > >
> > > > > + if (!dev)
> > > > > + return 0;
> > > >
> > > > How can that happen?
> > > >
> > > > > +
> > > > > if (!dev->driver || strcmp(dev->driver->name, "i915") ||
> > > >
> > > > That's crazy to assume, but whatever :(
> > > Explained here:
> > > https://lore.kernel.org/all/20220418175932.1809770-2-
> > wonchung@google.c
> > > om/
> >
> > Still crazy :(
> >
> > >
> > > >
> > > > > subcomponent != I915_COMPONENT_PXP)
> > > > > return 0;
> > > > >
> > > > > base = base->parent;
> > > > > - if (!base)
> > > > > + if (!base) /* mei device */
> > > >
> > > > Why does this mean that?
> > > >
> > > > Where is that documented?
> > > >
> > > > > return 0;
> > > > >
> > > > > - base = base->parent;
> > > > > - dev = dev->parent;
> > > > > + base = base->parent; /* pci device */
> > > >
> > > > Again, why is this the case?
> > > >
> > > > > + /* for dgfx */
> > > > > + if (base && dev == base)
> > > > > + return 1;
> > > > >
> > > > > + /* for pch */
> > > > > + dev = dev->parent;
> > > >
> > > > You are digging through a random device tree and assuming that you
> > "know"
> > > > what the topology is going to be, that feels very very fragile and
> > > > ripe for problems going forward.
> > >
> > > I don't think it is random.
> >
> > Today it is one specific way, but how do you know this always will be this
> > way?
> >
> > > > How do you ensure that this really is they way the tree is for ALL
> > systems?
> > >
> > > Yes we take the topology assumption in PCI hierarchy.
> > > There is a case where both GFX and MEI are in PCH and you cannot stick
> > additional PCI extender or anything else there.
> > > And case where MEI is child on a standalone graphics card
Re: [Intel-gfx] [PATCH v4 06/15] mei: pxp: support matching with a gfx discrete card
On Fri, Sep 09, 2022 at 06:51:21AM +, Winkler, Tomas wrote:
>
>
> > -Original Message-
> > From: Greg Kroah-Hartman
> > Sent: Friday, September 09, 2022 09:16
> > To: Ceraolo Spurio, Daniele
> > Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
> > Winkler, Tomas ; Lubart, Vitaly
> > ; Teres Alexis, Alan Previn
> >
> > Subject: Re: [PATCH v4 06/15] mei: pxp: support matching with a gfx discrete
> > card
> >
> > On Thu, Sep 08, 2022 at 05:16:03PM -0700, Daniele Ceraolo Spurio wrote:
> > > From: Tomas Winkler
> > >
> > > With on-boards graphics card, both i915 and MEI are in the same device
> > > hierarchy with the same parent, while for discrete gfx card the MEI is
> > > its child device.
> > > Adjust the match function for that scenario by matching MEI parent
> > > device with i915.
> > >
> > > V2:
> > > 1. More detailed commit message
> > > 2. Check for dev is not null before it is accessed.
> > >
> > > Signed-off-by: Tomas Winkler
> > > Signed-off-by: Daniele Ceraolo Spurio
> > >
> > > Cc: Vitaly Lubart
> > > Cc: Greg Kroah-Hartman
> > > Reviewed-by: Alan Previn
> > > ---
> > > drivers/misc/mei/pxp/mei_pxp.c | 13 ++---
> > > 1 file changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/misc/mei/pxp/mei_pxp.c
> > > b/drivers/misc/mei/pxp/mei_pxp.c index 17c5d201603f..afc047627800
> > > 100644
> > > --- a/drivers/misc/mei/pxp/mei_pxp.c
> > > +++ b/drivers/misc/mei/pxp/mei_pxp.c
> > > @@ -159,17 +159,24 @@ static int mei_pxp_component_match(struct
> > device
> > > *dev, int subcomponent, {
> > > struct device *base = data;
> > >
> > > + if (!dev)
> > > + return 0;
> >
> > How can that happen?
> >
> > > +
> > > if (!dev->driver || strcmp(dev->driver->name, "i915") ||
> >
> > That's crazy to assume, but whatever :(
> Explained here:
> https://lore.kernel.org/all/20220418175932.1809770-2-wonch...@google.com/
Still crazy :(
>
> >
> > > subcomponent != I915_COMPONENT_PXP)
> > > return 0;
> > >
> > > base = base->parent;
> > > - if (!base)
> > > + if (!base) /* mei device */
> >
> > Why does this mean that?
> >
> > Where is that documented?
> >
> > > return 0;
> > >
> > > - base = base->parent;
> > > - dev = dev->parent;
> > > + base = base->parent; /* pci device */
> >
> > Again, why is this the case?
> >
> > > + /* for dgfx */
> > > + if (base && dev == base)
> > > + return 1;
> > >
> > > + /* for pch */
> > > + dev = dev->parent;
> >
> > You are digging through a random device tree and assuming that you "know"
> > what the topology is going to be, that feels very very fragile and ripe for
> > problems going forward.
>
> I don't think it is random.
Today it is one specific way, but how do you know this always will be
this way?
> > How do you ensure that this really is they way the tree is for ALL systems?
>
> Yes we take the topology assumption in PCI hierarchy.
> There is a case where both GFX and MEI are in PCH and you cannot stick
> additional PCI extender or anything else there.
> And case where MEI is child on a standalone graphics card this is set in
> software so topology is not going to change unless we rewrite
> everything. Be happy to hear your insights.
This is ripe to break in the future if someone makes a differently
structured device as there is nothing forcing the current layout to
always be this way by hardware designers.
The goal of the driver model is to NOT have these types of hard-coded
topology assumptions because, supprise, assumptions like this have
always come back to bite people in the end.
This is your driver, so that's fine, but really this feels very very
wrong and you will have a hard time guaranteeing that this will always
be this way for the next 20+ years of hardware designs. So why not do
it properly from the beginning and pass in the correct pointers to
different places?
There is a very good reason that the driver model/core does not make it
easy to determine what type of device a 'struct device *' is, because
you shouldn't have to rely on that type of thing ever. You are taking
it one step further and just assuming that you know what the type is
here, with no real way to ensure that this is the case.
In short, this feels like a very bad design as it is very fragile.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v4 05/15] mei: pxp: add command streamer API to the PXP driver
On Fri, Sep 09, 2022 at 06:38:33AM +, Winkler, Tomas wrote:
> >
> > On Thu, Sep 08, 2022 at 05:16:02PM -0700, Daniele Ceraolo Spurio wrote:
> > > +static ssize_t mei_pxp_gsc_command(struct device *dev, u8 client_id,
> > u32 fence_id,
> > > +struct scatterlist *sg_in, size_t
> > > total_in_len,
> > > +struct scatterlist *sg_out)
> > > +{
> > > + struct mei_cl_device *cldev;
> > > +
> > > + if (!dev || !sg_in || !sg_out)
> > > + return -EINVAL;
> >
> > How can these ever be NULL? Doesn't the core control this, so why would
> > that happen?
> This is any interface function between modules, I think it is not healthy to
> take assumptions here about how caller
> behaves, this is not an inner functions. This is important even for catching
> programmatical mistakes.
It is a static function being called from a framework. Enforce this in
the framework, don't sprinkle this stuff everywhere, the kernel is NOT
defensive about internal users like this otherwise it will overwhelm
every function call.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v3 02/15] mei: add support to GSC extended header
On Fri, Sep 09, 2022 at 06:21:10AM +, Winkler, Tomas wrote: > > > > > > > > On Fri, Aug 19, 2022 at 03:53:22PM -0700, Daniele Ceraolo Spurio wrote: > > > > > --- a/drivers/misc/mei/hw-me.c > > > > > +++ b/drivers/misc/mei/hw-me.c > > > > > @@ -590,7 +590,10 @@ static int mei_me_hbuf_write(struct > > > > > mei_device > > > > *dev, > > > > > u32 dw_cnt; > > > > > int empty_slots; > > > > > > > > > > - if (WARN_ON(!hdr || !data || hdr_len & 0x3)) > > > > > + if (WARN_ON(!hdr || hdr_len & 0x3)) > > > > > + return -EINVAL; > > > > > + > > > > > + if (WARN_ON(!data && data_len)) > > > > > > > > Do not add more WARN_ON() calls, please just handle this properly > > > > and do not reboot people's machines for a coding error :( > > > > > > As far as I understand WARN_ON() will produce solely a backtrace , > > > > Except when you have panic_on_warn() enabled in your systems, as many > > do :( > Was not aware, thanks for bringing to my attention. > > > > This particular condition should never ever happen in theory, > > > > Then don't check it! > In theory :) > > > anyhow we can use dev_err() here as well. > > > > That would be best. > That how it was resolved in the V4 series that Daniele has sent, will be > glad if you can review. Already done.
Re: [Intel-gfx] [PATCH v4 06/15] mei: pxp: support matching with a gfx discrete card
On Thu, Sep 08, 2022 at 05:16:03PM -0700, Daniele Ceraolo Spurio wrote:
> From: Tomas Winkler
>
> With on-boards graphics card, both i915 and MEI
> are in the same device hierarchy with the same parent,
> while for discrete gfx card the MEI is its child device.
> Adjust the match function for that scenario
> by matching MEI parent device with i915.
>
> V2:
> 1. More detailed commit message
> 2. Check for dev is not null before it is accessed.
>
> Signed-off-by: Tomas Winkler
> Signed-off-by: Daniele Ceraolo Spurio
> Cc: Vitaly Lubart
> Cc: Greg Kroah-Hartman
> Reviewed-by: Alan Previn
> ---
> drivers/misc/mei/pxp/mei_pxp.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/misc/mei/pxp/mei_pxp.c b/drivers/misc/mei/pxp/mei_pxp.c
> index 17c5d201603f..afc047627800 100644
> --- a/drivers/misc/mei/pxp/mei_pxp.c
> +++ b/drivers/misc/mei/pxp/mei_pxp.c
> @@ -159,17 +159,24 @@ static int mei_pxp_component_match(struct device *dev,
> int subcomponent,
> {
> struct device *base = data;
>
> + if (!dev)
> + return 0;
How can that happen?
> +
> if (!dev->driver || strcmp(dev->driver->name, "i915") ||
That's crazy to assume, but whatever :(
> subcomponent != I915_COMPONENT_PXP)
> return 0;
>
> base = base->parent;
> - if (!base)
> + if (!base) /* mei device */
Why does this mean that?
Where is that documented?
> return 0;
>
> - base = base->parent;
> - dev = dev->parent;
> + base = base->parent; /* pci device */
Again, why is this the case?
> + /* for dgfx */
> + if (base && dev == base)
> + return 1;
>
> + /* for pch */
> + dev = dev->parent;
You are digging through a random device tree and assuming that you
"know" what the topology is going to be, that feels very very fragile
and ripe for problems going forward.
How do you ensure that this really is they way the tree is for ALL
systems?
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v4 05/15] mei: pxp: add command streamer API to the PXP driver
On Thu, Sep 08, 2022 at 05:16:02PM -0700, Daniele Ceraolo Spurio wrote:
> +static ssize_t mei_pxp_gsc_command(struct device *dev, u8 client_id, u32
> fence_id,
> +struct scatterlist *sg_in, size_t
> total_in_len,
> +struct scatterlist *sg_out)
> +{
> + struct mei_cl_device *cldev;
> +
> + if (!dev || !sg_in || !sg_out)
> + return -EINVAL;
How can these ever be NULL? Doesn't the core control this, so why would
that happen?
Don't check for things that can never happen.
> +
> + cldev = to_mei_cl_device(dev);
> +
> + return mei_cldev_send_gsc_command(cldev, client_id, fence_id, sg_in,
> total_in_len, sg_out);
> +}
> +
> static const struct i915_pxp_component_ops mei_pxp_ops = {
> .owner = THIS_MODULE,
> .send = mei_pxp_send_message,
> .recv = mei_pxp_receive_message,
> + .gsc_command = mei_pxp_gsc_command,
> };
>
> static int mei_component_master_bind(struct device *dev)
> diff --git a/include/drm/i915_pxp_tee_interface.h
> b/include/drm/i915_pxp_tee_interface.h
> index af593ec64469..a702b6ec17f7 100644
> --- a/include/drm/i915_pxp_tee_interface.h
> +++ b/include/drm/i915_pxp_tee_interface.h
> @@ -8,6 +8,7 @@
>
> #include
> #include
> +struct scatterlist;
>
> /**
> * struct i915_pxp_component_ops - ops for PXP services.
> @@ -23,6 +24,10 @@ struct i915_pxp_component_ops {
>
> int (*send)(struct device *dev, const void *message, size_t size);
> int (*recv)(struct device *dev, void *buffer, size_t size);
> + ssize_t (*gsc_command)(struct device *dev, u8 client_id, u32 fence_id,
> +struct scatterlist *sg_in, size_t total_in_len,
> +struct scatterlist *sg_out);
No documentation for this new callback?
The build should give you are warning :(
Re: [Intel-gfx] [PATCH v4 03/15] mei: adjust extended header kdocs
On Thu, Sep 08, 2022 at 05:16:00PM -0700, Daniele Ceraolo Spurio wrote: > From: Tomas Winkler > > Fix kdoc for struct mei_ext_hdr and mei_ext_begin(). Shouldn't this go in the function that made the changes in the first place? > > V4: New in the series Again, below the --- line please. thanks, greg k-h
Re: [Intel-gfx] [PATCH v4 01/15] mei: add support to GSC extended header
On Thu, Sep 08, 2022 at 05:15:58PM -0700, Daniele Ceraolo Spurio wrote:
> From: Tomas Winkler
>
> GSC extend header is of variable size and data
> is provided in a sgl list inside the header
> and not in the data buffers, need to enable the path.
>
> V2:
> 1. Add missing kdoc for mei_cl_cb
> 2. In mei_me_hbuf_write()
>use dev_err() when validationg parameters instead of WARN_ON()
This stuff goes below the --- line, as the documentation states, right?
Why is it here?
>
> Signed-off-by: Tomas Winkler
> Signed-off-by: Daniele Ceraolo Spurio
> Cc: Vitaly Lubart
> Cc: Greg Kroah-Hartman
> ---
> drivers/misc/mei/client.c| 55 --
> drivers/misc/mei/hbm.c | 13
> drivers/misc/mei/hw-me.c | 7 -
> drivers/misc/mei/hw.h| 57
> drivers/misc/mei/interrupt.c | 47 -
> drivers/misc/mei/mei_dev.h | 4 +++
> 6 files changed, 160 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
> index 0b2fbe1335a7..6c8b71ae32c8 100644
> --- a/drivers/misc/mei/client.c
> +++ b/drivers/misc/mei/client.c
> @@ -322,6 +322,7 @@ void mei_io_cb_free(struct mei_cl_cb *cb)
>
> list_del(>list);
> kfree(cb->buf.data);
> + kfree(cb->ext_hdr);
> kfree(cb);
> }
>
> @@ -401,6 +402,7 @@ static struct mei_cl_cb *mei_io_cb_init(struct mei_cl *cl,
> cb->buf_idx = 0;
> cb->fop_type = type;
> cb->vtag = 0;
> + cb->ext_hdr = NULL;
>
> return cb;
> }
> @@ -1740,6 +1742,17 @@ static inline u8 mei_ext_hdr_set_vtag(void *ext, u8
> vtag)
> return vtag_hdr->hdr.length;
> }
>
> +static inline bool mei_ext_hdr_is_gsc(struct mei_ext_hdr *ext)
> +{
> + return ext && ext->type == MEI_EXT_HDR_GSC;
> +}
> +
> +static inline u8 mei_ext_hdr_set_gsc(struct mei_ext_hdr *ext, struct
> mei_ext_hdr *gsc_hdr)
> +{
> + memcpy(ext, gsc_hdr, mei_ext_hdr_len(gsc_hdr));
> + return ext->length;
> +}
> +
> /**
> * mei_msg_hdr_init - allocate and initialize mei message header
> *
> @@ -1752,14 +1765,17 @@ static struct mei_msg_hdr *mei_msg_hdr_init(const
> struct mei_cl_cb *cb)
> size_t hdr_len;
> struct mei_ext_meta_hdr *meta;
> struct mei_msg_hdr *mei_hdr;
> - bool is_ext, is_vtag;
> + bool is_ext, is_hbm, is_gsc, is_vtag;
> + struct mei_ext_hdr *next_ext;
>
> if (!cb)
> return ERR_PTR(-EINVAL);
>
> /* Extended header for vtag is attached only on the first fragment */
> is_vtag = (cb->vtag && cb->buf_idx == 0);
> - is_ext = is_vtag;
> + is_hbm = cb->cl->me_cl->client_id == 0;
> + is_gsc = ((!is_hbm) && cb->cl->dev->hbm_f_gsc_supported &&
> mei_ext_hdr_is_gsc(cb->ext_hdr));
> + is_ext = is_vtag || is_gsc;
>
> /* Compute extended header size */
> hdr_len = sizeof(*mei_hdr);
> @@ -1771,6 +1787,9 @@ static struct mei_msg_hdr *mei_msg_hdr_init(const
> struct mei_cl_cb *cb)
> if (is_vtag)
> hdr_len += sizeof(struct mei_ext_hdr_vtag);
>
> + if (is_gsc)
> + hdr_len += mei_ext_hdr_len(cb->ext_hdr);
> +
> setup_hdr:
> mei_hdr = kzalloc(hdr_len, GFP_KERNEL);
> if (!mei_hdr)
> @@ -1785,10 +1804,20 @@ static struct mei_msg_hdr *mei_msg_hdr_init(const
> struct mei_cl_cb *cb)
> goto out;
>
> meta = (struct mei_ext_meta_hdr *)mei_hdr->extension;
> + meta->size = 0;
> + next_ext = (struct mei_ext_hdr *)meta->hdrs;
> if (is_vtag) {
> meta->count++;
> - meta->size += mei_ext_hdr_set_vtag(meta->hdrs, cb->vtag);
> + meta->size += mei_ext_hdr_set_vtag(next_ext, cb->vtag);
> + next_ext = mei_ext_next(next_ext);
> + }
> +
> + if (is_gsc) {
> + meta->count++;
> + meta->size += mei_ext_hdr_set_gsc(next_ext, cb->ext_hdr);
> + next_ext = mei_ext_next(next_ext);
> }
> +
> out:
> mei_hdr->length = hdr_len - sizeof(*mei_hdr);
> return mei_hdr;
> @@ -1812,14 +1841,14 @@ int mei_cl_irq_write(struct mei_cl *cl, struct
> mei_cl_cb *cb,
> struct mei_msg_hdr *mei_hdr = NULL;
> size_t hdr_len;
> size_t hbuf_len, dr_len;
> - size_t buf_len;
> + size_t buf_len = 0;
> size_t data_len;
> int hbuf_slots;
> u32 dr_slots;
> u32 dma_len;
> int rets;
> bool first_ch
Re: [Intel-gfx] [PATCH v3 02/15] mei: add support to GSC extended header
On Thu, Sep 08, 2022 at 09:24:07PM +, Winkler, Tomas wrote: > > > > > On Fri, Aug 19, 2022 at 03:53:22PM -0700, Daniele Ceraolo Spurio wrote: > > > --- a/drivers/misc/mei/hw-me.c > > > +++ b/drivers/misc/mei/hw-me.c > > > @@ -590,7 +590,10 @@ static int mei_me_hbuf_write(struct mei_device > > *dev, > > > u32 dw_cnt; > > > int empty_slots; > > > > > > - if (WARN_ON(!hdr || !data || hdr_len & 0x3)) > > > + if (WARN_ON(!hdr || hdr_len & 0x3)) > > > + return -EINVAL; > > > + > > > + if (WARN_ON(!data && data_len)) > > > > Do not add more WARN_ON() calls, please just handle this properly and do > > not reboot people's machines for a coding error :( > > As far as I understand WARN_ON() will produce solely a backtrace , Except when you have panic_on_warn() enabled in your systems, as many do :( > This particular condition should never ever happen in theory, Then don't check it! > anyhow we can use dev_err() here as well. That would be best. thanks, greg k-h
Re: [Intel-gfx] [PATCH v7 00/15] GSC support for XeHP SDV and DG2
On Sat, Aug 06, 2022 at 03:26:21PM +0300, Tomas Winkler wrote: > Add GSC support for XeHP SDV and DG2 platforms. > > The series includes changes for the mei driver: > - add ability to use polling instead of interrupts > - add ability to use extended timeouts > - setup extended operational memory for GSC > > The series includes changes for the i915 driver: > - allocate extended operational memory for GSC > - GSC on XeHP SDV offsets and definitions > > This patch set should be merged via gfx tree as > the auxiliary device belongs there. > Greg, your ACK is required for the drives/misc/mei code base, > please review the patches. With the exception that you all don't know what year it is: Acked-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v7 10/15] mei: mkhi: add memory ready command
On Sat, Aug 06, 2022 at 03:26:31PM +0300, Tomas Winkler wrote: > Add GSC memory ready command. > The command indicates to the firmware that extend operation > memory was setup and the firmware may enter PXP mode. > > CC: Daniele Ceraolo Spurio > Signed-off-by: Tomas Winkler > Signed-off-by: Alexander Usyskin > --- > drivers/misc/mei/mkhi.h | 14 +- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/misc/mei/mkhi.h b/drivers/misc/mei/mkhi.h > index 27a9b476904e..056b76e73d40 100644 > --- a/drivers/misc/mei/mkhi.h > +++ b/drivers/misc/mei/mkhi.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > /* > - * Copyright (c) 2003-2020, Intel Corporation. All rights reserved. > + * Copyright (c) 2003-2021, Intel Corporation. All rights reserved. It is 2022 :(
Re: [Intel-gfx] [PATCH v3 02/15] mei: add support to GSC extended header
On Fri, Aug 19, 2022 at 03:53:22PM -0700, Daniele Ceraolo Spurio wrote: > --- a/drivers/misc/mei/hw-me.c > +++ b/drivers/misc/mei/hw-me.c > @@ -590,7 +590,10 @@ static int mei_me_hbuf_write(struct mei_device *dev, > u32 dw_cnt; > int empty_slots; > > - if (WARN_ON(!hdr || !data || hdr_len & 0x3)) > + if (WARN_ON(!hdr || hdr_len & 0x3)) > + return -EINVAL; > + > + if (WARN_ON(!data && data_len)) Do not add more WARN_ON() calls, please just handle this properly and do not reboot people's machines for a coding error :( thanks, greg k-h
Re: [Intel-gfx] [PATCH v2 00/15] drm/i915: HuC loading for DG2
On Thu, Aug 18, 2022 at 04:02:28PM -0700, Daniele Ceraolo Spurio wrote: > Note that this series includes several mei patches that add support for > sending the HuC loading command via mei-gsc. These patches depend on the > GSC support for DG2 [1], which has been included squashed in a single > patch to make the series apply and allow CI to run. We plan to merge > those patches through the drm tree because i915 is the sole user. Doesn't look like you cc:ed me on any of the mei patches, which is odd, and ensure I can't review them :( And why are mei changes needed only for drm? thanks, greg k-h
Re: [Intel-gfx] [PATCH v6 02/14] mei: add slow_fw flag to the mei auxiliary device
On Sun, Jul 24, 2022 at 11:24:16AM +0300, Tomas Winkler wrote:
> From: Alexander Usyskin
>
> Add slow_fw flag to the mei auxiliary device info
> to inform the mei driver about slow underlying firmware.
> Such firmware will require to use larger operation timeouts.
>
> Signed-off-by: Alexander Usyskin
> Signed-off-by: Tomas Winkler
> Reviewed-by: Daniele Ceraolo Spurio
> ---
> include/linux/mei_aux.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/mei_aux.h b/include/linux/mei_aux.h
> index 587f25128848..a29f4064b9c0 100644
> --- a/include/linux/mei_aux.h
> +++ b/include/linux/mei_aux.h
> @@ -11,6 +11,7 @@ struct mei_aux_device {
> struct auxiliary_device aux_dev;
> int irq;
> struct resource bar;
> + bool slow_fw;
Please spell this out "slow_firmware", and also document it somewhere.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v6 00/14] GSC support for XeHP SDV and DG2
On Sun, Jul 24, 2022 at 11:24:14AM +0300, Tomas Winkler wrote: > Add GSC support for XeHP SDV and DG2 platforms. > > The series includes changes for the mei driver: > - add ability to use polling instead of interrupts > - add ability to use extended timeouts > - setup extended operational memory for GSC > > The series includes changes for the i915 driver: > - allocate extended operational memory for GSC > - GSC on XeHP SDV offsets and definitions > > Greg KH, please review and ACK the MEI patches. > We are pushing these patches through gfx tree as > the auxiliary device belongs there. Please wait until after -rc1 is out, my trees are closed for new work at this point in time, and so should the gfx tree :) thanks, greg k-h
Re: [Intel-gfx] [PATCH v5 00/14] GSC support for XeHP SDV and DG2 platforms
On Wed, Jul 06, 2022 at 02:43:31PM +0300, Alexander Usyskin wrote: > Add GSC support for XeHP SDV and DG2 platforms. > > The series includes changes for the mei driver: > - add ability to use polling instead of interrupts > - add ability to use extended timeouts > - setup extended operational memory for GSC > > The series includes changes for the i915 driver: > - allocate extended operational memory for GSC > - GSC on XeHP SDV offsets and definitions > > Greg KH, please review and ACK the MEI patches. > We are pushing these patches through gfx tree as > the auxiliary device belongs there. > > V2: rebase over merged DG1 series and DG2 enablement patch, > fix commit messages > > V3: rebase over latest tip > > V4: add missed changelog in pxp dbugfs patch > > V5: rebase over latest tip > fix changelog in pxp dbugfs patch > put HAX patch last to the ease of merging You did more than just this from v4 to v5 :( It's as if you want to make it hard to review these... greg k-h
Re: [Intel-gfx] [PATCH v4 00/14] GSC support for XeHP SDV and DG2 platforms
On Wed, Jun 29, 2022 at 02:28:59PM +0300, Alexander Usyskin wrote: > Add GSC support for XeHP SDV and DG2 platforms. > > The series includes changes for the mei driver: > - add ability to use polling instead of interrupts > - add ability to use extended timeouts > - setup extended operational memory for GSC > > The series includes changes for the i915 driver: > - allocate extended operational memory for GSC > - GSC on XeHP SDV offsets and definitions > > Greg KH, please review and ACK the MEI patches. > (The patch 13 is one that you asked to change in prev version) > We are pushing these patches through gfx tree as > the auxiliary device belongs there. You forgot to review or sign off on any of these, you know better than that... Please run this through the Intel patch-review process before resending this out for non-Intel people to see, so you don't waste our time. thanks, greg k-h
Re: [Intel-gfx] [PATCH v4 13/14] mei: debugfs: add pxp mode to devstate in debugfs
On Wed, Jun 29, 2022 at 02:29:12PM +0300, Alexander Usyskin wrote: > From: Tomas Winkler > > Add pxp mode devstate to debugfs to monitor > pxp state machine progress. During debug > it is important to understand in what state > the pxp handshake is. > > CC: Vitaly Lubart > Signed-off-by: Tomas Winkler Also, you forgot to sign off on this, that's grounds for rejection alone...
Re: [Intel-gfx] [PATCH v4 13/14] mei: debugfs: add pxp mode to devstate in debugfs
On Wed, Jun 29, 2022 at 02:29:12PM +0300, Alexander Usyskin wrote:
> From: Tomas Winkler
>
> Add pxp mode devstate to debugfs to monitor
> pxp state machine progress. During debug
> it is important to understand in what state
> the pxp handshake is.
You have 72 columns for changelog text, as your editor will tell you
when you run git commit...
Anyway, it's better than nothing, but really, do you think this is a
good changelog description? That last sentence means nothing. I too
think it is important to understand things...
{sigh}
Anyway, you all are pushing this hard for some crazy reason, and it's
only a debugfs file, so it doesn't matter much, but it feels indicative
of other patches from your employer for the past few years so I'll be a
pain and ask for you to fix it up again.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v3 00/14] GSC support for XeHP SDV and DG2 platforms
On Sun, Jun 19, 2022 at 04:37:07PM +0300, Alexander Usyskin wrote: > Add GSC support for XeHP SDV and DG2 platforms. > > The series includes changes for the mei driver: > - add ability to use polling instead of interrupts > - add ability to use extended timeouts > - setup extended operational memory for GSC > > The series includes changes for the i915 driver: > - allocate extended operational memory for GSC > - GSC on XeHP SDV offsets and definitions > > Greg KH, please review and ACK the MEI patches. > We are pushing these patches through gfx tree as > the auxiliary device belongs there. patch 13 in the series obviously needs work, the others are fine with me. thanks, greg k-h
Re: [Intel-gfx] [PATCH v3 13/14] mei: debugfs: add pxp mode to devstate in debugfs
On Sun, Jun 19, 2022 at 04:37:20PM +0300, Alexander Usyskin wrote:
> From: Tomas Winkler
>
> CC: Vitaly Lubart
> Signed-off-by: Tomas Winkler
We can not take patches without any changelog text, you know this :(
> ---
> drivers/misc/mei/debugfs.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/misc/mei/debugfs.c b/drivers/misc/mei/debugfs.c
> index 1ce61e9e24fc..4074fec866a6 100644
> --- a/drivers/misc/mei/debugfs.c
> +++ b/drivers/misc/mei/debugfs.c
> @@ -86,6 +86,20 @@ static int mei_dbgfs_active_show(struct seq_file *m, void
> *unused)
> }
> DEFINE_SHOW_ATTRIBUTE(mei_dbgfs_active);
>
> +static const char *mei_dev_pxp_mode_str(enum mei_dev_pxp_mode state)
> +{
> +#define MEI_PXP_MODE(state) case MEI_DEV_PXP_##state: return #state
> + switch (state) {
> + MEI_PXP_MODE(DEFAULT);
> + MEI_PXP_MODE(INIT);
> + MEI_PXP_MODE(SETUP);
> + MEI_PXP_MODE(READY);
Just spell out the case and return lines, don't create macros for no
good reason please.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH 0/1] add support for enum module parameters
On Wed, Apr 20, 2022 at 08:13:47AM +0300, Kalle Valo wrote: > + linux-wireless, netdev > > Jani Nikula writes: > > > On Thu, 14 Apr 2022, Greg Kroah-Hartman wrote: > >> On Thu, Apr 14, 2022 at 03:30:32PM +0300, Jani Nikula wrote: > >>> Hey, I've sent this before, ages ago, but haven't really followed > >>> through with it. I still think it would be useful for many scenarios > >>> where a plain number is a clumsy interface for a module param. > >>> > >>> Thoughts? > >> > >> We should not be adding new module parameters anyway (they operate on > >> code, not data/devices), so what would this be used for? > > > > I think it's just easier to use names than random values, and this also > > gives you range check on the input. > > > > I also keep telling people not to add new module parameters, but it's > > not like they're going away anytime soon. > > > > If there's a solution to being able to pass device specific debug > > parameters at probe time, I'm all ears. At least i915 has a bunch of > > things which can't really be changed after probe, when debugfs for the > > device is around. Module parameters aren't ideal, but debugfs doesn't > > work for this. > > Wireless drivers would also desperately need to pass device specific > parameters at (or before) probe time. And not only debug parameters but > also configuration parameters, for example firmware memory allocations > schemes (optimise for features vs number of clients etc) and whatnot. > > Any ideas how to implement that? Is there any prior work for anything > like this? This is pretty hard limiting usability of upstream wireless > drivers and I really want to find a proper solution. Again, configfs? That should be what that subsystem was designed for... thanks, greg k-h
Re: [Intel-gfx] [PATCH 0/1] add support for enum module parameters
On Thu, Apr 14, 2022 at 05:22:47PM +0300, Jani Nikula wrote: > On Thu, 14 Apr 2022, Greg Kroah-Hartman wrote: > > On Thu, Apr 14, 2022 at 03:30:32PM +0300, Jani Nikula wrote: > >> Hey, I've sent this before, ages ago, but haven't really followed > >> through with it. I still think it would be useful for many scenarios > >> where a plain number is a clumsy interface for a module param. > >> > >> Thoughts? > > > > We should not be adding new module parameters anyway (they operate on > > code, not data/devices), so what would this be used for? > > I think it's just easier to use names than random values, and this also > gives you range check on the input. > > I also keep telling people not to add new module parameters, but it's > not like they're going away anytime soon. Existing ones can not go away (or change), but we do not have to add new ones. > If there's a solution to being able to pass device specific debug > parameters at probe time, I'm all ears. At least i915 has a bunch of > things which can't really be changed after probe, when debugfs for the > device is around. Module parameters aren't ideal, but debugfs doesn't > work for this. configfs?
Re: [Intel-gfx] [PATCH 0/1] add support for enum module parameters
On Thu, Apr 14, 2022 at 03:30:32PM +0300, Jani Nikula wrote: > Hey, I've sent this before, ages ago, but haven't really followed > through with it. I still think it would be useful for many scenarios > where a plain number is a clumsy interface for a module param. > > Thoughts? We should not be adding new module parameters anyway (they operate on code, not data/devices), so what would this be used for? thanks, greg k-h
[Intel-gfx] [PATCH 5.4 066/475] drm/edid: check basic audio support on CEA extension block
From: Cooper Chiou
commit 5662abf6e21338be6d085d6375d3732ac6147fd2 upstream.
Tag code stored in bit7:5 for CTA block byte[3] is not the same as
CEA extension block definition. Only check CEA block has
basic audio support.
v3: update commit message.
Cc: sta...@vger.kernel.org
Cc: Jani Nikula
Cc: Shawn C Lee
Cc: intel-gfx
Signed-off-by: Cooper Chiou
Signed-off-by: Lee Shawn C
Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
Reviewed-by: Jani Nikula
Signed-off-by: Jani Nikula
Link:
https://patchwork.freedesktop.org/patch/msgid/20220324061218.32739-1-shawn.c@intel.com
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_edid.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4380,7 +4380,8 @@ bool drm_detect_monitor_audio(struct edi
if (!edid_ext)
goto end;
- has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0);
+ has_audio = (edid_ext[0] == CEA_EXT &&
+ (edid_ext[3] & EDID_BASIC_AUDIO) != 0);
if (has_audio) {
DRM_DEBUG_KMS("Monitor has basic audio support\n");
[Intel-gfx] [PATCH 4.19 051/338] drm/edid: check basic audio support on CEA extension block
From: Cooper Chiou
commit 5662abf6e21338be6d085d6375d3732ac6147fd2 upstream.
Tag code stored in bit7:5 for CTA block byte[3] is not the same as
CEA extension block definition. Only check CEA block has
basic audio support.
v3: update commit message.
Cc: sta...@vger.kernel.org
Cc: Jani Nikula
Cc: Shawn C Lee
Cc: intel-gfx
Signed-off-by: Cooper Chiou
Signed-off-by: Lee Shawn C
Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
Reviewed-by: Jani Nikula
Signed-off-by: Jani Nikula
Link:
https://patchwork.freedesktop.org/patch/msgid/20220324061218.32739-1-shawn.c@intel.com
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_edid.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4323,7 +4323,8 @@ bool drm_detect_monitor_audio(struct edi
if (!edid_ext)
goto end;
- has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0);
+ has_audio = (edid_ext[0] == CEA_EXT &&
+ (edid_ext[3] & EDID_BASIC_AUDIO) != 0);
if (has_audio) {
DRM_DEBUG_KMS("Monitor has basic audio support\n");
[Intel-gfx] [PATCH 5.10 096/599] drm/edid: check basic audio support on CEA extension block
From: Cooper Chiou
commit 5662abf6e21338be6d085d6375d3732ac6147fd2 upstream.
Tag code stored in bit7:5 for CTA block byte[3] is not the same as
CEA extension block definition. Only check CEA block has
basic audio support.
v3: update commit message.
Cc: sta...@vger.kernel.org
Cc: Jani Nikula
Cc: Shawn C Lee
Cc: intel-gfx
Signed-off-by: Cooper Chiou
Signed-off-by: Lee Shawn C
Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
Reviewed-by: Jani Nikula
Signed-off-by: Jani Nikula
Link:
https://patchwork.freedesktop.org/patch/msgid/20220324061218.32739-1-shawn.c@intel.com
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_edid.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4806,7 +4806,8 @@ bool drm_detect_monitor_audio(struct edi
if (!edid_ext)
goto end;
- has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0);
+ has_audio = (edid_ext[0] == CEA_EXT &&
+ (edid_ext[3] & EDID_BASIC_AUDIO) != 0);
if (has_audio) {
DRM_DEBUG_KMS("Monitor has basic audio support\n");
[Intel-gfx] [PATCH 5.15 141/913] drm/edid: check basic audio support on CEA extension block
From: Cooper Chiou
commit 5662abf6e21338be6d085d6375d3732ac6147fd2 upstream.
Tag code stored in bit7:5 for CTA block byte[3] is not the same as
CEA extension block definition. Only check CEA block has
basic audio support.
v3: update commit message.
Cc: sta...@vger.kernel.org
Cc: Jani Nikula
Cc: Shawn C Lee
Cc: intel-gfx
Signed-off-by: Cooper Chiou
Signed-off-by: Lee Shawn C
Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
Reviewed-by: Jani Nikula
Signed-off-by: Jani Nikula
Link:
https://patchwork.freedesktop.org/patch/msgid/20220324061218.32739-1-shawn.c@intel.com
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_edid.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4776,7 +4776,8 @@ bool drm_detect_monitor_audio(struct edi
if (!edid_ext)
goto end;
- has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0);
+ has_audio = (edid_ext[0] == CEA_EXT &&
+ (edid_ext[3] & EDID_BASIC_AUDIO) != 0);
if (has_audio) {
DRM_DEBUG_KMS("Monitor has basic audio support\n");
[Intel-gfx] [PATCH 5.16 0149/1017] drm/edid: check basic audio support on CEA extension block
From: Cooper Chiou
commit 5662abf6e21338be6d085d6375d3732ac6147fd2 upstream.
Tag code stored in bit7:5 for CTA block byte[3] is not the same as
CEA extension block definition. Only check CEA block has
basic audio support.
v3: update commit message.
Cc: sta...@vger.kernel.org
Cc: Jani Nikula
Cc: Shawn C Lee
Cc: intel-gfx
Signed-off-by: Cooper Chiou
Signed-off-by: Lee Shawn C
Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
Reviewed-by: Jani Nikula
Signed-off-by: Jani Nikula
Link:
https://patchwork.freedesktop.org/patch/msgid/20220324061218.32739-1-shawn.c@intel.com
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_edid.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4848,7 +4848,8 @@ bool drm_detect_monitor_audio(struct edi
if (!edid_ext)
goto end;
- has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0);
+ has_audio = (edid_ext[0] == CEA_EXT &&
+ (edid_ext[3] & EDID_BASIC_AUDIO) != 0);
if (has_audio) {
DRM_DEBUG_KMS("Monitor has basic audio support\n");
[Intel-gfx] [PATCH 5.17 0144/1126] drm/edid: check basic audio support on CEA extension block
From: Cooper Chiou
commit 5662abf6e21338be6d085d6375d3732ac6147fd2 upstream.
Tag code stored in bit7:5 for CTA block byte[3] is not the same as
CEA extension block definition. Only check CEA block has
basic audio support.
v3: update commit message.
Cc: sta...@vger.kernel.org
Cc: Jani Nikula
Cc: Shawn C Lee
Cc: intel-gfx
Signed-off-by: Cooper Chiou
Signed-off-by: Lee Shawn C
Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
Reviewed-by: Jani Nikula
Signed-off-by: Jani Nikula
Link:
https://patchwork.freedesktop.org/patch/msgid/20220324061218.32739-1-shawn.c@intel.com
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_edid.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4848,7 +4848,8 @@ bool drm_detect_monitor_audio(struct edi
if (!edid_ext)
goto end;
- has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0);
+ has_audio = (edid_ext[0] == CEA_EXT &&
+ (edid_ext[3] & EDID_BASIC_AUDIO) != 0);
if (has_audio) {
DRM_DEBUG_KMS("Monitor has basic audio support\n");
Re: [Intel-gfx] [PATCH RFC 1/3] mm: Add f_ops->populate()
On Sun, Mar 06, 2022 at 07:32:05AM +0200, Jarkko Sakkinen wrote:
> Sometimes you might want to use MAP_POPULATE to ask a device driver to
> initialize the device memory in some specific manner. SGX driver can use
> this to request more memory by issuing ENCLS[EAUG] x86 opcode for each
> page in the address range.
>
> Add f_ops->populate() with the same parameters as f_ops->mmap() and make
> it conditionally called inside call_mmap(). Update call sites
> accodingly.
> ---
> Signed-off-by: Jarkko Sakkinen
> v3:
> - if (!ret && do_populate && file->f_op->populate)
> + if (!ret && do_populate && file->f_op->populate &&
> + !!(vma->vm_flags & (VM_IO | VM_PFNMAP)))
> (reported by Matthew Wilcox)
> v2:
> - if (!ret && do_populate)
> + if (!ret && do_populate && file->f_op->populate)
> (reported by Jan Harkes)
> ---
> arch/mips/kernel/vdso.c| 2 +-
> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +-
> fs/coda/file.c | 2 +-
> fs/overlayfs/file.c| 2 +-
> include/linux/fs.h | 12 ++--
> include/linux/mm.h | 2 +-
> ipc/shm.c | 2 +-
> mm/mmap.c | 10 +-
> mm/nommu.c | 4 ++--
> 9 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/arch/mips/kernel/vdso.c b/arch/mips/kernel/vdso.c
> index 3d0cf471f2fe..89f3f3da9abd 100644
> --- a/arch/mips/kernel/vdso.c
> +++ b/arch/mips/kernel/vdso.c
> @@ -102,7 +102,7 @@ int arch_setup_additional_pages(struct linux_binprm
> *bprm, int uses_interp)
> base = mmap_region(NULL, STACK_TOP, PAGE_SIZE,
> VM_READ | VM_EXEC |
> VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC,
> - 0, NULL);
> + 0, NULL, false);
> if (IS_ERR_VALUE(base)) {
> ret = base;
> goto out;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> index 1b526039a60d..4c71f64d6a79 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> @@ -107,7 +107,7 @@ static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf,
> struct vm_area_struct *
> if (!obj->base.filp)
> return -ENODEV;
>
> - ret = call_mmap(obj->base.filp, vma);
> + ret = call_mmap(obj->base.filp, vma, false);
> if (ret)
> return ret;
>
> diff --git a/fs/coda/file.c b/fs/coda/file.c
> index 29dd87be2fb8..e14f312fdbf8 100644
> --- a/fs/coda/file.c
> +++ b/fs/coda/file.c
> @@ -173,7 +173,7 @@ coda_file_mmap(struct file *coda_file, struct
> vm_area_struct *vma)
> spin_unlock(>c_lock);
>
> vma->vm_file = get_file(host_file);
> - ret = call_mmap(vma->vm_file, vma);
> + ret = call_mmap(vma->vm_file, vma, false);
>
> if (ret) {
> /* if call_mmap fails, our caller will put host_file so we
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index fa125feed0ff..b963a9397e80 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -503,7 +503,7 @@ static int ovl_mmap(struct file *file, struct
> vm_area_struct *vma)
> vma_set_file(vma, realfile);
>
> old_cred = ovl_override_creds(file_inode(file)->i_sb);
> - ret = call_mmap(vma->vm_file, vma);
> + ret = call_mmap(vma->vm_file, vma, false);
> revert_creds(old_cred);
> ovl_file_accessed(file);
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e2d892b201b0..2909e2d14af8 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -42,6 +42,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -1993,6 +1994,7 @@ struct file_operations {
> long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> int (*mmap) (struct file *, struct vm_area_struct *);
> + int (*populate)(struct file *, struct vm_area_struct *);
> unsigned long mmap_supported_flags;
> int (*open) (struct inode *, struct file *);
> int (*flush) (struct file *, fl_owner_t id);
> @@ -2074,9 +2076,15 @@ static inline ssize_t call_write_iter(struct file
> *file, struct kiocb *kio,
> return file->f_op->write_iter(kio, iter);
> }
>
> -static inline int call_mmap(struct file *file, struct vm_area_struct *vma)
> +static inline int call_mmap(struct file *file, struct vm_area_struct *vma,
> bool do_populate)
> {
> - return file->f_op->mmap(file, vma);
> + int ret = file->f_op->mmap(file, vma);
> +
> + if (!ret && do_populate && file->f_op->populate &&
> + !!(vma->vm_flags & (VM_IO | VM_PFNMAP)))
> + ret = file->f_op->populate(file,
Re: [Intel-gfx] [PATCH v6 0/5] Add driver for GSC controller
On Sun, Feb 13, 2022 at 11:14:53AM +0200, Alexander Usyskin wrote: > GSC is a graphics system controller, it provides > a chassis controller for graphics discrete cards. > > There are two MEI interfaces in GSC: HECI1 and HECI2. > > This series includes instantiation of the auxiliary devices for HECI2 > and mei-gsc auxiliary device driver that binds to the auxiliary device. > > In v2 the platform device was replaced by the auxiliary device. > v3 is the rebase over drm-tip to make public CI running. > In v4 the not needed debug prints and empty line were removed, > 'select' were replaced by 'depends on' in MEI Kconfig, > the new include file now listed in the MAINTATINERS file. > V5, rebase and add Greg KH Reviewed-by > V6, rebase and drop redundant assignments found by the kernel test robot. > > Reviewed-by: Greg Kroah-Hartman This line needs to be added to the individual patches if you want any tool or maintainer to pick that up. thanks, greg k-h
Re: [Intel-gfx] [PATCH 1/2] drm: Add HPD state to drm_connector_oob_hotplug_event()
On Mon, Feb 07, 2022 at 08:43:27PM -0800, Bjorn Andersson wrote:
> In some implementations, such as the Qualcomm platforms, the display
> driver has no way to query the current HPD state and as such it's
> impossible to distinguish between disconnect and attention events.
>
> Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD
> state.
>
> Also push the test for unchanged state in the displayport altmode driver
> into the i915 driver, to allow other drivers to act upon each update.
>
> Signed-off-by: Bjorn Andersson
> ---
>
> Note that the Intel driver has only been compile tested with this patch.
>
> drivers/gpu/drm/drm_connector.c | 6 --
> drivers/gpu/drm/i915/display/intel_dp.c | 14 +++---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/usb/typec/altmodes/displayport.c | 9 ++---
> include/drm/drm_connector.h | 5 +++--
> 5 files changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index a50c82bc2b2f..ad7295597c0f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2825,6 +2825,7 @@ struct drm_connector
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
> /**
> * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to
> connector
> * @connector_fwnode: fwnode_handle to report the event on
> + * @hpd_state: number of data lanes available
"number"?
> *
> * On some hardware a hotplug event notification may come from outside the
> display
> * driver / device. An example of this is some USB Type-C setups where the
> hardware
> @@ -2834,7 +2835,8 @@ struct drm_connector
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
> * This function can be used to report these out-of-band events after
> obtaining
> * a drm_connector reference through calling drm_connector_find_by_fwnode().
> */
> -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
> + bool hpd_state)
This is a boolean, how can it be a number?
And having a "flag" like this is a pain, how do you know what the
parameter really means?
> {
> struct drm_connector *connector;
>
> @@ -2843,7 +2845,7 @@ void drm_connector_oob_hotplug_event(struct
> fwnode_handle *connector_fwnode)
> return;
>
> if (connector->funcs->oob_hotplug_event)
> - connector->funcs->oob_hotplug_event(connector);
> + connector->funcs->oob_hotplug_event(connector, hpd_state);
>
> drm_connector_put(connector);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 146b83916005..00520867d37b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4816,15 +4816,23 @@ static int intel_dp_connector_atomic_check(struct
> drm_connector *conn,
> return intel_modeset_synced_crtcs(state, conn);
> }
>
> -static void intel_dp_oob_hotplug_event(struct drm_connector *connector)
> +static void intel_dp_oob_hotplug_event(struct drm_connector *connector, bool
> hpd_state)
> {
> struct intel_encoder *encoder =
> intel_attached_encoder(to_intel_connector(connector));
> struct drm_i915_private *i915 = to_i915(connector->dev);
> + bool need_work = false;
>
> spin_lock_irq(>irq_lock);
> - i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
> + if (hpd_state != i915->hotplug.oob_hotplug_state) {
> + i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
> +
> + i915->hotplug.oob_hotplug_state = hpd_state;
> + need_work = true;
> + }
> spin_unlock_irq(>irq_lock);
> - queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
> +
> + if (need_work)
> + queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
> }
>
> static const struct drm_connector_funcs intel_dp_connector_funcs = {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8c1706fd81f9..543ebf1cfcf4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -149,6 +149,9 @@ struct i915_hotplug {
> /* Whether or not to count short HPD IRQs in HPD storms */
> u8 hpd_short_storm_enabled;
>
> + /* Last state reported by oob_hotplug_event */
> + bool oob_hotplug_state;
> +
> /*
>* if we get a HPD irq from DP and a HPD irq from non-DP
>* the non-DP HPD could block the workqueue on a mode config
> diff --git a/drivers/usb/typec/altmodes/displayport.c
> b/drivers/usb/typec/altmodes/displayport.c
> index c1d8c23baa39..a4596be4d34a 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -59,7 +59,6 @@ struct dp_altmode {
>
Re: [Intel-gfx] [PATCH 2/2] drm/msm/dp: Implement oob_hotplug_event()
On Mon, Feb 07, 2022 at 08:43:28PM -0800, Bjorn Andersson wrote:
> The Qualcomm DisplayPort driver contains traces of the necessary
> plumbing to hook up USB HPD, in the form of the dp_hpd module and the
> dp_usbpd_cb struct. Use this as basis for implementing the
> oob_hotplug_event() callback, by amending the dp_hpd module with the
> missing logic.
>
> Overall the solution is similar to what's done downstream, but upstream
> all the code to disect the HPD notification lives on the calling side of
> drm_connector_oob_hotplug_event().
>
> drm_connector_oob_hotplug_event() performs the lookup of the
> drm_connector based on fwnode, hence the need to assign the fwnode in
> dp_drm_connector_init().
>
> Signed-off-by: Bjorn Andersson
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 8
> drivers/gpu/drm/msm/dp/dp_display.h | 2 ++
> drivers/gpu/drm/msm/dp/dp_drm.c | 10 ++
> drivers/gpu/drm/msm/dp/dp_hpd.c | 19 +++
> drivers/gpu/drm/msm/dp/dp_hpd.h | 4
> 5 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 7cc4d21f2091..124a2f794382 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -414,6 +414,13 @@ static int dp_display_usbpd_configure_cb(struct device
> *dev)
> return dp_display_process_hpd_high(dp);
> }
>
> +void dp_display_oob_hotplug_event(struct msm_dp *dp_display, bool hpd_state)
> +{
> + struct dp_display_private *dp = container_of(dp_display, struct
> dp_display_private, dp_display);
> +
> + dp->usbpd->oob_event(dp->usbpd, hpd_state);
> +}
> +
> static int dp_display_usbpd_disconnect_cb(struct device *dev)
> {
> struct dp_display_private *dp = dev_get_dp_display_private(dev);
> @@ -1251,6 +1258,7 @@ static int dp_display_probe(struct platform_device
> *pdev)
> dp->pdev = pdev;
> dp->name = "drm_dp";
> dp->dp_display.connector_type = desc->connector_type;
> + dp->dp_display.dev = >dev;
You did not properly reference count this pointer you just saved. What
is to keep that pointer from going away without you knowing about it?
And you already have a pointer to pdev, why save another one here?
>
> rc = dp_init_sub_modules(dp);
> if (rc) {
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
> b/drivers/gpu/drm/msm/dp/dp_display.h
> index e3adcd578a90..1f856b3bca79 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -11,6 +11,7 @@
> #include "disp/msm_disp_snapshot.h"
>
> struct msm_dp {
> + struct device *dev;
> struct drm_device *drm_dev;
> struct device *codec_dev;
So you now have pointers to 3 different devices here? What does 'dev'
point to that the other ones do not? This needs to be documented really
well here.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v4 0/5] Add driver for GSC controller
On Thu, Jan 27, 2022 at 01:20:58PM +0200, Alexander Usyskin wrote: > GSC is a graphics system controller, it provides > a chassis controller for graphics discrete cards. > > There are two MEI interfaces in GSC: HECI1 and HECI2. > > This series includes instantiation of the auxiliary devices for HECI2 > and mei-gsc auxiliary device driver that binds to the auxiliary device. > > In v2 the platform device was replaced by the auxiliary device. > v3 is the rebase over drm-tip to make public CI running. > In v4 the not needed debug prints and empty line were removed, > 'select' were replaced by 'depends on' in MEI Kconfig, > the new include file now listed in the MAINTATINERS file. > > Greg KH, please review and ACK the MEI patches after main merge window > closure. > We are pushing all through gfx tree as the auxiliary device belongs there. Reviewed-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH 01/21] MAINTAINERS: Add entry for fbdev core
On Tue, Feb 01, 2022 at 11:19:54AM +0100, Thomas Zimmermann wrote: > > > Am 31.01.22 um 22:05 schrieb Daniel Vetter: > > Ever since Tomi extracted the core code in 2014 it's been defacto me > > maintaining this, with help from others from dri-devel and sometimes > > Linus (but those are mostly merge conflicts): > > > > $ git shortlog -ns drivers/video/fbdev/core/ | head -n5 > > 35 Daniel Vetter > > 23 Linus Torvalds > > 10 Hans de Goede > > 9 Dave Airlie > > 6 Peter Rosin > > > > I think ideally we'd also record that the various firmware fb drivers > > (efifb, vesafb, ...) are also maintained in drm-misc because for the > > past few years the patches have either been to fix handover issues > > with drm drivers, or caused handover issues with drm drivers. So any > > other tree just doesn't make sense. But also, there's plenty of > > outdated MAINTAINER entries for these with people and git trees that > > haven't been active in years, so maybe let's just leave them alone. > > And furthermore distros are now adopting simpledrm as the firmware fb > > driver, so hopefully the need to care about the fbdev firmware drivers > > will go down going forward. > > > > Note that drm-misc is group maintained, I expect that to continue like > > we've done before, so no new expectations that patches all go through > > my hands. That would be silly. This also means I'm happy to put any > > other volunteer's name in the M: line, but otherwise git log says I'm > > the one who's stuck with this. > > > > Cc: Dave Airlie > > Cc: Jani Nikula > > Cc: Linus Torvalds > > Cc: Linux Fbdev development list > > Cc: Pavel Machek > > Cc: Sam Ravnborg > > Cc: Greg Kroah-Hartman > > Cc: Javier Martinez Canillas > > Cc: DRI Development > > Cc: Linux Kernel Mailing List > > Cc: Claudio Suarez > > Cc: Tomi Valkeinen > > Cc: Geert Uytterhoeven > > Cc: Thomas Zimmermann > > Cc: Daniel Vetter > > Cc: Sven Schnelle > > Cc: Gerd Hoffmann > > Signed-off-by: Daniel Vetter > > Acked-by: Thomas Zimmermann Acked-by: Greg Kroah-Hartman
Re: [Intel-gfx] [PATCH v4 0/5] Add driver for GSC controller
On Thu, Jan 27, 2022 at 12:31:07PM +0100, Greg Kroah-Hartman wrote: > On Thu, Jan 27, 2022 at 01:20:58PM +0200, Alexander Usyskin wrote: > > GSC is a graphics system controller, it provides > > a chassis controller for graphics discrete cards. > > > > There are two MEI interfaces in GSC: HECI1 and HECI2. > > > > This series includes instantiation of the auxiliary devices for HECI2 > > and mei-gsc auxiliary device driver that binds to the auxiliary device. > > > > In v2 the platform device was replaced by the auxiliary device. > > v3 is the rebase over drm-tip to make public CI running. > > In v4 the not needed debug prints and empty line were removed, > > 'select' were replaced by 'depends on' in MEI Kconfig, > > the new include file now listed in the MAINTATINERS file. > > > > Greg KH, please review and ACK the MEI patches after main merge window > > closure. > > The merge window already closed, did you forget to update your email? > > > We are pushing all through gfx tree as the auxiliary device belongs there. > > I'll get to this in a week or so, please be patient. In the meantime, while you are waiting, please review other patches on the mailing lists to help with maintainer's workloads. greg k-h
Re: [Intel-gfx] [PATCH v4 0/5] Add driver for GSC controller
On Thu, Jan 27, 2022 at 01:20:58PM +0200, Alexander Usyskin wrote: > GSC is a graphics system controller, it provides > a chassis controller for graphics discrete cards. > > There are two MEI interfaces in GSC: HECI1 and HECI2. > > This series includes instantiation of the auxiliary devices for HECI2 > and mei-gsc auxiliary device driver that binds to the auxiliary device. > > In v2 the platform device was replaced by the auxiliary device. > v3 is the rebase over drm-tip to make public CI running. > In v4 the not needed debug prints and empty line were removed, > 'select' were replaced by 'depends on' in MEI Kconfig, > the new include file now listed in the MAINTATINERS file. > > Greg KH, please review and ACK the MEI patches after main merge window > closure. The merge window already closed, did you forget to update your email? > We are pushing all through gfx tree as the auxiliary device belongs there. I'll get to this in a week or so, please be patient. greg k-h
Re: [Intel-gfx] [PATCH v2 4/5] mei: gsc: add runtime pm handlers
On Wed, Jan 19, 2022 at 05:58:06PM +0200, Alexander Usyskin wrote:
> From: Tomas Winkler
>
> Implement runtime handlers for mei-gsc, to track
> idle state of the device properly.
>
> CC: Rodrigo Vivi
> Signed-off-by: Tomas Winkler
> Signed-off-by: Alexander Usyskin
> ---
> drivers/misc/mei/gsc-me.c | 80 ++-
> 1 file changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/mei/gsc-me.c b/drivers/misc/mei/gsc-me.c
> index f58e54d2c1fc..fddae8009b62 100644
> --- a/drivers/misc/mei/gsc-me.c
> +++ b/drivers/misc/mei/gsc-me.c
> @@ -158,7 +158,85 @@ static int __maybe_unused mei_gsc_pm_resume(struct
> device *device)
> return 0;
> }
>
> -static SIMPLE_DEV_PM_OPS(mei_gsc_pm_ops, mei_gsc_pm_suspend,
> mei_gsc_pm_resume);
> +static int __maybe_unused mei_gsc_pm_runtime_idle(struct device *device)
> +{
> + struct mei_device *dev;
> +
> + dev_dbg(device, "rpm: me: runtime_idle\n");
> +
> + dev = dev_get_drvdata(device);
> + if (!dev)
> + return -ENODEV;
> + if (mei_write_is_idle(dev))
> + pm_runtime_autosuspend(device);
> +
> + return -EBUSY;
> +}
> +
> +static int __maybe_unused mei_gsc_pm_runtime_suspend(struct device *device)
> +{
> + struct mei_device *dev;
> + struct mei_me_hw *hw;
> + int ret;
> +
> + dev_dbg(device, "rpm: me: runtime suspend\n");
No need for debugging code to remain, use ftrace if you really need it.
Same for other dev_dbg() calls in this patch, they can all be removed.
thanks,
greg k-h
Re: [Intel-gfx] [PATCH v2 2/5] mei: add support for graphics system controller (gsc) devices
On Wed, Jan 19, 2022 at 05:58:04PM +0200, Alexander Usyskin wrote: > From: Tomas Winkler > > GSC is a graphics system controller, based on CSE, it provides > a chassis controller for graphics discrete cards, as well as it > supports media protection on selected devices. > > mei_gsc binds to a auxiliary devices exposed by Intel discrete > driver i915. > > Signed-off-by: Alexander Usyskin > Signed-off-by: Tomas Winkler > --- > drivers/misc/mei/Kconfig | 14 +++ > drivers/misc/mei/Makefile | 3 + > drivers/misc/mei/gsc-me.c | 192 ++ > drivers/misc/mei/hw-me.c | 27 +- > drivers/misc/mei/hw-me.h | 2 + > 5 files changed, 236 insertions(+), 2 deletions(-) > create mode 100644 drivers/misc/mei/gsc-me.c > > diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig > index 0e0bcd0da852..ec119bb98251 100644 > --- a/drivers/misc/mei/Kconfig > +++ b/drivers/misc/mei/Kconfig > @@ -46,6 +46,20 @@ config INTEL_MEI_TXE > Supported SoCs: > Intel Bay Trail > > +config INTEL_MEI_GSC > + tristate "Intel MEI GSC embedded device" > + select INTEL_MEI > + select INTEL_MEI_ME Please don't select, why not just depend on? thanks, greg k-h
