Re: [PATCH 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
Hi Dmitry On 6/15/2022 10:55 PM, Dmitry Baryshkov wrote: On 14/06/2022 22:32, Abhinav Kumar wrote: intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Please adjust the Fixes tags in all three commits. I didn't notice this beforehand and Stephen has complained. Is something wrong with the tag? Format and hash looked right to me. Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..e991d4ba8a40 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) - phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); - - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) - phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); - - if (!phys->hw_intf && !phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "no intf or wb block assigned at idx: %d\n", i); - return; - } - - if (phys->hw_intf && phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "invalid phys both intf and wb block at idx: %d\n", i); - return; - } - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(>vsync_cnt, 0); atomic_set(>underrun_cnt, 0); + + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) + phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); + + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); } + mutex_unlock(_enc->enc_lock); return ret;
Re: [PATCH 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
On 14/06/2022 22:32, Abhinav Kumar wrote: intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Please adjust the Fixes tags in all three commits. I didn't notice this beforehand and Stephen has complained. Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..e991d4ba8a40 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) - phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); - - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) - phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); - - if (!phys->hw_intf && !phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "no intf or wb block assigned at idx: %d\n", i); - return; - } - - if (phys->hw_intf && phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "invalid phys both intf and wb block at idx: %d\n", i); - return; - } - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(>vsync_cnt, 0); atomic_set(>underrun_cnt, 0); + + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) + phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); + + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); } + mutex_unlock(_enc->enc_lock); return ret; -- With best wishes Dmitry
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On 5/26/2022 4:32 PM, Arnd Bergmann wrote: On Wed, May 25, 2022 at 11:35 PM kernel test robot wrote: .__mulsi3.o.cmd: No such file or directory Makefile:686: arch/h8300/Makefile: No such file or directory Makefile:765: arch/h8300/Makefile: No such file or directory arch/Kconfig:10: can't open file "arch/h8300/Kconfig" Please stop building h8300 after the asm-generic tree is merged, the architecture is getting removed. Arnd Hi Arnd, Thanks for the advice, we have stopped building h8300 for new kernel. Best Regards, Rong Chen
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
Pushed to drm-misc-next. Alex On Wed, Jun 15, 2022 at 7:26 PM Stephen Rothwell wrote: > > Hi all, > > On Wed, 15 Jun 2022 13:52:34 -0700 Nathan Chancellor > wrote: > > > > On Wed, Jun 15, 2022 at 04:45:16PM -0400, Alex Deucher wrote: > > > On Wed, Jun 15, 2022 at 4:24 PM Nathan Chancellor > > > wrote: > > > > > > > > On Wed, Jun 15, 2022 at 03:28:52PM -0400, Alex Deucher wrote: > > > > > On Wed, Jun 15, 2022 at 3:01 PM Randy Dunlap > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 6/14/22 23:01, Stephen Rothwell wrote: > > > > > > > Hi all, > > > > > > > > > > > > > > Changes since 20220614: > > > > > > > > > > > > > > > > > > > on i386: > > > > > > # CONFIG_DEBUG_FS is not set > > > > > > > > > > > > > > > > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In > > > > > > function ‘amdgpu_dm_crtc_late_register’: > > > > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: > > > > > > error: implicit declaration of function ‘crtc_debugfs_init’; did > > > > > > you mean ‘amdgpu_debugfs_init’? > > > > > > [-Werror=implicit-function-declaration] > > > > > > crtc_debugfs_init(crtc); > > > > > > ^ > > > > > > amdgpu_debugfs_init > > > > > > > > > > > > > > > > > > Full randconfig file is attached. > > > > > > > > > > I tried building with your config and I can't repro this. As Harry > > > > > noted, that function and the whole secure display feature depend on > > > > > debugfs. It should never be built without CONFIG_DEBUG_FS. See > > > > > drivers/gpu/drm/amd/display/Kconfig: > > > > > > > > > > > config DRM_AMD_SECURE_DISPLAY > > > > > > bool "Enable secure display support" > > > > > > default n > > > > > > depends on DEBUG_FS > > > > > > depends on DRM_AMD_DC_DCN > > > > > > help > > > > > > Choose this option if you want to > > > > > > support secure display > > > > > > > > > > > > This option enables the calculation > > > > > > of crc of specific region via debugfs. > > > > > > Cooperate with specific DMCU FW. > > > > > > > > > > amdgpu_dm_crtc_late_register is guarded by > > > > > CONIG_DRM_AMD_SECURE_DISPLAY. It's not clear to me how we could hit > > > > > this. > > > > > > > > I think the problem is that you are not looking at the right tree. > > > > > > > > The kernel test robot reported [1] [2] this error is caused by commit > > > > 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm"), which > > > > is in the drm-misc tree on the drm-misc-next branch. That change removes > > > > the #ifdef around amdgpu_dm_crtc_late_register(), meaning that > > > > crtc_debugfs_init() can be called without CONFIG_DRM_AMD_SECURE_DISPLAY > > > > and CONFIG_DEBUG_FS. > > > > > > > > $ git show -s --format='%h ("%s")' > > > > abf0ba5a34ea ("drm/bridge: it6505: Add missing CRYPTO_HASH > > > > dependency") > > > > > > > > $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig > > > > > > > > $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU > > > > > > > > $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig > > > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o > > > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In > > > > function ‘amdgpu_dm_crtc_late_register’: > > > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6622:9: > > > > error: implicit declaration of function ‘crtc_debugfs_init’; did you > > > > mean ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] > > > >6622 | crtc_debugfs_init(crtc); > > > > | ^ > > > > | amdgpu_debugfs_init > > > > cc1: all warnings being treated as errors > > > > > > > > Contrast that with the current top of your tree: > > > > > > > > $ git show -s --format='%h ("%s")' > > > > c435f61d0eb3 ("drm/amd/display: Drop unnecessary guard from DC > > > > resource") > > > > > > > > $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig > > > > > > > > $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU > > > > > > > > $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig > > > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o > > > > > > > > $ echo $? > > > > 0 > > > > > > > > Randy's patch [3] seems like it should resolve the issue just fine but > > > > it needs to be applied to drm-misc-next, not the amdgpu tree. > > > > > > Thanks for tracking this down. I think something like the attached > > > patch is cleaner since the whole thing is only valid for debugfs. > > > > Makes sense! I tested the below patch with and without DEBUG_FS and saw > > no errors. > > > > > From b0bcacd86344998e0ca757f89c6c4cd3b6298999 Mon Sep 17 00:00:00 2001 > > > From: Alex Deucher > > > Date: Wed, 15 Jun 2022 16:40:39 -0400 > > > Subject: [PATCH] drm/amdgpu/display: fix build when CONFIG_DEBUG_FS is > > > not set > > > > > >
Re: [PATCH v2 4/5] drm/msm: move KMS aspace init to the separate helper
Quoting Dmitry Baryshkov (2022-05-04 17:16:04) > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index a37a3bbc04d9..98ae0036ab57 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -262,6 +263,46 @@ static int msm_drm_uninit(struct device *dev) > > #include > > +struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev) > +{ [...] > + } > + > + aspace = msm_gem_address_space_create(mmu, "mdp_kms", > + 0x1000, 0x1 - 0x1000); > + if (IS_ERR(aspace)) { > + mmu->funcs->destroy(mmu); > + return aspace; > + } > + > + return aspace; This can be 'return aspace' one time instead of two. > +} > + > bool msm_use_mmu(struct drm_device *dev) > { > struct msm_drm_private *priv = dev->dev_private;
Re: [PATCH v7] drm/msm/dp: force link training for display resolution change
Quoting Kuogee Hsieh (2022-06-15 08:53:57) > Display resolution change is implemented through drm modeset. Older > modeset (resolution) has to be disabled first before newer modeset > (resolution) can be enabled. Display disable will turn off both > pixel clock and main link clock so that main link have to be > re-trained during display enable to have new video stream flow > again. At current implementation, display enable function manually > kicks up irq_hpd_handle which will read panel link status and start > link training if link status is not in sync state. > > However, there is rare case that a particular panel links status keep > staying in sync for some period of time after main link had been shut > down previously at display disabled. In this case, main link retraining > will not be executed by irq_hdp_handle(). Hence video stream of newer > display resolution will fail to be transmitted to panel due to main > link is not in sync between host and panel. > > This patch will bypass irq_hpd_handle() in favor of directly call > dp_ctrl_on_stream() to always perform link training in regardless of > main link status. So that no unexpected exception resolution change > failure cases will happen. Also this implementation are more efficient > than manual kicking off irq_hpd_handle function. > > Changes in v2: > -- set force_link_train flag on DP only (is_edp == false) > > Changes in v3: > -- revise commit text > -- add Fixes tag > > Changes in v4: > -- revise commit text > > Changes in v5: > -- fix spelling at commit text > > Changes in v6: > -- split dp_ctrl_on_stream() for phy test case > -- revise commit text for modeset > > Changes in v7: > -- drop 0 assignment at local variable (ret = 0) > > Fixes: 62671d2ef24b ("drm/msm/dp: fixes wrong connection state caused by > failure of link train") > Signed-off-by: Kuogee Hsieh > --- > drivers/gpu/drm/msm/dp/dp_ctrl.c| 31 +++ > drivers/gpu/drm/msm/dp/dp_ctrl.h| 3 ++- > drivers/gpu/drm/msm/dp/dp_display.c | 13 ++--- > 3 files changed, 31 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c > b/drivers/gpu/drm/msm/dp/dp_ctrl.c > index af7a80c..01028b5 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c > @@ -1551,7 +1551,7 @@ static int dp_ctrl_process_phy_test_request(struct > dp_ctrl_private *ctrl) > > ret = dp_ctrl_on_link(>dp_ctrl); > if (!ret) > - ret = dp_ctrl_on_stream(>dp_ctrl); > + ret = dp_ctrl_on_stream_phy_test_report(>dp_ctrl); > else > DRM_ERROR("failed to enable DP link controller\n"); > > @@ -1807,7 +1807,27 @@ static int dp_ctrl_link_retrain(struct dp_ctrl_private > *ctrl) > return dp_ctrl_setup_main_link(ctrl, _step); > } > > -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl) > +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl) > +{ > + int ret; > + struct dp_ctrl_private *ctrl; > + > + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); > + > + ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock; There are a few different places where we assign 'pixel_rate' in this file. Can they be consolidated? Is it a concern that wide_bus isn't considered here? I was really hoping that we could reuse code instead of duplicate it, DRY principle and all. Maybe even better, can we remove 'pixel_rate' entirely from being stashed? See further down for my patch. > + > + ret = dp_ctrl_enable_stream_clocks(ctrl); > + if (ret) { > + DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret); > + return ret; > + } > + > + dp_ctrl_send_phy_test_pattern(ctrl); > + > + return 0; > +} > + > +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train) > { > int ret = 0; > bool mainlink_ready = false; > @@ -1843,12 +1863,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl) > goto end; > } > > - if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) { > - dp_ctrl_send_phy_test_pattern(ctrl); > - return 0; > - } > - > - if (!dp_ctrl_channel_eq_ok(ctrl)) > + if (force_link_train || !dp_ctrl_channel_eq_ok(ctrl)) > dp_ctrl_link_retrain(ctrl); > > /* stop txing train pattern to end link training */ > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h > b/drivers/gpu/drm/msm/dp/dp_ctrl.h > index 0745fde..9a39b00 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h > @@ -21,7 +21,8 @@ struct dp_ctrl { > }; > > int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl); > -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl); > +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train); > +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl); Please don't expose the API if it's only used in dp_ctrl.c > int
Re: [Freedreno] [PATCH v2 1/5] drm/msm/dpu: check both DPU and MDSS devices for the IOMMU
On 6/15/2022 11:42 AM, Dmitry Baryshkov wrote: On 15/06/2022 20:55, Abhinav Kumar wrote: On 5/4/2022 5:16 PM, Dmitry Baryshkov wrote: Follow the lead of MDP5 driver and check both DPU and MDSS devices for the IOMMU specifiers. Historically DPU devices had IOMMU specified in the MDSS device tree node, but as some of MDP5 devices are being converted to the supported by the DPU driver, the driver should adapt and check both devices. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 143d6643be53..5ccda0766f6c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1004,14 +1004,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) struct msm_mmu *mmu; struct device *dpu_dev = dpu_kms->dev->dev; struct device *mdss_dev = dpu_dev->parent; + struct device *iommu_dev; domain = iommu_domain_alloc(_bus_type); if (!domain) return 0; - /* IOMMUs are a part of MDSS device tree binding, not the - * MDP/DPU device. */ - mmu = msm_iommu_new(mdss_dev, domain); + /* + * IOMMUs can be a part of MDSS device tree binding, or the + * MDP/DPU device. + */ Can you please explain this a little more? So even if some of the mdp5 devices are getting converted to use DPU driver, their device trees are also updated right? No. The DT describes the device, not the Linux drivers. So while updating the drivers we should not change the DT. Alright, agreed Reviewed-by: Abhinav Kumar In other words, if DPU driver was using mdss_dev to initialize the iommu, why should the new devices which are going to use DPU have the binding in the dpu_dev? + if (dev_iommu_fwspec_get(dpu_dev)) + iommu_dev = dpu_dev; + else + iommu_dev = mdss_dev; + + mmu = msm_iommu_new(iommu_dev, domain); if (IS_ERR(mmu)) { iommu_domain_free(domain); return PTR_ERR(mmu);
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
Hi all, On Wed, 15 Jun 2022 13:52:34 -0700 Nathan Chancellor wrote: > > On Wed, Jun 15, 2022 at 04:45:16PM -0400, Alex Deucher wrote: > > On Wed, Jun 15, 2022 at 4:24 PM Nathan Chancellor > > wrote: > > > > > > On Wed, Jun 15, 2022 at 03:28:52PM -0400, Alex Deucher wrote: > > > > On Wed, Jun 15, 2022 at 3:01 PM Randy Dunlap > > > > wrote: > > > > > > > > > > > > > > > > > > > > On 6/14/22 23:01, Stephen Rothwell wrote: > > > > > > Hi all, > > > > > > > > > > > > Changes since 20220614: > > > > > > > > > > > > > > > > on i386: > > > > > # CONFIG_DEBUG_FS is not set > > > > > > > > > > > > > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In > > > > > function ‘amdgpu_dm_crtc_late_register’: > > > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: > > > > > error: implicit declaration of function ‘crtc_debugfs_init’; did you > > > > > mean ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] > > > > > crtc_debugfs_init(crtc); > > > > > ^ > > > > > amdgpu_debugfs_init > > > > > > > > > > > > > > > Full randconfig file is attached. > > > > > > > > I tried building with your config and I can't repro this. As Harry > > > > noted, that function and the whole secure display feature depend on > > > > debugfs. It should never be built without CONFIG_DEBUG_FS. See > > > > drivers/gpu/drm/amd/display/Kconfig: > > > > > > > > > config DRM_AMD_SECURE_DISPLAY > > > > > bool "Enable secure display support" > > > > > default n > > > > > depends on DEBUG_FS > > > > > depends on DRM_AMD_DC_DCN > > > > > help > > > > > Choose this option if you want to > > > > > support secure display > > > > > > > > > > This option enables the calculation > > > > > of crc of specific region via debugfs. > > > > > Cooperate with specific DMCU FW. > > > > > > > > amdgpu_dm_crtc_late_register is guarded by > > > > CONIG_DRM_AMD_SECURE_DISPLAY. It's not clear to me how we could hit > > > > this. > > > > > > I think the problem is that you are not looking at the right tree. > > > > > > The kernel test robot reported [1] [2] this error is caused by commit > > > 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm"), which > > > is in the drm-misc tree on the drm-misc-next branch. That change removes > > > the #ifdef around amdgpu_dm_crtc_late_register(), meaning that > > > crtc_debugfs_init() can be called without CONFIG_DRM_AMD_SECURE_DISPLAY > > > and CONFIG_DEBUG_FS. > > > > > > $ git show -s --format='%h ("%s")' > > > abf0ba5a34ea ("drm/bridge: it6505: Add missing CRYPTO_HASH dependency") > > > > > > $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig > > > > > > $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU > > > > > > $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig > > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o > > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In > > > function ‘amdgpu_dm_crtc_late_register’: > > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6622:9: > > > error: implicit declaration of function ‘crtc_debugfs_init’; did you mean > > > ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] > > >6622 | crtc_debugfs_init(crtc); > > > | ^ > > > | amdgpu_debugfs_init > > > cc1: all warnings being treated as errors > > > > > > Contrast that with the current top of your tree: > > > > > > $ git show -s --format='%h ("%s")' > > > c435f61d0eb3 ("drm/amd/display: Drop unnecessary guard from DC > > > resource") > > > > > > $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig > > > > > > $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU > > > > > > $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig > > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o > > > > > > $ echo $? > > > 0 > > > > > > Randy's patch [3] seems like it should resolve the issue just fine but > > > it needs to be applied to drm-misc-next, not the amdgpu tree. > > > > Thanks for tracking this down. I think something like the attached > > patch is cleaner since the whole thing is only valid for debugfs. > > Makes sense! I tested the below patch with and without DEBUG_FS and saw > no errors. > > > From b0bcacd86344998e0ca757f89c6c4cd3b6298999 Mon Sep 17 00:00:00 2001 > > From: Alex Deucher > > Date: Wed, 15 Jun 2022 16:40:39 -0400 > > Subject: [PATCH] drm/amdgpu/display: fix build when CONFIG_DEBUG_FS is not > > set > > > > amdgpu_dm_crtc_late_register is only used when CONFIG_DEBUG_FS > > so make it dependent on that. > > > > Fixes: 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm") > > Reported-by: Randy Dunlap > > Reported-by: Nathan Chancellor > > Tested-by: Nathan Chancellor # build > > > Signed-off-by: Alex Deucher > > --- > >
[PATCH] drm/msm/dpu: set preferred mode for writeback connector
After [1] was merged to IGT, we use either the first supported mode in the list OR the preferred mode to determine the primary plane to use for the sub-test due to the IGT API [2]. Since writeback does not set any preferred mode, this was selecting 4k as that was the first entry in the list. We use maxlinewidth to add the list of supported modes for the writeback connector which is the right thing to do, however since we do not have dual-SSPP support yet for DPU, this fails the bandwidth check in dpu_core_perf_crtc_check(). Till we have dual-SSPP support, workaround this mismatch between the list of supported modes and maxlinewidth limited modes by marking 640x480 as the preferred mode for DPU writeback because kms_writeback tests 640x480 mode only [3]. [1]: https://patchwork.freedesktop.org/patch/486441/ [2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/lib/igt_kms.c#L1562 [3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L68 Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c index 399115e4e217..104cc59d6466 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c @@ -10,9 +10,14 @@ static int dpu_wb_conn_get_modes(struct drm_connector *connector) struct drm_device *dev = connector->dev; struct msm_drm_private *priv = dev->dev_private; struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); + int count; - return drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, + count = drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, dev->mode_config.max_height); + + drm_set_preferred_mode(connector, 640, 480); + + return count; } static const struct drm_connector_funcs dpu_wb_conn_funcs = { -- 2.7.4
Re: Please add another drm/msm tree to the linux-next
Hi Dmitry, On Wed, 15 Jun 2022 17:19:42 +0300 Dmitry Baryshkov wrote: > > I would appreciate if you could add > > https://gitlab.freedesktop.org/lumag/msm.git msm-next-lumag > > to the linux-next tree. > > This tree is a part of drm/msm maintenance structure. As a co-maintainer I > collect and test display patches, while Rob concenctrates on GPU part of the > driver. Later during the release cycle these patchesare pulled by Rob Clark > directly into msm-next. > > During last cycle Rob suggested adding this tree to the linux-next effort, so > that the patches receive better integration testing during the Linux > development cycle. Added from today. Thanks for adding your subsystem tree as a participant of linux-next. As you may know, this is not a judgement of your code. The purpose of linux-next is for integration testing and to lower the impact of conflicts between subsystems in the next merge window. You will need to ensure that the patches/commits in your tree/series have been: * submitted under GPL v2 (or later) and include the Contributor's Signed-off-by, * posted to the relevant mailing list, * reviewed by you (or another maintainer of your subsystem tree), * successfully unit tested, and * destined for the current or next Linux merge window. Basically, this should be just what you would send to Linus (or ask him to fetch). It is allowed to be rebased if you deem it necessary. -- Cheers, Stephen Rothwell s...@canb.auug.org.au pgp4VHggx49n3.pgp Description: OpenPGP digital signature
Re: [PATCH v2 08/15] mfd: mt6370: Add Mediatek MT6370 support
On Mon, 13 Jun 2022, ChiaEn Wu wrote: > From: ChiYuan Huang > > Add Mediatek MT6370 MFD support. > > Signed-off-by: ChiYuan Huang > --- > drivers/mfd/Kconfig | 13 ++ > drivers/mfd/Makefile | 1 + > drivers/mfd/mt6370.c | 349 +++ > 3 files changed, 363 insertions(+) > create mode 100644 drivers/mfd/mt6370.c > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 3b59456f5545..d9a7524a3e0e 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -937,6 +937,19 @@ config MFD_MT6360 > PMIC part includes 2-channel BUCKs and 2-channel LDOs > LDO part includes 4-channel LDOs > > +config MFD_MT6370 > + tristate "Mediatek MT6370 SubPMIC" > + select MFD_CORE > + select REGMAP_I2C > + select REGMAP_IRQ > + depends on I2C > + help > + Say Y here to enable MT6370 SubPMIC functional support. > + It integrate single cell battery charger with adc monitoring, RGB s/integrates/consists of a/ "ADC" > + LEDs, dual channel flashlight, WLED backlight driver, display bias > + voltage supply, one general purpose LDO, and cc logic > + controller with USBPD commmunication capable. The last part makes no sense - "and is USBPD"? > config MFD_MT6397 > tristate "MediaTek MT6397 PMIC Support" > select MFD_CORE > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index 858cacf659d6..62b27125420e 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -242,6 +242,7 @@ obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC)+= > intel_soc_pmic_bxtwc.o > obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o > obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI)+= intel_soc_pmic_chtdc_ti.o > obj-$(CONFIG_MFD_MT6360) += mt6360-core.o > +obj-$(CONFIG_MFD_MT6370) += mt6370.o > mt6397-objs := mt6397-core.o mt6397-irq.o mt6358-irq.o > obj-$(CONFIG_MFD_MT6397) += mt6397.o > obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD) += intel_soc_pmic_mrfld.o > diff --git a/drivers/mfd/mt6370.c b/drivers/mfd/mt6370.c > new file mode 100644 > index ..6af9f73c9c0c > --- /dev/null > +++ b/drivers/mfd/mt6370.c > @@ -0,0 +1,349 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +enum { > + MT6370_USBC_I2C = 0, > + MT6370_PMU_I2C, > + MT6370_MAX_I2C > +}; > + > +#define MT6370_REG_DEV_INFO 0x100 > +#define MT6370_REG_CHG_IRQ1 0x1C0 > +#define MT6370_REG_CHG_MASK1 0x1E0 > + > +#define MT6370_VENID_MASKGENMASK(7, 4) > + > +#define MT6370_NUM_IRQREGS 16 > +#define MT6370_USBC_I2CADDR 0x4E > +#define MT6370_REG_ADDRLEN 2 > +#define MT6370_REG_MAXADDR 0x1FF > + > +/* IRQ definitions */ > +#define MT6370_IRQ_DIRCHGON 0 > +#define MT6370_IRQ_CHG_TREG 4 > +#define MT6370_IRQ_CHG_AICR 5 > +#define MT6370_IRQ_CHG_MIVR 6 > +#define MT6370_IRQ_PWR_RDY 7 > +#define MT6370_IRQ_FL_CHG_VINOVP 11 > +#define MT6370_IRQ_CHG_VSYSUV12 > +#define MT6370_IRQ_CHG_VSYSOV13 > +#define MT6370_IRQ_CHG_VBATOV14 > +#define MT6370_IRQ_CHG_VINOVPCHG 15 > +#define MT6370_IRQ_TS_BAT_COLD 20 > +#define MT6370_IRQ_TS_BAT_COOL 21 > +#define MT6370_IRQ_TS_BAT_WARM 22 > +#define MT6370_IRQ_TS_BAT_HOT23 > +#define MT6370_IRQ_TS_STATC 24 > +#define MT6370_IRQ_CHG_FAULT 25 > +#define MT6370_IRQ_CHG_STATC 26 > +#define MT6370_IRQ_CHG_TMR 27 > +#define MT6370_IRQ_CHG_BATABS28 > +#define MT6370_IRQ_CHG_ADPBAD29 > +#define MT6370_IRQ_CHG_RVP 30 > +#define MT6370_IRQ_TSHUTDOWN 31 > +#define MT6370_IRQ_CHG_IINMEAS 32 > +#define MT6370_IRQ_CHG_ICCMEAS 33 > +#define MT6370_IRQ_CHGDET_DONE 34 > +#define MT6370_IRQ_WDTMR 35 > +#define MT6370_IRQ_SSFINISH 36 > +#define MT6370_IRQ_CHG_RECHG 37 > +#define MT6370_IRQ_CHG_TERM 38 > +#define MT6370_IRQ_CHG_IEOC 39 > +#define MT6370_IRQ_ADC_DONE 40 > +#define MT6370_IRQ_PUMPX_DONE41 > +#define MT6370_IRQ_BST_BATUV 45 > +#define MT6370_IRQ_BST_MIDOV 46 > +#define MT6370_IRQ_BST_OLP 47 > +#define MT6370_IRQ_ATTACH48 > +#define MT6370_IRQ_DETACH49 > +#define MT6370_IRQ_HVDCP_STPDONE 51 > +#define MT6370_IRQ_HVDCP_VBUSDET_DONE52 > +#define MT6370_IRQ_HVDCP_DET 53 > +#define MT6370_IRQ_CHGDET54 > +#define MT6370_IRQ_DCDT 55 > +#define MT6370_IRQ_DIRCHG_VGOK 59 > +#define MT6370_IRQ_DIRCHG_WDTMR 60 > +#define MT6370_IRQ_DIRCHG_UC 61 > +#define MT6370_IRQ_DIRCHG_OC 62 > +#define MT6370_IRQ_DIRCHG_OV 63 > +#define MT6370_IRQ_OVPCTRL_SWON 67
[PATCH 2/2] drm/bridge/tc358775: Fix DSI clock division for vsync delay calculation
Use the same PCLK divide option (divide DSI clock to generate pixel clock) which is set to LVDS Configuration Register (LVCFG) also for a VSync delay calculation. Without this change an auxiliary variable could underflow during the calculation for some dual-link LVDS panels and then calculated VSync delay is wrong. This leads to a shifted picture on a panel. Tested-by: Jiri Vanek Signed-off-by: Jiri Vanek --- drivers/gpu/drm/bridge/tc358775.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c index cd2721ab02a9..fecb8558b49a 100644 --- a/drivers/gpu/drm/bridge/tc358775.c +++ b/drivers/gpu/drm/bridge/tc358775.c @@ -430,7 +430,7 @@ static void tc_bridge_enable(struct drm_bridge *bridge) val = TC358775_VPCTRL_MSF(1); dsiclk = mode->crtc_clock * 3 * tc->bpc / tc->num_dsi_lanes / 1000; - clkdiv = dsiclk / DIVIDE_BY_3 * tc->lvds_link; + clkdiv = dsiclk / (tc->lvds_link == DUAL_LINK ? DIVIDE_BY_6 : DIVIDE_BY_3); byteclk = dsiclk / 4; t1 = hactive * (tc->bpc * 3 / 8) / tc->num_dsi_lanes; t2 = ((10 / clkdiv)) * (hactive + hback_porch + hsync_len + hfront_porch) / 1000; -- 2.30.2
[PATCH 1/2] drm/bridge/tc358775: Return before displaying inappropriate error message
Function for reading from i2c device register displays error message even if reading ends correctly. Add return to avoid falling through into the fail label. Signed-off-by: Jiri Vanek --- drivers/gpu/drm/bridge/tc358775.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c index 62a7ef352daa..cd2721ab02a9 100644 --- a/drivers/gpu/drm/bridge/tc358775.c +++ b/drivers/gpu/drm/bridge/tc358775.c @@ -339,6 +339,7 @@ static void d2l_read(struct i2c_client *i2c, u16 addr, u32 *val) goto fail; pr_debug("d2l: I2C : addr:%04x value:%08x\n", addr, *val); + return; fail: dev_err(>dev, "Error %d reading from subaddress 0x%x\n", -- 2.30.2
[PATCH 0/2] Fixes for TC358775 DSI to LVDS bridge
This patchset fixes two bugs in the driver for TC358775 DSI to LVDS bridge. Jiri Vanek (2): drm/bridge/tc358775: Return before displaying inappropriate error message drm/bridge/tc358775: Fix DSI clock division for vsync delay calculation drivers/gpu/drm/bridge/tc358775.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.30.2
[PATCH v3 5/6] ARM: dts: renesas: Use new media bus type macros
Now that a header exists with macros for the media interface bus-type values, replace hardcoding numerical constants with the corresponding macros in the DT sources. Signed-off-by: Laurent Pinchart --- arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts | 11 +++ .../dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi | 4 +++- .../dts/r8a7742-iwg21d-q7-dbcm-ov7725-single.dtsi | 4 +++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts index 4e58c54cde17..33ac4bd1e63b 100644 --- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts @@ -7,6 +7,9 @@ */ /dts-v1/; + +#include + #include "r8a7742-iwg21d-q7.dts" / { @@ -242,7 +245,7 @@ port { vin0ep: endpoint { remote-endpoint = <>; bus-width = <8>; - bus-type = <6>; + bus-type = ; }; }; }; @@ -273,7 +276,7 @@ port { vin1ep: endpoint { remote-endpoint = <>; bus-width = <8>; - bus-type = <6>; + bus-type = ; }; }; }; @@ -305,7 +308,7 @@ vin2ep: endpoint { remote-endpoint = <>; bus-width = <8>; data-shift = <8>; - bus-type = <6>; + bus-type = ; }; }; }; @@ -335,7 +338,7 @@ port { vin3ep: endpoint { remote-endpoint = <>; bus-width = <8>; - bus-type = <6>; + bus-type = ; }; }; }; diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi index 40cef0b1d1e6..c73160df619d 100644 --- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi @@ -7,6 +7,8 @@ * Copyright (C) 2020 Renesas Electronics Corp. */ +#include + #define CAM_ENABLED1 _PARENT_I2C { @@ -26,7 +28,7 @@ port { CAM_EP: endpoint { bus-width = <8>; data-shift = <2>; - bus-type = <6>; + bus-type = ; pclk-sample = <1>; remote-endpoint = <_EP>; }; diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov7725-single.dtsi b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov7725-single.dtsi index f5e77f024251..a7f5cfec64b8 100644 --- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov7725-single.dtsi +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov7725-single.dtsi @@ -7,6 +7,8 @@ * Copyright (C) 2020 Renesas Electronics Corp. */ +#include + #define CAM_ENABLED1 _PARENT_I2C { @@ -21,7 +23,7 @@ ov7725@21 { port { CAM_EP: endpoint { bus-width = <8>; - bus-type = <6>; + bus-type = ; remote-endpoint = <_EP>; }; }; -- Regards, Laurent Pinchart
[PATCH v3 6/6] ARM: dts: stm32: Use new media bus type macros
Now that a header exists with macros for the media interface bus-type values, replace hardcoding numerical constants with the corresponding macros in the DT sources. Signed-off-by: Laurent Pinchart --- arch/arm/boot/dts/stm32429i-eval.dts | 3 ++- arch/arm/boot/dts/stm32mp157c-ev1.dts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts index 0d98aca01736..5fae11e6607b 100644 --- a/arch/arm/boot/dts/stm32429i-eval.dts +++ b/arch/arm/boot/dts/stm32429i-eval.dts @@ -50,6 +50,7 @@ #include "stm32f429-pinctrl.dtsi" #include #include +#include / { model = "STMicroelectronics STM32429i-EVAL board"; @@ -186,7 +187,7 @@ { port { dcmi_0: endpoint { remote-endpoint = <_0>; - bus-type = <5>; + bus-type = ; bus-width = <8>; hsync-active = <0>; vsync-active = <0>; diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts index d142dd30e16b..306d41a6138f 100644 --- a/arch/arm/boot/dts/stm32mp157c-ev1.dts +++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts @@ -8,6 +8,7 @@ #include "stm32mp157c-ed1.dts" #include #include +#include / { model = "STMicroelectronics STM32MP157C eval daughter on eval mother"; @@ -90,7 +91,7 @@ { port { dcmi_0: endpoint { remote-endpoint = <_0>; - bus-type = <5>; + bus-type = ; bus-width = <8>; hsync-active = <0>; vsync-active = <0>; -- Regards, Laurent Pinchart
[PATCH v3 4/6] ARM: dts: omap: Use new media bus type macros
Now that a header exists with macros for the media interface bus-type values, replace hardcoding numerical constants with the corresponding macros in the DT sources. Signed-off-by: Laurent Pinchart --- arch/arm/boot/dts/omap3-n900.dts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts index d40c3d2c4914..9cad9d6a83e2 100644 --- a/arch/arm/boot/dts/omap3-n900.dts +++ b/arch/arm/boot/dts/omap3-n900.dts @@ -9,6 +9,7 @@ #include "omap34xx.dtsi" #include #include +#include /* * Default secure signed bootloader (Nokia X-Loader) does not enable L3 firewall @@ -194,7 +195,7 @@ port@1 { csi_isp: endpoint { remote-endpoint = <_cam1>; - bus-type = <3>; /* CCP2 */ + bus-type = ; clock-lanes = <1>; data-lanes = <0>; lane-polarity = <0 0>; @@ -835,7 +836,7 @@ cam1: camera@3e { port { csi_cam1: endpoint { - bus-type = <3>; /* CCP2 */ + bus-type = ; strobe = <1>; clock-inv = <0>; crc = <1>; -- Regards, Laurent Pinchart
[PATCH v3 3/6] ARM: dts: freescale: Use new media bus type macros
Now that a header exists with macros for the media interface bus-type values, replace hardcoding numerical constants with the corresponding macros in the DT sources. Signed-off-by: Laurent Pinchart --- arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi index 1a18c41ce385..d98111f2100f 100644 --- a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi +++ b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi @@ -2,6 +2,8 @@ // // Copyright (C) 2015 Freescale Semiconductor, Inc. +#include + / { chosen { stdout-path = @@ -170,7 +172,7 @@ { port { parallel_from_ov5640: endpoint { remote-endpoint = <_to_parallel>; - bus-type = <5>; /* Parallel bus */ + bus-type = ; }; }; }; -- Regards, Laurent Pinchart
[PATCH v3 2/6] dt-bindings: Use new video interface bus type macros in examples
Now that a header exists with macros for the media interface bus-type values, replace hardcoding numerical constants with the corresponding macros in the DT binding examples. Signed-off-by: Laurent Pinchart --- Changes since v2: - Go back to PARALLEL Changes since v1: - Rename PARALLEL to BT601 --- .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 1 + Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml | 3 ++- Documentation/devicetree/bindings/media/i2c/ovti,ov772x.yaml | 3 ++- .../devicetree/bindings/media/marvell,mmp2-ccic.yaml | 3 ++- Documentation/devicetree/bindings/media/microchip,xisc.yaml | 3 ++- Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml| 4 +++- 6 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml index 35a48515836e..b0e5585f93e2 100644 --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml @@ -118,6 +118,7 @@ additionalProperties: false examples: - | #include +#include i2c0 { #address-cells = <1>; diff --git a/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml b/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml index 39395ea8c318..edde4201116f 100644 --- a/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml +++ b/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml @@ -104,6 +104,7 @@ additionalProperties: false examples: - | #include +#include i2c2 { #address-cells = <1>; @@ -124,7 +125,7 @@ examples: remote-endpoint = <_ep>; link-frequencies = /bits/ 64 <19920 21000 49920>; -bus-type = <4>; +bus-type = ; }; }; }; diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov772x.yaml index 44529425ce3a..161e6d598e1c 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov772x.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov772x.yaml @@ -105,6 +105,7 @@ additionalProperties: false examples: - | #include +#include i2c0 { #address-cells = <1>; @@ -118,7 +119,7 @@ examples: port { ov772x_0: endpoint { -bus-type = <5>; +bus-type = ; vsync-active = <0>; hsync-active = <0>; pclk-sample = <0>; diff --git a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.yaml b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.yaml index b39b84c5f012..0e3478551e13 100644 --- a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.yaml +++ b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.yaml @@ -68,6 +68,7 @@ additionalProperties: false examples: - | #include +#include #include camera@d420a000 { @@ -83,7 +84,7 @@ examples: port { camera0_0: endpoint { remote-endpoint = <_0>; - bus-type = <5>; /* Parallel */ + bus-type = ; hsync-active = <1>; /* Active high */ vsync-active = <1>; /* Active high */ pclk-sample = <0>; /* Falling */ diff --git a/Documentation/devicetree/bindings/media/microchip,xisc.yaml b/Documentation/devicetree/bindings/media/microchip,xisc.yaml index 8b37fccab5e2..25f5f79d40ce 100644 --- a/Documentation/devicetree/bindings/media/microchip,xisc.yaml +++ b/Documentation/devicetree/bindings/media/microchip,xisc.yaml @@ -106,6 +106,7 @@ examples: #include #include #include +#include xisc: xisc@e1408000 { compatible = "microchip,sama7g5-isc"; @@ -118,7 +119,7 @@ examples: port { xisc_in: endpoint { - bus-type = <5>; /* Parallel */ + bus-type = ; remote-endpoint = <_out>; hsync-active = <1>; vsync-active = <1>; diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml index 9c1262a276b5..285c6075950a 100644 --- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml +++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml @@ -90,7 +90,9 @@ examples: - | #include #include +#include #include +# dcmi: dcmi@4c006000 { compatible = "st,stm32-dcmi"; reg = <0x4c006000 0x400>; @@ -104,7 +106,7 @@ examples: port { dcmi_0: endpoint { remote-endpoint = <_0>; -
[PATCH v3 1/6] dt-bindings: media: Add macros for video interface bus types
Add a new dt-bindings/media/video-interfaces.h header that defines macros corresponding to the bus types from media/video-interfaces.yaml. This allows avoiding hardcoded constants in device tree sources. Signed-off-by: Laurent Pinchart --- Changes since v2: - Go back to PARALLEL Changes since v1: - Dual-license under GPL-2.0-only or MIT - Rename PARALLEL TO BT601 --- include/dt-bindings/media/video-interfaces.h | 16 1 file changed, 16 insertions(+) create mode 100644 include/dt-bindings/media/video-interfaces.h diff --git a/include/dt-bindings/media/video-interfaces.h b/include/dt-bindings/media/video-interfaces.h new file mode 100644 index ..68ac4e05e37f --- /dev/null +++ b/include/dt-bindings/media/video-interfaces.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */ +/* + * Copyright (C) 2022 Laurent Pinchart + */ + +#ifndef __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ +#define __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ + +#define MEDIA_BUS_TYPE_CSI2_CPHY 1 +#define MEDIA_BUS_TYPE_CSI12 +#define MEDIA_BUS_TYPE_CCP23 +#define MEDIA_BUS_TYPE_CSI2_DPHY 4 +#define MEDIA_BUS_TYPE_PARALLEL5 +#define MEDIA_BUS_TYPE_BT656 6 + +#endif /* __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ */ -- Regards, Laurent Pinchart
[PATCH v3 0/6] dt-bindings: Add macros for video interface bus types
Hello, This small patch series is the result of me getting a bus-type numerical value wrong in a device tree file and spending too long debugging the issue. Hopefully there's nothing controversial here. Compared to v2, the PARALLEL bus type came back to replace BT601, as it turned out that BT601 doesn't actually describe what is usually referred to as the parallel bus type. Patch 3/3 has been split in per-vendor patches at the request of Alexandre Torgue. Laurent Pinchart (6): dt-bindings: media: Add macros for video interface bus types dt-bindings: Use new video interface bus type macros in examples ARM: dts: freescale: Use new media bus type macros ARM: dts: omap: Use new media bus type macros ARM: dts: renesas: Use new media bus type macros ARM: dts: stm32: Use new media bus type macros .../display/bridge/analogix,anx7625.yaml | 1 + .../devicetree/bindings/media/i2c/mipi-ccs.yaml | 3 ++- .../bindings/media/i2c/ovti,ov772x.yaml | 3 ++- .../bindings/media/marvell,mmp2-ccic.yaml| 3 ++- .../bindings/media/microchip,xisc.yaml | 3 ++- .../devicetree/bindings/media/st,stm32-dcmi.yaml | 4 +++- arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 4 +++- arch/arm/boot/dts/omap3-n900.dts | 5 +++-- arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts | 11 +++ .../r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi| 4 +++- .../r8a7742-iwg21d-q7-dbcm-ov7725-single.dtsi| 4 +++- arch/arm/boot/dts/stm32429i-eval.dts | 3 ++- arch/arm/boot/dts/stm32mp157c-ev1.dts| 3 ++- include/dt-bindings/media/video-interfaces.h | 16 14 files changed, 51 insertions(+), 16 deletions(-) create mode 100644 include/dt-bindings/media/video-interfaces.h base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3 -- Regards, Laurent Pinchart
Re: [PATCH v2 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
On Thu, 16 Jun 2022 at 00:22, Abhinav Kumar wrote: > > intf and wb resources are not dependent on the rm global > state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). > > Move the allocation of intf and wb resources to dpu_encoder_setup_display() > so that we can utilize the hw caps even during atomic_check() phase. > > Since dpu_encoder_setup_display() already has protection against > setting invalid intf_idx and wb_idx, these checks can now > be dropped as well. > > changes in v2: > - add phys->hw_intf and phys->hw_wb checks back > > Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") > Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 36 > ++--- > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 3a462e327e0e..3be73211d631 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct > drm_encoder *drm_enc, > phys->hw_pp = dpu_enc->hw_pp[i]; > phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); > > - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) > - phys->hw_intf = dpu_rm_get_intf(_kms->rm, > phys->intf_idx); > - > - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) > - phys->hw_wb = dpu_rm_get_wb(_kms->rm, > phys->wb_idx); > - > - if (!phys->hw_intf && !phys->hw_wb) { > - DPU_ERROR_ENC(dpu_enc, > - "no intf or wb block assigned at idx: > %d\n", i); > - return; > - } > - > - if (phys->hw_intf && phys->hw_wb) { > - DPU_ERROR_ENC(dpu_enc, > - "invalid phys both intf and wb block > at idx: %d\n", i); > - return; > - } > - > phys->cached_mode = crtc_state->adjusted_mode; > if (phys->ops.atomic_mode_set) > phys->ops.atomic_mode_set(phys, crtc_state, > conn_state); > @@ -2293,7 +2275,25 @@ static int dpu_encoder_setup_display(struct > dpu_encoder_virt *dpu_enc, > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; > atomic_set(>vsync_cnt, 0); > atomic_set(>underrun_cnt, 0); > + > + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) > + phys->hw_intf = dpu_rm_get_intf(_kms->rm, > phys->intf_idx); > + > + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) > + phys->hw_wb = dpu_rm_get_wb(_kms->rm, > phys->wb_idx); > + > + if (!phys->hw_intf && !phys->hw_wb) { > + DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned > at idx: %d\n", i); > + ret = -EINVAL; > + } > + > + if (phys->hw_intf && phys->hw_wb) { > + DPU_ERROR_ENC(dpu_enc, > + "invalid phys both intf and wb block > at idx: %d\n", i); > + ret = -EINVAL; > + } > } > + > mutex_unlock(_enc->enc_lock); > > return ret; > -- > 2.7.4 > -- With best wishes Dmitry
Re: [PATCH] drm/amdgpu/display: fix build when CONFIG_DEBUG_FS is not set
On 2022-06-15 17:00, Alex Deucher wrote: > amdgpu_dm_crtc_late_register is only used when CONFIG_DEBUG_FS > so make it dependent on that. > > Fixes: 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm") > Cc: Bhanuprakash Modem > Cc: Harry Wentland > Cc: Arun R Murthy > Cc: Jani Nikula > Reported-by: Randy Dunlap > Reported-by: Nathan Chancellor > Signed-off-by: Alex Deucher > Tested-by: Nathan Chancellor # build > Link: https://lists.freedesktop.org/archives/dri-devel/2022-June/359496.html Reviewed-by: Harry Wentland Harry > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 > 1 file changed, 4 insertions(+) > > 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 c9004f7e700d..33cd7a3d4ecb 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -6594,12 +6594,14 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) > return >base; > } > > +#ifdef CONFIG_DEBUG_FS > static int amdgpu_dm_crtc_late_register(struct drm_crtc *crtc) > { > crtc_debugfs_init(crtc); > > return 0; > } > +#endif > > static inline int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable) > { > @@ -6693,7 +6695,9 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs > = { > .enable_vblank = dm_enable_vblank, > .disable_vblank = dm_disable_vblank, > .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp, > +#if defined(CONFIG_DEBUG_FS) > .late_register = amdgpu_dm_crtc_late_register, > +#endif > }; > > static enum drm_connector_status
[PATCH v2 3/3] drm/msm/dpu: remove hard-coded linewidth limit for writeback
Remove the hard-coded limit for writeback and lets start using the one from catalog instead. Fixes: d7d0e73f7de3 ("introduce the dpu_encoder_phys_* for writeback") Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index 59da348ff339..fc1d4fda69b5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -20,8 +20,6 @@ #include "dpu_crtc.h" #include "disp/msm_disp_snapshot.h" -#define DEFAULT_MAX_WRITEBACK_WIDTH 2048 - #define to_dpu_encoder_phys_wb(x) \ container_of(x, struct dpu_encoder_phys_wb, base) @@ -278,9 +276,9 @@ static int dpu_encoder_phys_wb_atomic_check( DPU_ERROR("invalid fb h=%d, mode h=%d\n", fb->height, mode->vdisplay); return -EINVAL; - } else if (fb->width > DEFAULT_MAX_WRITEBACK_WIDTH) { + } else if (fb->width > phys_enc->hw_wb->caps->maxlinewidth) { DPU_ERROR("invalid fb w=%d, maxlinewidth=%u\n", - fb->width, DEFAULT_MAX_WRITEBACK_WIDTH); + fb->width, phys_enc->hw_wb->caps->maxlinewidth); return -EINVAL; } -- 2.7.4
[PATCH v2 2/3] drm/msm/dpu: fix maxlinewidth for writeback block
Writeback block for sm8250 was using the default maxlinewidth of 2048. But this is not right as it supports upto 4096. This should have no effect on most resolutions as we are still limiting upto maxlinewidth of SSPP for adding the modes. Fix the maxlinewidth for writeback block on sm8250. Fixes: 53324b99bd7b ("add writeback blocks to the sm8250 DPU catalog") Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 400ebceb56bb..dd7537e32f88 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -1285,7 +1285,7 @@ static const struct dpu_intf_cfg qcm2290_intf[] = { * Writeback blocks config */ #define WB_BLK(_name, _id, _base, _features, _clk_ctrl, \ - __xin_id, vbif_id, _reg, _wb_done_bit) \ + __xin_id, vbif_id, _reg, _max_linewidth, _wb_done_bit) \ { \ .name = _name, .id = _id, \ .base = _base, .len = 0x2c8, \ @@ -1295,13 +1295,13 @@ static const struct dpu_intf_cfg qcm2290_intf[] = { .clk_ctrl = _clk_ctrl, \ .xin_id = __xin_id, \ .vbif_idx = vbif_id, \ - .maxlinewidth = DEFAULT_DPU_LINE_WIDTH, \ + .maxlinewidth = _max_linewidth, \ .intr_wb_done = DPU_IRQ_IDX(_reg, _wb_done_bit) \ } static const struct dpu_wb_cfg sm8250_wb[] = { WB_BLK("wb_2", WB_2, 0x65000, WB_SM8250_MASK, DPU_CLK_CTRL_WB2, 6, - VBIF_RT, MDP_SSPP_TOP0_INTR, 4), + VBIF_RT, MDP_SSPP_TOP0_INTR, 4096, 4), }; /* -- 2.7.4
[PATCH v2 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. changes in v2: - add phys->hw_intf and phys->hw_wb checks back Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 36 ++--- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..3be73211d631 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) - phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); - - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) - phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); - - if (!phys->hw_intf && !phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "no intf or wb block assigned at idx: %d\n", i); - return; - } - - if (phys->hw_intf && phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "invalid phys both intf and wb block at idx: %d\n", i); - return; - } - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,25 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(>vsync_cnt, 0); atomic_set(>underrun_cnt, 0); + + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) + phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); + + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); + + if (!phys->hw_intf && !phys->hw_wb) { + DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned at idx: %d\n", i); + ret = -EINVAL; + } + + if (phys->hw_intf && phys->hw_wb) { + DPU_ERROR_ENC(dpu_enc, + "invalid phys both intf and wb block at idx: %d\n", i); + ret = -EINVAL; + } } + mutex_unlock(_enc->enc_lock); return ret; -- 2.7.4
Re: [PATCH v3 01/14] dt-bindings: display/msm: hdmi: split and convert to yaml
On 09/06/2022 05:23, Dmitry Baryshkov wrote: > Convert Qualcomm HDMI binding into HDMI TX and PHY yaml bindings. > > Changes to schema: > HDMI: > - fixed reg-names numbering to match 0..3 instead 0,1,3,4 > - dropped qcom,tx-ddc-* from example, they were not documented > - make phy-names deprecated, drop it from the examples > > PHY: > - moved into phy/ directory > - split into QMP and non-QMP PHY schemas > > Co-developed-by: David Heidelberg > Signed-off-by: David Heidelberg > Signed-off-by: Dmitry Baryshkov Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
[PATCH] drm/amdgpu/display: fix build when CONFIG_DEBUG_FS is not set
amdgpu_dm_crtc_late_register is only used when CONFIG_DEBUG_FS so make it dependent on that. Fixes: 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm") Cc: Bhanuprakash Modem Cc: Harry Wentland Cc: Arun R Murthy Cc: Jani Nikula Reported-by: Randy Dunlap Reported-by: Nathan Chancellor Signed-off-by: Alex Deucher Tested-by: Nathan Chancellor # build Link: https://lists.freedesktop.org/archives/dri-devel/2022-June/359496.html --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 1 file changed, 4 insertions(+) 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 c9004f7e700d..33cd7a3d4ecb 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6594,12 +6594,14 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) return >base; } +#ifdef CONFIG_DEBUG_FS static int amdgpu_dm_crtc_late_register(struct drm_crtc *crtc) { crtc_debugfs_init(crtc); return 0; } +#endif static inline int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable) { @@ -6693,7 +6695,9 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { .enable_vblank = dm_enable_vblank, .disable_vblank = dm_disable_vblank, .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp, +#if defined(CONFIG_DEBUG_FS) .late_register = amdgpu_dm_crtc_late_register, +#endif }; static enum drm_connector_status -- 2.35.3
[pull] amdgpu drm-fixes-5.19
Hi Dave, Daniel, Fixes for 5.19. The following changes since commit 1f192b9e8d8a5c619b33a868fb1af063af65ce5d: Merge tag 'drm-misc-fixes-2022-06-09' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2022-06-10 13:29:22 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-fixes-5.19-2022-06-15 for you to fetch changes up to 4fd17f2ac0aa4e48823ac2ede5b050fb70300bf4: drm/amd/display: Cap OLED brightness per max frame-average luminance (2022-06-14 13:11:11 -0400) amd-drm-fixes-5.19-2022-06-15: amdgpu: - Fix regression in GTT size reporting - OLED backlight fix Michel Dänzer (1): drm/amdgpu: Fix GTT size reporting in amdgpu_ioctl Roman Li (1): drm/amd/display: Cap OLED brightness per max frame-average luminance drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 -- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 2 files changed, 4 insertions(+), 6 deletions(-)
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
On Wed, Jun 15, 2022 at 04:45:16PM -0400, Alex Deucher wrote: > On Wed, Jun 15, 2022 at 4:24 PM Nathan Chancellor wrote: > > > > On Wed, Jun 15, 2022 at 03:28:52PM -0400, Alex Deucher wrote: > > > On Wed, Jun 15, 2022 at 3:01 PM Randy Dunlap > > > wrote: > > > > > > > > > > > > > > > > On 6/14/22 23:01, Stephen Rothwell wrote: > > > > > Hi all, > > > > > > > > > > Changes since 20220614: > > > > > > > > > > > > > on i386: > > > > # CONFIG_DEBUG_FS is not set > > > > > > > > > > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In > > > > function ‘amdgpu_dm_crtc_late_register’: > > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: > > > > error: implicit declaration of function ‘crtc_debugfs_init’; did you > > > > mean ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] > > > > crtc_debugfs_init(crtc); > > > > ^ > > > > amdgpu_debugfs_init > > > > > > > > > > > > Full randconfig file is attached. > > > > > > I tried building with your config and I can't repro this. As Harry > > > noted, that function and the whole secure display feature depend on > > > debugfs. It should never be built without CONFIG_DEBUG_FS. See > > > drivers/gpu/drm/amd/display/Kconfig: > > > > > > > config DRM_AMD_SECURE_DISPLAY > > > > bool "Enable secure display support" > > > > default n > > > > depends on DEBUG_FS > > > > depends on DRM_AMD_DC_DCN > > > > help > > > > Choose this option if you want to > > > > support secure display > > > > > > > > This option enables the calculation > > > > of crc of specific region via debugfs. > > > > Cooperate with specific DMCU FW. > > > > > > amdgpu_dm_crtc_late_register is guarded by > > > CONIG_DRM_AMD_SECURE_DISPLAY. It's not clear to me how we could hit > > > this. > > > > I think the problem is that you are not looking at the right tree. > > > > The kernel test robot reported [1] [2] this error is caused by commit > > 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm"), which > > is in the drm-misc tree on the drm-misc-next branch. That change removes > > the #ifdef around amdgpu_dm_crtc_late_register(), meaning that > > crtc_debugfs_init() can be called without CONFIG_DRM_AMD_SECURE_DISPLAY > > and CONFIG_DEBUG_FS. > > > > $ git show -s --format='%h ("%s")' > > abf0ba5a34ea ("drm/bridge: it6505: Add missing CRYPTO_HASH dependency") > > > > $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig > > > > $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU > > > > $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function > > ‘amdgpu_dm_crtc_late_register’: > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6622:9: > > error: implicit declaration of function ‘crtc_debugfs_init’; did you mean > > ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] > >6622 | crtc_debugfs_init(crtc); > > | ^ > > | amdgpu_debugfs_init > > cc1: all warnings being treated as errors > > > > Contrast that with the current top of your tree: > > > > $ git show -s --format='%h ("%s")' > > c435f61d0eb3 ("drm/amd/display: Drop unnecessary guard from DC resource") > > > > $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig > > > > $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU > > > > $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o > > > > $ echo $? > > 0 > > > > Randy's patch [3] seems like it should resolve the issue just fine but > > it needs to be applied to drm-misc-next, not the amdgpu tree. > > Thanks for tracking this down. I think something like the attached > patch is cleaner since the whole thing is only valid for debugfs. Makes sense! I tested the below patch with and without DEBUG_FS and saw no errors. > From b0bcacd86344998e0ca757f89c6c4cd3b6298999 Mon Sep 17 00:00:00 2001 > From: Alex Deucher > Date: Wed, 15 Jun 2022 16:40:39 -0400 > Subject: [PATCH] drm/amdgpu/display: fix build when CONFIG_DEBUG_FS is not set > > amdgpu_dm_crtc_late_register is only used when CONFIG_DEBUG_FS > so make it dependent on that. > > Fixes: 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm") > Reported-by: Randy Dunlap > Reported-by: Nathan Chancellor Tested-by: Nathan Chancellor # build > Signed-off-by: Alex Deucher > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 > 1 file changed, 4 insertions(+) > > 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 c9004f7e700d..33cd7a3d4ecb 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
On Wed, Jun 15, 2022 at 4:24 PM Nathan Chancellor wrote: > > On Wed, Jun 15, 2022 at 03:28:52PM -0400, Alex Deucher wrote: > > On Wed, Jun 15, 2022 at 3:01 PM Randy Dunlap wrote: > > > > > > > > > > > > On 6/14/22 23:01, Stephen Rothwell wrote: > > > > Hi all, > > > > > > > > Changes since 20220614: > > > > > > > > > > on i386: > > > # CONFIG_DEBUG_FS is not set > > > > > > > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In > > > function ‘amdgpu_dm_crtc_late_register’: > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: > > > error: implicit declaration of function ‘crtc_debugfs_init’; did you mean > > > ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] > > > crtc_debugfs_init(crtc); > > > ^ > > > amdgpu_debugfs_init > > > > > > > > > Full randconfig file is attached. > > > > I tried building with your config and I can't repro this. As Harry > > noted, that function and the whole secure display feature depend on > > debugfs. It should never be built without CONFIG_DEBUG_FS. See > > drivers/gpu/drm/amd/display/Kconfig: > > > > > config DRM_AMD_SECURE_DISPLAY > > > bool "Enable secure display support" > > > default n > > > depends on DEBUG_FS > > > depends on DRM_AMD_DC_DCN > > > help > > > Choose this option if you want to > > > support secure display > > > > > > This option enables the calculation > > > of crc of specific region via debugfs. > > > Cooperate with specific DMCU FW. > > > > amdgpu_dm_crtc_late_register is guarded by > > CONIG_DRM_AMD_SECURE_DISPLAY. It's not clear to me how we could hit > > this. > > I think the problem is that you are not looking at the right tree. > > The kernel test robot reported [1] [2] this error is caused by commit > 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm"), which > is in the drm-misc tree on the drm-misc-next branch. That change removes > the #ifdef around amdgpu_dm_crtc_late_register(), meaning that > crtc_debugfs_init() can be called without CONFIG_DRM_AMD_SECURE_DISPLAY > and CONFIG_DEBUG_FS. > > $ git show -s --format='%h ("%s")' > abf0ba5a34ea ("drm/bridge: it6505: Add missing CRYPTO_HASH dependency") > > $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig > > $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU > > $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function > ‘amdgpu_dm_crtc_late_register’: > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6622:9: error: > implicit declaration of function ‘crtc_debugfs_init’; did you mean > ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] >6622 | crtc_debugfs_init(crtc); > | ^ > | amdgpu_debugfs_init > cc1: all warnings being treated as errors > > Contrast that with the current top of your tree: > > $ git show -s --format='%h ("%s")' > c435f61d0eb3 ("drm/amd/display: Drop unnecessary guard from DC resource") > > $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig > > $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU > > $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o > > $ echo $? > 0 > > Randy's patch [3] seems like it should resolve the issue just fine but > it needs to be applied to drm-misc-next, not the amdgpu tree. Thanks for tracking this down. I think something like the attached patch is cleaner since the whole thing is only valid for debugfs. Alex > > [1]: https://lore.kernel.org/202205241843.8ewkesia-...@intel.com/ > [2]: https://lore.kernel.org/202205240207.kmdlusrc-...@intel.com/ > [3]: https://lore.kernel.org/20220614155726.26211-1-rdun...@infradead.org/ > > Cheers, > Nathan From b0bcacd86344998e0ca757f89c6c4cd3b6298999 Mon Sep 17 00:00:00 2001 From: Alex Deucher Date: Wed, 15 Jun 2022 16:40:39 -0400 Subject: [PATCH] drm/amdgpu/display: fix build when CONFIG_DEBUG_FS is not set amdgpu_dm_crtc_late_register is only used when CONFIG_DEBUG_FS so make it dependent on that. Fixes: 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm") Reported-by: Randy Dunlap Reported-by: Nathan Chancellor Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 1 file changed, 4 insertions(+) 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 c9004f7e700d..33cd7a3d4ecb 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6594,12 +6594,14 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) return >base; } +#ifdef CONFIG_DEBUG_FS static int amdgpu_dm_crtc_late_register(struct drm_crtc
Re: [Linaro-mm-sig] Re: [PATCH] drivers: tty: serial: Add missing of_node_put() in serial-tegra.c
On Wed, 15 Jun 2022 at 22:23, Dave Airlie wrote: > > On Wed, 15 Jun 2022 at 20:53, Greg KH wrote: > > > > On Wed, Jun 15, 2022 at 06:48:33PM +0800, heliang wrote: > > > In tegra_uart_init(), of_find_matching_node() will return a node > > > pointer with refcount incremented. We should use of_node_put() > > > when it is not used anymore. > > > > > > Signed-off-by: heliang > > > > We need a real name please, one you sign documents with. > > How do we enforce that? What if Wong, Adele or Beyonce submit a patch? > > What happens if that patch gets reposted, with S-o-b: He Liang > or Hel Iang, Heli Ang? Do you know any of those are > real names? What happens if they post a real name in > Mandarin/Thai/Cyrillic, can you validate it? > > Really we require you have an identity attached to an email. If there > is a problem in the future, we'd prefer the email continues to work so > that you are contactable. If you are submitting a small amount of > changes it's probably never going to matter. If you are submitting > larger bodies of work of course it would be good to have a company or > larger org attached to track things down legally later, but again that > isn't always possible. > > I don't think alienating the numerous developers who no longer use > their legal names are identified by one name, but haven't changed > their legal one yet people who get married and change their legal name > but don't change their contribution name and I could run this sentence > on forever. Yeah like absolute best case trying to "enforce" this just results in encouraging people to come up with entirely fake but English looking names for themselves. Which ... just no. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
Hi-- On 6/15/22 13:13, Alex Deucher wrote: > On Wed, Jun 15, 2022 at 3:44 PM Randy Dunlap wrote: >> >> >> >> On 6/15/22 12:28, Alex Deucher wrote: >>> On Wed, Jun 15, 2022 at 3:01 PM Randy Dunlap wrote: On 6/14/22 23:01, Stephen Rothwell wrote: > Hi all, > > Changes since 20220614: > on i386: # CONFIG_DEBUG_FS is not set ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function ‘amdgpu_dm_crtc_late_register’: ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: error: implicit declaration of function ‘crtc_debugfs_init’; did you mean ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] crtc_debugfs_init(crtc); ^ amdgpu_debugfs_init Full randconfig file is attached. >>> >>> I tried building with your config and I can't repro this. As Harry >>> noted, that function and the whole secure display feature depend on >>> debugfs. It should never be built without CONFIG_DEBUG_FS. See >>> drivers/gpu/drm/amd/display/Kconfig: >> >> Did you try building with today's linux-next tree? >> (whatever is in it) >> >> I have seen this build error multiple times so it shouldn't >> be so difficult to repro it. >> >> config DRM_AMD_SECURE_DISPLAY bool "Enable secure display support" default n depends on DEBUG_FS depends on DRM_AMD_DC_DCN help Choose this option if you want to support secure display This option enables the calculation of crc of specific region via debugfs. Cooperate with specific DMCU FW. >>> >>> amdgpu_dm_crtc_late_register is guarded by >>> CONIG_DRM_AMD_SECURE_DISPLAY. It's not clear to me how we could hit >>> this. > I was just about to ask what the paragraph above means. It was confusing to say the least. > I was able to repro it. In linux-next the > CONFIG_DRM_AMD_SECURE_DISPLAY ifdefs in > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c seem to be missing. > I guess they were lost when the amdgpu branch was merged into > linux-next. The attached patch restores the > CONFIG_DRM_AMD_SECURE_DISPLAY protections. OK, that builds for me. Thanks. -- ~Randy
Re: [PATCH] drm/dp/mst: Read the extended DPCD capabilities during system resume
If you add a Cc: to sta...@vger.kernel.org then this is: Reviewed-by: Lyude Paul I assume you have the privileges to push this to drm-misc-next On Tue, 2022-06-14 at 12:45 +0300, Imre Deak wrote: > The WD22TB4 Thunderbolt dock at least will revert its DP_MAX_LINK_RATE > from HBR3 to HBR2 after system suspend/resume if the DP_DP13_DPCD_REV > registers are not read subsequently also as required. > > Fix this by reading DP_DP13_DPCD_REV registers as well, matching what is > done during connector detection. While at it also fix up the same call > in drm_dp_mst_dump_topology(). > > Cc: Lyude Paul > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292 > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c > b/drivers/gpu/drm/display/drm_dp_mst_topology.c > index 67b3b9697da7f..18f2b6075b780 100644 > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > @@ -3860,9 +3860,7 @@ int drm_dp_mst_topology_mgr_resume(struct > drm_dp_mst_topology_mgr *mgr, > if (!mgr->mst_primary) > goto out_fail; > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd, > - DP_RECEIVER_CAP_SIZE); > - if (ret != DP_RECEIVER_CAP_SIZE) { > + if (drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd) < 0) { > drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during > suspend?\n"); > goto out_fail; > } > @@ -4911,8 +4909,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > u8 buf[DP_PAYLOAD_TABLE_SIZE]; > int ret; > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, > DP_RECEIVER_CAP_SIZE); > - if (ret) { > + if (drm_dp_read_dpcd_caps(mgr->aux, buf) < 0) { > seq_printf(m, "dpcd read failed\n"); > goto out; > } -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
On Wed, Jun 15, 2022 at 03:28:52PM -0400, Alex Deucher wrote: > On Wed, Jun 15, 2022 at 3:01 PM Randy Dunlap wrote: > > > > > > > > On 6/14/22 23:01, Stephen Rothwell wrote: > > > Hi all, > > > > > > Changes since 20220614: > > > > > > > on i386: > > # CONFIG_DEBUG_FS is not set > > > > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function > > ‘amdgpu_dm_crtc_late_register’: > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: > > error: implicit declaration of function ‘crtc_debugfs_init’; did you mean > > ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] > > crtc_debugfs_init(crtc); > > ^ > > amdgpu_debugfs_init > > > > > > Full randconfig file is attached. > > I tried building with your config and I can't repro this. As Harry > noted, that function and the whole secure display feature depend on > debugfs. It should never be built without CONFIG_DEBUG_FS. See > drivers/gpu/drm/amd/display/Kconfig: > > > config DRM_AMD_SECURE_DISPLAY > > bool "Enable secure display support" > > default n > > depends on DEBUG_FS > > depends on DRM_AMD_DC_DCN > > help > > Choose this option if you want to > > support secure display > > > > This option enables the calculation > > of crc of specific region via debugfs. > > Cooperate with specific DMCU FW. > > amdgpu_dm_crtc_late_register is guarded by > CONIG_DRM_AMD_SECURE_DISPLAY. It's not clear to me how we could hit > this. I think the problem is that you are not looking at the right tree. The kernel test robot reported [1] [2] this error is caused by commit 4cd79f614b50 ("drm/amd/display: Move connector debugfs to drm"), which is in the drm-misc tree on the drm-misc-next branch. That change removes the #ifdef around amdgpu_dm_crtc_late_register(), meaning that crtc_debugfs_init() can be called without CONFIG_DRM_AMD_SECURE_DISPLAY and CONFIG_DEBUG_FS. $ git show -s --format='%h ("%s")' abf0ba5a34ea ("drm/bridge: it6505: Add missing CRYPTO_HASH dependency") $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function ‘amdgpu_dm_crtc_late_register’: drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6622:9: error: implicit declaration of function ‘crtc_debugfs_init’; did you mean ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] 6622 | crtc_debugfs_init(crtc); | ^ | amdgpu_debugfs_init cc1: all warnings being treated as errors Contrast that with the current top of your tree: $ git show -s --format='%h ("%s")' c435f61d0eb3 ("drm/amd/display: Drop unnecessary guard from DC resource") $ make -skj"$(nproc)" ARCH=x86_64 mrproper defconfig $ scripts/config -d BLK_DEV_IO_TRACE -d DEBUG_FS -e DRM_AMDGPU $ make -skj"$(nproc)" ARCH=x86_64 olddefconfig drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o $ echo $? 0 Randy's patch [3] seems like it should resolve the issue just fine but it needs to be applied to drm-misc-next, not the amdgpu tree. [1]: https://lore.kernel.org/202205241843.8ewkesia-...@intel.com/ [2]: https://lore.kernel.org/202205240207.kmdlusrc-...@intel.com/ [3]: https://lore.kernel.org/20220614155726.26211-1-rdun...@infradead.org/ Cheers, Nathan
Re: [Linaro-mm-sig] Re: [PATCH] drivers: tty: serial: Add missing of_node_put() in serial-tegra.c
On Wed, 15 Jun 2022 at 20:53, Greg KH wrote: > > On Wed, Jun 15, 2022 at 06:48:33PM +0800, heliang wrote: > > In tegra_uart_init(), of_find_matching_node() will return a node > > pointer with refcount incremented. We should use of_node_put() > > when it is not used anymore. > > > > Signed-off-by: heliang > > We need a real name please, one you sign documents with. How do we enforce that? What if Wong, Adele or Beyonce submit a patch? What happens if that patch gets reposted, with S-o-b: He Liang or Hel Iang, Heli Ang? Do you know any of those are real names? What happens if they post a real name in Mandarin/Thai/Cyrillic, can you validate it? Really we require you have an identity attached to an email. If there is a problem in the future, we'd prefer the email continues to work so that you are contactable. If you are submitting a small amount of changes it's probably never going to matter. If you are submitting larger bodies of work of course it would be good to have a company or larger org attached to track things down legally later, but again that isn't always possible. I don't think alienating the numerous developers who no longer use their legal names are identified by one name, but haven't changed their legal one yet people who get married and change their legal name but don't change their contribution name and I could run this sentence on forever. Dave.
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
On Wed, Jun 15, 2022 at 3:44 PM Randy Dunlap wrote: > > > > On 6/15/22 12:28, Alex Deucher wrote: > > On Wed, Jun 15, 2022 at 3:01 PM Randy Dunlap wrote: > >> > >> > >> > >> On 6/14/22 23:01, Stephen Rothwell wrote: > >>> Hi all, > >>> > >>> Changes since 20220614: > >>> > >> > >> on i386: > >> # CONFIG_DEBUG_FS is not set > >> > >> > >> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In > >> function ‘amdgpu_dm_crtc_late_register’: > >> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: > >> error: implicit declaration of function ‘crtc_debugfs_init’; did you mean > >> ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] > >> crtc_debugfs_init(crtc); > >> ^ > >> amdgpu_debugfs_init > >> > >> > >> Full randconfig file is attached. > > > > I tried building with your config and I can't repro this. As Harry > > noted, that function and the whole secure display feature depend on > > debugfs. It should never be built without CONFIG_DEBUG_FS. See > > drivers/gpu/drm/amd/display/Kconfig: > > Did you try building with today's linux-next tree? > (whatever is in it) > > I have seen this build error multiple times so it shouldn't > be so difficult to repro it. > > > >> config DRM_AMD_SECURE_DISPLAY > >> bool "Enable secure display support" > >> default n > >> depends on DEBUG_FS > >> depends on DRM_AMD_DC_DCN > >> help > >> Choose this option if you want to > >> support secure display > >> > >> This option enables the calculation > >> of crc of specific region via debugfs. > >> Cooperate with specific DMCU FW. > > > > amdgpu_dm_crtc_late_register is guarded by > > CONIG_DRM_AMD_SECURE_DISPLAY. It's not clear to me how we could hit > > this. I was able to repro it. In linux-next the CONFIG_DRM_AMD_SECURE_DISPLAY ifdefs in drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c seem to be missing. I guess they were lost when the amdgpu branch was merged into linux-next. The attached patch restores the CONFIG_DRM_AMD_SECURE_DISPLAY protections. Thanks, Alex > > > -- > ~Randy From f67ab0ba8da873a238dcd336acf92c01e4aff031 Mon Sep 17 00:00:00 2001 From: Alex Deucher Date: Wed, 15 Jun 2022 16:12:00 -0400 Subject: [PATCH] drm/amdgpu: fix merge of amdgpu -next branch Add CONFIG_DRM_AMD_SECURE_DISPLAY protections that were accidently dropped. Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 1 file changed, 4 insertions(+) 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 c9004f7e700d..c0386f237fdc 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6594,12 +6594,14 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) return >base; } +#ifdef CONFIG_DRM_AMD_SECURE_DISPLAY static int amdgpu_dm_crtc_late_register(struct drm_crtc *crtc) { crtc_debugfs_init(crtc); return 0; } +#endif static inline int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable) { @@ -6693,7 +6695,9 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { .enable_vblank = dm_enable_vblank, .disable_vblank = dm_disable_vblank, .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp, +#if defined(CONFIG_DRM_AMD_SECURE_DISPLAY) .late_register = amdgpu_dm_crtc_late_register, +#endif }; static enum drm_connector_status -- 2.35.3
Re: [PATCH 0/8] drm: Clean up drm_crtc.h
Hi Ville, On Mon, Jun 13, 2022 at 11:03:09PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Eliminate unnecessary includes from drm_crtc.h to avoid > pointless rebuilds of the entire universe when touching > some random header. > > I didn't really feel like splitting this up per-driver since > that would have ended up being metric ton of one liners. > I'm thinking the conflicts (if any) should be trivial enough > to deal with even with bigger patches. Thanks for doing this! I would have been fine with a single bigger commit, as this is tedious manually jobs. It really does not make much sense if there is added 1 or 4 includes files when reviewing. And the bots will tell if build is broken somewhere. If we relax a little in the patch granularity, the chances we will see more similar cleanups are higher. I looked at a few - for the rest I trust the robots. So with the reported builderrros fixed and my alphabetic order comment addressed, then the full series are: Acked-by: Sam Ravnborg Sam
Re: [PATCH 3/8] drm: Drop drm_blend.h from drm_crtc.h
Hi Ville. On Mon, Jun 13, 2022 at 11:03:12PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > drm_crtc.h has no need for drm_blend.h, so don't include it. > Avoids useless rebuilds of the entire universe when > touching drm_blend.h. s/drm_blend.h/drm_crtc.h/ > > Quite a few placs do currently depend on drm_blend.h without > actually including it directly. All of those need to be fixed > up. > > Signed-off-by: Ville Syrjälä With the commit message fixed: Acked-by: Sam Ravnborg
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
On 6/15/22 12:28, Alex Deucher wrote: > On Wed, Jun 15, 2022 at 3:01 PM Randy Dunlap wrote: >> >> >> >> On 6/14/22 23:01, Stephen Rothwell wrote: >>> Hi all, >>> >>> Changes since 20220614: >>> >> >> on i386: >> # CONFIG_DEBUG_FS is not set >> >> >> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function >> ‘amdgpu_dm_crtc_late_register’: >> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: >> error: implicit declaration of function ‘crtc_debugfs_init’; did you mean >> ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] >> crtc_debugfs_init(crtc); >> ^ >> amdgpu_debugfs_init >> >> >> Full randconfig file is attached. > > I tried building with your config and I can't repro this. As Harry > noted, that function and the whole secure display feature depend on > debugfs. It should never be built without CONFIG_DEBUG_FS. See > drivers/gpu/drm/amd/display/Kconfig: Did you try building with today's linux-next tree? (whatever is in it) I have seen this build error multiple times so it shouldn't be so difficult to repro it. >> config DRM_AMD_SECURE_DISPLAY >> bool "Enable secure display support" >> default n >> depends on DEBUG_FS >> depends on DRM_AMD_DC_DCN >> help >> Choose this option if you want to >> support secure display >> >> This option enables the calculation >> of crc of specific region via debugfs. >> Cooperate with specific DMCU FW. > > amdgpu_dm_crtc_late_register is guarded by > CONIG_DRM_AMD_SECURE_DISPLAY. It's not clear to me how we could hit > this. -- ~Randy
Re: [PATCH v3 2/8] drm: Drop drm_framebuffer.h from drm_crtc.h
On Tue, Jun 14, 2022 at 12:54:49PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > drm_crtc.h has no need for drm_frambuffer.h, so don't include it. > Avoids useless rebuilds of the entire universe when > touching drm_framebuffer.h. > > Quite a few placs do currently depend on drm_framebuffer.h without > actually including it directly. All of those need to be fixed > up. > > v2: Fix up msm some more > v2: Deal with ingenic and shmobile as well > > Signed-off-by: Ville Syrjälä Acked-by: Sam Ravnborg > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 + > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h | 1 + > drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c | 1 + > drivers/gpu/drm/arm/hdlcd_crtc.c | 1 + > drivers/gpu/drm/arm/malidp_crtc.c| 1 + > drivers/gpu/drm/arm/malidp_mw.c | 1 + > drivers/gpu/drm/arm/malidp_planes.c | 1 + > drivers/gpu/drm/armada/armada_fb.h | 2 ++ > drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 1 + > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 1 + > drivers/gpu/drm/drm_atomic.c | 1 + > drivers/gpu/drm/drm_atomic_helper.c | 1 + > drivers/gpu/drm/drm_atomic_state_helper.c| 1 + > drivers/gpu/drm/drm_atomic_uapi.c| 1 + > drivers/gpu/drm/drm_crtc.c | 1 + > drivers/gpu/drm/drm_crtc_helper.c| 1 + > drivers/gpu/drm/drm_damage_helper.c | 1 + > drivers/gpu/drm/drm_fb_helper.c | 1 + > drivers/gpu/drm/drm_gem_atomic_helper.c | 1 + > drivers/gpu/drm/drm_mipi_dbi.c | 1 + > drivers/gpu/drm/drm_mode_config.c| 1 + > drivers/gpu/drm/drm_modeset_helper.c | 1 + > drivers/gpu/drm/drm_writeback.c | 1 + > drivers/gpu/drm/exynos/exynos5433_drm_decon.c| 1 + > drivers/gpu/drm/exynos/exynos7_drm_decon.c | 1 + > drivers/gpu/drm/exynos/exynos_drm_fb.c | 1 + > drivers/gpu/drm/exynos/exynos_drm_fbdev.c| 1 + > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 1 + > drivers/gpu/drm/exynos/exynos_drm_plane.c| 1 + > drivers/gpu/drm/exynos/exynos_drm_vidi.c | 1 + > drivers/gpu/drm/exynos/exynos_mixer.c| 1 + > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 1 + > drivers/gpu/drm/gma500/framebuffer.c | 1 + > drivers/gpu/drm/gma500/gma_display.c | 1 + > drivers/gpu/drm/gma500/oaktrail_crtc.c | 1 + > drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 1 + > drivers/gpu/drm/i915/display/intel_display_types.h | 1 + > drivers/gpu/drm/imx/dcss/dcss-plane.c| 1 + > drivers/gpu/drm/imx/ipuv3-plane.c| 1 + > drivers/gpu/drm/ingenic/ingenic-drm-drv.c| 1 + > drivers/gpu/drm/ingenic/ingenic-ipu.c| 1 + > drivers/gpu/drm/kmb/kmb_plane.c | 1 + > drivers/gpu/drm/logicvc/logicvc_layer.c | 1 + > drivers/gpu/drm/mcde/mcde_display.c | 1 + > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 1 + > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 2 ++ > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 1 + > drivers/gpu/drm/meson/meson_overlay.c| 1 + > drivers/gpu/drm/meson/meson_plane.c | 1 + > drivers/gpu/drm/mgag200/mgag200_mode.c | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 2 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c| 1 + > drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 1 + > drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 1 + > drivers/gpu/drm/msm/disp/mdp_format.c| 2 ++ > drivers/gpu/drm/msm/msm_debugfs.c| 1 + > drivers/gpu/drm/msm/msm_fb.c | 1 + > drivers/gpu/drm/msm/msm_fbdev.c | 1 + > drivers/gpu/drm/mxsfb/mxsfb_kms.c| 1 + > drivers/gpu/drm/omapdrm/omap_debugfs.c | 1 + > drivers/gpu/drm/omapdrm/omap_fb.c| 1 + > drivers/gpu/drm/omapdrm/omap_fbdev.c | 1 + > drivers/gpu/drm/omapdrm/omap_plane.c | 1 + > drivers/gpu/drm/pl111/pl111_display.c| 1 + > drivers/gpu/drm/pl111/pl111_drv.c
Re: [PATCH 1/8] drm: Drop drm_edid.h from drm_crtc.h
Hi Ville, On Mon, Jun 13, 2022 at 11:03:10PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > drm_crtc.h has no need for drm_edid.h, so don't include it. > Avoids useless rebuilds of the entire universe when > touching drm_edid.h. > > Quite a few placs do currently depend on drm_edid.h without > actually including it directly. All of those need to be fixed > up. This is a very nice reduction in implicit includes. > > diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c > index 204c869d9fe2..43de2ac8f27e 100644 > --- a/drivers/gpu/drm/arm/malidp_mw.c > +++ b/drivers/gpu/drm/arm/malidp_mw.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > #include Please maintain the alphabetic order of the includes. This reduces the risk of conflict when we have multiple edits in different parallel commits. > diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c > index 14a058a42854..4f2fd69c4a4e 100644 > --- a/drivers/gpu/drm/imx/imx-ldb.c > +++ b/drivers/gpu/drm/imx/imx-ldb.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include ditto > #include > #include > #include > diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c > index 2b1fdf2cbbce..9a2fa352a433 100644 > --- a/drivers/gpu/drm/imx/imx-tve.c > +++ b/drivers/gpu/drm/imx/imx-tve.c > @@ -19,6 +19,7 @@ > > #include > #include > +#include ditto > #include > #include > #include > diff --git a/drivers/gpu/drm/imx/parallel-display.c > b/drivers/gpu/drm/imx/parallel-display.c > index 63ba2ad84679..5a91a5c82057 100644 > --- a/drivers/gpu/drm/imx/parallel-display.c > +++ b/drivers/gpu/drm/imx/parallel-display.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include ditto > #include > #include > #include > diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c > b/drivers/gpu/drm/vboxvideo/vbox_mode.c > index 4017b0a621fc..52eaa10712ec 100644 > --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c > +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c > @@ -16,6 +16,7 @@ > #include > #include > #include > +#include ditto > #include > #include > #include With the above fixed: Acked-by: Sam Ravnborg
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
On Wed, Jun 15, 2022 at 3:01 PM Randy Dunlap wrote: > > > > On 6/14/22 23:01, Stephen Rothwell wrote: > > Hi all, > > > > Changes since 20220614: > > > > on i386: > # CONFIG_DEBUG_FS is not set > > > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function > ‘amdgpu_dm_crtc_late_register’: > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: error: > implicit declaration of function ‘crtc_debugfs_init’; did you mean > ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] > crtc_debugfs_init(crtc); > ^ > amdgpu_debugfs_init > > > Full randconfig file is attached. I tried building with your config and I can't repro this. As Harry noted, that function and the whole secure display feature depend on debugfs. It should never be built without CONFIG_DEBUG_FS. See drivers/gpu/drm/amd/display/Kconfig: > config DRM_AMD_SECURE_DISPLAY > bool "Enable secure display support" > default n > depends on DEBUG_FS > depends on DRM_AMD_DC_DCN > help > Choose this option if you want to > support secure display > > This option enables the calculation > of crc of specific region via debugfs. > Cooperate with specific DMCU FW. amdgpu_dm_crtc_late_register is guarded by CONIG_DRM_AMD_SECURE_DISPLAY. It's not clear to me how we could hit this. Alex > > -- > ~Randy
Re: [PATCH 01/19] drm/gma500: Unify *_lvds_get_max_backlight()
Hi Patrik, > > > > With or without this change the patch is: > > Reviewed-by: Sam Ravnborg > > Hi Sam, > Thanks for having a look. > > I've intentionally tried to change as little as possible from the > version I copied so that any functional change is easy to spot and the > series becomes easy to review. Would it be ok if I do cleanups as a > followup series? That would be perfect! Sam
Re: [Intel-gfx] [PATCH v7] drm/i915/display: disable HPD workers before display driver unregister
On Tue, Jun 14, 2022 at 05:06:40PM +0200, Andrzej Hajda wrote: > On 10.06.2022 20:37, Ville Syrjälä wrote: > > On Fri, Jun 10, 2022 at 06:00:24PM +0200, Andrzej Hajda wrote: > >> Handling HPD during driver removal is pointless, and can cause different > >> use-after-free/concurrency issues: > >> 1. Setup of deferred fbdev after fbdev unregistration. > >> 2. Access to DP-AUX after DP-AUX removal. > >> > >> Below stacktraces of both cases observed on CI: > >> > >> [272.634530] general protection fault, probably for non-canonical address > >> 0x6b6b6b6b6b6b6b6b: [#1] PREEMPT SMP NOPTI > >> [272.634536] CPU: 0 PID: 6030 Comm: i915_selftest Tainted: G U > >>5.18.0-rc5-CI_DRM_11603-g12dccf4f5eef+ #1 > >> [272.634541] Hardware name: Intel Corporation Raptor Lake Client > >> Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.2397.A01.2109300731 > >> 09/30/2021 > >> [272.634545] RIP: 0010:fb_do_apertures_overlap.part.14+0x26/0x60 > >> ... > >> [272.634582] Call Trace: > >> [272.634583] > >> [272.634585] do_remove_conflicting_framebuffers+0x59/0xa0 > >> [272.634589] remove_conflicting_framebuffers+0x2d/0xc0 > >> [272.634592] remove_conflicting_pci_framebuffers+0xc8/0x110 > >> [272.634595] drm_aperture_remove_conflicting_pci_framebuffers+0x52/0x70 > >> [272.634604] i915_driver_probe+0x63a/0xdd0 [i915] > >> > >> [283.405824] cpu_latency_qos_update_request called for unknown object > >> [283.405866] WARNING: CPU: 2 PID: 240 at kernel/power/qos.c:296 > >> cpu_latency_qos_update_request+0x2d/0x100 > >> [283.405912] CPU: 2 PID: 240 Comm: kworker/u64:9 Not tainted > >> 5.18.0-rc6-Patchwork_103738v3-g1672d1c43e43+ #1 > >> [283.405915] Hardware name: Intel Corporation Raptor Lake Client > >> Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.2397.A01.2109300731 > >> 09/30/2021 > >> [283.405916] Workqueue: i915-dp i915_digport_work_func [i915] > >> [283.406020] RIP: 0010:cpu_latency_qos_update_request+0x2d/0x100 > >> ... > >> [283.406040] Call Trace: > >> [283.406041] > >> [283.406044] intel_dp_aux_xfer+0x60e/0x8e0 [i915] > >> [283.406131] ? finish_swait+0x80/0x80 > >> [283.406139] intel_dp_aux_transfer+0xc5/0x2b0 [i915] > >> [283.406218] drm_dp_dpcd_access+0x79/0x130 [drm_display_helper] > >> [283.406227] drm_dp_dpcd_read+0xe2/0xf0 [drm_display_helper] > >> [283.406233] intel_dp_hpd_pulse+0x134/0x570 [i915] > >> [283.406308] ? __down_killable+0x70/0x140 > >> [283.406313] i915_digport_work_func+0xba/0x150 [i915] > >> > >> Signed-off-by: Andrzej Hajda > >> --- > >> Hi All, > >> > >> I am not sure about changes in shutdown path, any comments welcome. > >> I suspect suspend path have also some common bits, but I am little > >> bit afraid of touching it. > >> > >> Changes: > >> v1 - v6: > >> - chasing the bug appearing only on public CI. > >> v7: > >> - shutdown path adjusted (suggested by Jani) > >> > >> Regards > >> Andrzej > >> --- > >> drivers/gpu/drm/i915/display/intel_display.c | 11 --- > >> drivers/gpu/drm/i915/i915_driver.c | 5 ++--- > >> 2 files changed, 6 insertions(+), 10 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c > >> b/drivers/gpu/drm/i915/display/intel_display.c > >> index 186b37925d23f2..f9952ee8289fb2 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display.c > >> +++ b/drivers/gpu/drm/i915/display/intel_display.c > >> @@ -10490,13 +10490,6 @@ void intel_modeset_driver_remove_noirq(struct > >> drm_i915_private *i915) > >> */ > >>intel_hpd_poll_fini(i915); > >> > >> - /* > >> - * MST topology needs to be suspended so we don't have any calls to > >> - * fbdev after it's finalized. MST will be destroyed later as part of > >> - * drm_mode_config_cleanup() > >> - */ > >> - intel_dp_mst_suspend(i915); > >> - > >>/* poll work can call into fbdev, hence clean that up afterwards */ > >>intel_fbdev_fini(i915); > >> > >> @@ -10588,6 +10581,10 @@ void intel_display_driver_unregister(struct > >> drm_i915_private *i915) > >>if (!HAS_DISPLAY(i915)) > >>return; > >> > >> + intel_dp_mst_suspend(i915); > >> + intel_hpd_cancel_work(i915); > >> + drm_kms_helper_poll_disable(>drm); > >> + > >>intel_fbdev_unregister(i915); > >>intel_audio_deinit(i915); > >> > >> diff --git a/drivers/gpu/drm/i915/i915_driver.c > >> b/drivers/gpu/drm/i915/i915_driver.c > >> index d26dcca7e654aa..82cdccf072e2bc 100644 > >> --- a/drivers/gpu/drm/i915/i915_driver.c > >> +++ b/drivers/gpu/drm/i915/i915_driver.c > >> @@ -1070,15 +1070,14 @@ void i915_driver_shutdown(struct drm_i915_private > >> *i915) > >>i915_gem_suspend(i915); > >> > >>if (HAS_DISPLAY(i915)) { > >> + intel_dp_mst_suspend(i915); > >> + intel_hpd_cancel_work(i915); > >>drm_kms_helper_poll_disable(>drm); > >> > >>drm_atomic_helper_shutdown(>drm); > > > > You can't suspend MST before this since this is what actually turns the > > displays
Re: linux-next: Tree for Jun 15 (drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c)
On 6/14/22 23:01, Stephen Rothwell wrote: > Hi all, > > Changes since 20220614: > on i386: # CONFIG_DEBUG_FS is not set ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function ‘amdgpu_dm_crtc_late_register’: ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6599:2: error: implicit declaration of function ‘crtc_debugfs_init’; did you mean ‘amdgpu_debugfs_init’? [-Werror=implicit-function-declaration] crtc_debugfs_init(crtc); ^ amdgpu_debugfs_init Full randconfig file is attached. -- ~Randy config-amdgpu-not-debugfs.gz Description: application/gzip
Re: [Freedreno] [PATCH v2 1/5] drm/msm/dpu: check both DPU and MDSS devices for the IOMMU
On 15/06/2022 20:55, Abhinav Kumar wrote: On 5/4/2022 5:16 PM, Dmitry Baryshkov wrote: Follow the lead of MDP5 driver and check both DPU and MDSS devices for the IOMMU specifiers. Historically DPU devices had IOMMU specified in the MDSS device tree node, but as some of MDP5 devices are being converted to the supported by the DPU driver, the driver should adapt and check both devices. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 143d6643be53..5ccda0766f6c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1004,14 +1004,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) struct msm_mmu *mmu; struct device *dpu_dev = dpu_kms->dev->dev; struct device *mdss_dev = dpu_dev->parent; + struct device *iommu_dev; domain = iommu_domain_alloc(_bus_type); if (!domain) return 0; - /* IOMMUs are a part of MDSS device tree binding, not the - * MDP/DPU device. */ - mmu = msm_iommu_new(mdss_dev, domain); + /* + * IOMMUs can be a part of MDSS device tree binding, or the + * MDP/DPU device. + */ Can you please explain this a little more? So even if some of the mdp5 devices are getting converted to use DPU driver, their device trees are also updated right? No. The DT describes the device, not the Linux drivers. So while updating the drivers we should not change the DT. In other words, if DPU driver was using mdss_dev to initialize the iommu, why should the new devices which are going to use DPU have the binding in the dpu_dev? + if (dev_iommu_fwspec_get(dpu_dev)) + iommu_dev = dpu_dev; + else + iommu_dev = mdss_dev; + + mmu = msm_iommu_new(iommu_dev, domain); if (IS_ERR(mmu)) { iommu_domain_free(domain); return PTR_ERR(mmu); -- With best wishes Dmitry
Re: [PATCH 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
On 6/15/2022 11:34 AM, Dmitry Baryshkov wrote: On 15/06/2022 20:11, Abhinav Kumar wrote: On 6/15/2022 10:04 AM, Dmitry Baryshkov wrote: On 15/06/2022 19:40, Abhinav Kumar wrote: On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: On 14/06/2022 22:32, Abhinav Kumar wrote: intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..e991d4ba8a40 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) - phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); - - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) - phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); - - if (!phys->hw_intf && !phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "no intf or wb block assigned at idx: %d\n", i); - return; - } - - if (phys->hw_intf && phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "invalid phys both intf and wb block at idx: %d\n", i); - return; - } Please retain these checks in dpu_encoder_setup_display(). It checks that we really have got the intf or wb. For example one might have specified the INTF that leads to INTF_NONE interface. Or non-existing/not supported WB. Right, so the reason I omitted that was dpu_encoder_setup_display() already has these checks: https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 Please check lines 2273-2284. Only if all those checks succeeded we call dpu_encoder_virt_add_phys_encs which increments num_phys_encs. As I wrote, it checks indices from phys_params, but not the acquired hardware instances. Right but today, both the get_intf() and get_wb() just return the intf/wb corresponding to the index. So as long as the index is valid how will checking hw_wb or hw_intf be different? static inline struct dpu_hw_intf *dpu_rm_get_intf(struct dpu_rm *rm, enum dpu_intf intf_idx) { return rm->hw_intf[intf_idx - INTF_0]; } /** * dpu_rm_get_wb - Return a struct dpu_hw_wb instance given it's index. * @rm: DPU Resource Manager handle * @wb_idx: WB index */ static inline struct dpu_hw_wb *dpu_rm_get_wb(struct dpu_rm *rm, enum dpu_wb wb_idx) { return rm->hw_wb[wb_idx - WB_0]; } WB_0 is valid, but dpu_rm_get_wb(WB_0) will return NULL. INTF_0 is valid, but dpu_rm_get_intf(INTF_0) on qcm2290 will return NULL. Etc. Ah okay got it, let me add them back in v2. Thats why I dropped those. Let me know if you have more questions. - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(>vsync_cnt, 0); atomic_set(>underrun_cnt, 0); + + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) + phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); + + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); } + mutex_unlock(_enc->enc_lock); return ret;
[PATCH v2 1/2] dt-bindings: arm: sunxi: Add binding for RenewWorldOutReach R16-Vista-E board
Add a binding for the RenewWorldOutReach R16-Vista-E board based on allwinner R16. Signed-off-by: Christopher Vollo Signed-off-by: Jagan Teki Signed-off-by: Suniel Mahesh --- Changes for v2: - Add missing compatible string - insert missing signatures of contributors --- Documentation/devicetree/bindings/arm/sunxi.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml index 95278a6a9a8e..52b8c9aba6f3 100644 --- a/Documentation/devicetree/bindings/arm/sunxi.yaml +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml @@ -787,6 +787,12 @@ properties: - const: allwinner,r7-tv-dongle - const: allwinner,sun5i-a10s + - description: RenewWorldOutreach R16-Vista-E +items: + - const: renewworldoutreach,r16-vista-e + - const: allwinner,sun8i-r16 + - const: allwinner,sun8i-a33 + - description: RerVision H3-DVK items: - const: rervision,h3-dvk -- 2.25.1
[PATCH] drivers: tty: serial: Add missing of_node_put() in serial-tegra.c
In tegra_uart_init(), of_find_matching_node() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: Liang He --- drivers/tty/serial/serial-tegra.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c index d942ab152f5a..5c4850a3762c 100644 --- a/drivers/tty/serial/serial-tegra.c +++ b/drivers/tty/serial/serial-tegra.c @@ -1667,6 +1667,7 @@ static int __init tegra_uart_init(void) node = of_find_matching_node(NULL, tegra_uart_of_match); if (node) match = of_match_node(tegra_uart_of_match, node); + of_node_put(node); if (match) cdata = match->data; if (cdata) -- 2.25.1
[PATCH] drm: check for allocation failure of kstrdup_const()
As the possible failure of the kstrdup_const(), it should be better to check it and handle it. Signed-off-by: Li Qiong --- drivers/gpu/drm/drm_managed.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c index 4cf214de50c4..d9bd4a9da559 100644 --- a/drivers/gpu/drm/drm_managed.c +++ b/drivers/gpu/drm/drm_managed.c @@ -151,6 +151,8 @@ int __drmm_add_action(struct drm_device *dev, } dr->node.name = kstrdup_const(name, GFP_KERNEL); + if (!dr->node.name) + return -ENOMEM; if (data) { void_ptr = (void **)>data; *void_ptr = data; @@ -197,6 +199,8 @@ void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) return NULL; } dr->node.name = kstrdup_const("kmalloc", GFP_KERNEL); + if (!dr->node.name) + return NULL; add_dr(dev, dr); -- 2.25.1
[PATCH] drivers: tty: serial: Add missing of_node_put() in serial-tegra.c
In tegra_uart_init(), of_find_matching_node() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: heliang --- drivers/tty/serial/serial-tegra.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c index d942ab152f5a..5c4850a3762c 100644 --- a/drivers/tty/serial/serial-tegra.c +++ b/drivers/tty/serial/serial-tegra.c @@ -1667,6 +1667,7 @@ static int __init tegra_uart_init(void) node = of_find_matching_node(NULL, tegra_uart_of_match); if (node) match = of_match_node(tegra_uart_of_match, node); + of_node_put(node); if (match) cdata = match->data; if (cdata) -- 2.25.1
[PATCH v2 0/2] Add R16 Vista E board from RenewWorldOutreach
This patch series adds the R16-Vista-E board from RenewWorldOutreach based on allwinner R16(A33). Patch 1/2 adds the dt-bindings for the board. Patch 2/2 adds the board with the following below features: General features: - 1GB RAM - microSD slot - Realtek Wifi - 1 x USB 2.0 - HDMI IN - HDMI OUT - Audio out - MIPI DSI - TI DLPC3433 It has also connectors to connect an external mini keypad. Suniel Mahesh (2): dt-bindings: arm: sunxi: Add binding for RenewWorldOutReach R16-Vista-E board ARM: dts: sun8i: Add R16 Vista E board from RenewWorldOutreach .../devicetree/bindings/arm/sunxi.yaml| 6 + arch/arm/boot/dts/Makefile| 1 + arch/arm/boot/dts/sun8i-r16-renew-vista-e.dts | 361 ++ 3 files changed, 368 insertions(+) create mode 100644 arch/arm/boot/dts/sun8i-r16-renew-vista-e.dts -- 2.25.1
Re: [PATCH 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
On 15/06/2022 20:11, Abhinav Kumar wrote: On 6/15/2022 10:04 AM, Dmitry Baryshkov wrote: On 15/06/2022 19:40, Abhinav Kumar wrote: On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: On 14/06/2022 22:32, Abhinav Kumar wrote: intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..e991d4ba8a40 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) - phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); - - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) - phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); - - if (!phys->hw_intf && !phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "no intf or wb block assigned at idx: %d\n", i); - return; - } - - if (phys->hw_intf && phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "invalid phys both intf and wb block at idx: %d\n", i); - return; - } Please retain these checks in dpu_encoder_setup_display(). It checks that we really have got the intf or wb. For example one might have specified the INTF that leads to INTF_NONE interface. Or non-existing/not supported WB. Right, so the reason I omitted that was dpu_encoder_setup_display() already has these checks: https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 Please check lines 2273-2284. Only if all those checks succeeded we call dpu_encoder_virt_add_phys_encs which increments num_phys_encs. As I wrote, it checks indices from phys_params, but not the acquired hardware instances. Right but today, both the get_intf() and get_wb() just return the intf/wb corresponding to the index. So as long as the index is valid how will checking hw_wb or hw_intf be different? static inline struct dpu_hw_intf *dpu_rm_get_intf(struct dpu_rm *rm, enum dpu_intf intf_idx) { return rm->hw_intf[intf_idx - INTF_0]; } /** * dpu_rm_get_wb - Return a struct dpu_hw_wb instance given it's index. * @rm: DPU Resource Manager handle * @wb_idx: WB index */ static inline struct dpu_hw_wb *dpu_rm_get_wb(struct dpu_rm *rm, enum dpu_wb wb_idx) { return rm->hw_wb[wb_idx - WB_0]; } WB_0 is valid, but dpu_rm_get_wb(WB_0) will return NULL. INTF_0 is valid, but dpu_rm_get_intf(INTF_0) on qcm2290 will return NULL. Etc. Thats why I dropped those. Let me know if you have more questions. - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(>vsync_cnt, 0); atomic_set(>underrun_cnt, 0); + + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) + phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); + + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); } + mutex_unlock(_enc->enc_lock); return ret; -- With best wishes Dmitry
Re: [PATCH v2] dma-buf: Move sysfs work out of DMA-BUF export path
On Wed, 15 Jun 2022 at 19:43, T.J. Mercier wrote: > > On Wed, Jun 1, 2022 at 5:40 AM Daniel Vetter wrote: > > > > On Mon, May 30, 2022 at 08:12:16AM +0200, Christian König wrote: > > > Am 25.05.22 um 23:05 schrieb T.J. Mercier: > > > > On Wed, May 25, 2022 at 7:38 AM Daniel Vetter wrote: > > > > > On Tue, May 17, 2022 at 08:13:24AM +0200, Greg Kroah-Hartman wrote: > > > > > > On Mon, May 16, 2022 at 05:08:05PM -0700, T.J. Mercier wrote: > > > > > > > On Mon, May 16, 2022 at 12:21 PM Christian König > > > > > > > wrote: > > > > > > > > Am 16.05.22 um 20:08 schrieb T.J. Mercier: > > > > > > > > > On Mon, May 16, 2022 at 10:20 AM Christian König > > > > > > > > > wrote: > > > > > > > > > > Am 16.05.22 um 19:13 schrieb T.J. Mercier: > > > > > > > > > > > Recently, we noticed an issue where a process went into > > > > > > > > > > > direct reclaim > > > > > > > > > > > while holding the kernfs rw semaphore for sysfs in write > > > > > > > > > > > (exclusive) > > > > > > > > > > > mode. This caused processes who were doing DMA-BUF > > > > > > > > > > > exports and releases > > > > > > > > > > > to go into uninterruptible sleep since they needed to > > > > > > > > > > > acquire the same > > > > > > > > > > > semaphore for the DMA-BUF sysfs entry creation/deletion. > > > > > > > > > > > In order to avoid > > > > > > > > > > > blocking DMA-BUF export for an indeterminate amount of > > > > > > > > > > > time while > > > > > > > > > > > another process is holding the sysfs rw semaphore in > > > > > > > > > > > exclusive mode, > > > > > > > > > > > this patch moves the per-buffer sysfs file creation to > > > > > > > > > > > the default work > > > > > > > > > > > queue. Note that this can lead to a short-term inaccuracy > > > > > > > > > > > in the dmabuf > > > > > > > > > > > sysfs statistics, but this is a tradeoff to prevent the > > > > > > > > > > > hot path from > > > > > > > > > > > being blocked. A work_struct is added to dma_buf to > > > > > > > > > > > achieve this, but as > > > > > > > > > > > it is unioned with the kobject in the sysfs_entry, > > > > > > > > > > > dma_buf does not > > > > > > > > > > > increase in size. > > > > > > > > > > I'm still not very keen of this approach as it strongly > > > > > > > > > > feels like we > > > > > > > > > > are working around shortcoming somewhere else. > > > > > > > > > > > > > > > > > > > My read of the thread for the last version is that we're > > > > > > > > > running into > > > > > > > > > a situation where sysfs is getting used for something it > > > > > > > > > wasn't > > > > > > > > > originally intended for, but we're also stuck with this sysfs > > > > > > > > > functionality for dmabufs. > > > > > > > > > > > > > > > > > > > > Fixes: bdb8d06dfefd ("dmabuf: Add the capability to > > > > > > > > > > > expose DMA-BUF stats in sysfs") > > > > > > > > > > > Originally-by: Hridya Valsaraju > > > > > > > > > > > Signed-off-by: T.J. Mercier > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > See the originally submitted patch by Hridya Valsaraju > > > > > > > > > > > here: > > > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2022%2F1%2F4%2F1066data=05%7C01%7Cchristian.koenig%40amd.com%7C8f00afd44b9744c45f5708da3e926503%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637891095771223650%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=pubWqUyqhCWpXHhJHsoqarc3GLtB6IFB1rhgfsL4a1M%3Dreserved=0 > > > > > > > > > > > > > > > > > > > > > > v2 changes: > > > > > > > > > > > - Defer only sysfs creation instead of creation and > > > > > > > > > > > teardown per > > > > > > > > > > > Christian König > > > > > > > > > > > > > > > > > > > > > > - Use a work queue instead of a kthread for deferred work > > > > > > > > > > > per > > > > > > > > > > > Christian König > > > > > > > > > > > --- > > > > > > > > > > > drivers/dma-buf/dma-buf-sysfs-stats.c | 56 > > > > > > > > > > > --- > > > > > > > > > > > include/linux/dma-buf.h | 14 ++- > > > > > > > > > > > 2 files changed, 54 insertions(+), 16 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c > > > > > > > > > > > b/drivers/dma-buf/dma-buf-sysfs-stats.c > > > > > > > > > > > index 2bba0babcb62..67b0a298291c 100644 > > > > > > > > > > > --- a/drivers/dma-buf/dma-buf-sysfs-stats.c > > > > > > > > > > > +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c > > > > > > > > > > > @@ -11,6 +11,7 @@ > > > > > > > > > > > #include > > > > > > > > > > > #include > > > > > > > > > > > #include > > > > > > > > > > > +#include > > > > > > > > > > > > > > > > > > > > > > #include "dma-buf-sysfs-stats.h" > > > > > > > > > > > > > > > > > > > > > > @@ -168,10 +169,46 @@ void > > > > > > > > > > > dma_buf_uninit_sysfs_statistics(void) > > > > > > > > > > >
Re: [PATCH v2 3/5] drm/msm: Stop using iommu_present()
On 5/4/2022 5:16 PM, Dmitry Baryshkov wrote: Even if some IOMMU has registered itself on the platform "bus", that doesn't necessarily mean it provides translation for the device we care about. Replace iommu_present() with a more appropriate check. On Qualcomm platforms the IOMMU can be specified either for the MDP/DPU device or for its parent MDSS device depending on the actual platform. Check both of them, since that is how both DPU and MDP5 drivers work. Co-developed-by: Robin Murphy Signed-off-by: Dmitry Baryshkov I have the same question as patch (1) of this series here but i will let you answer it there, so this one is, Reviewed-by: Abhinav Kumar --- drivers/gpu/drm/msm/msm_drv.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 4a3dda23e3e0..a37a3bbc04d9 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -266,8 +266,14 @@ bool msm_use_mmu(struct drm_device *dev) { struct msm_drm_private *priv = dev->dev_private; - /* a2xx comes with its own MMU */ - return priv->is_a2xx || iommu_present(_bus_type); + /* +* a2xx comes with its own MMU +* On other platforms IOMMU can be declared specified either for the +* MDP/DPU device or for its parent, MDSS device. +*/ + return priv->is_a2xx || + device_iommu_mapped(dev->dev) || + device_iommu_mapped(dev->dev->parent); } static int msm_init_vram(struct drm_device *dev)
Re: [PATCH v2 2/5] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage
On 5/4/2022 5:16 PM, Dmitry Baryshkov wrote: Move iommu_domain_alloc() in front of adress space/IOMMU initialization. This allows us to drop final bits of struct mdp5_cfg_platform which remained from the pre-DT days. Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h | 6 -- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 -- 3 files changed, 4 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c index 1bf9ff5dbabc..714effb967ff 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c @@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = { { .revision = 3, .config = { .hw = _config } }, }; -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev); - const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler *cfg_handler) { return cfg_handler->config.hw; @@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms, uint32_t major, uint32_t minor) { struct drm_device *dev = mdp5_kms->dev; - struct platform_device *pdev = to_platform_device(dev->dev); struct mdp5_cfg_handler *cfg_handler; const struct mdp5_cfg_handler *cfg_handlers; - struct mdp5_cfg_platform *pconfig; int i, ret = 0, num_handlers; cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL); @@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms, cfg_handler->revision = minor; cfg_handler->config.hw = mdp5_cfg; - pconfig = mdp5_get_config(pdev); - memcpy(_handler->config.platform, pconfig, sizeof(*pconfig)); - DBG("MDP5: %s hw config selected", mdp5_cfg->name); return cfg_handler; @@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms, return ERR_PTR(ret); } - -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev) -{ - static struct mdp5_cfg_platform config = {}; - - config.iommu = iommu_domain_alloc(_bus_type); - - return -} diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h index 6b03d7899309..c2502cc33864 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h @@ -104,14 +104,8 @@ struct mdp5_cfg_hw { uint32_t max_clk; }; -/* platform config data (ie. from DT, or pdata) */ -struct mdp5_cfg_platform { - struct iommu_domain *iommu; -}; - struct mdp5_cfg { const struct mdp5_cfg_hw *hw; - struct mdp5_cfg_platform platform; }; struct mdp5_kms; diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 9b7bbc3adb97..1c67c2c828cd 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev) struct msm_gem_address_space *aspace; int irq, i, ret; struct device *iommu_dev; + struct iommu_domain *iommu; ret = mdp5_init(to_platform_device(dev->dev), dev); @@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev) } mdelay(16); - if (config->platform.iommu) { + iommu = iommu_domain_alloc(_bus_type); + if (iommu) { struct msm_mmu *mmu; iommu_dev = >dev; if (!dev_iommu_fwspec_get(iommu_dev)) iommu_dev = iommu_dev->parent; - mmu = msm_iommu_new(iommu_dev, config->platform.iommu); + mmu = msm_iommu_new(iommu_dev, iommu); aspace = msm_gem_address_space_create(mmu, "mdp5", 0x1000, 0x1 - 0x1000);
Re: [PULL] drm-misc-next
On Wed, Jun 08, 2022 at 12:34:41PM +0200, Thomas Zimmermann wrote: > Hi Dave and Daniel, > > here's the first PR for drm-misc-next that will go into v5.20. > > Best regards > Thomas > > drm-misc-next-2022-06-08: > drm-misc-next for 5.20: > > UAPI Changes: > > * connector: export bpc limits in debugfs > > * dma-buf: Print buffer name in debugfs > > Cross-subsystem Changes: > > * dma-buf: Improve dma-fence handling; Cleanups > > * fbdev: Device-unregistering fixes > > Core Changes: > > * client: Only use driver-validated modes to avoid blank screen > > * dp-aux: Make probing more reliable; Small fixes > > * edit: CEA data-block iterators; Introduce struct drm_edid; Many cleanups > > * gem: Don't use framebuffer format's non-exising color planes > > * probe-helper: Use 640x480 as DisplayPort fallback; Refactoring > > * scheduler: Don't kill jobs in interrupt context > > Driver Changes: > > * amdgpu: Use atomic fence helpers in DM; Fix VRAM address calculation; >Export CRTC bpc settings via debugfs > > * bridge: Add TI-DLPC3433; anx7625: Fixes; fy07024di26a30d: Optional >GPIO reset; icn6211: Cleanups; ldb: Add reg and reg-name properties >to bindings, Kconfig fixes; lt9611: Fix display sensing; lt9611uxc: >Fixes; nwl-dsi: Fixes; ps8640: Cleanups; st7735r: Fixes; tc358767: >DSI/DPI refactoring and DSI-to-eDP support, Fixes; ti-sn65dsi83: >Fixes; > > * gma500: Cleanup connector I2C handling > > * hyperv: Unify VRAM allocation of Gen1 and Gen2 > > * i915: export CRTC bpc settings via debugfs > > * meson: Support YUV422 output; Refcount fixes > > * mgag200: Support damage clipping; Support gamma handling; Protect >concurrent HW access; Fixes to connector; Store model-specific limits >in device-info structure; Cleanups > > * nouveau: Fixes and Cleanups > > * panel: Kconfig fixes > > * panfrost: Valhall support > > * r128: Fix bit-shift overflow > > * rockchip: Locking fixes in error path; Minor cleanups > > * ssd130x: Fix built-in linkage > > * ttm: Cleanups > > * udl; Always advertize VGA connector > > * fbdev/vesa: Support COMPILE_TEST > The following changes since commit 6071c4c2a319da360b0bf2bc397d4fefad10b2c8: > > drm/qxl: add drm_gem_plane_helper_prepare_fb (2022-05-05 12:30:10 +0200) > > are available in the Git repository at: > > git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2022-06-08 > > for you to fetch changes up to dfa687bffc8a4a21ed929c7dececf01b8f1f52ee: > > drm/bridge: lt9611uxc: Cancel only driver's work (2022-06-07 14:57:47 +0200) Pulled, thanks. -Daniel > > > drm-misc-next for 5.20: > > UAPI Changes: > > * connector: export bpc limits in debugfs > > * dma-buf: Print buffer name in debugfs > > Cross-subsystem Changes: > > * dma-buf: Improve dma-fence handling; Cleanups > > * fbdev: Device-unregistering fixes > > Core Changes: > > * client: Only use driver-validated modes to avoid blank screen > > * dp-aux: Make probing more reliable; Small fixes > > * edit: CEA data-block iterators; Introduce struct drm_edid; Many cleanups > > * gem: Don't use framebuffer format's non-exising color planes > > * probe-helper: Use 640x480 as DisplayPort fallback; Refactoring > > * scheduler: Don't kill jobs in interrupt context > > Driver Changes: > > * amdgpu: Use atomic fence helpers in DM; Fix VRAM address calculation; >Export CRTC bpc settings via debugfs > > * bridge: Add TI-DLPC3433; anx7625: Fixes; fy07024di26a30d: Optional >GPIO reset; icn6211: Cleanups; ldb: Add reg and reg-name properties >to bindings, Kconfig fixes; lt9611: Fix display sensing; lt9611uxc: >Fixes; nwl-dsi: Fixes; ps8640: Cleanups; st7735r: Fixes; tc358767: >DSI/DPI refactoring and DSI-to-eDP support, Fixes; ti-sn65dsi83: >Fixes; > > * gma500: Cleanup connector I2C handling > > * hyperv: Unify VRAM allocation of Gen1 and Gen2 > > * i915: export CRTC bpc settings via debugfs > > * meson: Support YUV422 output; Refcount fixes > > * mgag200: Support damage clipping; Support gamma handling; Protect >concurrent HW access; Fixes to connector; Store model-specific limits >in device-info structure; Cleanups > > * nouveau: Fixes and Cleanups > > * panel: Kconfig fixes > > * panfrost: Valhall support > > * r128: Fix bit-shift overflow > > * rockchip: Locking fixes in error path; Minor cleanups > > * ssd130x: Fix built-in linkage > > * ttm: Cleanups > > * udl; Always advertize VGA connector > > * fbdev/vesa: Support COMPILE_TEST > > > Alyssa Rosenzweig (9): > dt-bindings: Add compatible for Mali Valhall (JM) > drm/panfrost: Handle HW_ISSUE_TTRX_2968_TTRX_3162 > drm/panfrost: Constify argument to has_hw_issue > drm/panfrost: Handle HW_ISSUE_TTRX_3076 > drm/panfrost:
Re: [PATCH v4 0/7] usb: typec: Introduce typec-switch binding
I should add: Series submission suggestions (of course, open to better suggestions too): - Patches 1-3 can go through the USB repo. - Patches 4-7 can: + also go through the USB repo after the maintainer acks have been received + go through the DRM repo (after creating a branch from USB repo's usb-next branch after Patches 1-3 have been applied). (I"ll add the above to the cover letter if there is a v5). Thanks! On Wed, Jun 15, 2022 at 10:21 AM Prashant Malani wrote: > > This series introduces a binding for Type-C data lane switches. These > control the routing and operating modes of USB Type-C data lanes based > on the PD messaging from the Type-C port driver regarding connected > peripherals. > > The first patch introduces a change to the Type-C mux class mode-switch > matching code, while the second adds a config guard to a Type-C header. > The next couple of patches introduce the new "typec-switch" binding as > well as one user of it (the ANX7625 drm bridge). > > The remaining patches add functionality to the anx7625 driver to > register the mode-switches, as well as program its crosspoint > switch depending on which Type-C port has a DisplayPort (DP) peripheral > connected to it. > > v3: > https://lore.kernel.org/linux-usb/20220614193558.1163205-1-pmal...@chromium.org/ > > Changes since v3: > - Some more modifications to the anx7625 binding patch (4/7). > - Picked up 1 more Reviewed-by tag. > > Pin-Yen Lin (1): > drm/bridge: anx7625: Add typec_mux_set callback function > > Prashant Malani (6): > usb: typec: mux: Allow muxes to specify mode-switch > usb: typec: mux: Add CONFIG guards for functions > dt-bindings: usb: Add Type-C switch binding > dt-bindings: drm/bridge: anx7625: Add mode-switch support > drm/bridge: anx7625: Register number of Type C switches > drm/bridge: anx7625: Register Type-C mode switches > > .../display/bridge/analogix,anx7625.yaml | 64 > .../devicetree/bindings/usb/typec-switch.yaml | 74 + > drivers/gpu/drm/bridge/analogix/anx7625.c | 148 ++ > drivers/gpu/drm/bridge/analogix/anx7625.h | 20 +++ > drivers/usb/typec/mux.c | 8 +- > include/linux/usb/typec_mux.h | 44 +- > 6 files changed, 350 insertions(+), 8 deletions(-) > create mode 100644 Documentation/devicetree/bindings/usb/typec-switch.yaml > > -- > 2.36.1.476.g0c4daa206d-goog >
Re: [PATCH 05/10] drm: selftest: convert drm_format selftest to KUnit
Hi "Maíra, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm/drm-next] [also build test WARNING on linus/master v5.19-rc2 next-20220615] [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/intel-lab-lkp/linux/commits/Ma-ra-Canal/drm-selftest-Convert-to-KUnit/20220615-220404 base: git://anongit.freedesktop.org/drm/drm drm-next config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220616/202206160150.au7h8kxk-...@intel.com/config) compiler: gcc-11 (Debian 11.3.0-3) 11.3.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/ab0435b88de6bc132f814d07fce5bf1f964da00c git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Ma-ra-Canal/drm-selftest-Convert-to-KUnit/20220615-220404 git checkout ab0435b88de6bc132f814d07fce5bf1f964da00c # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/tests/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot All warnings (new ones prefixed by >>): drivers/gpu/drm/tests/test-drm_format.c: In function 'igt_check_drm_format_min_pitch': >> drivers/gpu/drm/tests/test-drm_format.c:268:1: warning: the frame size of >> 2572 bytes is larger than 2048 bytes [-Wframe-larger-than=] 268 | } | ^ vim +268 drivers/gpu/drm/tests/test-drm_format.c 90 91 static void igt_check_drm_format_min_pitch(struct kunit *test) 92 { 93 const struct drm_format_info *info = NULL; 94 95 /* Test invalid arguments */ 96 KUNIT_EXPECT_FALSE(test, drm_format_info_min_pitch(info, 0, 0)); 97 KUNIT_EXPECT_FALSE(test, drm_format_info_min_pitch(info, -1, 0)); 98 KUNIT_EXPECT_FALSE(test, drm_format_info_min_pitch(info, 1, 0)); 99 100 /* Test 1 plane 8 bits per pixel format */ 101 info = drm_format_info(DRM_FORMAT_RGB332); 102 KUNIT_EXPECT_TRUE(test, info); 103 KUNIT_EXPECT_FALSE(test, drm_format_info_min_pitch(info, 0, 0)); 104 KUNIT_EXPECT_FALSE(test, drm_format_info_min_pitch(info, -1, 0)); 105 KUNIT_EXPECT_FALSE(test, drm_format_info_min_pitch(info, 1, 0)); 106 107 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 1), 1); 108 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 2), 2); 109 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 640), 640); 110 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 1024), 1024); 111 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 1920), 1920); 112 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 4096), 4096); 113 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 671), 671); 114 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, UINT_MAX), 115 (uint64_t)UINT_MAX); 116 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)), 117 (uint64_t)(UINT_MAX - 1)); 118 119 /* Test 1 plane 16 bits per pixel format */ 120 info = drm_format_info(DRM_FORMAT_XRGB); 121 KUNIT_EXPECT_TRUE(test, info); 122 KUNIT_EXPECT_FALSE(test, drm_format_info_min_pitch(info, 0, 0)); 123 KUNIT_EXPECT_FALSE(test, drm_format_info_min_pitch(info, -1, 0)); 124 KUNIT_EXPECT_FALSE(test, drm_format_info_min_pitch(info, 1, 0)); 125 126 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 1), 2); 127 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 2), 4); 128 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 640), 1280); 129 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 1024), 2048); 130 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 1920), 3840); 131 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 4096), 8192); 132 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, 671), 1342); 133 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, UINT_MAX), 134 (uint64_t)UINT_MAX * 2); 135 KUNIT_EXPECT_EQ(test, drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)), 136 (uint64_t)(UINT_MAX - 1) * 2); 137 138 /* Test 1 plane 24 bits per pixel format */ 139 info = drm_format_info(DRM_FORMAT_RGB888); 140 KUNIT_EXPECT_TRUE(test, info); 141
Re: [PATCH 2/2] drm/msm: Don't overwrite hw fence in hw_init
On 6/15/22 10:01 AM, Rob Clark wrote: From: Rob Clark Prior to the last commit, this could result in setting the GPU written fence value back to an older value, if we had missed updating completed_fence prior to suspend. This was mostly harmless as the GPU would eventually overwrite it again with the correct value. But we should just not do this. Instead just leave a sanity check that the fence looks plausible (in case the GPU scribbled on memory). Reported-by: Steev Klimaszewski Fixes: 95d1deb02a9c ("drm/msm/gem: Add fenced vma unpin") Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 11 --- drivers/gpu/drm/msm/msm_gpu.c | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index e1aef4875e2f..dd044d557c7c 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -498,10 +498,15 @@ int adreno_hw_init(struct msm_gpu *gpu) ring->cur = ring->start; ring->next = ring->start; - - /* reset completed fence seqno: */ - ring->memptrs->fence = ring->fctx->completed_fence; ring->memptrs->rptr = 0; + + /* Detect and clean up an impossible fence, ie. if GPU managed +* to scribble something invalid, we don't want that to confuse +* us into mistakingly believing that submits have completed. +*/ + if (fence_before(ring->fctx->last_fence, ring->memptrs->fence)) { + ring->memptrs->fence = ring->fctx->last_fence; + } } return 0; diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index b61078f0cd0f..8c00f9187c03 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -430,7 +430,7 @@ static void recover_worker(struct kthread_work *work) * one more to clear the faulting submit */ if (ring == cur_ring) - fence++; + ring->memptrs->fence = ++fence; msm_update_fence(ring->fctx, fence); } Tested on the Lenovo Yoga C630 Tested-by: Steev Klimaszewski
Re: [PATCH 1/2] drm/msm: Drop update_fences()
On 6/15/22 10:01 AM, Rob Clark wrote: From: Rob Clark I noticed while looking at some traces, that we could miss calls to msm_update_fence(), as the irq could have raced with retire_submits() which could have already popped the last submit on a ring out of the queue of in-flight submits. But walking the list of submits in the irq handler isn't really needed, as dma_fence_is_signaled() will dtrt. So lets just drop it entirely. Reported-by: Steev Klimaszewski Fixes: 95d1deb02a9c ("drm/msm/gem: Add fenced vma unpin") Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 22 ++ 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index e59a757578df..b61078f0cd0f 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -176,24 +176,6 @@ int msm_gpu_hw_init(struct msm_gpu *gpu) return ret; } -static void update_fences(struct msm_gpu *gpu, struct msm_ringbuffer *ring, - uint32_t fence) -{ - struct msm_gem_submit *submit; - unsigned long flags; - - spin_lock_irqsave(>submit_lock, flags); - list_for_each_entry(submit, >submits, node) { - if (fence_after(submit->seqno, fence)) - break; - - msm_update_fence(submit->ring->fctx, - submit->hw_fence->seqno); - dma_fence_signal(submit->hw_fence); - } - spin_unlock_irqrestore(>submit_lock, flags); -} - #ifdef CONFIG_DEV_COREDUMP static ssize_t msm_gpu_devcoredump_read(char *buffer, loff_t offset, size_t count, void *data, size_t datalen) @@ -450,7 +432,7 @@ static void recover_worker(struct kthread_work *work) if (ring == cur_ring) fence++; - update_fences(gpu, ring, fence); + msm_update_fence(ring->fctx, fence); } if (msm_gpu_active(gpu)) { @@ -753,7 +735,7 @@ void msm_gpu_retire(struct msm_gpu *gpu) int i; for (i = 0; i < gpu->nr_rings; i++) - update_fences(gpu, gpu->rb[i], gpu->rb[i]->memptrs->fence); + msm_update_fence(gpu->rb[i]->fctx, gpu->rb[i]->memptrs->fence); kthread_queue_work(gpu->worker, >retire_work); update_sw_cntrs(gpu); Tested on the Lenovo Yoga C630 Tested-by: Steev Klimaszewski
Re: [Freedreno] [PATCH v2 1/3] drm/msm/dpu: Move LM CRC code into separate method
On 6/15/2022 9:17 AM, Dmitry Baryshkov wrote: On 15/06/2022 19:11, Jessica Zhang wrote: On 6/15/2022 2:35 AM, Dmitry Baryshkov wrote: On Wed, 15 Jun 2022 at 00:13, Jessica Zhang wrote: Move layer mixer-specific section of dpu_crtc_get_crc() into a separate helper method. This way, we can make it easier to get CRCs from other HW blocks by adding other get_crc helper methods. Changes since V1: - Moved common bitmasks to dpu_hw_util.h - Moved common CRC methods to dpu_hw_util.c - Updated copyrights - Changed crcs array to a dynamically allocated array and added it as a member of crtc_state Signed-off-by: Jessica Zhang Please split this into separate patches. One for hw_util, one for the rest Sure --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 88 + drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 4 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 42 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 46 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 16 5 files changed, 124 insertions(+), 72 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index b56f777dbd0e..16742a66878e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2014-2021 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark @@ -88,6 +89,11 @@ static int dpu_crtc_verify_crc_source(struct drm_crtc *crtc, enum dpu_crtc_crc_source source = dpu_crtc_parse_crc_source(src_name); struct dpu_crtc_state *crtc_state = to_dpu_crtc_state(crtc->state); + if (crtc_state->crcs) { + kfree(crtc_state->crcs); + crtc_state->crcs = NULL; + } + if (source < 0) { DRM_DEBUG_DRIVER("Invalid source %s for CRTC%d\n", src_name, crtc->index); return -EINVAL; @@ -96,20 +102,37 @@ static int dpu_crtc_verify_crc_source(struct drm_crtc *crtc, if (source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER) *values_cnt = crtc_state->num_mixers; + crtc_state->crcs = kcalloc(*values_cnt, sizeof(crtc_state->crcs), GFP_KERNEL); + I do not quite like the idea of constantly allocating and freeing the crcs array. I'd suggest defining sensible MAX_CRC_VALUES, using a static array and verifying that no one over commits the MAX_CRC_VALUES. If at some point we decide that we really need the dynamic array, we can implement it later. We already had dynamic arrays and Rob had to fix it. See 00326bfa4e63 ("drm/msm/dpu: Remove dynamic allocation from atomic context"). Ah, got it... the reason I used a dynamic array here was because AFAIK we don't have a defined upper limit for how many drm_encoders can be connected to a CRTC simultaneously. Do you have a suggestion on what value we can set for MAX_CRC_VALUES? Three? Two for two hw_intfs? Do you mean 2 hw_intfs per drm_encoder or 2 hw_intfs overall? IIRC we also wanted to take into account the possibility of there being multiple drm_encoders attached to a single CRTC. Also, looking through Rob's fix, the warning was occuring because we were trying to call kcalloc in an IRQ context. However, the way I'm currently doing dynamic allocation will avoid the warning (since I'm doing kcalloc in verify_crc_source, which is called during the debugfs read/write/open and not during vblank). So I don't believe that we'll encounter the warning related to Rob's fix with my current implementation of the crcs dynamic array. Thanks, Jessica Zhang -- With best wishes Dmitry
Re: [Freedreno] [PATCH v2 1/5] drm/msm/dpu: check both DPU and MDSS devices for the IOMMU
On 5/4/2022 5:16 PM, Dmitry Baryshkov wrote: Follow the lead of MDP5 driver and check both DPU and MDSS devices for the IOMMU specifiers. Historically DPU devices had IOMMU specified in the MDSS device tree node, but as some of MDP5 devices are being converted to the supported by the DPU driver, the driver should adapt and check both devices. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 143d6643be53..5ccda0766f6c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1004,14 +1004,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) struct msm_mmu *mmu; struct device *dpu_dev = dpu_kms->dev->dev; struct device *mdss_dev = dpu_dev->parent; + struct device *iommu_dev; domain = iommu_domain_alloc(_bus_type); if (!domain) return 0; - /* IOMMUs are a part of MDSS device tree binding, not the -* MDP/DPU device. */ - mmu = msm_iommu_new(mdss_dev, domain); + /* +* IOMMUs can be a part of MDSS device tree binding, or the +* MDP/DPU device. +*/ Can you please explain this a little more? So even if some of the mdp5 devices are getting converted to use DPU driver, their device trees are also updated right? In other words, if DPU driver was using mdss_dev to initialize the iommu, why should the new devices which are going to use DPU have the binding in the dpu_dev? + if (dev_iommu_fwspec_get(dpu_dev)) + iommu_dev = dpu_dev; + else + iommu_dev = mdss_dev; + + mmu = msm_iommu_new(iommu_dev, domain); if (IS_ERR(mmu)) { iommu_domain_free(domain); return PTR_ERR(mmu);
Re: [PATCH v4 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support
On 15/06/2022 10:20, Prashant Malani wrote: > Analogix 7625 can be used in systems to switch USB Type-C DisplayPort > alternate mode lane traffic between 2 Type-C ports. > > Update the binding to accommodate this usage by introducing a switch > property. > > Reviewed-by: Nícolas F. R. A. Prado > Tested-by: Nícolas F. R. A. Prado > Signed-off-by: Prashant Malani > --- > > Changes since v3: > - Fix unevaluatedProperties usage. > - Add additionalProperties to top level "switches" nodes. > - Make quotes consistent. > - Add '^..$' to regex. > (All suggested by Krzysztof Kozlowski) Looks good to me. Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2] dma-buf: Move sysfs work out of DMA-BUF export path
On Wed, Jun 1, 2022 at 5:40 AM Daniel Vetter wrote: > > On Mon, May 30, 2022 at 08:12:16AM +0200, Christian König wrote: > > Am 25.05.22 um 23:05 schrieb T.J. Mercier: > > > On Wed, May 25, 2022 at 7:38 AM Daniel Vetter wrote: > > > > On Tue, May 17, 2022 at 08:13:24AM +0200, Greg Kroah-Hartman wrote: > > > > > On Mon, May 16, 2022 at 05:08:05PM -0700, T.J. Mercier wrote: > > > > > > On Mon, May 16, 2022 at 12:21 PM Christian König > > > > > > wrote: > > > > > > > Am 16.05.22 um 20:08 schrieb T.J. Mercier: > > > > > > > > On Mon, May 16, 2022 at 10:20 AM Christian König > > > > > > > > wrote: > > > > > > > > > Am 16.05.22 um 19:13 schrieb T.J. Mercier: > > > > > > > > > > Recently, we noticed an issue where a process went into > > > > > > > > > > direct reclaim > > > > > > > > > > while holding the kernfs rw semaphore for sysfs in write > > > > > > > > > > (exclusive) > > > > > > > > > > mode. This caused processes who were doing DMA-BUF exports > > > > > > > > > > and releases > > > > > > > > > > to go into uninterruptible sleep since they needed to > > > > > > > > > > acquire the same > > > > > > > > > > semaphore for the DMA-BUF sysfs entry creation/deletion. In > > > > > > > > > > order to avoid > > > > > > > > > > blocking DMA-BUF export for an indeterminate amount of time > > > > > > > > > > while > > > > > > > > > > another process is holding the sysfs rw semaphore in > > > > > > > > > > exclusive mode, > > > > > > > > > > this patch moves the per-buffer sysfs file creation to the > > > > > > > > > > default work > > > > > > > > > > queue. Note that this can lead to a short-term inaccuracy > > > > > > > > > > in the dmabuf > > > > > > > > > > sysfs statistics, but this is a tradeoff to prevent the hot > > > > > > > > > > path from > > > > > > > > > > being blocked. A work_struct is added to dma_buf to achieve > > > > > > > > > > this, but as > > > > > > > > > > it is unioned with the kobject in the sysfs_entry, dma_buf > > > > > > > > > > does not > > > > > > > > > > increase in size. > > > > > > > > > I'm still not very keen of this approach as it strongly feels > > > > > > > > > like we > > > > > > > > > are working around shortcoming somewhere else. > > > > > > > > > > > > > > > > > My read of the thread for the last version is that we're > > > > > > > > running into > > > > > > > > a situation where sysfs is getting used for something it wasn't > > > > > > > > originally intended for, but we're also stuck with this sysfs > > > > > > > > functionality for dmabufs. > > > > > > > > > > > > > > > > > > Fixes: bdb8d06dfefd ("dmabuf: Add the capability to expose > > > > > > > > > > DMA-BUF stats in sysfs") > > > > > > > > > > Originally-by: Hridya Valsaraju > > > > > > > > > > Signed-off-by: T.J. Mercier > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > See the originally submitted patch by Hridya Valsaraju here: > > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2022%2F1%2F4%2F1066data=05%7C01%7Cchristian.koenig%40amd.com%7C8f00afd44b9744c45f5708da3e926503%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637891095771223650%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=pubWqUyqhCWpXHhJHsoqarc3GLtB6IFB1rhgfsL4a1M%3Dreserved=0 > > > > > > > > > > > > > > > > > > > > v2 changes: > > > > > > > > > > - Defer only sysfs creation instead of creation and > > > > > > > > > > teardown per > > > > > > > > > > Christian König > > > > > > > > > > > > > > > > > > > > - Use a work queue instead of a kthread for deferred work > > > > > > > > > > per > > > > > > > > > > Christian König > > > > > > > > > > --- > > > > > > > > > > drivers/dma-buf/dma-buf-sysfs-stats.c | 56 > > > > > > > > > > --- > > > > > > > > > > include/linux/dma-buf.h | 14 ++- > > > > > > > > > > 2 files changed, 54 insertions(+), 16 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c > > > > > > > > > > b/drivers/dma-buf/dma-buf-sysfs-stats.c > > > > > > > > > > index 2bba0babcb62..67b0a298291c 100644 > > > > > > > > > > --- a/drivers/dma-buf/dma-buf-sysfs-stats.c > > > > > > > > > > +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c > > > > > > > > > > @@ -11,6 +11,7 @@ > > > > > > > > > > #include > > > > > > > > > > #include > > > > > > > > > > #include > > > > > > > > > > +#include > > > > > > > > > > > > > > > > > > > > #include "dma-buf-sysfs-stats.h" > > > > > > > > > > > > > > > > > > > > @@ -168,10 +169,46 @@ void > > > > > > > > > > dma_buf_uninit_sysfs_statistics(void) > > > > > > > > > > kset_unregister(dma_buf_stats_kset); > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > +static void sysfs_add_workfn(struct work_struct *work) > > > > > > > > > > +{ > > > > > > > > > > + struct dma_buf_sysfs_entry *sysfs_entry = > > > > > > > > > > +
[PATCH v4 7/7] drm/bridge: anx7625: Add typec_mux_set callback function
From: Pin-Yen Lin Add the callback function when the driver receives state changes of the Type-C port. The callback function configures the crosspoint switch of the anx7625 bridge chip, which can change the output pins of the signals according to the port state. Reviewed-by: AngeloGioacchino Del Regno Reviewed-by: Nícolas F. R. A. Prado Tested-by: Nícolas F. R. A. Prado Signed-off-by: Pin-Yen Lin Signed-off-by: Prashant Malani --- Changes since v3: - Added Reviewed-by tag from Angelo. Changes since v2: - Moved num_typec_switches check to beginning of function - Made dp_connected assignments fit on one line (and removed unnecessary parentheses) - Added Reviewed-by and Tested-by tags. Changes since v1: - No changes. drivers/gpu/drm/bridge/analogix/anx7625.c | 56 +++ drivers/gpu/drm/bridge/analogix/anx7625.h | 13 ++ 2 files changed, 69 insertions(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index bd21f159b973..5992fc8beeeb 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -2582,9 +2583,64 @@ static void anx7625_runtime_disable(void *data) pm_runtime_disable(data); } +static void anx7625_set_crosspoint_switch(struct anx7625_data *ctx, + enum typec_orientation orientation) +{ + if (orientation == TYPEC_ORIENTATION_NORMAL) { + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0, + SW_SEL1_SSRX_RX1 | SW_SEL1_DPTX0_RX2); + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1, + SW_SEL2_SSTX_TX1 | SW_SEL2_DPTX1_TX2); + } else if (orientation == TYPEC_ORIENTATION_REVERSE) { + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0, + SW_SEL1_SSRX_RX2 | SW_SEL1_DPTX0_RX1); + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1, + SW_SEL2_SSTX_TX2 | SW_SEL2_DPTX1_TX1); + } +} + +static void anx7625_typec_two_ports_update(struct anx7625_data *ctx) +{ + if (ctx->typec_ports[0].dp_connected && ctx->typec_ports[1].dp_connected) + /* Both ports available, do nothing to retain the current one. */ + return; + else if (ctx->typec_ports[0].dp_connected) + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL); + else if (ctx->typec_ports[1].dp_connected) + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE); +} + static int anx7625_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *state) { + struct anx7625_port_data *data = typec_mux_get_drvdata(mux); + struct anx7625_data *ctx = data->ctx; + struct device *dev = >client->dev; + bool new_dp_connected, old_dp_connected; + + if (ctx->num_typec_switches == 1) + return 0; + + old_dp_connected = ctx->typec_ports[0].dp_connected || ctx->typec_ports[1].dp_connected; + + dev_dbg(dev, "mux_set dp_connected: c0=%d, c1=%d\n", + ctx->typec_ports[0].dp_connected, ctx->typec_ports[1].dp_connected); + + data->dp_connected = (state->alt && state->alt->svid == USB_TYPEC_DP_SID && + state->alt->mode == USB_TYPEC_DP_MODE); + + new_dp_connected = ctx->typec_ports[0].dp_connected || ctx->typec_ports[1].dp_connected; + + /* dp on, power on first */ + if (!old_dp_connected && new_dp_connected) + pm_runtime_get_sync(dev); + + anx7625_typec_two_ports_update(ctx); + + /* dp off, power off last */ + if (old_dp_connected && !new_dp_connected) + pm_runtime_put_sync(dev); + return 0; } diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h index 76cfc64f7574..7d6c6fdf9a3a 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.h +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h @@ -55,6 +55,18 @@ #define HPD_STATUS_CHANGE 0x80 #define HPD_STATUS 0x80 +#define TCPC_SWITCH_0 0xB4 +#define SW_SEL1_DPTX0_RX2 BIT(0) +#define SW_SEL1_DPTX0_RX1 BIT(1) +#define SW_SEL1_SSRX_RX2 BIT(4) +#define SW_SEL1_SSRX_RX1 BIT(5) + +#define TCPC_SWITCH_1 0xB5 +#define SW_SEL2_DPTX1_TX2 BIT(0) +#define SW_SEL2_DPTX1_TX1 BIT(1) +#define SW_SEL2_SSTX_TX2 BIT(4) +#define SW_SEL2_SSTX_TX1 BIT(5) + / END of I2C Address 0x58 / /***/ @@ -444,6 +456,7 @@ struct anx7625_i2c_client { }; struct anx7625_port_data { + bool dp_connected; struct typec_mux_dev *typec_mux; struct anx7625_data *ctx; }; -- 2.36.1.476.g0c4daa206d-goog
[PATCH v4 6/7] drm/bridge: anx7625: Register Type-C mode switches
When the DT node has "switches" available, register a Type-C mode-switch for each listed "switch". This allows the driver to receive state information about what operating mode a Type-C port and its connected peripherals are in, as well as status information (like VDOs) related to that state. The callback function is currently a stub, but subsequent patches will implement the required functionality. Reviewed-by: Nícolas F. R. A. Prado Tested-by: Nícolas F. R. A. Prado Signed-off-by: Prashant Malani --- Changes since v3: - No changes. Changes since v2: - Updated dev_info() to dev_warn() print, but added a check to ensure it only triggers on non -ENODEV errors. - Made conflict resolutions resulting from changes introduced in Patch v3 5/7 (add ret variable here instead of in Patch v3 5/7). - Added Reviewed-by and Tested-by tags. Changes since v1: - No changes. drivers/gpu/drm/bridge/analogix/anx7625.c | 82 +-- drivers/gpu/drm/bridge/analogix/anx7625.h | 6 ++ 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index e3d4c2738b8c..bd21f159b973 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -2581,10 +2582,61 @@ static void anx7625_runtime_disable(void *data) pm_runtime_disable(data); } +static int anx7625_typec_mux_set(struct typec_mux_dev *mux, +struct typec_mux_state *state) +{ + return 0; +} + +static int anx7625_register_mode_switch(struct device *dev, struct device_node *node, + struct anx7625_data *ctx) +{ + struct anx7625_port_data *port_data; + struct typec_mux_desc mux_desc = {}; + char name[32]; + u32 port_num; + int ret; + + ret = of_property_read_u32(node, "reg", _num); + if (ret) + return ret; + + if (port_num >= ctx->num_typec_switches) { + dev_err(dev, "Invalid port number specified: %d\n", port_num); + return -EINVAL; + } + + port_data = >typec_ports[port_num]; + port_data->ctx = ctx; + mux_desc.fwnode = >fwnode; + mux_desc.drvdata = port_data; + snprintf(name, sizeof(name), "%s-%u", node->name, port_num); + mux_desc.name = name; + mux_desc.set = anx7625_typec_mux_set; + + port_data->typec_mux = typec_mux_register(dev, _desc); + if (IS_ERR(port_data->typec_mux)) { + ret = PTR_ERR(port_data->typec_mux); + dev_err(dev, "Mode switch register for port %d failed: %d", port_num, ret); + } + + return ret; +} + +static void anx7625_unregister_typec_switches(struct anx7625_data *ctx) +{ + int i; + + for (i = 0; i < ctx->num_typec_switches; i++) + typec_mux_unregister(ctx->typec_ports[i].typec_mux); +} + static int anx7625_register_typec_switches(struct device *device, struct anx7625_data *ctx) { - struct device_node *of = of_get_child_by_name(device->of_node, "switches"); + struct device_node *of, *sw; + int ret = 0; + of = of_get_child_by_name(device->of_node, "switches"); if (!of) return -ENODEV; @@ -2592,7 +2644,27 @@ static int anx7625_register_typec_switches(struct device *device, struct anx7625 if (ctx->num_typec_switches <= 0) return -ENODEV; - return 0; + ctx->typec_ports = devm_kzalloc(device, + ctx->num_typec_switches * sizeof(struct anx7625_port_data), + GFP_KERNEL); + if (!ctx->typec_ports) + return -ENOMEM; + + /* Register switches for each connector. */ + for_each_available_child_of_node(of, sw) { + if (!of_property_read_bool(sw, "mode-switch")) + continue; + ret = anx7625_register_mode_switch(device, sw, ctx); + if (ret) { + dev_err(device, "Failed to register mode switch: %d\n", ret); + break; + } + } + + if (ret) + anx7625_unregister_typec_switches(ctx); + + return ret; } static int anx7625_i2c_probe(struct i2c_client *client, @@ -2701,8 +2773,8 @@ static int anx7625_i2c_probe(struct i2c_client *client, queue_work(platform->workqueue, >work); ret = anx7625_register_typec_switches(dev, platform); - if (ret) - dev_dbg(dev, "Didn't register Type C switches, err: %d\n", ret); + if (ret && ret != -ENODEV) + dev_warn(dev, "Didn't register Type C switches, err: %d\n", ret); platform->bridge.funcs = _bridge_funcs; platform->bridge.of_node = client->dev.of_node; @@ -2757,6 +2829,8
[PATCH v4 5/7] drm/bridge: anx7625: Register number of Type C switches
Parse the "switches" node, if available, and count and store the number of Type-C switches within it. Since we currently don't do anything with this info, no functional changes are expected from this change. This patch sets a foundation for the actual registering of Type-C switches with the Type-C connector class framework. Reviewed-by: Nícolas F. R. A. Prado Tested-by: Nícolas F. R. A. Prado Signed-off-by: Prashant Malani --- Changes since v3: - No changes. Changes since v2: - Move ret variable to Patch v3 6/7. - Make error print a dev_dbg, since it is noisy. - Added Reviewed-by and Tested-by tags. Changes since v1: - No changes. drivers/gpu/drm/bridge/analogix/anx7625.c | 18 ++ drivers/gpu/drm/bridge/analogix/anx7625.h | 1 + 2 files changed, 19 insertions(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 53a5da6c49dd..e3d4c2738b8c 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -2581,6 +2581,20 @@ static void anx7625_runtime_disable(void *data) pm_runtime_disable(data); } +static int anx7625_register_typec_switches(struct device *device, struct anx7625_data *ctx) +{ + struct device_node *of = of_get_child_by_name(device->of_node, "switches"); + + if (!of) + return -ENODEV; + + ctx->num_typec_switches = of_get_child_count(of); + if (ctx->num_typec_switches <= 0) + return -ENODEV; + + return 0; +} + static int anx7625_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -2686,6 +2700,10 @@ static int anx7625_i2c_probe(struct i2c_client *client, if (platform->pdata.intp_irq) queue_work(platform->workqueue, >work); + ret = anx7625_register_typec_switches(dev, platform); + if (ret) + dev_dbg(dev, "Didn't register Type C switches, err: %d\n", ret); + platform->bridge.funcs = _bridge_funcs; platform->bridge.of_node = client->dev.of_node; if (!anx7625_of_panel_on_aux_bus(>dev)) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h index e257a84db962..d5cbca708842 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.h +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h @@ -473,6 +473,7 @@ struct anx7625_data { struct drm_connector *connector; struct mipi_dsi_device *dsi; struct drm_dp_aux aux; + int num_typec_switches; }; #endif /* __ANX7625_H__ */ -- 2.36.1.476.g0c4daa206d-goog
[PATCH v4 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support
Analogix 7625 can be used in systems to switch USB Type-C DisplayPort alternate mode lane traffic between 2 Type-C ports. Update the binding to accommodate this usage by introducing a switch property. Reviewed-by: Nícolas F. R. A. Prado Tested-by: Nícolas F. R. A. Prado Signed-off-by: Prashant Malani --- Changes since v3: - Fix unevaluatedProperties usage. - Add additionalProperties to top level "switches" nodes. - Make quotes consistent. - Add '^..$' to regex. (All suggested by Krzysztof Kozlowski) Changes since v2: - Added Reviewed-by and Tested-by tags. Changes since v1: - Introduced patternProperties for "switch" children (suggested by Krzysztof Kozlowski). - Added unevaluatedProperties descriptor (suggested by Krzysztof Kozlowski). - Added "address-cells" and "size-cells" properties to "switches". .../display/bridge/analogix,anx7625.yaml | 64 +++ 1 file changed, 64 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml index 35a48515836e..bc6f7644db31 100644 --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml @@ -105,6 +105,34 @@ properties: - port@0 - port@1 + switches: +type: object +description: Set of switches controlling DisplayPort traffic on + outgoing RX/TX lanes to Type C ports. +additionalProperties: false + +properties: + '#address-cells': +const: 1 + + '#size-cells': +const: 0 + +patternProperties: + '^switch@[01]$': +$ref: /schemas/usb/typec-switch.yaml# +unevaluatedProperties: false + +properties: + reg: +maxItems: 1 + +required: + - reg + +required: + - switch@0 + required: - compatible - reg @@ -167,5 +195,41 @@ examples: }; }; }; +switches { +#address-cells = <1>; +#size-cells = <0>; +switch@0 { +compatible = "typec-switch"; +reg = <0>; +mode-switch; + +ports { +#address-cells = <1>; +#size-cells = <0>; +port@0 { +reg = <0>; +anx_typec0: endpoint { +remote-endpoint = <_port0>; +}; +}; +}; +}; +switch@1 { +compatible = "typec-switch"; +reg = <1>; +mode-switch; + +ports { +#address-cells = <1>; +#size-cells = <0>; +port@0 { +reg = <0>; +anx_typec1: endpoint { +remote-endpoint = <_port1>; +}; +}; +}; +}; +}; }; }; -- 2.36.1.476.g0c4daa206d-goog
[PATCH v4 3/7] dt-bindings: usb: Add Type-C switch binding
Introduce a binding which represents a component that can control the routing of USB Type-C data lines as well as address data line orientation (based on CC lines' orientation). Reviewed-by: Nícolas F. R. A. Prado Tested-by: Nícolas F. R. A. Prado Signed-off-by: Prashant Malani --- Changes since v3: - No changes. Changes since v2: - Added Reviewed-by and Tested-by tags. Changes since v1: - Removed "items" from compatible. - Fixed indentation in example. .../devicetree/bindings/usb/typec-switch.yaml | 74 +++ 1 file changed, 74 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/typec-switch.yaml diff --git a/Documentation/devicetree/bindings/usb/typec-switch.yaml b/Documentation/devicetree/bindings/usb/typec-switch.yaml new file mode 100644 index ..78b0190c8543 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/typec-switch.yaml @@ -0,0 +1,74 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/usb/typec-switch.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: USB Type-C Switch + +maintainers: + - Prashant Malani + +description: + A USB Type-C switch represents a component which routes USB Type-C data + lines to various protocol host controllers (e.g USB, VESA DisplayPort, + Thunderbolt etc.) depending on which mode the Type-C port, port partner + and cable are operating in. It can also modify lane routing based on + the orientation of a connected Type-C peripheral. + +properties: + compatible: +const: typec-switch + + mode-switch: +type: boolean +description: Specify that this switch can handle alternate mode switching. + + orientation-switch: +type: boolean +description: Specify that this switch can handle orientation switching. + + ports: +$ref: /schemas/graph.yaml#/properties/ports +description: OF graph binding modelling data lines to the Type-C switch. + +properties: + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: Link between the switch and a Type-C connector. + +required: + - port@0 + +required: + - compatible + - ports + +anyOf: + - required: + - mode-switch + - required: + - orientation-switch + +additionalProperties: true + +examples: + - | +drm-bridge { +usb-switch { +compatible = "typec-switch"; +mode-switch; +orientation-switch; +ports { +#address-cells = <1>; +#size-cells = <0>; + +port@0 { +reg = <0>; +anx_ep: endpoint { +remote-endpoint = <_controller>; +}; +}; +}; +}; +}; -- 2.36.1.476.g0c4daa206d-goog
[PATCH v4 2/7] usb: typec: mux: Add CONFIG guards for functions
There are some drivers that can use the Type C mux API, but don't have to. Introduce CONFIG guards for the mux functions so that drivers can include the header file and not run into compilation errors on systems which don't have CONFIG_TYPEC enabled. When CONFIG_TYPEC is not enabled, the Type C mux functions will be stub versions of the original calls. Reported-by: kernel test robot Reviewed-by: Nícolas F. R. A. Prado Tested-by: Nícolas F. R. A. Prado Signed-off-by: Prashant Malani --- Changes since v3: - No changes. Changes since v2: - Fix up return types for some of the stubs. Remove 1 unnecessary stub in the else condition. - Remove unnecessary IS_MODULE config guard. - Added Reviewed-by and Tested-by tags. Changes since v1: - Added static inline to stub functions. - Updated function signature of stub functions from "struct typec_mux" to "struct typec_mux_dev" in accordance with updates from commit 713fd49b430c ("usb: typec: mux: Introduce indirection") include/linux/usb/typec_mux.h | 44 ++- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h index ee57781dcf28..9292f0e07846 100644 --- a/include/linux/usb/typec_mux.h +++ b/include/linux/usb/typec_mux.h @@ -58,17 +58,13 @@ struct typec_mux_desc { void *drvdata; }; +#if IS_ENABLED(CONFIG_TYPEC) + struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode, const struct typec_altmode_desc *desc); void typec_mux_put(struct typec_mux *mux); int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state); -static inline struct typec_mux * -typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc) -{ - return fwnode_typec_mux_get(dev_fwnode(dev), desc); -} - struct typec_mux_dev * typec_mux_register(struct device *parent, const struct typec_mux_desc *desc); void typec_mux_unregister(struct typec_mux_dev *mux); @@ -76,4 +72,40 @@ void typec_mux_unregister(struct typec_mux_dev *mux); void typec_mux_set_drvdata(struct typec_mux_dev *mux, void *data); void *typec_mux_get_drvdata(struct typec_mux_dev *mux); +#else + +static inline struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode, + const struct typec_altmode_desc *desc) +{ + return NULL; +} + +static inline void typec_mux_put(struct typec_mux *mux) {} + +static inline int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state) +{ + return 0; +} + +static inline struct typec_mux_dev * +typec_mux_register(struct device *parent, const struct typec_mux_desc *desc) +{ + return ERR_PTR(-EOPNOTSUPP); +} +static inline void typec_mux_unregister(struct typec_mux_dev *mux) {} + +static inline void typec_mux_set_drvdata(struct typec_mux_dev *mux, void *data) {} +static inline void *typec_mux_get_drvdata(struct typec_mux_dev *mux) +{ + return ERR_PTR(-EOPNOTSUPP); +} + +#endif /* CONFIG_TYPEC */ + +static inline struct typec_mux * +typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc) +{ + return fwnode_typec_mux_get(dev_fwnode(dev), desc); +} + #endif /* __USB_TYPEC_MUX */ -- 2.36.1.476.g0c4daa206d-goog
[PATCH v4 1/7] usb: typec: mux: Allow muxes to specify mode-switch
Loosen the typec_mux_match() requirements so that searches where an alt mode is not specified, but the target mux device lists the "mode-switch" property, return a success. This is helpful in Type C port drivers which would like to get a pointer to the mux switch associated with a Type C port, but don't want to specify a particular alt mode. Signed-off-by: Prashant Malani Reviewed-by: Heikki Krogerus Reviewed-by: AngeloGioacchino Del Regno Reviewed-by: Nícolas F. R. A. Prado Tested-by: Nícolas F. R. A. Prado --- Changes since v3: - No changes. Changes since v2: - Included Reviewed-by and Tested-by tags from v2. Changes since v1: - No changes. drivers/usb/typec/mux.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c index fd55c2c516a5..464330776cd6 100644 --- a/drivers/usb/typec/mux.c +++ b/drivers/usb/typec/mux.c @@ -281,9 +281,13 @@ static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id, if (match) goto find_mux; - /* Accessory Mode muxes */ if (!desc) { - match = fwnode_property_present(fwnode, "accessory"); + /* +* Accessory Mode muxes & muxes which explicitly specify +* the required identifier can avoid SVID matching. +*/ + match = fwnode_property_present(fwnode, "accessory") || + fwnode_property_present(fwnode, id); if (match) goto find_mux; return NULL; -- 2.36.1.476.g0c4daa206d-goog
[PATCH v4 0/7] usb: typec: Introduce typec-switch binding
This series introduces a binding for Type-C data lane switches. These control the routing and operating modes of USB Type-C data lanes based on the PD messaging from the Type-C port driver regarding connected peripherals. The first patch introduces a change to the Type-C mux class mode-switch matching code, while the second adds a config guard to a Type-C header. The next couple of patches introduce the new "typec-switch" binding as well as one user of it (the ANX7625 drm bridge). The remaining patches add functionality to the anx7625 driver to register the mode-switches, as well as program its crosspoint switch depending on which Type-C port has a DisplayPort (DP) peripheral connected to it. v3: https://lore.kernel.org/linux-usb/20220614193558.1163205-1-pmal...@chromium.org/ Changes since v3: - Some more modifications to the anx7625 binding patch (4/7). - Picked up 1 more Reviewed-by tag. Pin-Yen Lin (1): drm/bridge: anx7625: Add typec_mux_set callback function Prashant Malani (6): usb: typec: mux: Allow muxes to specify mode-switch usb: typec: mux: Add CONFIG guards for functions dt-bindings: usb: Add Type-C switch binding dt-bindings: drm/bridge: anx7625: Add mode-switch support drm/bridge: anx7625: Register number of Type C switches drm/bridge: anx7625: Register Type-C mode switches .../display/bridge/analogix,anx7625.yaml | 64 .../devicetree/bindings/usb/typec-switch.yaml | 74 + drivers/gpu/drm/bridge/analogix/anx7625.c | 148 ++ drivers/gpu/drm/bridge/analogix/anx7625.h | 20 +++ drivers/usb/typec/mux.c | 8 +- include/linux/usb/typec_mux.h | 44 +- 6 files changed, 350 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/typec-switch.yaml -- 2.36.1.476.g0c4daa206d-goog
Re: [PATCH 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
On 6/15/2022 10:04 AM, Dmitry Baryshkov wrote: On 15/06/2022 19:40, Abhinav Kumar wrote: On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: On 14/06/2022 22:32, Abhinav Kumar wrote: intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..e991d4ba8a40 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) - phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); - - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) - phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); - - if (!phys->hw_intf && !phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "no intf or wb block assigned at idx: %d\n", i); - return; - } - - if (phys->hw_intf && phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "invalid phys both intf and wb block at idx: %d\n", i); - return; - } Please retain these checks in dpu_encoder_setup_display(). It checks that we really have got the intf or wb. For example one might have specified the INTF that leads to INTF_NONE interface. Or non-existing/not supported WB. Right, so the reason I omitted that was dpu_encoder_setup_display() already has these checks: https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 Please check lines 2273-2284. Only if all those checks succeeded we call dpu_encoder_virt_add_phys_encs which increments num_phys_encs. As I wrote, it checks indices from phys_params, but not the acquired hardware instances. Right but today, both the get_intf() and get_wb() just return the intf/wb corresponding to the index. So as long as the index is valid how will checking hw_wb or hw_intf be different? static inline struct dpu_hw_intf *dpu_rm_get_intf(struct dpu_rm *rm, enum dpu_intf intf_idx) { return rm->hw_intf[intf_idx - INTF_0]; } /** * dpu_rm_get_wb - Return a struct dpu_hw_wb instance given it's index. * @rm: DPU Resource Manager handle * @wb_idx: WB index */ static inline struct dpu_hw_wb *dpu_rm_get_wb(struct dpu_rm *rm, enum dpu_wb wb_idx) { return rm->hw_wb[wb_idx - WB_0]; } Thats why I dropped those. Let me know if you have more questions. - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(>vsync_cnt, 0); atomic_set(>underrun_cnt, 0); + + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) + phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); + + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); } + mutex_unlock(_enc->enc_lock); return ret;
Re: [Intel-gfx] [PATCH 2/6] drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
On Wed, Jun 15, 2022 at 04:27:36PM +0100, Mauro Carvalho Chehab wrote: From: Chris Wilson On gen12 HW, ensure that the TLB of the OA unit is also invalidated as just invalidating the TLB of an engine is not enough. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Signed-off-by: Chris Wilson Cc: Fei Yang Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/gt/intel_gt.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index d5ed6a6ac67c..61b7ec5118f9 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -10,6 +10,7 @@ #include "pxp/intel_pxp.h" #include "i915_drv.h" +#include "i915_perf_oa_regs.h" #include "intel_context.h" #include "intel_engine_pm.h" #include "intel_engine_regs.h" @@ -1259,6 +1260,15 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) awake |= engine->mask; } + /* Wa_2207587034:tgl,dg1,rkl,adl-s,adl-p */ + if (awake && + (IS_TIGERLAKE(i915) || +IS_DG1(i915) || +IS_ROCKETLAKE(i915) || +IS_ALDERLAKE_S(i915) || +IS_ALDERLAKE_P(i915))) + intel_uncore_write_fw(uncore, GEN12_OA_TLB_INV_CR, 1); + This patch can be dropped since this is being done in i915/i915_perf.c -> gen12_oa_disable and is synchronized with OA use cases. Regards, Umesh for_each_engine_masked(engine, gt, awake, tmp) { struct reg_and_bit rb; -- 2.36.1
Re: [PATCH 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
On 15/06/2022 19:40, Abhinav Kumar wrote: On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: On 14/06/2022 22:32, Abhinav Kumar wrote: intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..e991d4ba8a40 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) - phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); - - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) - phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); - - if (!phys->hw_intf && !phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "no intf or wb block assigned at idx: %d\n", i); - return; - } - - if (phys->hw_intf && phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "invalid phys both intf and wb block at idx: %d\n", i); - return; - } Please retain these checks in dpu_encoder_setup_display(). It checks that we really have got the intf or wb. For example one might have specified the INTF that leads to INTF_NONE interface. Or non-existing/not supported WB. Right, so the reason I omitted that was dpu_encoder_setup_display() already has these checks: https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 Please check lines 2273-2284. Only if all those checks succeeded we call dpu_encoder_virt_add_phys_encs which increments num_phys_encs. As I wrote, it checks indices from phys_params, but not the acquired hardware instances. Thats why I dropped those. Let me know if you have more questions. - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(>vsync_cnt, 0); atomic_set(>underrun_cnt, 0); + + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) + phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); + + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); } + mutex_unlock(_enc->enc_lock); return ret; -- With best wishes Dmitry
Re: [PATCH v2 6/7] drm/bridge: anx7625: Register Type-C mode switches
On Wed, Jun 15, 2022 at 1:45 AM AngeloGioacchino Del Regno wrote: > > Il 14/06/22 18:57, Prashant Malani ha scritto: > > On Tue, Jun 14, 2022 at 1:18 AM AngeloGioacchino Del Regno > > wrote: > >> > >> Il 09/06/22 20:09, Prashant Malani ha scritto: > >>> When the DT node has "switches" available, register a Type-C mode-switch > >>> for each listed "switch". This allows the driver to receive state > >>> information about what operating mode a Type-C port and its connected > >>> peripherals are in, as well as status information (like VDOs) related to > >>> that state. > >>> > >>> The callback function is currently a stub, but subsequent patches will > >>> implement the required functionality. > >>> > >>> Signed-off-by: Prashant Malani > >>> --- > >>> > >>> Changes since v2: > >>> - No changes. > >>> > >>>drivers/gpu/drm/bridge/analogix/anx7625.c | 73 +++ > >>>drivers/gpu/drm/bridge/analogix/anx7625.h | 6 ++ > >>>2 files changed, 79 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > >>> b/drivers/gpu/drm/bridge/analogix/anx7625.c > >>> index 07ed44c6b839..d41a21103bd3 100644 > >>> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > >>> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > >>> @@ -15,6 +15,7 @@ > >>>#include > >>>#include > >>>#include > >>> +#include > >>>#include > >>> > >>>#include > >>> @@ -2581,9 +2582,59 @@ static void anx7625_runtime_disable(void *data) > >>>pm_runtime_disable(data); > >>>} > >>> > >>> +static int anx7625_typec_mux_set(struct typec_mux_dev *mux, > >>> + struct typec_mux_state *state) > >>> +{ > >>> + return 0; > >>> +} > >>> + > >>> +static int anx7625_register_mode_switch(struct device *dev, struct > >>> device_node *node, > >>> + struct anx7625_data *ctx) > >>> +{ > >>> + struct anx7625_port_data *port_data; > >>> + struct typec_mux_desc mux_desc = {}; > >>> + char name[32]; > >>> + u32 port_num; > >>> + int ret; > >>> + > >>> + ret = of_property_read_u32(node, "reg", _num); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + if (port_num >= ctx->num_typec_switches) { > >>> + dev_err(dev, "Invalid port number specified: %d\n", > >>> port_num); > >>> + return -EINVAL; > >>> + } > >>> + > >>> + port_data = >typec_ports[port_num]; > >>> + port_data->ctx = ctx; > >>> + mux_desc.fwnode = >fwnode; > >>> + mux_desc.drvdata = port_data; > >>> + snprintf(name, sizeof(name), "%s-%u", node->name, port_num); > >>> + mux_desc.name = name; > >>> + mux_desc.set = anx7625_typec_mux_set; > >>> + > >>> + port_data->typec_mux = typec_mux_register(dev, _desc); > >>> + if (IS_ERR(port_data->typec_mux)) { > >>> + ret = PTR_ERR(port_data->typec_mux); > >>> + dev_err(dev, "Mode switch register for port %d failed: %d", > >>> port_num, ret); > >>> + } > >> > >> Please return 0 at the end of this function. > >> > >> if (IS_ERR()) { > >> ..code.. > >> return ret; > >> } > >> > >> return 0; > >> } > > > > May I ask why? We're not missing any return paths. I would rather we > > keep it as is (which has the valid return value). > > > > I know that you're not missing any return paths. > > That's only because the proposed one is a common pattern in the kernel > and it's only for consistency. Thanks for the additional details. Since this isn't addressing any specific bug, and I notice varied usages of "return ret" in this file itself [1][2], I'd prefer keeping it as is. [1]: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix/anx7625.c#L296 [2]: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix/anx7625.c#L436 > > Regards, > Angelo >
Re: [PATCH] drm/radeon: Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Wed, Jun 15, 2022 at 8:33 AM hongao wrote: > > Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi for more > efficiency > > Tested on "Oland [Radeon HD 8570 / R7 240/340 OEM]" & "Caicos [R5 230]" Can you verify that drm_display_info.is_hdmi has been populated when all of these functions are called? Alex > > Signed-off-by: hongao > --- > drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++--- > drivers/gpu/drm/radeon/radeon_connectors.c | 12 ++-- > drivers/gpu/drm/radeon/radeon_display.c| 2 +- > drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++-- > 4 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c > b/drivers/gpu/drm/radeon/atombios_encoders.c > index 70bd84b7ef2b..393d471ba396 100644 > --- a/drivers/gpu/drm/radeon/atombios_encoders.c > +++ b/drivers/gpu/drm/radeon/atombios_encoders.c > @@ -714,7 +714,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder) > if (radeon_connector->use_digital && > (radeon_connector->audio == RADEON_AUDIO_ENABLE)) > return ATOM_ENCODER_MODE_HDMI; > - else if > (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) && > + else if (connector->display_info.is_hdmi && > (radeon_connector->audio == > RADEON_AUDIO_AUTO)) > return ATOM_ENCODER_MODE_HDMI; > else if (radeon_connector->use_digital) > @@ -733,7 +733,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder) > if (radeon_audio != 0) { > if (radeon_connector->audio == RADEON_AUDIO_ENABLE) > return ATOM_ENCODER_MODE_HDMI; > - else if > (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) && > + else if (connector->display_info.is_hdmi && > (radeon_connector->audio == > RADEON_AUDIO_AUTO)) > return ATOM_ENCODER_MODE_HDMI; > else > @@ -757,7 +757,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder) > } else if (radeon_audio != 0) { > if (radeon_connector->audio == RADEON_AUDIO_ENABLE) > return ATOM_ENCODER_MODE_HDMI; > - else if > (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) && > + else if (connector->display_info.is_hdmi && > (radeon_connector->audio == > RADEON_AUDIO_AUTO)) > return ATOM_ENCODER_MODE_HDMI; > else > diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c > b/drivers/gpu/drm/radeon/radeon_connectors.c > index 58db79921cd3..2fbec7bdd56b 100644 > --- a/drivers/gpu/drm/radeon/radeon_connectors.c > +++ b/drivers/gpu/drm/radeon/radeon_connectors.c > @@ -130,7 +130,7 @@ int radeon_get_monitor_bpc(struct drm_connector > *connector) > case DRM_MODE_CONNECTOR_DVII: > case DRM_MODE_CONNECTOR_HDMIB: > if (radeon_connector->use_digital) { > - if > (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { > + if (connector->display_info.is_hdmi) { > if (connector->display_info.bpc) > bpc = connector->display_info.bpc; > } > @@ -138,7 +138,7 @@ int radeon_get_monitor_bpc(struct drm_connector > *connector) > break; > case DRM_MODE_CONNECTOR_DVID: > case DRM_MODE_CONNECTOR_HDMIA: > - if > (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { > + if (connector->display_info.is_hdmi) { > if (connector->display_info.bpc) > bpc = connector->display_info.bpc; > } > @@ -147,7 +147,7 @@ int radeon_get_monitor_bpc(struct drm_connector > *connector) > dig_connector = radeon_connector->con_priv; > if ((dig_connector->dp_sink_type == > CONNECTOR_OBJECT_ID_DISPLAYPORT) || > (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) > || > - > drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { > + connector->display_info.is_hdmi) { > if (connector->display_info.bpc) > bpc = connector->display_info.bpc; > } > @@ -171,7 +171,7 @@ int radeon_get_monitor_bpc(struct drm_connector > *connector) > break; > } > > - if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { > + if (connector->display_info.is_hdmi) { > /* hdmi deep color only
Re: [PATCH 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: On 14/06/2022 22:32, Abhinav Kumar wrote: intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..e991d4ba8a40 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) - phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); - - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) - phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); - - if (!phys->hw_intf && !phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "no intf or wb block assigned at idx: %d\n", i); - return; - } - - if (phys->hw_intf && phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "invalid phys both intf and wb block at idx: %d\n", i); - return; - } Please retain these checks in dpu_encoder_setup_display(). It checks that we really have got the intf or wb. For example one might have specified the INTF that leads to INTF_NONE interface. Or non-existing/not supported WB. Right, so the reason I omitted that was dpu_encoder_setup_display() already has these checks: https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 Please check lines 2273-2284. Only if all those checks succeeded we call dpu_encoder_virt_add_phys_encs which increments num_phys_encs. Thats why I dropped those. Let me know if you have more questions. - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(>vsync_cnt, 0); atomic_set(>underrun_cnt, 0); + + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) + phys->hw_intf = dpu_rm_get_intf(_kms->rm, phys->intf_idx); + + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx); } + mutex_unlock(_enc->enc_lock); return ret;
[PATCH] drm/msm: Fix %d vs %u
From: Rob Clark In debugging fence rollover, I noticed that GPU state capture and devcore dumps were showing me negative fence numbers. Let's fix that and some related signed vs unsigned confusion. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index dd044d557c7c..ce3b508b7c2b 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -790,11 +790,11 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state, for (i = 0; i < gpu->nr_rings; i++) { drm_printf(p, " - id: %d\n", i); drm_printf(p, "iova: 0x%016llx\n", state->ring[i].iova); - drm_printf(p, "last-fence: %d\n", state->ring[i].seqno); - drm_printf(p, "retired-fence: %d\n", state->ring[i].fence); - drm_printf(p, "rptr: %d\n", state->ring[i].rptr); - drm_printf(p, "wptr: %d\n", state->ring[i].wptr); - drm_printf(p, "size: %d\n", MSM_GPU_RINGBUFFER_SZ); + drm_printf(p, "last-fence: %u\n", state->ring[i].seqno); + drm_printf(p, "retired-fence: %u\n", state->ring[i].fence); + drm_printf(p, "rptr: %u\n", state->ring[i].rptr); + drm_printf(p, "wptr: %u\n", state->ring[i].wptr); + drm_printf(p, "size: %u\n", MSM_GPU_RINGBUFFER_SZ); adreno_show_object(p, >ring[i].data, state->ring[i].data_size, >ring[i].encoded); -- 2.36.1
Re: [PATCH -next] drm/bridge: it6505: Add missing CRYPTO_HASH dependency
On Mon, 13 Jun 2022 at 16:54, Zheng Bin wrote: > > The driver uses crypto hash functions so it needs to select CRYPTO_HASH. > This fixes build errors: > > drivers/gpu/drm/bridge/ite-it6505.o: in function `it6505_hdcp_wait_ksv_list': > ite-it6505.c:(.text+0x4c26): undefined reference to `crypto_alloc_shash' > ite-it6505.c:(.text+0x4c6d): undefined reference to `crypto_shash_digest' > ite-it6505.c:(.text+0x4c7d): undefined reference to `crypto_destroy_tfm' > ite-it6505.c:(.text+0x4d69): undefined reference to `crypto_destroy_tfm' > > Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver") > Signed-off-by: Zheng Bin > --- > drivers/gpu/drm/bridge/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 8ffd601e68f9..1afe99dac0ff 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -94,6 +94,8 @@ config DRM_ITE_IT6505 > select DRM_KMS_HELPER > select DRM_DP_HELPER > select EXTCON > +select CRYPTO > +select CRYPTO_HASH > help >ITE IT6505 DisplayPort bridge chip driver. > > -- > 2.31.1 > Reviewed-by: Robert Foss Applied to drm-misc-next
[PATCH] drm/msm: Fix fence rollover issue
From: Rob Clark And while we are at it, let's start the fence counter close to the rollover point so that if issues slip in, they are more obvious. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_fence.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c index 3df255402a33..a35a6746c7cd 100644 --- a/drivers/gpu/drm/msm/msm_fence.c +++ b/drivers/gpu/drm/msm/msm_fence.c @@ -28,6 +28,14 @@ msm_fence_context_alloc(struct drm_device *dev, volatile uint32_t *fenceptr, fctx->fenceptr = fenceptr; spin_lock_init(>spinlock); + /* +* Start out close to the 32b fence rollover point, so we can +* catch bugs with fence comparisons. +*/ + fctx->last_fence = 0xff00; + fctx->completed_fence = fctx->last_fence; + *fctx->fenceptr = fctx->last_fence; + return fctx; } @@ -46,11 +54,12 @@ bool msm_fence_completed(struct msm_fence_context *fctx, uint32_t fence) (int32_t)(*fctx->fenceptr - fence) >= 0; } -/* called from workqueue */ +/* called from irq handler */ void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence) { spin_lock(>spinlock); - fctx->completed_fence = max(fence, fctx->completed_fence); + if (fence_after(fence, fctx->completed_fence)) + fctx->completed_fence = fence; spin_unlock(>spinlock); } -- 2.36.1
Re: [PATCH v2 1/3] drm/msm/dpu: Move LM CRC code into separate method
On 15/06/2022 19:11, Jessica Zhang wrote: On 6/15/2022 2:35 AM, Dmitry Baryshkov wrote: On Wed, 15 Jun 2022 at 00:13, Jessica Zhang wrote: Move layer mixer-specific section of dpu_crtc_get_crc() into a separate helper method. This way, we can make it easier to get CRCs from other HW blocks by adding other get_crc helper methods. Changes since V1: - Moved common bitmasks to dpu_hw_util.h - Moved common CRC methods to dpu_hw_util.c - Updated copyrights - Changed crcs array to a dynamically allocated array and added it as a member of crtc_state Signed-off-by: Jessica Zhang Please split this into separate patches. One for hw_util, one for the rest Sure --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 88 + drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 4 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 42 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 46 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 16 5 files changed, 124 insertions(+), 72 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index b56f777dbd0e..16742a66878e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2014-2021 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark @@ -88,6 +89,11 @@ static int dpu_crtc_verify_crc_source(struct drm_crtc *crtc, enum dpu_crtc_crc_source source = dpu_crtc_parse_crc_source(src_name); struct dpu_crtc_state *crtc_state = to_dpu_crtc_state(crtc->state); + if (crtc_state->crcs) { + kfree(crtc_state->crcs); + crtc_state->crcs = NULL; + } + if (source < 0) { DRM_DEBUG_DRIVER("Invalid source %s for CRTC%d\n", src_name, crtc->index); return -EINVAL; @@ -96,20 +102,37 @@ static int dpu_crtc_verify_crc_source(struct drm_crtc *crtc, if (source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER) *values_cnt = crtc_state->num_mixers; + crtc_state->crcs = kcalloc(*values_cnt, sizeof(crtc_state->crcs), GFP_KERNEL); + I do not quite like the idea of constantly allocating and freeing the crcs array. I'd suggest defining sensible MAX_CRC_VALUES, using a static array and verifying that no one over commits the MAX_CRC_VALUES. If at some point we decide that we really need the dynamic array, we can implement it later. We already had dynamic arrays and Rob had to fix it. See 00326bfa4e63 ("drm/msm/dpu: Remove dynamic allocation from atomic context"). Ah, got it... the reason I used a dynamic array here was because AFAIK we don't have a defined upper limit for how many drm_encoders can be connected to a CRTC simultaneously. Do you have a suggestion on what value we can set for MAX_CRC_VALUES? Three? Two for two hw_intfs? -- With best wishes Dmitry
Re: [PATCH v2 3/3] drm/msm/dpu: Add interface support for CRC debugfs
On 6/15/2022 2:44 AM, Dmitry Baryshkov wrote: On Wed, 15 Jun 2022 at 00:13, Jessica Zhang wrote: Add support for writing CRC values for the interface block to the debugfs by calling the necessary MISR setup/collect methods. Changes since V1: - Set values_cnt to only include phys with backing hw_intf - Loop over all drm_encs connected to crtc Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 49 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 64 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 22 +++ 4 files changed, 134 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 16742a66878e..8c9933b2337f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -79,6 +79,8 @@ static enum dpu_crtc_crc_source dpu_crtc_parse_crc_source(const char *src_name) if (!strcmp(src_name, "auto") || !strcmp(src_name, "lm")) return DPU_CRTC_CRC_SOURCE_LAYER_MIXER; + if (!strcmp(src_name, "intf")) + return DPU_CRTC_CRC_SOURCE_INTF; What about "encoder" / DPU_CRTC_CRC_SOURCE_ENCODER? You basically offload CRC generation/collection to the dpu_encoder, so I'd ignore the fact that only INTF's support MISR generation and use a more generic word here. return DPU_CRTC_CRC_SOURCE_INVALID; } @@ -99,8 +101,14 @@ static int dpu_crtc_verify_crc_source(struct drm_crtc *crtc, return -EINVAL; } - if (source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER) + if (source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER) { *values_cnt = crtc_state->num_mixers; + } else if (source == DPU_CRTC_CRC_SOURCE_INTF) { + struct drm_encoder *drm_enc; Zero values_cnt here. Noted. + + drm_for_each_encoder_mask(drm_enc, crtc->dev, crtc->state->encoder_mask) + *values_cnt += dpu_encoder_get_num_phys(drm_enc); + } crtc_state->crcs = kcalloc(*values_cnt, sizeof(crtc_state->crcs), GFP_KERNEL); @@ -123,6 +131,14 @@ static void dpu_crtc_setup_lm_misr(struct dpu_crtc_state *crtc_state) } } +static void dpu_crtc_setup_encoder_misr(struct drm_crtc *crtc) +{ + struct drm_encoder *drm_enc; + + drm_for_each_encoder_mask(drm_enc, crtc->dev, crtc->state->encoder_mask) + dpu_encoder_setup_misr(drm_enc); +} + static int dpu_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) { enum dpu_crtc_crc_source source = dpu_crtc_parse_crc_source(src_name); @@ -175,6 +191,8 @@ static int dpu_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) if (source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER) dpu_crtc_setup_lm_misr(crtc_state); + else if (source == DPU_CRTC_CRC_SOURCE_INTF) + dpu_crtc_setup_encoder_misr(crtc); else? > cleanup: drm_modeset_unlock(>mutex); @@ -220,11 +238,31 @@ static int dpu_crtc_get_lm_crc(struct drm_crtc *crtc, struct dpu_crtc_state *crt drm_crtc_accurate_vblank_count(crtc), crtc_state->crcs); } -static int dpu_crtc_get_crc(struct drm_crtc *crtc) +static int dpu_crtc_get_encoder_crc(struct drm_crtc *crtc) { - struct dpu_crtc_state *crtc_state; + struct drm_encoder *drm_enc; + struct dpu_crtc_state *crtc_state = to_dpu_crtc_state(crtc->state); + int rc, pos = 0; - crtc_state = to_dpu_crtc_state(crtc->state); + drm_for_each_encoder_mask(drm_enc, crtc->dev, crtc->state->encoder_mask) { + rc = dpu_encoder_get_crc(drm_enc, crtc_state->crcs, pos); + if (rc < 0) { + if (rc != -ENODATA) + DRM_DEBUG_DRIVER("MISR read failed\n"); + + return rc; + } + + pos += rc; + } + + return drm_crtc_add_crc_entry(crtc, true, + drm_crtc_accurate_vblank_count(crtc), crtc_state->crcs); +} + +static int dpu_crtc_get_crc(struct drm_crtc *crtc) +{ + struct dpu_crtc_state *crtc_state = to_dpu_crtc_state(crtc->state); Unnecessary change here. Please move it to the patch 1, which refactors this function. Noted. /* Skip first 2 frames in case of "uncooked" CRCs */ if (crtc_state->crc_frame_skip_count < 2) { @@ -235,6 +273,9 @@ static int dpu_crtc_get_crc(struct drm_crtc *crtc) if (crtc_state->crc_source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER) return dpu_crtc_get_lm_crc(crtc, crtc_state); + if (crtc_state->crc_source == DPU_CRTC_CRC_SOURCE_INTF) + return dpu_crtc_get_encoder_crc(crtc); + return 0; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index
Re: [PATCH v2 1/3] drm/msm/dpu: Move LM CRC code into separate method
On 6/15/2022 2:35 AM, Dmitry Baryshkov wrote: On Wed, 15 Jun 2022 at 00:13, Jessica Zhang wrote: Move layer mixer-specific section of dpu_crtc_get_crc() into a separate helper method. This way, we can make it easier to get CRCs from other HW blocks by adding other get_crc helper methods. Changes since V1: - Moved common bitmasks to dpu_hw_util.h - Moved common CRC methods to dpu_hw_util.c - Updated copyrights - Changed crcs array to a dynamically allocated array and added it as a member of crtc_state Signed-off-by: Jessica Zhang Please split this into separate patches. One for hw_util, one for the rest Sure --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 88 + drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 4 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 42 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 46 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 16 5 files changed, 124 insertions(+), 72 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index b56f777dbd0e..16742a66878e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2014-2021 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark @@ -88,6 +89,11 @@ static int dpu_crtc_verify_crc_source(struct drm_crtc *crtc, enum dpu_crtc_crc_source source = dpu_crtc_parse_crc_source(src_name); struct dpu_crtc_state *crtc_state = to_dpu_crtc_state(crtc->state); + if (crtc_state->crcs) { + kfree(crtc_state->crcs); + crtc_state->crcs = NULL; + } + if (source < 0) { DRM_DEBUG_DRIVER("Invalid source %s for CRTC%d\n", src_name, crtc->index); return -EINVAL; @@ -96,20 +102,37 @@ static int dpu_crtc_verify_crc_source(struct drm_crtc *crtc, if (source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER) *values_cnt = crtc_state->num_mixers; + crtc_state->crcs = kcalloc(*values_cnt, sizeof(crtc_state->crcs), GFP_KERNEL); + I do not quite like the idea of constantly allocating and freeing the crcs array. I'd suggest defining sensible MAX_CRC_VALUES, using a static array and verifying that no one over commits the MAX_CRC_VALUES. If at some point we decide that we really need the dynamic array, we can implement it later. We already had dynamic arrays and Rob had to fix it. See 00326bfa4e63 ("drm/msm/dpu: Remove dynamic allocation from atomic context"). Ah, got it... the reason I used a dynamic array here was because AFAIK we don't have a defined upper limit for how many drm_encoders can be connected to a CRTC simultaneously. Do you have a suggestion on what value we can set for MAX_CRC_VALUES? return 0; } +static void dpu_crtc_setup_lm_misr(struct dpu_crtc_state *crtc_state) +{ + struct dpu_crtc_mixer *m; + int i; + + for (i = 0; i < crtc_state->num_mixers; ++i) { + m = _state->mixers[i]; + + if (!m->hw_lm || !m->hw_lm->ops.setup_misr) + continue; + + /* Calculate MISR over 1 frame */ + m->hw_lm->ops.setup_misr(m->hw_lm, true, 1); + } +} + static int dpu_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) { enum dpu_crtc_crc_source source = dpu_crtc_parse_crc_source(src_name); enum dpu_crtc_crc_source current_source; struct dpu_crtc_state *crtc_state; struct drm_device *drm_dev = crtc->dev; - struct dpu_crtc_mixer *m; bool was_enabled; bool enable = false; - int i, ret = 0; + int ret = 0; if (source < 0) { DRM_DEBUG_DRIVER("Invalid CRC source %s for CRTC%d\n", src_name, crtc->index); @@ -137,6 +160,10 @@ static int dpu_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) goto cleanup; } else if (was_enabled && !enable) { + if (crtc_state->crcs) { + kfree(crtc_state->crcs); + crtc_state->crcs = NULL; + } drm_crtc_vblank_put(crtc); } @@ -146,16 +173,8 @@ static int dpu_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) crtc_state->crc_frame_skip_count = 0; - for (i = 0; i < crtc_state->num_mixers; ++i) { - m = _state->mixers[i]; - - if (!m->hw_lm || !m->hw_lm->ops.setup_misr) - continue; - - /* Calculate MISR over 1 frame */ - m->hw_lm->ops.setup_misr(m->hw_lm, true, 1); - } - + if (source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER) +
[PATCH] drm/arm/hdlcd: Simplify IRQ install/uninstall
Since we no longer need to conform to the structure of the various DRM IRQ callbacks, we can streamline the code by consolidating the piecemeal functions and passing around our private data structure directly. We're also a platform device so should never see IRQ_NOTCONNECTED either. Furthermore we can also get rid of all the unnecesary read-modify-write operations, since on install we know we cleared the whole interrupt mask before enabling the debug IRQs, and thus on uninstall we're always clearing everything as well. Signed-off-by: Robin Murphy --- drivers/gpu/drm/arm/hdlcd_drv.c | 62 + 1 file changed, 16 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c index 1f1171f2f16a..7d6aa9b3b577 100644 --- a/drivers/gpu/drm/arm/hdlcd_drv.c +++ b/drivers/gpu/drm/arm/hdlcd_drv.c @@ -41,8 +41,7 @@ static irqreturn_t hdlcd_irq(int irq, void *arg) { - struct drm_device *drm = arg; - struct hdlcd_drm_private *hdlcd = drm->dev_private; + struct hdlcd_drm_private *hdlcd = arg; unsigned long irq_status; irq_status = hdlcd_read(hdlcd, HDLCD_REG_INT_STATUS); @@ -70,61 +69,32 @@ static irqreturn_t hdlcd_irq(int irq, void *arg) return IRQ_HANDLED; } -static void hdlcd_irq_preinstall(struct drm_device *drm) -{ - struct hdlcd_drm_private *hdlcd = drm->dev_private; - /* Ensure interrupts are disabled */ - hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0); - hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0); -} - -static void hdlcd_irq_postinstall(struct drm_device *drm) -{ -#ifdef CONFIG_DEBUG_FS - struct hdlcd_drm_private *hdlcd = drm->dev_private; - unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK); - - /* enable debug interrupts */ - irq_mask |= HDLCD_DEBUG_INT_MASK; - - hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask); -#endif -} - -static int hdlcd_irq_install(struct drm_device *drm, int irq) +static int hdlcd_irq_install(struct hdlcd_drm_private *hdlcd) { int ret; - if (irq == IRQ_NOTCONNECTED) - return -ENOTCONN; + /* Ensure interrupts are disabled */ + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0); + hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0); - hdlcd_irq_preinstall(drm); - - ret = request_irq(irq, hdlcd_irq, 0, drm->driver->name, drm); + ret = request_irq(hdlcd->irq, hdlcd_irq, 0, "hdlcd", hdlcd); if (ret) return ret; - hdlcd_irq_postinstall(drm); +#ifdef CONFIG_DEBUG_FS + /* enable debug interrupts */ + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, HDLCD_DEBUG_INT_MASK); +#endif return 0; } -static void hdlcd_irq_uninstall(struct drm_device *drm) +static void hdlcd_irq_uninstall(struct hdlcd_drm_private *hdlcd) { - struct hdlcd_drm_private *hdlcd = drm->dev_private; /* disable all the interrupts that we might have enabled */ - unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK); + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0); -#ifdef CONFIG_DEBUG_FS - /* disable debug interrupts */ - irq_mask &= ~HDLCD_DEBUG_INT_MASK; -#endif - - /* disable vsync interrupts */ - irq_mask &= ~HDLCD_INTERRUPT_VSYNC; - hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask); - - free_irq(hdlcd->irq, drm); + free_irq(hdlcd->irq, hdlcd); } static int hdlcd_load(struct drm_device *drm, unsigned long flags) @@ -184,7 +154,7 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags) goto irq_fail; hdlcd->irq = ret; - ret = hdlcd_irq_install(drm, hdlcd->irq); + ret = hdlcd_irq_install(hdlcd); if (ret < 0) { DRM_ERROR("failed to install IRQ handler\n"); goto irq_fail; @@ -342,7 +312,7 @@ static int hdlcd_drm_bind(struct device *dev) err_unload: of_node_put(hdlcd->crtc.port); hdlcd->crtc.port = NULL; - hdlcd_irq_uninstall(drm); + hdlcd_irq_uninstall(hdlcd); of_reserved_mem_device_release(drm->dev); err_free: drm_mode_config_cleanup(drm); @@ -364,7 +334,7 @@ static void hdlcd_drm_unbind(struct device *dev) hdlcd->crtc.port = NULL; pm_runtime_get_sync(dev); drm_atomic_helper_shutdown(drm); - hdlcd_irq_uninstall(drm); + hdlcd_irq_uninstall(hdlcd); pm_runtime_put(dev); if (pm_runtime_enabled(dev)) pm_runtime_disable(dev); -- 2.36.1.dirty
[PATCH v2] drm/arm/hdlcd: Take over EFI framebuffer properly
The Arm Juno board EDK2 port has provided an EFI GOP display via HDLCD0 for some time now, which works nicely as an early framebuffer. However, once the HDLCD driver probes and takes over the hardware, it should take over the logical framebuffer as well, otherwise the now-defunct GOP device hangs about and virtual console output inevitably disappears into the wrong place most of the time. We'll do this after binding the HDMI encoder, since that's the most likely thing to fail, and the EFI console is still better than nothing when that happens. However, the two HDLCD controllers on Juno are independent, and many users will still be using older firmware without any display support, so we'll only bother if we find that the HDLCD we're probing is already enabled. And if it is, then we'll also stop it, since otherwise the display can end up shifted if it's still scanning out while the rest of the registers are subsequently reconfigured. Signed-off-by: Robin Murphy --- Since I ended up adding (relatively) a lot here, I didn't want to second-guess Javier's opinion so left off the R-b tag from v1. drivers/gpu/drm/arm/hdlcd_drv.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c index e89ae0ec60eb..1f1171f2f16a 100644 --- a/drivers/gpu/drm/arm/hdlcd_drv.c +++ b/drivers/gpu/drm/arm/hdlcd_drv.c @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -314,6 +315,12 @@ static int hdlcd_drm_bind(struct device *dev) goto err_vblank; } + /* If EFI left us running, take over from efifb/sysfb */ + if (hdlcd_read(hdlcd, HDLCD_REG_COMMAND)) { + hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0); + drm_aperture_remove_framebuffers(false, _driver); + } + drm_mode_config_reset(drm); drm_kms_helper_poll_init(drm); -- 2.36.1.dirty
Re: [PATCH] drm/bridge: anx7625: Zero error variable when panel bridge not present
On Tue, 14 Jun 2022 at 09:52, AngeloGioacchino Del Regno wrote: > > Il 13/06/22 18:37, Nícolas F. R. A. Prado ha scritto: > > While parsing the DT, the anx7625 driver checks for the presence of a > > panel bridge on endpoint 1. If it is missing, pdata->panel_bridge stores > > the error pointer and the function returns successfully without first > > cleaning that variable. This is an issue since other functions later > > check for the presence of a panel bridge by testing the trueness of that > > variable. > > > > In order to ensure proper behavior, zero out pdata->panel_bridge before > > returning when no panel bridge is found. > > > > Fixes: 9e82ea0fb1df ("drm/bridge: anx7625: switch to > > devm_drm_of_get_bridge") > > Signed-off-by: Nícolas F. R. A. Prado > > > > I would've preferred s/zero out/cleanup/g but it's also fine as you wrote it. > Besides, good catch! > > Reviewed-by: AngeloGioacchino Del Regno > > Applied to drm-misc-next
[PATCH v7] drm/msm/dp: force link training for display resolution change
Display resolution change is implemented through drm modeset. Older modeset (resolution) has to be disabled first before newer modeset (resolution) can be enabled. Display disable will turn off both pixel clock and main link clock so that main link have to be re-trained during display enable to have new video stream flow again. At current implementation, display enable function manually kicks up irq_hpd_handle which will read panel link status and start link training if link status is not in sync state. However, there is rare case that a particular panel links status keep staying in sync for some period of time after main link had been shut down previously at display disabled. In this case, main link retraining will not be executed by irq_hdp_handle(). Hence video stream of newer display resolution will fail to be transmitted to panel due to main link is not in sync between host and panel. This patch will bypass irq_hpd_handle() in favor of directly call dp_ctrl_on_stream() to always perform link training in regardless of main link status. So that no unexpected exception resolution change failure cases will happen. Also this implementation are more efficient than manual kicking off irq_hpd_handle function. Changes in v2: -- set force_link_train flag on DP only (is_edp == false) Changes in v3: -- revise commit text -- add Fixes tag Changes in v4: -- revise commit text Changes in v5: -- fix spelling at commit text Changes in v6: -- split dp_ctrl_on_stream() for phy test case -- revise commit text for modeset Changes in v7: -- drop 0 assignment at local variable (ret = 0) Fixes: 62671d2ef24b ("drm/msm/dp: fixes wrong connection state caused by failure of link train") Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/dp/dp_ctrl.c| 31 +++ drivers/gpu/drm/msm/dp/dp_ctrl.h| 3 ++- drivers/gpu/drm/msm/dp/dp_display.c | 13 ++--- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index af7a80c..01028b5 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1551,7 +1551,7 @@ static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl) ret = dp_ctrl_on_link(>dp_ctrl); if (!ret) - ret = dp_ctrl_on_stream(>dp_ctrl); + ret = dp_ctrl_on_stream_phy_test_report(>dp_ctrl); else DRM_ERROR("failed to enable DP link controller\n"); @@ -1807,7 +1807,27 @@ static int dp_ctrl_link_retrain(struct dp_ctrl_private *ctrl) return dp_ctrl_setup_main_link(ctrl, _step); } -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl) +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl) +{ + int ret; + struct dp_ctrl_private *ctrl; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock; + + ret = dp_ctrl_enable_stream_clocks(ctrl); + if (ret) { + DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret); + return ret; + } + + dp_ctrl_send_phy_test_pattern(ctrl); + + return 0; +} + +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train) { int ret = 0; bool mainlink_ready = false; @@ -1843,12 +1863,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl) goto end; } - if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) { - dp_ctrl_send_phy_test_pattern(ctrl); - return 0; - } - - if (!dp_ctrl_channel_eq_ok(ctrl)) + if (force_link_train || !dp_ctrl_channel_eq_ok(ctrl)) dp_ctrl_link_retrain(ctrl); /* stop txing train pattern to end link training */ diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h index 0745fde..9a39b00 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h @@ -21,7 +21,8 @@ struct dp_ctrl { }; int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl); -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl); +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train); +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl); int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl); int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl); int dp_ctrl_off(struct dp_ctrl *dp_ctrl); diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index c388323..b6d25ab 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -872,7 +872,7 @@ static int dp_display_enable(struct dp_display_private *dp, u32 data) return 0; } - rc = dp_ctrl_on_stream(dp->ctrl); + rc = dp_ctrl_on_stream(dp->ctrl, data); if (!rc) dp_display->power_on = true; @@ -1654,6 +1654,7 @@ void dp_bridge_enable(struct
Re: [PATCH] drm/sun4i: Fix crash during suspend after component bind failure
Dne sreda, 15. junij 2022 ob 07:42:53 CEST je Samuel Holland napisal(a): > If the component driver fails to bind, or is unbound, the driver data > for the top-level platform device points to a freed drm_device. If the > system is then suspended, the driver passes this dangling pointer to > drm_mode_config_helper_suspend(), which crashes. > > Fix this by only setting the driver data while the platform driver holds > a reference to the drm_device. > > Fixes: 624b4b48d9d8 ("drm: sun4i: Add support for suspending the display driver") > Signed-off-by: Samuel Holland Reviewed-by: Jernej Skrabec Best regards, Jernej
[PATCH] drm/rockchip: Detach from ARM DMA domain in attach_device
Since commit 1ea2a07a532b ("iommu: Add DMA ownership management interfaces") the Rockchip display driver on the Firefly RK3288 fails to initialise properly. This is because ARM DMA domain is still attached. Let's follow the lead of exynos and tegra and add code to explicitly remove the ARM domain before attaching a new one. Suggested-by: Robin Murphy Signed-off-by: Steven Price --- See also the thread[1] where I reported the regression. [1] https://lore.kernel.org/linux-kernel/da9cca0a-ec5b-2e73-9de0-a930f7d947b2%40arm.com --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 67d38f53d3e5..13ed33e74457 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -23,6 +23,14 @@ #include #include +#if defined(CONFIG_ARM_DMA_USE_IOMMU) +#include +#else +#define arm_iommu_detach_device(...) ({ }) +#define arm_iommu_release_mapping(...) ({ }) +#define to_dma_iommu_mapping(dev) NULL +#endif + #include "rockchip_drm_drv.h" #include "rockchip_drm_fb.h" #include "rockchip_drm_gem.h" @@ -49,6 +57,15 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev, if (!private->domain) return 0; + if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) { + struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev); + + if (mapping) { + arm_iommu_detach_device(dev); + arm_iommu_release_mapping(mapping); + } + } + ret = iommu_attach_device(private->domain, dev); if (ret) { DRM_DEV_ERROR(dev, "Failed to attach iommu device\n"); -- 2.25.1
[PATCH 2/6] drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
From: Chris Wilson On gen12 HW, ensure that the TLB of the OA unit is also invalidated as just invalidating the TLB of an engine is not enough. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Signed-off-by: Chris Wilson Cc: Fei Yang Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/gt/intel_gt.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index d5ed6a6ac67c..61b7ec5118f9 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -10,6 +10,7 @@ #include "pxp/intel_pxp.h" #include "i915_drv.h" +#include "i915_perf_oa_regs.h" #include "intel_context.h" #include "intel_engine_pm.h" #include "intel_engine_regs.h" @@ -1259,6 +1260,15 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) awake |= engine->mask; } + /* Wa_2207587034:tgl,dg1,rkl,adl-s,adl-p */ + if (awake && + (IS_TIGERLAKE(i915) || +IS_DG1(i915) || +IS_ROCKETLAKE(i915) || +IS_ALDERLAKE_S(i915) || +IS_ALDERLAKE_P(i915))) + intel_uncore_write_fw(uncore, GEN12_OA_TLB_INV_CR, 1); + for_each_engine_masked(engine, gt, awake, tmp) { struct reg_and_bit rb; -- 2.36.1
[PATCH 1/6] drm/i915/gt: Ignore TLB invalidations on idle engines
From: Chris Wilson As an extension of the current skip TLB invalidations, check if the device is powered down prior to any engine activity, as, on such cases, all the TLBs were already invalidated, so an explicit TLB invalidation is not needed. This becomes more significant with GuC, as it can only do so when the connection to the GuC is awake. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Signed-off-by: Chris Wilson Cc: Fei Yang Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/gem/i915_gem_pages.c | 10 + drivers/gpu/drm/i915/gt/intel_gt.c| 26 +-- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 3 +++ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c index 97c820eee115..6835279943df 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c @@ -6,14 +6,15 @@ #include +#include "gt/intel_gt.h" +#include "gt/intel_gt_pm.h" + #include "i915_drv.h" #include "i915_gem_object.h" #include "i915_scatterlist.h" #include "i915_gem_lmem.h" #include "i915_gem_mman.h" -#include "gt/intel_gt.h" - void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj, struct sg_table *pages, unsigned int sg_page_sizes) @@ -217,10 +218,11 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj) if (test_and_clear_bit(I915_BO_WAS_BOUND_BIT, >flags)) { struct drm_i915_private *i915 = to_i915(obj->base.dev); + struct intel_gt *gt = to_gt(i915); intel_wakeref_t wakeref; - with_intel_runtime_pm_if_active(>runtime_pm, wakeref) - intel_gt_invalidate_tlbs(to_gt(i915)); + with_intel_gt_pm_if_awake(gt, wakeref) + intel_gt_invalidate_tlbs(gt); } return pages; diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index f33290358c51..d5ed6a6ac67c 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -11,6 +11,7 @@ #include "i915_drv.h" #include "intel_context.h" +#include "intel_engine_pm.h" #include "intel_engine_regs.h" #include "intel_gt.h" #include "intel_gt_buffer_pool.h" @@ -1216,6 +1217,7 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) struct drm_i915_private *i915 = gt->i915; struct intel_uncore *uncore = gt->uncore; struct intel_engine_cs *engine; + intel_engine_mask_t awake, tmp; enum intel_engine_id id; const i915_reg_t *regs; unsigned int num = 0; @@ -1239,12 +1241,27 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) GEM_TRACE("\n"); - assert_rpm_wakelock_held(>runtime_pm); - mutex_lock(>tlb_invalidate_lock); intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL); + awake = 0; for_each_engine(engine, gt, id) { + struct reg_and_bit rb; + + if (!intel_engine_pm_is_awake(engine)) + continue; + + rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num); + if (!i915_mmio_reg_offset(rb.reg)) + continue; + + intel_uncore_write_fw(uncore, rb.reg, rb.bit); + awake |= engine->mask; + } + + for_each_engine_masked(engine, gt, awake, tmp) { + struct reg_and_bit rb; + /* * HW architecture suggest typical invalidation time at 40us, * with pessimistic cases up to 100us and a recommendation to @@ -1252,13 +1269,8 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) */ const unsigned int timeout_us = 100; const unsigned int timeout_ms = 4; - struct reg_and_bit rb; rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num); - if (!i915_mmio_reg_offset(rb.reg)) - continue; - - intel_uncore_write_fw(uncore, rb.reg, rb.bit); if (__intel_wait_for_register_fw(uncore, rb.reg, rb.bit, 0, timeout_us, timeout_ms, diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index bc898df7a48c..a334787a4939 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -55,6 +55,9 @@ static inline void intel_gt_pm_might_put(struct intel_gt *gt) for (tmp = 1, intel_gt_pm_get(gt); tmp; \ intel_gt_pm_put(gt), tmp = 0)
[PATCH 3/6] drm/i915/gt: Skip TLB invalidations once wedged
From: Chris Wilson Skip all further TLB invalidations once the device is wedged and had been reset, as, on such cases, it can no longer process instructions on the GPU and the user no longer has access to the TLB's in each engine. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Signed-off-by: Chris Wilson Cc: Fei Yang Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/gt/intel_gt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 61b7ec5118f9..fb4fd5273ca4 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -1226,6 +1226,9 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) if (I915_SELFTEST_ONLY(gt->awake == -ENODEV)) return; + if (intel_gt_is_wedged(gt)) + return; + if (GRAPHICS_VER(i915) == 12) { regs = gen12_regs; num = ARRAY_SIZE(gen12_regs); -- 2.36.1
[PATCH 0/6] Fix TLB invalidate issues with Broadwell
i915 selftest hangcheck is causing the i915 driver timeouts, as reported by Intel CI bot: http://gfx-ci.fi.intel.com/cibuglog-ng/issuefilterassoc/24297?query_key=42a999f48fa6ecce068bc8126c069be7c31153b4 When such test runs, the only output is: [ 68.811639] i915: Performing live selftests with st_random_seed=0xe138eac7 st_timeout=500 [ 68.811792] i915: Running hangcheck [ 68.811859] i915: Running intel_hangcheck_live_selftests/igt_hang_sanitycheck [ 68.816910] i915 :00:02.0: [drm] Cannot find any crtc or sizes [ 68.841597] i915: Running intel_hangcheck_live_selftests/igt_reset_nop [ 69.346347] igt_reset_nop: 80 resets [ 69.362695] i915: Running intel_hangcheck_live_selftests/igt_reset_nop_engine [ 69.863559] igt_reset_nop_engine(rcs0): 709 resets [ 70.364924] igt_reset_nop_engine(bcs0): 903 resets [ 70.866005] igt_reset_nop_engine(vcs0): 659 resets [ 71.367934] igt_reset_nop_engine(vcs1): 549 resets [ 71.869259] igt_reset_nop_engine(vecs0): 553 resets [ 71.882592] i915: Running intel_hangcheck_live_selftests/igt_reset_idle_engine [ 72.383554] rcs0: Completed 16605 idle resets [ 72.884599] bcs0: Completed 18641 idle resets [ 73.385592] vcs0: Completed 17517 idle resets [ 73.886658] vcs1: Completed 15474 idle resets [ 74.387600] vecs0: Completed 17983 idle resets [ 74.387667] i915: Running intel_hangcheck_live_selftests/igt_reset_active_engine [ 74.889017] rcs0: Completed 747 active resets [ 75.174240] intel_engine_reset(bcs0) failed, err:-110 [ 75.174301] bcs0: Completed 525 active resets After that, the machine just silently hangs. Bisecting the issue, the patch that introduced the regression is: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Reverting it fix the issues, but introduce other problems, as TLB won't be invalidated anymore. So, instead, let's fix the root cause. It turns that the TLB flush logic ends conflicting with i915 reset, which is called during selftest hangcheck. So, the TLB cache should be serialized, but other TLB fix patches are required for this one to work. Tested on an Intel NUC5i7RYB with an i7-5557U Broadwell CPU. Chris Wilson (6): drm/i915/gt: Ignore TLB invalidations on idle engines drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations drm/i915/gt: Skip TLB invalidations once wedged drm/i915/gt: Only invalidate TLBs exposed to user manipulation drm/i915/gt: Serialize GRDOM access between multiple engine resets drm/i915/gt: Serialize TLB invalidates with GT resets drivers/gpu/drm/i915/gem/i915_gem_pages.c | 10 +++--- drivers/gpu/drm/i915/gt/intel_gt.c| 43 +++ drivers/gpu/drm/i915/gt/intel_gt_pm.h | 3 ++ drivers/gpu/drm/i915/gt/intel_reset.c | 37 ++- drivers/gpu/drm/i915/i915_vma.c | 3 +- 5 files changed, 75 insertions(+), 21 deletions(-) -- 2.36.1
[PATCH 4/6] drm/i915/gt: Only invalidate TLBs exposed to user manipulation
From: Chris Wilson Don't flush TLBs when the buffer is only used in the GGTT under full control of the kernel, as there's no risk of of concurrent access and stale access from prefetch. We only need to invalidate the TLB if they are accessible by the user. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Signed-off-by: Chris Wilson Cc: Fei Yang Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/i915_vma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 0bffb70b3c5f..7989986161e8 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -537,7 +537,8 @@ int i915_vma_bind(struct i915_vma *vma, bind_flags); } - set_bit(I915_BO_WAS_BOUND_BIT, >obj->flags); + if (bind_flags & I915_VMA_LOCAL_BIND) + set_bit(I915_BO_WAS_BOUND_BIT, >obj->flags); atomic_or(bind_flags, >flags); return 0; -- 2.36.1
[PATCH 6/6] drm/i915/gt: Serialize TLB invalidates with GT resets
From: Chris Wilson Avoid trying to invalidate the TLB in the middle of performing an engine reset, as this may result in the reset timing out. Currently, the TLB invalidate is only serialised by its own mutex, forgoing the uncore lock, but we can take the uncore->lock as well to serialise the mmio access, thereby serialising with the GDRST. Tested on a NUC5i7RYB, BIOS RYBDWi35.86A.0380.2019.0517.1530 with i915 selftest/hangcheck. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Reported-by: Mauro Carvalho Chehab Tested-by: Mauro Carvalho Chehab Reviewed-by: Mauro Carvalho Chehab Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/gt/intel_gt.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index fb4fd5273ca4..33eb93586858 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -1248,6 +1248,8 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) mutex_lock(>tlb_invalidate_lock); intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL); + spin_lock_irq(>lock); /* seralise invalidate with GT reset */ + awake = 0; for_each_engine(engine, gt, id) { struct reg_and_bit rb; @@ -1272,6 +1274,8 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) IS_ALDERLAKE_P(i915))) intel_uncore_write_fw(uncore, GEN12_OA_TLB_INV_CR, 1); + spin_unlock_irq(>lock); + for_each_engine_masked(engine, gt, awake, tmp) { struct reg_and_bit rb; -- 2.36.1
[PATCH 5/6] drm/i915/gt: Serialize GRDOM access between multiple engine resets
From: Chris Wilson Don't allow two engines to be reset in parallel, as they would both try to select a reset bit (and send requests to common registers) and wait on that register, at the same time. Serialize control of the reset requests/acks using the uncore->lock, which will also ensure that no other GT state changes at the same time as the actual reset. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Reported-by: Mika Kuoppala Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/gt/intel_reset.c | 37 --- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index a5338c3fde7a..c68d36fb5bbd 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -300,9 +300,9 @@ static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask) return err; } -static int gen6_reset_engines(struct intel_gt *gt, - intel_engine_mask_t engine_mask, - unsigned int retry) +static int __gen6_reset_engines(struct intel_gt *gt, + intel_engine_mask_t engine_mask, + unsigned int retry) { struct intel_engine_cs *engine; u32 hw_mask; @@ -321,6 +321,20 @@ static int gen6_reset_engines(struct intel_gt *gt, return gen6_hw_domain_reset(gt, hw_mask); } +static int gen6_reset_engines(struct intel_gt *gt, + intel_engine_mask_t engine_mask, + unsigned int retry) +{ + unsigned long flags; + int ret; + + spin_lock_irqsave(>uncore->lock, flags); + ret = __gen6_reset_engines(gt, engine_mask, retry); + spin_unlock_irqrestore(>uncore->lock, flags); + + return ret; +} + static struct intel_engine_cs *find_sfc_paired_vecs_engine(struct intel_engine_cs *engine) { int vecs_id; @@ -487,9 +501,9 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine) rmw_clear_fw(uncore, sfc_lock.lock_reg, sfc_lock.lock_bit); } -static int gen11_reset_engines(struct intel_gt *gt, - intel_engine_mask_t engine_mask, - unsigned int retry) +static int __gen11_reset_engines(struct intel_gt *gt, +intel_engine_mask_t engine_mask, +unsigned int retry) { struct intel_engine_cs *engine; intel_engine_mask_t tmp; @@ -583,8 +597,11 @@ static int gen8_reset_engines(struct intel_gt *gt, struct intel_engine_cs *engine; const bool reset_non_ready = retry >= 1; intel_engine_mask_t tmp; + unsigned long flags; int ret; + spin_lock_irqsave(>uncore->lock, flags); + for_each_engine_masked(engine, gt, engine_mask, tmp) { ret = gen8_engine_reset_prepare(engine); if (ret && !reset_non_ready) @@ -612,17 +629,19 @@ static int gen8_reset_engines(struct intel_gt *gt, * This is best effort, so ignore any error from the initial reset. */ if (IS_DG2(gt->i915) && engine_mask == ALL_ENGINES) - gen11_reset_engines(gt, gt->info.engine_mask, 0); + __gen11_reset_engines(gt, gt->info.engine_mask, 0); if (GRAPHICS_VER(gt->i915) >= 11) - ret = gen11_reset_engines(gt, engine_mask, retry); + ret = __gen11_reset_engines(gt, engine_mask, retry); else - ret = gen6_reset_engines(gt, engine_mask, retry); + ret = __gen6_reset_engines(gt, engine_mask, retry); skip_reset: for_each_engine_masked(engine, gt, engine_mask, tmp) gen8_engine_reset_cancel(engine); + spin_unlock_irqrestore(>uncore->lock, flags); + return ret; } -- 2.36.1
Re: [Intel-gfx] [PATCH 3/3] drm/doc/rfc: VM_BIND uapi definition
On Wed, Jun 15, 2022 at 08:22:23AM +0100, Tvrtko Ursulin wrote: On 14/06/2022 17:42, Niranjana Vishwanathapura wrote: On Tue, Jun 14, 2022 at 05:07:37PM +0100, Tvrtko Ursulin wrote: On 14/06/2022 17:02, Tvrtko Ursulin wrote: On 14/06/2022 16:43, Niranjana Vishwanathapura wrote: On Tue, Jun 14, 2022 at 08:16:41AM +0100, Tvrtko Ursulin wrote: On 14/06/2022 00:39, Matthew Brost wrote: On Mon, Jun 13, 2022 at 07:09:06PM +0100, Tvrtko Ursulin wrote: On 13/06/2022 18:49, Niranjana Vishwanathapura wrote: On Mon, Jun 13, 2022 at 05:22:02PM +0100, Tvrtko Ursulin wrote: On 13/06/2022 16:05, Niranjana Vishwanathapura wrote: On Mon, Jun 13, 2022 at 09:24:18AM +0100, Tvrtko Ursulin wrote: On 10/06/2022 17:14, Niranjana Vishwanathapura wrote: On Fri, Jun 10, 2022 at 05:48:39PM +0300, Lionel Landwerlin wrote: On 10/06/2022 13:37, Tvrtko Ursulin wrote: On 10/06/2022 08:07, Niranjana Vishwanathapura wrote: VM_BIND and related uapi definitions Signed-off-by: Niranjana Vishwanathapura --- Documentation/gpu/rfc/i915_vm_bind.h | 490 +++ 1 file changed, 490 insertions(+) create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h new file mode 100644 index ..9fc854969cfb --- /dev/null +++ b/Documentation/gpu/rfc/i915_vm_bind.h @@ -0,0 +1,490 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2022 Intel Corporation + */ + +/** + * DOC: I915_PARAM_HAS_VM_BIND + * + * VM_BIND feature availability. + * See typedef drm_i915_getparam_t param. + * bit[0]: If set, VM_BIND is supported, otherwise not. + * bits[8-15]: VM_BIND implementation version. + * version 0 will not have VM_BIND/UNBIND timeline fence array support. + */ +#define I915_PARAM_HAS_VM_BIND 57 + +/** + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND + * + * Flag to opt-in for VM_BIND mode of binding during VM creation. + * See struct drm_i915_gem_vm_control flags. + * + * The older execbuf2 ioctl will not support VM_BIND mode of operation. + * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any + * execlist (See struct drm_i915_gem_execbuffer3 for more details). + * + */ +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0) + +/** + * DOC: I915_CONTEXT_CREATE_FLAGS_LONG_RUNNING + * + * Flag to declare context as long running. + * See struct drm_i915_gem_context_create_ext flags. + * + * Usage of dma-fence expects that they complete in reasonable amount of time. + * Compute on the other hand can be long running. Hence it is not appropriate + * for compute contexts to export request completion dma-fence to user. + * The dma-fence usage will be limited to in-kernel consumption only. + * Compute contexts need to use user/memory fence. + * + * So, long running contexts do not support output fences. Hence, + * I915_EXEC_FENCE_SIGNAL (See _i915_gem_exec_fence.flags) is expected + * to be not used. DRM_I915_GEM_WAIT ioctl call is also not supported for + * objects mapped to long running contexts. + */ +#define I915_CONTEXT_CREATE_FLAGS_LONG_RUNNING (1u << 2) + +/* VM_BIND related ioctls */ +#define DRM_I915_GEM_VM_BIND 0x3d +#define DRM_I915_GEM_VM_UNBIND 0x3e +#define DRM_I915_GEM_EXECBUFFER3 0x3f +#define DRM_I915_GEM_WAIT_USER_FENCE 0x40 + +#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind) +#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind) +#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3) +#define DRM_IOCTL_I915_GEM_WAIT_USER_FENCE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_WAIT_USER_FENCE, struct drm_i915_gem_wait_user_fence) + +/** + * struct drm_i915_gem_vm_bind - VA to object mapping to bind. + * + * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU + * virtual address (VA) range to the section of an object that should be bound + * in the device page table of the specified address space (VM). + * The VA range specified must be unique (ie., not currently bound) and can + * be mapped to whole object or a section of the object (partial binding). + * Multiple VA mappings can be created to the same section of the object + * (aliasing). + * + * The @queue_idx specifies the queue to use for binding. Same queue can be + * used for both VM_BIND and VM_UNBIND calls. All submitted bind and unbind + * operations in a queue are performed in the order of submission. + * + * The @start, @offset and @length should be 4K page aligned. However the DG2 + * and XEHPSDV has 64K page size for device local-memory and has compact page + * table. On those platforms, for binding device local-memory objects, the + * @start should be 2M aligned, @offset and @length should be 64K aligned. + * Also, on those platforms, it is not allowed to bind an device