All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.