date:20211103

[PATCH v2] drm/amd/amdgpu: Avoid writing GMC registers under sriov in gmc9

2021-11-03 Thread YuBiao Wang

[Why]
For Vega10, disabling gart of gfxhub could mess up KIQ and PSP
under sriov mode, and lead to DMAR on host side.

[How]
Skip writing GMC registers under sriov.

Signed-off-by: YuBiao Wang 
---
 drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 26 +---
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
index bda1542ef1dd..f9a7349eb601 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -348,18 +348,20 @@ static void gfxhub_v1_0_gart_disable(struct amdgpu_device 
*adev)
WREG32_SOC15_OFFSET(GC, 0, mmVM_CONTEXT0_CNTL,
i * hub->ctx_distance, 0);
 
-   /* Setup TLB control */
-   tmp = RREG32_SOC15(GC, 0, mmMC_VM_MX_L1_TLB_CNTL);
-   tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 0);
-   tmp = REG_SET_FIELD(tmp,
-   MC_VM_MX_L1_TLB_CNTL,
-   ENABLE_ADVANCED_DRIVER_MODEL,
-   0);
-   WREG32_SOC15_RLC(GC, 0, mmMC_VM_MX_L1_TLB_CNTL, tmp);
-
-   /* Setup L2 cache */
-   WREG32_FIELD15(GC, 0, VM_L2_CNTL, ENABLE_L2_CACHE, 0);
-   WREG32_SOC15(GC, 0, mmVM_L2_CNTL3, 0);
+   if (!amdgpu_sriov_vf(adev)) {
+   /* Setup TLB control */
+   tmp = RREG32_SOC15(GC, 0, mmMC_VM_MX_L1_TLB_CNTL);
+   tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 
0);
+   tmp = REG_SET_FIELD(tmp,
+   MC_VM_MX_L1_TLB_CNTL,
+   ENABLE_ADVANCED_DRIVER_MODEL,
+   0);
+   WREG32_SOC15_RLC(GC, 0, mmMC_VM_MX_L1_TLB_CNTL, tmp);
+
+   /* Setup L2 cache */
+   WREG32_FIELD15(GC, 0, VM_L2_CNTL, ENABLE_L2_CACHE, 0);
+   WREG32_SOC15(GC, 0, mmVM_L2_CNTL3, 0);
+   }
 }
 
 /**
-- 
2.25.1

[PATCH] drm/amdgpu: fix SI handling in amdgpu_device_asic_has_dc_support()

2021-11-03 Thread Alex Deucher

Properly handle SI DC support when CONFIG_DRM_AMD_DC_SI is not
set.

Fixes: f7f12b25823c0d ("drm/amdgpu: default to true in 
amdgpu_device_asic_has_dc_support")
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 95fec36e385e..db3728a11481 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3166,11 +3166,21 @@ bool amdgpu_device_asic_has_dc_support(enum 
amd_asic_type asic_type)
 {
switch (asic_type) {
 #if defined(CONFIG_DRM_AMD_DC)
-#if defined(CONFIG_DRM_AMD_DC_SI)
case CHIP_TAHITI:
case CHIP_PITCAIRN:
case CHIP_VERDE:
case CHIP_OLAND:
+   /*
+* We have systems in the wild with these ASICs that require
+* LVDS and VGA support which is not supported with DC.
+*
+* Fallback to the non-DC driver here by default so as not to
+* cause regressions.
+*/
+#if defined(CONFIG_DRM_AMD_DC_SI)
+   return amdgpu_dc > 0;
+#else
+   return false;
 #endif
case CHIP_BONAIRE:
case CHIP_KAVERI:
-- 
2.31.1

Re: [PATCH] drm/amd/amdkfd: Don't sent command to HWS on kfd reset

2021-11-03 Thread Felix Kuehling




On 2021-11-03 11:04 a.m., shaoyunl wrote:

When kfd need to be reset, sent command to HWS might cause hang and get 
unnecessary timeout.
This change try not to touch HW in pre_reset and keep queues to be in the 
evicted state
when the reset is done, so they are not put back on the runlist. These queues 
will be destroied
on process termination.

Signed-off-by: shaoyunl 
---
  drivers/gpu/drm/amd/amdkfd/kfd_device.c   | 6 +-
  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 ++
  drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 6 +-
  4 files changed, 13 insertions(+), 3 deletions(-)
  mode change 100644 => 100755 drivers/gpu/drm/amd/amdkfd/kfd_device.c
  mode change 100644 => 100755 
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
  mode change 100644 => 100755 drivers/gpu/drm/amd/amdkfd/kfd_priv.h
  mode change 100644 => 100755 drivers/gpu/drm/amd/amdkfd/kfd_process.c

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
old mode 100644
new mode 100755
index c8aade17efef..536ef766d09e
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -1100,6 +1100,8 @@ int kgd2kfd_pre_reset(struct kfd_dev *kfd)
if (!kfd->init_complete)
return 0;
  
+	kfd->is_resetting = true;

+
kfd_smi_event_update_gpu_reset(kfd, false);
  
  	kfd->dqm->ops.pre_reset(kfd->dqm);

@@ -1132,6 +1134,8 @@ int kgd2kfd_post_reset(struct kfd_dev *kfd)
  
  	kfd_smi_event_update_gpu_reset(kfd, true);
  
+	kfd->is_resetting = false;

+
return 0;
  }
  
@@ -1168,7 +1172,7 @@ int kgd2kfd_resume(struct kfd_dev *kfd, bool run_pm)

return ret;
  
  	/* for runtime resume, skip unlocking kfd */

