AMDGPU VCE 1: some info needed

2021-01-07 Thread Alexandre Demers
Hi there,

Some of you may remember I was working on porting VCE 1 from Radeon to
AMDGPU a few years ago... about 3 and a half years. I hadn't had time
to work on it until last Holidays. But why do I persist in this work?
Because GCN 1st gen was still used in some GPU produced 4 years ago
(Radeon 520 and just before R5 and R7 in the entry level).

I'm pretty happy with where the code is sitting now, however I have
some questions.

1- should the firmware be validated like it was under Radeon and as it
is done for the newly ported UVD 3.1 code? This would mean having to
work with keyselect, isn't it?

2- last time I worked on VCE 1.0, Christian was saying that it was
possible a new VCE firmware could be provided for AMDGPU. Then, it
wasn't that clear, GCN 1.0 (SI) being in trouble and it was considered
to strip it from AMDGPU. And a few months ago, UVD and DC were added
for SI to AMDGPU and a new UVD firmware was released (yeah!). So, is
it possible to have a new VCE firmware? I produced an "updated" tahiti
VCE file where a header is added (script available on my account on
GitHub). Still, if this can be useful, I'd prefer an official
firmware.

3- is there any documentation about VCE 1.0 that would help me
complete this work?
3.1- Some variables that were previously defined are not available
under sid.c, vce_v1_0_d.h, vce_v1_0_sh_mask.h and others. Since the
new values (mostly in the range of 0x8xxx) are completely different
from the ones defined under Radeon (in the range of 0x2), I'd like
to be sure to use the good ones. I would assume the masks and shifts
are still valid though.

3.2- Some statuses are undefined, sometimes magic values appear here
and there without being ever defined or documented (status 0x337f
anyone?), even under CIK or they don't seem to be easily portable from
other VCE versions. Having a name for a value is really helpful
without an official documentation, when the code is supposed to be
self-documented. I've been able to identify some of them by looking at
variables used under Radeon or under AMDGPU's UVD 3.1. Interestingly,
some variables were previously defined under Radeon, but were left
aside in AMDGPU...

3.3- Being able to know how to properly set/reset which part, in what
order, etc.

4- Any input about 40 bit address limitation on VCE 1.0 and how to
handle it if it applies?

5- Any chance to have some code reviewed even if it still doesn't work
if I send it on this list?

6- I have some patches on the side to help document the code and
define variables (even for Radeon), a few typos fixed, etc. Should I
send them on this list?

Cheers

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


Re: [kbuild-all] Re: [PATCH v3 8/8] drm: Upcast struct drm_device.dev to struct pci_device; replace pdev

2021-01-07 Thread Rong Chen

Hi Thomas,

Thanks for the feedback, do you mean the patch was applied to a wrong base?

Best Regards,
Rong Chen

On 1/7/21 6:45 PM, Thomas Zimmermann wrote:

AFAICT these are false positives. The instances have been fixed already.

Am 07.01.21 um 10:45 schrieb kernel test robot:

Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next linus/master v5.11-rc2 
next-20210104]

