Re: [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 variants

[Zhang, Jerry]

在 2018年11月28日，00:11，Alex Deucher  写道：
> 
> On Tue, Nov 27, 2018 at 4:56 AM Christian König
>  wrote:
>> 
>> Am 27.11.18 um 02:47 schrieb Zhang, Jerry(Junwei):
>> 
>> On 11/26/18 5:28 PM, Christian König wrote:
>> 
>> Am 26.11.18 um 03:38 schrieb Zhang, Jerry(Junwei):
>> 
>> On 11/24/18 3:32 AM, Deucher, Alexander wrote:
>> 
>> Is this required?  Are the harvesting fuses incorrect?  If the blocks are 
>> harvested, we should bail out of the blocks properly during init.  Also, 
>> please make this more explicit if we still need it.  E.g.,
>> 
>> 
>> 
>> The harvest fuse is indeed disabling UVD and VCE, as it's a mining card.
>> Then any command to UVD/VCE causing NULL pointer issue, like amdgpu_test.
>> 
>> 
>> In this case we should fix the NULL pointer issue instead. Do you have a 
>> backtrace for this?
>> 
>> 
>> Sorry to miss the detail.
>> The NULL pointer is caused by UVD is not initialized as it's disabled in 
>> VBIOS for this kind of card.
>> 
>> 
>> Yeah, but that should be handled correctly.
>> 
>> 
>> When cs submit, it will check ring->funcs->parse_cs in amdgpu_cs_ib_fill().
>> However, uvd_v6_0_early_init() skip the set ring function, as 
>> CC_HARVEST_FUSES is set UVD/VCE disabled.
>> Then the access to UVD/VCE ring's funcs will cause NULL pointer issue.
>> 
>> BTW, Windows driver disables UVD/VCE for it as well.
>> 
>> 
>> You are approaching this from the wrong side. The fact that UVD/VCE is 
>> disabled should already be handled correctly.
>> 
>> The problem is rather that in a couple of places (amdgpu_ctx_init for 
>> example) we assume that we have at least one UVD/VCE ring.
>> 
>> Alex is right that checking the fuses should be sufficient and we rather 
>> need to fix the handling here instead of adding another workaround.
> 
> Exactly.  There are already cards out there with no UVD or VCE, so we
> need to fix this if it's a problem.  It sounds like userspace is
> submitting work to the VCE or UVD rings without checking whether or
> not the device supports them in the first place.  We should do a
> better job of guarding against that in the kernel.

Thanks your all.
Got that meaning now.

we may also print some message that UVD/VCE is not initialized, since it looks 
initialized successfully.
```
[   15.730219] [drm] add ip block number 7 
```
I could check it after the vacation(back next week).

BTW, is that handled by the patch series of [PATCH 1/6] drm/amdgpu: add VCN 
JPEG support amdgpu_ctx_num_entities?
Try to apply the patches, seems amdgpu_test hang at Userptr Test, verified on 
latest staging build
Please confirm that.

[ 4388.759743] BUG: unable to handle kernel NULL pointer dereference at 
0008
[ 4388.759782] IP: amddrm_sched_entity_flush+0x2d/0x1d0 [amd_sched]
[ 4388.759807] PGD 0 P4D 0
[ 4388.759820] Oops:  [#1] SMP PTI
[ 4388.759834] Modules linked in: amdgpu(OE) amdchash(OE) amdttm(OE) 
amd_sched(OE) amdkcl(OE) amd_iommu_v2 drm_kms_helper drm i2c_algo_bit 
fb_sys_fops syscopyarea sysfillrect sysimgblt nls_utf8 cifs ccm rpcsec_gss_krb5 
nfsv4 nfs fscache b
infmt_misc nls_iso8859_1 snd_hda_codec_realtek snd_hda_codec_generic intel_rapl 
x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel snd_hda_codec_hdmi kvm 
snd_hda_intel irqbypass crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hda_co
re snd_hwdep ghash_clmulni_intel snd_seq_midi snd_seq_midi_event pcbc snd_pcm 
snd_rawmidi snd_seq snd_seq_device snd_timer aesni_intel aes_x86_64 crypto_simd 
eeepc_wmi glue_helper snd cryptd asus_wmi intel_cstate soundcore shpchp intel_ra
pl_perf mei_me wmi_bmof intel_wmi_thunderbolt sparse_keymap serio_raw mei 
acpi_pad mac_hid sch_fq_codel
[ 4388.760141]  nfsd auth_rpcgss nfs_acl parport_pc lockd ppdev grace lp sunrpc 
parport ip_tables x_tables autofs4 mxm_wmi e1000e psmouse ptp pps_core ahci 
libahci wmi video
[ 4388.760212] CPU: 7 PID: 915 Comm: amdgpu_test Tainted: G   OE
4.15.0-39-generic #42-Ubuntu
[ 4388.760250] Hardware name: System manufacturer System Product Name/Z170-A, 
BIOS 1302 11/09/2015
[ 4388.760287] RIP: 0010:amddrm_sched_entity_flush+0x2d/0x1d0 [amd_sched]
[ 4388.760314] RSP: 0018:a37b8166bd38 EFLAGS: 00010246
[ 4388.760337] RAX:  RBX: 88776740e5f8 RCX: 
[ 4388.760366] RDX:  RSI: 00fa RDI: 88776740e5f8
[ 4388.760396] RBP: a37b8166bd88 R08: 8877765dab10 R09: 
[ 4388.760425] R10:  R11: 0064 R12: 00fa
[ 4388.760455] R13: 8877606fdf18 R14: 8877606fdef8 R15: 00fa
[ 4388.760484] FS:  7f05b21a1580() GS:8877765c() 
knlGS:
[ 4388.760518] CS:  0010 DS:  ES:  CR0: 80050033
[ 4388.760542] CR2: 0008 CR3: 3020a005 CR4: 003606e0
[ 4388.760572] DR0:  DR1:  DR2: 
[ 4388.760601] DR3:  DR6: fffe0ff0 DR7: 0400
[ 4388.760630] Call Trace:
[ 

[PATCH] drm/amd/display: fix uninitialized symbol 'id' in bios_parser_get_src_obj

[Yang Xiao]

From: Young Xiao 

See commit a8f976473196 ("drm/amd/display: Bunch of smatch error and
warning fixes in DC") for detail.

Signed-off-by: Young Xiao 
---
 drivers/gpu/drm/amd/display/dc/bios/bios_parser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c 
b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
index 0e1dc1b..6420546 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
@@ -188,7 +188,7 @@ static enum bp_result bios_parser_get_src_obj(struct 
dc_bios *dcb,
struct graphics_object_id *src_object_id)
 {
uint32_t number;
-   uint16_t *id;
+   uint16_t *id = NULL;
ATOM_OBJECT *object;
struct bios_parser *bp = BP_FROM_DCB(dcb);
 
@@ -204,7 +204,7 @@ static enum bp_result bios_parser_get_src_obj(struct 
dc_bios *dcb,
 
number = get_src_obj_list(bp, object, );
 
-   if (number <= index)
+   if (number <= index || !id)
return BP_RESULT_BADINPUT;
 
*src_object_id = object_id_from_bios_object_id(id[index]);
-- 
2.7.4

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

Re: [PATCH 8/8] drm/amdgpu: Moved doorbell structures to seperate file

[Alex Deucher]

On Mon, Nov 26, 2018 at 12:29 PM Oak Zeng  wrote:
>
> Move doorbell structures, enum definitions and helper functions
> from amdgpu.h to amdgpu_doorbell.h. No functional change
>
> Change-Id: I09f7b84869b6d3c688b7a2506ff78d62b3de23f5
> Signed-off-by: Oak Zeng 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h  | 227 +---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 249 
> +++
>  2 files changed, 250 insertions(+), 226 deletions(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 2fc5713..70342aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -81,6 +81,7 @@
>  #include "amdgpu_job.h"
>  #include "amdgpu_bo_list.h"
>  #include "amdgpu_gem.h"
> +#include "amdgpu_doorbell.h"
>
>  #define MAX_GPU_INSTANCE   16
>
> @@ -361,173 +362,6 @@ int amdgpu_fence_slab_init(void);
>  void amdgpu_fence_slab_fini(void);
>
>  /*
> - * GPU doorbell structures, functions & helpers
> - */
> -typedef enum _AMDGPU_DOORBELL_ASSIGNMENT
> -{
> -   AMDGPU_DOORBELL_KIQ = 0x000,
> -   AMDGPU_DOORBELL_HIQ = 0x001,
> -   AMDGPU_DOORBELL_DIQ = 0x002,
> -   AMDGPU_DOORBELL_MEC_RING0   = 0x010,
> -   AMDGPU_DOORBELL_MEC_RING1   = 0x011,
> -   AMDGPU_DOORBELL_MEC_RING2   = 0x012,
> -   AMDGPU_DOORBELL_MEC_RING3   = 0x013,
> -   AMDGPU_DOORBELL_MEC_RING4   = 0x014,
> -   AMDGPU_DOORBELL_MEC_RING5   = 0x015,
> -   AMDGPU_DOORBELL_MEC_RING6   = 0x016,
> -   AMDGPU_DOORBELL_MEC_RING7   = 0x017,
> -   AMDGPU_DOORBELL_GFX_RING0   = 0x020,
> -   AMDGPU_DOORBELL_sDMA_ENGINE0= 0x1E0,
> -   AMDGPU_DOORBELL_sDMA_ENGINE1= 0x1E1,
> -   AMDGPU_DOORBELL_IH  = 0x1E8,
> -   AMDGPU_DOORBELL_MAX_ASSIGNMENT  = 0x3FF,
> -   AMDGPU_DOORBELL_INVALID = 0x
> -} AMDGPU_DOORBELL_ASSIGNMENT;
> -
> -struct amdgpu_doorbell {
> -   /* doorbell mmio */
> -   resource_size_t base;
> -   resource_size_t size;
> -   u32 __iomem *ptr;
> -   u32 num_doorbells;  /* Number of doorbells 
> actually reserved for amdgpu. */
> -};
> -
> -typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
> -{
> -   /* Compute + GFX: 0~255 */
> -   AMDGPU_VEGA20_DOORBELL_KIQ = 0x000,
> -   AMDGPU_VEGA20_DOORBELL_HIQ = 0x001,
> -   AMDGPU_VEGA20_DOORBELL_DIQ = 0x002,
> -   AMDGPU_VEGA20_DOORBELL_MEC_RING0   = 0x003,
> -   AMDGPU_VEGA20_DOORBELL_MEC_RING1   = 0x004,
> -   AMDGPU_VEGA20_DOORBELL_MEC_RING2   = 0x005,
> -   AMDGPU_VEGA20_DOORBELL_MEC_RING3   = 0x006,
> -   AMDGPU_VEGA20_DOORBELL_MEC_RING4   = 0x007,
> -   AMDGPU_VEGA20_DOORBELL_MEC_RING5   = 0x008,
> -   AMDGPU_VEGA20_DOORBELL_MEC_RING6   = 0x009,
> -   AMDGPU_VEGA20_DOORBELL_MEC_RING7   = 0x00A,
> -   AMDGPU_VEGA20_DOORBELL_USERQUEUE_START = 0x00B,
> -   AMDGPU_VEGA20_DOORBELL_USERQUEUE_END   = 0x08A,
> -   AMDGPU_VEGA20_DOORBELL_GFX_RING0   = 0x08B,
> -   /* SDMA:256~335*/
> -   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE0= 0x100,
> -   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE1= 0x10A,
> -   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE2= 0x114,
> -   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE3= 0x11E,
> -   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE4= 0x128,
> -   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE5= 0x132,
> -   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE6= 0x13C,
> -   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE7= 0x146,
> -   /* IH: 376~391 */
> -   AMDGPU_VEGA20_DOORBELL_IH  = 0x178,
> -   /* MMSCH: 392~407
> -* overlap the doorbell assignment with VCN as they are  mutually 
> exclusive
> -* VCE engine's doorbell is 32 bit and two VCE ring share one QWORD
> -*/
> -   AMDGPU_VEGA20_DOORBELL64_VCN0_1  = 0x188, /* lower 32 
> bits for VNC0 and upper 32 bits for VNC1 */
> -   AMDGPU_VEGA20_DOORBELL64_VCN2_3  = 0x189,
> -   AMDGPU_VEGA20_DOORBELL64_VCN4_5  = 0x18A,
> -   AMDGPU_VEGA20_DOORBELL64_VCN6_7  = 0x18B,
> -
> -   AMDGPU_VEGA20_DOORBELL64_UVD_RING0_1 = 0x188,
> -   AMDGPU_VEGA20_DOORBELL64_UVD_RING2_3 = 0x189,
> -   AMDGPU_VEGA20_DOORBELL64_UVD_RING4_5 = 0x18A,
> -   AMDGPU_VEGA20_DOORBELL64_UVD_RING6_7   

Re: [PATCH 7/8] drm/amdgpu: Use asic specific doorbell index instead of macro definition

[Alex Deucher]

On Mon, Nov 26, 2018 at 12:29 PM Oak Zeng  wrote:
>
> Change-Id: I84475efcfb482c474fccb133010abb5df5f4
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

With a patch description added:
Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 27 ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c|  2 +-
>  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 10 +-
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  |  8 
>  drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  2 +-
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 25 +
>  drivers/gpu/drm/amd/amdgpu/tonga_ih.c  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c  |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/vce_v4_0.c  |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/vega10_ih.c |  2 +-
>  12 files changed, 36 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 1c1fed6..d693b804 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -181,25 +181,14 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device 
> *adev)
>  * process in case of 64-bit doorbells so we
>  * can use each doorbell assignment twice.
>  */
> -   if (adev->asic_type == CHIP_VEGA10) {
> -   gpu_resources.sdma_doorbell[0][i] =
> -   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 
> + (i >> 1);
> -   gpu_resources.sdma_doorbell[0][i+1] =
> -   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 
> + 0x200 + (i >> 1);
> -   gpu_resources.sdma_doorbell[1][i] =
> -   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 
> + (i >> 1);
> -   gpu_resources.sdma_doorbell[1][i+1] =
> -   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 
> + 0x200 + (i >> 1);
> -   } else {
> -   gpu_resources.sdma_doorbell[0][i] =
> -   AMDGPU_DOORBELL64_sDMA_ENGINE0 + (i 
> >> 1);
> -   gpu_resources.sdma_doorbell[0][i+1] =
> -   AMDGPU_DOORBELL64_sDMA_ENGINE0 + 
> 0x200 + (i >> 1);
> -   gpu_resources.sdma_doorbell[1][i] =
> -   AMDGPU_DOORBELL64_sDMA_ENGINE1 + (i 
> >> 1);
> -   gpu_resources.sdma_doorbell[1][i+1] =
> -   AMDGPU_DOORBELL64_sDMA_ENGINE1 + 
> 0x200 + (i >> 1);
> -   }
> +   gpu_resources.sdma_doorbell[0][i] =
> +   adev->doorbell_index.sdma_engine0 + (i >> 1);
> +   gpu_resources.sdma_doorbell[0][i+1] =
> +   adev->doorbell_index.sdma_engine0 + 0x200 + 
> (i >> 1);
> +   gpu_resources.sdma_doorbell[1][i] =
> +   adev->doorbell_index.sdma_engine1 + (i >> 1);
> +   gpu_resources.sdma_doorbell[1][i+1] =
> +   adev->doorbell_index.sdma_engine1 + 0x200 + 
> (i >> 1);
> }
> /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
>  * SDMA, IH and VCN. So don't use them for the CP.
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index a8b1c9c..fdbc2c2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -532,7 +532,7 @@ static int amdgpu_device_doorbell_init(struct 
> amdgpu_device *adev)
> adev->doorbell.size = pci_resource_len(adev->pdev, 2);
>
> adev->doorbell.num_doorbells = min_t(u32, adev->doorbell.size / 
> sizeof(u32),
> -
> AMDGPU_DOORBELL_MAX_ASSIGNMENT+1);
> +
> adev->doorbell_index.max_assignment+1);
> if (adev->doorbell.num_doorbells == 0)
> return -EINVAL;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 6a70c0b..97a60da 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -250,7 +250,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
> ring->adev = NULL;
> ring->ring_obj = NULL;
> ring->use_doorbell = true;
> -   ring->doorbell_index = AMDGPU_DOORBELL_KIQ;
> +   ring->doorbell_index = 

Re: [PATCH 6/8] drm/amdgpu: Call doorbell index init on device initialization

[Alex Deucher]

On Mon, Nov 26, 2018 at 12:29 PM Oak Zeng  wrote:
>
> Also call functioin amdgpu_device_doorbell_init after
> amdgpu_device_ip_early_init because the former depends
> on the later to set up asic-specific init_doorbell_index
> function
>
> Change-Id: I2f004bbbe2565035460686f4fc16e86b77a2a9b5
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index cb06e68..a8b1c9c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -513,6 +513,8 @@ void amdgpu_device_pci_config_reset(struct amdgpu_device 
> *adev)
>   */
>  static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
>  {
> +   amdgpu_asic_init_doorbell_index(adev);
> +
> /* No doorbell on SI hardware generation */
> if (adev->asic_type < CHIP_BONAIRE) {
> adev->doorbell.base = 0;
> @@ -2464,9 +2466,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)adev->rmmio_base);
> DRM_INFO("register mmio size: %u\n", (unsigned)adev->rmmio_size);
>
> -   /* doorbell bar mapping */
> -   amdgpu_device_doorbell_init(adev);
> -
> /* io port mapping */
> for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> if (pci_resource_flags(adev->pdev, i) & IORESOURCE_IO) {
> @@ -2485,6 +2484,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> if (r)
> return r;
>
> +   /* doorbell bar mapping and doorbell index init*/
> +   amdgpu_device_doorbell_init(adev);
> +
> /* if we have > 1 VGA cards, then disable the amdgpu VGA resources */
> /* this will fail for cards that aren't VGA class devices, just
>  * ignore it */
> --
> 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 5/8] drm/amdgpu: Doorbell layout for vega20 and future asic

[Alex Deucher]

On Mon, Nov 26, 2018 at 12:29 PM Oak Zeng  wrote:
>
> v2: Use enum definition instead of hardcoded value
>
> Change-Id: I04d22fb717ac50483c0835f160a2e860e344f358
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

With a patch description added:
Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h  | 50 
> 
>  drivers/gpu/drm/amd/amdgpu/soc15.c   | 22 +++-
>  drivers/gpu/drm/amd/amdgpu/soc15.h   |  1 +
>  drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 33 ++
>  4 files changed, 105 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 719da32..2fc5713 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -392,6 +392,56 @@ struct amdgpu_doorbell {
> u32 num_doorbells;  /* Number of doorbells 
> actually reserved for amdgpu. */
>  };
>
> +typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
> +{
> +   /* Compute + GFX: 0~255 */
> +   AMDGPU_VEGA20_DOORBELL_KIQ = 0x000,
> +   AMDGPU_VEGA20_DOORBELL_HIQ = 0x001,
> +   AMDGPU_VEGA20_DOORBELL_DIQ = 0x002,
> +   AMDGPU_VEGA20_DOORBELL_MEC_RING0   = 0x003,
> +   AMDGPU_VEGA20_DOORBELL_MEC_RING1   = 0x004,
> +   AMDGPU_VEGA20_DOORBELL_MEC_RING2   = 0x005,
> +   AMDGPU_VEGA20_DOORBELL_MEC_RING3   = 0x006,
> +   AMDGPU_VEGA20_DOORBELL_MEC_RING4   = 0x007,
> +   AMDGPU_VEGA20_DOORBELL_MEC_RING5   = 0x008,
> +   AMDGPU_VEGA20_DOORBELL_MEC_RING6   = 0x009,
> +   AMDGPU_VEGA20_DOORBELL_MEC_RING7   = 0x00A,
> +   AMDGPU_VEGA20_DOORBELL_USERQUEUE_START = 0x00B,
> +   AMDGPU_VEGA20_DOORBELL_USERQUEUE_END   = 0x08A,
> +   AMDGPU_VEGA20_DOORBELL_GFX_RING0   = 0x08B,
> +   /* SDMA:256~335*/
> +   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE0= 0x100,
> +   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE1= 0x10A,
> +   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE2= 0x114,
> +   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE3= 0x11E,
> +   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE4= 0x128,
> +   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE5= 0x132,
> +   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE6= 0x13C,
> +   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE7= 0x146,
> +   /* IH: 376~391 */
> +   AMDGPU_VEGA20_DOORBELL_IH  = 0x178,
> +   /* MMSCH: 392~407
> +* overlap the doorbell assignment with VCN as they are  mutually 
> exclusive
> +* VCE engine's doorbell is 32 bit and two VCE ring share one QWORD
> +*/
> +   AMDGPU_VEGA20_DOORBELL64_VCN0_1  = 0x188, /* lower 32 
> bits for VNC0 and upper 32 bits for VNC1 */
> +   AMDGPU_VEGA20_DOORBELL64_VCN2_3  = 0x189,
> +   AMDGPU_VEGA20_DOORBELL64_VCN4_5  = 0x18A,
> +   AMDGPU_VEGA20_DOORBELL64_VCN6_7  = 0x18B,
> +
> +   AMDGPU_VEGA20_DOORBELL64_UVD_RING0_1 = 0x188,
> +   AMDGPU_VEGA20_DOORBELL64_UVD_RING2_3 = 0x189,
> +   AMDGPU_VEGA20_DOORBELL64_UVD_RING4_5 = 0x18A,
> +   AMDGPU_VEGA20_DOORBELL64_UVD_RING6_7 = 0x18B,
> +
> +   AMDGPU_VEGA20_DOORBELL64_VCE_RING0_1 = 0x18C,
> +   AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
> +   AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
> +   AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
> +   AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT= 0x18F,
> +   AMDGPU_VEGA20_DOORBELL_INVALID   = 0x
> +} AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
> +
>  /*
>   * 64bit doorbell, offset are in QWORD, occupy 2KB doorbell space
>   */
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
> b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index cae25dd..83624e1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -616,6 +616,23 @@ static const struct amdgpu_asic_funcs soc15_asic_funcs =
> .init_doorbell_index = _doorbell_index_init,
>  };
>
> +static const struct amdgpu_asic_funcs vega20_asic_funcs =
> +{
> +   .read_disabled_bios = _read_disabled_bios,
> +   .read_bios_from_rom = _read_bios_from_rom,
> +   .read_register = _read_register,
> +   .reset = _asic_reset,
> +   .set_vga_state = _vga_set_state,
> +   .get_xclk = _get_xclk,
> +   .set_uvd_clocks = _set_uvd_clocks,
> +   .set_vce_clocks = _set_vce_clocks,
> +   .get_config_memsize = _get_config_memsize,
> +   .flush_hdp = _flush_hdp,
> +   .invalidate_hdp = _invalidate_hdp,
> +   .need_full_reset = _need_full_reset,
> +   

Re: [PATCH 4/8] drm/amdgpu: Doorbell index initialization for ASICs before vega10

[Alex Deucher]

On Mon, Nov 26, 2018 at 12:29 PM Oak Zeng  wrote:
>
> v2: Use enum definition instead of hardcoded number
>
> Change-Id: Id64eb98f5b1c24b51eb2fd5a083086fc3515813d
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

With a patch description added:
Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/cik.c |  1 +
>  drivers/gpu/drm/amd/amdgpu/cik.h |  1 +
>  drivers/gpu/drm/amd/amdgpu/vi.c  | 19 +++
>  drivers/gpu/drm/amd/amdgpu/vi.h  |  1 +
>  4 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c 
> b/drivers/gpu/drm/amd/amdgpu/cik.c
> index f41f5f5..71c50d8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik.c
> @@ -1755,6 +1755,7 @@ static const struct amdgpu_asic_funcs cik_asic_funcs =
> .flush_hdp = _flush_hdp,
> .invalidate_hdp = _invalidate_hdp,
> .need_full_reset = _need_full_reset,
> +   .init_doorbell_index = _doorbell_index_init,
>  };
>
>  static int cik_common_early_init(void *handle)
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik.h 
> b/drivers/gpu/drm/amd/amdgpu/cik.h
> index e49c6f1..54c625a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik.h
> +++ b/drivers/gpu/drm/amd/amdgpu/cik.h
> @@ -30,4 +30,5 @@ void cik_srbm_select(struct amdgpu_device *adev,
>  u32 me, u32 pipe, u32 queue, u32 vmid);
>  int cik_set_ip_blocks(struct amdgpu_device *adev);
>
> +void legacy_doorbell_index_init(struct amdgpu_device *adev);
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
> index 07880d3..ff2906c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> @@ -955,6 +955,7 @@ static const struct amdgpu_asic_funcs vi_asic_funcs =
> .flush_hdp = _flush_hdp,
> .invalidate_hdp = _invalidate_hdp,
> .need_full_reset = _need_full_reset,
> +   .init_doorbell_index = _doorbell_index_init,
>  };
>
>  #define CZ_REV_BRISTOL(rev) \
> @@ -1712,3 +1713,21 @@ int vi_set_ip_blocks(struct amdgpu_device *adev)
>
> return 0;
>  }
> +
> +void legacy_doorbell_index_init(struct amdgpu_device *adev)
> +{
> +   adev->doorbell_index.kiq = AMDGPU_DOORBELL_KIQ;
> +   adev->doorbell_index.mec_ring0 = AMDGPU_DOORBELL_MEC_RING0;
> +   adev->doorbell_index.mec_ring1 = AMDGPU_DOORBELL_MEC_RING1;
> +   adev->doorbell_index.mec_ring2 = AMDGPU_DOORBELL_MEC_RING2;
> +   adev->doorbell_index.mec_ring3 = AMDGPU_DOORBELL_MEC_RING3;
> +   adev->doorbell_index.mec_ring4 = AMDGPU_DOORBELL_MEC_RING4;
> +   adev->doorbell_index.mec_ring5 = AMDGPU_DOORBELL_MEC_RING5;
> +   adev->doorbell_index.mec_ring6 = AMDGPU_DOORBELL_MEC_RING6;
> +   adev->doorbell_index.mec_ring7 = AMDGPU_DOORBELL_MEC_RING7;
> +   adev->doorbell_index.gfx_ring0 = AMDGPU_DOORBELL_GFX_RING0;
> +   adev->doorbell_index.sdma_engine0 = AMDGPU_DOORBELL_sDMA_ENGINE0;
> +   adev->doorbell_index.sdma_engine1 = AMDGPU_DOORBELL_sDMA_ENGINE1;
> +   adev->doorbell_index.ih = AMDGPU_DOORBELL_IH;
> +   adev->doorbell_index.max_assignment = AMDGPU_DOORBELL_MAX_ASSIGNMENT;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.h b/drivers/gpu/drm/amd/amdgpu/vi.h
> index 0429fe3..8de0772 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vi.h
> +++ b/drivers/gpu/drm/amd/amdgpu/vi.h
> @@ -30,4 +30,5 @@ void vi_srbm_select(struct amdgpu_device *adev,
> u32 me, u32 pipe, u32 queue, u32 vmid);
>  int vi_set_ip_blocks(struct amdgpu_device *adev);
>
> +void legacy_doorbell_index_init(struct amdgpu_device *adev);
>  #endif
> --
> 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 3/8] drm/amdgpu: Vega10 doorbell index initialization

[Alex Deucher]

On Mon, Nov 26, 2018 at 5:51 PM Oak Zeng  wrote:
>
> v2: Use enum definition instead of hardcoded value
> v3: Remove unused enum definition
>
> Change-Id: Ib72058337f0aa53adfc6c6aae5341a7cd665111a
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h  | 14 --
>  drivers/gpu/drm/amd/amdgpu/soc15.c   |  1 +
>  drivers/gpu/drm/amd/amdgpu/soc15.h   |  1 +
>  drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 28 
> 
>  4 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 686652d..ea4dbcf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -434,20 +434,14 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
>  * default non-graphics QWORD index is 0xe0 - 0xFF inclusive
>  */
>
> -   /* sDMA engines  reserved from 0xe0 -0xef  */
> -   AMDGPU_DOORBELL64_sDMA_ENGINE0= 0xE0,
> -   AMDGPU_DOORBELL64_sDMA_HI_PRI_ENGINE0 = 0xE1,
> -   AMDGPU_DOORBELL64_sDMA_ENGINE1= 0xE8,
> -   AMDGPU_DOORBELL64_sDMA_HI_PRI_ENGINE1 = 0xE9,
> -
> /* For vega10 sriov, the sdma doorbell must be fixed as follow
>  * to keep the same setting with host driver, or it will
>  * happen conflicts
>  */
> -   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0= 0xF0,
> -   AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 = 0xF1,
> -   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1= 0xF2,
> -   AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 = 0xF3,
> +   AMDGPU_DOORBELL64_sDMA_ENGINE0= 0xF0,
> +   AMDGPU_DOORBELL64_sDMA_HI_PRI_ENGINE0 = 0xF1,
> +   AMDGPU_DOORBELL64_sDMA_ENGINE1= 0xF2,
> +   AMDGPU_DOORBELL64_sDMA_HI_PRI_ENGINE1 = 0xF3,
>
> /* Interrupt handler */
> AMDGPU_DOORBELL64_IH  = 0xF4,  /* For legacy 
> interrupt ring buffer */
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
> b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 4cc0dcb..cae25dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -613,6 +613,7 @@ static const struct amdgpu_asic_funcs soc15_asic_funcs =
> .flush_hdp = _flush_hdp,
> .invalidate_hdp = _invalidate_hdp,
> .need_full_reset = _need_full_reset,
> +   .init_doorbell_index = _doorbell_index_init,
>  };
>
>  static int soc15_common_early_init(void *handle)
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.h 
> b/drivers/gpu/drm/amd/amdgpu/soc15.h
> index f8ad780..d37c57d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.h
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.h
> @@ -58,4 +58,5 @@ void soc15_program_register_sequence(struct amdgpu_device 
> *adev,
>  int vega10_reg_base_init(struct amdgpu_device *adev);
>  int vega20_reg_base_init(struct amdgpu_device *adev);
>
> +void vega10_doorbell_index_init(struct amdgpu_device *adev);
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c 
> b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> index c5c9b2b..422674b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> @@ -56,4 +56,32 @@ int vega10_reg_base_init(struct amdgpu_device *adev)
> return 0;
>  }
>
> +void vega10_doorbell_index_init(struct amdgpu_device *adev)
> +{
> +   adev->doorbell_index.kiq = AMDGPU_DOORBELL64_KIQ;
> +   adev->doorbell_index.mec_ring0 = AMDGPU_DOORBELL64_MEC_RING0;
> +   adev->doorbell_index.mec_ring1 = AMDGPU_DOORBELL64_MEC_RING1;
> +   adev->doorbell_index.mec_ring2 = AMDGPU_DOORBELL64_MEC_RING2;
> +   adev->doorbell_index.mec_ring3 = AMDGPU_DOORBELL64_MEC_RING3;
> +   adev->doorbell_index.mec_ring4 = AMDGPU_DOORBELL64_MEC_RING4;
> +   adev->doorbell_index.mec_ring5 = AMDGPU_DOORBELL64_MEC_RING5;
> +   adev->doorbell_index.mec_ring6 = AMDGPU_DOORBELL64_MEC_RING6;
> +   adev->doorbell_index.mec_ring7 = AMDGPU_DOORBELL64_MEC_RING7;
> +   adev->doorbell_index.userqueue_start = 
> AMDGPU_DOORBELL64_USERQUEUE_START;
> +   adev->doorbell_index.userqueue_end = AMDGPU_DOORBELL64_USERQUEUE_END;
> +   adev->doorbell_index.gfx_ring0 = AMDGPU_DOORBELL64_GFX_RING0;
> +   adev->doorbell_index.sdma_engine0 = AMDGPU_DOORBELL64_sDMA_ENGINE0;
> +   adev->doorbell_index.sdma_engine1 = AMDGPU_DOORBELL64_sDMA_ENGINE1;
> +   adev->doorbell_index.ih = AMDGPU_DOORBELL64_IH;
> +   adev->doorbell_index.uvd_vce.uvd_ring0_1 = 
> AMDGPU_DOORBELL64_UVD_RING0_1;
> +   adev->doorbell_index.uvd_vce.uvd_ring2_3 = 
> AMDGPU_DOORBELL64_UVD_RING2_3;
> +   adev->doorbell_index.uvd_vce.uvd_ring4_5 = 
> AMDGPU_DOORBELL64_UVD_RING4_5;
> +   adev->doorbell_index.uvd_vce.uvd_ring6_7 = 
> 

Re: [PATCH 1/8] drm/amdgpu: Add field in amdgpu_dev to hold reserved doorbell index

[Alex Deucher]

On Mon, Nov 26, 2018 at 12:29 PM Oak Zeng  wrote:
>
> This is a preparation work to make reserved doorbell index per device,
> instead of using a global macro definition. By doing this, we can easily
> change doorbell layout for future ASICs while not affecting ASICs in
> production.
>
> Change-Id: If08e2bc9d0749748ed4083ba4eb32a4698763085
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h | 50 
> +
>  1 file changed, 50 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 2c80453..b7ee4ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -813,6 +813,55 @@ struct amd_powerplay {
> uint32_t pp_feature;
>  };
>
> +/* Reserved doorbells for amdgpu (including multimedia).
> + * KFD can use all the rest in the 2M doorbell bar.
> + * For asic before vega10, doorbell is 32-bit, so the
> + * index/offset is in dword. For vega10 and after, doorbell
> + * can be 64-bit, so the index defined is in qword.
> + */
> +struct amdgpu_doorbell_index {
> +   uint32_t kiq;
> +   uint32_t mec_ring0;
> +   uint32_t mec_ring1;
> +   uint32_t mec_ring2;
> +   uint32_t mec_ring3;
> +   uint32_t mec_ring4;
> +   uint32_t mec_ring5;
> +   uint32_t mec_ring6;
> +   uint32_t mec_ring7;
> +   uint32_t userqueue_start;
> +   uint32_t userqueue_end;
> +   uint32_t gfx_ring0;
> +   uint32_t sdma_engine0;
> +   uint32_t sdma_engine1;
> +   uint32_t sdma_engine2;
> +   uint32_t sdma_engine3;
> +   uint32_t sdma_engine4;
> +   uint32_t sdma_engine5;
> +   uint32_t sdma_engine6;
> +   uint32_t sdma_engine7;
> +   uint32_t ih;
> +   union {
> +   struct {
> +   uint32_t vcn_ring0_1;
> +   uint32_t vcn_ring2_3;
> +   uint32_t vcn_ring4_5;
> +   uint32_t vcn_ring6_7;
> +   } vcn;
> +   struct {
> +   uint32_t uvd_ring0_1;
> +   uint32_t uvd_ring2_3;
> +   uint32_t uvd_ring4_5;
> +   uint32_t uvd_ring6_7;
> +   uint32_t vce_ring0_1;
> +   uint32_t vce_ring2_3;
> +   uint32_t vce_ring4_5;
> +   uint32_t vce_ring6_7;
> +   } uvd_vce;
> +   };
> +   uint32_t max_assignment;
> +};
> +
>  #define AMDGPU_RESET_MAGIC_NUM 64
>  struct amdgpu_device {
> struct device   *dev;
> @@ -1026,6 +1075,7 @@ struct amdgpu_device {
> unsigned long last_mm_index;
> boolin_gpu_reset;
> struct mutex  lock_reset;
> +   struct amdgpu_doorbell_index doorbell_index;
>  };
>
>  static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device 
> *bdev)
> --
> 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 2/8] drm/amdgpu: Add asic func interface to init doorbell index

[Alex Deucher]

On Mon, Nov 26, 2018 at 12:29 PM Oak Zeng  wrote:
>
> Change-Id: I7e8a9c9dfd4f3bd0902679a1771c1a043ece2674
> Signed-off-by: Oak Zeng 

With a patch description added,
Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index b7ee4ef..719da32 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -654,6 +654,8 @@ struct amdgpu_asic_funcs {
>struct amdgpu_ring *ring);
> /* check if the asic needs a full reset of if soft reset will work */
> bool (*need_full_reset)(struct amdgpu_device *adev);
> +   /* initialize doorbell layout for specific asic*/
> +   void (*init_doorbell_index)(struct amdgpu_device *adev);
>  };
>
>  /*
> @@ -1212,6 +1214,7 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
>  #define amdgpu_asic_flush_hdp(adev, r) (adev)->asic_funcs->flush_hdp((adev), 
> (r))
>  #define amdgpu_asic_invalidate_hdp(adev, r) 
> (adev)->asic_funcs->invalidate_hdp((adev), (r))
>  #define amdgpu_asic_need_full_reset(adev) 
> (adev)->asic_funcs->need_full_reset((adev))
> +#define amdgpu_asic_init_doorbell_index(adev) 
> (adev)->asic_funcs->init_doorbell_index((adev))
>
>  /* Common functions */
>  bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
> --
> 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] drm/amdgpu: add the checking to avoid NULL pointer dereference

