drm/amdgpu: fix S3 failure on specific platform

Submitted by Qingqing.Wang@amd.com on July 4, 2017, 4:26 a.m.

Details

Message ID 1499142367-3809-1-git-send-email-Qingqing.Wang@amd.com
State New
Headers show
Series "drm/amdgpu: fix S3 failure on specific platform" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Qingqing.Wang@amd.com July 4, 2017, 4:26 a.m.
Change-Id: Ie932508ad6949f8bfc7c8db1f5874d3440d09fc6
Signed-off-by: Ken Wang <Qingqing.Wang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 3 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++++++
 2 files changed, 11 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ecc33c4..54c30fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1702,6 +1702,9 @@  struct amdgpu_device {
 	/* record hw reset is performed */
 	bool has_hw_reset;
 
+	/* record last mm index being written through WREG32*/
+	unsigned long last_mm_index;
+
 };
 
 static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 21e504a..24b908c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -124,6 +124,10 @@  void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
 {
 	trace_amdgpu_mm_wreg(adev->pdev->device, reg, v);
 
+	if (adev->asic_type >= CHIP_VEGA10 && reg == 0) {
+		adev->last_mm_index = v;
+	}
+
 	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev)) {
 		BUG_ON(in_interrupt());
 		return amdgpu_virt_kiq_wreg(adev, reg, v);
@@ -139,6 +143,10 @@  void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
 		writel(v, ((void __iomem *)adev->rmmio) + (mmMM_DATA * 4));
 		spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);
 	}
+
+	if (adev->asic_type >= CHIP_VEGA10 && reg == 1 && adev->last_mm_index == 0x5702C) {
+		udelay(500);
+	}
 }
 
 u32 amdgpu_io_rreg(struct amdgpu_device *adev, u32 reg)

Comments

> -----Original Message-----

> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf

> Of Ken Wang

> Sent: Tuesday, July 04, 2017 12:26 AM

> To: amd-gfx@lists.freedesktop.org

> Cc: Wang, Ken

> Subject: [PATCH] drm/amdgpu: fix S3 failure on specific platform

> 

> Change-Id: Ie932508ad6949f8bfc7c8db1f5874d3440d09fc6

> Signed-off-by: Ken Wang <Qingqing.Wang@amd.com>


Do we need to add this to the io_rreg and io_wreg functions as well?  Atom uses them for IIO tables.  Might also want to give a brief description of the problem.  Something like:
Certain MC registers need a delay after writing them to properly update in the init sequence.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>


> ---

>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 3 +++

>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++++++

>  2 files changed, 11 insertions(+)

> 

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> index ecc33c4..54c30fe 100644

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> @@ -1702,6 +1702,9 @@ struct amdgpu_device {

>  	/* record hw reset is performed */

>  	bool has_hw_reset;

> 

> +	/* record last mm index being written through WREG32*/

> +	unsigned long last_mm_index;

> +

>  };

> 

>  static inline struct amdgpu_device *amdgpu_ttm_adev(struct

> ttm_bo_device *bdev)

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> index 21e504a..24b908c 100644

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> @@ -124,6 +124,10 @@ void amdgpu_mm_wreg(struct amdgpu_device

> *adev, uint32_t reg, uint32_t v,

>  {

>  	trace_amdgpu_mm_wreg(adev->pdev->device, reg, v);

> 

> +	if (adev->asic_type >= CHIP_VEGA10 && reg == 0) {

> +		adev->last_mm_index = v;

> +	}

> +

>  	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) &&

> amdgpu_sriov_runtime(adev)) {

>  		BUG_ON(in_interrupt());

>  		return amdgpu_virt_kiq_wreg(adev, reg, v);

> @@ -139,6 +143,10 @@ void amdgpu_mm_wreg(struct amdgpu_device

> *adev, uint32_t reg, uint32_t v,

>  		writel(v, ((void __iomem *)adev->rmmio) + (mmMM_DATA

> * 4));

>  		spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);

>  	}

> +

> +	if (adev->asic_type >= CHIP_VEGA10 && reg == 1 && adev-

> >last_mm_index == 0x5702C) {

> +		udelay(500);

> +	}

>  }

> 

>  u32 amdgpu_io_rreg(struct amdgpu_device *adev, u32 reg)

> --

> 2.7.4

> 

> _______________________________________________

> amd-gfx mailing list

> amd-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
sure, I will add the delay to io_wreg.
Am 04.07.2017 um 06:47 schrieb Deucher, Alexander:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
>> Of Ken Wang
>> Sent: Tuesday, July 04, 2017 12:26 AM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Wang, Ken
>> Subject: [PATCH] drm/amdgpu: fix S3 failure on specific platform
>>
>> Change-Id: Ie932508ad6949f8bfc7c8db1f5874d3440d09fc6
>> Signed-off-by: Ken Wang <Qingqing.Wang@amd.com>
> Do we need to add this to the io_rreg and io_wreg functions as well?  Atom uses them for IIO tables.  Might also want to give a brief description of the problem.  Something like:
> Certain MC registers need a delay after writing them to properly update in the init sequence.
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

I still have strong concerns adding such an ASIC specific workaround in 
the low level register access functions.

Since this register is only accessed from the VBIOS I thing the proper 
place to add it would be in the ATOM table parser.

Regards,
Christian.

