AMDGPU VCE 1: some info needed
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
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"
[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
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
[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
[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)
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.
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
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"
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"
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"
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"
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"
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
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
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
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.
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
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.
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.
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.
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.
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
> -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
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.
[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
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
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
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
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
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
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
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
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
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
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"
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"
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
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
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
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
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
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
[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
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
[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
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
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
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
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
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
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
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
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
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
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
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
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
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
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