[Sharma, Deepak]

-Original Message-
From: amd-gfx  On Behalf Of Christian 
König
Sent: Tuesday, November 27, 2018 1:52 AM
To: Sharma, Deepak ; Zhou, David(ChunMing) 
; Koenig, Christian ; Sharma, 
Deepak ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer 
dereference

Am 27.11.18 um 01:40 schrieb Deepak Sharma:
>
> On 11/26/18 1:57 AM, Zhou, David(ChunMing) wrote:
>>
>>> -Original Message-
>>> From: Christian König 
>>> Sent: Monday, November 26, 2018 5:23 PM
>>> To: Sharma, Deepak ; Zhou, David(ChunMing)
>>> ; Koenig, Christian ;
>>> amd-gfx@lists.freedesktop.org
>>> Subject: Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer
>>> dereference
>>>
>>> Am 26.11.18 um 02:59 schrieb Sharma, Deepak:
 在 2018/11/24 2:10, Koenig, Christian 写道:
> Am 23.11.18 um 15:10 schrieb Zhou, David(ChunMing):
>> 在 2018/11/23 21:30, Koenig, Christian 写道:
>>> Am 23.11.18 um 14:27 schrieb Zhou, David(ChunMing):
 在 2018/11/22 19:25, Christian König 写道:
> Am 22.11.18 um 07:56 schrieb Sharma, Deepak:
>> when returned fence is not valid mostly due to userspace ignored
>> previous error causes NULL pointer dereference.
> Again, this is clearly incorrect. The my other mails on the
> earlier patch.
 Sorry for I didn't get your history, but looks from the patch
 itself, it is still a valid patch, isn't it?
