Re: [PATCH 2/2] drm/nouveau/kms: Add INHERIT ioctl to nvkm/nvif for reading IOR state
Hi Lyude, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.3-rc5 next-20230406] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Lyude-Paul/drm-nouveau-kms-Add-INHERIT-ioctl-to-nvkm-nvif-for-reading-IOR-state/20230408-062329 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230407222133.1425969-2-lyude%40redhat.com patch subject: [PATCH 2/2] drm/nouveau/kms: Add INHERIT ioctl to nvkm/nvif for reading IOR state config: arm64-buildonly-randconfig-r001-20230403 (https://download.01.org/0day-ci/archive/20230408/202304081129.amxcmyn2-...@intel.com/config) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 2c57868e2e877f73c339796c3374ae660bb77f0d) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm64 cross compiling tool for clang build # apt-get install binutils-aarch64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/a3d963915cf6f2d87b57146f7bc57a6a89d90cf6 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Lyude-Paul/drm-nouveau-kms-Add-INHERIT-ioctl-to-nvkm-nvif-for-reading-IOR-state/20230408-062329 git checkout a3d963915cf6f2d87b57146f7bc57a6a89d90cf6 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/gpu/drm/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-kbuild-all/202304081129.amxcmyn2-...@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpu/drm/nouveau/dispnv50/disp.c:2554:1: warning: no previous >> prototype for function 'nv50_display_read_hw_state' [-Wmissing-prototypes] nv50_display_read_hw_state(struct nouveau_drm *drm) ^ drivers/gpu/drm/nouveau/dispnv50/disp.c:2553:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void ^ static drivers/gpu/drm/nouveau/dispnv50/disp.c:2618:1: warning: no previous prototype for function 'nv50_display_create' [-Wmissing-prototypes] nv50_display_create(struct drm_device *dev) ^ drivers/gpu/drm/nouveau/dispnv50/disp.c:2617:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int ^ static 2 warnings generated. vim +/nv50_display_read_hw_state +2554 drivers/gpu/drm/nouveau/dispnv50/disp.c 2551 2552 /* Read back the currently programmed display state */ 2553 void > 2554 nv50_display_read_hw_state(struct nouveau_drm *drm) 2555 { 2556 struct drm_device *dev = drm->dev; 2557 struct drm_encoder *encoder; 2558 struct drm_modeset_acquire_ctx ctx; 2559 struct nv50_disp *disp = nv50_disp(dev); 2560 int ret; 2561 2562 DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); 2563 2564 drm_for_each_encoder(encoder, dev) { 2565 if (encoder->encoder_type == DRM_MODE_ENCODER_DPMST) 2566 continue; 2567 2568 nv50_display_read_hw_or_state(dev, disp, nouveau_encoder(encoder)); 2569 } 2570 2571 DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); 2572 } 2573 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
Re: [PATCH 2/2] drm/nouveau/kms: Add INHERIT ioctl to nvkm/nvif for reading IOR state
Hi Lyude, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.3-rc5 next-20230406] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Lyude-Paul/drm-nouveau-kms-Add-INHERIT-ioctl-to-nvkm-nvif-for-reading-IOR-state/20230408-062329 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230407222133.1425969-2-lyude%40redhat.com patch subject: [PATCH 2/2] drm/nouveau/kms: Add INHERIT ioctl to nvkm/nvif for reading IOR state config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230408/202304080927.xi7meodx-...@intel.com/config) compiler: sparc64-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/a3d963915cf6f2d87b57146f7bc57a6a89d90cf6 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Lyude-Paul/drm-nouveau-kms-Add-INHERIT-ioctl-to-nvkm-nvif-for-reading-IOR-state/20230408-062329 git checkout a3d963915cf6f2d87b57146f7bc57a6a89d90cf6 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/gpu/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-kbuild-all/202304080927.xi7meodx-...@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpu/drm/nouveau/dispnv50/disp.c:2554:1: warning: no previous >> prototype for 'nv50_display_read_hw_state' [-Wmissing-prototypes] 2554 | nv50_display_read_hw_state(struct nouveau_drm *drm) | ^~ drivers/gpu/drm/nouveau/dispnv50/disp.c:2618:1: warning: no previous prototype for 'nv50_display_create' [-Wmissing-prototypes] 2618 | nv50_display_create(struct drm_device *dev) | ^~~ vim +/nv50_display_read_hw_state +2554 drivers/gpu/drm/nouveau/dispnv50/disp.c 2551 2552 /* Read back the currently programmed display state */ 2553 void > 2554 nv50_display_read_hw_state(struct nouveau_drm *drm) 2555 { 2556 struct drm_device *dev = drm->dev; 2557 struct drm_encoder *encoder; 2558 struct drm_modeset_acquire_ctx ctx; 2559 struct nv50_disp *disp = nv50_disp(dev); 2560 int ret; 2561 2562 DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); 2563 2564 drm_for_each_encoder(encoder, dev) { 2565 if (encoder->encoder_type == DRM_MODE_ENCODER_DPMST) 2566 continue; 2567 2568 nv50_display_read_hw_or_state(dev, disp, nouveau_encoder(encoder)); 2569 } 2570 2571 DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); 2572 } 2573 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
[PATCH v5 2/4] drm/msm/dpu: enable DSPP_2/3 for LM_2/3 on sm8450
Mark DSPP_2 and DSPP_3 as used for LM_2 and LM_3 Fixes: 100d7ef6995d ("drm/msm/dpu: add support for SM8450") Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h index 02a259b6b426..e111ca1f4bf5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h @@ -107,9 +107,9 @@ static const struct dpu_lm_cfg sm8450_lm[] = { LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK, _lm_sblk, PINGPONG_1, LM_0, DSPP_1), LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK, - _lm_sblk, PINGPONG_2, LM_3, 0), + _lm_sblk, PINGPONG_2, LM_3, DSPP_2), LM_BLK("lm_3", LM_3, 0x47000, MIXER_SDM845_MASK, - _lm_sblk, PINGPONG_3, LM_2, 0), + _lm_sblk, PINGPONG_3, LM_2, DSPP_3), LM_BLK("lm_4", LM_4, 0x48000, MIXER_SDM845_MASK, _lm_sblk, PINGPONG_4, LM_5, 0), LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK, -- 2.39.2
[PATCH v5 3/4] drm/msm/dpu: enable DSPP and DSC on sc8180x
Enable DSPP and DSC hardware blocks on sc8180x platform. Signed-off-by: Dmitry Baryshkov --- .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 28 +-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h index c57400265f28..085db379083e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h @@ -102,9 +102,9 @@ static const struct dpu_sspp_cfg sc8180x_sspp[] = { static const struct dpu_lm_cfg sc8180x_lm[] = { LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK, - _lm_sblk, PINGPONG_0, LM_1, 0), + _lm_sblk, PINGPONG_0, LM_1, DSPP_0), LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK, - _lm_sblk, PINGPONG_1, LM_0, 0), + _lm_sblk, PINGPONG_1, LM_0, DSPP_1), LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK, _lm_sblk, PINGPONG_2, LM_3, 0), LM_BLK("lm_3", LM_3, 0x47000, MIXER_SDM845_MASK, @@ -115,6 +115,17 @@ static const struct dpu_lm_cfg sc8180x_lm[] = { _lm_sblk, PINGPONG_5, LM_4, 0), }; +static const struct dpu_dspp_cfg sc8180x_dspp[] = { + DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK, +_dspp_sblk), + DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK, +_dspp_sblk), + DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK, +_dspp_sblk), + DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK, +_dspp_sblk), +}; + static const struct dpu_pingpong_cfg sc8180x_pp[] = { PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, MERGE_3D_0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), @@ -142,6 +153,15 @@ static const struct dpu_merge_3d_cfg sc8180x_merge_3d[] = { MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200), }; +static const struct dpu_dsc_cfg sc8180x_dsc[] = { + DSC_BLK("dsc_0", DSC_0, 0x8, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_4", DSC_4, 0x81000, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_5", DSC_5, 0x81400, BIT(DPU_DSC_OUTPUT_CTRL)), +}; + static const struct dpu_intf_cfg sc8180x_intf[] = { INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), INTF_BLK("intf_1", INTF_1, 0x6a800, 0x2bc, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), @@ -190,6 +210,10 @@ const struct dpu_mdss_cfg dpu_sc8180x_cfg = { .sspp = sc8180x_sspp, .mixer_count = ARRAY_SIZE(sc8180x_lm), .mixer = sc8180x_lm, + .dspp_count = ARRAY_SIZE(sc8180x_dspp), + .dspp = sc8180x_dspp, + .dsc_count = ARRAY_SIZE(sc8180x_dsc), + .dsc = sc8180x_dsc, .pingpong_count = ARRAY_SIZE(sc8180x_pp), .pingpong = sc8180x_pp, .merge_3d_count = ARRAY_SIZE(sc8180x_merge_3d), -- 2.39.2
[PATCH v5 4/4] drm/msm/dpu: use CTL_SC7280_MASK for sm8450's ctl_0
On sm8450 platform the CTL_0 doesn't differ from the rest of CTL blocks, so switch it to CTL_SC7280_MASK too. Some background: original commit 100d7ef6995d ("drm/msm/dpu: add support for SM8450") had all (relevant at that time) bit spelled individually. Then commit 0e91bcbb0016 ("drm/msm/dpu: Add SM8350 to hw catalog"), despite being a mismerge, correctly changed all other CTL entries to use CTL_SC7280_MASK, except CTL_0. While the current BLOCK_SOC_MASK style is not ideal (and while we are working on a better scheme), let's follow its usage as a least minimal surprise. For example, sc8280xp, a close associate of sm8450, also uses CTL_SC7280_MASK. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h index e111ca1f4bf5..221358b9892e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h @@ -47,7 +47,7 @@ static const struct dpu_ctl_cfg sm8450_ctl[] = { { .name = "ctl_0", .id = CTL_0, .base = 0x15000, .len = 0x204, - .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_SPLIT_DISPLAY) | BIT(DPU_CTL_FETCH_ACTIVE), + .features = BIT(DPU_CTL_SPLIT_DISPLAY) | CTL_SC7280_MASK, .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9), }, { -- 2.39.2
[PATCH v5 0/4] drm/msm/dpu: more catalog fixes
This is a spin-off from [1]. Since most of the patches have been merged, split away the small fixes. Continue the versioning of the patches. Changes since v4: - Fix commit message of sc8280xp patch. It is split display/panel rather than split source (Abhinav) - Added DSC_4 and DSC_5 to sc8180x patch (Abhinav) - Expanded commit message of the sm8450's CTL patch to describe all reasons to use existing masks for the SoC CTL features. [1] https://patchwork.freedesktop.org/series/113910/ Dmitry Baryshkov (4): drm/msm/dpu: enable DPU_CTL_SPLIT_DISPLAY for sc8280xp drm/msm/dpu: enable DSPP_2/3 for LM_2/3 on sm8450 drm/msm/dpu: enable DSPP and DSC on sc8180x drm/msm/dpu: use CTL_SC7280_MASK for sm8450's ctl_0 .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 28 +-- .../msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 5 ++-- .../msm/disp/dpu1/catalog/dpu_8_1_sm8450.h| 6 ++-- 3 files changed, 32 insertions(+), 7 deletions(-) -- 2.39.2
[PATCH v5 1/4] drm/msm/dpu: enable DPU_CTL_SPLIT_DISPLAY for sc8280xp
Theoretically, since sm8150 we should be using a single CTL for the split panel case, but since we do not support it for now, fallback to DPU_CTL_SPLIT_DISPLAY. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h index 9aab110b8c44..622992ae53ef 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h @@ -42,17 +42,18 @@ static const struct dpu_mdp_cfg sc8280xp_mdp[] = { }, }; +/* FIXME: get rid of DPU_CTL_SPLIT_DISPLAY in favour of proper ACTIVE_CTL support */ static const struct dpu_ctl_cfg sc8280xp_ctl[] = { { .name = "ctl_0", .id = CTL_0, .base = 0x15000, .len = 0x204, - .features = CTL_SC7280_MASK, + .features = BIT(DPU_CTL_SPLIT_DISPLAY) | CTL_SC7280_MASK, .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9), }, { .name = "ctl_1", .id = CTL_1, .base = 0x16000, .len = 0x204, - .features = CTL_SC7280_MASK, + .features = BIT(DPU_CTL_SPLIT_DISPLAY) | CTL_SC7280_MASK, .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10), }, { -- 2.39.2
Re: [PATCH v4 00/42] drm/msm/dpu: rework HW catalog
On Tue, 04 Apr 2023 16:05:40 +0300, Dmitry Baryshkov wrote: > This huge series attempts to restructure the DPU HW catalog into a > manageable and reviewable data set. In order to ease review and testing > I merged all the necessary fixes into this series. Also I cherry-picked > & slightly fixed Konrad's patch adding size to the SSPP and INTF macros. > > First 6 patches clean up the catalog a bit in order to make it more > suitable for refactoring. > > [...] Applied, thanks! [02/42] drm/msm/dpu: Allow variable SSPP_BLK size https://gitlab.freedesktop.org/lumag/msm/-/commit/8f940ddbc4f1 [03/42] drm/msm/dpu: Allow variable INTF_BLK size https://gitlab.freedesktop.org/lumag/msm/-/commit/8399a5ff18dc [04/42] drm/msm/dpu: constify DSC data structures https://gitlab.freedesktop.org/lumag/msm/-/commit/fc4fcfb0744b [05/42] drm/msm/dpu: mark remaining pp data as const https://gitlab.freedesktop.org/lumag/msm/-/commit/ac1c5ed678e8 [06/42] drm/msm/dpu: move UBWC/memory configuration to separate struct https://gitlab.freedesktop.org/lumag/msm/-/commit/fbbd8cce803a [07/42] drm/msm/dpu: split SM8550 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/9cc547933636 [09/42] drm/msm/dpu: split SC8280XP catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/225978f43986 [10/42] drm/msm/dpu: split SC7280 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/f0f2c32a662c [11/42] drm/msm/dpu: split SM8350 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/b8ece0c61e13 [12/42] drm/msm/dpu: split SM6115 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/01f2e9a70be1 [13/42] drm/msm/dpu: split QCM2290 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/c22a42bd3eb7 [14/42] drm/msm/dpu: split SC7180 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/c9cd1552e95b [15/42] drm/msm/dpu: split SM8250 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/2f36168e3257 [16/42] drm/msm/dpu: split SC8180X catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/97e2c8037694 [17/42] drm/msm/dpu: split SM8150 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/25035306871e [18/42] drm/msm/dpu: split MSM8998 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/1c611c481e8d [19/42] drm/msm/dpu: split SDM845 catalog entry to the separate file https://gitlab.freedesktop.org/lumag/msm/-/commit/9a4425f404c3 [20/42] drm/msm/dpu: duplicate sdm845 catalog entries https://gitlab.freedesktop.org/lumag/msm/-/commit/460c410f02e4 [21/42] drm/msm/dpu: duplicate sc7180 catalog entries https://gitlab.freedesktop.org/lumag/msm/-/commit/7ea3e251a84e [22/42] drm/msm/dpu: duplicate sm8150 catalog entries https://gitlab.freedesktop.org/lumag/msm/-/commit/8589ccd71067 [23/42] drm/msm/dpu: duplicate sm8250 catalog entries https://gitlab.freedesktop.org/lumag/msm/-/commit/586c11233ea8 [24/42] drm/msm/dpu: duplicate sm8350 catalog entries https://gitlab.freedesktop.org/lumag/msm/-/commit/9bea40825512 [25/42] drm/msm/dpu: expand sc8180x catalog https://gitlab.freedesktop.org/lumag/msm/-/commit/2861ce202cd8 [26/42] drm/msm/dpu: expand sc7180 catalog https://gitlab.freedesktop.org/lumag/msm/-/commit/02538790a8d4 [27/42] drm/msm/dpu: expand sm6115 catalog https://gitlab.freedesktop.org/lumag/msm/-/commit/5ce224840b9e [28/42] drm/msm/dpu: expand sm8550 catalog https://gitlab.freedesktop.org/lumag/msm/-/commit/463ba323aeb4 [29/42] drm/msm/dpu: use defined symbol for sc8280xp's maxwidth https://gitlab.freedesktop.org/lumag/msm/-/commit/8f41187a0649 [30/42] drm/msm/dpu: catalog: add comments regarding DPU_CTL_SPLIT_DISPLAY https://gitlab.freedesktop.org/lumag/msm/-/commit/5a7e3c008d35 [33/42] drm/msm/dpu: drop duplicate vig_sblk instances https://gitlab.freedesktop.org/lumag/msm/-/commit/d16b77dd8658 [35/42] drm/msm/dpu: inline IRQ_n_MASK defines https://gitlab.freedesktop.org/lumag/msm/-/commit/e5edf654536f [41/42] drm/msm/dpu: fetch DPU configuration from match data https://gitlab.freedesktop.org/lumag/msm/-/commit/dac76a0144d3 [42/42] drm/msm/dpu: drop unused macros from hw catalog https://gitlab.freedesktop.org/lumag/msm/-/commit/ac7e7c9c65ec Best regards, -- Dmitry Baryshkov
Re: [PATCH v4 34/42] drm/msm/dpu: enable DSPP and DSC on sc8180x
On 08/04/2023 02:43, Abhinav Kumar wrote: On 4/4/2023 6:06 AM, Dmitry Baryshkov wrote: Enable DSPP and DSC hardware blocks on sc8180x platform. Signed-off-by: Dmitry Baryshkov --- .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 26 +-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h index fb8cdcd6bfe9..93d303cc0dc5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h @@ -102,9 +102,9 @@ static const struct dpu_sspp_cfg sc8180x_sspp[] = { static const struct dpu_lm_cfg sc8180x_lm[] = { LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK, - _lm_sblk, PINGPONG_0, LM_1, 0), + _lm_sblk, PINGPONG_0, LM_1, DSPP_0), LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK, - _lm_sblk, PINGPONG_1, LM_0, 0), + _lm_sblk, PINGPONG_1, LM_0, DSPP_1), LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK, _lm_sblk, PINGPONG_2, LM_3, 0), LM_BLK("lm_3", LM_3, 0x47000, MIXER_SDM845_MASK, @@ -115,6 +115,17 @@ static const struct dpu_lm_cfg sc8180x_lm[] = { _lm_sblk, PINGPONG_5, LM_4, 0), }; +static const struct dpu_dspp_cfg sc8180x_dspp[] = { + DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK, + _dspp_sblk), + DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK, + _dspp_sblk), + DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK, + _dspp_sblk), + DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK, + _dspp_sblk), +}; I was looking at DSPP_SC7180_MASK. This has only PCC. Today the only DSPP feature we are supporting seems to be PCC as the "gc" base is not used. In that aspect this is fine. Perhaps getting rid of DSPP_SC7180_MASK and just using the feature mask directly is more appropriate here. So BIT(DPU_DSPP_PCC). I dont know if you want to handle that in a separate series to replace DSPP_SC7180_MASK with BIT(DPU_DSPP_PCC) I have not yet had time to look on the DSPP details and/or different features per SoC. So, I followed current approach. If anybody has time to take a look and cleanup DSPP handling, I'd be grateful. If not, it will wait for somebody to volunteer (or for you or me to have time for that). So do we also need to correct the msm8998 DSPP mask because gc is really not programmed today from what I can see. So that mask really is not doing anything. + static const struct dpu_pingpong_cfg sc8180x_pp[] = { PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, MERGE_3D_0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), @@ -142,6 +153,13 @@ static const struct dpu_merge_3d_cfg sc8180x_merge_3d[] = { MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200), }; +static const struct dpu_dsc_cfg sc8180x_dsc[] = { + DSC_BLK("dsc_0", DSC_0, 0x8, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)), +}; There is also DSC_4 and DSC_5 at 0x81000 and 0x81400 resp. Hmm. I was using sdmshrike-sde.dtsi as a reference. I'll add two more DSC units. Rest LGTM. Thanks + static const struct dpu_intf_cfg sc8180x_intf[] = { INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), INTF_BLK("intf_1", INTF_1, 0x6a800, 0x2bc, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), @@ -190,6 +208,10 @@ static const struct dpu_mdss_cfg sc8180x_dpu_cfg = { .sspp = sc8180x_sspp, .mixer_count = ARRAY_SIZE(sc8180x_lm), .mixer = sc8180x_lm, + .dspp_count = ARRAY_SIZE(sc8180x_dspp), + .dspp = sc8180x_dspp, + .dsc_count = ARRAY_SIZE(sc8180x_dsc), + .dsc = sc8180x_dsc, .pingpong_count = ARRAY_SIZE(sc8180x_pp), .pingpong = sc8180x_pp, .merge_3d_count = ARRAY_SIZE(sc8180x_merge_3d), -- With best wishes Dmitry
Re: [PATCH v4 34/42] drm/msm/dpu: enable DSPP and DSC on sc8180x
On 4/4/2023 6:06 AM, Dmitry Baryshkov wrote: Enable DSPP and DSC hardware blocks on sc8180x platform. Signed-off-by: Dmitry Baryshkov --- .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 26 +-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h index fb8cdcd6bfe9..93d303cc0dc5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h @@ -102,9 +102,9 @@ static const struct dpu_sspp_cfg sc8180x_sspp[] = { static const struct dpu_lm_cfg sc8180x_lm[] = { LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK, - _lm_sblk, PINGPONG_0, LM_1, 0), + _lm_sblk, PINGPONG_0, LM_1, DSPP_0), LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK, - _lm_sblk, PINGPONG_1, LM_0, 0), + _lm_sblk, PINGPONG_1, LM_0, DSPP_1), LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK, _lm_sblk, PINGPONG_2, LM_3, 0), LM_BLK("lm_3", LM_3, 0x47000, MIXER_SDM845_MASK, @@ -115,6 +115,17 @@ static const struct dpu_lm_cfg sc8180x_lm[] = { _lm_sblk, PINGPONG_5, LM_4, 0), }; +static const struct dpu_dspp_cfg sc8180x_dspp[] = { + DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK, +_dspp_sblk), + DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK, +_dspp_sblk), + DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK, +_dspp_sblk), + DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK, +_dspp_sblk), +}; I was looking at DSPP_SC7180_MASK. This has only PCC. Today the only DSPP feature we are supporting seems to be PCC as the "gc" base is not used. In that aspect this is fine. Perhaps getting rid of DSPP_SC7180_MASK and just using the feature mask directly is more appropriate here. So BIT(DPU_DSPP_PCC). I dont know if you want to handle that in a separate series to replace DSPP_SC7180_MASK with BIT(DPU_DSPP_PCC) So do we also need to correct the msm8998 DSPP mask because gc is really not programmed today from what I can see. So that mask really is not doing anything. + static const struct dpu_pingpong_cfg sc8180x_pp[] = { PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, MERGE_3D_0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), @@ -142,6 +153,13 @@ static const struct dpu_merge_3d_cfg sc8180x_merge_3d[] = { MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200), }; +static const struct dpu_dsc_cfg sc8180x_dsc[] = { + DSC_BLK("dsc_0", DSC_0, 0x8, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)), + DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)), +}; There is also DSC_4 and DSC_5 at 0x81000 and 0x81400 resp. Rest LGTM. + static const struct dpu_intf_cfg sc8180x_intf[] = { INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), INTF_BLK("intf_1", INTF_1, 0x6a800, 0x2bc, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), @@ -190,6 +208,10 @@ static const struct dpu_mdss_cfg sc8180x_dpu_cfg = { .sspp = sc8180x_sspp, .mixer_count = ARRAY_SIZE(sc8180x_lm), .mixer = sc8180x_lm, + .dspp_count = ARRAY_SIZE(sc8180x_dspp), + .dspp = sc8180x_dspp, + .dsc_count = ARRAY_SIZE(sc8180x_dsc), + .dsc = sc8180x_dsc, .pingpong_count = ARRAY_SIZE(sc8180x_pp), .pingpong = sc8180x_pp, .merge_3d_count = ARRAY_SIZE(sc8180x_merge_3d),
Re: (subset) [PATCH v3 0/5] SM8[12]50 GPU speedbin
On Fri, 31 Mar 2023 03:14:48 +0200, Konrad Dybcio wrote: > This series brings SM8[12]50 (A6[45]0) speedbin support along with a > touch-up for 8150, allowing Adreno to cooperate with the display hw. > > Tested on Xperia 5 II (SM8250 Edo PDX206) and Xperia 5 (SM8150 Kumano > Bahamut). > > v2 -> v3: > - Don't swap speedbin 2 (with fuse val 3) and speedbin 3 (with fuse val 2) > on SM8250 (no functional change, this is all a software construct but > let's stick with the official mapping) [2/5], [5/5] > > [...] Applied, thanks! [3/5] arm64: dts: qcom: sm8150: Don't start Adreno in headless mode commit: 1642ab96efa427f88e8a54c11b01b1b333ce58bd [4/5] arm64: dts: qcom: sm8150: Add GPU speedbin support commit: b53ae6b63181f4575fc62cd0efb341c8151b0a74 [5/5] arm64: dts: qcom: sm8250: Add GPU speedbin support commit: 2a50d1a038be17972220a810c28e9b777cdfcb22 Best regards, -- Bjorn Andersson
Re: [Nouveau] [PATCH] drm/nouveau/fb: add missing sysmen flush callbacks
Reviewed-by: Lyude Paul On Wed, 2023-04-05 at 13:04 +0200, Karol Herbst wrote: > Closes: https://gitlab.freedesktop.org/drm/nouveau/-/issues/203 > Fixes: 5728d064190e1 ("drm/nouveau/fb: handle sysmem flush page from common > code") > Signed-off-by: Karol Herbst > --- > drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf108.c | 1 + > drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk104.c | 1 + > drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk110.c | 1 + > drivers/gpu/drm/nouveau/nvkm/subdev/fb/gm107.c | 1 + > 4 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf108.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf108.c > index 76678dd60f93f..c4c6f67af7ccc 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf108.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf108.c > @@ -31,6 +31,7 @@ gf108_fb = { > .init = gf100_fb_init, > .init_page = gf100_fb_init_page, > .intr = gf100_fb_intr, > + .sysmem.flush_page_init = gf100_fb_sysmem_flush_page_init, > .ram_new = gf108_ram_new, > .default_bigpage = 17, > }; > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk104.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk104.c > index f73442ccb424b..433fa966ba231 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk104.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk104.c > @@ -77,6 +77,7 @@ gk104_fb = { > .init = gf100_fb_init, > .init_page = gf100_fb_init_page, > .intr = gf100_fb_intr, > + .sysmem.flush_page_init = gf100_fb_sysmem_flush_page_init, > .ram_new = gk104_ram_new, > .default_bigpage = 17, > .clkgate_pack = gk104_fb_clkgate_pack, > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk110.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk110.c > index 45d6cdffafeed..4dc283dedf8b5 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk110.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gk110.c > @@ -59,6 +59,7 @@ gk110_fb = { > .init = gf100_fb_init, > .init_page = gf100_fb_init_page, > .intr = gf100_fb_intr, > + .sysmem.flush_page_init = gf100_fb_sysmem_flush_page_init, > .ram_new = gk104_ram_new, > .default_bigpage = 17, > .clkgate_pack = gk110_fb_clkgate_pack, > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gm107.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gm107.c > index de52462a92bf0..90bfff616d35b 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gm107.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gm107.c > @@ -31,6 +31,7 @@ gm107_fb = { > .init = gf100_fb_init, > .init_page = gf100_fb_init_page, > .intr = gf100_fb_intr, > + .sysmem.flush_page_init = gf100_fb_sysmem_flush_page_init, > .ram_new = gm107_ram_new, > .default_bigpage = 17, > }; -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
[PATCH 2/2] drm/nouveau/kms: Add INHERIT ioctl to nvkm/nvif for reading IOR state
Now that we're supporting things like Ada and the GSP, there's situations where we really need to actually know the display state that we're starting with when loading the driver in order to prevent breaking GSP expectations. The first step in doing this is making it so that we can read the current state of IORs from nvkm in DRM, so that we can fill in said into into the atomic state. We do this by introducing an INHERIT ioctl to nvkm/nvif. This is basically another form of ACQUIRE, except that it will only acquire the given output path for userspace if it's already set up in hardware. This way, we can go through and probe each outp object we have in DRM in order to figure out the current hardware state of each one. If the outp isn't in use, it simply returns -ENODEV. This is also part of the work that will be required for implementing GSP support for display. While the GSP should mostly work without this commit, this commit should fix some edge case bugs that can occur on initial driver load. This also paves the way for some of the initial groundwork for fastboot support. Signed-off-by: Lyude Paul --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 103 +- drivers/gpu/drm/nouveau/include/nvif/if0012.h | 18 +++ drivers/gpu/drm/nouveau/include/nvif/outp.h | 5 + drivers/gpu/drm/nouveau/nvif/outp.c | 68 .../gpu/drm/nouveau/nvkm/engine/disp/outp.c | 40 +-- .../gpu/drm/nouveau/nvkm/engine/disp/outp.h | 3 + .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 64 +++ 7 files changed, 288 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index ed9d374147b8..1c2dfae75c76 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -1711,7 +1711,8 @@ nv50_sor_create(struct drm_connector *connector, struct dcb_output *dcbe) drm_connector_attach_encoder(connector, encoder); - disp->core->func->sor->get_caps(disp, nv_encoder, ffs(dcbe->or) - 1); + nv_encoder->or = ffs(dcbe->or) - 1; + disp->core->func->sor->get_caps(disp, nv_encoder, nv_encoder->or); nv50_outp_dump_caps(drm, nv_encoder); if (dcbe->type == DCB_OUTPUT_DP) { @@ -2473,6 +2474,103 @@ nv50_display_fini(struct drm_device *dev, bool runtime, bool suspend) cancel_work_sync(>hpd_work); } +static inline void +nv50_display_read_hw_or_state(struct drm_device *dev, struct nv50_disp *disp, + struct nouveau_encoder *outp) +{ + struct drm_crtc *crtc; + struct drm_connector_list_iter conn_iter; + struct drm_connector *conn; + struct nv50_head_atom *armh; + const u32 encoder_mask = drm_encoder_mask(>base.base); + bool found_conn = false, found_head = false; + u8 proto; + int head_idx; + int ret; + + switch (outp->dcb->type) { + case DCB_OUTPUT_TMDS: + ret = nvif_outp_inherit_tmds(>outp, ); + break; + case DCB_OUTPUT_DP: + ret = nvif_outp_inherit_dp(>outp, ); + break; + case DCB_OUTPUT_LVDS: + ret = nvif_outp_inherit_lvds(>outp, ); + break; + case DCB_OUTPUT_ANALOG: + ret = nvif_outp_inherit_rgb_crt(>outp, ); + break; + default: + drm_dbg_kms(dev, "Readback for %s not implemented yet, skipping\n", + outp->base.base.name); + drm_WARN_ON(dev, true); + return; + } + if (ret >= 0) { + head_idx = ret; + ret = 0; + } else if (ret == -ENODEV) { + return; + } + + drm_for_each_crtc(crtc, dev) { + if (crtc->index != head_idx) + continue; + + armh = nv50_head_atom(crtc->state); + found_head = true; + break; + } + if (drm_WARN_ON(dev, !found_head)) + return; + + /* Figure out which connector is being used by this encoder */ + drm_connector_list_iter_begin(dev, _iter); + nouveau_for_each_non_mst_connector_iter(conn, _iter) { + if (nouveau_connector(conn)->index == outp->dcb->connector) { + found_conn = true; + break; + } + } + drm_connector_list_iter_end(_iter); + if (drm_WARN_ON(dev, !found_conn)) + return; + + armh->state.encoder_mask = encoder_mask; + armh->state.connector_mask = drm_connector_mask(conn); + armh->state.active = true; + armh->state.enable = true; + + outp->crtc = crtc; + outp->ctrl = NVVAL(NV507D, SOR_SET_CONTROL, PROTOCOL, proto) | BIT(crtc->index); + + conn->state->crtc = crtc; + conn->state->best_encoder = >base.base; +} + +/* Read back the currently programmed display state
[PATCH 1/2] drm/nouveau/nvkm/outp: Use WARN_ON() in conditionals in nvkm_outp_init_route()
Signed-off-by: Lyude Paul --- drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c index 6094805fbd63..06b19883a06b 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c @@ -229,10 +229,8 @@ nvkm_outp_init_route(struct nvkm_outp *outp) return; ior = nvkm_ior_find(disp, type, -1); - if (!ior) { - WARN_ON(1); + if (WARN_ON(!ior)) return; - } /* Determine the specific OR, if any, this device is attached to. */ if (ior->func->route.get) { @@ -248,10 +246,8 @@ nvkm_outp_init_route(struct nvkm_outp *outp) } ior = nvkm_ior_find(disp, type, id); - if (!ior) { - WARN_ON(1); + if (WARN_ON(!ior)) return; - } /* Determine if the OR is already configured for this device. */ ior->func->state(ior, >arm); -- 2.39.2
Re: [PATCH v15 01/10] device property: Add remote endpoint to devcon matcher
Quoting Pin-yen Lin (2023-03-31 02:11:36) > From: Prashant Malani > > When searching the device graph for device matches, check the > remote-endpoint itself for a match. > > Some drivers register devices for individual endpoints. This allows > the matcher code to evaluate those for a match too, instead > of only looking at the remote parent devices. This is required when a > device supports two mode switches in its endpoints, so we can't simply > register the mode switch with the parent node. Looking at this in isolation I have no idea what a mode switch is and how it is related to drivers/base/property.c. Can you expand on this commit text? Maybe say two "usb typec mode switches"? And maybe include an example graph node snippet?
Re: [PATCH v5 2/9] video/aperture: use generic code to figure out the vga default device
On 4/6/23 15:21, Thomas Zimmermann wrote: From: Daniel Vetter Since vgaarb has been promoted to be a core piece of the pci subsystem we don't have to open code random guesses anymore, we actually know this in a platform agnostic way, and there's no need for an x86 specific hack. See also commit 1d38fe6ee6a8 ("PCI/VGA: Move vgaarb to drivers/pci") This should not result in any functional change, and the non-x86 multi-gpu pci systems are probably rare enough to not matter (I don't know of any tbh). But it's a nice cleanup, so let's do it. There's been a few questions on previous iterations on dri-devel and irc: - fb_is_primary_device() seems to be yet another implementation of this theme, and at least on x86 it checks for both vga_default_device OR rom shadowing. There shouldn't ever be a case where rom shadowing gives any additional hints about the boot vga device, but if there is then the default vga selection in vgaarb should probably be fixed. And not special-case checks replicated all over. - Thomas also brought up that on most !x86 systems fb_is_primary_device() returns 0, except on sparc/parisc. But these 2 special cases are about platform specific devices and not pci, so shouldn't have any interactions. Nearly all graphics cards on parisc machines are actually PCI cards, but the way we handle the handover to graphics mode with STIcore doesn't conflicts with your planned aperture changes. So no problem as far as I can see for parisc... Helge
Re: [PATCH] drm/amd/display: remove unused matching_stream_ptrs variable
On Fri, Apr 7, 2023 at 10:52 AM Nick Desaulniers wrote: > > Jimmy, can you review? > > The change LGTM; but I'm not sure if there was something else intended here. Nevermind, Jimmy's email address bounced. Reviewed-by: Nick Desaulniers > > On Sat, Mar 25, 2023 at 6:45 AM Tom Rix wrote: > > > > clang with W=1 reports > > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_enc_cfg.c:625:6: > > error: > > variable 'matching_stream_ptrs' set but not used > > [-Werror,-Wunused-but-set-variable] > > int matching_stream_ptrs = 0; > > ^ > > This variable is not used so remove it. > > > > Signed-off-by: Tom Rix > > --- > > drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c | 5 + > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c > > b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c > > index 41198c729d90..30c0644d4418 100644 > > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c > > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c > > @@ -622,7 +622,6 @@ bool link_enc_cfg_validate(struct dc *dc, struct > > dc_state *state) > > int i, j; > > uint8_t valid_count = 0; > > uint8_t dig_stream_count = 0; > > - int matching_stream_ptrs = 0; > > int eng_ids_per_ep_id[MAX_PIPES] = {0}; > > int ep_ids_per_eng_id[MAX_PIPES] = {0}; > > int valid_bitmap = 0; > > @@ -645,9 +644,7 @@ bool link_enc_cfg_validate(struct dc *dc, struct > > dc_state *state) > > struct link_enc_assignment assignment = > > state->res_ctx.link_enc_cfg_ctx.link_enc_assignments[i]; > > > > if (assignment.valid) { > > - if (assignment.stream == state->streams[i]) > > - matching_stream_ptrs++; > > - else > > + if (assignment.stream != state->streams[i]) > > valid_stream_ptrs = false; > > } > > } > > -- > > 2.27.0 > > > > > -- > Thanks, > ~Nick Desaulniers -- Thanks, ~Nick Desaulniers
Re: [PATCH] drm/nouveau/acr: remove unused loc variable
On Fri, Mar 31, 2023 at 1:42 PM Tom Rix wrote: > > clang with W=1 reports > drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c:221:7: error: variable > 'loc' set but not used [-Werror,-Wunused-but-set-variable] > u32 loc, sig, cnt, *meta; > ^ > This variable is not used so remove it. > > Signed-off-by: Tom Rix Ben, any idea if this was supposed to be used somewhere? If not: Fixes: 4b569ded09fd ("drm/nouveau/acr/ga102: initial support") Reviewed-by: Nick Desaulniers > --- > drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c > index f36a359d4531..bd104a030243 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c > @@ -218,7 +218,7 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev > *subdev, > const struct firmware *hsbl; > const struct nvfw_ls_hsbl_bin_hdr *hdr; > const struct nvfw_ls_hsbl_hdr *hshdr; > - u32 loc, sig, cnt, *meta; > + u32 sig, cnt, *meta; > > ret = nvkm_firmware_load_name(subdev, path, "hs_bl_sig", ver, > ); > if (ret) > @@ -227,7 +227,6 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev > *subdev, > hdr = nvfw_ls_hsbl_bin_hdr(subdev, hsbl->data); > hshdr = nvfw_ls_hsbl_hdr(subdev, hsbl->data + > hdr->header_offset); > meta = (u32 *)(hsbl->data + hshdr->meta_data_offset); > - loc = *(u32 *)(hsbl->data + hshdr->patch_loc); > sig = *(u32 *)(hsbl->data + hshdr->patch_sig); > cnt = *(u32 *)(hsbl->data + hshdr->num_sig); > > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: [PATCH] drm/qxl: remove unused num_relocs variable
On Fri, Mar 31, 2023 at 10:24 AM Tom Rix wrote: > > clang with W=1 reports > drivers/gpu/drm/qxl/qxl_ioctl.c:149:14: error: variable > 'num_relocs' set but not used [-Werror,-Wunused-but-set-variable] > int i, ret, num_relocs; > ^ > This variable is not used so remove it. > > Signed-off-by: Tom Rix Thanks for the patch! Fixes: 74d9a6335dce ("drm/qxl: Simplify cleaning qxl processing command") Reviewed-by: Nick Desaulniers That commit should also have removed the label out_free_bos IMO since having two labels for the same statement is a code smell. Tom, do you mind sending a v2 with that folded in? > --- > drivers/gpu/drm/qxl/qxl_ioctl.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c > index 30f58b21372a..3422206d59d4 100644 > --- a/drivers/gpu/drm/qxl/qxl_ioctl.c > +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c > @@ -146,7 +146,7 @@ static int qxl_process_single_command(struct qxl_device > *qdev, > struct qxl_release *release; > struct qxl_bo *cmd_bo; > void *fb_cmd; > - int i, ret, num_relocs; > + int i, ret; > int unwritten; > > switch (cmd->type) { > @@ -201,7 +201,6 @@ static int qxl_process_single_command(struct qxl_device > *qdev, > } > > /* fill out reloc info structs */ > - num_relocs = 0; > for (i = 0; i < cmd->relocs_num; ++i) { > struct drm_qxl_reloc reloc; > struct drm_qxl_reloc __user *u = u64_to_user_ptr(cmd->relocs); > @@ -231,7 +230,6 @@ static int qxl_process_single_command(struct qxl_device > *qdev, > reloc_info[i].dst_bo = cmd_bo; > reloc_info[i].dst_offset = reloc.dst_offset + > release->release_offset; > } > - num_relocs++; > > /* reserve and validate the reloc dst bo */ > if (reloc.reloc_type == QXL_RELOC_TYPE_BO || > reloc.src_handle) { > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: [PATCH] drm/amd/pm: remove unused num_of_active_display variable
On Fri, Mar 31, 2023 at 9:40 AM Tom Rix wrote: > > clang with W=1 reports > drivers/gpu/drm/amd/amdgpu/../pm/swsmu/amdgpu_smu.c:1700:6: error: variable > 'num_of_active_display' set but not used [-Werror,-Wunused-but-set-variable] > int num_of_active_display = 0; > ^ > This variable is not used so remove it. > > Signed-off-by: Tom Rix Thanks for the patch! Fixes: commit 75145aab7a0d ("drm/amdgpu/swsmu: clean up a bunch of stale interfaces") Reviewed-by: Nick Desaulniers > --- > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 7 --- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > index b5d64749990e..f93f7a9ed631 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > @@ -1696,8 +1696,6 @@ static int smu_display_configuration_change(void > *handle, > const struct > amd_pp_display_configuration *display_config) > { > struct smu_context *smu = handle; > - int index = 0; > - int num_of_active_display = 0; > > if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) > return -EOPNOTSUPP; > @@ -1708,11 +1706,6 @@ static int smu_display_configuration_change(void > *handle, > smu_set_min_dcef_deep_sleep(smu, > > display_config->min_dcef_deep_sleep_set_clk / 100); > > - for (index = 0; index < > display_config->num_path_including_non_display; index++) { > - if (display_config->displays[index].controller_id != 0) > - num_of_active_display++; > - } > - > return 0; > } > > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: [PATCH] drm/nouveau/svm: remove unused ret variable
On Wed, Mar 29, 2023 at 4:14 PM Tom Rix wrote: > > clang with W=1 reports > drivers/gpu/drm/nouveau/nouveau_svm.c:929:6: error: variable > 'ret' set but not used [-Werror,-Wunused-but-set-variable] > int ret; > ^ > This variable is not used so remove it. > > Signed-off-by: Tom Rix > --- > drivers/gpu/drm/nouveau/nouveau_svm.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c > b/drivers/gpu/drm/nouveau/nouveau_svm.c > index a74ba8d84ba7..e072d610f2f9 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_svm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c > @@ -926,15 +926,14 @@ nouveau_pfns_map(struct nouveau_svmm *svmm, struct > mm_struct *mm, > unsigned long addr, u64 *pfns, unsigned long npages) > { > struct nouveau_pfnmap_args *args = nouveau_pfns_to_args(pfns); > - int ret; > > args->p.addr = addr; > args->p.size = npages << PAGE_SHIFT; > > mutex_lock(>mutex); > > - ret = nvif_object_ioctl(>vmm->vmm.object, args, > - struct_size(args, p.phys, npages), NULL); > + nvif_object_ioctl(>vmm->vmm.object, args, > + struct_size(args, p.phys, npages), NULL); nvif_object_ioctl() may return -ENOSYS. Seeing other call sites have comments like: 114 /*XXX: warn? */ 134 /*XXX: warn? */ or other places where the return code isn't checked makes me think that a better approach would be to 1. plumb error codes back up through nouveau_pfns_map() (and other call sites of nvif_object_ioctl) 2. consider making nvif_object_ioctl __must_check > > mutex_unlock(>mutex); > } > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: [PATCH] drm/amd/display: remove unused matching_stream_ptrs variable
Jimmy, can you review? The change LGTM; but I'm not sure if there was something else intended here. On Sat, Mar 25, 2023 at 6:45 AM Tom Rix wrote: > > clang with W=1 reports > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_enc_cfg.c:625:6: error: > variable 'matching_stream_ptrs' set but not used > [-Werror,-Wunused-but-set-variable] > int matching_stream_ptrs = 0; > ^ > This variable is not used so remove it. > > Signed-off-by: Tom Rix > --- > drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c > b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c > index 41198c729d90..30c0644d4418 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c > @@ -622,7 +622,6 @@ bool link_enc_cfg_validate(struct dc *dc, struct dc_state > *state) > int i, j; > uint8_t valid_count = 0; > uint8_t dig_stream_count = 0; > - int matching_stream_ptrs = 0; > int eng_ids_per_ep_id[MAX_PIPES] = {0}; > int ep_ids_per_eng_id[MAX_PIPES] = {0}; > int valid_bitmap = 0; > @@ -645,9 +644,7 @@ bool link_enc_cfg_validate(struct dc *dc, struct dc_state > *state) > struct link_enc_assignment assignment = > state->res_ctx.link_enc_cfg_ctx.link_enc_assignments[i]; > > if (assignment.valid) { > - if (assignment.stream == state->streams[i]) > - matching_stream_ptrs++; > - else > + if (assignment.stream != state->streams[i]) > valid_stream_ptrs = false; > } > } > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: [PATCH] drm/vmwgfx: remove unused mksstat_init_record function
On Fri, Mar 24, 2023 at 12:54 PM Tom Rix wrote: > > clang with W=1 reports > drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:716:21: error: unused function > 'mksstat_init_record' [-Werror,-Wunused-function] > static inline char *mksstat_init_record(mksstat_kern_stats_t stat_idx, > ^ > This function is not used so remove it. > > Signed-off-by: Tom Rix > --- > drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 26 -- > 1 file changed, 26 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c > index e76976a95a1e..ca1a3fe44fa5 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c > @@ -702,32 +702,6 @@ static inline void hypervisor_ppn_remove(PPN64 pfn) > /* Header to the text description of mksGuestStat instance descriptor */ > #define MKSSTAT_KERNEL_DESCRIPTION "vmwgfx" > > -/** > - * mksstat_init_record: Initializes an MKSGuestStatCounter-based record > - * for the respective mksGuestStat index. > - * > - * @stat_idx: Index of the MKSGuestStatCounter-based mksGuestStat record. > - * @pstat: Pointer to array of MKSGuestStatCounterTime. > - * @pinfo: Pointer to array of MKSGuestStatInfoEntry. > - * @pstrs: Pointer to current end of the name/description sequence. > - * Return: Pointer to the new end of the names/description sequence. > - */ > - > -static inline char *mksstat_init_record(mksstat_kern_stats_t stat_idx, > - MKSGuestStatCounterTime *pstat, MKSGuestStatInfoEntry *pinfo, char > *pstrs) > -{ > - char *const pstrd = pstrs + > strlen(mksstat_kern_name_desc[stat_idx][0]) + 1; > - strcpy(pstrs, mksstat_kern_name_desc[stat_idx][0]); > - strcpy(pstrd, mksstat_kern_name_desc[stat_idx][1]); > - > - pinfo[stat_idx].name.s = pstrs; > - pinfo[stat_idx].description.s = pstrd; > - pinfo[stat_idx].flags = MKS_GUEST_STAT_FLAG_NONE; This was the last user of MKS_GUEST_STAT_FLAG_NONE, is there anything else to clean up there? Otherwise LGTM. Reviewed-by: Nick Desaulniers > - pinfo[stat_idx].stat.counter = (MKSGuestStatCounter > *)[stat_idx]; > - > - return pstrd + strlen(mksstat_kern_name_desc[stat_idx][1]) + 1; > -} > - > /** > * mksstat_init_record_time: Initializes an MKSGuestStatCounterTime-based > record > * for the respective mksGuestStat index. > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: [PATCH] drm/vmwgfx: remove unused vmw_overlay function
On Tue, Mar 21, 2023 at 11:24 AM Tom Rix wrote: > > clang with W=1 reports > drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c:56:35: error: > unused function 'vmw_overlay' [-Werror,-Wunused-function] > static inline struct vmw_overlay *vmw_overlay(struct drm_device *dev) > ^ > This function is not used, so remove it. > > Signed-off-by: Tom Rix Thanks for the patch! Reviewed-by: Nick Desaulniers > --- > drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c > index 8d171d71cb8a..7e112319a23c 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c > @@ -53,12 +53,6 @@ struct vmw_overlay { > struct vmw_stream stream[VMW_MAX_NUM_STREAMS]; > }; > > -static inline struct vmw_overlay *vmw_overlay(struct drm_device *dev) > -{ > - struct vmw_private *dev_priv = vmw_priv(dev); > - return dev_priv ? dev_priv->overlay_priv : NULL; > -} > - > struct vmw_escape_header { > uint32_t cmd; > SVGAFifoCmdEscape body; > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: (subset) [PATCH v2 00/23] drm/msm+PM+icc: Make job_run() reclaim-safe
On Mon, 20 Mar 2023 07:43:22 -0700, Rob Clark wrote: > From: Rob Clark > > Inspired by > https://lore.kernel.org/dri-devel/20200604081224.863494-10-daniel.vet...@ffwll.ch/ > it seemed like a good idea to get rid of memory allocation in job_run() > fence signaling path, and use lockdep annotations to yell at us about > anything that could deadlock against shrinker/reclaim. Anything that > can trigger reclaim, or block on any other thread that has triggered > reclaim, can block the GPU shrinker from releasing memory if it is > waiting the job to complete, causing deadlock. > > [...] Applied, thanks! [20/23] soc: qcom: smd-rpm: Use GFP_ATOMIC in write path commit: 5808c532ca0a983d643319caca44f2bcb148298f Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v6 0/9] Fix DSI host idx detection on HW revision clash
On Sat, 18 Mar 2023 14:42:46 +0100, Konrad Dybcio wrote: > v5 -> v6: > - Squash both fixes that concerned the deprecated QCM2290 compatible to > avoid warnings > > v5: > https://lore.kernel.org/r/20230307-topic-dsi_qcm-v5-0-9d4235b77...@linaro.org > > v4 -> v5: > - Drop superfluous items: level in [8/10] > - Remove the header define for the qcm2290 config in [6/10] instead of > [7/10] > - Pick up tags > > [...] Applied, thanks! [9/9] arm64: dts: qcom: sm6115: Use the correct DSI compatible commit: 1e6e0c1c971e5e02047a05c015510cc203530dc2 Best regards, -- Bjorn Andersson
Re: [PATCH] drm/kmb: remove unused set_test_mode_src_osc_freq_target_low, hi_bits functions
On Sat, Mar 18, 2023 at 6:39 AM Tom Rix wrote: > > clang with W=1 reports > drivers/gpu/drm/kmb/kmb_dsi.c:822:2: error: unused function > 'set_test_mode_src_osc_freq_target_low_bits' [-Werror,-Wunused-function] > set_test_mode_src_osc_freq_target_low_bits(struct kmb_dsi *kmb_dsi, > ^ > drivers/gpu/drm/kmb/kmb_dsi.c:834:2: error: unused function > 'set_test_mode_src_osc_freq_target_hi_bits' [-Werror,-Wunused-function] > set_test_mode_src_osc_freq_target_hi_bits(struct kmb_dsi *kmb_dsi, > ^ > These static functions are not used, so remove them. > > Signed-off-by: Tom Rix Thanks for the patch! Reviewed-by: Nick Desaulniers > --- > drivers/gpu/drm/kmb/kmb_dsi.c | 28 > 1 file changed, 28 deletions(-) > > diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c > index cf7cf0b07541..ed99b14375aa 100644 > --- a/drivers/gpu/drm/kmb/kmb_dsi.c > +++ b/drivers/gpu/drm/kmb/kmb_dsi.c > @@ -818,34 +818,6 @@ static void test_mode_send(struct kmb_dsi *kmb_dsi, u32 > dphy_no, > } > } > > -static inline void > - set_test_mode_src_osc_freq_target_low_bits(struct kmb_dsi *kmb_dsi, > - u32 dphy_no, > - u32 freq) > -{ > - /* Typical rise/fall time=166, refer Table 1207 databook, > -* sr_osc_freq_target[7:0] > -*/ > - test_mode_send(kmb_dsi, dphy_no, TEST_CODE_SLEW_RATE_DDL_CYCLES, > - (freq & 0x7f)); > -} > - > -static inline void > - set_test_mode_src_osc_freq_target_hi_bits(struct kmb_dsi *kmb_dsi, > - u32 dphy_no, > - u32 freq) > -{ > - u32 data; > - > - /* Flag this as high nibble */ > - data = ((freq >> 6) & 0x1f) | (1 << 7); > - > - /* Typical rise/fall time=166, refer Table 1207 databook, > -* sr_osc_freq_target[11:7] > -*/ > - test_mode_send(kmb_dsi, dphy_no, TEST_CODE_SLEW_RATE_DDL_CYCLES, > data); > -} > - > static void mipi_tx_get_vco_params(struct vco_params *vco) > { > int i; > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: [PATCH v8 09/10] arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller
On 01/04/2023 01:12, Mark Yacoub wrote: From: Sean Paul Add the register ranges required for HDCP key injection and HDCP TrustZone interaction as described in the dt-bindings for the sc7180 dp controller. Reviewed-by: Douglas Anderson Signed-off-by: Sean Paul Signed-off-by: Mark Yacoub --- Changes in v3: -Split off into a new patch containing just the dts change (Stephen) -Add hdcp compatible string (Stephen) Changes in v4: -Rebase on Bjorn's multi-dp patchset Changes in v5: -Put the tz register offsets in trogdor dtsi (Rob C) Changes in v6: -Rebased: Removed modifications in sc7180.dtsi as it's already upstream Changes in v7: -Change registers offset Changes in v8: -None arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 8 1 file changed, 8 insertions(+) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v8 09/10] arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller
On 01/04/2023 01:12, Mark Yacoub wrote: From: Sean Paul Add the register ranges required for HDCP key injection and HDCP TrustZone interaction as described in the dt-bindings for the sc7180 dp controller. Reviewed-by: Douglas Anderson Signed-off-by: Sean Paul Signed-off-by: Mark Yacoub --- Changes in v3: -Split off into a new patch containing just the dts change (Stephen) -Add hdcp compatible string (Stephen) Changes in v4: -Rebase on Bjorn's multi-dp patchset Changes in v5: -Put the tz register offsets in trogdor dtsi (Rob C) Changes in v6: -Rebased: Removed modifications in sc7180.dtsi as it's already upstream Changes in v7: -Change registers offset Changes in v8: -None arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 8 1 file changed, 8 insertions(+) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v8 04/10] drm/hdcp: Expand HDCP helper library for enable/disable/check
On 01/04/2023 01:12, Mark Yacoub wrote: From: Sean Paul Expand upon the HDCP helper library to manage HDCP enable, disable, and check. Previous to this patch, the majority of the state management and sink interaction is tucked inside the Intel driver with the understanding that once a new platform supported HDCP we could make good decisions about what should be centralized. With the addition of HDCP support for Qualcomm, it's time to migrate the protocol-specific bits of HDCP authentication, key exchange, and link checks to the HDCP helper. In terms of functionality, this migration is 1:1 with the Intel driver, however things are laid out a bit differently than with intel_hdcp.c, which is why this is a separate patch from the i915 transition to the helper. On i915, the shim vtable is used to account for HDMI vs. DP vs. DP-MST differences whereas the helper library uses a LUT to account for the register offsets and a remote read function to route the messages. On i915, storing the sink information in the source is done inline whereas now we use the new drm_hdcp_helper_funcs vtable to store and fetch information to/from source hw. Finally, instead of calling enable/disable directly from the driver, we'll leave that decision to the helper and by calling drm_hdcp_helper_atomic_commit() from the driver. All told, this will centralize the protocol and state handling in the helper, ensuring we collect all of our bugs^Wlogic in one place. I have a generic question regarding vtable design. Currently low level accessors are a part of main helper vtable. They are set by the driver. But when the driver calls drm_hdcp_helper_initialize_hdmi() or drm_hdcp_helper_initialize_dp(), we also know, which low level backend should be used. Can be have a struct describing hdmi vs dp functions and move that to the helper too? For the reference: earlier revisions had it in form of if (aux) spread all over the low level functions. I asked to split those accessors (thank you!) but I think we can still have them as a set of functions internal to the helper. Acked-by: Jani Nikula Signed-off-by: Sean Paul Signed-off-by: Mark Yacoub --- Changes in v2: -Fixed set-but-unused variable identified by 0-day Changes in v3: -Fixed uninitialized variable warning identified by 0-day Changes in v4: -None Changes in v5: -None Changes in v6: -Fixed typo in function descriptions -Rebased: Moved the new code between drm_hdcp.h and drm_hdcp_helper.c/h -Add missing headers. Reported-by: kernel test robot Changes in v7: - Add a |driver_data| field to some functions in drm_hdcp_helper_funcs that are called by the driver so drivers can pass anything they such as bridges - Isolate all non-common code between HDMI and DP into separate functions instead of manually checking for datra->aux Changes in v8 (suraj): -Try hdcp 1.x if hdcp2_capable returns an error instead of goto out. -set the enabled type to either 1 or 0 depending on the prop as hdcp2 can support either. drivers/gpu/drm/display/drm_hdcp_helper.c | 1215 + include/drm/display/drm_hdcp.h| 287 + include/drm/display/drm_hdcp_helper.h | 51 +- 3 files changed, 1552 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/display/drm_hdcp_helper.c b/drivers/gpu/drm/display/drm_hdcp_helper.c index 3ee1a6ae26c53..3bc0308cc95d8 100644 --- a/drivers/gpu/drm/display/drm_hdcp_helper.c +++ b/drivers/gpu/drm/display/drm_hdcp_helper.c @@ -6,13 +6,18 @@ * Ramalingam C */ +#include #include #include #include +#include +#include #include #include #include +#include +#include #include #include #include @@ -507,3 +512,1213 @@ bool drm_hdcp_has_changed(struct drm_connector *connector, return old_hdcp != new_hdcp; } EXPORT_SYMBOL(drm_hdcp_has_changed); + +struct drm_hdcp_hdcp1_receiver_reg_lut { + unsigned int bksv; + unsigned int ri; + unsigned int aksv; + unsigned int an; + unsigned int ainfo; + unsigned int v[5]; + unsigned int bcaps; + unsigned int bcaps_mask_repeater_present; + unsigned int bstatus; +}; + +static const struct drm_hdcp_hdcp1_receiver_reg_lut drm_hdcp_hdcp1_ddc_lut = { + .bksv = DRM_HDCP_DDC_BKSV, + .ri = DRM_HDCP_DDC_RI_PRIME, + .aksv = DRM_HDCP_DDC_AKSV, + .an = DRM_HDCP_DDC_AN, + .ainfo = DRM_HDCP_DDC_AINFO, + .v = { DRM_HDCP_DDC_V_PRIME(0), DRM_HDCP_DDC_V_PRIME(1), + DRM_HDCP_DDC_V_PRIME(2), DRM_HDCP_DDC_V_PRIME(3), + DRM_HDCP_DDC_V_PRIME(4) }, + .bcaps = DRM_HDCP_DDC_BCAPS, + .bcaps_mask_repeater_present = DRM_HDCP_DDC_BCAPS_REPEATER_PRESENT, + .bstatus = DRM_HDCP_DDC_BSTATUS, +}; + +static const struct drm_hdcp_hdcp1_receiver_reg_lut drm_hdcp_hdcp1_dpcd_lut = { + .bksv = DP_AUX_HDCP_BKSV, + .ri = DP_AUX_HDCP_RI_PRIME, + .aksv = DP_AUX_HDCP_AKSV, + .an = DP_AUX_HDCP_AN, + .ainfo =
Re: [PATCH 01/68] hwmon: constify pointers to hwmon_channel_info
On Thu, Apr 06, 2023 at 10:29:56PM +0200, Krzysztof Kozlowski wrote: > HWmon core receives an array of pointers to hwmon_channel_info and it > does not modify it, thus it can be array of const pointers for safety. > This allows drivers to make them also const. > > Signed-off-by: Krzysztof Kozlowski Series applied. Next time please let me know the base for your patch series. It took me a while to figure out that it is based on linux-next. Guenter
[PATCH 2/2] drm/nouveau: constify pointers to hwmon_channel_info
Statically allocated array of pointed to hwmon_channel_info can be made const for safety. Signed-off-by: Krzysztof Kozlowski --- This depends on hwmon core patch: https://lore.kernel.org/all/20230406203103.3011503-2-krzysztof.kozlow...@linaro.org/ Therefore I propose this should also go via hwmon tree. Cc: Jean Delvare Cc: Guenter Roeck Cc: linux-hw...@vger.kernel.org --- drivers/gpu/drm/nouveau/nouveau_hwmon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c b/drivers/gpu/drm/nouveau/nouveau_hwmon.c index e844be49e11e..db30a4c2cd4d 100644 --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c @@ -211,7 +211,7 @@ static const struct attribute_group temp1_auto_point_sensor_group = { #define N_ATTR_GROUPS 3 -static const struct hwmon_channel_info *nouveau_info[] = { +static const struct hwmon_channel_info * const nouveau_info[] = { HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL), HWMON_CHANNEL_INFO(temp, -- 2.34.1
[PATCH 1/2] drm/i915: constify pointers to hwmon_channel_info
Statically allocated array of pointed to hwmon_channel_info can be made const for safety. Signed-off-by: Krzysztof Kozlowski --- This depends on hwmon core patch: https://lore.kernel.org/all/20230406203103.3011503-2-krzysztof.kozlow...@linaro.org/ Therefore I propose this should also go via hwmon tree. Cc: Jean Delvare Cc: Guenter Roeck Cc: linux-hw...@vger.kernel.org --- drivers/gpu/drm/i915/i915_hwmon.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c index 596dd2c07010..87b527a54272 100644 --- a/drivers/gpu/drm/i915/i915_hwmon.c +++ b/drivers/gpu/drm/i915/i915_hwmon.c @@ -267,7 +267,7 @@ static const struct attribute_group *hwm_groups[] = { NULL }; -static const struct hwmon_channel_info *hwm_info[] = { +static const struct hwmon_channel_info * const hwm_info[] = { HWMON_CHANNEL_INFO(in, HWMON_I_INPUT), HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_CRIT), HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), @@ -275,7 +275,7 @@ static const struct hwmon_channel_info *hwm_info[] = { NULL }; -static const struct hwmon_channel_info *hwm_gt_info[] = { +static const struct hwmon_channel_info * const hwm_gt_info[] = { HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), NULL }; -- 2.34.1
Re: [CI] drm/i915/mtl: Define GuC firmware version for MTL
On Fri, Apr 07, 2023 at 05:45:52AM -0400, Rodrigo Vivi wrote: On Thu, Apr 06, 2023 at 05:37:58PM -0700, Lucas De Marchi wrote: On Fri, Mar 31, 2023 at 03:52:16PM -0700, john.c.harri...@intel.com wrote: > From: John Harrison > > First release of GuC for Meteorlake. > > NB: As this is still pre-release and likely to change, use explicit > versioning for now. The official, full release will use reduced > version naming. > > Signed-off-by: John Harrison Applied to topic/core-for-CI with acks by Rodrigo and Tvrtko. Reference issue: https://gitlab.freedesktop.org/drm/intel/-/issues/8343 Going forward we should plan to have these kind of patches in drm-intel-gt-next itself rather than using the CI branch. The CI branch is not the proper place. To be clear, since MTL is under force probe and not normally enabled, it's ok to bump the major version without retaining compabibility with the previous version, as per https://docs.kernel.org/driver-api/firmware/firmware-usage-guidelines.html I think the main blocker right now to use drm-intel-gt-next would be because MODULE_FIRMWARE() is unaware of force_probe. Then distros would start getting a warning when creating their initrd that they may have missed a firmware. But that firmware itself is not present in the upstream linux-firmware repo. We can think about a way to hide the fw def for *new* firmware (not the legacy ones) that are using the mmp version. Maybe we should simply move to the final version path sooner than later? Even if that means more updates in the blobs at linux-firmware.git, that would allow the OSVs to have this final patch sooner in their trees. it doesn't help much OSVs: the firmware being used here is the full version, not the major-only version. And if we commit this firmware with the major-only version, there is a chance the distro will update the kernel without updating the firmware and we will have the wrong firmware there, one that is not supposed to be used. Also, if we commit all the "temporary" versions in the linux-firmware.git repo, we may start to blow up its size. I'm not sure this is a best practice for HW that is not available to the general public. Options I see for future: 1) a) keep these temporary fw in our drm-firmware repo; b) hide the fw def under a kconfig 2) a) accept this kind o changes in the CI branch 3) a) commit to linux-firmware.git, knowing this is not a version that will be generally used b) commit to kernel in the usual way Any other? Note that this also impacts xe. Right now xe is not merged upstream, so this is just ignored: there we have already the firmware version for MTL and for PVC in the kernel. Eventually we will need a similar mechanism though, so better we agree upfront what that is. Lucas De Marchi For now, let's keep this as is and use the CI branch to assess the driver health with GuC. thanks Lucas De Marchi > --- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > index 264c952f777bb..1ac6f9f340e31 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > @@ -79,6 +79,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw, > * security fixes, etc. to be enabled. > */ > #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \ > + fw_def(METEORLAKE, 0, guc_mmp(mtl, 70, 6, 5)) \ >fw_def(DG2, 0, guc_maj(dg2, 70, 5)) \ >fw_def(ALDERLAKE_P, 0, guc_maj(adlp, 70, 5)) \ >fw_def(ALDERLAKE_P, 0, guc_mmp(adlp, 70, 1, 1)) \ > -- > 2.39.1 >
[PATCH v5 20/20] staging: media: tegra-video: add support for Tegra20 parallel input
The VI peripheral of Tegra supports capturing from MIPI CSI-2 or parallel video (called VIP in the docs). The staging tegra-video driver currently implements MIPI CSI-2 video capture for Tegra210. Add support for parallel video capture (VIP) on Tegra20. With the generalizations added to the VI driver in previous commits, this is only a matter of adding the vip.c and tegra20.c implementations and registering them. Unfortunately there was no documentation available for the VI or VIP peripherals of Tegra20 (or any other Tegra chips). This implementation has been based entirely on the code from a vendor kernel based on Linux 3.1 and massively adapted to fit into the tegra-video driver. Parts of this code is definitely non-optimal to say the least (especially tegra20_vi_enable() and the single-frame capture logic), but it was impossible to improve it. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- Changed in v5: - Fixed small typo - Upgraded copyright year Changed in v4: - Added review tags Changed in v3 (suggested by Dmitry Osipenko): - merged the VIP patch and the Tegra20 patch to avoid chicken-egg problem due to the two modules depending on each other at build time - move tegra20_vip_soc to vip.c - remove channel@0 node from device tree parsing - remove unused variable Changed in v2: - fix tegra20_vi_enable() to clear bit when on==false - clamp width/height from set/try_fmt to avoid returning sizeimage=0 (fixes v4l2-compliance) --- drivers/staging/media/tegra-video/Makefile | 2 + drivers/staging/media/tegra-video/tegra20.c | 661 drivers/staging/media/tegra-video/vi.c | 3 + drivers/staging/media/tegra-video/vi.h | 3 + drivers/staging/media/tegra-video/video.c | 5 + drivers/staging/media/tegra-video/video.h | 1 + drivers/staging/media/tegra-video/vip.c | 290 + drivers/staging/media/tegra-video/vip.h | 68 ++ 8 files changed, 1033 insertions(+) create mode 100644 drivers/staging/media/tegra-video/tegra20.c create mode 100644 drivers/staging/media/tegra-video/vip.c create mode 100644 drivers/staging/media/tegra-video/vip.h diff --git a/drivers/staging/media/tegra-video/Makefile b/drivers/staging/media/tegra-video/Makefile index dfa2ef8f99ef..6c7552e05109 100644 --- a/drivers/staging/media/tegra-video/Makefile +++ b/drivers/staging/media/tegra-video/Makefile @@ -2,7 +2,9 @@ tegra-video-objs := \ video.o \ vi.o \ + vip.o \ csi.o +tegra-video-$(CONFIG_ARCH_TEGRA_2x_SOC) += tegra20.o tegra-video-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o obj-$(CONFIG_VIDEO_TEGRA) += tegra-video.o diff --git a/drivers/staging/media/tegra-video/tegra20.c b/drivers/staging/media/tegra-video/tegra20.c new file mode 100644 index ..c25286772603 --- /dev/null +++ b/drivers/staging/media/tegra-video/tegra20.c @@ -0,0 +1,661 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Tegra20-specific VI implementation + * + * Copyright (C) 2023 SKIDATA GmbH + * Author: Luca Ceresoli + */ + +/* + * This source file contains Tegra20 supported video formats, + * VI and VIP SoC specific data, operations and registers accessors. + */ + +#include +#include +#include +#include +#include +#include + +#include "vip.h" +#include "vi.h" + +#define TEGRA_VI_SYNCPT_WAIT_TIMEOUT msecs_to_jiffies(200) + +/* This are just good-sense numbers. The actual min/max is not documented. */ +#define TEGRA20_MIN_WIDTH 32U +#define TEGRA20_MIN_HEIGHT 32U +#define TEGRA20_MAX_WIDTH 2048U +#define TEGRA20_MAX_HEIGHT 2048U + +/* -- + * Registers + */ + +#define TEGRA_VI_CONT_SYNCPT_OUT_1 0x0060 +#define VI_CONT_SYNCPT_OUT_1_CONTINUOUS_SYNCPT BIT(8) +#define VI_CONT_SYNCPT_OUT_1_SYNCPT_IDX_SFT 0 + +#define TEGRA_VI_VI_INPUT_CONTROL 0x0088 +#define VI_INPUT_FIELD_DETECTBIT(27) +#define VI_INPUT_BT656 BIT(25) +#define VI_INPUT_YUV_INPUT_FORMAT_SFT8 /* bits [9:8] */ +#define VI_INPUT_YUV_INPUT_FORMAT_UYVY (0 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_YUV_INPUT_FORMAT_VYUY (1 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_YUV_INPUT_FORMAT_YUYV (2 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_YUV_INPUT_FORMAT_YVYU (3 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_INPUT_FORMAT_SFT2 /* bits [5:2] */ +#define VI_INPUT_INPUT_FORMAT_YUV422 (0 << VI_INPUT_INPUT_FORMAT_SFT) +#define VI_INPUT_VIP_INPUT_ENABLEBIT(1) + +#define TEGRA_VI_VI_CORE_CONTROL 0x008c +#define VI_VI_CORE_CONTROL_PLANAR_CONV_IN_SEL_EXTBIT(31) +#define
[PATCH v5 19/20] staging: media: tegra-video: add H/V flip controls
Tegra20 can do horizontal and vertical image flip, but Tegra210 cannot (either the hardware, or this driver). In preparation to adding Tegra20 support, add a flag in struct tegra_vi_soc so the generic vi.c code knows whether the flip controls should be added or not. Also provide a generic implementation that simply sets two flags in the channel struct. The Tegra20 implementation will enable flipping at stream start based on those flags. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- Changed in v5: - Fixed typo in comment Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/vi.c | 14 +- drivers/staging/media/tegra-video/vi.h | 8 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index 5ab24977ec46..39e9df895ede 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -30,7 +30,7 @@ #include "vi.h" #include "video.h" -#define MAX_CID_CONTROLS 1 +#define MAX_CID_CONTROLS 3 /** * struct tegra_vi_graph_entity - Entity in the video graph @@ -910,6 +910,12 @@ static int vi_s_ctrl(struct v4l2_ctrl *ctrl) case V4L2_CID_TEGRA_SYNCPT_TIMEOUT_RETRY: chan->syncpt_timeout_retry = ctrl->val; break; + case V4L2_CID_HFLIP: + chan->hflip = ctrl->val; + break; + case V4L2_CID_VFLIP: + chan->vflip = ctrl->val; + break; default: return -EINVAL; } @@ -981,6 +987,12 @@ static int tegra_channel_setup_ctrl_handler(struct tegra_vi_channel *chan) v4l2_ctrl_handler_free(>ctrl_handler); return ret; } + + if (chan->vi->soc->has_h_v_flip) { + v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0); + v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); + } + #endif /* setup the controls */ diff --git a/drivers/staging/media/tegra-video/vi.h b/drivers/staging/media/tegra-video/vi.h index cadf80b742a8..778c0ec475ab 100644 --- a/drivers/staging/media/tegra-video/vi.h +++ b/drivers/staging/media/tegra-video/vi.h @@ -74,6 +74,7 @@ struct tegra_vi_ops { * @hw_revision: VI hw_revision * @vi_max_channels: supported max streaming channels * @vi_max_clk_hz: VI clock max frequency + * @has_h_v_flip: the chip can do H and V flip, and the driver implements it */ struct tegra_vi_soc { const struct tegra_video_format *video_formats; @@ -83,6 +84,7 @@ struct tegra_vi_soc { u32 hw_revision; unsigned int vi_max_channels; unsigned int vi_max_clk_hz; + bool has_h_v_flip:1; }; /** @@ -172,6 +174,9 @@ struct tegra_vi { * @tpg_fmts_bitmap: a bitmap for supported TPG formats * @pg_mode: test pattern generator mode (disabled/direct/patch) * @notifier: V4L2 asynchronous subdevs notifier + * + * @hflip: Horizontal flip is enabled + * @vflip: Vertical flip is enabled */ struct tegra_vi_channel { struct list_head list; @@ -222,6 +227,9 @@ struct tegra_vi_channel { enum tegra_vi_pg_mode pg_mode; struct v4l2_async_notifier notifier; + + bool hflip:1; + bool vflip:1; }; /** -- 2.34.1
[PATCH v5 18/20] staging: media: tegra-video: add hooks for planar YUV and H/V flip
Tegra20 supports planar YUV422 capture, which can be implemented by writing U and V base address registers in addition to the "main" base buffer address register. It also supports H and V flip, which among others requires to write the start address (i.e. the 1st offset to write, at the end of the buffer or line) in more registers for Y and, for planar formats, U and V. Add minimal hooks in VI to allow per-SoC optional support to those features: - variables in struct tegra_vi for the U and V buffer base offsets - variables in struct tegra_vi for the Y, U and V buffer start offsets - an optional per-soc VI operation to compute those values on queue setup Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/vi.c | 4 drivers/staging/media/tegra-video/vi.h | 14 ++ 2 files changed, 18 insertions(+) diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index b74cdd1d1c82..5ab24977ec46 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -102,6 +102,7 @@ tegra_get_format_by_fourcc(struct tegra_vi *vi, u32 fourcc) /* * videobuf2 queue operations */ + static int tegra_channel_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers, unsigned int *nplanes, @@ -117,6 +118,9 @@ static int tegra_channel_queue_setup(struct vb2_queue *vq, sizes[0] = chan->format.sizeimage; alloc_devs[0] = chan->vi->dev; + if (chan->vi->ops->channel_queue_setup) + chan->vi->ops->channel_queue_setup(chan); + return 0; } diff --git a/drivers/staging/media/tegra-video/vi.h b/drivers/staging/media/tegra-video/vi.h index 02f940f2e2eb..cadf80b742a8 100644 --- a/drivers/staging/media/tegra-video/vi.h +++ b/drivers/staging/media/tegra-video/vi.h @@ -47,6 +47,7 @@ struct tegra_vi_channel; * @channel_host1x_syncpt_free: free all synchronization points * @vi_fmt_align: modify `pix` to fit the hardware alignment * requirements and fill image geometry + * @channel_queue_setup: additional operations at the end of vb2_ops::queue_setup * @vi_start_streaming: starts media pipeline, subdevice streaming, sets up * VI for capture and runs capture start and capture finish * kthreads for capturing frames to buffer and returns them back. @@ -58,6 +59,7 @@ struct tegra_vi_ops { int (*channel_host1x_syncpt_init)(struct tegra_vi_channel *chan); void (*channel_host1x_syncpt_free)(struct tegra_vi_channel *chan); void (*vi_fmt_align)(struct v4l2_pix_format *pix, unsigned int bpp); + void (*channel_queue_setup)(struct tegra_vi_channel *chan); int (*vi_start_streaming)(struct vb2_queue *vq, u32 count); void (*vi_stop_streaming)(struct vb2_queue *vq); }; @@ -148,6 +150,12 @@ struct tegra_vi { * @queue: vb2 buffers queue * @sequence: V4L2 buffers sequence number * + * @addr_offset_u: U plane base address, relative to buffer base address (only for planar) + * @addr_offset_v: V plane base address, relative to buffer base address (only for planar) + * @start_offset: 1st Y byte to write, relative to buffer base address (for H/V flip) + * @start_offset_u: 1st U byte to write, relative to buffer base address (for H/V flip) + * @start_offset_v: 1st V byte to write, relative to buffer base address (for H/V flip) + * * @capture: list of queued buffers for capture * @start_lock: protects the capture queued list * @done: list of capture done queued buffers @@ -189,6 +197,12 @@ struct tegra_vi_channel { struct vb2_queue queue; u32 sequence; + unsigned int addr_offset_u; + unsigned int addr_offset_v; + unsigned int start_offset; + unsigned int start_offset_u; + unsigned int start_offset_v; + struct list_head capture; /* protects the capture queued list */ spinlock_t start_lock; -- 2.34.1
[PATCH v5 17/20] staging: media: tegra-video: add syncpts for Tegra20 to struct tegra_vi
In preparation to implement Tegra20 parallel video capture, add a variable to hold the required syncpt and document all the syncpt variables. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags Changed in v3: - recycle mw_ack_sp[0] instead of adding out_sp No changes in v2 --- drivers/staging/media/tegra-video/vi.h | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/tegra-video/vi.h b/drivers/staging/media/tegra-video/vi.h index dfb5870b1411..02f940f2e2eb 100644 --- a/drivers/staging/media/tegra-video/vi.h +++ b/drivers/staging/media/tegra-video/vi.h @@ -117,11 +117,13 @@ struct tegra_vi { * @vi: Tegra video input device structure * @frame_start_sp: host1x syncpoint pointer to synchronize programmed capture * start condition with hardware frame start events through host1x - * syncpoint counters. + * syncpoint counters. (Tegra210) * @mw_ack_sp: host1x syncpoint pointer to synchronize programmed memory write * ack trigger condition with hardware memory write done at end of - * frame through host1x syncpoint counters. + * frame through host1x syncpoint counters (On Tegra20 used for the + * OUT_1 syncpt) * @sp_incr_lock: protects cpu syncpoint increment. + * @next_out_sp_idx: next expected value for mw_ack_sp[0], i.e. OUT_1 (Tegra20) * * @kthread_start_capture: kthread to start capture of single frame when * vb buffer is available. This thread programs VI CSI hardware @@ -175,6 +177,7 @@ struct tegra_vi_channel { struct host1x_syncpt *mw_ack_sp[GANG_PORTS_MAX]; /* protects the cpu syncpoint increment */ spinlock_t sp_incr_lock[GANG_PORTS_MAX]; + u32 next_out_sp_idx; struct task_struct *kthread_start_capture; wait_queue_head_t start_wait; -- 2.34.1
[PATCH v5 16/20] staging: media: tegra-video: move syncpt init/free to a per-soc op
tegra_channel_host1x_syncpt_init() gets the host1x syncpts needed for the Tegra210 implementation, and tegra_channel_host1x_syncpts_free() puts them. Tegra20 needs to get and put a different syncpt. In preparation for adding Tegra20 support, move these functions to new ops in the soc-specific `struct tegra_vi_ops` . No functional changes. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/tegra210.c | 52 drivers/staging/media/tegra-video/vi.c | 52 ++-- drivers/staging/media/tegra-video/vi.h | 5 ++ 3 files changed, 60 insertions(+), 49 deletions(-) diff --git a/drivers/staging/media/tegra-video/tegra210.c b/drivers/staging/media/tegra-video/tegra210.c index b4fcd4e93b8c..da99f19a39e7 100644 --- a/drivers/staging/media/tegra-video/tegra210.c +++ b/drivers/staging/media/tegra-video/tegra210.c @@ -179,6 +179,56 @@ static u32 vi_csi_read(struct tegra_vi_channel *chan, u8 portno, /* * Tegra210 VI channel capture operations */ + +static int tegra210_channel_host1x_syncpt_init(struct tegra_vi_channel *chan) +{ + struct tegra_vi *vi = chan->vi; + unsigned long flags = HOST1X_SYNCPT_CLIENT_MANAGED; + struct host1x_syncpt *fs_sp; + struct host1x_syncpt *mw_sp; + int ret, i; + + for (i = 0; i < chan->numgangports; i++) { + fs_sp = host1x_syncpt_request(>client, flags); + if (!fs_sp) { + dev_err(vi->dev, "failed to request frame start syncpoint\n"); + ret = -ENOMEM; + goto free_syncpts; + } + + mw_sp = host1x_syncpt_request(>client, flags); + if (!mw_sp) { + dev_err(vi->dev, "failed to request memory ack syncpoint\n"); + host1x_syncpt_put(fs_sp); + ret = -ENOMEM; + goto free_syncpts; + } + + chan->frame_start_sp[i] = fs_sp; + chan->mw_ack_sp[i] = mw_sp; + spin_lock_init(>sp_incr_lock[i]); + } + + return 0; + +free_syncpts: + for (i = 0; i < chan->numgangports; i++) { + host1x_syncpt_put(chan->mw_ack_sp[i]); + host1x_syncpt_put(chan->frame_start_sp[i]); + } + return ret; +} + +static void tegra210_channel_host1x_syncpt_free(struct tegra_vi_channel *chan) +{ + int i; + + for (i = 0; i < chan->numgangports; i++) { + host1x_syncpt_put(chan->mw_ack_sp[i]); + host1x_syncpt_put(chan->frame_start_sp[i]); + } +} + static void tegra210_fmt_align(struct v4l2_pix_format *pix, unsigned int bpp) { unsigned int min_bpl; @@ -753,6 +803,8 @@ static const struct tegra_video_format tegra210_video_formats[] = { /* Tegra210 VI operations */ static const struct tegra_vi_ops tegra210_vi_ops = { + .channel_host1x_syncpt_init = tegra210_channel_host1x_syncpt_init, + .channel_host1x_syncpt_free = tegra210_channel_host1x_syncpt_free, .vi_fmt_align = tegra210_fmt_align, .vi_start_streaming = tegra210_vi_start_streaming, .vi_stop_streaming = tegra210_vi_stop_streaming, diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index 8df233049c81..b74cdd1d1c82 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -1062,21 +1062,11 @@ static int vi_fmts_bitmap_init(struct tegra_vi_channel *chan) return 0; } -static void tegra_channel_host1x_syncpts_free(struct tegra_vi_channel *chan) -{ - int i; - - for (i = 0; i < chan->numgangports; i++) { - host1x_syncpt_put(chan->mw_ack_sp[i]); - host1x_syncpt_put(chan->frame_start_sp[i]); - } -} - static void tegra_channel_cleanup(struct tegra_vi_channel *chan) { v4l2_ctrl_handler_free(>ctrl_handler); media_entity_cleanup(>video.entity); - tegra_channel_host1x_syncpts_free(chan); + chan->vi->ops->channel_host1x_syncpt_free(chan); mutex_destroy(>video_lock); } @@ -1094,42 +1084,6 @@ void tegra_channels_cleanup(struct tegra_vi *vi) } } -static int tegra_channel_host1x_syncpt_init(struct tegra_vi_channel *chan) -{ - struct tegra_vi *vi = chan->vi; - unsigned long flags = HOST1X_SYNCPT_CLIENT_MANAGED; - struct host1x_syncpt *fs_sp; - struct host1x_syncpt *mw_sp; - int ret, i; - - for (i = 0; i < chan->numgangports; i++) { - fs_sp = host1x_syncpt_request(>client, flags); - if (!fs_sp) { - dev_err(vi->dev, "failed to request frame start syncpoint\n"); - ret = -ENOMEM; - goto free_syncpts; - } - - mw_sp =
[PATCH v5 14/20] staging: media: tegra-video: move MIPI calibration calls from VI to CSI
The CSI module does not handle all the MIPI lane calibration procedure, leaving a small part of it to the VI module. In doing this, tegra_channel_enable_stream() (vi.c) manipulates the private data of the upstream subdev casting it to struct 'tegra_csi_channel', which will be wrong after introducing a VIP (parallel video input) channel. This prevents adding support for the VIP module. It also breaks the logical isolation between modules. Since the lane calibration requirement does not exist in the parallel input module, moving the calibration function to a per-module op is not optimal. Instead move the calibration procedure in the CSI module, together with the rest of the calibration procedures. After this change, tegra_channel_enable_stream() just calls v4l2_subdev_call() to ask for a stream start/stop to the CSI module, which in turn knows all the CSI-specific details to implement it. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/csi.c | 44 drivers/staging/media/tegra-video/vi.c | 54 ++--- 2 files changed, 48 insertions(+), 50 deletions(-) diff --git a/drivers/staging/media/tegra-video/csi.c b/drivers/staging/media/tegra-video/csi.c index 9a03d5ccdf3c..b93fc879ef3a 100644 --- a/drivers/staging/media/tegra-video/csi.c +++ b/drivers/staging/media/tegra-video/csi.c @@ -328,12 +328,42 @@ static int tegra_csi_enable_stream(struct v4l2_subdev *subdev) } csi_chan->pg_mode = chan->pg_mode; + + /* +* Tegra CSI receiver can detect the first LP to HS transition. +* So, start the CSI stream-on prior to sensor stream-on and +* vice-versa for stream-off. +*/ ret = csi->ops->csi_start_streaming(csi_chan); if (ret < 0) goto finish_calibration; + if (csi_chan->mipi) { + struct v4l2_subdev *src_subdev; + /* +* TRM has incorrectly documented to wait for done status from +* calibration logic after CSI interface power on. +* As per the design, calibration results are latched and applied +* to the pads only when the link is in LP11 state which will happen +* during the sensor stream-on. +* CSI subdev stream-on triggers start of MIPI pads calibration. +* Wait for calibration to finish here after sensor subdev stream-on. +*/ + src_subdev = tegra_channel_get_remote_source_subdev(chan); + ret = v4l2_subdev_call(src_subdev, video, s_stream, true); + err = tegra_mipi_finish_calibration(csi_chan->mipi); + + if (ret < 0 && ret != -ENOIOCTLCMD) + goto disable_csi_stream; + + if (err < 0) + dev_warn(csi->dev, "MIPI calibration failed: %d\n", err); + } + return 0; +disable_csi_stream: + csi->ops->csi_stop_streaming(csi_chan); finish_calibration: if (csi_chan->mipi) tegra_mipi_finish_calibration(csi_chan->mipi); @@ -352,10 +382,24 @@ static int tegra_csi_enable_stream(struct v4l2_subdev *subdev) static int tegra_csi_disable_stream(struct v4l2_subdev *subdev) { + struct tegra_vi_channel *chan = v4l2_get_subdev_hostdata(subdev); struct tegra_csi_channel *csi_chan = to_csi_chan(subdev); struct tegra_csi *csi = csi_chan->csi; int err; + /* +* Stream-off subdevices in reverse order to stream-on. +* Remote source subdev in TPG mode is same as CSI subdev. +*/ + if (csi_chan->mipi) { + struct v4l2_subdev *src_subdev; + + src_subdev = tegra_channel_get_remote_source_subdev(chan); + err = v4l2_subdev_call(src_subdev, video, s_stream, false); + if (err < 0 && err != -ENOIOCTLCMD) + dev_err_probe(csi->dev, err, "source subdev stream off failed\n"); + } + csi->ops->csi_stop_streaming(csi_chan); if (csi_chan->mipi) { diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index b88532d8d2c9..c76c2a404889 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -197,49 +197,15 @@ tegra_channel_get_remote_source_subdev(struct tegra_vi_channel *chan) static int tegra_channel_enable_stream(struct tegra_vi_channel *chan) { - struct v4l2_subdev *csi_subdev, *src_subdev; - struct tegra_csi_channel *csi_chan; - int ret, err; + struct v4l2_subdev *subdev; + int ret; - /* -* Tegra CSI receiver can detect the first LP to HS transition. -* So, start the CSI stream-on prior to sensor stream-on and -* vice-versa for stream-off. -*/ -
[PATCH v5 15/20] staging: media: tegra-video: add a per-soc enable/disable op
The Tegra20 VI needs an additional operation to enable the VI, add an operation for that. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/vi.c | 7 +++ drivers/staging/media/tegra-video/vi.h | 4 2 files changed, 11 insertions(+) diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index c76c2a404889..8df233049c81 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -1950,6 +1950,9 @@ static int tegra_vi_probe(struct platform_device *pdev) vi->client.ops = _client_ops; vi->client.dev = >dev; + if (vi->ops->vi_enable) + vi->ops->vi_enable(vi, true); + ret = host1x_client_register(>client); if (ret < 0) { dev_err(>dev, @@ -1960,6 +1963,8 @@ static int tegra_vi_probe(struct platform_device *pdev) return 0; rpm_disable: + if (vi->ops->vi_enable) + vi->ops->vi_enable(vi, false); pm_runtime_disable(>dev); return ret; } @@ -1976,6 +1981,8 @@ static int tegra_vi_remove(struct platform_device *pdev) return err; } + if (vi->ops->vi_enable) + vi->ops->vi_enable(vi, false); pm_runtime_disable(>dev); return 0; diff --git a/drivers/staging/media/tegra-video/vi.h b/drivers/staging/media/tegra-video/vi.h index b424c967c6f2..886b10e7d723 100644 --- a/drivers/staging/media/tegra-video/vi.h +++ b/drivers/staging/media/tegra-video/vi.h @@ -37,8 +37,11 @@ enum tegra_vi_pg_mode { TEGRA_VI_PG_PATCH, }; +struct tegra_vi; + /** * struct tegra_vi_ops - Tegra VI operations + * @vi_enable: soc-specific operations needed to enable/disable the VI peripheral * @vi_fmt_align: modify `pix` to fit the hardware alignment * requirements and fill image geometry * @vi_start_streaming: starts media pipeline, subdevice streaming, sets up @@ -48,6 +51,7 @@ enum tegra_vi_pg_mode { * back any queued buffers. */ struct tegra_vi_ops { + int (*vi_enable)(struct tegra_vi *vi, bool on); void (*vi_fmt_align)(struct v4l2_pix_format *pix, unsigned int bpp); int (*vi_start_streaming)(struct vb2_queue *vq, u32 count); void (*vi_stop_streaming)(struct vb2_queue *vq); -- 2.34.1
[PATCH v5 13/20] staging: media: tegra-video: move default format to soc-specific data
The tegra_default_format in vi.c is specific to Tegra210 CSI. In preparation for adding Tegra20 VIP support, move the default format to a new field in the soc-specific `struct tegra_vi_soc`. Instead of an entire format struct, only store a pointer to an item in the existing format array. No functional changes. The format pointed to is the same that used to be in vi.c. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- Changed in v5: - Minor update after removal of "staging: media: tegra-video: fix .vidioc_enum_fmt_vid_cap to return all formats" patch Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/tegra210.c | 2 ++ drivers/staging/media/tegra-video/vi.c | 13 ++--- drivers/staging/media/tegra-video/vi.h | 2 ++ 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/staging/media/tegra-video/tegra210.c b/drivers/staging/media/tegra-video/tegra210.c index d19ff6b49ae8..b4fcd4e93b8c 100644 --- a/drivers/staging/media/tegra-video/tegra210.c +++ b/drivers/staging/media/tegra-video/tegra210.c @@ -766,8 +766,10 @@ const struct tegra_vi_soc tegra210_vi_soc = { .hw_revision = 3, .vi_max_channels = 6, #if IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG) + .default_video_format = _video_formats[0], .vi_max_clk_hz = 49920, #else + .default_video_format = _video_formats[4], .vi_max_clk_hz = 99840, #endif }; diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index 35d7cda1373f..b88532d8d2c9 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -45,15 +45,6 @@ struct tegra_vi_graph_entity { struct v4l2_subdev *subdev; }; -static const struct tegra_video_format tegra_default_format = { - .img_dt = TEGRA_IMAGE_DT_RAW10, - .bit_width = 10, - .code = MEDIA_BUS_FMT_SRGGB10_1X10, - .bpp = 2, - .img_fmt = TEGRA_IMAGE_FORMAT_DEF, - .fourcc = V4L2_PIX_FMT_SRGGB10, -}; - static inline struct tegra_vi * host1x_client_to_vi(struct host1x_client *client) { @@ -1103,7 +1094,7 @@ static int vi_fmts_bitmap_init(struct tegra_vi_channel *chan) * there are no matched formats. */ if (!match_code) { - match_code = tegra_default_format.code; + match_code = chan->vi->soc->default_video_format->code; index = tegra_get_format_idx_by_code(chan->vi, match_code, 0); if (WARN_ON(index < 0)) return -EINVAL; @@ -1200,7 +1191,7 @@ static int tegra_channel_init(struct tegra_vi_channel *chan) init_waitqueue_head(>done_wait); /* initialize the video format */ - chan->fmtinfo = _default_format; + chan->fmtinfo = chan->vi->soc->default_video_format; chan->format.pixelformat = chan->fmtinfo->fourcc; chan->format.colorspace = V4L2_COLORSPACE_SRGB; chan->format.field = V4L2_FIELD_NONE; diff --git a/drivers/staging/media/tegra-video/vi.h b/drivers/staging/media/tegra-video/vi.h index 213955c7545d..b424c967c6f2 100644 --- a/drivers/staging/media/tegra-video/vi.h +++ b/drivers/staging/media/tegra-video/vi.h @@ -58,6 +58,7 @@ struct tegra_vi_ops { * * @video_formats: supported video formats * @nformats: total video formats + * @default_video_format: default video format (pointer to a @video_formats item) * @ops: vi operations * @hw_revision: VI hw_revision * @vi_max_channels: supported max streaming channels @@ -66,6 +67,7 @@ struct tegra_vi_ops { struct tegra_vi_soc { const struct tegra_video_format *video_formats; const unsigned int nformats; + const struct tegra_video_format *default_video_format; const struct tegra_vi_ops *ops; u32 hw_revision; unsigned int vi_max_channels; -- 2.34.1
[PATCH v5 12/20] staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc op
tegra_channel_fmt_align() takes care of the size constraints, alignment and rounding requirements of the Tegra210 VI peripheral. Tegra20 has different constraints. In preparation for adding Tegra20 support, move this function to a new op in the soc-specific `struct tegra_vi_ops` . Also move to tegra210.c the T210-specific defines used in the moved code. No functional changes. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/tegra210.c | 36 ++ drivers/staging/media/tegra-video/vi.c | 40 +++- drivers/staging/media/tegra-video/vi.h | 9 ++--- 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/drivers/staging/media/tegra-video/tegra210.c b/drivers/staging/media/tegra-video/tegra210.c index d58370a84737..d19ff6b49ae8 100644 --- a/drivers/staging/media/tegra-video/tegra210.c +++ b/drivers/staging/media/tegra-video/tegra210.c @@ -17,6 +17,13 @@ #include "csi.h" #include "vi.h" +#define TEGRA210_MIN_WIDTH 32U +#define TEGRA210_MAX_WIDTH 32768U +#define TEGRA210_MIN_HEIGHT32U +#define TEGRA210_MAX_HEIGHT32768U + +#define SURFACE_ALIGN_BYTES64 + #define TEGRA_VI_SYNCPT_WAIT_TIMEOUT msecs_to_jiffies(200) /* Tegra210 VI registers */ @@ -172,6 +179,34 @@ static u32 vi_csi_read(struct tegra_vi_channel *chan, u8 portno, /* * Tegra210 VI channel capture operations */ +static void tegra210_fmt_align(struct v4l2_pix_format *pix, unsigned int bpp) +{ + unsigned int min_bpl; + unsigned int max_bpl; + unsigned int bpl; + + /* +* The transfer alignment requirements are expressed in bytes. +* Clamp the requested width and height to the limits. +*/ + pix->width = clamp(pix->width, TEGRA210_MIN_WIDTH, TEGRA210_MAX_WIDTH); + pix->height = clamp(pix->height, TEGRA210_MIN_HEIGHT, TEGRA210_MAX_HEIGHT); + + /* Clamp the requested bytes per line value. If the maximum bytes per +* line value is zero, the module doesn't support user configurable +* line sizes. Override the requested value with the minimum in that +* case. +*/ + min_bpl = pix->width * bpp; + max_bpl = rounddown(TEGRA210_MAX_WIDTH, SURFACE_ALIGN_BYTES); + bpl = roundup(pix->bytesperline, SURFACE_ALIGN_BYTES); + + pix->bytesperline = clamp(bpl, min_bpl, max_bpl); + pix->sizeimage = pix->bytesperline * pix->height; + if (pix->pixelformat == V4L2_PIX_FMT_NV16) + pix->sizeimage *= 2; +} + static int tegra_channel_capture_setup(struct tegra_vi_channel *chan, u8 portno) { @@ -718,6 +753,7 @@ static const struct tegra_video_format tegra210_video_formats[] = { /* Tegra210 VI operations */ static const struct tegra_vi_ops tegra210_vi_ops = { + .vi_fmt_align = tegra210_fmt_align, .vi_start_streaming = tegra210_vi_start_streaming, .vi_stop_streaming = tegra210_vi_stop_streaming, }; diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index db98d06351b4..35d7cda1373f 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -473,36 +473,6 @@ static int tegra_channel_get_format(struct file *file, void *fh, return 0; } -static void tegra_channel_fmt_align(struct tegra_vi_channel *chan, - struct v4l2_pix_format *pix, - unsigned int bpp) -{ - unsigned int min_bpl; - unsigned int max_bpl; - unsigned int bpl; - - /* -* The transfer alignment requirements are expressed in bytes. -* Clamp the requested width and height to the limits. -*/ - pix->width = clamp(pix->width, TEGRA_MIN_WIDTH, TEGRA_MAX_WIDTH); - pix->height = clamp(pix->height, TEGRA_MIN_HEIGHT, TEGRA_MAX_HEIGHT); - - /* Clamp the requested bytes per line value. If the maximum bytes per -* line value is zero, the module doesn't support user configurable -* line sizes. Override the requested value with the minimum in that -* case. -*/ - min_bpl = pix->width * bpp; - max_bpl = rounddown(TEGRA_MAX_WIDTH, SURFACE_ALIGN_BYTES); - bpl = roundup(pix->bytesperline, SURFACE_ALIGN_BYTES); - - pix->bytesperline = clamp(bpl, min_bpl, max_bpl); - pix->sizeimage = pix->bytesperline * pix->height; - if (pix->pixelformat == V4L2_PIX_FMT_NV16) - pix->sizeimage *= 2; -} - static int __tegra_channel_try_format(struct tegra_vi_channel *chan, struct v4l2_pix_format *pix) { @@ -578,7 +548,7 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan, return ret; v4l2_fill_pix_format(pix, ); -
[PATCH v5 11/20] staging: media: tegra-video: Kconfig: allow TPG only on Tegra210
We are about to add support for the Tegra20 parallel video capture, which has no TPG. In preparation for that, limit the VIDEO_TEGRA_TPG option to Tegra210 which is the only implementation currently provided by this driver. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/tegra-video/Kconfig b/drivers/staging/media/tegra-video/Kconfig index df1b2cff2417..c53441822fdf 100644 --- a/drivers/staging/media/tegra-video/Kconfig +++ b/drivers/staging/media/tegra-video/Kconfig @@ -15,5 +15,6 @@ config VIDEO_TEGRA config VIDEO_TEGRA_TPG bool "NVIDIA Tegra VI driver TPG mode" depends on VIDEO_TEGRA + depends on ARCH_TEGRA_210_SOC help Say yes here to enable Tegra internal TPG mode -- 2.34.1
[PATCH v5 10/20] staging: media: tegra-video: remove unneeded include
There is only a pointer reference to struct tegra_vi in video.h, thus vi.h is not needed. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/video.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/media/tegra-video/video.h b/drivers/staging/media/tegra-video/video.h index fadaf2189dc9..1e9be1474a9c 100644 --- a/drivers/staging/media/tegra-video/video.h +++ b/drivers/staging/media/tegra-video/video.h @@ -12,7 +12,6 @@ #include #include "vi.h" -#include "csi.h" struct tegra_video_device { struct v4l2_device v4l2_dev; -- 2.34.1
[PATCH v5 08/20] staging: media: tegra-video: move private struct declaration to C file
struct tegra_vi_graph_entity is an internal implementation detail of the VI module. Move its declaration from vi.h to vi.c. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/vi.c | 13 + drivers/staging/media/tegra-video/vi.h | 13 - 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index ce4ff4cbf587..db98d06351b4 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -32,6 +32,19 @@ #define MAX_CID_CONTROLS 1 +/** + * struct tegra_vi_graph_entity - Entity in the video graph + * + * @asd: subdev asynchronous registration information + * @entity: media entity from the corresponding V4L2 subdev + * @subdev: V4L2 subdev + */ +struct tegra_vi_graph_entity { + struct v4l2_async_subdev asd; + struct media_entity *entity; + struct v4l2_subdev *subdev; +}; + static const struct tegra_video_format tegra_default_format = { .img_dt = TEGRA_IMAGE_DT_RAW10, .bit_width = 10, diff --git a/drivers/staging/media/tegra-video/vi.h b/drivers/staging/media/tegra-video/vi.h index 5396bf53ab75..9959cbe02ca0 100644 --- a/drivers/staging/media/tegra-video/vi.h +++ b/drivers/staging/media/tegra-video/vi.h @@ -98,19 +98,6 @@ struct tegra_vi { struct list_head vi_chans; }; -/** - * struct tegra_vi_graph_entity - Entity in the video graph - * - * @asd: subdev asynchronous registration information - * @entity: media entity from the corresponding V4L2 subdev - * @subdev: V4L2 subdev - */ -struct tegra_vi_graph_entity { - struct v4l2_async_subdev asd; - struct media_entity *entity; - struct v4l2_subdev *subdev; -}; - /** * struct tegra_vi_channel - Tegra video channel * -- 2.34.1
[PATCH v5 09/20] staging: media: tegra-video: move tegra210_csi_soc to C file
This declaration is used only in csi.c, no need to export it elsewhere. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags This patch was added in v3. --- drivers/staging/media/tegra-video/csi.c | 4 drivers/staging/media/tegra-video/csi.h | 4 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/media/tegra-video/csi.c b/drivers/staging/media/tegra-video/csi.c index 426e653bd55d..9a03d5ccdf3c 100644 --- a/drivers/staging/media/tegra-video/csi.c +++ b/drivers/staging/media/tegra-video/csi.c @@ -792,6 +792,10 @@ static int tegra_csi_remove(struct platform_device *pdev) return 0; } +#if defined(CONFIG_ARCH_TEGRA_210_SOC) +extern const struct tegra_csi_soc tegra210_csi_soc; +#endif + static const struct of_device_id tegra_csi_of_id_table[] = { #if defined(CONFIG_ARCH_TEGRA_210_SOC) { .compatible = "nvidia,tegra210-csi", .data = _csi_soc }, diff --git a/drivers/staging/media/tegra-video/csi.h b/drivers/staging/media/tegra-video/csi.h index 6960ea2e3d36..3e6e5ee1bb1e 100644 --- a/drivers/staging/media/tegra-video/csi.h +++ b/drivers/staging/media/tegra-video/csi.h @@ -151,10 +151,6 @@ struct tegra_csi { struct list_head csi_chans; }; -#if defined(CONFIG_ARCH_TEGRA_210_SOC) -extern const struct tegra_csi_soc tegra210_csi_soc; -#endif - void tegra_csi_error_recover(struct v4l2_subdev *subdev); void tegra_csi_calc_settle_time(struct tegra_csi_channel *csi_chan, u8 csi_port_num, -- 2.34.1
[PATCH v5 07/20] staging: media: tegra-video: slightly simplify cleanup on errors
of_node_put(node) does nothing if node == NULL, so it can be moved to the cleanup section at the bottom. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/vi.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index 1c0424bf1ab0..ce4ff4cbf587 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -1352,7 +1352,7 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi) struct device_node *node = vi->dev->of_node; struct device_node *ep = NULL; struct device_node *ports; - struct device_node *port; + struct device_node *port = NULL; unsigned int port_num; struct device_node *parent; struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 }; @@ -1375,7 +1375,6 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi) dev_err(vi->dev, "invalid port num %d for %pOF\n", port_num, port); ret = -EINVAL; - of_node_put(port); goto cleanup; } @@ -1398,13 +1397,12 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi) lanes = v4l2_ep.bus.mipi_csi2.num_data_lanes; ret = tegra_vi_channel_alloc(vi, port_num, port, lanes); - if (ret < 0) { - of_node_put(port); + if (ret < 0) goto cleanup; - } } cleanup: + of_node_put(port); of_node_put(ports); return ret; } -- 2.34.1
[PATCH v5 06/20] staging: media: tegra-video: improve error messages
tegra_vi_channels_alloc() can primarily fail for two reasons: 1. "ports" node not found 2. port_num > vi->soc->vi_max_channels Case 1 prints nothing, case 2 has a dev_err(). The caller [tegra_vi_init()] has a generic dev_err() on any failure. This mean that in case 2 we print two messages, and in case 1 we only print a generic message. Remove the generic message and add a specific message when case 1 happens, so that we always have one specific message without even increasing the number of dev_dbg*() calls. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/vi.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index 79dd02a1e29b..1c0424bf1ab0 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -1361,7 +1361,7 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi) ports = of_get_child_by_name(node, "ports"); if (!ports) - return -ENODEV; + return dev_err_probe(vi->dev, -ENODEV, "%pOF: missing 'ports' node\n", node); for_each_child_of_node(ports, port) { if (!of_node_name_eq(port, "port")) @@ -1921,11 +1921,8 @@ static int tegra_vi_init(struct host1x_client *client) ret = tegra_vi_tpg_channels_alloc(vi); else ret = tegra_vi_channels_alloc(vi); - if (ret < 0) { - dev_err(vi->dev, - "failed to allocate vi channels: %d\n", ret); + if (ret < 0) goto free_chans; - } ret = tegra_vi_channels_init(vi); if (ret < 0) -- 2.34.1
[PATCH v5 05/20] staging: media: tegra-video: fix typos in comment
Add "skip" in "so we can *skip* the current channel" or it doesn't make sense. Also add articles where appropriate to fix English grammar. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/vi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index 44c56dc85980..79dd02a1e29b 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -1859,10 +1859,10 @@ static int tegra_vi_graph_init(struct tegra_vi *vi) * Walk the links to parse the full graph. Each channel will have * one endpoint of the composite node. Start by parsing the * composite node and parse the remote entities in turn. -* Each channel will register v4l2 async notifier to make the graph -* independent between the channels so we can the current channel +* Each channel will register a v4l2 async notifier to make the graph +* independent between the channels so we can skip the current channel * in case of something wrong during graph parsing and continue with -* next channels. +* the next channels. */ list_for_each_entry(chan, >vi_chans, list) { struct fwnode_handle *ep, *remote; -- 2.34.1
[PATCH v5 04/20] staging: media: tegra-video: document tegra_channel_get_remote_source_subdev
Clarify what this function does. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/vi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index 11dd142c98c5..44c56dc85980 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -164,6 +164,9 @@ tegra_channel_get_remote_csi_subdev(struct tegra_vi_channel *chan) return media_entity_to_v4l2_subdev(pad->entity); } +/* + * Walk up the chain until the initial source (e.g. image sensor) + */ struct v4l2_subdev * tegra_channel_get_remote_source_subdev(struct tegra_vi_channel *chan) { -- 2.34.1
[PATCH v5 02/20] dt-bindings: display: tegra: vi: add 'vip' property and example
The Tegra20 VI peripheral can receive parallel input from the VIP parallel input module. Add it to the allowed properties and augment the existing nvidia,tegra20-vi example to show a 'vip' property. Signed-off-by: Luca Ceresoli Reviewed-by: Rob Herring --- No changes in v5 Changed in RESEND,v4: - Add Reviewed-by: Rob Herring Changed in v4: - complete the removal of 'channel@0' Changed in v3 (suggested by Rob Herring): - drop 'endpoint', unneeded as there's no extra properties in the endpoints Changed in v2 (suggested by Krzysztof Kozlowski): - rename "i2c3" -> "ic2" - add review tag --- .../display/tegra/nvidia,tegra20-vi.yaml | 59 +++ MAINTAINERS | 1 + 2 files changed, 60 insertions(+) diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml index a42bf33d1e7d..2181855a0920 100644 --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml @@ -73,6 +73,18 @@ properties: avdd-dsi-csi-supply: description: DSI/CSI power supply. Must supply 1.2 V. + vip: +$ref: /schemas/display/tegra/nvidia,tegra20-vip.yaml + + ports: +$ref: /schemas/graph.yaml#/properties/ports + +properties: + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: + Input from the VIP (parallel input capture) module + patternProperties: "^csi@[0-9a-f]+$": type: object @@ -108,6 +120,22 @@ examples: #include #include +i2c { +#address-cells = <1>; +#size-cells = <0>; +camera@48 { +compatible = "aptina,mt9v111"; +reg = <0x48>; +clocks = <_clk>; + +port { +mt9v111_out: endpoint { +remote-endpoint = <_vip_in>; +}; +}; +}; +}; + vi@5408 { compatible = "nvidia,tegra20-vi"; reg = <0x5408 0x0004>; @@ -115,6 +143,37 @@ examples: clocks = <_car TEGRA20_CLK_VI>; resets = <_car 100>; reset-names = "vi"; + +vip { +compatible = "nvidia,tegra20-vip"; +ports { +#address-cells = <1>; +#size-cells = <0>; +port@0 { +reg = <0>; +vi_vip_in: endpoint { +remote-endpoint = <_out>; +}; +}; +port@1 { +reg = <1>; +vi_vip_out: endpoint { +remote-endpoint = <_in>; +}; +}; +}; +}; + +ports { +#address-cells = <1>; +#size-cells = <0>; +port@0 { +reg = <0>; +vi_in: endpoint { +remote-endpoint = <_vip_out>; +}; +}; +}; }; - | diff --git a/MAINTAINERS b/MAINTAINERS index 4e326a9ec281..219eb8fb76b3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20615,6 +20615,7 @@ L: linux-me...@vger.kernel.org L: linux-te...@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml F: drivers/staging/media/tegra-video/ -- 2.34.1
[PATCH v5 03/20] staging: media: tegra-video: improve documentation of tegra_video_format fields
Some fields are irrelevant for Tegra20/VIP. Add a note to clarify that. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/vi.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/tegra-video/vi.h b/drivers/staging/media/tegra-video/vi.h index a68e2c02c7b0..5396bf53ab75 100644 --- a/drivers/staging/media/tegra-video/vi.h +++ b/drivers/staging/media/tegra-video/vi.h @@ -260,11 +260,11 @@ enum tegra_image_dt { /** * struct tegra_video_format - Tegra video format description * - * @img_dt: image data type - * @bit_width: format width in bits per component + * @img_dt: MIPI CSI-2 data type (for CSI-2 only) + * @bit_width: format width in bits per component (for CSI/Tegra210 only) * @code: media bus format code * @bpp: bytes per pixel (when stored in memory) - * @img_fmt: image format + * @img_fmt: image format (for CSI/Tegra210 only) * @fourcc: V4L2 pixel format FCC identifier */ struct tegra_video_format { -- 2.34.1
[PATCH v5 00/20] Add Tegra20 parallel video input capture
New in v5: dropped the patch that was removing lots of the logic behind enum_format, after discussion with Hans. The rest is unmodified except for rebasing and fixing a couple typos in comments. Full details follow. Tegra20 and other Tegra SoCs have a video input (VI) peripheral that can receive from either MIPI CSI-2 or parallel video (called respectively "CSI" and "VIP" in the documentation). The kernel currently has a staging driver for Tegra210 CSI capture. This patch set adds support for Tegra20 VIP capture. Unfortunately I had no real documentation available to base this work on. I only had a working downstream 3.1 kernel, so I started with the driver found there and heavily reworked it to fit into the mainline tegra-video driver structure. The existing code appears written with the intent of being modular and allow adding new input mechanisms and new SoCs while keeping a unique VI core module. However its modularity and extensibility was not enough to add Tegra20 VIP support, so I added some hooks to turn hard-coded behaviour into per-SoC or per-bus customizable code. There are also a fix, some generic cleanups and DT bindings. Quick tour of the patches: * Device tree bindings 01. dt-bindings: display: tegra: add Tegra20 VIP 02. dt-bindings: display: tegra: vi: add 'vip' property and example * Minor improvements to logging, comments, cleanups 03. staging: media: tegra-video: improve documentation of tegra_video_format fields 04. staging: media: tegra-video: document tegra_channel_get_remote_source_subdev 05. staging: media: tegra-video: fix typos in comment 06. staging: media: tegra-video: improve error messages 07. staging: media: tegra-video: slightly simplify cleanup on errors 08. staging: media: tegra-video: move private struct declaration to C file 09. staging: media: tegra-video: move tegra210_csi_soc to C file 10. staging: media: tegra-video: remove unneeded include * Preparation to make the VI module generic enough to host Tegra20 and VIP 11. staging: media: tegra-video: Kconfig: allow TPG only on Tegra210 12. staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc op 13. staging: media: tegra-video: move default format to soc-specific data 14. staging: media: tegra-video: move MIPI calibration calls from VI to CSI 15. staging: media: tegra-video: add a per-soc enable/disable op 16. staging: media: tegra-video: move syncpt init/free to a per-soc op 17. staging: media: tegra-video: add syncpts for Tegra20 to struct tegra_vi 18. staging: media: tegra-video: add hooks for planar YUV and H/V flip 19. staging: media: tegra-video: add H/V flip controls * Implementation of VIP and Tegra20 20. staging: media: tegra-video: add support for Tegra20 parallel input Enjoy! Changed in v5: - removed patch 3 as requested by Hans Verkuil; now the driver is kept video-node-centric and the enum_format logic is unchanged - rebased on top of that - trivial fixes (typos) Changed in RESEND,v4: - add Rob's review tag on patch 2 Changed in v4: - fixed the leftovers after the removal of 'channel@0' in DT - added review tags by Dimtry Changed in v3: - removed the 'channel@0' node from the device tree representation of vip - squashed the last two patches (VIP + T20) into one - small cleanups - rebase on v6.2-rc1 Changed in v2: - improved dt-bindings patches based on reviews - removed patches 3 and 4 adding DT labels without a mainline user - two small fixes to the last patch [v4,resend] https://lore.kernel.org/linux-tegra/20230309144320.2937553-1-luca.ceres...@bootlin.com/ [v4] https://lore.kernel.org/linux-tegra/20230130141603.323221-1-luca.ceres...@bootlin.com/ [v3] https://lore.kernel.org/linux-media/20221229133205.981397-1-luca.ceres...@bootlin.com/ [v2] https://lore.kernel.org/linux-tegra/20221222100328.6e341874@booty/T/#t [v1] https://lore.kernel.org/linux-tegra/20221124155634.5bc2a423@booty/T/#t Luca Luca Ceresoli (20): dt-bindings: display: tegra: add Tegra20 VIP dt-bindings: display: tegra: vi: add 'vip' property and example staging: media: tegra-video: improve documentation of tegra_video_format fields staging: media: tegra-video: document tegra_channel_get_remote_source_subdev staging: media: tegra-video: fix typos in comment staging: media: tegra-video: improve error messages staging: media: tegra-video: slightly simplify cleanup on errors staging: media: tegra-video: move private struct declaration to C file staging: media: tegra-video: move tegra210_csi_soc to C file staging: media: tegra-video: remove unneeded include staging: media: tegra-video: Kconfig: allow TPG only on Tegra210 staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc op staging: media: tegra-video: move default format to soc-specific data staging: media: tegra-video: move MIPI calibration calls from VI to CSI staging: media: tegra-video: add a per-soc enable/disable op
[PATCH v5 01/20] dt-bindings: display: tegra: add Tegra20 VIP
VIP is the parallel video capture component within the video input subsystem of Tegra20 (and other Tegra chips, apparently). Signed-off-by: Luca Ceresoli Reviewed-by: Krzysztof Kozlowski Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags - remove leftover lines after removal of 'channel@0' Changed in v3: - remove channel@0 node (Krzysztof, Rob, Dmitry) - add myself as a maintainer of the whole Tegra video driver (Dmitry) Changed in v2 (suggested by Krzysztof Kozlowski): - remove redundant "bindings" from subject line - remove $nodename - add channel@0 description - add reg: const: 0 --- .../display/tegra/nvidia,tegra20-vip.yaml | 41 +++ MAINTAINERS | 2 + 2 files changed, 43 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml new file mode 100644 index ..14294edb8d8c --- /dev/null +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NVIDIA Tegra VIP (parallel video capture) controller + +maintainers: + - Luca Ceresoli + +properties: + compatible: +enum: + - nvidia,tegra20-vip + + ports: +$ref: /schemas/graph.yaml#/properties/ports + +properties: + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: + Port receiving the video stream from the sensor + + port@1: +$ref: /schemas/graph.yaml#/properties/port +description: + Port sending the video stream to the VI + +required: + - port@0 + - port@1 + +unevaluatedProperties: false + +required: + - compatible + - ports + +# see nvidia,tegra20-vi.yaml for an example diff --git a/MAINTAINERS b/MAINTAINERS index 90abe83c02f3..4e326a9ec281 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20610,10 +20610,12 @@ TEGRA VIDEO DRIVER M: Thierry Reding M: Jonathan Hunter M: Sowjanya Komatineni +M: Luca Ceresoli L: linux-me...@vger.kernel.org L: linux-te...@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml F: drivers/staging/media/tegra-video/ TEGRA XUSB PADCTL DRIVER -- 2.34.1
Re: [PATCH v3 36/65] clk: versatile: sp810: Add a determine_rate hook
On 04/04/2023 11:11, Maxime Ripard wrote: > The Versatile sp810 "timerclken" clock implements a mux with a > set_parent hook, but doesn't provide a determine_rate implementation. > > This is a bit odd, since set_parent() is there to, as its name implies, > change the parent of a clock. Explanation of this mystery is pretty simple - the original patch: commit 6e973d2c438502dcf956e76305258ba7d1c7d1d3 Author: Pawel Moll Date: Thu Apr 18 18:23:22 2013 +0100 clk: vexpress: Add separate SP810 driver predates introduction of determine_rate to clk_ops... commit 71472c0c06cf9a3d1540762ea205654c584e3bc4 Author: James Hogan Date: Mon Jul 29 12:25:00 2013 +0100 clk: add support for clock reparent on set_rate and clearly no one (the author included ;-) bothered to have another look at this side of the driver. > And if it was an oversight, then we are at least explicit about our > behavior now and it can be further refined down the line. It's been one hell of a memory lane trip, but my recollection suggest that the main goal of the driver was simply initialisation of the mux to select the 1MHz parent, because the other option - 32kHz - just didn't make any sense whatsoever. And that would be the case on every single platform using SP810 I know (or at least: knew), so it's seems to me that making the state permanent, as you're suggesting (or I think you're suggesting?) it's the right thing to do. Thanks! Paweł
Re: [PATCH v3 01/13] firmware/sysfb: Fix EFI/VESA format selection
Thomas Zimmermann wrote: > Select color format for EFI/VESA firmware scanout buffer from the > number of bits per pixel and the position of the individual color > components. Fixes the selected format for the buffer in several odd > cases. For example, XRGB1555 has been reported as ARGB1555 because > of the different use of depth and transparency in VESA and Linux. > > Bits-per-pixel is always the pixel's raw number of bits; including > alpha and filler bits. It is preferred over color depth, which has a > different meaning among various components and standards. > > Also do not compare reserved bits and transparency bits to each other. > These values have different meanings, as reserved bits include filler > bits while transparency does not. > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Javier Martinez Canillas > --- > drivers/firmware/sysfb_simplefb.c | 43 ++- > 1 file changed, 37 insertions(+), 6 deletions(-) > [patch elided] Starting at linux-6.3-rc1 my simplefb picks the wrong mode and garbles the display This is on a 16-year old i686 laptop. I can post lshw or dmidecode output if it helps. The issue is still present in 6.3-rc5. Screenshot: https://www.panix.com/~pa/linux-6.3-simplefb/bug-simplefb-bad.jpg Compare: https://www.panix.com/~pa/linux-6.3-simplefb/bug-simplefb-good.jpg This happens during early boot. Grub picks a 1024x786 mode and leaves it for the kernel. The screenshots are from a rescueshell in early userspace. The dmesg excerpts at the bottom might give some clues. I bisected it to f35cd3fa77293c2cd03e94b6a6151e1a7d9309cf firmware/sysfb: Fix EFI/VESA format selection which is this patch. It's not the end of the world: 1) I have a workaround, booting with vga=0x318; 2) the screen is usable even without the workaround; 3) the final fbcon driver takes over after the switch_root. Nevertheless, it would be nice to get this fixed this before 6.3. I may be the only one with this problem. Who else runs -rc kernels on such old hardware ? I'll answer questions as best I can and test any patches thrown at me. --pa
Re: [PATCH 0/2] drm/vkms: A couple of trivial cleanups
Javier Martinez Canillas writes: > Hello, > > This series contains two trivial cleanups for the vkms driver. > > Patch #1 just gets rid of a wrapper helper that wasn't really adding that > much value and patch #2 drops the header > that was only used to call the drm_simple_encoder_init() function helper. > > Best regards, > Javier > > > Javier Martinez Canillas (2): > drm/vkms: Drop vkms_connector_destroy() wrapper > drm/vkms: Remove include > Pushed to drm-misc (drm-misc-next). Thanks! -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Get mutex and rpm ref just once in hwm_power_max_write
On Wed, Apr 05, 2023 at 09:45:20PM -0700, Ashutosh Dixit wrote: > In preparation for follow-on patches, refactor hwm_power_max_write to take > hwmon_lock and runtime pm wakeref at start of the function and release them > at the end, therefore acquiring these just once each. > > Signed-off-by: Ashutosh Dixit Reviewed-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/i915_hwmon.c | 28 +++- > 1 file changed, 15 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c > b/drivers/gpu/drm/i915/i915_hwmon.c > index 8e7dccc8d3a0e..7f44e809ca155 100644 > --- a/drivers/gpu/drm/i915/i915_hwmon.c > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -396,31 +396,33 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val) > { > struct i915_hwmon *hwmon = ddat->hwmon; > intel_wakeref_t wakeref; > + int ret = 0; > u32 nval; > > + mutex_lock(>hwmon_lock); > + wakeref = intel_runtime_pm_get(ddat->uncore->rpm); > + > /* Disable PL1 limit and verify, because the limit cannot be disabled > on all platforms */ > if (val == PL1_DISABLE) { > - mutex_lock(>hwmon_lock); > - with_intel_runtime_pm(ddat->uncore->rpm, wakeref) { > - intel_uncore_rmw(ddat->uncore, hwmon->rg.pkg_rapl_limit, > - PKG_PWR_LIM_1_EN, 0); > - nval = intel_uncore_read(ddat->uncore, > hwmon->rg.pkg_rapl_limit); > - } > - mutex_unlock(>hwmon_lock); > + intel_uncore_rmw(ddat->uncore, hwmon->rg.pkg_rapl_limit, > + PKG_PWR_LIM_1_EN, 0); > + nval = intel_uncore_read(ddat->uncore, > hwmon->rg.pkg_rapl_limit); > > if (nval & PKG_PWR_LIM_1_EN) > - return -ENODEV; > - return 0; > + ret = -ENODEV; > + goto exit; > } > > /* Computation in 64-bits to avoid overflow. Round to nearest. */ > nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, > SF_POWER); > nval = PKG_PWR_LIM_1_EN | REG_FIELD_PREP(PKG_PWR_LIM_1, nval); > > - hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit, > - PKG_PWR_LIM_1_EN | PKG_PWR_LIM_1, > - nval); > - return 0; > + intel_uncore_rmw(ddat->uncore, hwmon->rg.pkg_rapl_limit, > + PKG_PWR_LIM_1_EN | PKG_PWR_LIM_1, nval); > +exit: > + intel_runtime_pm_put(ddat->uncore->rpm, wakeref); > + mutex_unlock(>hwmon_lock); > + return ret; > } > > static int > -- > 2.38.0 >
Re: [Intel-gfx] [PATCH 2/3] drm/i915/guc: Disable PL1 power limit when loading GuC firmware
On Wed, Apr 05, 2023 at 09:45:21PM -0700, Ashutosh Dixit wrote: > On dGfx, the PL1 power limit being enabled and set to a low value results > in a low GPU operating freq. It also negates the freq raise operation which > is done before GuC firmware load. As a result GuC firmware load can time > out. Such timeouts were seen in the GL #8062 bug below (where the PL1 power > limit was enabled and set to a low value). Therefore disable the PL1 power > limit when allowed by HW when loading GuC firmware. > > v2: > - Take mutex (to disallow writes to power1_max) across GuC reset/fw load > - Add hwm_power_max_restore to error return code path > > v3 (Jani N): > - Add/remove explanatory comments > - Function renames > - Type corrections > - Locking annotation > > v4: > - Don't hold the lock across GuC reset (Rodrigo) > - New locking scheme (suggested by Rodrigo) > - Eliminate rpm_get in power_max_disable/restore, not needed (Tvrtko) > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8062 > Signed-off-by: Ashutosh Dixit > --- > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 9 ++ > drivers/gpu/drm/i915/i915_hwmon.c | 40 +++ > drivers/gpu/drm/i915/i915_hwmon.h | 7 + > 3 files changed, 56 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c > b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > index 4ccb4be4c9cba..aa8e35a5636a0 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > @@ -18,6 +18,7 @@ > #include "intel_uc.h" > > #include "i915_drv.h" > +#include "i915_hwmon.h" > > static const struct intel_uc_ops uc_ops_off; > static const struct intel_uc_ops uc_ops_on; > @@ -461,6 +462,7 @@ static int __uc_init_hw(struct intel_uc *uc) > struct intel_guc *guc = >guc; > struct intel_huc *huc = >huc; > int ret, attempts; > + bool pl1en; we need to initialize this to make warn free builds happy... what's our default btw? false? true? we need to read it back? > > GEM_BUG_ON(!intel_uc_supports_guc(uc)); > GEM_BUG_ON(!intel_uc_wants_guc(uc)); > @@ -491,6 +493,9 @@ static int __uc_init_hw(struct intel_uc *uc) > else > attempts = 1; > > + /* Disable a potentially low PL1 power limit to allow freq to be raised > */ > + i915_hwmon_power_max_disable(gt->i915, ); > + > intel_rps_raise_unslice(_to_gt(uc)->rps); > > while (attempts--) { > @@ -547,6 +552,8 @@ static int __uc_init_hw(struct intel_uc *uc) > intel_rps_lower_unslice(_to_gt(uc)->rps); > } > > + i915_hwmon_power_max_restore(gt->i915, pl1en); > + > guc_info(guc, "submission %s\n", > str_enabled_disabled(intel_uc_uses_guc_submission(uc))); > guc_info(guc, "SLPC %s\n", > str_enabled_disabled(intel_uc_uses_guc_slpc(uc))); > > @@ -563,6 +570,8 @@ static int __uc_init_hw(struct intel_uc *uc) > /* Return GT back to RPn */ > intel_rps_lower_unslice(_to_gt(uc)->rps); > > + i915_hwmon_power_max_restore(gt->i915, pl1en); > + > __uc_sanitize(uc); > > if (!ret) { > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c > b/drivers/gpu/drm/i915/i915_hwmon.c > index 7f44e809ca155..9ab8971679fe3 100644 > --- a/drivers/gpu/drm/i915/i915_hwmon.c > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -50,6 +50,7 @@ struct hwm_drvdata { > struct hwm_energy_info ei; /* Energy info for > energy1_input */ > char name[12]; > int gt_n; > + bool reset_in_progress; > }; > > struct i915_hwmon { > @@ -400,6 +401,10 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val) > u32 nval; > > mutex_lock(>hwmon_lock); > + if (hwmon->ddat.reset_in_progress) { > + ret = -EAGAIN; > + goto unlock; > + } > wakeref = intel_runtime_pm_get(ddat->uncore->rpm); > > /* Disable PL1 limit and verify, because the limit cannot be disabled > on all platforms */ > @@ -421,6 +426,7 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val) >PKG_PWR_LIM_1_EN | PKG_PWR_LIM_1, nval); > exit: > intel_runtime_pm_put(ddat->uncore->rpm, wakeref); > +unlock: > mutex_unlock(>hwmon_lock); > return ret; > } > @@ -472,6 +478,40 @@ hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int > chan, long val) > } > } > > +void i915_hwmon_power_max_disable(struct drm_i915_private *i915, bool *old) > +{ > + struct i915_hwmon *hwmon = i915->hwmon; > + u32 r; > + > + if (!hwmon || !i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit)) > + return; > + > + mutex_lock(>hwmon_lock); > + > + hwmon->ddat.reset_in_progress = true; > + r = intel_uncore_rmw(hwmon->ddat.uncore, hwmon->rg.pkg_rapl_limit, > + PKG_PWR_LIM_1_EN, 0); > + *old = !!(r & PKG_PWR_LIM_1_EN); > + > + mutex_unlock(>hwmon_lock); > +} > + > +void i915_hwmon_power_max_restore(struct drm_i915_private *i915, bool
Re: [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Block waiting for GuC reset to complete
On Wed, Apr 05, 2023 at 09:45:22PM -0700, Ashutosh Dixit wrote: > Instead of erroring out when GuC reset is in progress, block waiting for > GuC reset to complete which is a more reasonable uapi behavior. > > Signed-off-by: Ashutosh Dixit > --- > drivers/gpu/drm/i915/i915_hwmon.c | 13 ++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c > b/drivers/gpu/drm/i915/i915_hwmon.c > index 9ab8971679fe3..4343efb48e61b 100644 > --- a/drivers/gpu/drm/i915/i915_hwmon.c > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -51,6 +51,7 @@ struct hwm_drvdata { > char name[12]; > int gt_n; > bool reset_in_progress; > + wait_queue_head_t wqh; > }; > > struct i915_hwmon { > @@ -400,10 +401,15 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val) > int ret = 0; > u32 nval; > > +retry: > mutex_lock(>hwmon_lock); > if (hwmon->ddat.reset_in_progress) { > - ret = -EAGAIN; > - goto unlock; > + mutex_unlock(>hwmon_lock); > + ret = wait_event_interruptible(ddat->wqh, > +!hwmon->ddat.reset_in_progress); this is indeed very clever! maybe just use the timeout version to be on the safeside and then return the -EAGAIN on timeout? my fear is probably due to the lack of knowledge on this wait queue, but I'm wondering what could go wrong if due to some funny race you enter this check right after wake_up_all below has passed and then you will be here indefinitely waiting... > + if (ret) > + return ret; > + goto retry; > } > wakeref = intel_runtime_pm_get(ddat->uncore->rpm); > > @@ -426,7 +432,6 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val) >PKG_PWR_LIM_1_EN | PKG_PWR_LIM_1, nval); > exit: > intel_runtime_pm_put(ddat->uncore->rpm, wakeref); > -unlock: > mutex_unlock(>hwmon_lock); > return ret; > } > @@ -508,6 +513,7 @@ void i915_hwmon_power_max_restore(struct drm_i915_private > *i915, bool old) > intel_uncore_rmw(hwmon->ddat.uncore, hwmon->rg.pkg_rapl_limit, >PKG_PWR_LIM_1_EN, old ? PKG_PWR_LIM_1_EN : 0); > hwmon->ddat.reset_in_progress = false; > + wake_up_all(>ddat.wqh); > > mutex_unlock(>hwmon_lock); > } > @@ -784,6 +790,7 @@ void i915_hwmon_register(struct drm_i915_private *i915) > ddat->uncore = >uncore; > snprintf(ddat->name, sizeof(ddat->name), "i915"); > ddat->gt_n = -1; > + init_waitqueue_head(>wqh); > > for_each_gt(gt, i915, i) { > ddat_gt = hwmon->ddat_gt + i; > -- > 2.38.0 >
Re: [PATCH v5 1/9] drm/gma500: Use drm_aperture_remove_conflicting_pci_framebuffers
Thomas Zimmermann writes: > From: Daniel Vetter > > This one nukes all framebuffers, which is a bit much. In reality > gma500 is igpu and never shipped with anything discrete, so there should > not be any difference. > > v2: Unfortunately the framebuffer sits outside of the pci bars for > gma500, and so only using the pci helpers won't be enough. Otoh if we > only use non-pci helper, then we don't get the vga handling, and > subsequent refactoring to untangle these special cases won't work. > > It's not pretty, but the simplest fix (since gma500 really is the only > quirky pci driver like this we have) is to just have both calls. > > v4: > - fix Daniel's S-o-b address > > v5: > - add back an S-o-b tag with Daniel's Intel address > > Signed-off-by: Daniel Vetter > Signed-off-by: Daniel Vetter > Cc: Patrik Jakobsson > Cc: Thomas Zimmermann > Cc: Javier Martinez Canillas > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH] drm/msm/adreno: fix sparse warnings in a6xx code
Dmitry Baryshkov writes: > Sparse reports plenty of warnings against the a6xx code because of > a6xx_gmu::mmio and a6xx_gmu::rscc members. For some reason they were > defined as __iomem pointers rather than pointers to __iomem memory. > Correct the __iomem attribute. > > Fixes: 02ef80c54e7c ("drm/msm/a6xx: update pdc/rscc GMU registers for > A640/A650") > Reported-by: kernel test robot > Link: > https://lore.kernel.org/oe-kbuild-all/202304070550.nrbhjcvp-...@intel.com/ > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [CI] drm/i915/mtl: Define GuC firmware version for MTL
On Thu, Apr 06, 2023 at 05:37:58PM -0700, Lucas De Marchi wrote: > On Fri, Mar 31, 2023 at 03:52:16PM -0700, john.c.harri...@intel.com wrote: > > From: John Harrison > > > > First release of GuC for Meteorlake. > > > > NB: As this is still pre-release and likely to change, use explicit > > versioning for now. The official, full release will use reduced > > version naming. > > > > Signed-off-by: John Harrison > > Applied to topic/core-for-CI with acks by Rodrigo and Tvrtko. > Reference issue: https://gitlab.freedesktop.org/drm/intel/-/issues/8343 > > Going forward we should plan to have these kind of patches in > drm-intel-gt-next itself rather than using the CI branch. The CI branch > is not the proper place. > > To be clear, since MTL is under force probe and not normally enabled, > it's ok to bump the major version without retaining compabibility with > the previous version, as per > https://docs.kernel.org/driver-api/firmware/firmware-usage-guidelines.html > > > I think the main blocker right now to use drm-intel-gt-next would be > because MODULE_FIRMWARE() is unaware of force_probe. Then distros > would start getting a warning when creating their initrd that they may > have missed a firmware. But that firmware itself is not present in the > upstream linux-firmware repo. We can think about a way to hide the fw > def for *new* firmware (not the legacy ones) that are using the mmp > version. Maybe we should simply move to the final version path sooner than later? Even if that means more updates in the blobs at linux-firmware.git, that would allow the OSVs to have this final patch sooner in their trees. > > For now, let's keep this as is and use the CI branch to assess the > driver health with GuC. > > > thanks > Lucas De Marchi > > > --- > > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > > b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > > index 264c952f777bb..1ac6f9f340e31 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > > @@ -79,6 +79,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw, > > * security fixes, etc. to be enabled. > > */ > > #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \ > > + fw_def(METEORLAKE, 0, guc_mmp(mtl, 70, 6, 5)) \ > > fw_def(DG2, 0, guc_maj(dg2, 70, 5)) \ > > fw_def(ALDERLAKE_P, 0, guc_maj(adlp, 70, 5)) \ > > fw_def(ALDERLAKE_P, 0, guc_mmp(adlp, 70, 1, 1)) \ > > -- > > 2.39.1 > >
[PATCH 1/8] drm/i915/mtl: Define MOCS and PAT tables for MTL
From: Fei Yang On MTL, GT can no longer allocate on LLC - only the CPU can. This, along with addition of support for ADM/L4 cache calls a MOCS/PAT table update. Also defines PTE encode functions for MTL as it has different PAT index definition than previous platforms. BSpec: 44509, 45101, 44235 Cc: Matt Roper Cc: Lucas De Marchi Signed-off-by: Madhumitha Tolakanahalli Pradeep Signed-off-by: Aravind Iddamsetty Signed-off-by: Fei Yang --- drivers/gpu/drm/i915/gt/gen8_ppgtt.c| 28 + drivers/gpu/drm/i915/gt/gen8_ppgtt.h| 3 + drivers/gpu/drm/i915/gt/intel_ggtt.c| 27 + drivers/gpu/drm/i915/gt/intel_gtt.c | 23 +++- drivers/gpu/drm/i915/gt/intel_gtt.h | 20 ++- drivers/gpu/drm/i915/gt/intel_mocs.c| 76 +++-- drivers/gpu/drm/i915/gt/selftest_mocs.c | 2 +- drivers/gpu/drm/i915/i915_pci.c | 1 + 8 files changed, 173 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index 4daaa6f55668..df4073d32114 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -55,6 +55,34 @@ static u64 gen8_pte_encode(dma_addr_t addr, return pte; } +static u64 mtl_pte_encode(dma_addr_t addr, + enum i915_cache_level level, + u32 flags) +{ + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW; + + if (unlikely(flags & PTE_READ_ONLY)) + pte &= ~GEN8_PAGE_RW; + + if (flags & PTE_LM) + pte |= GEN12_PPGTT_PTE_LM | GEN12_PPGTT_PTE_NC; + + switch (level) { + case I915_CACHE_NONE: + pte |= GEN12_PPGTT_PTE_PAT1; + break; + case I915_CACHE_LLC: + case I915_CACHE_L3_LLC: + pte |= GEN12_PPGTT_PTE_PAT0 | GEN12_PPGTT_PTE_PAT1; + break; + case I915_CACHE_WT: + pte |= GEN12_PPGTT_PTE_PAT0; + break; + } + + return pte; +} + static void gen8_ppgtt_notify_vgt(struct i915_ppgtt *ppgtt, bool create) { struct drm_i915_private *i915 = ppgtt->vm.i915; diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.h b/drivers/gpu/drm/i915/gt/gen8_ppgtt.h index f541d19264b4..6b8ce7f4d25a 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.h +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.h @@ -18,5 +18,8 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt, u64 gen8_ggtt_pte_encode(dma_addr_t addr, enum i915_cache_level level, u32 flags); +u64 mtl_ggtt_pte_encode(dma_addr_t addr, + unsigned int pat_index, + u32 flags); #endif diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 3c7f1ed92f5b..4a16bfcde1de 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -220,6 +220,33 @@ static void guc_ggtt_invalidate(struct i915_ggtt *ggtt) } } +u64 mtl_ggtt_pte_encode(dma_addr_t addr, + enum i915_cache_level level, + u32 flags) +{ + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT; + + GEM_BUG_ON(addr & ~GEN12_GGTT_PTE_ADDR_MASK); + + if (flags & PTE_LM) + pte |= GEN12_GGTT_PTE_LM; + + switch (level) { + case I915_CACHE_NONE: + pte |= MTL_GGTT_PTE_PAT1; + break; + case I915_CACHE_LLC: + case I915_CACHE_L3_LLC: + pte |= MTL_GGTT_PTE_PAT0 | MTL_GGTT_PTE_PAT1; + break; + case I915_CACHE_WT: + pte |= MTL_GGTT_PTE_PAT0; + break; + } + + return pte; +} + u64 gen8_ggtt_pte_encode(dma_addr_t addr, enum i915_cache_level level, u32 flags) diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index 4f436ba7a3c8..1e1b34e22cf5 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -468,6 +468,25 @@ void gtt_write_workarounds(struct intel_gt *gt) } } +static void mtl_setup_private_ppat(struct intel_uncore *uncore) +{ + intel_uncore_write(uncore, GEN12_PAT_INDEX(0), + MTL_PPAT_L4_0_WB); + intel_uncore_write(uncore, GEN12_PAT_INDEX(1), + MTL_PPAT_L4_1_WT); + intel_uncore_write(uncore, GEN12_PAT_INDEX(2), + MTL_PPAT_L4_3_UC); + intel_uncore_write(uncore, GEN12_PAT_INDEX(3), + MTL_PPAT_L4_0_WB | MTL_2_COH_1W); + intel_uncore_write(uncore, GEN12_PAT_INDEX(4), + MTL_PPAT_L4_0_WB | MTL_3_COH_2W); + + /* +* Remaining PAT entries are left at the hardware-default +* fully-cached setting +*/ +} + static void tgl_setup_private_ppat(struct intel_uncore *uncore) { /* TGL
[PATCH 3/8] drm/i915/mtl: workaround coherency issue for Media
From: Fei Yang This patch implements Wa_22016122933. In MTL, memory writes initiated by Media tile update the whole cache line even for partial writes. This creates a coherency problem for cacheable memory if both CPU and GPU are writing data to different locations within a single cache line. CTB communication is impacted by this issue because the head and tail pointers are adjacent words within a cache line (see struct guc_ct_buffer_desc), where one is written by GuC and the other by the host. This patch circumvents the issue by making CPU/GPU shared memory uncacheable (WC on CPU side, and PAT index 2 for GPU). Also for CTB which is being updated by both CPU and GuC, mfence instruction is added to make sure the CPU writes are visible to GPU right away (flush the write combining buffer). While fixing the CTB issue, we noticed some random GSC firmware loading failure because the share buffers are cacheable (WB) on CPU side but uncached on GPU side. To fix these issues we need to map such shared buffers as WC on CPU side. Since such allocations are not all done through GuC allocator, to avoid too many code changes, the i915_coherent_map_type() is now hard coded to return WC for MTL. BSpec: 45101 Signed-off-by: Fei Yang --- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 5 - drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c | 13 + drivers/gpu/drm/i915/gt/uc/intel_guc.c| 7 +++ drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 18 -- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c index ecd86130b74f..89fc8ea6bcfc 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c @@ -469,7 +469,10 @@ enum i915_map_type i915_coherent_map_type(struct drm_i915_private *i915, struct drm_i915_gem_object *obj, bool always_coherent) { - if (i915_gem_object_is_lmem(obj)) + /* +* Wa_22016122933: always return I915_MAP_WC for MTL +*/ + if (i915_gem_object_is_lmem(obj) || IS_METEORLAKE(i915)) return I915_MAP_WC; if (HAS_LLC(i915) || always_coherent) return I915_MAP_WB; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c index 1d9fdfb11268..236673c02f9a 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c @@ -110,6 +110,13 @@ static int gsc_fw_load_prepare(struct intel_gsc_uc *gsc) if (obj->base.size < gsc->fw.size) return -ENOSPC; + /* +* Wa_22016122933: For MTL the shared memory needs to be mapped +* as WC on CPU side and UC (PAT index 2) on GPU side +*/ + if (IS_METEORLAKE(i915)) + i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE); + dst = i915_gem_object_pin_map_unlocked(obj, i915_coherent_map_type(i915, obj, true)); if (IS_ERR(dst)) @@ -125,6 +132,12 @@ static int gsc_fw_load_prepare(struct intel_gsc_uc *gsc) memset(dst, 0, obj->base.size); memcpy(dst, src, gsc->fw.size); + /* +* Wa_22016122933: Making sure the data in dst is +* visible to GSC right away +*/ + intel_guc_write_barrier(>uc.guc); + i915_gem_object_unpin_map(gsc->fw.obj); i915_gem_object_unpin_map(obj); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c index d76508fa3af7..f9bddaa876d9 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c @@ -743,6 +743,13 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size) if (IS_ERR(obj)) return ERR_CAST(obj); + /* +* Wa_22016122933: For MTL the shared memory needs to be mapped +* as WC on CPU side and UC (PAT index 2) on GPU side +*/ + if (IS_METEORLAKE(gt->i915)) + i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE); + vma = i915_vma_instance(obj, >ggtt->vm, NULL); if (IS_ERR(vma)) goto err; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c index 1803a633ed64..98e682b7df07 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c @@ -415,12 +415,6 @@ static int ct_write(struct intel_guc_ct *ct, } GEM_BUG_ON(tail > size); - /* -* make sure H2G buffer update and LRC tail update (if this triggering a -* submission) are visible before updating the descriptor tail -*/ - intel_guc_write_barrier(ct_to_guc(ct)); - /* update local copies */ ctb->tail = tail; GEM_BUG_ON(atomic_read(>space) < len +
[PATCH 5/8] drm/i915: preparation for using PAT index
From: Fei Yang This patch is a preparation for replacing enum i915_cache_level with PAT index. Caching policy for buffer objects is set through the PAT index in PTE, the old i915_cache_level is not sufficient to represent all caching modes supported by the hardware. Preparing the transition by adding some platform dependent data structures and helper functions to translate the cache_level to pat_index. cachelevel_to_pat: a platform dependent array mapping cache_level to pat_index. max_pat_index: the maximum PAT index supported by the hardware. Needed for validating the PAT index passed in from user space. i915_gem_get_pat_index: function to convert cache_level to PAT index. obj_to_i915(obj): macro moved to header file for wider usage. I915_MAX_CACHE_LEVEL: upper bound of i915_cache_level for the convenience of coding. Cc: Chris Wilson Cc: Matt Roper Cc: Andi Shyti Signed-off-by: Fei Yang --- drivers/gpu/drm/i915/gem/i915_gem_object.c| 9 +++ drivers/gpu/drm/i915/gem/i915_gem_object.h| 4 + .../gpu/drm/i915/gem/i915_gem_object_types.h | 1 + drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 2 - drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 6 ++ drivers/gpu/drm/i915/gt/intel_ggtt.c | 6 ++ drivers/gpu/drm/i915/i915_pci.c | 75 +-- drivers/gpu/drm/i915/intel_device_info.h | 5 ++ .../gpu/drm/i915/selftests/mock_gem_device.c | 6 ++ 9 files changed, 104 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 4666bb82f312..8c70a0ec7d2f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -45,6 +45,15 @@ static struct kmem_cache *slab_objects; static const struct drm_gem_object_funcs i915_gem_object_funcs; +unsigned int i915_gem_get_pat_index(struct drm_i915_private *i915, + enum i915_cache_level level) +{ + if (drm_WARN_ON(>drm, level >= I915_MAX_CACHE_LEVEL)) + return 0; + + return INTEL_INFO(i915)->cachelevel_to_pat[level]; +} + struct drm_i915_gem_object *i915_gem_object_alloc(void) { struct drm_i915_gem_object *obj; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 885ccde9dc3c..4c92e17b4337 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -20,6 +20,8 @@ enum intel_region_id; +#define obj_to_i915(obj__) to_i915((obj__)->base.dev) + static inline bool i915_gem_object_size_2big(u64 size) { struct drm_i915_gem_object *obj; @@ -30,6 +32,8 @@ static inline bool i915_gem_object_size_2big(u64 size) return false; } +unsigned int i915_gem_get_pat_index(struct drm_i915_private *i915, + enum i915_cache_level level); void i915_gem_init__objects(struct drm_i915_private *i915); void i915_objects_module_exit(void); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 5dcbbef31d44..890f3ad497c5 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -194,6 +194,7 @@ enum i915_cache_level { * engine. */ I915_CACHE_WT, + I915_MAX_CACHE_LEVEL, }; enum i915_map_type { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c index b1672e054b21..214763942aa2 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c @@ -460,8 +460,6 @@ void i915_gem_shrinker_taints_mutex(struct drm_i915_private *i915, fs_reclaim_release(GFP_KERNEL); } -#define obj_to_i915(obj__) to_i915((obj__)->base.dev) - /** * i915_gem_object_make_unshrinkable - Hide the object from the shrinker. By * default all object types that support shrinking(see IS_SHRINKABLE), will also diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index 11b91e0453c8..7a4b1d1afce9 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -78,6 +78,12 @@ static u64 mtl_pte_encode(dma_addr_t addr, case I915_CACHE_WT: pte |= GEN12_PPGTT_PTE_PAT0; break; + default: + /* This should never happen. Added to deal with the compile +* error due to the addition of I915_MAX_CACHE_LEVEL. Will +* be removed by the pat_index patch. +*/ + break; } return pte; diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index ba3109338aee..91056b9a60a9 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -242,6
[PATCH 0/8] drm/i915/mtl: Define MOCS and PAT tables for MTL
From: Fei Yang The series includes patches needed to enable MTL. Also add new extension for GEM_CREATE uAPI to let user space set cache policy for buffer objects. Fei Yang (8): drm/i915/mtl: Define MOCS and PAT tables for MTL drm/i915/mtl: enforce mtl PTE encode drm/i915/mtl: workaround coherency issue for Media drm/i915/mtl: end support for set caching ioctl drm/i915: preparation for using PAT index drm/i915: use pat_index instead of cache_level drm/i915: making mtl pte encode generic for gen12 drm/i915: Allow user to set cache at BO creation drivers/gpu/drm/i915/display/intel_dpt.c | 14 ++-- drivers/gpu/drm/i915/gem/i915_gem_create.c| 36 drivers/gpu/drm/i915/gem/i915_gem_domain.c| 30 +++ .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 10 ++- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 3 +- drivers/gpu/drm/i915/gem/i915_gem_object.c| 61 +- drivers/gpu/drm/i915/gem/i915_gem_object.h| 8 ++ .../gpu/drm/i915/gem/i915_gem_object_types.h | 26 +- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 5 +- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 9 +- drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 2 - drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 4 +- drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 16 ++-- .../gpu/drm/i915/gem/selftests/huge_pages.c | 2 +- .../drm/i915/gem/selftests/i915_gem_migrate.c | 2 +- .../drm/i915/gem/selftests/i915_gem_mman.c| 2 +- drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 10 ++- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 81 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.h | 6 +- drivers/gpu/drm/i915/gt/intel_ggtt.c | 84 +-- drivers/gpu/drm/i915/gt/intel_gtt.c | 23 - drivers/gpu/drm/i915/gt/intel_gtt.h | 38 ++--- drivers/gpu/drm/i915/gt/intel_migrate.c | 47 ++- drivers/gpu/drm/i915/gt/intel_migrate.h | 13 ++- drivers/gpu/drm/i915/gt/intel_mocs.c | 76 - drivers/gpu/drm/i915/gt/intel_ppgtt.c | 6 +- drivers/gpu/drm/i915/gt/selftest_migrate.c| 47 ++- drivers/gpu/drm/i915/gt/selftest_mocs.c | 2 +- drivers/gpu/drm/i915/gt/selftest_reset.c | 8 +- drivers/gpu/drm/i915/gt/selftest_timeline.c | 2 +- drivers/gpu/drm/i915/gt/selftest_tlb.c| 4 +- drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c | 13 +++ drivers/gpu/drm/i915/gt/uc/intel_guc.c| 7 ++ drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 18 ++-- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 10 ++- drivers/gpu/drm/i915/i915_debugfs.c | 55 +--- drivers/gpu/drm/i915/i915_gem.c | 16 +++- drivers/gpu/drm/i915/i915_gpu_error.c | 8 +- drivers/gpu/drm/i915/i915_pci.c | 76 +++-- drivers/gpu/drm/i915/i915_vma.c | 16 ++-- drivers/gpu/drm/i915/i915_vma.h | 2 +- drivers/gpu/drm/i915/i915_vma_types.h | 2 - drivers/gpu/drm/i915/intel_device_info.h | 5 ++ drivers/gpu/drm/i915/selftests/i915_gem.c | 5 +- .../gpu/drm/i915/selftests/i915_gem_evict.c | 4 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 15 ++-- .../drm/i915/selftests/intel_memory_region.c | 4 +- .../gpu/drm/i915/selftests/mock_gem_device.c | 6 ++ drivers/gpu/drm/i915/selftests/mock_gtt.c | 8 +- include/uapi/drm/i915_drm.h | 36 tools/include/uapi/drm/i915_drm.h | 36 51 files changed, 788 insertions(+), 231 deletions(-) -- 2.25.1
[PATCH 4/8] drm/i915/mtl: end support for set caching ioctl
From: Fei Yang The design is to keep Buffer Object's caching policy immutable through out its life cycle. This patch ends the support for set caching ioctl from MTL onward. While doing that we also set BO's to be 1-way coherent at creation time because GPU is no longer automatically snooping CPU cache. Signed-off-by: Fei Yang --- drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++ drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 9 - 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c index d2d5a24301b2..bb3575b1479f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c @@ -337,6 +337,9 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data, if (IS_DGFX(i915)) return -ENODEV; + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) + return -EOPNOTSUPP; + switch (args->caching) { case I915_CACHING_NONE: level = I915_CACHE_NONE; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 37d1efcd3ca6..e602c323896b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -601,7 +601,14 @@ static int shmem_object_init(struct intel_memory_region *mem, obj->write_domain = I915_GEM_DOMAIN_CPU; obj->read_domains = I915_GEM_DOMAIN_CPU; - if (HAS_LLC(i915)) + /* +* MTL doesn't snooping CPU cache by default for GPU access (namely +* 1-way coherency). However some UMD's are currently depending on +* that. Make 1-way coherent the default setting for MTL. A follow +* up patch will extend the GEM_CREATE uAPI to allow UMD's specify +* caching mode at BO creation time +*/ + if (HAS_LLC(i915) || (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))) /* On some devices, we can have the GPU use the LLC (the CPU * cache) for about a 10% performance improvement * compared to uncached. Graphics requests other than -- 2.25.1
[PATCH 7/8] drm/i915: making mtl pte encode generic for gen12
From: Fei Yang PTE encode is platform dependent. After replacing cache_level with pat_index, the newly introduced mtl_pte_encode is actually generic for all gen12 platforms, thus rename it to gen12_pte_encode and apply it to all gen12 platforms. Cc: Chris Wilson Cc: Matt Roper Cc: Andi Shyti Signed-off-by: Fei Yang --- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index f76ec2cb29ef..e393e20b5894 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -60,7 +60,7 @@ static u64 gen8_pte_encode(dma_addr_t addr, return pte; } -static u64 mtl_pte_encode(dma_addr_t addr, +static u64 gen12_pte_encode(dma_addr_t addr, unsigned int pat_index, u32 flags) { @@ -999,8 +999,8 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt, */ ppgtt->vm.alloc_scratch_dma = alloc_pt_dma; - if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) - ppgtt->vm.pte_encode = mtl_pte_encode; + if (GRAPHICS_VER(gt->i915) >= 12) + ppgtt->vm.pte_encode = gen12_pte_encode; else ppgtt->vm.pte_encode = gen8_pte_encode; -- 2.25.1
[PATCH 2/8] drm/i915/mtl: enforce mtl PTE encode
From: Fei Yang PTE encode functions are platform dependent. This patch ensures the correct PTE encode function is used by calling pte_encode function pointer instead of the hardcoded gen8 version of PTE encode. Signed-off-by: Fei Yang --- drivers/gpu/drm/i915/display/intel_dpt.c | 2 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 17 ++--- drivers/gpu/drm/i915/gt/intel_ggtt.c | 9 ++--- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c index b8027392144d..c5eacfdba1a5 100644 --- a/drivers/gpu/drm/i915/display/intel_dpt.c +++ b/drivers/gpu/drm/i915/display/intel_dpt.c @@ -300,7 +300,7 @@ intel_dpt_create(struct intel_framebuffer *fb) vm->vma_ops.bind_vma= dpt_bind_vma; vm->vma_ops.unbind_vma = dpt_unbind_vma; - vm->pte_encode = gen8_ggtt_pte_encode; + vm->pte_encode = vm->gt->ggtt->vm.pte_encode; dpt->obj = dpt_obj; dpt->obj->is_dpt = true; diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index df4073d32114..11b91e0453c8 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -455,7 +455,7 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, u32 flags) { struct i915_page_directory *pd; - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags); + const gen8_pte_t pte_encode = ppgtt->vm.pte_encode(0, cache_level, flags); gen8_pte_t *vaddr; pd = i915_pd_entry(pdp, gen8_pd_index(idx, 2)); @@ -608,7 +608,7 @@ static void gen8_ppgtt_insert_huge(struct i915_address_space *vm, enum i915_cache_level cache_level, u32 flags) { - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags); + const gen8_pte_t pte_encode = vm->pte_encode(0, cache_level, flags); unsigned int rem = sg_dma_len(iter->sg); u64 start = vma_res->start; @@ -771,7 +771,7 @@ static void gen8_ppgtt_insert_entry(struct i915_address_space *vm, GEM_BUG_ON(pt->is_compact); vaddr = px_vaddr(pt); - vaddr[gen8_pd_index(idx, 0)] = gen8_pte_encode(addr, level, flags); + vaddr[gen8_pd_index(idx, 0)] = vm->pte_encode(addr, level, flags); drm_clflush_virt_range([gen8_pd_index(idx, 0)], sizeof(*vaddr)); } @@ -801,7 +801,7 @@ static void __xehpsdv_ppgtt_insert_entry_lm(struct i915_address_space *vm, } vaddr = px_vaddr(pt); - vaddr[gen8_pd_index(idx, 0) / 16] = gen8_pte_encode(addr, level, flags); + vaddr[gen8_pd_index(idx, 0) / 16] = vm->pte_encode(addr, level, flags); } static void xehpsdv_ppgtt_insert_entry(struct i915_address_space *vm, @@ -848,8 +848,8 @@ static int gen8_init_scratch(struct i915_address_space *vm) pte_flags |= PTE_LM; vm->scratch[0]->encode = - gen8_pte_encode(px_dma(vm->scratch[0]), - I915_CACHE_NONE, pte_flags); + vm->pte_encode(px_dma(vm->scratch[0]), + I915_CACHE_NONE, pte_flags); for (i = 1; i <= vm->top; i++) { struct drm_i915_gem_object *obj; @@ -991,7 +991,10 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt, */ ppgtt->vm.alloc_scratch_dma = alloc_pt_dma; - ppgtt->vm.pte_encode = gen8_pte_encode; + if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) + ppgtt->vm.pte_encode = mtl_pte_encode; + else + ppgtt->vm.pte_encode = gen8_pte_encode; ppgtt->vm.bind_async_flags = I915_VMA_LOCAL_BIND; ppgtt->vm.insert_entries = gen8_ppgtt_insert; diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 4a16bfcde1de..ba3109338aee 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -274,7 +274,7 @@ static void gen8_ggtt_insert_page(struct i915_address_space *vm, gen8_pte_t __iomem *pte = (gen8_pte_t __iomem *)ggtt->gsm + offset / I915_GTT_PAGE_SIZE; - gen8_set_pte(pte, gen8_ggtt_pte_encode(addr, level, flags)); + gen8_set_pte(pte, ggtt->vm.pte_encode(addr, level, flags)); ggtt->invalidate(ggtt); } @@ -284,8 +284,8 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm, enum i915_cache_level level, u32 flags) { - const gen8_pte_t pte_encode = gen8_ggtt_pte_encode(0, level, flags); struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm); + const gen8_pte_t pte_encode = ggtt->vm.pte_encode(0, level, flags); gen8_pte_t __iomem *gte; gen8_pte_t __iomem *end; struct sgt_iter iter; @@ -1008,7 +1008,10 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
[PATCH 8/8] drm/i915: Allow user to set cache at BO creation
From: Fei Yang To comply with the design that buffer objects shall have immutable cache setting through out its life cycle, {set, get}_caching ioctl's are no longer supported from MTL onward. With that change caching policy can only be set at object creation time. The current code applies a default (platform dependent) cache setting for all objects. However this is not optimal for performance tuning. The patch extends the existing gem_create uAPI to let user set PAT index for the object at creation time. The new extension is platform independent, so UMD's can switch to using this extension for older platforms as well, while {set, get}_caching are still supported on these legacy paltforms for compatibility reason. Cc: Chris Wilson Cc: Matt Roper Cc: Andi Shyti Signed-off-by: Fei Yang --- drivers/gpu/drm/i915/gem/i915_gem_create.c | 36 ++ include/uapi/drm/i915_drm.h| 36 ++ tools/include/uapi/drm/i915_drm.h | 36 ++ 3 files changed, 108 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index bfe1dbda4cb7..723c3ddd6c74 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -245,6 +245,7 @@ struct create_ext { unsigned int n_placements; unsigned int placement_mask; unsigned long flags; + unsigned int pat_index; }; static void repr_placements(char *buf, size_t size, @@ -394,11 +395,39 @@ static int ext_set_protected(struct i915_user_extension __user *base, void *data return 0; } +static int ext_set_pat(struct i915_user_extension __user *base, void *data) +{ + struct create_ext *ext_data = data; + struct drm_i915_private *i915 = ext_data->i915; + struct drm_i915_gem_create_ext_set_pat ext; + unsigned int max_pat_index; + + BUILD_BUG_ON(sizeof(struct drm_i915_gem_create_ext_set_pat) != +offsetofend(struct drm_i915_gem_create_ext_set_pat, rsvd)); + + if (copy_from_user(, base, sizeof(ext))) + return -EFAULT; + + max_pat_index = INTEL_INFO(i915)->max_pat_index; + + if (ext.pat_index > max_pat_index) { + drm_dbg(>drm, "PAT index is invalid: %u\n", + ext.pat_index); + return -EINVAL; + } + + ext_data->pat_index = ext.pat_index; + + return 0; +} + static const i915_user_extension_fn create_extensions[] = { [I915_GEM_CREATE_EXT_MEMORY_REGIONS] = ext_set_placements, [I915_GEM_CREATE_EXT_PROTECTED_CONTENT] = ext_set_protected, + [I915_GEM_CREATE_EXT_SET_PAT] = ext_set_pat, }; +#define PAT_INDEX_NOT_SET 0x /** * i915_gem_create_ext_ioctl - Creates a new mm object and returns a handle to it. * @dev: drm device pointer @@ -418,6 +447,7 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, if (args->flags & ~I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS) return -EINVAL; + ext_data.pat_index = PAT_INDEX_NOT_SET; ret = i915_user_extensions(u64_to_user_ptr(args->extensions), create_extensions, ARRAY_SIZE(create_extensions), @@ -454,5 +484,11 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, if (IS_ERR(obj)) return PTR_ERR(obj); + if (ext_data.pat_index != PAT_INDEX_NOT_SET) { + i915_gem_object_set_pat_index(obj, ext_data.pat_index); + /* Mark pat_index is set by UMD */ + obj->cache_level = I915_CACHE_INVAL; + } + return i915_gem_publish(obj, file, >size, >handle); } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index dba7c5a5b25e..03c5c314846e 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -3630,9 +3630,13 @@ struct drm_i915_gem_create_ext { * * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see * struct drm_i915_gem_create_ext_protected_content. +* +* For I915_GEM_CREATE_EXT_SET_PAT usage see +* struct drm_i915_gem_create_ext_set_pat. */ #define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0 #define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1 +#define I915_GEM_CREATE_EXT_SET_PAT 2 __u64 extensions; }; @@ -3747,6 +3751,38 @@ struct drm_i915_gem_create_ext_protected_content { __u32 flags; }; +/** + * struct drm_i915_gem_create_ext_set_pat - The + * I915_GEM_CREATE_EXT_SET_PAT extension. + * + * If this extension is provided, the specified caching policy (PAT index) is + * applied to the buffer object. + * + * Below is an example on how to create an object with specific caching policy: + * + * .. code-block:: C + * + * struct drm_i915_gem_create_ext_set_pat set_pat_ext = { + * .base = { .name = I915_GEM_CREATE_EXT_SET_PAT },
[PATCH 6/8] drm/i915: use pat_index instead of cache_level
From: Fei Yang Currently the KMD is using enum i915_cache_level to set caching policy for buffer objects. This is flaky because the PAT index which really controls the caching behavior in PTE has far more levels than what's defined in the enum. In addition, the PAT index is platform dependent, having to translate between i915_cache_level and PAT index is not reliable, and makes the code more complicated. >From UMD's perspective there is also a necessity to set caching policy for performance fine tuning. It's much easier for the UMD to directly use PAT index because the behavior of each PAT index is clearly defined in Bspec. Haivng the abstracted i915_cache_level sitting in between would only cause more ambiguity. For these reasons this patch replaces i915_cache_level with PAT index. Also note, the cache_level is not completely removed yet, because the KMD still has the need of creating buffer objects with simple cache settings such as cached, uncached, or writethrough. For these simple cases, using cache_level would help simplify the code. Cc: Chris Wilson Cc: Matt Roper Signed-off-by: Fei Yang --- drivers/gpu/drm/i915/display/intel_dpt.c | 12 +-- drivers/gpu/drm/i915/gem/i915_gem_domain.c| 27 ++ .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 10 ++- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 3 +- drivers/gpu/drm/i915/gem/i915_gem_object.c| 52 +++- drivers/gpu/drm/i915/gem/i915_gem_object.h| 4 + .../gpu/drm/i915/gem/i915_gem_object_types.h | 25 +- drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 4 +- drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 16 ++-- .../gpu/drm/i915/gem/selftests/huge_pages.c | 2 +- .../drm/i915/gem/selftests/i915_gem_migrate.c | 2 +- .../drm/i915/gem/selftests/i915_gem_mman.c| 2 +- drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 10 ++- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 76 - drivers/gpu/drm/i915/gt/gen8_ppgtt.h | 3 +- drivers/gpu/drm/i915/gt/intel_ggtt.c | 82 +-- drivers/gpu/drm/i915/gt/intel_gtt.h | 20 ++--- drivers/gpu/drm/i915/gt/intel_migrate.c | 47 ++- drivers/gpu/drm/i915/gt/intel_migrate.h | 13 ++- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 6 +- drivers/gpu/drm/i915/gt/selftest_migrate.c| 47 ++- drivers/gpu/drm/i915/gt/selftest_reset.c | 8 +- drivers/gpu/drm/i915/gt/selftest_timeline.c | 2 +- drivers/gpu/drm/i915/gt/selftest_tlb.c| 4 +- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 10 ++- drivers/gpu/drm/i915/i915_debugfs.c | 55 ++--- drivers/gpu/drm/i915/i915_gem.c | 16 +++- drivers/gpu/drm/i915/i915_gpu_error.c | 8 +- drivers/gpu/drm/i915/i915_vma.c | 16 ++-- drivers/gpu/drm/i915/i915_vma.h | 2 +- drivers/gpu/drm/i915/i915_vma_types.h | 2 - drivers/gpu/drm/i915/selftests/i915_gem.c | 5 +- .../gpu/drm/i915/selftests/i915_gem_evict.c | 4 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 15 ++-- .../drm/i915/selftests/intel_memory_region.c | 4 +- drivers/gpu/drm/i915/selftests/mock_gtt.c | 8 +- 36 files changed, 383 insertions(+), 239 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c index c5eacfdba1a5..7c5fddb203ba 100644 --- a/drivers/gpu/drm/i915/display/intel_dpt.c +++ b/drivers/gpu/drm/i915/display/intel_dpt.c @@ -43,24 +43,24 @@ static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte) static void dpt_insert_page(struct i915_address_space *vm, dma_addr_t addr, u64 offset, - enum i915_cache_level level, + unsigned int pat_index, u32 flags) { struct i915_dpt *dpt = i915_vm_to_dpt(vm); gen8_pte_t __iomem *base = dpt->iomem; gen8_set_pte(base + offset / I915_GTT_PAGE_SIZE, -vm->pte_encode(addr, level, flags)); +vm->pte_encode(addr, pat_index, flags)); } static void dpt_insert_entries(struct i915_address_space *vm, struct i915_vma_resource *vma_res, - enum i915_cache_level level, + unsigned int pat_index, u32 flags) { struct i915_dpt *dpt = i915_vm_to_dpt(vm); gen8_pte_t __iomem *base = dpt->iomem; - const gen8_pte_t pte_encode = vm->pte_encode(0, level, flags); + const gen8_pte_t pte_encode = vm->pte_encode(0, pat_index, flags); struct sgt_iter sgt_iter; dma_addr_t addr; int i; @@ -83,7 +83,7 @@ static void dpt_clear_range(struct i915_address_space *vm, static void dpt_bind_vma(struct i915_address_space *vm, struct i915_vm_pt_stash *stash, struct i915_vma_resource
[PATCH 4/5] drm/mediatek: Add casting before assign
Add casting before assign to avoid the unintentional integer overflow or unintended sign extension. Signed-off-by: Jason-JH.Lin Fixes: 1a64a7aff8da ("drm/mediatek: Fix cursor plane no update") --- drivers/gpu/drm/mediatek/mtk_drm_gem.c | 2 +- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 20 +++- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c b/drivers/gpu/drm/mediatek/mtk_drm_gem.c index 9b8f72ed12e4..537e83b95001 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c @@ -121,7 +121,7 @@ int mtk_drm_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev, int ret; args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8); - args->size = args->pitch * args->height; + args->size = (unsigned long)args->pitch * args->height; mtk_gem = mtk_drm_gem_create(dev, args->size, false); if (IS_ERR(mtk_gem)) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 31f9420aff6f..a1337f386bbf 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -140,7 +140,7 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state, struct drm_framebuffer *fb = new_state->fb; struct drm_gem_object *gem; struct mtk_drm_gem_obj *mtk_gem; - unsigned int pitch, format; + unsigned int pitch, format, cpp; u64 modifier; dma_addr_t addr; dma_addr_t hdr_addr = 0; @@ -151,11 +151,12 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state, addr = mtk_gem->dma_addr; pitch = fb->pitches[0]; format = fb->format->format; + cpp = (unsigned int)fb->format->cpp[0]; modifier = fb->modifier; if (modifier == DRM_FORMAT_MOD_LINEAR) { - addr += (new_state->src.x1 >> 16) * fb->format->cpp[0]; - addr += (new_state->src.y1 >> 16) * pitch; + addr += (dma_addr_t)(new_state->src.x1 >> 16) * cpp; + addr += (dma_addr_t)(new_state->src.y1 >> 16) * pitch; } else { int width_in_blocks = ALIGN(fb->width, AFBC_DATA_BLOCK_WIDTH) / AFBC_DATA_BLOCK_WIDTH; @@ -167,17 +168,18 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state, hdr_pitch = width_in_blocks * AFBC_HEADER_BLOCK_SIZE; pitch = width_in_blocks * AFBC_DATA_BLOCK_WIDTH * - AFBC_DATA_BLOCK_HEIGHT * fb->format->cpp[0]; + AFBC_DATA_BLOCK_HEIGHT * cpp; hdr_size = ALIGN(hdr_pitch * height_in_blocks, AFBC_HEADER_ALIGNMENT); - hdr_addr = addr + hdr_pitch * y_offset_in_blocks + - AFBC_HEADER_BLOCK_SIZE * x_offset_in_blocks; + hdr_addr = addr + + (dma_addr_t)hdr_pitch * y_offset_in_blocks + + (dma_addr_t)AFBC_HEADER_BLOCK_SIZE * x_offset_in_blocks; /* The data plane is offset by 1 additional block. */ addr = addr + hdr_size + - pitch * y_offset_in_blocks + - AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT * - fb->format->cpp[0] * (x_offset_in_blocks + 1); + (dma_addr_t)pitch * y_offset_in_blocks + + (dma_addr_t)AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT * + (dma_addr_t)cpp * (x_offset_in_blocks + 1); } mtk_plane_state->pending.enable = true; -- 2.18.0
[PATCH 5/5] drm/mediatek: Fix dereference before null check
Null-checking state suggests that it may be null, but it has already been dereferenced on drm_atomic_get_new_plane_state(state, plane). The parameter state will never be NULL currently, so just remove the state is NULL flow in this function. Signed-off-by: Jason-JH.Lin Fixes: 5ddb0bd4ddc3 ("drm/atomic: Pass the full state to planes async atomic check and update") --- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index a1337f386bbf..e14b2920d242 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -103,8 +103,7 @@ static void mtk_drm_plane_destroy_state(struct drm_plane *plane, static int mtk_plane_atomic_async_check(struct drm_plane *plane, struct drm_atomic_state *state) { - struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, - plane); + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane); struct drm_crtc_state *crtc_state; int ret; @@ -122,11 +121,7 @@ static int mtk_plane_atomic_async_check(struct drm_plane *plane, if (ret) return ret; - if (state) - crtc_state = drm_atomic_get_existing_crtc_state(state, - new_plane_state->crtc); - else /* Special case for asynchronous cursor updates. */ - crtc_state = new_plane_state->crtc->state; + crtc_state = drm_atomic_get_existing_crtc_state(state, new_plane_state->crtc); return drm_atomic_helper_check_plane_state(plane->state, crtc_state, DRM_PLANE_NO_SCALING, -- 2.18.0
[PATCH 0/5] Fix mediatek-drm coverity issues
Add this patch series to fix these mediatek-drm coverity issues. Jason-JH.Lin (5): drm/mediatek: Remove freeing not dynamic allocated memory drm/mediatek: Add cnt checking for coverity issue drm/mediatek: Add initialization for mtk_gem_obj drm/mediatek: Add casting before assign drm/mediatek: Fix dereference before null check drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 ++-- drivers/gpu/drm/mediatek/mtk_drm_gem.c | 4 ++-- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 29 +++- 4 files changed, 17 insertions(+), 21 deletions(-) -- 2.18.0
[PATCH 1/5] drm/mediatek: Remove freeing not dynamic allocated memory
Fixing the coverity issue of: mtk_drm_cmdq_pkt_destroy frees address of mtk_crtc->cmdq_handle So remove the free function. Signed-off-by: Jason-JH.Lin Fixes: 7627122fd1c0 ("drm/mediatek: Add cmdq_handle in mtk_crtc") --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 4bc45cdbddf1..c7b03e564095 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -148,7 +148,6 @@ static void mtk_drm_cmdq_pkt_destroy(struct cmdq_pkt *pkt) dma_unmap_single(client->chan->mbox->dev, pkt->pa_base, pkt->buf_size, DMA_TO_DEVICE); kfree(pkt->va_base); - kfree(pkt); } #endif -- 2.18.0
[PATCH 3/5] drm/mediatek: Add initialization for mtk_gem_obj
Calling mtk_gem_obj = kzalloc() which returns uninitialized memory, because mtk_gem_obj is uninitialized. It may cause using uninitialized value mtk_gem_obj->base.resv when calling drm_gem_object_init(). So add initialization for mtk_gem_obj. Signed-off-by: Jason-JH.Lin Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.") --- drivers/gpu/drm/mediatek/mtk_drm_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c b/drivers/gpu/drm/mediatek/mtk_drm_gem.c index a25b28d3ee90..9b8f72ed12e4 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c @@ -33,7 +33,7 @@ static const struct drm_gem_object_funcs mtk_drm_gem_object_funcs = { static struct mtk_drm_gem_obj *mtk_drm_gem_init(struct drm_device *dev, unsigned long size) { - struct mtk_drm_gem_obj *mtk_gem_obj; + struct mtk_drm_gem_obj *mtk_gem_obj = NULL; int ret; size = round_up(size, PAGE_SIZE); -- 2.18.0
[PATCH 2/5] drm/mediatek: Add cnt checking for coverity issue
CERT-C Characters and Strings (CERT STR31-C) all_drm_priv[cnt] evaluates to an address that could be at negative offset of an array. In mtk_drm_get_all_drm_priv(): Guarantee that storage for strings has sufficient space for character data and the null terminator. So change cnt to unsigned int and check its max value. Signed-off-by: Jason-JH.Lin Fixes: 1ef7ed48356c ("drm/mediatek: Modify mediatek-drm for mt8195 multi mmsys support") --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 86255a066faf..fcfa10332166 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -378,7 +378,7 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev) const struct of_device_id *of_id; struct device_node *node; struct device *drm_dev; - int cnt = 0; + unsigned int cnt = 0; int i, j; for_each_child_of_node(phandle->parent, node) { @@ -397,7 +397,7 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev) continue; all_drm_priv[cnt] = dev_get_drvdata(drm_dev); - if (all_drm_priv[cnt] && all_drm_priv[cnt]->mtk_drm_bound) + if (cnt < MAX_CRTC && all_drm_priv[cnt] && all_drm_priv[cnt]->mtk_drm_bound) cnt++; } -- 2.18.0