* [PATCH v4] drm/amdgpu: Restore msix after FLR @ 2021-07-02 3:23 Peng Ju Zhou 2021-07-02 6:56 ` Christian König 2021-07-05 5:51 ` Lazar, Lijo 0 siblings, 2 replies; 4+ messages in thread From: Peng Ju Zhou @ 2021-07-02 3:23 UTC (permalink / raw) To: amd-gfx; +Cc: Emily.Deng From: "Emily.Deng" <Emily.Deng@amd.com> After FLR, the msix will be cleared, so need to re-enable it. Signed-off-by: Emily.Deng <Emily.Deng@amd.com> Signed-off-by: Peng Ju Zhou <PengJu.Zhou@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index 90f50561b43a..034420c38352 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c @@ -277,6 +277,19 @@ static bool amdgpu_msi_ok(struct amdgpu_device *adev) return true; } +void amdgpu_restore_msix(struct amdgpu_device *adev) +{ + u16 ctrl; + + pci_read_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS, &ctrl); + if (!(ctrl & PCI_MSIX_FLAGS_ENABLE)) + return; + + ctrl &= ~PCI_MSIX_FLAGS_ENABLE; + pci_write_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS, ctrl); + ctrl |= PCI_MSIX_FLAGS_ENABLE; + pci_write_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS, ctrl); +} /** * amdgpu_irq_init - initialize interrupt handling * @@ -558,6 +571,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct amdgpu_device *adev) { int i, j, k; + amdgpu_restore_msix(adev); for (i = 0; i < AMDGPU_IRQ_CLIENTID_MAX; ++i) { if (!adev->irq.client[i].sources) continue; -- 2.17.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4] drm/amdgpu: Restore msix after FLR 2021-07-02 3:23 [PATCH v4] drm/amdgpu: Restore msix after FLR Peng Ju Zhou @ 2021-07-02 6:56 ` Christian König 2021-07-05 5:51 ` Lazar, Lijo 1 sibling, 0 replies; 4+ messages in thread From: Christian König @ 2021-07-02 6:56 UTC (permalink / raw) To: Peng Ju Zhou, amd-gfx; +Cc: Emily.Deng Am 02.07.21 um 05:23 schrieb Peng Ju Zhou: > From: "Emily.Deng" <Emily.Deng@amd.com> > > After FLR, the msix will be cleared, so need to re-enable it. > > Signed-off-by: Emily.Deng <Emily.Deng@amd.com> > Signed-off-by: Peng Ju Zhou <PengJu.Zhou@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > index 90f50561b43a..034420c38352 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > @@ -277,6 +277,19 @@ static bool amdgpu_msi_ok(struct amdgpu_device *adev) > return true; > } > > +void amdgpu_restore_msix(struct amdgpu_device *adev) I think this function should be static and maybe add a one line comment like "Clear and re-set the MSIX flags if they where set before to trigger re-enabling it". With that done feel free to add an Acked-by: Christian König <christian.koenig@amd.com>, but I would also wait what Alex has to say about that. Regards, Christian > +{ > + u16 ctrl; > + > + pci_read_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS, &ctrl); > + if (!(ctrl & PCI_MSIX_FLAGS_ENABLE)) > + return; > + > + ctrl &= ~PCI_MSIX_FLAGS_ENABLE; > + pci_write_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS, ctrl); > + ctrl |= PCI_MSIX_FLAGS_ENABLE; > + pci_write_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS, ctrl); > +} > /** > * amdgpu_irq_init - initialize interrupt handling > * > @@ -558,6 +571,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct amdgpu_device *adev) > { > int i, j, k; > > + amdgpu_restore_msix(adev); > for (i = 0; i < AMDGPU_IRQ_CLIENTID_MAX; ++i) { > if (!adev->irq.client[i].sources) > continue; _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] drm/amdgpu: Restore msix after FLR 2021-07-02 3:23 [PATCH v4] drm/amdgpu: Restore msix after FLR Peng Ju Zhou 2021-07-02 6:56 ` Christian König @ 2021-07-05 5:51 ` Lazar, Lijo 2021-07-07 5:18 ` Zhou, Peng Ju 1 sibling, 1 reply; 4+ messages in thread From: Lazar, Lijo @ 2021-07-05 5:51 UTC (permalink / raw) To: Peng Ju Zhou, amd-gfx; +Cc: Emily.Deng On 7/2/2021 8:53 AM, Peng Ju Zhou wrote: > From: "Emily.Deng" <Emily.Deng@amd.com> > > After FLR, the msix will be cleared, so need to re-enable it. > > Signed-off-by: Emily.Deng <Emily.Deng@amd.com> > Signed-off-by: Peng Ju Zhou <PengJu.Zhou@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > index 90f50561b43a..034420c38352 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > @@ -277,6 +277,19 @@ static bool amdgpu_msi_ok(struct amdgpu_device *adev) > return true; > } > > +void amdgpu_restore_msix(struct amdgpu_device *adev) > +{ > + u16 ctrl; > + > + pci_read_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS, &ctrl); > + if (!(ctrl & PCI_MSIX_FLAGS_ENABLE)) > + return; > + > + ctrl &= ~PCI_MSIX_FLAGS_ENABLE; > + pci_write_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS, ctrl); > + ctrl |= PCI_MSIX_FLAGS_ENABLE; > + pci_write_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS, ctrl); This field behavior is determined by PCIE spec and toggling is not required to enable MSIX. Also, FLR will clear this field on PF, so the ENABLE flag check to see if MSIX is already enabled doesn't make sense for PF case. From the code logic, a rough guess is this code is trying to reset the VF's config space field in PF after VF FLR and enable MSI-X back. If that is the case, make an explicit check so that this is only done on VF devices. Thanks, Lijo > +} > /** > * amdgpu_irq_init - initialize interrupt handling > * > @@ -558,6 +571,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct amdgpu_device *adev) > { > int i, j, k; > > + amdgpu_restore_msix(adev); > for (i = 0; i < AMDGPU_IRQ_CLIENTID_MAX; ++i) { > if (!adev->irq.client[i].sources) > continue; > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v4] drm/amdgpu: Restore msix after FLR 2021-07-05 5:51 ` Lazar, Lijo @ 2021-07-07 5:18 ` Zhou, Peng Ju 0 siblings, 0 replies; 4+ messages in thread From: Zhou, Peng Ju @ 2021-07-07 5:18 UTC (permalink / raw) To: Lazar, Lijo, amd-gfx; +Cc: Deng, Emily [AMD Official Use Only] Adding VF check and static for function, patch sent out as v5. ---------------------------------------------------------------------- BW Pengju Zhou > -----Original Message----- > From: Lazar, Lijo <Lijo.Lazar@amd.com> > Sent: Monday, July 5, 2021 1:52 PM > To: Zhou, Peng Ju <PengJu.Zhou@amd.com>; amd-gfx@lists.freedesktop.org > Cc: Deng, Emily <Emily.Deng@amd.com> > Subject: Re: [PATCH v4] drm/amdgpu: Restore msix after FLR > > > > On 7/2/2021 8:53 AM, Peng Ju Zhou wrote: > > From: "Emily.Deng" <Emily.Deng@amd.com> > > > > After FLR, the msix will be cleared, so need to re-enable it. > > > > Signed-off-by: Emily.Deng <Emily.Deng@amd.com> > > Signed-off-by: Peng Ju Zhou <PengJu.Zhou@amd.com> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > > index 90f50561b43a..034420c38352 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > > @@ -277,6 +277,19 @@ static bool amdgpu_msi_ok(struct amdgpu_device > *adev) > > return true; > > } > > > > +void amdgpu_restore_msix(struct amdgpu_device *adev) { > > + u16 ctrl; > > + > > + pci_read_config_word(adev->pdev, adev->pdev->msix_cap + > PCI_MSIX_FLAGS, &ctrl); > > + if (!(ctrl & PCI_MSIX_FLAGS_ENABLE)) > > + return; > > + > > + ctrl &= ~PCI_MSIX_FLAGS_ENABLE; > > + pci_write_config_word(adev->pdev, adev->pdev->msix_cap + > PCI_MSIX_FLAGS, ctrl); > > + ctrl |= PCI_MSIX_FLAGS_ENABLE; > > + pci_write_config_word(adev->pdev, adev->pdev->msix_cap + > > +PCI_MSIX_FLAGS, ctrl); > This field behavior is determined by PCIE spec and toggling is not required to > enable MSIX. Also, FLR will clear this field on PF, so the ENABLE flag check to > see if MSIX is already enabled doesn't make sense for PF case. > > From the code logic, a rough guess is this code is trying to reset the VF's config > space field in PF after VF FLR and enable MSI-X back. If that is the case, make > an explicit check so that this is only done on VF devices. > > Thanks, > Lijo > > > +} > > /** > > * amdgpu_irq_init - initialize interrupt handling > > * > > @@ -558,6 +571,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct > amdgpu_device *adev) > > { > > int i, j, k; > > > > + amdgpu_restore_msix(adev); > > for (i = 0; i < AMDGPU_IRQ_CLIENTID_MAX; ++i) { > > if (!adev->irq.client[i].sources) > > continue; > > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-07 5:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-02 3:23 [PATCH v4] drm/amdgpu: Restore msix after FLR Peng Ju Zhou 2021-07-02 6:56 ` Christian König 2021-07-05 5:51 ` Lazar, Lijo 2021-07-07 5:18 ` Zhou, Peng Ju
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.