>>> No, the semantic of amdgpu_ctx_get_fence() is that we return NULL
>>> when the fence is already signaled.
>>>
>>> So this patch could totally break userspace because it changes the
>>> behavior when we try to sync to an already signaled fence.
>> Ah, I got your meaning, how about attached patch?
> Yeah something like this, but I would just give the
> DRM_SYNCOBJ_CREATE_SIGNALED instead.
>
> I mean that's what this flag is good for isn't it?
 Yeah, I give a flag initally when creating patch, but as you know, there 
 is a
>>> swich case not be able to use that flag:
     case AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD:
     fd = get_unused_fd_flags(O_CLOEXEC);
     if (fd < 0) {
     dma_fence_put(fence);
     return fd;
     }

     sync_file = sync_file_create(fence);
     dma_fence_put(fence);
     if (!sync_file) {
     put_unused_fd(fd);
     return -ENOMEM;
     }

     fd_install(fd, sync_file->file);
     info->out.handle = fd;
     return 0;

 So I change to stub fence instead.
>>> Yeah, I've missed that case. Not sure if the sync_file can deal with a NULL
>>> fence.
>>>
>>> We should then probably move the stub fence function into
>>> dma_fence_stub.c under drivers/dma-buf to keep the stuff together.
>> Yes, you wrap it to review first with your stub fence, we can do it 
>> separately first.
>>
>> -David
 -David

 I have not applied this patch.
 The issue was trying to address is when amdgpu_cs_ioctl() failed due to
>>> low memory (ENOMEM) but userspace chose to proceed and called
>>> amdgpu_cs_fence_to_handle_ioctl().
 In amdgpu_cs_fence_to_handle_ioctl(), fence is null and later causing
 NULL pointer dereference, this patch was to avoid that and system panic
>>> But I understand now that its a valid case retuning NULL if fence was 
>>> already
>>> signaled but need to handle case so avoid kernel panic. Seems David patch
>>> should fix this, I will test it tomorrow.
>>>
>>> Mhm, but don't we bail out with an error if we ask for a failed command
>>> submission? If not that sounds like a bug as well.
>>>
>>> Christian.
>>>
> Where do we do that?
> I see error
> [drm:amdgpu_cs_ioctl] *ERROR* amdgpu_cs_list_validate(validated) failed.
> [drm:amdgpu_cs_ioctl] *ERROR* Not enough memory for command submission!
> BUG: unable to handle kernel NULL pointer dereference at 0008
> Did some more debugging,dma_fence_is_array() is causing NULL pointer
> dereference call through sync_file_ioctl.
>
> Also I think changes in David patch cant be applied on
> amd-staging-drm-next, which all patches I should take to get it correctly?

> Mhm, what you say here actually doesn't make much sense.

Yes  I did mixed different issue here at end , please ignore it.

When the sequence number is invalid because the submission failed 
amdgpu_ctx_get_fence() returns an error:
>     if (seq >= centity->sequence) {
>     spin_unlock(>ring_lock);
>     return ERR_PTR(-EINVAL);
>     }

This error is then handled in amdgpu_cs_fence_to_handle_ioctl():
>     fence = amdgpu_cs_get_fence(adev, filp, >in.fence);
>     if (IS_ERR(fence))
>     return PTR_ERR(fence);

So the error handling for this is actually in place.

The only thing that could go wrong is that userspace sends a 

Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer dereference

[Sharma, Deepak]

Thanks David ,

I did backport the drm patch to my kernel  after that I am not getting -ENOMEM 
from amdgpu_cs_ioctl while running tests  so have not been able to test patch 
to handle signaled fence. As this issue is hard to reproduce , will give some 
more try.

But yes the problem is there and need to handle when fence is null that your 
patch seems to handle it correctly.

-Deepak

From: Zhou, David(ChunMing)
Sent: Monday, November 26, 2018 6:40 PM
To: Sharma, Deepak; Zhou, David(ChunMing); Koenig, Christian; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer 
dereference

Yeah, you need another drm patch as well when you apply my patch. Attached.

-David


On 2018年11月27日 08:40, Sharma, Deepak wrote:
>
> On 11/26/18 1:57 AM, Zhou, David(ChunMing) wrote:
>>
>>> -Original Message-
>>> From: Christian König 
>>> Sent: Monday, November 26, 2018 5:23 PM
>>> To: Sharma, Deepak ; Zhou, David(ChunMing)
>>> ; Koenig, Christian ;
>>> amd-gfx@lists.freedesktop.org
>>> Subject: Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer
>>> dereference
>>>
>>> Am 26.11.18 um 02:59 schrieb Sharma, Deepak:
 在 2018/11/24 2:10, Koenig, Christian 写道:
> Am 23.11.18 um 15:10 schrieb Zhou, David(ChunMing):
>> 在 2018/11/23 21:30, Koenig, Christian 写道:
>>> Am 23.11.18 um 14:27 schrieb Zhou, David(ChunMing):
 在 2018/11/22 19:25, Christian König 写道:
> Am 22.11.18 um 07:56 schrieb Sharma, Deepak:
>> when returned fence is not valid mostly due to userspace ignored
>> previous error causes NULL pointer dereference.
> Again, this is clearly incorrect. The my other mails on the
> earlier patch.
 Sorry for I didn't get your history, but looks from the patch
 itself, it is still a valid patch, isn't it?
>>> No, the semantic of amdgpu_ctx_get_fence() is that we return NULL
>>> when the fence is already signaled.
>>>
>>> So this patch could totally break userspace because it changes the
>>> behavior when we try to sync to an already signaled fence.
>> Ah, I got your meaning, how about attached patch?
> Yeah something like this, but I would just give the
> DRM_SYNCOBJ_CREATE_SIGNALED instead.
>
> I mean that's what this flag is good for isn't it?
 Yeah, I give a flag initally when creating patch, but as you know, there 
 is a
>>> swich case not be able to use that flag:
 case AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD:
 fd = get_unused_fd_flags(O_CLOEXEC);
 if (fd < 0) {
 dma_fence_put(fence);
 return fd;
 }

 sync_file = sync_file_create(fence);
 dma_fence_put(fence);
 if (!sync_file) {
 put_unused_fd(fd);
 return -ENOMEM;
 }

 fd_install(fd, sync_file->file);
 info->out.handle = fd;
 return 0;

 So I change to stub fence instead.
>>> Yeah, I've missed that case. Not sure if the sync_file can deal with a NULL
>>> fence.
>>>
>>> We should then probably move the stub fence function into
>>> dma_fence_stub.c under drivers/dma-buf to keep the stuff together.
>> Yes, you wrap it to review first with your stub fence, we can do it 
>> separately first.
>>
>> -David
 -David

 I have not applied this patch.
 The issue was trying to address is when amdgpu_cs_ioctl() failed due to
>>> low memory (ENOMEM) but userspace chose to proceed and called
>>> amdgpu_cs_fence_to_handle_ioctl().
 In amdgpu_cs_fence_to_handle_ioctl(), fence is null and later causing
 NULL pointer dereference, this patch was to avoid that and system panic
>>> But I understand now that its a valid case retuning NULL if fence was 
>>> already
>>> signaled but need to handle case so avoid kernel panic. Seems David patch
>>> should fix this, I will test it tomorrow.
>>>
>>> Mhm, but don't we bail out with an error if we ask for a failed command
>>> submission? If not that sounds like a bug as well.
>>>
>>> Christian.
>>>
> Where do we do that?
> I see error
> [drm:amdgpu_cs_ioctl] *ERROR* amdgpu_cs_list_validate(validated) failed.
> [drm:amdgpu_cs_ioctl] *ERROR* Not enough memory for command submission!
> BUG: unable to handle kernel NULL pointer dereference at 0008
> Did some more debugging,dma_fence_is_array() is causing NULL pointer
> dereference call through sync_file_ioctl.
>
> Also I think changes in David patch cant be applied on
> amd-staging-drm-next, which all patches I should take to get it correctly?
>
 -Deepak
> Christian.
>
>> -David
>>> If that patch was applied then please revert it immediately.
>>>
>>> Christian.
>>>
 -David
> If you have already pushed the patch then please revert.
>

Re: [PATCH 1/2] drm/amd/display: Use private obj helpers for dm_atomic_state

[Kazlauskas, Nicholas]

On 11/27/18 4:20 PM, Li, Sun peng (Leo) wrote:
> 
> 
> On 2018-11-22 12:34 p.m., Nicholas Kazlauskas wrote:
>> [Why]
>> Two non-blocking commits in succession can result in a sequence where
>> the same dc->current_state is queried for both commits.
>>
>> 1. 1st commit -> check -> commit -> swaps atomic state -> queues work
>> 2. 2nd commit -> check -> commit -> swaps atomic state -> queues work
>> 3. 1st commit work finishes
>>
>> The issue with this sequence is that the same dc->current_state is
>> read in both atomic checks. If the first commit modifies streams or
>> planes those will be missing from the dc->current_state for the
>> second atomic check. This result in many stream and plane errors in
>> atomic commit tail.
>>
>> [How]
>> The driver still needs to track old to new state to determine if the
>> commit in its current implementation. Updating the dc_state in
>> atomic tail is wrong since the dc_state swap should be happening as
>> part of drm_atomic_helper_swap_state *before* the worker queue kicks
>> its work off.
>>
>> The simplest replacement for the subclassing (which doesn't properly
>> manage the old to new atomic state swap) is to use the drm private
>> object helpers. While some of the dc_state members could be merged
>> into dm_crtc_state or dm_plane_state and copied over that way it is
>> easier for now to just treat the whole dc_state structure as a single
>> private object.
>>
>> This allows amdgpu_dm to drop the dc->current_state copy from within
>> atomic check. It's replaced by a copy from the current atomic state
>> which is propagated correctly for the sequence described above.
>>
>> Since access to the dm_state private object is now locked this should
>> also fix issues that could arise if submitting non-blocking commits
>> from different threads.
>>
>> Cc: Harry Wentland 
>> Cc: Leo Li 
>> Signed-off-by: Nicholas Kazlauskas 
> 
> Patch 1/2 is
> Reviewed-by: Leo Li 
> 
> Leo

Thanks for the review.

Since the second patch is actually independent of this one (they solve 
different problems) I'm fine with pushing this one first and putting 
some more thought into the second. I'm thinking that the locking 
solution might be what we want in the end for that but it'll need some 
more time.

Nicholas Kazlauskas

>> ---
>>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 290 ++
>>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  13 +-
>>2 files changed, 234 insertions(+), 69 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 8433d31cdea8..3ae438d9849f 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -968,45 +968,6 @@ const struct amdgpu_ip_block_version dm_ip_block =
>>};
>>
>>
>> -static struct drm_atomic_state *
>> -dm_atomic_state_alloc(struct drm_device *dev)
>> -{
>> -struct dm_atomic_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
>> -
>> -if (!state)
>> -return NULL;
>> -
>> -if (drm_atomic_state_init(dev, >base) < 0)
>> -goto fail;
>> -
>> -return >base;
>> -
>> -fail:
>> -kfree(state);
>> -return NULL;
>> -}
>> -
>> -static void
>> -dm_atomic_state_clear(struct drm_atomic_state *state)
>> -{
>> -struct dm_atomic_state *dm_state = to_dm_atomic_state(state);
>> -
>> -if (dm_state->context) {
>> -dc_release_state(dm_state->context);
>> -dm_state->context = NULL;
>> -}
>> -
>> -drm_atomic_state_default_clear(state);
>> -}
>> -
>> -static void
>> -dm_atomic_state_alloc_free(struct drm_atomic_state *state)
>> -{
>> -struct dm_atomic_state *dm_state = to_dm_atomic_state(state);
>> -drm_atomic_state_default_release(state);
>> -kfree(dm_state);
>> -}
>> -
>>/**
>> * DOC: atomic
>> *
>> @@ -1018,9 +979,6 @@ static const struct drm_mode_config_funcs 
>> amdgpu_dm_mode_funcs = {
>>  .output_poll_changed = drm_fb_helper_output_poll_changed,
>>  .atomic_check = amdgpu_dm_atomic_check,
>>  .atomic_commit = amdgpu_dm_atomic_commit,
>> -.atomic_state_alloc = dm_atomic_state_alloc,
>> -.atomic_state_clear = dm_atomic_state_clear,
>> -.atomic_state_free = dm_atomic_state_alloc_free
>>};
>>
>>static struct drm_mode_config_helper_funcs 
>> amdgpu_dm_mode_config_helperfuncs = {
>> @@ -1542,8 +1500,117 @@ static int dcn10_register_irq_handlers(struct 
>> amdgpu_device *adev)
>>}
>>#endif
>>
>> +/*
>> + * Acquires the lock for the atomic state object and returns
>> + * the new atomic state.
>> + *
>> + * This should only be called during atomic check.
>> + */
>> +static int dm_atomic_get_state(struct drm_atomic_state *state,
>> +   struct dm_atomic_state **dm_state)
>> +{
>> +struct drm_device *dev = state->dev;
>> +struct amdgpu_device *adev = dev->dev_private;
>> +struct 

Re: [PATCH 1/2] drm/amd/display: Use private obj helpers for dm_atomic_state

[Li, Sun peng (Leo)]

On 2018-11-22 12:34 p.m., Nicholas Kazlauskas wrote:
> [Why]
> Two non-blocking commits in succession can result in a sequence where
> the same dc->current_state is queried for both commits.
> 
> 1. 1st commit -> check -> commit -> swaps atomic state -> queues work
> 2. 2nd commit -> check -> commit -> swaps atomic state -> queues work
> 3. 1st commit work finishes
> 
> The issue with this sequence is that the same dc->current_state is
> read in both atomic checks. If the first commit modifies streams or
> planes those will be missing from the dc->current_state for the
> second atomic check. This result in many stream and plane errors in
> atomic commit tail.
> 
> [How]
> The driver still needs to track old to new state to determine if the
> commit in its current implementation. Updating the dc_state in
> atomic tail is wrong since the dc_state swap should be happening as
> part of drm_atomic_helper_swap_state *before* the worker queue kicks
> its work off.
> 
> The simplest replacement for the subclassing (which doesn't properly
> manage the old to new atomic state swap) is to use the drm private
> object helpers. While some of the dc_state members could be merged
> into dm_crtc_state or dm_plane_state and copied over that way it is
> easier for now to just treat the whole dc_state structure as a single
> private object.
> 
> This allows amdgpu_dm to drop the dc->current_state copy from within
> atomic check. It's replaced by a copy from the current atomic state
> which is propagated correctly for the sequence described above.
> 
> Since access to the dm_state private object is now locked this should
> also fix issues that could arise if submitting non-blocking commits
> from different threads.
> 
> Cc: Harry Wentland 
> Cc: Leo Li 
> Signed-off-by: Nicholas Kazlauskas 

Patch 1/2 is
Reviewed-by: Leo Li 

Leo
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 290 ++
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  13 +-
>   2 files changed, 234 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 8433d31cdea8..3ae438d9849f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -968,45 +968,6 @@ const struct amdgpu_ip_block_version dm_ip_block =
>   };
>   
>   
> -static struct drm_atomic_state *
> -dm_atomic_state_alloc(struct drm_device *dev)
> -{
> - struct dm_atomic_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
> -
> - if (!state)
> - return NULL;
> -
> - if (drm_atomic_state_init(dev, >base) < 0)
> - goto fail;
> -
> - return >base;
> -
> -fail:
> - kfree(state);
> - return NULL;
> -}
> -
> -static void
> -dm_atomic_state_clear(struct drm_atomic_state *state)
> -{
> - struct dm_atomic_state *dm_state = to_dm_atomic_state(state);
> -
> - if (dm_state->context) {
> - dc_release_state(dm_state->context);
> - dm_state->context = NULL;
> - }
> -
> - drm_atomic_state_default_clear(state);
> -}
> -
> -static void
> -dm_atomic_state_alloc_free(struct drm_atomic_state *state)
> -{
> - struct dm_atomic_state *dm_state = to_dm_atomic_state(state);
> - drm_atomic_state_default_release(state);
> - kfree(dm_state);
> -}
> -
>   /**
>* DOC: atomic
>*
> @@ -1018,9 +979,6 @@ static const struct drm_mode_config_funcs 
> amdgpu_dm_mode_funcs = {
>   .output_poll_changed = drm_fb_helper_output_poll_changed,
>   .atomic_check = amdgpu_dm_atomic_check,
>   .atomic_commit = amdgpu_dm_atomic_commit,
> - .atomic_state_alloc = dm_atomic_state_alloc,
> - .atomic_state_clear = dm_atomic_state_clear,
> - .atomic_state_free = dm_atomic_state_alloc_free
>   };
>   
>   static struct drm_mode_config_helper_funcs 
> amdgpu_dm_mode_config_helperfuncs = {
> @@ -1542,8 +1500,117 @@ static int dcn10_register_irq_handlers(struct 
> amdgpu_device *adev)
>   }
>   #endif
>   
> +/*
> + * Acquires the lock for the atomic state object and returns
> + * the new atomic state.
> + *
> + * This should only be called during atomic check.
> + */
> +static int dm_atomic_get_state(struct drm_atomic_state *state,
> +struct dm_atomic_state **dm_state)
> +{
> + struct drm_device *dev = state->dev;
> + struct amdgpu_device *adev = dev->dev_private;
> + struct amdgpu_display_manager *dm = >dm;
> + struct drm_private_state *priv_state;
> + int ret;
> +
> + if (*dm_state)
> + return 0;
> +
> + ret = drm_modeset_lock(>atomic_obj_lock, state->acquire_ctx);
> + if (ret)
> + return ret;
> +
> + priv_state = drm_atomic_get_private_obj_state(state, >atomic_obj);
> + if (IS_ERR(priv_state))
> + return PTR_ERR(priv_state);
> +
> + *dm_state = to_dm_atomic_state(priv_state);
> +
> + return 0;
> +}
> +
> +struct 

[PATCH 2/6] drm/amdgpu: make amdgpu_ctx_init more robust

[Alex Deucher]

