Re: [PATCH] drm/amd/amdgpu: Avoid fault when allocating an empty buffer object

[Huang Rui]

On Mon, Sep 17, 2018 at 02:07:11PM -0400, Tom St Denis wrote:
> Signed-off-by: Tom St Denis 

Reviewed-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 84d82d5382f9..c1387efc0c91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
>   if (r)
>   return r;
>  
> - amdgpu_bo_unreserve(*bo_ptr);
> + if (*bo_ptr)
> + amdgpu_bo_unreserve(*bo_ptr);
>  
>   return 0;
>  }
> -- 
> 2.17.1
> 
> ___
> 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

Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

[Huang Rui]

On Mon, Sep 17, 2018 at 08:07:24PM +0200, Christian König wrote:
> Don't try to unreserve a BO we doesn't allocated.
> 
> Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel BOs
> 
> Signed-off-by: Christian König 

If we set size as 0 while create bo, the bo_ptr will be NULL after that.

Acked-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 84d82d5382f9..c1387efc0c91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
>   if (r)
>   return r;
>  
> - amdgpu_bo_unreserve(*bo_ptr);
> + if (*bo_ptr)
> + amdgpu_bo_unreserve(*bo_ptr);
>  
>   return 0;
>  }
> -- 
> 2.14.1
> 
> ___
> 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

RE: [PATCH] drm/amd/dc: Trigger set power state task when display configuration changes

[Zhu, Rex]

Hi David,

Yes, There is an issue you pointed out on Raven. Because the force_dpm_level 
was added for UMD-P state and the min clocks were hardcode. The display's clock 
requests were ignored.
I will fix this issue.
Thanks.

Best Regards
Rex

From: Francis, David
Sent: Monday, September 17, 2018 9:31 PM
To: Zhu, Rex ; Deucher, Alexander ; 
Wentland, Harry 
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/dc: Trigger set power state task when display 
configuration changes


On Linux, the clocks are already set by the time that function call is reached. 
 amdgpu_pm_compute clocks resets the clocks to their default value.


From: Zhu, Rex
Sent: September 17, 2018 4:12:33 AM
To: Deucher, Alexander; Wentland, Harry; Francis, David
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amd/dc: Trigger set power state task when display 
configuration changes


+Harry and David.



Best Regards

Rex



From: Deucher, Alexander
Sent: Friday, September 14, 2018 9:40 PM
To: Zhu, Rex mailto:rex@amd.com>>; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/dc: Trigger set power state task when display 
configuration changes



Acked-by: Alex Deucher 
mailto:alexander.deuc...@amd.com>>



From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Rex Zhu mailto:rex@amd.com>>
Sent: Friday, September 14, 2018 1:57:07 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhu, Rex
Subject: [PATCH] drm/amd/dc: Trigger set power state task when display 
configuration changes



Revert "drm/amd/display: Remove call to amdgpu_pm_compute_clocks"

This reverts commit dcd473770e86517543691bdb227103d6c781cd0a.

when display configuration changes, dc need to update the changes
to powerplay, also need to trigger a power state task.
amdgpu_pm_compute_clocks is the interface to set power state task
either dpm enabled or powerplay enabled

Signed-off-by: Rex Zhu mailto:rex@amd.com>>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
index 6d16b4a..0fab64a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
@@ -105,6 +105,8 @@ bool dm_pp_apply_display_requirements(
 adev->powerplay.pp_funcs->display_configuration_change(
 adev->powerplay.pp_handle,
 >pm.pm_display_cfg);
+
+   amdgpu_pm_compute_clocks(adev);
 }

 return true;
--
1.9.1

___
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

Re: [PATCH] list: introduce list_bulk_move_tail helper

[Zhang, Jerry(Junwei)]

On 09/17/2018 08:08 PM, Christian König wrote:

Move all entries between @first and including @last before @head.

This is useful for LRU lists where a whole block of entries should be
moved to the end of the list.

Used as a band aid in TTM, but better placed in the common list headers.

Signed-off-by: Christian König 

Reviewed-by: Junwei Zhang 

---
  drivers/gpu/drm/ttm/ttm_bo.c | 25 +
  include/linux/list.h | 23 +++
  2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index b2a33bf1ef10..26b889f86670 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -247,20 +247,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
  }
  EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
  
-static void ttm_list_move_bulk_tail(struct list_head *list,

-   struct list_head *first,
-   struct list_head *last)
-{
-   first->prev->next = last->next;
-   last->next->prev = first->prev;
-
-   list->prev->next = first;
-   first->prev = list->prev;
-
-   last->next = list;
-   list->prev = last;
-}
-
  void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
  {
unsigned i;
@@ -276,8 +262,8 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
*bulk)
reservation_object_assert_held(pos->last->resv);
  
  		man = >first->bdev->man[TTM_PL_TT];

-   ttm_list_move_bulk_tail(>lru[i], >first->lru,
-   >last->lru);
+   list_bulk_move_tail(>lru[i], >first->lru,
+   >last->lru);
}
  
  	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {

@@ -291,8 +277,8 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
*bulk)
reservation_object_assert_held(pos->last->resv);
  
  		man = >first->bdev->man[TTM_PL_VRAM];

-   ttm_list_move_bulk_tail(>lru[i], >first->lru,
-   >last->lru);
+   list_bulk_move_tail(>lru[i], >first->lru,
+   >last->lru);
}
  
  	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {

@@ -306,8 +292,7 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
*bulk)
reservation_object_assert_held(pos->last->resv);
  
  		lru = >first->bdev->glob->swap_lru[i];

-   ttm_list_move_bulk_tail(lru, >first->swap,
-   >last->swap);
+   list_bulk_move_tail(lru, >first->swap, >last->swap);
}
  }
  EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
diff --git a/include/linux/list.h b/include/linux/list.h
index de04cc5ed536..edb7628e46ed 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -183,6 +183,29 @@ static inline void list_move_tail(struct list_head *list,
list_add_tail(list, head);
  }
  
