| Message ID | 20230308133624.2131582-1-andrzej.hajda@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/gt: prevent forcewake releases during BAR resize | expand |
On 3/8/2023 2:36 PM, Andrzej Hajda wrote: > Tests on DG2 machines show that releasing forcewakes during BAR resize > results later in forcewake ack timeouts. Do we have a fdo/issues url for that? Having that as References would be nice. > Since forcewakes can be realeased > asynchronously the simplest way to prevent it is to get all forcewakes > for duration of BAR resizing. > > v2: hold rpm as well during resizing (Rodrigo) > > Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> Acked-by: Nirmoy Das <nirmoy.das@intel.com> > --- > Please ignore resend of v1, my mistake. > > Regards > Andrzej > --- > drivers/gpu/drm/i915/gt/intel_region_lmem.c | 25 +++++++++++++++------ > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c > index 89fdfc67f8d1e0..2a3217e2890fc7 100644 > --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c > +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c > @@ -54,6 +54,7 @@ static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t > struct resource *root_res; > resource_size_t rebar_size; > resource_size_t current_size; > + intel_wakeref_t wakeref; > u32 pci_cmd; > int i; > > @@ -102,15 +103,25 @@ static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t > return; > } > > - /* First disable PCI memory decoding references */ > - pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); > - pci_write_config_dword(pdev, PCI_COMMAND, > - pci_cmd & ~PCI_COMMAND_MEMORY); > + /* > + * Releasing forcewake during BAR resizing results in later forcewake > + * ack timeouts and former can happen any time - it is asynchronous. > + * Grabbing all forcewakes prevents it. > + */ > + with_intel_runtime_pm(i915->uncore.rpm, wakeref) { > + intel_uncore_forcewake_get(&i915->uncore, FORCEWAKE_ALL); > > - _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); > + /* First disable PCI memory decoding references */ > + pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); > + pci_write_config_dword(pdev, PCI_COMMAND, > + pci_cmd & ~PCI_COMMAND_MEMORY); > > - pci_assign_unassigned_bus_resources(pdev->bus); > - pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); > + _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); > + > + pci_assign_unassigned_bus_resources(pdev->bus); > + pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); > + intel_uncore_forcewake_put(&i915->uncore, FORCEWAKE_ALL); > + } > } > #else > static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t lmem_size) {}
On Wed, Mar 08, 2023 at 02:36:24PM +0100, Andrzej Hajda wrote: > Tests on DG2 machines show that releasing forcewakes during BAR resize > results later in forcewake ack timeouts. Since forcewakes can be realeased > asynchronously the simplest way to prevent it is to get all forcewakes > for duration of BAR resizing. > > v2: hold rpm as well during resizing (Rodrigo) oh, I really dislike this with_rpm... a get and put directly would end up with a much clear patch... :/ but anyway: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> > --- > Please ignore resend of v1, my mistake. > > Regards > Andrzej > --- > drivers/gpu/drm/i915/gt/intel_region_lmem.c | 25 +++++++++++++++------ > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c > index 89fdfc67f8d1e0..2a3217e2890fc7 100644 > --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c > +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c > @@ -54,6 +54,7 @@ static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t > struct resource *root_res; > resource_size_t rebar_size; > resource_size_t current_size; > + intel_wakeref_t wakeref; > u32 pci_cmd; > int i; > > @@ -102,15 +103,25 @@ static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t > return; > } > > - /* First disable PCI memory decoding references */ > - pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); > - pci_write_config_dword(pdev, PCI_COMMAND, > - pci_cmd & ~PCI_COMMAND_MEMORY); > + /* > + * Releasing forcewake during BAR resizing results in later forcewake > + * ack timeouts and former can happen any time - it is asynchronous. > + * Grabbing all forcewakes prevents it. > + */ > + with_intel_runtime_pm(i915->uncore.rpm, wakeref) { > + intel_uncore_forcewake_get(&i915->uncore, FORCEWAKE_ALL); > > - _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); > + /* First disable PCI memory decoding references */ > + pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); > + pci_write_config_dword(pdev, PCI_COMMAND, > + pci_cmd & ~PCI_COMMAND_MEMORY); > > - pci_assign_unassigned_bus_resources(pdev->bus); > - pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); > + _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); > + > + pci_assign_unassigned_bus_resources(pdev->bus); > + pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); > + intel_uncore_forcewake_put(&i915->uncore, FORCEWAKE_ALL); > + } > } > #else > static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t lmem_size) {} > -- > 2.34.1 >
Hi Andrzej, On Wed, Mar 08, 2023 at 02:36:24PM +0100, Andrzej Hajda wrote: > Tests on DG2 machines show that releasing forcewakes during BAR resize > results later in forcewake ack timeouts. Since forcewakes can be realeased > asynchronously the simplest way to prevent it is to get all forcewakes > for duration of BAR resizing. > > v2: hold rpm as well during resizing (Rodrigo) > > Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Thanks, Andi
On 08.03.2023 18:29, Das, Nirmoy wrote: > > On 3/8/2023 2:36 PM, Andrzej Hajda wrote: >> Tests on DG2 machines show that releasing forcewakes during BAR resize >> results later in forcewake ack timeouts. > Do we have a fdo/issues url for that? Having that as References would be > nice. The patch is result of issue reported internally, but...grepping bugtracker I've found potential candidates: Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6530 Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7853 Maybe it would be good to add them on merge. Anyway thx for a-b. Regards Andrzej >> Since forcewakes can be realeased >> asynchronously the simplest way to prevent it is to get all forcewakes >> for duration of BAR resizing. >> >> v2: hold rpm as well during resizing (Rodrigo) >> >> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> > > > Acked-by: Nirmoy Das <nirmoy.das@intel.com> > > >> --- >> Please ignore resend of v1, my mistake. >> >> Regards >> Andrzej >> --- >> drivers/gpu/drm/i915/gt/intel_region_lmem.c | 25 +++++++++++++++------ >> 1 file changed, 18 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c >> b/drivers/gpu/drm/i915/gt/intel_region_lmem.c >> index 89fdfc67f8d1e0..2a3217e2890fc7 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c >> +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c >> @@ -54,6 +54,7 @@ static void i915_resize_lmem_bar(struct >> drm_i915_private *i915, resource_size_t >> struct resource *root_res; >> resource_size_t rebar_size; >> resource_size_t current_size; >> + intel_wakeref_t wakeref; >> u32 pci_cmd; >> int i; >> @@ -102,15 +103,25 @@ static void i915_resize_lmem_bar(struct >> drm_i915_private *i915, resource_size_t >> return; >> } >> - /* First disable PCI memory decoding references */ >> - pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); >> - pci_write_config_dword(pdev, PCI_COMMAND, >> - pci_cmd & ~PCI_COMMAND_MEMORY); >> + /* >> + * Releasing forcewake during BAR resizing results in later >> forcewake >> + * ack timeouts and former can happen any time - it is asynchronous. >> + * Grabbing all forcewakes prevents it. >> + */ >> + with_intel_runtime_pm(i915->uncore.rpm, wakeref) { >> + intel_uncore_forcewake_get(&i915->uncore, FORCEWAKE_ALL); >> - _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); >> + /* First disable PCI memory decoding references */ >> + pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); >> + pci_write_config_dword(pdev, PCI_COMMAND, >> + pci_cmd & ~PCI_COMMAND_MEMORY); >> - pci_assign_unassigned_bus_resources(pdev->bus); >> - pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); >> + _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); >> + >> + pci_assign_unassigned_bus_resources(pdev->bus); >> + pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); >> + intel_uncore_forcewake_put(&i915->uncore, FORCEWAKE_ALL); >> + } >> } >> #else >> static void i915_resize_lmem_bar(struct drm_i915_private *i915, >> resource_size_t lmem_size) {}
On 3/9/2023 8:54 AM, Andrzej Hajda wrote: > On 08.03.2023 18:29, Das, Nirmoy wrote: >> >> On 3/8/2023 2:36 PM, Andrzej Hajda wrote: >>> Tests on DG2 machines show that releasing forcewakes during BAR resize >>> results later in forcewake ack timeouts. >> Do we have a fdo/issues url for that? Having that as References would >> be nice. > > The patch is result of issue reported internally, but...grepping > bugtracker I've found potential candidates: > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6530 > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7853 > > Maybe it would be good to add them on merge. Works for me. Thanks, Nirmoy > Anyway thx for a-b. > > Regards > Andrzej > >>> Since forcewakes can be realeased >>> asynchronously the simplest way to prevent it is to get all forcewakes >>> for duration of BAR resizing. >>> >>> v2: hold rpm as well during resizing (Rodrigo) >>> >>> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> >> >> >> Acked-by: Nirmoy Das <nirmoy.das@intel.com> >> >> >>> --- >>> Please ignore resend of v1, my mistake. >>> >>> Regards >>> Andrzej >>> --- >>> drivers/gpu/drm/i915/gt/intel_region_lmem.c | 25 >>> +++++++++++++++------ >>> 1 file changed, 18 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c >>> b/drivers/gpu/drm/i915/gt/intel_region_lmem.c >>> index 89fdfc67f8d1e0..2a3217e2890fc7 100644 >>> --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c >>> +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c >>> @@ -54,6 +54,7 @@ static void i915_resize_lmem_bar(struct >>> drm_i915_private *i915, resource_size_t >>> struct resource *root_res; >>> resource_size_t rebar_size; >>> resource_size_t current_size; >>> + intel_wakeref_t wakeref; >>> u32 pci_cmd; >>> int i; >>> @@ -102,15 +103,25 @@ static void i915_resize_lmem_bar(struct >>> drm_i915_private *i915, resource_size_t >>> return; >>> } >>> - /* First disable PCI memory decoding references */ >>> - pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); >>> - pci_write_config_dword(pdev, PCI_COMMAND, >>> - pci_cmd & ~PCI_COMMAND_MEMORY); >>> + /* >>> + * Releasing forcewake during BAR resizing results in later >>> forcewake >>> + * ack timeouts and former can happen any time - it is >>> asynchronous. >>> + * Grabbing all forcewakes prevents it. >>> + */ >>> + with_intel_runtime_pm(i915->uncore.rpm, wakeref) { >>> + intel_uncore_forcewake_get(&i915->uncore, FORCEWAKE_ALL); >>> - _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); >>> + /* First disable PCI memory decoding references */ >>> + pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); >>> + pci_write_config_dword(pdev, PCI_COMMAND, >>> + pci_cmd & ~PCI_COMMAND_MEMORY); >>> - pci_assign_unassigned_bus_resources(pdev->bus); >>> - pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); >>> + _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); >>> + >>> + pci_assign_unassigned_bus_resources(pdev->bus); >>> + pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); >>> + intel_uncore_forcewake_put(&i915->uncore, FORCEWAKE_ALL); >>> + } >>> } >>> #else >>> static void i915_resize_lmem_bar(struct drm_i915_private *i915, >>> resource_size_t lmem_size) {} >
On 08.03.2023 14:36, Andrzej Hajda wrote: > Tests on DG2 machines show that releasing forcewakes during BAR resize > results later in forcewake ack timeouts. Since forcewakes can be realeased > asynchronously the simplest way to prevent it is to get all forcewakes > for duration of BAR resizing. > > v2: hold rpm as well during resizing (Rodrigo) > > Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> Queued to drm-intel-gt-next. Regards Andrzej
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c index 89fdfc67f8d1e0..2a3217e2890fc7 100644 --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c @@ -54,6 +54,7 @@ static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t struct resource *root_res; resource_size_t rebar_size; resource_size_t current_size; + intel_wakeref_t wakeref; u32 pci_cmd; int i; @@ -102,15 +103,25 @@ static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t return; } - /* First disable PCI memory decoding references */ - pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); - pci_write_config_dword(pdev, PCI_COMMAND, - pci_cmd & ~PCI_COMMAND_MEMORY); + /* + * Releasing forcewake during BAR resizing results in later forcewake + * ack timeouts and former can happen any time - it is asynchronous. + * Grabbing all forcewakes prevents it. + */ + with_intel_runtime_pm(i915->uncore.rpm, wakeref) { + intel_uncore_forcewake_get(&i915->uncore, FORCEWAKE_ALL); - _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); + /* First disable PCI memory decoding references */ + pci_read_config_dword(pdev, PCI_COMMAND, &pci_cmd); + pci_write_config_dword(pdev, PCI_COMMAND, + pci_cmd & ~PCI_COMMAND_MEMORY); - pci_assign_unassigned_bus_resources(pdev->bus); - pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); + _resize_bar(i915, GEN12_LMEM_BAR, rebar_size); + + pci_assign_unassigned_bus_resources(pdev->bus); + pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd); + intel_uncore_forcewake_put(&i915->uncore, FORCEWAKE_ALL); + } } #else static void i915_resize_lmem_bar(struct drm_i915_private *i915, resource_size_t lmem_size) {}
Tests on DG2 machines show that releasing forcewakes during BAR resize results later in forcewake ack timeouts. Since forcewakes can be realeased asynchronously the simplest way to prevent it is to get all forcewakes for duration of BAR resizing. v2: hold rpm as well during resizing (Rodrigo) Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> --- Please ignore resend of v1, my mistake. Regards Andrzej --- drivers/gpu/drm/i915/gt/intel_region_lmem.c | 25 +++++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-)