Only set up the scheduling entities if the rings are
valid.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 83 -
 1 file changed, 61 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 95f4c4139fc6..d12eb863285b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -74,7 +74,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
   struct amdgpu_ctx *ctx)
 {
unsigned num_entities = amdgput_ctx_total_num_entities();
-   unsigned i, j;
+   unsigned i, j, k;
int r;
 
if (priority < 0 || priority >= DRM_SCHED_PRIORITY_MAX)
@@ -123,46 +123,85 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
struct drm_sched_rq *rqs[AMDGPU_MAX_RINGS];
-   unsigned num_rings;
+   unsigned num_rings = 0;
 
switch (i) {
case AMDGPU_HW_IP_GFX:
-   rings[0] = >gfx.gfx_ring[0];
-   num_rings = 1;
+   for (j = 0; j < adev->gfx.num_gfx_rings; j++) {
+   if (adev->gfx.gfx_ring[j].sched.ready)
+   rings[num_rings++] = 
>gfx.gfx_ring[j];
+   }
break;
case AMDGPU_HW_IP_COMPUTE:
-   for (j = 0; j < adev->gfx.num_compute_rings; ++j)
-   rings[j] = >gfx.compute_ring[j];
-   num_rings = adev->gfx.num_compute_rings;
+   for (j = 0; j < adev->gfx.num_compute_rings; ++j) {
+   if (adev->gfx.compute_ring[j].sched.ready)
+   rings[num_rings++] = 
>gfx.compute_ring[j];
+   }
break;
case AMDGPU_HW_IP_DMA:
-   for (j = 0; j < adev->sdma.num_instances; ++j)
-   rings[j] = >sdma.instance[j].ring;
-   num_rings = adev->sdma.num_instances;
+   for (j = 0; j < adev->sdma.num_instances; ++j) {
+   if (adev->sdma.instance[j].ring.sched.ready)
+   rings[num_rings++] = 
>sdma.instance[j].ring;
+   }
break;
case AMDGPU_HW_IP_UVD:
-   rings[0] = >uvd.inst[0].ring;
-   num_rings = 1;
+   for (j = 0; j < adev->uvd.num_uvd_inst; j++) {
+   if (adev->uvd.harvest_config & (1 << j))
+   continue;
+
+   if (adev->uvd.inst[j].ring.sched.ready)
+   rings[num_rings++] = 
>uvd.inst[j].ring;
+   }
break;
case AMDGPU_HW_IP_VCE:
-   rings[0] = >vce.ring[0];
-   num_rings = 1;
+   for (j = 0; j < adev->vce.num_rings; j++) {
+   if (adev->vce.ring[j].sched.ready)
+   rings[num_rings++] = >vce.ring[j];
+   }
break;
case AMDGPU_HW_IP_UVD_ENC:
-   rings[0] = >uvd.inst[0].ring_enc[0];
-   num_rings = 1;
+   for (j = 0; j < adev->uvd.num_uvd_inst; j++) {
+   if (adev->uvd.harvest_config & (1 << j))
+   continue;
+   for (k = 0; k < adev->uvd.num_enc_rings; k++) {
+   if 
(adev->uvd.inst[j].ring_enc[k].sched.ready)
+   rings[num_rings++] = 
>uvd.inst[j].ring_enc[k];
+   }
+   }
break;
case AMDGPU_HW_IP_VCN_DEC:
-   rings[0] = >vcn.ring_dec;
-   num_rings = 1;
+   if (adev->vcn.ring_dec.sched.ready)
+   rings[num_rings++] = >vcn.ring_dec;
break;
case AMDGPU_HW_IP_VCN_ENC:
-   rings[0] = >vcn.ring_enc[0];
-   num_rings = 1;
+   for (j = 0; j < adev->vcn.num_enc_rings; j++) {
+   if (adev->vcn.ring_enc[j].sched.ready)
+   rings[num_rings++] = 
>vcn.ring_enc[j];
+   }
+   break;
+   case AMDGPU_HW_IP_VCN_JPEG:
+   

[PATCH 4/6] drm/amdgpu: convert amdgpu_ctx_init to use the new helper

[Alex Deucher]

Use the new helper to get the valid ring count and array.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 62 ++---
 1 file changed, 2 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index d12eb863285b..9d2dfa5b83ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -74,7 +74,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
   struct amdgpu_ctx *ctx)
 {
unsigned num_entities = amdgput_ctx_total_num_entities();
-   unsigned i, j, k;
+   unsigned i, j;
int r;
 
if (priority < 0 || priority >= DRM_SCHED_PRIORITY_MAX)
@@ -125,65 +125,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
struct drm_sched_rq *rqs[AMDGPU_MAX_RINGS];
unsigned num_rings = 0;
 
-   switch (i) {
-   case AMDGPU_HW_IP_GFX:
-   for (j = 0; j < adev->gfx.num_gfx_rings; j++) {
-   if (adev->gfx.gfx_ring[j].sched.ready)
-   rings[num_rings++] = 
>gfx.gfx_ring[j];
-   }
-   break;
-   case AMDGPU_HW_IP_COMPUTE:
-   for (j = 0; j < adev->gfx.num_compute_rings; ++j) {
-   if (adev->gfx.compute_ring[j].sched.ready)
-   rings[num_rings++] = 
>gfx.compute_ring[j];
-   }
-   break;
-   case AMDGPU_HW_IP_DMA:
-   for (j = 0; j < adev->sdma.num_instances; ++j) {
-   if (adev->sdma.instance[j].ring.sched.ready)
-   rings[num_rings++] = 
>sdma.instance[j].ring;
-   }
-   break;
-   case AMDGPU_HW_IP_UVD:
-   for (j = 0; j < adev->uvd.num_uvd_inst; j++) {
-   if (adev->uvd.harvest_config & (1 << j))
-   continue;
-
-   if (adev->uvd.inst[j].ring.sched.ready)
-   rings[num_rings++] = 
>uvd.inst[j].ring;
-   }
-   break;
-   case AMDGPU_HW_IP_VCE:
-   for (j = 0; j < adev->vce.num_rings; j++) {
-   if (adev->vce.ring[j].sched.ready)
-   rings[num_rings++] = >vce.ring[j];
-   }
-   break;
-   case AMDGPU_HW_IP_UVD_ENC:
-   for (j = 0; j < adev->uvd.num_uvd_inst; j++) {
-   if (adev->uvd.harvest_config & (1 << j))
-   continue;
-   for (k = 0; k < adev->uvd.num_enc_rings; k++) {
-   if 
(adev->uvd.inst[j].ring_enc[k].sched.ready)
-   rings[num_rings++] = 
>uvd.inst[j].ring_enc[k];
-   }
-   }
-   break;
-   case AMDGPU_HW_IP_VCN_DEC:
-   if (adev->vcn.ring_dec.sched.ready)
-   rings[num_rings++] = >vcn.ring_dec;
-   break;
-   case AMDGPU_HW_IP_VCN_ENC:
-   for (j = 0; j < adev->vcn.num_enc_rings; j++) {
-   if (adev->vcn.ring_enc[j].sched.ready)
-   rings[num_rings++] = 
>vcn.ring_enc[j];
-   }
-   break;
-   case AMDGPU_HW_IP_VCN_JPEG:
-   if (adev->vcn.ring_jpeg.sched.ready)
-   rings[num_rings++] = >vcn.ring_jpeg;
-   break;
-   }
+   num_rings = amdgpu_ring_get_valid_rings(adev, i, rings);
 
/* if there are no rings, then the IP doesn't exist or is 
disabled  */
if (num_rings == 0)
-- 
2.13.6

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

[PATCH 6/6] drm/amdgpu: don't expose entities that are not valid for a ctx

[Alex Deucher]

If a particular engine is not available, don't expose its
scheduling entity.  This will cause the CS ioctl to fail
rather than trying to use an uninitialized ring if a particular
IP is not available.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 13 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h |  1 +
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 9d2dfa5b83ea..3f76d1a7034b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -150,11 +150,13 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
for (j = 0; j < num_rings; ++j)
rqs[j] = [j]->sched.sched_rq[priority];
 
-   for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
+   for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
r = drm_sched_entity_init(>entities[i][j].entity,
  rqs, num_rings, >guilty);
-   if (r)
-   goto error_cleanup_entities;
+   if (r)
+   goto error_cleanup_entities;
+   ctx->entities[i][j].valid = true;
+   }
}
 
return 0;
@@ -210,6 +212,11 @@ int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx, u32 
hw_ip, u32 instance,
return -EINVAL;
}
 
+   if (!ctx->entities[hw_ip][ring].valid) {
+   DRM_DEBUG("invalid entity: %d %d\n", hw_ip, ring);
+   return -EINVAL;
+   }
+
*entity = >entities[hw_ip][ring].entity;
return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index b3b012c0a7da..6dd6c206daeb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -33,6 +33,7 @@ struct amdgpu_ctx_entity {
uint64_tsequence;
struct dma_fence**fences;
struct drm_sched_entity entity;
+   bool valid;
 };
 
 struct amdgpu_ctx {
-- 
2.13.6

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

[PATCH 3/6] drm/amdgpu: add amdgpu_ring_get_valid_rings helper

[Alex Deucher]

Returns the number of valid rings (i.e., ready for work) for
a particular engine type. Optionally returns an array of rings
as well.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 95 
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  3 +
 2 files changed, 98 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 5b75bdc8dc28..c25fcbc2603e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -526,3 +526,98 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
ring->sched.ready = !r;
return r;
 }
+
+unsigned amdgpu_ring_get_valid_rings(struct amdgpu_device *adev,
+int hw_ip, struct amdgpu_ring **rings)
+{
+   unsigned num_rings = 0;
+   int i, j;
+
+   switch (hw_ip) {
+   case AMDGPU_HW_IP_GFX:
+   for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
+   if (adev->gfx.gfx_ring[i].sched.ready) {
+   if (rings)
+   rings[num_rings] = 
>gfx.gfx_ring[i];
+   num_rings++;
+   }
+   }
+   break;
+   case AMDGPU_HW_IP_COMPUTE:
+   for (i = 0; i < adev->gfx.num_compute_rings; i++) {
+   if (adev->gfx.compute_ring[i].sched.ready) {
+   if (rings)
+   rings[num_rings] = 
>gfx.compute_ring[i];
+   num_rings++;
+   }
+   }
+   break;
+   case AMDGPU_HW_IP_DMA:
+   for (i = 0; i < adev->sdma.num_instances; i++) {
+   if (adev->sdma.instance[i].ring.sched.ready) {
+   if (rings)
+   rings[num_rings] = 
>sdma.instance[i].ring;
+   num_rings++;
+   }
+   }
+   break;
+   case AMDGPU_HW_IP_UVD:
+   for (i = 0; i < adev->uvd.num_uvd_inst; i++) {
+   if (adev->uvd.harvest_config & (1 << i))
+   continue;
+   if (adev->uvd.inst[i].ring.sched.ready) {
+   if (rings)
+   rings[num_rings] = 
>uvd.inst[i].ring;
+   num_rings++;
+   }
+   }
+   break;
+   case AMDGPU_HW_IP_VCE:
+   for (i = 0; i < adev->vce.num_rings; i++) {
+   if (adev->vce.ring[i].sched.ready) {
+   if (rings)
+   rings[num_rings] = >vce.ring[i];
+   num_rings++;
+   }
+   }
+   break;
+   case AMDGPU_HW_IP_UVD_ENC:
+   for (i = 0; i < adev->uvd.num_uvd_inst; i++) {
+   if (adev->uvd.harvest_config & (1 << i))
+   continue;
+   for (j = 0; j < adev->uvd.num_enc_rings; j++) {
+   if (adev->uvd.inst[i].ring_enc[j].sched.ready) {
+   if (rings)
+   rings[num_rings] = 
>uvd.inst[i].ring_enc[j];
+   num_rings++;
+   }
+   }
+   }
+   break;
+   case AMDGPU_HW_IP_VCN_DEC:
+   if (adev->vcn.ring_dec.sched.ready) {
+   if (rings)
+   rings[num_rings] = >vcn.ring_dec;
+   num_rings++;
+   }
+   break;
+   case AMDGPU_HW_IP_VCN_ENC:
+   for (i = 0; i < adev->vcn.num_enc_rings; i++) {
+   if (adev->vcn.ring_enc[i].sched.ready) {
+   if (rings)
+   rings[num_rings] = 
>vcn.ring_enc[i];
+   num_rings++;
+   }
+   }
+   break;
+   case AMDGPU_HW_IP_VCN_JPEG:
+   if (adev->vcn.ring_jpeg.sched.ready) {
+   if (rings)
+   rings[num_rings] = >vcn.ring_jpeg;
+   num_rings++;
+   }
+   break;
+   }
+
+   return num_rings;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 0beb01fef83f..f3fb26644d65 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -315,4 +315,7 @@ static inline void amdgpu_ring_write_multiple(struct 
amdgpu_ring *ring,
 
 int 

[PATCH 1/6] drm/amdgpu: add VCN JPEG support amdgpu_ctx_num_entities

[Alex Deucher]

Looks like it was missed when setting support was added.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index f9b54236102d..95f4c4139fc6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -39,6 +39,7 @@ const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] 
= {
[AMDGPU_HW_IP_UVD_ENC]  =   1,
[AMDGPU_HW_IP_VCN_DEC]  =   1,
[AMDGPU_HW_IP_VCN_ENC]  =   1,
+   [AMDGPU_HW_IP_VCN_JPEG] =   1,
 };
 
 static int amdgput_ctx_total_num_entities(void)
-- 
2.13.6

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

[PATCH 5/6] drm/amdgpu: convert amdgpu_hw_ip_info to use new helper

[Alex Deucher]

Use the new helper to get the number of rings.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 38 +++--
 1 file changed, 3 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 08d04f68dfeb..e97e18b7e0fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -327,7 +327,7 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
uint32_t ib_size_alignment = 0;
enum amd_ip_block_type type;
unsigned int num_rings = 0;
-   unsigned int i, j;
+   unsigned int i;
 
if (info->query_hw_ip.ip_instance >= AMDGPU_HW_IP_INSTANCE_MAX_COUNT)
return -EINVAL;
@@ -335,80 +335,46 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
switch (info->query_hw_ip.type) {
case AMDGPU_HW_IP_GFX:
type = AMD_IP_BLOCK_TYPE_GFX;
-   for (i = 0; i < adev->gfx.num_gfx_rings; i++)
-   if (adev->gfx.gfx_ring[i].sched.ready)
-   ++num_rings;
ib_start_alignment = 32;
ib_size_alignment = 32;
break;
case AMDGPU_HW_IP_COMPUTE:
type = AMD_IP_BLOCK_TYPE_GFX;
-   for (i = 0; i < adev->gfx.num_compute_rings; i++)
-   if (adev->gfx.compute_ring[i].sched.ready)
-   ++num_rings;
ib_start_alignment = 32;
ib_size_alignment = 32;
break;
case AMDGPU_HW_IP_DMA:
type = AMD_IP_BLOCK_TYPE_SDMA;
-   for (i = 0; i < adev->sdma.num_instances; i++)
-   if (adev->sdma.instance[i].ring.sched.ready)
-   ++num_rings;
ib_start_alignment = 256;
ib_size_alignment = 4;
break;
case AMDGPU_HW_IP_UVD:
type = AMD_IP_BLOCK_TYPE_UVD;
-   for (i = 0; i < adev->uvd.num_uvd_inst; i++) {
-   if (adev->uvd.harvest_config & (1 << i))
-   continue;
-
-   if (adev->uvd.inst[i].ring.sched.ready)
-   ++num_rings;
-   }
ib_start_alignment = 64;
ib_size_alignment = 64;
break;
case AMDGPU_HW_IP_VCE:
type = AMD_IP_BLOCK_TYPE_VCE;
-   for (i = 0; i < adev->vce.num_rings; i++)
-   if (adev->vce.ring[i].sched.ready)
-   ++num_rings;
ib_start_alignment = 4;
ib_size_alignment = 1;
break;
case AMDGPU_HW_IP_UVD_ENC:
type = AMD_IP_BLOCK_TYPE_UVD;
-   for (i = 0; i < adev->uvd.num_uvd_inst; i++) {
-   if (adev->uvd.harvest_config & (1 << i))
-   continue;
-
-   for (j = 0; j < adev->uvd.num_enc_rings; j++)
-   if (adev->uvd.inst[i].ring_enc[j].sched.ready)
-   ++num_rings;
-   }
ib_start_alignment = 64;
ib_size_alignment = 64;
break;
case AMDGPU_HW_IP_VCN_DEC:
type = AMD_IP_BLOCK_TYPE_VCN;
-   if (adev->vcn.ring_dec.sched.ready)
-   ++num_rings;
ib_start_alignment = 16;
ib_size_alignment = 16;
break;
case AMDGPU_HW_IP_VCN_ENC:
type = AMD_IP_BLOCK_TYPE_VCN;
-   for (i = 0; i < adev->vcn.num_enc_rings; i++)
-   if (adev->vcn.ring_enc[i].sched.ready)
-   ++num_rings;
ib_start_alignment = 64;
ib_size_alignment = 1;
break;
case AMDGPU_HW_IP_VCN_JPEG:
type = AMD_IP_BLOCK_TYPE_VCN;
-   if (adev->vcn.ring_jpeg.sched.ready)
-   ++num_rings;
ib_start_alignment = 16;
ib_size_alignment = 16;
break;
@@ -416,6 +382,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
return -EINVAL;
}
 
+   num_rings = amdgpu_ring_get_valid_rings(adev, info->query_hw_ip.type, 
NULL);
+
for (i = 0; i < adev->num_ip_blocks; i++)
if (adev->ip_blocks[i].version->type == type &&
adev->ip_blocks[i].status.valid)
-- 
2.13.6

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

Re: [PATCH] drm/amd/display: Fix compile error with ACPI disabled

[Wentland, Harry]

On 2018-11-27 11:18 a.m., David Francis wrote:
> The fallback code for getting default backlight caps was using
> the wrong variable name.  Fix it.
> 
> Fixes: 
> https://lists.freedesktop.org/archives/dri-devel/2018-November/197752.html
> Signed-off-by: David Francis 

Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 27df3ae945be..aa3f8200fa69 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1615,8 +1615,8 @@ static void amdgpu_dm_update_backlight_caps(struct 
> amdgpu_display_manager *dm)
>   AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
>   }
>  #else
> - dm->backlight_min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> - dm->backlight_max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
> + dm->backlight_caps.min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> + dm->backlight_caps.max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
>  #endif
>  }
>  
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] pci: fix incorrect value returned from pcie_get_speed_cap

[Bjorn Helgaas]

On Tue, Nov 27, 2018 at 11:49:43AM -0500, Mikulas Patocka wrote:
> On Mon, 26 Nov 2018, Bjorn Helgaas wrote:
> > On Mon, Nov 19, 2018 at 06:47:04PM -0600, Bjorn Helgaas wrote:
> > > On Tue, Oct 30, 2018 at 12:36:08PM -0400, Mikulas Patocka wrote:
> > > > The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks. We must 
> > > > mask
> > > > the register and compare it against them.
> > > > 
> > > > This patch fixes errors "amdgpu: [powerplay] failed to send message 261
> > > > ret is 0" errors when PCIe-v3 card is plugged into PCIe-v1 slot, because
> > > > the slot is being incorrectly reported as PCIe-v3 capable.
> > > > 
> > > > Signed-off-by: Mikulas Patocka 
> > > > Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max 
> > > > supported link speed")
> > > > Cc: sta...@vger.kernel.org  # v4.17+
> > > > 
> > > > ---
> > > >  drivers/pci/pci.c |8 
> > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > Index: linux-4.19/drivers/pci/pci.c
> > > > ===
> > > > --- linux-4.19.orig/drivers/pci/pci.c   2018-10-30 16:58:58.0 
> > > > +0100
> > > > +++ linux-4.19/drivers/pci/pci.c2018-10-30 16:58:58.0 
> > > > +0100
> > > > @@ -5492,13 +5492,13 @@ enum pci_bus_speed pcie_get_speed_cap(st
> > > >  
> > > > pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, );
> > > > if (lnkcap) {
> > > > -   if (lnkcap & PCI_EXP_LNKCAP_SLS_16_0GB)
> > > > +   if ((lnkcap & PCI_EXP_LNKCAP_SLS) == 
> > > > PCI_EXP_LNKCAP_SLS_16_0GB)
> > > > return PCIE_SPEED_16_0GT;
> > > > -   else if (lnkcap & PCI_EXP_LNKCAP_SLS_8_0GB)
> > > > +   else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == 
> > > > PCI_EXP_LNKCAP_SLS_8_0GB)
> > > > return PCIE_SPEED_8_0GT;
> > > > -   else if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
> > > > +   else if ((lnkcap & PCI_EXP_LNKCAP_SLS) 
> > > > ==PCI_EXP_LNKCAP_SLS_5_0GB)
> > > > return PCIE_SPEED_5_0GT;
> > > > -   else if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
> > > > +   else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == 
> > > > PCI_EXP_LNKCAP_SLS_2_5GB)
> > > > return PCIE_SPEED_2_5GT;
> > > > }
> > 
> > > We also need similar fixes in pci_set_bus_speed(), pcie_speeds()
> > > (hfi1), cobalt_pcie_status_show(), hba_ioctl_callback(),
> > > qla24xx_pci_info_str(), and maybe a couple other places.
> > 
> > Does anybody want to volunteer to fix the places above as well?  I
> > found them by grepping for PCI_EXP_LNKCAP, and they're all broken in
> > ways similar to pcie_get_speed_cap().  Possibly some of these places
> > could use pcie_get_speed_cap() directly.
> 
> They are not broken, they are masking the value with PCI_EXP_LNKCAP_SLS - 
> that is correct.