-   if (!run_pm) {
+   if (!run_pm && !kfd->is_resetting) {


This is not needed. post_reset calls kfd_resume, not kgd2kfd_resume. It 
should never get here.


Regards,
  Felix



count = atomic_dec_return(_locked);
WARN_ONCE(count < 0, "KFD suspend / resume ref. error");
if (count == 0)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
old mode 100644
new mode 100755
index e9601d4dfb77..0a60317509c8
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1430,7 +1430,7 @@ static int unmap_queues_cpsch(struct device_queue_manager 
*dqm,
  
  	if (!dqm->sched_running)

return 0;
-   if (dqm->is_hws_hang)
+   if (dqm->is_hws_hang || dqm->is_resetting)
return -EIO;
if (!dqm->active_runlist)
return retval;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
old mode 100644
new mode 100755
index bfe7bacccb73..e4bcc2a09ca8
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -275,6 +275,8 @@ struct kfd_dev {
struct device_queue_manager *dqm;
  
  	bool init_complete;

+   bool is_resetting;
+
/*
 * Interrupts of interest to KFD are copied
 * from the HW ring into a SW ring.
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
old mode 100644
new mode 100755
index f8a8fdb95832..f29b3932e3dc
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1715,7 +1715,11 @@ int kfd_process_evict_queues(struct kfd_process *p)
  
  		r = pdd->dev->dqm->ops.evict_process_queues(pdd->dev->dqm,

>qpd);
-   if (r) {
+   /* evict return -EIO if HWS is hang or asic is resetting, in 
this case
+* we would like to set all the queues to be in evicted state 
to prevent
+* them been add back since they actually not be saved right 
now.
+*/
+   if (r && r != -EIO) {
pr_err("Failed to evict process queues\n");
goto fail;
}

[PATCH] drm/amd/amdgpu: Avoid writing GMC registers under sriov in gmc9

2021-11-03 Thread YuBiao Wang

[Why]
For Vega10, disabling gart of gfxhub and mmhub could mess up KIQ and PSP
under sriov mode, and lead to DMAR on host side.

[How]
Skip writing GMC registers under sriov.

Signed-off-by: YuBiao Wang 
---
 drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 26 +---
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
index bda1542ef1dd..f9a7349eb601 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -348,18 +348,20 @@ static void gfxhub_v1_0_gart_disable(struct amdgpu_device 
*adev)
WREG32_SOC15_OFFSET(GC, 0, mmVM_CONTEXT0_CNTL,
i * hub->ctx_distance, 0);
 
-   /* Setup TLB control */
-   tmp = RREG32_SOC15(GC, 0, mmMC_VM_MX_L1_TLB_CNTL);
-   tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 0);
-   tmp = REG_SET_FIELD(tmp,
-   MC_VM_MX_L1_TLB_CNTL,
-   ENABLE_ADVANCED_DRIVER_MODEL,
-   0);
-   WREG32_SOC15_RLC(GC, 0, mmMC_VM_MX_L1_TLB_CNTL, tmp);
-
-   /* Setup L2 cache */
-   WREG32_FIELD15(GC, 0, VM_L2_CNTL, ENABLE_L2_CACHE, 0);
-   WREG32_SOC15(GC, 0, mmVM_L2_CNTL3, 0);
+   if (!amdgpu_sriov_vf(adev)) {
+   /* Setup TLB control */
+   tmp = RREG32_SOC15(GC, 0, mmMC_VM_MX_L1_TLB_CNTL);
+   tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 
0);
+   tmp = REG_SET_FIELD(tmp,
+   MC_VM_MX_L1_TLB_CNTL,
+   ENABLE_ADVANCED_DRIVER_MODEL,
+   0);
+   WREG32_SOC15_RLC(GC, 0, mmMC_VM_MX_L1_TLB_CNTL, tmp);
+
+   /* Setup L2 cache */
+   WREG32_FIELD15(GC, 0, VM_L2_CNTL, ENABLE_L2_CACHE, 0);
+   WREG32_SOC15(GC, 0, mmVM_L2_CNTL3, 0);
+   }
 }
 
 /**
-- 
2.25.1

[pull] amdgpu, amdkfd drm-fixes-5.16

2021-11-03 Thread Alex Deucher

Hi Dave, Daniel,

Fixes for 5.16.

The following changes since commit d9bd054177fbd2c4762546aec40fc3071bfe4cc0:

  Merge tag 'amd-drm-next-5.16-2021-10-29' of 
https://gitlab.freedesktop.org/agd5f/linux into drm-next (2021-11-02 12:40:58 
+1000)

are available in the Git repository at:

  https://gitlab.freedesktop.org/agd5f/linux.git 
tags/amd-drm-fixes-5.16-2021-11-03

for you to fetch changes up to 78469728809b8604dc37ae4e6b12ae12decac5be:

  drm/amd/display: 3.2.160 (2021-11-03 12:32:34 -0400)


amd-drm-fixes-5.16-2021-11-03:

amdgpu:
- GPU reset fix
- Aldebaran fix
- Yellow Carp fixes
- DCN2.1 DMCUB fix
- IOMMU regression fix for Picasso
- DSC display fixes
- BPC display calculation fixes
- Other misc display fixes

amdkfd:
- SVM fixes
- Fix gfx version for renoir


Aaron Liu (1):
  drm/amdgpu: update RLC_PG_DELAY_3 Value to 200us for yellow carp

Anson Jacob (1):
  drm/amd/display: Fix dcn10_log_hubp_states printf format string

Anthony Koo (1):
  drm/amd/display: [FW Promotion] Release 0.0.91

Aric Cyr (1):
  drm/amd/display: 3.2.160

Aurabindo Pillai (1):
  drm/amd/display: add condition check for dmub notification

Bing Guo (1):
  drm/amd/display: Fix bpc calculation for specific encodings

Felipe Clark (1):
  drm/amd/display: Fix dummy p-state hang on monitors with extreme timing

Felix Kuehling (3):
  drm/amdkfd: Fix SVM_ATTR_PREFERRED_LOC
  drm/amdkfd: Avoid thrashing of stack and heap
  drm/amdkfd: Handle incomplete migration to system memory

Graham Sider (1):
  drm/amdkfd: update gfx target version for Renoir

Hersen Wu (1):
  drm/amd/display: dsc engine not disabled after unplug dsc mst hub

Jake Wang (3):
  drm/amd/display: Added HPO HW control shutdown support
  drm/amd/display: Add MPC meory shutdown support
  drm/amd/display: Added new DMUB boot option for power optimization

James Zhu (1):
  drm/amdgpu: remove duplicated kfd_resume_iommu

Jimmy Kizito (1):
  drm/amd/display: Clear encoder assignments when state cleared.

Jingwen Chen (1):
  drm/amd/amdgpu: fix bad job hw_fence use after free in advance tdr

Mario Limonciello (6):
  drm/amdgpu: Convert SMU version to decimal in debugfs
  drm/amdgpu/pm: drop pp_power_profile_mode support for yellow carp
  drm/amd/pm: Add missing mutex for pp_get_power_profile_mode
  drm/amd/pm: Adjust returns when power_profile_mode is not supported
  drm/amdgpu/pm: Don't show pp_power_profile_mode for unsupported devices
  drm/amd/display: Look at firmware version to determine using dmub on dcn21

Oak Zeng (1):
  drm/amdgpu: use correct register mask to extract field

Roman Li (1):
  drm/amd/display: Force disable planes on any pipe split change

Wenjing Liu (1):
  drm/amd/display: fix register write sequence for LINK_SQUARE_PATTERN

Yu-ting Shen (1):
  drm/amd/display: avoid link loss short pulse stuck the system

 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   8 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|   9 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c |   5 +-
 drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c   |  18 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_device.c|   2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c   |  45 +--
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c   |  44 --
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  41 --
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 150 -
 drivers/gpu/drm/amd/display/dc/core/dc.c   |   8 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link.c  |   2 +
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c   |   8 ++
 .../gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c  |  22 +++
 drivers/gpu/drm/amd/display/dc/dc.h|   3 +-
 drivers/gpu/drm/amd/display/dc/dc_dp_types.h   |   3 +
 drivers/gpu/drm/amd/display/dc/dce/dce_hwseq.h |   4 +-
 .../amd/display/dc/dce110/dce110_hw_sequencer.c|   6 +
 .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c  |   2 +-
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c |   3 +
 drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c   |   7 +-
 .../gpu/drm/amd/display/dc/dcn30/dcn30_resource.c  |   7 +-
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c |  78 +++
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h |   1 +
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c  |   1 +
 .../gpu/drm/amd/display/dc/dcn31/dcn31_resource.c  |   6 +-
 .../amd/display/dc/dml/dcn30/display_mode_vba_30.c |  13 +-
 .../amd/display/dc/dml/dcn31/display_mode_vba_31.c |  14 +-
 drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h|   1 +
 .../drm/amd/display/dc/inc/hw_sequencer_private.h  |   1 +
 drivers/gpu/drm/amd/display/dmub/dmub_srv.h|   1 +
 drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h|   4 +-

RE: [PATCH] drm/amdgpu/powerplay: fix sysfs_emit/sysfs_emit_at handling

2021-11-03 Thread Yu, Lang

[AMD Official Use Only]

Yes, I missed such conversions in powerplay. Thanks!

Reviewed-by: Lang Yu  

>-Original Message-
>From: Deucher, Alexander 
>Sent: Thursday, November 4, 2021 8:59 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander ; Yu, Lang
>; Powell, Darren 
>Subject: [PATCH] drm/amdgpu/powerplay: fix sysfs_emit/sysfs_emit_at handling
>
>sysfs_emit and sysfs_emit_at requrie a page boundary aligned buf address. Make
>them happy!
>
>v2: fix sysfs_emit -> sysfs_emit_at missed conversions
>
>Cc: Lang Yu 
>Cc: Darren Powell 
>Fixes: 6db0c87a0a8e ("amdgpu/pm: Replace hwmgr smu usage of sprintf with
>sysfs_emit")
>Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1774
>Signed-off-by: Alex Deucher 
>---
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c   |  8 ++--
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c| 10 +++---
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c|  2 ++
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h| 13 +
> .../gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c  | 12 +---
>  .../gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c  |  4
>  .../gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c  | 14
>++
> 7 files changed, 51 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>index 1de3ae77e03e..258c573acc97 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>@@ -1024,6 +1024,8 @@ static int smu10_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>   uint32_t min_freq, max_freq = 0;
>   uint32_t ret = 0;
>
>+  phm_get_sysfs_buf(, );
>+
>   switch (type) {
>   case PP_SCLK:
>   smum_send_msg_to_smc(hwmgr,
>PPSMC_MSG_GetGfxclkFrequency, ); @@ -1065,7 +1067,7 @@ static int
>smu10_print_clock_levels(struct pp_hwmgr *hwmgr,
>   if (ret)
>   return ret;
>
>-  size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
>+  size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
>   size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
>   (data->gfx_actual_soft_min_freq > 0) ? data-
>>gfx_actual_soft_min_freq : min_freq);
>   size += sysfs_emit_at(buf, size, "1: %10uMhz\n", @@ -
>1081,7 +1083,7 @@ static int smu10_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>   if (ret)
>   return ret;
>
>-  size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
>+  size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
>   size += sysfs_emit_at(buf, size,
>"SCLK: %7uMHz %10uMHz\n",
>   min_freq, max_freq);
>   }
>@@ -1456,6 +1458,8 @@ static int smu10_get_power_profile_mode(struct
>pp_hwmgr *hwmgr, char *buf)
>   if (!buf)
>   return -EINVAL;
>
>+  phm_get_sysfs_buf(, );
>+
>   size += sysfs_emit_at(buf, size, "%s %16s %s %s %s %s\n",title[0],
>   title[1], title[2], title[3], title[4], title[5]);
>
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>index e7803ce8f67a..aceebf584225 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>@@ -4914,6 +4914,8 @@ static int smu7_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>   int size = 0;
>   uint32_t i, now, clock, pcie_speed;
>
>+  phm_get_sysfs_buf(, );
>+
>   switch (type) {
>   case PP_SCLK:
>   smum_send_msg_to_smc(hwmgr,
>PPSMC_MSG_API_GetSclkFrequency, ); @@ -4963,7 +4965,7 @@ static int
>smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
>   break;
>   case OD_SCLK:
>   if (hwmgr->od_enabled) {
>-  size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
>+  size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
>   for (i = 0; i < odn_sclk_table->num_of_pl; i++)
>   size += sysfs_emit_at(buf, size,
>"%d: %10uMHz %10umV\n",
>   i, odn_sclk_table->entries[i].clock/100,
>@@ -4972,7 +4974,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>   break;
>   case OD_MCLK:
>   if (hwmgr->od_enabled) {
>-  size = sysfs_emit(buf, "%s:\n", "OD_MCLK");
>+  size += sysfs_emit_at(buf, size, "%s:\n", "OD_MCLK");
>   for (i = 0; i < odn_mclk_table->num_of_pl; i++)
>   size += sysfs_emit_at(buf, size,
>"%d: %10uMHz %10umV\n",
>   i, odn_mclk_table->entries[i].clock/100,
>@@ -4981,7 +4983,7 @@ static int

[PATCH] drm/amdgpu/powerplay: fix sysfs_emit/sysfs_emit_at handling

2021-11-03 Thread Alex Deucher

sysfs_emit and sysfs_emit_at requrie a page boundary
aligned buf address. Make them happy!

v2: fix sysfs_emit -> sysfs_emit_at missed conversions

Cc: Lang Yu 
Cc: Darren Powell 
Fixes: 6db0c87a0a8e ("amdgpu/pm: Replace hwmgr smu usage of sprintf with 
sysfs_emit")
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1774
Signed-off-by: Alex Deucher 
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c   |  8 ++--
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c| 10 +++---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c|  2 ++
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h| 13 +
 .../gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c  | 12 +---
 .../gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c  |  4 
 .../gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c  | 14 ++
 7 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
index 1de3ae77e03e..258c573acc97 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
@@ -1024,6 +1024,8 @@ static int smu10_print_clock_levels(struct pp_hwmgr 
*hwmgr,
uint32_t min_freq, max_freq = 0;
uint32_t ret = 0;
 
+   phm_get_sysfs_buf(, );
+
switch (type) {
case PP_SCLK:
smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetGfxclkFrequency, );
@@ -1065,7 +1067,7 @@ static int smu10_print_clock_levels(struct pp_hwmgr 
*hwmgr,
if (ret)
return ret;
 
-   size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
+   size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
(data->gfx_actual_soft_min_freq > 0) ? 
data->gfx_actual_soft_min_freq : min_freq);
size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
@@ -1081,7 +1083,7 @@ static int smu10_print_clock_levels(struct pp_hwmgr 
*hwmgr,
if (ret)
return ret;
 
-   size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
+   size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
size += sysfs_emit_at(buf, size, "SCLK: %7uMHz 
%10uMHz\n",
min_freq, max_freq);
}
@@ -1456,6 +1458,8 @@ static int smu10_get_power_profile_mode(struct pp_hwmgr 
*hwmgr, char *buf)
if (!buf)
return -EINVAL;
 
+   phm_get_sysfs_buf(, );
+
size += sysfs_emit_at(buf, size, "%s %16s %s %s %s %s\n",title[0],
title[1], title[2], title[3], title[4], title[5]);
 
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
index e7803ce8f67a..aceebf584225 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
@@ -4914,6 +4914,8 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
int size = 0;
uint32_t i, now, clock, pcie_speed;
 
+   phm_get_sysfs_buf(, );
+
switch (type) {
case PP_SCLK:
smum_send_msg_to_smc(hwmgr, PPSMC_MSG_API_GetSclkFrequency, 
);
@@ -4963,7 +4965,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
break;
case OD_SCLK:
if (hwmgr->od_enabled) {
-   size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
+   size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
for (i = 0; i < odn_sclk_table->num_of_pl; i++)
size += sysfs_emit_at(buf, size, "%d: %10uMHz 
%10umV\n",
i, odn_sclk_table->entries[i].clock/100,
@@ -4972,7 +4974,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
break;
case OD_MCLK:
if (hwmgr->od_enabled) {
-   size = sysfs_emit(buf, "%s:\n", "OD_MCLK");
+   size += sysfs_emit_at(buf, size, "%s:\n", "OD_MCLK");
for (i = 0; i < odn_mclk_table->num_of_pl; i++)
size += sysfs_emit_at(buf, size, "%d: %10uMHz 
%10umV\n",
i, odn_mclk_table->entries[i].clock/100,
@@ -4981,7 +4983,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
break;
case OD_RANGE:
if (hwmgr->od_enabled) {
-   size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
+   size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
size += sysfs_emit_at(buf, size, "SCLK: %7uMHz 
%10uMHz\n",

Re: [PATCH 8/8] drm/amdgpu: add drm buddy support to amdgpu

2021-11-03 Thread Matthew Auld


On 25/10/2021 14:00, Arunpravin wrote:

- Remove drm_mm references and replace with drm buddy functionalities
- Add res cursor support for drm buddy

Signed-off-by: Arunpravin 





+   spin_lock(>lock);
+   r = drm_buddy_alloc(mm, (uint64_t)place->fpfn << PAGE_SHIFT,
+   (uint64_t)lpfn << PAGE_SHIFT,
+   (uint64_t)n_pages << PAGE_SHIFT,
+min_page_size, >blocks,
+node->flags);



Is spinlock + GFP_KERNEL allowed?


+   spin_unlock(>lock);
+
+   if (unlikely(r))
+   goto error_free_blocks;
+
pages_left -= pages;
++i;
  
  		if (pages > pages_left)

pages = pages_left;
}
-   spin_unlock(>lock);
+
+   /* Free unused pages for contiguous allocation */
+   if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
+   uint64_t actual_size = (uint64_t)node->base.num_pages << 
PAGE_SHIFT;
+
+   r = drm_buddy_free_unused_pages(mm,
+   actual_size,
+   >blocks);


Needs some locking.

Re: [PATCH 6/8] drm/i915: add free_unused_pages support to i915

2021-11-03 Thread Matthew Auld


On 25/10/2021 14:00, Arunpravin wrote:

add drm_buddy_free_unused_pages() support on
contiguous allocation

Signed-off-by: Arunpravin 
---
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c 
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 963468228392..162947af8e04 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -98,6 +98,14 @@ static int i915_ttm_buddy_man_alloc(struct 
ttm_resource_manager *man,
if (unlikely(err))
goto err_free_blocks;
  
+	if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {

+   err = drm_buddy_free_unused_pages(mm, (uint64_t)n_pages << 
PAGE_SHIFT,
+  _res->blocks);
+
+   if (unlikely(err))
+   goto err_free_blocks;


That needs some locking, mark_free/mark_split are all globally visible. 
Some concurrent user might steal the block, or worse.



+   }
+
*res = _res->base;
return 0;

Re: [PATCH 5/8] drm: Implement method to free unused pages

2021-11-03 Thread Matthew Auld


On 25/10/2021 14:00, Arunpravin wrote:

On contiguous allocation, we round up the size
to the *next* power of 2, implement a function
to free the unused pages after the newly allocate block.

Signed-off-by: Arunpravin 


Ideally this gets added with some user, so we can see it in action? 
Maybe squash the next patch here?



---
  drivers/gpu/drm/drm_buddy.c | 103 
  include/drm/drm_buddy.h |   4 ++
  2 files changed, 107 insertions(+)

diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 9d3547bcc5da..0da8510736eb 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -284,6 +284,109 @@ static inline bool contains(u64 s1, u64 e1, u64 s2, u64 
e2)
return s1 <= s2 && e1 >= e2;
  }
  
+/**

+ * drm_buddy_free_unused_pages - free unused pages
+ *
+ * @mm: DRM buddy manager
+ * @actual_size: original size requested
+ * @blocks: output list head to add allocated blocks
+ *
+ * For contiguous allocation, we round up the size to the nearest
+ * power of two value, drivers consume *actual* size, so remaining
+ * portions are unused and it can be freed.
+ *
+ * Returns:
+ * 0 on success, error code on failure.
+ */
+int drm_buddy_free_unused_pages(struct drm_buddy_mm *mm,


drm_buddy_block_trim?


+   u64 actual_size,


new_size?


+   struct list_head *blocks)
+{
+   struct drm_buddy_block *block;
+   struct drm_buddy_block *buddy;
+   u64 actual_start;
+   u64 actual_end;
+   LIST_HEAD(dfs);
+   u64 count = 0;
+   int err;
+
+   if (!list_is_singular(blocks))
+   return -EINVAL;
+
+   block = list_first_entry_or_null(blocks,
+struct drm_buddy_block,
+link);
+
+   if (!block)
+   return -EINVAL;


list_is_singular() already ensures that I guess?



+
+   if (actual_size > drm_buddy_block_size(mm, block))
+   return -EINVAL;
+
+   if (actual_size == drm_buddy_block_size(mm, block))
+   return 0;


Probably need to check the alignment of the actual_size, and also check 
that it is non-zero?



+
+   list_del(>link);
+
+   actual_start = drm_buddy_block_offset(block);
+   actual_end = actual_start + actual_size - 1;
+
+   if (drm_buddy_block_is_allocated(block))


That should rather be a programmer error.


+   mark_free(mm, block);
+
+   list_add(>tmp_link, );
+
+   while (1) {
+   block = list_first_entry_or_null(,
+struct drm_buddy_block,
+tmp_link);
+
+   if (!block)
+   break;
+
+   list_del(>tmp_link);
+
+   if (count == actual_size)
+   return 0;



Check for overlaps somewhere here to avoid needless searching and splitting?


+
+   if (contains(actual_start, actual_end, 
drm_buddy_block_offset(block),
+   (drm_buddy_block_offset(block) + 
drm_buddy_block_size(mm, block) - 1))) {


Could maybe record the start/end for better readability?


+   BUG_ON(!drm_buddy_block_is_free(block));
+
+   /* Allocate only required blocks */
+   mark_allocated(block);
+   mm->avail -= drm_buddy_block_size(mm, block);
+   list_add_tail(>link, blocks);
+   count += drm_buddy_block_size(mm, block);
+   continue;
+   }
+
+   if (drm_buddy_block_order(block) == 0)
+   continue;


Should be impossible with overlaps check added.


+
+   if (!drm_buddy_block_is_split(block)) {


That should always be true.


+   err = split_block(mm, block);
+
+   if (unlikely(err))
+   goto err_undo;
+   }
+
+   list_add(>right->tmp_link, );
+   list_add(>left->tmp_link, );
+   }
+
+   return -ENOSPC;



Would it make sense to factor out part of the alloc_range for this? It 
looks roughly the same.



+
+err_undo:
+   buddy = get_buddy(block);
+   if (buddy &&
+   (drm_buddy_block_is_free(block) &&
+drm_buddy_block_is_free(buddy)))
+   __drm_buddy_free(mm, block);
+   return err;



Where do we add the block back to the original list? Did we not just 
leak it?




+}
+EXPORT_SYMBOL(drm_buddy_free_unused_pages);
+
  static struct drm_buddy_block *
  alloc_range(struct drm_buddy_mm *mm,
u64 start, u64 end,
diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
index cd8021d2d6e7..1dfc80c88e1f 100644
--- a/include/drm/drm_buddy.h
+++ b/include/drm/drm_buddy.h
@@ -145,6 +145,10 @@ int drm_buddy_alloc(struct drm_buddy_mm

Re: [PATCH 3/8] drm: implement top-down allocation method

2021-11-03 Thread Matthew Auld


On 25/10/2021 14:00, Arunpravin wrote:

Implemented a function which walk through the order list,
compares the offset and returns the maximum offset block,
this method is unpredictable in obtaining the high range
address blocks which depends on allocation and deallocation.
for instance, if driver requests address at a low specific
range, allocator traverses from the root block and splits
the larger blocks until it reaches the specific block and
in the process of splitting, lower orders in the freelist
are occupied with low range address blocks and for the
subsequent TOPDOWN memory request we may return the low
range blocks.To overcome this issue, we may go with the
below approach.

The other approach, sorting each order list entries in
ascending order and compares the last entry of each
order list in the freelist and return the max block.
This creates sorting overhead on every drm_buddy_free()
request and split up of larger blocks for a single page
request.

Signed-off-by: Arunpravin 
---
  drivers/gpu/drm/drm_buddy.c | 42 +++--
  include/drm/drm_buddy.h |  1 +
  2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 406e3d521903..9d3547bcc5da 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -362,6 +362,27 @@ alloc_range(struct drm_buddy_mm *mm,
return ERR_PTR(err);
  }
  
+static struct drm_buddy_block *

+get_maxblock(struct list_head *head)
+{
+   struct drm_buddy_block *max_block = NULL, *node;
+
+   max_block = list_first_entry_or_null(head,
+struct drm_buddy_block,
+link);
+
+   if (!max_block)
+   return NULL;
+
+   list_for_each_entry(node, head, link) {
+   if (drm_buddy_block_offset(node) >
+   drm_buddy_block_offset(max_block))


Alignment.


+   max_block = node;
+   }


I suppose there will be pathological cases where this will unnecessarily 
steal the mappable portion? But in practice maybe this is good enough?



+
+   return max_block;
+}
+
  static struct drm_buddy_block *
  alloc_from_freelist(struct drm_buddy_mm *mm,
unsigned int order,
@@ -372,13 +393,22 @@ alloc_from_freelist(struct drm_buddy_mm *mm,
int err;
  
  	for (i = order; i <= mm->max_order; ++i) {

-   if (!list_empty(>free_list[i])) {
-   block = list_first_entry_or_null(>free_list[i],
-struct drm_buddy_block,
-link);
+   if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) {
+   if (!list_empty(>free_list[i])) {


AFAIK no need to keep checking list_empty(), below also.


+   block = get_maxblock(>free_list[i]);
  
-			if (block)

-   break;
+   if (block)
+   break;
+   }
+   } else {
+   if (!list_empty(>free_list[i])) {
+   block = 
list_first_entry_or_null(>free_list[i],
+struct 
drm_buddy_block,
+link);
+
+   if (block)
+   break;
+   }
}
}
  
diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h

index c7bb5509a7ad..cd8021d2d6e7 100644
--- a/include/drm/drm_buddy.h
+++ b/include/drm/drm_buddy.h
@@ -28,6 +28,7 @@
  })
  
  #define DRM_BUDDY_RANGE_ALLOCATION (1 << 0)

+#define DRM_BUDDY_TOPDOWN_ALLOCATION (1 << 1)
  
  struct drm_buddy_block {

  #define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)

Re: [PATCH v2 2/8] drm: improve drm_buddy_alloc function

2021-11-03 Thread Matthew Auld


On 25/10/2021 14:00, Arunpravin wrote:

- Make drm_buddy_alloc a single function to handle
   range allocation and non-range allocation demands

- Implemented a new function alloc_range() which allocates
   the requested power-of-two block comply with range limitations

- Moved order computation and memory alignment logic from
   i915 driver to drm buddy

V2:
merged below changes to keep the build unbroken
- drm_buddy_alloc_range() becomes obsolete and may be removed
- enable ttm range allocation (fpfn / lpfn) support in i915 driver
- apply enhanced drm_buddy_alloc() function to i915 driver

Signed-off-by: Arunpravin 
---
  drivers/gpu/drm/drm_buddy.c   | 265 +++---
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c |  67 ++---
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.h |   2 +
  include/drm/drm_buddy.h   |  22 +-
  4 files changed, 207 insertions(+), 149 deletions(-)

diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 39eb1d224bec..406e3d521903 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -274,63 +274,6 @@ void drm_buddy_free_list(struct drm_buddy_mm *mm, struct 
list_head *objects)
  }
  EXPORT_SYMBOL(drm_buddy_free_list);
  
-/**

- * drm_buddy_alloc - allocate power-of-two blocks
- *
- * @mm: DRM buddy manager to allocate from
- * @order: size of the allocation
- *
- * The order value here translates to:
- *
- * 0 = 2^0 * mm->chunk_size
- * 1 = 2^1 * mm->chunk_size
- * 2 = 2^2 * mm->chunk_size
- *
- * Returns:
- * allocated ptr to the _buddy_block on success
- */
-struct drm_buddy_block *
-drm_buddy_alloc(struct drm_buddy_mm *mm, unsigned int order)
-{
-   struct drm_buddy_block *block = NULL;
-   unsigned int i;
-   int err;
-
-   for (i = order; i <= mm->max_order; ++i) {
-   block = list_first_entry_or_null(>free_list[i],
-struct drm_buddy_block,
-link);
-   if (block)
-   break;
-   }
-
-   if (!block)
-   return ERR_PTR(-ENOSPC);
-
-   BUG_ON(!drm_buddy_block_is_free(block));
-
-   while (i != order) {
-   err = split_block(mm, block);
-   if (unlikely(err))
-   goto out_free;
-
-   /* Go low */
-   block = block->left;
-   i--;
-   }
-
-   mark_allocated(block);
-   mm->avail -= drm_buddy_block_size(mm, block);
-   kmemleak_update_trace(block);
-   return block;
-
-out_free:
-   if (i != order)
-   __drm_buddy_free(mm, block);
-   return ERR_PTR(err);
-}
-EXPORT_SYMBOL(drm_buddy_alloc);
-
  static inline bool overlaps(u64 s1, u64 e1, u64 s2, u64 e2)
  {
return s1 <= e2 && e1 >= s2;
@@ -341,52 +284,22 @@ static inline bool contains(u64 s1, u64 e1, u64 s2, u64 
e2)
return s1 <= s2 && e1 >= e2;
  }
  
-/**

- * drm_buddy_alloc_range - allocate range
- *
- * @mm: DRM buddy manager to allocate from
- * @blocks: output list head to add allocated blocks
- * @start: start of the allowed range for this block
- * @size: size of the allocation
- *
- * Intended for pre-allocating portions of the address space, for example to
- * reserve a block for the initial framebuffer or similar, hence the 
expectation
- * here is that drm_buddy_alloc() is still the main vehicle for
- * allocations, so if that's not the case then the drm_mm range allocator is
- * probably a much better fit, and so you should probably go use that instead.
- *
- * Note that it's safe to chain together multiple alloc_ranges
- * with the same blocks list
- *
- * Returns:
- * 0 on success, error code on failure.
- */
-int drm_buddy_alloc_range(struct drm_buddy_mm *mm,
- struct list_head *blocks,
- u64 start, u64 size)
+static struct drm_buddy_block *
+alloc_range(struct drm_buddy_mm *mm,
+   u64 start, u64 end,
+   unsigned int order)
  {
struct drm_buddy_block *block;
struct drm_buddy_block *buddy;
-   LIST_HEAD(allocated);
LIST_HEAD(dfs);
-   u64 end;
int err;
int i;
  
-	if (size < mm->chunk_size)

-   return -EINVAL;
-
-   if (!IS_ALIGNED(size | start, mm->chunk_size))
-   return -EINVAL;
-
-   if (range_overflows(start, size, mm->size))
-   return -EINVAL;
+   end = end - 1;
  
  	for (i = 0; i < mm->n_roots; ++i)

list_add_tail(>roots[i]->tmp_link, );
  
-	end = start + size - 1;

-
do {
u64 block_start;
u64 block_end;
@@ -394,31 +307,32 @@ int drm_buddy_alloc_range(struct drm_buddy_mm *mm,
block = list_first_entry_or_null(,
 struct drm_buddy_block,
 tmp_link);
+


No need for the newline.

Re: [PATCH 01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate

2021-11-03 Thread Ville Syrjälä

On Wed, Nov 03, 2021 at 01:02:11PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 02, 2021 at 03:59:32PM +0100, Maxime Ripard wrote:
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct 
> > drm_connector *connector,
> > u32 max_tmds_clock = hf_vsdb[5] * 5000;
> > struct drm_scdc *scdc = >scdc;
> >  
> > -   if (max_tmds_clock > 34) {
> > +   if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> > display->max_tmds_clock = max_tmds_clock;
> > DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
> > display->max_tmds_clock);
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
> > b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index d2e61f6c6e08..0666203d52b7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct intel_encoder 
> > *encoder,
> > if (scdc->scrambling.low_rates)
> > pipe_config->hdmi_scrambling = true;
> >  
> > -   if (pipe_config->port_clock > 34) {
> > +   if (pipe_config->port_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> > pipe_config->hdmi_scrambling = true;
> > pipe_config->hdmi_high_tmds_clock_ratio = true;
> > }
> 
> All of that is HDMI 2.0 stuff. So this just makes it all super
> confusing IMO. Nak.

So reading throgh HDMI 1.4 again it does specify 340 MHz as some kind
of upper limit for the physical cable. But nowhere else is that number
really mentioned AFAICS. HDMI 2.0 does talk quite a bit about the 340
Mcsc limit in various places.

I wonder what people would think of a couple of helpers like:
- drm_hdmi_{can,must}_use_scrambling()
- drm_hdmi_is_high_tmds_clock_ratio()
or something along those lines? At least with those the code would
read decently and I wouldn't have to wonder what this HDMI 1.4 TMDS
clock limit really is.

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/amd/amdkfd: Don't sent command to HWS on kfd reset

2021-11-03 Thread Alex Deucher

On Wed, Nov 3, 2021 at 11:05 AM shaoyunl  wrote:
>
> When kfd need to be reset, sent command to HWS might cause hang and get 
> unnecessary timeout.
> This change try not to touch HW in pre_reset and keep queues to be in the 
> evicted state
> when the reset is done, so they are not put back on the runlist. These queues 
> will be destroied
> on process termination.
>
> Signed-off-by: shaoyunl 
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c   | 6 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 ++
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 6 +-
>  4 files changed, 13 insertions(+), 3 deletions(-)
>  mode change 100644 => 100755 drivers/gpu/drm/amd/amdkfd/kfd_device.c
>  mode change 100644 => 100755 
> drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>  mode change 100644 => 100755 drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>  mode change 100644 => 100755 drivers/gpu/drm/amd/amdkfd/kfd_process.c

Please fix the mode change.

Alex

>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> old mode 100644
> new mode 100755
> index c8aade17efef..536ef766d09e
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -1100,6 +1100,8 @@ int kgd2kfd_pre_reset(struct kfd_dev *kfd)
> if (!kfd->init_complete)
> return 0;
>
> +   kfd->is_resetting = true;
> +
> kfd_smi_event_update_gpu_reset(kfd, false);
>
> kfd->dqm->ops.pre_reset(kfd->dqm);
> @@ -1132,6 +1134,8 @@ int kgd2kfd_post_reset(struct kfd_dev *kfd)
>
> kfd_smi_event_update_gpu_reset(kfd, true);
>
> +   kfd->is_resetting = false;
> +
> return 0;
>  }
>
> @@ -1168,7 +1172,7 @@ int kgd2kfd_resume(struct kfd_dev *kfd, bool run_pm)
> return ret;
>
> /* for runtime resume, skip unlocking kfd */
> -   if (!run_pm) {
> +   if (!run_pm && !kfd->is_resetting) {
> count = atomic_dec_return(_locked);
> WARN_ONCE(count < 0, "KFD suspend / resume ref. error");
> if (count == 0)
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> old mode 100644
> new mode 100755
> index e9601d4dfb77..0a60317509c8
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -1430,7 +1430,7 @@ static int unmap_queues_cpsch(struct 
> device_queue_manager *dqm,
>
> if (!dqm->sched_running)
> return 0;
> -   if (dqm->is_hws_hang)
> +   if (dqm->is_hws_hang || dqm->is_resetting)
> return -EIO;
> if (!dqm->active_runlist)
> return retval;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> old mode 100644
> new mode 100755
> index bfe7bacccb73..e4bcc2a09ca8
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -275,6 +275,8 @@ struct kfd_dev {
> struct device_queue_manager *dqm;
>
> bool init_complete;
> +   bool is_resetting;
> +
> /*
>  * Interrupts of interest to KFD are copied
>  * from the HW ring into a SW ring.
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> old mode 100644
> new mode 100755
> index f8a8fdb95832..f29b3932e3dc
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1715,7 +1715,11 @@ int kfd_process_evict_queues(struct kfd_process *p)
>
> r = pdd->dev->dqm->ops.evict_process_queues(pdd->dev->dqm,
> >qpd);
> -   if (r) {
> +   /* evict return -EIO if HWS is hang or asic is resetting, in 
> this case
> +* we would like to set all the queues to be in evicted state 
> to prevent
> +* them been add back since they actually not be saved right 
> now.
> +*/
> +   if (r && r != -EIO) {
> pr_err("Failed to evict process queues\n");
> goto fail;
> }
> --
> 2.17.1
>

Re: [PATCH v9 10/10] drm: use DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES bitmap to tracefs

2021-11-03 Thread Jason Baron




On 10/27/21 12:36 AM, Jim Cromie wrote:
> Use new macro to create a sysfs control bitmap knob to control
> print-to-trace in: /sys/module/drm/parameters/trace
> 
> todo: reconsider this api, ie a single macro expecting both debug &
> trace terms (2 each), followed by a single description and the
> bitmap-spec::
> 
> Good: declares bitmap once for both interfaces
> 
> Bad: arg-type/count handling (expecting 4 args) is ugly,
>  especially preceding the bitmap-init var-args.
> 

Hi Jim,

I agree having the bitmap declared twice seems redundant. But I like having 
fewer args and not necessarily combining the trace/log variants of
DEBUG_CATEGORIES. hmmm...what if the DEFINE_DYNAMIC_DEBUG_CATEGORIES() took a 
pointer to the array of struct dyndbg_bitdesc map[] directly as the
final argument instead of the __VA_ARGS__? Then, we could just declare the map 
once?

Thanks,

-Jason

> Signed-off-by: Jim Cromie 
> ---
>  drivers/gpu/drm/drm_print.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index ce662d0f339b..7b49fbc5e21d 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -73,6 +73,25 @@ DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug, __drm_debug,
>   [7] = { DRM_DBG_CAT_LEASE },
>   [8] = { DRM_DBG_CAT_DP },
>   [9] = { DRM_DBG_CAT_DRMRES });
> +
> +#ifdef CONFIG_TRACING
> +unsigned long __drm_trace;
> +EXPORT_SYMBOL(__drm_trace);
> +DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES(trace, __drm_trace,
> +   DRM_DEBUG_DESC,
> +   [0] = { DRM_DBG_CAT_CORE },
> +   [1] = { DRM_DBG_CAT_DRIVER },
> +   [2] = { DRM_DBG_CAT_KMS },
> +   [3] = { DRM_DBG_CAT_PRIME },
> +   [4] = { DRM_DBG_CAT_ATOMIC },
> +   [5] = { DRM_DBG_CAT_VBL },
> +   [6] = { DRM_DBG_CAT_STATE },
> +   [7] = { DRM_DBG_CAT_LEASE },
> +   [8] = { DRM_DBG_CAT_DP },
> +   [9] = { DRM_DBG_CAT_DRMRES });
> +
> +struct trace_array *trace_arr;
> +#endif
>  #endif
>  
>  void __drm_puts_coredump(struct drm_printer *p, const char *str)
>

Re: [PATCH 2/2] drm/amdkfd: fix boot failure when iommu is disabled in Picasso.

2021-11-03 Thread Lazar, Lijo

[Public]

It's a conditional case for some kind of early reset. Haven't checked details, 
on a quick glance it appeared to call iommu init again.

https://elixir.bootlin.com/linux/v5.15/source/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c#L5160

Thanks,
Lijo

Re: [PATCH 6/6] drm/radeon: use dma_resv_wait_timeout() instead of manually waiting

2021-11-03 Thread Das, Nirmoy


Acked-by: Nirmoy Das 

On 10/28/2021 3:26 PM, Christian König wrote:

Don't touch the exclusive fence manually here, but rather use the
general dma_resv function. We did that for better hw reset handling but
this doesn't necessary work correctly.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/radeon/radeon_uvd.c | 13 +
  1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index 2ea86919d953..377f9cdb5b53 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -469,7 +469,6 @@ static int radeon_uvd_cs_msg(struct radeon_cs_parser *p, 
struct radeon_bo *bo,
  {
int32_t *msg, msg_type, handle;
unsigned img_size = 0;
-   struct dma_fence *f;
void *ptr;
  
  	int i, r;

@@ -479,13 +478,11 @@ static int radeon_uvd_cs_msg(struct radeon_cs_parser *p, 
struct radeon_bo *bo,
return -EINVAL;
}
  
-	f = dma_resv_excl_fence(bo->tbo.base.resv);

-   if (f) {
-   r = radeon_fence_wait((struct radeon_fence *)f, false);
-   if (r) {
-   DRM_ERROR("Failed waiting for UVD message (%d)!\n", r);
-   return r;
-   }
+   r = dma_resv_wait_timeout(bo->tbo.base.resv, false, false,
+ MAX_SCHEDULE_TIMEOUT);
+   if (r <= 0) {
+   DRM_ERROR("Failed waiting for UVD message (%d)!\n", r);
+   return r ? r : -ETIME;
}
  
  	r = radeon_bo_kmap(bo, );

Re: [PATCH 2/2] drm/amdkfd: fix boot failure when iommu is disabled in Picasso.

2021-11-03 Thread James Zhu




On 2021-11-02 11:53 p.m., Lazar, Lijo wrote:



On 11/3/2021 12:53 AM, James Zhu wrote:

From: Yifan Zhang 

When IOMMU disabled in sbios and kfd in iommuv2 path, iommuv2
init will fail. But this failure should not block amdgpu driver init.

Reported-by: youling 
Tested-by: youling 
Signed-off-by: Yifan Zhang 
Reviewed-by: James Zhu 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 
  drivers/gpu/drm/amd/amdkfd/kfd_device.c    | 3 +++
  2 files changed, 3 insertions(+), 4 deletions(-)

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

index e56bc925afcf..f77823ce7ae8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2398,10 +2398,6 @@ static int amdgpu_device_ip_init(struct 
amdgpu_device *adev)

  if (!adev->gmc.xgmi.pending_reset)
  amdgpu_amdkfd_device_init(adev);
  -    r = amdgpu_amdkfd_resume_iommu(adev);
-    if (r)
-    goto init_failed;
-
  amdgpu_fru_get_product_info(adev);
    init_failed:
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c

index be26c4016ade..7677ced16a27 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -1031,6 +1031,9 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
    svm_migrate_init(kfd->adev);
  +    if(kgd2kfd_resume_iommu(kfd))
+    goto device_iommu_error;
+


This also brings a duplicate iommu resume in the reset path -
https://elixir.bootlin.com/linux/v5.15/source/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c#L4612 



[JZ] Hi Lijo, please help clarify this duplicate case.

Thanks!

James



Thanks,
Lijo

  if (kfd_resume(kfd))
  goto kfd_resume_error;

Re: [PATCH] drm/amdgpu: remove duplicated kfd_resume_iommu

2021-11-03 Thread Alex Deucher

On Tue, Nov 2, 2021 at 9:34 PM James Zhu  wrote:
>
> Remove duplicated kfd_resume_iommu which already runs
> in mdgpu_amdkfd_device_init.
>
> Signed-off-by: James Zhu 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index e56bc925afcf..f77823ce7ae8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2398,10 +2398,6 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
> *adev)
> if (!adev->gmc.xgmi.pending_reset)
> amdgpu_amdkfd_device_init(adev);
>
> -   r = amdgpu_amdkfd_resume_iommu(adev);
> -   if (r)
> -   goto init_failed;
> -
> amdgpu_fru_get_product_info(adev);
>
>  init_failed:
> --
> 2.25.1
>

Re: [PATCH] drm/amdgpu: remove duplicated kfd_resume_iommu

2021-11-03 Thread Alex Deucher

On Wed, Nov 3, 2021 at 10:50 AM Alex Deucher  wrote:
>
>
>
> On Wed, Nov 3, 2021 at 10:34 AM Zhu, James  wrote:
>>
>> [AMD Official Use Only]
>>
>>
>> Hi Alex,
>>
>> Finally figured out the root cause for this broken,
>>
>> Linux 5.14.15  + afd1818 can fix the issue.

I think this applies to 5.15 as well.  Only drm-next (5.16) needs this patch.

Alex

>
>
> I'll do that for stable.
>
>>
>> Linux 5.15rc7 re-apply "init iommu after amdkfd device init" and "move 
>> iommu_resume before ip init/resume" which overwrote afd1818 caused the issue 
>> again.
>>
>> 714d9e4 drm/amdgpu: init iommu after amdkfd device init
>>
>> f02abeb drm/amdgpu: move iommu_resume before ip init/resume
>>
>> afd1818 drm/amdkfd: fix boot failure when iommu is disabled in Picasso.
>>
>> 286826d drm/amdgpu: init iommu after amdkfd device init
>>
>> 9cec53c drm/amdgpu: move iommu_resume before ip init/resume
>>
>>
>>
>> So, do we just discard this patch, and revert 714d9e4 and  f02abeb?
>
>
> I'll do that for 5.15+
>
> Thanks for sorting this out.
>
> Alex
>
>>
>>
>> Thanks & Best Regards!
>>
>>
>> James Zhu
>>
>> 
>> From: Alex Deucher 
>> Sent: Tuesday, November 2, 2021 10:01 PM
>> To: Zhu, James 
>> Cc: amd-gfx list ; Deucher, Alexander 
>> ; Zhang, Yifan ; James Zhu 
>> ; Ken Moffat 
>> Subject: Re: [PATCH] drm/amdgpu: remove duplicated kfd_resume_iommu
>>
>> On Tue, Nov 2, 2021 at 9:34 PM James Zhu  wrote:
>> >
>> > Remove duplicated kfd_resume_iommu which already runs
>> > in mdgpu_amdkfd_device_init.
>> >
>> > Signed-off-by: James Zhu 
>>
>> Once you get confirmation, please add:
>> Bug: 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D214859data=04%7C01%7CJames.Zhu%40amd.com%7C8662c25150e94d9d664708d99e6deb2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637715017208277821%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=a6WyuNGhOU5OT3J8GQtXSQ3O5r942D2p%2BbruFUncT0E%3Dreserved=0
>> Bug: 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1770data=04%7C01%7CJames.Zhu%40amd.com%7C8662c25150e94d9d664708d99e6deb2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637715017208287813%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=E1MFXdprEaldLux2AoXNEeDWL5E85WFv8CrfZODTa%2F4%3Dreserved=0
>>
>> Acked-by: Alex Deucher 
>>
>>
>> > ---
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 
>> >  1 file changed, 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> > index e56bc925afcf..f77823ce7ae8 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> > @@ -2398,10 +2398,6 @@ static int amdgpu_device_ip_init(struct 
>> > amdgpu_device *adev)
>> > if (!adev->gmc.xgmi.pending_reset)
>> > amdgpu_amdkfd_device_init(adev);
>> >
>> > -   r = amdgpu_amdkfd_resume_iommu(adev);
>> > -   if (r)
>> > -   goto init_failed;
>> > -
>> > amdgpu_fru_get_product_info(adev);
>> >
>> >  init_failed:
>> > --
>> > 2.25.1
>> >

[PATCH] drm/amd/amdkfd: Don't sent command to HWS on kfd reset

2021-11-03 Thread shaoyunl

When kfd need to be reset, sent command to HWS might cause hang and get 
unnecessary timeout.
This change try not to touch HW in pre_reset and keep queues to be in the 
evicted state
when the reset is done, so they are not put back on the runlist. These queues 
will be destroied
on process termination.

Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c   | 6 +-
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 ++
 drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 6 +-
 4 files changed, 13 insertions(+), 3 deletions(-)
 mode change 100644 => 100755 drivers/gpu/drm/amd/amdkfd/kfd_device.c
 mode change 100644 => 100755 
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
 mode change 100644 => 100755 drivers/gpu/drm/amd/amdkfd/kfd_priv.h
 mode change 100644 => 100755 drivers/gpu/drm/amd/amdkfd/kfd_process.c

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
old mode 100644
new mode 100755
index c8aade17efef..536ef766d09e
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -1100,6 +1100,8 @@ int kgd2kfd_pre_reset(struct kfd_dev *kfd)
if (!kfd->init_complete)
return 0;
 
+   kfd->is_resetting = true;
+
kfd_smi_event_update_gpu_reset(kfd, false);
 
kfd->dqm->ops.pre_reset(kfd->dqm);
@@ -1132,6 +1134,8 @@ int kgd2kfd_post_reset(struct kfd_dev *kfd)
 
kfd_smi_event_update_gpu_reset(kfd, true);
 
+   kfd->is_resetting = false;
+
return 0;
 }
 
@@ -1168,7 +1172,7 @@ int kgd2kfd_resume(struct kfd_dev *kfd, bool run_pm)
return ret;
 
/* for runtime resume, skip unlocking kfd */
-   if (!run_pm) {
+   if (!run_pm && !kfd->is_resetting) {
count = atomic_dec_return(_locked);
WARN_ONCE(count < 0, "KFD suspend / resume ref. error");
if (count == 0)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
old mode 100644
new mode 100755
index e9601d4dfb77..0a60317509c8
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1430,7 +1430,7 @@ static int unmap_queues_cpsch(struct device_queue_manager 
*dqm,
 
if (!dqm->sched_running)
return 0;
-   if (dqm->is_hws_hang)
+   if (dqm->is_hws_hang || dqm->is_resetting)
return -EIO;
if (!dqm->active_runlist)
return retval;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
old mode 100644
new mode 100755
index bfe7bacccb73..e4bcc2a09ca8
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -275,6 +275,8 @@ struct kfd_dev {
struct device_queue_manager *dqm;
 
bool init_complete;
+   bool is_resetting;
+
/*
 * Interrupts of interest to KFD are copied
 * from the HW ring into a SW ring.
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
old mode 100644
new mode 100755
index f8a8fdb95832..f29b3932e3dc
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1715,7 +1715,11 @@ int kfd_process_evict_queues(struct kfd_process *p)
 
r = pdd->dev->dqm->ops.evict_process_queues(pdd->dev->dqm,
>qpd);
-   if (r) {
+   /* evict return -EIO if HWS is hang or asic is resetting, in 
this case
+* we would like to set all the queues to be in evicted state 
to prevent
+* them been add back since they actually not be saved right 
now.
+*/
+   if (r && r != -EIO) {
pr_err("Failed to evict process queues\n");
goto fail;
}
-- 
2.17.1

Re: [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic

2021-11-03 Thread Javier Martinez Canillas

Hello Thomas,

On 11/3/21 14:01, Thomas Zimmermann wrote:

[snip]

>>
>>
>> Javier Martinez Canillas (5):
>>drm/i915: Fix comment about modeset parameters
>>drm: Move nomodeset kernel parameter handler to the DRM subsystem
>>drm: Rename vgacon_text_force() function to drm_modeset_disabled()
>>drm: Add a drm_drv_enabled() helper function
>>drm: Use drm_drv_enabled() instead of drm_modeset_disabled()
> 
> There's too much churn here IMHO. Please merge patches 2+3 and 4+5. And 
> I'd put patch (4+5) first, so you have the drivers out of the way. After 
> that patch (2+3) should only modify drm_drv_enabled().
>

Sure, I'm happy with less patches.

Thanks for your feedback.
 
> Best regards
> Thomas
> 
Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH] drm/amd/display: Fix error handling on waiting for completion

2021-11-03 Thread Michel Dänzer



[ Adding dri-devel ]

On 2021-11-01 16:00, Wang, Chao-kai (Stylon) wrote:
> 
> The problem with -ERESTARTSYS is the same half-baked atomic state with 
> modifications we made in the interrupted atomic check, is reused in the next 
> retry and fails the atomic check. What we expect in the next retry is with 
> the original atomic state. I am going to dig deeper and see if at DRM side we 
> can go back to use to the original atomic state in the retry.

I suspect either DC/DM needs to be able to handle the modified state on retry, 
or it needs to restore the original state before it returns -ERESTARTSYS.


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer

Re: [PATCH 1/3] drm/amd/display: Don't allow partial copy_from_user

2021-11-03 Thread Rodrigo Siqueira Jordao


Hi Harry,

lgtm.
Btw, it looks like that the other patches from this series are already 
applied to amd-staging-drm-next.


Reviewed-by: Rodrigo Siqueira 


On 2021-10-27 10:26 a.m., Harry Wentland wrote:

There is no reason to allow for partial buffers from
userspace in our debugfs. In this particular case
callers will zero out the wr_buf but if callers in
the future don't do that we might be looking at
corrupt data.

Linus puts it better than I can in
https://lkml.org/lkml/2021/10/26/993

Signed-off-by: Harry Wentland 
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c  | 10 --
  1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 1a68a674913c..b30307ccff12 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -78,12 +78,10 @@ static int parse_write_buffer_into_params(char *wr_buf, 
uint32_t wr_buf_size,
  
  	wr_buf_ptr = wr_buf;
  
-	r = copy_from_user(wr_buf_ptr, buf, wr_buf_size);

-
-   /* r is bytes not be copied */
-   if (r >= wr_buf_size) {
-   DRM_DEBUG_DRIVER("user data not be read\n");
-   return -EINVAL;
+   /* r is bytes not be copied */
+   if (copy_from_user(wr_buf_ptr, buf, wr_buf_size)) {
+   DRM_DEBUG_DRIVER("user data could not be read successfully\n");
+   return -EFAULT;
}
  
  	/* check number of parameters. isspace could not differ space and \n */

Re: [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled()

2021-11-03 Thread Javier Martinez Canillas

On 11/3/21 13:57, Thomas Zimmermann wrote:

[snip]

>>   
>> -if (vgacon_text_force()) {
>> +if (drm_modeset_disabled()) {
>>  DRM_ERROR("amdgpu kernel modesetting disabled.\n");
> 
> Please remove all such error messages from drivers. 
> drm_modeset_disabled() should print a unified message instead.
>

Agreed.

>> -static bool vgacon_text_mode_force;
>> +static bool drm_nomodeset;
>>   
>> -bool vgacon_text_force(void)
>> +bool drm_modeset_disabled(void)
> 
> I suggest to rename this function to drm_check_modeset() and have it 
> return a negative errno code on failure. This gives maximum flexibility 
> and reduces errors in drivers. Right now the drivers return something 
> like -EINVAL, which seems wrong. Returning -ENODEV seems more appropriate.
>

Good idea. I'll do it in v2 as well.
 
Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

2021-11-03 Thread Javier Martinez Canillas

Hello Thomas,

On 11/3/21 13:41, Thomas Zimmermann wrote:
> Hi
> 
> Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:
>> The "nomodeset" kernel cmdline parameter is handled by the vgacon driver
>> but the exported vgacon_text_force() symbol is only used by DRM drivers.
>>
>> It makes much more sense for the parameter logic to be in the subsystem
>> of the drivers that are making use of it. Let's move that to DRM.
>>
>> Suggested-by: Daniel Vetter 
>> Signed-off-by: Javier Martinez Canillas 
>> ---
>>
>>   drivers/gpu/drm/Makefile|  2 ++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
>>   drivers/gpu/drm/ast/ast_drv.c   |  1 -
>>   drivers/gpu/drm/drm_nomodeset.c | 26 +
>>   drivers/gpu/drm/i915/i915_module.c  |  2 --
>>   drivers/gpu/drm/mgag200/mgag200_drv.c   |  1 -
>>   drivers/gpu/drm/nouveau/nouveau_drm.c   |  1 -
>>   drivers/gpu/drm/qxl/qxl_drv.c   |  1 -
>>   drivers/gpu/drm/radeon/radeon_drv.c |  1 -
>>   drivers/gpu/drm/tiny/bochs.c|  1 -
>>   drivers/gpu/drm/tiny/cirrus.c   |  1 -
>>   drivers/gpu/drm/vboxvideo/vbox_drv.c|  1 -
>>   drivers/gpu/drm/virtio/virtgpu_drv.c|  1 -
>>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  1 -
>>   drivers/video/console/vgacon.c  | 21 
>>   include/drm/drm_mode_config.h   |  6 ++
>>   include/linux/console.h |  6 --
>>   17 files changed, 35 insertions(+), 41 deletions(-)
>>   create mode 100644 drivers/gpu/drm/drm_nomodeset.c
>>
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 1c41156deb5f..0e2d60ea93ca 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o 
>> drm_privacy_screen_x86.
>>   
>>   obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
>>   
>> +obj-y += drm_nomodeset.o
> 
> Repeating my other comment, should this rather be protected by a 
> separate config symbol that is selected by CONFIG_DRM?
>

I actually thought about that and my opinion is that obj-y reflects
what we really want here or do you envision this getting disabled
in some cases ?

Probably the problem is Kbuild descending into the drivers/gpu dir
even when CONFIG_DRM is not set. Maybe what we want is something
like the following instead?

diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 835c88318cec..ef12ee05ba6e 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -3,6 +3,7 @@
 # taken to initialize them in the correct order. Link order is the only way
 # to ensure this currently.
 obj-$(CONFIG_TEGRA_HOST1X) += host1x/
-obj-y  += drm/ vga/
+obj-$(CONFIG_DRM)  += drm/
+obj-y  += vga/
 obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
 obj-$(CONFIG_TRACE_GPU_MEM)+= trace/

Best regards, -- 
Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

2021-11-03 Thread Javier Martinez Canillas

Hello Jani,

On 11/3/21 13:56, Jani Nikula wrote:

[snip]

>>  
>> +obj-y += drm_nomodeset.o
> 
> This is a subtle functional change. With this, you'll always have
> __setup("nomodeset", text_mode) builtin and the parameter available. And
> using nomodeset will print out the pr_warn() splat from text_mode(). But
> removing nomodeset will have no impact if CONFIG_VGA_CONSOLE=n as that
> leads to vgacon_text_force() always returning false.
>

Yes, that's what I decided at the end to make it unconditional. That
way the same behaviour is preserved (even when only DRM drivers are
using the exported symbol).
 
> To not make functional changes, this should be:
> 
> obj-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
>

Right, that should work.

> Now, going with the cleanup in this series, maybe we should make the
> functional change, and break the connection to CONFIG_VGA_CONSOLE
> altogether, also in the header?
> 
> (Maybe we'll also need a proxy drm kconfig option to only have
> drm_modeset.o builtin when CONFIG_DRM != n.)
>

See my other email. I believe the issue is drivers/gpu/drm always
being included even when CONFIG_DRM is not set.

Best regards, -- 
Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic

2021-11-03 Thread Thomas Zimmermann


Hi

Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:

[ resend with all relevant people as Cc now, sorry to others for the spam ]

There is a lot of historical baggage on this parameter. It's defined in
the vgacon driver as a "nomodeset" parameter, but it's handler function is
called text_mode() that sets a variable named vgacon_text_mode_force whose
value is queried with a function named vgacon_text_force().

All this implies that it's about forcing text mode for VGA, yet it is not
used in neither vgacon nor other console driver. The only users for these
are DRM drivers, that check for the vgacon_text_force() return value to
determine whether the driver could be loaded or not.

That makes it quite confusing to read the code, because the variables and
function names don't reflect what they actually do and also are not in the
same subsystem as the drivers that make use of them.

This patch-set attempts to cleanup the code by moving the nomodseset param
to the DRM subsystem and do some renaming to make their intention clearer.

There is also another aspect that could be improved, and is the fact that
drivers are checking for the nomodeset being set as an indication if have
to be loaded.

But there may be other reasons why this could be the case, so it is better
to encapsulate the logic in a separate function to make clear what's about.

Patch #1 is just a trivial fix for a comment that isn't referring to the
correct kernel parameter.

Patch #2 moves the nomodeset logic to the DRM subsystem.

Patch #3 renames the vgacon_text_force() function and accompaning logic as
drm_modeset_disabled(), which is what this function is really about.

Patch #4 adds a drm_drv_enabled() function that could be used by drivers
to check if could be enabled.

Patch #5 uses the drm_drv_enabled() function to check this instead of just
checking if nomodeset has been set.


Javier Martinez Canillas (5):
   drm/i915: Fix comment about modeset parameters
   drm: Move nomodeset kernel parameter handler to the DRM subsystem
   drm: Rename vgacon_text_force() function to drm_modeset_disabled()
   drm: Add a drm_drv_enabled() helper function
   drm: Use drm_drv_enabled() instead of drm_modeset_disabled()


There's too much churn here IMHO. Please merge patches 2+3 and 4+5. And 
I'd put patch (4+5) first, so you have the drivers out of the way. After 
that patch (2+3) should only modify drm_drv_enabled().


Best regards
Thomas



  drivers/gpu/drm/Makefile|  2 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  5 ++---
  drivers/gpu/drm/ast/ast_drv.c   |  3 +--
  drivers/gpu/drm/drm_drv.c   | 21 
  drivers/gpu/drm/drm_nomodeset.c | 26 +
  drivers/gpu/drm/i915/i915_module.c  | 10 +-
  drivers/gpu/drm/mgag200/mgag200_drv.c   |  3 +--
  drivers/gpu/drm/nouveau/nouveau_drm.c   |  3 +--
  drivers/gpu/drm/qxl/qxl_drv.c   |  3 +--
  drivers/gpu/drm/radeon/radeon_drv.c |  3 +--
  drivers/gpu/drm/tiny/bochs.c|  3 +--
  drivers/gpu/drm/tiny/cirrus.c   |  3 +--
  drivers/gpu/drm/vboxvideo/vbox_drv.c|  5 +
  drivers/gpu/drm/virtio/virtgpu_drv.c|  3 +--
  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  3 +--
  drivers/video/console/vgacon.c  | 21 
  include/drm/drm_drv.h   |  1 +
  include/drm/drm_mode_config.h   |  6 ++
  include/linux/console.h |  6 --
  19 files changed, 73 insertions(+), 57 deletions(-)
  create mode 100644 drivers/gpu/drm/drm_nomodeset.c



--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled()

2021-11-03 Thread Thomas Zimmermann


Hi

Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:

This function is used by some DRM drivers to determine if the "nomodeset"
kernel command line parameter was set and prevent these drivers to probe.

But the function name is quite confusing and does not reflect what really
the drivers are testing when calling it. Use a better naming now that it
is part of the DRM subsystem.

Also, vgacon_text_force() is guarded by #ifdef CONFIG_VGA_CONSOLE already
so there is no need to do the same when calling the function.

Suggested-by: Suggested-by: Daniel Vetter 
Signed-off-by: Javier Martinez Canillas 
---

  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
  drivers/gpu/drm/ast/ast_drv.c   |  2 +-
  drivers/gpu/drm/drm_nomodeset.c | 16 
  drivers/gpu/drm/i915/i915_module.c  |  2 +-
  drivers/gpu/drm/mgag200/mgag200_drv.c   |  2 +-
  drivers/gpu/drm/nouveau/nouveau_drm.c   |  2 +-
  drivers/gpu/drm/qxl/qxl_drv.c   |  2 +-
  drivers/gpu/drm/radeon/radeon_drv.c |  2 +-
  drivers/gpu/drm/tiny/bochs.c|  2 +-
  drivers/gpu/drm/tiny/cirrus.c   |  2 +-
  drivers/gpu/drm/vboxvideo/vbox_drv.c|  4 +---
  drivers/gpu/drm/virtio/virtgpu_drv.c|  2 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  2 +-
  include/drm/drm_mode_config.h   |  4 ++--
  14 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 2680a2aaa877..f7bd2616cf23 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2513,7 +2513,7 @@ static int __init amdgpu_init(void)
  {
int r;
  
-	if (vgacon_text_force()) {

+   if (drm_modeset_disabled()) {
DRM_ERROR("amdgpu kernel modesetting disabled.\n");


Please remove all such error messages from drivers. 
drm_modeset_disabled() should print a unified message instead.




return -EINVAL;
}
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 048be607b182..6706050414c3 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -232,7 +232,7 @@ static struct pci_driver ast_pci_driver = {
  
  static int __init ast_init(void)

  {
-   if (vgacon_text_force() && ast_modeset == -1)
+   if (drm_modeset_disabled() && ast_modeset == -1)
return -EINVAL;
  
  	if (ast_modeset == 0)

diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
index 1ac9a8d5a8fe..dfc8b30f0625 100644
--- a/drivers/gpu/drm/drm_nomodeset.c
+++ b/drivers/gpu/drm/drm_nomodeset.c
@@ -3,17 +3,17 @@
  #include 
  #include 
  
-static bool vgacon_text_mode_force;

+static bool drm_nomodeset;
  
-bool vgacon_text_force(void)

+bool drm_modeset_disabled(void)


I suggest to rename this function to drm_check_modeset() and have it 
return a negative errno code on failure. This gives maximum flexibility 
and reduces errors in drivers. Right now the drivers return something 
like -EINVAL, which seems wrong. Returning -ENODEV seems more appropriate.



  {
-   return vgacon_text_mode_force;
+   return drm_nomodeset;
  }
-EXPORT_SYMBOL(vgacon_text_force);
+EXPORT_SYMBOL(drm_modeset_disabled);
  
-static int __init text_mode(char *str)

+static int __init disable_modeset(char *str)
  {
-   vgacon_text_mode_force = true;
+   drm_nomodeset = true;
  
  	pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");

pr_warn("Any video related functionality will be severely degraded, and you 
may not even be able to suspend the system properly\n");
@@ -22,5 +22,5 @@ static int __init text_mode(char *str)
return 1;
  }
  
-/* force text mode - used by kernel modesetting */

-__setup("nomodeset", text_mode);
+/* Disable kernel modesetting */
+__setup("nomodeset", disable_modeset);
diff --git a/drivers/gpu/drm/i915/i915_module.c 
b/drivers/gpu/drm/i915/i915_module.c
index 14a59226519d..3e5531040e4d 100644
--- a/drivers/gpu/drm/i915/i915_module.c
+++ b/drivers/gpu/drm/i915/i915_module.c
@@ -29,7 +29,7 @@ static int i915_check_nomodeset(void)
if (i915_modparams.modeset == 0)
use_kms = false;
  
-	if (vgacon_text_force() && i915_modparams.modeset == -1)

+   if (drm_modeset_disabled() && i915_modparams.modeset == -1)
use_kms = false;
  
  	if (!use_kms) {

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 685e766db6a4..7ee87564bade 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -377,7 +377,7 @@ static struct pci_driver mgag200_pci_driver = {
  
  static int __init mgag200_init(void)

  {
-   if (vgacon_text_force() && mgag200_modeset == -1)
+   if (drm_modeset_disabled() && mgag200_modeset == -1)
return -EINVAL;
  
  	if (mgag200_modeset == 0)

diff --git

Re: [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

2021-11-03 Thread Jani Nikula

On Wed, 03 Nov 2021, Javier Martinez Canillas  wrote:
> The "nomodeset" kernel cmdline parameter is handled by the vgacon driver
> but the exported vgacon_text_force() symbol is only used by DRM drivers.
>
> It makes much more sense for the parameter logic to be in the subsystem
> of the drivers that are making use of it. Let's move that to DRM.
>
> Suggested-by: Daniel Vetter 
> Signed-off-by: Javier Martinez Canillas 
> ---
>
>  drivers/gpu/drm/Makefile|  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
>  drivers/gpu/drm/ast/ast_drv.c   |  1 -
>  drivers/gpu/drm/drm_nomodeset.c | 26 +
>  drivers/gpu/drm/i915/i915_module.c  |  2 --
>  drivers/gpu/drm/mgag200/mgag200_drv.c   |  1 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c   |  1 -
>  drivers/gpu/drm/qxl/qxl_drv.c   |  1 -
>  drivers/gpu/drm/radeon/radeon_drv.c |  1 -
>  drivers/gpu/drm/tiny/bochs.c|  1 -
>  drivers/gpu/drm/tiny/cirrus.c   |  1 -
>  drivers/gpu/drm/vboxvideo/vbox_drv.c|  1 -
>  drivers/gpu/drm/virtio/virtgpu_drv.c|  1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  1 -
>  drivers/video/console/vgacon.c  | 21 
>  include/drm/drm_mode_config.h   |  6 ++
>  include/linux/console.h |  6 --
>  17 files changed, 35 insertions(+), 41 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_nomodeset.c
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1c41156deb5f..0e2d60ea93ca 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o 
> drm_privacy_screen_x86.
>  
>  obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
>  
> +obj-y += drm_nomodeset.o

This is a subtle functional change. With this, you'll always have
__setup("nomodeset", text_mode) builtin and the parameter available. And
using nomodeset will print out the pr_warn() splat from text_mode(). But
removing nomodeset will have no impact if CONFIG_VGA_CONSOLE=n as that
leads to vgacon_text_force() always returning false.

To not make functional changes, this should be:

obj-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o

Now, going with the cleanup in this series, maybe we should make the
functional change, and break the connection to CONFIG_VGA_CONSOLE
altogether, also in the header?

(Maybe we'll also need a proxy drm kconfig option to only have
drm_modeset.o builtin when CONFIG_DRM != n.)

> +
>  drm_cma_helper-y := drm_gem_cma_helper.o
>  obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index c718fb5f3f8a..2680a2aaa877 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -31,7 +31,6 @@
>  #include "amdgpu_drv.h"
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -2515,7 +2514,7 @@ static int __init amdgpu_init(void)
>   int r;
>  
>   if (vgacon_text_force()) {
> - DRM_ERROR("VGACON disables amdgpu kernel modesetting.\n");
> + DRM_ERROR("amdgpu kernel modesetting disabled.\n");
>   return -EINVAL;
>   }
>  
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 86d5cd7b6318..048be607b182 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -26,7 +26,6 @@
>   * Authors: Dave Airlie 
>   */
>  
> -#include 
>  #include 
>  #include 
>  
> diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
> new file mode 100644
> index ..1ac9a8d5a8fe
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_nomodeset.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include 
> +#include 
> +
> +static bool vgacon_text_mode_force;
> +
> +bool vgacon_text_force(void)
> +{
> + return vgacon_text_mode_force;
> +}
> +EXPORT_SYMBOL(vgacon_text_force);
> +
> +static int __init text_mode(char *str)
> +{
> + vgacon_text_mode_force = true;
> +
> + pr_warn("You have booted with nomodeset. This means your GPU drivers 
> are DISABLED\n");
> + pr_warn("Any video related functionality will be severely degraded, and 
> you may not even be able to suspend the system properly\n");
> + pr_warn("Unless you actually understand what nomodeset does, you should 
> reboot without enabling it\n");
> +
> + return 1;
> +}
> +
> +/* force text mode - used by kernel modesetting */
> +__setup("nomodeset", text_mode);
> diff --git a/drivers/gpu/drm/i915/i915_module.c 
> b/drivers/gpu/drm/i915/i915_module.c
> index c7507266aa83..14a59226519d 100644
> --- a/drivers/gpu/drm/i915/i915_module.c
> +++ b/drivers/gpu/drm/i915/i915_module.c
> @@ -4,8 +4,6 @@
>   * Copyright © 2021 Intel Corporation
>   */
>  
> -#include 
> -
>  #include "gem/i915_gem_context.h"
>  #include "gem/i915_gem_object.h"
>  #include

[RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic

2021-11-03 Thread Javier Martinez Canillas

[ resend with all relevant people as Cc now, sorry to others for the spam ]

There is a lot of historical baggage on this parameter. It's defined in
the vgacon driver as a "nomodeset" parameter, but it's handler function is
called text_mode() that sets a variable named vgacon_text_mode_force whose
value is queried with a function named vgacon_text_force().

All this implies that it's about forcing text mode for VGA, yet it is not
used in neither vgacon nor other console driver. The only users for these
are DRM drivers, that check for the vgacon_text_force() return value to
determine whether the driver could be loaded or not.

That makes it quite confusing to read the code, because the variables and
function names don't reflect what they actually do and also are not in the
same subsystem as the drivers that make use of them.

This patch-set attempts to cleanup the code by moving the nomodseset param
to the DRM subsystem and do some renaming to make their intention clearer.

There is also another aspect that could be improved, and is the fact that
drivers are checking for the nomodeset being set as an indication if have
to be loaded.

But there may be other reasons why this could be the case, so it is better
to encapsulate the logic in a separate function to make clear what's about.

Patch #1 is just a trivial fix for a comment that isn't referring to the
correct kernel parameter.

Patch #2 moves the nomodeset logic to the DRM subsystem.

Patch #3 renames the vgacon_text_force() function and accompaning logic as
drm_modeset_disabled(), which is what this function is really about.

Patch #4 adds a drm_drv_enabled() function that could be used by drivers
to check if could be enabled.

Patch #5 uses the drm_drv_enabled() function to check this instead of just
checking if nomodeset has been set.


Javier Martinez Canillas (5):
  drm/i915: Fix comment about modeset parameters
  drm: Move nomodeset kernel parameter handler to the DRM subsystem
  drm: Rename vgacon_text_force() function to drm_modeset_disabled()
  drm: Add a drm_drv_enabled() helper function
  drm: Use drm_drv_enabled() instead of drm_modeset_disabled()

 drivers/gpu/drm/Makefile|  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  5 ++---
 drivers/gpu/drm/ast/ast_drv.c   |  3 +--
 drivers/gpu/drm/drm_drv.c   | 21 
 drivers/gpu/drm/drm_nomodeset.c | 26 +
 drivers/gpu/drm/i915/i915_module.c  | 10 +-
 drivers/gpu/drm/mgag200/mgag200_drv.c   |  3 +--
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  3 +--
 drivers/gpu/drm/qxl/qxl_drv.c   |  3 +--
 drivers/gpu/drm/radeon/radeon_drv.c |  3 +--
 drivers/gpu/drm/tiny/bochs.c|  3 +--
 drivers/gpu/drm/tiny/cirrus.c   |  3 +--
 drivers/gpu/drm/vboxvideo/vbox_drv.c|  5 +
 drivers/gpu/drm/virtio/virtgpu_drv.c|  3 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  3 +--
 drivers/video/console/vgacon.c  | 21 
 include/drm/drm_drv.h   |  1 +
 include/drm/drm_mode_config.h   |  6 ++
 include/linux/console.h |  6 --
 19 files changed, 73 insertions(+), 57 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_nomodeset.c

-- 
2.33.1

[RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

2021-11-03 Thread Javier Martinez Canillas

The "nomodeset" kernel cmdline parameter is handled by the vgacon driver
but the exported vgacon_text_force() symbol is only used by DRM drivers.

It makes much more sense for the parameter logic to be in the subsystem
of the drivers that are making use of it. Let's move that to DRM.

Suggested-by: Daniel Vetter 
Signed-off-by: Javier Martinez Canillas 
---

 drivers/gpu/drm/Makefile|  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
 drivers/gpu/drm/ast/ast_drv.c   |  1 -
 drivers/gpu/drm/drm_nomodeset.c | 26 +
 drivers/gpu/drm/i915/i915_module.c  |  2 --
 drivers/gpu/drm/mgag200/mgag200_drv.c   |  1 -
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  1 -
 drivers/gpu/drm/qxl/qxl_drv.c   |  1 -
 drivers/gpu/drm/radeon/radeon_drv.c |  1 -
 drivers/gpu/drm/tiny/bochs.c|  1 -
 drivers/gpu/drm/tiny/cirrus.c   |  1 -
 drivers/gpu/drm/vboxvideo/vbox_drv.c|  1 -
 drivers/gpu/drm/virtio/virtgpu_drv.c|  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  1 -
 drivers/video/console/vgacon.c  | 21 
 include/drm/drm_mode_config.h   |  6 ++
 include/linux/console.h |  6 --
 17 files changed, 35 insertions(+), 41 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_nomodeset.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1c41156deb5f..0e2d60ea93ca 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o 
drm_privacy_screen_x86.
 
 obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
 
+obj-y += drm_nomodeset.o
+
 drm_cma_helper-y := drm_gem_cma_helper.o
 obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index c718fb5f3f8a..2680a2aaa877 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -31,7 +31,6 @@
 #include "amdgpu_drv.h"
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -2515,7 +2514,7 @@ static int __init amdgpu_init(void)
int r;
 
if (vgacon_text_force()) {
-   DRM_ERROR("VGACON disables amdgpu kernel modesetting.\n");
+   DRM_ERROR("amdgpu kernel modesetting disabled.\n");
return -EINVAL;
}
 
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 86d5cd7b6318..048be607b182 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -26,7 +26,6 @@
  * Authors: Dave Airlie 
  */
 
-#include 
 #include 
 #include 
 
diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
new file mode 100644
index ..1ac9a8d5a8fe
--- /dev/null
+++ b/drivers/gpu/drm/drm_nomodeset.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+
+static bool vgacon_text_mode_force;
+
+bool vgacon_text_force(void)
+{
+   return vgacon_text_mode_force;
+}
+EXPORT_SYMBOL(vgacon_text_force);
+
+static int __init text_mode(char *str)
+{
+   vgacon_text_mode_force = true;
+
+   pr_warn("You have booted with nomodeset. This means your GPU drivers 
are DISABLED\n");
+   pr_warn("Any video related functionality will be severely degraded, and 
you may not even be able to suspend the system properly\n");
+   pr_warn("Unless you actually understand what nomodeset does, you should 
reboot without enabling it\n");
+
+   return 1;
+}
+
+/* force text mode - used by kernel modesetting */
+__setup("nomodeset", text_mode);
diff --git a/drivers/gpu/drm/i915/i915_module.c 
b/drivers/gpu/drm/i915/i915_module.c
index c7507266aa83..14a59226519d 100644
--- a/drivers/gpu/drm/i915/i915_module.c
+++ b/drivers/gpu/drm/i915/i915_module.c
@@ -4,8 +4,6 @@
  * Copyright © 2021 Intel Corporation
  */
 
-#include 
-
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_object.h"
 #include "i915_active.h"
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 6b9243713b3c..685e766db6a4 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -6,7 +6,6 @@
  *  Dave Airlie
  */
 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 1f828c9f691c..029997f50d1a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -22,7 +22,6 @@
  * Authors: Ben Skeggs
  */
 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index fc47b0deb021..3cd6bd9f059d 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -29,7 +29,6 @@
 
 #include "qxl_drv.h"
 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c

[RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled()

2021-11-03 Thread Javier Martinez Canillas

This function is used by some DRM drivers to determine if the "nomodeset"
kernel command line parameter was set and prevent these drivers to probe.

But the function name is quite confusing and does not reflect what really
the drivers are testing when calling it. Use a better naming now that it
is part of the DRM subsystem.

Also, vgacon_text_force() is guarded by #ifdef CONFIG_VGA_CONSOLE already
so there is no need to do the same when calling the function.

Suggested-by: Suggested-by: Daniel Vetter 
Signed-off-by: Javier Martinez Canillas 
---

 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 drivers/gpu/drm/ast/ast_drv.c   |  2 +-
 drivers/gpu/drm/drm_nomodeset.c | 16 
 drivers/gpu/drm/i915/i915_module.c  |  2 +-
 drivers/gpu/drm/mgag200/mgag200_drv.c   |  2 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  2 +-
 drivers/gpu/drm/qxl/qxl_drv.c   |  2 +-
 drivers/gpu/drm/radeon/radeon_drv.c |  2 +-
 drivers/gpu/drm/tiny/bochs.c|  2 +-
 drivers/gpu/drm/tiny/cirrus.c   |  2 +-
 drivers/gpu/drm/vboxvideo/vbox_drv.c|  4 +---
 drivers/gpu/drm/virtio/virtgpu_drv.c|  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  2 +-
 include/drm/drm_mode_config.h   |  4 ++--
 14 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 2680a2aaa877..f7bd2616cf23 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2513,7 +2513,7 @@ static int __init amdgpu_init(void)
 {
int r;
 
-   if (vgacon_text_force()) {
+   if (drm_modeset_disabled()) {
DRM_ERROR("amdgpu kernel modesetting disabled.\n");
return -EINVAL;
}
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 048be607b182..6706050414c3 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -232,7 +232,7 @@ static struct pci_driver ast_pci_driver = {
 
 static int __init ast_init(void)
 {
-   if (vgacon_text_force() && ast_modeset == -1)
+   if (drm_modeset_disabled() && ast_modeset == -1)
return -EINVAL;
 
if (ast_modeset == 0)
diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
index 1ac9a8d5a8fe..dfc8b30f0625 100644
--- a/drivers/gpu/drm/drm_nomodeset.c
+++ b/drivers/gpu/drm/drm_nomodeset.c
@@ -3,17 +3,17 @@
 #include 
 #include 
 
-static bool vgacon_text_mode_force;
+static bool drm_nomodeset;
 
-bool vgacon_text_force(void)
+bool drm_modeset_disabled(void)
 {
-   return vgacon_text_mode_force;
+   return drm_nomodeset;
 }
-EXPORT_SYMBOL(vgacon_text_force);
+EXPORT_SYMBOL(drm_modeset_disabled);
 
-static int __init text_mode(char *str)
+static int __init disable_modeset(char *str)
 {
-   vgacon_text_mode_force = true;
+   drm_nomodeset = true;
 
pr_warn("You have booted with nomodeset. This means your GPU drivers 
are DISABLED\n");
pr_warn("Any video related functionality will be severely degraded, and 
you may not even be able to suspend the system properly\n");
@@ -22,5 +22,5 @@ static int __init text_mode(char *str)
return 1;
 }
 
-/* force text mode - used by kernel modesetting */
-__setup("nomodeset", text_mode);
+/* Disable kernel modesetting */
+__setup("nomodeset", disable_modeset);
diff --git a/drivers/gpu/drm/i915/i915_module.c 
b/drivers/gpu/drm/i915/i915_module.c
index 14a59226519d..3e5531040e4d 100644
--- a/drivers/gpu/drm/i915/i915_module.c
+++ b/drivers/gpu/drm/i915/i915_module.c
@@ -29,7 +29,7 @@ static int i915_check_nomodeset(void)
if (i915_modparams.modeset == 0)
use_kms = false;
 
-   if (vgacon_text_force() && i915_modparams.modeset == -1)
+   if (drm_modeset_disabled() && i915_modparams.modeset == -1)
use_kms = false;
 
if (!use_kms) {
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 685e766db6a4..7ee87564bade 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -377,7 +377,7 @@ static struct pci_driver mgag200_pci_driver = {
 
 static int __init mgag200_init(void)
 {
-   if (vgacon_text_force() && mgag200_modeset == -1)
+   if (drm_modeset_disabled() && mgag200_modeset == -1)
return -EINVAL;
 
if (mgag200_modeset == 0)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 029997f50d1a..903d0e626954 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1321,7 +1321,7 @@ nouveau_drm_init(void)
nouveau_display_options();
 
if (nouveau_modeset == -1) {
-   if (vgacon_text_force())
+   if (drm_modeset_disabled())
nouveau_modeset = 0;
}
 
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c

Re: [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

2021-11-03 Thread Thomas Zimmermann


Hi

Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:

The "nomodeset" kernel cmdline parameter is handled by the vgacon driver
but the exported vgacon_text_force() symbol is only used by DRM drivers.

It makes much more sense for the parameter logic to be in the subsystem
of the drivers that are making use of it. Let's move that to DRM.

Suggested-by: Daniel Vetter 
Signed-off-by: Javier Martinez Canillas 
---

  drivers/gpu/drm/Makefile|  2 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
  drivers/gpu/drm/ast/ast_drv.c   |  1 -
  drivers/gpu/drm/drm_nomodeset.c | 26 +
  drivers/gpu/drm/i915/i915_module.c  |  2 --
  drivers/gpu/drm/mgag200/mgag200_drv.c   |  1 -
  drivers/gpu/drm/nouveau/nouveau_drm.c   |  1 -
  drivers/gpu/drm/qxl/qxl_drv.c   |  1 -
  drivers/gpu/drm/radeon/radeon_drv.c |  1 -
  drivers/gpu/drm/tiny/bochs.c|  1 -
  drivers/gpu/drm/tiny/cirrus.c   |  1 -
  drivers/gpu/drm/vboxvideo/vbox_drv.c|  1 -
  drivers/gpu/drm/virtio/virtgpu_drv.c|  1 -
  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  1 -
  drivers/video/console/vgacon.c  | 21 
  include/drm/drm_mode_config.h   |  6 ++
  include/linux/console.h |  6 --
  17 files changed, 35 insertions(+), 41 deletions(-)
  create mode 100644 drivers/gpu/drm/drm_nomodeset.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1c41156deb5f..0e2d60ea93ca 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o 
drm_privacy_screen_x86.
  
  obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
  
+obj-y += drm_nomodeset.o


Repeating my other comment, should this rather be protected by a 
separate config symbol that is selected by CONFIG_DRM?


Best regards
Thomas


+
  drm_cma_helper-y := drm_gem_cma_helper.o
  obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c

index c718fb5f3f8a..2680a2aaa877 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -31,7 +31,6 @@
  #include "amdgpu_drv.h"
  
  #include 

-#include 
  #include 
  #include 
  #include 
@@ -2515,7 +2514,7 @@ static int __init amdgpu_init(void)
int r;
  
  	if (vgacon_text_force()) {

-   DRM_ERROR("VGACON disables amdgpu kernel modesetting.\n");
+   DRM_ERROR("amdgpu kernel modesetting disabled.\n");
return -EINVAL;
}
  
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c

index 86d5cd7b6318..048be607b182 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -26,7 +26,6 @@
   * Authors: Dave Airlie 
   */
  
-#include 

  #include 
  #include 
  
diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c

new file mode 100644
index ..1ac9a8d5a8fe
--- /dev/null
+++ b/drivers/gpu/drm/drm_nomodeset.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+
+static bool vgacon_text_mode_force;
+
+bool vgacon_text_force(void)
+{
+   return vgacon_text_mode_force;
+}
+EXPORT_SYMBOL(vgacon_text_force);
+
+static int __init text_mode(char *str)
+{
+   vgacon_text_mode_force = true;
+
+   pr_warn("You have booted with nomodeset. This means your GPU drivers are 
DISABLED\n");
+   pr_warn("Any video related functionality will be severely degraded, and you 
may not even be able to suspend the system properly\n");
+   pr_warn("Unless you actually understand what nomodeset does, you should 
reboot without enabling it\n");
+
+   return 1;
+}
+
+/* force text mode - used by kernel modesetting */
+__setup("nomodeset", text_mode);
diff --git a/drivers/gpu/drm/i915/i915_module.c 
b/drivers/gpu/drm/i915/i915_module.c
index c7507266aa83..14a59226519d 100644
--- a/drivers/gpu/drm/i915/i915_module.c
+++ b/drivers/gpu/drm/i915/i915_module.c
@@ -4,8 +4,6 @@
   * Copyright © 2021 Intel Corporation
   */
  
-#include 

-
  #include "gem/i915_gem_context.h"
  #include "gem/i915_gem_object.h"
  #include "i915_active.h"
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 6b9243713b3c..685e766db6a4 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -6,7 +6,6 @@
   *  Dave Airlie
   */
  
-#include 

  #include 
  #include 
  #include 
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 1f828c9f691c..029997f50d1a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -22,7 +22,6 @@
   * Authors: Ben Skeggs
   */
  
-#include 

  #include 
  #include 
  #include 
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index

Re: [PATCH 01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate

2021-11-03 Thread Ville Syrjälä

On Tue, Nov 02, 2021 at 03:59:32PM +0100, Maxime Ripard wrote:
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct 
> drm_connector *connector,
>   u32 max_tmds_clock = hf_vsdb[5] * 5000;
>   struct drm_scdc *scdc = >scdc;
>  
> - if (max_tmds_clock > 34) {
> + if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
>   display->max_tmds_clock = max_tmds_clock;
>   DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
>   display->max_tmds_clock);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index d2e61f6c6e08..0666203d52b7 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct intel_encoder 
> *encoder,
>   if (scdc->scrambling.low_rates)
>   pipe_config->hdmi_scrambling = true;
>  
> - if (pipe_config->port_clock > 34) {
> + if (pipe_config->port_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
>   pipe_config->hdmi_scrambling = true;
>   pipe_config->hdmi_high_tmds_clock_ratio = true;
>   }

All of that is HDMI 2.0 stuff. So this just makes it all super
confusing IMO. Nak.

-- 
Ville Syrjälä
Intel

Re: [PATCH 01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate

2021-11-03 Thread Neil Armstrong

On 02/11/2021 15:59, Maxime Ripard wrote:
> A lot of drivers open-code the HDMI 1.4 maximum pixel rate in their
> driver to test whether the resolutions are supported or if the
> scrambling needs to be enabled.
> 
> Let's create a common define for everyone to use it.
> 
> Cc: Alex Deucher 
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Andrzej Hajda 
> Cc: Benjamin Gaignard 
> Cc: "Christian König" 
> Cc: Emma Anholt 
> Cc: intel-...@lists.freedesktop.org
> Cc: Jani Nikula 
> Cc: Jernej Skrabec 
> Cc: Jerome Brunet 
> Cc: Jonas Karlman 
> Cc: Jonathan Hunter 
> Cc: Joonas Lahtinen 
> Cc: Kevin Hilman 
> Cc: Laurent Pinchart 
> Cc: linux-amlo...@lists.infradead.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-te...@vger.kernel.org
> Cc: Martin Blumenstingl 
> Cc: Neil Armstrong 
> Cc: "Pan, Xinhui" 
> Cc: Robert Foss 
> Cc: Rodrigo Vivi 
> Cc: Thierry Reding 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c  | 4 ++--
>  drivers/gpu/drm/drm_edid.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c  | 2 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c  | 4 ++--
>  drivers/gpu/drm/radeon/radeon_encoders.c   | 2 +-
>  drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.c | 2 +-
>  drivers/gpu/drm/tegra/sor.c| 8 
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ++--
>  include/drm/drm_connector.h| 2 ++
>  9 files changed, 16 insertions(+), 14 deletions(-)

For meson & bridge/synopsys/dw-hdmi:

Acked-by: Neil Armstrong 

> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 62ae63565d3a..3a58db357be0 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -46,7 +46,7 @@
>  /* DW-HDMI Controller >= 0x200a are at least compliant with SCDC version 1 */
>  #define SCDC_MIN_SOURCE_VERSION  0x1
>  
> -#define HDMI14_MAX_TMDSCLK   34000
> +#define HDMI14_MAX_TMDSCLK   (DRM_HDMI_14_MAX_TMDS_CLK_KHZ * 1000)
>  
>  enum hdmi_datamap {
>   RGB444_8B = 0x01,
> @@ -1264,7 +1264,7 @@ static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi,
>* for low rates is not supported either
>*/
>   if (!display->hdmi.scdc.scrambling.low_rates &&
> - display->max_tmds_clock <= 34)
> + display->max_tmds_clock <= DRM_HDMI_14_MAX_TMDS_CLK_KHZ)
>   return false;
>  
>   return true;
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7aa2a56a71c8..ec8fb2d098ae 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct 
> drm_connector *connector,
>   u32 max_tmds_clock = hf_vsdb[5] * 5000;
>   struct drm_scdc *scdc = >scdc;
>  
> - if (max_tmds_clock > 34) {
> + if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
>   display->max_tmds_clock = max_tmds_clock;
>   DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
>   display->max_tmds_clock);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index d2e61f6c6e08..0666203d52b7 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct intel_encoder 
> *encoder,
>   if (scdc->scrambling.low_rates)
>   pipe_config->hdmi_scrambling = true;
>  
> - if (pipe_config->port_clock > 34) {
> + if (pipe_config->port_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
>   pipe_config->hdmi_scrambling = true;
>   pipe_config->hdmi_high_tmds_clock_ratio = true;
>   }
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
> b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 0afbd1e70bfc..8078667aea0e 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -434,7 +434,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void 
> *data,
>   readl_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING));
>  
>   DRM_DEBUG_DRIVER("\"%s\" div%d\n", mode->name,
> -  mode->clock > 34 ? 40 : 10);
> +  mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ ? 40 : 10);
>  
>   /* Enable clocks */
>   regmap_update_bits(priv->hhi, HHI_HDMI_CLK_CNTL, 0x, 0x100);
> @@ -457,7 +457,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void 
> *data,
>   dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12));
>  
>   /* TMDS pattern setup */
> - if (mode->clock > 34 &&
> + if (mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ &&
>   dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_YUV8_1X24) {
>   dw_hdmi->data->top_write(dw_hdmi,

Re: [PATCH 6/6] drm/radeon: use dma_resv_wait_timeout() instead of manually waiting

2021-11-03 Thread Christian König


Ping, Alex do you have a moment for that one here?

Am 28.10.21 um 15:26 schrieb Christian König:

Don't touch the exclusive fence manually here, but rather use the
general dma_resv function. We did that for better hw reset handling but
this doesn't necessary work correctly.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/radeon/radeon_uvd.c | 13 +
  1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index 2ea86919d953..377f9cdb5b53 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -469,7 +469,6 @@ static int radeon_uvd_cs_msg(struct radeon_cs_parser *p, 
struct radeon_bo *bo,
  {
int32_t *msg, msg_type, handle;
unsigned img_size = 0;
-   struct dma_fence *f;
void *ptr;
  
  	int i, r;

@@ -479,13 +478,11 @@ static int radeon_uvd_cs_msg(struct radeon_cs_parser *p, 
struct radeon_bo *bo,
return -EINVAL;
}
  
-	f = dma_resv_excl_fence(bo->tbo.base.resv);

-   if (f) {
-   r = radeon_fence_wait((struct radeon_fence *)f, false);
-   if (r) {
-   DRM_ERROR("Failed waiting for UVD message (%d)!\n", r);
-   return r;
-   }
+   r = dma_resv_wait_timeout(bo->tbo.base.resv, false, false,
+ MAX_SCHEDULE_TIMEOUT);
+   if (r <= 0) {
+   DRM_ERROR("Failed waiting for UVD message (%d)!\n", r);
+   return r ? r : -ETIME;
}
  
  	r = radeon_bo_kmap(bo, );

37 matches

Mail list logo