>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 3 +++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++++++
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index ecc33c4..54c30fe 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1702,6 +1702,9 @@ struct amdgpu_device {
>>   	/* record hw reset is performed */
>>   	bool has_hw_reset;
>>
>> +	/* record last mm index being written through WREG32*/
>> +	unsigned long last_mm_index;
>> +
>>   };
>>
>>   static inline struct amdgpu_device *amdgpu_ttm_adev(struct
>> ttm_bo_device *bdev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 21e504a..24b908c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -124,6 +124,10 @@ void amdgpu_mm_wreg(struct amdgpu_device
>> *adev, uint32_t reg, uint32_t v,
>>   {
>>   	trace_amdgpu_mm_wreg(adev->pdev->device, reg, v);
>>
>> +	if (adev->asic_type >= CHIP_VEGA10 && reg == 0) {
>> +		adev->last_mm_index = v;
>> +	}
>> +
>>   	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) &&
>> amdgpu_sriov_runtime(adev)) {
>>   		BUG_ON(in_interrupt());
>>   		return amdgpu_virt_kiq_wreg(adev, reg, v);
>> @@ -139,6 +143,10 @@ void amdgpu_mm_wreg(struct amdgpu_device
>> *adev, uint32_t reg, uint32_t v,
>>   		writel(v, ((void __iomem *)adev->rmmio) + (mmMM_DATA
>> * 4));
>>   		spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);
>>   	}
>> +
>> +	if (adev->asic_type >= CHIP_VEGA10 && reg == 1 && adev-
>>> last_mm_index == 0x5702C) {
>> +		udelay(500);
>> +	}
>>   }
>>
>>   u32 amdgpu_io_rreg(struct amdgpu_device *adev, u32 reg)
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> -----Original Message-----

> From: Christian König [mailto:deathsimple@vodafone.de]

> Sent: Tuesday, July 04, 2017 3:50 AM

> To: Deucher, Alexander; Wang, Ken; amd-gfx@lists.freedesktop.org

> Subject: Re: [PATCH] drm/amdgpu: fix S3 failure on specific platform

> 

> Am 04.07.2017 um 06:47 schrieb Deucher, Alexander:

> >> -----Original Message-----

> >> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On

> Behalf

> >> Of Ken Wang

> >> Sent: Tuesday, July 04, 2017 12:26 AM

> >> To: amd-gfx@lists.freedesktop.org

> >> Cc: Wang, Ken

> >> Subject: [PATCH] drm/amdgpu: fix S3 failure on specific platform

> >>

> >> Change-Id: Ie932508ad6949f8bfc7c8db1f5874d3440d09fc6

> >> Signed-off-by: Ken Wang <Qingqing.Wang@amd.com>

> > Do we need to add this to the io_rreg and io_wreg functions as well?  Atom

> uses them for IIO tables.  Might also want to give a brief description of the

> problem.  Something like:

> > Certain MC registers need a delay after writing them to properly update in

> the init sequence.

> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> 

> I still have strong concerns adding such an ASIC specific workaround in

> the low level register access functions.

> 

> Since this register is only accessed from the VBIOS I thing the proper

> place to add it would be in the ATOM table parser.


We should be able to implement something similar in the atom code, depending on whether the table in question uses the IIO stuff or "regular"  register access.

Alex

> 

> Regards,

> Christian.

> 

> >

> >> ---

> >>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 3 +++

> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++++++

> >>   2 files changed, 11 insertions(+)

> >>

> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> >> index ecc33c4..54c30fe 100644

> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> >> @@ -1702,6 +1702,9 @@ struct amdgpu_device {

> >>   	/* record hw reset is performed */

> >>   	bool has_hw_reset;

> >>

> >> +	/* record last mm index being written through WREG32*/

> >> +	unsigned long last_mm_index;

> >> +

> >>   };

> >>

> >>   static inline struct amdgpu_device *amdgpu_ttm_adev(struct

> >> ttm_bo_device *bdev)

> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> >> index 21e504a..24b908c 100644

> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> >> @@ -124,6 +124,10 @@ void amdgpu_mm_wreg(struct amdgpu_device

> >> *adev, uint32_t reg, uint32_t v,

> >>   {

> >>   	trace_amdgpu_mm_wreg(adev->pdev->device, reg, v);

> >>

> >> +	if (adev->asic_type >= CHIP_VEGA10 && reg == 0) {

> >> +		adev->last_mm_index = v;

> >> +	}

> >> +

> >>   	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) &&

> >> amdgpu_sriov_runtime(adev)) {

> >>   		BUG_ON(in_interrupt());

> >>   		return amdgpu_virt_kiq_wreg(adev, reg, v);

> >> @@ -139,6 +143,10 @@ void amdgpu_mm_wreg(struct amdgpu_device

> >> *adev, uint32_t reg, uint32_t v,

> >>   		writel(v, ((void __iomem *)adev->rmmio) + (mmMM_DATA

> >> * 4));

> >>   		spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);

> >>   	}

> >> +

> >> +	if (adev->asic_type >= CHIP_VEGA10 && reg == 1 && adev-

> >>> last_mm_index == 0x5702C) {

> >> +		udelay(500);

> >> +	}

> >>   }

> >>

> >>   u32 amdgpu_io_rreg(struct amdgpu_device *adev, u32 reg)

> >> --

> >> 2.7.4

> >>

> >> _______________________________________________

> >> amd-gfx mailing list

> >> amd-gfx@lists.freedesktop.org

> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

> > _______________________________________________

> > amd-gfx mailing list

> > amd-gfx@lists.freedesktop.org

> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx

>