+/**

+ * list_bulk_move_tail - move a subsection of a list to its tail
+ * @head: the head that will follow our entry
+ * @first: first entry to move
+ * @last: last entry to move, can be the same as first
+ *
+ * Move all entries between @first and including @last before @head.
+ * All three entries must belong to the same linked list.
+ */
+static inline void list_bulk_move_tail(struct list_head *head,
+  struct list_head *first,
+  struct list_head *last)
+{
+   first->prev->next = last->next;
+   last->next->prev = first->prev;
+
+   head->prev->next = first;
+   first->prev = head->prev;
+
+   last->next = head;
+   head->prev = last;
+}
+
  /**
   * list_is_last - tests whether @list is the last entry in list @head
   * @list: the entry to test


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: update vram_info structure in atomfirmware.h

[Huang Rui]

On Mon, Sep 17, 2018 at 08:28:39PM +0800, Hawking Zhang wrote:
> atomfirmware has structure changes in varm_info. Updated it
> to the latest one.
> 
> Change-Id: Ie5d60413e5db1dfb4aaf23dc94bc5fd4ed0a01cd
> Signed-off-by: Hawking Zhang 
> Reviewed-by: Alex Deucher 

Series are Reviewed-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c |  2 +-
>  drivers/gpu/drm/amd/include/atomfirmware.h   | 20 +++-
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> index 2369158..5461d0d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> @@ -174,7 +174,7 @@ static int convert_atom_mem_type_to_vram_type (struct 
> amdgpu_device *adev,
>   case ATOM_DGPU_VRAM_TYPE_GDDR5:
>   vram_type = AMDGPU_VRAM_TYPE_GDDR5;
>   break;
> - case ATOM_DGPU_VRAM_TYPE_HBM:
> + case ATOM_DGPU_VRAM_TYPE_HBM2:
>   vram_type = AMDGPU_VRAM_TYPE_HBM;
>   break;
>   default:
> diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h 
> b/drivers/gpu/drm/amd/include/atomfirmware.h
> index 6109a45..8ae7adb 100644
> --- a/drivers/gpu/drm/amd/include/atomfirmware.h
> +++ b/drivers/gpu/drm/amd/include/atomfirmware.h
> @@ -179,7 +179,7 @@ enum atom_voltage_type
>  
>  enum atom_dgpu_vram_type{
>ATOM_DGPU_VRAM_TYPE_GDDR5 = 0x50,
> -  ATOM_DGPU_VRAM_TYPE_HBM   = 0x60,
> +  ATOM_DGPU_VRAM_TYPE_HBM2  = 0x60,
>  };
>  
>  enum atom_dp_vs_preemph_def{
> @@ -1699,10 +1699,10 @@ struct atom_vram_module_v9
>  {
>// Design Specific Values
>uint32_t  memory_size;   // Total memory size in unit of 
> MB for CONFIG_MEMSIZE zeros
> -  uint32_t  channel_enable;// for 32 channel ASIC usage
> -  uint32_t  umcch_addrcfg;
> -  uint32_t  umcch_addrsel;
> -  uint32_t  umcch_colsel;
> +  uint32_t  channel_enable;// bit vector, each bit indicate 
> specific channel enable or not
> +  uint32_t  max_mem_clk;   // max memory clock of this 
> memory in unit of 10kHz, =0 means it is not defined
> +  uint16_t  reserved[3];
> +  uint16_t  mem_voltage;   // mem_voltage
>uint16_t  vram_module_size;  // Size of atom_vram_module_v9
>uint8_t   ext_memory_id; // Current memory module ID
>uint8_t   memory_type;   // enum of atom_dgpu_vram_type
> @@ -1712,20 +1712,22 @@ struct atom_vram_module_v9
>uint8_t   tunningset_id; // MC phy registers set per. 
>uint8_t   vender_rev_id; // [7:4] Revision, [3:0] Vendor 
> code
>uint8_t   refreshrate;   // [1:0]=RefreshFactor (00=8ms, 
> 01=16ms, 10=32ms,11=64ms)
> -  uint16_t  vram_rsd2; // reserved
> +  uint8_t   hbm_ven_rev_id; // hbm_ven_rev_id
> +  uint8_t   vram_rsd2;  // reserved
>chardram_pnstring[20];   // part number end with '0'. 
>  };
>  
> -
>  struct atom_vram_info_header_v2_3
>  {
> -  struct   atom_common_table_header  table_header;
> +  struct   atom_common_table_header table_header;
>uint16_t mem_adjust_tbloffset; // offset of 
> atom_umc_init_reg_block structure for memory vendor specific UMC adjust 
> setting
>uint16_t mem_clk_patch_tbloffset;  // offset of 
> atom_umc_init_reg_block structure for memory clock specific UMC setting
>uint16_t mc_adjust_pertile_tbloffset;  // offset of 
> atom_umc_init_reg_block structure for Per Byte Offset Preset Settings
>uint16_t mc_phyinit_tbloffset; // offset of 
> atom_umc_init_reg_block structure for MC phy init set
>uint16_t dram_data_remap_tbloffset;// reserved for now
> -  uint16_t vram_rsd2[3];
> +  uint16_t tmrs_seq_offset;  // offset of HBM 
> tmrs
> +  uint16_t post_ucode_init_offset;   // offset of 
> atom_umc_init_reg_block structure for MC phy init after MC uCode complete umc 
> init
> +  uint16_t vram_rsd2;
>uint8_t  vram_module_num;  // indicate number 
> of VRAM module
>uint8_t  vram_rsd1[2];
>uint8_t  mc_phy_tile_num;  // indicate the MCD 
> tile number which use in DramDataRemapTbl and usMcAdjustPerTileTblOffset
> -- 
> 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

Re: [PATCH] list: introduce list_bulk_move_tail helper

[Huang Rui]

On Mon, Sep 17, 2018 at 02:08:34PM +0200, Christian König wrote:
> Move all entries between @first and including @last before @head.
> 
> This is useful for LRU lists where a whole block of entries should be
> moved to the end of the list.
> 
> Used as a band aid in TTM, but better placed in the common list headers.
> 
> Signed-off-by: Christian König 
> ---

For TTM change, patch is Reviewed-by: Huang Rui 

For new list_bulk_move_tail helper in hinclude/linux/list.h, may we have
your comments from LKML?

Thanks,
Ray

>  drivers/gpu/drm/ttm/ttm_bo.c | 25 +
>  include/linux/list.h | 23 +++
>  2 files changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index b2a33bf1ef10..26b889f86670 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -247,20 +247,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object 
> *bo,
>  }
>  EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
>  
> -static void ttm_list_move_bulk_tail(struct list_head *list,
> - struct list_head *first,
> - struct list_head *last)
> -{
> - first->prev->next = last->next;
> - last->next->prev = first->prev;
> -
> - list->prev->next = first;
> - first->prev = list->prev;
> -
> - last->next = list;
> - list->prev = last;
> -}
> -
>  void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
>  {
>   unsigned i;
> @@ -276,8 +262,8 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
> *bulk)
>   reservation_object_assert_held(pos->last->resv);
>  
>   man = >first->bdev->man[TTM_PL_TT];
> - ttm_list_move_bulk_tail(>lru[i], >first->lru,
> - >last->lru);
> + list_bulk_move_tail(>lru[i], >first->lru,
> + >last->lru);
>   }
>  
>   for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> @@ -291,8 +277,8 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
> *bulk)
>   reservation_object_assert_held(pos->last->resv);
>  
>   man = >first->bdev->man[TTM_PL_VRAM];
> - ttm_list_move_bulk_tail(>lru[i], >first->lru,
> - >last->lru);
> + list_bulk_move_tail(>lru[i], >first->lru,
> + >last->lru);
>   }
>  
>   for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> @@ -306,8 +292,7 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
> *bulk)
>   reservation_object_assert_held(pos->last->resv);
>  
>   lru = >first->bdev->glob->swap_lru[i];
> - ttm_list_move_bulk_tail(lru, >first->swap,
> - >last->swap);
> + list_bulk_move_tail(lru, >first->swap, >last->swap);
>   }
>  }
>  EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
> diff --git a/include/linux/list.h b/include/linux/list.h
> index de04cc5ed536..edb7628e46ed 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -183,6 +183,29 @@ static inline void list_move_tail(struct list_head *list,
>   list_add_tail(list, head);
>  }
>  
> +/**
> + * list_bulk_move_tail - move a subsection of a list to its tail
> + * @head: the head that will follow our entry
> + * @first: first entry to move
> + * @last: last entry to move, can be the same as first
> + *
> + * Move all entries between @first and including @last before @head.
> + * All three entries must belong to the same linked list.
> + */
> +static inline void list_bulk_move_tail(struct list_head *head,
> +struct list_head *first,
> +struct list_head *last)
> +{
> + first->prev->next = last->next;
> + last->next->prev = first->prev;
> +
> + head->prev->next = first;
> + first->prev = head->prev;
> +
> + last->next = head;
> + head->prev = last;
> +}
> +
>  /**
>   * list_is_last - tests whether @list is the last entry in list @head
>   * @list: the entry to test
> -- 
> 2.14.1
> 
> ___
> 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

Re: [PATCH] drm/radeon: change function signature to pass full range

[Alex Deucher]

On Mon, Sep 17, 2018 at 4:29 PM Mathieu Malaterre  wrote:
>
>
>
> On Fri, Apr 13, 2018 at 9:59 AM Huang Rui  wrote:
>>
>> On Thu, Apr 12, 2018 at 09:33:33PM +0200, Mathieu Malaterre wrote:
>> > In function ‘radeon_process_i2c_ch’ a comparison of a u8 value against
>> > 255 is done. Since it is always false, change the signature of this
>> > function to use an `int` instead, which match the type used in caller:
>> > `radeon_atom_hw_i2c_xfer`.
>> >
>> > Fix the following warning triggered with W=1:
>> >
>> >   CC [M]  drivers/gpu/drm/radeon/atombios_i2c.o
>> >   drivers/gpu/drm/radeon/atombios_i2c.c: In function 
>> > ‘radeon_process_i2c_ch’:
>> >   drivers/gpu/drm/radeon/atombios_i2c.c:71:11: warning: comparison is 
>> > always false due to limited range of data type [-Wtype-limits]
>> >if (num > ATOM_MAX_HW_I2C_READ) {
>> >^
>> >
>> > Signed-off-by: Mathieu Malaterre 
>>
>> Reviewed-by: Huang Rui 
>>
>
> Any chance to be included in the next pull request ? thanks

Applied.  thanks for the reminder.

Alex

>
>>
>> > ---
>> >  drivers/gpu/drm/radeon/atombios_i2c.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/radeon/atombios_i2c.c 
>> > b/drivers/gpu/drm/radeon/atombios_i2c.c
>> > index 4157780585a0..9022e9af11a0 100644
>> > --- a/drivers/gpu/drm/radeon/atombios_i2c.c
>> > +++ b/drivers/gpu/drm/radeon/atombios_i2c.c
>> > @@ -35,7 +35,7 @@
>> >
>> >  static int radeon_process_i2c_ch(struct radeon_i2c_chan *chan,
>> >u8 slave_addr, u8 flags,
>> > -  u8 *buf, u8 num)
>> > +  u8 *buf, int num)
>> >  {
>> >   struct drm_device *dev = chan->dev;
>> >   struct radeon_device *rdev = dev->dev_private;
>> > --
>> > 2.11.0
>> >
>> > ___
>> > 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
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] amdgpu uses raw rlc_hdr values, causing kernel OOPS on big endian architectures

[Alex Deucher]

On Mon, Sep 17, 2018 at 3:41 AM A. Wilcox  wrote:
>
> adev->gfx.rlc in gfx_v8_0_init_microcode has the values from rlc_hdr
> already processed by le32_to_cpu.  Using the rlc_hdr values on
> big-endian machines causes a kernel Oops due to writing well outside of
> the array (0x2400 instead of 0x24).  gfx_v9_0 had the same issue and
> was fixed in the same manner (but was not tested locally; I do not have
> a v9 card).

Applied.  thanks!

Alex

>
> [8.143396] Unable to handle kernel paging request for data at
> address 0xc0080615b000
> [8.143396] Faulting instruction address: 0xc00805f063c8
> [8.143399] Oops: Kernel access of bad area, sig: 11 [#1]
> [8.143429] BE SMP NR_CPUS=256 NUMA PowerNV
> [8.143461] Modules linked in: binfmt_misc amdgpu(+) ast ttm
> drm_kms_helper sysimgblt syscopyarea sysfillrect fb_sys_fops drm joydev
> mac_hid tg3 ipmi_powernv ipmi_msghandler agpgart i2c_algo_bit shpchp
> [8.143615] CPU: 0 PID: 2402 Comm: kworker/0:3 Not tainted
> 4.14.48-mc8-easy #1
> [8.143679] Workqueue: events .work_for_cpu_fn
> [8.143728] task: c003e7fd8000 task.stack: c003e7fe
> [8.143783] NIP:  c00805f063c8 LR: c00805f06388 CTR:
> c027efd0
> [8.143869] REGS: c003e7fe3430 TRAP: 0300   Not tainted
> (4.14.48-mc8-easy)
> [8.143950] MSR:  90009032   CR:
> 28002444  XER: 2004
> [8.144040] CFAR: c00805f063d0 DAR: c0080615b000 DSISR:
> 4000 SOFTE: 1
> [8.144787] NIP [c00805f063c8] .gfx_v8_0_sw_init+0x5a8/0x15b0
> [amdgpu]
> [8.144911] LR [c00805f06388] .gfx_v8_0_sw_init+0x568/0x15b0 [amdgpu]
> [8.144976] Call Trace:
> [8.145049] [c003e7fe36b0] [c00805f06388]
> .gfx_v8_0_sw_init+0x568/0x15b0 [amdgpu] (unreliable)
> [8.145194] [c003e7fe37c0] [c00805e484c4]
> .amdgpu_device_init+0xf34/0x1750 [amdgpu]
> [8.145302] [c003e7fe38f0] [c00805e4a94c]
> .amdgpu_driver_load_kms+0x9c/0x2a0 [amdgpu]
> [8.145420] [c003e7fe3980] [c00804f4d200]
> .drm_dev_register+0x1c0/0x250 [drm]
> [8.145548] [c003e7fe3a30] [c00805e416c4]
> .amdgpu_pci_probe+0x164/0x1a0 [amdgpu]
> [8.145601] [c003e7fe3ac0] [c0618ed0]
> .local_pci_probe+0x60/0x130
> [8.145683] [c003e7fe3b60] [c00e8780]
> .work_for_cpu_fn+0x30/0x50
> [8.145774] [c003e7fe3be0] [c00ecea8]
> .process_one_work+0x2a8/0x550
> [8.145854] [c003e7fe3c80] [c00ed430]
> .worker_thread+0x2e0/0x600
> [8.145924] [c003e7fe3d70] [c00f5338] .kthread+0x158/0x1a0
> [8.145973] [c003e7fe3e30] [c000bd4c]
> .ret_from_kernel_thread+0x58/0x8c
> [8.146054] Instruction dump:
> [8.146089] 38db004c 3920 7d00342c 7d08da14 815b0048 554af0be
> 7f8a4840 409d004c 792a1764 e8ff46f0 39290001 79290020 <7cc8542c>
> 7cc7512e 4bd8 6000
> [8.146252] ---[ end trace 8b0ede048bbb20ae ]---
>
> --
> A. Wilcox (awilfox)
> Project Lead, Adélie Linux
> https://www.adelielinux.org
> ___
> 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

BUG: [amdgpu]] *ERROR* ring gfx timeout

[Daniel Andersson]

Hey,

GPU hangs for a while(~20s) and dmesg is:

110.343379] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx
timeout, last signaled seq=7273, last emitted seq=7275
[  110.343385] [drm] GPU recovery disabled.

4.18.8-arch1-1-ARCH & Vega10:

Reproduce by:
git clone git://github.com/bkaradzic/bx.git
git clone git://github.com/bkaradzic/bimg.git
git clone git://github.com/bkaradzic/bgfx.git

cd bgfx
make -jX linux-release64
cd examples/runtime
../../.build/linux64_gcc/bin/examplesRelease
Switch to example "37-gpudrivenrendering" from the drop down menu

/ Daniel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg

[Jonathan Cameron]

On Wed, 12 Sep 2018 17:08:52 +0200
Arnd Bergmann  wrote:

> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
> 
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
> 
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
> 
> Signed-off-by: Arnd Bergmann 
> ---

For IIO part.

Acked-by: Jonathan Cameron 

Thanks,
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index a062cfddc5af..22844b94b0e9 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1630,7 +1630,7 @@ static const struct file_operations iio_buffer_fileops 
> = {
>   .owner = THIS_MODULE,
>   .llseek = noop_llseek,
>   .unlocked_ioctl = iio_ioctl,
> - .compat_ioctl = iio_ioctl,
> + .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>  

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/radeon: change function signature to pass full range

[Mathieu Malaterre]

On Fri, Apr 13, 2018 at 9:59 AM Huang Rui  wrote:

> On Thu, Apr 12, 2018 at 09:33:33PM +0200, Mathieu Malaterre wrote:
> > In function ‘radeon_process_i2c_ch’ a comparison of a u8 value against
> > 255 is done. Since it is always false, change the signature of this
> > function to use an `int` instead, which match the type used in caller:
> > `radeon_atom_hw_i2c_xfer`.
> >
> > Fix the following warning triggered with W=1:
> >
> >   CC [M]  drivers/gpu/drm/radeon/atombios_i2c.o
> >   drivers/gpu/drm/radeon/atombios_i2c.c: In function
> ‘radeon_process_i2c_ch’:
> >   drivers/gpu/drm/radeon/atombios_i2c.c:71:11: warning: comparison is
> always false due to limited range of data type [-Wtype-limits]
> >if (num > ATOM_MAX_HW_I2C_READ) {
> >^
> >
> > Signed-off-by: Mathieu Malaterre 
>
> Reviewed-by: Huang Rui 
>
>
Any chance to be included in the next pull request ? thanks


> > ---
> >  drivers/gpu/drm/radeon/atombios_i2c.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/atombios_i2c.c
> b/drivers/gpu/drm/radeon/atombios_i2c.c
> > index 4157780585a0..9022e9af11a0 100644
> > --- a/drivers/gpu/drm/radeon/atombios_i2c.c
> > +++ b/drivers/gpu/drm/radeon/atombios_i2c.c
> > @@ -35,7 +35,7 @@
> >
> >  static int radeon_process_i2c_ch(struct radeon_i2c_chan *chan,
> >u8 slave_addr, u8 flags,
> > -  u8 *buf, u8 num)
> > +  u8 *buf, int num)
> >  {
> >   struct drm_device *dev = chan->dev;
> >   struct radeon_device *rdev = dev->dev_private;
> > --
> > 2.11.0
> >
> > ___
> > 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

Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

[James Zhu]

On 2018-09-17 02:07 PM, Christian König wrote:

Don't try to unreserve a BO we doesn't allocated.

Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel BOs

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 84d82d5382f9..c1387efc0c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
if (r)
return r;
  
-	amdgpu_bo_unreserve(*bo_ptr);

+   if (*bo_ptr)

Can we put this check inside ttm_bo_unreserve?
James Zhu

+   amdgpu_bo_unreserve(*bo_ptr);
  
  	return 0;

  }


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: Avoid fault when allocating an empty buffer object

[Tom St Denis]

On 2018-09-17 2:08 p.m., Christian König wrote:

Am 17.09.2018 um 20:07 schrieb Tom St Denis:

Signed-off-by: Tom St Denis 


Reviewed-by: Christian König 

You was faster than me :)


Been a while since I got a patch on the kernel side... need to keep up 
numbers :-)


Tom




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 84d82d5382f9..c1387efc0c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device 
*adev,

  if (r)
  return r;
-    amdgpu_bo_unreserve(*bo_ptr);
+    if (*bo_ptr)
+    amdgpu_bo_unreserve(*bo_ptr);
  return 0;
  }




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: Avoid fault when allocating an empty buffer object

[Christian König]

Am 17.09.2018 um 20:07 schrieb Tom St Denis:

Signed-off-by: Tom St Denis 


Reviewed-by: Christian König 

You was faster than me :)


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 84d82d5382f9..c1387efc0c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
if (r)
return r;
  
-	amdgpu_bo_unreserve(*bo_ptr);

+   if (*bo_ptr)
+   amdgpu_bo_unreserve(*bo_ptr);
  
  	return 0;

  }


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: ttm crash on init

[Tom St Denis]

I've sent a patch to the list that fixes the bug on my end.

Cheers,

Tom

On 2018-09-17 2:01 p.m., Tom St Denis wrote:

On 2018-09-17 1:55 p.m., Christian König wrote:

Am 17.09.2018 um 19:50 schrieb Tom St Denis:

On 2018-09-17 1:45 p.m., Christian König wrote:

Mhm, not the slightest idea.

That nearly looks like adev->stolen_vga_memory already contains 
something.


Nope,

[   51.564605] >>>adev->stolen_vga_memory == (null)
[   51.564619] kasan: CONFIG_KASAN_INLINE enabled
[   51.564877] kasan: GPF could be caused by NULL-ptr deref or user 
memory access
[   51.565071] general protection fault:  [#1] SMP 
DEBUG_PAGEALLOC KASAN NOPTI
[   51.565254] CPU: 6 PID: 3863 Comm: modprobe Not tainted 
4.19.0-rc1+ #30
[   51.565425] Hardware name: System manufacturer System Product 
Name/TUF B350M-PLUS GAMING, BIOS 4011 04/19/2018

[   51.565714] RIP: 0010:amdgpu_bo_create_kernel+0x59/0x1a0 [amdgpu]

That's me printing out the value of the value for stolen_vga_memory 
before the call to allocate it.


What does amdgpu_bo_create_kernel+0x59 points to?


I've never really got line numbers to work with the kernel but if I had 
to guess I'd say right here


int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
     unsigned long size, int align,
     u32 domain, struct amdgpu_bo **bo_ptr,
     u64 *gpu_addr, void **cpu_addr)
{
 int r;

 r = amdgpu_bo_create_reserved(adev, size, align, domain, bo_ptr,
   gpu_addr, cpu_addr);

 if (r)
     return r;

*bo_ptr is NULL ===>    amdgpu_bo_unreserve(*bo_ptr);

 return 0;
}

Which then results in

static inline void amdgpu_bo_unreserve(struct amdgpu_bo *bo)
{
 ttm_bo_unreserve(>tbo);
}

Which then passes the address NULL + offsetof(tbo) to ttm_bo_unreserve:

static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo)
{
     if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
     spin_lock(>bdev->glob->lru_lock);
     ttm_bo_add_to_lru(bo);
     spin_unlock(>bdev->glob->lru_lock);
     }
     reservation_object_unlock(bo->resv);
}


Which likely faults on reading bo->mem.placement since the address is 
bogus.


The report is from amdgpu_bo_create_kernel because everything is a macro 
or inlined... :-)


Tom



Christian.



Tom




Christian.

Am 17.09.2018 um 18:47 schrieb Tom St Denis:

On 2018-09-17 12:21 p.m., Tom St Denis wrote:
(attached).  I'll try to bisect in a second.  Is anyone aware of 
this?


Tom


Bisection led to:

a327772a5655ff4fb104c8aae6515faa461df466 is the first bad commit
commit a327772a5655ff4fb104c8aae6515faa461df466
Author: Christian König 
Date:   Fri Sep 14 21:06:50 2018 +0200

    drm/amdgpu: drop size check

    We no don't allocate zero sized kernel BOs any longer.

    Signed-off-by: Christian König 
    Reviewed-by: Alex Deucher 

:04 04 265e4fa231d367d354e4c66600b8f98a4d2f04c4 
3702baaeb2423361dcd7eac8c533edace760ae3e M  drivers



As the culprit.

Cheers,
Tom




___
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

Re: ttm crash on init

[Christian König]

Am 17.09.2018 um 20:01 schrieb Tom St Denis:

On 2018-09-17 1:55 p.m., Christian König wrote:

Am 17.09.2018 um 19:50 schrieb Tom St Denis:

On 2018-09-17 1:45 p.m., Christian König wrote:

Mhm, not the slightest idea.

That nearly looks like adev->stolen_vga_memory already contains 
something.


Nope,

[   51.564605] >>>adev->stolen_vga_memory == (null)
[   51.564619] kasan: CONFIG_KASAN_INLINE enabled
[   51.564877] kasan: GPF could be caused by NULL-ptr deref or user 
memory access
[   51.565071] general protection fault:  [#1] SMP 
DEBUG_PAGEALLOC KASAN NOPTI
[   51.565254] CPU: 6 PID: 3863 Comm: modprobe Not tainted 
4.19.0-rc1+ #30
[   51.565425] Hardware name: System manufacturer System Product 
Name/TUF B350M-PLUS GAMING, BIOS 4011 04/19/2018

[   51.565714] RIP: 0010:amdgpu_bo_create_kernel+0x59/0x1a0 [amdgpu]

That's me printing out the value of the value for stolen_vga_memory 
before the call to allocate it.


What does amdgpu_bo_create_kernel+0x59 points to?


I've never really got line numbers to work with the kernel but if I 
had to guess I'd say right here


int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
    unsigned long size, int align,
    u32 domain, struct amdgpu_bo **bo_ptr,
    u64 *gpu_addr, void **cpu_addr)
{
int r;

r = amdgpu_bo_create_reserved(adev, size, align, domain, bo_ptr,
  gpu_addr, cpu_addr);

if (r)
    return r;

*bo_ptr is NULL ===>    amdgpu_bo_unreserve(*bo_ptr);


Ah, of course! Thanks for pointing out the obvious, totally forgot that 
there is still another function in the call chain.


Patch to fix is on the list,
Christian.



return 0;
}

Which then results in

static inline void amdgpu_bo_unreserve(struct amdgpu_bo *bo)
{
ttm_bo_unreserve(>tbo);
}

Which then passes the address NULL + offsetof(tbo) to ttm_bo_unreserve:

static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo)
{
    if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
    spin_lock(>bdev->glob->lru_lock);
    ttm_bo_add_to_lru(bo);
spin_unlock(>bdev->glob->lru_lock);
    }
    reservation_object_unlock(bo->resv);
}


Which likely faults on reading bo->mem.placement since the address is 
bogus.


The report is from amdgpu_bo_create_kernel because everything is a 
macro or inlined... :-)


Tom



Christian.



Tom




Christian.

Am 17.09.2018 um 18:47 schrieb Tom St Denis:

On 2018-09-17 12:21 p.m., Tom St Denis wrote:
(attached).  I'll try to bisect in a second.  Is anyone aware of 
this?


Tom


Bisection led to:

a327772a5655ff4fb104c8aae6515faa461df466 is the first bad commit
commit a327772a5655ff4fb104c8aae6515faa461df466
Author: Christian König 
Date:   Fri Sep 14 21:06:50 2018 +0200

    drm/amdgpu: drop size check

    We no don't allocate zero sized kernel BOs any longer.

    Signed-off-by: Christian König 
    Reviewed-by: Alex Deucher 

:04 04 265e4fa231d367d354e4c66600b8f98a4d2f04c4 
3702baaeb2423361dcd7eac8c533edace760ae3e M  drivers



As the culprit.

Cheers,
Tom




___
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


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: don't try to unreserve NULL pointer

[Christian König]

Don't try to unreserve a BO we doesn't allocated.

Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel BOs

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 84d82d5382f9..c1387efc0c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
if (r)
return r;
 
-   amdgpu_bo_unreserve(*bo_ptr);
+   if (*bo_ptr)
+   amdgpu_bo_unreserve(*bo_ptr);
 
return 0;
 }
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/amdgpu: Avoid fault when allocating an empty buffer object

[Tom St Denis]

Signed-off-by: Tom St Denis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 84d82d5382f9..c1387efc0c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
if (r)
return r;
 
-   amdgpu_bo_unreserve(*bo_ptr);
+   if (*bo_ptr)
+   amdgpu_bo_unreserve(*bo_ptr);
 
return 0;
 }
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: ttm crash on init

[Tom St Denis]

On 2018-09-17 1:55 p.m., Christian König wrote:

Am 17.09.2018 um 19:50 schrieb Tom St Denis:

On 2018-09-17 1:45 p.m., Christian König wrote:

Mhm, not the slightest idea.

That nearly looks like adev->stolen_vga_memory already contains 
something.


Nope,

[   51.564605] >>>adev->stolen_vga_memory == (null)
[   51.564619] kasan: CONFIG_KASAN_INLINE enabled
[   51.564877] kasan: GPF could be caused by NULL-ptr deref or user 
memory access
[   51.565071] general protection fault:  [#1] SMP DEBUG_PAGEALLOC 
KASAN NOPTI
[   51.565254] CPU: 6 PID: 3863 Comm: modprobe Not tainted 4.19.0-rc1+ 
#30
[   51.565425] Hardware name: System manufacturer System Product 
Name/TUF B350M-PLUS GAMING, BIOS 4011 04/19/2018

[   51.565714] RIP: 0010:amdgpu_bo_create_kernel+0x59/0x1a0 [amdgpu]

That's me printing out the value of the value for stolen_vga_memory 
before the call to allocate it.


What does amdgpu_bo_create_kernel+0x59 points to?


I've never really got line numbers to work with the kernel but if I had 
to guess I'd say right here


int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
unsigned long size, int align,
u32 domain, struct amdgpu_bo **bo_ptr,
u64 *gpu_addr, void **cpu_addr)
{
int r;

r = amdgpu_bo_create_reserved(adev, size, align, domain, bo_ptr,
  gpu_addr, cpu_addr);

if (r)
return r;

*bo_ptr is NULL ===> amdgpu_bo_unreserve(*bo_ptr);

return 0;
}

Which then results in

static inline void amdgpu_bo_unreserve(struct amdgpu_bo *bo)
{
ttm_bo_unreserve(>tbo);
}

Which then passes the address NULL + offsetof(tbo) to ttm_bo_unreserve:

static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo)
{
if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
spin_lock(>bdev->glob->lru_lock);
ttm_bo_add_to_lru(bo);
spin_unlock(>bdev->glob->lru_lock);
}
reservation_object_unlock(bo->resv);
}


Which likely faults on reading bo->mem.placement since the address is bogus.

The report is from amdgpu_bo_create_kernel because everything is a macro 
or inlined... :-)


Tom



Christian.



Tom




Christian.

Am 17.09.2018 um 18:47 schrieb Tom St Denis:

On 2018-09-17 12:21 p.m., Tom St Denis wrote:

(attached).  I'll try to bisect in a second.  Is anyone aware of this?

Tom


Bisection led to:

a327772a5655ff4fb104c8aae6515faa461df466 is the first bad commit
commit a327772a5655ff4fb104c8aae6515faa461df466
Author: Christian König 
Date:   Fri Sep 14 21:06:50 2018 +0200

    drm/amdgpu: drop size check

    We no don't allocate zero sized kernel BOs any longer.

    Signed-off-by: Christian König 
    Reviewed-by: Alex Deucher 

:04 04 265e4fa231d367d354e4c66600b8f98a4d2f04c4 
3702baaeb2423361dcd7eac8c533edace760ae3e M  drivers



As the culprit.

Cheers,
Tom




___
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

Re: ttm crash on init

[Christian König]

Am 17.09.2018 um 19:50 schrieb Tom St Denis:

On 2018-09-17 1:45 p.m., Christian König wrote:

Mhm, not the slightest idea.

That nearly looks like adev->stolen_vga_memory already contains 
something.


Nope,

[   51.564605] >>>adev->stolen_vga_memory == (null)
[   51.564619] kasan: CONFIG_KASAN_INLINE enabled
[   51.564877] kasan: GPF could be caused by NULL-ptr deref or user 
memory access
[   51.565071] general protection fault:  [#1] SMP DEBUG_PAGEALLOC 
KASAN NOPTI
[   51.565254] CPU: 6 PID: 3863 Comm: modprobe Not tainted 4.19.0-rc1+ 
#30
[   51.565425] Hardware name: System manufacturer System Product 
Name/TUF B350M-PLUS GAMING, BIOS 4011 04/19/2018

[   51.565714] RIP: 0010:amdgpu_bo_create_kernel+0x59/0x1a0 [amdgpu]

That's me printing out the value of the value for stolen_vga_memory 
before the call to allocate it.


What does amdgpu_bo_create_kernel+0x59 points to?

Christian.



Tom




Christian.

Am 17.09.2018 um 18:47 schrieb Tom St Denis:

On 2018-09-17 12:21 p.m., Tom St Denis wrote:

(attached).  I'll try to bisect in a second.  Is anyone aware of this?

Tom


Bisection led to:

a327772a5655ff4fb104c8aae6515faa461df466 is the first bad commit
commit a327772a5655ff4fb104c8aae6515faa461df466
Author: Christian König 
Date:   Fri Sep 14 21:06:50 2018 +0200

    drm/amdgpu: drop size check

    We no don't allocate zero sized kernel BOs any longer.

    Signed-off-by: Christian König 
    Reviewed-by: Alex Deucher 

:04 04 265e4fa231d367d354e4c66600b8f98a4d2f04c4 
3702baaeb2423361dcd7eac8c533edace760ae3e M  drivers



As the culprit.

Cheers,
Tom




___
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

Re: ttm crash on init

[Tom St Denis]

On 2018-09-17 1:45 p.m., Christian König wrote:

Mhm, not the slightest idea.

That nearly looks like adev->stolen_vga_memory already contains something.


Nope,

[   51.564605] >>>adev->stolen_vga_memory ==   (null)
[   51.564619] kasan: CONFIG_KASAN_INLINE enabled
[   51.564877] kasan: GPF could be caused by NULL-ptr deref or user 
memory access
[   51.565071] general protection fault:  [#1] SMP DEBUG_PAGEALLOC 
KASAN NOPTI

[   51.565254] CPU: 6 PID: 3863 Comm: modprobe Not tainted 4.19.0-rc1+ #30
[   51.565425] Hardware name: System manufacturer System Product 
Name/TUF B350M-PLUS GAMING, BIOS 4011 04/19/2018

[   51.565714] RIP: 0010:amdgpu_bo_create_kernel+0x59/0x1a0 [amdgpu]

That's me printing out the value of the value for stolen_vga_memory 
before the call to allocate it.


Tom




Christian.

Am 17.09.2018 um 18:47 schrieb Tom St Denis:

On 2018-09-17 12:21 p.m., Tom St Denis wrote:

(attached).  I'll try to bisect in a second.  Is anyone aware of this?

Tom


Bisection led to:

a327772a5655ff4fb104c8aae6515faa461df466 is the first bad commit
commit a327772a5655ff4fb104c8aae6515faa461df466
Author: Christian König 
Date:   Fri Sep 14 21:06:50 2018 +0200

    drm/amdgpu: drop size check

    We no don't allocate zero sized kernel BOs any longer.

    Signed-off-by: Christian König 
    Reviewed-by: Alex Deucher 

:04 04 265e4fa231d367d354e4c66600b8f98a4d2f04c4 
3702baaeb2423361dcd7eac8c533edace760ae3e M  drivers



As the culprit.

Cheers,
Tom




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: ttm crash on init

[Christian König]

Mhm, not the slightest idea.

That nearly looks like adev->stolen_vga_memory already contains something.

Christian.

Am 17.09.2018 um 18:47 schrieb Tom St Denis:

On 2018-09-17 12:21 p.m., Tom St Denis wrote:

(attached).  I'll try to bisect in a second.  Is anyone aware of this?

Tom


Bisection led to:

a327772a5655ff4fb104c8aae6515faa461df466 is the first bad commit
commit a327772a5655ff4fb104c8aae6515faa461df466
Author: Christian König 
Date:   Fri Sep 14 21:06:50 2018 +0200

    drm/amdgpu: drop size check

    We no don't allocate zero sized kernel BOs any longer.

    Signed-off-by: Christian König 
    Reviewed-by: Alex Deucher 

:04 04 265e4fa231d367d354e4c66600b8f98a4d2f04c4 
3702baaeb2423361dcd7eac8c533edace760ae3e M  drivers



As the culprit.

Cheers,
Tom


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: ttm crash on init

[Tom St Denis]

On 2018-09-17 12:21 p.m., Tom St Denis wrote:

(attached).  I'll try to bisect in a second.  Is anyone aware of this?

Tom


Bisection led to:

a327772a5655ff4fb104c8aae6515faa461df466 is the first bad commit
commit a327772a5655ff4fb104c8aae6515faa461df466
Author: Christian König 
Date:   Fri Sep 14 21:06:50 2018 +0200

drm/amdgpu: drop size check

We no don't allocate zero sized kernel BOs any longer.

Signed-off-by: Christian König 
Reviewed-by: Alex Deucher 

:04 04 265e4fa231d367d354e4c66600b8f98a4d2f04c4 
3702baaeb2423361dcd7eac8c533edace760ae3e M  drivers



As the culprit.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

ttm crash on init

[Tom St Denis]

(attached).  I'll try to bisect in a second.  Is anyone aware of this?

Tom
[0.00] Linux version 4.19.0-rc1+ (root@raven) (gcc version 8.1.1 20180712 (Red Hat 8.1.1-5) (GCC)) #29 SMP Fri Sep 14 07:30:30 EDT 2018
[0.00] Command line: BOOT_IMAGE=/vmlinuz-4.19.0-rc1+ root=UUID=66163c80-0ca1-4beb-aeba-5cc130b813e6 ro rhgb quiet modprobe.blacklist=amdgpu,radeon LANG=en_CA.UTF-8
[0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
[0.00] x86/fpu: xstate_offset[2]:  576, xstate_sizes[2]:  256
[0.00] x86/fpu: Enabled xstate features 0x7, context size is 832 bytes, using 'compacted' format.
[0.00] BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x0009d3ff] usable
[0.00] BIOS-e820: [mem 0x0009d400-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000e-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0x03ff] usable
[0.00] BIOS-e820: [mem 0x0400-0x04009fff] ACPI NVS
[0.00] BIOS-e820: [mem 0x0400a000-0x09bf] usable
[0.00] BIOS-e820: [mem 0x09c0-0x09ff] reserved
[0.00] BIOS-e820: [mem 0x0a00-0x0aff] usable
[0.00] BIOS-e820: [mem 0x0b00-0x0b01] reserved
[0.00] BIOS-e820: [mem 0x0b02-0x73963fff] usable
[0.00] BIOS-e820: [mem 0x73964000-0x7397cfff] ACPI data
[0.00] BIOS-e820: [mem 0x7397d000-0x7a5aafff] usable
[0.00] BIOS-e820: [mem 0x7a5ab000-0x7a6c2fff] reserved
[0.00] BIOS-e820: [mem 0x7a6c3000-0x7a6cefff] ACPI data
[0.00] BIOS-e820: [mem 0x7a6cf000-0x7a7d1fff] usable
[0.00] BIOS-e820: [mem 0x7a7d2000-0x7ab89fff] ACPI NVS
[0.00] BIOS-e820: [mem 0x7ab8a000-0x7b942fff] reserved
[0.00] BIOS-e820: [mem 0x7b943000-0x7dff] usable
[0.00] BIOS-e820: [mem 0x7e00-0xbfff] reserved
[0.00] BIOS-e820: [mem 0xf800-0xfbff] reserved
[0.00] BIOS-e820: [mem 0xfd80-0xfdff] reserved
[0.00] BIOS-e820: [mem 0xfea0-0xfea0] reserved
[0.00] BIOS-e820: [mem 0xfeb8-0xfec01fff] reserved
[0.00] BIOS-e820: [mem 0xfec1-0xfec10fff] reserved
[0.00] BIOS-e820: [mem 0xfec3-0xfec30fff] reserved
[0.00] BIOS-e820: [mem 0xfed0-0xfed00fff] reserved
[0.00] BIOS-e820: [mem 0xfed4-0xfed44fff] reserved
[0.00] BIOS-e820: [mem 0xfed8-0xfed8] reserved
[0.00] BIOS-e820: [mem 0xfedc2000-0xfedc] reserved
[0.00] BIOS-e820: [mem 0xfedd4000-0xfedd5fff] reserved
[0.00] BIOS-e820: [mem 0xfee0-0xfeef] reserved
[0.00] BIOS-e820: [mem 0xff00-0x] reserved
[0.00] BIOS-e820: [mem 0x0001-0x00023f33] usable
[0.00] NX (Execute Disable) protection: active
[0.00] SMBIOS 3.1.1 present.
[0.00] DMI: System manufacturer System Product Name/TUF B350M-PLUS GAMING, BIOS 4011 04/19/2018
[0.00] tsc: Fast TSC calibration failed
[0.00] e820: update [mem 0x-0x0fff] usable ==> reserved
[0.00] e820: remove [mem 0x000a-0x000f] usable
[0.00] last_pfn = 0x23f340 max_arch_pfn = 0x4
[0.00] MTRR default type: uncachable
[0.00] MTRR fixed ranges enabled:
[0.00]   0-9 write-back
[0.00]   A-B write-through
[0.00]   C-F write-protect
[0.00] MTRR variable ranges enabled:
[0.00]   0 base  mask 8000 write-back
[0.00]   1 base 8000 mask C000 write-back
[0.00]   2 disabled
[0.00]   3 disabled
[0.00]   4 disabled
[0.00]   5 disabled
[0.00]   6 disabled
[0.00]   7 disabled
[0.00] TOM2: 00024000 aka 9216M
[0.00] x86/PAT: Configuration [0-7]: WB  WC  UC- UC  WB  WP  UC- WT  
[0.00] e820: update [mem 0xc000-0x] usable ==> reserved
[0.00] last_pfn = 0x7e000 max_arch_pfn = 0x4
[0.00] Scanning 1 areas for low memory corruption
[0.00] Base memory trampoline at [(ptrval)] 97000 size 24576
[0.00] BRK [0x1f83bb000, 0x1f83bbfff] PGTABLE
[0.00] BRK [0x1f83bc000, 0x1f83bcfff] PGTABLE
[0.00] BRK [0x1f83bd000, 0x1f83bdfff] 

RE: [PATCH 1/3] drm/amd/powerplay: update OD feature judgement

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx  On Behalf Of Evan
> Quan
> Sent: Monday, September 17, 2018 3:14 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan 
> Subject: [PATCH 1/3] drm/amd/powerplay: update OD feature judgement
> 
> Update the conditions to judge whether an OD feature should be supported
> on vega20.
> 
> Change-Id: Iaabdd4db8f685fb94c960263fe38a21b36377aa2
> Signed-off-by: Evan Quan 

Series is:
Acked-by: Alex Deucher 

> ---
>  .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 81 +---
> ---
>  .../drm/amd/powerplay/hwmgr/vega20_pptable.h  |  2 +
>  2 files changed, 57 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index ce123096c365..6295244a1737 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -47,6 +47,8 @@
>  #include "pp_overdriver.h"
>  #include "pp_thermal.h"
> 
> +#define VOLTAGE_SCALE 4
> +
>  static void vega20_set_default_registry_data(struct pp_hwmgr *hwmgr)  {
>   struct vega20_hwmgr *data =
> @@ -832,58 +834,85 @@ static int vega20_od8_set_feature_capabilities(
>   struct phm_ppt_v3_information *pptable_information =
>   (struct phm_ppt_v3_information *)hwmgr->pptable;
>   struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr-
> >backend);
> + PPTable_t *pp_table = &(data->smc_state_table.pp_table);
>   struct vega20_od8_settings *od_settings = &(data->od8_settings);
> 
>   od_settings->overdrive8_capabilities = 0;
> 
>   if (data->smu_features[GNLD_DPM_GFXCLK].enabled) {
> - if (pptable_information-
> >od_settings_max[ATOM_VEGA20_ODSETTING_GFXCLKFMAX] > 0 &&
> - pptable_information-
> >od_settings_min[ATOM_VEGA20_ODSETTING_GFXCLKFMAX] > 0 &&
> - pptable_information-
> >od_settings_max[ATOM_VEGA20_ODSETTING_GFXCLKFMIN] > 0 &&
> - pptable_information-
> >od_settings_min[ATOM_VEGA20_ODSETTING_GFXCLKFMIN] > 0)
> + if (pptable_information-
> >od_feature_capabilities[ATOM_VEGA20_ODFEATURE_GFXCLK_LIMITS] &&
> + pptable_information-
> >od_settings_max[OD8_SETTING_GFXCLK_FMAX] > 0 &&
> + pptable_information-
> >od_settings_min[OD8_SETTING_GFXCLK_FMIN] > 0 &&
> + (pptable_information-
> >od_settings_max[OD8_SETTING_GFXCLK_FMAX] >=
> + pptable_information-
> >od_settings_min[OD8_SETTING_GFXCLK_FMIN]))
>   od_settings->overdrive8_capabilities |=
> OD8_GFXCLK_LIMITS;
> 
> - if (pptable_information-
> >od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P1] >
> 0 &&
> - pptable_information-
> >od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P2] >
> 0 &&
> - pptable_information-
> >od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P3] >
> 0 &&
> - pptable_information-
> >od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P1] >
> 0 &&
> - pptable_information-
> >od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P2] >
> 0 &&
> - pptable_information-
> >od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P3] >
> 0 &&
> - pptable_information-
> >od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEOF
> FSET_P1] > 0 &&
> - pptable_information-
> >od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEOF
> FSET_P2] > 0 &&
> - pptable_information-
> >od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEOF
> FSET_P3] > 0 &&
> - pptable_information-
> >od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEO
> FFSET_P1] > 0 &&
> - pptable_information-
> >od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEO
> FFSET_P2] > 0 &&
> - pptable_information-
> >od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEO
> FFSET_P3] > 0)
> + if (pptable_information-
> >od_feature_capabilities[ATOM_VEGA20_ODFEATURE_GFXCLK_CURVE] &&
> + (pptable_information-
> >od_settings_min[OD8_SETTING_GFXCLK_VOLTAGE1] >=
> +  pp_table->MinVoltageGfx / VOLTAGE_SCALE) &&
> + (pptable_information-
> >od_settings_max[OD8_SETTING_GFXCLK_VOLTAGE3] <=
> +  pp_table->MaxVoltageGfx / VOLTAGE_SCALE) &&
> + (pptable_information-
> >od_settings_max[OD8_SETTING_GFXCLK_VOLTAGE3] >=
> +
> +pptable_information-
> >od_settings_min[OD8_SETTING_GFXCLK_VOLTAGE1]))
>   od_settings->overdrive8_capabilities |=
> OD8_GFXCLK_CURVE;
>   }
> 
>   if (data->smu_features[GNLD_DPM_UCLK].enabled) {
> - if (pptable_information-
> >od_settings_min[ATOM_VEGA20_ODSETTING_UCLKFMAX] > 0 &&
> - pptable_information-
> >od_settings_max[ATOM_VEGA20_ODSETTING_UCLKFMAX] > 0)
> + if 

RE: [PATCH 2/7] drm/amdgpu: fix up GDS/GWS/OA shifting

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx  On Behalf Of
> Christian König
> Sent: Saturday, September 15, 2018 3:52 AM
> To: Alex Deucher 
> Cc: amd-gfx list 
> Subject: Re: [PATCH 2/7] drm/amdgpu: fix up GDS/GWS/OA shifting
> 
> Am 14.09.2018 um 22:26 schrieb Alex Deucher:
> > On Fri, Sep 14, 2018 at 3:12 PM Christian König
> >  wrote:
> >> That only worked by pure coincident. Completely remove the shifting
> >> and always apply correct PAGE_SHIFT.
> >>
> >> Signed-off-by: Christian König 
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 12 ++--
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h|  7 ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 12 +++-
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 14 +++---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  6 +-
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 15 +++
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  9 -
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  |  9 -
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 12 +---
> >>   9 files changed, 25 insertions(+), 71 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >> index d762d78e5102..8836186eb5ef 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >> @@ -721,16 +721,16 @@ static int amdgpu_cs_parser_bos(struct
> amdgpu_cs_parser *p,
> >>  e->bo_va = amdgpu_vm_bo_find(vm,
> >> ttm_to_amdgpu_bo(e->tv.bo));
> >>
> >>  if (gds) {
> >> -   p->job->gds_base = amdgpu_bo_gpu_offset(gds);
> >> -   p->job->gds_size = amdgpu_bo_size(gds);
> >> +   p->job->gds_base = amdgpu_bo_gpu_offset(gds) >>
> PAGE_SHIFT;
> >> +   p->job->gds_size = amdgpu_bo_size(gds) >> PAGE_SHIFT;
> >>  }
> >>  if (gws) {
> >> -   p->job->gws_base = amdgpu_bo_gpu_offset(gws);
> >> -   p->job->gws_size = amdgpu_bo_size(gws);
> >> +   p->job->gws_base = amdgpu_bo_gpu_offset(gws) >>
> PAGE_SHIFT;
> >> +   p->job->gws_size = amdgpu_bo_size(gws) >> PAGE_SHIFT;
> >>  }
> >>  if (oa) {
> >> -   p->job->oa_base = amdgpu_bo_gpu_offset(oa);
> >> -   p->job->oa_size = amdgpu_bo_size(oa);
> >> +   p->job->oa_base = amdgpu_bo_gpu_offset(oa) >> PAGE_SHIFT;
> >> +   p->job->oa_size = amdgpu_bo_size(oa) >> PAGE_SHIFT;
> >>  }
> >>
> >>  if (!r && p->uf_entry.tv.bo) { diff --git
> >> a/drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h
> >> index e73728d90388..ecbcefe49a98 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h
> >> @@ -24,13 +24,6 @@
> >>   #ifndef __AMDGPU_GDS_H__
> >>   #define __AMDGPU_GDS_H__
> >>
> >> -/* Because TTM request that alloacted buffer should be PAGE_SIZE
> >> aligned,
> >> - * we should report GDS/GWS/OA size as PAGE_SIZE aligned
> >> - * */
> >> -#define AMDGPU_GDS_SHIFT   2
> >> -#define AMDGPU_GWS_SHIFT   PAGE_SHIFT
> >> -#define AMDGPU_OA_SHIFTPAGE_SHIFT
> >> -
> >>   struct amdgpu_ring;
> >>   struct amdgpu_bo;
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >> index d30a0838851b..7b3d1ebda9df 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >> @@ -244,16 +244,10 @@ int amdgpu_gem_create_ioctl(struct
> drm_device *dev, void *data,
> >>  return -EINVAL;
> >>  }
> >>  flags |= AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
> >> -   if (args->in.domains == AMDGPU_GEM_DOMAIN_GDS)
> >> -   size = size << AMDGPU_GDS_SHIFT;
> >> -   else if (args->in.domains == AMDGPU_GEM_DOMAIN_GWS)
> >> -   size = size << AMDGPU_GWS_SHIFT;
> >> -   else if (args->in.domains == AMDGPU_GEM_DOMAIN_OA)
> >> -   size = size << AMDGPU_OA_SHIFT;
> >> -   else
> >> -   return -EINVAL;
> >> +   /* GDS allocations must be DW aligned */
> >> +   if (args->in.domains & AMDGPU_GEM_DOMAIN_GDS)
> >> +   size = ALIGN(size, 4);
> >>  }
> >> -   size = roundup(size, PAGE_SIZE);
> >>
> >>  if (flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID) {
> >>  r = amdgpu_bo_reserve(vm->root.base.bo, false); diff
> >> --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> index b766270d86cb..64cc483db973 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> @@ -528,13 +528,13 @@ static int amdgpu_info_ioctl(struct drm_device
> *dev, 

Re: [PATCH] drm/amd/dc: Trigger set power state task when display configuration changes

[Francis, David]

On Linux, the clocks are already set by the time that function call is reached. 
 amdgpu_pm_compute clocks resets the clocks to their default value.


From: Zhu, Rex
Sent: September 17, 2018 4:12:33 AM
To: Deucher, Alexander; Wentland, Harry; Francis, David
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amd/dc: Trigger set power state task when display 
configuration changes


+Harry and David.



Best Regards

Rex



From: Deucher, Alexander
Sent: Friday, September 14, 2018 9:40 PM
To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/dc: Trigger set power state task when display 
configuration changes



Acked-by: Alex Deucher 
mailto:alexander.deuc...@amd.com>>



From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Rex Zhu mailto:rex@amd.com>>
Sent: Friday, September 14, 2018 1:57:07 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhu, Rex
Subject: [PATCH] drm/amd/dc: Trigger set power state task when display 
configuration changes



Revert "drm/amd/display: Remove call to amdgpu_pm_compute_clocks"

This reverts commit dcd473770e86517543691bdb227103d6c781cd0a.

when display configuration changes, dc need to update the changes
to powerplay, also need to trigger a power state task.
amdgpu_pm_compute_clocks is the interface to set power state task
either dpm enabled or powerplay enabled

Signed-off-by: Rex Zhu mailto:rex@amd.com>>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
index 6d16b4a..0fab64a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
@@ -105,6 +105,8 @@ bool dm_pp_apply_display_requirements(
 adev->powerplay.pp_funcs->display_configuration_change(
 adev->powerplay.pp_handle,
 >pm.pm_display_cfg);
+
+   amdgpu_pm_compute_clocks(adev);
 }

 return true;
--
1.9.1

___
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

[PATCH 1/2] drm/amdgpu: update vram_info structure in atomfirmware.h

[Hawking Zhang]

atomfirmware has structure changes in varm_info. Updated it
to the latest one.

Change-Id: Ie5d60413e5db1dfb4aaf23dc94bc5fd4ed0a01cd
Signed-off-by: Hawking Zhang 
Reviewed-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c |  2 +-
 drivers/gpu/drm/amd/include/atomfirmware.h   | 20 +++-
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index 2369158..5461d0d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -174,7 +174,7 @@ static int convert_atom_mem_type_to_vram_type (struct 
amdgpu_device *adev,
case ATOM_DGPU_VRAM_TYPE_GDDR5:
vram_type = AMDGPU_VRAM_TYPE_GDDR5;
break;
-   case ATOM_DGPU_VRAM_TYPE_HBM:
+   case ATOM_DGPU_VRAM_TYPE_HBM2:
vram_type = AMDGPU_VRAM_TYPE_HBM;
break;
default:
diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h 
b/drivers/gpu/drm/amd/include/atomfirmware.h
index 6109a45..8ae7adb 100644
--- a/drivers/gpu/drm/amd/include/atomfirmware.h
+++ b/drivers/gpu/drm/amd/include/atomfirmware.h
@@ -179,7 +179,7 @@ enum atom_voltage_type
 
 enum atom_dgpu_vram_type{
   ATOM_DGPU_VRAM_TYPE_GDDR5 = 0x50,
-  ATOM_DGPU_VRAM_TYPE_HBM   = 0x60,
+  ATOM_DGPU_VRAM_TYPE_HBM2  = 0x60,
 };
 
 enum atom_dp_vs_preemph_def{
@@ -1699,10 +1699,10 @@ struct atom_vram_module_v9
 {
   // Design Specific Values
   uint32_t  memory_size;   // Total memory size in unit of MB 
for CONFIG_MEMSIZE zeros
-  uint32_t  channel_enable;// for 32 channel ASIC usage
-  uint32_t  umcch_addrcfg;
-  uint32_t  umcch_addrsel;
-  uint32_t  umcch_colsel;
+  uint32_t  channel_enable;// bit vector, each bit indicate 
specific channel enable or not
+  uint32_t  max_mem_clk;   // max memory clock of this memory 
in unit of 10kHz, =0 means it is not defined
+  uint16_t  reserved[3];
+  uint16_t  mem_voltage;   // mem_voltage
   uint16_t  vram_module_size;  // Size of atom_vram_module_v9
   uint8_t   ext_memory_id; // Current memory module ID
   uint8_t   memory_type;   // enum of atom_dgpu_vram_type
@@ -1712,20 +1712,22 @@ struct atom_vram_module_v9
   uint8_t   tunningset_id; // MC phy registers set per. 
   uint8_t   vender_rev_id; // [7:4] Revision, [3:0] Vendor code
   uint8_t   refreshrate;   // [1:0]=RefreshFactor (00=8ms, 
01=16ms, 10=32ms,11=64ms)
-  uint16_t  vram_rsd2; // reserved
+  uint8_t   hbm_ven_rev_id;   // hbm_ven_rev_id
+  uint8_t   vram_rsd2;// reserved
   chardram_pnstring[20];   // part number end with '0'. 
 };
 
-
 struct atom_vram_info_header_v2_3
 {
-  struct   atom_common_table_header  table_header;
+  struct   atom_common_table_header table_header;
   uint16_t mem_adjust_tbloffset; // offset of 
atom_umc_init_reg_block structure for memory vendor specific UMC adjust setting
   uint16_t mem_clk_patch_tbloffset;  // offset of 
atom_umc_init_reg_block structure for memory clock specific UMC setting
   uint16_t mc_adjust_pertile_tbloffset;  // offset of 
atom_umc_init_reg_block structure for Per Byte Offset Preset Settings
   uint16_t mc_phyinit_tbloffset; // offset of 
atom_umc_init_reg_block structure for MC phy init set
   uint16_t dram_data_remap_tbloffset;// reserved for now
-  uint16_t vram_rsd2[3];
+  uint16_t tmrs_seq_offset;  // offset of HBM tmrs
+  uint16_t post_ucode_init_offset;   // offset of 
atom_umc_init_reg_block structure for MC phy init after MC uCode complete umc 
init
+  uint16_t vram_rsd2;
   uint8_t  vram_module_num;  // indicate number of 
VRAM module
   uint8_t  vram_rsd1[2];
   uint8_t  mc_phy_tile_num;  // indicate the MCD 
tile number which use in DramDataRemapTbl and usMcAdjustPerTileTblOffset
-- 
2.7.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdgpu: fix unknown vram mem type for vega20

[Hawking Zhang]

vega20 should use umc_info v3_3 instead of v3_1. There are
serveral versions of umc_info for vega series. Compared to
various versions of these structures, vram_info strucure is
unified for vega series. The patch switch to query mem_type
from vram_info structure for all the vega series dGPU.

Change-Id: If8d22b687ec5d0f4445527e69841df83479cc485
Signed-off-by: Hawking Zhang 
Reviewed-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index 5461d0d..b61e1dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -117,6 +117,10 @@ union igp_info {
 union umc_info {
struct atom_umc_info_v3_1 v31;
 };
+
+union vram_info {
+   struct atom_vram_info_header_v2_3 v23;
+};
 /*
  * Return vram width from integrated system info table, if available,
  * or 0 if not.
@@ -195,7 +199,7 @@ int amdgpu_atomfirmware_get_vram_type(struct amdgpu_device 
*adev)
int index;
u16 data_offset, size;
union igp_info *igp_info;
-   union umc_info *umc_info;
+   union vram_info *vram_info;
u8 frev, crev;
u8 mem_type;
 
@@ -204,7 +208,7 @@ int amdgpu_atomfirmware_get_vram_type(struct amdgpu_device 
*adev)
integratedsysteminfo);
else
index = 
get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
-   umc_info);
+   vram_info);
if (amdgpu_atom_parse_data_header(mode_info->atom_context,
  index, ,
  , , _offset)) {
@@ -219,11 +223,11 @@ int amdgpu_atomfirmware_get_vram_type(struct 
amdgpu_device *adev)
return 0;
}
} else {
-   umc_info = (union umc_info *)
+   vram_info = (union vram_info *)
(mode_info->atom_context->bios + data_offset);
switch (crev) {
-   case 1:
-   mem_type = umc_info->v31.vram_type;
+   case 3:
+   mem_type = 
vram_info->v23.vram_module[0].memory_type;
return convert_atom_mem_type_to_vram_type(adev, 
mem_type);
default:
return 0;
-- 
2.7.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/7] drm/amdgpu: fix up GDS/GWS/OA shifting

[Christian König]

Am 15.09.2018 um 09:52 schrieb Christian König:

Am 14.09.2018 um 22:26 schrieb Alex Deucher:

On Fri, Sep 14, 2018 at 3:12 PM Christian König
 wrote:

That only worked by pure coincident. Completely remove the shifting and
always apply correct PAGE_SHIFT.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 12 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h    |  7 ---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 14 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  6 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 15 +++
  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  9 -
  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  |  9 -
  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 12 +---
  9 files changed, 25 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

index d762d78e5102..8836186eb5ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -721,16 +721,16 @@ static int amdgpu_cs_parser_bos(struct 
amdgpu_cs_parser *p,
 e->bo_va = amdgpu_vm_bo_find(vm, 
ttm_to_amdgpu_bo(e->tv.bo));


 if (gds) {
-   p->job->gds_base = amdgpu_bo_gpu_offset(gds);
-   p->job->gds_size = amdgpu_bo_size(gds);
+   p->job->gds_base = amdgpu_bo_gpu_offset(gds) >> 
PAGE_SHIFT;

+   p->job->gds_size = amdgpu_bo_size(gds) >> PAGE_SHIFT;
 }
 if (gws) {
-   p->job->gws_base = amdgpu_bo_gpu_offset(gws);
-   p->job->gws_size = amdgpu_bo_size(gws);
+   p->job->gws_base = amdgpu_bo_gpu_offset(gws) >> 
PAGE_SHIFT;

+   p->job->gws_size = amdgpu_bo_size(gws) >> PAGE_SHIFT;
 }
 if (oa) {
-   p->job->oa_base = amdgpu_bo_gpu_offset(oa);
-   p->job->oa_size = amdgpu_bo_size(oa);
+   p->job->oa_base = amdgpu_bo_gpu_offset(oa) >> 
PAGE_SHIFT;

+   p->job->oa_size = amdgpu_bo_size(oa) >> PAGE_SHIFT;
 }

 if (!r && p->uf_entry.tv.bo) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h

index e73728d90388..ecbcefe49a98 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h
@@ -24,13 +24,6 @@
  #ifndef __AMDGPU_GDS_H__
  #define __AMDGPU_GDS_H__

-/* Because TTM request that alloacted buffer should be PAGE_SIZE 
aligned,

- * we should report GDS/GWS/OA size as PAGE_SIZE aligned
- * */
-#define AMDGPU_GDS_SHIFT   2
-#define AMDGPU_GWS_SHIFT   PAGE_SHIFT
-#define AMDGPU_OA_SHIFT    PAGE_SHIFT
-
  struct amdgpu_ring;
  struct amdgpu_bo;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

index d30a0838851b..7b3d1ebda9df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -244,16 +244,10 @@ int amdgpu_gem_create_ioctl(struct drm_device 
*dev, void *data,

 return -EINVAL;
 }
 flags |= AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
-   if (args->in.domains == AMDGPU_GEM_DOMAIN_GDS)
-   size = size << AMDGPU_GDS_SHIFT;
-   else if (args->in.domains == AMDGPU_GEM_DOMAIN_GWS)
-   size = size << AMDGPU_GWS_SHIFT;
-   else if (args->in.domains == AMDGPU_GEM_DOMAIN_OA)
-   size = size << AMDGPU_OA_SHIFT;
-   else
-   return -EINVAL;
+   /* GDS allocations must be DW aligned */
+   if (args->in.domains & AMDGPU_GEM_DOMAIN_GDS)
+   size = ALIGN(size, 4);
 }
-   size = roundup(size, PAGE_SIZE);

 if (flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID) {
 r = amdgpu_bo_reserve(vm->root.base.bo, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

index b766270d86cb..64cc483db973 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -528,13 +528,13 @@ static int amdgpu_info_ioctl(struct drm_device 
*dev, void *data, struct drm_file

 struct drm_amdgpu_info_gds gds_info;

 memset(_info, 0, sizeof(gds_info));
-   gds_info.gds_gfx_partition_size = 
adev->gds.mem.gfx_partition_size >> AMDGPU_GDS_SHIFT;
-   gds_info.compute_partition_size = 
adev->gds.mem.cs_partition_size >> AMDGPU_GDS_SHIFT;
-   gds_info.gds_total_size = adev->gds.mem.total_size 
>> AMDGPU_GDS_SHIFT;
-   gds_info.gws_per_gfx_partition = 
adev->gds.gws.gfx_partition_size >> AMDGPU_GWS_SHIFT;
-   gds_info.gws_per_compute_partition = 
adev->gds.gws.cs_partition_size >> AMDGPU_GWS_SHIFT;
- 

Re: [PATCH 4/7] drm/amdgpu: initialize GDS/GWS/OA domains even when they are zero sized

[Christian König]

Am 14.09.2018 um 22:14 schrieb Alex Deucher:

On Fri, Sep 14, 2018 at 3:12 PM Christian König
 wrote:

Stops crashing on SI.

Signed-off-by: Christian König 

Presumably ttm allows this?


Yes, actually explicitly tested this.

Christian.


Acked-by: Alex Deucher 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 48 +
  1 file changed, 18 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3e450159fe1f..710e7751c567 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1843,34 +1843,25 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
  (unsigned)(gtt_size / (1024 * 1024)));

 /* Initialize various on-chip memory pools */
-   /* GDS Memory */
-   if (adev->gds.mem.total_size) {
-   r = ttm_bo_init_mm(>mman.bdev, AMDGPU_PL_GDS,
-  adev->gds.mem.total_size);
-   if (r) {
-   DRM_ERROR("Failed initializing GDS heap.\n");
-   return r;
-   }
+   r = ttm_bo_init_mm(>mman.bdev, AMDGPU_PL_GDS,
+  adev->gds.mem.total_size);
+   if (r) {
+   DRM_ERROR("Failed initializing GDS heap.\n");
+   return r;
 }

-   /* GWS */
-   if (adev->gds.gws.total_size) {
-   r = ttm_bo_init_mm(>mman.bdev, AMDGPU_PL_GWS,
-  adev->gds.gws.total_size);
-   if (r) {
-   DRM_ERROR("Failed initializing gws heap.\n");
-   return r;
-   }
+   r = ttm_bo_init_mm(>mman.bdev, AMDGPU_PL_GWS,
+  adev->gds.gws.total_size);
+   if (r) {
+   DRM_ERROR("Failed initializing gws heap.\n");
+   return r;
 }

-   /* OA */
-   if (adev->gds.oa.total_size) {
-   r = ttm_bo_init_mm(>mman.bdev, AMDGPU_PL_OA,
-  adev->gds.oa.total_size);
-   if (r) {
-   DRM_ERROR("Failed initializing oa heap.\n");
-   return r;
-   }
+   r = ttm_bo_init_mm(>mman.bdev, AMDGPU_PL_OA,
+  adev->gds.oa.total_size);
+   if (r) {
+   DRM_ERROR("Failed initializing oa heap.\n");
+   return r;
 }

 /* Register debugfs entries for amdgpu_ttm */
@@ -1907,12 +1898,9 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)

 ttm_bo_clean_mm(>mman.bdev, TTM_PL_VRAM);
 ttm_bo_clean_mm(>mman.bdev, TTM_PL_TT);
-   if (adev->gds.mem.total_size)
-   ttm_bo_clean_mm(>mman.bdev, AMDGPU_PL_GDS);
-   if (adev->gds.gws.total_size)
-   ttm_bo_clean_mm(>mman.bdev, AMDGPU_PL_GWS);
-   if (adev->gds.oa.total_size)
-   ttm_bo_clean_mm(>mman.bdev, AMDGPU_PL_OA);
+   ttm_bo_clean_mm(>mman.bdev, AMDGPU_PL_GDS);
+   ttm_bo_clean_mm(>mman.bdev, AMDGPU_PL_GWS);
+   ttm_bo_clean_mm(>mman.bdev, AMDGPU_PL_OA);
 ttm_bo_device_release(>mman.bdev);
 amdgpu_ttm_global_fini(adev);
 adev->mman.initialized = false;
--
2.14.1

___
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

[PATCH] list: introduce list_bulk_move_tail helper

[Christian König]

Move all entries between @first and including @last before @head.

This is useful for LRU lists where a whole block of entries should be
moved to the end of the list.

Used as a band aid in TTM, but better placed in the common list headers.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo.c | 25 +
 include/linux/list.h | 23 +++
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index b2a33bf1ef10..26b889f86670 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -247,20 +247,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
 }
 EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
 
-static void ttm_list_move_bulk_tail(struct list_head *list,
-   struct list_head *first,
-   struct list_head *last)
-{
-   first->prev->next = last->next;
-   last->next->prev = first->prev;
-
-   list->prev->next = first;
-   first->prev = list->prev;
-
-   last->next = list;
-   list->prev = last;
-}
-
 void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
 {
unsigned i;
@@ -276,8 +262,8 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
*bulk)
reservation_object_assert_held(pos->last->resv);
 
man = >first->bdev->man[TTM_PL_TT];
-   ttm_list_move_bulk_tail(>lru[i], >first->lru,
-   >last->lru);
+   list_bulk_move_tail(>lru[i], >first->lru,
+   >last->lru);
}
 
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
@@ -291,8 +277,8 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
*bulk)
reservation_object_assert_held(pos->last->resv);
 
man = >first->bdev->man[TTM_PL_VRAM];
-   ttm_list_move_bulk_tail(>lru[i], >first->lru,
-   >last->lru);
+   list_bulk_move_tail(>lru[i], >first->lru,
+   >last->lru);
}
 
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
@@ -306,8 +292,7 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
*bulk)
reservation_object_assert_held(pos->last->resv);
 
lru = >first->bdev->glob->swap_lru[i];
-   ttm_list_move_bulk_tail(lru, >first->swap,
-   >last->swap);
+   list_bulk_move_tail(lru, >first->swap, >last->swap);
}
 }
 EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
diff --git a/include/linux/list.h b/include/linux/list.h
index de04cc5ed536..edb7628e46ed 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -183,6 +183,29 @@ static inline void list_move_tail(struct list_head *list,
list_add_tail(list, head);
 }
 
+/**
+ * list_bulk_move_tail - move a subsection of a list to its tail
+ * @head: the head that will follow our entry
+ * @first: first entry to move
+ * @last: last entry to move, can be the same as first
+ *
+ * Move all entries between @first and including @last before @head.
+ * All three entries must belong to the same linked list.
+ */
+static inline void list_bulk_move_tail(struct list_head *head,
+  struct list_head *first,
+  struct list_head *last)
+{
+   first->prev->next = last->next;
+   last->next->prev = first->prev;
+
+   head->prev->next = first;
+   first->prev = head->prev;
+
+   last->next = head;
+   head->prev = last;
+}
+
 /**
  * list_is_last - tests whether @list is the last entry in list @head
  * @list: the entry to test
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm/amdgpu: fix shadow BO restoring

[Christian König]

Am 17.09.2018 um 11:10 schrieb Huang Rui:

On Fri, Sep 14, 2018 at 03:42:57PM +0200, Christian König wrote:

Don't grab the reservation lock any more and simplify the handling quite
a bit.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 109 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  46 
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |   8 +--
  3 files changed, 43 insertions(+), 120 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 899342c6dfad..1cbc372964f8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2954,54 +2954,6 @@ static int amdgpu_device_ip_post_soft_reset(struct 
amdgpu_device *adev)
return 0;
  }
  
-/**

- * amdgpu_device_recover_vram_from_shadow - restore shadowed VRAM buffers
- *
- * @adev: amdgpu_device pointer
- * @ring: amdgpu_ring for the engine handling the buffer operations
- * @bo: amdgpu_bo buffer whose shadow is being restored
- * @fence: dma_fence associated with the operation
- *
- * Restores the VRAM buffer contents from the shadow in GTT.  Used to
- * restore things like GPUVM page tables after a GPU reset where
- * the contents of VRAM might be lost.
- * Returns 0 on success, negative error code on failure.
- */
-static int amdgpu_device_recover_vram_from_shadow(struct amdgpu_device *adev,
- struct amdgpu_ring *ring,
- struct amdgpu_bo *bo,
- struct dma_fence **fence)
-{
-   uint32_t domain;
-   int r;
-
-   if (!bo->shadow)
-   return 0;
-
-   r = amdgpu_bo_reserve(bo, true);
-   if (r)
-   return r;
-   domain = amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type);
-   /* if bo has been evicted, then no need to recover */
-   if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
-   r = amdgpu_bo_validate(bo->shadow);
-   if (r) {
-   DRM_ERROR("bo validate failed!\n");
-   goto err;
-   }
-
-   r = amdgpu_bo_restore_from_shadow(adev, ring, bo,
-NULL, fence, true);
-   if (r) {
-   DRM_ERROR("recover page table failed!\n");
-   goto err;
-   }
-   }
-err:
-   amdgpu_bo_unreserve(bo);
-   return r;
-}
-
  /**
   * amdgpu_device_recover_vram - Recover some VRAM contents
   *
@@ -3010,16 +2962,15 @@ static int 
amdgpu_device_recover_vram_from_shadow(struct amdgpu_device *adev,
   * Restores the contents of VRAM buffers from the shadows in GTT.  Used to
   * restore things like GPUVM page tables after a GPU reset where
   * the contents of VRAM might be lost.
- * Returns 0 on success, 1 on failure.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
   */
  static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
  {
-   struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring;
-   struct amdgpu_bo *bo, *tmp;
struct dma_fence *fence = NULL, *next = NULL;
-   long r = 1;
-   int i = 0;
-   long tmo;
+   struct amdgpu_bo *shadow;
+   long r = 1, tmo;
  
  	if (amdgpu_sriov_runtime(adev))

tmo = msecs_to_jiffies(8000);
@@ -3028,44 +2979,40 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
  
  	DRM_INFO("recover vram bo from shadow start\n");

mutex_lock(>shadow_list_lock);
-   list_for_each_entry_safe(bo, tmp, >shadow_list, shadow_list) {
-   next = NULL;
-   amdgpu_device_recover_vram_from_shadow(adev, ring, bo, );
+   list_for_each_entry(shadow, >shadow_list, shadow_list) {
+
+   /* No need to recover an evicted BO */
+   if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
+   shadow->parent->tbo.mem.mem_type != TTM_PL_VRAM)
+   continue;
+
+   r = amdgpu_bo_restore_shadow(shadow, );
+   if (r)
+   break;
+
if (fence) {
r = dma_fence_wait_timeout(fence, false, tmo);
-   if (r == 0)
-   pr_err("wait fence %p[%d] timeout\n", fence, i);
-   else if (r < 0)
-   pr_err("wait fence %p[%d] interrupted\n", 
fence, i);
-   if (r < 1) {
-   dma_fence_put(fence);
-   fence = next;
+   dma_fence_put(fence);
+   fence = next;
+   if (r <= 0)
break;
-   }
-   i++;
+   } else {
+   fence = 

Re: [PATCH] drm/ttm: once more fix ttm_bo_bulk_move_lru_tail

[Huang Rui]

On Fri, Sep 14, 2018 at 05:22:16PM +0800, Michel Dänzer wrote:
> On 2018-09-14 10:22 a.m., Huang Rui wrote:
> > On Thu, Sep 13, 2018 at 07:32:24PM +0800, Christian König wrote:
> >> Am 13.09.2018 um 10:31 schrieb Huang Rui:
> >>> On Wed, Sep 12, 2018 at 09:23:55PM +0200, Christian König wrote:
>  While cutting the lists we sometimes accidentally added a list_head from
>  the stack to the LRUs, effectively corrupting the list.
> 
>  Remove the list cutting and use explicit list manipulation instead.
> >>> This patch actually fixes the corruption bug. Was it a defect of
> >>> list_cut_position or list_splice handlers?
> >>
> >> We somehow did something illegal with list_cut_position. I haven't 
> >> narrowed it down till the end, but we ended up with list_heads from the 
> >> stack to the lru.
> > 
> > I am confused, in theory, even we do any manipulation with list helper, it
> > should not trigger the list corruption. The usage of those helpers should
> > ensure the list operation safely...
> 
> There's nothing the helpers can do about being passed in pointers to
> stack memory. It's a bug in the code using the helpers.
> 

Actually, I was checking carefully with list cut and splice in our case,
and it didn't find any illegal use. However, I also agree with the explicit
list manipulation is more clear and simple for now.

Thanks,
Ray
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] [RFC]drm: add syncobj timeline support v5

[zhoucm1]

On 2018年09月17日 16:37, Christian König wrote:

Am 14.09.2018 um 12:37 schrieb Chunming Zhou:
This patch is for VK_KHR_timeline_semaphore extension, semaphore is 
called syncobj in kernel side:
This extension introduces a new type of syncobj that has an integer 
payload

identifying a point in a timeline. Such timeline syncobjs support the
following operations:
    * CPU query - A host operation that allows querying the payload 
of the

  timeline syncobj.
    * CPU wait - A host operation that allows a blocking wait for a
  timeline syncobj to reach a specified value.
    * Device wait - A device operation that allows waiting for a
  timeline syncobj to reach a specified value.
    * Device signal - A device operation that allows advancing the
  timeline syncobj to a specified value.

Since it's a timeline, that means the front time point(PT) always is 
signaled before the late PT.

a. signal PT design:
Signal PT fence N depends on PT[N-1] fence and signal opertion fence, 
when PT[N] fence is signaled,

the timeline will increase to value of PT[N].
b. wait PT design:
Wait PT fence is signaled by reaching timeline point value, when 
timeline is increasing, will compare
wait PTs value with new timeline value, if PT value is lower than 
timeline value, then wait PT will be
signaled, otherwise keep in list. syncobj wait operation can wait on 
any point of timeline,
so need a RB tree to order them. And wait PT could ahead of signal 
PT, we need a sumission fence to

perform that.

v2:
1. remove unused DRM_SYNCOBJ_CREATE_TYPE_NORMAL. (Christian)
2. move unexposed denitions to .c file. (Daniel Vetter)
3. split up the change to drm_syncobj_find_fence() in a separate 
patch. (Christian)
4. split up the change to drm_syncobj_replace_fence() in a separate 
patch.
5. drop the submission_fence implementation and instead use 
wait_event() for that. (Christian)

6. WARN_ON(point != 0) for NORMAL type syncobj case. (Daniel Vetter)

v3:
1. replace normal syncobj with timeline implemenation. (Vetter and 
Christian)
 a. normal syncobj signal op will create a signal PT to tail of 
signal pt list.
 b. normal syncobj wait op will create a wait pt with last signal 
point, and this wait PT is only signaled by related signal point PT.

2. many bug fix and clean up
3. stub fence moving is moved to other patch.

v4：
1. fix RB tree loop with while(node=rb_first(...)). (Christian)
2. fix syncobj lifecycle. (Christian)
3. only enable_signaling when there is wait_pt. (Christian)
4. fix timeline path issues.
5. write a timeline test in libdrm

v5: (Christian)
1. semaphore is called syncobj in kernel side.
2. don't need 'timeline' characters in some function name.
3. keep syncobj cb

normal syncobj is tested by ./deqp-vk -n dEQP-VK*semaphore*
timeline syncobj is tested by ./amdgpu_test -s 9

Signed-off-by: Chunming Zhou 
Cc: Christian Konig 
Cc: Dave Airlie 
Cc: Daniel Rakos 
Cc: Daniel Vetter 
---
  drivers/gpu/drm/drm_syncobj.c  | 294 ++---
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   4 +-
  include/drm/drm_syncobj.h  |  62 +++--
  include/uapi/drm/drm.h |   1 +
  4 files changed, 292 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c 
b/drivers/gpu/drm/drm_syncobj.c

index e9ce623d049e..e78d076f2703 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -56,6 +56,9 @@
  #include "drm_internal.h"
  #include 
  +/* merge normal syncobj to timeline syncobj, the point interval is 
1 */

+#define DRM_SYNCOBJ_NORMAL_POINT 1
+
  struct drm_syncobj_stub_fence {
  struct dma_fence base;
  spinlock_t lock;
@@ -82,6 +85,11 @@ static const struct dma_fence_ops 
drm_syncobj_stub_fence_ops = {

  .release = drm_syncobj_stub_fence_release,
  };
  +struct drm_syncobj_signal_pt {
+    struct dma_fence_array *base;
+    u64    value;
+    struct list_head list;
+};
    /**
   * drm_syncobj_find - lookup and reference a sync object.
@@ -124,7 +132,7 @@ static int 
drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,

  {
  int ret;
  -    *fence = drm_syncobj_fence_get(syncobj);
+    ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
  if (*fence)


Don't we need to check ret here instead?

both ok, if you like check ret, will update it in v6.




  return 1;
  @@ -133,10 +141,10 @@ static int 
drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,

   * have the lock, try one more time just to be sure we don't add a
   * callback when a fence has already been set.
   */
-    if (syncobj->fence) {
-    *fence = 
dma_fence_get(rcu_dereference_protected(syncobj->fence,

- lockdep_is_held(>lock)));
-    ret = 1;
+    if (fence) {
+    drm_syncobj_search_fence(syncobj, 0, 0, fence);
+    if (*fence)
+    ret = 1;


That doesn't looks correct to me, drm_syncobj_search_fence() would try 
to grab the lock once more.


That 

Re: [PATCH 1/2] drm/amdgpu: add amdgpu_vm_entries_mask

[Huang Rui]

On Mon, Sep 17, 2018 at 05:00:42PM +0800, Christian König wrote:
> Am 17.09.2018 um 10:52 schrieb Huang Rui:
> > On Sat, Sep 15, 2018 at 10:05:40AM +0200, Christian König wrote:
> >> We can't get the mask for the root directory from the number of entries.
> >>
> >> So add a new function to avoid that problem.
> >>
> >> Signed-off-by: Christian König 
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 
> >> +++---
> >>   1 file changed, 27 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> index a7f9aaa47c49..aaf54fc8cafe 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> @@ -190,6 +190,26 @@ static unsigned amdgpu_vm_num_entries(struct 
> >> amdgpu_device *adev,
> >>return AMDGPU_VM_PTE_COUNT(adev);
> >>   }
> >>   
> >> +/**
> >> + * amdgpu_vm_entries_mask - the mask to get the entry number of a PD/PT
> >> + *
> >> + * @adev: amdgpu_device pointer
> >> + * @level: VMPT level
> >> + *
> >> + * Returns:
> >> + * The mask to extract the entry number of a PD/PT from an address.
> >> + */
> >> +static uint32_t amdgpu_vm_entries_mask(struct amdgpu_device *adev,
> >> + unsigned int level)
> >> +{
> >> +  if (level <= adev->vm_manager.root_level)
> >> +  return 0x;
> >> +  else if (level != AMDGPU_VM_PTB)
> >> +  return 0x1f;
> > I think the mask should be 0x1ff (9 bits) while the level is on PDB. The
> > purpose here is to figure out the PTE entry id with the mask, am I right.
> > So the mask should bit width of one entry.
> 
> Oh, yes of course that is a typo.
> 
> Thanks for catching,
> Christian.

No problem~
With that fixed, feel free to add my RB:
Reviewed-by: Huang Rui 

> 
> >
> > Thanks,
> > Ray
> >
> >> +  else
> >> +  return AMDGPU_VM_PTE_COUNT(adev) - 1;
> >> +}
> >> +
> >>   /**
> >>* amdgpu_vm_bo_size - returns the size of the BOs in bytes
> >>*
> >> @@ -399,17 +419,17 @@ static void amdgpu_vm_pt_start(struct amdgpu_device 
> >> *adev,
> >>   static bool amdgpu_vm_pt_descendant(struct amdgpu_device *adev,
> >>struct amdgpu_vm_pt_cursor *cursor)
> >>   {
> >> -  unsigned num_entries, shift, idx;
> >> +  unsigned mask, shift, idx;
> >>   
> >>if (!cursor->entry->entries)
> >>return false;
> >>   
> >>BUG_ON(!cursor->entry->base.bo);
> >> -  num_entries = amdgpu_vm_num_entries(adev, cursor->level);
> >> +  mask = amdgpu_vm_entries_mask(adev, cursor->level);
> >>shift = amdgpu_vm_level_shift(adev, cursor->level);
> >>   
> >>++cursor->level;
> >> -  idx = (cursor->pfn >> shift) % num_entries;
> >> +  idx = (cursor->pfn >> shift) & mask;
> >>cursor->parent = cursor->entry;
> >>cursor->entry = >entry->entries[idx];
> >>return true;
> >> @@ -1599,7 +1619,7 @@ static int amdgpu_vm_update_ptes(struct 
> >> amdgpu_pte_update_params *params,
> >>amdgpu_vm_pt_start(adev, params->vm, start, );
> >>while (cursor.pfn < end) {
> >>struct amdgpu_bo *pt = cursor.entry->base.bo;
> >> -  unsigned shift, parent_shift, num_entries;
> >> +  unsigned shift, parent_shift, mask;
> >>uint64_t incr, entry_end, pe_start;
> >>   
> >>if (!pt)
> >> @@ -1654,9 +1674,9 @@ static int amdgpu_vm_update_ptes(struct 
> >> amdgpu_pte_update_params *params,
> >>   
> >>/* Looks good so far, calculate parameters for the update */
> >>incr = AMDGPU_GPU_PAGE_SIZE << shift;
> >> -  num_entries = amdgpu_vm_num_entries(adev, cursor.level);
> >> -  pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
> >> -  entry_end = num_entries << shift;
> >> +  mask = amdgpu_vm_entries_mask(adev, cursor.level);
> >> +  pe_start = ((cursor.pfn >> shift) & mask) * 8;
> >> +  entry_end = (mask + 1) << shift;
> >>entry_end += cursor.pfn & ~(entry_end - 1);
> >>entry_end = min(entry_end, end);
> >>   
> >> -- 
> >> 2.14.1
> >>
> >> ___
> >> 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

Re: [PATCH 5/5] drm/amdgpu: fix shadow BO restoring

[Huang Rui]

On Fri, Sep 14, 2018 at 03:42:57PM +0200, Christian König wrote:
> Don't grab the reservation lock any more and simplify the handling quite
> a bit.
> 
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 109 
> -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  46 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |   8 +--
>  3 files changed, 43 insertions(+), 120 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 899342c6dfad..1cbc372964f8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2954,54 +2954,6 @@ static int amdgpu_device_ip_post_soft_reset(struct 
> amdgpu_device *adev)
>   return 0;
>  }
>  
> -/**
> - * amdgpu_device_recover_vram_from_shadow - restore shadowed VRAM buffers
> - *
> - * @adev: amdgpu_device pointer
> - * @ring: amdgpu_ring for the engine handling the buffer operations
> - * @bo: amdgpu_bo buffer whose shadow is being restored
> - * @fence: dma_fence associated with the operation
> - *
> - * Restores the VRAM buffer contents from the shadow in GTT.  Used to
> - * restore things like GPUVM page tables after a GPU reset where
> - * the contents of VRAM might be lost.
> - * Returns 0 on success, negative error code on failure.
> - */
> -static int amdgpu_device_recover_vram_from_shadow(struct amdgpu_device *adev,
> -   struct amdgpu_ring *ring,
> -   struct amdgpu_bo *bo,
> -   struct dma_fence **fence)
> -{
> - uint32_t domain;
> - int r;
> -
> - if (!bo->shadow)
> - return 0;
> -
> - r = amdgpu_bo_reserve(bo, true);
> - if (r)
> - return r;
> - domain = amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type);
> - /* if bo has been evicted, then no need to recover */
> - if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
> - r = amdgpu_bo_validate(bo->shadow);
> - if (r) {
> - DRM_ERROR("bo validate failed!\n");
> - goto err;
> - }
> -
> - r = amdgpu_bo_restore_from_shadow(adev, ring, bo,
> -  NULL, fence, true);
> - if (r) {
> - DRM_ERROR("recover page table failed!\n");
> - goto err;
> - }
> - }
> -err:
> - amdgpu_bo_unreserve(bo);
> - return r;
> -}
> -
>  /**
>   * amdgpu_device_recover_vram - Recover some VRAM contents
>   *
> @@ -3010,16 +2962,15 @@ static int 
> amdgpu_device_recover_vram_from_shadow(struct amdgpu_device *adev,
>   * Restores the contents of VRAM buffers from the shadows in GTT.  Used to
>   * restore things like GPUVM page tables after a GPU reset where
>   * the contents of VRAM might be lost.
> - * Returns 0 on success, 1 on failure.
> + *
> + * Returns:
> + * 0 on success, negative error code on failure.
>   */
>  static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
>  {
> - struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring;
> - struct amdgpu_bo *bo, *tmp;
>   struct dma_fence *fence = NULL, *next = NULL;
> - long r = 1;
> - int i = 0;
> - long tmo;
> + struct amdgpu_bo *shadow;
> + long r = 1, tmo;
>  
>   if (amdgpu_sriov_runtime(adev))
>   tmo = msecs_to_jiffies(8000);
> @@ -3028,44 +2979,40 @@ static int amdgpu_device_recover_vram(struct 
> amdgpu_device *adev)
>  
>   DRM_INFO("recover vram bo from shadow start\n");
>   mutex_lock(>shadow_list_lock);
> - list_for_each_entry_safe(bo, tmp, >shadow_list, shadow_list) {
> - next = NULL;
> - amdgpu_device_recover_vram_from_shadow(adev, ring, bo, );
> + list_for_each_entry(shadow, >shadow_list, shadow_list) {
> +
> + /* No need to recover an evicted BO */
> + if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
> + shadow->parent->tbo.mem.mem_type != TTM_PL_VRAM)
> + continue;
> +
> + r = amdgpu_bo_restore_shadow(shadow, );
> + if (r)
> + break;
> +
>   if (fence) {
>   r = dma_fence_wait_timeout(fence, false, tmo);
> - if (r == 0)
> - pr_err("wait fence %p[%d] timeout\n", fence, i);
> - else if (r < 0)
> - pr_err("wait fence %p[%d] interrupted\n", 
> fence, i);
> - if (r < 1) {
> - dma_fence_put(fence);
> - fence = next;
> + dma_fence_put(fence);
> + fence = next;
> + if (r <= 0)
>   break;
> - }
> -

Re: [PATCH 1/2] drm/amdgpu: add amdgpu_vm_entries_mask

[Christian König]

Am 17.09.2018 um 10:52 schrieb Huang Rui:

On Sat, Sep 15, 2018 at 10:05:40AM +0200, Christian König wrote:

We can't get the mask for the root directory from the number of entries.

So add a new function to avoid that problem.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 +++---
  1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a7f9aaa47c49..aaf54fc8cafe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -190,6 +190,26 @@ static unsigned amdgpu_vm_num_entries(struct amdgpu_device 
*adev,
return AMDGPU_VM_PTE_COUNT(adev);
  }
  
+/**

+ * amdgpu_vm_entries_mask - the mask to get the entry number of a PD/PT
+ *
+ * @adev: amdgpu_device pointer
+ * @level: VMPT level
+ *
+ * Returns:
+ * The mask to extract the entry number of a PD/PT from an address.
+ */
+static uint32_t amdgpu_vm_entries_mask(struct amdgpu_device *adev,
+  unsigned int level)
+{
+   if (level <= adev->vm_manager.root_level)
+   return 0x;
+   else if (level != AMDGPU_VM_PTB)
+   return 0x1f;

I think the mask should be 0x1ff (9 bits) while the level is on PDB. The
purpose here is to figure out the PTE entry id with the mask, am I right.
So the mask should bit width of one entry.


Oh, yes of course that is a typo.

Thanks for catching,
Christian.



Thanks,
Ray


+   else
+   return AMDGPU_VM_PTE_COUNT(adev) - 1;
+}
+
  /**
   * amdgpu_vm_bo_size - returns the size of the BOs in bytes
   *
@@ -399,17 +419,17 @@ static void amdgpu_vm_pt_start(struct amdgpu_device *adev,
  static bool amdgpu_vm_pt_descendant(struct amdgpu_device *adev,
struct amdgpu_vm_pt_cursor *cursor)
  {
-   unsigned num_entries, shift, idx;
+   unsigned mask, shift, idx;
  
  	if (!cursor->entry->entries)

return false;
  
  	BUG_ON(!cursor->entry->base.bo);

-   num_entries = amdgpu_vm_num_entries(adev, cursor->level);
+   mask = amdgpu_vm_entries_mask(adev, cursor->level);
shift = amdgpu_vm_level_shift(adev, cursor->level);
  
  	++cursor->level;

-   idx = (cursor->pfn >> shift) % num_entries;
+   idx = (cursor->pfn >> shift) & mask;
cursor->parent = cursor->entry;
cursor->entry = >entry->entries[idx];
return true;
@@ -1599,7 +1619,7 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
amdgpu_vm_pt_start(adev, params->vm, start, );
while (cursor.pfn < end) {
struct amdgpu_bo *pt = cursor.entry->base.bo;
-   unsigned shift, parent_shift, num_entries;
+   unsigned shift, parent_shift, mask;
uint64_t incr, entry_end, pe_start;
  
  		if (!pt)

@@ -1654,9 +1674,9 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
  
  		/* Looks good so far, calculate parameters for the update */

incr = AMDGPU_GPU_PAGE_SIZE << shift;
-   num_entries = amdgpu_vm_num_entries(adev, cursor.level);
-   pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
-   entry_end = num_entries << shift;
+   mask = amdgpu_vm_entries_mask(adev, cursor.level);
+   pe_start = ((cursor.pfn >> shift) & mask) * 8;
+   entry_end = (mask + 1) << shift;
entry_end += cursor.pfn & ~(entry_end - 1);
entry_end = min(entry_end, end);
  
--

2.14.1

___
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

Re: [PATCH 1/2] drm/amdgpu: add amdgpu_vm_entries_mask

[Huang Rui]

On Sat, Sep 15, 2018 at 10:05:40AM +0200, Christian König wrote:
> We can't get the mask for the root directory from the number of entries.
> 
> So add a new function to avoid that problem.
> 
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 
> +++---
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a7f9aaa47c49..aaf54fc8cafe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -190,6 +190,26 @@ static unsigned amdgpu_vm_num_entries(struct 
> amdgpu_device *adev,
>   return AMDGPU_VM_PTE_COUNT(adev);
>  }
>  
> +/**
> + * amdgpu_vm_entries_mask - the mask to get the entry number of a PD/PT
> + *
> + * @adev: amdgpu_device pointer
> + * @level: VMPT level
> + *
> + * Returns:
> + * The mask to extract the entry number of a PD/PT from an address.
> + */
> +static uint32_t amdgpu_vm_entries_mask(struct amdgpu_device *adev,
> +unsigned int level)
> +{
> + if (level <= adev->vm_manager.root_level)
> + return 0x;
> + else if (level != AMDGPU_VM_PTB)
> + return 0x1f;

I think the mask should be 0x1ff (9 bits) while the level is on PDB. The
purpose here is to figure out the PTE entry id with the mask, am I right.
So the mask should bit width of one entry. 

Thanks,
Ray

> + else
> + return AMDGPU_VM_PTE_COUNT(adev) - 1;
> +}
> +
>  /**
>   * amdgpu_vm_bo_size - returns the size of the BOs in bytes
>   *
> @@ -399,17 +419,17 @@ static void amdgpu_vm_pt_start(struct amdgpu_device 
> *adev,
>  static bool amdgpu_vm_pt_descendant(struct amdgpu_device *adev,
>   struct amdgpu_vm_pt_cursor *cursor)
>  {
> - unsigned num_entries, shift, idx;
> + unsigned mask, shift, idx;
>  
>   if (!cursor->entry->entries)
>   return false;
>  
>   BUG_ON(!cursor->entry->base.bo);
> - num_entries = amdgpu_vm_num_entries(adev, cursor->level);
> + mask = amdgpu_vm_entries_mask(adev, cursor->level);
>   shift = amdgpu_vm_level_shift(adev, cursor->level);
>  
>   ++cursor->level;
> - idx = (cursor->pfn >> shift) % num_entries;
> + idx = (cursor->pfn >> shift) & mask;
>   cursor->parent = cursor->entry;
>   cursor->entry = >entry->entries[idx];
>   return true;
> @@ -1599,7 +1619,7 @@ static int amdgpu_vm_update_ptes(struct 
> amdgpu_pte_update_params *params,
>   amdgpu_vm_pt_start(adev, params->vm, start, );
>   while (cursor.pfn < end) {
>   struct amdgpu_bo *pt = cursor.entry->base.bo;
> - unsigned shift, parent_shift, num_entries;
> + unsigned shift, parent_shift, mask;
>   uint64_t incr, entry_end, pe_start;
>  
>   if (!pt)
> @@ -1654,9 +1674,9 @@ static int amdgpu_vm_update_ptes(struct 
> amdgpu_pte_update_params *params,
>  
>   /* Looks good so far, calculate parameters for the update */
>   incr = AMDGPU_GPU_PAGE_SIZE << shift;
> - num_entries = amdgpu_vm_num_entries(adev, cursor.level);
> - pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
> - entry_end = num_entries << shift;
> + mask = amdgpu_vm_entries_mask(adev, cursor.level);
> + pe_start = ((cursor.pfn >> shift) & mask) * 8;
> + entry_end = (mask + 1) << shift;
>   entry_end += cursor.pfn & ~(entry_end - 1);
>   entry_end = min(entry_end, end);
>  
> -- 
> 2.14.1
> 
> ___
> 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

Re: [PATCH] [RFC]drm: add syncobj timeline support v5

[Christian König]

Am 14.09.2018 um 12:37 schrieb Chunming Zhou:

This patch is for VK_KHR_timeline_semaphore extension, semaphore is called 
syncobj in kernel side:
This extension introduces a new type of syncobj that has an integer payload
identifying a point in a timeline. Such timeline syncobjs support the
following operations:
* CPU query - A host operation that allows querying the payload of the
  timeline syncobj.
* CPU wait - A host operation that allows a blocking wait for a
  timeline syncobj to reach a specified value.
* Device wait - A device operation that allows waiting for a
  timeline syncobj to reach a specified value.
* Device signal - A device operation that allows advancing the
  timeline syncobj to a specified value.

Since it's a timeline, that means the front time point(PT) always is signaled 
before the late PT.
a. signal PT design:
Signal PT fence N depends on PT[N-1] fence and signal opertion fence, when 
PT[N] fence is signaled,
the timeline will increase to value of PT[N].
b. wait PT design:
Wait PT fence is signaled by reaching timeline point value, when timeline is 
increasing, will compare
wait PTs value with new timeline value, if PT value is lower than timeline 
value, then wait PT will be
signaled, otherwise keep in list. syncobj wait operation can wait on any point 
of timeline,
so need a RB tree to order them. And wait PT could ahead of signal PT, we need 
a sumission fence to
perform that.

v2:
1. remove unused DRM_SYNCOBJ_CREATE_TYPE_NORMAL. (Christian)
2. move unexposed denitions to .c file. (Daniel Vetter)
3. split up the change to drm_syncobj_find_fence() in a separate patch. 
(Christian)
4. split up the change to drm_syncobj_replace_fence() in a separate patch.
5. drop the submission_fence implementation and instead use wait_event() for 
that. (Christian)
6. WARN_ON(point != 0) for NORMAL type syncobj case. (Daniel Vetter)

v3:
1. replace normal syncobj with timeline implemenation. (Vetter and Christian)
 a. normal syncobj signal op will create a signal PT to tail of signal pt 
list.
 b. normal syncobj wait op will create a wait pt with last signal point, 
and this wait PT is only signaled by related signal point PT.
2. many bug fix and clean up
3. stub fence moving is moved to other patch.

v4：
1. fix RB tree loop with while(node=rb_first(...)). (Christian)
2. fix syncobj lifecycle. (Christian)
3. only enable_signaling when there is wait_pt. (Christian)
4. fix timeline path issues.
5. write a timeline test in libdrm

v5: (Christian)
1. semaphore is called syncobj in kernel side.
2. don't need 'timeline' characters in some function name.
3. keep syncobj cb

normal syncobj is tested by ./deqp-vk -n dEQP-VK*semaphore*
timeline syncobj is tested by ./amdgpu_test -s 9

Signed-off-by: Chunming Zhou 
Cc: Christian Konig 
Cc: Dave Airlie 
Cc: Daniel Rakos 
Cc: Daniel Vetter 
---
  drivers/gpu/drm/drm_syncobj.c  | 294 ++---
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   4 +-
  include/drm/drm_syncobj.h  |  62 +++--
  include/uapi/drm/drm.h |   1 +
  4 files changed, 292 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index e9ce623d049e..e78d076f2703 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -56,6 +56,9 @@
  #include "drm_internal.h"
  #include 
  
+/* merge normal syncobj to timeline syncobj, the point interval is 1 */

+#define DRM_SYNCOBJ_NORMAL_POINT 1
+
  struct drm_syncobj_stub_fence {
struct dma_fence base;
spinlock_t lock;
@@ -82,6 +85,11 @@ static const struct dma_fence_ops drm_syncobj_stub_fence_ops 
= {
.release = drm_syncobj_stub_fence_release,
  };
  
+struct drm_syncobj_signal_pt {

+   struct dma_fence_array *base;
+   u64value;
+   struct list_head list;
+};
  
  /**

   * drm_syncobj_find - lookup and reference a sync object.
@@ -124,7 +132,7 @@ static int drm_syncobj_fence_get_or_add_callback(struct 
drm_syncobj *syncobj,
  {
int ret;
  
-	*fence = drm_syncobj_fence_get(syncobj);

+   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
if (*fence)


Don't we need to check ret here instead?


return 1;
  
@@ -133,10 +141,10 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,

 * have the lock, try one more time just to be sure we don't add a
 * callback when a fence has already been set.
 */
-   if (syncobj->fence) {
-   *fence = dma_fence_get(rcu_dereference_protected(syncobj->fence,
-
lockdep_is_held(>lock)));
-   ret = 1;
+   if (fence) {
+   drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   if (*fence)
+   ret = 1;


That doesn't looks correct to me, drm_syncobj_search_fence() would try 
to grab the 

RE: [PATCH] drm/amd/dc: Trigger set power state task when display configuration changes

[Zhu, Rex]

+Harry and David.

Best Regards
Rex

From: Deucher, Alexander
Sent: Friday, September 14, 2018 9:40 PM
To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/dc: Trigger set power state task when display 
configuration changes


Acked-by: Alex Deucher 
mailto:alexander.deuc...@amd.com>>


From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Rex Zhu mailto:rex@amd.com>>
Sent: Friday, September 14, 2018 1:57:07 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhu, Rex
Subject: [PATCH] drm/amd/dc: Trigger set power state task when display 
configuration changes

Revert "drm/amd/display: Remove call to amdgpu_pm_compute_clocks"

This reverts commit dcd473770e86517543691bdb227103d6c781cd0a.

when display configuration changes, dc need to update the changes
to powerplay, also need to trigger a power state task.
amdgpu_pm_compute_clocks is the interface to set power state task
either dpm enabled or powerplay enabled

Signed-off-by: Rex Zhu mailto:rex@amd.com>>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
index 6d16b4a..0fab64a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
@@ -105,6 +105,8 @@ bool dm_pp_apply_display_requirements(
 adev->powerplay.pp_funcs->display_configuration_change(
 adev->powerplay.pp_handle,
 >pm.pm_display_cfg);
+
+   amdgpu_pm_compute_clocks(adev);
 }

 return true;
--
1.9.1

___
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

Re: [PATCH 2/2] drm/amdgpu: fix parameter documentation for amdgpu_vm_free_pts

[Huang Rui]

On Sat, Sep 15, 2018 at 10:05:41AM +0200, Christian König wrote:
> The function was modified without updating the documentation.
> 
> Signed-off-by: Christian König 
> ---

Reviewed-by: Huang Rui 

>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index aaf54fc8cafe..cad275eaba74 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -961,8 +961,7 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   * amdgpu_vm_free_pts - free PD/PT levels
>   *
>   * @adev: amdgpu device structure
> - * @parent: PD/PT starting level to free
> - * @level: level of parent structure
> + * @vm: amdgpu vm structure
>   *
>   * Free the page directory or page table level and all sub levels.
>   */
> -- 
> 2.14.1
> 
> ___
> 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

Re: [BUG] gpu: drm: amdgpu: Possible sleep-in-atomic-context bugs in amdgpu_uvd_ring_begin_use()

[Jia-Ju Bai]

On 2018/9/15 17:23, Koenig, Christian wrote:

No, the problem is the function pointer analysis.

In other words the KIQ ring is sometimes used in atomic and even 
interrupt context.


But the UVD ring is never used in atomic context.

But I don't see a way a static analysis could ever figure that out.



Okay, thanks for your explanation :)
Besides, I find that amdgpu_virt_kiq_rreg() calls msleep(), so mdelay() 
should be used instead.



Best wishes,
Jia-Ju Bai



Am 15.09.2018 11:18 schrieb Jia-Ju Bai :
Sorry, I am still not clear why the call chain I proposed is incorrect...

I find a conditional in amdgpu_mm_wreg():

if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
return amdgpu_virt_kiq_wreg(adev, reg, v);

Is amdgpu_virt_kiq_wreg() never called from WREG32() or RREG32()?


Best wishes,
Jia-Ju Bai


On 2018/9/15 17:10, Koenig, Christian wrote:
amdgpu_ring_alloc() does call amdgpu_uvd_begin_use(), but never in 
the call chain you proposed.


Thinking about it I actually don't see a way a statically analysis 
could ever figure that out.


Christian.

Am 15.09.2018 11:05 schrieb Jia-Ju Bai :

Thanks for your reply.

On 2018/9/15 17:01, Koenig, Christian wrote:

Sorry to say that but your analysis tool is buggy.

The proposed call paths will never trigger.


Could you please explain which piece of the call path is incorrect?
I am not very sure of my function pointer analysis.
Does amdgpu_ring_alloc() never calls amdgpu_uvd_ring_begin_use()?

Thanks in advance.


Best wishes,
Jia-Ju Bai


Regards,
Christian.

Am 15.09.2018 10:59 schrieb Jia-Ju Bai
 :
The driver may sleep with holding a spinlock.
The function call paths (from bottom to top) in Linux-4.17 are:

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/vi.c, 207:
 amdgpu_mm_wreg in vi_gc_cac_rreg
drivers/gpu/drm/amd/amdgpu/vi.c, 206:
 _raw_spin_lock_irqsave in vi_gc_cac_rreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/soc15.c, 106:
 amdgpu_mm_wreg in soc15_pcie_rreg
drivers/gpu/drm/amd/amdgpu/soc15.c, 105:
 _raw_spin_lock_irqsave in soc15_pcie_rreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/cik.c, 139:
 amdgpu_mm_wreg in cik_uvd_ctx_wreg
drivers/gpu/drm/amd/amdgpu/cik.c, 138:
 _raw_spin_lock_irqsave in cik_uvd_ctx_wreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c, 126:
 amdgpu_mm_wreg in dce_v6_0_audio_endpt_rreg

Re: [BUG] gpu: drm: amdgpu: Possible sleep-in-atomic-context bugs in amdgpu_uvd_ring_begin_use()

[Jia-Ju Bai]

Sorry, I am still not clear why the call chain I proposed is incorrect...

I find a conditional in amdgpu_mm_wreg():

if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
return amdgpu_virt_kiq_wreg(adev, reg, v);

Is amdgpu_virt_kiq_wreg() never called from WREG32() or RREG32()?


Best wishes,
Jia-Ju Bai


On 2018/9/15 17:10, Koenig, Christian wrote:
amdgpu_ring_alloc() does call amdgpu_uvd_begin_use(), but never in the 
call chain you proposed.


Thinking about it I actually don't see a way a statically analysis 
could ever figure that out.


Christian.

Am 15.09.2018 11:05 schrieb Jia-Ju Bai :

Thanks for your reply.

On 2018/9/15 17:01, Koenig, Christian wrote:

Sorry to say that but your analysis tool is buggy.

The proposed call paths will never trigger.


Could you please explain which piece of the call path is incorrect?
I am not very sure of my function pointer analysis.
Does amdgpu_ring_alloc() never calls amdgpu_uvd_ring_begin_use()?

Thanks in advance.


Best wishes,
Jia-Ju Bai


Regards,
Christian.

Am 15.09.2018 10:59 schrieb Jia-Ju Bai
 :
The driver may sleep with holding a spinlock.
The function call paths (from bottom to top) in Linux-4.17 are:

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/vi.c, 207:
 amdgpu_mm_wreg in vi_gc_cac_rreg
drivers/gpu/drm/amd/amdgpu/vi.c, 206:
 _raw_spin_lock_irqsave in vi_gc_cac_rreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/soc15.c, 106:
 amdgpu_mm_wreg in soc15_pcie_rreg
drivers/gpu/drm/amd/amdgpu/soc15.c, 105:
 _raw_spin_lock_irqsave in soc15_pcie_rreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/cik.c, 139:
 amdgpu_mm_wreg in cik_uvd_ctx_wreg
drivers/gpu/drm/amd/amdgpu/cik.c, 138:
 _raw_spin_lock_irqsave in cik_uvd_ctx_wreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c, 126:
 amdgpu_mm_wreg in dce_v6_0_audio_endpt_rreg
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c, 125:
 _raw_spin_lock_irqsave in dce_v6_0_audio_endpt_rreg

Note that [FUNC_PTR] means a function pointer call is used.

These bugs are found by my static analysis tool DSAC.


Best wishes,
Jia-Ju Bai





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] amdgpu uses raw rlc_hdr values, causing kernel OOPS on big endian architectures

[A. Wilcox]

adev->gfx.rlc in gfx_v8_0_init_microcode has the values from rlc_hdr
already processed by le32_to_cpu.  Using the rlc_hdr values on
big-endian machines causes a kernel Oops due to writing well outside of
the array (0x2400 instead of 0x24).  gfx_v9_0 had the same issue and
was fixed in the same manner (but was not tested locally; I do not have
a v9 card).

[8.143396] Unable to handle kernel paging request for data at
address 0xc0080615b000
[8.143396] Faulting instruction address: 0xc00805f063c8
[8.143399] Oops: Kernel access of bad area, sig: 11 [#1]
[8.143429] BE SMP NR_CPUS=256 NUMA PowerNV
[8.143461] Modules linked in: binfmt_misc amdgpu(+) ast ttm
drm_kms_helper sysimgblt syscopyarea sysfillrect fb_sys_fops drm joydev
mac_hid tg3 ipmi_powernv ipmi_msghandler agpgart i2c_algo_bit shpchp
[8.143615] CPU: 0 PID: 2402 Comm: kworker/0:3 Not tainted
4.14.48-mc8-easy #1
[8.143679] Workqueue: events .work_for_cpu_fn
[8.143728] task: c003e7fd8000 task.stack: c003e7fe
[8.143783] NIP:  c00805f063c8 LR: c00805f06388 CTR:
c027efd0
[8.143869] REGS: c003e7fe3430 TRAP: 0300   Not tainted
(4.14.48-mc8-easy)
[8.143950] MSR:  90009032   CR:
28002444  XER: 2004
[8.144040] CFAR: c00805f063d0 DAR: c0080615b000 DSISR:
4000 SOFTE: 1
[8.144787] NIP [c00805f063c8] .gfx_v8_0_sw_init+0x5a8/0x15b0
[amdgpu]
[8.144911] LR [c00805f06388] .gfx_v8_0_sw_init+0x568/0x15b0 [amdgpu]
[8.144976] Call Trace:
[8.145049] [c003e7fe36b0] [c00805f06388]
.gfx_v8_0_sw_init+0x568/0x15b0 [amdgpu] (unreliable)
[8.145194] [c003e7fe37c0] [c00805e484c4]
.amdgpu_device_init+0xf34/0x1750 [amdgpu]
[8.145302] [c003e7fe38f0] [c00805e4a94c]
.amdgpu_driver_load_kms+0x9c/0x2a0 [amdgpu]
[8.145420] [c003e7fe3980] [c00804f4d200]
.drm_dev_register+0x1c0/0x250 [drm]
[8.145548] [c003e7fe3a30] [c00805e416c4]
.amdgpu_pci_probe+0x164/0x1a0 [amdgpu]
[8.145601] [c003e7fe3ac0] [c0618ed0]
.local_pci_probe+0x60/0x130
[8.145683] [c003e7fe3b60] [c00e8780]
.work_for_cpu_fn+0x30/0x50
[8.145774] [c003e7fe3be0] [c00ecea8]
.process_one_work+0x2a8/0x550
[8.145854] [c003e7fe3c80] [c00ed430]
.worker_thread+0x2e0/0x600
[8.145924] [c003e7fe3d70] [c00f5338] .kthread+0x158/0x1a0
[8.145973] [c003e7fe3e30] [c000bd4c]
.ret_from_kernel_thread+0x58/0x8c
[8.146054] Instruction dump:
[8.146089] 38db004c 3920 7d00342c 7d08da14 815b0048 554af0be
7f8a4840 409d004c 792a1764 e8ff46f0 39290001 79290020 <7cc8542c>
7cc7512e 4bd8 6000
[8.146252] ---[ end trace 8b0ede048bbb20ae ]---

-- 
A. Wilcox (awilfox)
Project Lead, Adélie Linux
https://www.adelielinux.org
From b92303fbefdb34d3c55ae223ca9b02e60625aae5 Mon Sep 17 00:00:00 2001
From: "A. Wilcox" 
Date: Sun, 1 Jul 2018 22:44:52 -0500
Subject: [PATCH] drm/amdgpu: use processed values for counting

adev->gfx.rlc has the values from rlc_hdr already processed by
le32_to_cpu.  Using the rlc_hdr values on big-endian machines causes
a kernel Oops due to writing well outside of the array (0x2400
instead of 0x24).

Signed-off-by: A. Wilcox 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 4 ++--
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 818874b13c99..112dfbf7a1fc 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -1094,14 +1094,14 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device 
*adev)
 
tmp = (unsigned int *)((uintptr_t)rlc_hdr +

le32_to_cpu(rlc_hdr->reg_list_format_array_offset_bytes));
-   for (i = 0 ; i < (rlc_hdr->reg_list_format_size_bytes >> 2); i++)
+   for (i = 0 ; i < (adev->gfx.rlc.reg_list_format_size_bytes >> 2); i++)
adev->gfx.rlc.register_list_format[i] = le32_to_cpu(tmp[i]);
 
adev->gfx.rlc.register_restore = adev->gfx.rlc.register_list_format + i;
 
tmp = (unsigned int *)((uintptr_t)rlc_hdr +
le32_to_cpu(rlc_hdr->reg_list_array_offset_bytes));
-   for (i = 0 ; i < (rlc_hdr->reg_list_size_bytes >> 2); i++)
+   for (i = 0 ; i < (adev->gfx.rlc.reg_list_size_bytes >> 2); i++)
adev->gfx.rlc.register_restore[i] = le32_to_cpu(tmp[i]);
 
if (adev->asic_type >= CHIP_POLARIS10 && adev->asic_type <= 
CHIP_POLARIS12) {
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index a69153435ea7..d5e1f80adbc8 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -577,14 +577,14 @@ static int gfx_v9_0_init_microcode(struct amdgpu_device 
*adev)
 
tmp = (unsigned int *)((uintptr_t)rlc_hdr +

Re: [BUG] gpu: drm: amdgpu: Possible sleep-in-atomic-context bugs in amdgpu_uvd_ring_begin_use()

[Jia-Ju Bai]

Thanks for your reply.

On 2018/9/15 17:01, Koenig, Christian wrote:

Sorry to say that but your analysis tool is buggy.

The proposed call paths will never trigger.


Could you please explain which piece of the call path is incorrect?
I am not very sure of my function pointer analysis.
Does amdgpu_ring_alloc() never calls amdgpu_uvd_ring_begin_use()?

Thanks in advance.


Best wishes,
Jia-Ju Bai



Regards,
Christian.

Am 15.09.2018 10:59 schrieb Jia-Ju Bai :
The driver may sleep with holding a spinlock.
The function call paths (from bottom to top) in Linux-4.17 are:

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/vi.c, 207:
 amdgpu_mm_wreg in vi_gc_cac_rreg
drivers/gpu/drm/amd/amdgpu/vi.c, 206:
 _raw_spin_lock_irqsave in vi_gc_cac_rreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/soc15.c, 106:
 amdgpu_mm_wreg in soc15_pcie_rreg
drivers/gpu/drm/amd/amdgpu/soc15.c, 105:
 _raw_spin_lock_irqsave in soc15_pcie_rreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/cik.c, 139:
 amdgpu_mm_wreg in cik_uvd_ctx_wreg
drivers/gpu/drm/amd/amdgpu/cik.c, 138:
 _raw_spin_lock_irqsave in cik_uvd_ctx_wreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
 mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
 amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
 [FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
 amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
 amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c, 126:
 amdgpu_mm_wreg in dce_v6_0_audio_endpt_rreg
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c, 125:
 _raw_spin_lock_irqsave in dce_v6_0_audio_endpt_rreg

Note that [FUNC_PTR] means a function pointer call is used.

These bugs are found by my static analysis tool DSAC.


Best wishes,
Jia-Ju Bai


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg

[Al Viro]

On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote:
 
> Acked-by: Darren Hart (VMware) 
> 
> As for a longer term solution, would it be possible to init fops in such
> a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg
> so we don't have to duplicate this boilerplate for every ioctl fops
> structure?

Bad idea, that...  Because several years down the road somebody will add
an ioctl that takes an unsigned int for argument.  Without so much as looking
at your magical mystery macro being used to initialize file_operations.

FWIW, I would name that helper in more blunt way - something like
compat_ioctl_only_compat_pointer_ioctls_here()...
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[BUG] gpu: drm: amdgpu: Possible sleep-in-atomic-context bugs in amdgpu_uvd_ring_begin_use()

[Jia-Ju Bai]

The driver may sleep with holding a spinlock.
The function call paths (from bottom to top) in Linux-4.17 are:

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
[FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/vi.c, 207:
amdgpu_mm_wreg in vi_gc_cac_rreg
drivers/gpu/drm/amd/amdgpu/vi.c, 206:
_raw_spin_lock_irqsave in vi_gc_cac_rreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
[FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/soc15.c, 106:
amdgpu_mm_wreg in soc15_pcie_rreg
drivers/gpu/drm/amd/amdgpu/soc15.c, 105:
_raw_spin_lock_irqsave in soc15_pcie_rreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
[FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/cik.c, 139:
amdgpu_mm_wreg in cik_uvd_ctx_wreg
drivers/gpu/drm/amd/amdgpu/cik.c, 138:
_raw_spin_lock_irqsave in cik_uvd_ctx_wreg

[FUNC] mutex_lock_nested
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c, 1477:
mutex_lock_nested in amdgpu_dpm_enable_uvd
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c, 1154:
amdgpu_dpm_enable_uvd in amdgpu_uvd_ring_begin_use
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c, 80:
[FUNC_PTR]amdgpu_uvd_ring_begin_use in amdgpu_ring_alloc
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c, 199:
amdgpu_ring_alloc in amdgpu_virt_kiq_wreg
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, 207:
amdgpu_virt_kiq_wreg in amdgpu_mm_wreg
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c, 126:
amdgpu_mm_wreg in dce_v6_0_audio_endpt_rreg
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c, 125:
_raw_spin_lock_irqsave in dce_v6_0_audio_endpt_rreg

Note that [FUNC_PTR] means a function pointer call is used.

These bugs are found by my static analysis tool DSAC.


Best wishes,
Jia-Ju Bai
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] amdgpu/dcn10 : fix compile warning

[Peng Hao]

drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:
In function ‘dcn10_update_mpcc’:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:1903:9:
warning: missing braces around initializer [-Wmissing-braces]

Signed-off-by: Peng Hao 
---
 drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index cfcc54f..a06a035 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -1900,7 +1900,7 @@ static void update_dpp(struct dpp *dpp, struct 
dc_plane_state *plane_state)
 static void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
 {
struct hubp *hubp = pipe_ctx->plane_res.hubp;
-   struct mpcc_blnd_cfg blnd_cfg = {0};
+   struct mpcc_blnd_cfg blnd_cfg = { {0} };
bool per_pixel_alpha = pipe_ctx->plane_state->per_pixel_alpha && 
pipe_ctx->bottom_pipe;
int mpcc_id;
struct mpcc *new_mpcc;
-- 
1.8.3.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/3] drm/amd/powerplay: retrieve the updated clock table after OD

[Evan Quan]

With OD settings applied, the clock table will be updated accordingly.
We need to retrieve the new clock tables then.

Change-Id: Iad4e95d3f195a0217456d41e495730578209062b
Signed-off-by: Evan Quan 
---
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 114 ++
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.h|   2 +
 2 files changed, 90 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 491daf02467a..73fb3a41bd7e 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -516,6 +516,47 @@ static int vega20_setup_single_dpm_table(struct pp_hwmgr 
*hwmgr,
return ret;
 }
 
+static int vega20_setup_gfxclk_dpm_table(struct pp_hwmgr *hwmgr)
+{
+   struct vega20_hwmgr *data =
+   (struct vega20_hwmgr *)(hwmgr->backend);
+   struct vega20_single_dpm_table *dpm_table;
+   int ret = 0;
+
+   dpm_table = &(data->dpm_table.gfx_table);
+   if (data->smu_features[GNLD_DPM_GFXCLK].enabled) {
+   ret = vega20_setup_single_dpm_table(hwmgr, dpm_table, 
PPCLK_GFXCLK);
+   PP_ASSERT_WITH_CODE(!ret,
+   "[SetupDefaultDpmTable] failed to get gfxclk 
dpm levels!",
+   return ret);
+   } else {
+   dpm_table->count = 1;
+   dpm_table->dpm_levels[0].value = 
data->vbios_boot_state.gfx_clock / 100;
+   }
+
+   return ret;
+}
+
+static int vega20_setup_memclk_dpm_table(struct pp_hwmgr *hwmgr)
+{
+   struct vega20_hwmgr *data =
+   (struct vega20_hwmgr *)(hwmgr->backend);
+   struct vega20_single_dpm_table *dpm_table;
+   int ret = 0;
+
+   dpm_table = &(data->dpm_table.mem_table);
+   if (data->smu_features[GNLD_DPM_UCLK].enabled) {
+   ret = vega20_setup_single_dpm_table(hwmgr, dpm_table, 
PPCLK_UCLK);
+   PP_ASSERT_WITH_CODE(!ret,
+   "[SetupDefaultDpmTable] failed to get memclk 
dpm levels!",
+   return ret);
+   } else {
+   dpm_table->count = 1;
+   dpm_table->dpm_levels[0].value = 
data->vbios_boot_state.mem_clock / 100;
+   }
+
+   return ret;
+}
 
 /*
  * This function is to initialize all DPM state tables
@@ -549,28 +590,16 @@ static int vega20_setup_default_dpm_tables(struct 
pp_hwmgr *hwmgr)
 
/* gfxclk */
dpm_table = &(data->dpm_table.gfx_table);
-   if (data->smu_features[GNLD_DPM_GFXCLK].enabled) {
-   ret = vega20_setup_single_dpm_table(hwmgr, dpm_table, 
PPCLK_GFXCLK);
-   PP_ASSERT_WITH_CODE(!ret,
-   "[SetupDefaultDpmTable] failed to get gfxclk 
dpm levels!",
-   return ret);
-   } else {
-   dpm_table->count = 1;
-   dpm_table->dpm_levels[0].value = 
data->vbios_boot_state.gfx_clock / 100;
-   }
+   ret = vega20_setup_gfxclk_dpm_table(hwmgr);
+   if (ret)
+   return ret;
vega20_init_dpm_state(&(dpm_table->dpm_state));
 
/* memclk */
dpm_table = &(data->dpm_table.mem_table);
-   if (data->smu_features[GNLD_DPM_UCLK].enabled) {
-   ret = vega20_setup_single_dpm_table(hwmgr, dpm_table, 
PPCLK_UCLK);
-   PP_ASSERT_WITH_CODE(!ret,
-   "[SetupDefaultDpmTable] failed to get memclk 
dpm levels!",
-   return ret);
-   } else {
-   dpm_table->count = 1;
-   dpm_table->dpm_levels[0].value = 
data->vbios_boot_state.mem_clock / 100;
-   }
+   ret = vega20_setup_memclk_dpm_table(hwmgr);
+   if (ret)
+   return ret;
vega20_init_dpm_state(&(dpm_table->dpm_state));
 
/* eclk */
@@ -1183,6 +1212,9 @@ static int vega20_od8_set_settings(
 {
OverDriveTable_t od_table;
int ret = 0;
+   struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr->backend);
+   struct vega20_od8_single_setting *od8_settings =
+   data->od8_settings.od8_settings_array;
 
ret = vega20_copy_table_from_smc(hwmgr, (uint8_t *)(_table), 
TABLE_OVERDRIVE);
PP_ASSERT_WITH_CODE(!ret,
@@ -1194,6 +1226,10 @@ static int vega20_od8_set_settings(
od_table.GfxclkFmin = (uint16_t)value;
break;
case OD8_SETTING_GFXCLK_FMAX:
+   if (value < od8_settings[OD8_SETTING_GFXCLK_FMAX].min_value ||
+   value > od8_settings[OD8_SETTING_GFXCLK_FMAX].max_value)
+   return -EINVAL;
+
od_table.GfxclkFmax = (uint16_t)value;
break;
case OD8_SETTING_GFXCLK_FREQ1:
@@ -1215,6 +1251,9 @@ static int vega20_od8_set_settings(
od_table.GfxclkVolt3 = (uint16_t)value;

[PATCH 2/3] drm/amd/powerplay: update OD to take voltage value instead of offset

[Evan Quan]

With the latest SMC fw, we are able to get the voltage value for
specific frequency point. So, we update the OD relates to take
absolute voltage instead of offset.

Change-Id: I4ac8518f518cf3d70e59b16e3ea2102cd63c52d6
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c|  12 +-
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 112 +-
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.h|   4 +
 .../drm/amd/powerplay/inc/smu11_driver_if.h   |   6 +-
 .../gpu/drm/amd/powerplay/inc/vega20_ppsmc.h  |   3 +-
 5 files changed, 96 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 396c826100e6..8c334fc808c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -502,7 +502,7 @@ static ssize_t amdgpu_set_pp_table(struct device *dev,
  *
  * - maximum memory clock labeled OD_MCLK
  *
- * - three  points labeled OD_VDDC_CURVE.
+ * - three  points labeled OD_VDDC_CURVE.
  *   They can be used to calibrate the sclk voltage curve.
  *
  * - a list of valid ranges for sclk, mclk, and voltage curve points
@@ -519,11 +519,11 @@ static ssize_t amdgpu_set_pp_table(struct device *dev,
  *   "m 1 800" will update maximum mclk to be 800Mhz.
  *
  *   For sclk voltage curve, enter the new values by writing a
- *   string that contains "vc point clock voff" to the file. The
- *   points are indexed by 0, 1 and 2. E.g., "vc 0 300 10" will
- *   update point1 with clock set as 300Mhz and voltage increased
- *   by 10mV. "vc 2 1000 -10" will update point3 with clock set
- *   as 1000Mhz and voltage drop by 10mV.
+ *   string that contains "vc point clock voltage" to the file. The
+ *   points are indexed by 0, 1 and 2. E.g., "vc 0 300 600" will
+ *   update point1 with clock set as 300Mhz and voltage as
+ *   600mV. "vc 2 1000 1000" will update point3 with clock set
+ *   as 1000Mhz and voltage 1000mV.
  *
  * - When you have edited all of the states as needed, write "c" (commit)
  *   to the file to commit your changes
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 6295244a1737..491daf02467a 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -1003,6 +1003,26 @@ static int vega20_od8_set_feature_id(
return 0;
 }
 
+static int vega20_od8_get_gfx_clock_base_voltage(
+   struct pp_hwmgr *hwmgr,
+   uint32_t *voltage,
+   uint32_t freq)
+{
+   int ret = 0;
+
+   ret = smum_send_msg_to_smc_with_parameter(hwmgr,
+   PPSMC_MSG_GetAVFSVoltageByDpm,
+   ((AVFS_CURVE << 24) | (OD8_HOTCURVE_TEMPERATURE << 16) 
| freq));
+   PP_ASSERT_WITH_CODE(!ret,
+   "[GetBaseVoltage] failed to get GFXCLK AVFS voltage 
from SMU!",
+   return ret);
+
+   vega20_read_arg_from_smc(hwmgr, voltage);
+   *voltage = *voltage / VOLTAGE_SCALE;
+
+   return 0;
+}
+
 static int vega20_od8_initialize_default_settings(
struct pp_hwmgr *hwmgr)
 {
@@ -1038,18 +1058,41 @@ static int vega20_od8_initialize_default_settings(
}
 
if (od8_settings->overdrive8_capabilities & OD8_GFXCLK_CURVE) {
+   od_table->GfxclkFreq1 = od_table->GfxclkFmin;

od8_settings->od8_settings_array[OD8_SETTING_GFXCLK_FREQ1].default_value =
od_table->GfxclkFreq1;
-   
od8_settings->od8_settings_array[OD8_SETTING_GFXCLK_VOLTAGE1].default_value =
-   od_table->GfxclkOffsetVolt1;
-   
od8_settings->od8_settings_array[OD8_SETTING_GFXCLK_FREQ2].default_value =
-   od_table->GfxclkFreq2;
-   
od8_settings->od8_settings_array[OD8_SETTING_GFXCLK_VOLTAGE2].default_value =
-   od_table->GfxclkOffsetVolt2;
+
+   od_table->GfxclkFreq3 = od_table->GfxclkFmax;

od8_settings->od8_settings_array[OD8_SETTING_GFXCLK_FREQ3].default_value =
od_table->GfxclkFreq3;
-   
od8_settings->od8_settings_array[OD8_SETTING_GFXCLK_VOLTAGE3].default_value =
-   od_table->GfxclkOffsetVolt3;
+
+   od_table->GfxclkFreq2 = (od_table->GfxclkFreq1 + 
od_table->GfxclkFreq3) / 2;
+   
od8_settings->od8_settings_array[OD8_SETTING_GFXCLK_FREQ2].default_value =
+   od_table->GfxclkFreq2;
+
+   
PP_ASSERT_WITH_CODE(!vega20_od8_get_gfx_clock_base_voltage(hwmgr,
+  
&(od8_settings->od8_settings_array[OD8_SETTING_GFXCLK_VOLTAGE1].default_value),
+od_table->GfxclkFreq1),
+   "[PhwVega20_OD8_InitializeDefaultSettings] 
Failed to get Base clock voltage from SMU!",
+

[PATCH 1/3] drm/amd/powerplay: update OD feature judgement

[Evan Quan]

Update the conditions to judge whether an OD feature
should be supported on vega20.

Change-Id: Iaabdd4db8f685fb94c960263fe38a21b36377aa2
Signed-off-by: Evan Quan 
---
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 81 +--
 .../drm/amd/powerplay/hwmgr/vega20_pptable.h  |  2 +
 2 files changed, 57 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index ce123096c365..6295244a1737 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -47,6 +47,8 @@
 #include "pp_overdriver.h"
 #include "pp_thermal.h"
 
+#define VOLTAGE_SCALE 4
+
 static void vega20_set_default_registry_data(struct pp_hwmgr *hwmgr)
 {
struct vega20_hwmgr *data =
@@ -832,58 +834,85 @@ static int vega20_od8_set_feature_capabilities(
struct phm_ppt_v3_information *pptable_information =
(struct phm_ppt_v3_information *)hwmgr->pptable;
struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr->backend);
+   PPTable_t *pp_table = &(data->smc_state_table.pp_table);
struct vega20_od8_settings *od_settings = &(data->od8_settings);
 
od_settings->overdrive8_capabilities = 0;
 
if (data->smu_features[GNLD_DPM_GFXCLK].enabled) {
-   if 
(pptable_information->od_settings_max[ATOM_VEGA20_ODSETTING_GFXCLKFMAX] > 0 &&
-   
pptable_information->od_settings_min[ATOM_VEGA20_ODSETTING_GFXCLKFMAX] > 0 &&
-   
pptable_information->od_settings_max[ATOM_VEGA20_ODSETTING_GFXCLKFMIN] > 0 &&
-   
pptable_information->od_settings_min[ATOM_VEGA20_ODSETTING_GFXCLKFMIN] > 0)
+   if 
(pptable_information->od_feature_capabilities[ATOM_VEGA20_ODFEATURE_GFXCLK_LIMITS]
 &&
+   
pptable_information->od_settings_max[OD8_SETTING_GFXCLK_FMAX] > 0 &&
+   
pptable_information->od_settings_min[OD8_SETTING_GFXCLK_FMIN] > 0 &&
+   
(pptable_information->od_settings_max[OD8_SETTING_GFXCLK_FMAX] >=
+   
pptable_information->od_settings_min[OD8_SETTING_GFXCLK_FMIN]))
od_settings->overdrive8_capabilities |= 
OD8_GFXCLK_LIMITS;
 
-   if 
(pptable_information->od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P1] 
> 0 &&
-   
pptable_information->od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P2] 
> 0 &&
-   
pptable_information->od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P3] 
> 0 &&
-   
pptable_information->od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P1] 
> 0 &&
-   
pptable_information->od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P2] 
> 0 &&
-   
pptable_information->od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEFREQ_P3] 
> 0 &&
-   
pptable_information->od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEOFFSET_P1]
 > 0 &&
-   
pptable_information->od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEOFFSET_P2]
 > 0 &&
-   
pptable_information->od_settings_min[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEOFFSET_P3]
 > 0 &&
-   
pptable_information->od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEOFFSET_P1]
 > 0 &&
-   
pptable_information->od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEOFFSET_P2]
 > 0 &&
-   
pptable_information->od_settings_max[ATOM_VEGA20_ODSETTING_VDDGFXCURVEVOLTAGEOFFSET_P3]
 > 0)
+   if 
(pptable_information->od_feature_capabilities[ATOM_VEGA20_ODFEATURE_GFXCLK_CURVE]
 &&
+   
(pptable_information->od_settings_min[OD8_SETTING_GFXCLK_VOLTAGE1] >=
+pp_table->MinVoltageGfx / VOLTAGE_SCALE) &&
+   
(pptable_information->od_settings_max[OD8_SETTING_GFXCLK_VOLTAGE3] <=
+pp_table->MaxVoltageGfx / VOLTAGE_SCALE) &&
+   
(pptable_information->od_settings_max[OD8_SETTING_GFXCLK_VOLTAGE3] >=
+
pptable_information->od_settings_min[OD8_SETTING_GFXCLK_VOLTAGE1]))
od_settings->overdrive8_capabilities |= 
OD8_GFXCLK_CURVE;
}
 
if (data->smu_features[GNLD_DPM_UCLK].enabled) {
-   if 
(pptable_information->od_settings_min[ATOM_VEGA20_ODSETTING_UCLKFMAX] > 0 &&
-   
pptable_information->od_settings_max[ATOM_VEGA20_ODSETTING_UCLKFMAX] > 0)
+   if 
(pptable_information->od_feature_capabilities[ATOM_VEGA20_ODFEATURE_UCLK_MAX] &&
+   pptable_information->od_settings_min[OD8_SETTING_UCLK_FMAX] 
> 0 &&
+   pptable_information->od_settings_max[OD8_SETTING_UCLK_FMAX] 
> 0 &&
+   
(pptable_information->od_settings_max[OD8_SETTING_UCLK_FMAX] >=
+