You're right, "broken" is an overstatement except in terms of
maintainance: they do happen to work correctly, but there's no reason
to reimplement the same functionality several places in slightly
different ways.

pcie_get_speed_cap() looks at LNKCAP2, then LNKCAP.  The other places
look only at LNKCAP.  Since these are all finding only the max link
speed (which can be determined using only LKNCAP), not the set of
supported speeds (which requires both LNKCAP2 and LNKCAP), they should
all do it the same way.

> pci_set_bus_speed:
> pcie_capability_read_dword(bridge, PCI_EXP_LNKCAP, );
> bus->max_bus_speed = pcie_link_speed[linkcap & 
> PCI_EXP_LNKCAP_SLS];
> 
> pcie_speeds:
>   if ((linkcap & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_8_0GB)
> 
> cobalt_pcie_status_show:
>   just prints the values without doing anything with them
> 
> hba_ioctl_callback:
>   gai->pci.link_speed_max = (u8)(caps & PCI_EXP_LNKCAP_SLS);
> 
>   gai->pci.link_width_max = (u8)((caps & PCI_EXP_LNKCAP_MLW) >> 4);
> 
> qla24xx_pci_info_str:
>   lspeed = lstat & PCI_EXP_LNKCAP_SLS;
>   lwidth = (lstat & PCI_EXP_LNKCAP_MLW) >> 4;
> 
> Mikulas
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH RFC 5/5] drm/amdgpu: Add accounting of buffer object creation request via DRM cgroup

[Kenny Ho]

Ah I see.  Thank you for the clarification.

Regards,
Kenny
On Tue, Nov 27, 2018 at 3:31 PM Christian König
 wrote:
>
> Am 27.11.18 um 19:15 schrieb Kenny Ho:
> > Hey Christian,
> >
> > Sorry for the late reply, I missed this for some reason.
> >
> > On Wed, Nov 21, 2018 at 5:00 AM Christian König
> >  wrote:
> >>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> >>> index 370e9a5536ef..531726443104 100644
> >>> --- a/include/uapi/drm/amdgpu_drm.h
> >>> +++ b/include/uapi/drm/amdgpu_drm.h
> >>> @@ -72,6 +72,18 @@ extern "C" {
> >>>#define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + 
> >>> DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
> >>>#define DRM_IOCTL_AMDGPU_SCHED  DRM_IOW(DRM_COMMAND_BASE + 
> >>> DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
> >>>
> >>> +enum amdgpu_mem_domain {
> >>> + AMDGPU_MEM_DOMAIN_CPU,
> >>> + AMDGPU_MEM_DOMAIN_GTT,
> >>> + AMDGPU_MEM_DOMAIN_VRAM,
> >>> + AMDGPU_MEM_DOMAIN_GDS,
> >>> + AMDGPU_MEM_DOMAIN_GWS,
> >>> + AMDGPU_MEM_DOMAIN_OA,
> >>> + __MAX_AMDGPU_MEM_DOMAIN
> >>> +};
> >> Well that is a clear NAK since it duplicates the TTM defines. Please use
> >> that one instead and don't make this UAPI.
> > This is defined to help with the chunk of changes below.  The
> > AMDGPU_GEM_DOMAIN* already exists and this is similar to how TTM has
> > TTM_PL_* to help with the creation of TTM_PL_FLAG_*:
> > https://elixir.bootlin.com/linux/v4.20-rc4/source/include/drm/ttm/ttm_placement.h#L36
> >
> > I don't disagree that there is a duplication here but it's
> > pre-existing so if you can help clarify my confusion that would be
> > much appreciated.
>
> The AMDGPU_GEM_DOMAIN are masks which are used in the frontend IOCTL
> interface to create BOs.
>
> TTM defines the backend pools where the memory is then allocated from to
> fill the BOs.
>
> So you are mixing frontend and backend here.
>
> In other words for the whole cgroup interface you should not make a
> single change to amdgpu_drm.h or otherwise you are doing something wrong.
>
> Regards,
> Christian.
>
> >
> > Reards,
> > Kenny
> >
> >>> +
> >>> +extern char const *amdgpu_mem_domain_names[];
> >>> +
> >>>/**
> >>> * DOC: memory domains
> >>> *
> >>> @@ -95,12 +107,12 @@ extern "C" {
> >>> * %AMDGPU_GEM_DOMAIN_OAOrdered append, used by 3D or Compute 
> >>> engines
> >>> * for appending data.
> >>> */
> >>> -#define AMDGPU_GEM_DOMAIN_CPU0x1
> >>> -#define AMDGPU_GEM_DOMAIN_GTT0x2
> >>> -#define AMDGPU_GEM_DOMAIN_VRAM   0x4
> >>> -#define AMDGPU_GEM_DOMAIN_GDS0x8
> >>> -#define AMDGPU_GEM_DOMAIN_GWS0x10
> >>> -#define AMDGPU_GEM_DOMAIN_OA 0x20
> >>> +#define AMDGPU_GEM_DOMAIN_CPU(1 << AMDGPU_MEM_DOMAIN_CPU)
> >>> +#define AMDGPU_GEM_DOMAIN_GTT(1 << AMDGPU_MEM_DOMAIN_GTT)
> >>> +#define AMDGPU_GEM_DOMAIN_VRAM   (1 << 
> >>> AMDGPU_MEM_DOMAIN_VRAM)
> >>> +#define AMDGPU_GEM_DOMAIN_GDS(1 << AMDGPU_MEM_DOMAIN_GDS)
> >>> +#define AMDGPU_GEM_DOMAIN_GWS(1 << AMDGPU_MEM_DOMAIN_GWS)
> >>> +#define AMDGPU_GEM_DOMAIN_OA (1 << AMDGPU_MEM_DOMAIN_OA)
> >>>#define AMDGPU_GEM_DOMAIN_MASK  (AMDGPU_GEM_DOMAIN_CPU | \
> >>> AMDGPU_GEM_DOMAIN_GTT | \
> >>> AMDGPU_GEM_DOMAIN_VRAM | \
> > ___
> > 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 5/5] drm/amdgpu: Add accounting of buffer object creation request via DRM cgroup

[Christian König]

Am 27.11.18 um 19:15 schrieb Kenny Ho:

Hey Christian,

Sorry for the late reply, I missed this for some reason.

On Wed, Nov 21, 2018 at 5:00 AM Christian König
 wrote:

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 370e9a5536ef..531726443104 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -72,6 +72,18 @@ extern "C" {
   #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
   #define DRM_IOCTL_AMDGPU_SCHED  DRM_IOW(DRM_COMMAND_BASE + 
DRM_AMDGPU_SCHED, union drm_amdgpu_sched)

+enum amdgpu_mem_domain {
+ AMDGPU_MEM_DOMAIN_CPU,
+ AMDGPU_MEM_DOMAIN_GTT,
+ AMDGPU_MEM_DOMAIN_VRAM,
+ AMDGPU_MEM_DOMAIN_GDS,
+ AMDGPU_MEM_DOMAIN_GWS,
+ AMDGPU_MEM_DOMAIN_OA,
+ __MAX_AMDGPU_MEM_DOMAIN
+};

Well that is a clear NAK since it duplicates the TTM defines. Please use
that one instead and don't make this UAPI.

This is defined to help with the chunk of changes below.  The
AMDGPU_GEM_DOMAIN* already exists and this is similar to how TTM has
TTM_PL_* to help with the creation of TTM_PL_FLAG_*:
https://elixir.bootlin.com/linux/v4.20-rc4/source/include/drm/ttm/ttm_placement.h#L36

I don't disagree that there is a duplication here but it's
pre-existing so if you can help clarify my confusion that would be
much appreciated.


The AMDGPU_GEM_DOMAIN are masks which are used in the frontend IOCTL 
interface to create BOs.


TTM defines the backend pools where the memory is then allocated from to 
fill the BOs.


So you are mixing frontend and backend here.

In other words for the whole cgroup interface you should not make a 
single change to amdgpu_drm.h or otherwise you are doing something wrong.


Regards,
Christian.



Reards,
Kenny


+
+extern char const *amdgpu_mem_domain_names[];
+
   /**
* DOC: memory domains
*
@@ -95,12 +107,12 @@ extern "C" {
* %AMDGPU_GEM_DOMAIN_OAOrdered append, used by 3D or Compute engines
* for appending data.
*/
-#define AMDGPU_GEM_DOMAIN_CPU0x1
-#define AMDGPU_GEM_DOMAIN_GTT0x2
-#define AMDGPU_GEM_DOMAIN_VRAM   0x4
-#define AMDGPU_GEM_DOMAIN_GDS0x8
-#define AMDGPU_GEM_DOMAIN_GWS0x10
-#define AMDGPU_GEM_DOMAIN_OA 0x20
+#define AMDGPU_GEM_DOMAIN_CPU(1 << AMDGPU_MEM_DOMAIN_CPU)
+#define AMDGPU_GEM_DOMAIN_GTT(1 << AMDGPU_MEM_DOMAIN_GTT)
+#define AMDGPU_GEM_DOMAIN_VRAM   (1 << AMDGPU_MEM_DOMAIN_VRAM)
+#define AMDGPU_GEM_DOMAIN_GDS(1 << AMDGPU_MEM_DOMAIN_GDS)
+#define AMDGPU_GEM_DOMAIN_GWS(1 << AMDGPU_MEM_DOMAIN_GWS)
+#define AMDGPU_GEM_DOMAIN_OA (1 << AMDGPU_MEM_DOMAIN_OA)
   #define AMDGPU_GEM_DOMAIN_MASK  (AMDGPU_GEM_DOMAIN_CPU | \
AMDGPU_GEM_DOMAIN_GTT | \
AMDGPU_GEM_DOMAIN_VRAM | \

___
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/display: Fix compile error with ACPI disabled

[Deucher, Alexander]

Acked-by: Alex Deucher 


From: amd-gfx  on behalf of David 
Francis 
Sent: Tuesday, November 27, 2018 11:18:14 AM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Cc: Francis, David
Subject: [PATCH] drm/amd/display: Fix compile error with ACPI disabled

The fallback code for getting default backlight caps was using
the wrong variable name.  Fix it.

Fixes: 
https://lists.freedesktop.org/archives/dri-devel/2018-November/197752.html
Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 27df3ae945be..aa3f8200fa69 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1615,8 +1615,8 @@ static void amdgpu_dm_update_backlight_caps(struct 
amdgpu_display_manager *dm)
 AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
 }
 #else
-   dm->backlight_min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
-   dm->backlight_max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
+   dm->backlight_caps.min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
+   dm->backlight_caps.max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
 #endif
 }

--
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: How to use vce1 in Radeon Pro WX5100

[Alex Deucher]

On Tue, Nov 27, 2018 at 10:51 AM Sunnanyong (Nanyong Sun, Intelligent
Computing Solution Development Dep)  wrote:
>
> Hi all，
>
>  I found two vce block under /sys/kernel/debug/dri/1/amdgpu_ring_vce0 & 
> amdgpu_ring_vce1.
>
> when I use ffmpeg to do H.264 encoding , I use “umr –top”  and press “c” to 
> show the VCE loading,  and
>
> I found that only vce0 had loading , and vce1 always be 0%, like this:
>
> VCE Bits:
>
> VCE0 => 100%   |VCE1 => 0 %   |
>
>   So I think vce1 block has never been used, and how to select vce1 to do 
> encode ?

There is only one VCE engine.  The rings are just front ends that feed
the single engine, similar to how there are multiple compute rings
that feed the shader array in the graphics block.  Some asics have a
dual pipe VCE engine.  Still single engine, but higher performance
depending on the workload.  You will only see the VCE1 bit go active
if you have an asic with a dual pipe VCE engine.  I think the WX5100
is a polaris board and they are only single pipe.

Alex

>
> My environment:
>
> Radeon Pro WX5100
>
> Mesa18.0.5
>
> Linux kernel 4.19
>
>
>
> Please help!
>
> ___
> 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/display: Fix NULL ptr when calculating refresh rate

[Lakha, Bhawanpreet]

Reviewed-by: Bhawanpreet Lakha 


From: amd-gfx  on behalf of Jerry 
(Fangzhi) Zuo 
Sent: November 27, 2018 1:45:54 PM
To: amd-gfx@lists.freedesktop.org; Wentland, Harry; Lakha, Bhawanpreet
Cc: Zuo, Jerry
Subject: [PATCH] drm/amd/display: Fix NULL ptr when calculating refresh rate

Calculate preferred refresh rate only when preferred mode exists.

Signed-off-by: Jerry (Fangzhi) Zuo 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 27df3ae945be..3039faf004a8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2835,7 +2835,7 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
 bool native_mode_found = false;
 bool scale = dm_state ? (dm_state->scaling != RMX_OFF) : false;
 int mode_refresh;
-   int preferred_refresh;
+   int preferred_refresh = 0;

 struct dc_sink *sink = NULL;
 if (aconnector == NULL) {
@@ -2889,13 +2889,12 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
 decide_crtc_timing_for_drm_display_mode(
 , preferred_mode,
 dm_state ? (dm_state->scaling != RMX_OFF) : 
false);
+   preferred_refresh = drm_mode_vrefresh(preferred_mode);
 }

 if (!dm_state)
 drm_mode_set_crtcinfo(, 0);

-   preferred_refresh = drm_mode_vrefresh(preferred_mode);
-
 /*
 * If scaling is enabled and refresh rate didn't change
 * we copy the vic and polarities of the old timings
--
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] pci: fix incorrect value returned from pcie_get_speed_cap

[Mikulas Patocka]

On Mon, 26 Nov 2018, Bjorn Helgaas wrote:

> On Mon, Nov 19, 2018 at 06:47:04PM -0600, Bjorn Helgaas wrote:
> > On Tue, Oct 30, 2018 at 12:36:08PM -0400, Mikulas Patocka wrote:
> > > The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks. We must mask
> > > the register and compare it against them.
> > > 
> > > This patch fixes errors "amdgpu: [powerplay] failed to send message 261
> > > ret is 0" errors when PCIe-v3 card is plugged into PCIe-v1 slot, because
> > > the slot is being incorrectly reported as PCIe-v3 capable.
> > > 
> > > Signed-off-by: Mikulas Patocka 
> > > Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max supported 
> > > link speed")
> > > Cc: sta...@vger.kernel.org# v4.17+
> > > 
> > > ---
> > >  drivers/pci/pci.c |8 
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > Index: linux-4.19/drivers/pci/pci.c
> > > ===
> > > --- linux-4.19.orig/drivers/pci/pci.c 2018-10-30 16:58:58.0 
> > > +0100
> > > +++ linux-4.19/drivers/pci/pci.c  2018-10-30 16:58:58.0 +0100
> > > @@ -5492,13 +5492,13 @@ enum pci_bus_speed pcie_get_speed_cap(st
> > >  
> > >   pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, );
> > >   if (lnkcap) {
> > > - if (lnkcap & PCI_EXP_LNKCAP_SLS_16_0GB)
> > > + if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_16_0GB)
> > >   return PCIE_SPEED_16_0GT;
> > > - else if (lnkcap & PCI_EXP_LNKCAP_SLS_8_0GB)
> > > + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == 
> > > PCI_EXP_LNKCAP_SLS_8_0GB)
> > >   return PCIE_SPEED_8_0GT;
> > > - else if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
> > > + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) 
> > > ==PCI_EXP_LNKCAP_SLS_5_0GB)
> > >   return PCIE_SPEED_5_0GT;
> > > - else if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
> > > + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == 
> > > PCI_EXP_LNKCAP_SLS_2_5GB)
> > >   return PCIE_SPEED_2_5GT;
> > >   }
> 
> > We also need similar fixes in pci_set_bus_speed(), pcie_speeds()
> > (hfi1), cobalt_pcie_status_show(), hba_ioctl_callback(),
> > qla24xx_pci_info_str(), and maybe a couple other places.
> 
> Does anybody want to volunteer to fix the places above as well?  I
> found them by grepping for PCI_EXP_LNKCAP, and they're all broken in
> ways similar to pcie_get_speed_cap().  Possibly some of these places
> could use pcie_get_speed_cap() directly.
> 
> Bjorn
> 

They are not broken, they are masking the value with PCI_EXP_LNKCAP_SLS - 
that is correct.

pci_set_bus_speed:
pcie_capability_read_dword(bridge, PCI_EXP_LNKCAP, );
bus->max_bus_speed = pcie_link_speed[linkcap & 
PCI_EXP_LNKCAP_SLS];

pcie_speeds:
if ((linkcap & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_8_0GB)

cobalt_pcie_status_show:
just prints the values without doing anything with them

hba_ioctl_callback:
gai->pci.link_speed_max = (u8)(caps & PCI_EXP_LNKCAP_SLS);

gai->pci.link_width_max = (u8)((caps & PCI_EXP_LNKCAP_MLW) >> 4);

qla24xx_pci_info_str:
lspeed = lstat & PCI_EXP_LNKCAP_SLS;
lwidth = (lstat & PCI_EXP_LNKCAP_MLW) >> 4;

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

Re: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ

[S, Shirish]

This is for the devices with type-c ports.

Wherein the signal type is same 32 (DISPLAY PORT)  for both HDMI and DP  
monitors connected to the system via type-c dongles/convertors.

Regards,
Shirish S

Get Outlook for Android


From: Deucher, Alexander
Sent: Wednesday, November 28, 2018 12:06:26 AM
To: S, Shirish; Li, Sun peng (Leo); Wentland, Harry
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ


Is this a DP to HDMI adapter?  I think 4k@60 should be valid on DP in general 
on ST/CZ, but Harry or Leo should comment.


Alex


From: S, Shirish
Sent: Tuesday, November 27, 2018 3:58:12 AM
To: Deucher, Alexander; Li, Sun peng (Leo); Wentland, Harry
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ


However, while using Type-C connectors noted that the signal type is actually 
SIGNAL_TYPE_DISPLAY_PORT and found that the check was missing.

Hence have added the same in https://patchwork.freedesktop.org/patch/264033/



Regards,

Shirish S



From: S, Shirish
Sent: Tuesday, November 27, 2018 9:54 AM
To: Deucher, Alexander ; Li, Sun peng (Leo) 
; Wentland, Harry 
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ



Thanks Alex, found that patch.

My patch is no more required.





Regards,

Shirish S



From: Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>
Sent: Monday, November 26, 2018 7:46 PM
To: S, Shirish mailto:shiris...@amd.com>>; Li, Sun peng 
(Leo) mailto:sunpeng...@amd.com>>; Wentland, Harry 
mailto:harry.wentl...@amd.com>>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ



I thought there was a patch to do this already that got sent out a few weeks 
ago.  Basically limit ST/CZ to modes that do not require a retimer.  Is an 
additional patch needed?



Alex



From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of S, Shirish mailto:shiris...@amd.com>>
Sent: Monday, November 26, 2018 1:36:30 AM
To: Li, Sun peng (Leo); Wentland, Harry
Cc: amd-gfx@lists.freedesktop.org; S, 
Shirish
Subject: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ



[Why]
ST/CZ (dce110) advertises modes such as 4k@60Hz etc.,
that it cannot handle correctly, hence  resulting in
several issues like flickering, black lines/flashes and so on.

[How]
These modes are basically high pixel clock ones, hence
limit the same to be advertised to avoid bad user experiences

Signed-off-by: Shirish S mailto:shiris...@amd.com>>
Suggested-by: Harry Wentland 
mailto:harry.wentl...@amd.com>>
---
 .../gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c| 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
index 1b2fe0d..1b8fe99 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
@@ -1121,6 +1121,16 @@ bool dce110_timing_generator_validate_timing(
 if (!timing)
 return false;

+   /* Limit all modes that have a high pixel clock
+* which seems to be problematic on dce110
+* These include: 4k@60Hz, 1080p@144Hz,1440p@120Hz
+* based on the below formula:
+* refresh rate = pixel clock / (htotal * vtotal)
+*/
+   if (timing->pix_clk_khz > 30)
+   return false;
+
+
 hsync_offset = timing->h_border_right + timing->h_front_porch;
 h_sync_start = timing->h_addressable + hsync_offset;

