[PATCH] drm/amdgpu/powerplay: Apply avfs cks-off voltages on Polaris30
Instead of EVV cks-off voltages, avfs cks-off voltages can avoid the overshoot voltages when switching sclk. Signed-off-by: Kenneth Feng --- drivers/gpu/drm/amd/powerplay/inc/smu7_ppsmc.h | 2 ++ drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c | 9 + 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu7_ppsmc.h b/drivers/gpu/drm/amd/powerplay/inc/smu7_ppsmc.h index 62f36ba..c1a99df 100644 --- a/drivers/gpu/drm/amd/powerplay/inc/smu7_ppsmc.h +++ b/drivers/gpu/drm/amd/powerplay/inc/smu7_ppsmc.h @@ -386,6 +386,8 @@ typedef uint16_t PPSMC_Result; #define PPSMC_MSG_AgmResetPsm ((uint16_t) 0x403) #define PPSMC_MSG_ReadVftCell ((uint16_t) 0x404) +#define PPSMC_MSG_ApplyAvfsCksOffVoltage ((uint16_t) 0x415) + #define PPSMC_MSG_GFX_CU_PG_ENABLE((uint16_t) 0x280) #define PPSMC_MSG_GFX_CU_PG_DISABLE ((uint16_t) 0x281) #define PPSMC_MSG_GetCurrPkgPwr ((uint16_t) 0x282) diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c index 94898b2..6012dbf 100644 --- a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c +++ b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c @@ -1988,6 +1988,7 @@ static int polaris10_program_mem_timing_parameters(struct pp_hwmgr *hwmgr) int polaris10_thermal_avfs_enable(struct pp_hwmgr *hwmgr) { struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend); + struct amdgpu_device *adev = (struct amdgpu_device *)hwmgr->adev; if (!hwmgr->avfs_supported) return 0; @@ -1997,6 +1998,14 @@ int polaris10_thermal_avfs_enable(struct pp_hwmgr *hwmgr) smum_send_msg_to_smc(hwmgr, PPSMC_MSG_EnableAvfs); + /* P30 case for applying Avfs cks-off voltages to avoid the overshoot +* when switching to the highest sclk frequency +*/ + if ((adev->pdev->device == 0x67df) && + ((adev->pdev->revision == 0xe1) || + (adev->pdev->revision == 0xf7))) + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ApplyAvfsCksOffVoltage); + return 0; } -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/powerplay: rv dal-pplib interface refactor powerplay part
Hi Hersen, Can we change "pr_info" to "pr_debug"? I am afraid there may be a bunch of "was not implemented" in kernel message on old asics. Except that, Patch is Reviewed-by: Rex Zhu Best Regards Rex From: Wu, Hersen Sent: Thursday, December 6, 2018 2:03 AM To: amd-gfx@lists.freedesktop.org; Zhu, Rex; Deucher, Alexander Subject: RE: [PATCH] drm/amd/powerplay: rv dal-pplib interface refactor powerplay part Hello, Alex, Rex, Would you please help review the change? Thanks, Hersen [WHY] clarify dal input parameters to pplib interface, remove un-used parameters. dal knows exactly which parameters needed and their effects at pplib and smu sides. current dal sequence for dcn1_update_clock to pplib: 1.smu10_display_clock_voltage_request for dcefclk 2.smu10_display_clock_voltage_request for fclk 3.phm_store_dal_configuration_data { set_min_deep_sleep_dcfclk set_active_display_count store_cc6_data --- this data never be referenced new sequence will be: 1. set_display_count --- need add new pplib interface 2. set_min_deep_sleep_dcfclk -- new pplib interface 3. set_hard_min_dcfclk_by_freq 4. set_hard_min_fclk_by_freq after this code refactor, smu10_display_clock_voltage_request, phm_store_dal_configuration_data will not be needed for rv. [HOW] step 1: add new functions at pplib interface step 2: add new functions at amdgpu dm and dc Signed-off-by: hersen wu --- drivers/gpu/drm/amd/include/kgd_pp_interface.h | 4 ++ drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 76 ++ .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c | 45 - drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 36 +- .../gpu/drm/amd/powerplay/inc/hardwaremanager.h| 3 + drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 4 +- 6 files changed, 162 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h index 980e696..1479ea1 100644 --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h @@ -276,6 +276,10 @@ struct amd_pm_funcs { struct amd_pp_simple_clock_info *clocks); int (*notify_smu_enable_pwe)(void *handle); int (*enable_mgpu_fan_boost)(void *handle); + int (*set_active_display_count)(void *handle, uint32_t count); + int (*set_hard_min_dcefclk_by_freq)(void *handle, uint32_t clock); + int (*set_hard_min_fclk_by_freq)(void *handle, uint32_t clock); + int (*set_min_deep_sleep_dcefclk)(void *handle, uint32_t clock); }; #endif diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c index d6aa1d4..53dde16 100644 --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c @@ -1332,6 +1332,78 @@ static int pp_enable_mgpu_fan_boost(void *handle) return 0; } +static int pp_set_min_deep_sleep_dcefclk(void *handle, uint32_t clock) +{ + struct pp_hwmgr *hwmgr = handle; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + if (hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk == NULL) { + pr_info("%s was not implemented.\n", __func__); + return -EINVAL;; + } + + mutex_lock(&hwmgr->smu_lock); + hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk(hwmgr, clock); + mutex_unlock(&hwmgr->smu_lock); + + return 0; +} + +static int pp_set_hard_min_dcefclk_by_freq(void *handle, uint32_t +clock) { + struct pp_hwmgr *hwmgr = handle; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + if (hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq == NULL) { + pr_info("%s was not implemented.\n", __func__); + return -EINVAL;; + } + + mutex_lock(&hwmgr->smu_lock); + hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq(hwmgr, clock); + mutex_unlock(&hwmgr->smu_lock); + + return 0; +} + +static int pp_set_hard_min_fclk_by_freq(void *handle, uint32_t clock) { + struct pp_hwmgr *hwmgr = handle; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + if (hwmgr->hwmgr_func->set_hard_min_fclk_by_freq == NULL) { + pr_info("%s was not implemented.\n", __func__); + return -EINVAL;; + } + + mutex_lock(&hwmgr->smu_lock); + hwmgr->hwmgr_func->set_hard_min_fclk_by_freq(hwmgr, clock); + mutex_unlock(&hwmgr->smu_lock); + + return 0; +} + +static int pp_set_active_display_count(void *handle, uint32_t count) { + struct pp_hwmgr *hwmgr = handle; + int ret = 0; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + mutex_lock(&hwmgr->smu_lock); + ret = phm_set_active_display_count(hwmgr, count); + mutex_unlock(&hwmgr->smu_lock); + + return ret; +} + stati
Re: [PATCH] drm/amdgpu: both support PCO FP5/AM4 rlc fw
Reviewed-by: Alex Deucher From: amd-gfx on behalf of Aaron Liu Sent: Wednesday, December 5, 2018 8:26:52 PM To: amd-gfx@lists.freedesktop.org Cc: Liu, Aaron Subject: [PATCH] drm/amdgpu: both support PCO FP5/AM4 rlc fw For Picasso && AM4 SOCKET board, we use picasso_rlc_am4.bin For Picasso && FP5 SOCKET board, we use picasso_rlc.bin Judgment method: PCO AM4: revision >= 0xC8 && revision <= 0xCF or revision >= 0xD8 && revision <= 0xDF otherwise is PCO FP5 Change-Id: I359f0a3d1bc7d4d49c871cb3fb82797c7b91b259 Signed-off-by: Aaron Liu --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 94740ea..7556716 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -86,6 +86,7 @@ MODULE_FIRMWARE("amdgpu/picasso_me.bin"); MODULE_FIRMWARE("amdgpu/picasso_mec.bin"); MODULE_FIRMWARE("amdgpu/picasso_mec2.bin"); MODULE_FIRMWARE("amdgpu/picasso_rlc.bin"); +MODULE_FIRMWARE("amdgpu/picasso_rlc_am4.bin"); MODULE_FIRMWARE("amdgpu/raven2_ce.bin"); MODULE_FIRMWARE("amdgpu/raven2_pfp.bin"); @@ -645,7 +646,20 @@ static int gfx_v9_0_init_microcode(struct amdgpu_device *adev) adev->gfx.ce_fw_version = le32_to_cpu(cp_hdr->header.ucode_version); adev->gfx.ce_feature_version = le32_to_cpu(cp_hdr->ucode_feature_version); - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc.bin", chip_name); + /* +* For Picasso && AM4 SOCKET board, we use picasso_rlc_am4.bin +* instead of picasso_rlc.bin. +* Judgment method: +* PCO AM4: revision >= 0xC8 && revision <= 0xCF +* or revision >= 0xD8 && revision <= 0xDF +* otherwise is PCO FP5 +*/ + if (!strcmp(chip_name, "picasso") && + (((adev->pdev->revision >= 0xC8) && (adev->pdev->revision <= 0xCF)) || + ((adev->pdev->revision >= 0xD8) && (adev->pdev->revision <= 0xDF + snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc_am4.bin", chip_name); + else + snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc.bin", chip_name); err = request_firmware(&adev->gfx.rlc_fw, fw_name, adev->dev); if (err) goto out; -- 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/amdgpu: Skip ring soft recovery when fence was NULL
amdgpu_ring_soft_recovery would have Call-Trace, when s_fence->parent was NULL inside amdgpu_job_timedout. Check fence first, as drm_sched_hw_job_reset did. Change-Id: Ibb062e36feb4e2522a59641fe0d2d76b9773cda7 Signed-off-by: Wentao Lou --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index 5b75bdc..335a0ed 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -397,7 +397,7 @@ bool amdgpu_ring_soft_recovery(struct amdgpu_ring *ring, unsigned int vmid, { ktime_t deadline = ktime_add_us(ktime_get(), 1); - if (!ring->funcs->soft_recovery) + if (!ring->funcs->soft_recovery || !fence) return false; atomic_inc(&ring->adev->gpu_reset_counter); -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: both support PCO FP5/AM4 rlc fw
> -Original Message- > From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf > Of Aaron Liu > Sent: Thursday, December 06, 2018 9:27 AM > To: amd-gfx@lists.freedesktop.org > Cc: Liu, Aaron > Subject: [PATCH] drm/amdgpu: both support PCO FP5/AM4 rlc fw > > For Picasso && AM4 SOCKET board, we use picasso_rlc_am4.bin For Picasso > && FP5 SOCKET board, we use picasso_rlc.bin > > Judgment method: > PCO AM4: revision >= 0xC8 && revision <= 0xCF > or revision >= 0xD8 && revision <= 0xDF otherwise is PCO FP5 > > Change-Id: I359f0a3d1bc7d4d49c871cb3fb82797c7b91b259 > Signed-off-by: Aaron Liu Reviewed-by: Huang Rui > --- > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 +++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > index 94740ea..7556716 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > @@ -86,6 +86,7 @@ MODULE_FIRMWARE("amdgpu/picasso_me.bin"); > MODULE_FIRMWARE("amdgpu/picasso_mec.bin"); > MODULE_FIRMWARE("amdgpu/picasso_mec2.bin"); > MODULE_FIRMWARE("amdgpu/picasso_rlc.bin"); > +MODULE_FIRMWARE("amdgpu/picasso_rlc_am4.bin"); > > MODULE_FIRMWARE("amdgpu/raven2_ce.bin"); > MODULE_FIRMWARE("amdgpu/raven2_pfp.bin"); > @@ -645,7 +646,20 @@ static int gfx_v9_0_init_microcode(struct > amdgpu_device *adev) > adev->gfx.ce_fw_version = le32_to_cpu(cp_hdr- > >header.ucode_version); > adev->gfx.ce_feature_version = le32_to_cpu(cp_hdr- > >ucode_feature_version); > > - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc.bin", > chip_name); > + /* > + * For Picasso && AM4 SOCKET board, we use picasso_rlc_am4.bin > + * instead of picasso_rlc.bin. > + * Judgment method: > + * PCO AM4: revision >= 0xC8 && revision <= 0xCF > + * or revision >= 0xD8 && revision <= 0xDF > + * otherwise is PCO FP5 > + */ > + if (!strcmp(chip_name, "picasso") && > + (((adev->pdev->revision >= 0xC8) && (adev->pdev->revision > <= 0xCF)) || > + ((adev->pdev->revision >= 0xD8) && (adev->pdev->revision > <= 0xDF > + snprintf(fw_name, sizeof(fw_name), > "amdgpu/%s_rlc_am4.bin", chip_name); > + else > + snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc.bin", > chip_name); > err = request_firmware(&adev->gfx.rlc_fw, fw_name, adev->dev); > if (err) > goto out; > -- > 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/amdgpu: both support PCO FP5/AM4 rlc fw
For Picasso && AM4 SOCKET board, we use picasso_rlc_am4.bin For Picasso && FP5 SOCKET board, we use picasso_rlc.bin Judgment method: PCO AM4: revision >= 0xC8 && revision <= 0xCF or revision >= 0xD8 && revision <= 0xDF otherwise is PCO FP5 Change-Id: I359f0a3d1bc7d4d49c871cb3fb82797c7b91b259 Signed-off-by: Aaron Liu --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 94740ea..7556716 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -86,6 +86,7 @@ MODULE_FIRMWARE("amdgpu/picasso_me.bin"); MODULE_FIRMWARE("amdgpu/picasso_mec.bin"); MODULE_FIRMWARE("amdgpu/picasso_mec2.bin"); MODULE_FIRMWARE("amdgpu/picasso_rlc.bin"); +MODULE_FIRMWARE("amdgpu/picasso_rlc_am4.bin"); MODULE_FIRMWARE("amdgpu/raven2_ce.bin"); MODULE_FIRMWARE("amdgpu/raven2_pfp.bin"); @@ -645,7 +646,20 @@ static int gfx_v9_0_init_microcode(struct amdgpu_device *adev) adev->gfx.ce_fw_version = le32_to_cpu(cp_hdr->header.ucode_version); adev->gfx.ce_feature_version = le32_to_cpu(cp_hdr->ucode_feature_version); - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc.bin", chip_name); + /* +* For Picasso && AM4 SOCKET board, we use picasso_rlc_am4.bin +* instead of picasso_rlc.bin. +* Judgment method: +* PCO AM4: revision >= 0xC8 && revision <= 0xCF +* or revision >= 0xD8 && revision <= 0xDF +* otherwise is PCO FP5 +*/ + if (!strcmp(chip_name, "picasso") && + (((adev->pdev->revision >= 0xC8) && (adev->pdev->revision <= 0xCF)) || + ((adev->pdev->revision >= 0xD8) && (adev->pdev->revision <= 0xDF + snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc_am4.bin", chip_name); + else + snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc.bin", chip_name); err = request_firmware(&adev->gfx.rlc_fw, fw_name, adev->dev); if (err) goto out; -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/10] drm/amdgpu: send IVs to the KFD only after processing them v3
Patches 1-3 are Reviewed-by: Felix Kuehling I applied all 10 patches and tested them with kfdtest on Fiji and Vega10. It seems to not break anything obvious. I think I found a problem in patch 9 and have a question about patch 8 regarding the context in which interrupt processing functions would run. I sent separate emails for both. Regards, Felix On 2018-12-05 4:15 a.m., Christian König wrote: > This allows us to filter out VM faults in the GMC code. > > v2: don't filter out all faults > v3: fix copy&paste typo, send all IV to the KFD, don't change message level > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 38 +++-- > 1 file changed, 17 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > index 6b6524f04ce0..79b6f456f2c5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > @@ -149,9 +149,6 @@ static void amdgpu_irq_callback(struct amdgpu_device > *adev, > if (!amdgpu_ih_prescreen_iv(adev)) > return; > > - /* Before dispatching irq to IP blocks, send it to amdkfd */ > - amdgpu_amdkfd_interrupt(adev, (const void *) &ih->ring[ring_index]); > - > entry.iv_entry = (const uint32_t *)&ih->ring[ring_index]; > amdgpu_ih_decode_iv(adev, &entry); > > @@ -371,39 +368,38 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev, > unsigned client_id = entry->client_id; > unsigned src_id = entry->src_id; > struct amdgpu_irq_src *src; > + bool handled = false; > int r; > > trace_amdgpu_iv(entry); > > if (client_id >= AMDGPU_IRQ_CLIENTID_MAX) { > DRM_DEBUG("Invalid client_id in IV: %d\n", client_id); > - return; > - } > > - if (src_id >= AMDGPU_MAX_IRQ_SRC_ID) { > + } else if (src_id >= AMDGPU_MAX_IRQ_SRC_ID) { > DRM_DEBUG("Invalid src_id in IV: %d\n", src_id); > - return; > - } > > - if (adev->irq.virq[src_id]) { > + } else if (adev->irq.virq[src_id]) { > generic_handle_irq(irq_find_mapping(adev->irq.domain, src_id)); > - } else { > - if (!adev->irq.client[client_id].sources) { > - DRM_DEBUG("Unregistered interrupt client_id: %d src_id: > %d\n", > - client_id, src_id); > - return; > - } > > - src = adev->irq.client[client_id].sources[src_id]; > - if (!src) { > - DRM_DEBUG("Unhandled interrupt src_id: %d\n", src_id); > - return; > - } > + } else if (!adev->irq.client[client_id].sources) { > + DRM_DEBUG("Unregistered interrupt client_id: %d src_id: %d\n", > + client_id, src_id); > > + } else if ((src = adev->irq.client[client_id].sources[src_id])) { > r = src->funcs->process(adev, src, entry); > - if (r) > + if (r < 0) > DRM_ERROR("error processing interrupt (%d)\n", r); > + else if (r) > + handled = true; > + > + } else { > + DRM_DEBUG("Unhandled interrupt src_id: %d\n", src_id); > } > + > + /* Send it to amdkfd as well if it isn't already handled */ > + if (!handled) > + amdgpu_amdkfd_interrupt(adev, entry->iv_entry); > } > > /** ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/10] drm/amdgpu: add support for self irq on Vega10
On 2018-12-05 4:15 a.m., Christian König wrote: > This finally enables processing of ring 1 & 2. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 68 -- > 1 file changed, 63 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c > b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c > index 22638192d7dd..4a753e40a837 100644 > --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c > +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c > @@ -272,7 +272,7 @@ static void vega10_ih_irq_disable(struct amdgpu_device > *adev) > static u32 vega10_ih_get_wptr(struct amdgpu_device *adev, > struct amdgpu_ih_ring *ih) > { > - u32 wptr, tmp; > + u32 wptr, reg, tmp; > > wptr = le32_to_cpu(*ih->wptr_cpu); > > @@ -288,9 +288,18 @@ static u32 vega10_ih_get_wptr(struct amdgpu_device *adev, >wptr, ih->rptr, tmp); > ih->rptr = tmp; > > - tmp = RREG32_NO_KIQ(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL)); > + if (ih == &adev->irq.ih) > + reg = SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL); > + else if (ih == &adev->irq.ih1) > + reg = SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL); > + else if (ih == &adev->irq.ih2) > + reg = SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL); All three look the same to me. Did you mean to use mmIH_RB_CNTL_RING1/2 for ih1 and ih2 respectively? Regards, Felix > + else > + BUG(); > + > + tmp = RREG32_NO_KIQ(reg); > tmp = REG_SET_FIELD(tmp, IH_RB_CNTL, WPTR_OVERFLOW_CLEAR, 1); > - WREG32_NO_KIQ(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL), tmp); > + WREG32_NO_KIQ(reg, tmp); > } > return (wptr & ih->ptr_mask); > } > @@ -352,9 +361,52 @@ static void vega10_ih_set_rptr(struct amdgpu_device > *adev, > /* XXX check if swapping is necessary on BE */ > *ih->rptr_cpu = ih->rptr; > WDOORBELL32(ih->doorbell_index, ih->rptr); > - } else { > + } else if (ih == &adev->irq.ih) { > WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR, ih->rptr); > + } else if (ih == &adev->irq.ih1) { > + WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING1, ih->rptr); > + } else if (ih == &adev->irq.ih2) { > + WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING2, ih->rptr); > + } > +} > + > +/** > + * vega10_ih_self_irq - dispatch work for ring 1 and 2 > + * > + * @adev: amdgpu_device pointer > + * @source: irq source > + * @entry: IV with WPTR update > + * > + * Update the WPTR from the IV and schedule work to handle the entries. > + */ > +static int vega10_ih_self_irq(struct amdgpu_device *adev, > + struct amdgpu_irq_src *source, > + struct amdgpu_iv_entry *entry) > +{ > + uint32_t wptr = cpu_to_le32(entry->src_data[0]); > + > + switch (entry->ring_id) { > + case 1: > + *adev->irq.ih1.wptr_cpu = wptr; > + schedule_work(&adev->irq.ih1_work); > + break; > + case 2: > + *adev->irq.ih2.wptr_cpu = wptr; > + schedule_work(&adev->irq.ih2_work); > + break; > + default: break; > } > + return 0; > +} > + > +static const struct amdgpu_irq_src_funcs vega10_ih_self_irq_funcs = { > + .process = vega10_ih_self_irq, > +}; > + > +static void vega10_ih_set_self_irq_funcs(struct amdgpu_device *adev) > +{ > + adev->irq.self_irq.num_types = 0; > + adev->irq.self_irq.funcs = &vega10_ih_self_irq_funcs; > } > > static int vega10_ih_early_init(void *handle) > @@ -362,13 +414,19 @@ static int vega10_ih_early_init(void *handle) > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > vega10_ih_set_interrupt_funcs(adev); > + vega10_ih_set_self_irq_funcs(adev); > return 0; > } > > static int vega10_ih_sw_init(void *handle) > { > - int r; > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > + int r; > + > + r = amdgpu_irq_add_id(adev, SOC15_IH_CLIENTID_IH, 0, > + &adev->irq.self_irq); > + if (r) > + return r; > > r = amdgpu_ih_ring_init(adev, &adev->irq.ih, 256 * 1024, true); > if (r) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 08/10] drm/amdgpu: add support for processing IH ring 1 & 2
Depending on the interrupt ring, the IRQ dispatch and processing functions will run in interrupt context or in a worker thread. Is there a way for the processing functions to find out which context it's running in? That may influence decisions whether to process interrupts in the same thread or schedule another worker. Regards, Felix On 2018-12-05 4:15 a.m., Christian König wrote: > Previously we only added the ring buffer memory, now add the handling as > well. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 33 + > drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h | 4 ++- > 2 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > index b8e543e23166..8bfb3dab46f7 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > @@ -176,6 +176,36 @@ irqreturn_t amdgpu_irq_handler(int irq, void *arg) > return ret; > } > > +/** > + * amdgpu_irq_handle_ih1 - kick of processing for IH1 > + * > + * @work: work structure in struct amdgpu_irq > + * > + * Kick of processing IH ring 1. > + */ > +static void amdgpu_irq_handle_ih1(struct work_struct *work) > +{ > + struct amdgpu_device *adev = container_of(work, struct amdgpu_device, > + irq.ih1_work); > + > + amdgpu_ih_process(adev, &adev->irq.ih1, amdgpu_irq_callback); > +} > + > +/** > + * amdgpu_irq_handle_ih2 - kick of processing for IH2 > + * > + * @work: work structure in struct amdgpu_irq > + * > + * Kick of processing IH ring 2. > + */ > +static void amdgpu_irq_handle_ih2(struct work_struct *work) > +{ > + struct amdgpu_device *adev = container_of(work, struct amdgpu_device, > + irq.ih2_work); > + > + amdgpu_ih_process(adev, &adev->irq.ih2, amdgpu_irq_callback); > +} > + > /** > * amdgpu_msi_ok - check whether MSI functionality is enabled > * > @@ -240,6 +270,9 @@ int amdgpu_irq_init(struct amdgpu_device *adev) > amdgpu_hotplug_work_func); > } > > + INIT_WORK(&adev->irq.ih1_work, amdgpu_irq_handle_ih1); > + INIT_WORK(&adev->irq.ih2_work, amdgpu_irq_handle_ih2); > + > adev->irq.installed = true; > r = drm_irq_install(adev->ddev, adev->ddev->pdev->irq); > if (r) { > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h > index 7e06fa64321a..c27decfda494 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h > @@ -89,7 +89,9 @@ struct amdgpu_irq { > > /* interrupt rings */ > struct amdgpu_ih_ring ih, ih1, ih2; > - const struct amdgpu_ih_funcs*ih_funcs; > + const struct amdgpu_ih_funcs*ih_funcs; > + struct work_struct ih1_work, ih2_work; > + struct amdgpu_irq_src self_irq; > > /* gen irq stuff */ > struct irq_domain *domain; /* GPU irq controller domain */ ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu/acpi: NULL check before some freeing functions is not needed
Applied. thanks! Alex On Wed, Dec 5, 2018 at 2:02 PM Lyude Paul wrote: > > Reviewed-by: Lyude Paul > > Thanks! > > On Wed, 2018-12-05 at 15:43 +0800, Wen Yang wrote: > > kfree(NULL) is safe, so removes NULL check before freeing the mem. > > This patch also fix the ifnullfree.cocci warnings. > > > > Signed-off-by: Wen Yang > > CC: Alex Deucher > > CC: christian.koe...@amd.com > > CC: "David (ChunMing) Zhou" > > CC: David Airlie (maintainer:DRM DRIVERS) > > CC: Lyude Paul > > CC: Rex Zhu > > CC: Jim Qu > > CC: amd-gfx@lists.freedesktop.org > > CC: dri-de...@lists.freedesktop.org > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > > index 7f0afc526419..996bfce149f2 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > > @@ -816,6 +816,5 @@ int amdgpu_acpi_init(struct amdgpu_device *adev) > > void amdgpu_acpi_fini(struct amdgpu_device *adev) > > { > > unregister_acpi_notifier(&adev->acpi_nb); > > - if (adev->atif) > > - kfree(adev->atif); > > + kfree(adev->atif); > > } > -- > Cheers, > Lyude Paul > > ___ > 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: Add fast path for cursor plane updates
On 2018-12-05 3:44 p.m., Grodzovsky, Andrey wrote: > > > On 12/05/2018 03:42 PM, Kazlauskas, Nicholas wrote: >> On 2018-12-05 3:26 p.m., Grodzovsky, Andrey wrote: >>> >>> On 12/05/2018 02:59 PM, Nicholas Kazlauskas wrote: [Why] Legacy cursor plane updates from drm helpers go through the full atomic codepath. A high volume of cursor updates through this slow code path can cause subsequent page-flips to skip vblank intervals since each individual update is slow. This problem is particularly noticeable for the compton compositor. [How] A fast path for cursor plane updates is added by using DRM asynchronous commit support provided by async_check and async_update. These don't do a full state/flip_done dependency stall and they don't block other commit work. However, DC still expects itself to be single-threaded for anything that can issue register writes. Screen corruption or hangs can occur if write sequences overlap. Every call that potentially perform register writes needs to be guarded for asynchronous updates to work. The dc_lock mutex was added for this. >>> As far as page flip related register writes concerned this I think this >>> was never an issue - we always >>> supported fully concurrent page flips on different CRTCs meaning writing >>> surface address concurrently >>> on different pipes. So Are you sure you can't do cursor update >>> concurrently against at least page flips ? >>> I am not sure about full updates. >>> >>> Andrey >> I think this was true before we started locking the pipes around cursor >> updates (and probably only for dce). The problem that occur here is >> writing to the lock register from multiple threads at the same time. >> >> In general I think the "contract" between dm/dc now is that anything >> from dc that does register write sequences can't be called from multiple >> threads. >> >> Nicholas Kazlauskas > > Ok, do note that you also serialize all the page flips now which looks > unneeded - maybe consider r/w lock to at least avoid that. > > Andrey Yeah, they definitely shouldn't be happening at the same time as the cursor calls. I'd need to look into whether it's okay now to do concurrent writes from the stream update functions though, but I'd imagine it's not something we'd want to be doing. A r/w lock would work if we could though. Nicholas Kazlauskas > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106175 Cc: Leo Li Cc: Harry Wentland Signed-off-by: Nicholas Kazlauskas --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 67 ++- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 8 +++ 2 files changed, 73 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 23d61570df17..4df14a50a8ac 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -57,6 +57,7 @@ #include #include +#include #include #include #include @@ -133,6 +134,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state); static int amdgpu_dm_atomic_check(struct drm_device *dev, struct drm_atomic_state *state); +static void handle_cursor_update(struct drm_plane *plane, + struct drm_plane_state *old_plane_state); @@ -402,6 +405,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) /* Zero all the fields */ memset(&init_data, 0, sizeof(init_data)); + mutex_init(&adev->dm.dc_lock); + if(amdgpu_dm_irq_init(adev)) { DRM_ERROR("amdgpu: failed to initialize DM IRQ support.\n"); goto error; @@ -516,6 +521,9 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev) /* DC Destroy TODO: Replace destroy DAL */ if (adev->dm.dc) dc_destroy(&adev->dm.dc); + + mutex_destroy(&adev->dm.dc_lock); + return; } @@ -3615,10 +3623,43 @@ static int dm_plane_atomic_check(struct drm_plane *plane, return -EINVAL; } +static int dm_plane_atomic_async_check(struct drm_plane *plane, + struct drm_plane_state *new_plane_state) +{ + /* Only support async updates on cursor planes. */ + if (plane->type != DRM_PLANE_TYPE_CURSOR) + return -EINVAL; + + return 0; +} + +static void dm_plane_atomic_async_update(struct drm_plane *plane, +
Re: [PATCH] drm/amd/display: Add fast path for cursor plane updates
On 12/05/2018 03:42 PM, Kazlauskas, Nicholas wrote: > On 2018-12-05 3:26 p.m., Grodzovsky, Andrey wrote: >> >> On 12/05/2018 02:59 PM, Nicholas Kazlauskas wrote: >>> [Why] >>> Legacy cursor plane updates from drm helpers go through the full >>> atomic codepath. A high volume of cursor updates through this slow >>> code path can cause subsequent page-flips to skip vblank intervals >>> since each individual update is slow. >>> >>> This problem is particularly noticeable for the compton compositor. >>> >>> [How] >>> A fast path for cursor plane updates is added by using DRM asynchronous >>> commit support provided by async_check and async_update. These don't do >>> a full state/flip_done dependency stall and they don't block other >>> commit work. >>> >>> However, DC still expects itself to be single-threaded for anything >>> that can issue register writes. Screen corruption or hangs can occur >>> if write sequences overlap. Every call that potentially perform >>> register writes needs to be guarded for asynchronous updates to work. >>> The dc_lock mutex was added for this. >> As far as page flip related register writes concerned this I think this >> was never an issue - we always >> supported fully concurrent page flips on different CRTCs meaning writing >> surface address concurrently >> on different pipes. So Are you sure you can't do cursor update >> concurrently against at least page flips ? >> I am not sure about full updates. >> >> Andrey > I think this was true before we started locking the pipes around cursor > updates (and probably only for dce). The problem that occur here is > writing to the lock register from multiple threads at the same time. > > In general I think the "contract" between dm/dc now is that anything > from dc that does register write sequences can't be called from multiple > threads. > > Nicholas Kazlauskas Ok, do note that you also serialize all the page flips now which looks unneeded - maybe consider r/w lock to at least avoid that. Andrey > >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106175 >>> >>> Cc: Leo Li >>> Cc: Harry Wentland >>> Signed-off-by: Nicholas Kazlauskas >>> --- >>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 67 ++- >>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 8 +++ >>> 2 files changed, 73 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 23d61570df17..4df14a50a8ac 100644 >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> @@ -57,6 +57,7 @@ >>> >>> #include >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -133,6 +134,8 @@ static void amdgpu_dm_atomic_commit_tail(struct >>> drm_atomic_state *state); >>> static int amdgpu_dm_atomic_check(struct drm_device *dev, >>> struct drm_atomic_state *state); >>> >>> +static void handle_cursor_update(struct drm_plane *plane, >>> +struct drm_plane_state *old_plane_state); >>> >>> >>> >>> @@ -402,6 +405,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) >>> /* Zero all the fields */ >>> memset(&init_data, 0, sizeof(init_data)); >>> >>> + mutex_init(&adev->dm.dc_lock); >>> + >>> if(amdgpu_dm_irq_init(adev)) { >>> DRM_ERROR("amdgpu: failed to initialize DM IRQ >>> support.\n"); >>> goto error; >>> @@ -516,6 +521,9 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev) >>> /* DC Destroy TODO: Replace destroy DAL */ >>> if (adev->dm.dc) >>> dc_destroy(&adev->dm.dc); >>> + >>> + mutex_destroy(&adev->dm.dc_lock); >>> + >>> return; >>> } >>> >>> @@ -3615,10 +3623,43 @@ static int dm_plane_atomic_check(struct drm_plane >>> *plane, >>> return -EINVAL; >>> } >>> >>> +static int dm_plane_atomic_async_check(struct drm_plane *plane, >>> + struct drm_plane_state *new_plane_state) >>> +{ >>> + /* Only support async updates on cursor planes. */ >>> + if (plane->type != DRM_PLANE_TYPE_CURSOR) >>> + return -EINVAL; >>> + >>> + return 0; >>> +} >>> + >>> +static void dm_plane_atomic_async_update(struct drm_plane *plane, >>> +struct drm_plane_state *new_state) >>> +{ >>> + struct drm_plane_state *old_state = >>> + drm_atomic_get_old_plane_state(new_state->state, plane); >>> + >>> + if (plane->state->fb != new_state->fb) >>> + drm_atomic_set_fb_for_plane(plane->state, new_state->fb); >>> + >>> + plane->state->src_x = new_state->src_x; >>> + plane->state->src_y = new_state->src_y; >>> + plane->state->src_w = new_state->src_w; >>> + plane->state->src_h = new_sta
Re: [PATCH] drm/amd/display: Add fast path for cursor plane updates
On 2018-12-05 3:26 p.m., Grodzovsky, Andrey wrote: > > > On 12/05/2018 02:59 PM, Nicholas Kazlauskas wrote: >> [Why] >> Legacy cursor plane updates from drm helpers go through the full >> atomic codepath. A high volume of cursor updates through this slow >> code path can cause subsequent page-flips to skip vblank intervals >> since each individual update is slow. >> >> This problem is particularly noticeable for the compton compositor. >> >> [How] >> A fast path for cursor plane updates is added by using DRM asynchronous >> commit support provided by async_check and async_update. These don't do >> a full state/flip_done dependency stall and they don't block other >> commit work. >> >> However, DC still expects itself to be single-threaded for anything >> that can issue register writes. Screen corruption or hangs can occur >> if write sequences overlap. Every call that potentially perform >> register writes needs to be guarded for asynchronous updates to work. >> The dc_lock mutex was added for this. > > As far as page flip related register writes concerned this I think this > was never an issue - we always > supported fully concurrent page flips on different CRTCs meaning writing > surface address concurrently > on different pipes. So Are you sure you can't do cursor update > concurrently against at least page flips ? > I am not sure about full updates. > > Andrey I think this was true before we started locking the pipes around cursor updates (and probably only for dce). The problem that occur here is writing to the lock register from multiple threads at the same time. In general I think the "contract" between dm/dc now is that anything from dc that does register write sequences can't be called from multiple threads. Nicholas Kazlauskas > >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106175 >> >> Cc: Leo Li >> Cc: Harry Wentland >> Signed-off-by: Nicholas Kazlauskas >> --- >>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 67 ++- >>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 8 +++ >>2 files changed, 73 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 23d61570df17..4df14a50a8ac 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -57,6 +57,7 @@ >> >>#include >>#include >> +#include >>#include >>#include >>#include >> @@ -133,6 +134,8 @@ static void amdgpu_dm_atomic_commit_tail(struct >> drm_atomic_state *state); >>static int amdgpu_dm_atomic_check(struct drm_device *dev, >>struct drm_atomic_state *state); >> >> +static void handle_cursor_update(struct drm_plane *plane, >> + struct drm_plane_state *old_plane_state); >> >> >> >> @@ -402,6 +405,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) >> /* Zero all the fields */ >> memset(&init_data, 0, sizeof(init_data)); >> >> +mutex_init(&adev->dm.dc_lock); >> + >> if(amdgpu_dm_irq_init(adev)) { >> DRM_ERROR("amdgpu: failed to initialize DM IRQ support.\n"); >> goto error; >> @@ -516,6 +521,9 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev) >> /* DC Destroy TODO: Replace destroy DAL */ >> if (adev->dm.dc) >> dc_destroy(&adev->dm.dc); >> + >> +mutex_destroy(&adev->dm.dc_lock); >> + >> return; >>} >> >> @@ -3615,10 +3623,43 @@ static int dm_plane_atomic_check(struct drm_plane >> *plane, >> return -EINVAL; >>} >> >> +static int dm_plane_atomic_async_check(struct drm_plane *plane, >> + struct drm_plane_state *new_plane_state) >> +{ >> +/* Only support async updates on cursor planes. */ >> +if (plane->type != DRM_PLANE_TYPE_CURSOR) >> +return -EINVAL; >> + >> +return 0; >> +} >> + >> +static void dm_plane_atomic_async_update(struct drm_plane *plane, >> + struct drm_plane_state *new_state) >> +{ >> +struct drm_plane_state *old_state = >> +drm_atomic_get_old_plane_state(new_state->state, plane); >> + >> +if (plane->state->fb != new_state->fb) >> +drm_atomic_set_fb_for_plane(plane->state, new_state->fb); >> + >> +plane->state->src_x = new_state->src_x; >> +plane->state->src_y = new_state->src_y; >> +plane->state->src_w = new_state->src_w; >> +plane->state->src_h = new_state->src_h; >> +plane->state->crtc_x = new_state->crtc_x; >> +plane->state->crtc_y = new_state->crtc_y; >> +plane->state->crtc_w = new_state->crtc_w; >> +plane->state->crtc_h = new_state->crtc_h; >> + >> +handle_cursor_update(plane, old_state); >> +} >> + >>static const struct drm_plane_helper_funcs dm_plane_helper_funcs = { >> .prepare_fb = dm_plane_helper_p
Re: [PATCH] drm/amd/display: Add fast path for cursor plane updates
On 12/05/2018 02:59 PM, Nicholas Kazlauskas wrote: > [Why] > Legacy cursor plane updates from drm helpers go through the full > atomic codepath. A high volume of cursor updates through this slow > code path can cause subsequent page-flips to skip vblank intervals > since each individual update is slow. > > This problem is particularly noticeable for the compton compositor. > > [How] > A fast path for cursor plane updates is added by using DRM asynchronous > commit support provided by async_check and async_update. These don't do > a full state/flip_done dependency stall and they don't block other > commit work. > > However, DC still expects itself to be single-threaded for anything > that can issue register writes. Screen corruption or hangs can occur > if write sequences overlap. Every call that potentially perform > register writes needs to be guarded for asynchronous updates to work. > The dc_lock mutex was added for this. As far as page flip related register writes concerned this I think this was never an issue - we always supported fully concurrent page flips on different CRTCs meaning writing surface address concurrently on different pipes. So Are you sure you can't do cursor update concurrently against at least page flips ? I am not sure about full updates. Andrey > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106175 > > Cc: Leo Li > Cc: Harry Wentland > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 67 ++- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 8 +++ > 2 files changed, 73 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 23d61570df17..4df14a50a8ac 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -57,6 +57,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -133,6 +134,8 @@ static void amdgpu_dm_atomic_commit_tail(struct > drm_atomic_state *state); > static int amdgpu_dm_atomic_check(struct drm_device *dev, > struct drm_atomic_state *state); > > +static void handle_cursor_update(struct drm_plane *plane, > + struct drm_plane_state *old_plane_state); > > > > @@ -402,6 +405,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) > /* Zero all the fields */ > memset(&init_data, 0, sizeof(init_data)); > > + mutex_init(&adev->dm.dc_lock); > + > if(amdgpu_dm_irq_init(adev)) { > DRM_ERROR("amdgpu: failed to initialize DM IRQ support.\n"); > goto error; > @@ -516,6 +521,9 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev) > /* DC Destroy TODO: Replace destroy DAL */ > if (adev->dm.dc) > dc_destroy(&adev->dm.dc); > + > + mutex_destroy(&adev->dm.dc_lock); > + > return; > } > > @@ -3615,10 +3623,43 @@ static int dm_plane_atomic_check(struct drm_plane > *plane, > return -EINVAL; > } > > +static int dm_plane_atomic_async_check(struct drm_plane *plane, > +struct drm_plane_state *new_plane_state) > +{ > + /* Only support async updates on cursor planes. */ > + if (plane->type != DRM_PLANE_TYPE_CURSOR) > + return -EINVAL; > + > + return 0; > +} > + > +static void dm_plane_atomic_async_update(struct drm_plane *plane, > + struct drm_plane_state *new_state) > +{ > + struct drm_plane_state *old_state = > + drm_atomic_get_old_plane_state(new_state->state, plane); > + > + if (plane->state->fb != new_state->fb) > + drm_atomic_set_fb_for_plane(plane->state, new_state->fb); > + > + plane->state->src_x = new_state->src_x; > + plane->state->src_y = new_state->src_y; > + plane->state->src_w = new_state->src_w; > + plane->state->src_h = new_state->src_h; > + plane->state->crtc_x = new_state->crtc_x; > + plane->state->crtc_y = new_state->crtc_y; > + plane->state->crtc_w = new_state->crtc_w; > + plane->state->crtc_h = new_state->crtc_h; > + > + handle_cursor_update(plane, old_state); > +} > + > static const struct drm_plane_helper_funcs dm_plane_helper_funcs = { > .prepare_fb = dm_plane_helper_prepare_fb, > .cleanup_fb = dm_plane_helper_cleanup_fb, > .atomic_check = dm_plane_atomic_check, > + .atomic_async_check = dm_plane_atomic_async_check, > + .atomic_async_update = dm_plane_atomic_async_update > }; > > /* > @@ -4307,6 +4348,7 @@ static int get_cursor_position(struct drm_plane *plane, > struct drm_crtc *crtc, > static void handle_cursor_update(struct drm_plane *plane, >struct drm_plane_state *old_plane_state) > { > + struct amdgpu_device *adev = plane->dev->dev_private; >
[PATCH] drm/amd/display: Add fast path for cursor plane updates
[Why] Legacy cursor plane updates from drm helpers go through the full atomic codepath. A high volume of cursor updates through this slow code path can cause subsequent page-flips to skip vblank intervals since each individual update is slow. This problem is particularly noticeable for the compton compositor. [How] A fast path for cursor plane updates is added by using DRM asynchronous commit support provided by async_check and async_update. These don't do a full state/flip_done dependency stall and they don't block other commit work. However, DC still expects itself to be single-threaded for anything that can issue register writes. Screen corruption or hangs can occur if write sequences overlap. Every call that potentially perform register writes needs to be guarded for asynchronous updates to work. The dc_lock mutex was added for this. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106175 Cc: Leo Li Cc: Harry Wentland Signed-off-by: Nicholas Kazlauskas --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 67 ++- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 8 +++ 2 files changed, 73 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 23d61570df17..4df14a50a8ac 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -57,6 +57,7 @@ #include #include +#include #include #include #include @@ -133,6 +134,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state); static int amdgpu_dm_atomic_check(struct drm_device *dev, struct drm_atomic_state *state); +static void handle_cursor_update(struct drm_plane *plane, +struct drm_plane_state *old_plane_state); @@ -402,6 +405,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) /* Zero all the fields */ memset(&init_data, 0, sizeof(init_data)); + mutex_init(&adev->dm.dc_lock); + if(amdgpu_dm_irq_init(adev)) { DRM_ERROR("amdgpu: failed to initialize DM IRQ support.\n"); goto error; @@ -516,6 +521,9 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev) /* DC Destroy TODO: Replace destroy DAL */ if (adev->dm.dc) dc_destroy(&adev->dm.dc); + + mutex_destroy(&adev->dm.dc_lock); + return; } @@ -3615,10 +3623,43 @@ static int dm_plane_atomic_check(struct drm_plane *plane, return -EINVAL; } +static int dm_plane_atomic_async_check(struct drm_plane *plane, + struct drm_plane_state *new_plane_state) +{ + /* Only support async updates on cursor planes. */ + if (plane->type != DRM_PLANE_TYPE_CURSOR) + return -EINVAL; + + return 0; +} + +static void dm_plane_atomic_async_update(struct drm_plane *plane, +struct drm_plane_state *new_state) +{ + struct drm_plane_state *old_state = + drm_atomic_get_old_plane_state(new_state->state, plane); + + if (plane->state->fb != new_state->fb) + drm_atomic_set_fb_for_plane(plane->state, new_state->fb); + + plane->state->src_x = new_state->src_x; + plane->state->src_y = new_state->src_y; + plane->state->src_w = new_state->src_w; + plane->state->src_h = new_state->src_h; + plane->state->crtc_x = new_state->crtc_x; + plane->state->crtc_y = new_state->crtc_y; + plane->state->crtc_w = new_state->crtc_w; + plane->state->crtc_h = new_state->crtc_h; + + handle_cursor_update(plane, old_state); +} + static const struct drm_plane_helper_funcs dm_plane_helper_funcs = { .prepare_fb = dm_plane_helper_prepare_fb, .cleanup_fb = dm_plane_helper_cleanup_fb, .atomic_check = dm_plane_atomic_check, + .atomic_async_check = dm_plane_atomic_async_check, + .atomic_async_update = dm_plane_atomic_async_update }; /* @@ -4307,6 +4348,7 @@ static int get_cursor_position(struct drm_plane *plane, struct drm_crtc *crtc, static void handle_cursor_update(struct drm_plane *plane, struct drm_plane_state *old_plane_state) { + struct amdgpu_device *adev = plane->dev->dev_private; struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(plane->state->fb); struct drm_crtc *crtc = afb ? plane->state->crtc : old_plane_state->crtc; struct dm_crtc_state *crtc_state = crtc ? to_dm_crtc_state(crtc->state) : NULL; @@ -4331,9 +4373,12 @@ static void handle_cursor_update(struct drm_plane *plane, if (!position.enable) { /* turn off cursor */ - if (crtc_state && crtc_state->stream) + if (crtc_state && crtc_state->stream) { + mutex_lock(&adev->dm.dc_lock); dc_s
[pull] amdgpu, ttm drm-fixes-4.20
Hi Dave, Fixes for 4.20: - Fix banding regression on 6 bpc panels - Vega20 fix for six 4k displays - Fix LRU handling in ttm_buffer_object_transfer - Use proper MC firmware for newer polaris variants - Vega20 powerplay fixes - VCN suspend/resume fix for PCO - Misc other fixes The following changes since commit ebcdcef30333660d3314158bac362425ade3d28c: Merge tag 'drm-misc-fixes-2018-11-28-1' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2018-11-29 10:11:15 +1000) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-fixes-4.20 for you to fetch changes up to 0a9b89b2e2e7b6d90f81ddc47e489be1043e01b1: drm/amdgpu/vcn: Update vcn.cur_state during suspend (2018-12-05 14:12:02 -0500) Alex Deucher (3): drm/amdgpu: add VCN JPEG support amdgpu_ctx_num_entities drm/amdgpu/gmc8: update MC firmware for polaris drm/amdgpu/gmc8: always load MC firmware in the driver Christian König (2): drm/ttm: fix LRU handling in ttm_buffer_object_transfer drm/amdgpu: wait for IB test on first device open Evan Quan (3): drm/amd/powerplay: support new pptable upload on Vega20 drm/amd/powerplay: issue pre-display settings for display change event drm/amd/powerplay: support SoftMin/Max setting for some specific DPM James Zhu (1): drm/amdgpu/vcn: Update vcn.cur_state during suspend Junwei Zhang (1): drm/amdgpu: update mc firmware image for polaris12 variants Nicholas Kazlauskas (2): drm/amd/display: Fix unintialized max_bpc state values drm/amd/display: Fix overflow/truncation from strncpy. Roman Li (1): drm/amd/display: Fix 6x4K displays light-up on Vega20 (v2) tianci yin (1): drm/amd/powerplay: improve OD code robustness wentalou (1): drm/amdgpu: enlarge maximum waiting time of KIQ drivers/gpu/drm/amd/amdgpu/amdgpu.h| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 6 +-- drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 44 +- drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 3 +- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++-- .../amd/display/dc/dce110/dce110_hw_sequencer.c| 2 + .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c | 4 +- drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c| 3 ++ drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c | 2 - drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 12 +++-- drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 12 +++-- drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 54 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 4 +- 14 files changed, 104 insertions(+), 53 deletions(-) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu/acpi: NULL check before some freeing functions is not needed
Reviewed-by: Lyude Paul Thanks! On Wed, 2018-12-05 at 15:43 +0800, Wen Yang wrote: > kfree(NULL) is safe, so removes NULL check before freeing the mem. > This patch also fix the ifnullfree.cocci warnings. > > Signed-off-by: Wen Yang > CC: Alex Deucher > CC: christian.koe...@amd.com > CC: "David (ChunMing) Zhou" > CC: David Airlie (maintainer:DRM DRIVERS) > CC: Lyude Paul > CC: Rex Zhu > CC: Jim Qu > CC: amd-gfx@lists.freedesktop.org > CC: dri-de...@lists.freedesktop.org > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > index 7f0afc526419..996bfce149f2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > @@ -816,6 +816,5 @@ int amdgpu_acpi_init(struct amdgpu_device *adev) > void amdgpu_acpi_fini(struct amdgpu_device *adev) > { > unregister_acpi_notifier(&adev->acpi_nb); > - if (adev->atif) > - kfree(adev->atif); > + kfree(adev->atif); > } -- Cheers, Lyude Paul ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: linux-next: Tree for Dec 5 (gpu/drm/amd/display)
On 12/4/18 10:01 PM, Stephen Rothwell wrote: > Hi all, > > Changes since 20181204: > on i386: ld: drivers/gpu/drm/amd/display/dc/dce/dce_i2c_sw.o: in function `wait_for_scl_high_sw': dce_i2c_sw.c:(.text+0x2f3): undefined reference to `__bad_udelay' ld: drivers/gpu/drm/amd/display/dc/dce/dce_i2c_sw.o: in function `stop_sync_sw': dce_i2c_sw.c:(.text+0x58e): undefined reference to `__bad_udelay' ld: drivers/gpu/drm/amd/display/dc/dce/dce_i2c_sw.o: in function `write_byte_sw': dce_i2c_sw.c:(.text+0xb47): undefined reference to `__bad_udelay' ld: dce_i2c_sw.c:(.text+0xc48): undefined reference to `__bad_udelay' ld: drivers/gpu/drm/amd/display/dc/i2caux/i2c_sw_engine.o: in function `wait_for_scl_high': i2c_sw_engine.c:(.text+0x59c): undefined reference to `__bad_udelay' ld: drivers/gpu/drm/amd/display/dc/i2caux/i2c_sw_engine.o:i2c_sw_engine.c:(.text+0x837): more undefined references to `__bad_udelay' follow Full randconfig file is attached. -- ~Randy # # Automatically generated file; DO NOT EDIT. # Linux/i386 4.20.0-rc5 Kernel Configuration # # # Compiler: gcc (SUSE Linux) 4.8.5 # CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=40805 CONFIG_CLANG_VERSION=0 CONFIG_CONSTRUCTORS=y CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_BROKEN_ON_SMP=y CONFIG_INIT_ENV_ARG_LIMIT=32 # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION="" # CONFIG_LOCALVERSION_AUTO is not set CONFIG_BUILD_SALT="" CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y CONFIG_KERNEL_GZIP=y # CONFIG_KERNEL_BZIP2 is not set # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="(none)" CONFIG_SWAP=y # CONFIG_SYSVIPC is not set CONFIG_POSIX_MQUEUE=y CONFIG_POSIX_MQUEUE_SYSCTL=y CONFIG_CROSS_MEMORY_ATTACH=y CONFIG_USELIB=y CONFIG_AUDIT=y CONFIG_HAVE_ARCH_AUDITSYSCALL=y CONFIG_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_IRQ_CHIP=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_SIM=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_GENERIC_MSI_IRQ_DOMAIN=y CONFIG_GENERIC_IRQ_MATRIX_ALLOCATOR=y CONFIG_GENERIC_IRQ_RESERVATION_MODE=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y # CONFIG_GENERIC_IRQ_DEBUGFS is not set CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_ARCH_CLOCKSOURCE_DATA=y CONFIG_ARCH_CLOCKSOURCE_INIT=y CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y # # Timers subsystem # CONFIG_HZ_PERIODIC=y # CONFIG_NO_HZ_IDLE is not set # CONFIG_NO_HZ is not set # CONFIG_HIGH_RES_TIMERS is not set # CONFIG_PREEMPT_NONE is not set # CONFIG_PREEMPT_VOLUNTARY is not set CONFIG_PREEMPT=y CONFIG_PREEMPT_COUNT=y # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y # CONFIG_IRQ_TIME_ACCOUNTING is not set CONFIG_BSD_PROCESS_ACCT=y # CONFIG_BSD_PROCESS_ACCT_V3 is not set # CONFIG_TASKSTATS is not set # CONFIG_PSI is not set # # RCU Subsystem # CONFIG_PREEMPT_RCU=y # CONFIG_RCU_EXPERT is not set CONFIG_SRCU=y CONFIG_TREE_SRCU=y CONFIG_TASKS_RCU=y CONFIG_RCU_STALL_COMMON=y CONFIG_RCU_NEED_SEGCBLIST=y CONFIG_BUILD_BIN2C=y CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_LOG_BUF_SHIFT=17 CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=13 CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH=y CONFIG_CGROUPS=y CONFIG_PAGE_COUNTER=y CONFIG_MEMCG=y # CONFIG_MEMCG_SWAP is not set CONFIG_MEMCG_KMEM=y # CONFIG_BLK_CGROUP is not set CONFIG_CGROUP_SCHED=y CONFIG_FAIR_GROUP_SCHED=y CONFIG_CFS_BANDWIDTH=y # CONFIG_RT_GROUP_SCHED is not set CONFIG_CGROUP_PIDS=y # CONFIG_CGROUP_RDMA is not set # CONFIG_CGROUP_FREEZER is not set CONFIG_CGROUP_DEVICE=y CONFIG_CGROUP_CPUACCT=y # CONFIG_CGROUP_PERF is not set CONFIG_SOCK_CGROUP_DATA=y CONFIG_NAMESPACES=y CONFIG_UTS_NS=y # CONFIG_IPC_NS is not set # CONFIG_USER_NS is not set CONFIG_PID_NS=y CONFIG_NET_NS=y # CONFIG_CHECKPOINT_RESTORE is not set CONFIG_SCHED_AUTOGROUP=y CONFIG_SYSFS_DEPRECATED=y # CONFIG_SYSFS_DEPRECATED_V2 is not set # CONFIG_RELAY is not set # CONFIG_BLK_DEV_INITRD is not set # CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set CONFIG_CC_OPTIMIZE_FOR_DEBUGGING=y CONFIG_SYSCTL=y CONFIG_HAVE_UID16=y CONFIG_SYSCTL_EXCEPTION_TRACE=y CONFIG_HAVE_PCSPKR_PLATFORM=y CONFIG_BPF=y # CONFIG_EXPERT is not set CONFIG_UID16=y CONFIG_MULTIUSER=y CONFIG_SGETMASK_SYSCALL=y CONFIG_SYSFS_SYSCALL=y CONFIG_FHANDLE=y CONFIG_POSIX_TIMERS=y CONFIG_PRINTK=y CONFIG_PRINTK_NMI=y CONFIG_BUG=y CONFIG_ELF_CORE=y CONFIG_PCSPKR_PLATFORM=y CONFIG_BASE_FULL=y CONFIG_FUTEX=y CONFIG_FUTEX_PI=y CONFIG_EPOLL=y CONFIG_SIGNALFD=y CONFIG_TIMERFD=y CONFIG_EVENTFD=y CONFIG_SHMEM=y CONFIG_AIO=y CONFIG_ADVISE_SYSCALLS=y CONFIG_MEMBARRIER=y
Re: [PATCH] drm/amd/powerplay: rv dal-pplib interface refactor powerplay part
Looks reasonable to me. Acked-by: Alex Deucher From: Wu, Hersen Sent: Wednesday, December 5, 2018 1:03:57 PM To: amd-gfx@lists.freedesktop.org; Zhu, Rex; Deucher, Alexander Subject: RE: [PATCH] drm/amd/powerplay: rv dal-pplib interface refactor powerplay part Hello, Alex, Rex, Would you please help review the change? Thanks, Hersen [WHY] clarify dal input parameters to pplib interface, remove un-used parameters. dal knows exactly which parameters needed and their effects at pplib and smu sides. current dal sequence for dcn1_update_clock to pplib: 1.smu10_display_clock_voltage_request for dcefclk 2.smu10_display_clock_voltage_request for fclk 3.phm_store_dal_configuration_data { set_min_deep_sleep_dcfclk set_active_display_count store_cc6_data --- this data never be referenced new sequence will be: 1. set_display_count --- need add new pplib interface 2. set_min_deep_sleep_dcfclk -- new pplib interface 3. set_hard_min_dcfclk_by_freq 4. set_hard_min_fclk_by_freq after this code refactor, smu10_display_clock_voltage_request, phm_store_dal_configuration_data will not be needed for rv. [HOW] step 1: add new functions at pplib interface step 2: add new functions at amdgpu dm and dc Signed-off-by: hersen wu --- drivers/gpu/drm/amd/include/kgd_pp_interface.h | 4 ++ drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 76 ++ .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c | 45 - drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 36 +- .../gpu/drm/amd/powerplay/inc/hardwaremanager.h| 3 + drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 4 +- 6 files changed, 162 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h index 980e696..1479ea1 100644 --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h @@ -276,6 +276,10 @@ struct amd_pm_funcs { struct amd_pp_simple_clock_info *clocks); int (*notify_smu_enable_pwe)(void *handle); int (*enable_mgpu_fan_boost)(void *handle); + int (*set_active_display_count)(void *handle, uint32_t count); + int (*set_hard_min_dcefclk_by_freq)(void *handle, uint32_t clock); + int (*set_hard_min_fclk_by_freq)(void *handle, uint32_t clock); + int (*set_min_deep_sleep_dcefclk)(void *handle, uint32_t clock); }; #endif diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c index d6aa1d4..53dde16 100644 --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c @@ -1332,6 +1332,78 @@ static int pp_enable_mgpu_fan_boost(void *handle) return 0; } +static int pp_set_min_deep_sleep_dcefclk(void *handle, uint32_t clock) +{ + struct pp_hwmgr *hwmgr = handle; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + if (hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk == NULL) { + pr_info("%s was not implemented.\n", __func__); + return -EINVAL;; + } + + mutex_lock(&hwmgr->smu_lock); + hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk(hwmgr, clock); + mutex_unlock(&hwmgr->smu_lock); + + return 0; +} + +static int pp_set_hard_min_dcefclk_by_freq(void *handle, uint32_t +clock) { + struct pp_hwmgr *hwmgr = handle; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + if (hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq == NULL) { + pr_info("%s was not implemented.\n", __func__); + return -EINVAL;; + } + + mutex_lock(&hwmgr->smu_lock); + hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq(hwmgr, clock); + mutex_unlock(&hwmgr->smu_lock); + + return 0; +} + +static int pp_set_hard_min_fclk_by_freq(void *handle, uint32_t clock) { + struct pp_hwmgr *hwmgr = handle; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + if (hwmgr->hwmgr_func->set_hard_min_fclk_by_freq == NULL) { + pr_info("%s was not implemented.\n", __func__); + return -EINVAL;; + } + + mutex_lock(&hwmgr->smu_lock); + hwmgr->hwmgr_func->set_hard_min_fclk_by_freq(hwmgr, clock); + mutex_unlock(&hwmgr->smu_lock); + + return 0; +} + +static int pp_set_active_display_count(void *handle, uint32_t count) { + struct pp_hwmgr *hwmgr = handle; + int ret = 0; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + mutex_lock(&hwmgr->smu_lock); + ret = phm_set_active_display_count(hwmgr, count); + mutex_unlock(&hwmgr->smu_lock); + + return ret; +} + static const struct amd_pm_funcs pp_dpm_funcs = { .load_firmware = pp_dpm_load_fw, .wait_for_fw_loading_complete = pp_dpm_fw_loading_complete, @@ -
RE: [PATCH] drm/amd/powerplay: rv dal-pplib interface refactor powerplay part
Hello, Alex, Rex, Would you please help review the change? Thanks, Hersen [WHY] clarify dal input parameters to pplib interface, remove un-used parameters. dal knows exactly which parameters needed and their effects at pplib and smu sides. current dal sequence for dcn1_update_clock to pplib: 1.smu10_display_clock_voltage_request for dcefclk 2.smu10_display_clock_voltage_request for fclk 3.phm_store_dal_configuration_data { set_min_deep_sleep_dcfclk set_active_display_count store_cc6_data --- this data never be referenced new sequence will be: 1. set_display_count --- need add new pplib interface 2. set_min_deep_sleep_dcfclk -- new pplib interface 3. set_hard_min_dcfclk_by_freq 4. set_hard_min_fclk_by_freq after this code refactor, smu10_display_clock_voltage_request, phm_store_dal_configuration_data will not be needed for rv. [HOW] step 1: add new functions at pplib interface step 2: add new functions at amdgpu dm and dc Signed-off-by: hersen wu --- drivers/gpu/drm/amd/include/kgd_pp_interface.h | 4 ++ drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 76 ++ .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c | 45 - drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 36 +- .../gpu/drm/amd/powerplay/inc/hardwaremanager.h| 3 + drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 4 +- 6 files changed, 162 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h index 980e696..1479ea1 100644 --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h @@ -276,6 +276,10 @@ struct amd_pm_funcs { struct amd_pp_simple_clock_info *clocks); int (*notify_smu_enable_pwe)(void *handle); int (*enable_mgpu_fan_boost)(void *handle); + int (*set_active_display_count)(void *handle, uint32_t count); + int (*set_hard_min_dcefclk_by_freq)(void *handle, uint32_t clock); + int (*set_hard_min_fclk_by_freq)(void *handle, uint32_t clock); + int (*set_min_deep_sleep_dcefclk)(void *handle, uint32_t clock); }; #endif diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c index d6aa1d4..53dde16 100644 --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c @@ -1332,6 +1332,78 @@ static int pp_enable_mgpu_fan_boost(void *handle) return 0; } +static int pp_set_min_deep_sleep_dcefclk(void *handle, uint32_t clock) +{ + struct pp_hwmgr *hwmgr = handle; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + if (hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk == NULL) { + pr_info("%s was not implemented.\n", __func__); + return -EINVAL;; + } + + mutex_lock(&hwmgr->smu_lock); + hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk(hwmgr, clock); + mutex_unlock(&hwmgr->smu_lock); + + return 0; +} + +static int pp_set_hard_min_dcefclk_by_freq(void *handle, uint32_t +clock) { + struct pp_hwmgr *hwmgr = handle; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + if (hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq == NULL) { + pr_info("%s was not implemented.\n", __func__); + return -EINVAL;; + } + + mutex_lock(&hwmgr->smu_lock); + hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq(hwmgr, clock); + mutex_unlock(&hwmgr->smu_lock); + + return 0; +} + +static int pp_set_hard_min_fclk_by_freq(void *handle, uint32_t clock) { + struct pp_hwmgr *hwmgr = handle; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + if (hwmgr->hwmgr_func->set_hard_min_fclk_by_freq == NULL) { + pr_info("%s was not implemented.\n", __func__); + return -EINVAL;; + } + + mutex_lock(&hwmgr->smu_lock); + hwmgr->hwmgr_func->set_hard_min_fclk_by_freq(hwmgr, clock); + mutex_unlock(&hwmgr->smu_lock); + + return 0; +} + +static int pp_set_active_display_count(void *handle, uint32_t count) { + struct pp_hwmgr *hwmgr = handle; + int ret = 0; + + if (!hwmgr || !hwmgr->pm_en) + return -EINVAL; + + mutex_lock(&hwmgr->smu_lock); + ret = phm_set_active_display_count(hwmgr, count); + mutex_unlock(&hwmgr->smu_lock); + + return ret; +} + static const struct amd_pm_funcs pp_dpm_funcs = { .load_firmware = pp_dpm_load_fw, .wait_for_fw_loading_complete = pp_dpm_fw_loading_complete, @@ -1378,4 +1450,8 @@ static const struct amd_pm_funcs pp_dpm_funcs = { .get_display_mode_validation_clocks = pp_get_display_mode_validation_clocks, .notify_smu_enable_pwe = pp_notify_smu_enable_pwe, .enable_mgpu_fan_boost = pp_enable_mgpu_fan_boost, + .set_active_display
[PATCH v2] drm/amd/display: Add below the range support for FreeSync
[Why] When the flip-rate is below the minimum supported variable refresh rate range for the monitor the front porch wait will timeout and be frequently misaligned resulting in stuttering and/or flickering. The FreeSync module can still maintain a smooth and flicker free image when the monitor has a refresh rate range such that the maximum refresh > 2 * minimum refresh by utilizing low framerate compensation, "below the range". [How] Hook up the pre-flip and post-flip handlers from the FreeSync module. These adjust the minimum/maximum vrr range to duplicate frames when appropriate by tracking flip timestamps. Cc: Harry Wentland Signed-off-by: Nicholas Kazlauskas Acked-by: Leo Li --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 ++- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +- 2 files changed, 62 insertions(+), 19 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 23d61570df17..e2de064426fc 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -328,12 +328,29 @@ static void dm_crtc_high_irq(void *interrupt_params) struct common_irq_params *irq_params = interrupt_params; struct amdgpu_device *adev = irq_params->adev; struct amdgpu_crtc *acrtc; + struct dm_crtc_state *acrtc_state; acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VBLANK); if (acrtc) { drm_crtc_handle_vblank(&acrtc->base); amdgpu_dm_crtc_handle_crc_irq(&acrtc->base); + + acrtc_state = to_dm_crtc_state(acrtc->base.state); + + if (acrtc_state->stream && + acrtc_state->vrr_params.supported && + acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE) { + mod_freesync_handle_v_update( + adev->dm.freesync_module, + acrtc_state->stream, + &acrtc_state->vrr_params); + + dc_stream_adjust_vmin_vmax( + adev->dm.dc, + acrtc_state->stream, + &acrtc_state->vrr_params.adjust); + } } } @@ -3001,7 +3018,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) dc_stream_retain(state->stream); } - state->adjust = cur->adjust; + state->vrr_params = cur->vrr_params; state->vrr_infopacket = cur->vrr_infopacket; state->abm_level = cur->abm_level; state->vrr_supported = cur->vrr_supported; @@ -4396,9 +4413,11 @@ struct dc_stream_status *dc_state_get_stream_status( static void update_freesync_state_on_stream( struct amdgpu_display_manager *dm, struct dm_crtc_state *new_crtc_state, - struct dc_stream_state *new_stream) + struct dc_stream_state *new_stream, + struct dc_plane_state *surface, + u32 flip_timestamp_in_us) { - struct mod_vrr_params vrr = {0}; + struct mod_vrr_params vrr_params = new_crtc_state->vrr_params; struct dc_info_packet vrr_infopacket = {0}; struct mod_freesync_config config = new_crtc_state->freesync_config; @@ -4425,43 +,52 @@ static void update_freesync_state_on_stream( mod_freesync_build_vrr_params(dm->freesync_module, new_stream, - &config, &vrr); + &config, &vrr_params); + + if (surface) { + mod_freesync_handle_preflip( + dm->freesync_module, + surface, + new_stream, + flip_timestamp_in_us, + &vrr_params); + } mod_freesync_build_vrr_infopacket( dm->freesync_module, new_stream, - &vrr, + &vrr_params, PACKET_TYPE_VRR, TRANSFER_FUNC_UNKNOWN, &vrr_infopacket); new_crtc_state->freesync_timing_changed = - (memcmp(&new_crtc_state->adjust, - &vrr.adjust, - sizeof(vrr.adjust)) != 0); + (memcmp(&new_crtc_state->vrr_params.adjust, + &vrr_params.adjust, + sizeof(vrr_params.adjust)) != 0); new_crtc_state->freesync_vrr_info_changed = (memcmp(&new_crtc_state->vrr_infopacket, &vrr_infopacket, sizeof(vrr_infopacket)) != 0); - new_crtc_state->adjust = vrr.adjust; + new_crtc_state->vrr_params = vrr_params; new_crtc_state->vrr_infopacket = vrr_infopacket; - new_stream->adjust = new_crtc_state->adjust; + new_stream->adjust = new_crtc_s
Re: [PATCH 3/3] drm/amdgpu/psp: Destroy psp ring when doing gpu reset
Series is: Acked-by: Alex Deucher From: amd-gfx on behalf of Xiangliang Yu Sent: Wednesday, December 5, 2018 1:46:31 AM To: amd-gfx@lists.freedesktop.org Cc: Min, Frank; Yu, Xiangliang Subject: [PATCH 3/3] drm/amdgpu/psp: Destroy psp ring when doing gpu reset PSP ring need to be destroy before starting reinit for vf. This patche move it from hypervisor driver into guest. Signed-off-by: Xiangliang Yu Signed-off-by: Frank Min --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 3142f84..6759d89 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -548,8 +548,10 @@ static int psp_load_fw(struct amdgpu_device *adev) int ret; struct psp_context *psp = &adev->psp; - if (amdgpu_sriov_vf(adev) && adev->in_gpu_reset != 0) + if (amdgpu_sriov_vf(adev) && adev->in_gpu_reset) { + psp_ring_destroy(psp, PSP_RING_TYPE__KM); goto skip_memalloc; + } psp->cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL); if (!psp->cmd) -- 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 1/4] drm/edid: Pass connector to AVI inforframe functions
On Wed, Dec 05, 2018 at 08:40:34AM +0100, Andrzej Hajda wrote: > On 04.12.2018 20:02, Ville Syrjälä wrote: > > On Tue, Dec 04, 2018 at 08:03:53AM +0100, Andrzej Hajda wrote: > >> On 03.12.2018 22:48, Ville Syrjälä wrote: > >>> On Thu, Nov 29, 2018 at 09:46:16AM +0100, Andrzej Hajda wrote: > Quite late, hopefully not too late. > > > On 21.11.2018 12:51, Ville Syrjälä wrote: > > On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote: > >>> return; > >>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c > >>> b/drivers/gpu/drm/bridge/sil-sii8620.c > >>> index a6e8f4591e63..0cc293a6ac24 100644 > >>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c > >>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c > >>> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct > >>> sii8620 *ctx, > >>> int ret; > >>> > >>> ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, > >>> -mode, > >>> -true); > >>> +NULL, mode); > >>> if (ctx->use_packed_pixel) > >>> frm.avi.colorspace = HDMI_COLORSPACE_YUV422; > >>> > >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >>> index 64c3cf027518..88b720b63126 100644 > >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >>> @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi > >>> *hdmi, struct drm_display_mode *mode) > >>> u8 val; > >>> > >>> /* Initialise info frame from DRM mode */ > >>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); > >>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, > >>> + &hdmi->connector, > >>> mode); > >>> > >>> if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format)) > >>> frame.colorspace = HDMI_COLORSPACE_YUV444; > >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >>> index b506e3622b08..501ac05ba7da 100644 > >>> --- a/drivers/gpu/drm/drm_edid.c > >>> +++ b/drivers/gpu/drm/drm_edid.c > >>> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct > >>> drm_connector *connector, > >>> } > >>> EXPORT_SYMBOL(drm_set_preferred_mode); > >>> > >>> +static bool is_hdmi2_sink(struct drm_connector *connector) > >> You're usually known for adding const all around, why not const pointer > >> here and in all the other drm_* functions that call this? > > My current approach is to constify states/fbs/etc. but not so much > > crtcs/connectors/etc. Too much const can sometimes get in the way > > of things requiring that you remove the const later. But I guess > > in this case the const shouldn't really get in the way of anything > > because these are pretty much supposed to be pure functions. > > > >>> +{ > >>> + /* > >>> + * FIXME: sil-sii8620 doesn't have a connector around when > >>> + * we need one, so we have to be prepared for a NULL connector. > >>> + */ > >>> + if (!connector) > >>> + return false; > >> This actually changes the is_hdmi2_sink value for sil-sii8620. > > Hmm. No idea why they would have set that to true when everyone else is > > passing false. > Because false does not work :) More precisely MHLv3 (used in Sii8620) > uses CTA-861-F standard for infoframes, which is specific to HDMI2.0. > > Unfortunately I have no access to MHL specs, but my experiments and > vendor drivers strongly suggests it is done this way. > > This is important in case of 4K modes which are handled differently by > HDMI 1.4 and HDMI2.0. > >>> HDMI 2.0 handles 4k just like 1.4 handled it when you use one of > >>> the 4k modes defined in 1.4. Only if you use features beyond 1.4 do we > >>> switch over to the HDMI 2.0 specific signalling. > >> > >> The difference is in infoframes: > >> > >> HDMI 1.4 sets AVI infoframe VIC to 0, and sends HDMI_VIC in VSI. > >> > >> HDMI 2.0 sets AVI infoframe to non zero VICs introduced by > >> HDMI2.0/CEA-861-F, VSI can be omitted if I remember correctly, unless 3d > >> is in use. > > Like I said, The HDMI 1.4 method is used even with HDMI 2.0 sinks unless > > some feature gets used which can't be signalled via the HDMI 1.4 vendor > > specific infoframe. > > > Do you mean that 4K VICs 95, 94, 93, 98 defined in CEA-861-F are not > used at all for non-3d video in HDMI 2.0? > > Chapter 10.1 of HDMI2.0 spec says clearly: > > > When transmitting any additional Video Format for which a VIC value > > has been defined in > > CEA
Re: [PATCH] drm/amd/display: Add below the range support for FreeSync
On 2018-12-05 9:30 a.m., Li, Sun peng (Leo) wrote: > > > On 2018-12-05 8:40 a.m., Nicholas Kazlauskas wrote: >> [Why] >> When application flip-rate is below the minimum vrr refresh rate. >> >> Variable refresh rate monitors extend the front porch duration until >> flip or timeout occurs. For cases where the application flip-rate is > > Did something get cut off here? With that fixed, > Acked-by: Leo Li Those top two paragraphs were supposed to be cut, I think I formatted the wrong patch for that. The code itself is the correct version, however. Thanks. Nicholas Kazlauskas > >> >> When the flip-rate is below the minimum supported variable refresh rate >> range for the monitor the front porch wait will timeout and be >> frequently misaligned resulting in stuttering and/or flickering. >> >> The FreeSync module can still maintain a smooth and flicker free >> image when the monitor has a refresh rate range such that the maximum >> refresh > 2 * minimum refresh by utilizing low framerate compensation, >> "below the range". >> >> [How] >> Hook up the pre-flip and post-flip handlers from the FreeSync module. >> These adjust the minimum/maximum vrr range to duplicate frames >> when appropriate by tracking flip timestamps. >> >> Cc: Leo Li >> Cc: Harry Wentland >> Signed-off-by: Nicholas Kazlauskas >> --- >>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 ++- >>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +- >>2 files changed, 62 insertions(+), 19 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 23d61570df17..e2de064426fc 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -328,12 +328,29 @@ static void dm_crtc_high_irq(void *interrupt_params) >> struct common_irq_params *irq_params = interrupt_params; >> struct amdgpu_device *adev = irq_params->adev; >> struct amdgpu_crtc *acrtc; >> +struct dm_crtc_state *acrtc_state; >> >> acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - >> IRQ_TYPE_VBLANK); >> >> if (acrtc) { >> drm_crtc_handle_vblank(&acrtc->base); >> amdgpu_dm_crtc_handle_crc_irq(&acrtc->base); >> + >> +acrtc_state = to_dm_crtc_state(acrtc->base.state); >> + >> +if (acrtc_state->stream && >> +acrtc_state->vrr_params.supported && >> +acrtc_state->freesync_config.state == >> VRR_STATE_ACTIVE_VARIABLE) { >> +mod_freesync_handle_v_update( >> +adev->dm.freesync_module, >> +acrtc_state->stream, >> +&acrtc_state->vrr_params); >> + >> +dc_stream_adjust_vmin_vmax( >> +adev->dm.dc, >> +acrtc_state->stream, >> +&acrtc_state->vrr_params.adjust); >> +} >> } >>} >> >> @@ -3001,7 +3018,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) >> dc_stream_retain(state->stream); >> } >> >> -state->adjust = cur->adjust; >> +state->vrr_params = cur->vrr_params; >> state->vrr_infopacket = cur->vrr_infopacket; >> state->abm_level = cur->abm_level; >> state->vrr_supported = cur->vrr_supported; >> @@ -4396,9 +4413,11 @@ struct dc_stream_status *dc_state_get_stream_status( >>static void update_freesync_state_on_stream( >> struct amdgpu_display_manager *dm, >> struct dm_crtc_state *new_crtc_state, >> -struct dc_stream_state *new_stream) >> +struct dc_stream_state *new_stream, >> +struct dc_plane_state *surface, >> +u32 flip_timestamp_in_us) >>{ >> -struct mod_vrr_params vrr = {0}; >> +struct mod_vrr_params vrr_params = new_crtc_state->vrr_params; >> struct dc_info_packet vrr_infopacket = {0}; >> struct mod_freesync_config config = new_crtc_state->freesync_config; >> >> @@ -4425,43 +,52 @@ static void update_freesync_state_on_stream( >> >> mod_freesync_build_vrr_params(dm->freesync_module, >>new_stream, >> - &config, &vrr); >> + &config, &vrr_params); >> + >> +if (surface) { >> +mod_freesync_handle_preflip( >> +dm->freesync_module, >> +surface, >> +new_stream, >> +flip_timestamp_in_us, >> +&vrr_params); >> +} >> >> mod_freesync_build_vrr_infopacket( >> dm->freesync_module, >> new_stream, >> -&vrr, >> +&vrr_params, >> PACKET_TYPE_VRR, >> TRANSFER_FUNC_UNKNOWN, >> &vrr_infopacket); >> >> new_crtc_state->freesync_timing_changed = >>
[PATCH] drm/amd/include: Add mmhub 9.4 reg offsets and shift-mask
From: Leo Li In particular, we need the mmMC_VM_XGMI_LFB_CNTL register, for determining if xGMI is enabled on VG20. This will be used by DC to determine the correct spread spectrum adjustment for display and audio clocks. Signed-off-by: Leo Li Reviewed-by: Alex Deucher --- .../include/asic_reg/mmhub/mmhub_9_4_0_offset.h| 32 .../include/asic_reg/mmhub/mmhub_9_4_0_sh_mask.h | 35 ++ 2 files changed, 67 insertions(+) create mode 100644 drivers/gpu/drm/amd/include/asic_reg/mmhub/mmhub_9_4_0_offset.h create mode 100644 drivers/gpu/drm/amd/include/asic_reg/mmhub/mmhub_9_4_0_sh_mask.h diff --git a/drivers/gpu/drm/amd/include/asic_reg/mmhub/mmhub_9_4_0_offset.h b/drivers/gpu/drm/amd/include/asic_reg/mmhub/mmhub_9_4_0_offset.h new file mode 100644 index 000..8f51587 --- /dev/null +++ b/drivers/gpu/drm/amd/include/asic_reg/mmhub/mmhub_9_4_0_offset.h @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2018 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +#ifndef _mmhub_9_4_0_OFFSET_HEADER +#define _mmhub_9_4_0_OFFSET_HEADER + + +// addressBlock: mmhub_utcl2_vmsharedpfdec +// base address: 0x6a040 +#define mmMC_VM_XGMI_LFB_CNTL 0x0823 +#define mmMC_VM_XGMI_LFB_CNTL_BASE_IDX 0 +#define mmMC_VM_XGMI_LFB_SIZE 0x0824 +#define mmMC_VM_XGMI_LFB_SIZE_BASE_IDX 0 + +#endif diff --git a/drivers/gpu/drm/amd/include/asic_reg/mmhub/mmhub_9_4_0_sh_mask.h b/drivers/gpu/drm/amd/include/asic_reg/mmhub/mmhub_9_4_0_sh_mask.h new file mode 100644 index 000..0a6b072 --- /dev/null +++ b/drivers/gpu/drm/amd/include/asic_reg/mmhub/mmhub_9_4_0_sh_mask.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2018 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +#ifndef _mmhub_9_4_0_SH_MASK_HEADER +#define _mmhub_9_4_0_SH_MASK_HEADER + + +// addressBlock: mmhub_utcl2_vmsharedpfdec +//MC_VM_XGMI_LFB_CNTL +#define MC_VM_XGMI_LFB_CNTL__PF_LFB_REGION__SHIFT 0x0 +#define MC_VM_XGMI_LFB_CNTL__PF_MAX_REGION__SHIFT 0x4 +#define MC_VM_XGMI_LFB_CNTL__PF_LFB_REGION_MASK 0x0007L +#define MC_VM_XGMI_LFB_CNTL__PF_MAX_REGION_MASK 0x0070L +//MC_VM_XGMI_LFB_SIZE +#define MC_VM_XGMI_LFB_SIZE__PF_LFB_SIZE__SHIFT 0x0 +#define MC_VM_XGMI_LFB_SIZE__PF_LFB_SIZE_MASK 0xL + +#
Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions
On Tue, Nov 20, 2018 at 06:13:42PM +0200, Ville Syrjala wrote: > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c > b/drivers/gpu/drm/i2c/tda998x_drv.c > index a7c39f39793f..38c66fbc8276 100644 > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > @@ -849,7 +849,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct > drm_display_mode *mode) > { > union hdmi_infoframe frame; > > - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); > + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, > + &priv->connector, mode); > frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL; > > tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame); For this, Acked-by: Russell King Thanks. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions
On Wed, Dec 05, 2018 at 09:46:40AM +0100, Andrzej Hajda wrote: > On 05.12.2018 07:32, Laurent Pinchart wrote: > > Hi Ville, > > > > On Tuesday, 4 December 2018 21:13:20 EET Ville Syrjälä wrote: > >> On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote: > >>> On 03.12.2018 22:38, Ville Syrjälä wrote: > On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote: > > On 21.11.2018 19:19, Laurent Pinchart wrote: > >> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote: > >>> From: Ville Syrjälä > >>> > >>> Make life easier for drivers by simply passing the connector > >>> to drm_hdmi_avi_infoframe_from_display_mode() and > >>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't > >>> need to worry about is_hdmi2_sink mess. > >> While this is good for display controller drivers, the change isn't > >> great for bridge drivers. Down the road we're looking at moving > >> connector support out of the bridge drivers. Adding an additional > >> dependency to connectors in the bridges will make that more > >> difficult. Ideally bridges should retrieve the information from their > >> sink, regardless of whether it is a connector or another bridge. > > I agree with it, and case of sii8620 shows that there are cases where > > bridge has no direct access to the connector. > It's just a matter of plumbing it through. > >>> What do you mean exactly? > >> void bridge_foo(... > >> + ,struct drm_connector *connector); > >> > > On the other side, since you are passing connector to > > drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode > > parameter and rename the function to > > drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and > > mode set on the connector differs? > Connectors don't have a mode. > >>> As they are passing video stream they should have it, even if not > >>> directly, for example: > >>> > >>> connector->state->crtc->mode > >> That's not really how atomic works. One shouldn't go digging > >> through the obj->state pointers when we're not holding the > >> relevant locks anymore. The atomic way would be to pass either > >> both crtc state and connector state, or drm_atomic_state + > >> crtc/connector. > > > Usually infoframe contents is generated in modesetting/enable callbacks > so the locks should be in place. Not when doing a nonblocking modeset. > > And the locks should be hold for > drm_hdmi_avi_infoframe_from_display_mode as well - it wouldn't be correct if > > generated infoframe is not relevant to actual video mode. I guess that > if connector->state->crtc->mode > > differs from mode passed to drm_hdmi_avi_infoframe_from_display_mode it > is a sign of possible problem. > > And if they do not differ passing mode as an extra argument is redundant. > > > > Or a bridge state ? With chained bridges the mode can vary along the > > pipeline, > > the CRTC adjusted mode will only cover the link between the CRTC and the > > first > > bridge. It's only a matter of time until we need to store other > > intermediate > > modes in states. I'd rather prepare for that instead of passing the CRTC > > state > > to bridges. > > > Yes, optional bridge states seems reasonable, volunteers needed :) > > > Regards > > Andrzej > > > > > >>> In moment of creating infoframe it should be set properly. > >>> > >> Please see below for an additional comment. > >> > >>> Cc: Alex Deucher > >>> Cc: "Christian König" > >>> Cc: "David (ChunMing) Zhou" > >>> Cc: Archit Taneja > >>> Cc: Andrzej Hajda > >>> Cc: Laurent Pinchart > >>> Cc: Inki Dae > >>> Cc: Joonyoung Shim > >> Cc: Seung-Woo Kim > >>> Cc: Kyungmin Park > >>> Cc: Russell King > >>> Cc: CK Hu > >>> Cc: Philipp Zabel > >>> Cc: Rob Clark > >>> Cc: Ben Skeggs > >>> Cc: Tomi Valkeinen > >>> Cc: Sandy Huang > >>> Cc: "Heiko Stübner" > >>> Cc: Benjamin Gaignard > >>> Cc: Vincent Abriou > >>> Cc: Thierry Reding > >>> Cc: Eric Anholt > >>> Cc: Shawn Guo > >>> Cc: Ilia Mirkin > >>> Cc: amd-gfx@lists.freedesktop.org > >>> Cc: linux-arm-...@vger.kernel.org > >>> Cc: freedr...@lists.freedesktop.org > >>> Cc: nouv...@lists.freedesktop.org > >>> Cc: linux-te...@vger.kernel.org > >>> Signed-off-by: Ville Syrjälä > >>> --- > >>> > >>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- > >>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- > >>> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 3 ++- > >>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > >>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 ++-- > >>> drivers/gpu/drm/bridge/sii902x.c | 3 ++- > >>> drivers/gpu/drm/bridge/sil-sii8620.c | 3 +-- > >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 ++- > >>> drivers/gpu/drm/drm
Re: [PATCH] drm/amd/display: Add below the range support for FreeSync
On 2018-12-05 8:40 a.m., Nicholas Kazlauskas wrote: > [Why] > When application flip-rate is below the minimum vrr refresh rate. > > Variable refresh rate monitors extend the front porch duration until > flip or timeout occurs. For cases where the application flip-rate is Did something get cut off here? With that fixed, Acked-by: Leo Li > > When the flip-rate is below the minimum supported variable refresh rate > range for the monitor the front porch wait will timeout and be > frequently misaligned resulting in stuttering and/or flickering. > > The FreeSync module can still maintain a smooth and flicker free > image when the monitor has a refresh rate range such that the maximum > refresh > 2 * minimum refresh by utilizing low framerate compensation, > "below the range". > > [How] > Hook up the pre-flip and post-flip handlers from the FreeSync module. > These adjust the minimum/maximum vrr range to duplicate frames > when appropriate by tracking flip timestamps. > > Cc: Leo Li > Cc: Harry Wentland > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 ++- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +- > 2 files changed, 62 insertions(+), 19 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 23d61570df17..e2de064426fc 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -328,12 +328,29 @@ static void dm_crtc_high_irq(void *interrupt_params) > struct common_irq_params *irq_params = interrupt_params; > struct amdgpu_device *adev = irq_params->adev; > struct amdgpu_crtc *acrtc; > + struct dm_crtc_state *acrtc_state; > > acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - > IRQ_TYPE_VBLANK); > > if (acrtc) { > drm_crtc_handle_vblank(&acrtc->base); > amdgpu_dm_crtc_handle_crc_irq(&acrtc->base); > + > + acrtc_state = to_dm_crtc_state(acrtc->base.state); > + > + if (acrtc_state->stream && > + acrtc_state->vrr_params.supported && > + acrtc_state->freesync_config.state == > VRR_STATE_ACTIVE_VARIABLE) { > + mod_freesync_handle_v_update( > + adev->dm.freesync_module, > + acrtc_state->stream, > + &acrtc_state->vrr_params); > + > + dc_stream_adjust_vmin_vmax( > + adev->dm.dc, > + acrtc_state->stream, > + &acrtc_state->vrr_params.adjust); > + } > } > } > > @@ -3001,7 +3018,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) > dc_stream_retain(state->stream); > } > > - state->adjust = cur->adjust; > + state->vrr_params = cur->vrr_params; > state->vrr_infopacket = cur->vrr_infopacket; > state->abm_level = cur->abm_level; > state->vrr_supported = cur->vrr_supported; > @@ -4396,9 +4413,11 @@ struct dc_stream_status *dc_state_get_stream_status( > static void update_freesync_state_on_stream( > struct amdgpu_display_manager *dm, > struct dm_crtc_state *new_crtc_state, > - struct dc_stream_state *new_stream) > + struct dc_stream_state *new_stream, > + struct dc_plane_state *surface, > + u32 flip_timestamp_in_us) > { > - struct mod_vrr_params vrr = {0}; > + struct mod_vrr_params vrr_params = new_crtc_state->vrr_params; > struct dc_info_packet vrr_infopacket = {0}; > struct mod_freesync_config config = new_crtc_state->freesync_config; > > @@ -4425,43 +,52 @@ static void update_freesync_state_on_stream( > > mod_freesync_build_vrr_params(dm->freesync_module, > new_stream, > - &config, &vrr); > + &config, &vrr_params); > + > + if (surface) { > + mod_freesync_handle_preflip( > + dm->freesync_module, > + surface, > + new_stream, > + flip_timestamp_in_us, > + &vrr_params); > + } > > mod_freesync_build_vrr_infopacket( > dm->freesync_module, > new_stream, > - &vrr, > + &vrr_params, > PACKET_TYPE_VRR, > TRANSFER_FUNC_UNKNOWN, > &vrr_infopacket); > > new_crtc_state->freesync_timing_changed = > - (memcmp(&new_crtc_state->adjust, > - &vrr.adjust, > - sizeof(vrr.adjust)) != 0); > + (memcmp(&new_crtc_state->vrr_params.adjust, > + &vrr_params.adjust, > + sizeof(vrr_params.adjust)) != 0); > >
Re: [Intel-gfx] [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
Quoting Kuehling, Felix (2018-12-03 22:55:16) > > On 2018-11-28 4:14 a.m., Joonas Lahtinen wrote: > > Quoting Ho, Kenny (2018-11-27 17:41:17) > >> 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. > > I think the ask in return to the early series from Intal was to agree > > on the variables that could be common to all of DRM subsystem. > > > > So we can only choose the lowest common denominator, right? > > > > Any core count out of total core count should translate nicely into a > > fraction, so what would be the problem with percentage amounts? > How would you handle overcommitment with a percentage? That is, more > than 100% of the GPU cores assigned to cgroups. Which cgroups end up > sharing cores would be up to chance. I see your point. With time-slicing, you really can't overcommit. So would assume that there would have to be second level of detail provided for overcommitting (and deciding which cgroups are to share GPU cores). > If we allow specifying a set of GPU cores, we can be more specific in > assigning and sharing resources between cgroups. As Matt outlined in the other reply to this thread, we don't really have the concept of GPU cores. We do have the command streamers, but the granularity is bit low. In your architecture, does it matter which specific cores are shared, or is it just a question of which specific cgroups would share some cores in case of overcommit? If we tack in the priority in addition to the percentage, you could make a choice to share cores only at an identical priority level only. That'd mean that in the case of overcommit, you'd aim to keep as many high priority levels free of overcommit as possible and then for lower priority cgroups you'd start overcommitting. Would that even partially address the concern? Regards, Joonas > > Regards, > Felix > > > > > > Regards, Joonas > > > >> 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 >
[PATCH] drm/amd/display: Add below the range support for FreeSync
[Why] When application flip-rate is below the minimum vrr refresh rate. Variable refresh rate monitors extend the front porch duration until flip or timeout occurs. For cases where the application flip-rate is When the flip-rate is below the minimum supported variable refresh rate range for the monitor the front porch wait will timeout and be frequently misaligned resulting in stuttering and/or flickering. The FreeSync module can still maintain a smooth and flicker free image when the monitor has a refresh rate range such that the maximum refresh > 2 * minimum refresh by utilizing low framerate compensation, "below the range". [How] Hook up the pre-flip and post-flip handlers from the FreeSync module. These adjust the minimum/maximum vrr range to duplicate frames when appropriate by tracking flip timestamps. Cc: Leo Li Cc: Harry Wentland Signed-off-by: Nicholas Kazlauskas --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 ++- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +- 2 files changed, 62 insertions(+), 19 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 23d61570df17..e2de064426fc 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -328,12 +328,29 @@ static void dm_crtc_high_irq(void *interrupt_params) struct common_irq_params *irq_params = interrupt_params; struct amdgpu_device *adev = irq_params->adev; struct amdgpu_crtc *acrtc; + struct dm_crtc_state *acrtc_state; acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VBLANK); if (acrtc) { drm_crtc_handle_vblank(&acrtc->base); amdgpu_dm_crtc_handle_crc_irq(&acrtc->base); + + acrtc_state = to_dm_crtc_state(acrtc->base.state); + + if (acrtc_state->stream && + acrtc_state->vrr_params.supported && + acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE) { + mod_freesync_handle_v_update( + adev->dm.freesync_module, + acrtc_state->stream, + &acrtc_state->vrr_params); + + dc_stream_adjust_vmin_vmax( + adev->dm.dc, + acrtc_state->stream, + &acrtc_state->vrr_params.adjust); + } } } @@ -3001,7 +3018,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) dc_stream_retain(state->stream); } - state->adjust = cur->adjust; + state->vrr_params = cur->vrr_params; state->vrr_infopacket = cur->vrr_infopacket; state->abm_level = cur->abm_level; state->vrr_supported = cur->vrr_supported; @@ -4396,9 +4413,11 @@ struct dc_stream_status *dc_state_get_stream_status( static void update_freesync_state_on_stream( struct amdgpu_display_manager *dm, struct dm_crtc_state *new_crtc_state, - struct dc_stream_state *new_stream) + struct dc_stream_state *new_stream, + struct dc_plane_state *surface, + u32 flip_timestamp_in_us) { - struct mod_vrr_params vrr = {0}; + struct mod_vrr_params vrr_params = new_crtc_state->vrr_params; struct dc_info_packet vrr_infopacket = {0}; struct mod_freesync_config config = new_crtc_state->freesync_config; @@ -4425,43 +,52 @@ static void update_freesync_state_on_stream( mod_freesync_build_vrr_params(dm->freesync_module, new_stream, - &config, &vrr); + &config, &vrr_params); + + if (surface) { + mod_freesync_handle_preflip( + dm->freesync_module, + surface, + new_stream, + flip_timestamp_in_us, + &vrr_params); + } mod_freesync_build_vrr_infopacket( dm->freesync_module, new_stream, - &vrr, + &vrr_params, PACKET_TYPE_VRR, TRANSFER_FUNC_UNKNOWN, &vrr_infopacket); new_crtc_state->freesync_timing_changed = - (memcmp(&new_crtc_state->adjust, - &vrr.adjust, - sizeof(vrr.adjust)) != 0); + (memcmp(&new_crtc_state->vrr_params.adjust, + &vrr_params.adjust, + sizeof(vrr_params.adjust)) != 0); new_crtc_state->freesync_vrr_info_changed = (memcmp(&new_crtc_state->vrr_infopacket, &vrr_infopacket, sizeof(vrr_infopacket)) != 0); - new_crtc_state->adjust = vrr.adjust;
Re: [PATCH 1/9] dma-buf: make fence sequence numbers 64 bit v2
Hi David, with a bit of luck that should do it. Can you please run your test as well as the igt tests on this once more and see if everything works as expected? Thanks, Christian. Am 05.12.18 um 14:08 schrieb Christian König: For a lot of use cases we need 64bit sequence numbers. Currently drivers overload the dma_fence structure to store the additional bits. Stop doing that and make the sequence number in the dma_fence always 64bit. For compatibility with hardware which can do only 32bit sequences the comparisons in __dma_fence_is_later only takes the lower 32bits as significant when the upper 32bits are all zero. v2: change the logic in __dma_fence_is_later Signed-off-by: Christian König --- drivers/dma-buf/dma-fence.c| 2 +- drivers/dma-buf/sw_sync.c | 2 +- drivers/dma-buf/sync_file.c| 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +- drivers/gpu/drm/i915/i915_sw_fence.c | 2 +- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/vgem/vgem_fence.c | 4 ++-- include/linux/dma-fence.h | 22 +++--- 8 files changed, 24 insertions(+), 16 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 136ec04d683f..3aa8733f832a 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -649,7 +649,7 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout); */ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, - spinlock_t *lock, u64 context, unsigned seqno) + spinlock_t *lock, u64 context, u64 seqno) { BUG_ON(!lock); BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name); diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 53c1d6d36a64..32dcf7b4c935 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -172,7 +172,7 @@ static bool timeline_fence_enable_signaling(struct dma_fence *fence) static void timeline_fence_value_str(struct dma_fence *fence, char *str, int size) { - snprintf(str, size, "%d", fence->seqno); + snprintf(str, size, "%lld", fence->seqno); } static void timeline_fence_timeline_value_str(struct dma_fence *fence, diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 35dd06479867..4f6305ca52c8 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -144,7 +144,7 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len) } else { struct dma_fence *fence = sync_file->fence; - snprintf(buf, len, "%s-%s%llu-%d", + snprintf(buf, len, "%s-%s%llu-%lld", fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), fence->context, @@ -258,7 +258,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, i_b++; } else { - if (pt_a->seqno - pt_b->seqno <= INT_MAX) + if (__dma_fence_is_later(pt_a->seqno, pt_b->seqno)) add_fence(fences, &i, pt_a); else add_fence(fences, &i, pt_b); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c index 12f2bf97611f..bfaf5c6323be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c @@ -388,7 +388,7 @@ void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager *sa_manager, soffset, eoffset, eoffset - soffset); if (i->fence) - seq_printf(m, " protected by 0x%08x on context %llu", + seq_printf(m, " protected by 0x%016llx on context %llu", i->fence->seqno, i->fence->context); seq_printf(m, "\n"); diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c index 6dbeed079ae5..11bcdabd5177 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence.c +++ b/drivers/gpu/drm/i915/i915_sw_fence.c @@ -393,7 +393,7 @@ static void timer_i915_sw_fence_wake(struct timer_list *t) if (!fence) return; - pr_notice("Asynchronous wait on fence %s:%s:%x timed out (hint:%pS)\n", + pr_notice("Asynchronous wait on fence %s:%s:%llx timed out (hint:%pS)\n", cb->dma->ops->get_driver_name(cb->dma), cb->dma->ops->get_timeline_name(cb->dma), cb->dma->seqno, diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 217ed3ee1cab..f28a66c67d34 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1236,7 +1236,7 @@ static void print_request(struct drm_printer *m, x = print_sched_attr(rq->i915, &r
[PATCH 8/9] drm/amdgpu: add timeline support in amdgpu CS v2
From: Chunming Zhou syncobj wait/signal operation is appending in command submission. v2: separate to two kinds in/out_deps functions Signed-off-by: Chunming Zhou Cc: Daniel Rakos Cc: Jason Ekstrand Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Christian König Cc: Chris Wilson --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 10 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 147 +++-- include/uapi/drm/amdgpu_drm.h | 8 ++ 3 files changed, 140 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index aa26a144c7f5..c1cd1c8ee6ab 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -429,6 +429,12 @@ struct amdgpu_cs_chunk { void*kdata; }; +struct amdgpu_cs_post_dep { + struct drm_syncobj *syncobj; + struct dma_fence_chain *chain; + u64 point; +}; + struct amdgpu_cs_parser { struct amdgpu_device*adev; struct drm_file *filp; @@ -458,8 +464,8 @@ struct amdgpu_cs_parser { /* user fence */ struct amdgpu_bo_list_entry uf_entry; - unsigned num_post_dep_syncobjs; - struct drm_syncobj **post_dep_syncobjs; + unsignednum_post_deps; + struct amdgpu_cs_post_dep *post_deps; }; static inline u32 amdgpu_get_ib_value(struct amdgpu_cs_parser *p, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 149b3065119b..c61fb0e6881b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -214,6 +214,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs case AMDGPU_CHUNK_ID_DEPENDENCIES: case AMDGPU_CHUNK_ID_SYNCOBJ_IN: case AMDGPU_CHUNK_ID_SYNCOBJ_OUT: + case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT: + case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL: break; default: @@ -803,9 +805,11 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, ttm_eu_backoff_reservation(&parser->ticket, &parser->validated); - for (i = 0; i < parser->num_post_dep_syncobjs; i++) - drm_syncobj_put(parser->post_dep_syncobjs[i]); - kfree(parser->post_dep_syncobjs); + for (i = 0; i < parser->num_post_deps; i++) { + drm_syncobj_put(parser->post_deps[i].syncobj); + kfree(parser->post_deps[i].chain); + } + kfree(parser->post_deps); dma_fence_put(parser->fence); @@ -1107,13 +,18 @@ static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p, } static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, -uint32_t handle) +uint32_t handle, u64 point, +u64 flags) { - int r; struct dma_fence *fence; - r = drm_syncobj_find_fence(p->filp, handle, 0, 0, &fence); - if (r) + int r; + + r = drm_syncobj_find_fence(p->filp, handle, point, flags, &fence); + if (r) { + DRM_ERROR("syncobj %u failed to find fence @ %llu (%d)!\n", + handle, point, r); return r; + } r = amdgpu_sync_fence(p->adev, &p->job->sync, fence, true); dma_fence_put(fence); @@ -1124,46 +1133,115 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, static int amdgpu_cs_process_syncobj_in_dep(struct amdgpu_cs_parser *p, struct amdgpu_cs_chunk *chunk) { + struct drm_amdgpu_cs_chunk_sem *deps; unsigned num_deps; int i, r; - struct drm_amdgpu_cs_chunk_sem *deps; deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; num_deps = chunk->length_dw * 4 / sizeof(struct drm_amdgpu_cs_chunk_sem); + for (i = 0; i < num_deps; ++i) { + r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle, + 0, 0); + if (r) + return r; + } + + return 0; +} + +static int amdgpu_cs_process_syncobj_timeline_in_dep(struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ + struct drm_amdgpu_cs_chunk_syncobj *syncobj_deps; + unsigned num_deps; + int i, r; + + syncobj_deps = (struct drm_amdgpu_cs_chunk_syncobj *)chunk->kdata; + num_deps = chunk->length_dw * 4 / + sizeof(struct drm_amdgpu_cs_chunk_syncobj); for (i = 0; i < num_deps; ++i) { - r = amdgpu_syncobj_lookup_and
[PATCH 6/9] drm/syncobj: add timeline payload query ioctl v4
From: Chunming Zhou user mode can query timeline payload. v2: check return value of copy_to_user v3: handle querying entry by entry v4: rebase on new chain container, simplify interface Signed-off-by: Chunming Zhou Cc: Daniel Rakos Cc: Jason Ekstrand Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Christian König Cc: Chris Wilson --- drivers/gpu/drm/drm_internal.h | 2 ++ drivers/gpu/drm/drm_ioctl.c| 2 ++ drivers/gpu/drm/drm_syncobj.c | 40 include/uapi/drm/drm.h | 11 +++ 4 files changed, 55 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 566d44e3c782..9c4826411a3c 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -189,6 +189,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); /* drm_framebuffer.c */ void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index a9a17ed35cc4..7578ef6dc1d1 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -681,6 +681,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_UNLOCKED), diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 348079bb0965..1d830760992b 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1061,3 +1061,43 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, return ret; } + +int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_timeline_query *args = data; + struct drm_syncobj **syncobjs; + uint64_t __user *points = u64_to_user_ptr(args->points); + uint32_t i; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -ENODEV; + + if (args->count_handles == 0) + return -EINVAL; + + ret = drm_syncobj_array_find(file_private, +u64_to_user_ptr(args->handles), +args->count_handles, +&syncobjs); + if (ret < 0) + return ret; + + for (i = 0; i < args->count_handles; i++) { + struct dma_fence_chain *chain; + struct dma_fence *fence; + uint64_t point; + + fence = drm_syncobj_fence_get(syncobjs[i]); + chain = to_dma_fence_chain(fence); + point = chain ? fence->seqno : 0; + ret = copy_to_user(&points[i], &point, sizeof(uint64_t)); + ret = ret ? -EFAULT : 0; + if (ret) + break; + } + drm_syncobj_array_free(syncobjs, args->count_handles); + + return ret; +} diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 627032df23e6..14ca02a3d5f2 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -768,6 +768,15 @@ struct drm_syncobj_array { __u32 pad; }; +struct drm_syncobj_timeline_query { + __u64 handles; + /* points are timeline syncobjs payloads returned by query ioctl */ + __u64 points; + __u32 count_handles; + __u32 pad; +}; + + /* Query current scanout sequence number */ struct drm_crtc_get_sequence { __u32 crtc_id; /* requested crtc_id */ @@ -925,6 +934,8 @@ extern "C" { #define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct drm_mode_revoke_lease) #define DRM_IOCTL_SYNCOBJ_TIMELINE_WAITDRM_IOWR(0xCA, struct drm_syncobj_timeline_wait) +#define DRM_IOCTL_SYNCOBJ_QUERYDRM_IOWR(0xCB, struct drm_syncobj_timeline_query) + /** * Device specific ioctls should only be in their respective headers * The device specific ioctl range is from 0x40 to 0x9f. -- 2.14.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/a
[PATCH 7/9] drm/syncobj: use the timeline point in drm_syncobj_find_fence v3
Implement finding the right timeline point in drm_syncobj_find_fence. v2: return -EINVAL when the point is not submitted yet. v3: fix reference counting bug, add flags handling as well Signed-off-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 43 --- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 1d830760992b..54fd7e34ceae 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -231,16 +231,53 @@ int drm_syncobj_find_fence(struct drm_file *file_private, struct dma_fence **fence) { struct drm_syncobj *syncobj = drm_syncobj_find(file_private, handle); - int ret = 0; + struct syncobj_wait_entry wait; + int ret; if (!syncobj) return -ENOENT; *fence = drm_syncobj_fence_get(syncobj); - if (!*fence) { + drm_syncobj_put(syncobj); + + if (*fence) { + ret = dma_fence_chain_find_seqno(fence, point); + if (!ret) + return 0; + dma_fence_put(*fence); + } else { ret = -EINVAL; } - drm_syncobj_put(syncobj); + + if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) + return ret; + + memset(&wait, 0, sizeof(wait)); + wait.task = current; + wait.point = point; + drm_syncobj_fence_add_wait(syncobj, &wait); + + do { + set_current_state(TASK_INTERRUPTIBLE); + if (wait.fence) { + ret = 0; + break; + } + + if (signal_pending(current)) { + ret = -ERESTARTSYS; + break; + } + + schedule(); + } while (1); + + __set_current_state(TASK_RUNNING); + *fence = wait.fence; + + if (wait.node.next) + drm_syncobj_remove_wait(syncobj, &wait); + return ret; } EXPORT_SYMBOL(drm_syncobj_find_fence); -- 2.14.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/9] drm/syncobj: remove drm_syncobj_cb and cleanup
This completes "drm/syncobj: Drop add/remove_callback from driver interface" and cleans up the implementation a bit. Signed-off-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 91 ++- include/drm/drm_syncobj.h | 21 -- 2 files changed, 30 insertions(+), 82 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index db30a0e89db8..e19525af0cce 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -56,6 +56,16 @@ #include "drm_internal.h" #include +struct syncobj_wait_entry { + struct list_head node; + struct task_struct *task; + struct dma_fence *fence; + struct dma_fence_cb fence_cb; +}; + +static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj, + struct syncobj_wait_entry *wait); + /** * drm_syncobj_find - lookup and reference a sync object. * @file_private: drm file private pointer @@ -82,58 +92,33 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, } EXPORT_SYMBOL(drm_syncobj_find); -static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, - struct drm_syncobj_cb *cb, - drm_syncobj_func_t func) +static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj, + struct syncobj_wait_entry *wait) { - cb->func = func; - list_add_tail(&cb->node, &syncobj->cb_list); -} - -static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj, -struct dma_fence **fence, -struct drm_syncobj_cb *cb, -drm_syncobj_func_t func) -{ - int ret; - - *fence = drm_syncobj_fence_get(syncobj); - if (*fence) - return 1; + if (wait->fence) + return; spin_lock(&syncobj->lock); /* We've already tried once to get a fence and failed. Now that we * have the lock, try one more time just to be sure we don't add a * callback when a fence has already been set. */ - if (syncobj->fence) { - *fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, - lockdep_is_held(&syncobj->lock))); - ret = 1; - } else { - *fence = NULL; - drm_syncobj_add_callback_locked(syncobj, cb, func); - ret = 0; - } + if (syncobj->fence) + wait->fence = dma_fence_get( + rcu_dereference_protected(syncobj->fence, 1)); + else + list_add_tail(&wait->node, &syncobj->cb_list); spin_unlock(&syncobj->lock); - - return ret; } -void drm_syncobj_add_callback(struct drm_syncobj *syncobj, - struct drm_syncobj_cb *cb, - drm_syncobj_func_t func) +static void drm_syncobj_remove_wait(struct drm_syncobj *syncobj, + struct syncobj_wait_entry *wait) { - spin_lock(&syncobj->lock); - drm_syncobj_add_callback_locked(syncobj, cb, func); - spin_unlock(&syncobj->lock); -} + if (!wait->node.next) + return; -void drm_syncobj_remove_callback(struct drm_syncobj *syncobj, -struct drm_syncobj_cb *cb) -{ spin_lock(&syncobj->lock); - list_del_init(&cb->node); + list_del_init(&wait->node); spin_unlock(&syncobj->lock); } @@ -148,7 +133,7 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *fence) { struct dma_fence *old_fence; - struct drm_syncobj_cb *cur, *tmp; + struct syncobj_wait_entry *cur, *tmp; if (fence) dma_fence_get(fence); @@ -162,7 +147,7 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, if (fence != old_fence) { list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { list_del_init(&cur->node); - cur->func(syncobj, cur); + syncobj_wait_syncobj_func(syncobj, cur); } } @@ -608,13 +593,6 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, &args->handle); } -struct syncobj_wait_entry { - struct task_struct *task; - struct dma_fence *fence; - struct dma_fence_cb fence_cb; - struct drm_syncobj_cb syncobj_cb; -}; - static void syncobj_wait_fence_func(struct dma_fence *fence, struct dma_fence_cb *cb) { @@ -625,11 +603,8 @@ static void syncobj_wait_fence_func(struct dma_fence *fence, } s
[PATCH 2/9] dma-buf: add new dma_fence_chain container v3
Lockless container implementation similar to a dma_fence_array, but with only two elements per node and automatic garbage collection. v2: properly document dma_fence_chain_for_each, add dma_fence_chain_find_seqno, drop prev reference during garbage collection if it's not a chain fence. v3: use head and iterator for dma_fence_chain_for_each Signed-off-by: Christian König --- drivers/dma-buf/Makefile | 3 +- drivers/dma-buf/dma-fence-chain.c | 234 ++ include/linux/dma-fence-chain.h | 81 + 3 files changed, 317 insertions(+), 1 deletion(-) create mode 100644 drivers/dma-buf/dma-fence-chain.c create mode 100644 include/linux/dma-fence-chain.h diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 0913a6ccab5a..1f006e083eb9 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,4 +1,5 @@ -obj-y := dma-buf.o dma-fence.o dma-fence-array.o reservation.o seqno-fence.o +obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ +reservation.o seqno-fence.o obj-$(CONFIG_SYNC_FILE)+= sync_file.o obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o obj-$(CONFIG_UDMABUF) += udmabuf.o diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c new file mode 100644 index ..1ec234d66081 --- /dev/null +++ b/drivers/dma-buf/dma-fence-chain.c @@ -0,0 +1,234 @@ +/* + * fence-chain: chain fences together in a timeline + * + * Copyright (C) 2018 Advanced Micro Devices, Inc. + * Authors: + * Christian König + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include + +static bool dma_fence_chain_enable_signaling(struct dma_fence *fence); + +/** + * dma_fence_chain_get_prev - use RCU to get a reference to the previous fence + * @chain: chain node to get the previous node from + * + * Use dma_fence_get_rcu_safe to get a reference to the previous fence of the + * chain node. + */ +static struct dma_fence *dma_fence_chain_get_prev(struct dma_fence_chain *chain) +{ + struct dma_fence *prev; + + rcu_read_lock(); + prev = dma_fence_get_rcu_safe(&chain->prev); + rcu_read_unlock(); + return prev; +} + +/** + * dma_fence_chain_walk - chain walking function + * @fence: current chain node + * + * Walk the chain to the next node. Returns the next fence or NULL if we are at + * the end of the chain. Garbage collects chain nodes which are already + * signaled. + */ +struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence) +{ + struct dma_fence_chain *chain, *prev_chain; + struct dma_fence *prev, *replacement, *tmp; + + chain = to_dma_fence_chain(fence); + if (!chain) { + dma_fence_put(fence); + return NULL; + } + + while ((prev = dma_fence_chain_get_prev(chain))) { + + prev_chain = to_dma_fence_chain(prev); + if (prev_chain) { + if (!dma_fence_is_signaled(prev_chain->fence)) + break; + + replacement = dma_fence_chain_get_prev(prev_chain); + } else { + if (!dma_fence_is_signaled(prev)) + break; + + replacement = NULL; + } + + tmp = cmpxchg(&chain->prev, prev, replacement); + if (tmp == prev) + dma_fence_put(tmp); + else + dma_fence_put(replacement); + dma_fence_put(prev); + } + + dma_fence_put(fence); + return prev; +} +EXPORT_SYMBOL(dma_fence_chain_walk); + +/** + * dma_fence_chain_find_seqno - find fence chain node by seqno + * @pfence: pointer to the chain node where to start + * @seqno: the sequence number to search for + * + * Advance the fence pointer to the chain node which will signal this sequence + * number. If no sequence number is provided then this is a no-op. + * + * Returns EINVAL if the fence is not a chain node or the sequence number has + * not yet advanced far enough. + */ +int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) +{ + struct dma_fence_chain *chain; + + if (!seqno) + return 0; + + chain = to_dma_fence_chain(*pfence); + if (!chain || chain->base.seqno < seqno) + return -EINVAL; + + dma_fence_chain_for_each(*pfence, &chain->base) { + if ((*pfence)->context != chain->base.context || + to_dma_fence
[PATCH 1/9] dma-buf: make fence sequence numbers 64 bit v2
For a lot of use cases we need 64bit sequence numbers. Currently drivers overload the dma_fence structure to store the additional bits. Stop doing that and make the sequence number in the dma_fence always 64bit. For compatibility with hardware which can do only 32bit sequences the comparisons in __dma_fence_is_later only takes the lower 32bits as significant when the upper 32bits are all zero. v2: change the logic in __dma_fence_is_later Signed-off-by: Christian König --- drivers/dma-buf/dma-fence.c| 2 +- drivers/dma-buf/sw_sync.c | 2 +- drivers/dma-buf/sync_file.c| 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +- drivers/gpu/drm/i915/i915_sw_fence.c | 2 +- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/vgem/vgem_fence.c | 4 ++-- include/linux/dma-fence.h | 22 +++--- 8 files changed, 24 insertions(+), 16 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 136ec04d683f..3aa8733f832a 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -649,7 +649,7 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout); */ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, - spinlock_t *lock, u64 context, unsigned seqno) + spinlock_t *lock, u64 context, u64 seqno) { BUG_ON(!lock); BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name); diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 53c1d6d36a64..32dcf7b4c935 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -172,7 +172,7 @@ static bool timeline_fence_enable_signaling(struct dma_fence *fence) static void timeline_fence_value_str(struct dma_fence *fence, char *str, int size) { - snprintf(str, size, "%d", fence->seqno); + snprintf(str, size, "%lld", fence->seqno); } static void timeline_fence_timeline_value_str(struct dma_fence *fence, diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 35dd06479867..4f6305ca52c8 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -144,7 +144,7 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len) } else { struct dma_fence *fence = sync_file->fence; - snprintf(buf, len, "%s-%s%llu-%d", + snprintf(buf, len, "%s-%s%llu-%lld", fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), fence->context, @@ -258,7 +258,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, i_b++; } else { - if (pt_a->seqno - pt_b->seqno <= INT_MAX) + if (__dma_fence_is_later(pt_a->seqno, pt_b->seqno)) add_fence(fences, &i, pt_a); else add_fence(fences, &i, pt_b); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c index 12f2bf97611f..bfaf5c6323be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c @@ -388,7 +388,7 @@ void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager *sa_manager, soffset, eoffset, eoffset - soffset); if (i->fence) - seq_printf(m, " protected by 0x%08x on context %llu", + seq_printf(m, " protected by 0x%016llx on context %llu", i->fence->seqno, i->fence->context); seq_printf(m, "\n"); diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c index 6dbeed079ae5..11bcdabd5177 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence.c +++ b/drivers/gpu/drm/i915/i915_sw_fence.c @@ -393,7 +393,7 @@ static void timer_i915_sw_fence_wake(struct timer_list *t) if (!fence) return; - pr_notice("Asynchronous wait on fence %s:%s:%x timed out (hint:%pS)\n", + pr_notice("Asynchronous wait on fence %s:%s:%llx timed out (hint:%pS)\n", cb->dma->ops->get_driver_name(cb->dma), cb->dma->ops->get_timeline_name(cb->dma), cb->dma->seqno, diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 217ed3ee1cab..f28a66c67d34 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1236,7 +1236,7 @@ static void print_request(struct drm_printer *m, x = print_sched_attr(rq->i915, &rq->sched.attr, buf, x, sizeof(buf)); - drm_printf(m, "%s%x%s [%llx:%x]%s @ %dms: %s\n", + drm_printf(m, "%s%x%s [%llx:%llx]%s @ %dms: %s\n", prefix,
[PATCH 9/9] drm/amdgpu: update version for timeline syncobj support in amdgpu
From: Chunming Zhou Signed-off-by: Chunming Zhou --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 90f474f98b6e..316bfc1a6a75 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -71,9 +71,10 @@ * - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk). * - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE. * - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation. + * - 3.28.0 - Add syncobj timeline support to AMDGPU_CS. */ #define KMS_DRIVER_MAJOR 3 -#define KMS_DRIVER_MINOR 27 +#define KMS_DRIVER_MINOR 28 #define KMS_DRIVER_PATCHLEVEL 0 int amdgpu_vram_limit = 0; -- 2.14.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 5/9] drm/syncobj: add support for timeline point wait v8
From: Chunming Zhou points array is one-to-one match with syncobjs array. v2: add seperate ioctl for timeline point wait, otherwise break uapi. v3: userspace can specify two kinds waits:: a. Wait for time point to be completed. b. and wait for time point to become available v4: rebase v5: add comment for xxx_WAIT_AVAILABLE v6: rebase and rework on new container v7: drop _WAIT_COMPLETED, it is the default anyway v8: correctly handle garbage collected fences Signed-off-by: Chunming Zhou Signed-off-by: Christian König Cc: Daniel Rakos Cc: Jason Ekstrand Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Chris Wilson --- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 153 + include/uapi/drm/drm.h | 15 4 files changed, 143 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 0c4eb4a9ab31..566d44e3c782 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 94bd872d56c4..a9a17ed35cc4 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -675,6 +675,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, drm_syncobj_timeline_wait_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 51f798e2194f..348079bb0965 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -61,6 +61,7 @@ struct syncobj_wait_entry { struct task_struct *task; struct dma_fence *fence; struct dma_fence_cb fence_cb; + u64point; }; static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj, @@ -95,6 +96,8 @@ EXPORT_SYMBOL(drm_syncobj_find); static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj, struct syncobj_wait_entry *wait) { + struct dma_fence *fence; + if (wait->fence) return; @@ -103,11 +106,15 @@ static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj, * have the lock, try one more time just to be sure we don't add a * callback when a fence has already been set. */ - if (syncobj->fence) - wait->fence = dma_fence_get( - rcu_dereference_protected(syncobj->fence, 1)); - else + fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, 1)); + if (!fence || dma_fence_chain_find_seqno(&fence, wait->point)) { + dma_fence_put(fence); list_add_tail(&wait->node, &syncobj->cb_list); + } else if (!fence) { + wait->fence = dma_fence_get_stub(); + } else { + wait->fence = fence; + } spin_unlock(&syncobj->lock); } @@ -148,10 +155,8 @@ void drm_syncobj_add_point(struct drm_syncobj *syncobj, dma_fence_chain_init(chain, prev, fence, point); rcu_assign_pointer(syncobj->fence, &chain->base); - list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { - list_del_init(&cur->node); + list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) syncobj_wait_syncobj_func(syncobj, cur); - } spin_unlock(&syncobj->lock); /* Walk the chain once to trigger garbage collection */ @@ -182,10 +187,8 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, rcu_assign_pointer(syncobj->fence, fence); if (fence != old_fence) { - list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { - list_del_init(&cur->node); + list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) syncobj_wait_syncobj_func(syncobj, cur); -
[PATCH 4/9] drm/syncobj: add new drm_syncobj_add_point interface v2
Use the dma_fence_chain object to create a timeline of fence objects instead of just replacing the existing fence. v2: rebase and cleanup Signed-off-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 37 + include/drm/drm_syncobj.h | 5 + 2 files changed, 42 insertions(+) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index e19525af0cce..51f798e2194f 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -122,6 +122,43 @@ static void drm_syncobj_remove_wait(struct drm_syncobj *syncobj, spin_unlock(&syncobj->lock); } +/** + * drm_syncobj_add_point - add new timeline point to the syncobj + * @syncobj: sync object to add timeline point do + * @chain: chain node to use to add the point + * @fence: fence to encapsulate in the chain node + * @point: sequence number to use for the point + * + * Add the chain node as new timeline point to the syncobj. + */ +void drm_syncobj_add_point(struct drm_syncobj *syncobj, + struct dma_fence_chain *chain, + struct dma_fence *fence, + uint64_t point) +{ + struct syncobj_wait_entry *cur, *tmp; + struct dma_fence *prev; + + dma_fence_get(fence); + + spin_lock(&syncobj->lock); + + prev = rcu_dereference_protected(syncobj->fence, +lockdep_is_held(&syncobj->lock)); + dma_fence_chain_init(chain, prev, fence, point); + rcu_assign_pointer(syncobj->fence, &chain->base); + + list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { + list_del_init(&cur->node); + syncobj_wait_syncobj_func(syncobj, cur); + } + spin_unlock(&syncobj->lock); + + /* Walk the chain once to trigger garbage collection */ + dma_fence_chain_for_each(prev, fence); +} +EXPORT_SYMBOL(drm_syncobj_add_point); + /** * drm_syncobj_replace_fence - replace fence in a sync object. * @syncobj: Sync object to replace fence in diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 7c6ed845c70d..8acb4ae4f311 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -27,6 +27,7 @@ #define __DRM_SYNCOBJ_H__ #include "linux/dma-fence.h" +#include "linux/dma-fence-chain.h" /** * struct drm_syncobj - sync object. @@ -110,6 +111,10 @@ drm_syncobj_fence_get(struct drm_syncobj *syncobj) struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, u32 handle); +void drm_syncobj_add_point(struct drm_syncobj *syncobj, + struct dma_fence_chain *chain, + struct dma_fence *fence, + uint64_t point); void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *fence); int drm_syncobj_find_fence(struct drm_file *file_private, -- 2.14.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: regression in ttm
On 2018-12-05 12:41 p.m., Koenig, Christian wrote: > Am 05.12.18 um 12:33 schrieb Michel Dänzer: >> On 2018-12-05 12:24 p.m., Koenig, Christian wrote: >>> Am 05.12.18 um 11:29 schrieb Michel Dänzer: On 2018-12-04 6:35 p.m., Koenig, Christian wrote: > Thanks, going to take a look tomorrow. I also hit this with Bonaire in my development system. I wonder why you didn't? How are you running piglit? >>> With more memory. The issue is in TTM and only happens when you start to >>> evict something. >> How much memory do you have, how many CPU cores, and how many piglit >> tests running in parallel? 16G (1G VRAM) / 8/16 (SMT) / 16 here. >> >> Which piglit profile are you using? gpu here (with the >> glx-multithread-texture test excluded, because that hangs due to a Mesa >> bug with so many CPU cores). >> >> Are you running piglit with/out --process-isolation false? With it here. > > I usually run piglit as last thing in the evening on my Vega/Ryzen > system. That box has 16GB VRAM, 32GB system. > > No idea what the piglit settings actually are since I'm using the Ubuntu > package to update it. My script just runs "piglit run > /usr/lib/x86_64-linux-gnu/piglit/tests/gpu.py ~/Temp/". Maybe you can try adding --process-isolation false. If nothing else, it should reduce the time needed for the piglit run significantly. Other than that, I guess the amount of VRAM is the most significant difference, though I'd expect some of the piglit tests to exhaust that anyway. > I have to confess that I rarely look out for regressions in the actual > result, but rather look out for lockups or KASAN reports from the kernel > in dmesg. That's fine, if there's nothing in dmesg, it's usually not the kernel's fault. :) -- 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: regression in ttm
Am 05.12.18 um 12:33 schrieb Michel Dänzer: > On 2018-12-05 12:24 p.m., Koenig, Christian wrote: >> Am 05.12.18 um 11:29 schrieb Michel Dänzer: >>> On 2018-12-04 6:35 p.m., Koenig, Christian wrote: Thanks, going to take a look tomorrow. >>> I also hit this with Bonaire in my development system. I wonder why you >>> didn't? How are you running piglit? >> With more memory. The issue is in TTM and only happens when you start to >> evict something. > How much memory do you have, how many CPU cores, and how many piglit > tests running in parallel? 16G (1G VRAM) / 8/16 (SMT) / 16 here. > > Which piglit profile are you using? gpu here (with the > glx-multithread-texture test excluded, because that hangs due to a Mesa > bug with so many CPU cores). > > Are you running piglit with/out --process-isolation false? With it here. I usually run piglit as last thing in the evening on my Vega/Ryzen system. That box has 16GB VRAM, 32GB system. No idea what the piglit settings actually are since I'm using the Ubuntu package to update it. My script just runs "piglit run /usr/lib/x86_64-linux-gnu/piglit/tests/gpu.py ~/Temp/". I have to confess that I rarely look out for regressions in the actual result, but rather look out for lockups or KASAN reports from the kernel in dmesg. Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: regression in ttm
On 2018-12-05 12:24 p.m., Koenig, Christian wrote: > Am 05.12.18 um 11:29 schrieb Michel Dänzer: >> On 2018-12-04 6:35 p.m., Koenig, Christian wrote: >>> Thanks, going to take a look tomorrow. >> I also hit this with Bonaire in my development system. I wonder why you >> didn't? How are you running piglit? > > With more memory. The issue is in TTM and only happens when you start to > evict something. How much memory do you have, how many CPU cores, and how many piglit tests running in parallel? 16G (1G VRAM) / 8/16 (SMT) / 16 here. Which piglit profile are you using? gpu here (with the glx-multithread-texture test excluded, because that hangs due to a Mesa bug with so many CPU cores). Are you running piglit with/out --process-isolation false? With it here. -- 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/amdgpu: Skip ring soft recovery when fence parent was NULL
Am 05.12.18 um 12:03 schrieb wentalou: amdgpu_ring_soft_recovery would have Call-Trace, when s_job->s_fence->parent was NULL inside amdgpu_job_timedout. Check parent first, as drm_sched_hw_job_reset did. Change-Id: I0b674ffd96afd44bcefe37a66fb157b1dbba61a0 Signed-off-by: Wentao Lou --- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index e0af44f..2945615 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -33,7 +33,7 @@ static void amdgpu_job_timedout(struct drm_sched_job *s_job) struct amdgpu_ring *ring = to_amdgpu_ring(s_job->sched); struct amdgpu_job *job = to_amdgpu_job(s_job); - if (amdgpu_ring_soft_recovery(ring, job->vmid, s_job->s_fence->parent)) { + if (s_job->s_fence->parent && amdgpu_ring_soft_recovery(ring, job->vmid, s_job->s_fence->parent)) { Of hand that line looks to long to me, but apart from that the patch looks good. Maybe add the NULL check to amdgpu_ring_soft_recovery instead? Christian. DRM_ERROR("ring %s timeout, but soft recovered\n", s_job->sched->name); return; ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: regression in ttm
Am 05.12.18 um 11:29 schrieb Michel Dänzer: > On 2018-12-04 6:35 p.m., Koenig, Christian wrote: >> Thanks, going to take a look tomorrow. > I also hit this with Bonaire in my development system. I wonder why you > didn't? How are you running piglit? With more memory. The issue is in TTM and only happens when you start to evict something. Patch to fix this is on dri-devel, Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Skip ring soft recovery when fence parent was NULL
amdgpu_ring_soft_recovery would have Call-Trace, when s_job->s_fence->parent was NULL inside amdgpu_job_timedout. Check parent first, as drm_sched_hw_job_reset did. Change-Id: I0b674ffd96afd44bcefe37a66fb157b1dbba61a0 Signed-off-by: Wentao Lou --- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index e0af44f..2945615 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -33,7 +33,7 @@ static void amdgpu_job_timedout(struct drm_sched_job *s_job) struct amdgpu_ring *ring = to_amdgpu_ring(s_job->sched); struct amdgpu_job *job = to_amdgpu_job(s_job); - if (amdgpu_ring_soft_recovery(ring, job->vmid, s_job->s_fence->parent)) { + if (s_job->s_fence->parent && amdgpu_ring_soft_recovery(ring, job->vmid, s_job->s_fence->parent)) { DRM_ERROR("ring %s timeout, but soft recovered\n", s_job->sched->name); return; -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: regression in ttm
On 2018-12-04 6:35 p.m., Koenig, Christian wrote: > Thanks, going to take a look tomorrow. I also hit this with Bonaire in my development system. I wonder why you didn't? How are you running piglit? -- 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 04/10] drm/syncobj: remove drm_syncobj_cb and cleanup
Hi Daniel, can I get a review for this one? It is essentially just a follow up cleanup on one of your patches and shouldn't have any functional effect. Thanks, Christian. Am 04.12.18 um 12:59 schrieb Christian König: This completes "drm/syncobj: Drop add/remove_callback from driver interface" and cleans up the implementation a bit. Signed-off-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 91 ++- include/drm/drm_syncobj.h | 21 -- 2 files changed, 30 insertions(+), 82 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index db30a0e89db8..e19525af0cce 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -56,6 +56,16 @@ #include "drm_internal.h" #include +struct syncobj_wait_entry { + struct list_head node; + struct task_struct *task; + struct dma_fence *fence; + struct dma_fence_cb fence_cb; +}; + +static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj, + struct syncobj_wait_entry *wait); + /** * drm_syncobj_find - lookup and reference a sync object. * @file_private: drm file private pointer @@ -82,58 +92,33 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, } EXPORT_SYMBOL(drm_syncobj_find); -static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, - struct drm_syncobj_cb *cb, - drm_syncobj_func_t func) +static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj, + struct syncobj_wait_entry *wait) { - cb->func = func; - list_add_tail(&cb->node, &syncobj->cb_list); -} - -static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj, -struct dma_fence **fence, -struct drm_syncobj_cb *cb, -drm_syncobj_func_t func) -{ - int ret; - - *fence = drm_syncobj_fence_get(syncobj); - if (*fence) - return 1; + if (wait->fence) + return; spin_lock(&syncobj->lock); /* We've already tried once to get a fence and failed. Now that we * have the lock, try one more time just to be sure we don't add a * callback when a fence has already been set. */ - if (syncobj->fence) { - *fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, - lockdep_is_held(&syncobj->lock))); - ret = 1; - } else { - *fence = NULL; - drm_syncobj_add_callback_locked(syncobj, cb, func); - ret = 0; - } + if (syncobj->fence) + wait->fence = dma_fence_get( + rcu_dereference_protected(syncobj->fence, 1)); + else + list_add_tail(&wait->node, &syncobj->cb_list); spin_unlock(&syncobj->lock); - - return ret; } -void drm_syncobj_add_callback(struct drm_syncobj *syncobj, - struct drm_syncobj_cb *cb, - drm_syncobj_func_t func) +static void drm_syncobj_remove_wait(struct drm_syncobj *syncobj, + struct syncobj_wait_entry *wait) { - spin_lock(&syncobj->lock); - drm_syncobj_add_callback_locked(syncobj, cb, func); - spin_unlock(&syncobj->lock); -} + if (!wait->node.next) + return; -void drm_syncobj_remove_callback(struct drm_syncobj *syncobj, -struct drm_syncobj_cb *cb) -{ spin_lock(&syncobj->lock); - list_del_init(&cb->node); + list_del_init(&wait->node); spin_unlock(&syncobj->lock); } @@ -148,7 +133,7 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *fence) { struct dma_fence *old_fence; - struct drm_syncobj_cb *cur, *tmp; + struct syncobj_wait_entry *cur, *tmp; if (fence) dma_fence_get(fence); @@ -162,7 +147,7 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, if (fence != old_fence) { list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { list_del_init(&cur->node); - cur->func(syncobj, cur); + syncobj_wait_syncobj_func(syncobj, cur); } } @@ -608,13 +593,6 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, &args->handle); } -struct syncobj_wait_entry { - struct task_struct *task; - struct dma_fence *fence; - struct dma_fence_cb fence_cb; - struct dr
[PATCH AUTOSEL 4.14 53/69] drm/amdgpu: Add delay after enable RLC ucode
From: shaoyunl [ Upstream commit ad97d9de45835b6a0f71983b0ae0cffd7306730a ] Driver shouldn't try to access any GFX registers until RLC is idle. During the test, it took 12 seconds for RLC to clear the BUSY bit in RLC_GPM_STAT register which is un-acceptable for driver. As per RLC engineer, it would take RLC Ucode less than 10,000 GFXCLK cycles to finish its critical section. In a lowest 300M enginer clock setting(default from vbios), 50 us delay is enough. This commit fix the hang when RLC introduce the work around for XGMI which requires more cycles to setup more registers than normal Signed-off-by: shaoyunl Acked-by: Felix Kuehling Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 3981915e2311..b2eecfc9042e 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -1992,12 +1992,13 @@ static void gfx_v9_0_rlc_start(struct amdgpu_device *adev) #endif WREG32_FIELD15(GC, 0, RLC_CNTL, RLC_ENABLE_F32, 1); + udelay(50); /* carrizo do enable cp interrupt after cp inited */ - if (!(adev->flags & AMD_IS_APU)) + if (!(adev->flags & AMD_IS_APU)) { gfx_v9_0_enable_gui_idle_interrupt(adev, true); - - udelay(50); + udelay(50); + } #ifdef AMDGPU_RLC_DEBUG_RETRY /* RLC_GPM_GENERAL_6 : RLC Ucode version */ -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 4.19 099/123] drm/amdgpu: Add delay after enable RLC ucode
From: shaoyunl [ Upstream commit ad97d9de45835b6a0f71983b0ae0cffd7306730a ] Driver shouldn't try to access any GFX registers until RLC is idle. During the test, it took 12 seconds for RLC to clear the BUSY bit in RLC_GPM_STAT register which is un-acceptable for driver. As per RLC engineer, it would take RLC Ucode less than 10,000 GFXCLK cycles to finish its critical section. In a lowest 300M enginer clock setting(default from vbios), 50 us delay is enough. This commit fix the hang when RLC introduce the work around for XGMI which requires more cycles to setup more registers than normal Signed-off-by: shaoyunl Acked-by: Felix Kuehling Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index ef00d14f8645..325e2213cac5 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -2243,12 +2243,13 @@ static void gfx_v9_0_rlc_start(struct amdgpu_device *adev) #endif WREG32_FIELD15(GC, 0, RLC_CNTL, RLC_ENABLE_F32, 1); + udelay(50); /* carrizo do enable cp interrupt after cp inited */ - if (!(adev->flags & AMD_IS_APU)) + if (!(adev->flags & AMD_IS_APU)) { gfx_v9_0_enable_gui_idle_interrupt(adev, true); - - udelay(50); + udelay(50); + } #ifdef AMDGPU_RLC_DEBUG_RETRY /* RLC_GPM_GENERAL_6 : RLC Ucode version */ -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 10/10] drm/amdgpu: disable IH ring 1 & 2 WPTR overflow on Vega10
That should add back pressure on the client. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c index 4a753e40a837..b65515e6dff9 100644 --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c @@ -205,6 +205,8 @@ static int vega10_ih_irq_init(struct amdgpu_device *adev) ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1); ih_rb_cntl = vega10_ih_rb_cntl(ih, ih_rb_cntl); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1, + WPTR_OVERFLOW_ENABLE, 0); WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl); /* set rptr, wptr to 0 */ @@ -220,6 +222,8 @@ static int vega10_ih_irq_init(struct amdgpu_device *adev) ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1); ih_rb_cntl = vega10_ih_rb_cntl(ih, ih_rb_cntl); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2, + WPTR_OVERFLOW_ENABLE, 0); WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl); /* set rptr, wptr to 0 */ -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 02/10] drm/amdgpu: remove VM fault_credit handling
printk_ratelimit() is much better suited to limit the number of reported VM faults. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 37 - drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 5 drivers/gpu/drm/amd/amdgpu/cik_ih.c | 18 +--- drivers/gpu/drm/amd/amdgpu/cz_ih.c | 18 +--- drivers/gpu/drm/amd/amdgpu/iceland_ih.c | 18 +--- drivers/gpu/drm/amd/amdgpu/tonga_ih.c | 18 +--- drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 7 ++--- 7 files changed, 6 insertions(+), 115 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index f059ec314f2b..e73d152659a2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -3054,7 +3054,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, } INIT_KFIFO(vm->faults); - vm->fault_credit = 16; return 0; @@ -3266,42 +3265,6 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm) amdgpu_vmid_free_reserved(adev, vm, i); } -/** - * amdgpu_vm_pasid_fault_credit - Check fault credit for given PASID - * - * @adev: amdgpu_device pointer - * @pasid: PASID do identify the VM - * - * This function is expected to be called in interrupt context. - * - * Returns: - * True if there was fault credit, false otherwise - */ -bool amdgpu_vm_pasid_fault_credit(struct amdgpu_device *adev, - unsigned int pasid) -{ - struct amdgpu_vm *vm; - - spin_lock(&adev->vm_manager.pasid_lock); - vm = idr_find(&adev->vm_manager.pasid_idr, pasid); - if (!vm) { - /* VM not found, can't track fault credit */ - spin_unlock(&adev->vm_manager.pasid_lock); - return true; - } - - /* No lock needed. only accessed by IRQ handler */ - if (!vm->fault_credit) { - /* Too many faults in this VM */ - spin_unlock(&adev->vm_manager.pasid_lock); - return false; - } - - vm->fault_credit--; - spin_unlock(&adev->vm_manager.pasid_lock); - return true; -} - /** * amdgpu_vm_manager_init - init the VM manager * diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 2a8898d19c8b..e8dcfd59fc93 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h @@ -229,9 +229,6 @@ struct amdgpu_vm { /* Up to 128 pending retry page faults */ DECLARE_KFIFO(faults, u64, 128); - /* Limit non-retry fault storms */ - unsigned intfault_credit; - /* Points to the KFD process VM info */ struct amdkfd_process_info *process_info; @@ -299,8 +296,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, unsigned int pasid); void amdgpu_vm_release_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm); void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm); -bool amdgpu_vm_pasid_fault_credit(struct amdgpu_device *adev, - unsigned int pasid); void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm, struct list_head *validated, struct amdgpu_bo_list_entry *entry); diff --git a/drivers/gpu/drm/amd/amdgpu/cik_ih.c b/drivers/gpu/drm/amd/amdgpu/cik_ih.c index b5775c6a857b..3e6c8c4067cb 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/cik_ih.c @@ -237,23 +237,7 @@ static u32 cik_ih_get_wptr(struct amdgpu_device *adev) */ static bool cik_ih_prescreen_iv(struct amdgpu_device *adev) { - u32 ring_index = adev->irq.ih.rptr >> 2; - u16 pasid; - - switch (le32_to_cpu(adev->irq.ih.ring[ring_index]) & 0xff) { - case 146: - case 147: - pasid = le32_to_cpu(adev->irq.ih.ring[ring_index + 2]) >> 16; - if (!pasid || amdgpu_vm_pasid_fault_credit(adev, pasid)) - return true; - break; - default: - /* Not a VM fault */ - return true; - } - - adev->irq.ih.rptr += 16; - return false; + return true; } /** diff --git a/drivers/gpu/drm/amd/amdgpu/cz_ih.c b/drivers/gpu/drm/amd/amdgpu/cz_ih.c index df5ac4d85a00..447b3cbc47e5 100644 --- a/drivers/gpu/drm/amd/amdgpu/cz_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/cz_ih.c @@ -216,23 +216,7 @@ static u32 cz_ih_get_wptr(struct amdgpu_device *adev) */ static bool cz_ih_prescreen_iv(struct amdgpu_device *adev) { - u32 ring_index = adev->irq.ih.rptr >> 2; - u16 pasid; - - switch (le32_to_cpu(adev->irq.ih.ring[ring_index]) & 0xff) { - case 146: - case 147: - pasid = le32_to_cpu(adev->irq.ih.ring[ring_index + 2]) >> 16; -
[PATCH 04/10] drm/amdgpu: add IH ring to ih_get_wptr/ih_set_rptr v2
Let's start to support multiple rings. v2: decode IV is needed as well Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 6 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 13 +++--- drivers/gpu/drm/amd/amdgpu/cik_ih.c | 29 +++-- drivers/gpu/drm/amd/amdgpu/cz_ih.c | 31 +++--- drivers/gpu/drm/amd/amdgpu/iceland_ih.c | 29 +++-- drivers/gpu/drm/amd/amdgpu/si_ih.c | 31 +++--- drivers/gpu/drm/amd/amdgpu/tonga_ih.c | 43 ++- drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 56 + 8 files changed, 128 insertions(+), 110 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c index 8af67f649660..fb8dd6179926 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c @@ -137,7 +137,7 @@ int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, if (!ih->enabled || adev->shutdown) return IRQ_NONE; - wptr = amdgpu_ih_get_wptr(adev); + wptr = amdgpu_ih_get_wptr(adev, ih); restart_ih: /* is somebody else already processing irqs? */ @@ -154,11 +154,11 @@ int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, ih->rptr &= ih->ptr_mask; } - amdgpu_ih_set_rptr(adev); + amdgpu_ih_set_rptr(adev, ih); atomic_set(&ih->lock, 0); /* make sure wptr hasn't changed while processing */ - wptr = amdgpu_ih_get_wptr(adev); + wptr = amdgpu_ih_get_wptr(adev, ih); if (wptr != ih->rptr) goto restart_ih; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h index f877bb78d10a..d810fd73d574 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h @@ -50,15 +50,16 @@ struct amdgpu_ih_ring { /* provided by the ih block */ struct amdgpu_ih_funcs { /* ring read/write ptr handling, called from interrupt context */ - u32 (*get_wptr)(struct amdgpu_device *adev); - void (*decode_iv)(struct amdgpu_device *adev, + u32 (*get_wptr)(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih); + void (*decode_iv)(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, struct amdgpu_iv_entry *entry); - void (*set_rptr)(struct amdgpu_device *adev); + void (*set_rptr)(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih); }; -#define amdgpu_ih_get_wptr(adev) (adev)->irq.ih_funcs->get_wptr((adev)) -#define amdgpu_ih_decode_iv(adev, iv) (adev)->irq.ih_funcs->decode_iv((adev), (iv)) -#define amdgpu_ih_set_rptr(adev) (adev)->irq.ih_funcs->set_rptr((adev)) +#define amdgpu_ih_get_wptr(adev, ih) (adev)->irq.ih_funcs->get_wptr((adev), (ih)) +#define amdgpu_ih_decode_iv(adev, iv) \ + (adev)->irq.ih_funcs->decode_iv((adev), (ih), (iv)) +#define amdgpu_ih_set_rptr(adev, ih) (adev)->irq.ih_funcs->set_rptr((adev), (ih)) int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, unsigned ring_size, bool use_bus_addr); diff --git a/drivers/gpu/drm/amd/amdgpu/cik_ih.c b/drivers/gpu/drm/amd/amdgpu/cik_ih.c index 8a8b4967a101..884aa9b81e86 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/cik_ih.c @@ -183,11 +183,12 @@ static void cik_ih_irq_disable(struct amdgpu_device *adev) * Used by cik_irq_process(). * Returns the value of the wptr. */ -static u32 cik_ih_get_wptr(struct amdgpu_device *adev) +static u32 cik_ih_get_wptr(struct amdgpu_device *adev, + struct amdgpu_ih_ring *ih) { u32 wptr, tmp; - wptr = le32_to_cpu(adev->wb.wb[adev->irq.ih.wptr_offs]); + wptr = le32_to_cpu(adev->wb.wb[ih->wptr_offs]); if (wptr & IH_RB_WPTR__RB_OVERFLOW_MASK) { wptr &= ~IH_RB_WPTR__RB_OVERFLOW_MASK; @@ -196,13 +197,13 @@ static u32 cik_ih_get_wptr(struct amdgpu_device *adev) * this should allow us to catchup. */ dev_warn(adev->dev, "IH ring buffer overflow (0x%08X, 0x%08X, 0x%08X)\n", - wptr, adev->irq.ih.rptr, (wptr + 16) & adev->irq.ih.ptr_mask); - adev->irq.ih.rptr = (wptr + 16) & adev->irq.ih.ptr_mask; +wptr, ih->rptr, (wptr + 16) & ih->ptr_mask); + ih->rptr = (wptr + 16) & ih->ptr_mask; tmp = RREG32(mmIH_RB_CNTL); tmp |= IH_RB_CNTL__WPTR_OVERFLOW_CLEAR_MASK; WREG32(mmIH_RB_CNTL, tmp); } - return (wptr & adev->irq.ih.ptr_mask); + return (wptr & ih->ptr_mask); } /*CIK IV Ring @@ -237,16 +238,17 @@ static u32 cik_ih_get_wptr(struct amdgpu_device *adev) * position and also advance the position. */ static void cik_ih_decode_iv(struct amdgpu_device *adev, +struct am
[PATCH 09/10] drm/amdgpu: add support for self irq on Vega10
This finally enables processing of ring 1 & 2. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 68 -- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c index 22638192d7dd..4a753e40a837 100644 --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c @@ -272,7 +272,7 @@ static void vega10_ih_irq_disable(struct amdgpu_device *adev) static u32 vega10_ih_get_wptr(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih) { - u32 wptr, tmp; + u32 wptr, reg, tmp; wptr = le32_to_cpu(*ih->wptr_cpu); @@ -288,9 +288,18 @@ static u32 vega10_ih_get_wptr(struct amdgpu_device *adev, wptr, ih->rptr, tmp); ih->rptr = tmp; - tmp = RREG32_NO_KIQ(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL)); + if (ih == &adev->irq.ih) + reg = SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL); + else if (ih == &adev->irq.ih1) + reg = SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL); + else if (ih == &adev->irq.ih2) + reg = SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL); + else + BUG(); + + tmp = RREG32_NO_KIQ(reg); tmp = REG_SET_FIELD(tmp, IH_RB_CNTL, WPTR_OVERFLOW_CLEAR, 1); - WREG32_NO_KIQ(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_CNTL), tmp); + WREG32_NO_KIQ(reg, tmp); } return (wptr & ih->ptr_mask); } @@ -352,9 +361,52 @@ static void vega10_ih_set_rptr(struct amdgpu_device *adev, /* XXX check if swapping is necessary on BE */ *ih->rptr_cpu = ih->rptr; WDOORBELL32(ih->doorbell_index, ih->rptr); - } else { + } else if (ih == &adev->irq.ih) { WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR, ih->rptr); + } else if (ih == &adev->irq.ih1) { + WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING1, ih->rptr); + } else if (ih == &adev->irq.ih2) { + WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING2, ih->rptr); + } +} + +/** + * vega10_ih_self_irq - dispatch work for ring 1 and 2 + * + * @adev: amdgpu_device pointer + * @source: irq source + * @entry: IV with WPTR update + * + * Update the WPTR from the IV and schedule work to handle the entries. + */ +static int vega10_ih_self_irq(struct amdgpu_device *adev, + struct amdgpu_irq_src *source, + struct amdgpu_iv_entry *entry) +{ + uint32_t wptr = cpu_to_le32(entry->src_data[0]); + + switch (entry->ring_id) { + case 1: + *adev->irq.ih1.wptr_cpu = wptr; + schedule_work(&adev->irq.ih1_work); + break; + case 2: + *adev->irq.ih2.wptr_cpu = wptr; + schedule_work(&adev->irq.ih2_work); + break; + default: break; } + return 0; +} + +static const struct amdgpu_irq_src_funcs vega10_ih_self_irq_funcs = { + .process = vega10_ih_self_irq, +}; + +static void vega10_ih_set_self_irq_funcs(struct amdgpu_device *adev) +{ + adev->irq.self_irq.num_types = 0; + adev->irq.self_irq.funcs = &vega10_ih_self_irq_funcs; } static int vega10_ih_early_init(void *handle) @@ -362,13 +414,19 @@ static int vega10_ih_early_init(void *handle) struct amdgpu_device *adev = (struct amdgpu_device *)handle; vega10_ih_set_interrupt_funcs(adev); + vega10_ih_set_self_irq_funcs(adev); return 0; } static int vega10_ih_sw_init(void *handle) { - int r; struct amdgpu_device *adev = (struct amdgpu_device *)handle; + int r; + + r = amdgpu_irq_add_id(adev, SOC15_IH_CLIENTID_IH, 0, + &adev->irq.self_irq); + if (r) + return r; r = amdgpu_ih_ring_init(adev, &adev->irq.ih, 256 * 1024, true); if (r) -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 01/10] drm/amdgpu: send IVs to the KFD only after processing them v3
This allows us to filter out VM faults in the GMC code. v2: don't filter out all faults v3: fix copy&paste typo, send all IV to the KFD, don't change message level Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 38 +++-- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index 6b6524f04ce0..79b6f456f2c5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c @@ -149,9 +149,6 @@ static void amdgpu_irq_callback(struct amdgpu_device *adev, if (!amdgpu_ih_prescreen_iv(adev)) return; - /* Before dispatching irq to IP blocks, send it to amdkfd */ - amdgpu_amdkfd_interrupt(adev, (const void *) &ih->ring[ring_index]); - entry.iv_entry = (const uint32_t *)&ih->ring[ring_index]; amdgpu_ih_decode_iv(adev, &entry); @@ -371,39 +368,38 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev, unsigned client_id = entry->client_id; unsigned src_id = entry->src_id; struct amdgpu_irq_src *src; + bool handled = false; int r; trace_amdgpu_iv(entry); if (client_id >= AMDGPU_IRQ_CLIENTID_MAX) { DRM_DEBUG("Invalid client_id in IV: %d\n", client_id); - return; - } - if (src_id >= AMDGPU_MAX_IRQ_SRC_ID) { + } else if (src_id >= AMDGPU_MAX_IRQ_SRC_ID) { DRM_DEBUG("Invalid src_id in IV: %d\n", src_id); - return; - } - if (adev->irq.virq[src_id]) { + } else if (adev->irq.virq[src_id]) { generic_handle_irq(irq_find_mapping(adev->irq.domain, src_id)); - } else { - if (!adev->irq.client[client_id].sources) { - DRM_DEBUG("Unregistered interrupt client_id: %d src_id: %d\n", - client_id, src_id); - return; - } - src = adev->irq.client[client_id].sources[src_id]; - if (!src) { - DRM_DEBUG("Unhandled interrupt src_id: %d\n", src_id); - return; - } + } else if (!adev->irq.client[client_id].sources) { + DRM_DEBUG("Unregistered interrupt client_id: %d src_id: %d\n", + client_id, src_id); + } else if ((src = adev->irq.client[client_id].sources[src_id])) { r = src->funcs->process(adev, src, entry); - if (r) + if (r < 0) DRM_ERROR("error processing interrupt (%d)\n", r); + else if (r) + handled = true; + + } else { + DRM_DEBUG("Unhandled interrupt src_id: %d\n", src_id); } + + /* Send it to amdkfd as well if it isn't already handled */ + if (!handled) + amdgpu_amdkfd_interrupt(adev, entry->iv_entry); } /** -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 03/10] drm/amdgpu: move IV prescreening into the GMC code
The GMC/VM subsystem is causing the faults, so move the handling here as well. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 - drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 4 -- drivers/gpu/drm/amd/amdgpu/cik_ih.c | 13 drivers/gpu/drm/amd/amdgpu/cz_ih.c | 13 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 59 ++ drivers/gpu/drm/amd/amdgpu/iceland_ih.c | 13 drivers/gpu/drm/amd/amdgpu/si_ih.c | 14 - drivers/gpu/drm/amd/amdgpu/tonga_ih.c | 13 drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 82 - 9 files changed, 59 insertions(+), 154 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h index 9ce8c93ec19b..f877bb78d10a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h @@ -51,14 +51,12 @@ struct amdgpu_ih_ring { struct amdgpu_ih_funcs { /* ring read/write ptr handling, called from interrupt context */ u32 (*get_wptr)(struct amdgpu_device *adev); - bool (*prescreen_iv)(struct amdgpu_device *adev); void (*decode_iv)(struct amdgpu_device *adev, struct amdgpu_iv_entry *entry); void (*set_rptr)(struct amdgpu_device *adev); }; #define amdgpu_ih_get_wptr(adev) (adev)->irq.ih_funcs->get_wptr((adev)) -#define amdgpu_ih_prescreen_iv(adev) (adev)->irq.ih_funcs->prescreen_iv((adev)) #define amdgpu_ih_decode_iv(adev, iv) (adev)->irq.ih_funcs->decode_iv((adev), (iv)) #define amdgpu_ih_set_rptr(adev) (adev)->irq.ih_funcs->set_rptr((adev)) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index 79b6f456f2c5..b7968f426862 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c @@ -145,10 +145,6 @@ static void amdgpu_irq_callback(struct amdgpu_device *adev, u32 ring_index = ih->rptr >> 2; struct amdgpu_iv_entry entry; - /* Prescreening of high-frequency interrupts */ - if (!amdgpu_ih_prescreen_iv(adev)) - return; - entry.iv_entry = (const uint32_t *)&ih->ring[ring_index]; amdgpu_ih_decode_iv(adev, &entry); diff --git a/drivers/gpu/drm/amd/amdgpu/cik_ih.c b/drivers/gpu/drm/amd/amdgpu/cik_ih.c index 3e6c8c4067cb..8a8b4967a101 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/cik_ih.c @@ -228,18 +228,6 @@ static u32 cik_ih_get_wptr(struct amdgpu_device *adev) * [127:96] - reserved */ -/** - * cik_ih_prescreen_iv - prescreen an interrupt vector - * - * @adev: amdgpu_device pointer - * - * Returns true if the interrupt vector should be further processed. - */ -static bool cik_ih_prescreen_iv(struct amdgpu_device *adev) -{ - return true; -} - /** * cik_ih_decode_iv - decode an interrupt vector * @@ -445,7 +433,6 @@ static const struct amd_ip_funcs cik_ih_ip_funcs = { static const struct amdgpu_ih_funcs cik_ih_funcs = { .get_wptr = cik_ih_get_wptr, - .prescreen_iv = cik_ih_prescreen_iv, .decode_iv = cik_ih_decode_iv, .set_rptr = cik_ih_set_rptr }; diff --git a/drivers/gpu/drm/amd/amdgpu/cz_ih.c b/drivers/gpu/drm/amd/amdgpu/cz_ih.c index 447b3cbc47e5..9d3ea298e116 100644 --- a/drivers/gpu/drm/amd/amdgpu/cz_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/cz_ih.c @@ -207,18 +207,6 @@ static u32 cz_ih_get_wptr(struct amdgpu_device *adev) return (wptr & adev->irq.ih.ptr_mask); } -/** - * cz_ih_prescreen_iv - prescreen an interrupt vector - * - * @adev: amdgpu_device pointer - * - * Returns true if the interrupt vector should be further processed. - */ -static bool cz_ih_prescreen_iv(struct amdgpu_device *adev) -{ - return true; -} - /** * cz_ih_decode_iv - decode an interrupt vector * @@ -426,7 +414,6 @@ static const struct amd_ip_funcs cz_ih_ip_funcs = { static const struct amdgpu_ih_funcs cz_ih_funcs = { .get_wptr = cz_ih_get_wptr, - .prescreen_iv = cz_ih_prescreen_iv, .decode_iv = cz_ih_decode_iv, .set_rptr = cz_ih_set_rptr }; diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c index 016c7aab4a29..ce150de723c9 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c @@ -244,6 +244,62 @@ static int gmc_v9_0_vm_fault_interrupt_state(struct amdgpu_device *adev, return 0; } +/** + * vega10_ih_prescreen_iv - prescreen an interrupt vector + * + * @adev: amdgpu_device pointer + * + * Returns true if the interrupt vector should be further processed. + */ +static bool gmc_v9_0_prescreen_iv(struct amdgpu_device *adev, + struct amdgpu_iv_entry *entry, + uint64_t addr) +{ + struct amdgpu_vm *vm; + u64 key; + int r; + + /* No PASID, can't identify faulting process */ + if (!entry->pasid) + return tr
[PATCH 05/10] drm/amdgpu: simplify IH programming
Calculate all the addresses and pointers in amdgpu_ih.c Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 34 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 23 +--- drivers/gpu/drm/amd/amdgpu/cik_ih.c | 9 +++ drivers/gpu/drm/amd/amdgpu/cz_ih.c | 11 drivers/gpu/drm/amd/amdgpu/iceland_ih.c | 9 +++ drivers/gpu/drm/amd/amdgpu/si_ih.c | 9 +++ drivers/gpu/drm/amd/amdgpu/tonga_ih.c | 27 +-- drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 36 + 8 files changed, 73 insertions(+), 85 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c index fb8dd6179926..d0a5db777b6d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c @@ -52,6 +52,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, ih->use_bus_addr = use_bus_addr; if (use_bus_addr) { + dma_addr_t dma_addr; + if (ih->ring) return 0; @@ -59,21 +61,26 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, * add them to the end of the ring allocation. */ ih->ring = dma_alloc_coherent(adev->dev, ih->ring_size + 8, - &ih->rb_dma_addr, GFP_KERNEL); + &dma_addr, GFP_KERNEL); if (ih->ring == NULL) return -ENOMEM; memset((void *)ih->ring, 0, ih->ring_size + 8); - ih->wptr_offs = (ih->ring_size / 4) + 0; - ih->rptr_offs = (ih->ring_size / 4) + 1; + ih->gpu_addr = dma_addr; + ih->wptr_addr = dma_addr + ih->ring_size; + ih->wptr_cpu = &ih->ring[ih->ring_size / 4]; + ih->rptr_addr = dma_addr + ih->ring_size + 4; + ih->rptr_cpu = &ih->ring[(ih->ring_size / 4) + 1]; } else { - r = amdgpu_device_wb_get(adev, &ih->wptr_offs); + unsigned wptr_offs, rptr_offs; + + r = amdgpu_device_wb_get(adev, &wptr_offs); if (r) return r; - r = amdgpu_device_wb_get(adev, &ih->rptr_offs); + r = amdgpu_device_wb_get(adev, &rptr_offs); if (r) { - amdgpu_device_wb_free(adev, ih->wptr_offs); + amdgpu_device_wb_free(adev, wptr_offs); return r; } @@ -82,10 +89,15 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, &ih->ring_obj, &ih->gpu_addr, (void **)&ih->ring); if (r) { - amdgpu_device_wb_free(adev, ih->rptr_offs); - amdgpu_device_wb_free(adev, ih->wptr_offs); + amdgpu_device_wb_free(adev, rptr_offs); + amdgpu_device_wb_free(adev, wptr_offs); return r; } + + ih->wptr_addr = adev->wb.gpu_addr + wptr_offs * 4; + ih->wptr_cpu = &adev->wb.wb[wptr_offs]; + ih->rptr_addr = adev->wb.gpu_addr + rptr_offs * 4; + ih->rptr_cpu = &adev->wb.wb[rptr_offs]; } return 0; } @@ -109,13 +121,13 @@ void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih) * add them to the end of the ring allocation. */ dma_free_coherent(adev->dev, ih->ring_size + 8, - (void *)ih->ring, ih->rb_dma_addr); + (void *)ih->ring, ih->gpu_addr); ih->ring = NULL; } else { amdgpu_bo_free_kernel(&ih->ring_obj, &ih->gpu_addr, (void **)&ih->ring); - amdgpu_device_wb_free(adev, ih->wptr_offs); - amdgpu_device_wb_free(adev, ih->rptr_offs); + amdgpu_device_wb_free(adev, (ih->wptr_addr - ih->gpu_addr) / 4); + amdgpu_device_wb_free(adev, (ih->rptr_addr - ih->gpu_addr) / 4); } } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h index d810fd73d574..1ccb1831382a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h @@ -31,20 +31,25 @@ struct amdgpu_iv_entry; * R6xx+ IH ring */ struct amdgpu_ih_ring { - struct amdgpu_bo*ring_obj; - volatile uint32_t *ring; - unsignedrptr; unsignedring_size; - uint64_tgpu_addr; uint32_tptr_mask; - atomic_tlock; - b
[PATCH 07/10] drm/amdgpu: add the IH to the IV trace
To distinct on which IH ring an IV was found. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 11 +++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index b7968f426862..b8e543e23166 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c @@ -148,6 +148,8 @@ static void amdgpu_irq_callback(struct amdgpu_device *adev, entry.iv_entry = (const uint32_t *)&ih->ring[ring_index]; amdgpu_ih_decode_iv(adev, &entry); + trace_amdgpu_iv(ih - &adev->irq.ih, &entry); + amdgpu_irq_dispatch(adev, &entry); } @@ -367,8 +369,6 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev, bool handled = false; int r; - trace_amdgpu_iv(entry); - if (client_id >= AMDGPU_IRQ_CLIENTID_MAX) { DRM_DEBUG("Invalid client_id in IV: %d\n", client_id); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h index 626abca770a0..6e388d7753c1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h @@ -76,9 +76,10 @@ TRACE_EVENT(amdgpu_mm_wreg, ); TRACE_EVENT(amdgpu_iv, - TP_PROTO(struct amdgpu_iv_entry *iv), - TP_ARGS(iv), + TP_PROTO(unsigned ih, struct amdgpu_iv_entry *iv), + TP_ARGS(ih, iv), TP_STRUCT__entry( +__field(unsigned, ih) __field(unsigned, client_id) __field(unsigned, src_id) __field(unsigned, ring_id) @@ -90,6 +91,7 @@ TRACE_EVENT(amdgpu_iv, __array(unsigned, src_data, 4) ), TP_fast_assign( + __entry->ih = ih; __entry->client_id = iv->client_id; __entry->src_id = iv->src_id; __entry->ring_id = iv->ring_id; @@ -103,8 +105,9 @@ TRACE_EVENT(amdgpu_iv, __entry->src_data[2] = iv->src_data[2]; __entry->src_data[3] = iv->src_data[3]; ), - TP_printk("client_id:%u src_id:%u ring:%u vmid:%u timestamp: %llu pasid:%u src_data: %08x %08x %08x %08x", - __entry->client_id, __entry->src_id, + TP_printk("ih: %u client_id:%u src_id:%u ring:%u vmid:%u " + "timestamp: %llu pasid:%u src_data: %08x %08x %08x %08x", + __entry->ih, __entry->client_id, __entry->src_id, __entry->ring_id, __entry->vmid, __entry->timestamp, __entry->pasid, __entry->src_data[0], __entry->src_data[1], -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 06/10] drm/amdgpu: enable IH ring 1 and ring 2 v3
The entries are ignored for now, but it at least stops crashing the hardware when somebody tries to push something to the other IH rings. v2: limit ring size, add TODO comment v3: only program rings if they are actually allocated Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h | 4 +- drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 143 2 files changed, 121 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h index f6ce171cb8aa..7e06fa64321a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h @@ -87,8 +87,8 @@ struct amdgpu_irq { /* status, etc. */ boolmsi_enabled; /* msi enabled */ - /* interrupt ring */ - struct amdgpu_ih_ring ih; + /* interrupt rings */ + struct amdgpu_ih_ring ih, ih1, ih2; const struct amdgpu_ih_funcs*ih_funcs; /* gen irq stuff */ diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c index 3e9ebb0de94d..22638192d7dd 100644 --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c @@ -50,6 +50,22 @@ static void vega10_ih_enable_interrupts(struct amdgpu_device *adev) ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, ENABLE_INTR, 1); WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL, ih_rb_cntl); adev->irq.ih.enabled = true; + + if (adev->irq.ih1.ring_size) { + ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1, + RB_ENABLE, 1); + WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl); + adev->irq.ih1.enabled = true; + } + + if (adev->irq.ih2.ring_size) { + ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2, + RB_ENABLE, 1); + WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl); + adev->irq.ih2.enabled = true; + } } /** @@ -71,6 +87,53 @@ static void vega10_ih_disable_interrupts(struct amdgpu_device *adev) WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR, 0); adev->irq.ih.enabled = false; adev->irq.ih.rptr = 0; + + if (adev->irq.ih1.ring_size) { + ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1, + RB_ENABLE, 0); + WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl); + /* set rptr, wptr to 0 */ + WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING1, 0); + WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR_RING1, 0); + adev->irq.ih1.enabled = false; + adev->irq.ih1.rptr = 0; + } + + if (adev->irq.ih2.ring_size) { + ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2, + RB_ENABLE, 0); + WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl); + /* set rptr, wptr to 0 */ + WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING2, 0); + WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR_RING2, 0); + adev->irq.ih2.enabled = false; + adev->irq.ih2.rptr = 0; + } +} + +static uint32_t vega10_ih_rb_cntl(struct amdgpu_ih_ring *ih, uint32_t ih_rb_cntl) +{ + int rb_bufsz = order_base_2(ih->ring_size / 4); + + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, + MC_SPACE, ih->use_bus_addr ? 1 : 4); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, + WPTR_OVERFLOW_CLEAR, 1); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, + WPTR_OVERFLOW_ENABLE, 1); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, RB_SIZE, rb_bufsz); + /* Ring Buffer write pointer writeback. If enabled, IH_RB_WPTR register +* value is written to memory +*/ + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, + WPTR_WRITEBACK_ENABLE, 1); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_SNOOP, 1); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_RO, 0); + ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_VMID, 0); + + return ih_rb_cntl; } /** @@ -86,9 +149,8 @@ static void vega10_ih_disable_interrupts(struct amdgpu_device *adev) */ static int vega10_ih_irq_init(struct amdgpu_device *adev) { - struct amdgpu_ih_ring *ih = &adev->irq.ih; + struct a
[PATCH 08/10] drm/amdgpu: add support for processing IH ring 1 & 2
Previously we only added the ring buffer memory, now add the handling as well. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 33 + drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h | 4 ++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index b8e543e23166..8bfb3dab46f7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c @@ -176,6 +176,36 @@ irqreturn_t amdgpu_irq_handler(int irq, void *arg) return ret; } +/** + * amdgpu_irq_handle_ih1 - kick of processing for IH1 + * + * @work: work structure in struct amdgpu_irq + * + * Kick of processing IH ring 1. + */ +static void amdgpu_irq_handle_ih1(struct work_struct *work) +{ + struct amdgpu_device *adev = container_of(work, struct amdgpu_device, + irq.ih1_work); + + amdgpu_ih_process(adev, &adev->irq.ih1, amdgpu_irq_callback); +} + +/** + * amdgpu_irq_handle_ih2 - kick of processing for IH2 + * + * @work: work structure in struct amdgpu_irq + * + * Kick of processing IH ring 2. + */ +static void amdgpu_irq_handle_ih2(struct work_struct *work) +{ + struct amdgpu_device *adev = container_of(work, struct amdgpu_device, + irq.ih2_work); + + amdgpu_ih_process(adev, &adev->irq.ih2, amdgpu_irq_callback); +} + /** * amdgpu_msi_ok - check whether MSI functionality is enabled * @@ -240,6 +270,9 @@ int amdgpu_irq_init(struct amdgpu_device *adev) amdgpu_hotplug_work_func); } + INIT_WORK(&adev->irq.ih1_work, amdgpu_irq_handle_ih1); + INIT_WORK(&adev->irq.ih2_work, amdgpu_irq_handle_ih2); + adev->irq.installed = true; r = drm_irq_install(adev->ddev, adev->ddev->pdev->irq); if (r) { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h index 7e06fa64321a..c27decfda494 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h @@ -89,7 +89,9 @@ struct amdgpu_irq { /* interrupt rings */ struct amdgpu_ih_ring ih, ih1, ih2; - const struct amdgpu_ih_funcs*ih_funcs; + const struct amdgpu_ih_funcs*ih_funcs; + struct work_struct ih1_work, ih2_work; + struct amdgpu_irq_src self_irq; /* gen irq stuff */ struct irq_domain *domain; /* GPU irq controller domain */ -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions
Hi Andrzej, On Wednesday, 5 December 2018 10:46:40 EET Andrzej Hajda wrote: > On 05.12.2018 07:32, Laurent Pinchart wrote: > > On Tuesday, 4 December 2018 21:13:20 EET Ville Syrjälä wrote: > >> On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote: > >>> On 03.12.2018 22:38, Ville Syrjälä wrote: > On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote: > > On 21.11.2018 19:19, Laurent Pinchart wrote: > >> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote: > >>> From: Ville Syrjälä > >>> > >>> Make life easier for drivers by simply passing the connector > >>> to drm_hdmi_avi_infoframe_from_display_mode() and > >>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't > >>> need to worry about is_hdmi2_sink mess. > >> > >> While this is good for display controller drivers, the change isn't > >> great for bridge drivers. Down the road we're looking at moving > >> connector support out of the bridge drivers. Adding an additional > >> dependency to connectors in the bridges will make that more > >> difficult. Ideally bridges should retrieve the information from their > >> sink, regardless of whether it is a connector or another bridge. > > > > I agree with it, and case of sii8620 shows that there are cases where > > bridge has no direct access to the connector. > > It's just a matter of plumbing it through. > >>> > >>> What do you mean exactly? > >> > >> void bridge_foo(... > >> + ,struct drm_connector *connector); > >> > > On the other side, since you are passing connector to > > drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode > > parameter and rename the function to > > drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and > > mode set on the connector differs? > > Connectors don't have a mode. > >>> > >>> As they are passing video stream they should have it, even if not > >>> directly, for example: > >>> > >>> connector->state->crtc->mode > >> > >> That's not really how atomic works. One shouldn't go digging > >> through the obj->state pointers when we're not holding the > >> relevant locks anymore. The atomic way would be to pass either > >> both crtc state and connector state, or drm_atomic_state + > >> crtc/connector. > > Usually infoframe contents is generated in modesetting/enable callbacks > so the locks should be in place. > > And the locks should be hold for > drm_hdmi_avi_infoframe_from_display_mode as well - it wouldn't be correct if > > generated infoframe is not relevant to actual video mode. I guess that > if connector->state->crtc->mode > > differs from mode passed to drm_hdmi_avi_infoframe_from_display_mode it > is a sign of possible problem. > > And if they do not differ passing mode as an extra argument is redundant. > > > Or a bridge state ? With chained bridges the mode can vary along the > > pipeline, the CRTC adjusted mode will only cover the link between the > > CRTC and the first bridge. It's only a matter of time until we need to > > store other intermediate modes in states. I'd rather prepare for that > > instead of passing the CRTC state to bridges. > > Yes, optional bridge states seems reasonable, volunteers needed :) I'll give it a go eventually, if nobody beats me to it. The exact timing will depend on many variables so I can't estimate it I'm afraid. All I'm asking is to avoid extending the drm_bridge API in a way that would make introduction of bridge states more complex. If someone needs bridge states, for instance to solve the above issue, then they should be added. [snip] -- Regards, Laurent Pinchart ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu/acpi: NULL check before some freeing functions is not needed
kfree(NULL) is safe, so removes NULL check before freeing the mem. This patch also fix the ifnullfree.cocci warnings. Signed-off-by: Wen Yang CC: Alex Deucher CC: christian.koe...@amd.com CC: "David (ChunMing) Zhou" CC: David Airlie (maintainer:DRM DRIVERS) CC: Lyude Paul CC: Rex Zhu CC: Jim Qu CC: amd-gfx@lists.freedesktop.org CC: dri-de...@lists.freedesktop.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index 7f0afc526419..996bfce149f2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -816,6 +816,5 @@ int amdgpu_acpi_init(struct amdgpu_device *adev) void amdgpu_acpi_fini(struct amdgpu_device *adev) { unregister_acpi_notifier(&adev->acpi_nb); - if (adev->atif) - kfree(adev->atif); + kfree(adev->atif); } -- 2.19.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions
On 05.12.2018 07:32, Laurent Pinchart wrote: > Hi Ville, > > On Tuesday, 4 December 2018 21:13:20 EET Ville Syrjälä wrote: >> On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote: >>> On 03.12.2018 22:38, Ville Syrjälä wrote: On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote: > On 21.11.2018 19:19, Laurent Pinchart wrote: >> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote: >>> From: Ville Syrjälä >>> >>> Make life easier for drivers by simply passing the connector >>> to drm_hdmi_avi_infoframe_from_display_mode() and >>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't >>> need to worry about is_hdmi2_sink mess. >> While this is good for display controller drivers, the change isn't >> great for bridge drivers. Down the road we're looking at moving >> connector support out of the bridge drivers. Adding an additional >> dependency to connectors in the bridges will make that more >> difficult. Ideally bridges should retrieve the information from their >> sink, regardless of whether it is a connector or another bridge. > I agree with it, and case of sii8620 shows that there are cases where > bridge has no direct access to the connector. It's just a matter of plumbing it through. >>> What do you mean exactly? >> void bridge_foo(... >> + ,struct drm_connector *connector); >> > On the other side, since you are passing connector to > drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode > parameter and rename the function to > drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and > mode set on the connector differs? Connectors don't have a mode. >>> As they are passing video stream they should have it, even if not >>> directly, for example: >>> >>> connector->state->crtc->mode >> That's not really how atomic works. One shouldn't go digging >> through the obj->state pointers when we're not holding the >> relevant locks anymore. The atomic way would be to pass either >> both crtc state and connector state, or drm_atomic_state + >> crtc/connector. Usually infoframe contents is generated in modesetting/enable callbacks so the locks should be in place. And the locks should be hold for drm_hdmi_avi_infoframe_from_display_mode as well - it wouldn't be correct if generated infoframe is not relevant to actual video mode. I guess that if connector->state->crtc->mode differs from mode passed to drm_hdmi_avi_infoframe_from_display_mode it is a sign of possible problem. And if they do not differ passing mode as an extra argument is redundant. > Or a bridge state ? With chained bridges the mode can vary along the > pipeline, > the CRTC adjusted mode will only cover the link between the CRTC and the > first > bridge. It's only a matter of time until we need to store other intermediate > modes in states. I'd rather prepare for that instead of passing the CRTC > state > to bridges. Yes, optional bridge states seems reasonable, volunteers needed :) Regards Andrzej > >>> In moment of creating infoframe it should be set properly. >>> >> Please see below for an additional comment. >> >>> Cc: Alex Deucher >>> Cc: "Christian König" >>> Cc: "David (ChunMing) Zhou" >>> Cc: Archit Taneja >>> Cc: Andrzej Hajda >>> Cc: Laurent Pinchart >>> Cc: Inki Dae >>> Cc: Joonyoung Shim >> Cc: Seung-Woo Kim >>> Cc: Kyungmin Park >>> Cc: Russell King >>> Cc: CK Hu >>> Cc: Philipp Zabel >>> Cc: Rob Clark >>> Cc: Ben Skeggs >>> Cc: Tomi Valkeinen >>> Cc: Sandy Huang >>> Cc: "Heiko Stübner" >>> Cc: Benjamin Gaignard >>> Cc: Vincent Abriou >>> Cc: Thierry Reding >>> Cc: Eric Anholt >>> Cc: Shawn Guo >>> Cc: Ilia Mirkin >>> Cc: amd-gfx@lists.freedesktop.org >>> Cc: linux-arm-...@vger.kernel.org >>> Cc: freedr...@lists.freedesktop.org >>> Cc: nouv...@lists.freedesktop.org >>> Cc: linux-te...@vger.kernel.org >>> Signed-off-by: Ville Syrjälä >>> --- >>> >>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- >>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- >>> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 3 ++- >>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- >>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 ++-- >>> drivers/gpu/drm/bridge/sii902x.c | 3 ++- >>> drivers/gpu/drm/bridge/sil-sii8620.c | 3 +-- >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 ++- >>> drivers/gpu/drm/drm_edid.c| 33 >>> drivers/gpu/drm/exynos/exynos_hdmi.c | 3 ++- >>> drivers/gpu/drm/i2c/tda998x_drv.c | 3 ++- >>> drivers/gpu/drm/i915/intel_hdmi.c | 14 +- >>> drivers/gpu/drm/i915/intel_lspcon.c | 15 ++- >>> drivers/gpu/drm/i915/intel_sdvo.c |