[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: 
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Move-struct-drm_device-pdev-to-legacy/20210107-161007

base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-s021-20210107 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
 # apt-get install sparse
 # sparse version: v0.6.3-208-g46a52ca4-dirty
 # 
https://github.com/0day-ci/linux/commit/380912f7b62c23322562c40e19efd7ad84d57e9c

 git remote add linux-review https://github.com/0day-ci/linux
 git fetch --no-tags linux-review 
Thomas-Zimmermann/drm-Move-struct-drm_device-pdev-to-legacy/20210107-161007

 git checkout 380912f7b62c23322562c40e19efd7ad84d57e9c
 # save the attached .config to linux build tree
 make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' 
ARCH=x86_64


If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

    drivers/gpu/drm/gma500/oaktrail_device.c: In function 
'oaktrail_chip_setup':
drivers/gpu/drm/gma500/oaktrail_device.c:509:26: error: 'struct 
drm_device' has no member named 'pdev'; did you mean 'dev'?

  509 |  if (pci_enable_msi(dev->pdev))
  |  ^~~~
  |  dev
--
    drivers/gpu/drm/gma500/oaktrail_lvds.c: In function 
'oaktrail_lvds_set_power':
drivers/gpu/drm/gma500/oaktrail_lvds.c:63:25: error: 'struct 
drm_device' has no member named 'pdev'; did you mean 'dev'?

   63 |   pm_request_idle(>pdev->dev);
  | ^~~~
  | dev
--
    drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c: In function 'get_clock':
    drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c:69:11: warning: 
variable 'tmp' set but not used [-Wunused-but-set-variable]

   69 |  u32 val, tmp;
  |   ^~~
    drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c: In function 'get_data':
    drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c:83:11: warning: 
variable 'tmp' set but not used [-Wunused-but-set-variable]

   83 |  u32 val, tmp;
  |   ^~~
    drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c: In function 
'oaktrail_lvds_i2c_init':
drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c:148:35: error: 'struct 
drm_device' has no member named 'pdev'; did you mean 'dev'?

  148 |  chan->adapter.dev.parent = >pdev->dev;
  |   ^~~~
  |   dev
--
    drivers/gpu/drm/vmwgfx/vmwgfx_drv.c: In function 'vmw_driver_load':
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:661:22: error: 'struct 
drm_device' has no member named 'pdev'; did you mean 'dev'?

  661 |  pci_set_master(dev->pdev);
  |  ^~~~
  |  dev
    In file included from drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:31:
    drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:690:47: error: 'struct 
drm_device' has no member named 'pdev'; did you mean 'dev'?

  690 |  dev_priv->io_start = pci_resource_start(dev->pdev, 0);
  |   ^~~~
    include/linux/pci.h:1854:40: note: in definition of macro 
'pci_resource_start'
 1854 | #define pci_resource_start(dev, bar) 
((dev)->resource[(bar)].start)

  |    ^~~
    drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:691:49: error: 'struct 
drm_device' has no member named 'pdev'; did you mean 'dev'?

  691 |  dev_priv->vram_start = pci_resource_start(dev->pdev, 1);
  | ^~~~
    include/linux/pci.h:1854:40: note: in definition of macro 
'pci_resource_start'
 1854 | #define pci_resource_start(dev, bar) 
((dev)->resource[(bar)].start)

  |    ^~~
    drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:692:49: error: 'struct 
drm_device' has no member named 'pdev'; did you mean 'dev'?

  692 |  dev_priv->mmio_start = pci_resource_start(dev->pdev, 2);
  | ^~~~
    include/linux/pci.h:1854:40: note: in definition of macro 
'pci_resource_start'
 1854 | #define pci_reso

RE: [PATCH 3/3] Revert "drm/amd/display: Expose new CRC window property"

2021-01-07 Thread Lin, Wayne
[AMD Public Use]

Thanks Siqueira.
Comments below.

> -Original Message-
> From: Siqueira, Rodrigo 
> Sent: Friday, January 8, 2021 4:53 AM
> To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> Cc: Lin, Wayne ; Deucher, Alexander 
> ; Wentland, Harry
> ; Li, Roman ; R, Bindu 
> ; Daniel Vetter 
> Subject: [PATCH 3/3] Revert "drm/amd/display: Expose new CRC window property"
>
> This reverts commit 110d586ba77ed573eb7464ca69b6490ec0b70c5f.
>
> Cc: Wayne Lin 
> Cc: Alexander Deucher 
> Cc: Harry Wentland 
> Cc: Roman Li 
> Cc: Bindu R 
> Cc: Daniel Vetter 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 142 +-  
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |
> 19 ---  .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c |  56 +--
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h |   3 -
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c |   2 +
>  5 files changed, 12 insertions(+), 210 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index a06554745238..0515ed0d125c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -938,41 +938,6 @@ static void mmhub_read_system_context(struct 
> amdgpu_device *adev, struct dc_phy_  }  #endif
>
> -#ifdef CONFIG_DEBUG_FS
> -static int create_crtc_crc_properties(struct amdgpu_display_manager *dm) -{
> -dm->crc_win_x_start_property =
> -drm_property_create_range(adev_to_drm(dm->adev),
> -  DRM_MODE_PROP_ATOMIC,
> -  "AMD_CRC_WIN_X_START", 0, U16_MAX);
> -if (!dm->crc_win_x_start_property)
> -return -ENOMEM;
> -
> -dm->crc_win_y_start_property =
> -drm_property_create_range(adev_to_drm(dm->adev),
> -  DRM_MODE_PROP_ATOMIC,
> -  "AMD_CRC_WIN_Y_START", 0, U16_MAX);
> -if (!dm->crc_win_y_start_property)
> -return -ENOMEM;
> -
> -dm->crc_win_x_end_property =
> -drm_property_create_range(adev_to_drm(dm->adev),
> -  DRM_MODE_PROP_ATOMIC,
> -  "AMD_CRC_WIN_X_END", 0, U16_MAX);
> -if (!dm->crc_win_x_end_property)
> -return -ENOMEM;
> -
> -dm->crc_win_y_end_property =
> -drm_property_create_range(adev_to_drm(dm->adev),
> -  DRM_MODE_PROP_ATOMIC,
> -  "AMD_CRC_WIN_Y_END", 0, U16_MAX);
> -if (!dm->crc_win_y_end_property)
> -return -ENOMEM;
> -
> -return 0;
> -}
> -#endif
> -
>  static int amdgpu_dm_init(struct amdgpu_device *adev)  {
>  struct dc_init_data init_data;
> @@ -1119,10 +1084,6 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>
>  dc_init_callbacks(adev->dm.dc, _params);
>  }
> -#endif
> -#ifdef CONFIG_DEBUG_FS
> -if (create_crtc_crc_properties(>dm))
> -DRM_ERROR("amdgpu: failed to create crc property.\n");
>  #endif
>  if (amdgpu_dm_initialize_drm_device(adev)) {
>  DRM_ERROR(
> @@ -5456,64 +5417,12 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
>  state->crc_src = cur->crc_src;
>  state->cm_has_degamma = cur->cm_has_degamma;
>  state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb; -#ifdef CONFIG_DEBUG_FS
> -state->crc_window = cur->crc_window;
> -#endif
> +
>  /* TODO Duplicate dc_stream after objects are stream object is flattened */
>
>  return >base;
>  }
>
> -#ifdef CONFIG_DEBUG_FS
> -static int amdgpu_dm_crtc_atomic_set_property(struct drm_crtc *crtc,
> -struct drm_crtc_state *crtc_state,
> -struct drm_property *property,
> -uint64_t val)
> -{
> -struct drm_device *dev = crtc->dev;
> -struct amdgpu_device *adev = drm_to_adev(dev);
> -struct dm_crtc_state *dm_new_state =
> -to_dm_crtc_state(crtc_state);
> -
> -if (property == adev->dm.crc_win_x_start_property)
> -dm_new_state->crc_window.x_start = val;
> -else if (property == adev->dm.crc_win_y_start_property)
> -dm_new_state->crc_window.y_start = val;
> -else if (property == adev->dm.crc_win_x_end_property)
> -dm_new_state->crc_window.x_end = val;
> -else if (property == adev->dm.crc_win_y_end_property)
> -dm_new_state->crc_window.y_end = val;
> -else
> -return -EINVAL;
> -
> -return 0;
> -}
> -
> -static int amdgpu_dm_crtc_atomic_get_property(struct drm_crtc *crtc,
> -const struct drm_crtc_state *state,
> -struct drm_property *property,
> -uint64_t *val)
> -{
> -struct drm_device *dev = crtc->dev;
> -struct amdgpu_device *adev = drm_to_adev(dev);
> -struct dm_crtc_state *dm_state =
> -to_dm_crtc_state(state);
> -
> -if (property == adev->dm.crc_win_x_start_property)
> -*val = dm_state->crc_window.x_start;
> -else if (property == adev->dm.crc_win_y_start_property)
> -*val = dm_state->crc_window.y_start;
> -else if (property == adev->dm.crc_win_x_end_property)
> -*val = dm_state->crc_window.x_end;
> -else if (property == adev->dm.crc_win_y_end_property)
> -*val = dm_state->crc_window.y_end;
> -else
> -return -EINVAL;
> -
> -return 0;
> -}
> -#endif
> -
>  static inline int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable)  {
>  enum dc_irq_source irq_source;
> @@ -5599,10 +5508,6 @@ static const struct drm_crtc_funcs 
> amdgpu_dm_crtc_funcs 

RE: [PATCH] amdgpu/sriov Stop data exchange for wholegpu reset

2021-01-07 Thread Zhang, Jack (Jian)
Ping

-Original Message-
From: Jack Zhang  
Sent: Thursday, January 7, 2021 6:47 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Jack (Jian) ; Zhang, Jack (Jian) 
; Chen, JingWen 
Subject: [PATCH] amdgpu/sriov Stop data exchange for wholegpu reset

[Why]
When host trigger a whole gpu reset, guest will keep waiting till host finish 
reset. But there's a work queue in guest exchanging data between vf which 
need to access frame buffer. During whole gpu reset, frame buffer is not 
accessable, and this causes the call trace.

[How]
After vf get reset notification from pf, stop data exchange.

Signed-off-by: Jingwen Chen 
Signed-off-by: Jack Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 1 +
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c| 1 +
 drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c| 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 83ca5cbffe2c..3e212862cf5d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -571,6 +571,7 @@ void amdgpu_virt_fini_data_exchange(struct amdgpu_device 
*adev)
DRM_INFO("clean up the vf2pf work item\n");
flush_delayed_work(>virt.vf2pf_work);
cancel_delayed_work_sync(>virt.vf2pf_work);
+   adev->virt.vf2pf_update_interval_ms = 0;
}
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c 
b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
index 7767ccca526b..3ee481557fc9 100644
--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
@@ -255,6 +255,7 @@ static void xgpu_ai_mailbox_flr_work(struct work_struct 
*work)
if (!down_read_trylock(>reset_sem))
return;
 
+   amdgpu_virt_fini_data_exchange(adev);
atomic_set(>in_gpu_reset, 1);
 
do {
diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c 
b/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
index dd5c1e6ce009..48e588d3c409 100644
--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
@@ -276,6 +276,7 @@ static void xgpu_nv_mailbox_flr_work(struct work_struct 
*work)
if (!down_read_trylock(>reset_sem))
return;
 
+   amdgpu_virt_fini_data_exchange(adev);
atomic_set(>in_gpu_reset, 1);
 
do {
--
2.25.1

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


RE: [PATCH v2] drm/amdgpu:Limit the resolution for virtual_display

2021-01-07 Thread Deng, Emily
[AMD Official Use Only - Internal Distribution Only]

Ping ..

Best wishes
Emily Deng



>-Original Message-
>From: Emily Deng 
>Sent: Thursday, January 7, 2021 11:29 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily 
>Subject: [PATCH v2] drm/amdgpu:Limit the resolution for virtual_display
>
>From: "Emily.Deng" 
>
>Limit the resolution not bigger than 16384, which means
>dev->mode_info.num_crtc * common_modes[i].w not bigger than 16384.
>
>v2:
>  Refine the code
>
>Signed-off-by: Emily.Deng 
>---
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>index 2b16c8faca34..fd2b3a6dfd60 100644
>--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>@@ -319,6 +319,7 @@ dce_virtual_encoder(struct drm_connector
>*connector)  static int dce_virtual_get_modes(struct drm_connector
>*connector)  {
> struct drm_device *dev = connector->dev;
>+struct amdgpu_device *adev = dev->dev_private;
> struct drm_display_mode *mode = NULL;
> unsigned i;
> static const struct mode_size {
>@@ -350,8 +351,10 @@ static int dce_virtual_get_modes(struct
>drm_connector *connector)
> };
>
> for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
>-mode = drm_cvt_mode(dev, common_modes[i].w,
>common_modes[i].h, 60, false, false, false);
>-drm_mode_probed_add(connector, mode);
>+if (adev->mode_info.num_crtc * common_modes[i].w <=
>16384) {
>+mode = drm_cvt_mode(dev, common_modes[i].w,
>common_modes[i].h, 60, false, false, false);
>+drm_mode_probed_add(connector, mode);
>+}
> }
>
> return 0;
>--
>2.25.1

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


RE: [PATCH] drm/amdgpu: Decrease compute timeout to 10 s for sriov multiple VF

2021-01-07 Thread Deng, Emily
[AMD Official Use Only - Internal Distribution Only]

Ping ..

Best wishes
Emily Deng



>-Original Message-
>From: amd-gfx  On Behalf Of Emily
>Deng
>Sent: Thursday, January 7, 2021 10:50 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily 
>Subject: [PATCH] drm/amdgpu: Decrease compute timeout to 10 s for sriov
>multiple VF
>
>From: "Emily.Deng" 
>
>For multiple VF, after engine hang,as host driver will first encounter FLR, so
>has no meanning to set compute to 60s.
>
>Signed-off-by: Emily.Deng 
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>index 5527c549db82..ce07b9b975ff 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>@@ -3133,7 +3133,10 @@ static int
>amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
>  */
> adev->gfx_timeout = msecs_to_jiffies(1);
> adev->sdma_timeout = adev->video_timeout = adev->gfx_timeout;
>-if (amdgpu_sriov_vf(adev) || amdgpu_passthrough(adev))
>+if (amdgpu_sriov_vf(adev))
>+adev->compute_timeout =
>amdgpu_sriov_is_pp_one_vf(adev) ?
>+msecs_to_jiffies(6) :
>msecs_to_jiffies(1)
>+else if (amdgpu_passthrough(adev))
> adev->compute_timeout =  msecs_to_jiffies(6);
> else
> adev->compute_timeout = MAX_SCHEDULE_TIMEOUT;
>--
>2.25.1
>
>___
>amd-gfx mailing list
>amd-gfx@lists.freedesktop.org
>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.f
>reedesktop.org%2Fmailman%2Flistinfo%2Famd-
>gfxdata=04%7C01%7CEmily.Deng%40amd.com%7C29287057e1d84e2
>e912708d8b2b6f196%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7
>C637455846125410803%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
>MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sda
>ta=YsS0ylgUl2p2vXWbYftPBoFn59xdKKOpBdTVoxbOE2Y%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: fix DRM_INFO flood if display core is not supported (bug 210921)

2021-01-07 Thread Alexandre Demers
This fix bug 210921 where DRM_INFO floods log when hitting an unsupported ASIC 
in
amdgpu_device_asic_has_dc_support(). This info should be only called once.

Signed-off-by: Alexandre Demers 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1cb7d73f7317..9246c2ae7b63 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3034,7 +3034,7 @@ bool amdgpu_device_asic_has_dc_support(enum amd_asic_type 
asic_type)
 #endif
default:
if (amdgpu_dc > 0)
-   DRM_INFO("Display Core has been requested via kernel 
parameter "
+   DRM_INFO_ONCE("Display Core has been requested via 
kernel parameter "
 "but isn't supported by ASIC, 
ignoring\n");
return false;
}
-- 
2.30.0

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


Re: [PATCH] drm: Check actual format for legacy pageflip.

2021-01-07 Thread Daniel Vetter
On Thu, Jan 7, 2021 at 7:16 PM Mario Kleiner  wrote:
>
> On Thu, Jan 7, 2021 at 7:04 PM Daniel Vetter  wrote:
>>
>> On Thu, Jan 7, 2021 at 7:00 PM Mario Kleiner  
>> wrote:
>> >
>> > On Thu, Jan 7, 2021 at 6:57 PM Daniel Vetter  wrote:
>> >>
>> >> On Sat, Jan 02, 2021 at 04:31:36PM +0100, Mario Kleiner wrote:
>> >> > On Sat, Jan 2, 2021 at 3:02 PM Bas Nieuwenhuizen
>> >> >  wrote:
>> >> > >
>> >> > > With modifiers one can actually have different format_info structs
>> >> > > for the same format, which now matters for AMDGPU since we convert
>> >> > > implicit modifiers to explicit modifiers with multiple planes.
>> >> > >
>> >> > > I checked other drivers and it doesn't look like they end up 
>> >> > > triggering
>> >> > > this case so I think this is safe to relax.
>> >> > >
>> >> > > Signed-off-by: Bas Nieuwenhuizen 
>> >> > > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for 
>> >> > > converted metadata.")
>> >> > > ---
>> >> > >  drivers/gpu/drm/drm_plane.c | 2 +-
>> >> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> > >
>> >> > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>> >> > > index e6231947f987..f5085990cfac 100644
>> >> > > --- a/drivers/gpu/drm/drm_plane.c
>> >> > > +++ b/drivers/gpu/drm/drm_plane.c
>> >> > > @@ -1163,7 +1163,7 @@ int drm_mode_page_flip_ioctl(struct drm_device 
>> >> > > *dev,
>> >> > > if (ret)
>> >> > > goto out;
>> >> > >
>> >> > > -   if (old_fb->format != fb->format) {
>> >> > > +   if (old_fb->format->format != fb->format->format) {
>> >> >
>> >> > This was btw. the original way before Ville made it more strict about
>> >> > 4 years ago, to catch issues related to tiling, and more complex
>> >> > layouts, like the dcc tiling/retiling introduced by your modifier
>> >> > patches. That's why I hope my alternative patch is a good solution for
>> >> > atomic drivers while keeping the strictness for potential legacy
>> >> > drivers.
>> >>
>> >> Yeah this doesn't work in full generality, because hw might need to do a
>> >> full modeset to do a full modeset to reallocate resources (like scanout
>> >> fifo space) if the format changes.
>> >>
>> >> But for atomic drivers that should be caught in ->atomic_check, which
>> >> should result in -EINVAL, so should do the right thing. So it should be
>> >> all good, but imo needs a comment to explain what's going on:
>> >>
>> >> /*
>> >>  * Only check the FOURCC format code, excluding modifiers. This is
>> >>  * enough for all legacy drivers. Atomic drivers have their own
>> >>  * checks in their ->atomic_check implementation, which will
>> >>  * return -EINVAL if any hw or driver constraint is violated due
>> >>  * to modifier changes.
>> >>  */
>> >>
>> >> Also can you pls cc: intel-gfx to get this vetted by the intel-gfx ci?
>> >>
>> >> With that:
>> >>
>> >> Reviewed-by: Daniel Vetter 
>> >>
>> >
>> > Ah, my "atomic expert", posting simultaneously with myself :). Happy new 
>> > year. Opinions on my variant, just replied a minute ago?
>>
>> Full disclosure, Ville wanted to do something similar since forever
>> I'm not a huge fan of removing limitations of legacy ioctls. Worst
>> case we break something, best case no gain in features since why don't
>> you just use atomic. Since this (amdgpu modifiers) broke something we
>> have to fix it, hence I'd go with the more minimal version from Bas
>> here.
>>
>
> Fair point. Means though that somebody will have to convert many user-space 
> clients, e.g., all OSS Vulkan drivers. And XOrg could not do that, as the 
> kernel uabi even blocks use of atomic drmSetClientCap(...ATOMIC...) for any 
> process whose taskname starts with 'X', as a workaround for a modesetting-ddx 
> with broken atomic implementation. So at least for (pun ahead) "X" 
> applications, atomic modesetting is not an option.

If you ask for atomic v2 in the setclientcap ioctl you'll get atomic
even if you're X. The issue is no one's caring enough to fix it up
Xorg atomic support to make that happen.

And yes the vulkan drivers should attempt at least some atomic, the
reason for that was that amdgpu didn't have atomic back when the
original code was typed. Maybe it'll happen with more modifier use,
now that both amdgpu and i915 support them.
-Daniel

> For my use cases, X11/XOrg native is still the only display server capable 
> enough to fulfill the needs, although I'm mixing in a bit of 
> Vulkan/WSI/DirectDisplay for direct DRM/KMS access to work around some 
> limitations, e.g., to get HDR or fp16 support.
>
>> But in general your patch should be correct too.
>> -Daniel
>>
>
> Thanks for the feedback. I rest my case.
> -mario
>
>>
>> >
>> > thanks,
>> > -mario
>> >
>> >> >
>> >> > -mario
>> >> >
>> >> > > DRM_DEBUG_KMS("Page flip is not allowed to change 
>> >> > > frame buffer format.\n");
>> >> > > ret = -EINVAL;
>> >> > > goto out;

Re: [PATCH 0/3] Revert "drm/amd/display: Expose new CRC window property" and changes associated with this commit

2021-01-07 Thread Alex Deucher
On Thu, Jan 7, 2021 at 3:53 PM Rodrigo Siqueira
 wrote:
>
> Hi,
>
> A couple of weeks ago, Daniel highlighted  [1] some issue related to a
> patch entitle "drm/amd/display: Expose new CRC window property". After
> discussion, we realize that we can revert that patch because we will
> need to create a debugfs or full UAPI for CRC soon, which will make this
> code obsolete. We got two other patches related to this same code; for
> this reason, this patchset reverts all changes associated with that
> specific commit.
>
> Cc: Wayne Lin 
> Cc: Alexander Deucher 
> Cc: Harry Wentland 
> Cc: Roman Li 
> Cc: Bindu R 
> Cc: Daniel Vetter 

Series is:
Acked-by: Alex Deucher 

>
> 1. https://www.spinics.net/lists/dri-devel/msg283767.html
>
> Thanks
>
> Rodrigo Siqueira (3):
>   Revert "drm/amd/display: Fix unused variable warning"
>   Revert "drm/amdgpu/disply: fix documentation warnings in display
> manager"
>   Revert "drm/amd/display: Expose new CRC window property"
>
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 142 ++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  38 -
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c |  56 +--
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h |   5 +-
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c |   2 +
>  5 files changed, 14 insertions(+), 229 deletions(-)
>
> --
> 2.25.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] Revert "drm/amd/display: Fixed Intermittent blue screen on OLED panel"

2021-01-07 Thread Alex Deucher
On Thu, Jan 7, 2021 at 4:23 PM Rodrigo Siqueira
 wrote:
>
> The patch
>
> commit a861736dae64 ("drm/amd/display: Fixed Intermittent blue screen on OLED 
> panel")
>
> causes power regression for many users. It seems that this change causes
> the MCLK to get forced high; this creates a regression for many users
> since their devices were not able to drop to a low state after this
> change. For this reason, this reverts commit
> a861736dae644a0d7abbca0c638ae6aad28feeb8.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1407
> Cc: Aurabindo Pillai 
> Cc: Alex Deucher 
> Cc: Harry Wentland 
> Cc: Naveed Ashfaq 
> Cc: Hersen Wu 
> Cc: Roman Li 
> Signed-off-by: Rodrigo Siqueira 

Acked-by: Alex Deucher 

> ---
>  .../amd/display/dc/dml/dcn20/display_mode_vba_20v2.c  | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
> index 860e72a51534..80170f9721ce 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
> @@ -2635,14 +2635,15 @@ static void 
> dml20v2_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndP
> }
>
> if (mode_lib->vba.DRAMClockChangeSupportsVActive &&
> -   mode_lib->vba.MinActiveDRAMClockChangeMargin > 60 &&
> -   
> mode_lib->vba.PrefetchMode[mode_lib->vba.VoltageLevel][mode_lib->vba.maxMpcComb]
>  == 0) {
> +   mode_lib->vba.MinActiveDRAMClockChangeMargin > 60) {
> mode_lib->vba.DRAMClockChangeWatermark += 25;
>
> for (k = 0; k < mode_lib->vba.NumberOfActivePlanes; ++k) {
> -   if (mode_lib->vba.DRAMClockChangeWatermark >
> -   dml_max(mode_lib->vba.StutterEnterPlusExitWatermark, 
> mode_lib->vba.UrgentWatermark))
> -   mode_lib->vba.MinTTUVBlank[k] += 25;
> +   if 
> (mode_lib->vba.PrefetchMode[mode_lib->vba.VoltageLevel][mode_lib->vba.maxMpcComb]
>  == 0) {
> +   if (mode_lib->vba.DRAMClockChangeWatermark >
> +   
> dml_max(mode_lib->vba.StutterEnterPlusExitWatermark, 
> mode_lib->vba.UrgentWatermark))
> +   mode_lib->vba.MinTTUVBlank[k] += 25;
> +   }
> }
>
> mode_lib->vba.DRAMClockChangeSupport[0][0] = 
> dm_dram_clock_change_vactive;
> --
> 2.25.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] Revert "drm/amd/display: Fixed Intermittent blue screen on OLED panel"

2021-01-07 Thread Rodrigo Siqueira
The patch

commit a861736dae64 ("drm/amd/display: Fixed Intermittent blue screen on OLED 
panel")

causes power regression for many users. It seems that this change causes
the MCLK to get forced high; this creates a regression for many users
since their devices were not able to drop to a low state after this
change. For this reason, this reverts commit
a861736dae644a0d7abbca0c638ae6aad28feeb8.

Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1407
Cc: Aurabindo Pillai 
Cc: Alex Deucher 
Cc: Harry Wentland 
Cc: Naveed Ashfaq 
Cc: Hersen Wu 
Cc: Roman Li 
Signed-off-by: Rodrigo Siqueira 
---
 .../amd/display/dc/dml/dcn20/display_mode_vba_20v2.c  | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
index 860e72a51534..80170f9721ce 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
@@ -2635,14 +2635,15 @@ static void 
dml20v2_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndP
}
 
if (mode_lib->vba.DRAMClockChangeSupportsVActive &&
-   mode_lib->vba.MinActiveDRAMClockChangeMargin > 60 &&
-   
mode_lib->vba.PrefetchMode[mode_lib->vba.VoltageLevel][mode_lib->vba.maxMpcComb]
 == 0) {
+   mode_lib->vba.MinActiveDRAMClockChangeMargin > 60) {
mode_lib->vba.DRAMClockChangeWatermark += 25;
 
for (k = 0; k < mode_lib->vba.NumberOfActivePlanes; ++k) {
-   if (mode_lib->vba.DRAMClockChangeWatermark >
-   dml_max(mode_lib->vba.StutterEnterPlusExitWatermark, 
mode_lib->vba.UrgentWatermark))
-   mode_lib->vba.MinTTUVBlank[k] += 25;
+   if 
(mode_lib->vba.PrefetchMode[mode_lib->vba.VoltageLevel][mode_lib->vba.maxMpcComb]
 == 0) {
+   if (mode_lib->vba.DRAMClockChangeWatermark >
+   
dml_max(mode_lib->vba.StutterEnterPlusExitWatermark, 
mode_lib->vba.UrgentWatermark))
+   mode_lib->vba.MinTTUVBlank[k] += 25;
+   }
}
 
mode_lib->vba.DRAMClockChangeSupport[0][0] = 
dm_dram_clock_change_vactive;
-- 
2.25.1

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


[PATCH 3/3] Revert "drm/amd/display: Expose new CRC window property"

2021-01-07 Thread Rodrigo Siqueira
This reverts commit 110d586ba77ed573eb7464ca69b6490ec0b70c5f.

Cc: Wayne Lin 
Cc: Alexander Deucher 
Cc: Harry Wentland 
Cc: Roman Li 
Cc: Bindu R 
Cc: Daniel Vetter 
Signed-off-by: Rodrigo Siqueira 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 142 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  19 ---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c |  56 +--
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h |   3 -
 drivers/gpu/drm/amd/display/dc/core/dc_link.c |   2 +
 5 files changed, 12 insertions(+), 210 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index a06554745238..0515ed0d125c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -938,41 +938,6 @@ static void mmhub_read_system_context(struct amdgpu_device 
*adev, struct dc_phy_
 }
 #endif
 
-#ifdef CONFIG_DEBUG_FS
-static int create_crtc_crc_properties(struct amdgpu_display_manager *dm)
-{
-   dm->crc_win_x_start_property =
-   drm_property_create_range(adev_to_drm(dm->adev),
- DRM_MODE_PROP_ATOMIC,
- "AMD_CRC_WIN_X_START", 0, U16_MAX);
-   if (!dm->crc_win_x_start_property)
-   return -ENOMEM;
-
-   dm->crc_win_y_start_property =
-   drm_property_create_range(adev_to_drm(dm->adev),
- DRM_MODE_PROP_ATOMIC,
- "AMD_CRC_WIN_Y_START", 0, U16_MAX);
-   if (!dm->crc_win_y_start_property)
-   return -ENOMEM;
-
-   dm->crc_win_x_end_property =
-   drm_property_create_range(adev_to_drm(dm->adev),
- DRM_MODE_PROP_ATOMIC,
- "AMD_CRC_WIN_X_END", 0, U16_MAX);
-   if (!dm->crc_win_x_end_property)
-   return -ENOMEM;
-
-   dm->crc_win_y_end_property =
-   drm_property_create_range(adev_to_drm(dm->adev),
- DRM_MODE_PROP_ATOMIC,
- "AMD_CRC_WIN_Y_END", 0, U16_MAX);
-   if (!dm->crc_win_y_end_property)
-   return -ENOMEM;
-
-   return 0;
-}
-#endif
-
 static int amdgpu_dm_init(struct amdgpu_device *adev)
 {
struct dc_init_data init_data;
@@ -1119,10 +1084,6 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 
dc_init_callbacks(adev->dm.dc, _params);
}
-#endif
-#ifdef CONFIG_DEBUG_FS
-   if (create_crtc_crc_properties(>dm))
-   DRM_ERROR("amdgpu: failed to create crc property.\n");
 #endif
if (amdgpu_dm_initialize_drm_device(adev)) {
DRM_ERROR(
@@ -5456,64 +5417,12 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
state->crc_src = cur->crc_src;
state->cm_has_degamma = cur->cm_has_degamma;
state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
-#ifdef CONFIG_DEBUG_FS
-   state->crc_window = cur->crc_window;
-#endif
+
/* TODO Duplicate dc_stream after objects are stream object is 
flattened */
 
return >base;
 }
 
-#ifdef CONFIG_DEBUG_FS
-static int amdgpu_dm_crtc_atomic_set_property(struct drm_crtc *crtc,
-   struct drm_crtc_state *crtc_state,
-   struct drm_property *property,
-   uint64_t val)
-{
-   struct drm_device *dev = crtc->dev;
-   struct amdgpu_device *adev = drm_to_adev(dev);
-   struct dm_crtc_state *dm_new_state =
-   to_dm_crtc_state(crtc_state);
-
-   if (property == adev->dm.crc_win_x_start_property)
-   dm_new_state->crc_window.x_start = val;
-   else if (property == adev->dm.crc_win_y_start_property)
-   dm_new_state->crc_window.y_start = val;
-   else if (property == adev->dm.crc_win_x_end_property)
-   dm_new_state->crc_window.x_end = val;
-   else if (property == adev->dm.crc_win_y_end_property)
-   dm_new_state->crc_window.y_end = val;
-   else
-   return -EINVAL;
-
-   return 0;
-}
-
-static int amdgpu_dm_crtc_atomic_get_property(struct drm_crtc *crtc,
-   const struct drm_crtc_state *state,
-   struct drm_property *property,
-   uint64_t *val)
-{
-   struct drm_device *dev = crtc->dev;
-   struct amdgpu_device *adev = drm_to_adev(dev);
-   struct dm_crtc_state *dm_state =
-   to_dm_crtc_state(state);
-
-   if (property == adev->dm.crc_win_x_start_property)
-   *val = dm_state->crc_window.x_start;
-   else if (property == adev->dm.crc_win_y_start_property)
-   *val = dm_state->crc_window.y_start;
-   

[PATCH 1/3] Revert "drm/amd/display: Fix unused variable warning"

2021-01-07 Thread Rodrigo Siqueira
This reverts commit b5d8f1d02ba7021cad1bd5ad8460ce5611c479d8.

Cc: Wayne Lin 
Cc: Alexander Deucher 
Cc: Harry Wentland 
Cc: Roman Li 
Cc: Bindu R 
Cc: Daniel Vetter 
Signed-off-by: Rodrigo Siqueira 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 99c7f9eb44aa..a06554745238 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8549,7 +8549,8 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
acrtc->dm_irq_params.stream = dm_new_crtc_state->stream;
manage_dm_interrupts(adev, acrtc, true);
}
-   if (IS_ENABLED(CONFIG_DEBUG_FS) && new_crtc_state->active &&
+#ifdef CONFIG_DEBUG_FS
+   if (new_crtc_state->active &&

amdgpu_dm_is_valid_crc_source(dm_new_crtc_state->crc_src)) {
/**
 * Frontend may have changed so reapply the CRC capture
@@ -8570,6 +8571,7 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
amdgpu_dm_crtc_configure_crc_source(
crtc, dm_new_crtc_state, 
dm_new_crtc_state->crc_src);
}
+#endif
}
 
for_each_new_crtc_in_state(state, crtc, new_crtc_state, j)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h
index eba2f1d35d07..0235bfb246e5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h
@@ -46,13 +46,13 @@ static inline bool amdgpu_dm_is_valid_crc_source(enum 
amdgpu_dm_pipe_crc_source
 }
 
 /* amdgpu_dm_crc.c */
+#ifdef CONFIG_DEBUG_FS
 bool amdgpu_dm_crc_window_is_default(struct dm_crtc_state *dm_crtc_state);
 bool amdgpu_dm_crc_window_changed(struct dm_crtc_state *dm_new_crtc_state,
struct dm_crtc_state 
*dm_old_crtc_state);
 int amdgpu_dm_crtc_configure_crc_source(struct drm_crtc *crtc,
struct dm_crtc_state *dm_crtc_state,
enum amdgpu_dm_pipe_crc_source source);
-#ifdef CONFIG_DEBUG_FS
 int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name);
 int amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc,
 const char *src_name,
-- 
2.25.1

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


[PATCH 2/3] Revert "drm/amdgpu/disply: fix documentation warnings in display manager"

2021-01-07 Thread Rodrigo Siqueira
This reverts commit 1206904465c8a9eebff9ca5a65effc8cf8f3cb84.

Cc: Wayne Lin 
Cc: Alexander Deucher 
Cc: Harry Wentland 
Cc: Roman Li 
Cc: Bindu R 
Cc: Daniel Vetter 
Signed-off-by: Rodrigo Siqueira 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 21 +--
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 27b32ce7b6c9..ef394e941d9d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -344,29 +344,10 @@ struct amdgpu_display_manager {
uint32_t active_vblank_irq_count;
 
 #ifdef CONFIG_DEBUG_FS
-   /**
-* @crc_win_x_start_property:
-*
-* X start of the crc calculation window
-*/
+   /* set the crc calculation window*/
struct drm_property *crc_win_x_start_property;
-   /**
-* @crc_win_y_start_property:
-*
-* Y start of the crc calculation window
-*/
struct drm_property *crc_win_y_start_property;
-   /**
-* @crc_win_x_end_property:
-*
-* X end of the crc calculation window
-*/
struct drm_property *crc_win_x_end_property;
-   /**
-* @crc_win_y_end_property:
-*
-* Y end of the crc calculation window
-*/
struct drm_property *crc_win_y_end_property;
 #endif
/**
-- 
2.25.1

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


[PATCH 0/3] Revert "drm/amd/display: Expose new CRC window property" and changes associated with this commit

2021-01-07 Thread Rodrigo Siqueira
Hi,

A couple of weeks ago, Daniel highlighted  [1] some issue related to a
patch entitle "drm/amd/display: Expose new CRC window property". After
discussion, we realize that we can revert that patch because we will
need to create a debugfs or full UAPI for CRC soon, which will make this
code obsolete. We got two other patches related to this same code; for
this reason, this patchset reverts all changes associated with that
specific commit.

Cc: Wayne Lin 
Cc: Alexander Deucher 
Cc: Harry Wentland 
Cc: Roman Li 
Cc: Bindu R 
Cc: Daniel Vetter 

1. https://www.spinics.net/lists/dri-devel/msg283767.html 

Thanks

Rodrigo Siqueira (3):
  Revert "drm/amd/display: Fix unused variable warning"
  Revert "drm/amdgpu/disply: fix documentation warnings in display
manager"
  Revert "drm/amd/display: Expose new CRC window property"

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 142 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  38 -
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c |  56 +--
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h |   5 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link.c |   2 +
 5 files changed, 14 insertions(+), 229 deletions(-)

-- 
2.25.1

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


Re: [PATCH 1/2] drm/radeon: stop re-init the TTM page pool

2021-01-07 Thread Christian König

Am 07.01.21 um 19:07 schrieb Daniel Vetter:

On Tue, Jan 05, 2021 at 07:23:08PM +0100, Christian König wrote:

Drivers are not supposed to init the page pool directly any more.

Signed-off-by: Christian König 

Please include reported-by credits and link to the bug reports on
lore.kernel.org when merging this. Also I guess this should have a Fixes:
line?


I'm not aware of a bug report, but the reported-by/Fixes lines are 
indeed missing.


BTW: Any idea why dim add-link doesn't work?


And maybe some words on how/why stuff blows up.


Just a typo. I've forgot to remove two lines in radeon while rebasing 
and still had the symbols exported so never noticed this.


Christian.


-Daniel


---
  drivers/gpu/drm/radeon/radeon_ttm.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index d4328ff57757..35b715f82ed8 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -729,9 +729,6 @@ int radeon_ttm_init(struct radeon_device *rdev)
}
rdev->mman.initialized = true;
  
-	ttm_pool_init(>mman.bdev.pool, rdev->dev, rdev->need_swiotlb,

- dma_addressing_limited(>pdev->dev));
-
r = radeon_ttm_init_vram(rdev);
if (r) {
DRM_ERROR("Failed initializing VRAM heap.\n");
--
2.25.1

___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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


Re: [PATCH 2/2] drm/amd/amdgpu: Use IP discovery data to determine VCN enablement instead of MMSCH

2021-01-07 Thread Leo Liu



On 2021-01-05 5:54 p.m., Bokun Zhang wrote:

In the past, we use MMSCH to determine whether a VCN is enabled or not.
This is not reliable since after a FLR, MMSCH may report junk data.

It is better to use IP discovery data.

Change-Id: I8b6c32c34017b20dcaebffdaa78bb07178e9d03c
Signed-off-by: Bokun Zhang 
---
  drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 73 +--
  1 file changed, 45 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index def583916294..02cac6e33219 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -27,6 +27,7 @@
  #include "amdgpu_pm.h"
  #include "soc15.h"
  #include "soc15d.h"
+#include "soc15_hw_ip.h"
  #include "vcn_v2_0.h"
  #include "mmsch_v3_0.h"
  
@@ -60,6 +61,17 @@ static int amdgpu_ucode_id_vcns[] = {

AMDGPU_UCODE_ID_VCN1
  };
  
+#define VCN_BLOCK_ENCODE_DISABLE_MASK 0x80

+#define VCN_BLOCK_DECODE_DISABLE_MASK 0x40
+#define VCN_BLOCK_QUEUE_DISABLE_MASK 0xC0
+
+enum vcn_ring_type {
+   VCN_ENCODE_RING,
+   VCN_DECODE_RING,
+   VCN_UNIFIED_RING,
+};
+
+static bool vcn_v3_0_is_disabled_vcn(struct amdgpu_device *adev, enum 
vcn_ring_type type, uint32_t vcn_instance);
  static int vcn_v3_0_start_sriov(struct amdgpu_device *adev);
  static void vcn_v3_0_set_dec_ring_funcs(struct amdgpu_device *adev);
  static void vcn_v3_0_set_enc_ring_funcs(struct amdgpu_device *adev);
@@ -311,18 +323,26 @@ static int vcn_v3_0_hw_init(void *handle)
continue;
  
  			ring = >vcn.inst[i].ring_dec;

-   if (ring->sched.ready) {
+   if (vcn_v3_0_is_disabled_vcn(adev, VCN_DECODE_RING, i)) 
{
Since this is for SRIOV path only, and this doesn't apply to bare-metal, 
so please rename the function to something like xxx_sriov instead.


Regards,

Leo



+   ring->sched.ready = false;
+   dev_info(adev->dev, "ring %s is disabled by 
hypervisor\n", ring->name);
+   } else {
ring->wptr = 0;
ring->wptr_old = 0;
vcn_v3_0_dec_ring_set_wptr(ring);
+   ring->sched.ready = true;
}
  
  			for (j = 0; j < adev->vcn.num_enc_rings; ++j) {

ring = >vcn.inst[i].ring_enc[j];
-   if (ring->sched.ready) {
+   if (vcn_v3_0_is_disabled_vcn(adev, 
VCN_ENCODE_RING, i)) {
+   ring->sched.ready = false;
+   dev_info(adev->dev, "ring %s is disabled by 
hypervisor\n", ring->name);
+   } else {
ring->wptr = 0;
ring->wptr_old = 0;
vcn_v3_0_enc_ring_set_wptr(ring);
+   ring->sched.ready = true;
}
}
}
@@ -1266,6 +1286,29 @@ static int vcn_v3_0_start(struct amdgpu_device *adev)
return 0;
  }
  
+static bool vcn_v3_0_is_disabled_vcn(struct amdgpu_device *adev, enum vcn_ring_type type, uint32_t vcn_instance)

+{
+   bool ret = false;
+
+   int major;
+   int minor;
+   int revision;
+
+   /* if cannot find IP data, then this VCN does not exist */
+   if (amdgpu_discovery_get_ip_version(adev, VCN_HWID, vcn_instance, , 
, ) != 0)
+   return true;
+
+   if ((type == VCN_ENCODE_RING) && (revision & 
VCN_BLOCK_ENCODE_DISABLE_MASK)) {
+   ret = true;
+   } else if ((type == VCN_DECODE_RING) && (revision & 
VCN_BLOCK_DECODE_DISABLE_MASK)) {
+   ret = true;
+   } else if ((type == VCN_UNIFIED_RING) && (revision & 
VCN_BLOCK_QUEUE_DISABLE_MASK)) {
+   ret = true;
+   }
+
+   return ret;
+}
+
  static int vcn_v3_0_start_sriov(struct amdgpu_device *adev)
  {
int i, j;
@@ -1283,8 +1326,6 @@ static int vcn_v3_0_start_sriov(struct amdgpu_device 
*adev)
uint32_t table_size;
uint32_t size, size_dw;
  
-	bool is_vcn_ready;

-
struct mmsch_v3_0_cmd_direct_write
direct_wt = { {0} };
struct mmsch_v3_0_cmd_direct_read_modify_write
@@ -1476,30 +1517,6 @@ static int vcn_v3_0_start_sriov(struct amdgpu_device 
*adev)
}
}
  
-	/* 6, check each VCN's init_status

-* if it remains as 0, then this VCN is not assigned to current VF
-* do not start ring for this VCN
-*/
-   size = sizeof(struct mmsch_v3_0_init_header);
-   table_loc = (uint32_t *)table->cpu_addr;
-   memcpy(, (void *)table_loc, size);
-
-   for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
-   if 

Re: [PATCH] drm: Check actual format for legacy pageflip.

2021-01-07 Thread Mario Kleiner
On Thu, Jan 7, 2021 at 7:04 PM Daniel Vetter  wrote:

> On Thu, Jan 7, 2021 at 7:00 PM Mario Kleiner 
> wrote:
> >
> > On Thu, Jan 7, 2021 at 6:57 PM Daniel Vetter  wrote:
> >>
> >> On Sat, Jan 02, 2021 at 04:31:36PM +0100, Mario Kleiner wrote:
> >> > On Sat, Jan 2, 2021 at 3:02 PM Bas Nieuwenhuizen
> >> >  wrote:
> >> > >
> >> > > With modifiers one can actually have different format_info structs
> >> > > for the same format, which now matters for AMDGPU since we convert
> >> > > implicit modifiers to explicit modifiers with multiple planes.
> >> > >
> >> > > I checked other drivers and it doesn't look like they end up
> triggering
> >> > > this case so I think this is safe to relax.
> >> > >
> >> > > Signed-off-by: Bas Nieuwenhuizen 
> >> > > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for
> converted metadata.")
> >> > > ---
> >> > >  drivers/gpu/drm/drm_plane.c | 2 +-
> >> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> > >
> >> > > diff --git a/drivers/gpu/drm/drm_plane.c
> b/drivers/gpu/drm/drm_plane.c
> >> > > index e6231947f987..f5085990cfac 100644
> >> > > --- a/drivers/gpu/drm/drm_plane.c
> >> > > +++ b/drivers/gpu/drm/drm_plane.c
> >> > > @@ -1163,7 +1163,7 @@ int drm_mode_page_flip_ioctl(struct
> drm_device *dev,
> >> > > if (ret)
> >> > > goto out;
> >> > >
> >> > > -   if (old_fb->format != fb->format) {
> >> > > +   if (old_fb->format->format != fb->format->format) {
> >> >
> >> > This was btw. the original way before Ville made it more strict about
> >> > 4 years ago, to catch issues related to tiling, and more complex
> >> > layouts, like the dcc tiling/retiling introduced by your modifier
> >> > patches. That's why I hope my alternative patch is a good solution for
> >> > atomic drivers while keeping the strictness for potential legacy
> >> > drivers.
> >>
> >> Yeah this doesn't work in full generality, because hw might need to do a
> >> full modeset to do a full modeset to reallocate resources (like scanout
> >> fifo space) if the format changes.
> >>
> >> But for atomic drivers that should be caught in ->atomic_check, which
> >> should result in -EINVAL, so should do the right thing. So it should be
> >> all good, but imo needs a comment to explain what's going on:
> >>
> >> /*
> >>  * Only check the FOURCC format code, excluding modifiers. This
> is
> >>  * enough for all legacy drivers. Atomic drivers have their own
> >>  * checks in their ->atomic_check implementation, which will
> >>  * return -EINVAL if any hw or driver constraint is violated due
> >>  * to modifier changes.
> >>  */
> >>
> >> Also can you pls cc: intel-gfx to get this vetted by the intel-gfx ci?
> >>
> >> With that:
> >>
> >> Reviewed-by: Daniel Vetter 
> >>
> >
> > Ah, my "atomic expert", posting simultaneously with myself :). Happy new
> year. Opinions on my variant, just replied a minute ago?
>
> Full disclosure, Ville wanted to do something similar since forever
> I'm not a huge fan of removing limitations of legacy ioctls. Worst
> case we break something, best case no gain in features since why don't
> you just use atomic. Since this (amdgpu modifiers) broke something we
> have to fix it, hence I'd go with the more minimal version from Bas
> here.
>
>
Fair point. Means though that somebody will have to convert many user-space
clients, e.g., all OSS Vulkan drivers. And XOrg could not do that, as the
kernel uabi even blocks use of atomic drmSetClientCap(...ATOMIC...) for any
process whose taskname starts with 'X', as a workaround for a
modesetting-ddx with broken atomic implementation. So at least for (pun
ahead) "X" applications, atomic modesetting is not an option.

For my use cases, X11/XOrg native is still the only display server capable
enough to fulfill the needs, although I'm mixing in a bit of
Vulkan/WSI/DirectDisplay for direct DRM/KMS access to work around some
limitations, e.g., to get HDR or fp16 support.

But in general your patch should be correct too.
> -Daniel
>
>
Thanks for the feedback. I rest my case.
-mario


> >
> > thanks,
> > -mario
> >
> >> >
> >> > -mario
> >> >
> >> > > DRM_DEBUG_KMS("Page flip is not allowed to change
> frame buffer format.\n");
> >> > > ret = -EINVAL;
> >> > > goto out;
> >> > > --
> >> > > 2.29.2
> >> > >
> >>
> >> --
> >> Daniel Vetter
> >> Software Engineer, Intel Corporation
> >> http://blog.ffwll.ch
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 1/2] drm/radeon: stop re-init the TTM page pool

2021-01-07 Thread Daniel Vetter
On Tue, Jan 05, 2021 at 07:23:08PM +0100, Christian König wrote:
> Drivers are not supposed to init the page pool directly any more.
> 
> Signed-off-by: Christian König 

Please include reported-by credits and link to the bug reports on
lore.kernel.org when merging this. Also I guess this should have a Fixes:
line?

And maybe some words on how/why stuff blows up.
-Daniel

> ---
>  drivers/gpu/drm/radeon/radeon_ttm.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
> b/drivers/gpu/drm/radeon/radeon_ttm.c
> index d4328ff57757..35b715f82ed8 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -729,9 +729,6 @@ int radeon_ttm_init(struct radeon_device *rdev)
>   }
>   rdev->mman.initialized = true;
>  
> - ttm_pool_init(>mman.bdev.pool, rdev->dev, rdev->need_swiotlb,
> -   dma_addressing_limited(>pdev->dev));
> -
>   r = radeon_ttm_init_vram(rdev);
>   if (r) {
>   DRM_ERROR("Failed initializing VRAM heap.\n");
> -- 
> 2.25.1
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm: Check actual format for legacy pageflip.

2021-01-07 Thread Daniel Vetter
On Thu, Jan 7, 2021 at 7:00 PM Mario Kleiner  wrote:
>
> On Thu, Jan 7, 2021 at 6:57 PM Daniel Vetter  wrote:
>>
>> On Sat, Jan 02, 2021 at 04:31:36PM +0100, Mario Kleiner wrote:
>> > On Sat, Jan 2, 2021 at 3:02 PM Bas Nieuwenhuizen
>> >  wrote:
>> > >
>> > > With modifiers one can actually have different format_info structs
>> > > for the same format, which now matters for AMDGPU since we convert
>> > > implicit modifiers to explicit modifiers with multiple planes.
>> > >
>> > > I checked other drivers and it doesn't look like they end up triggering
>> > > this case so I think this is safe to relax.
>> > >
>> > > Signed-off-by: Bas Nieuwenhuizen 
>> > > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for converted 
>> > > metadata.")
>> > > ---
>> > >  drivers/gpu/drm/drm_plane.c | 2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>> > > index e6231947f987..f5085990cfac 100644
>> > > --- a/drivers/gpu/drm/drm_plane.c
>> > > +++ b/drivers/gpu/drm/drm_plane.c
>> > > @@ -1163,7 +1163,7 @@ int drm_mode_page_flip_ioctl(struct drm_device 
>> > > *dev,
>> > > if (ret)
>> > > goto out;
>> > >
>> > > -   if (old_fb->format != fb->format) {
>> > > +   if (old_fb->format->format != fb->format->format) {
>> >
>> > This was btw. the original way before Ville made it more strict about
>> > 4 years ago, to catch issues related to tiling, and more complex
>> > layouts, like the dcc tiling/retiling introduced by your modifier
>> > patches. That's why I hope my alternative patch is a good solution for
>> > atomic drivers while keeping the strictness for potential legacy
>> > drivers.
>>
>> Yeah this doesn't work in full generality, because hw might need to do a
>> full modeset to do a full modeset to reallocate resources (like scanout
>> fifo space) if the format changes.
>>
>> But for atomic drivers that should be caught in ->atomic_check, which
>> should result in -EINVAL, so should do the right thing. So it should be
>> all good, but imo needs a comment to explain what's going on:
>>
>> /*
>>  * Only check the FOURCC format code, excluding modifiers. This is
>>  * enough for all legacy drivers. Atomic drivers have their own
>>  * checks in their ->atomic_check implementation, which will
>>  * return -EINVAL if any hw or driver constraint is violated due
>>  * to modifier changes.
>>  */
>>
>> Also can you pls cc: intel-gfx to get this vetted by the intel-gfx ci?
>>
>> With that:
>>
>> Reviewed-by: Daniel Vetter 
>>
>
> Ah, my "atomic expert", posting simultaneously with myself :). Happy new 
> year. Opinions on my variant, just replied a minute ago?

Full disclosure, Ville wanted to do something similar since forever
I'm not a huge fan of removing limitations of legacy ioctls. Worst
case we break something, best case no gain in features since why don't
you just use atomic. Since this (amdgpu modifiers) broke something we
have to fix it, hence I'd go with the more minimal version from Bas
here.

But in general your patch should be correct too.
-Daniel

>
> thanks,
> -mario
>
>> >
>> > -mario
>> >
>> > > DRM_DEBUG_KMS("Page flip is not allowed to change frame 
>> > > buffer format.\n");
>> > > ret = -EINVAL;
>> > > goto out;
>> > > --
>> > > 2.29.2
>> > >
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm: Check actual format for legacy pageflip.

2021-01-07 Thread Mario Kleiner
On Thu, Jan 7, 2021 at 6:57 PM Daniel Vetter  wrote:

> On Sat, Jan 02, 2021 at 04:31:36PM +0100, Mario Kleiner wrote:
> > On Sat, Jan 2, 2021 at 3:02 PM Bas Nieuwenhuizen
> >  wrote:
> > >
> > > With modifiers one can actually have different format_info structs
> > > for the same format, which now matters for AMDGPU since we convert
> > > implicit modifiers to explicit modifiers with multiple planes.
> > >
> > > I checked other drivers and it doesn't look like they end up triggering
> > > this case so I think this is safe to relax.
> > >
> > > Signed-off-by: Bas Nieuwenhuizen 
> > > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for
> converted metadata.")
> > > ---
> > >  drivers/gpu/drm/drm_plane.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > > index e6231947f987..f5085990cfac 100644
> > > --- a/drivers/gpu/drm/drm_plane.c
> > > +++ b/drivers/gpu/drm/drm_plane.c
> > > @@ -1163,7 +1163,7 @@ int drm_mode_page_flip_ioctl(struct drm_device
> *dev,
> > > if (ret)
> > > goto out;
> > >
> > > -   if (old_fb->format != fb->format) {
> > > +   if (old_fb->format->format != fb->format->format) {
> >
> > This was btw. the original way before Ville made it more strict about
> > 4 years ago, to catch issues related to tiling, and more complex
> > layouts, like the dcc tiling/retiling introduced by your modifier
> > patches. That's why I hope my alternative patch is a good solution for
> > atomic drivers while keeping the strictness for potential legacy
> > drivers.
>
> Yeah this doesn't work in full generality, because hw might need to do a
> full modeset to do a full modeset to reallocate resources (like scanout
> fifo space) if the format changes.
>
> But for atomic drivers that should be caught in ->atomic_check, which
> should result in -EINVAL, so should do the right thing. So it should be
> all good, but imo needs a comment to explain what's going on:
>
> /*
>  * Only check the FOURCC format code, excluding modifiers. This is
>  * enough for all legacy drivers. Atomic drivers have their own
>  * checks in their ->atomic_check implementation, which will
>  * return -EINVAL if any hw or driver constraint is violated due
>  * to modifier changes.
>  */
>
> Also can you pls cc: intel-gfx to get this vetted by the intel-gfx ci?
>
> With that:
>
> Reviewed-by: Daniel Vetter 
>
>
Ah, my "atomic expert", posting simultaneously with myself :). Happy new
year. Opinions on my variant, just replied a minute ago?

thanks,
-mario

>
> > -mario
> >
> > > DRM_DEBUG_KMS("Page flip is not allowed to change
> frame buffer format.\n");
> > > ret = -EINVAL;
> > > goto out;
> > > --
> > > 2.29.2
> > >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm: Check actual format for legacy pageflip.

2021-01-07 Thread Mario Kleiner
On Thu, Jan 7, 2021 at 6:21 PM Liu, Zhan  wrote:
>
> [AMD Official Use Only - Internal Distribution Only]
>
> > -Original Message-
> > From: Liu, Zhan
> > Sent: 2021/January/06, Wednesday 10:04 AM
> > To: Bas Nieuwenhuizen ; Mario Kleiner
> > 
> > Cc: dri-devel ; amd-gfx list  > g...@lists.freedesktop.org>; Deucher, Alexander
> > ; Daniel Vetter ;
> > Kazlauskas, Nicholas ; Ville Syrjälä
> > 
> > Subject: RE: [PATCH] drm: Check actual format for legacy pageflip.
> >
> >
> > > -Original Message-
> > > From: Liu, Zhan 
> > > Sent: 2021/January/04, Monday 3:46 PM
> > > To: Bas Nieuwenhuizen ; Mario Kleiner
> > > 
> > > Cc: dri-devel ; amd-gfx list  > > g...@lists.freedesktop.org>; Deucher, Alexander
> > > ; Daniel Vetter ;
> > > Kazlauskas, Nicholas ; Ville Syrjälä
> > > 
> > > Subject: Re: [PATCH] drm: Check actual format for legacy pageflip.
> > >
> > >
> > >
> > > + Ville
> > >
> > > On Sat, Jan 2, 2021 at 4:31 PM Mario Kleiner
> > > 
> > > wrote:
> > > >
> > > > On Sat, Jan 2, 2021 at 3:02 PM Bas Nieuwenhuizen
> > > >  wrote:
> > > > >
> > > > > With modifiers one can actually have different format_info structs
> > > > > for the same format, which now matters for AMDGPU since we convert
> > > > > implicit modifiers to explicit modifiers with multiple planes.
> > > > >
> > > > > I checked other drivers and it doesn't look like they end up
> > > > > triggering this case so I think this is safe to relax.
> > > > >
> > > > > Signed-off-by: Bas Nieuwenhuizen 
> > > > > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for
> > > > >converted metadata.")
> > > > > ---
> > > > >  drivers/gpu/drm/drm_plane.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_plane.c
> > > > > b/drivers/gpu/drm/drm_plane.c index e6231947f987..f5085990cfac
> > > > > 100644
> > > > > --- a/drivers/gpu/drm/drm_plane.c
> > > > > +++ b/drivers/gpu/drm/drm_plane.c
> > > > > @@ -1163,7 +1163,7 @@ int drm_mode_page_flip_ioctl(struct
> > > drm_device
> > > > >*dev,
> > > > > if (ret)
> > > > > goto out;
> > > > >
> > > > > -   if (old_fb->format != fb->format) {
> > > > > +   if (old_fb->format->format != fb->format->format) {
> > > >
> > >
> > > I agree with this patch, though considering the original way was made
> > > by Ville, I will wait for Ville's input first. Adding my "Acked-by"
here.
> > >
> > > This patch is:
> > > Acked-by: Zhan Liu 
>
> Since there is no objection from the community on this patch over the
past few days, and this patch totally makes sense to me, this patch is:
>
> Reviewed-by: Zhan Liu 
>

Well, there is my alternative one-line patch, which is equally simple and
solves the problem in a similar way that doesn't undo Ville's stricter
checks, but it doesn't seem to get any attention:

https://lists.freedesktop.org/archives/dri-devel/2021-January/292763.html

Mine keeps the check as strict as before for non-atomic drivers, but
removes the check for atomic drivers, given the assumption that they should
be able to do without it.

In the end both patches solve the problem in the short term, also
satisfying my (users) needs, and the future is unknown. But it would be
nice to get an opinion from an atomic expert which one is the more future
proof / elegant / final solution to stick to in the face of potential
future atomic kms drivers

With that said, i will add to Bas patch a

Reported-by: Mario Kleiner 
Acked-by: Mario Kleiner 

thanks,
-mario

> >
> > Ping...
> >
> > >
> > > > This was btw. the original way before Ville made it more strict
> > > > about
> > > > 4 years ago, to catch issues related to tiling, and more complex
> > > > layouts, like the dcc tiling/retiling introduced by your modifier
> > > > patches. That's why I hope my alternative patch is a good solution
> > > > for atomic drivers while keeping the strictness for potential legacy
> > > > drivers.
> > > >
> > > > -mario
> > > >
> > > > > DRM_DEBUG_KMS("Page flip is not allowed to change
> > > > >frame buffer format.\n");
> > > > > ret = -EINVAL;
> > > > > goto out;
> > > > > --
> > > > > 2.29.2
> > > > >
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm: Check actual format for legacy pageflip.

2021-01-07 Thread Daniel Vetter
On Sat, Jan 02, 2021 at 04:31:36PM +0100, Mario Kleiner wrote:
> On Sat, Jan 2, 2021 at 3:02 PM Bas Nieuwenhuizen
>  wrote:
> >
> > With modifiers one can actually have different format_info structs
> > for the same format, which now matters for AMDGPU since we convert
> > implicit modifiers to explicit modifiers with multiple planes.
> >
> > I checked other drivers and it doesn't look like they end up triggering
> > this case so I think this is safe to relax.
> >
> > Signed-off-by: Bas Nieuwenhuizen 
> > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for converted 
> > metadata.")
> > ---
> >  drivers/gpu/drm/drm_plane.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index e6231947f987..f5085990cfac 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -1163,7 +1163,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> > if (ret)
> > goto out;
> >
> > -   if (old_fb->format != fb->format) {
> > +   if (old_fb->format->format != fb->format->format) {
> 
> This was btw. the original way before Ville made it more strict about
> 4 years ago, to catch issues related to tiling, and more complex
> layouts, like the dcc tiling/retiling introduced by your modifier
> patches. That's why I hope my alternative patch is a good solution for
> atomic drivers while keeping the strictness for potential legacy
> drivers.

Yeah this doesn't work in full generality, because hw might need to do a
full modeset to do a full modeset to reallocate resources (like scanout
fifo space) if the format changes.

But for atomic drivers that should be caught in ->atomic_check, which
should result in -EINVAL, so should do the right thing. So it should be
all good, but imo needs a comment to explain what's going on:

/*
 * Only check the FOURCC format code, excluding modifiers. This is
 * enough for all legacy drivers. Atomic drivers have their own
 * checks in their ->atomic_check implementation, which will
 * return -EINVAL if any hw or driver constraint is violated due
 * to modifier changes.
 */

Also can you pls cc: intel-gfx to get this vetted by the intel-gfx ci?

With that:

Reviewed-by: Daniel Vetter 

> 
> -mario
> 
> > DRM_DEBUG_KMS("Page flip is not allowed to change frame 
> > buffer format.\n");
> > ret = -EINVAL;
> > goto out;
> > --
> > 2.29.2
> >

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH v2] drm/amdgpu: Do not change amdgpu framebuffer format during page flip

2021-01-07 Thread Liu, Zhan



> -Original Message-
> From: Daniel Vetter 
> Sent: 2021/January/07, Thursday 12:33 PM
> To: Koenig, Christian 
> Cc: Liu, Zhan ; amd-gfx@lists.freedesktop.org; Cornij,
> Nikola ; Wang, Chao-kai (Stylon)
> ; Wang, Chao-kai (Stylon)
> ; dri-de...@lists.freedesktop.org; Kazlauskas,
> Nicholas ; b...@basnieuwenhuizen.nl
> Subject: Re: [PATCH v2] drm/amdgpu: Do not change amdgpu framebuffer
> format during page flip
> 
> On Sun, Jan 03, 2021 at 04:43:37PM +0100, Christian König wrote:
> > Am 29.12.20 um 22:10 schrieb Zhan Liu:
> > > [Why]
> > > Driver cannot change amdgpu framebuffer (afb) format while doing
> > > page flip. Force system doing so will cause ioctl error, and result
> > > in breaking several functionalities including FreeSync.
> > >
> > > If afb format is forced to change during page flip, following
> > > message will appear in dmesg.log:
> > >
> > > "[drm:drm_mode_page_flip_ioctl [drm]] Page flip is not allowed to
> > > change frame buffer format."
> > >
> > > [How]
> > > Do not change afb format while doing page flip. It is okay to check
> > > whether afb format is valid here, however, forcing afb format change
> > > shouldn't happen here.
> >
> > I don't think this we can do this.
> >
> > It is perfectly valid for a page flip to switch between linear and
> > tiled formats on an I+A or A+A laptop.
> 
> It is, but that's not the bug here. struct drm_framebuffer.format is supposed
> to be invariant over the lifetime of a drm_fb object, you need to set it when
> the fb is created and never change it afterwards. So the patch here isn't yet
> the real deal.
> 
> Also this means the implicit tiling information cannot be changed after a fb 
> is
> created for a given bo, but we've discussed this at length and that limitation
> should be all ok.
> -Daniel

Thank you Christian and Daniel for the input!

Bas recently submitted an alternative patch ([PATCH] drm: Check actual format 
for legacy pageflip.) 
which addresses the same issue, and his patch makes more sense to me, so I will 
abandon my patch in this case.

Thanks,
Zhan


> 
> >
> > Christian.
> >
> > >
> > > Signed-off-by: Zhan Liu 
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > Thanks Nick and Bas. Here is my second patch for review.
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > > index a638709e9c92..8a12e27ff4ec 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > > @@ -832,7 +832,8 @@ static int convert_tiling_flags_to_modifier(struct
> amdgpu_framebuffer *afb)
> > >   if (!format_info)
> > >   return -EINVAL;
> > > - afb->base.format = format_info;
> > > + if (!afb->base.format)
> > > + afb->base.format = format_info;
> > >   }
> > >   }
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > s.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-
> develdata=04%7C01%7C
> >
> zhan.liu%40amd.com%7Cda23e6e33a7e46dfc4e308d8b33242c8%7C3dd896
> 1fe4884e
> >
> 608e11a82d994e183d%7C0%7C0%7C637456375746425509%7CUnknown%
> 7CTWFpbGZsb3
> >
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%7
> >
> C1000sdata=5lCm4d6FHihfFHUf5mVym0O6lKmZHgR89%2F2Eqj2ojhg
> %3Dr
> > eserved=0
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ff
> wll.ch%2Fdata=04%7C01%7Czhan.liu%40amd.com%7Cda23e6e33a7e
> 46dfc4e308d8b33242c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
> C0%7C637456375746425509%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
> mp;sdata=44x858klbIcVeRtP%2BuJST2K3xuCLisjbfhV9rEQrzpA%3Drese
> rved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v2] drm/amdgpu: Do not change amdgpu framebuffer format during page flip

2021-01-07 Thread Daniel Vetter
On Sun, Jan 03, 2021 at 04:43:37PM +0100, Christian König wrote:
> Am 29.12.20 um 22:10 schrieb Zhan Liu:
> > [Why]
> > Driver cannot change amdgpu framebuffer (afb) format while doing
> > page flip. Force system doing so will cause ioctl error, and result in
> > breaking several functionalities including FreeSync.
> > 
> > If afb format is forced to change during page flip, following message
> > will appear in dmesg.log:
> > 
> > "[drm:drm_mode_page_flip_ioctl [drm]]
> > Page flip is not allowed to change frame buffer format."
> > 
> > [How]
> > Do not change afb format while doing page flip. It is okay to check
> > whether afb format is valid here, however, forcing afb format change
> > shouldn't happen here.
> 
> I don't think this we can do this.
> 
> It is perfectly valid for a page flip to switch between linear and tiled
> formats on an I+A or A+A laptop.

It is, but that's not the bug here. struct drm_framebuffer.format is
supposed to be invariant over the lifetime of a drm_fb object, you need to
set it when the fb is created and never change it afterwards. So the patch
here isn't yet the real deal.

Also this means the implicit tiling information cannot be changed after a
fb is created for a given bo, but we've discussed this at length and that
limitation should be all ok.
-Daniel

> 
> Christian.
> 
> > 
> > Signed-off-by: Zhan Liu 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > Thanks Nick and Bas. Here is my second patch for review.
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > index a638709e9c92..8a12e27ff4ec 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > @@ -832,7 +832,8 @@ static int convert_tiling_flags_to_modifier(struct 
> > amdgpu_framebuffer *afb)
> > if (!format_info)
> > return -EINVAL;
> > -   afb->base.format = format_info;
> > +   if (!afb->base.format)
> > +   afb->base.format = format_info;
> > }
> > }
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm: Check actual format for legacy pageflip.

2021-01-07 Thread Liu, Zhan
[AMD Official Use Only - Internal Distribution Only]

> -Original Message-
> From: Liu, Zhan
> Sent: 2021/January/06, Wednesday 10:04 AM
> To: Bas Nieuwenhuizen ; Mario Kleiner
> 
> Cc: dri-devel ; amd-gfx list  g...@lists.freedesktop.org>; Deucher, Alexander
> ; Daniel Vetter ;
> Kazlauskas, Nicholas ; Ville Syrjälä
> 
> Subject: RE: [PATCH] drm: Check actual format for legacy pageflip.
>
>
> > -Original Message-
> > From: Liu, Zhan 
> > Sent: 2021/January/04, Monday 3:46 PM
> > To: Bas Nieuwenhuizen ; Mario Kleiner
> > 
> > Cc: dri-devel ; amd-gfx list  > g...@lists.freedesktop.org>; Deucher, Alexander
> > ; Daniel Vetter ;
> > Kazlauskas, Nicholas ; Ville Syrjälä
> > 
> > Subject: Re: [PATCH] drm: Check actual format for legacy pageflip.
> >
> >
> >
> > + Ville
> >
> > On Sat, Jan 2, 2021 at 4:31 PM Mario Kleiner
> > 
> > wrote:
> > >
> > > On Sat, Jan 2, 2021 at 3:02 PM Bas Nieuwenhuizen
> > >  wrote:
> > > >
> > > > With modifiers one can actually have different format_info structs
> > > > for the same format, which now matters for AMDGPU since we convert
> > > > implicit modifiers to explicit modifiers with multiple planes.
> > > >
> > > > I checked other drivers and it doesn't look like they end up
> > > > triggering this case so I think this is safe to relax.
> > > >
> > > > Signed-off-by: Bas Nieuwenhuizen 
> > > > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for
> > > >converted metadata.")
> > > > ---
> > > >  drivers/gpu/drm/drm_plane.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_plane.c
> > > > b/drivers/gpu/drm/drm_plane.c index e6231947f987..f5085990cfac
> > > > 100644
> > > > --- a/drivers/gpu/drm/drm_plane.c
> > > > +++ b/drivers/gpu/drm/drm_plane.c
> > > > @@ -1163,7 +1163,7 @@ int drm_mode_page_flip_ioctl(struct
> > drm_device
> > > >*dev,
> > > > if (ret)
> > > > goto out;
> > > >
> > > > -   if (old_fb->format != fb->format) {
> > > > +   if (old_fb->format->format != fb->format->format) {
> > >
> >
> > I agree with this patch, though considering the original way was made
> > by Ville, I will wait for Ville's input first. Adding my "Acked-by" here.
> >
> > This patch is:
> > Acked-by: Zhan Liu 

Since there is no objection from the community on this patch over the past few 
days, and this patch totally makes sense to me, this patch is:

Reviewed-by: Zhan Liu 

>
> Ping...
>
> >
> > > This was btw. the original way before Ville made it more strict
> > > about
> > > 4 years ago, to catch issues related to tiling, and more complex
> > > layouts, like the dcc tiling/retiling introduced by your modifier
> > > patches. That's why I hope my alternative patch is a good solution
> > > for atomic drivers while keeping the strictness for potential legacy
> > > drivers.
> > >
> > > -mario
> > >
> > > > DRM_DEBUG_KMS("Page flip is not allowed to change
> > > >frame buffer format.\n");
> > > > ret = -EINVAL;
> > > > goto out;
> > > > --
> > > > 2.29.2
> > > >
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 29/35] drm/amdgpu: svm bo enable_signal call condition

2021-01-07 Thread Felix Kuehling
Am 2021-01-07 um 11:28 a.m. schrieb Christian König:
> Am 07.01.21 um 17:16 schrieb Felix Kuehling:
>> Am 2021-01-07 um 5:56 a.m. schrieb Christian König:
>>
>>> Am 07.01.21 um 04:01 schrieb Felix Kuehling:
 From: Alex Sierra 

 [why]
 To support svm bo eviction mechanism.

 [how]
 If the BO crated has AMDGPU_AMDKFD_CREATE_SVM_BO flag set,
 enable_signal callback will be called inside amdgpu_evict_flags.
 This also causes gutting of the BO by removing all placements,
 so that TTM won't actually do an eviction. Instead it will discard
 the memory held by the BO. This is needed for HMM migration to user
 mode system memory pages.
>>> I don't think that this will work. What exactly are you doing here?
>> We discussed this a while ago when we talked about pipelined gutting.
>> And you actually helped us out with a fix for that
>> (https://patchwork.freedesktop.org/patch/379039/).
>
> That's not what I meant. The pipelined gutting is ok, but why the
> enable_signaling()?

That's what triggers our eviction fence callback
amdkfd_fence_enable_signaling that schedules the worker doing the
eviction. Without pipelined gutting we'd be getting that callback from
the GPU scheduler when it tries to execute the job that does the
migration. With pipelined gutting we have to call this somewhere ourselves.

I guess we could schedule the eviction worker directly without going
through the fence callback. I think we did it this way because it's more
similar to our KFD BO eviction handling where the worker gets scheduled
by the fence callback.

Regards,
  Felix


>
> Christian.
>
>>
>> SVM BOs are BOs in VRAM containing data for HMM ranges. When such a BO
>> is evicted by TTM, we do an HMM migration of the data to system memory
>> (triggered by kgd2kfd_schedule_evict_and_restore_process in patch 30).
>> That means we don't need TTM to copy the BO contents to GTT any more.
>> Instead we want to use pipelined gutting to allow the VRAM to be freed
>> once the fence signals that the HMM migration is done (the
>> dma_fence_signal call near the end of svm_range_evict_svm_bo_worker in
>> patch 28).
>>
>> Regards,
>>    Felix
>>
>>
>>> As Daniel pointed out HMM and dma_fences are fundamentally
>>> incompatible.
>>>
>>> Christian.
>>>
 Signed-off-by: Alex Sierra 
 Signed-off-by: Felix Kuehling 
 ---
    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 ++
    1 file changed, 14 insertions(+)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
 index f423f42cb9b5..62d4da95d22d 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
 @@ -107,6 +107,20 @@ static void amdgpu_evict_flags(struct
 ttm_buffer_object *bo,
    }
      abo = ttm_to_amdgpu_bo(bo);
 +    if (abo->flags & AMDGPU_AMDKFD_CREATE_SVM_BO) {
 +    struct dma_fence *fence;
 +    struct dma_resv *resv = >base._resv;
 +
 +    rcu_read_lock();
 +    fence = rcu_dereference(resv->fence_excl);
 +    if (fence && !fence->ops->signaled)
 +    dma_fence_enable_sw_signaling(fence);
 +
 +    placement->num_placement = 0;
 +    placement->num_busy_placement = 0;
 +    rcu_read_unlock();
 +    return;
 +    }
    switch (bo->mem.mem_type) {
    case AMDGPU_PL_GDS:
    case AMDGPU_PL_GWS:
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v3 01/12] drm: Add dummy page per device or GEM object

2021-01-07 Thread Andrey Grodzovsky


On 1/7/21 11:30 AM, Daniel Vetter wrote:

On Thu, Jan 07, 2021 at 11:26:52AM -0500, Andrey Grodzovsky wrote:

On 1/7/21 11:21 AM, Daniel Vetter wrote:

On Tue, Jan 05, 2021 at 04:04:16PM -0500, Andrey Grodzovsky wrote:

On 11/23/20 3:01 AM, Christian König wrote:

Am 23.11.20 um 05:54 schrieb Andrey Grodzovsky:

On 11/21/20 9:15 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

Will be used to reroute CPU mapped BO's page faults once
device is removed.

Uff, one page for each exported DMA-buf? That's not something we can do.

We need to find a different approach here.

Can't we call alloc_page() on each fault and link them together
so they are freed when the device is finally reaped?

For sure better to optimize and allocate on demand when we reach
this corner case, but why the linking ?
Shouldn't drm_prime_gem_destroy be good enough place to free ?

I want to avoid keeping the page in the GEM object.

What we can do is to allocate a page on demand for each fault and link
the together in the bdev instead.

And when the bdev is then finally destroyed after the last application
closed we can finally release all of them.

Christian.

Hey, started to implement this and then realized that by allocating a page
for each fault indiscriminately
we will be allocating a new page for each faulting virtual address within a
VA range belonging the same BO
and this is obviously too much and not the intention. Should I instead use
let's say a hashtable with the hash
key being faulting BO address to actually keep allocating and reusing same
dummy zero page per GEM BO
(or for that matter DRM file object address for non imported BOs) ?

Why do we need a hashtable? All the sw structures to track this should
still be around:
- if gem_bo->dma_buf is set the buffer is currently exported as a dma-buf,
so defensively allocate a per-bo page
- otherwise allocate a per-file page


That exactly what we have in current implementation



Or is the idea to save the struct page * pointer? That feels a bit like
over-optimizing stuff. Better to have a simple implementation first and
then tune it if (and only if) any part of it becomes a problem for normal
usage.


Exactly - the idea is to avoid adding extra pointer to drm_gem_object,
Christian suggested to instead keep a linked list of dummy pages to be
allocated on demand once we hit a vm_fault. I will then also prefault the entire
VA range from vma->vm_end - vma->vm_start to vma->vm_end and map them
to that single dummy page.

This strongly feels like premature optimization. If you're worried about
the overhead on amdgpu, pay down the debt by removing one of the redundant
pointers between gem and ttm bo structs (I think we still have some) :-)

Until we've nuked these easy ones we shouldn't play "avoid 1
pointer just because" games with hashtables.
-Daniel



Well, if you and Christian can agree on this approach and suggest maybe what 
pointer is
redundant and can be removed from GEM struct so we can use the 'credit' to add 
the dummy page

to GEM I will be happy to follow through.

P.S Hash table is off the table anyway and we are talking only about linked list 
here since by prefaulting
the entire VA range for a vmf->vma i will be avoiding redundant page faults to 
same VMA VA range and so
don't need to search and reuse an existing dummy page but simply create a new 
one for each next fault.


Andrey





Andrey



-Daniel


Andrey



Andrey



Regards,
Christian.


Signed-off-by: Andrey Grodzovsky 
---
    drivers/gpu/drm/drm_file.c  |  8 
    drivers/gpu/drm/drm_prime.c | 10 ++
    include/drm/drm_file.h  |  2 ++
    include/drm/drm_gem.h   |  2 ++
    4 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 0ac4566..ff3d39f 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -193,6 +193,12 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
    goto out_prime_destroy;
    }
    +    file->dummy_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+    if (!file->dummy_page) {
+    ret = -ENOMEM;
+    goto out_prime_destroy;
+    }
+
    return file;
      out_prime_destroy:
@@ -289,6 +295,8 @@ void drm_file_free(struct drm_file *file)
    if (dev->driver->postclose)
    dev->driver->postclose(dev, file);
    +    __free_page(file->dummy_page);
+
    drm_prime_destroy_file_private(>prime);
      WARN_ON(!list_empty(>event_list));
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 1693aa7..987b45c 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -335,6 +335,13 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
      ret = drm_prime_add_buf_handle(_priv->prime,
    dma_buf, *handle);
+
+    if (!ret) {
+    obj->dummy_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+    if (!obj->dummy_page)
+    ret = -ENOMEM;
+    }
+
 

Re: [PATCH v3 01/12] drm: Add dummy page per device or GEM object

2021-01-07 Thread Daniel Vetter
On Thu, Jan 07, 2021 at 11:26:52AM -0500, Andrey Grodzovsky wrote:
> 
> On 1/7/21 11:21 AM, Daniel Vetter wrote:
> > On Tue, Jan 05, 2021 at 04:04:16PM -0500, Andrey Grodzovsky wrote:
> > > On 11/23/20 3:01 AM, Christian König wrote:
> > > > Am 23.11.20 um 05:54 schrieb Andrey Grodzovsky:
> > > > > On 11/21/20 9:15 AM, Christian König wrote:
> > > > > > Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:
> > > > > > > Will be used to reroute CPU mapped BO's page faults once
> > > > > > > device is removed.
> > > > > > Uff, one page for each exported DMA-buf? That's not something we 
> > > > > > can do.
> > > > > > 
> > > > > > We need to find a different approach here.
> > > > > > 
> > > > > > Can't we call alloc_page() on each fault and link them together
> > > > > > so they are freed when the device is finally reaped?
> > > > > 
> > > > > For sure better to optimize and allocate on demand when we reach
> > > > > this corner case, but why the linking ?
> > > > > Shouldn't drm_prime_gem_destroy be good enough place to free ?
> > > > I want to avoid keeping the page in the GEM object.
> > > > 
> > > > What we can do is to allocate a page on demand for each fault and link
> > > > the together in the bdev instead.
> > > > 
> > > > And when the bdev is then finally destroyed after the last application
> > > > closed we can finally release all of them.
> > > > 
> > > > Christian.
> > > 
> > > Hey, started to implement this and then realized that by allocating a page
> > > for each fault indiscriminately
> > > we will be allocating a new page for each faulting virtual address within 
> > > a
> > > VA range belonging the same BO
> > > and this is obviously too much and not the intention. Should I instead use
> > > let's say a hashtable with the hash
> > > key being faulting BO address to actually keep allocating and reusing same
> > > dummy zero page per GEM BO
> > > (or for that matter DRM file object address for non imported BOs) ?
> > Why do we need a hashtable? All the sw structures to track this should
> > still be around:
> > - if gem_bo->dma_buf is set the buffer is currently exported as a dma-buf,
> >so defensively allocate a per-bo page
> > - otherwise allocate a per-file page
> 
> 
> That exactly what we have in current implementation
> 
> 
> > 
> > Or is the idea to save the struct page * pointer? That feels a bit like
> > over-optimizing stuff. Better to have a simple implementation first and
> > then tune it if (and only if) any part of it becomes a problem for normal
> > usage.
> 
> 
> Exactly - the idea is to avoid adding extra pointer to drm_gem_object,
> Christian suggested to instead keep a linked list of dummy pages to be
> allocated on demand once we hit a vm_fault. I will then also prefault the 
> entire
> VA range from vma->vm_end - vma->vm_start to vma->vm_end and map them
> to that single dummy page.

This strongly feels like premature optimization. If you're worried about
the overhead on amdgpu, pay down the debt by removing one of the redundant
pointers between gem and ttm bo structs (I think we still have some) :-)

Until we've nuked these easy ones we shouldn't play "avoid 1
pointer just because" games with hashtables.
-Daniel

> 
> Andrey
> 
> 
> > -Daniel
> > 
> > > Andrey
> > > 
> > > 
> > > > > Andrey
> > > > > 
> > > > > 
> > > > > > Regards,
> > > > > > Christian.
> > > > > > 
> > > > > > > Signed-off-by: Andrey Grodzovsky 
> > > > > > > ---
> > > > > > >    drivers/gpu/drm/drm_file.c  |  8 
> > > > > > >    drivers/gpu/drm/drm_prime.c | 10 ++
> > > > > > >    include/drm/drm_file.h  |  2 ++
> > > > > > >    include/drm/drm_gem.h   |  2 ++
> > > > > > >    4 files changed, 22 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/drm_file.c 
> > > > > > > b/drivers/gpu/drm/drm_file.c
> > > > > > > index 0ac4566..ff3d39f 100644
> > > > > > > --- a/drivers/gpu/drm/drm_file.c
> > > > > > > +++ b/drivers/gpu/drm/drm_file.c
> > > > > > > @@ -193,6 +193,12 @@ struct drm_file *drm_file_alloc(struct 
> > > > > > > drm_minor *minor)
> > > > > > >    goto out_prime_destroy;
> > > > > > >    }
> > > > > > >    +    file->dummy_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
> > > > > > > +    if (!file->dummy_page) {
> > > > > > > +    ret = -ENOMEM;
> > > > > > > +    goto out_prime_destroy;
> > > > > > > +    }
> > > > > > > +
> > > > > > >    return file;
> > > > > > >      out_prime_destroy:
> > > > > > > @@ -289,6 +295,8 @@ void drm_file_free(struct drm_file *file)
> > > > > > >    if (dev->driver->postclose)
> > > > > > >    dev->driver->postclose(dev, file);
> > > > > > >    +    __free_page(file->dummy_page);
> > > > > > > +
> > > > > > >    drm_prime_destroy_file_private(>prime);
> > > > > > >      WARN_ON(!list_empty(>event_list));
> > > > > > > diff --git a/drivers/gpu/drm/drm_prime.c 
> > > > > > > b/drivers/gpu/drm/drm_prime.c
> > > > > > > index 1693aa7..987b45c 100644
> > > > > > > --- 

Re: [PATCH 29/35] drm/amdgpu: svm bo enable_signal call condition

2021-01-07 Thread Christian König

Am 07.01.21 um 17:16 schrieb Felix Kuehling:

Am 2021-01-07 um 5:56 a.m. schrieb Christian König:


Am 07.01.21 um 04:01 schrieb Felix Kuehling:

From: Alex Sierra 

[why]
To support svm bo eviction mechanism.

[how]
If the BO crated has AMDGPU_AMDKFD_CREATE_SVM_BO flag set,
enable_signal callback will be called inside amdgpu_evict_flags.
This also causes gutting of the BO by removing all placements,
so that TTM won't actually do an eviction. Instead it will discard
the memory held by the BO. This is needed for HMM migration to user
mode system memory pages.

I don't think that this will work. What exactly are you doing here?

We discussed this a while ago when we talked about pipelined gutting.
And you actually helped us out with a fix for that
(https://patchwork.freedesktop.org/patch/379039/).


That's not what I meant. The pipelined gutting is ok, but why the 
enable_signaling()?


Christian.



SVM BOs are BOs in VRAM containing data for HMM ranges. When such a BO
is evicted by TTM, we do an HMM migration of the data to system memory
(triggered by kgd2kfd_schedule_evict_and_restore_process in patch 30).
That means we don't need TTM to copy the BO contents to GTT any more.
Instead we want to use pipelined gutting to allow the VRAM to be freed
once the fence signals that the HMM migration is done (the
dma_fence_signal call near the end of svm_range_evict_svm_bo_worker in
patch 28).

Regards,
   Felix



As Daniel pointed out HMM and dma_fences are fundamentally incompatible.

Christian.


Signed-off-by: Alex Sierra 
Signed-off-by: Felix Kuehling 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 ++
   1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index f423f42cb9b5..62d4da95d22d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -107,6 +107,20 @@ static void amdgpu_evict_flags(struct
ttm_buffer_object *bo,
   }
     abo = ttm_to_amdgpu_bo(bo);
+    if (abo->flags & AMDGPU_AMDKFD_CREATE_SVM_BO) {
+    struct dma_fence *fence;
+    struct dma_resv *resv = >base._resv;
+
+    rcu_read_lock();
+    fence = rcu_dereference(resv->fence_excl);
+    if (fence && !fence->ops->signaled)
+    dma_fence_enable_sw_signaling(fence);
+
+    placement->num_placement = 0;
+    placement->num_busy_placement = 0;
+    rcu_read_unlock();
+    return;
+    }
   switch (bo->mem.mem_type) {
   case AMDGPU_PL_GDS:
   case AMDGPU_PL_GWS:


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


Re: [PATCH v3 01/12] drm: Add dummy page per device or GEM object

2021-01-07 Thread Andrey Grodzovsky

Typo Correction bellow

On 1/7/21 11:26 AM, Andrey Grodzovsky wrote:


Or is the idea to save the struct page * pointer? That feels a bit like
over-optimizing stuff. Better to have a simple implementation first and
then tune it if (and only if) any part of it becomes a problem for normal
usage.



Exactly - the idea is to avoid adding extra pointer to drm_gem_object,
Christian suggested to instead keep a linked list of dummy pages to be
allocated on demand once we hit a vm_fault. I will then also prefault the entire
VA range from vma->vm_end - vma->vm_start to vma->vm_end and map them
to that single dummy page.



Obviously the range is from  vma->vm_start to vma->vm_end

Andrey




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


Re: [PATCH v3 01/12] drm: Add dummy page per device or GEM object

2021-01-07 Thread Andrey Grodzovsky


On 1/7/21 11:21 AM, Daniel Vetter wrote:

On Tue, Jan 05, 2021 at 04:04:16PM -0500, Andrey Grodzovsky wrote:

On 11/23/20 3:01 AM, Christian König wrote:

Am 23.11.20 um 05:54 schrieb Andrey Grodzovsky:

On 11/21/20 9:15 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

Will be used to reroute CPU mapped BO's page faults once
device is removed.

Uff, one page for each exported DMA-buf? That's not something we can do.

We need to find a different approach here.

Can't we call alloc_page() on each fault and link them together
so they are freed when the device is finally reaped?


For sure better to optimize and allocate on demand when we reach
this corner case, but why the linking ?
Shouldn't drm_prime_gem_destroy be good enough place to free ?

I want to avoid keeping the page in the GEM object.

What we can do is to allocate a page on demand for each fault and link
the together in the bdev instead.

And when the bdev is then finally destroyed after the last application
closed we can finally release all of them.

Christian.


Hey, started to implement this and then realized that by allocating a page
for each fault indiscriminately
we will be allocating a new page for each faulting virtual address within a
VA range belonging the same BO
and this is obviously too much and not the intention. Should I instead use
let's say a hashtable with the hash
key being faulting BO address to actually keep allocating and reusing same
dummy zero page per GEM BO
(or for that matter DRM file object address for non imported BOs) ?

Why do we need a hashtable? All the sw structures to track this should
still be around:
- if gem_bo->dma_buf is set the buffer is currently exported as a dma-buf,
   so defensively allocate a per-bo page
- otherwise allocate a per-file page



That exactly what we have in current implementation




Or is the idea to save the struct page * pointer? That feels a bit like
over-optimizing stuff. Better to have a simple implementation first and
then tune it if (and only if) any part of it becomes a problem for normal
usage.



Exactly - the idea is to avoid adding extra pointer to drm_gem_object,
Christian suggested to instead keep a linked list of dummy pages to be
allocated on demand once we hit a vm_fault. I will then also prefault the entire
VA range from vma->vm_end - vma->vm_start to vma->vm_end and map them
to that single dummy page.

Andrey



-Daniel


Andrey



Andrey



Regards,
Christian.


Signed-off-by: Andrey Grodzovsky 
---
   drivers/gpu/drm/drm_file.c  |  8 
   drivers/gpu/drm/drm_prime.c | 10 ++
   include/drm/drm_file.h  |  2 ++
   include/drm/drm_gem.h   |  2 ++
   4 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 0ac4566..ff3d39f 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -193,6 +193,12 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
   goto out_prime_destroy;
   }
   +    file->dummy_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+    if (!file->dummy_page) {
+    ret = -ENOMEM;
+    goto out_prime_destroy;
+    }
+
   return file;
     out_prime_destroy:
@@ -289,6 +295,8 @@ void drm_file_free(struct drm_file *file)
   if (dev->driver->postclose)
   dev->driver->postclose(dev, file);
   +    __free_page(file->dummy_page);
+
   drm_prime_destroy_file_private(>prime);
     WARN_ON(!list_empty(>event_list));
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 1693aa7..987b45c 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -335,6 +335,13 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
     ret = drm_prime_add_buf_handle(_priv->prime,
   dma_buf, *handle);
+
+    if (!ret) {
+    obj->dummy_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+    if (!obj->dummy_page)
+    ret = -ENOMEM;
+    }
+
   mutex_unlock(_priv->prime.lock);
   if (ret)
   goto fail;
@@ -1020,6 +1027,9 @@ void drm_prime_gem_destroy(struct
drm_gem_object *obj, struct sg_table *sg)
   dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL);
   dma_buf = attach->dmabuf;
   dma_buf_detach(attach->dmabuf, attach);
+
+    __free_page(obj->dummy_page);
+
   /* remove the reference */
   dma_buf_put(dma_buf);
   }
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 716990b..2a011fc 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -346,6 +346,8 @@ struct drm_file {
    */
   struct drm_prime_file_private prime;
   +    struct page *dummy_page;
+
   /* private: */
   #if IS_ENABLED(CONFIG_DRM_LEGACY)
   unsigned long lock_count; /* DRI1 legacy lock count */
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 337a483..76a97a3 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -311,6 +311,8 @@ struct drm_gem_object {
    

Re: [PATCH 00/35] Add HMM-based SVM memory manager to KFD

2021-01-07 Thread Felix Kuehling
Am 2021-01-07 um 4:23 a.m. schrieb Daniel Vetter:
> On Wed, Jan 06, 2021 at 10:00:52PM -0500, Felix Kuehling wrote:
>> This is the first version of our HMM based shared virtual memory manager
>> for KFD. There are still a number of known issues that we're working through
>> (see below). This will likely lead to some pretty significant changes in
>> MMU notifier handling and locking on the migration code paths. So don't
>> get hung up on those details yet.
>>
>> But I think this is a good time to start getting feedback. We're pretty
>> confident about the ioctl API, which is both simple and extensible for the
>> future. (see patches 4,16) The user mode side of the API can be found here:
>> https://github.com/RadeonOpenCompute/ROCT-Thunk-Interface/blob/fxkamd/hmm-wip/src/svm.c
>>
>> I'd also like another pair of eyes on how we're interfacing with the GPU VM
>> code in amdgpu_vm.c (see patches 12,13), retry page fault handling (24,25),
>> and some retry IRQ handling changes (32).
>>
>>
>> Known issues:
>> * won't work with IOMMU enabled, we need to dma_map all pages properly
>> * still working on some race conditions and random bugs
>> * performance is not great yet
> Still catching up, but I think there's another one for your list:
>
>  * hmm gpu context preempt vs page fault handling. I've had a short
>discussion about this one with Christian before the holidays, and also
>some private chats with Jerome. It's nasty since no easy fix, much less
>a good idea what's the best approach here.

Do you have a pointer to that discussion or any more details?

Thanks,
  Felix


>
> I'll try to look at this more in-depth when I'm catching up on mails.
> -Daniel
>
>> Alex Sierra (12):
>>   drm/amdgpu: replace per_device_list by array
>>   drm/amdkfd: helper to convert gpu id and idx
>>   drm/amdkfd: add xnack enabled flag to kfd_process
>>   drm/amdkfd: add ioctl to configure and query xnack retries
>>   drm/amdkfd: invalidate tables on page retry fault
>>   drm/amdkfd: page table restore through svm API
>>   drm/amdkfd: SVM API call to restore page tables
>>   drm/amdkfd: add svm_bo reference for eviction fence
>>   drm/amdgpu: add param bit flag to create SVM BOs
>>   drm/amdkfd: add svm_bo eviction mechanism support
>>   drm/amdgpu: svm bo enable_signal call condition
>>   drm/amdgpu: add svm_bo eviction to enable_signal cb
>>
>> Philip Yang (23):
>>   drm/amdkfd: select kernel DEVICE_PRIVATE option
>>   drm/amdkfd: add svm ioctl API
>>   drm/amdkfd: Add SVM API support capability bits
>>   drm/amdkfd: register svm range
>>   drm/amdkfd: add svm ioctl GET_ATTR op
>>   drm/amdgpu: add common HMM get pages function
>>   drm/amdkfd: validate svm range system memory
>>   drm/amdkfd: register overlap system memory range
>>   drm/amdkfd: deregister svm range
>>   drm/amdgpu: export vm update mapping interface
>>   drm/amdkfd: map svm range to GPUs
>>   drm/amdkfd: svm range eviction and restore
>>   drm/amdkfd: register HMM device private zone
>>   drm/amdkfd: validate vram svm range from TTM
>>   drm/amdkfd: support xgmi same hive mapping
>>   drm/amdkfd: copy memory through gart table
>>   drm/amdkfd: HMM migrate ram to vram
>>   drm/amdkfd: HMM migrate vram to ram
>>   drm/amdgpu: reserve fence slot to update page table
>>   drm/amdgpu: enable retry fault wptr overflow
>>   drm/amdkfd: refine migration policy with xnack on
>>   drm/amdkfd: add svm range validate timestamp
>>   drm/amdkfd: multiple gpu migrate vram to vram
>>
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|3 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h|4 +-
>>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c  |   16 +-
>>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   13 +-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c|   83 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h|7 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h|5 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |   90 +-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c|   47 +-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|   10 +
>>  drivers/gpu/drm/amd/amdgpu/vega10_ih.c|   32 +-
>>  drivers/gpu/drm/amd/amdgpu/vega20_ih.c|   32 +-
>>  drivers/gpu/drm/amd/amdkfd/Kconfig|1 +
>>  drivers/gpu/drm/amd/amdkfd/Makefile   |4 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c  |  170 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_iommu.c|8 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c  |  866 ++
>>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.h  |   59 +
>>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h |   52 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_process.c  |  200 +-
>>  .../amd/amdkfd/kfd_process_queue_manager.c|6 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c  | 2564 +
>>  drivers/gpu/drm/amd/amdkfd/kfd_svm.h  |  135 +
>>  drivers/gpu/drm/amd/amdkfd/kfd_topology.c |1 +
>>  drivers/gpu/drm/amd/amdkfd/kfd_topology.h |   10 +-

Re: [PATCH v3 01/12] drm: Add dummy page per device or GEM object

2021-01-07 Thread Daniel Vetter
On Tue, Jan 05, 2021 at 04:04:16PM -0500, Andrey Grodzovsky wrote:
> 
> On 11/23/20 3:01 AM, Christian König wrote:
> > Am 23.11.20 um 05:54 schrieb Andrey Grodzovsky:
> > > 
> > > On 11/21/20 9:15 AM, Christian König wrote:
> > > > Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:
> > > > > Will be used to reroute CPU mapped BO's page faults once
> > > > > device is removed.
> > > > 
> > > > Uff, one page for each exported DMA-buf? That's not something we can do.
> > > > 
> > > > We need to find a different approach here.
> > > > 
> > > > Can't we call alloc_page() on each fault and link them together
> > > > so they are freed when the device is finally reaped?
> > > 
> > > 
> > > For sure better to optimize and allocate on demand when we reach
> > > this corner case, but why the linking ?
> > > Shouldn't drm_prime_gem_destroy be good enough place to free ?
> > 
> > I want to avoid keeping the page in the GEM object.
> > 
> > What we can do is to allocate a page on demand for each fault and link
> > the together in the bdev instead.
> > 
> > And when the bdev is then finally destroyed after the last application
> > closed we can finally release all of them.
> > 
> > Christian.
> 
> 
> Hey, started to implement this and then realized that by allocating a page
> for each fault indiscriminately
> we will be allocating a new page for each faulting virtual address within a
> VA range belonging the same BO
> and this is obviously too much and not the intention. Should I instead use
> let's say a hashtable with the hash
> key being faulting BO address to actually keep allocating and reusing same
> dummy zero page per GEM BO
> (or for that matter DRM file object address for non imported BOs) ?

Why do we need a hashtable? All the sw structures to track this should
still be around:
- if gem_bo->dma_buf is set the buffer is currently exported as a dma-buf,
  so defensively allocate a per-bo page
- otherwise allocate a per-file page

Or is the idea to save the struct page * pointer? That feels a bit like
over-optimizing stuff. Better to have a simple implementation first and
then tune it if (and only if) any part of it becomes a problem for normal
usage.
-Daniel

> 
> Andrey
> 
> 
> > 
> > > 
> > > Andrey
> > > 
> > > 
> > > > 
> > > > Regards,
> > > > Christian.
> > > > 
> > > > > 
> > > > > Signed-off-by: Andrey Grodzovsky 
> > > > > ---
> > > > >   drivers/gpu/drm/drm_file.c  |  8 
> > > > >   drivers/gpu/drm/drm_prime.c | 10 ++
> > > > >   include/drm/drm_file.h  |  2 ++
> > > > >   include/drm/drm_gem.h   |  2 ++
> > > > >   4 files changed, 22 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > > > > index 0ac4566..ff3d39f 100644
> > > > > --- a/drivers/gpu/drm/drm_file.c
> > > > > +++ b/drivers/gpu/drm/drm_file.c
> > > > > @@ -193,6 +193,12 @@ struct drm_file *drm_file_alloc(struct drm_minor 
> > > > > *minor)
> > > > >   goto out_prime_destroy;
> > > > >   }
> > > > >   +    file->dummy_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
> > > > > +    if (!file->dummy_page) {
> > > > > +    ret = -ENOMEM;
> > > > > +    goto out_prime_destroy;
> > > > > +    }
> > > > > +
> > > > >   return file;
> > > > >     out_prime_destroy:
> > > > > @@ -289,6 +295,8 @@ void drm_file_free(struct drm_file *file)
> > > > >   if (dev->driver->postclose)
> > > > >   dev->driver->postclose(dev, file);
> > > > >   +    __free_page(file->dummy_page);
> > > > > +
> > > > >   drm_prime_destroy_file_private(>prime);
> > > > >     WARN_ON(!list_empty(>event_list));
> > > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > > > > index 1693aa7..987b45c 100644
> > > > > --- a/drivers/gpu/drm/drm_prime.c
> > > > > +++ b/drivers/gpu/drm/drm_prime.c
> > > > > @@ -335,6 +335,13 @@ int drm_gem_prime_fd_to_handle(struct drm_device 
> > > > > *dev,
> > > > >     ret = drm_prime_add_buf_handle(_priv->prime,
> > > > >   dma_buf, *handle);
> > > > > +
> > > > > +    if (!ret) {
> > > > > +    obj->dummy_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
> > > > > +    if (!obj->dummy_page)
> > > > > +    ret = -ENOMEM;
> > > > > +    }
> > > > > +
> > > > >   mutex_unlock(_priv->prime.lock);
> > > > >   if (ret)
> > > > >   goto fail;
> > > > > @@ -1020,6 +1027,9 @@ void drm_prime_gem_destroy(struct
> > > > > drm_gem_object *obj, struct sg_table *sg)
> > > > >   dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL);
> > > > >   dma_buf = attach->dmabuf;
> > > > >   dma_buf_detach(attach->dmabuf, attach);
> > > > > +
> > > > > +    __free_page(obj->dummy_page);
> > > > > +
> > > > >   /* remove the reference */
> > > > >   dma_buf_put(dma_buf);
> > > > >   }
> > > > > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> > > > > index 716990b..2a011fc 100644
> > > > > --- a/include/drm/drm_file.h
> > > > > +++ 

Re: [PATCH 29/35] drm/amdgpu: svm bo enable_signal call condition

2021-01-07 Thread Felix Kuehling

Am 2021-01-07 um 5:56 a.m. schrieb Christian König:

> Am 07.01.21 um 04:01 schrieb Felix Kuehling:
>> From: Alex Sierra 
>>
>> [why]
>> To support svm bo eviction mechanism.
>>
>> [how]
>> If the BO crated has AMDGPU_AMDKFD_CREATE_SVM_BO flag set,
>> enable_signal callback will be called inside amdgpu_evict_flags.
>> This also causes gutting of the BO by removing all placements,
>> so that TTM won't actually do an eviction. Instead it will discard
>> the memory held by the BO. This is needed for HMM migration to user
>> mode system memory pages.
>
> I don't think that this will work. What exactly are you doing here?
We discussed this a while ago when we talked about pipelined gutting.
And you actually helped us out with a fix for that
(https://patchwork.freedesktop.org/patch/379039/).

SVM BOs are BOs in VRAM containing data for HMM ranges. When such a BO
is evicted by TTM, we do an HMM migration of the data to system memory
(triggered by kgd2kfd_schedule_evict_and_restore_process in patch 30).
That means we don't need TTM to copy the BO contents to GTT any more.
Instead we want to use pipelined gutting to allow the VRAM to be freed
once the fence signals that the HMM migration is done (the
dma_fence_signal call near the end of svm_range_evict_svm_bo_worker in
patch 28).

Regards,
  Felix


>
> As Daniel pointed out HMM and dma_fences are fundamentally incompatible.
>
> Christian.
>
>>
>> Signed-off-by: Alex Sierra 
>> Signed-off-by: Felix Kuehling 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 ++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index f423f42cb9b5..62d4da95d22d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -107,6 +107,20 @@ static void amdgpu_evict_flags(struct
>> ttm_buffer_object *bo,
>>   }
>>     abo = ttm_to_amdgpu_bo(bo);
>> +    if (abo->flags & AMDGPU_AMDKFD_CREATE_SVM_BO) {
>> +    struct dma_fence *fence;
>> +    struct dma_resv *resv = >base._resv;
>> +
>> +    rcu_read_lock();
>> +    fence = rcu_dereference(resv->fence_excl);
>> +    if (fence && !fence->ops->signaled)
>> +    dma_fence_enable_sw_signaling(fence);
>> +
>> +    placement->num_placement = 0;
>> +    placement->num_busy_placement = 0;
>> +    rcu_read_unlock();
>> +    return;
>> +    }
>>   switch (bo->mem.mem_type) {
>>   case AMDGPU_PL_GDS:
>>   case AMDGPU_PL_GWS:
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: don't limit gtt size on apus

2021-01-07 Thread Christian König

Am 06.01.21 um 18:04 schrieb Joshua Ashton:



On 1/6/21 2:59 PM, Christian König wrote:

Am 06.01.21 um 15:18 schrieb Joshua Ashton:

[SNIP]
For Vulkan we (both RADV and AMDVLK) use GTT as the total size. 
Usage in modern games is essentially "bindless" so there is no 
way to track at a per-submission level what memory needs to be 
resident. (and even with tracking applications are allowed to 
use all the memory in a single draw call, which would be 
unsplittable anyway ...)


Yeah, that is a really good point.

The issue is that we need some limitation since 3/4 of system 
memory is way to much and the max texture size test in piglit can 
cause a system crash.


The alternative is a better OOM handling, so that an application 
which uses to much system memory through the driver stack has a 
more likely chance to get killed. Cause currently that is either 
X or Wayland :(


Christian.


As I understand it, what is being exposed right now is essentially 
max(vram size, 3GiB) limited by 3/4ths of the memory. Previously, 
before the revert what was being taken was just max(3GiB, 3/4ths).


If you had < 3GiB of system memory that seems like a bit of an 
issue that could easily leat to OOM to me?


Not really, as I said GTT is only the memory the GPU can lock at 
the same time. It is perfectly possible to have that larger than 
the available system memory.


In other words this is *not* to prevent using to much system 
memory, for this we have an additional limit inside TTM. But 
instead to have a reasonable limit for applications to not use to 
much memory at the same time.




Worth noting that this GTT size here also affects the memory 
reporting and budgeting for applications. If the user has 1GiB of 
total system memory and 3GiB set here, then 3GiB will be the budget 
and size exposed to applications too...


Yeah, that's indeed problematic.



(On APUs,) we really don't want to expose more GTT than system 
memory. Apps will eat into it and end up swapping or running into 
OOM or swapping *very* quickly. (I imagine this is likely what was 
being run into before the revert.)


No, the issue is that some applications try to allocate textures way 
above some reasonable limit.


Alternatively, in RADV and other user space drivers like AMDVLK, we 
could limit this to the system memory size or 3/4ths ourselves. 
Although that's kinda gross and I don't think that's the correct 
path...


Ok, let me explain from the other side: We have this limitation 
because otherwise some tests like the maximum texture size test for 
OpenGL crashes the system. And this is independent of your system 
configuration.


We could of course add another limit for the texture size in 
OpenGL/RADV/AMDVLK, but I agree that this is rather awkward.




Are you hitting on something smaller than 3/4ths right now? I 
remember the source commit mentioned they only had 1GiB of system 
memory available, so that could be possible if you had a carveout 
of < 786MiB...


What do you mean with that? I don't have a test system at hand for 
this if that's what you are asking for.


This was mainly a question to whoever did the revert. The question 
to find out some extra info about what they are using at the time.


You don't need a specific system configuration for this, just try to 
run the max texture size test in piglit.


Regards,
Christian.


I see... I have not managed to reproduce a hang as described in the 
revert commit, but I have had a soft crash and delay with the OOM 
killer ending X.org after a little bit when GTT > system memory.


I tested with max-texture-size on both Renoir and Picasso the 
following conditions:

16GiB RAM + 12 GiB GTT -> test works fine
16GiB RAM + 64 GiB GTT -> OOM killer kills X.org after a little bit of 
waiting (piglit died with it)

2 GiB RAM + 1.5GiB GTT -> test works fine

I also tested on my Radeon VII and it worked fine regardless of the 
GTT size there, although that card has more than enough video memory 
any way for nothing to be an issue there .
Limiting my system memory to 2GiB, the card's memory and visible 
memory to 1GiB and the GTT to 1.75GiB, the test works fine.


The only time I ever had problems with a crash or pesudo-hang (waiting 
for OOM killer but the system was locked up) was whenever GTT was > 
system memory (ie. in the reverted commit)


If I edited my commit to universally use 3/4ths of the system memory 
for GTT for all hardware, would that be considered to be merged?


Well maybe 1/2 and only on APUs. And you need to find somebody with 
another Raven to test that. Maybe Nirmoy has time for this.


Regards,
Christian.



Thanks!
- Joshie ✨





- Joshie ✨




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


[PATCH 5.10 01/20] Revert "drm/amd/display: Fix memory leaks in S3 resume"

2021-01-07 Thread Greg Kroah-Hartman
From: Alex Deucher 

This reverts commit a135a1b4c4db1f3b8cbed9676a40ede39feb3362.

This leads to blank screens on some boards after replugging a
display.  Revert until we understand the root cause and can
fix both the leak and the blank screen after replug.

Cc: Stylon Wang 
Cc: Harry Wentland 
Cc: Nicholas Kazlauskas 
Cc: Andre Tomt 
Cc: Oleksandr Natalenko 
Signed-off-by: Alex Deucher 
Cc: sta...@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2278,8 +2278,7 @@ void amdgpu_dm_update_connector_after_de
 
drm_connector_update_edid_property(connector,
   aconnector->edid);
-   aconnector->num_modes = drm_add_edid_modes(connector, 
aconnector->edid);
-   drm_connector_list_update(connector);
+   drm_add_edid_modes(connector, aconnector->edid);
 
if (aconnector->dc_link->aux_mode)
drm_dp_cec_set_edid(>dm_dp_aux.aux,


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


[PATCH 5.4 01/13] Revert "drm/amd/display: Fix memory leaks in S3 resume"

2021-01-07 Thread Greg Kroah-Hartman
From: Alex Deucher 

This reverts commit a135a1b4c4db1f3b8cbed9676a40ede39feb3362.

This leads to blank screens on some boards after replugging a
display.  Revert until we understand the root cause and can
fix both the leak and the blank screen after replug.

Cc: Stylon Wang 
Cc: Harry Wentland 
Cc: Nicholas Kazlauskas 
Cc: Andre Tomt 
Cc: Oleksandr Natalenko 
Signed-off-by: Alex Deucher 
Cc: sta...@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1434,8 +1434,7 @@ amdgpu_dm_update_connector_after_detect(
 
drm_connector_update_edid_property(connector,
   aconnector->edid);
-   aconnector->num_modes = drm_add_edid_modes(connector, 
aconnector->edid);
-   drm_connector_list_update(connector);
+   drm_add_edid_modes(connector, aconnector->edid);
 
if (aconnector->dc_link->aux_mode)
drm_dp_cec_set_edid(>dm_dp_aux.aux,


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


Re: [PATCH 32/35] drm/amdgpu: enable retry fault wptr overflow

2021-01-07 Thread Christian König

Am 07.01.21 um 04:01 schrieb Felix Kuehling:

From: Philip Yang 

If xnack is on, VM retry fault interrupt send to IH ring1, and ring1
will be full quickly. IH cannot receive other interrupts, this causes
deadlock if migrating buffer using sdma and waiting for sdma done while
handling retry fault.

Remove VMC from IH storm client, enable ring1 write pointer overflow,
then IH will drop retry fault interrupts and be able to receive other
interrupts while driver is handling retry fault.

IH ring1 write pointer doesn't writeback to memory by IH, and ring1
write pointer recorded by self-irq is not updated, so always read
the latest ring1 write pointer from register.

Signed-off-by: Philip Yang 
Signed-off-by: Felix Kuehling 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 32 +-
  drivers/gpu/drm/amd/amdgpu/vega20_ih.c | 32 +-
  2 files changed, 22 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c 
b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
index 88626d83e07b..ca8efa5c6978 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
@@ -220,10 +220,8 @@ static int vega10_ih_enable_ring(struct amdgpu_device 
*adev,
tmp = vega10_ih_rb_cntl(ih, tmp);
if (ih == >irq.ih)
tmp = REG_SET_FIELD(tmp, IH_RB_CNTL, RPTR_REARM, 
!!adev->irq.msi_enabled);
-   if (ih == >irq.ih1) {
-   tmp = REG_SET_FIELD(tmp, IH_RB_CNTL, WPTR_OVERFLOW_ENABLE, 0);
+   if (ih == >irq.ih1)
tmp = REG_SET_FIELD(tmp, IH_RB_CNTL, RB_FULL_DRAIN_ENABLE, 1);
-   }
if (amdgpu_sriov_vf(adev)) {
if (psp_reg_program(>psp, ih_regs->psp_reg_id, tmp)) {
dev_err(adev->dev, "PSP program IH_RB_CNTL failed!\n");
@@ -265,7 +263,6 @@ static int vega10_ih_irq_init(struct amdgpu_device *adev)
u32 ih_chicken;
int ret;
int i;
-   u32 tmp;
  
  	/* disable irqs */

ret = vega10_ih_toggle_interrupts(adev, false);
@@ -291,15 +288,6 @@ static int vega10_ih_irq_init(struct amdgpu_device *adev)
}
}
  
-	tmp = RREG32_SOC15(OSSSYS, 0, mmIH_STORM_CLIENT_LIST_CNTL);

-   tmp = REG_SET_FIELD(tmp, IH_STORM_CLIENT_LIST_CNTL,
-   CLIENT18_IS_STORM_CLIENT, 1);
-   WREG32_SOC15(OSSSYS, 0, mmIH_STORM_CLIENT_LIST_CNTL, tmp);
-
-   tmp = RREG32_SOC15(OSSSYS, 0, mmIH_INT_FLOOD_CNTL);
-   tmp = REG_SET_FIELD(tmp, IH_INT_FLOOD_CNTL, FLOOD_CNTL_ENABLE, 1);
-   WREG32_SOC15(OSSSYS, 0, mmIH_INT_FLOOD_CNTL, tmp);
-
pci_set_master(adev->pdev);
  
  	/* enable interrupts */

@@ -345,11 +333,17 @@ static u32 vega10_ih_get_wptr(struct amdgpu_device *adev,
u32 wptr, tmp;
struct amdgpu_ih_regs *ih_regs;
  
-	wptr = le32_to_cpu(*ih->wptr_cpu);

-   ih_regs = >ih_regs;
+   if (ih == >irq.ih) {
+   /* Only ring0 supports writeback. On other rings fall back
+* to register-based code with overflow checking below.
+*/
+   wptr = le32_to_cpu(*ih->wptr_cpu);
  
-	if (!REG_GET_FIELD(wptr, IH_RB_WPTR, RB_OVERFLOW))

-   goto out;
+   if (!REG_GET_FIELD(wptr, IH_RB_WPTR, RB_OVERFLOW))
+   goto out;
+   }
+
+   ih_regs = >ih_regs;
  
  	/* Double check that the overflow wasn't already cleared. */

wptr = RREG32_NO_KIQ(ih_regs->ih_rb_wptr);
@@ -440,15 +434,11 @@ static int vega10_ih_self_irq(struct amdgpu_device *adev,
  struct amdgpu_irq_src *source,
  struct amdgpu_iv_entry *entry)
  {
-   uint32_t wptr = cpu_to_le32(entry->src_data[0]);
-
switch (entry->ring_id) {
case 1:
-   *adev->irq.ih1.wptr_cpu = wptr;
schedule_work(>irq.ih1_work);
break;
case 2:
-   *adev->irq.ih2.wptr_cpu = wptr;
schedule_work(>irq.ih2_work);
break;
default: break;
diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_ih.c 
b/drivers/gpu/drm/amd/amdgpu/vega20_ih.c
index 42032ca380cc..60d1bd51781e 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega20_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega20_ih.c
@@ -220,10 +220,8 @@ static int vega20_ih_enable_ring(struct amdgpu_device 
*adev,
tmp = vega20_ih_rb_cntl(ih, tmp);
if (ih == >irq.ih)
tmp = REG_SET_FIELD(tmp, IH_RB_CNTL, RPTR_REARM, 
!!adev->irq.msi_enabled);
-   if (ih == >irq.ih1) {
-   tmp = REG_SET_FIELD(tmp, IH_RB_CNTL, WPTR_OVERFLOW_ENABLE, 0);
+   if (ih == >irq.ih1)
tmp = REG_SET_FIELD(tmp, IH_RB_CNTL, RB_FULL_DRAIN_ENABLE, 1);
-   }
if (amdgpu_sriov_vf(adev)) {
if (psp_reg_program(>psp, ih_regs->psp_reg_id, tmp)) {
dev_err(adev->dev, "PSP program IH_RB_CNTL failed!\n");
@@ -297,7 +295,6 

Re: [PATCH 31/35] drm/amdgpu: reserve fence slot to update page table

2021-01-07 Thread Christian König

Am 07.01.21 um 04:01 schrieb Felix Kuehling:

From: Philip Yang 

Forgot to reserve a fence slot to use sdma to update page table, cause
below kernel BUG backtrace to handle vm retry fault while application is
exiting.

[  133.048143] kernel BUG at 
/home/yangp/git/compute_staging/kernel/drivers/dma-buf/dma-resv.c:281!
[  133.048487] Workqueue: events amdgpu_irq_handle_ih1 [amdgpu]
[  133.048506] RIP: 0010:dma_resv_add_shared_fence+0x204/0x280
[  133.048672]  amdgpu_vm_sdma_commit+0x134/0x220 [amdgpu]
[  133.048788]  amdgpu_vm_bo_update_range+0x220/0x250 [amdgpu]
[  133.048905]  amdgpu_vm_handle_fault+0x202/0x370 [amdgpu]
[  133.049031]  gmc_v9_0_process_interrupt+0x1ab/0x310 [amdgpu]
[  133.049165]  ? kgd2kfd_interrupt+0x9a/0x180 [amdgpu]
[  133.049289]  ? amdgpu_irq_dispatch+0xb6/0x240 [amdgpu]
[  133.049408]  amdgpu_irq_dispatch+0xb6/0x240 [amdgpu]
[  133.049534]  amdgpu_ih_process+0x9b/0x1c0 [amdgpu]
[  133.049657]  amdgpu_irq_handle_ih1+0x21/0x60 [amdgpu]
[  133.049669]  process_one_work+0x29f/0x640
[  133.049678]  worker_thread+0x39/0x3f0
[  133.049685]  ? process_one_work+0x640/0x640

Signed-off-by: Philip Yang 
Signed-off-by: Felix Kuehling 


Reviewed-by: Christian König 


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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index abdd4e7b4c3b..bd9de870f8f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -3301,7 +3301,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, 
unsigned int pasid,
struct amdgpu_bo *root;
uint64_t value, flags;
struct amdgpu_vm *vm;
-   long r;
+   int r;
bool is_compute_context = false;
  
  	spin_lock(>vm_manager.pasid_lock);

@@ -3359,6 +3359,12 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, 
unsigned int pasid,
value = 0;
}
  
+	r = dma_resv_reserve_shared(root->tbo.base.resv, 1);

+   if (r) {
+   pr_debug("failed %d to reserve fence slot\n", r);
+   goto error_unlock;
+   }
+
r = amdgpu_vm_bo_update_mapping(adev, adev, vm, true, false, NULL, addr,
addr, flags, value, NULL, NULL,
NULL);
@@ -3370,7 +3376,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, 
unsigned int pasid,
  error_unlock:
amdgpu_bo_unreserve(root);
if (r < 0)
-   DRM_ERROR("Can't handle page fault (%ld)\n", r);
+   DRM_ERROR("Can't handle page fault (%d)\n", r);
  
  error_unref:

amdgpu_bo_unref();


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


Re: [PATCH 29/35] drm/amdgpu: svm bo enable_signal call condition

2021-01-07 Thread Christian König

Am 07.01.21 um 04:01 schrieb Felix Kuehling:

From: Alex Sierra 

[why]
To support svm bo eviction mechanism.

[how]
If the BO crated has AMDGPU_AMDKFD_CREATE_SVM_BO flag set,
enable_signal callback will be called inside amdgpu_evict_flags.
This also causes gutting of the BO by removing all placements,
so that TTM won't actually do an eviction. Instead it will discard
the memory held by the BO. This is needed for HMM migration to user
mode system memory pages.


I don't think that this will work. What exactly are you doing here?

As Daniel pointed out HMM and dma_fences are fundamentally incompatible.

Christian.



Signed-off-by: Alex Sierra 
Signed-off-by: Felix Kuehling 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index f423f42cb9b5..62d4da95d22d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -107,6 +107,20 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
*bo,
}
  
  	abo = ttm_to_amdgpu_bo(bo);

+   if (abo->flags & AMDGPU_AMDKFD_CREATE_SVM_BO) {
+   struct dma_fence *fence;
+   struct dma_resv *resv = >base._resv;
+
+   rcu_read_lock();
+   fence = rcu_dereference(resv->fence_excl);
+   if (fence && !fence->ops->signaled)
+   dma_fence_enable_sw_signaling(fence);
+
+   placement->num_placement = 0;
+   placement->num_busy_placement = 0;
+   rcu_read_unlock();
+   return;
+   }
switch (bo->mem.mem_type) {
case AMDGPU_PL_GDS:
case AMDGPU_PL_GWS:


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


Re: [PATCH 12/35] drm/amdgpu: export vm update mapping interface

2021-01-07 Thread Christian König

Am 07.01.21 um 04:01 schrieb Felix Kuehling:

From: Philip Yang 

It will be used by kfd to map svm range to GPU, because svm range does
not have amdgpu_bo and bo_va, cannot use amdgpu_bo_update interface, use
amdgpu vm update interface directly.

Signed-off-by: Philip Yang 
Signed-off-by: Felix Kuehling 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 10 ++
  2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index fdbe7d4e8b8b..9c557e8bf0e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1589,15 +1589,14 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
   * Returns:
   * 0 for success, -EINVAL for failure.
   */
-static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
-  struct amdgpu_device *bo_adev,
-  struct amdgpu_vm *vm, bool immediate,
-  bool unlocked, struct dma_resv *resv,
-  uint64_t start, uint64_t last,
-  uint64_t flags, uint64_t offset,
-  struct drm_mm_node *nodes,
-  dma_addr_t *pages_addr,
-  struct dma_fence **fence)
+int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
+   struct amdgpu_device *bo_adev,
+   struct amdgpu_vm *vm, bool immediate,
+   bool unlocked, struct dma_resv *resv,
+   uint64_t start, uint64_t last, uint64_t flags,
+   uint64_t offset, struct drm_mm_node *nodes,
+   dma_addr_t *pages_addr,
+   struct dma_fence **fence)
  {
struct amdgpu_vm_update_params params;
enum amdgpu_sync_mode sync_mode;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 2bf4ef5fb3e1..73ca630520fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -366,6 +366,8 @@ struct amdgpu_vm_manager {
spinlock_t  pasid_lock;
  };
  
+struct amdgpu_bo_va_mapping;

+
  #define amdgpu_vm_copy_pte(adev, ib, pe, src, count) 
((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
  #define amdgpu_vm_write_pte(adev, ib, pe, value, count, incr) 
((adev)->vm_manager.vm_pte_funcs->write_pte((ib), (pe), (value), (count), 
(incr)))
  #define amdgpu_vm_set_pte_pde(adev, ib, pe, addr, count, incr, flags) 
((adev)->vm_manager.vm_pte_funcs->set_pte_pde((ib), (pe), (addr), (count), 
(incr), (flags)))
@@ -397,6 +399,14 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
  struct dma_fence **fence);
  int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
   struct amdgpu_vm *vm);
+int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
+   struct amdgpu_device *bo_adev,
+   struct amdgpu_vm *vm, bool immediate,
+   bool unlocked, struct dma_resv *resv,
+   uint64_t start, uint64_t last, uint64_t flags,
+   uint64_t offset, struct drm_mm_node *nodes,
+   dma_addr_t *pages_addr,
+   struct dma_fence **fence);
  int amdgpu_vm_bo_update(struct amdgpu_device *adev,
struct amdgpu_bo_va *bo_va,
bool clear);


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


Re: [PATCH 08/35] drm/amdgpu: add common HMM get pages function

2021-01-07 Thread Christian König

Am 07.01.21 um 04:01 schrieb Felix Kuehling:

From: Philip Yang 

Move the HMM get pages function from amdgpu_ttm and to amdgpu_mn. This
common function will be used by new svm APIs.

Signed-off-by: Philip Yang 
Signed-off-by: Felix Kuehling 


Acked-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c  | 83 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h  |  7 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 76 +++---
  3 files changed, 100 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index 828b5167ff12..997da4237a10 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -155,3 +155,86 @@ void amdgpu_mn_unregister(struct amdgpu_bo *bo)
mmu_interval_notifier_remove(>notifier);
bo->notifier.mm = NULL;
  }
+
+int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
+  struct mm_struct *mm, struct page **pages,
+  uint64_t start, uint64_t npages,
+  struct hmm_range **phmm_range, bool readonly,
+  bool mmap_locked)
+{
+   struct hmm_range *hmm_range;
+   unsigned long timeout;
+   unsigned long i;
+   unsigned long *pfns;
+   int r = 0;
+
+   hmm_range = kzalloc(sizeof(*hmm_range), GFP_KERNEL);
+   if (unlikely(!hmm_range))
+   return -ENOMEM;
+
+   pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
+   if (unlikely(!pfns)) {
+   r = -ENOMEM;
+   goto out_free_range;
+   }
+
+   hmm_range->notifier = notifier;
+   hmm_range->default_flags = HMM_PFN_REQ_FAULT;
+   if (!readonly)
+   hmm_range->default_flags |= HMM_PFN_REQ_WRITE;
+   hmm_range->hmm_pfns = pfns;
+   hmm_range->start = start;
+   hmm_range->end = start + npages * PAGE_SIZE;
+   timeout = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+
+retry:
+   hmm_range->notifier_seq = mmu_interval_read_begin(notifier);
+
+   if (likely(!mmap_locked))
+   mmap_read_lock(mm);
+
+   r = hmm_range_fault(hmm_range);
+
+   if (likely(!mmap_locked))
+   mmap_read_unlock(mm);
+   if (unlikely(r)) {
+   /*
+* FIXME: This timeout should encompass the retry from
+* mmu_interval_read_retry() as well.
+*/
+   if (r == -EBUSY && !time_after(jiffies, timeout))
+   goto retry;
+   goto out_free_pfns;
+   }
+
+   /*
+* Due to default_flags, all pages are HMM_PFN_VALID or
+* hmm_range_fault() fails. FIXME: The pages cannot be touched outside
+* the notifier_lock, and mmu_interval_read_retry() must be done first.
+*/
+   for (i = 0; pages && i < npages; i++)
+   pages[i] = hmm_pfn_to_page(pfns[i]);
+
+   *phmm_range = hmm_range;
+
+   return 0;
+
+out_free_pfns:
+   kvfree(pfns);
+out_free_range:
+   kfree(hmm_range);
+
+   return r;
+}
+
+int amdgpu_hmm_range_get_pages_done(struct hmm_range *hmm_range)
+{
+   int r;
+
+   r = mmu_interval_read_retry(hmm_range->notifier,
+   hmm_range->notifier_seq);
+   kvfree(hmm_range->hmm_pfns);
+   kfree(hmm_range);
+
+   return r;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
index a292238f75eb..7f7d37a457c3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
@@ -30,6 +30,13 @@
  #include 
  #include 
  
+int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,

+  struct mm_struct *mm, struct page **pages,
+  uint64_t start, uint64_t npages,
+  struct hmm_range **phmm_range, bool readonly,
+  bool mmap_locked);
+int amdgpu_hmm_range_get_pages_done(struct hmm_range *hmm_range);
+
  #if defined(CONFIG_HMM_MIRROR)
  int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr);
  void amdgpu_mn_unregister(struct amdgpu_bo *bo);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index aaad9e304ad9..f423f42cb9b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -32,7 +32,6 @@
  
  #include 

  #include 
-#include 
  #include 
  #include 
  #include 
@@ -843,10 +842,8 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, 
struct page **pages)
struct amdgpu_ttm_tt *gtt = (void *)ttm;
unsigned long start = gtt->userptr;
struct vm_area_struct *vma;
-   struct hmm_range *range;
-   unsigned long timeout;
struct mm_struct *mm;
-   unsigned long i;
+   bool readonly;
   

[PATCH] amdgpu/sriov Stop data exchange for wholegpu reset

2021-01-07 Thread Jack Zhang
[Why]
When host trigger a whole gpu reset, guest will keep
waiting till host finish reset. But there's a work
queue in guest exchanging data between vf which need
to access frame buffer. During whole gpu reset, frame
buffer is not accessable, and this causes the call trace.

[How]
After vf get reset notification from pf, stop data exchange.

Signed-off-by: Jingwen Chen 
Signed-off-by: Jack Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 1 +
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c| 1 +
 drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c| 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 83ca5cbffe2c..3e212862cf5d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -571,6 +571,7 @@ void amdgpu_virt_fini_data_exchange(struct amdgpu_device 
*adev)
DRM_INFO("clean up the vf2pf work item\n");
flush_delayed_work(>virt.vf2pf_work);
cancel_delayed_work_sync(>virt.vf2pf_work);
+   adev->virt.vf2pf_update_interval_ms = 0;
}
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c 
b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
index 7767ccca526b..3ee481557fc9 100644
--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
@@ -255,6 +255,7 @@ static void xgpu_ai_mailbox_flr_work(struct work_struct 
*work)
if (!down_read_trylock(>reset_sem))
return;
 
+   amdgpu_virt_fini_data_exchange(adev);
atomic_set(>in_gpu_reset, 1);
 
do {
diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c 
b/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
index dd5c1e6ce009..48e588d3c409 100644
--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
@@ -276,6 +276,7 @@ static void xgpu_nv_mailbox_flr_work(struct work_struct 
*work)
if (!down_read_trylock(>reset_sem))
return;
 
+   amdgpu_virt_fini_data_exchange(adev);
atomic_set(>in_gpu_reset, 1);
 
do {
-- 
2.25.1

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


Re: [PATCH v3 8/8] drm: Upcast struct drm_device.dev to struct pci_device; replace pdev

2021-01-07 Thread Thomas Zimmermann

AFAICT these are false positives. The instances have been fixed already.

Am 07.01.21 um 10:45 schrieb kernel test robot:

Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next linus/master v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Move-struct-drm_device-pdev-to-legacy/20210107-161007
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-s021-20210107 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
 # apt-get install sparse
 # sparse version: v0.6.3-208-g46a52ca4-dirty
 # 
https://github.com/0day-ci/linux/commit/380912f7b62c23322562c40e19efd7ad84d57e9c
 git remote add linux-review https://github.com/0day-ci/linux
 git fetch --no-tags linux-review 
Thomas-Zimmermann/drm-Move-struct-drm_device-pdev-to-legacy/20210107-161007
 git checkout 380912f7b62c23322562c40e19efd7ad84d57e9c
 # save the attached .config to linux build tree
 make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

drivers/gpu/drm/gma500/oaktrail_device.c: In function 'oaktrail_chip_setup':

drivers/gpu/drm/gma500/oaktrail_device.c:509:26: error: 'struct drm_device' has 
no member named 'pdev'; did you mean 'dev'?

  509 |  if (pci_enable_msi(dev->pdev))
  |  ^~~~
  |  dev
--
drivers/gpu/drm/gma500/oaktrail_lvds.c: In function 
'oaktrail_lvds_set_power':

drivers/gpu/drm/gma500/oaktrail_lvds.c:63:25: error: 'struct drm_device' has no 
member named 'pdev'; did you mean 'dev'?

   63 |   pm_request_idle(>pdev->dev);
  | ^~~~
  | dev
--
drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c: In function 'get_clock':
drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c:69:11: warning: variable 'tmp' 
set but not used [-Wunused-but-set-variable]
   69 |  u32 val, tmp;
  |   ^~~
drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c: In function 'get_data':
drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c:83:11: warning: variable 'tmp' 
set but not used [-Wunused-but-set-variable]
   83 |  u32 val, tmp;
  |   ^~~
drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c: In function 
'oaktrail_lvds_i2c_init':

drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c:148:35: error: 'struct drm_device' 
has no member named 'pdev'; did you mean 'dev'?

  148 |  chan->adapter.dev.parent = >pdev->dev;
  |   ^~~~
  |   dev
--
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c: In function 'vmw_driver_load':

drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:661:22: error: 'struct drm_device' has no 
member named 'pdev'; did you mean 'dev'?

  661 |  pci_set_master(dev->pdev);
  |  ^~~~
  |  dev
In file included from drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:31:
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:690:47: error: 'struct drm_device' has 
no member named 'pdev'; did you mean 'dev'?
  690 |  dev_priv->io_start = pci_resource_start(dev->pdev, 0);
  |   ^~~~
include/linux/pci.h:1854:40: note: in definition of macro 
'pci_resource_start'
 1854 | #define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start)
  |^~~
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:691:49: error: 'struct drm_device' has 
no member named 'pdev'; did you mean 'dev'?
  691 |  dev_priv->vram_start = pci_resource_start(dev->pdev, 1);
  | ^~~~
include/linux/pci.h:1854:40: note: in definition of macro 
'pci_resource_start'
 1854 | #define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start)
  |^~~
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:692:49: error: 'struct drm_device' has 
no member named 'pdev'; did you mean 'dev'?
  692 |  dev_priv->mmio_start = pci_resource_start(dev->pdev, 2);
  | ^~~~
include/linux/pci.h:1854:40: note: in definition of macro 
'pci_resource_start'
 1854 | #define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start)
  |^~~
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:842:33: error: 'struct drm_device' has 
no mem

RE: [PATCH 3/3] drm/amdgpu:Limit the resolution for virtual_display

2021-01-07 Thread Deng, Emily
[AMD Official Use Only - Internal Distribution Only]

>-Original Message-
>From: Michel Dänzer 
>Sent: Thursday, January 7, 2021 4:42 PM
>To: Deng, Emily ; Alex Deucher
>
>Cc: amd-gfx list 
>Subject: Re: [PATCH 3/3] drm/amdgpu:Limit the resolution for virtual_display
>
>On 2021-01-07 3:28 a.m., Deng, Emily wrote:
>>> From: Michel Dänzer  On 2021-01-06 11:40 a.m.,
>>> Deng, Emily wrote:
> From: Alex Deucher  On Tue, Jan 5, 2021 at
> 3:37 AM Emily.Deng  wrote:
>>
>> Limit the resolution not bigger than 16384, which means
>> dev->mode_info.num_crtc * common_modes[i].w not bigger than
>16384.
>>
>> Signed-off-by: Emily.Deng 
>> ---
>>drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 7 +--
>>1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>> b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>> index 2b16c8faca34..c23d37b02fd7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>> @@ -319,6 +319,7 @@ dce_virtual_encoder(struct drm_connector
>> *connector)  static int dce_virtual_get_modes(struct drm_connector
>> *connector)  {
>>   struct drm_device *dev = connector->dev;
>> +   struct amdgpu_device *adev = dev->dev_private;
>>   struct drm_display_mode *mode = NULL;
>>   unsigned i;
>>   static const struct mode_size { @@ -350,8 +351,10 @@
>> static int dce_virtual_get_modes(struct drm_connector *connector)
>>   };
>>
>>   for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
>> -   mode = drm_cvt_mode(dev, common_modes[i].w,
> common_modes[i].h, 60, false, false, false);
>> -   drm_mode_probed_add(connector, mode);
>> +   if (adev->mode_info.num_crtc <= 4 ||
>> + common_modes[i].w <= 2560) {
>
> You are also limiting the number of crtcs here.  Intended?  Won't
> this break 5 or 6 crtc configs?
>
> Alex
 Yes, it is intended,  for num_crtc bigger then 4, don't support
 resolution
>>> bigger then 2560, because of the max supported width is 16384 for xcb
>>> protocol.
>>>
>>> There's no such limitation with Wayland. I'd recommend against
>>> artificially imposing limits from X11 to the kernel.
>>>
>>>
>>> (As a side note, the X11 protocol limit should actually be 32768; the
>>> 16384 limit exposed in the RANDR extension comes from the kernel
>>> driver, specifically drmModeGetResources's max_width/height)
>> It is our test and debug result, that the follow variable only have 16bit. 
>> Will
>limit the resolution to 16384.
>> glamor_pixmap_from_fd(ScreenPtr screen,
>>int fd,
>>CARD16 width,
>>CARD16 height,
>>CARD16 stride, CARD8 depth, CARD8 bpp)
>
>I assume you're referring to the stride parameter, which is in bytes.
>
>This function is only used for pixmaps created from a dma-buf via DRI3.
>It does not limit the size of other pixmaps, so it does not limit the size of 
>the
>screen pixmap (which corresponds to the framebuffer size in the RANDR
>extension) in general.
>
>Also, this is an implementation detail, the limitation could be lifted by
>changing the type of the parameter (though that would be an ABI break for
>Xorg).
>
>Xwayland isn't affected by this:
>
>Screen 0: minimum 16 x 16, current 1920 x 1200, maximum 32767 x 32767
Yes, openGL driver will refer to the stride. As we have tried resolution bigger 
than 16384, the screen won't display well.
And seem no body has verified this. So we want to limit the max supported modes 
to not bigger than 16384.
>
>
>--
>Earthling Michel Dänzer   |
>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fredha
>t.com%2Fdata=04%7C01%7CEmily.Deng%40amd.com%7C6279eb2390
>0b436337b408d8b2e82635%7C3dd8961fe4884e608e11a82d994e183d%7C0%
>7C0%7C637456057455424961%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
>wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
>mp;sdata=u%2FuKB%2ByxJMzCr3nfU8rkFyjjI37gc%2BZ2rmHP9riZB5w%3D
>mp;reserved=0
>Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v3 8/8] drm: Upcast struct drm_device.dev to struct pci_device; replace pdev

2021-01-07 Thread kernel test robot
Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next linus/master v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Move-struct-drm_device-pdev-to-legacy/20210107-161007
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: microblaze-randconfig-r013-20210107 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/0day-ci/linux/commit/380912f7b62c23322562c40e19efd7ad84d57e9c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Thomas-Zimmermann/drm-Move-struct-drm_device-pdev-to-legacy/20210107-161007
git checkout 380912f7b62c23322562c40e19efd7ad84d57e9c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross 
ARCH=microblaze 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/virtio/virtgpu_drv.c: In function 'virtio_gpu_pci_quirk':
>> drivers/gpu/drm/virtio/virtgpu_drv.c:57:7: error: 'struct drm_device' has no 
>> member named 'pdev'; did you mean 'dev'?
  57 |  dev->pdev = pdev;
 |   ^~~~
 |   dev


vim +57 drivers/gpu/drm/virtio/virtgpu_drv.c

dc5698e80cf724 Dave Airlie 2013-09-09  46  
d516e75c71c985 Ezequiel Garcia 2019-01-08  47  static int 
virtio_gpu_pci_quirk(struct drm_device *dev, struct virtio_device *vdev)
d516e75c71c985 Ezequiel Garcia 2019-01-08  48  {
d516e75c71c985 Ezequiel Garcia 2019-01-08  49   struct pci_dev *pdev = 
to_pci_dev(vdev->dev.parent);
d516e75c71c985 Ezequiel Garcia 2019-01-08  50   const char *pname = 
dev_name(>dev);
d516e75c71c985 Ezequiel Garcia 2019-01-08  51   bool vga = (pdev->class >> 8) 
== PCI_CLASS_DISPLAY_VGA;
d516e75c71c985 Ezequiel Garcia 2019-01-08  52   char unique[20];
d516e75c71c985 Ezequiel Garcia 2019-01-08  53  
d516e75c71c985 Ezequiel Garcia 2019-01-08  54   DRM_INFO("pci: %s detected at 
%s\n",
d516e75c71c985 Ezequiel Garcia 2019-01-08  55vga ? "virtio-vga" : 
"virtio-gpu-pci",
d516e75c71c985 Ezequiel Garcia 2019-01-08  56pname);
d516e75c71c985 Ezequiel Garcia 2019-01-08 @57   dev->pdev = pdev;
d516e75c71c985 Ezequiel Garcia 2019-01-08  58   if (vga)
d516e75c71c985 Ezequiel Garcia 2019-01-08  59   
drm_fb_helper_remove_conflicting_pci_framebuffers(pdev,
d516e75c71c985 Ezequiel Garcia 2019-01-08  60   
  "virtiodrmfb");
d516e75c71c985 Ezequiel Garcia 2019-01-08  61  
d516e75c71c985 Ezequiel Garcia 2019-01-08  62   /*
d516e75c71c985 Ezequiel Garcia 2019-01-08  63* Normally the 
drm_dev_set_unique() call is done by core DRM.
d516e75c71c985 Ezequiel Garcia 2019-01-08  64* The following comment 
covers, why virtio cannot rely on it.
d516e75c71c985 Ezequiel Garcia 2019-01-08  65*
d516e75c71c985 Ezequiel Garcia 2019-01-08  66* Unlike the other virtual GPU 
drivers, virtio abstracts the
d516e75c71c985 Ezequiel Garcia 2019-01-08  67* underlying bus type by using 
struct virtio_device.
d516e75c71c985 Ezequiel Garcia 2019-01-08  68*
d516e75c71c985 Ezequiel Garcia 2019-01-08  69* Hence the dev_is_pci() 
check, used in core DRM, will fail
d516e75c71c985 Ezequiel Garcia 2019-01-08  70* and the unique returned will 
be the virtio_device "virtio0",
d516e75c71c985 Ezequiel Garcia 2019-01-08  71* while a "pci:..." one is 
required.
d516e75c71c985 Ezequiel Garcia 2019-01-08  72*
d516e75c71c985 Ezequiel Garcia 2019-01-08  73* A few other ideas were 
considered:
d516e75c71c985 Ezequiel Garcia 2019-01-08  74* - Extend the dev_is_pci() 
check [in drm_set_busid] to
d516e75c71c985 Ezequiel Garcia 2019-01-08  75*   consider virtio.
d516e75c71c985 Ezequiel Garcia 2019-01-08  76*   Seems like a bigger hack 
than what we have already.
d516e75c71c985 Ezequiel Garcia 2019-01-08  77*
d516e75c71c985 Ezequiel Garcia 2019-01-08  78* - Point drm_device::dev to 
the parent of the virtio_device
d516e75c71c985 Ezequiel Garcia 2019-01-08  79*   Semantic changes:
d516e75c71c985 Ezequiel Garcia 2019-01-08  80*   * Using the wrong device 
for i2c, framebuffer_alloc and
d516e75c71c985 Ezequiel Garcia 2019-01-08  81* prime import.
d516e75c71c985 Ezequiel Garcia 2019-01-08  82*   Visual changes:
d516e75c71c985 Ezequiel G

Re: [PATCH v3 8/8] drm: Upcast struct drm_device.dev to struct pci_device; replace pdev

2021-01-07 Thread kernel test robot
Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next linus/master v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Move-struct-drm_device-pdev-to-legacy/20210107-161007
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-s021-20210107 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-208-g46a52ca4-dirty
# 
https://github.com/0day-ci/linux/commit/380912f7b62c23322562c40e19efd7ad84d57e9c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Thomas-Zimmermann/drm-Move-struct-drm_device-pdev-to-legacy/20210107-161007
git checkout 380912f7b62c23322562c40e19efd7ad84d57e9c
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/gma500/oaktrail_device.c: In function 'oaktrail_chip_setup':
>> drivers/gpu/drm/gma500/oaktrail_device.c:509:26: error: 'struct drm_device' 
>> has no member named 'pdev'; did you mean 'dev'?
 509 |  if (pci_enable_msi(dev->pdev))
 |  ^~~~
 |  dev
--
   drivers/gpu/drm/gma500/oaktrail_lvds.c: In function 
'oaktrail_lvds_set_power':
>> drivers/gpu/drm/gma500/oaktrail_lvds.c:63:25: error: 'struct drm_device' has 
>> no member named 'pdev'; did you mean 'dev'?
  63 |   pm_request_idle(>pdev->dev);
 | ^~~~
 | dev
--
   drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c: In function 'get_clock':
   drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c:69:11: warning: variable 'tmp' 
set but not used [-Wunused-but-set-variable]
  69 |  u32 val, tmp;
 |   ^~~
   drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c: In function 'get_data':
   drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c:83:11: warning: variable 'tmp' 
set but not used [-Wunused-but-set-variable]
  83 |  u32 val, tmp;
 |   ^~~
   drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c: In function 
'oaktrail_lvds_i2c_init':
>> drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c:148:35: error: 'struct 
>> drm_device' has no member named 'pdev'; did you mean 'dev'?
 148 |  chan->adapter.dev.parent = >pdev->dev;
 |   ^~~~
 |   dev
--
   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c: In function 'vmw_driver_load':
>> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:661:22: error: 'struct drm_device' has 
>> no member named 'pdev'; did you mean 'dev'?
 661 |  pci_set_master(dev->pdev);
 |  ^~~~
 |  dev
   In file included from drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:31:
   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:690:47: error: 'struct drm_device' has 
no member named 'pdev'; did you mean 'dev'?
 690 |  dev_priv->io_start = pci_resource_start(dev->pdev, 0);
 |   ^~~~
   include/linux/pci.h:1854:40: note: in definition of macro 
'pci_resource_start'
1854 | #define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start)
 |^~~
   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:691:49: error: 'struct drm_device' has 
no member named 'pdev'; did you mean 'dev'?
 691 |  dev_priv->vram_start = pci_resource_start(dev->pdev, 1);
 | ^~~~
   include/linux/pci.h:1854:40: note: in definition of macro 
'pci_resource_start'
1854 | #define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start)
 |^~~
   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:692:49: error: 'struct drm_device' has 
no member named 'pdev'; did you mean 'dev'?
 692 |  dev_priv->mmio_start = pci_resource_start(dev->pdev, 2);
 | ^~~~
   include/linux/pci.h:1854:40: note: in definition of macro 
'pci_resource_start'
1854 | #define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start)
 |^~~
   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:842:33: error: 'struct drm_device' has 
no member named 'pdev'; did you mean 'dev'?
 842 |  ret = pci_request_regions(dev->pdev, "vmwgfx probe");
 

Re: [PATCH 00/35] Add HMM-based SVM memory manager to KFD

2021-01-07 Thread Daniel Vetter
On Wed, Jan 06, 2021 at 10:00:52PM -0500, Felix Kuehling wrote:
> This is the first version of our HMM based shared virtual memory manager
> for KFD. There are still a number of known issues that we're working through
> (see below). This will likely lead to some pretty significant changes in
> MMU notifier handling and locking on the migration code paths. So don't
> get hung up on those details yet.
> 
> But I think this is a good time to start getting feedback. We're pretty
> confident about the ioctl API, which is both simple and extensible for the
> future. (see patches 4,16) The user mode side of the API can be found here:
> https://github.com/RadeonOpenCompute/ROCT-Thunk-Interface/blob/fxkamd/hmm-wip/src/svm.c
> 
> I'd also like another pair of eyes on how we're interfacing with the GPU VM
> code in amdgpu_vm.c (see patches 12,13), retry page fault handling (24,25),
> and some retry IRQ handling changes (32).
> 
> 
> Known issues:
> * won't work with IOMMU enabled, we need to dma_map all pages properly
> * still working on some race conditions and random bugs
> * performance is not great yet

Still catching up, but I think there's another one for your list:

 * hmm gpu context preempt vs page fault handling. I've had a short
   discussion about this one with Christian before the holidays, and also
   some private chats with Jerome. It's nasty since no easy fix, much less
   a good idea what's the best approach here.

I'll try to look at this more in-depth when I'm catching up on mails.
-Daniel

> 
> Alex Sierra (12):
>   drm/amdgpu: replace per_device_list by array
>   drm/amdkfd: helper to convert gpu id and idx
>   drm/amdkfd: add xnack enabled flag to kfd_process
>   drm/amdkfd: add ioctl to configure and query xnack retries
>   drm/amdkfd: invalidate tables on page retry fault
>   drm/amdkfd: page table restore through svm API
>   drm/amdkfd: SVM API call to restore page tables
>   drm/amdkfd: add svm_bo reference for eviction fence
>   drm/amdgpu: add param bit flag to create SVM BOs
>   drm/amdkfd: add svm_bo eviction mechanism support
>   drm/amdgpu: svm bo enable_signal call condition
>   drm/amdgpu: add svm_bo eviction to enable_signal cb
> 
> Philip Yang (23):
>   drm/amdkfd: select kernel DEVICE_PRIVATE option
>   drm/amdkfd: add svm ioctl API
>   drm/amdkfd: Add SVM API support capability bits
>   drm/amdkfd: register svm range
>   drm/amdkfd: add svm ioctl GET_ATTR op
>   drm/amdgpu: add common HMM get pages function
>   drm/amdkfd: validate svm range system memory
>   drm/amdkfd: register overlap system memory range
>   drm/amdkfd: deregister svm range
>   drm/amdgpu: export vm update mapping interface
>   drm/amdkfd: map svm range to GPUs
>   drm/amdkfd: svm range eviction and restore
>   drm/amdkfd: register HMM device private zone
>   drm/amdkfd: validate vram svm range from TTM
>   drm/amdkfd: support xgmi same hive mapping
>   drm/amdkfd: copy memory through gart table
>   drm/amdkfd: HMM migrate ram to vram
>   drm/amdkfd: HMM migrate vram to ram
>   drm/amdgpu: reserve fence slot to update page table
>   drm/amdgpu: enable retry fault wptr overflow
>   drm/amdkfd: refine migration policy with xnack on
>   drm/amdkfd: add svm range validate timestamp
>   drm/amdkfd: multiple gpu migrate vram to vram
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h|4 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c  |   16 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   13 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c|   83 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h|7 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h|5 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |   90 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c|   47 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|   10 +
>  drivers/gpu/drm/amd/amdgpu/vega10_ih.c|   32 +-
>  drivers/gpu/drm/amd/amdgpu/vega20_ih.c|   32 +-
>  drivers/gpu/drm/amd/amdkfd/Kconfig|1 +
>  drivers/gpu/drm/amd/amdkfd/Makefile   |4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c  |  170 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_iommu.c|8 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c  |  866 ++
>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.h  |   59 +
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h |   52 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c  |  200 +-
>  .../amd/amdkfd/kfd_process_queue_manager.c|6 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c  | 2564 +
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.h  |  135 +
>  drivers/gpu/drm/amd/amdkfd/kfd_topology.c |1 +
>  drivers/gpu/drm/amd/amdkfd/kfd_topology.h |   10 +-
>  include/uapi/linux/kfd_ioctl.h|  169 +-
>  26 files changed, 4296 insertions(+), 291 deletions(-)
>  create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>  create mode 100644 

Re: [pull] amdgpu drm-fixes-5.11

2021-01-07 Thread Daniel Vetter
On Wed, Jan 06, 2021 at 05:27:21PM -0500, Alex Deucher wrote:
> Hi Dave, Daniel,
> 
> New URL.  FDO ran out of disk space, so I'm attempting to move to gitlab.
> Let me know if you run into any issues.

Worked fine. Did you puing linux-next to update your tree location? Also
legacy fd.o git seems back in shape, at least I can push.
-Daniel

> 
> Thanks
> 
> The following changes since commit 5b2fc08c455bbf749489254a81baeffdf4c0a693:
> 
>   Merge tag 'amd-drm-fixes-5.11-2020-12-23' of 
> git://people.freedesktop.org/~agd5f/linux into drm-next (2020-12-24 10:31:16 
> +1000)
> 
> are available in the Git repository at:
> 
>   https://gitlab.freedesktop.org/agd5f/linux.git 
> tags/amd-drm-fixes-5.11-2021-01-06
> 
> for you to fetch changes up to 5efc1f4b454c6179d35e7b0c3eda0ad5763a00fc:
> 
>   Revert "drm/amd/display: Fix memory leaks in S3 resume" (2021-01-06 
> 16:25:06 -0500)
> 
> 
> amd-drm-fixes-5.11-2021-01-06:
> 
> amdgpu:
> - Telemetry fix for VGH
> - Powerplay fixes for RV
> - Powerplay fixes for RN
> - RAS fixes for Sienna Cichlid
> - Blank screen regression fix
> - Drop DCN support for aarch64
> - Misc other fixes
> 
> 
> Alex Deucher (2):
>   drm/amdgpu/display: drop DCN support for aarch64
>   Revert "drm/amd/display: Fix memory leaks in S3 resume"
> 
> Arnd Bergmann (1):
>   drm/amd/display: Fix unused variable warning
> 
> Dennis Li (3):
>   drm/amdgpu: fix a memory protection fault when remove amdgpu device
>   drm/amdgpu: fix a GPU hang issue when remove device
>   drm/amdgpu: fix no bad_pages issue after umc ue injection
> 
> Hawking Zhang (1):
>   drm/amdgpu: switched to cached noretry setting for vangogh
> 
> Jiawei Gu (1):
>   drm/amdgpu: fix potential memory leak during navi12 deinitialization
> 
> John Clements (2):
>   drm/amd/pm: updated PM to I2C controller port on sienna cichlid
>   drm/amdgpu: enable ras eeprom support for sienna cichlid
> 
> Kevin Wang (1):
>   drm/amd/display: fix sysfs amdgpu_current_backlight_pwm NULL pointer 
> issue
> 
> Xiaojian Du (4):
>   drm/amd/pm: correct the sensor value of power for vangogh
>   drm/amd/pm: improve the fine grain tuning function for RV/RV2/PCO
>   drm/amd/pm: fix the failure when change power profile for renoir
>   drm/amd/pm: improve the fine grain tuning function for RV/RV2/PCO
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   4 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c|  25 +++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c|   8 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c |   8 +-
>  drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c|   2 +-
>  drivers/gpu/drm/amd/display/Kconfig|   2 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |   7 +-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h  |   2 +-
>  drivers/gpu/drm/amd/display/dc/calcs/Makefile  |   4 -
>  drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile|  21 ---
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c  |   7 +-
>  drivers/gpu/drm/amd/display/dc/dcn10/Makefile  |   7 -
>  .../gpu/drm/amd/display/dc/dcn10/dcn10_resource.c  |   7 -
>  drivers/gpu/drm/amd/display/dc/dcn20/Makefile  |   4 -
>  drivers/gpu/drm/amd/display/dc/dcn21/Makefile  |   4 -
>  drivers/gpu/drm/amd/display/dc/dcn30/Makefile  |   5 -
>  drivers/gpu/drm/amd/display/dc/dcn301/Makefile |   4 -
>  drivers/gpu/drm/amd/display/dc/dcn302/Makefile |   4 -
>  drivers/gpu/drm/amd/display/dc/dml/Makefile|   4 -
>  drivers/gpu/drm/amd/display/dc/dsc/Makefile|   4 -
>  drivers/gpu/drm/amd/display/dc/os_types.h  |   4 -
>  .../gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c   | 166 
> +++--
>  .../gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h   |   3 +
>  .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c|   2 +-
>  drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c   |   3 +-
>  drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c|   1 +
>  drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c |   1 +
>  27 files changed, 200 insertions(+), 113 deletions(-)

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 3/3] drm/amdgpu:Limit the resolution for virtual_display

2021-01-07 Thread Michel Dänzer

On 2021-01-07 3:28 a.m., Deng, Emily wrote:

From: Michel Dänzer 
On 2021-01-06 11:40 a.m., Deng, Emily wrote:

From: Alex Deucher  On Tue, Jan 5, 2021 at
3:37 AM Emily.Deng  wrote:


Limit the resolution not bigger than 16384, which means
dev->mode_info.num_crtc * common_modes[i].w not bigger than 16384.

Signed-off-by: Emily.Deng 
---
   drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 7 +--
   1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 2b16c8faca34..c23d37b02fd7 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -319,6 +319,7 @@ dce_virtual_encoder(struct drm_connector
*connector)  static int dce_virtual_get_modes(struct drm_connector
*connector)  {
  struct drm_device *dev = connector->dev;
+   struct amdgpu_device *adev = dev->dev_private;
  struct drm_display_mode *mode = NULL;
  unsigned i;
  static const struct mode_size { @@ -350,8 +351,10 @@ static
int dce_virtual_get_modes(struct drm_connector *connector)
  };

  for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
-   mode = drm_cvt_mode(dev, common_modes[i].w,

common_modes[i].h, 60, false, false, false);

-   drm_mode_probed_add(connector, mode);
+   if (adev->mode_info.num_crtc <= 4 ||
+ common_modes[i].w <= 2560) {


You are also limiting the number of crtcs here.  Intended?  Won't
this break 5 or 6 crtc configs?

Alex

Yes, it is intended,  for num_crtc bigger then 4, don't support resolution

bigger then 2560, because of the max supported width is 16384 for xcb
protocol.

There's no such limitation with Wayland. I'd recommend against artificially
imposing limits from X11 to the kernel.


(As a side note, the X11 protocol limit should actually be 32768; the
16384 limit exposed in the RANDR extension comes from the kernel driver,
specifically drmModeGetResources's max_width/height)

It is our test and debug result, that the follow variable only have 16bit. Will 
limit the resolution to 16384.
glamor_pixmap_from_fd(ScreenPtr screen,
   int fd,
   CARD16 width,
   CARD16 height,
   CARD16 stride, CARD8 depth, CARD8 bpp)


I assume you're referring to the stride parameter, which is in bytes.

This function is only used for pixmaps created from a dma-buf via DRI3. 
It does not limit the size of other pixmaps, so it does not limit the 
size of the screen pixmap (which corresponds to the framebuffer size in 
the RANDR extension) in general.


Also, this is an implementation detail, the limitation could be lifted 
by changing the type of the parameter (though that would be an ABI break 
for Xorg).


Xwayland isn't affected by this:

Screen 0: minimum 16 x 16, current 1920 x 1200, maximum 32767 x 32767


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


[PATCH v3 8/8] drm: Upcast struct drm_device.dev to struct pci_device; replace pdev

2021-01-07 Thread Thomas Zimmermann
We have DRM drivers based on USB, SPI and platform devices. All of them
are fine with storing their device reference in struct drm_device.dev.
PCI devices should be no exception. Therefore struct drm_device.pdev is
deprecated.

Instead upcast from struct drm_device.dev with to_pci_dev(). PCI-specific
code can use dev_is_pci() to test for a PCI device. This patch changes
the DRM core code and documentation accordingly. Struct drm_device.pdev
is being moved to legacy status.

Signed-off-by: Thomas Zimmermann 
Acked-by: Sam Ravnborg 
---
 drivers/gpu/drm/drm_agpsupport.c |  9 ++---
 drivers/gpu/drm/drm_bufs.c   |  4 ++--
 drivers/gpu/drm/drm_edid.c   |  7 ++-
 drivers/gpu/drm/drm_irq.c| 12 +++-
 drivers/gpu/drm/drm_pci.c| 26 +++---
 drivers/gpu/drm/drm_vm.c |  2 +-
 include/drm/drm_device.h | 12 +---
 7 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_agpsupport.c b/drivers/gpu/drm/drm_agpsupport.c
index 4c7ad46fdd21..a4040fe4f4ba 100644
--- a/drivers/gpu/drm/drm_agpsupport.c
+++ b/drivers/gpu/drm/drm_agpsupport.c
@@ -103,11 +103,13 @@ int drm_agp_info_ioctl(struct drm_device *dev, void *data,
  */
 int drm_agp_acquire(struct drm_device *dev)
 {
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
+
if (!dev->agp)
return -ENODEV;
if (dev->agp->acquired)
return -EBUSY;
-   dev->agp->bridge = agp_backend_acquire(dev->pdev);
+   dev->agp->bridge = agp_backend_acquire(pdev);
if (!dev->agp->bridge)
return -ENODEV;
dev->agp->acquired = 1;
@@ -402,14 +404,15 @@ int drm_agp_free_ioctl(struct drm_device *dev, void *data,
  */
 struct drm_agp_head *drm_agp_init(struct drm_device *dev)
 {
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
struct drm_agp_head *head = NULL;
 
head = kzalloc(sizeof(*head), GFP_KERNEL);
if (!head)
return NULL;
-   head->bridge = agp_find_bridge(dev->pdev);
+   head->bridge = agp_find_bridge(pdev);
if (!head->bridge) {
-   head->bridge = agp_backend_acquire(dev->pdev);
+   head->bridge = agp_backend_acquire(pdev);
if (!head->bridge) {
kfree(head);
return NULL;
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index aeb1327e3077..e3d77dfefb0a 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -326,7 +326,7 @@ static int drm_addmap_core(struct drm_device *dev, 
resource_size_t offset,
 * As we're limiting the address to 2^32-1 (or less),
 * casting it down to 32 bits is no problem, but we
 * need to point to a 64bit variable first. */
-   map->handle = dma_alloc_coherent(>pdev->dev,
+   map->handle = dma_alloc_coherent(dev->dev,
 map->size,
 >offset,
 GFP_KERNEL);
@@ -556,7 +556,7 @@ int drm_legacy_rmmap_locked(struct drm_device *dev, struct 
drm_local_map *map)
case _DRM_SCATTER_GATHER:
break;
case _DRM_CONSISTENT:
-   dma_free_coherent(>pdev->dev,
+   dma_free_coherent(dev->dev,
  map->size,
  map->handle,
  map->offset);
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 394cc55b3214..c2bbe7bee7b6 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -2075,9 +2076,13 @@ EXPORT_SYMBOL(drm_get_edid);
 struct edid *drm_get_edid_switcheroo(struct drm_connector *connector,
 struct i2c_adapter *adapter)
 {
-   struct pci_dev *pdev = connector->dev->pdev;
+   struct drm_device *dev = connector->dev;
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
struct edid *edid;
 
+   if (drm_WARN_ON_ONCE(dev, !dev_is_pci(dev->dev)))
+   return NULL;
+
vga_switcheroo_lock_ddc(pdev);
edid = drm_get_edid(connector, adapter);
vga_switcheroo_unlock_ddc(pdev);
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 803af4bbd214..c3bd664ea733 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -122,7 +122,7 @@ int drm_irq_install(struct drm_device *dev, int irq)
dev->driver->irq_preinstall(dev);
 
/* PCI devices require shared interrupts. */
-   if (dev->pdev)
+   if (dev_is_pci(dev->dev))
sh_flags = IRQF_SHARED;
 
ret = request_irq(irq, dev->driver->irq_handler,
@@ -140,7 +140,7 @@ int drm_irq_install(struct drm_device *dev, int irq)
if 

[PATCH v3 7/8] drm/nouveau: Remove references to struct drm_device.pdev

2021-01-07 Thread Thomas Zimmermann
Using struct drm_device.pdev is deprecated. Convert nouveau to struct
drm_device.dev. No functional changes.

v3:
* fix nv04_dfp_update_backlight() as well (Jeremy)

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Jeremy Cline 
Cc: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv04/arb.c  | 12 +++-
 drivers/gpu/drm/nouveau/dispnv04/dfp.c  |  5 +++--
 drivers/gpu/drm/nouveau/dispnv04/disp.h | 14 --
 drivers/gpu/drm/nouveau/dispnv04/hw.c   | 10 ++
 drivers/gpu/drm/nouveau/nouveau_abi16.c |  7 ---
 drivers/gpu/drm/nouveau/nouveau_acpi.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_bios.c  | 11 ---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 10 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  5 ++---
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |  6 --
 drivers/gpu/drm/nouveau/nouveau_vga.c   | 20 
 11 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/arb.c 
b/drivers/gpu/drm/nouveau/dispnv04/arb.c
index 9d4a2d97507e..1d3542d6006b 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/arb.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/arb.c
@@ -200,16 +200,17 @@ nv04_update_arb(struct drm_device *dev, int VClk, int bpp,
int MClk = nouveau_hw_get_clock(dev, PLL_MEMORY);
int NVClk = nouveau_hw_get_clock(dev, PLL_CORE);
uint32_t cfg1 = nvif_rd32(device, NV04_PFB_CFG1);
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
 
sim_data.pclk_khz = VClk;
sim_data.mclk_khz = MClk;
sim_data.nvclk_khz = NVClk;
sim_data.bpp = bpp;
sim_data.two_heads = nv_two_heads(dev);
-   if ((dev->pdev->device & 0x) == 0x01a0 /*CHIPSET_NFORCE*/ ||
-   (dev->pdev->device & 0x) == 0x01f0 /*CHIPSET_NFORCE2*/) {
+   if ((pdev->device & 0x) == 0x01a0 /*CHIPSET_NFORCE*/ ||
+   (pdev->device & 0x) == 0x01f0 /*CHIPSET_NFORCE2*/) {
uint32_t type;
-   int domain = pci_domain_nr(dev->pdev->bus);
+   int domain = pci_domain_nr(pdev->bus);
 
pci_read_config_dword(pci_get_domain_bus_and_slot(domain, 0, 1),
  0x7c, );
@@ -251,11 +252,12 @@ void
 nouveau_calc_arb(struct drm_device *dev, int vclk, int bpp, int *burst, int 
*lwm)
 {
struct nouveau_drm *drm = nouveau_drm(dev);
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
 
if (drm->client.device.info.family < NV_DEVICE_INFO_V0_KELVIN)
nv04_update_arb(dev, vclk, bpp, burst, lwm);
-   else if ((dev->pdev->device & 0xfff0) == 0x0240 /*CHIPSET_C51*/ ||
-(dev->pdev->device & 0xfff0) == 0x03d0 /*CHIPSET_C512*/) {
+   else if ((pdev->device & 0xfff0) == 0x0240 /*CHIPSET_C51*/ ||
+(pdev->device & 0xfff0) == 0x03d0 /*CHIPSET_C512*/) {
*burst = 128;
*lwm = 0x0480;
} else
diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c 
b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
index 42687ea2a4ca..ce3d8c6ef000 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
@@ -488,12 +488,13 @@ static void nv04_dfp_update_backlight(struct drm_encoder 
*encoder, int mode)
 #ifdef __powerpc__
struct drm_device *dev = encoder->dev;
struct nvif_object *device = _drm(dev)->client.device.object;
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
 
/* BIOS scripts usually take care of the backlight, thanks
 * Apple for your consistency.
 */
-   if (dev->pdev->device == 0x0174 || dev->pdev->device == 0x0179 ||
-   dev->pdev->device == 0x0189 || dev->pdev->device == 0x0329) {
+   if (pdev->device == 0x0174 || pdev->device == 0x0179 ||
+   pdev->device == 0x0189 || pdev->device == 0x0329) {
if (mode == DRM_MODE_DPMS_ON) {
nvif_mask(device, NV_PBUS_DEBUG_DUALHEAD_CTL, 1 << 31, 
1 << 31);
nvif_mask(device, NV_PCRTC_GPIO_EXT, 3, 1);
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.h 
b/drivers/gpu/drm/nouveau/dispnv04/disp.h
index 5ace5e906949..f0a24126641a 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.h
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.h
@@ -130,7 +130,7 @@ static inline bool
 nv_two_heads(struct drm_device *dev)
 {
struct nouveau_drm *drm = nouveau_drm(dev);
-   const int impl = dev->pdev->device & 0x0ff0;
+   const int impl = to_pci_dev(dev->dev)->device & 0x0ff0;
 
if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_CELSIUS && impl 
!= 0x0100 &&
impl != 0x0150 && impl != 0x01a0 && impl != 0x0200)
@@ -142,14 +142,14 @@ nv_two_heads(struct drm_device *dev)
 static inline bool
 nv_gf4_disp_arch(struct drm_device *dev)
 {
-   return nv_two_heads(dev) && (dev->pdev->device & 0x0ff0) != 0x0110;
+   return nv_two_heads(dev) && (to_pci_dev(dev->dev)->device & 0x0ff0) != 

[PATCH v3 6/8] drm/i915/gvt: Remove references to struct drm_device.pdev

2021-01-07 Thread Thomas Zimmermann
Using struct drm_device.pdev is deprecated. Convert i915 to struct
drm_device.dev. No functional changes.

Signed-off-by: Thomas Zimmermann 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/gvt/cfg_space.c |  5 +++--
 drivers/gpu/drm/i915/gvt/firmware.c  | 10 +-
 drivers/gpu/drm/i915/gvt/gtt.c   | 12 ++--
 drivers/gpu/drm/i915/gvt/gvt.c   |  6 +++---
 drivers/gpu/drm/i915/gvt/kvmgt.c |  4 ++--
 5 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/cfg_space.c 
b/drivers/gpu/drm/i915/gvt/cfg_space.c
index ad86c5eb5bba..b490e3db2e38 100644
--- a/drivers/gpu/drm/i915/gvt/cfg_space.c
+++ b/drivers/gpu/drm/i915/gvt/cfg_space.c
@@ -374,6 +374,7 @@ void intel_vgpu_init_cfg_space(struct intel_vgpu *vgpu,
   bool primary)
 {
struct intel_gvt *gvt = vgpu->gvt;
+   struct pci_dev *pdev = to_pci_dev(gvt->gt->i915->drm.dev);
const struct intel_gvt_device_info *info = >device_info;
u16 *gmch_ctl;
u8 next;
@@ -407,9 +408,9 @@ void intel_vgpu_init_cfg_space(struct intel_vgpu *vgpu,
memset(vgpu_cfg_space(vgpu) + INTEL_GVT_PCI_OPREGION, 0, 4);
 
vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_GTTMMIO].size =
-   pci_resource_len(gvt->gt->i915->drm.pdev, 0);
+   pci_resource_len(pdev, 0);
vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_APERTURE].size =
-   pci_resource_len(gvt->gt->i915->drm.pdev, 2);
+   pci_resource_len(pdev, 2);
 
memset(vgpu_cfg_space(vgpu) + PCI_ROM_ADDRESS, 0, 4);
 
diff --git a/drivers/gpu/drm/i915/gvt/firmware.c 
b/drivers/gpu/drm/i915/gvt/firmware.c
index 990a181094e3..1a8274a3f4b1 100644
--- a/drivers/gpu/drm/i915/gvt/firmware.c
+++ b/drivers/gpu/drm/i915/gvt/firmware.c
@@ -76,7 +76,7 @@ static int mmio_snapshot_handler(struct intel_gvt *gvt, u32 
offset, void *data)
 static int expose_firmware_sysfs(struct intel_gvt *gvt)
 {
struct intel_gvt_device_info *info = >device_info;
-   struct pci_dev *pdev = gvt->gt->i915->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(gvt->gt->i915->drm.dev);
struct gvt_firmware_header *h;
void *firmware;
void *p;
@@ -127,7 +127,7 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt)
 
 static void clean_firmware_sysfs(struct intel_gvt *gvt)
 {
-   struct pci_dev *pdev = gvt->gt->i915->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(gvt->gt->i915->drm.dev);
 
device_remove_bin_file(>dev, _attr);
vfree(firmware_attr.private);
@@ -151,7 +151,7 @@ static int verify_firmware(struct intel_gvt *gvt,
   const struct firmware *fw)
 {
struct intel_gvt_device_info *info = >device_info;
-   struct pci_dev *pdev = gvt->gt->i915->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(gvt->gt->i915->drm.dev);
struct gvt_firmware_header *h;
unsigned long id, crc32_start;
const void *mem;
@@ -205,7 +205,7 @@ static int verify_firmware(struct intel_gvt *gvt,
 int intel_gvt_load_firmware(struct intel_gvt *gvt)
 {
struct intel_gvt_device_info *info = >device_info;
-   struct pci_dev *pdev = gvt->gt->i915->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(gvt->gt->i915->drm.dev);
struct intel_gvt_firmware *firmware = >firmware;
struct gvt_firmware_header *h;
const struct firmware *fw;
@@ -240,7 +240,7 @@ int intel_gvt_load_firmware(struct intel_gvt *gvt)
 
gvt_dbg_core("request hw state firmware %s...\n", path);
 
-   ret = request_firmware(, path, >gt->i915->drm.pdev->dev);
+   ret = request_firmware(, path, gvt->gt->i915->drm.dev);
kfree(path);
 
if (ret)
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 897c007ea96a..6d12a5a401f6 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -746,7 +746,7 @@ static int detach_oos_page(struct intel_vgpu *vgpu,
 
 static void ppgtt_free_spt(struct intel_vgpu_ppgtt_spt *spt)
 {
-   struct device *kdev = >vgpu->gvt->gt->i915->drm.pdev->dev;
+   struct device *kdev = spt->vgpu->gvt->gt->i915->drm.dev;
 
trace_spt_free(spt->vgpu->id, spt, spt->guest_page.type);
 
@@ -831,7 +831,7 @@ static int reclaim_one_ppgtt_mm(struct intel_gvt *gvt);
 static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt(
struct intel_vgpu *vgpu, enum intel_gvt_gtt_type type)
 {
-   struct device *kdev = >gvt->gt->i915->drm.pdev->dev;
+   struct device *kdev = vgpu->gvt->gt->i915->drm.dev;
struct intel_vgpu_ppgtt_spt *spt = NULL;
dma_addr_t daddr;
int ret;
@@ -2402,7 +2402,7 @@ static int alloc_scratch_pages(struct intel_vgpu *vgpu,
vgpu->gvt->device_info.gtt_entry_size_shift;
void *scratch_pt;
int i;
-   struct device *dev = >gvt->gt->i915->drm.pdev->dev;
+   struct device *dev = 

[PATCH v3 4/8] drm/i915: Remove references to struct drm_device.pdev

2021-01-07 Thread Thomas Zimmermann
Using struct drm_device.pdev is deprecated. Convert i915 to struct
drm_device.dev. No functional changes.

v3:
* rebased
v2:
* move gt/ and gvt/ changes into separate patches

Signed-off-by: Thomas Zimmermann 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/display/intel_bios.c |  2 +-
 drivers/gpu/drm/i915/display/intel_cdclk.c| 14 ++---
 drivers/gpu/drm/i915/display/intel_csr.c  |  2 +-
 drivers/gpu/drm/i915/display/intel_dsi_vbt.c  |  2 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c|  2 +-
 drivers/gpu/drm/i915/display/intel_gmbus.c|  2 +-
 .../gpu/drm/i915/display/intel_lpe_audio.c|  5 +++--
 drivers/gpu/drm/i915/display/intel_opregion.c |  6 +++---
 drivers/gpu/drm/i915/display/intel_overlay.c  |  2 +-
 drivers/gpu/drm/i915/display/intel_panel.c|  4 ++--
 drivers/gpu/drm/i915/display/intel_quirks.c   |  2 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c |  2 +-
 drivers/gpu/drm/i915/display/intel_vga.c  |  8 
 drivers/gpu/drm/i915/gem/i915_gem_phys.c  |  6 +++---
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c |  2 +-
 drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
 drivers/gpu/drm/i915/i915_drv.c   | 20 +--
 drivers/gpu/drm/i915/i915_drv.h   |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c   |  5 ++---
 drivers/gpu/drm/i915/i915_getparam.c  |  5 +++--
 drivers/gpu/drm/i915/i915_gpu_error.c |  2 +-
 drivers/gpu/drm/i915/i915_irq.c   |  6 +++---
 drivers/gpu/drm/i915/i915_pmu.c   |  2 +-
 drivers/gpu/drm/i915/i915_suspend.c   |  4 ++--
 drivers/gpu/drm/i915/i915_switcheroo.c|  4 ++--
 drivers/gpu/drm/i915/i915_vgpu.c  |  2 +-
 drivers/gpu/drm/i915/intel_device_info.c  |  2 +-
 drivers/gpu/drm/i915/intel_region_lmem.c  |  8 
 drivers/gpu/drm/i915/intel_runtime_pm.c   |  2 +-
 drivers/gpu/drm/i915/intel_uncore.c   |  4 ++--
 .../gpu/drm/i915/selftests/mock_gem_device.c  |  1 -
 drivers/gpu/drm/i915/selftests/mock_gtt.c |  2 +-
 32 files changed, 66 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
b/drivers/gpu/drm/i915/display/intel_bios.c
index 06c3310446a2..6144872cf3aa 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -2088,7 +2088,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size)
 
 static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
 {
-   struct pci_dev *pdev = dev_priv->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
void __iomem *p = NULL, *oprom;
struct vbt_header *vbt;
u16 vbt_size;
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c 
b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 2e878cc274b7..bf83e9e75227 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -96,7 +96,7 @@ static void fixed_450mhz_get_cdclk(struct drm_i915_private 
*dev_priv,
 static void i85x_get_cdclk(struct drm_i915_private *dev_priv,
   struct intel_cdclk_config *cdclk_config)
 {
-   struct pci_dev *pdev = dev_priv->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
u16 hpllcc = 0;
 
/*
@@ -138,7 +138,7 @@ static void i85x_get_cdclk(struct drm_i915_private 
*dev_priv,
 static void i915gm_get_cdclk(struct drm_i915_private *dev_priv,
 struct intel_cdclk_config *cdclk_config)
 {
-   struct pci_dev *pdev = dev_priv->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
u16 gcfgc = 0;
 
pci_read_config_word(pdev, GCFGC, );
@@ -162,7 +162,7 @@ static void i915gm_get_cdclk(struct drm_i915_private 
*dev_priv,
 static void i945gm_get_cdclk(struct drm_i915_private *dev_priv,
 struct intel_cdclk_config *cdclk_config)
 {
-   struct pci_dev *pdev = dev_priv->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
u16 gcfgc = 0;
 
pci_read_config_word(pdev, GCFGC, );
@@ -256,7 +256,7 @@ static unsigned int intel_hpll_vco(struct drm_i915_private 
*dev_priv)
 static void g33_get_cdclk(struct drm_i915_private *dev_priv,
  struct intel_cdclk_config *cdclk_config)
 {
-   struct pci_dev *pdev = dev_priv->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
static const u8 div_3200[] = { 12, 10,  8,  7, 5, 16 };
static const u8 div_4000[] = { 14, 12, 10,  8, 6, 20 };
static const u8 div_4800[] = { 20, 14, 12, 10, 8, 24 };
@@ -305,7 +305,7 @@ static void g33_get_cdclk(struct drm_i915_private *dev_priv,
 static void pnv_get_cdclk(struct drm_i915_private *dev_priv,
  struct intel_cdclk_config *cdclk_config)
 {
-   struct pci_dev *pdev = dev_priv->drm.pdev;
+   

[PATCH v3 3/8] drm/hibmc: Remove references to struct drm_device.pdev

2021-01-07 Thread Thomas Zimmermann
Using struct drm_device.pdev is deprecated. Convert hibmc to struct
drm_device.dev. No functional changes.

v3:
* rebased

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Tian Tao 
Acked-by: Sam Ravnborg 
Cc: Xinliang Liu 
Cc: Tian Tao  
Cc: John Stultz 
Cc: Xinwei Kong 
Cc: Chen Feng 
---
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 13 ++--
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c   |  2 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c   | 61 +++
 3 files changed, 68 insertions(+), 8 deletions(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c 
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 0d4e9023f54d..0a2edc7b754a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -210,8 +210,8 @@ static void hibmc_hw_config(struct hibmc_drm_private *priv)
 
 static int hibmc_hw_map(struct hibmc_drm_private *priv)
 {
-   struct drm_device *dev = >dev;
struct pci_dev *pdev = dev->pdev;
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
resource_size_t addr, size, ioaddr, iosize;
 
ioaddr = pci_resource_start(pdev, 1);
@@ -255,13 +255,14 @@ static int hibmc_unload(struct drm_device *dev)
if (dev->irq_enabled)
drm_irq_uninstall(dev);
 
-   pci_disable_msi(dev->pdev);
+   pci_disable_msi(to_pci_dev(dev->dev));
 
return 0;
 }
 
 static int hibmc_load(struct drm_device *dev)
 {
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
struct hibmc_drm_private *priv = to_hibmc_drm_private(dev);
int ret;
 
@@ -269,8 +270,7 @@ static int hibmc_load(struct drm_device *dev)
if (ret)
goto err;
 
-   ret = drmm_vram_helper_init(dev, pci_resource_start(dev->pdev, 0),
-   priv->fb_size);
+   ret = drmm_vram_helper_init(dev, pci_resource_start(pdev, 0), 
priv->fb_size);
if (ret) {
drm_err(dev, "Error initializing VRAM MM; %d\n", ret);
goto err;
@@ -286,11 +286,11 @@ static int hibmc_load(struct drm_device *dev)
goto err;
}
 
-   ret = pci_enable_msi(dev->pdev);
+   ret = pci_enable_msi(pdev);
if (ret) {
drm_warn(dev, "enabling MSI failed: %d\n", ret);
} else {
-   ret = drm_irq_install(dev, dev->pdev->irq);
+   ret = drm_irq_install(dev, pdev->irq);
if (ret)
drm_warn(dev, "install irq failed: %d\n", ret);
}
@@ -326,7 +326,6 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
}
 
dev = >dev;
-   dev->pdev = pdev;
pci_set_drvdata(pdev, dev);
 
ret = pcim_enable_device(pdev);
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c 
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
index 86d712090d87..410bd019bb35 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
@@ -83,7 +83,7 @@ int hibmc_ddc_create(struct drm_device *drm_dev,
connector->adapter.owner = THIS_MODULE;
connector->adapter.class = I2C_CLASS_DDC;
snprintf(connector->adapter.name, I2C_NAME_SIZE, "HIS i2c bit bus");
-   connector->adapter.dev.parent = _dev->pdev->dev;
+   connector->adapter.dev.parent = drm_dev->dev;
i2c_set_adapdata(>adapter, connector);
connector->adapter.algo_data = >bit_data;
 
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c 
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
new file mode 100644
index ..77f075075db2
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Hisilicon Hibmc SoC drm driver
+ *
+ * Based on the bochs drm driver.
+ *
+ * Copyright (c) 2016 Huawei Limited.
+ *
+ * Author:
+ * Rongrong Zou 
+ * Rongrong Zou 
+ * Jianhua Li 
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "hibmc_drm_drv.h"
+
+int hibmc_mm_init(struct hibmc_drm_private *hibmc)
+{
+   struct drm_vram_mm *vmm;
+   int ret;
+   struct drm_device *dev = hibmc->dev;
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
+
+   vmm = drm_vram_helper_alloc_mm(dev, pci_resource_start(pdev, 0),
+  hibmc->fb_size);
+   if (IS_ERR(vmm)) {
+   ret = PTR_ERR(vmm);
+   drm_err(dev, "Error initializing VRAM MM; %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+void hibmc_mm_fini(struct hibmc_drm_private *hibmc)
+{
+   if (!hibmc->dev->vram_mm)
+   return;
+
+   drm_vram_helper_release_mm(hibmc->dev);
+}
+
+int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
+ struct drm_mode_create_dumb *args)
+{
+   return 

[PATCH v3 5/8] drm/i915/gt: Remove references to struct drm_device.pdev

2021-01-07 Thread Thomas Zimmermann
Using struct drm_device.pdev is deprecated. Convert i915 to struct
drm_device.dev. No functional changes.

Signed-off-by: Thomas Zimmermann 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c  | 10 +-
 drivers/gpu/drm/i915/gt/intel_ppgtt.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_rc6.c   |  4 ++--
 drivers/gpu/drm/i915/gt/intel_reset.c |  6 +++---
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 1847d3c2ea99..a1e872ecc3f1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1252,7 +1252,7 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
 
/* Waiting to drain ELSP? */
if (execlists_active(>execlists)) {
-   synchronize_hardirq(engine->i915->drm.pdev->irq);
+   synchronize_hardirq(to_pci_dev(engine->i915->drm.dev)->irq);
 
intel_engine_flush_submission(engine);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index eece0844fbe9..fd6c8fa54812 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -769,7 +769,7 @@ static unsigned int chv_get_total_gtt_size(u16 gmch_ctrl)
 static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
 {
struct drm_i915_private *i915 = ggtt->vm.i915;
-   struct pci_dev *pdev = i915->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
phys_addr_t phys_addr;
int ret;
 
@@ -839,7 +839,7 @@ static struct resource pci_resource(struct pci_dev *pdev, 
int bar)
 static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 {
struct drm_i915_private *i915 = ggtt->vm.i915;
-   struct pci_dev *pdev = i915->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
unsigned int size;
u16 snb_gmch_ctl;
 
@@ -983,7 +983,7 @@ static u64 iris_pte_encode(dma_addr_t addr,
 static int gen6_gmch_probe(struct i915_ggtt *ggtt)
 {
struct drm_i915_private *i915 = ggtt->vm.i915;
-   struct pci_dev *pdev = i915->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
unsigned int size;
u16 snb_gmch_ctl;
 
@@ -1046,7 +1046,7 @@ static int i915_gmch_probe(struct i915_ggtt *ggtt)
phys_addr_t gmadr_base;
int ret;
 
-   ret = intel_gmch_probe(i915->bridge_dev, i915->drm.pdev, NULL);
+   ret = intel_gmch_probe(i915->bridge_dev, to_pci_dev(i915->drm.dev), 
NULL);
if (!ret) {
drm_err(>drm, "failed to set up gmch\n");
return -EIO;
@@ -1091,7 +1091,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct 
intel_gt *gt)
 
ggtt->vm.gt = gt;
ggtt->vm.i915 = i915;
-   ggtt->vm.dma = >drm.pdev->dev;
+   ggtt->vm.dma = i915->drm.dev;
 
if (INTEL_GEN(i915) <= 5)
ret = i915_gmch_probe(ggtt);
diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c 
b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
index 46d9aceda64c..01b7d08532f2 100644
--- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
@@ -301,7 +301,7 @@ void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt 
*gt)
 
ppgtt->vm.gt = gt;
ppgtt->vm.i915 = i915;
-   ppgtt->vm.dma = >drm.pdev->dev;
+   ppgtt->vm.dma = i915->drm.dev;
ppgtt->vm.total = BIT_ULL(INTEL_INFO(i915)->ppgtt_size);
 
i915_address_space_init(>vm, VM_CLASS_PPGTT);
diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c 
b/drivers/gpu/drm/i915/gt/intel_rc6.c
index d7b8e4457fc2..cce53fb9589c 100644
--- a/drivers/gpu/drm/i915/gt/intel_rc6.c
+++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
@@ -485,14 +485,14 @@ static bool rc6_supported(struct intel_rc6 *rc6)
 static void rpm_get(struct intel_rc6 *rc6)
 {
GEM_BUG_ON(rc6->wakeref);
-   pm_runtime_get_sync(_to_i915(rc6)->drm.pdev->dev);
+   pm_runtime_get_sync(rc6_to_i915(rc6)->drm.dev);
rc6->wakeref = true;
 }
 
 static void rpm_put(struct intel_rc6 *rc6)
 {
GEM_BUG_ON(!rc6->wakeref);
-   pm_runtime_put(_to_i915(rc6)->drm.pdev->dev);
+   pm_runtime_put(rc6_to_i915(rc6)->drm.dev);
rc6->wakeref = false;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c 
b/drivers/gpu/drm/i915/gt/intel_reset.c
index 761b50eca33b..fa8f1e98dd0a 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -179,7 +179,7 @@ static int i915_do_reset(struct intel_gt *gt,
 intel_engine_mask_t engine_mask,
 unsigned int retry)
 {
-   struct pci_dev *pdev = gt->i915->drm.pdev;
+   struct pci_dev *pdev = to_pci_dev(gt->i915->drm.dev);
int err;
 
/* Assert reset for at least 20 usec, and wait for acknowledgement. */
@@ -208,7 

[PATCH v3 0/8] drm: Move struct drm_device.pdev to legacy

2021-01-07 Thread Thomas Zimmermann
I merged many of the patches that were ready in v2 into drm-misc-next. In
v3 remain only patches that need an r-b/a-b (i915/gt/gvt) or required
a change from v2.

The pdev field in struct drm_device points to a PCI device structure and
goes back to UMS-only days when all DRM drivers were for PCI devices.
Meanwhile we also support USB, SPI and platform devices. Each of those
uses the generic device stored in struct drm_device.dev.

To reduce duplication and remove the special case of PCI, this patchset
converts all modesetting drivers from pdev to dev and makes pdev a field
for legacy UMS drivers.

For PCI devices, the pointer in struct drm_device.dev can be upcasted to
struct pci_device; or tested for PCI with dev_is_pci(). In several places
the code can use the dev field directly.

After converting all drivers and the DRM core, the pdev fields becomes
only relevant for legacy drivers. In a later patchset, we may want to
convert these as well and remove pdev entirely.

v3:
* fix one pdev reference in nouveau (Jeremy)
* rebases
v2:
* move whitespace fixes into separate patches (Alex, Sam)
* move i915 gt/ and gvt/ changes into separate patches (Joonas)


Thomas Zimmermann (8):
  drm/amdgpu: Fix trailing whitespaces
  drm/amdgpu: Remove references to struct drm_device.pdev
  drm/hibmc: Remove references to struct drm_device.pdev
  drm/i915: Remove references to struct drm_device.pdev
  drm/i915/gt: Remove references to struct drm_device.pdev
  drm/i915/gvt: Remove references to struct drm_device.pdev
  drm/nouveau: Remove references to struct drm_device.pdev
  drm: Upcast struct drm_device.dev to struct pci_device; replace pdev

 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 23 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   | 10 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   | 10 +--
 drivers/gpu/drm/drm_agpsupport.c  |  9 ++-
 drivers/gpu/drm/drm_bufs.c|  4 +-
 drivers/gpu/drm/drm_edid.c|  7 ++-
 drivers/gpu/drm/drm_irq.c | 12 ++--
 drivers/gpu/drm/drm_pci.c | 26 
 drivers/gpu/drm/drm_vm.c  |  2 +-
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 13 ++--
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c   |  2 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c   | 61 +++
 drivers/gpu/drm/i915/display/intel_bios.c |  2 +-
 drivers/gpu/drm/i915/display/intel_cdclk.c| 14 ++---
 drivers/gpu/drm/i915/display/intel_csr.c  |  2 +-
 drivers/gpu/drm/i915/display/intel_dsi_vbt.c  |  2 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c|  2 +-
 drivers/gpu/drm/i915/display/intel_gmbus.c|  2 +-
 .../gpu/drm/i915/display/intel_lpe_audio.c|  5 +-
 drivers/gpu/drm/i915/display/intel_opregion.c |  6 +-
 drivers/gpu/drm/i915/display/intel_overlay.c  |  2 +-
 drivers/gpu/drm/i915/display/intel_panel.c|  4 +-
 drivers/gpu/drm/i915/display/intel_quirks.c   |  2 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c |  2 +-
 drivers/gpu/drm/i915/display/intel_vga.c  |  8 +--
 drivers/gpu/drm/i915/gem/i915_gem_phys.c  |  6 +-
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c  | 10 +--
 drivers/gpu/drm/i915/gt/intel_ppgtt.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_rc6.c   |  4 +-
 drivers/gpu/drm/i915/gt/intel_reset.c |  6 +-
 drivers/gpu/drm/i915/gvt/cfg_space.c  |  5 +-
 drivers/gpu/drm/i915/gvt/firmware.c   | 10 +--
 drivers/gpu/drm/i915/gvt/gtt.c| 12 ++--
 drivers/gpu/drm/i915/gvt/gvt.c|  6 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c  |  4 +-
 drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
 drivers/gpu/drm/i915/i915_drv.c   | 20 +++---
 drivers/gpu/drm/i915/i915_drv.h   |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c   |  5 +-
 drivers/gpu/drm/i915/i915_getparam.c  |  5 +-
 drivers/gpu/drm/i915/i915_gpu_error.c |  2 +-
 drivers/gpu/drm/i915/i915_irq.c   |  6 +-
 drivers/gpu/drm/i915/i915_pmu.c   |  2 +-
 drivers/gpu/drm/i915/i915_suspend.c   |  4 +-
 drivers/gpu/drm/i915/i915_switcheroo.c|  4 +-
 drivers/gpu/drm/i915/i915_vgpu.c  |  2 +-
 drivers/gpu/drm/i915/intel_device_info.c  |  2 +-
 drivers/gpu/drm/i915/intel_region_lmem.c  |  8 +--
 drivers/gpu/drm/i915/intel_runtime_pm.c   |  2 +-
 drivers/gpu/drm/i915/intel_uncore.c   |  4 +-
 .../gpu/drm/i915/selftests/mock_gem_device.c  |  1 -
 drivers/gpu/drm/i915/selftests/mock_gtt.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/arb.c| 12 ++--
 

[PATCH v3 2/8] drm/amdgpu: Remove references to struct drm_device.pdev

2021-01-07 Thread Thomas Zimmermann
Using struct drm_device.pdev is deprecated. Convert amdgpu to struct
drm_device.dev. No functional changes.

v3:
* rebased

Signed-off-by: Thomas Zimmermann 
Acked-by: Christian König 
Acked-by: Alex Deucher 
Acked-by: Sam Ravnborg 
Cc: Alex Deucher 
Cc: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 17 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 10 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 +-
 7 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7d16395ede0a..f7e2a878411e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1423,9 +1423,9 @@ static void amdgpu_switcheroo_set_state(struct pci_dev 
*pdev,
/* don't suspend or resume card normally */
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
-   pci_set_power_state(dev->pdev, PCI_D0);
-   amdgpu_device_load_pci_state(dev->pdev);
-   r = pci_enable_device(dev->pdev);
+   pci_set_power_state(pdev, PCI_D0);
+   amdgpu_device_load_pci_state(pdev);
+   r = pci_enable_device(pdev);
if (r)
DRM_WARN("pci_enable_device failed (%d)\n", r);
amdgpu_device_resume(dev, true);
@@ -1437,10 +1437,10 @@ static void amdgpu_switcheroo_set_state(struct pci_dev 
*pdev,
drm_kms_helper_poll_disable(dev);
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
amdgpu_device_suspend(dev, true);
-   amdgpu_device_cache_pci_state(dev->pdev);
+   amdgpu_device_cache_pci_state(pdev);
/* Shut down the device */
-   pci_disable_device(dev->pdev);
-   pci_set_power_state(dev->pdev, PCI_D3cold);
+   pci_disable_device(pdev);
+   pci_set_power_state(pdev, PCI_D3cold);
dev->switch_power_state = DRM_SWITCH_POWER_OFF;
}
 }
@@ -1703,8 +1703,7 @@ static void amdgpu_device_enable_virtual_display(struct 
amdgpu_device *adev)
adev->enable_virtual_display = false;
 
if (amdgpu_virtual_display) {
-   struct drm_device *ddev = adev_to_drm(adev);
-   const char *pci_address_name = pci_name(ddev->pdev);
+   const char *pci_address_name = pci_name(adev->pdev);
char *pciaddstr, *pciaddstr_tmp, *pciaddname_tmp, *pciaddname;
 
pciaddstr = kstrdup(amdgpu_virtual_display, GFP_KERNEL);
@@ -3397,7 +3396,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
}
}
 
-   pci_enable_pcie_error_reporting(adev->ddev.pdev);
+   pci_enable_pcie_error_reporting(adev->pdev);
 
/* Post card if necessary */
if (amdgpu_device_need_post(adev)) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index f764803c53a4..0150a51b65ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -926,6 +926,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device 
*dev,
   struct drm_file *file_priv,
   const struct drm_mode_fb_cmd2 *mode_cmd)
 {
+   struct amdgpu_device *adev = drm_to_adev(dev);
struct drm_gem_object *obj;
struct amdgpu_framebuffer *amdgpu_fb;
int ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 72efd579ec5e..b4ea67e12ada 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1204,7 +1204,6 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
if (ret)
return ret;
 
-   ddev->pdev = pdev;
pci_set_drvdata(pdev, ddev);
 
ret = amdgpu_driver_load_kms(adev, ent->driver_data);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 0bf7d36c6686..51cd49c6f38f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -271,7 +271,7 @@ static int amdgpufb_create(struct drm_fb_helper *helper,
DRM_INFO("fb depth is %d\n", fb->format->depth);
DRM_INFO("   pitch is %d\n", fb->pitches[0]);
 
-   vga_switcheroo_client_fb_set(adev_to_drm(adev)->pdev, info);
+   vga_switcheroo_client_fb_set(adev->pdev, info);
return 0;
 
 out:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index d0a1fee1f5f6..a5c42c3004a0 100644
--- 

[PATCH v3 1/8] drm/amdgpu: Fix trailing whitespaces

2021-01-07 Thread Thomas Zimmermann
Adhere to kernel coding style.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Christian König 
Acked-by: Alex Deucher 
Acked-by: Sam Ravnborg 
Cc: Alex Deucher 
Cc: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 8f451e809127..7d16395ede0a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4950,8 +4950,8 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev 
*pdev, pci_channel_sta
case pci_channel_io_normal:
return PCI_ERS_RESULT_CAN_RECOVER;
/* Fatal error, prepare for slot reset */
-   case pci_channel_io_frozen: 
-   /*  
+   case pci_channel_io_frozen:
+   /*
 * Cancel and wait for all TDRs in progress if failing to
 * set  adev->in_gpu_reset in amdgpu_device_lock_adev
 *
@@ -5042,7 +5042,7 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev 
*pdev)
goto out;
}
 
-   adev->in_pci_err_recovery = true;   
+   adev->in_pci_err_recovery = true;
r = amdgpu_device_pre_asic_reset(adev, NULL, _full_reset);
adev->in_pci_err_recovery = false;
if (r)
-- 
2.29.2

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