--
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

[PATCH] drm/amd/display: Fix NULL ptr when calculating refresh rate

[Jerry (Fangzhi) Zuo]

Calculate preferred refresh rate only when preferred mode exists.

Signed-off-by: Jerry (Fangzhi) Zuo 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 27df3ae945be..3039faf004a8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2835,7 +2835,7 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
bool native_mode_found = false;
bool scale = dm_state ? (dm_state->scaling != RMX_OFF) : false;
int mode_refresh;
-   int preferred_refresh;
+   int preferred_refresh = 0;
 
struct dc_sink *sink = NULL;
if (aconnector == NULL) {
@@ -2889,13 +2889,12 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
decide_crtc_timing_for_drm_display_mode(
, preferred_mode,
dm_state ? (dm_state->scaling != RMX_OFF) : 
false);
+   preferred_refresh = drm_mode_vrefresh(preferred_mode);
}
 
if (!dm_state)
drm_mode_set_crtcinfo(, 0);
 
-   preferred_refresh = drm_mode_vrefresh(preferred_mode);
-
/*
* If scaling is enabled and refresh rate didn't change
* we copy the vic and polarities of the old timings
-- 
2.14.1

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

[PATCH v3 3/3] drm/amdgpu: Refactor GPU reset for XGMI hive case v3

[Andrey Grodzovsky]

For XGMI hive case do reset in steps where each step iterates over
all devs in hive. This especially important for asic reset
since all PSP FW in hive must come up within a limited time
(around 1 sec) to properply negotiate the link.
Do this by  refactoring  amdgpu_device_gpu_recover and amdgpu_device_reset
into pre_asic_reset, asic_reset and post_asic_reset functions where is part
is exectued for all the GPUs in the hive before going to the next step.

v2: Update names for amdgpu_device_lock/unlock functions.

v3: Introduce per hive locking to avoid multiple resets for GPUs
in same hive.

Signed-off-by: Andrey Grodzovsky 
Reviewed-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 374 -
 2 files changed, 262 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 1ee3f0b..9053355 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1026,6 +1026,9 @@ struct amdgpu_device {
unsigned long last_mm_index;
boolin_gpu_reset;
struct mutex  lock_reset;
+
+   int asic_reset_res;
+   int resched;
 };
 
 static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index d118394..477d153 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3159,86 +3159,6 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
return 0;
 }
 
-/**
- * amdgpu_device_reset - reset ASIC/GPU for bare-metal or passthrough
- *
- * @adev: amdgpu device pointer
- *
- * attempt to do soft-reset or full-reset and reinitialize Asic
- * return 0 means succeeded otherwise failed
- */
-static int amdgpu_device_reset(struct amdgpu_device *adev)
-{
-   bool need_full_reset, vram_lost = 0;
-   int r;
-
-   need_full_reset = amdgpu_device_ip_need_full_reset(adev);
-
-   if (!need_full_reset) {
-   amdgpu_device_ip_pre_soft_reset(adev);
-   r = amdgpu_device_ip_soft_reset(adev);
-   amdgpu_device_ip_post_soft_reset(adev);
-   if (r || amdgpu_device_ip_check_soft_reset(adev)) {
-   DRM_INFO("soft reset failed, will fallback to full 
reset!\n");
-   need_full_reset = true;
-   }
-   }
-
-   if (need_full_reset) {
-   r = amdgpu_device_ip_suspend(adev);
-
-retry:
-   r = amdgpu_asic_reset(adev);
-   /* post card */
-   amdgpu_atom_asic_init(adev->mode_info.atom_context);
-
-   if (!r) {
-   dev_info(adev->dev, "GPU reset succeeded, trying to 
resume\n");
-   r = amdgpu_device_ip_resume_phase1(adev);
-   if (r)
-   goto out;
-
-   vram_lost = amdgpu_device_check_vram_lost(adev);
-   if (vram_lost) {
-   DRM_ERROR("VRAM is lost!\n");
-   atomic_inc(>vram_lost_counter);
-   }
-
-   r = amdgpu_gtt_mgr_recover(
-   >mman.bdev.man[TTM_PL_TT]);
-   if (r)
-   goto out;
-
-   r = amdgpu_device_fw_loading(adev);
-   if (r)
-   return r;
-
-   r = amdgpu_device_ip_resume_phase2(adev);
-   if (r)
-   goto out;
-
-   if (vram_lost)
-   amdgpu_device_fill_reset_magic(adev);
-   }
-   }
-
-out:
-   if (!r) {
-   amdgpu_irq_gpu_reset_resume_helper(adev);
-   r = amdgpu_ib_ring_tests(adev);
-   if (r) {
-   dev_err(adev->dev, "ib ring test failed (%d).\n", r);
-   r = amdgpu_device_ip_suspend(adev);
-   need_full_reset = true;
-   goto retry;
-   }
-   }
-
-   if (!r)
-   r = amdgpu_device_recover_vram(adev);
-
-   return r;
-}
 
 /**
  * amdgpu_device_reset_sriov - reset ASIC for SR-IOV vf
@@ -3337,31 +3257,16 @@ bool amdgpu_device_should_recover_gpu(struct 
amdgpu_device *adev)
return false;
 }
 
-/**
- * amdgpu_device_gpu_recover - reset the asic and recover scheduler
- *
- * @adev: amdgpu device pointer
- * @job: which job trigger hang
- *
- * Attempt to reset the GPU if it has hung (all asics).
- * Returns 0 for success or an error on failure.
- */
-int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
- struct amdgpu_job *job)
-{
- 

[PATCH v3 1/3] drm/amdgpu: Refactor amdgpu_xgmi_add_device v3

[Andrey Grodzovsky]

This is prep work for updating each PSP FW in hive after
GPU reset.
Split into build topology SW state and update each PSP FW in the hive.
Save topology and count of XGMI devices for reuse.

v2: Create seperate header for XGMI.
v3: Rebase

Signed-off-by: Andrey Grodzovsky 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  6 
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 56 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   | 36 +++
 4 files changed, 72 insertions(+), 28 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index adbad0e..1ee3f0b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1222,12 +1222,6 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, 
unsigned int pipe);
 long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd,
 unsigned long arg);
 
-
-/*
- * functions used by amdgpu_xgmi.c
- */
-int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
-
 /*
  * functions used by amdgpu_encoder.c
  */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index cb06e68..d118394 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -59,6 +59,8 @@
 #include "amdgpu_amdkfd.h"
 #include "amdgpu_pm.h"
 
+#include "amdgpu_xgmi.h"
+
 MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin");
 MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin");
 MODULE_FIRMWARE("amdgpu/raven_gpu_info.bin");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 909216a..59e667a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -24,6 +24,7 @@
 #include 
 #include "amdgpu.h"
 #include "amdgpu_psp.h"
+#include "amdgpu_xgmi.h"
 
 
 static DEFINE_MUTEX(xgmi_mutex);
@@ -34,12 +35,14 @@ static DEFINE_MUTEX(xgmi_mutex);
 struct amdgpu_hive_info {
uint64_thive_id;
struct list_headdevice_list;
+   struct psp_xgmi_topology_info   topology_info;
+   int number_devices;
 };
 
 static struct amdgpu_hive_info xgmi_hives[AMDGPU_MAX_XGMI_HIVE];
 static unsigned hive_count = 0;
 
-static struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device 
*adev)
+struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev)
 {
int i;
struct amdgpu_hive_info *tmp;
@@ -61,12 +64,33 @@ static struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct 
amdgpu_device *adev)
return tmp;
 }
 
+int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct 
amdgpu_device *adev)
+{
+   int ret = -EINVAL;
+
+   /* Each psp need to set the latest topology */
+   ret = psp_xgmi_set_topology_info(>psp,
+hive->number_devices,
+>topology_info);
+   if (ret)
+   dev_err(adev->dev,
+   "XGMI: Set topology failure on device %llx, hive %llx, 
ret %d",
+   adev->gmc.xgmi.node_id,
+   adev->gmc.xgmi.hive_id, ret);
+   else
+   dev_info(adev->dev, "XGMI: Add node %d to hive 0x%llx.\n",
+adev->gmc.xgmi.physical_node_id,
+adev->gmc.xgmi.hive_id);
+
+   return ret;
+}
+
 int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 {
-   struct psp_xgmi_topology_info *tmp_topology;
+   struct psp_xgmi_topology_info *hive_topology;
struct amdgpu_hive_info *hive;
struct amdgpu_xgmi  *entry;
-   struct amdgpu_device*tmp_adev;
+   struct amdgpu_device *tmp_adev = NULL;
 
int count = 0, ret = -EINVAL;
 
@@ -76,21 +100,21 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
adev->gmc.xgmi.node_id = psp_xgmi_get_node_id(>psp);
adev->gmc.xgmi.hive_id = psp_xgmi_get_hive_id(>psp);
 
-   tmp_topology = kzalloc(sizeof(struct psp_xgmi_topology_info), 
GFP_KERNEL);
-   if (!tmp_topology)
-   return -ENOMEM;
mutex_lock(_mutex);
hive = amdgpu_get_xgmi_hive(adev);
if (!hive)
goto exit;
 
+   hive_topology = >topology_info;
+
list_add_tail(>gmc.xgmi.head, >device_list);
list_for_each_entry(entry, >device_list, head)
-   tmp_topology->nodes[count++].node_id = entry->node_id;
+   hive_topology->nodes[count++].node_id = entry->node_id;
+   hive->number_devices = count;
 
/* Each psp need to get the latest topology */
list_for_each_entry(tmp_adev, >device_list, gmc.xgmi.head) {
-   ret = psp_xgmi_get_topology_info(_adev->psp, count, 
tmp_topology);
+   ret = 

[PATCH v3 0/3] Add support for XGMI hive reset

[Andrey Grodzovsky]

This set of patches adds support to reset entire XGMI hive
when reset is required.

Patches 1-2 refactoring a bit the XGMI infrastructure as
preparaton for the actual hive reset change.

Patch 5 is GPU reset/recovery refactored to support XGMI
hive reset.

Andrey Grodzovsky (3):
  drm/amdgpu: Refactor amdgpu_xgmi_add_device v3
  drm/amdgpu: Expose hive adev list and xgmi_mutex v3
  drm/amdgpu: Refactor GPU reset for XGMI hive case v3

 drivers/gpu/drm/amd/amdgpu/amdgpu.h|   9 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 376 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   |  68 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  39 +++
 4 files changed, 343 insertions(+), 149 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h

-- 
2.7.4

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

Re: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ

[Deucher, Alexander]

Is this a DP to HDMI adapter?  I think 4k@60 should be valid on DP in general 
on ST/CZ, but Harry or Leo should comment.


Alex


From: S, Shirish
Sent: Tuesday, November 27, 2018 3:58:12 AM
To: Deucher, Alexander; Li, Sun peng (Leo); Wentland, Harry
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ


However, while using Type-C connectors noted that the signal type is actually 
SIGNAL_TYPE_DISPLAY_PORT and found that the check was missing.

Hence have added the same in https://patchwork.freedesktop.org/patch/264033/



Regards,

Shirish S



From: S, Shirish
Sent: Tuesday, November 27, 2018 9:54 AM
To: Deucher, Alexander ; Li, Sun peng (Leo) 
; Wentland, Harry 
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ



Thanks Alex, found that patch.

My patch is no more required.





Regards,

Shirish S



From: Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>
Sent: Monday, November 26, 2018 7:46 PM
To: S, Shirish mailto:shiris...@amd.com>>; Li, Sun peng 
(Leo) mailto:sunpeng...@amd.com>>; Wentland, Harry 
mailto:harry.wentl...@amd.com>>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ



I thought there was a patch to do this already that got sent out a few weeks 
ago.  Basically limit ST/CZ to modes that do not require a retimer.  Is an 
additional patch needed?



Alex



From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of S, Shirish mailto:shiris...@amd.com>>
Sent: Monday, November 26, 2018 1:36:30 AM
To: Li, Sun peng (Leo); Wentland, Harry
Cc: amd-gfx@lists.freedesktop.org; S, 
Shirish
Subject: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ



[Why]
ST/CZ (dce110) advertises modes such as 4k@60Hz etc.,
that it cannot handle correctly, hence  resulting in
several issues like flickering, black lines/flashes and so on.

[How]
These modes are basically high pixel clock ones, hence
limit the same to be advertised to avoid bad user experiences

Signed-off-by: Shirish S mailto:shiris...@amd.com>>
Suggested-by: Harry Wentland 
mailto:harry.wentl...@amd.com>>
---
 .../gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c| 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
index 1b2fe0d..1b8fe99 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
@@ -1121,6 +1121,16 @@ bool dce110_timing_generator_validate_timing(
 if (!timing)
 return false;

+   /* Limit all modes that have a high pixel clock
+* which seems to be problematic on dce110
+* These include: 4k@60Hz, 1080p@144Hz,1440p@120Hz
+* based on the below formula:
+* refresh rate = pixel clock / (htotal * vtotal)
+*/
+   if (timing->pix_clk_khz > 30)
+   return false;
+
+
 hsync_offset = timing->h_border_right + timing->h_front_porch;
 h_sync_start = timing->h_addressable + hsync_offset;

--
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 RFC 5/5] drm/amdgpu: Add accounting of buffer object creation request via DRM cgroup

[Kenny Ho]

Hey Christian,

Sorry for the late reply, I missed this for some reason.

On Wed, Nov 21, 2018 at 5:00 AM Christian König
 wrote:
> > diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> > index 370e9a5536ef..531726443104 100644
> > --- a/include/uapi/drm/amdgpu_drm.h
> > +++ b/include/uapi/drm/amdgpu_drm.h
> > @@ -72,6 +72,18 @@ extern "C" {
> >   #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + 
> > DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
> >   #define DRM_IOCTL_AMDGPU_SCHED  DRM_IOW(DRM_COMMAND_BASE + 
> > DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
> >
> > +enum amdgpu_mem_domain {
> > + AMDGPU_MEM_DOMAIN_CPU,
> > + AMDGPU_MEM_DOMAIN_GTT,
> > + AMDGPU_MEM_DOMAIN_VRAM,
> > + AMDGPU_MEM_DOMAIN_GDS,
> > + AMDGPU_MEM_DOMAIN_GWS,
> > + AMDGPU_MEM_DOMAIN_OA,
> > + __MAX_AMDGPU_MEM_DOMAIN
> > +};
>
> Well that is a clear NAK since it duplicates the TTM defines. Please use
> that one instead and don't make this UAPI.
This is defined to help with the chunk of changes below.  The
AMDGPU_GEM_DOMAIN* already exists and this is similar to how TTM has
TTM_PL_* to help with the creation of TTM_PL_FLAG_*:
https://elixir.bootlin.com/linux/v4.20-rc4/source/include/drm/ttm/ttm_placement.h#L36

I don't disagree that there is a duplication here but it's
pre-existing so if you can help clarify my confusion that would be
much appreciated.

Reards,
Kenny

> > +
> > +extern char const *amdgpu_mem_domain_names[];
> > +
> >   /**
> >* DOC: memory domains
> >*
> > @@ -95,12 +107,12 @@ extern "C" {
> >* %AMDGPU_GEM_DOMAIN_OAOrdered append, used by 3D or Compute engines
> >* for appending data.
> >*/
> > -#define AMDGPU_GEM_DOMAIN_CPU0x1
> > -#define AMDGPU_GEM_DOMAIN_GTT0x2
> > -#define AMDGPU_GEM_DOMAIN_VRAM   0x4
> > -#define AMDGPU_GEM_DOMAIN_GDS0x8
> > -#define AMDGPU_GEM_DOMAIN_GWS0x10
> > -#define AMDGPU_GEM_DOMAIN_OA 0x20
> > +#define AMDGPU_GEM_DOMAIN_CPU(1 << AMDGPU_MEM_DOMAIN_CPU)
> > +#define AMDGPU_GEM_DOMAIN_GTT(1 << AMDGPU_MEM_DOMAIN_GTT)
> > +#define AMDGPU_GEM_DOMAIN_VRAM   (1 << AMDGPU_MEM_DOMAIN_VRAM)
> > +#define AMDGPU_GEM_DOMAIN_GDS(1 << AMDGPU_MEM_DOMAIN_GDS)
> > +#define AMDGPU_GEM_DOMAIN_GWS(1 << AMDGPU_MEM_DOMAIN_GWS)
> > +#define AMDGPU_GEM_DOMAIN_OA (1 << AMDGPU_MEM_DOMAIN_OA)
> >   #define AMDGPU_GEM_DOMAIN_MASK  (AMDGPU_GEM_DOMAIN_CPU | \
> >AMDGPU_GEM_DOMAIN_GTT | \
> >AMDGPU_GEM_DOMAIN_VRAM | \
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/display: Fix compile error with ACPI disabled

[David Francis]

The fallback code for getting default backlight caps was using
the wrong variable name.  Fix it.

Fixes: 
https://lists.freedesktop.org/archives/dri-devel/2018-November/197752.html
Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 27df3ae945be..aa3f8200fa69 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1615,8 +1615,8 @@ static void amdgpu_dm_update_backlight_caps(struct 
amdgpu_display_manager *dm)
AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
}
 #else
-   dm->backlight_min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
-   dm->backlight_max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
+   dm->backlight_caps.min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
+   dm->backlight_caps.max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
 #endif
 }
 
-- 
2.17.1

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

[PATCH] drm: radeon: fix overflow on 32bit systems

[Yang Xiao]

From: Young Xiao 

the type mem->start is unsigned long, so this can overflow on
32bit system, since the type addr is uint64_t.

Signed-off-by: Young Xiao 
---
 drivers/gpu/drm/radeon/radeon_vm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_vm.c 
b/drivers/gpu/drm/radeon/radeon_vm.c
index 7f1a9c7..51559d8 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -946,7 +946,7 @@ int radeon_vm_bo_update(struct radeon_device *rdev,
bo_va->flags &= ~RADEON_VM_PAGE_WRITEABLE;
 
if (mem) {
-   addr = mem->start << PAGE_SHIFT;
+   addr = (u64)mem->start << PAGE_SHIFT;
if (mem->mem_type != TTM_PL_SYSTEM) {
bo_va->flags |= RADEON_VM_PAGE_VALID;
}
-- 
2.7.4

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

How to use vce1 in Radeon Pro WX5100

[Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)]

Hi all，
 I found two vce block under /sys/kernel/debug/dri/1/amdgpu_ring_vce0 & 
amdgpu_ring_vce1.
when I use ffmpeg to do H.264 encoding , I use “umr �Ctop”  and press “c” to 
show the VCE loading,  and
I found that only vce0 had loading , and vce1 always be 0%, like this:
VCE Bits:
VCE0 => 100%   |VCE1 => 0 %   |
  So I think vce1 block has never been used, and how to select vce1 to do 
encode ?
My environment:
Radeon Pro WX5100
Mesa18.0.5
Linux kernel 4.19

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

[PATCH] drm/amd/pp: fix spelling mistake "dependancy" -> "dependency"

[Colin King]

From: Colin Ian King 

There are spelling mistakes in PP_ASSERT_WITH_CODE messages, fix these.

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 88f6b35ea6fe..5dcd21d29dbf 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -269,7 +269,7 @@ static int smu7_construct_voltage_tables(struct pp_hwmgr 
*hwmgr)

hwmgr->dyn_state.mvdd_dependency_on_mclk);
 
PP_ASSERT_WITH_CODE((0 == result),
-   "Failed to retrieve SVI2 MVDD table from 
dependancy table.",
+   "Failed to retrieve SVI2 MVDD table from 
dependency table.",
return result;);
}
 
@@ -288,7 +288,7 @@ static int smu7_construct_voltage_tables(struct pp_hwmgr 
*hwmgr)
result = 
phm_get_svi2_voltage_table_v0(&(data->vddci_voltage_table),

hwmgr->dyn_state.vddci_dependency_on_mclk);
PP_ASSERT_WITH_CODE((0 == result),
-   "Failed to retrieve SVI2 VDDCI table from 
dependancy table.",
+   "Failed to retrieve SVI2 VDDCI table from 
dependency table.",
return result);
}
 
@@ -317,7 +317,7 @@ static int smu7_construct_voltage_tables(struct pp_hwmgr 
*hwmgr)
table_info->vddc_lookup_table);
 
PP_ASSERT_WITH_CODE((0 == result),
-   "Failed to retrieve SVI2 VDDC table from dependancy 
table.", return result;);
+   "Failed to retrieve SVI2 VDDC table from dependency 
table.", return result;);
}
 
tmp = smum_get_mac_definition(hwmgr, SMU_MAX_LEVELS_VDDC);
-- 
2.19.1

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

RE: [Intel-gfx] [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices

[Ho, Kenny]

On Tue, Nov 27, 2018 at 4:46 AM Joonas Lahtinen 
 wrote:
> I think a more abstract property "% of GPU (processing power)" might
> be a more universal approach. One can then implement that through
> subdividing the resources or timeslicing them, depending on the GPU
> topology.
>
> Leasing 1/8th, 1/4th or 1/2 of the GPU would probably be the most
> applicable to cloud provider usecases, too. At least that's what I
> see done for the CPUs today.
I think there are opportunities to slice the gpu in more than one way (similar 
to the way it is done for cpu.)  We can potentially frame resources as 
continuous or discrete.  Percentage definitely fits well for continuous 
measurements such as time/time slices but I think there are places for discrete 
units such as core counts as well.

Regards,
Kenny

> That combined with the "GPU memory usable" property should be a good
> starting point to start subdividing the GPU resources for multiple
> users.
>
> Regards, Joonas
>
> >
> > Your feedback is highly appreciated.
> >
> > Best Regards,
> > Harish
> >
> >
> >
> > From: amd-gfx  on behalf of Tejun 
> > Heo 
> > Sent: Tuesday, November 20, 2018 5:30 PM
> > To: Ho, Kenny
> > Cc: cgro...@vger.kernel.org; intel-...@lists.freedesktop.org; 
> > y2ke...@gmail.com; amd-gfx@lists.freedesktop.org; 
> > dri-de...@lists.freedesktop.org
> > Subject: Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor 
> > specific DRM devices
> >
> >
> > Hello,
> >
> > On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote:
> > > By this reply, are you suggesting that vendor specific resources
> > > will never be acceptable to be managed under cgroup?  Let say a user
> >
> > I wouldn't say never but whatever which gets included as a cgroup
> > controller should have clearly defined resource abstractions and the
> > control schemes around them including support for delegation.  AFAICS,
> > gpu side still seems to have a long way to go (and it's not clear
> > whether that's somewhere it will or needs to end up).
> >
> > > want to have similar functionality as what cgroup is offering but to
> > > manage vendor specific resources, what would you suggest as a
> > > solution?  When you say keeping vendor specific resource regulation
> > > inside drm or specific drivers, do you mean we should replicate the
> > > cgroup infrastructure there or do you mean either drm or specific
> > > driver should query existing hierarchy (such as device or perhaps
> > > cpu) for the process organization information?
> > >
> > > To put the questions in more concrete terms, let say a user wants to
> > > expose certain part of a gpu to a particular cgroup similar to the
> > > way selective cpu cores are exposed to a cgroup via cpuset, how
> > > should we go about enabling such functionality?
> >
> > Do what the intel driver or bpf is doing?  It's not difficult to hook
> > into cgroup for identification purposes.
> >
> > Thanks.
> >
> > --
> > tejun
> > ___
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> >
> > amd-gfx Info Page - freedesktop.org
> > lists.freedesktop.org
> > To see the collection of prior postings to the list, visit the amd-gfx 
> > Archives.. Using amd-gfx: To post a message to all the list members, send 
> > email to amd-gfx@lists.freedesktop.org. You can subscribe to the list, or 
> > change your existing subscription, in the sections below.
> > 
> > ___
> > Intel-gfx mailing list
> > intel-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v7 3/5] drm: Document variable refresh properties

[Wentland, Harry]

On 2018-11-27 4:22 a.m., Daniel Vetter wrote:
> On Mon, Nov 12, 2018 at 04:12:10PM +, Wentland, Harry wrote:
>> On 2018-11-08 9:43 a.m., Nicholas Kazlauskas wrote:
>>> These include the drm_connector 'vrr_capable' and the drm_crtc
>>> 'vrr_enabled' properties.
>>>
>>> Signed-off-by: Nicholas Kazlauskas 
>>> Cc: Harry Wentland 
>>> Cc: Manasi Navare 
>>> Cc: Pekka Paalanen 
>>> Cc: Ville Syrjälä 
>>> Cc: Michel Dänzer 
>>
>> Looks good. Whole series is
>> Reviewed-by: Harry Wentland 
>>
>> How are we with the userspace patches? We should probably hold off
>> pushing the kernel patches until the userspace work is all reviewed.
> 
> Do some igts exist for this too? Especially for tricky pieces of uapi
> having a non-vendor reference code somewhere would be good, aside from
> testing and all that.

Not yet, unfortunately, although it's on our list of things to do.

Harry

> -Daniel
> 
>>
>> Harry
>>
>>> ---
>>>  Documentation/gpu/drm-kms.rst   |  7 
>>>  drivers/gpu/drm/drm_connector.c | 68 +
>>>  2 files changed, 75 insertions(+)
>>>
>>> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
>>> index 4b1501b4835b..8da2a178cf85 100644
>>> --- a/Documentation/gpu/drm-kms.rst
>>> +++ b/Documentation/gpu/drm-kms.rst
>>> @@ -575,6 +575,13 @@ Explicit Fencing Properties
>>>  .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
>>> :doc: explicit fencing properties
>>>  
>>> +
>>> +Variable Refresh Properties
>>> +---
>>> +
>>> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
>>> +   :doc: Variable refresh properties
>>> +
>>>  Existing KMS Properties
>>>  ---
>>>  
>>> diff --git a/drivers/gpu/drm/drm_connector.c 
>>> b/drivers/gpu/drm/drm_connector.c
>>> index 49290060ab7b..0e4287461997 100644
>>> --- a/drivers/gpu/drm/drm_connector.c
>>> +++ b/drivers/gpu/drm/drm_connector.c
>>> @@ -1255,6 +1255,74 @@ int drm_mode_create_scaling_mode_property(struct 
>>> drm_device *dev)
>>>  }
>>>  EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>>>  
>>> +/**
>>> + * DOC: Variable refresh properties
>>> + *
>>> + * Variable refresh rate capable displays can dynamically adjust their
>>> + * refresh rate by extending the duration of their vertical front porch
>>> + * until page flip or timeout occurs. This can reduce or remove stuttering
>>> + * and latency in scenarios where the page flip does not align with the
>>> + * vblank interval.
>>> + *
>>> + * An example scenario would be an application flipping at a constant rate
>>> + * of 48Hz on a 60Hz display. The page flip will frequently miss the vblank
>>> + * interval and the same contents will be displayed twice. This can be
>>> + * observed as stuttering for content with motion.
>>> + *
>>> + * If variable refresh rate was active on a display that supported a
>>> + * variable refresh range from 35Hz to 60Hz no stuttering would be 
>>> observable
>>> + * for the example scenario. The minimum supported variable refresh rate of
>>> + * 35Hz is below the page flip frequency and the vertical front porch can
>>> + * be extended until the page flip occurs. The vblank interval will be
>>> + * directly aligned to the page flip rate.
>>> + *
>>> + * Not all userspace content is suitable for use with variable refresh 
>>> rate.
>>> + * Large and frequent changes in vertical front porch duration may worsen
>>> + * perceived stuttering for input sensitive applications.
>>> + *
>>> + * Panel brightness will also vary with vertical front porch duration. Some
>>> + * panels may have noticeable differences in brightness between the minimum
>>> + * vertical front porch duration and the maximum vertical front porch 
>>> duration.
>>> + * Large and frequent changes in vertical front porch duration may produce
>>> + * observable flickering for such panels.
>>> + *
>>> + * Userspace control for variable refresh rate is supported via properties
>>> + * on the _connector and _crtc objects.
>>> + *
>>> + * "vrr_capable":
>>> + * Optional _connector boolean property that drivers should attach
>>> + * with drm_connector_attach_vrr_capable_property() on connectors that
>>> + * could support variable refresh rates. Drivers should update the
>>> + * property value by calling drm_connector_set_vrr_capable_property().
>>> + *
>>> + * Absence of the property should indicate absence of support.
>>> + *
>>> + * "vrr_enabled":
>>> + * Default _crtc boolean property that notifies the driver that the
>>> + * content on the CRTC is suitable for variable refresh rate presentation.
>>> + * The driver will take this property as a hint to enable variable
>>> + * refresh rate support if the receiver supports it, ie. if the
>>> + * "vrr_capable" property is true on the _connector object. The
>>> + * vertical front porch duration will be extended until page-flip or
>>> + * timeout when enabled.
>>> + *
>>> + * The minimum vertical front porch duration is defined as the vertical
>>> + * front porch duration for 

Re: [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 variants

[Christian König]

Am 27.11.18 um 02:47 schrieb Zhang, Jerry(Junwei):

On 11/26/18 5:28 PM, Christian König wrote:

Am 26.11.18 um 03:38 schrieb Zhang, Jerry(Junwei):

On 11/24/18 3:32 AM, Deucher, Alexander wrote:


Is this required? Are the harvesting fuses incorrect?  If the 
blocks are harvested, we should bail out of the blocks properly 
during init.  Also, please make this more explicit if we still need 
it.  E.g.,





The harvest fuse is indeed disabling UVD and VCE, as it's a mining card.
Then any command to UVD/VCE causing NULL pointer issue, like 
amdgpu_test.


In this case we should fix the NULL pointer issue instead. Do you 
have a backtrace for this?


Sorry to miss the detail.
The NULL pointer is caused by UVD is not initialized as it's disabled 
in VBIOS for this kind of card.


Yeah, but that should be handled correctly.



When cs submit, it will check ring->funcs->parse_cs in 
amdgpu_cs_ib_fill().
However, uvd_v6_0_early_init() skip the set ring function, as 
CC_HARVEST_FUSES is set UVD/VCE disabled.

Then the access to UVD/VCE ring's funcs will cause NULL pointer issue.

BTW, Windows driver disables UVD/VCE for it as well.


You are approaching this from the wrong side. The fact that UVD/VCE is 
disabled should already be handled correctly.


The problem is rather that in a couple of places (amdgpu_ctx_init for 
example) we assume that we have at least one UVD/VCE ring.


Alex is right that checking the fuses should be sufficient and we rather 
need to fix the handling here instead of adding another workaround.


Regards,
Christian.



Regards,
Jerry



Regards,
Christian.



AFAIW, windows also disable UVD and VCE in initialization.


   if ((adev->pdev->device == 0x67df) &&
  (adev->pdev->revision == 0xf7)) {

/* Some polaris12 variants don't support UVD/VCE */

  } else  {

amdgpu_device_ip_block_add(adev, _v6_3_ip_block);

amdgpu_device_ip_block_add(adev, _v3_4_ip_block);

    }




OK, will explicit the process.

Regards,
Jerry


That way if we re-arrange the order later, it will be easier to track.


Alex


*From:* amd-gfx  on behalf 
of Junwei Zhang 

*Sent:* Friday, November 23, 2018 3:32:27 AM
*To:* amd-gfx@lists.freedesktop.org
*Cc:* Zhang, Jerry
*Subject:* [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 
variants

Some variants don't support UVD and VCE.

Signed-off-by: Junwei Zhang 
---
 drivers/gpu/drm/amd/amdgpu/vi.c | 4 
 1 file changed, 4 insertions(+)

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

index f3a4cf1f013a..3338b013ded4 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -1660,6 +1660,10 @@ int vi_set_ip_blocks(struct amdgpu_device *adev)
amdgpu_device_ip_block_add(adev, _v11_2_ip_block);
 amdgpu_device_ip_block_add(adev, _v8_0_ip_block);
 amdgpu_device_ip_block_add(adev, _v3_1_ip_block);
+   /* Some polaris12 variants don't support UVD/VCE */
+   if ((adev->pdev->device == 0x67df) &&
+ (adev->pdev->revision == 0xf7))
+   break;
 amdgpu_device_ip_block_add(adev, _v6_3_ip_block);
 amdgpu_device_ip_block_add(adev, _v3_4_ip_block);
 break;
--
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





___
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: add the checking to avoid NULL pointer dereference

[Christian König]

Am 27.11.18 um 01:40 schrieb Deepak Sharma:


On 11/26/18 1:57 AM, Zhou, David(ChunMing) wrote:



-Original Message-
From: Christian König 
Sent: Monday, November 26, 2018 5:23 PM
To: Sharma, Deepak ; Zhou, David(ChunMing)
; Koenig, Christian ;
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer
dereference

Am 26.11.18 um 02:59 schrieb Sharma, Deepak:

在 2018/11/24 2:10, Koenig, Christian 写道:

Am 23.11.18 um 15:10 schrieb Zhou, David(ChunMing):

在 2018/11/23 21:30, Koenig, Christian 写道:

Am 23.11.18 um 14:27 schrieb Zhou, David(ChunMing):

在 2018/11/22 19:25, Christian König 写道:

Am 22.11.18 um 07:56 schrieb Sharma, Deepak:

when returned fence is not valid mostly due to userspace ignored
previous error causes NULL pointer dereference.

Again, this is clearly incorrect. The my other mails on the
earlier patch.

Sorry for I didn't get your history, but looks from the patch
itself, it is still a valid patch, isn't it?

No, the semantic of amdgpu_ctx_get_fence() is that we return NULL
when the fence is already signaled.

So this patch could totally break userspace because it changes the
behavior when we try to sync to an already signaled fence.

Ah, I got your meaning, how about attached patch?

Yeah something like this, but I would just give the
DRM_SYNCOBJ_CREATE_SIGNALED instead.

I mean that's what this flag is good for isn't it?

Yeah, I give a flag initally when creating patch, but as you know, there is a

swich case not be able to use that flag:

    case AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD:
    fd = get_unused_fd_flags(O_CLOEXEC);
    if (fd < 0) {
    dma_fence_put(fence);
    return fd;
    }

    sync_file = sync_file_create(fence);
    dma_fence_put(fence);
    if (!sync_file) {
    put_unused_fd(fd);
    return -ENOMEM;
    }

    fd_install(fd, sync_file->file);
    info->out.handle = fd;
    return 0;

So I change to stub fence instead.

Yeah, I've missed that case. Not sure if the sync_file can deal with a NULL
fence.

We should then probably move the stub fence function into
dma_fence_stub.c under drivers/dma-buf to keep the stuff together.

Yes, you wrap it to review first with your stub fence, we can do it separately 
first.

-David

-David

I have not applied this patch.
The issue was trying to address is when amdgpu_cs_ioctl() failed due to

low memory (ENOMEM) but userspace chose to proceed and called
amdgpu_cs_fence_to_handle_ioctl().

In amdgpu_cs_fence_to_handle_ioctl(), fence is null and later causing
NULL pointer dereference, this patch was to avoid that and system panic

But I understand now that its a valid case retuning NULL if fence was already
signaled but need to handle case so avoid kernel panic. Seems David patch
should fix this, I will test it tomorrow.

Mhm, but don't we bail out with an error if we ask for a failed command
submission? If not that sounds like a bug as well.

Christian.


Where do we do that?
I see error
[drm:amdgpu_cs_ioctl] *ERROR* amdgpu_cs_list_validate(validated) failed.
[drm:amdgpu_cs_ioctl] *ERROR* Not enough memory for command submission!
BUG: unable to handle kernel NULL pointer dereference at 0008
Did some more debugging,dma_fence_is_array() is causing NULL pointer
dereference call through sync_file_ioctl.

Also I think changes in David patch cant be applied on
amd-staging-drm-next, which all patches I should take to get it correctly?


Mhm, what you say here actually doesn't make much sense.

When the sequence number is invalid because the submission failed 
amdgpu_ctx_get_fence() returns an error:

    if (seq >= centity->sequence) {
    spin_unlock(>ring_lock);
    return ERR_PTR(-EINVAL);
    }


This error is then handled in amdgpu_cs_fence_to_handle_ioctl():

    fence = amdgpu_cs_get_fence(adev, filp, >in.fence);
    if (IS_ERR(fence))
    return PTR_ERR(fence);


So the error handling for this is actually in place.

The only thing that could go wrong is that userspace sends a sequence 
number which is to small.


The correct solution is to either set the flag as I suggested and make 
sync_file_create() capable of handling a NULL fence.


Or we make the stub fence global like David suggested.

Regards,
Christian.




-Deepak

Christian.


-David

If that patch was applied then please revert it immediately.

Christian.


-David

If you have already pushed the patch then please revert.

Christian.


Signed-off-by: Deepak Sharma 
---
     drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++
     1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 024dfbd87f11..14166cd8a12f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1403,6 +1403,8 @@ static struct 

Re: [Intel-gfx] [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices

[Joonas Lahtinen]

Quoting Kasiviswanathan, Harish (2018-11-26 22:59:30)
> Thanks Tejun,Eric and Christian for your replies.
> 
> We want GPUs resource management to work seamlessly with containers and 
> container orchestration. With the Intel / bpf based approach this is not 
> possible. 
> 
> From your response we gather the following. GPU resources need to be 
> abstracted. We will send a new proposal in same vein. Our current thinking is 
> to start with a single abstracted resource and build a framework that can be 
> expanded to include additional resources. We plan to start with “GPU cores”. 
> We believe all GPUs have some concept of cores or compute unit.

I think a more abstract property "% of GPU (processing power)" might
be a more universal approach. One can then implement that through
subdividing the resources or timeslicing them, depending on the GPU
topology.

Leasing 1/8th, 1/4th or 1/2 of the GPU would probably be the most
applicable to cloud provider usecases, too. At least that's what I
see done for the CPUs today.

That combined with the "GPU memory usable" property should be a good
starting point to start subdividing the GPU resources for multiple
users.

Regards, Joonas

> 
> Your feedback is highly appreciated.
> 
> Best Regards,
> Harish
> 
> 
> 
> From: amd-gfx  on behalf of Tejun Heo 
> 
> Sent: Tuesday, November 20, 2018 5:30 PM
> To: Ho, Kenny
> Cc: cgro...@vger.kernel.org; intel-...@lists.freedesktop.org; 
> y2ke...@gmail.com; amd-gfx@lists.freedesktop.org; 
> dri-de...@lists.freedesktop.org
> Subject: Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor 
> specific DRM devices
>   
> 
> Hello,
> 
> On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote:
> > By this reply, are you suggesting that vendor specific resources
> > will never be acceptable to be managed under cgroup?  Let say a user
> 
> I wouldn't say never but whatever which gets included as a cgroup
> controller should have clearly defined resource abstractions and the
> control schemes around them including support for delegation.  AFAICS,
> gpu side still seems to have a long way to go (and it's not clear
> whether that's somewhere it will or needs to end up).
> 
> > want to have similar functionality as what cgroup is offering but to
> > manage vendor specific resources, what would you suggest as a
> > solution?  When you say keeping vendor specific resource regulation
> > inside drm or specific drivers, do you mean we should replicate the
> > cgroup infrastructure there or do you mean either drm or specific
> > driver should query existing hierarchy (such as device or perhaps
> > cpu) for the process organization information?
> > 
> > To put the questions in more concrete terms, let say a user wants to
> > expose certain part of a gpu to a particular cgroup similar to the
> > way selective cpu cores are exposed to a cgroup via cpuset, how
> > should we go about enabling such functionality?
> 
> Do what the intel driver or bpf is doing?  It's not difficult to hook
> into cgroup for identification purposes.
> 
> Thanks.
> 
> -- 
> tejun
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> 
> amd-gfx Info Page - freedesktop.org
> lists.freedesktop.org
> To see the collection of prior postings to the list, visit the amd-gfx 
> Archives.. Using amd-gfx: To post a message to all the list members, send 
> email to amd-gfx@lists.freedesktop.org. You can subscribe to the list, or 
> change your existing subscription, in the sections below.
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: Avoid endless loop in GPUVM fragment processing

[Christian König]

Am 26.11.18 um 23:01 schrieb Kuehling, Felix:

Don't bounce back to the root level for fragment processing, because
huge pages are not supported at that level. This is unlikely to happen
with the default VM size on Vega, but can be exposed by limiting the
VM size with the amdgpu.vm_size module parameter.

Signed-off-by: Felix Kuehling 


Good catch! Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index be3e360..0877ff9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1656,9 +1656,11 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
if (!amdgpu_vm_pt_descendant(adev, ))
return -ENOENT;
continue;
-   } else if (frag >= parent_shift) {
+   } else if (frag >= parent_shift &&
+  cursor.level - 1 != adev->vm_manager.root_level) {
/* If the fragment size is even larger than the parent
-* shift we should go up one level and check it again.
+* shift we should go up one level and check it again
+* unless one level up is the root level.
 */
if (!amdgpu_vm_pt_ancestor())
return -ENOENT;


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

Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices

[Koenig, Christian]

Hi Harish,

Am 26.11.18 um 21:59 schrieb Kasiviswanathan, Harish:
> Thanks Tejun,Eric and Christian for your replies.
>
> We want GPUs resource management to work seamlessly with containers and 
> container orchestration. With the Intel / bpf based approach this is not 
> possible.

I think one lesson learned is that we should describe this goal in the 
patch covert letter when sending it out. That could have avoid something 
like have of the initial confusion.

>  From your response we gather the following. GPU resources need to be 
> abstracted. We will send a new proposal in same vein. Our current thinking is 
> to start with a single abstracted resource and build a framework that can be 
> expanded to include additional resources. We plan to start with “GPU cores”. 
> We believe all GPUs have some concept of cores or compute unit.

Sounds good, just one comment on creating a framework: Before doing 
something like this think for a moment if it doesn't make sense to 
rather extend the existing cgroup framework. That approach usually makes 
more sense because you rarely need something fundamentally new.

Regards,
Christian.

>
> Your feedback is highly appreciated.
>
> Best Regards,
> Harish
>
>
>
> From: amd-gfx  on behalf of Tejun Heo 
> 
> Sent: Tuesday, November 20, 2018 5:30 PM
> To: Ho, Kenny
> Cc: cgro...@vger.kernel.org; intel-...@lists.freedesktop.org; 
> y2ke...@gmail.com; amd-gfx@lists.freedesktop.org; 
> dri-de...@lists.freedesktop.org
> Subject: Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor 
> specific DRM devices
>
>
> Hello,
>
> On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote:
>> By this reply, are you suggesting that vendor specific resources
>> will never be acceptable to be managed under cgroup?  Let say a user
> I wouldn't say never but whatever which gets included as a cgroup
> controller should have clearly defined resource abstractions and the
> control schemes around them including support for delegation.  AFAICS,
> gpu side still seems to have a long way to go (and it's not clear
> whether that's somewhere it will or needs to end up).
>
>> want to have similar functionality as what cgroup is offering but to
>> manage vendor specific resources, what would you suggest as a
>> solution?  When you say keeping vendor specific resource regulation
>> inside drm or specific drivers, do you mean we should replicate the
>> cgroup infrastructure there or do you mean either drm or specific
>> driver should query existing hierarchy (such as device or perhaps
>> cpu) for the process organization information?
>>
>> To put the questions in more concrete terms, let say a user wants to
>> expose certain part of a gpu to a particular cgroup similar to the
>> way selective cpu cores are exposed to a cgroup via cpuset, how
>> should we go about enabling such functionality?
> Do what the intel driver or bpf is doing?  It's not difficult to hook
> into cgroup for identification purposes.
>
> Thanks.
>

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

Re: [PATCH 1/2] drm/amdgpu: Cast to uint64_t before left shift

[Christian König]

Am 26.11.18 um 23:01 schrieb Kuehling, Felix:

Avoid potential integer overflows with left shift in huge-page mapping
code by casting the operand to uin64_t first.

Signed-off-by: Felix Kuehling 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index dad0e23..be3e360 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -181,7 +181,7 @@ static unsigned amdgpu_vm_num_entries(struct amdgpu_device 
*adev,
  
  	if (level == adev->vm_manager.root_level)

/* For the root directory */
-   return round_up(adev->vm_manager.max_pfn, 1 << shift) >> shift;
+   return round_up(adev->vm_manager.max_pfn, 1ULL << shift) >> 
shift;
else if (level != AMDGPU_VM_PTB)
/* Everything in between */
return 512;
@@ -1666,10 +1666,10 @@ 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;
+   incr = (uint64_t)AMDGPU_GPU_PAGE_SIZE << shift;


I wonder if it doesn't make more sense to make the definition of 
AMDGPU_GPU_PAGE_SIZE 64bit?


Anyway the patch is Reviewed-by: Christian König 
 for now.


Regards,
Christian.


mask = amdgpu_vm_entries_mask(adev, cursor.level);
pe_start = ((cursor.pfn >> shift) & mask) * 8;
-   entry_end = (mask + 1) << shift;
+   entry_end = (uint64_t)(mask + 1) << shift;
entry_end += cursor.pfn & ~(entry_end - 1);
entry_end = min(entry_end, end);
  
@@ -1682,7 +1682,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,

  flags | AMDGPU_PTE_FRAG(frag));
  
  			pe_start += nptes * 8;

-   dst += nptes * AMDGPU_GPU_PAGE_SIZE << shift;
+   dst += (uint64_t)nptes * AMDGPU_GPU_PAGE_SIZE << shift;
  
  			frag_start = upd_end;

if (frag_start >= frag_end) {


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

Re: [PATCH v7 3/5] drm: Document variable refresh properties

[Daniel Vetter]

On Mon, Nov 12, 2018 at 04:12:10PM +, Wentland, Harry wrote:
> On 2018-11-08 9:43 a.m., Nicholas Kazlauskas wrote:
> > These include the drm_connector 'vrr_capable' and the drm_crtc
> > 'vrr_enabled' properties.
> > 
> > Signed-off-by: Nicholas Kazlauskas 
> > Cc: Harry Wentland 
> > Cc: Manasi Navare 
> > Cc: Pekka Paalanen 
> > Cc: Ville Syrjälä 
> > Cc: Michel Dänzer 
> 
> Looks good. Whole series is
> Reviewed-by: Harry Wentland 
> 
> How are we with the userspace patches? We should probably hold off
> pushing the kernel patches until the userspace work is all reviewed.

Do some igts exist for this too? Especially for tricky pieces of uapi
having a non-vendor reference code somewhere would be good, aside from
testing and all that.
-Daniel

> 
> Harry
> 
> > ---
> >  Documentation/gpu/drm-kms.rst   |  7 
> >  drivers/gpu/drm/drm_connector.c | 68 +
> >  2 files changed, 75 insertions(+)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 4b1501b4835b..8da2a178cf85 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -575,6 +575,13 @@ Explicit Fencing Properties
> >  .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
> > :doc: explicit fencing properties
> >  
> > +
> > +Variable Refresh Properties
> > +---
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > +   :doc: Variable refresh properties
> > +
> >  Existing KMS Properties
> >  ---
> >  
> > diff --git a/drivers/gpu/drm/drm_connector.c 
> > b/drivers/gpu/drm/drm_connector.c
> > index 49290060ab7b..0e4287461997 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1255,6 +1255,74 @@ int drm_mode_create_scaling_mode_property(struct 
> > drm_device *dev)
> >  }
> >  EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
> >  
> > +/**
> > + * DOC: Variable refresh properties
> > + *
> > + * Variable refresh rate capable displays can dynamically adjust their
> > + * refresh rate by extending the duration of their vertical front porch
> > + * until page flip or timeout occurs. This can reduce or remove stuttering
> > + * and latency in scenarios where the page flip does not align with the
> > + * vblank interval.
> > + *
> > + * An example scenario would be an application flipping at a constant rate
> > + * of 48Hz on a 60Hz display. The page flip will frequently miss the vblank
> > + * interval and the same contents will be displayed twice. This can be
> > + * observed as stuttering for content with motion.
> > + *
> > + * If variable refresh rate was active on a display that supported a
> > + * variable refresh range from 35Hz to 60Hz no stuttering would be 
> > observable
> > + * for the example scenario. The minimum supported variable refresh rate of
> > + * 35Hz is below the page flip frequency and the vertical front porch can
> > + * be extended until the page flip occurs. The vblank interval will be
> > + * directly aligned to the page flip rate.
> > + *
> > + * Not all userspace content is suitable for use with variable refresh 
> > rate.
> > + * Large and frequent changes in vertical front porch duration may worsen
> > + * perceived stuttering for input sensitive applications.
> > + *
> > + * Panel brightness will also vary with vertical front porch duration. Some
> > + * panels may have noticeable differences in brightness between the minimum
> > + * vertical front porch duration and the maximum vertical front porch 
> > duration.
> > + * Large and frequent changes in vertical front porch duration may produce
> > + * observable flickering for such panels.
> > + *
> > + * Userspace control for variable refresh rate is supported via properties
> > + * on the _connector and _crtc objects.
> > + *
> > + * "vrr_capable":
> > + * Optional _connector boolean property that drivers should attach
> > + * with drm_connector_attach_vrr_capable_property() on connectors that
> > + * could support variable refresh rates. Drivers should update the
> > + * property value by calling drm_connector_set_vrr_capable_property().
> > + *
> > + * Absence of the property should indicate absence of support.
> > + *
> > + * "vrr_enabled":
> > + * Default _crtc boolean property that notifies the driver that the
> > + * content on the CRTC is suitable for variable refresh rate presentation.
> > + * The driver will take this property as a hint to enable variable
> > + * refresh rate support if the receiver supports it, ie. if the
> > + * "vrr_capable" property is true on the _connector object. The
> > + * vertical front porch duration will be extended until page-flip or
> > + * timeout when enabled.
> > + *
> > + * The minimum vertical front porch duration is defined as the vertical
> > + * front porch duration for the current mode.
> > + *
> > + * The maximum vertical front porch duration is greater than or equal to
> > + * the minimum vertical front 

Re: [PATCH v7 3/5] drm: Document variable refresh properties

[Michel Dänzer]

On 2018-11-26 10:49 p.m., Wentland, Harry wrote:
> On 2018-11-12 12:05 p.m., Kazlauskas, Nicholas wrote:
>> On 11/12/18 11:12 AM, Wentland, Harry wrote:
>>> On 2018-11-08 9:43 a.m., Nicholas Kazlauskas wrote:
 These include the drm_connector 'vrr_capable' and the drm_crtc
 'vrr_enabled' properties.

 Signed-off-by: Nicholas Kazlauskas 
 Cc: Harry Wentland 
 Cc: Manasi Navare 
 Cc: Pekka Paalanen 
 Cc: Ville Syrjälä 
 Cc: Michel Dänzer 
>>>
>>> Looks good. Whole series is
>>> Reviewed-by: Harry Wentland 
>>>
>>> How are we with the userspace patches? We should probably hold off pushing 
>>> the kernel patches until the userspace work is all reviewed.
>>>
>>> Harry
>>
>> Thanks for the review.
>>
>> The xf86-video-amdgpu patches have been reviewed and the mesa patches 
>> have been partially reviewed.
> 
> Alex, Michel,
> 
> how do we merge changes like this that provide a kernel API that goes
> together with userspace changes?
> 
> I imagine we'd want to get the kernel changes in first, and then merge
> the xf86-video-amdgpu and mesa changes.

Yeah, that's how it's usually done.


> Any objections to getting this merged via the usual amd-stg?

None from me.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ

[S, Shirish]

However, while using Type-C connectors noted that the signal type is actually 
SIGNAL_TYPE_DISPLAY_PORT and found that the check was missing.
Hence have added the same in https://patchwork.freedesktop.org/patch/264033/

Regards,
Shirish S

From: S, Shirish
Sent: Tuesday, November 27, 2018 9:54 AM
To: Deucher, Alexander ; Li, Sun peng (Leo) 
; Wentland, Harry 
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ

Thanks Alex, found that patch.
My patch is no more required.


Regards,
Shirish S

From: Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>
Sent: Monday, November 26, 2018 7:46 PM
To: S, Shirish mailto:shiris...@amd.com>>; Li, Sun peng 
(Leo) mailto:sunpeng...@amd.com>>; Wentland, Harry 
mailto:harry.wentl...@amd.com>>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ


I thought there was a patch to do this already that got sent out a few weeks 
ago.  Basically limit ST/CZ to modes that do not require a retimer.  Is an 
additional patch needed?



Alex


From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of S, Shirish mailto:shiris...@amd.com>>
Sent: Monday, November 26, 2018 1:36:30 AM
To: Li, Sun peng (Leo); Wentland, Harry
Cc: amd-gfx@lists.freedesktop.org; S, 
Shirish
Subject: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ

[Why]
ST/CZ (dce110) advertises modes such as 4k@60Hz etc.,
that it cannot handle correctly, hence  resulting in
several issues like flickering, black lines/flashes and so on.

[How]
These modes are basically high pixel clock ones, hence
limit the same to be advertised to avoid bad user experiences

Signed-off-by: Shirish S mailto:shiris...@amd.com>>
Suggested-by: Harry Wentland 
mailto:harry.wentl...@amd.com>>
---
 .../gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c| 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
index 1b2fe0d..1b8fe99 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
@@ -1121,6 +1121,16 @@ bool dce110_timing_generator_validate_timing(
 if (!timing)
 return false;

+   /* Limit all modes that have a high pixel clock
+* which seems to be problematic on dce110
+* These include: 4k@60Hz, 1080p@144Hz,1440p@120Hz
+* based on the below formula:
+* refresh rate = pixel clock / (htotal * vtotal)
+*/
+   if (timing->pix_clk_khz > 30)
+   return false;
+
+
 hsync_offset = timing->h_border_right + timing->h_front_porch;
 h_sync_start = timing->h_addressable + hsync_offset;

--
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

[PATCH] drm/amd/display: Disable 4k@60 on DP as well for DCE11

[S, Shirish]

This patch extends the below patch to apply DP signal type, for exactly
the same reasons it was disabled for HDMI.

"1a0e348 drm/amd/display: Disable 4k 60 HDMI on DCE11"

Signed-off-by: Shirish S 
---
 drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c 
b/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c
index 3e18ea8..d578828 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c
@@ -662,6 +662,10 @@ bool dce110_link_encoder_validate_dp_output(
const struct dce110_link_encoder *enc110,
const struct dc_crtc_timing *crtc_timing)
 {
+   if (crtc_timing->pix_clk_khz >
+   enc110->base.features.max_hdmi_pixel_clock)
+   return false;
+
if (crtc_timing->pixel_encoding == PIXEL_ENCODING_YCBCR420)
return false;
 
-- 
2.7.4

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

[PATCH] drm/amd/amdgpu: Test for PCI root bus before assuming bus->self

[Yang Xiao]

From: Young_X 

If we assign an amd device to a virtual mechine, we can no longer
assume a fixed hardware topology, like the GPU having a parent device.
This patch simply adds pci_is_root_bus() tests to avoid passing a
NULL pointer to PCI access functions.

See commit e79d5c0870ee ("drm/amdgpu: check before checking pci bridge
registers") and commit 0bd252de78d4 ("radeon: Test for PCI root bus
before assuming bus->self") for details.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
index f41f5f5..8b51fa8 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik.c
@@ -1622,7 +1622,8 @@ static void cik_program_aspm(struct amdgpu_device *adev)
if (orig != data)
WREG32_PCIE(ixPCIE_LC_LINK_WIDTH_CNTL, data);
 
-   if (!disable_clkreq) {
+   if (!disable_clkreq &&
+   !pci_is_root_bus(adev->pdev->bus)) {
struct pci_dev *root = adev->pdev->bus->self;
u32 lnkcap;
 
-- 
2.7.4

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