Re: [PATCH v5] overflow: Introduce overflows_type() and castable_to_type()
Hi Kees, I've updated to v5 with the last comment of Nathan. Could you please kindly review what more is needed as we move forward with this patch? Br, G.G. On 10/24/22 11:11 PM, Gwan-gyeong Mun wrote: From: Kees Cook Implement a robust overflows_type() macro to test if a variable or constant value would overflow another variable or type. This can be used as a constant expression for static_assert() (which requires a constant expression[1][2]) when used on constant values. This must be constructed manually, since __builtin_add_overflow() does not produce a constant expression[3]. Additionally adds castable_to_type(), similar to __same_type(), but for checking if a constant value would overflow if cast to a given type. Add unit tests for overflows_type(), __same_type(), and castable_to_type() to the existing KUnit "overflow" test. [1] https://en.cppreference.com/w/c/language/_Static_assert [2] C11 standard (ISO/IEC 9899:2011): 6.7.10 Static assertions [3] https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html 6.56 Built-in Functions to Perform Arithmetic with Overflow Checking Built-in Function: bool __builtin_add_overflow (type1 a, type2 b, Cc: Luc Van Oostenryck Cc: Nathan Chancellor Cc: Nick Desaulniers Cc: Tom Rix Cc: Daniel Latypov Cc: Vitor Massaru Iha Cc: "Gustavo A. R. Silva" Cc: Jani Nikula Cc: Mauro Carvalho Chehab Cc: linux-harden...@vger.kernel.org Cc: l...@lists.linux.dev Co-developed-by: Gwan-gyeong Mun Signed-off-by: Gwan-gyeong Mun Signed-off-by: Kees Cook --- v5: drop the cc-disable-warning and just disable the warning directly (Nathan) v4: - move version v2 changelog commit message to under the --- marker (Mauro) - remove the #pragma addition in the code and modify the Makefile to handle the same feature (Jani) v3: - chagne to use uintptr_t type when checking for overflow of pointer type variable v2: - fix comment typo - wrap clang pragma to avoid GCC warnings - style nit cleanups - rename __castable_to_type() to castable_to_type() - remove prior overflows_type() definition v1: https://lore.kernel.org/lkml/20220926003743.409911-1-keesc...@chromium.org --- drivers/gpu/drm/i915/i915_user_extensions.c | 2 +- drivers/gpu/drm/i915/i915_utils.h | 4 - include/linux/compiler.h| 1 + include/linux/overflow.h| 48 +++ lib/Makefile| 4 + lib/overflow_kunit.c| 383 +++- 6 files changed, 436 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_user_extensions.c b/drivers/gpu/drm/i915/i915_user_extensions.c index c822d0aafd2d..e3f808372c47 100644 --- a/drivers/gpu/drm/i915/i915_user_extensions.c +++ b/drivers/gpu/drm/i915/i915_user_extensions.c @@ -51,7 +51,7 @@ int i915_user_extensions(struct i915_user_extension __user *ext, return err; if (get_user(next, >next_extension) || - overflows_type(next, ext)) + overflows_type(next, uintptr_t)) return -EFAULT; ext = u64_to_user_ptr(next); diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 6c14d13364bf..67a66d4d5c70 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -111,10 +111,6 @@ bool i915_error_injected(void); #define range_overflows_end_t(type, start, size, max) \ range_overflows_end((type)(start), (type)(size), (type)(max)) -/* Note we don't consider signbits :| */ -#define overflows_type(x, T) \ - (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T)) - #define ptr_mask_bits(ptr, n) ({ \ unsigned long __v = (unsigned long)(ptr); \ (typeof(ptr))(__v & -BIT(n)); \ diff --git a/include/linux/compiler.h b/include/linux/compiler.h index 973a1bfd7ef5..947a60b801db 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -236,6 +236,7 @@ static inline void *offset_to_ptr(const int *off) * bool and also pointer types. */ #define is_signed_type(type) (((type)(-1)) < (__force type)1) +#define is_unsigned_type(type) (!is_signed_type(type)) /* * This is needed in functions which generate the stack canary, see diff --git a/include/linux/overflow.h b/include/linux/overflow.h index 19dfdd74835e..58eb34aa2af9 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -127,6 +127,54 @@ static inline bool __must_check __must_check_overflow(bool overflow) (*_d >> _to_shift) != _a);\ })) +#define __overflows_type_constexpr(x, T) ( \ + is_unsigned_type(typeof(x)) ? \ + (x) > type_max(typeof(T)) ? 1 : 0\ + : is_unsigned_type(typeof(T)) ? \ +
Re: [PATCH v2] [next] drm/radeon: Replace one-element array with flexible-array member
On Sat, Oct 29, 2022 at 04:32:05PM +1300, Paulo Miguel Almeida wrote: > One-element arrays are deprecated, and we are replacing them with > flexible array members instead. So, replace one-element array with > flexible-array member in struct _ATOM_FAKE_EDID_PATCH_RECORD and > refactor the rest of the code accordingly. > > It's worth mentioning that doing a build before/after this patch results > in no binary output differences. Thanks for checking it! > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE > routines on memcpy() and help us make progress towards globally > enabling -fstrict-flex-arrays=3 [1]. > > Link: https://github.com/KSPP/linux/issues/79 > Link: https://github.com/KSPP/linux/issues/239 > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] > > Signed-off-by: Paulo Miguel Almeida Reviewed-by: Kees Cook -- Kees Cook
[PATCH v2] [next] drm/radeon: Replace one-element array with flexible-array member
One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element array with flexible-array member in struct _ATOM_FAKE_EDID_PATCH_RECORD and refactor the rest of the code accordingly. It's worth mentioning that doing a build before/after this patch results in no binary output differences. This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1]. Link: https://github.com/KSPP/linux/issues/79 Link: https://github.com/KSPP/linux/issues/239 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] Signed-off-by: Paulo Miguel Almeida --- Changelog: v2: no binary output differences patch; report binary changes findings on commit log. Res: Kees Cook. This request was made in an identical, yet different, patch but the same feedback applies. https://lore.kernel.org/lkml/y1x3mtrj8ckxx...@mail.google.com/ v1: https://lore.kernel.org/lkml/y1trhre3nk5ia...@mail.google.com/ --- drivers/gpu/drm/radeon/atombios.h| 2 +- drivers/gpu/drm/radeon/radeon_atombios.c | 7 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/atombios.h b/drivers/gpu/drm/radeon/atombios.h index da35a970fcc0..235e59b547a1 100644 --- a/drivers/gpu/drm/radeon/atombios.h +++ b/drivers/gpu/drm/radeon/atombios.h @@ -3615,7 +3615,7 @@ typedef struct _ATOM_FAKE_EDID_PATCH_RECORD { UCHAR ucRecordType; UCHAR ucFakeEDIDLength; - UCHAR ucFakeEDIDString[1];// This actually has ucFakeEdidLength elements. + UCHAR ucFakeEDIDString[];// This actually has ucFakeEdidLength elements. } ATOM_FAKE_EDID_PATCH_RECORD; typedef struct _ATOM_PANEL_RESOLUTION_PATCH_RECORD diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c index 204127bad89c..4ad5a328d920 100644 --- a/drivers/gpu/drm/radeon/radeon_atombios.c +++ b/drivers/gpu/drm/radeon/radeon_atombios.c @@ -1727,8 +1727,11 @@ struct radeon_encoder_atom_dig *radeon_atombios_get_lvds_info(struct } } record += fake_edid_record->ucFakeEDIDLength ? - fake_edid_record->ucFakeEDIDLength + 2 : - sizeof(ATOM_FAKE_EDID_PATCH_RECORD); + struct_size(fake_edid_record, + ucFakeEDIDString, + fake_edid_record->ucFakeEDIDLength) : + /* empty fake edid record must be 3 bytes long */ + sizeof(ATOM_FAKE_EDID_PATCH_RECORD) + 1; break; case LCD_PANEL_RESOLUTION_RECORD_TYPE: panel_res_record = (ATOM_PANEL_RESOLUTION_PATCH_RECORD *)record; -- 2.37.3
[PATCH] drm/amd/display: fix an incorrect comment
This is a copy-and-paste error. Fix the comment to match the macro definition. Signed-off-by: Alex Hung --- drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h index 63219ecd8478..1bf6b12f5663 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h @@ -29,4 +29,4 @@ void dcn10_resource_construct_fp(struct dc *dc); -#endif /* __DCN20_FPU_H__ */ +#endif /* __DCN10_FPU_H__ */ -- 2.38.1
[PATCH -next] drm: xlnx: zynqmp_dpsub: fix missing clk_disable_unprepare() in error path in zynqmp_dpsub_init_clocks()
If get video or audio clock failed, it should call clk_disable_unprepare() in error path. Fixes: c979296ef60c ("drm: xlnx: zynqmp_dpsub: Move audio clk from zynqmp_disp to zynqmp_dpsub") Fixes: 1682ade66308 ("drm: xlnx: zynqmp_dpsub: Move pclk from zynqmp_disp to zynqmp_dpsub") Signed-off-by: Yang Yingliang --- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c index bab862484d42..50abe222fcc3 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c @@ -109,16 +109,19 @@ static int zynqmp_dpsub_init_clocks(struct zynqmp_dpsub *dpsub) * live PL video clock isn't valid. */ dpsub->vid_clk = devm_clk_get(dpsub->dev, "dp_live_video_in_clk"); - if (!IS_ERR(dpsub->vid_clk)) + if (!IS_ERR(dpsub->vid_clk)) { dpsub->vid_clk_from_ps = false; - else if (PTR_ERR(dpsub->vid_clk) == -EPROBE_DEFER) - return PTR_ERR(dpsub->vid_clk); + } else if (PTR_ERR(dpsub->vid_clk) == -EPROBE_DEFER) { + ret = PTR_ERR(dpsub->vid_clk); + goto err_disable_clk; + } if (IS_ERR_OR_NULL(dpsub->vid_clk)) { dpsub->vid_clk = devm_clk_get(dpsub->dev, "dp_vtc_pixel_clk_in"); if (IS_ERR(dpsub->vid_clk)) { dev_err(dpsub->dev, "failed to init any video clock\n"); - return PTR_ERR(dpsub->vid_clk); + ret = PTR_ERR(dpsub->vid_clk); + goto err_disable_clk; } dpsub->vid_clk_from_ps = true; } @@ -142,6 +145,11 @@ static int zynqmp_dpsub_init_clocks(struct zynqmp_dpsub *dpsub) dev_info(dpsub->dev, "audio disabled due to missing clock\n"); return 0; + +err_disable_clk: + clk_disable_unprepare(dpsub->apb_clk); + + return ret; } static int zynqmp_dpsub_parse_dt(struct zynqmp_dpsub *dpsub) -- 2.25.1
Re: [PATCH V4 2/3] dt-bindings: display: panel: Add NewVision NV3051D bindings
On Fri, Oct 28, 2022 at 07:01:12PM -0400, Krzysztof Kozlowski wrote: > On 28/10/2022 16:50, Chris Morgan wrote: > > From: Chris Morgan > > > > Add documentation for the NewVision NV3051D panel bindings. > > Note that for the two expected consumers of this panel binding > > the underlying LCD model is unknown. Name "anbernic,rg353p-panel" > > is used because the hardware itself is known as "anbernic,rg353p". > > > > Signed-off-by: Chris Morgan > > Didn't you got here tag? Yes, I'm so sorry. I always seem to miss one detail each time, I promise I'll get better (eventually, I hope). This one should already have the "Reviewed-by: Rob Herring " but I forgot to include it. Literally the only change from v3 is the return of a function changing from int to void, since that changed in the 6.1 kernel. Thank you. > > Best regards, > Krzysztof >
[PATCH v2] [next] drm/amdgpu: Replace one-element array with flexible-array member
One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element array with flexible-array member in struct _ATOM_FAKE_EDID_PATCH_RECORD and refactor the rest of the code accordingly. Important to mention is that doing a build before/after this patch results in no binary output differences. This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1]. Link: https://github.com/KSPP/linux/issues/79 Link: https://github.com/KSPP/linux/issues/238 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] Signed-off-by: Paulo Miguel Almeida --- Changelog: v2: no binary output differences patch; report binary changes findings on commit log. Res: Kees Cook v1: https://lore.kernel.org/lkml/y1tkwdwpup+ud...@mail.google.com/ --- drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 7 +-- drivers/gpu/drm/amd/include/atombios.h | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c index 6be9ac2b9c5b..18ae9433e463 100644 --- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c +++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c @@ -2081,8 +2081,11 @@ amdgpu_atombios_encoder_get_lcd_info(struct amdgpu_encoder *encoder) } } record += fake_edid_record->ucFakeEDIDLength ? - fake_edid_record->ucFakeEDIDLength + 2 : - sizeof(ATOM_FAKE_EDID_PATCH_RECORD); + struct_size(fake_edid_record, + ucFakeEDIDString, + fake_edid_record->ucFakeEDIDLength) : + /* empty fake edid record must be 3 bytes long */ + sizeof(ATOM_FAKE_EDID_PATCH_RECORD) + 1; break; case LCD_PANEL_RESOLUTION_RECORD_TYPE: panel_res_record = (ATOM_PANEL_RESOLUTION_PATCH_RECORD *)record; diff --git a/drivers/gpu/drm/amd/include/atombios.h b/drivers/gpu/drm/amd/include/atombios.h index 15943bc21bc5..b5b1d073f8e2 100644 --- a/drivers/gpu/drm/amd/include/atombios.h +++ b/drivers/gpu/drm/amd/include/atombios.h @@ -4107,7 +4107,7 @@ typedef struct _ATOM_FAKE_EDID_PATCH_RECORD { UCHAR ucRecordType; UCHAR ucFakeEDIDLength; // = 128 means EDID length is 128 bytes, otherwise the EDID length = ucFakeEDIDLength*128 - UCHAR ucFakeEDIDString[1];// This actually has ucFakeEdidLength elements. + UCHAR ucFakeEDIDString[]; // This actually has ucFakeEdidLength elements. } ATOM_FAKE_EDID_PATCH_RECORD; typedef struct _ATOM_PANEL_RESOLUTION_PATCH_RECORD -- 2.37.3
Re: [PATCH] [next] drm/amdgpu: Replace one-element array with flexible-array member
On Fri, Oct 28, 2022 at 07:33:17PM +0200, Christian König wrote: > Am 28.10.22 um 18:36 schrieb Kees Cook: > > > All that said, converting away from them can be tricky, and I think such > > conversions need to explicitly show how they were checked for binary > > differences[2]. > > Oh, that's a great idea! Yes, if this can be guaranteed then the change is > obviously perfectly ok. > > > > > Paulo, can you please check for deltas and report your findings in the > > commit log? Note that add struct_size() use in the same patch may result > > in binary differences, so for more complex cases, you may want to split > > the 1-element conversion from the struct_size() conversions. > > > > -Kees Noted. I will reporting my findings on commit logs from now onwards. Given that I split the if-ternary to avoid checking "fake_edid_record->ucFakeEDIDLength" twice then (for the current patch), yes, there will be changes to *.o|ko files. Knowing that Christian would feel more confident with no binary changes at this point, I will send a different patch aiming solely on the replacement of 1-elem array without incurring binary changes. -- Paulo A.
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add missing steering table terminators
On Fri, Oct 28, 2022 at 04:04:18PM -0700, Lucas De Marchi wrote: > On Fri, Oct 28, 2022 at 03:40:22PM -0700, Matt Roper wrote: > > The termination entries were missing for a couple of the recently-added > > MTL steering tables. > > > > Fixes: f32898c94a10 ("drm/i915/xelpg: Add multicast steering") > > Fixes: a7ec65fc7e83 ("drm/i915/xelpmp: Add multicast steering for media GT") > > I was thinking if we would need separate commits so they can be > backported independently, but no... those commits were very close. > > > Signed-off-by: Matt Roper > > > Reviewed-by: Lucas De Marchi Thanks for the review. Applied to drm-intel-gt-next. Matt > > Lucas De Marchi > > > --- > > drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > > b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > > index 46cf2f3d1e8e..830edffe88cc 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > > @@ -128,11 +128,13 @@ static const struct intel_mmio_range > > xelpg_dss_steering_table[] = { > > { 0x00D800, 0x00D87F }, /* SLICE */ > > { 0x00DC00, 0x00DCFF }, /* SLICE */ > > { 0x00DE80, 0x00E8FF }, /* DSS (0xE000-0xE0FF reserved) */ > > + {}, > > }; > > > > static const struct intel_mmio_range xelpmp_oaddrm_steering_table[] = { > > { 0x393200, 0x39323F }, > > { 0x393400, 0x3934FF }, > > + {}, > > }; > > > > void intel_gt_mcr_init(struct intel_gt *gt) > > -- > > 2.37.3 > > -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation
Re: [PATCH 00/10] Connect VFIO to IOMMUFD
On Fri, Oct 28, 2022 at 04:53:21PM -0700, Nicolin Chen wrote: > On Tue, Oct 25, 2022 at 03:17:06PM -0300, Jason Gunthorpe wrote: > > This series provides an alternative container layer for VFIO implemented > > using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will > > not be compiled in. > > > > At this point iommufd can be injected by passing in a iommfd FD to > > VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd > > to obtain the compat IOAS and then connect up all the VFIO drivers as > > appropriate. > > > > This is temporary stopping point, a following series will provide a way to > > directly open a VFIO device FD and directly connect it to IOMMUFD using > > native ioctls that can expose the IOMMUFD features like hwpt, future > > vPASID and dynamic attachment. > > > > This series, in compat mode, has passed all the qemu tests we have > > available, including the test suites for the Intel GVT mdev. Aside from > > the temporary limitation with P2P memory this is belived to be fully > > compatible with VFIO. > > > > This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd > > Tested-by: Nicolin Chen Sorry, wrong email -- should be: Tested-by: Nicolin Chen > Tested this branch on ARM64+SMMUv3 with the iommufd selftest and > QEMU passthrough sanity using noiommu and virtio-iommu setups by > combining with both CONFIG_VFIO_CONTAINER=y and =n.
Re: [PATCH 00/10] Connect VFIO to IOMMUFD
On Tue, Oct 25, 2022 at 03:17:06PM -0300, Jason Gunthorpe wrote: > This series provides an alternative container layer for VFIO implemented > using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will > not be compiled in. > > At this point iommufd can be injected by passing in a iommfd FD to > VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd > to obtain the compat IOAS and then connect up all the VFIO drivers as > appropriate. > > This is temporary stopping point, a following series will provide a way to > directly open a VFIO device FD and directly connect it to IOMMUFD using > native ioctls that can expose the IOMMUFD features like hwpt, future > vPASID and dynamic attachment. > > This series, in compat mode, has passed all the qemu tests we have > available, including the test suites for the Intel GVT mdev. Aside from > the temporary limitation with P2P memory this is belived to be fully > compatible with VFIO. > > This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd Tested-by: Nicolin Chen Tested this branch on ARM64+SMMUv3 with the iommufd selftest and QEMU passthrough sanity using noiommu and virtio-iommu setups by combining with both CONFIG_VFIO_CONTAINER=y and =n.
Re: [Intel-gfx] [PULL] drm-intel-next
On Fri, Oct 28, 2022 at 02:22:33PM -0400, Rodrigo Vivi wrote: > Hi Dave and Daniel, > > Here goes the first chunk of drm-intel-next targeting 6.2 > > The highlight goes to Ville with many display related clean-up > and improvement, some other MTL enabling work and many other > fixes and small clean-ups. > > drm-intel-next-2022-10-28: ... > - ELD precompute and readout (Ville) A slight clarification seems to be in order. The ELD precompute+readout is in fact not in yet. This was just a pile of cleanups and minor fixes. The real ELD stuff will come later, once we figure out how we actually want to do it. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add missing steering table terminators
On Fri, Oct 28, 2022 at 03:40:22PM -0700, Matt Roper wrote: The termination entries were missing for a couple of the recently-added MTL steering tables. Fixes: f32898c94a10 ("drm/i915/xelpg: Add multicast steering") Fixes: a7ec65fc7e83 ("drm/i915/xelpmp: Add multicast steering for media GT") I was thinking if we would need separate commits so they can be backported independently, but no... those commits were very close. Signed-off-by: Matt Roper Reviewed-by: Lucas De Marchi Lucas De Marchi --- drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c index 46cf2f3d1e8e..830edffe88cc 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c @@ -128,11 +128,13 @@ static const struct intel_mmio_range xelpg_dss_steering_table[] = { { 0x00D800, 0x00D87F }, /* SLICE */ { 0x00DC00, 0x00DCFF }, /* SLICE */ { 0x00DE80, 0x00E8FF }, /* DSS (0xE000-0xE0FF reserved) */ + {}, }; static const struct intel_mmio_range xelpmp_oaddrm_steering_table[] = { { 0x393200, 0x39323F }, { 0x393400, 0x3934FF }, + {}, }; void intel_gt_mcr_init(struct intel_gt *gt) -- 2.37.3
Re: [PATCH V4 2/3] dt-bindings: display: panel: Add NewVision NV3051D bindings
On 28/10/2022 16:50, Chris Morgan wrote: > From: Chris Morgan > > Add documentation for the NewVision NV3051D panel bindings. > Note that for the two expected consumers of this panel binding > the underlying LCD model is unknown. Name "anbernic,rg353p-panel" > is used because the hardware itself is known as "anbernic,rg353p". > > Signed-off-by: Chris Morgan Didn't you got here tag? Best regards, Krzysztof
[RFC PATCH 3/3] drm/msm/dpu: Use color_fill property for DPU planes
Initialize and use the color_fill properties for planes in DPU driver. In addition, relax framebuffer requirements within atomic commit path and add checks for NULL framebuffers. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 7 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 66 ++- 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 13ce321283ff..157698b4f234 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -441,7 +441,12 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, sspp_idx - SSPP_VIG0, state->fb ? state->fb->base.id : -1); - format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); + if (pstate->base.fb) + format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); + else if (state->color_fill && !state->color_fill_format) + format = dpu_get_dpu_format(DRM_FORMAT_ABGR); + else + format = dpu_get_dpu_format(state->color_fill_format); if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) bg_alpha_enable = true; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 658005f609f4..f3be37e97b64 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -103,7 +103,6 @@ struct dpu_plane { enum dpu_sspp pipe; struct dpu_hw_pipe *pipe_hw; - uint32_t color_fill; bool is_error; bool is_rt_pipe; const struct dpu_mdss_cfg *catalog; @@ -697,7 +696,10 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu, * select fill format to match user property expectation, * h/w only supports RGB variants */ - fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR); + if (plane->state->color_fill && !plane->state->color_fill_format) + fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR); + else + fmt = dpu_get_dpu_format(plane->state->color_fill_format); /* update sspp */ if (fmt && pdpu->pipe_hw->ops.setup_solidfill) { @@ -720,6 +722,10 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu, fmt, DPU_SSPP_SOLID_FILL, pstate->multirect_index); + /* skip remaining processing on color fill */ + if (!plane->state->fb) + return 0; + if (pdpu->pipe_hw->ops.setup_rects) pdpu->pipe_hw->ops.setup_rects(pdpu->pipe_hw, _cfg, @@ -999,12 +1005,21 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, dst = drm_plane_state_dest(new_plane_state); - fb_rect.x2 = new_plane_state->fb->width; - fb_rect.y2 = new_plane_state->fb->height; + if (new_plane_state->fb) { + fb_rect.x2 = new_plane_state->fb->width; + fb_rect.y2 = new_plane_state->fb->height; + } max_linewidth = pdpu->catalog->caps->max_linewidth; - fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); + if (new_plane_state->fb) { + fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb)); + } else if (new_plane_state->color_fill) { + if (new_plane_state->color_fill_format) + fmt = dpu_get_dpu_format(new_plane_state->color_fill_format); + else + fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR); + } min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1; @@ -1016,7 +1031,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, return -EINVAL; /* check src bounds */ - } else if (!dpu_plane_validate_src(, _rect, min_src_size)) { + } else if (new_plane_state->fb && !dpu_plane_validate_src(, _rect, min_src_size)) { DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n", DRM_RECT_ARG()); return -E2BIG; @@ -1084,9 +1099,9 @@ void dpu_plane_flush(struct drm_plane *plane) if (pdpu->is_error) /* force white frame with 100% alpha pipe output on error */ _dpu_plane_color_fill(pdpu, 0xFF, 0xFF); - else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) + else if (!(plane->state->fb) && plane->state->color_fill) /* force 100% alpha */ - _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); + _dpu_plane_color_fill(pdpu, plane->state->color_fill, 0xFF); else if
[RFC PATCH 2/3] drm: Adjust atomic checks for solid fill color
Loosen the requirements for atomic and legacy commit so that, in cases where solid fill planes is enabled (and FB_ID is NULL), the commit can still go through. In addition, add framebuffer NULL checks in other areas to account for FB being NULL when solid fill is enabled. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/drm_atomic.c| 68 - drivers/gpu/drm/drm_atomic_helper.c | 34 +-- drivers/gpu/drm/drm_plane.c | 8 ++-- include/drm/drm_atomic_helper.h | 5 ++- 4 files changed, 64 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index f197f59f6d99..b576ed221431 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -601,8 +601,10 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state, uint32_t num_clips; int ret; - /* either *both* CRTC and FB must be set, or neither */ - if (crtc && !fb) { + /* When color_fill is disabled, +* either *both* CRTC and FB must be set, or neither +*/ + if (crtc && !fb && !new_plane_state->color_fill) { drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no FB\n", plane->base.id, plane->name); return -EINVAL; @@ -626,14 +628,16 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state, } /* Check whether this plane supports the fb pixel format. */ - ret = drm_plane_check_pixel_format(plane, fb->format->format, - fb->modifier); - if (ret) { - drm_dbg_atomic(plane->dev, - "[PLANE:%d:%s] invalid pixel format %p4cc, modifier 0x%llx\n", - plane->base.id, plane->name, - >format->format, fb->modifier); - return ret; + if (fb) { + ret = drm_plane_check_pixel_format(plane, fb->format->format, + fb->modifier); + + if (ret) + drm_dbg_atomic(plane->dev, + "[PLANE:%d:%s] invalid pixel format %p4cc, modifier 0x%llx\n", + plane->base.id, plane->name, + >format->format, fb->modifier); + return ret; } /* Give drivers some help against integer overflows */ @@ -649,28 +653,30 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state, return -ERANGE; } - fb_width = fb->width << 16; - fb_height = fb->height << 16; + if (fb) { + fb_width = fb->width << 16; + fb_height = fb->height << 16; - /* Make sure source coordinates are inside the fb. */ - if (new_plane_state->src_w > fb_width || - new_plane_state->src_x > fb_width - new_plane_state->src_w || - new_plane_state->src_h > fb_height || - new_plane_state->src_y > fb_height - new_plane_state->src_h) { - drm_dbg_atomic(plane->dev, - "[PLANE:%d:%s] invalid source coordinates " - "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n", - plane->base.id, plane->name, - new_plane_state->src_w >> 16, - ((new_plane_state->src_w & 0x) * 15625) >> 10, - new_plane_state->src_h >> 16, - ((new_plane_state->src_h & 0x) * 15625) >> 10, - new_plane_state->src_x >> 16, - ((new_plane_state->src_x & 0x) * 15625) >> 10, - new_plane_state->src_y >> 16, - ((new_plane_state->src_y & 0x) * 15625) >> 10, - fb->width, fb->height); - return -ENOSPC; + /* Make sure source coordinates are inside the fb. */ + if (new_plane_state->src_w > fb_width || + new_plane_state->src_x > fb_width - new_plane_state->src_w || + new_plane_state->src_h > fb_height || + new_plane_state->src_y > fb_height - new_plane_state->src_h) { + drm_dbg_atomic(plane->dev, + "[PLANE:%d:%s] invalid source coordinates " + "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n", + plane->base.id, plane->name, + new_plane_state->src_w >> 16, + ((new_plane_state->src_w & 0x) * 15625) >> 10, + new_plane_state->src_h >>
[RFC PATCH 1/3] drm: Introduce color fill properties for drm plane
Add support for COLOR_FILL and COLOR_FILL_FORMAT properties for drm_plane. In addition, add support for setting and getting the values of these properties. COLOR_FILL represents the color fill of a plane while COLOR_FILL_FORMAT represents the format of the color fill. Userspace can set enable solid fill on a plane by assigning COLOR_FILL to a uint64_t value, assigning the COLOR_FILL_FORMAT property to a uint32_t value, and setting the framebuffer to NULL. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/drm_atomic_uapi.c | 8 +++ drivers/gpu/drm/drm_blend.c | 38 +++ include/drm/drm_blend.h | 2 ++ include/drm/drm_plane.h | 28 +++ 4 files changed, 76 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 79730fa1dd8e..e1664463fca4 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -544,6 +544,10 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, state->src_w = val; } else if (property == config->prop_src_h) { state->src_h = val; + } else if (property == plane->color_fill_format_property) { + state->color_fill_format = val; + } else if (property == plane->color_fill_property) { + state->color_fill = val; } else if (property == plane->alpha_property) { state->alpha = val; } else if (property == plane->blend_mode_property) { @@ -616,6 +620,10 @@ drm_atomic_plane_get_property(struct drm_plane *plane, *val = state->src_w; } else if (property == config->prop_src_h) { *val = state->src_h; + } else if (property == plane->color_fill_format_property) { + *val = state->color_fill_format; + } else if (property == plane->color_fill_property) { + *val = state->color_fill; } else if (property == plane->alpha_property) { *val = state->alpha; } else if (property == plane->blend_mode_property) { diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index b4c8cab7158c..b8c2b263fa51 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -616,3 +616,41 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane, return 0; } EXPORT_SYMBOL(drm_plane_create_blend_mode_property); + +int drm_plane_create_color_fill_property(struct drm_plane *plane) +{ + struct drm_property *prop; + + prop = drm_property_create_range(plane->dev, 0, "color_fill", +0, 0x); + if (!prop) + return -ENOMEM; + + drm_object_attach_property(>base, prop, 0); + plane->color_fill_property = prop; + + if (plane->state) + plane->state->color_fill = 0; + + return 0; +} +EXPORT_SYMBOL(drm_plane_create_color_fill_property); + +int drm_plane_create_color_fill_format_property(struct drm_plane *plane) +{ + struct drm_property *prop; + + prop = drm_property_create_range(plane->dev, 0, "color_fill_format", +0, 0x); + if (!prop) + return -ENOMEM; + + drm_object_attach_property(>base, prop, 0); + plane->color_fill_format_property = prop; + + if (plane->state) + plane->state->color_fill_format = 0; + + return 0; +} +EXPORT_SYMBOL(drm_plane_create_color_fill_format_property); diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h index 88bdfec3bd88..3e96f5e83cce 100644 --- a/include/drm/drm_blend.h +++ b/include/drm/drm_blend.h @@ -58,4 +58,6 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, struct drm_atomic_state *state); int drm_plane_create_blend_mode_property(struct drm_plane *plane, unsigned int supported_modes); +int drm_plane_create_color_fill_property(struct drm_plane *plane); +int drm_plane_create_color_fill_format_property(struct drm_plane *plane); #endif diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 89ea54652e87..dcbfdb0e1f71 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -116,6 +116,20 @@ struct drm_plane_state { /** @src_h: height of visible portion of plane (in 16.16) */ uint32_t src_h, src_w; + /** +* @color_fill_format: +* Format of the color fill value. +*/ + uint32_t color_fill_format; + + /** +* @color_fill: +* Fill color of the plane with 0 as black and 0x as white. +* Can be set by user by setting the COLOR_FILL property. See +* drm_plane_create_color_fill_property() for more details. +*/ + uint32_t color_fill; + /** * @alpha: * Opacity of the plane with 0 as completely transparent and
[RFC PATCH 0/3] Support for Solid Fill Planes
Introduce and add support for COLOR_FILL and COLOR_FILL_FORMAT properties. When the color fill value is set, and the framebuffer is set to NULL, memory fetch will be disabled. In addition, loosen the NULL FB checks within the atomic commit callstack to allow a NULL FB when color_fill is nonzero and add FB checks in methods where the FB was previously assumed to be non-NULL. Finally, have the DPU driver use drm_plane_state.color_fill and drm_plane_state.color_fill_format instead of dpu_plane_state.color_fill, and add extra checks in the DPU atomic commit callstack to account for a NULL FB in cases where color_fill is set. Some drivers support hardware that have optimizations for solid fill planes. This series aims to expose these capabilities to userspace as some compositors have a solid fill flag (ex. SOLID_COLOR in the Android hardware composer HAL) that can be set by apps like the Android Gears app. Userspace can set the color_fill value by setting COLOR_FILL_FORMAT to a DRM format, setting COLOR_FILL to a color fill value, and setting the framebuffer to NULL. Jessica Zhang (3): drm: Introduce color fill properties for drm plane drm: Adjust atomic checks for solid fill color drm/msm/dpu: Use color_fill property for DPU planes drivers/gpu/drm/drm_atomic.c | 68 --- drivers/gpu/drm/drm_atomic_helper.c | 34 +++- drivers/gpu/drm/drm_atomic_uapi.c | 8 +++ drivers/gpu/drm/drm_blend.c | 38 + drivers/gpu/drm/drm_plane.c | 8 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 7 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 66 ++ include/drm/drm_atomic_helper.h | 5 +- include/drm/drm_blend.h | 2 + include/drm/drm_plane.h | 28 ++ 10 files changed, 188 insertions(+), 76 deletions(-) -- 2.38.0
[PATCH 2/2] drm/amdgpu: Set PROBE_PREFER_ASYNCHRONOUS
This driver often takes over 200ms to start, so it can improve boot speed to probe it asynchronously. I did a short review of the driver, and apart from an issue fixed in the parent patch ("drm/amdgpu: Move racy global PMU list into device"), there don't appear to be many cross-device dependencies or racy accesses to global state, so this should be safe. This driver was pinpointed as part of a survey of top slowest initcalls (i.e., are built in, and probing synchronously) on a lab of ChromeOS systems. Signed-off-by: Brian Norris --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 3c9fecdd6b2f..2d180e48df1b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2793,7 +2793,10 @@ static struct pci_driver amdgpu_kms_pci_driver = { .probe = amdgpu_pci_probe, .remove = amdgpu_pci_remove, .shutdown = amdgpu_pci_shutdown, - .driver.pm = _pm_ops, + .driver = { + .pm = _pm_ops, + .probe_type = PROBE_PREFER_ASYNCHRONOUS, + }, .err_handler = _pci_err_handler, .dev_groups = amdgpu_sysfs_groups, }; -- 2.38.1.273.g43a17bfeac-goog
[PATCH 1/2] drm/amdgpu: Move racy global PMU list into device
If there are multiple amdgpu devices, this list processing can be racy. We're really treating this like a per-device list, so make that explicit and remove the global list. Signed-off-by: Brian Norris --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 12 +--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 0e6ddf05c23c..e968b7f2417c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1063,6 +1063,10 @@ struct amdgpu_device { struct work_struct reset_work; booljob_hang; + +#if IS_ENABLED(CONFIG_PERF_EVENTS) + struct list_head pmu_list; +#endif }; static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c index 71ee361d0972..24f2055a2f23 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c @@ -23,6 +23,7 @@ #include #include +#include #include "amdgpu.h" #include "amdgpu_pmu.h" @@ -72,9 +73,6 @@ static ssize_t amdgpu_pmu_event_show(struct device *dev, amdgpu_pmu_attr->event_str, amdgpu_pmu_attr->type); } -static LIST_HEAD(amdgpu_pmu_list); - - struct amdgpu_pmu_attr { const char *name; const char *config; @@ -558,7 +556,7 @@ static int init_pmu_entry_by_type_and_add(struct amdgpu_pmu_entry *pmu_entry, pr_info("Detected AMDGPU %d Perf Events.\n", total_num_events); - list_add_tail(_entry->entry, _pmu_list); + list_add_tail(_entry->entry, _entry->adev->pmu_list); return 0; err_register: @@ -579,9 +577,7 @@ void amdgpu_pmu_fini(struct amdgpu_device *adev) { struct amdgpu_pmu_entry *pe, *temp; - list_for_each_entry_safe(pe, temp, _pmu_list, entry) { - if (pe->adev != adev) - continue; + list_for_each_entry_safe(pe, temp, >pmu_list, entry) { list_del(>entry); perf_pmu_unregister(>pmu); kfree(pe->pmu.attr_groups); @@ -623,6 +619,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) int ret = 0; struct amdgpu_pmu_entry *pmu_entry, *pmu_entry_df; + INIT_LIST_HEAD(>pmu_list); + switch (adev->asic_type) { case CHIP_VEGA20: pmu_entry_df = create_pmu_entry(adev, AMDGPU_PMU_PERF_TYPE_DF, -- 2.38.1.273.g43a17bfeac-goog
[PATCH] drm/i915/mtl: Add missing steering table terminators
The termination entries were missing for a couple of the recently-added MTL steering tables. Fixes: f32898c94a10 ("drm/i915/xelpg: Add multicast steering") Fixes: a7ec65fc7e83 ("drm/i915/xelpmp: Add multicast steering for media GT") Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c index 46cf2f3d1e8e..830edffe88cc 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c @@ -128,11 +128,13 @@ static const struct intel_mmio_range xelpg_dss_steering_table[] = { { 0x00D800, 0x00D87F }, /* SLICE */ { 0x00DC00, 0x00DCFF }, /* SLICE */ { 0x00DE80, 0x00E8FF }, /* DSS (0xE000-0xE0FF reserved) */ + {}, }; static const struct intel_mmio_range xelpmp_oaddrm_steering_table[] = { { 0x393200, 0x39323F }, { 0x393400, 0x3934FF }, + {}, }; void intel_gt_mcr_init(struct intel_gt *gt) -- 2.37.3
[PATCH v5] drm/tests: Add back seed value information
As reported by Michał the drm_mm and drm_buddy unit tests lost the printk with seed value after they were refactored into KUnit. Add kunit_info with seed value information to assure reproducibility. Reported-by: Michał Winiarski Signed-off-by: Arthur Grillo --- v1->v2: https://lore.kernel.org/all/20221026211458.68432-1-arthurgri...@riseup.net/ - Correct compilation issues - Change tags order - Remove useless line change - Write commit message in imperative form - Remove redundant message part - Correct some grammars nits - Correct checkpatch issues v2->v3: https://lore.kernel.org/all/20221027142903.200169-1-arthurgri...@riseup.net/ - Change .init to .suite_init - Correct some grammars nits v3->v4: https://lore.kernel.org/all/20221028141246.280079-1-arthurgri...@riseup.net/ - Correct compilation issues v4->v5: https://lore.kernel.org/all/20221028141715.290903-1-arthurgri...@riseup.net/ - Change functions names from init_suite to suite_init --- drivers/gpu/drm/tests/drm_buddy_test.c | 6 -- drivers/gpu/drm/tests/drm_mm_test.c| 8 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c index 62f69589a72d..f8ee714df396 100644 --- a/drivers/gpu/drm/tests/drm_buddy_test.c +++ b/drivers/gpu/drm/tests/drm_buddy_test.c @@ -726,11 +726,13 @@ static void drm_test_buddy_alloc_limit(struct kunit *test) drm_buddy_fini(); } -static int drm_buddy_init_test(struct kunit *test) +static int drm_buddy_suite_init(struct kunit_suite *suite) { while (!random_seed) random_seed = get_random_u32(); + kunit_info(suite, "Testing DRM buddy manager, with random_seed=0x%x\n", random_seed); + return 0; } @@ -746,7 +748,7 @@ static struct kunit_case drm_buddy_tests[] = { static struct kunit_suite drm_buddy_test_suite = { .name = "drm_buddy", - .init = drm_buddy_init_test, + .suite_init = drm_buddy_suite_init, .test_cases = drm_buddy_tests, }; diff --git a/drivers/gpu/drm/tests/drm_mm_test.c b/drivers/gpu/drm/tests/drm_mm_test.c index c4b66eeae203..89f12d3b4a21 100644 --- a/drivers/gpu/drm/tests/drm_mm_test.c +++ b/drivers/gpu/drm/tests/drm_mm_test.c @@ -2209,11 +2209,15 @@ static void drm_test_mm_color_evict_range(struct kunit *test) vfree(nodes); } -static int drm_mm_init_test(struct kunit *test) +static int drm_mm_suite_init(struct kunit_suite *suite) { while (!random_seed) random_seed = get_random_u32(); + kunit_info(suite, + "Testing DRM range manager, with random_seed=0x%x max_iterations=%u max_prime=%u\n", + random_seed, max_iterations, max_prime); + return 0; } @@ -2246,7 +2250,7 @@ static struct kunit_case drm_mm_tests[] = { static struct kunit_suite drm_mm_test_suite = { .name = "drm_mm", - .init = drm_mm_init_test, + .suite_init = drm_mm_suite_init, .test_cases = drm_mm_tests, }; -- 2.37.3
Re: [PATCH v4 00/10] drm/msm: Fix math issues in MSM DSC implementation
On 10/28/2022 1:09 PM, Marijn Suijten wrote: Hi Abhinav, On 2022-10-28 11:33:21, Abhinav Kumar wrote: Hi Marijn On 10/26/2022 11:28 AM, Marijn Suijten wrote: Various removals of complex yet unnecessary math, fixing all uses of drm_dsc_config::bits_per_pixel to deal with the fact that this field includes four fractional bits, and finally making sure that range_bpg_offset contains values 6-bits wide to prevent overflows in drm_dsc_pps_payload_pack(). Altogether this series is responsible for solving _all_ Display Stream Compression issues and artifacts on the Sony Tama (sdm845) Akatsuki smartphone (2880x1440p). Changes since v3: - Swap patch 7 and 8 to make sure msm_host is available inside dsi_populate_dsc_params(); - Reword patch 6 (Migrate to drm_dsc_compute_rc_parameters()) to more clearly explain why the FIXME wasn't solved initially, but why it can (and should!) be resolved now. v3: https://lore.kernel.org/linux-arm-msm/20221009184824.457416-1-marijn.suij...@somainline.org/T/#u Changes since v2: - Generalize mux_word_size setting depending on bits_per_component; - Migrate DSI's DSC calculations to drm_dsc_compute_rc_parameters(), implicitly addressing existing math issues; - Disallow any bits_per_component other than 8, until hardcoded values are updated and tested to support such cases. v2: https://lore.kernel.org/linux-arm-msm/20221005181657.784375-1-marijn.suij...@somainline.org/T/#u Changes since v1: - Propagate r-b's, except (obviously) in patches that were (heavily) modified; - Remove accidental debug code in dsi_cmd_dma_add; - Move Range BPG Offset masking out of DCS PPS packing, back into the DSI driver when it is assigned to drm_dsc_config (this series is now strictly focusing on drm/msm again); - Replace modulo-check resulting in conditional increment with DIV_ROUND_UP; - Remove repeated calculation of slice_chunk_size; - Use u16 instead of int when handling bits_per_pixel; - Use DRM_DEV_ERROR instead of pr_err in DSI code; - Also remove redundant target_bpp_x16 variable. v1: https://lore.kernel.org/linux-arm-msm/20221001190807.358691-1-marijn.suij...@somainline.org/T/#u Marijn Suijten (10): drm/msm/dsi: Remove useless math in DSC calculations drm/msm/dsi: Remove repeated calculation of slice_per_intf drm/msm/dsi: Use DIV_ROUND_UP instead of conditional increment on modulo drm/msm/dsi: Reuse earlier computed dsc->slice_chunk_size drm/msm/dsi: Appropriately set dsc->mux_word_size based on bpc drm/msm/dsi: Migrate to drm_dsc_compute_rc_parameters() drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits drm/msm/dsi: Disallow 8 BPC DSC configuration for alternative BPC values drm/msm/dpu1: Account for DSC's bits_per_pixel having 4 fractional bits drm/msm/dsi: Prevent signed BPG offsets from bleeding into adjacent bits drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 11 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 121 ++--- 2 files changed, 37 insertions(+), 95 deletions(-) -- 2.38.1 To keep the -fixes cycle to have only critical fixes (others are important too but are cleanups), I was thinking of absorbing patches 7,8,9 and 10 alone in the -fixes cycle and for patches 1-6, will go through the 6.2 push. Let me know if there are any concerns if we just take patches 7,8,9 and 10 separately. Unfortunately that isn't going to cut it. For starters patch 8 is only introducing additional validation but as long as no panel drivers set bpc != 8, this doesn't change anything: it is not a critical fix. Then, more importantly, as discussed in v2 reviews it was preferred to _not_ fix the broken code in dsi_populate_dsc_params() but migrate to drm_dsc_compute_rc_parameters() directly [1]. As such patch 6 (which performs the migration) is definitely a requirement for the fixes to be complete. Then again this patch looks weird when 5 is not applied, since both refactor how dsc->mux_word_size is assigned. At the same time it cannot be cleanly applied without patch 1 (Remove useless math in DSC calculations) nor patch 3 (Use DIV_ROUND_UP instead of conditional increment on modulo), but I just realized that patch 3 is now also useless as the code is being removed altogether while migrating to drm_dsc_compute_rc_parameters(). Same for patch 4 (Reuse earlier computed dsc->slice_chunk_size): while it may not seem obvious at first, the original code uses bits_per_pixel without considering the fractional bits, again resulting invalid values. Perhaps this should have been mentioned in the patch description, but I did not want to create an even larger chain of references pointing back and forth to future patches fixing the exact same bug. Unfortunately this patch doesn't apply cleanly without patch 2 (Remove repeated calculation of slice_per_intf) either. All in all, applying this series piecemeal requires careful consideration which of the
Re: [PATCH v1 9/9] arm64: dts: qcom: sm8350-hdk: Enable lt9611uxc dsi-hdmi bridge
On 28/10/2022 08:08, Robert Foss wrote: > The sm8350-hdk ships with the LT9611 UXC DSI/HDMI bridge chip. > > In order to toggle the board to enable the HDMI output, > switch #7 & #8 on the rightmost multi-switch package have > to be toggled to On. > > Signed-off-by: Robert Foss > --- > arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 106 > 1 file changed, 106 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > index 6e07feb4b3b2..b38b58f8 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > @@ -20,6 +20,17 @@ chosen { > stdout-path = "serial0:115200n8"; > }; > > + hdmi-out { Generic node names, so hdmi-connector or just connector. > + compatible = "hdmi-connector"; > + type = "a"; > + > + port { > + hdmi_con: endpoint { > + remote-endpoint = <_out>; > + }; > + }; > + }; > + > vph_pwr: vph-pwr-regulator { > compatible = "regulator-fixed"; > regulator-name = "vph_pwr"; > @@ -29,6 +40,32 @@ vph_pwr: vph-pwr-regulator { > regulator-always-on; > regulator-boot-on; > }; > + > + lt9611_1v2: lt9611-1v2 { Node names should be generic. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "regulator-fixed"; > + regulator-name = "LT9611_1V2"; > + > + vin-supply = <_pwr>; > + regulator-min-microvolt = <120>; > + regulator-max-microvolt = <120>; > + gpio = < 49 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + lt9611_3v3: lt9611-3v3 { Ditto > + compatible = "regulator-fixed"; > + regulator-name = "LT9611_3V3"; > + > + vin-supply = <_bob>; > + gpio = < 47 GPIO_ACTIVE_HIGH>; > + regulator-min-microvolt = <330>; > + regulator-max-microvolt = <330>; > + enable-active-high; > + regulator-boot-on; > + regulator-always-on; > + }; > }; > > { > @@ -220,6 +257,15 @@ { > { > status = "okay"; > vdda-supply = <_l6b_1p2>; > + > + ports { > + port@1 { > + endpoint { > + remote-endpoint = <_a>; > + data-lanes = <0 1 2 3>; > + }; > + }; > + }; > }; > > _phy { > @@ -231,6 +277,48 @@ _dma1 { > status = "okay"; > }; > > + { > + status = "okay"; status is the last property > + clock-frequency = <40>; > + > + lt9611_codec: hdmi-bridge@2b { > + compatible = "lontium,lt9611uxc"; > + reg = <0x2b>; > + status = "okay"; Why status? > + > + interrupts-extended = < 50 IRQ_TYPE_EDGE_FALLING>; > + reset-gpios = < 48 GPIO_ACTIVE_HIGH>; > + > + vdd-supply = <_1v2>; > + vcc-supply = <_3v3>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <_irq_pin _rst_pin>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + lt9611_a: endpoint { > + remote-endpoint = <_out>; > + }; > + }; > + > + port@2 { > + reg = <2>; > + > + lt9611_out: endpoint { > + remote-endpoint = <_con>; > + }; > + }; > + No need for blank line > + }; > + }; > +}; > + > { > status = "okay"; > }; > @@ -248,6 +336,10 @@ _id_0 { > status = "okay"; > }; > > +_id_2 { > + status = "okay"; > +}; > + > { > status = "okay"; > firmware-name = "qcom/sm8350/slpi.mbn"; > @@ -544,4 +636,18 @@ usb_hub_enabled_state: usb-hub-enabled-state { > drive-strength = <2>; > output-low; > }; > + > + lt9611_rst_pin: lt9611-rst-state { > + pins = "gpio48"; > + function = "normal"; > + > + output-high; > + input-disable; > + }; > + > + lt9611_irq_pin: lt9611-irq { Missing suffix 'state'. Does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Rebase your changes on last linux-next. Best regards, Krzysztof
Re: [PATCH v1 8/9] arm64: dts: qcom: sm8350-hdk: Enable display & dsi nodes
On 28/10/2022 08:08, Robert Foss wrote: > Enable the display subsystem and the dsi0 output for > the sm8350-hdk board. > > Signed-off-by: Robert Foss > --- > arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 22 ++ > 1 file changed, 22 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > index e6deb08c6da0..6e07feb4b3b2 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > @@ -213,10 +213,32 @@ { > firmware-name = "qcom/sm8350/cdsp.mbn"; > }; > > + { > + status = "okay"; > +}; > + > + { > + status = "okay"; Status is the last property. Best regards, Krzysztof
Re: [PATCH v1 7/9] arm64: dts: qcom: sm8350: Add display system nodes
On 28/10/2022 08:08, Robert Foss wrote: > Add mdss, mdss_mdp, dsi0, dsi0_phy nodes. With these > nodes the display subsystem is configured to support > one DSI output. > > Signed-off-by: Robert Foss > --- > arch/arm64/boot/dts/qcom/sm8350.dtsi | 196 ++- > 1 file changed, 192 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi > b/arch/arm64/boot/dts/qcom/sm8350.dtsi > index b6e44cd3b394..eaa3cdee1860 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi > @@ -3,6 +3,7 @@ > * Copyright (c) 2020, Linaro Limited > */ > > +#include > #include > #include > #include > @@ -2535,14 +2536,200 @@ usb_2_dwc3: usb@a80 { > }; > }; > > + mdss: mdss@ae0 { > + compatible = "qcom,sm8350-mdss"; > + reg = <0 0x0ae0 0 0x1000>; > + reg-names = "mdss"; > + > + interconnects = <_noc MASTER_MDP0 0 _virt > SLAVE_EBI1 0>, > + <_noc MASTER_MDP1 0 _virt > SLAVE_EBI1 0>; > + interconnect-names = "mdp0-mem", "mdp1-mem"; > + > + power-domains = < MDSS_GDSC>; > + resets = < DISP_CC_MDSS_CORE_BCR>; > + > + clocks = < DISP_CC_MDSS_AHB_CLK>, > + < GCC_DISP_HF_AXI_CLK>, > + < GCC_DISP_SF_AXI_CLK>, > + < DISP_CC_MDSS_MDP_CLK>; > + clock-names = "iface", "bus", "nrt_bus", "core"; > + > + interrupts = ; > + interrupt-controller; > + #interrupt-cells = <1>; > + > + status = "ok"; No need for this. > + > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + mdss_mdp: mdp@ae01000 { Node name: display-controller > + compatible = "qcom,sm8350-dpu"; > + reg = <0 0x0ae01000 0 0x8f000>, > + <0 0x0aeb 0 0x2008>; > + reg-names = "mdp", "vbif"; > + iommus = <_smmu 0x820 0x402>; > + > + clocks = < GCC_DISP_HF_AXI_CLK>, > + < GCC_DISP_SF_AXI_CLK>, > + < DISP_CC_MDSS_AHB_CLK>, > + < DISP_CC_MDSS_MDP_LUT_CLK>, > + < DISP_CC_MDSS_MDP_CLK>, > + < DISP_CC_MDSS_VSYNC_CLK>; > + clock-names = "bus", > + "nrt_bus", > + "iface", > + "lut", > + "core", > + "vsync"; > + > + assigned-clocks = < > DISP_CC_MDSS_VSYNC_CLK>; > + assigned-clock-rates = <1920>; > + > + operating-points-v2 = <_opp_table>; > + power-domains = < SM8350_MMCX>; > + > + interrupt-parent = <>; > + interrupts = <0>; > + > + status = "ok"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dpu_intf1_out: endpoint { > + remote-endpoint = > <_in>; > + }; > + }; > + }; > + > + mdp_opp_table: mdp-opp-table { I have doubts that it passes dtbs_checks... opp-table > + compatible = "operating-points-v2"; > + > + opp-2 { > + opp-hz = /bits/ 64 <2>; > + required-opps = > <_opp_low_svs>; > + }; > + > + opp-3 { > + opp-hz = /bits/ 64 <3>; > + required-opps = > <_opp_svs>; > + }; > + > + opp-34500 { > + opp-hz = /bits/ 64 <34500>; > +
[PATCH] drm/i915: Set PROBE_PREFER_ASYNCHRONOUS
This driver often takes a good 100ms to start, but in some particularly bad cases takes more than 1 second. In surveying risk for this driver, I poked around for cross-device shared state, which can be a source of race conditions. GVT support (intel_gvt_devices) seems potentially suspect, but it has an appropriate mutex, and doesn't seem to care about ordering -- if devices are present when the kvmgt module loads, they'll get picked up; and if they probe later than kvmgt, they'll attach then. Additionally, I see past discussions about this patch [1], which concluded that there were problems at the time due to the way hdac_i915.c called request_module("i915") and expected it to complete probing [2]. Work has since been merged [3] to fix that problem. This driver was pinpointed as part of a survey of drivers that take more than 100ms in their initcall (i.e., are built in, and probing synchronously) on a lab of ChromeOS systems. [1] [RFC] i915: make the probe asynchronous https://lore.kernel.org/all/20180604053219.2040-1-feng.t...@intel.com/ [2] https://lore.kernel.org/all/s5hin4d1e3f.wl-ti...@suse.de/ [3] Commit f9b54e1961c7 ("ALSA: hda/i915: Allow delayed i915 audio component binding") Signed-off-by: Brian Norris --- drivers/gpu/drm/i915/i915_pci.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 38460a0bd7cb..1cb1f87aea86 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1371,7 +1371,10 @@ static struct pci_driver i915_pci_driver = { .probe = i915_pci_probe, .remove = i915_pci_remove, .shutdown = i915_pci_shutdown, - .driver.pm = _pm_ops, + .driver = { + .pm = _pm_ops, + .probe_type = PROBE_PREFER_ASYNCHRONOUS, + }, }; int i915_pci_register_driver(void) -- 2.38.1.273.g43a17bfeac-goog
Re: [git pull] drm fixes for 6.1-rc3
The pull request you sent on Fri, 28 Oct 2022 13:53:24 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-10-28 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/e3493d682516e2b7ef69587ddf91b0371a1511d0 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH v4] drm/tests: Add back seed value information
Hi Arthur, Just a small nit below, but besides that: Reviewed-by: Maíra Canal On 10/28/22 11:17, Arthur Grillo wrote: > As reported by Michał the drm_mm and drm_buddy unit tests lost the > printk with seed value after they were refactored into KUnit. > > Add kunit_info with seed value information to assure reproducibility. > > Reported-by: Michał Winiarski > Signed-off-by: Arthur Grillo > --- > v1->v2: > https://lore.kernel.org/all/20221026211458.68432-1-arthurgri...@riseup.net/ > - Correct compilation issues > - Change tags order > - Remove useless line change > - Write commit message in imperative form > - Remove redundant message part > - Correct some grammars nits > - Correct checkpatch issues > > v2->v3: > https://lore.kernel.org/all/20221027142903.200169-1-arthurgri...@riseup.net/ > - Change .init to .suite_init > - Correct some grammars nits > > v3->v4: > - Correct compilation issues > > --- > drivers/gpu/drm/tests/drm_buddy_test.c | 6 -- > drivers/gpu/drm/tests/drm_mm_test.c| 8 ++-- > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c > b/drivers/gpu/drm/tests/drm_buddy_test.c > index 62f69589a72d..90ec5e8a485b 100644 > --- a/drivers/gpu/drm/tests/drm_buddy_test.c > +++ b/drivers/gpu/drm/tests/drm_buddy_test.c > @@ -726,11 +726,13 @@ static void drm_test_buddy_alloc_limit(struct kunit > *test) > drm_buddy_fini(); > } > > -static int drm_buddy_init_test(struct kunit *test) > +static int drm_buddy_init_suite(struct kunit_suite *suite) Just to keep complaint to the rest of the KUnit tests (such as kcsan, kfence, kmsan), could you change "init_suite" to "suite_init"? Same thing for the drm_mm test. Thanks for the quick respin of the patch! Best Regards, - Maíra Canal > { > while (!random_seed) > random_seed = get_random_u32(); > > + kunit_info(suite, "Testing DRM buddy manager, with random_seed=0x%x\n", > random_seed); > + > return 0; > } > > @@ -746,7 +748,7 @@ static struct kunit_case drm_buddy_tests[] = { > > static struct kunit_suite drm_buddy_test_suite = { > .name = "drm_buddy", > - .init = drm_buddy_init_test, > + .suite_init = drm_buddy_init_suite, > .test_cases = drm_buddy_tests, > }; >
[PATCH V4 3/3] drm/panel: Add NewVision NV3051D MIPI-DSI LCD panel
From: Chris Morgan Support NewVision NV3051D panels as found on the Anbernic RG353P and RG353V. The underlying LCD part number for the RG353x devices is unknown, so the device name and a fallback for the driver IC is used instead. Signed-off-by: Chris Morgan --- drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile| 1 + .../gpu/drm/panel/panel-newvision-nv3051d.c | 523 ++ 3 files changed, 533 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-newvision-nv3051d.c diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index a582ddd583c2..427c22bdbfb3 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -296,6 +296,15 @@ config DRM_PANEL_NEC_NL8048HL11 panel (found on the Zoom2/3/3630 SDP boards). To compile this driver as a module, choose M here. +config DRM_PANEL_NEWVISION_NV3051D + tristate "NewVision NV3051D DSI panel" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + This driver supports the NV3051D based panel found on the Anbernic + RG353P and RG353V. + config DRM_PANEL_NEWVISION_NV3052C tristate "NewVision NV3052C RGB/SPI panel" depends on OF && SPI diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 34e717382dbb..cb03b3a82738 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o +obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3051D) += panel-newvision-nv3051d.o obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35560) += panel-novatek-nt35560.o diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c new file mode 100644 index ..e7e607f4045d --- /dev/null +++ b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c @@ -0,0 +1,523 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * NV3051D MIPI-DSI panel driver for Anbernic RG353x + * Copyright (C) 2022 Chris Morgan + * + * based on + * + * Elida kd35t133 3.5" MIPI-DSI panel driver + * Copyright (C) Theobroma Systems 2020 + */ + +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include + +struct nv3051d_panel_info { + const struct drm_display_mode *display_modes; + unsigned int num_modes; + u16 width_mm, height_mm; + u32 bus_flags; +}; + +struct panel_nv3051d { + struct device *dev; + struct drm_panel panel; + struct gpio_desc *reset_gpio; + const struct nv3051d_panel_info *panel_info; + struct regulator *vdd; + bool prepared; +}; + +static inline struct panel_nv3051d *panel_to_panelnv3051d(struct drm_panel *panel) +{ + return container_of(panel, struct panel_nv3051d, panel); +} + +#define dsi_dcs_write_seq(dsi, cmd, seq...) do { \ + static const u8 b[] = { cmd, seq }; \ + int ret;\ + ret = mipi_dsi_dcs_write_buffer(dsi, b, ARRAY_SIZE(b)); \ + if (ret < 0)\ + return ret; \ + } while (0) + +static int panel_nv3051d_init_sequence(struct panel_nv3051d *ctx) +{ + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); + + /* +* Init sequence was supplied by device vendor with no +* documentation. +*/ + + dsi_dcs_write_seq(dsi, 0xFF, 0x30); + dsi_dcs_write_seq(dsi, 0xFF, 0x52); + dsi_dcs_write_seq(dsi, 0xFF, 0x01); + dsi_dcs_write_seq(dsi, 0xE3, 0x00); + dsi_dcs_write_seq(dsi, 0x03, 0x40); + dsi_dcs_write_seq(dsi, 0x04, 0x00); + dsi_dcs_write_seq(dsi, 0x05, 0x03); + dsi_dcs_write_seq(dsi, 0x24, 0x12); + dsi_dcs_write_seq(dsi, 0x25, 0x1E); + dsi_dcs_write_seq(dsi, 0x26, 0x28); + dsi_dcs_write_seq(dsi, 0x27, 0x52); + dsi_dcs_write_seq(dsi, 0x28, 0x57); + dsi_dcs_write_seq(dsi, 0x29, 0x01); + dsi_dcs_write_seq(dsi, 0x2A, 0xDF); + dsi_dcs_write_seq(dsi, 0x38, 0x9C); + dsi_dcs_write_seq(dsi, 0x39, 0xA7); + dsi_dcs_write_seq(dsi, 0x3A, 0x53); + dsi_dcs_write_seq(dsi, 0x44, 0x00); + dsi_dcs_write_seq(dsi, 0x49, 0x3C); + dsi_dcs_write_seq(dsi, 0x59, 0xFE); + dsi_dcs_write_seq(dsi, 0x5C, 0x00); + dsi_dcs_write_seq(dsi, 0x91, 0x77); + dsi_dcs_write_seq(dsi, 0x92,
[PATCH V4 0/3] drm/panel: Add NewVision NV3051D Panels
From: Chris Morgan Add the NewVision NV3051D panel as found on the Anbernic RG353P and RG353V. The underlying LCD panel itself is unknown (the NV3051D is the controller), so the device name is used for the panel with a fallback to the driver IC. Changes from V3: - Changed driver remove function from int to void to match change made to mipi_dsi_driver struct. Changes from V2: - Ensured dt_binding_check and dtbs_check successfully passed. - Corrected some minor formatting issues in documentation. - Added another mode per userspace request for 100hz. I was unable to find a supported 50hz mode that would also work, so for now only 60hz, 100hz, and 120hz are supported. Changes from V1: - Changed compatible string to the driver IC. - Updated documentation to use new compatible string with board name. - Refactored somewhat to make it easier to support other LCD panels with this kernel module. - Added support for 60hz mode. Adjusted pixel clock to ensure proper 60hz and 120hz (previously was running at 124hz). - Added vendor prefix for NewVision. Anbernic vendor prefix added in https://lore.kernel.org/linux-devicetree/20220906210324.28986-2-macroalph...@gmail.com Chris Morgan (3): dt-bindings: vendor-prefixes: add NewVision vendor prefix dt-bindings: display: panel: Add NewVision NV3051D bindings drm/panel: Add NewVision NV3051D MIPI-DSI LCD panel .../display/panel/newvision,nv3051d.yaml | 63 +++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile| 1 + .../gpu/drm/panel/panel-newvision-nv3051d.c | 523 ++ 5 files changed, 598 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml create mode 100644 drivers/gpu/drm/panel/panel-newvision-nv3051d.c -- 2.25.1
[PATCH V4 2/3] dt-bindings: display: panel: Add NewVision NV3051D bindings
From: Chris Morgan Add documentation for the NewVision NV3051D panel bindings. Note that for the two expected consumers of this panel binding the underlying LCD model is unknown. Name "anbernic,rg353p-panel" is used because the hardware itself is known as "anbernic,rg353p". Signed-off-by: Chris Morgan --- .../display/panel/newvision,nv3051d.yaml | 63 +++ 1 file changed, 63 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml diff --git a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml new file mode 100644 index ..407de7fb5499 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml @@ -0,0 +1,63 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/newvision,nv3051d.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NewVision NV3051D based LCD panel + +description: | + The NewVision NV3051D is a driver chip used to drive DSI panels. For now, + this driver only supports the 640x480 panels found in the Anbernic RG353 + based devices. + +maintainers: + - Chris Morgan + +allOf: + - $ref: panel-common.yaml# + +properties: + compatible: +items: + - enum: + - anbernic,rg353p-panel + - anbernic,rg353v-panel + - const: newvision,nv3051d + + reg: true + backlight: true + port: true + reset-gpios: true + vdd-supply: true + +required: + - compatible + - reg + - backlight + - vdd-supply + +additionalProperties: false + +examples: + - | +#include +dsi { +#address-cells = <1>; +#size-cells = <0>; +panel@0 { +compatible = "anbernic,rg353p-panel", "newvision,nv3051d"; +reg = <0>; +backlight = <>; +reset-gpios = < 0 GPIO_ACTIVE_LOW>; +vdd-supply = <_lcd>; + +port { +mipi_in_panel: endpoint { +remote-endpoint = <_out_panel>; +}; +}; +}; +}; + +... -- 2.25.1
[PATCH V4 1/3] dt-bindings: vendor-prefixes: add NewVision vendor prefix
From: Chris Morgan NewVision (also sometimes written as New Vision) is a company based in Shenzen that manufactures ICs for controlling LCD panels. https://www.newvisiondisplay.com/ Signed-off-by: Chris Morgan Acked-by: Krzysztof Kozlowski --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 6e323a380294..c6aa7b3d1455 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -883,6 +883,8 @@ patternProperties: description: Shenzhen Netxeon Technology CO., LTD "^neweast,.*": description: Guangdong Neweast Optoelectronics CO., LTD + "^newvision,.*": +description: New Vision Display (Shenzhen) Co., Ltd. "^nexbox,.*": description: Nexbox "^nextthing,.*": -- 2.25.1
[PATCH] drm/amd/display: drop vblank_lock from struct amdgpu_display_manager
As of commit 09a5df6c444c ("drm/amd/display: Fix multi-display support for idle opt workqueue"), vblank_lock is no longer being used. So, don't init it in amdgpu_dm_init() and remove it from struct amdgpu_display_manager. Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 7 --- 2 files changed, 8 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index eb4ce7216104..11afb4b24fd9 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -1394,7 +1394,6 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) mutex_init(>dm.dc_lock); mutex_init(>dm.audio_lock); - spin_lock_init(>dm.vblank_lock); if(amdgpu_dm_irq_init(adev)) { DRM_ERROR("amdgpu: failed to initialize DM IRQ support.\n"); diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index b5ce15c43bcc..b618b2586e7b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -365,13 +365,6 @@ struct amdgpu_display_manager { */ struct mutex audio_lock; - /** -* @vblank_lock: -* -* Guards access to deferred vblank work state. -*/ - spinlock_t vblank_lock; - /** * @audio_component: * -- 2.38.0
Re: [PATCH v4 00/10] drm/msm: Fix math issues in MSM DSC implementation
Hi Abhinav, On 2022-10-28 11:33:21, Abhinav Kumar wrote: > Hi Marijn > > On 10/26/2022 11:28 AM, Marijn Suijten wrote: > > Various removals of complex yet unnecessary math, fixing all uses of > > drm_dsc_config::bits_per_pixel to deal with the fact that this field > > includes four fractional bits, and finally making sure that > > range_bpg_offset contains values 6-bits wide to prevent overflows in > > drm_dsc_pps_payload_pack(). > > > > Altogether this series is responsible for solving _all_ Display Stream > > Compression issues and artifacts on the Sony Tama (sdm845) Akatsuki > > smartphone (2880x1440p). > > > > Changes since v3: > > - Swap patch 7 and 8 to make sure msm_host is available inside > >dsi_populate_dsc_params(); > > - Reword patch 6 (Migrate to drm_dsc_compute_rc_parameters()) to more > >clearly explain why the FIXME wasn't solved initially, but why it can > >(and should!) be resolved now. > > > > v3: > > https://lore.kernel.org/linux-arm-msm/20221009184824.457416-1-marijn.suij...@somainline.org/T/#u > > > > Changes since v2: > > - Generalize mux_word_size setting depending on bits_per_component; > > - Migrate DSI's DSC calculations to drm_dsc_compute_rc_parameters(), > >implicitly addressing existing math issues; > > - Disallow any bits_per_component other than 8, until hardcoded values > >are updated and tested to support such cases. > > > > v2: > > https://lore.kernel.org/linux-arm-msm/20221005181657.784375-1-marijn.suij...@somainline.org/T/#u > > > > Changes since v1: > > > > - Propagate r-b's, except (obviously) in patches that were (heavily) > >modified; > > - Remove accidental debug code in dsi_cmd_dma_add; > > - Move Range BPG Offset masking out of DCS PPS packing, back into the > >DSI driver when it is assigned to drm_dsc_config (this series is now > >strictly focusing on drm/msm again); > > - Replace modulo-check resulting in conditional increment with > >DIV_ROUND_UP; > > - Remove repeated calculation of slice_chunk_size; > > - Use u16 instead of int when handling bits_per_pixel; > > - Use DRM_DEV_ERROR instead of pr_err in DSI code; > > - Also remove redundant target_bpp_x16 variable. > > > > v1: > > https://lore.kernel.org/linux-arm-msm/20221001190807.358691-1-marijn.suij...@somainline.org/T/#u > > > > Marijn Suijten (10): > >drm/msm/dsi: Remove useless math in DSC calculations > >drm/msm/dsi: Remove repeated calculation of slice_per_intf > >drm/msm/dsi: Use DIV_ROUND_UP instead of conditional increment on > > modulo > >drm/msm/dsi: Reuse earlier computed dsc->slice_chunk_size > >drm/msm/dsi: Appropriately set dsc->mux_word_size based on bpc > >drm/msm/dsi: Migrate to drm_dsc_compute_rc_parameters() > >drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits > >drm/msm/dsi: Disallow 8 BPC DSC configuration for alternative BPC > > values > >drm/msm/dpu1: Account for DSC's bits_per_pixel having 4 fractional > > bits > >drm/msm/dsi: Prevent signed BPG offsets from bleeding into adjacent > > bits > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 11 +- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 121 ++--- > > 2 files changed, 37 insertions(+), 95 deletions(-) > > > > -- > > 2.38.1 > > > > To keep the -fixes cycle to have only critical fixes (others are > important too but are cleanups), I was thinking of absorbing patches > 7,8,9 and 10 alone in the -fixes cycle and for patches 1-6, will go > through the 6.2 push. > > Let me know if there are any concerns if we just take patches 7,8,9 and > 10 separately. Unfortunately that isn't going to cut it. For starters patch 8 is only introducing additional validation but as long as no panel drivers set bpc != 8, this doesn't change anything: it is not a critical fix. Then, more importantly, as discussed in v2 reviews it was preferred to _not_ fix the broken code in dsi_populate_dsc_params() but migrate to drm_dsc_compute_rc_parameters() directly [1]. As such patch 6 (which performs the migration) is definitely a requirement for the fixes to be complete. Then again this patch looks weird when 5 is not applied, since both refactor how dsc->mux_word_size is assigned. At the same time it cannot be cleanly applied without patch 1 (Remove useless math in DSC calculations) nor patch 3 (Use DIV_ROUND_UP instead of conditional increment on modulo), but I just realized that patch 3 is now also useless as the code is being removed altogether while migrating to drm_dsc_compute_rc_parameters(). Same for patch 4 (Reuse earlier computed dsc->slice_chunk_size): while it may not seem obvious at first, the original code uses bits_per_pixel without considering the fractional bits, again resulting invalid values. Perhaps this should have been mentioned in the patch description, but I did not want to create an even larger chain of references pointing back and forth to future patches fixing the
[PATCH 2/2] drm/i915/guc: Don't deadlock busyness stats vs reset
From: John Harrison The engine busyness stats has a worker function to do things like 64bit extend the 32bit hardware counters. The GuC's reset prepare function flushes out this worker function to ensure no corruption happens during the reset. Unforunately, the worker function has an infinite wait for active resets to finish before doing its work. Thus a deadlock would occur if the worker function had actually started just as the reset starts. Update the worker to abort if a reset is in progress rather than waiting for it to complete. It will still acquire the reset lock in the case where a reset was not already in progress. So the processing is still safe from corruption, but the deadlock can no longer occur. Signed-off-by: John Harrison --- drivers/gpu/drm/i915/gt/intel_reset.c | 15 ++- drivers/gpu/drm/i915/gt/intel_reset.h | 1 + drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 6 -- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index 3159df6cdd492..2f48c6e4420ea 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -1407,7 +1407,7 @@ void intel_gt_handle_error(struct intel_gt *gt, intel_runtime_pm_put(gt->uncore->rpm, wakeref); } -int intel_gt_reset_trylock(struct intel_gt *gt, int *srcu) +static int _intel_gt_reset_trylock(struct intel_gt *gt, int *srcu, bool retry) { might_lock(>reset.backoff_srcu); might_sleep(); @@ -1416,6 +1416,9 @@ int intel_gt_reset_trylock(struct intel_gt *gt, int *srcu) while (test_bit(I915_RESET_BACKOFF, >reset.flags)) { rcu_read_unlock(); + if (!retry) + return -EBUSY; + if (wait_event_interruptible(gt->reset.queue, !test_bit(I915_RESET_BACKOFF, >reset.flags))) @@ -1429,6 +1432,16 @@ int intel_gt_reset_trylock(struct intel_gt *gt, int *srcu) return 0; } +int intel_gt_reset_trylock_noretry(struct intel_gt *gt, int *srcu) +{ + return _intel_gt_reset_trylock(gt, srcu, false); +} + +int intel_gt_reset_trylock(struct intel_gt *gt, int *srcu) +{ + return _intel_gt_reset_trylock(gt, srcu, true); +} + void intel_gt_reset_unlock(struct intel_gt *gt, int tag) __releases(>reset.backoff_srcu) { diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h b/drivers/gpu/drm/i915/gt/intel_reset.h index adc734e673870..7f863726eb6a2 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.h +++ b/drivers/gpu/drm/i915/gt/intel_reset.h @@ -38,6 +38,7 @@ int __intel_engine_reset_bh(struct intel_engine_cs *engine, void __i915_request_reset(struct i915_request *rq, bool guilty); +int __must_check intel_gt_reset_trylock_noretry(struct intel_gt *gt, int *srcu); int __must_check intel_gt_reset_trylock(struct intel_gt *gt, int *srcu); void intel_gt_reset_unlock(struct intel_gt *gt, int tag); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 941613be3b9dd..1fa1bc7dde3df 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1401,9 +1401,11 @@ static void guc_timestamp_ping(struct work_struct *wrk) /* * Synchronize with gt reset to make sure the worker does not -* corrupt the engine/guc stats. +* corrupt the engine/guc stats. NB: can't actually block waiting +* for a reset to complete as the reset requires flushing out +* any running worker thread. So waiting would deadlock. */ - ret = intel_gt_reset_trylock(gt, ); + ret = intel_gt_reset_trylock_noretry(gt, ); if (ret) return; -- 2.37.3
[PATCH 1/2] drm/i915/guc: Properly initialise kernel contexts
From: John Harrison If a context has already been registered prior to first submission then context init code was not being called. The noticeable effect of that was the scheduling priority was left at zero (meaning super high priority) instead of being set to normal. This would occur with kernel contexts at start of day as they are manually pinned up front rather than on first submission. So add a call to initialise those when they are pinned. Signed-off-by: John Harrison --- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 4ccb29f9ac55c..941613be3b9dd 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -4111,6 +4111,9 @@ static inline void guc_kernel_context_pin(struct intel_guc *guc, if (context_guc_id_invalid(ce)) pin_guc_id(guc, ce); + if (!test_bit(CONTEXT_GUC_INIT, >flags)) + guc_context_init(ce); + try_context_registration(ce, true); } -- 2.37.3
[PATCH 0/2] Fix for two GuC issues
From: John Harrison Fix for a deadlock issue between the GuC busyness stats worker and GT resets. Also fix kernel contexts not getting the correct scheduling priority at start of day. Signed-off-by: John Harrison John Harrison (2): drm/i915/guc: Properly initialise kernel contexts drm/i915/guc: Don't deadlock busyness stats vs reset drivers/gpu/drm/i915/gt/intel_reset.c | 15 ++- drivers/gpu/drm/i915/gt/intel_reset.h | 1 + drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 9 +++-- 3 files changed, 22 insertions(+), 3 deletions(-) -- 2.37.3
[PATCH] drm/i915/mtl: Add MC6 Wa_14017210380 for SAMedia
This workaround is added for Media Tile of MTL A step. It is to help pcode workaround which handles the hardware bug seen on CXL splitter during package C2/C3 transitins due to MC6 entry/exit. As a part of workaround pcode expect kmd to send mailbox message "media busy" when components of Media tile is in use and "media not busy" when not in use. As per workaround description gucrc need to be disabled so enabled host based RC for Media tile. HSD: 14017210380 Cc: Rodrigo Vivi Cc: Radhakrishna Sripada Cc: Vinay Belgaumkar Cc: Chris Wilson Signed-off-by: Badal Nilawar --- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 33 +++ drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 13 - drivers/gpu/drm/i915/i915_drv.h | 4 +++ drivers/gpu/drm/i915/i915_reg.h | 9 +++ 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index f553e2173bda..398dbeb298ca 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -19,10 +19,37 @@ #include "intel_rc6.h" #include "intel_rps.h" #include "intel_wakeref.h" +#include "intel_pcode.h" #include "pxp/intel_pxp_pm.h" #define I915_GT_SUSPEND_IDLE_TIMEOUT (HZ / 2) +/* + * Wa_14017210380: mtl + */ + +static bool mtl_needs_media_mc6_wa(struct intel_gt *gt) +{ + return (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) && + gt->type == GT_MEDIA); +} + +static void mtl_mc6_wa_media_busy(struct intel_gt *gt) +{ + if (mtl_needs_media_mc6_wa(gt)) + snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE, + PCODE_MBOX_GT_STATE_MEDIA_BUSY, + PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0); +} + +static void mtl_mc6_wa_media_not_busy(struct intel_gt *gt) +{ + if (mtl_needs_media_mc6_wa(gt)) + snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE, + PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY, + PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0); +} + static void user_forcewake(struct intel_gt *gt, bool suspend) { int count = atomic_read(>user_wakeref); @@ -70,6 +97,9 @@ static int __gt_unpark(struct intel_wakeref *wf) GT_TRACE(gt, "\n"); + /* Wa_14017210380: mtl */ + mtl_mc6_wa_media_busy(gt); + /* * It seems that the DMC likes to transition between the DC states a lot * when there are no connected displays (no active power domains) during @@ -119,6 +149,9 @@ static int __gt_park(struct intel_wakeref *wf) GEM_BUG_ON(!wakeref); intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ, wakeref); + /* Wa_14017210380: mtl */ + mtl_mc6_wa_media_not_busy(gt); + return 0; } diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c index 8f8dd05835c5..cc6356ff84a5 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c @@ -11,9 +11,20 @@ static bool __guc_rc_supported(struct intel_guc *guc) { + struct intel_gt *gt = guc_to_gt(guc); + + /* +* Wa_14017210380: mtl +* Do not enable gucrc to avoid additional interrupts which +* may disrupt pcode wa. +*/ + if (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) && + gt->type == GT_MEDIA) + return false; + /* GuC RC is unavailable for pre-Gen12 */ return guc->submission_supported && - GRAPHICS_VER(guc_to_gt(guc)->i915) >= 12; + GRAPHICS_VER(gt->i915) >= 12; } static bool __guc_rc_selected(struct intel_guc *guc) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 05b3300cc4ed..659b92382ff2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -740,6 +740,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define IS_XEHPSDV_GRAPHICS_STEP(__i915, since, until) \ (IS_XEHPSDV(__i915) && IS_GRAPHICS_STEP(__i915, since, until)) +#define IS_MTL_GRAPHICS_STEP(__i915, variant, since, until) \ + (IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \ +IS_GRAPHICS_STEP(__i915, since, until)) + /* * DG2 hardware steppings are a bit unusual. The hardware design was forked to * create three variants (G10, G11, and G12) which each have distinct diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 1c0da50c0dc7..abe62cea083d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6678,6 +6678,15 @@ /* XEHP_PCODE_FREQUENCY_CONFIG param2 */ #define PCODE_MBOX_DOMAIN_NONE 0x0 #define PCODE_MBOX_DOMAIN_MEDIAFF 0x3 + +/* Wa_14017210380: mtl */ +#define PCODE_MBOX_GT_STATE 0x50 +/* sub-commands (param1)
Re: [PATCH v1 3/7] vfio/ccw: move private initialization to callback
On Fri, 2022-10-28 at 14:52 -0400, Matthew Rosato wrote: > On 10/19/22 12:21 PM, Eric Farman wrote: > > There's already a device initialization callback that is > > used to initialize the release completion workaround. > > As discussed off-list, maybe clarify what callback you're talking > about here and/or reference the commit that added it. Agreed. Will point out that it's private->release_comp, introduced with commit ebb72b765fb49 ("vfio/ccw: Use the new device life cycle helpers") > > > > > Move the other elements of the vfio_ccw_private struct that > > require distinct initialization over to that routine. > > > > Signed-off-by: Eric Farman > > --- > > drivers/s390/cio/vfio_ccw_drv.c | 57 +++-- > > > > drivers/s390/cio/vfio_ccw_ops.c | 43 ++ > > drivers/s390/cio/vfio_ccw_private.h | 7 +++- > > 3 files changed, 55 insertions(+), 52 deletions(-) > > > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c > > b/drivers/s390/cio/vfio_ccw_drv.c > > index 4ee953c8ae39..cc9ed2fd970f 100644 > > --- a/drivers/s390/cio/vfio_ccw_drv.c > > +++ b/drivers/s390/cio/vfio_ccw_drv.c > > @@ -24,10 +24,10 @@ > > #include "vfio_ccw_private.h" > > > > struct workqueue_struct *vfio_ccw_work_q; > > -static struct kmem_cache *vfio_ccw_io_region; > > -static struct kmem_cache *vfio_ccw_cmd_region; > > -static struct kmem_cache *vfio_ccw_schib_region; > > -static struct kmem_cache *vfio_ccw_crw_region; > > +struct kmem_cache *vfio_ccw_io_region; > > +struct kmem_cache *vfio_ccw_cmd_region; > > +struct kmem_cache *vfio_ccw_schib_region; > > +struct kmem_cache *vfio_ccw_crw_region; > > > > debug_info_t *vfio_ccw_debug_msg_id; > > debug_info_t *vfio_ccw_debug_trace_id; > > @@ -74,7 +74,7 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch) > > return ret; > > } > > > > -static void vfio_ccw_sch_io_todo(struct work_struct *work) > > +void vfio_ccw_sch_io_todo(struct work_struct *work) > > { > > struct vfio_ccw_private *private; > > struct irb *irb; > > @@ -110,7 +110,7 @@ static void vfio_ccw_sch_io_todo(struct > > work_struct *work) > > eventfd_signal(private->io_trigger, 1); > > } > > > > -static void vfio_ccw_crw_todo(struct work_struct *work) > > +void vfio_ccw_crw_todo(struct work_struct *work) > > { > > struct vfio_ccw_private *private; > > > > @@ -154,52 +154,7 @@ static struct vfio_ccw_private > > *vfio_ccw_alloc_private(struct subchannel *sch) > > if (!private) > > return ERR_PTR(-ENOMEM); > > Not sure we really still need vfio_ccw_alloc_private() now or whether > you can just kzalloc() inline right in vfio_ccw_sch_probe() Fair. It ends up ends up getting scrapped in patch 6 anyway, but that might clean things up just a smidge more. Will give it a whirl. > Either way: > > Reviewed-by: Matthew Rosato Thanks! > > > > > > - mutex_init(>io_mutex); > > - private->state = VFIO_CCW_STATE_STANDBY; > > - INIT_LIST_HEAD(>crw); > > - INIT_WORK(>io_work, vfio_ccw_sch_io_todo); > > - INIT_WORK(>crw_work, vfio_ccw_crw_todo); > > - > > - private->cp.guest_cp = kcalloc(CCWCHAIN_LEN_MAX, > > sizeof(struct ccw1), > > - GFP_KERNEL); > > - if (!private->cp.guest_cp) > > - goto out_free_private; > > - > > - private->io_region = kmem_cache_zalloc(vfio_ccw_io_region, > > - GFP_KERNEL | > > GFP_DMA); > > - if (!private->io_region) > > - goto out_free_cp; > > - > > - private->cmd_region = > > kmem_cache_zalloc(vfio_ccw_cmd_region, > > - GFP_KERNEL | > > GFP_DMA); > > - if (!private->cmd_region) > > - goto out_free_io; > > - > > - private->schib_region = > > kmem_cache_zalloc(vfio_ccw_schib_region, > > - GFP_KERNEL | > > GFP_DMA); > > - > > - if (!private->schib_region) > > - goto out_free_cmd; > > - > > - private->crw_region = > > kmem_cache_zalloc(vfio_ccw_crw_region, > > - GFP_KERNEL | > > GFP_DMA); > > - > > - if (!private->crw_region) > > - goto out_free_schib; > > return private; > > - > > -out_free_schib: > > - kmem_cache_free(vfio_ccw_schib_region, private- > > >schib_region); > > -out_free_cmd: > > - kmem_cache_free(vfio_ccw_cmd_region, private->cmd_region); > > -out_free_io: > > - kmem_cache_free(vfio_ccw_io_region, private->io_region); > > -out_free_cp: > > - kfree(private->cp.guest_cp); > > -out_free_private: > > - mutex_destroy(>io_mutex); > > - kfree(private); > > - return ERR_PTR(-ENOMEM); > > } > > > > static void vfio_ccw_free_private(struct vfio_ccw_private > > *private) > > diff --git a/drivers/s390/cio/vfio_ccw_ops.c > >
Re: [PATCH v1 3/7] vfio/ccw: move private initialization to callback
On 10/19/22 12:21 PM, Eric Farman wrote: > There's already a device initialization callback that is > used to initialize the release completion workaround. As discussed off-list, maybe clarify what callback you're talking about here and/or reference the commit that added it. > > Move the other elements of the vfio_ccw_private struct that > require distinct initialization over to that routine. > > Signed-off-by: Eric Farman > --- > drivers/s390/cio/vfio_ccw_drv.c | 57 +++-- > drivers/s390/cio/vfio_ccw_ops.c | 43 ++ > drivers/s390/cio/vfio_ccw_private.h | 7 +++- > 3 files changed, 55 insertions(+), 52 deletions(-) > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c > index 4ee953c8ae39..cc9ed2fd970f 100644 > --- a/drivers/s390/cio/vfio_ccw_drv.c > +++ b/drivers/s390/cio/vfio_ccw_drv.c > @@ -24,10 +24,10 @@ > #include "vfio_ccw_private.h" > > struct workqueue_struct *vfio_ccw_work_q; > -static struct kmem_cache *vfio_ccw_io_region; > -static struct kmem_cache *vfio_ccw_cmd_region; > -static struct kmem_cache *vfio_ccw_schib_region; > -static struct kmem_cache *vfio_ccw_crw_region; > +struct kmem_cache *vfio_ccw_io_region; > +struct kmem_cache *vfio_ccw_cmd_region; > +struct kmem_cache *vfio_ccw_schib_region; > +struct kmem_cache *vfio_ccw_crw_region; > > debug_info_t *vfio_ccw_debug_msg_id; > debug_info_t *vfio_ccw_debug_trace_id; > @@ -74,7 +74,7 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch) > return ret; > } > > -static void vfio_ccw_sch_io_todo(struct work_struct *work) > +void vfio_ccw_sch_io_todo(struct work_struct *work) > { > struct vfio_ccw_private *private; > struct irb *irb; > @@ -110,7 +110,7 @@ static void vfio_ccw_sch_io_todo(struct work_struct *work) > eventfd_signal(private->io_trigger, 1); > } > > -static void vfio_ccw_crw_todo(struct work_struct *work) > +void vfio_ccw_crw_todo(struct work_struct *work) > { > struct vfio_ccw_private *private; > > @@ -154,52 +154,7 @@ static struct vfio_ccw_private > *vfio_ccw_alloc_private(struct subchannel *sch) > if (!private) > return ERR_PTR(-ENOMEM); Not sure we really still need vfio_ccw_alloc_private() now or whether you can just kzalloc() inline right in vfio_ccw_sch_probe() Either way: Reviewed-by: Matthew Rosato > > - mutex_init(>io_mutex); > - private->state = VFIO_CCW_STATE_STANDBY; > - INIT_LIST_HEAD(>crw); > - INIT_WORK(>io_work, vfio_ccw_sch_io_todo); > - INIT_WORK(>crw_work, vfio_ccw_crw_todo); > - > - private->cp.guest_cp = kcalloc(CCWCHAIN_LEN_MAX, sizeof(struct ccw1), > -GFP_KERNEL); > - if (!private->cp.guest_cp) > - goto out_free_private; > - > - private->io_region = kmem_cache_zalloc(vfio_ccw_io_region, > -GFP_KERNEL | GFP_DMA); > - if (!private->io_region) > - goto out_free_cp; > - > - private->cmd_region = kmem_cache_zalloc(vfio_ccw_cmd_region, > - GFP_KERNEL | GFP_DMA); > - if (!private->cmd_region) > - goto out_free_io; > - > - private->schib_region = kmem_cache_zalloc(vfio_ccw_schib_region, > - GFP_KERNEL | GFP_DMA); > - > - if (!private->schib_region) > - goto out_free_cmd; > - > - private->crw_region = kmem_cache_zalloc(vfio_ccw_crw_region, > - GFP_KERNEL | GFP_DMA); > - > - if (!private->crw_region) > - goto out_free_schib; > return private; > - > -out_free_schib: > - kmem_cache_free(vfio_ccw_schib_region, private->schib_region); > -out_free_cmd: > - kmem_cache_free(vfio_ccw_cmd_region, private->cmd_region); > -out_free_io: > - kmem_cache_free(vfio_ccw_io_region, private->io_region); > -out_free_cp: > - kfree(private->cp.guest_cp); > -out_free_private: > - mutex_destroy(>io_mutex); > - kfree(private); > - return ERR_PTR(-ENOMEM); > } > > static void vfio_ccw_free_private(struct vfio_ccw_private *private) > diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c > index cf383c729d53..626b8eb3507b 100644 > --- a/drivers/s390/cio/vfio_ccw_ops.c > +++ b/drivers/s390/cio/vfio_ccw_ops.c > @@ -50,8 +50,51 @@ static int vfio_ccw_mdev_init_dev(struct vfio_device *vdev) > struct vfio_ccw_private *private = > container_of(vdev, struct vfio_ccw_private, vdev); > > + mutex_init(>io_mutex); > + private->state = VFIO_CCW_STATE_STANDBY; > + INIT_LIST_HEAD(>crw); > + INIT_WORK(>io_work, vfio_ccw_sch_io_todo); > + INIT_WORK(>crw_work, vfio_ccw_crw_todo); > init_completion(>release_comp); > + > + private->cp.guest_cp = kcalloc(CCWCHAIN_LEN_MAX, sizeof(struct ccw1), > +GFP_KERNEL); > +
Re: Try to address the DMA-buf coherency problem
Hi Christian, On Fri, 28 Oct 2022 at 18:50, Christian König wrote: > Am 28.10.22 um 17:46 schrieb Nicolas Dufresne: > > Though, its not generically possible to reverse these roles. If you want to > > do > > so, you endup having to do like Android (gralloc) and ChromeOS (minigbm), > > because you will have to allocate DRM buffers that knows about importer > > specific > > requirements. See link [1] for what it looks like for RK3399, with Motion > > Vector > > size calculation copied from the kernel driver into a userspace lib > > (arguably > > that was available from V4L2 sizeimage, but this is technically difficult to > > communicate within the software layers). If you could let the decoder export > > (with proper cache management) the non-generic code would not be needed. > > Yeah, but I can also reverse the argument: > > Getting the parameters for V4L right so that we can share the image is > tricky, but getting the parameters so that the stuff is actually > directly displayable by GPUs is even trickier. > > Essentially you need to look at both sides and interference to get to a > common ground, e.g. alignment, pitch, width/height, padding, etc. > > Deciding from which side to allocate from is just one step in this > process. For example most dGPUs can't display directly from system > memory altogether, but it is possible to allocate the DMA-buf through > the GPU driver and then write into device memory with P2P PCI transfers. > > So as far as I can see switching importer and exporter roles and even > having performant extra fallbacks should be a standard feature of userspace. > > > Another case where reversing the role is difficult is for case where you > > need to > > multiplex the streams (let's use a camera to illustrate) and share that with > > multiple processes. In these uses case, the DRM importers are volatile, > > which > > one do you abuse to do allocation from ? In multimedia server like > > PipeWire, you > > are not really aware if the camera will be used by DRM or not, and if > > something > > "special" is needed in term of role inversion. It is relatively easy to deal > > with matching modifiers, but using downstream (display/gpu) as an exporter > > is > > always difficult (and require some level of abuse and guessing). > > Oh, very good point! Yeah we do have use cases for this where an input > buffer is both displayed as well as encoded. This is the main issue, yeah. For a standard media player, they would try to allocate through V4L2 and decode through that into locally-allocated buffers. All they know is that there's a Wayland server at the other end of a socket somewhere which will want to import the FD. The server does give you some hints along the way: it will tell you that importing into a particular GPU target device is necessary as the ultimate fallback, and importing into a particular KMS device is preferable as the optimal path to hit an overlay. So let's say that the V4L2 client does what you're proposing: it allocates a buffer chain, schedules a decode into that buffer, and passes it along to the server to import. The server fails to import the buffer into the GPU, and tells the client this. The client then ... well, it doesn't know that it needs to allocate within the GPU instead, but it knows that doing so might be one thing which would make the request succeed. But the client is just a video player. It doesn't understand how to allocate BOs for Panfrost or AMD or etnaviv. So without a universal allocator (again ...), 'just allocate on the GPU' isn't a useful response to the client. I fully understand your point about APIs like Vulkan not sensibly allowing bracketing, and that's fine. On the other hand, a lot of extant usecases (camera/codec -> GPU/display, GPU -> codec, etc) on Arm just cannot fulfill complete coherency. On a lot of these platforms, despite what you might think about the CPU/GPU capabilities, the bottleneck is _always_ memory bandwidth, so mandating extra copies is an absolute non-starter, and would instantly cripple billions of devices. Lucas has been pretty gentle, but to be more clear, this is not an option and won't be for at least the next decade. So we obviously need a third way at this point, because 'all devices must always be coherent' vs. 'cache must be an unknown' can't work. How about this as a suggestion: we have some unused flags in the PRIME ioctls. Can we add a flag for 'import must be coherent'? That flag wouldn't be set for the existing ecosystem Lucas/Nicolas/myself are talking about, where we have explicit handover points and users are fully able to perform cache maintenance. For newer APIs where it's not possible to properly express that bracketing, they would always set that flag (unless we add an API carve-out where the client promises to do whatever is required to maintain that). Would that be viable? Cheers, Daniel
Re: [PATCH 10/10] iommufd: Allow iommufd to supply /dev/vfio/vfio
On Wed, Oct 26, 2022 at 03:31:33PM -0600, Alex Williamson wrote: > On Tue, 25 Oct 2022 15:50:45 -0300 > Jason Gunthorpe wrote: > > > If the VFIO container is compiled out, give a kconfig option for iommufd > > to provide the miscdev node with the same name and permissions as vfio > > uses. > > > > The compatibility node supports the same ioctls as VFIO and automatically > > enables the VFIO compatible pinned page accounting mode. > > I think I'd like to see some sort of breadcrumb when /dev/vfio/vfio is > provided by something other than the vfio container code. If we intend > to include this before P2P is resolved, that breadcrumb I don't belive I can get P2P done soon enough. I plan to do it after this is merged. Right now these two series are taking all my time. > (dmesg I'm guessing) might also list any known limitations of the > compatibility to save time with debugging. Thanks, Yes, that makes sense. Do you want a dmesg at module load time, on every open, or a sysfs something? What seems like it would make it into a bug report? Thanks, Jason
Re: [PATCH v1 2/7] vfio/ccw: remove private->sch
On 10/19/22 12:21 PM, Eric Farman wrote: > These places all rely on the ability to jump from a private > struct back to the subchannel struct. Rather than keeping a > copy in our back pocket, let's use the relationship provided > by the vfio_device embedded within the private. > > Signed-off-by: Eric Farman > --- > drivers/s390/cio/vfio_ccw_chp.c | 5 +++-- > drivers/s390/cio/vfio_ccw_drv.c | 3 +-- > drivers/s390/cio/vfio_ccw_fsm.c | 27 --- > drivers/s390/cio/vfio_ccw_ops.c | 12 ++-- > drivers/s390/cio/vfio_ccw_private.h | 7 --- > 5 files changed, 26 insertions(+), 28 deletions(-) > > diff --git a/drivers/s390/cio/vfio_ccw_chp.c b/drivers/s390/cio/vfio_ccw_chp.c > index 13b26a1c7988..d3f3a611f95b 100644 > --- a/drivers/s390/cio/vfio_ccw_chp.c > +++ b/drivers/s390/cio/vfio_ccw_chp.c > @@ -16,6 +16,7 @@ static ssize_t vfio_ccw_schib_region_read(struct > vfio_ccw_private *private, > char __user *buf, size_t count, > loff_t *ppos) > { > + struct subchannel *sch = to_subchannel(private->vdev.dev->parent); I'm not a big fan of the amount of indirection there, but I prefer this over back-pocketing the subchannel in vfio_ccw_private anyway Reviewed-by: Matthew Rosato
Re: [PATCH 04/10] vfio: Move storage of allow_unsafe_interrupts to vfio_main.c
On Wed, Oct 26, 2022 at 03:24:42PM -0600, Alex Williamson wrote: > On Tue, 25 Oct 2022 15:17:10 -0300 > Jason Gunthorpe wrote: > > > This legacy module knob has become uAPI, when set on the vfio_iommu_type1 > > it disables some security protections in the iommu drivers. Move the > > storage for this knob to vfio_main.c so that iommufd can access it too. > > I don't really understand this, we're changing the behavior of the > iommufd_device_attach() operation based on the modules options of > vfio_iommu_type1, The specific reason it was done is that we had a misconfigured test VM in the farm that needed it, and that VM has since been fixed. But it did highlight we should try to preserve this in some way. > which may not be loaded or even compiled into the > kernel. Our compatibility story falls apart when VFIO_CONTAINER is not > set, iommufd sneaks in to usurp /dev/vfio/vfio, and the user's module > options for type1 go unprocessed. There are two aspects here, trying to preseve the allow_unsafe_interrupts knob as it is already as some ABI in the best way we can. And the second is how do we make this work in the new world where there may be no type 1 module at all. This patch is not trying to address that topic. I am expecting a range of small adjustments before VFIO_CONTAINER=n becomes really fully viable. > I hate to suggest that type1 becomes a module that does nothing more > than maintain consistency of this variable when the full type1 isn't > available, but is that what we need to do? It is one idea, it depends how literal you want to be on "module parameters are ABI". IMHO it is a weak form of ABI and the need of this paramter in particular is not that common in modern times, AFAIK. So perhaps we just also expose it through vfio.ko and expect people to migrate. That would give a window were both options are available. Jason
Re: [PATCH v4 00/10] drm/msm: Fix math issues in MSM DSC implementation
Hi Marijn On 10/26/2022 11:28 AM, Marijn Suijten wrote: Various removals of complex yet unnecessary math, fixing all uses of drm_dsc_config::bits_per_pixel to deal with the fact that this field includes four fractional bits, and finally making sure that range_bpg_offset contains values 6-bits wide to prevent overflows in drm_dsc_pps_payload_pack(). Altogether this series is responsible for solving _all_ Display Stream Compression issues and artifacts on the Sony Tama (sdm845) Akatsuki smartphone (2880x1440p). Changes since v3: - Swap patch 7 and 8 to make sure msm_host is available inside dsi_populate_dsc_params(); - Reword patch 6 (Migrate to drm_dsc_compute_rc_parameters()) to more clearly explain why the FIXME wasn't solved initially, but why it can (and should!) be resolved now. v3: https://lore.kernel.org/linux-arm-msm/20221009184824.457416-1-marijn.suij...@somainline.org/T/#u Changes since v2: - Generalize mux_word_size setting depending on bits_per_component; - Migrate DSI's DSC calculations to drm_dsc_compute_rc_parameters(), implicitly addressing existing math issues; - Disallow any bits_per_component other than 8, until hardcoded values are updated and tested to support such cases. v2: https://lore.kernel.org/linux-arm-msm/20221005181657.784375-1-marijn.suij...@somainline.org/T/#u Changes since v1: - Propagate r-b's, except (obviously) in patches that were (heavily) modified; - Remove accidental debug code in dsi_cmd_dma_add; - Move Range BPG Offset masking out of DCS PPS packing, back into the DSI driver when it is assigned to drm_dsc_config (this series is now strictly focusing on drm/msm again); - Replace modulo-check resulting in conditional increment with DIV_ROUND_UP; - Remove repeated calculation of slice_chunk_size; - Use u16 instead of int when handling bits_per_pixel; - Use DRM_DEV_ERROR instead of pr_err in DSI code; - Also remove redundant target_bpp_x16 variable. v1: https://lore.kernel.org/linux-arm-msm/20221001190807.358691-1-marijn.suij...@somainline.org/T/#u Marijn Suijten (10): drm/msm/dsi: Remove useless math in DSC calculations drm/msm/dsi: Remove repeated calculation of slice_per_intf drm/msm/dsi: Use DIV_ROUND_UP instead of conditional increment on modulo drm/msm/dsi: Reuse earlier computed dsc->slice_chunk_size drm/msm/dsi: Appropriately set dsc->mux_word_size based on bpc drm/msm/dsi: Migrate to drm_dsc_compute_rc_parameters() drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits drm/msm/dsi: Disallow 8 BPC DSC configuration for alternative BPC values drm/msm/dpu1: Account for DSC's bits_per_pixel having 4 fractional bits drm/msm/dsi: Prevent signed BPG offsets from bleeding into adjacent bits drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 11 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 121 ++--- 2 files changed, 37 insertions(+), 95 deletions(-) -- 2.38.1 To keep the -fixes cycle to have only critical fixes (others are important too but are cleanups), I was thinking of absorbing patches 7,8,9 and 10 alone in the -fixes cycle and for patches 1-6, will go through the 6.2 push. Let me know if there are any concerns if we just take patches 7,8,9 and 10 separately. Thanks Abhinav
Re: [PATCH] mm/memremap: Introduce pgmap_request_folio() using pgmap offsets
On Thu, Oct 20, 2022 at 02:56:39PM -0700, Dan Williams wrote: > A 'struct dev_pagemap' (pgmap) represents a collection of ZONE_DEVICE > pages. The pgmap is a reference counted object that serves a similar > role as a 'struct request_queue'. Live references are obtained for each > in flight request / page, and once a page's reference count drops to > zero the associated pin of the pgmap is dropped as well. While a page is > idle nothing should be accessing it because that is effectively a > use-after-free situation. Unfortunately, all current ZONE_DEVICE > implementations deploy a layering violation to manage requests to > activate pages owned by a pgmap. Specifically, they take steps like walk > the pfns that were previously assigned at memremap_pages() time and use > pfn_to_page() to recall metadata like page->pgmap, or make use of other > data like page->zone_device_data. > > The first step towards correcting that situation is to provide a > API to get access to a pgmap page that does not require the caller to > know the pfn, nor access any fields of an idle page. Ideally this API > would be able to support dynamic page creation instead of the current > status quo of pre-allocating and initializing pages. > > On a prompt from Jason, introduce pgmap_request_folio() that operates on > an offset into a pgmap. It replaces the shortlived > pgmap_request_folios() that was continuing the layering violation of > assuming pages are available to be consulted before asking the pgmap to > make them available. > > For now this only converts the callers to lookup the pgmap and generate > the pgmap offset, but it does not do the deeper cleanup of teaching > those call sites to generate those arguments without walking the page > metadata. For next steps it appears the DEVICE_PRIVATE implementations > could plumb the pgmap into the necessary callsites and switch to using > gen_pool_alloc() to track which offsets of a pgmap are allocated. For > DAX, dax_direct_access() could switch from returning pfns to returning > the associated @pgmap and @pgmap_offset. Those changes are saved for > follow-on work. I like it, though it would be nice to see drivers converted away from pfn_to_pgmap_offset().. > /** > - * pgmap_request_folios - activate an contiguous span of folios in @pgmap > - * @pgmap: host page map for the folio array > - * @folio: start of the folio list, all subsequent folios have same > folio_size() > + * pgmap_request_folio - activate a folio of a given order in @pgmap > + * @pgmap: host page map of the folio to activate > + * @pgmap_offset: page-offset into the pgmap to request > + * @order: expected folio_order() of the folio > * > * Caller is responsible for @pgmap remaining live for the duration of > - * this call. Caller is also responsible for not racing requests for the > - * same folios. > + * this call. The order (size) of the folios in the pgmap are assumed > + * stable before this call. > */ I would probably add some discussion here that this enables refcounting on the folio and the pgmap_ops page free will be called once the folio is no longer being used. And explain that the pgmap user is responsible for tracking which pgmap_offsets are requested and which have been returned by free. It would be nice to say that this can only be called on free'd folios. > -bool pgmap_request_folios(struct dev_pagemap *pgmap, struct folio *folio, > - int nr_folios) > +struct folio *pgmap_request_folio(struct dev_pagemap *pgmap, > + pgoff_t pgmap_offset, int order) unsigned int order? > { > - struct folio *iter; > - int i; > + unsigned long pfn = pgmap_offset_to_pfn(pgmap, pgmap_offset); > + struct page *page = pfn_to_page(pfn); > + struct folio *folio; > + int v; > > - /* > - * All of the WARNs below are for catching bugs in future > - * development that changes the assumptions of: > - * 1/ uniform folios in @pgmap > - * 2/ @pgmap death does not race this routine. > - */ > - VM_WARN_ON_ONCE(!folio_span_valid(pgmap, folio, nr_folios)); > + if (WARN_ON_ONCE(page->pgmap != pgmap)) > + return NULL; Checking that pgmap_offset is not bigger than pgmap length is also a good assertion.. At that point if pgmap is not right then the struct page has been corrupted. > > if (WARN_ON_ONCE(percpu_ref_is_dying(>ref))) > - return false; > + return NULL; > > - for (iter = folio_next(folio), i = 1; i < nr_folios; > - iter = folio_next(folio), i++) > - if (WARN_ON_ONCE(folio_order(iter) != folio_order(folio))) > - return false; > + folio = page_folio(page); > + if (WARN_ON_ONCE(folio_order(folio) != order)) > + return NULL; Do you see a blocker to simply restructuring the pages into head/tail here? If the refcounts are all zero it should be safe? > + v = folio_ref_inc_return(folio); > +
[PULL] drm-intel-next
Hi Dave and Daniel, Here goes the first chunk of drm-intel-next targeting 6.2 The highlight goes to Ville with many display related clean-up and improvement, some other MTL enabling work and many other fixes and small clean-ups. drm-intel-next-2022-10-28: - Hotplug code clean-up and organization (Jani, Gustavo) - More VBT specific code clean-up, doc, organization, and improvements (Ville) - More MTL enabling work (Matt, RK, Anusha, Jose) - FBC related clean-ups and improvements (Ville) - Removing unused sw_fence_await_reservation (Niranjana) - Big chunch of display house clean-up (Ville) - Many Watermark fixes and clean-ups (Ville) - Fix device info for devices without display (Jani) - Fix TC port PLLs after readout (Ville) - DPLL ID clean-ups (Ville) - Prep work for finishing (de)gamma readout (Ville) - PSR fixes and improvements (Jouni, Jose) - Reject excessive dotclocks early (Ville) - DRRS related improvements (Ville) - Simplify uncore register updates (Andrzej) - Fix simulated GPU reset wrt. encoder HW readout (Imre) - Add a ADL-P workaround (Jose) - Fix clear mask in GEN7_MISCCPCTL update (Andrzej) - Temporarily disable runtime_pm for discrete (Anshuman) - Improve fbdev debugs (Nirmoy) - Fix DP FRL link training status (Ankit) - Other small display fixes (Ankit, Suraj) - Allow panel fixed modes to have differing sync polarities (Ville) - Clean up crtc state flag checks (Ville) - Fix race conditions during DKL PHY accesses (Imre) - Prep-work for cdclock squash and crawl modes (Anusha) - ELD precompute and readout (Ville) Thanks, Rodrigo. The following changes since commit 21f0b7dabf9c358e75a539b5554c0375bf1abe0a: drm/i915: Fix return type of mode_valid function hook (2022-09-15 10:28:55 +0300) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-2022-10-28 for you to fetch changes up to a6ebd538364b1e9e6048faaafbc0188172ed50c3: drm/i915/sdvo: Fix debug print (2022-10-28 14:46:21 +0300) - Hotplug code clean-up and organization (Jani, Gustavo) - More VBT specific code clean-up, doc, organization, and improvements (Ville) - More MTL enabling work (Matt, RK, Anusha, Jose) - FBC related clean-ups and improvements (Ville) - Removing unused sw_fence_await_reservation (Niranjana) - Big chunch of display house clean-up (Ville) - Many Watermark fixes and clean-ups (Ville) - Fix device info for devices without display (Jani) - Fix TC port PLLs after readout (Ville) - DPLL ID clean-ups (Ville) - Prep work for finishing (de)gamma readout (Ville) - PSR fixes and improvements (Jouni, Jose) - Reject excessive dotclocks early (Ville) - DRRS related improvements (Ville) - Simplify uncore register updates (Andrzej) - Fix simulated GPU reset wrt. encoder HW readout (Imre) - Add a ADL-P workaround (Jose) - Fix clear mask in GEN7_MISCCPCTL update (Andrzej) - Temporarily disable runtime_pm for discrete (Anshuman) - Improve fbdev debugs (Nirmoy) - Fix DP FRL link training status (Ankit) - Other small display fixes (Ankit, Suraj) - Allow panel fixed modes to have differing sync polarities (Ville) - Clean up crtc state flag checks (Ville) - Fix race conditions during DKL PHY accesses (Imre) - Prep-work for cdclock squash and crawl modes (Anusha) - ELD precompute and readout (Ville) Alan Previn (1): drm/i915/pxp: Add firmware status when ARB session fails Andrzej Hajda (5): drm/i915/display: remove drm_device aliases drm/i915/display: Use intel_uncore alias if defined drm/i915: make intel_uncore_rmw() write unconditionally drm/i915: use proper helper for register updates drm/i915: fix clear mask in GEN7_MISCCPCTL update Ankit Nautiyal (2): drm/i915/dp: Reset frl trained flag before restarting FRL training drm/i915/dp: Remove whitespace at the end of function. Anshuman Gupta (1): drm/i915/dgfx: Keep PCI autosuspend control 'on' by default on all dGPU Anusha Srivatsa (5): drm/i915/display: Add DC5 counter and DMC debugfs entries for MTL drm/i915/display: Change terminology for cdclk actions drm/i915/display: Introduce HAS_CDCLK_SQUASH macro drm/i915/display: Move chunks of code out of bxt_set_cdclk() drm/i915/display: Move squash_ctl register programming to its own function Gustavo Sousa (1): drm/i915: Move hotplug inversion logic into separate helper Imre Deak (6): drm/i915: Fix TypeC mode initialization during system resume drm/i915: Fix simulated GPU reset wrt. encoder HW readout drm/i915/tgl+: Add locking around DKL PHY register accesses drm/i915: Rename intel_tc_phy_regs.h to intel_mg_phy_regs.h drm/i915/tgl+: Move DKL PHY register definitions to intel_dkl_phy_regs.h drm/i915/tgl+: Sanitize DKL PHY register definitions Jani Nikula (4): drm/i915/hotplug: move hotplug storm debugfs to
Re: [PATCH] docs/sphinx: More depth in the rtd sidebar toc
Daniel Vetter writes: > We love to nest our documenation for good structure, but that means > the table of contents needs to keep up or you can't navigate them. > > Realized this trying to find the drm property documentation, which > with some shuffling around disappeared. Why I didn't realize we can do > this earlier, no idea. > > Since the relevant parts of the toc are only loaded if you're in the > right .html file there's no harm in going all the way to unlimited. > > Note that this has no impact on the classic theme (which doesn't have > the sidebar) nor on the various :toctree: rendered inline in the > output. > > Signed-off-by: Daniel Vetter > Cc: Jonathan Corbet > Cc: linux-...@vger.kernel.org > --- > Documentation/conf.py | 4 > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/conf.py b/Documentation/conf.py > index 934727e23e0e..5dc141c66726 100644 > --- a/Documentation/conf.py > +++ b/Documentation/conf.py > @@ -240,6 +240,10 @@ if html_theme == 'sphinx_rtd_theme' or html_theme == > 'sphinx_rtd_dark_mode': > # Add color-specific RTD normal mode > html_css_files.append('theme_rtd_colors.css') > > +html_theme_options = { > +'navigation_depth': -1, > +} > + > except ImportError: > html_theme = 'classic' So this patch isn't against docs-next, and applies to the RTD theme, which is no longer the default. I have no objection to it, but have you looked at how your docs come out with the alabaster theme? Thanks, jon
Re: [PATCH 5/5] drm/i915/mtl: don't expose GSC command streamer to the user
On Fri, Oct 28, 2022 at 10:14:05AM -0700, Ceraolo Spurio, Daniele wrote: > > > On 10/27/2022 8:40 PM, Matt Roper wrote: > > On Thu, Oct 27, 2022 at 03:15:54PM -0700, Daniele Ceraolo Spurio wrote: > > > There is no userspace user for this CS yet, we only need it for internal > > > kernel ops (e.g. HuC, PXP), so don't expose it. > > > > > > Signed-off-by: Daniele Ceraolo Spurio > > > Cc: Matt Roper > > Since we never expose it to userspace, we also never get to the point of > > doing an engine rename and removing the apostrophe. I assume we're okay > > with this engine continuing to show up as "other'6" in debug logs? > > I don't think it matters a lot in debug logs, but anyway it wouldn't be hard > to rename it to something different. What do you suggest to rename it to? > Since OTHER_CLASS doesn't have a uabi_class defined we can't use a count of > engines of that type like we do for the other classes. Just rename it > straight to hardcoded gsc0 ? Yeah, a hardcoded "gsc0" seems fine to me. I agree it doesn't matter too much either way, so I'll leave it up to you whether you add that rename or not. Matt > > Daniele > > > > > Reviewed-by: Matt Roper > > > > > --- > > > drivers/gpu/drm/i915/gt/intel_engine_user.c | 4 > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c > > > b/drivers/gpu/drm/i915/gt/intel_engine_user.c > > > index 79312b734690..ca795daca116 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c > > > @@ -211,6 +211,10 @@ void intel_engines_driver_register(struct > > > drm_i915_private *i915) > > > if (intel_gt_has_unrecoverable_error(engine->gt)) > > > continue; /* ignore incomplete engines */ > > > + /* don't expose GSC engine to user */ > > > + if (engine->class == OTHER_CLASS) > > > + continue; > > > + > > > GEM_BUG_ON(engine->class >= ARRAY_SIZE(uabi_classes)); > > > engine->uabi_class = uabi_classes[engine->class]; > > > -- > > > 2.37.3 > > > > -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation
Re: Try to address the DMA-buf coherency problem
Hi Nicolas, Am 28.10.22 um 17:46 schrieb Nicolas Dufresne: Hi, just dropping some real live use case, sorry I'm not really proposing solutions, I believe you are much more knowledgeable in this regard. Well, I think each of us has a lot of specialized knowledge. For example I don't know to much about gralloc/minigbm. So this input is very valuable. Le vendredi 28 octobre 2022 à 16:26 +0200, Christian König a écrit : Am 28.10.22 um 13:42 schrieb Lucas Stach: Am Freitag, dem 28.10.2022 um 10:40 +0200 schrieb Christian König: But essentially the right thing to do. The only alternative I can see is to reverse the role of exporter and importer. I don't think that would work generally either, as buffer exporter and importer isn't always a 1:1 thing. As soon as any attached importer has a different coherency behavior than the others, things fall apart. I've just mentioned it because somebody noted that when you reverse the roles of exporter and importer with the V4L driver and i915 then the use case suddenly starts working. Though, its not generically possible to reverse these roles. If you want to do so, you endup having to do like Android (gralloc) and ChromeOS (minigbm), because you will have to allocate DRM buffers that knows about importer specific requirements. See link [1] for what it looks like for RK3399, with Motion Vector size calculation copied from the kernel driver into a userspace lib (arguably that was available from V4L2 sizeimage, but this is technically difficult to communicate within the software layers). If you could let the decoder export (with proper cache management) the non-generic code would not be needed. Yeah, but I can also reverse the argument: Getting the parameters for V4L right so that we can share the image is tricky, but getting the parameters so that the stuff is actually directly displayable by GPUs is even trickier. Essentially you need to look at both sides and interference to get to a common ground, e.g. alignment, pitch, width/height, padding, etc. Deciding from which side to allocate from is just one step in this process. For example most dGPUs can't display directly from system memory altogether, but it is possible to allocate the DMA-buf through the GPU driver and then write into device memory with P2P PCI transfers. So as far as I can see switching importer and exporter roles and even having performant extra fallbacks should be a standard feature of userspace. Another case where reversing the role is difficult is for case where you need to multiplex the streams (let's use a camera to illustrate) and share that with multiple processes. In these uses case, the DRM importers are volatile, which one do you abuse to do allocation from ? In multimedia server like PipeWire, you are not really aware if the camera will be used by DRM or not, and if something "special" is needed in term of role inversion. It is relatively easy to deal with matching modifiers, but using downstream (display/gpu) as an exporter is always difficult (and require some level of abuse and guessing). Oh, very good point! Yeah we do have use cases for this where an input buffer is both displayed as well as encoded. Well, no. What I mean with coherency is that the devices don't need insert special operation to access each others data. This can be archived by multiple approaches, e.g. by the PCI coherency requirements, device internal connections (XGMI, NVLink, CXL etc...) as well as using uncached system memory. The key point is what we certainly don't want is special operations which say: Ok, now device A can access the data, now device B. because this breaks tons of use cases. I'm coming back again with the multiplexing case. We keep having mixed uses case with multiple receiver. In some case, data may endup on CPU while being encoded in HW. Current approach of disabling cache does work, but CPU algorithm truly suffer in performance. Doing a full memcpy to a cached buffer helps, but remains slower then if the cache had been snooped by the importer (encoder here) driver. Yeah, that was another reason why we ended up rather having an extra copy than working with uncached buffers for display as well. Regards, Christian.
Re: [PATCH v1 1/7] vfio/ccw: create a parent struct
On 10/19/22 12:21 PM, Eric Farman wrote: > Move the stuff associated with the mdev parent (and thus the > subchannel struct) into its own struct, and leave the rest in > the existing private structure. > > The subchannel will point to the parent, and the parent will point > to the private, for the areas where one or both are needed. Further > separation of these structs will follow. > > Signed-off-by: Eric Farman > --- > drivers/s390/cio/vfio_ccw_drv.c | 104 > drivers/s390/cio/vfio_ccw_ops.c | 9 ++- > drivers/s390/cio/vfio_ccw_parent.h | 28 > drivers/s390/cio/vfio_ccw_private.h | 5 -- > 4 files changed, 112 insertions(+), 34 deletions(-) > create mode 100644 drivers/s390/cio/vfio_ccw_parent.h > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c > index 7f5402fe857a..634760ca0dea 100644 > --- a/drivers/s390/cio/vfio_ccw_drv.c > +++ b/drivers/s390/cio/vfio_ccw_drv.c > @@ -20,6 +20,7 @@ > #include "chp.h" > #include "ioasm.h" > #include "css.h" > +#include "vfio_ccw_parent.h" > #include "vfio_ccw_private.h" > > struct workqueue_struct *vfio_ccw_work_q; > @@ -36,7 +37,8 @@ debug_info_t *vfio_ccw_debug_trace_id; > */ > int vfio_ccw_sch_quiesce(struct subchannel *sch) > { > - struct vfio_ccw_private *private = dev_get_drvdata(>dev); > + struct vfio_ccw_parent *parent = dev_get_drvdata(>dev); > + struct vfio_ccw_private *private = dev_get_drvdata(>dev); > DECLARE_COMPLETION_ONSTACK(completion); > int iretry, ret = 0; > > @@ -51,19 +53,21 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch) > break; > } > > - /* > - * Flush all I/O and wait for > - * cancel/halt/clear completion. > - */ > - private->completion = > - spin_unlock_irq(sch->lock); > + if (private) { Is it valid to ever reach this code with private == NULL? If no, then this should probably be a WARN_ON upfront? > + /* > + * Flush all I/O and wait for > + * cancel/halt/clear completion. > + */ > + private->completion = > + spin_unlock_irq(sch->lock); > > - if (ret == -EBUSY) > - wait_for_completion_timeout(, 3*HZ); > + if (ret == -EBUSY) > + wait_for_completion_timeout(, 3*HZ); > > - private->completion = NULL; > - flush_workqueue(vfio_ccw_work_q); > - spin_lock_irq(sch->lock); > + private->completion = NULL; > + flush_workqueue(vfio_ccw_work_q); > + spin_lock_irq(sch->lock); > + } > ret = cio_disable_subchannel(sch); > } while (ret == -EBUSY); > .. snip .. > diff --git a/drivers/s390/cio/vfio_ccw_parent.h > b/drivers/s390/cio/vfio_ccw_parent.h > new file mode 100644 > index ..834c00077802 > --- /dev/null > +++ b/drivers/s390/cio/vfio_ccw_parent.h > @@ -0,0 +1,28 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * MDEV Parent contents for vfio_ccw driver > + * > + * Copyright IBM Corp. 2022 > + */ > + > +#ifndef _VFIO_CCW_PARENT_H_ > +#define _VFIO_CCW_PARENT_H_ > + > +#include > + > +/** > + * struct vfio_ccw_parent > + * > + * @dev: embedded device struct > + * @parent: parent data structures for mdevs created > + * @mdev_type(s): identifying information for mdevs created > + */ > +struct vfio_ccw_parent { > + struct device dev; > + > + struct mdev_parent parent; > + struct mdev_typemdev_type; > + struct mdev_type*mdev_types[1]; > +}; Structure itself seems fine, but any reason we need a new file for it?
[PATCH] staging: fbtft: Use ARRAY_SIZE() to get argument count
The ARRAY_SIZE(foo) macro should be preferred over sizeof operator based computation such as sizeof(foo)/sizeof(foo[0]) for finding number of elements in an array. Issue identified using coccicheck. Signed-off-by: Deepak R Varma --- drivers/staging/fbtft/fbtft.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h index 2c2b5f1c1df3..5506a473be91 100644 --- a/drivers/staging/fbtft/fbtft.h +++ b/drivers/staging/fbtft/fbtft.h @@ -231,7 +231,7 @@ struct fbtft_par { bool polarity; }; -#define NUMARGS(...) (sizeof((int[]){__VA_ARGS__}) / sizeof(int)) +#define NUMARGS(...) ARRAY_SIZE(((int[]){ __VA_ARGS__ })) #define write_reg(par, ...)\ ((par)->fbtftops.write_register(par, NUMARGS(__VA_ARGS__), __VA_ARGS__)) -- 2.34.1
[PATCH v3] drm/tests: Add back seed value information
As reported by Michał the drm_mm and drm_buddy unit tests lost the printk with seed value after they were refactored into KUnit. Add kunit_info with seed value information to assure reproducibility. Reported-by: Michał Winiarski Signed-off-by: Arthur Grillo --- v1->v2: https://lore.kernel.org/all/20221026211458.68432-1-arthurgri...@riseup.net/ - Correct compilation issues - Change tags order - Remove useless line change - Write commit message in imperative form - Remove redundant message part - Correct some grammars nits - Correct checkpatch issues v2->v3: https://lore.kernel.org/all/20221027142903.200169-1-arthurgri...@riseup.net/ - Change .init to .suite_init - Correct some grammars nits --- drivers/gpu/drm/tests/drm_buddy_test.c | 6 -- drivers/gpu/drm/tests/drm_mm_test.c| 8 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c index 62f69589a72d..90ec5e8a485b 100644 --- a/drivers/gpu/drm/tests/drm_buddy_test.c +++ b/drivers/gpu/drm/tests/drm_buddy_test.c @@ -726,11 +726,13 @@ static void drm_test_buddy_alloc_limit(struct kunit *test) drm_buddy_fini(); } -static int drm_buddy_init_test(struct kunit *test) +static int drm_buddy_init_suite(struct kunit_suite *suite) { while (!random_seed) random_seed = get_random_u32(); + kunit_info(suite, "Testing DRM buddy manager, with random_seed=0x%x\n", random_seed); + return 0; } @@ -746,7 +748,7 @@ static struct kunit_case drm_buddy_tests[] = { static struct kunit_suite drm_buddy_test_suite = { .name = "drm_buddy", - .init = drm_buddy_init_test, + .suite_init = drm_buddy_init_suite, .test_cases = drm_buddy_tests, }; diff --git a/drivers/gpu/drm/tests/drm_mm_test.c b/drivers/gpu/drm/tests/drm_mm_test.c index c4b66eeae203..97b9e0cd3e91 100644 --- a/drivers/gpu/drm/tests/drm_mm_test.c +++ b/drivers/gpu/drm/tests/drm_mm_test.c @@ -2209,11 +2209,15 @@ static void drm_test_mm_color_evict_range(struct kunit *test) vfree(nodes); } -static int drm_mm_init_test(struct kunit *test) +static int drm_mm_init_suite(struct kunit_suite *suite) { while (!random_seed) random_seed = get_random_u32(); + kunit_info(sute, + "Testing DRM range manager, with random_seed=0x%x max_iterations=%u max_prime=%u\n", + random_seed, max_iterations, max_prime); + return 0; } @@ -2246,7 +2250,7 @@ static struct kunit_case drm_mm_tests[] = { static struct kunit_suite drm_mm_test_suite = { .name = "drm_mm", - .init = drm_mm_init_test, + .suite_init = drm_mm_init_suite, .test_cases = drm_mm_tests, }; -- 2.37.3
Re: [PATCH v3] dma-buf: cma_heap: Remove duplicated 'by' in comment
Hi, On 10/28/2022 12:25 PM, Mark-PK Tsai wrote: Remove duplicated 'by' from comment in cma_heap_allocate(). Signed-off-by: Mark-PK Tsai --- drivers/dma-buf/heaps/cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 28fb04eccdd0..cd386ce639f3 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -316,7 +316,7 @@ static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, kunmap_atomic(vaddr); /* * Avoid wasting time zeroing memory if the process -* has been killed by by SIGKILL +* has been killed by SIGKILL */ if (fatal_signal_pending(current)) goto free_cma; LGTM. Reviewed-By: Mukesh Ojha -Mukesh
Re: [PATCH v7 00/10] drm: bridge: Add Samsung MIPI DSIM bridge
Hello Jagan, On 10/5/22 17:12, Jagan Teki wrote: This series supports common bridge support for Samsung MIPI DSIM which is used in Exynos and i.MX8MM SoC's. The final bridge supports both the Exynos and i.MX8MM DSI devices. Changes for v7: * fix the drm bridge attach chain for exynos drm dsi driver * fix the hw_type checking logic Changes for v6: * handle previous bridge for exynos dsi while attaching bridge Changes for v5: * bridge changes to support multi-arch * updated and clear commit messages * add hw_type via plat data * removed unneeded quirk * rebased on linux-next Changes for v4: * include Inki Dae in MAINTAINERS * remove dsi_driver probe in exynos_drm_drv to support multi-arch build * update init handling to ensure host init done on first cmd transfer Changes for v3: * fix the mult-arch build * fix dsi host init * updated commit messages Changes for v2: * fix bridge handling * fix dsi host init * correct the commit messages Patch 0001: Samsung DSIM bridge Patch 0002: PHY optional Patch 0003: OF-graph or Child node lookup Patch 0004: DSI host initialization Patch 0005: atomic check Patch 0006: PMS_P offset via plat data Patch 0007: atomic_get_input_bus_fmts Patch 0008: input_bus_flags Patch 0009: document fsl,imx8mm-mipi-dsim Patch 0010: add i.MX8MM DSIM support Tested in Engicam i.Core MX8M Mini SoM. Repo: https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v7 Any inputs? I tried this series on Armadeus OPOS8MM Dev (i.MX8MM) board with the PowerTrip PH720128T004-ZBC02 DSI panel (720x1280, 2 lanes). It works after I fixed the logic of some video mode flags: in function samsung_dsim_init_link(struct samsung_dsim *dsi): -if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP)) +if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP) reg |= DSIM_HFP_MODE; The bit has to be set to disable HFP. Same logic for HPB, HSA and EOT packets. Regards, Jagan. Jagan Teki (10): drm: bridge: Add Samsung DSIM bridge driver drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices drm: bridge: samsung-dsim: Mark PHY as optional drm: bridge: samsung-dsim: Handle proper DSI host initialization drm: bridge: samsung-dsim: Add atomic_check drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts drm: bridge: samsung-dsim: Add input_bus_flags dt-bindings: display: exynos: dsim: Add NXP i.MX8MM support drm: bridge: samsung-dsim: Add i.MX8MM support .../bindings/display/exynos/exynos_dsim.txt |1 + MAINTAINERS |9 + drivers/gpu/drm/bridge/Kconfig| 12 + drivers/gpu/drm/bridge/Makefile |1 + drivers/gpu/drm/bridge/samsung-dsim.c | 1856 + drivers/gpu/drm/exynos/Kconfig|1 + drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1766 +--- include/drm/bridge/samsung-dsim.h | 115 + 8 files changed, 2108 insertions(+), 1653 deletions(-) create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c create mode 100644 include/drm/bridge/samsung-dsim.h -- Sébastien Szymanski, Armadeus Systems Software engineer
[PATCH v4] drm/tests: Add back seed value information
As reported by Michał the drm_mm and drm_buddy unit tests lost the printk with seed value after they were refactored into KUnit. Add kunit_info with seed value information to assure reproducibility. Reported-by: Michał Winiarski Signed-off-by: Arthur Grillo --- v1->v2: https://lore.kernel.org/all/20221026211458.68432-1-arthurgri...@riseup.net/ - Correct compilation issues - Change tags order - Remove useless line change - Write commit message in imperative form - Remove redundant message part - Correct some grammars nits - Correct checkpatch issues v2->v3: https://lore.kernel.org/all/20221027142903.200169-1-arthurgri...@riseup.net/ - Change .init to .suite_init - Correct some grammars nits v3->v4: - Correct compilation issues --- drivers/gpu/drm/tests/drm_buddy_test.c | 6 -- drivers/gpu/drm/tests/drm_mm_test.c| 8 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c index 62f69589a72d..90ec5e8a485b 100644 --- a/drivers/gpu/drm/tests/drm_buddy_test.c +++ b/drivers/gpu/drm/tests/drm_buddy_test.c @@ -726,11 +726,13 @@ static void drm_test_buddy_alloc_limit(struct kunit *test) drm_buddy_fini(); } -static int drm_buddy_init_test(struct kunit *test) +static int drm_buddy_init_suite(struct kunit_suite *suite) { while (!random_seed) random_seed = get_random_u32(); + kunit_info(suite, "Testing DRM buddy manager, with random_seed=0x%x\n", random_seed); + return 0; } @@ -746,7 +748,7 @@ static struct kunit_case drm_buddy_tests[] = { static struct kunit_suite drm_buddy_test_suite = { .name = "drm_buddy", - .init = drm_buddy_init_test, + .suite_init = drm_buddy_init_suite, .test_cases = drm_buddy_tests, }; diff --git a/drivers/gpu/drm/tests/drm_mm_test.c b/drivers/gpu/drm/tests/drm_mm_test.c index c4b66eeae203..4663e4611976 100644 --- a/drivers/gpu/drm/tests/drm_mm_test.c +++ b/drivers/gpu/drm/tests/drm_mm_test.c @@ -2209,11 +2209,15 @@ static void drm_test_mm_color_evict_range(struct kunit *test) vfree(nodes); } -static int drm_mm_init_test(struct kunit *test) +static int drm_mm_init_suite(struct kunit_suite *suite) { while (!random_seed) random_seed = get_random_u32(); + kunit_info(suite, + "Testing DRM range manager, with random_seed=0x%x max_iterations=%u max_prime=%u\n", + random_seed, max_iterations, max_prime); + return 0; } @@ -2246,7 +2250,7 @@ static struct kunit_case drm_mm_tests[] = { static struct kunit_suite drm_mm_test_suite = { .name = "drm_mm", - .init = drm_mm_init_test, + .suite_init = drm_mm_init_suite, .test_cases = drm_mm_tests, }; -- 2.37.3
Re: [PATCH] [next] drm/amdgpu: Replace one-element array with flexible-array member
Am 28.10.22 um 18:36 schrieb Kees Cook: On Fri, Oct 28, 2022 at 09:18:39AM +0200, Christian König wrote: Am 28.10.22 um 07:10 schrieb Paulo Miguel Almeida: One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element array with flexible-array member in struct _ATOM_FAKE_EDID_PATCH_RECORD and refactor the rest of the code accordingly. This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1]. Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FKSPP%2Flinux%2Fissues%2F79data=05%7C01%7Cchristian.koenig%40amd.com%7C600d3657cbe441ae969d08dab9028c1c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638025717852262567%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=h78kYVA3ee9fDDwD5XGNgYJuUAZtBitVpk2354cOLO4%3Dreserved=0 Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FKSPP%2Flinux%2Fissues%2F238data=05%7C01%7Cchristian.koenig%40amd.com%7C600d3657cbe441ae969d08dab9028c1c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638025717852262567%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=k1k7LwxIxIw5c9QM3gM2pA9DVGF4Kz20IJWs5tY4xzE%3Dreserved=0 Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D101836data=05%7C01%7Cchristian.koenig%40amd.com%7C600d3657cbe441ae969d08dab9028c1c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638025717852262567%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=LJB4Rs1xOE82UpLbqtZOgPgi7OmvR02T9fikpKamdiY%3Dreserved=0 [1] I'm not sure if that's a good idea. We had multiple attempts to refactor this now and it always caused a regression. Additional to that the header in question came from our BIOS team and isn't following Linux styles in general. Alex what do you think? Fake flexible arrays (i.e. 1-element arrays) are deprecated in Linux[1] (and, frankly, deprecated in C since 1999 and even well before then given the 0-sized extension that was added in GCC), so we can't continue to bring them into kernel sources. Their use breaks both compile-time and run-time bounds checking efforts, etc. I'm perfectly aware of that. The issue is that we have tried to fix this multiple times now and reverted back to the original behavior because some user with a 10-15 year old hardware complained that it broke his system. We can't really test every hw generation of the last 15 years for regressions. All that said, converting away from them can be tricky, and I think such conversions need to explicitly show how they were checked for binary differences[2]. Oh, that's a great idea! Yes, if this can be guaranteed then the change is obviously perfectly ok. Thanks, Christian. Paulo, can you please check for deltas and report your findings in the commit log? Note that add struct_size() use in the same patch may result in binary differences, so for more complex cases, you may want to split the 1-element conversion from the struct_size() conversions. -Kees [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.kernel.org%2Fprocess%2Fdeprecated.html%23zero-length-and-one-element-arraysdata=05%7C01%7Cchristian.koenig%40amd.com%7C600d3657cbe441ae969d08dab9028c1c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638025717852262567%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=6v1qt3zMrSTFDgnF9TO3aurqvG1fPjH2grRu47e2tEA%3Dreserved=0 [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Foutflux.net%2Fblog%2Farchives%2F2022%2F06%2F24%2Ffinding-binary-differences%2Fdata=05%7C01%7Cchristian.koenig%40amd.com%7C600d3657cbe441ae969d08dab9028c1c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638025717852262567%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=g3yCIXBAD0OJwK5EdxRfJVeSBevbA1WOeyFM%2BiZC%2F%2FM%3Dreserved=0
Re: [PATCH v1 1/7] vfio/ccw: create a parent struct
On Fri, 2022-10-28 at 12:51 -0400, Matthew Rosato wrote: > On 10/19/22 12:21 PM, Eric Farman wrote: > > Move the stuff associated with the mdev parent (and thus the > > subchannel struct) into its own struct, and leave the rest in > > the existing private structure. > > > > The subchannel will point to the parent, and the parent will point > > to the private, for the areas where one or both are needed. Further > > separation of these structs will follow. > > > > Signed-off-by: Eric Farman > > --- > > drivers/s390/cio/vfio_ccw_drv.c | 104 > > > > drivers/s390/cio/vfio_ccw_ops.c | 9 ++- > > drivers/s390/cio/vfio_ccw_parent.h | 28 > > drivers/s390/cio/vfio_ccw_private.h | 5 -- > > 4 files changed, 112 insertions(+), 34 deletions(-) > > create mode 100644 drivers/s390/cio/vfio_ccw_parent.h > > > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c > > b/drivers/s390/cio/vfio_ccw_drv.c > > index 7f5402fe857a..634760ca0dea 100644 > > --- a/drivers/s390/cio/vfio_ccw_drv.c > > +++ b/drivers/s390/cio/vfio_ccw_drv.c > > @@ -20,6 +20,7 @@ > > #include "chp.h" > > #include "ioasm.h" > > #include "css.h" > > +#include "vfio_ccw_parent.h" > > #include "vfio_ccw_private.h" > > > > struct workqueue_struct *vfio_ccw_work_q; > > @@ -36,7 +37,8 @@ debug_info_t *vfio_ccw_debug_trace_id; > > */ > > int vfio_ccw_sch_quiesce(struct subchannel *sch) > > { > > - struct vfio_ccw_private *private = dev_get_drvdata( > > >dev); > > + struct vfio_ccw_parent *parent = dev_get_drvdata( > > >dev); > > + struct vfio_ccw_private *private = dev_get_drvdata( > > >dev); > > DECLARE_COMPLETION_ONSTACK(completion); > > int iretry, ret = 0; > > > > @@ -51,19 +53,21 @@ int vfio_ccw_sch_quiesce(struct subchannel > > *sch) > > break; > > } > > > > - /* > > - * Flush all I/O and wait for > > - * cancel/halt/clear completion. > > - */ > > - private->completion = > > - spin_unlock_irq(sch->lock); > > + if (private) { > > Is it valid to ever reach this code with private == NULL? If no, > then this should probably be a WARN_ON upfront? Hrm, the caller jumps from private -> subchannel, so it would be weird if we couldn't then go back the other way. Probably impossible, I'll unwind these whitespace changes and put the WARN_ON on top. Thanks for the tip. > > > + /* > > + * Flush all I/O and wait for > > + * cancel/halt/clear completion. > > + */ > > + private->completion = > > + spin_unlock_irq(sch->lock); > > > > - if (ret == -EBUSY) > > - wait_for_completion_timeout(, > > 3*HZ); > > + if (ret == -EBUSY) > > + wait_for_completion_timeout( > > tion, 3*HZ); > > > > - private->completion = NULL; > > - flush_workqueue(vfio_ccw_work_q); > > - spin_lock_irq(sch->lock); > > + private->completion = NULL; > > + flush_workqueue(vfio_ccw_work_q); > > + spin_lock_irq(sch->lock); > > + } > > ret = cio_disable_subchannel(sch); > > } while (ret == -EBUSY); > > > > .. snip .. > > > diff --git a/drivers/s390/cio/vfio_ccw_parent.h > > b/drivers/s390/cio/vfio_ccw_parent.h > > new file mode 100644 > > index ..834c00077802 > > --- /dev/null > > +++ b/drivers/s390/cio/vfio_ccw_parent.h > > @@ -0,0 +1,28 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * MDEV Parent contents for vfio_ccw driver > > + * > > + * Copyright IBM Corp. 2022 > > + */ > > + > > +#ifndef _VFIO_CCW_PARENT_H_ > > +#define _VFIO_CCW_PARENT_H_ > > + > > +#include > > + > > +/** > > + * struct vfio_ccw_parent > > + * > > + * @dev: embedded device struct > > + * @parent: parent data structures for mdevs created > > + * @mdev_type(s): identifying information for mdevs created > > + */ > > +struct vfio_ccw_parent { > > + struct device dev; > > + > > + struct mdev_parent parent; > > + struct mdev_typemdev_type; > > + struct mdev_type*mdev_types[1]; > > +}; > > Structure itself seems fine, but any reason we need a new file for > it? > Not really. I could leave it in _private.h, but that file is just a dumping ground for everything so I thought this would be a good opportunity to start to cleaning that up. But it wouldn't bother me to leave that whole process to another day too.
Re: [PATCH 5/5] drm/i915/mtl: don't expose GSC command streamer to the user
On 10/27/2022 8:40 PM, Matt Roper wrote: On Thu, Oct 27, 2022 at 03:15:54PM -0700, Daniele Ceraolo Spurio wrote: There is no userspace user for this CS yet, we only need it for internal kernel ops (e.g. HuC, PXP), so don't expose it. Signed-off-by: Daniele Ceraolo Spurio Cc: Matt Roper Since we never expose it to userspace, we also never get to the point of doing an engine rename and removing the apostrophe. I assume we're okay with this engine continuing to show up as "other'6" in debug logs? I don't think it matters a lot in debug logs, but anyway it wouldn't be hard to rename it to something different. What do you suggest to rename it to? Since OTHER_CLASS doesn't have a uabi_class defined we can't use a count of engines of that type like we do for the other classes. Just rename it straight to hardcoded gsc0 ? Daniele Reviewed-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_engine_user.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c index 79312b734690..ca795daca116 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c @@ -211,6 +211,10 @@ void intel_engines_driver_register(struct drm_i915_private *i915) if (intel_gt_has_unrecoverable_error(engine->gt)) continue; /* ignore incomplete engines */ + /* don't expose GSC engine to user */ + if (engine->class == OTHER_CLASS) + continue; + GEM_BUG_ON(engine->class >= ARRAY_SIZE(uabi_classes)); engine->uabi_class = uabi_classes[engine->class]; -- 2.37.3
Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support
On 10/28/2022 1:38 AM, Tvrtko Ursulin wrote: On 27/10/2022 23:15, Daniele Ceraolo Spurio wrote: The GSC CS re-uses the same interrupt bits that the GSC used in older platforms. This means that we can now have an engine interrupt coming out of OTHER_CLASS, so we need to handle that appropriately. Signed-off-by: Daniele Ceraolo Spurio Cc: Matt Roper --- drivers/gpu/drm/i915/gt/intel_gt_irq.c | 78 ++ 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c index f26882fdc24c..34ff1ee7e931 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c @@ -81,35 +81,27 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8 instance, instance, iir); } -static void -gen11_engine_irq_handler(struct intel_gt *gt, const u8 class, - const u8 instance, const u16 iir) +static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 instance) { - struct intel_engine_cs *engine; - - /* - * Platforms with standalone media have their media engines in another - * GT. - */ - if (MEDIA_VER(gt->i915) >= 13 && - (class == VIDEO_DECODE_CLASS || class == VIDEO_ENHANCEMENT_CLASS)) { - if (!gt->i915->media_gt) - goto err; + struct intel_gt *media_gt = gt->i915->media_gt; - gt = gt->i915->media_gt; + /* we expect the non-media gt to be passed in */ + GEM_BUG_ON(gt == media_gt); + + if (!media_gt) + return gt; + + switch (class) { + case VIDEO_DECODE_CLASS: + case VIDEO_ENHANCEMENT_CLASS: + return media_gt; + case OTHER_CLASS: + if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, GSC0)) + return media_gt; + fallthrough; + default: + return gt; } - - if (instance <= MAX_ENGINE_INSTANCE) - engine = gt->engine_class[class][instance]; - else - engine = NULL; - - if (likely(engine)) - return intel_engine_cs_irq(engine, iir); - -err: - WARN_ONCE(1, "unhandled engine interrupt class=0x%x, instance=0x%x\n", - class, instance); } static void @@ -118,12 +110,24 @@ gen11_gt_identity_handler(struct intel_gt *gt, const u32 identity) const u8 class = GEN11_INTR_ENGINE_CLASS(identity); const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity); const u16 intr = GEN11_INTR_ENGINE_INTR(identity); + struct intel_engine_cs *engine; if (unlikely(!intr)) return; - if (class <= COPY_ENGINE_CLASS || class == COMPUTE_CLASS) - return gen11_engine_irq_handler(gt, class, instance, intr); + /* + * Platforms with standalone media have the media and GSC engines in + * another GT. + */ + gt = pick_gt(gt, class, instance); + + if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) + engine = gt->engine_class[class][instance]; + else + engine = NULL; + + if (engine) + return intel_engine_cs_irq(engine, intr); Drive by observation - you could fold the above two ifs into one since engine appears unused afterwards. engine can be NULL in both branches of the if statement, so to get a unified if we'd have to do something like: if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) { struct intel_engine_cs *engine = gt->engine_class[class][instance]; if (engine) return intel_engine_cs_irq(engine, intr); } Is this what you are suggesting? Daniele Regards, Tvrtko if (class == OTHER_CLASS) return gen11_other_irq_handler(gt, instance, intr); @@ -206,7 +210,7 @@ void gen11_gt_irq_reset(struct intel_gt *gt) intel_uncore_write(uncore, GEN11_VCS_VECS_INTR_ENABLE, 0); if (CCS_MASK(gt)) intel_uncore_write(uncore, GEN12_CCS_RSVD_INTR_ENABLE, 0); - if (HAS_HECI_GSC(gt->i915)) + if (HAS_HECI_GSC(gt->i915) || HAS_ENGINE(gt, GSC0)) intel_uncore_write(uncore, GEN11_GUNIT_CSME_INTR_ENABLE, 0); /* Restore masks irqs on RCS, BCS, VCS and VECS engines. */ @@ -233,7 +237,7 @@ void gen11_gt_irq_reset(struct intel_gt *gt) intel_uncore_write(uncore, GEN12_CCS0_CCS1_INTR_MASK, ~0); if (HAS_ENGINE(gt, CCS2) || HAS_ENGINE(gt, CCS3)) intel_uncore_write(uncore, GEN12_CCS2_CCS3_INTR_MASK, ~0); - if (HAS_HECI_GSC(gt->i915)) + if (HAS_HECI_GSC(gt->i915) || HAS_ENGINE(gt, GSC0)) intel_uncore_write(uncore, GEN11_GUNIT_CSME_INTR_MASK, ~0); intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_ENABLE, 0); @@ -249,7 +253,7 @@ void gen11_gt_irq_postinstall(struct intel_gt *gt) { struct intel_uncore *uncore = gt->uncore; u32 irqs = GT_RENDER_USER_INTERRUPT; - const u32 gsc_mask = GSC_IRQ_INTF(0) | GSC_IRQ_INTF(1); + u32 gsc_mask = 0; u32 dmask; u32 smask; @@ -261,6 +265,11 @@ void
Re: [PATCH] [next] drm/amdgpu: Replace one-element array with flexible-array member
On Fri, Oct 28, 2022 at 09:18:39AM +0200, Christian König wrote: > Am 28.10.22 um 07:10 schrieb Paulo Miguel Almeida: > > One-element arrays are deprecated, and we are replacing them with > > flexible array members instead. So, replace one-element array with > > flexible-array member in struct _ATOM_FAKE_EDID_PATCH_RECORD and > > refactor the rest of the code accordingly. > > > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE > > routines on memcpy() and help us make progress towards globally > > enabling -fstrict-flex-arrays=3 [1]. > > > > Link: https://github.com/KSPP/linux/issues/79 > > Link: https://github.com/KSPP/linux/issues/238 > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] > > I'm not sure if that's a good idea. We had multiple attempts to refactor > this now and it always caused a regression. > > Additional to that the header in question came from our BIOS team and isn't > following Linux styles in general. > > Alex what do you think? Fake flexible arrays (i.e. 1-element arrays) are deprecated in Linux[1] (and, frankly, deprecated in C since 1999 and even well before then given the 0-sized extension that was added in GCC), so we can't continue to bring them into kernel sources. Their use breaks both compile-time and run-time bounds checking efforts, etc. All that said, converting away from them can be tricky, and I think such conversions need to explicitly show how they were checked for binary differences[2]. Paulo, can you please check for deltas and report your findings in the commit log? Note that add struct_size() use in the same patch may result in binary differences, so for more complex cases, you may want to split the 1-element conversion from the struct_size() conversions. -Kees [1] https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays [2] https://outflux.net/blog/archives/2022/06/24/finding-binary-differences/ -- Kees Cook
Re: [PATCH v3] dma-buf: cma_heap: Remove duplicated 'by' in comment
On Thu, Oct 27, 2022 at 11:55 PM Mark-PK Tsai wrote: > > Remove duplicated 'by' from comment in cma_heap_allocate(). > > Signed-off-by: Mark-PK Tsai Thanks for sending this and going through a few iterations! Acked-by: John Stultz -john
Re: [PATCH v3 1/7] drm/ivpu: Introduce a new DRM driver for Intel VPU
Hi, thanks for in-depth review. On 10/25/2022 2:38 PM, Thomas Zimmermann wrote: > Hi, > > please find some review comments below. > > Am 24.09.22 um 17:11 schrieb Jacek Lawrynowicz: >> +static int ivpu_irq_init(struct ivpu_device *vdev) >> +{ >> + struct pci_dev *pdev = to_pci_dev(vdev->drm.dev); >> + int ret; >> + >> + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI | PCI_IRQ_MSIX); >> + if (ret < 0) { >> + ivpu_err(vdev, "Failed to allocate and MSI IRQ: %d\n", ret); >> + return ret; >> + } >> + >> + vdev->irq = pci_irq_vector(pdev, 0); >> + >> + ret = request_irq(vdev->irq, vdev->hw->ops->irq_handler, IRQF_SHARED, >> + DRIVER_NAME, vdev); > > There's devm_request_irq(). Within DRM, we have mostly adopted managed > cleanup. New drivers should be written that way. Sure, makes sense. >> + if (ret) { >> + ivpu_err(vdev, "Failed to request an IRQ %d\n", ret); >> + pci_free_irq_vectors(pdev); >> + } >> + >> + return ret; >> +} >> + >> +static void ivpu_irq_fini(struct ivpu_device *vdev) > > All these _fini functions should be replaced by managed cleanups with the > devm_, drmm_ and pcim_. There are sometimes multiple ways of supporting this, > but manually calling _fini should be avoided. Yes, with devm ivpu_irq_fini() is no longer needed. >> +{ >> + free_irq(vdev->irq, vdev); >> + pci_free_irq_vectors(to_pci_dev(vdev->drm.dev)); > > There's no pcim_alloc_irq_vectors(). It's better to add one or at least > provide such an implementation within your driver. It is not actually needed. pci_alloc_irq_vectors() is implicitly managed. It calls pcim_setup_msi_release() that will free irq vectors. >> +} >> + >> +static int ivpu_pci_init(struct ivpu_device *vdev) >> +{ >> + struct pci_dev *pdev = to_pci_dev(vdev->drm.dev); >> + struct resource *bar0 = >resource[0]; >> + struct resource *bar4 = >resource[4]; >> + int ret; >> + >> + ivpu_dbg(MISC, "Mapping BAR0 (RegV) %pR\n", bar0); >> + vdev->regv = devm_ioremap_resource(vdev->drm.dev, bar0); >> + if (IS_ERR(vdev->regv)) { >> + ivpu_err(vdev, "Failed to map bar 0\n"); >> + return -ENOMEM; > > Why not return PTR_ERR(vdev->regv) ? Yes, PTR_ERR should be returned here. >> + } >> + >> + ivpu_dbg(MISC, "Mapping BAR4 (RegB) %pR\n", bar4); >> + vdev->regb = devm_ioremap_resource(vdev->drm.dev, bar4); >> + if (IS_ERR(vdev->regb)) { >> + ivpu_err(vdev, "Failed to map bar 4\n"); >> + return -ENOMEM; > > Same OK >> + } >> + >> + ret = dma_set_mask_and_coherent(vdev->drm.dev, DMA_BIT_MASK(38)); >> + if (ret) { >> + ivpu_err(vdev, "Failed to set DMA mask: %d\n", ret); >> + return ret; >> + } >> + >> + /* Clear any pending errors */ >> + pcie_capability_clear_word(pdev, PCI_EXP_DEVSTA, 0x3f); >> + >> + ret = pci_enable_device(pdev); > > pcim_enable device() OK >> +static int ivpu_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> +{ >> + struct ivpu_device *vdev; >> + int ret; >> + >> + vdev = devm_drm_dev_alloc(>dev, , struct ivpu_device, drm); >> + if (IS_ERR(vdev)) >> + return PTR_ERR(vdev); >> + >> + pci_set_drvdata(pdev, vdev); >> + vdev->drm.dev_private = vdev; > > No more use of dev_private in new drivers. Your struct ivpu_device should > rather embed an instance of struct drm_device and you should upcast if > necessary. OK, we are not using it anyway. >> + >> + ret = ivpu_dev_init(vdev); >> + if (ret) { >> + dev_err(>dev, "Failed to initialize VPU device: %d\n", ret); >> + return ret; >> + } >> + >> + ret = drm_dev_register(>drm, 0); >> + if (ret) { >> + dev_err(>dev, "Failed to register DRM device: %d\n", ret); >> + ivpu_dev_fini(vdev); >> + } >> + >> + return ret; >> +} >> + >> +static void ivpu_remove(struct pci_dev *pdev) >> +{ >> + struct ivpu_device *vdev = pci_get_drvdata(pdev); >> + >> + drm_dev_unregister(>drm); >> + ivpu_dev_fini(vdev); >> +} >> + >> +static struct pci_driver ivpu_pci_driver = { >> + .name = KBUILD_MODNAME, >> + .id_table = ivpu_pci_ids, >> + .probe = ivpu_probe, >> + .remove = ivpu_remove, >> +}; >> + >> +static __init int ivpu_init(void) >> +{ >> + pr_info("Intel VPU driver version: %s", DRIVER_VERSION_STR); > > No log spam please. This is gone after using module_pci_driver(). >> + >> + return pci_register_driver(_pci_driver); >> +} >> + >> +static __exit void ivpu_fini(void) >> +{ >> + pci_unregister_driver(_pci_driver); >> +} >> + >> +module_init(ivpu_init); >> +module_exit(ivpu_fini); > > Maybe just module_pci_driver(). > > [1] https://elixir.bootlin.com/linux/latest/source/include/linux/pci.h#L1480 Sure, make sense. >> + >> +#define ivpu_err(vdev, fmt, ...) \ >> + dev_err((vdev)->drm.dev, "[%s] ERROR: " fmt, __func__, ##__VA_ARGS__) > > I see why you want your own dbg macros. But why do you duplicate DRM's print >
Re: [Intel-gfx] Developing a new backlight driver for specific OLED screen
Hi, I come back on my problem regarding the development of a specific driver which controls the brightness of my OLED device. > If it's eDP and uses some proprietary DPCD brightness control mechanism, > I think in practice it usually is somewhat dependent on the GPU. > > (OTOH I realize you don't mention eDP. If it's not eDP, DDC/CI is the > more likely way to control brightness than DPCD.) I succeed to control the brightness through the /dev/drm_dp_aux0 device. Since I only need access to the DP AUX channel, I would like to develop an independant (from the GPU) driver. Unfortunately I don't know how to get access to the DP AUX channel from this independant driver.. Do you have some ideas? I am totally agree with the fact that this display might only be used with an intel gfx card but I'm not sure that this code (which only use DP AUX read/write access) must be in the intel gfx driver code. >> Unfortunately I guess the mechanism is not shared with many OLED >> displays... > > Do you have a spec for it? How does it differ from the VESA eDP DPCD > brightness control? I don't have any specs but as far as I understood it configures some screen registers to scale the PWM of all OLED pixels depending on the display state. It uses its own vendor's ports and registers. And values sent on the display registers to set the desired brightness are computed with complex formulaes (and the calculation needs static tables of values and display information got from the display at startup).
Re: Try to address the DMA-buf coherency problem
Hi, just dropping some real live use case, sorry I'm not really proposing solutions, I believe you are much more knowledgeable in this regard. Le vendredi 28 octobre 2022 à 16:26 +0200, Christian König a écrit : > Am 28.10.22 um 13:42 schrieb Lucas Stach: > > Am Freitag, dem 28.10.2022 um 10:40 +0200 schrieb Christian König: > > > But essentially the right thing to do. The only alternative I can see is > > > to reverse the role of exporter and importer. > > > > > I don't think that would work generally either, as buffer exporter and > > importer isn't always a 1:1 thing. As soon as any attached importer has > > a different coherency behavior than the others, things fall apart. > > I've just mentioned it because somebody noted that when you reverse the > roles of exporter and importer with the V4L driver and i915 then the use > case suddenly starts working. Though, its not generically possible to reverse these roles. If you want to do so, you endup having to do like Android (gralloc) and ChromeOS (minigbm), because you will have to allocate DRM buffers that knows about importer specific requirements. See link [1] for what it looks like for RK3399, with Motion Vector size calculation copied from the kernel driver into a userspace lib (arguably that was available from V4L2 sizeimage, but this is technically difficult to communicate within the software layers). If you could let the decoder export (with proper cache management) the non-generic code would not be needed. Another case where reversing the role is difficult is for case where you need to multiplex the streams (let's use a camera to illustrate) and share that with multiple processes. In these uses case, the DRM importers are volatile, which one do you abuse to do allocation from ? In multimedia server like PipeWire, you are not really aware if the camera will be used by DRM or not, and if something "special" is needed in term of role inversion. It is relatively easy to deal with matching modifiers, but using downstream (display/gpu) as an exporter is always difficult (and require some level of abuse and guessing). [1] https://android.googlesource.com/platform/external/minigbm/+/refs/heads/master/rockchip.c#140 > > > > > > For DRM and most V4L2 devices I then fill in the dma_coherent flag > > > > > based on the > > > > > return value of dev_is_dma_coherent(). Exporting drivers are allowed > > > > > to clear > > > > > the flag for their buffers if special handling like the USWC flag in > > > > > amdgpu or > > > > > the uncached allocations for radeon/nouveau are in use. > > > > > > > > > I don't think the V4L2 part works for most ARM systems. The default > > > > there is for devices to be noncoherent unless explicitly marked > > > > otherwise. I don't think any of the "devices" writing the video buffers > > > > in cached memory with the CPU do this. While we could probably mark > > > > them as coherent, I don't think this is moving in the right direction. > > > Well why not? Those devices are coherent in the sense of the DMA API > > > that they don't need an extra CPU copy on sync_to_cpu/sync_to_device. > > > > > > We could come up with a better name for coherency, e.g. snooping for > > > example. But that is just an documentation detail. > > > > > I agree that those devices copying data into a CPU cacheable buffer > > should be marked as coherent, just not sure right now if other things > > like DMA mappings are done on that device, which would require the > > cache maintenance. > > Yeah, good point. > > > > And this the exact wrong approach as far as I can see. As Daniel noted > > > as well we absolutely need some kind of coherency between exporter and > > > importer. > > > > > I think it's important that we are very specific about the thing we are > > talking about here: I guess when you say coherency you mean hardware > > enforced coherency on cacheable memory, which is the default on > > x86/PCI. > > Well, no. What I mean with coherency is that the devices don't need > insert special operation to access each others data. > > This can be archived by multiple approaches, e.g. by the PCI coherency > requirements, device internal connections (XGMI, NVLink, CXL etc...) as > well as using uncached system memory. > > The key point is what we certainly don't want is special operations > which say: Ok, now device A can access the data, now device B. > because this breaks tons of use cases. I'm coming back again with the multiplexing case. We keep having mixed uses case with multiple receiver. In some case, data may endup on CPU while being encoded in HW. Current approach of disabling cache does work, but CPU algorithm truly suffer in performance. Doing a full memcpy to a cached buffer helps, but remains slower then if the cache had been snooped by the importer (encoder here) driver. > > > The other way to enforce coherency is to either insert cache > > maintenance operations, or make sure that the buffer is not cacheable
Re: [v6] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280
On 2022-10-01 20:15:06, Kalyan Thota wrote: > Flush mechanism for DSPP blocks has changed in sc7280 family, it > allows individual sub blocks to be flushed in coordination with > master flush control. > > Representation: master_flush && (PCC_flush | IGC_flush .. etc ) > > This change adds necessary support for the above design. > > Changes in v1: > - Few nits (Doug, Dmitry) > - Restrict sub-block flush programming to dpu_hw_ctl file (Dmitry) > > Changes in v2: > - Move the address offset to flush macro (Dmitry) > - Seperate ops for the sub block flush (Dmitry) > > Changes in v3: > - Reuse the DPU_DSPP_xx enum instead of a new one (Dmitry) > > Changes in v4: > - Use shorter version for unsigned int (Stephen) > > Changes in v5: > - Spurious patch please ignore. > > Changes in v6: > - Add SOB tag (Doug, Dmitry) > > Signed-off-by: Kalyan Thota > Reviewed-by: Dmitry Baryshkov > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +++- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 35 > -- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++-- > 5 files changed, 50 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 601d687..4170fbe 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -766,7 +766,7 @@ static void _dpu_crtc_setup_cp_blocks(struct drm_crtc > *crtc) > > /* stage config flush mask */ > ctl->ops.update_pending_flush_dspp(ctl, > - mixer[i].hw_dspp->idx); > + mixer[i].hw_dspp->idx, DPU_DSPP_PCC); > } > } > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > index 27f029f..0eecb2f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > @@ -65,7 +65,10 @@ > (PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2)) > > #define CTL_SC7280_MASK \ > - (BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | > BIT(DPU_CTL_VM_CFG)) > + (BIT(DPU_CTL_ACTIVE_CFG) | \ > + BIT(DPU_CTL_FETCH_ACTIVE) | \ > + BIT(DPU_CTL_VM_CFG) | \ > + BIT(DPU_CTL_DSPP_SUB_BLOCK_FLUSH)) > > #define MERGE_3D_SM8150_MASK (0) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > index 38aa38a..8148e91 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > @@ -161,10 +161,12 @@ enum { > * DSPP sub-blocks > * @DPU_DSPP_PCC Panel color correction block > * @DPU_DSPP_GC Gamma correction block > + * @DPU_DSPP_IGC Inverse Gamma correction block Lowercase G? Otherwise capitalize Correction as well to match the IGC abbrev., and do the same for the other abbreviations above. > */ > enum { > DPU_DSPP_PCC = 0x1, > DPU_DSPP_GC, > + DPU_DSPP_IGC, > DPU_DSPP_MAX > }; > > @@ -191,6 +193,7 @@ enum { > * @DPU_CTL_SPLIT_DISPLAY: CTL supports video mode split display > * @DPU_CTL_FETCH_ACTIVE:Active CTL for fetch HW (SSPPs) > * @DPU_CTL_VM_CFG: CTL config to support multiple VMs > + * @DPU_CTL_DSPP_BLOCK_FLUSH: CTL config to support dspp sub-block flush > * @DPU_CTL_MAX > */ > enum { > @@ -198,6 +201,7 @@ enum { > DPU_CTL_ACTIVE_CFG, > DPU_CTL_FETCH_ACTIVE, > DPU_CTL_VM_CFG, > + DPU_CTL_DSPP_SUB_BLOCK_FLUSH, > DPU_CTL_MAX > }; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > index a35ecb6..f26f484 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > @@ -33,6 +33,7 @@ > #define CTL_INTF_FLUSH0x110 > #define CTL_INTF_MASTER 0x134 > #define CTL_FETCH_PIPE_ACTIVE 0x0FC > +#define CTL_DSPP_n_FLUSH(n)((0x13C) + ((n - 1) * 4)) Use spaces for indentation just like the other defines. Replace 1 with DSPP_0. > > #define CTL_MIXER_BORDER_OUTBIT(24) > #define CTL_FLUSH_MASK_CTL BIT(17) > @@ -287,8 +288,9 @@ static void > dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx, > } > > static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx, > - enum dpu_dspp dspp) > + enum dpu_dspp dspp, u32 dspp_sub_blk) > { > + > switch (dspp) { > case DSPP_0: > ctx->pending_flush_mask |= BIT(13); > @@ -307,6 +309,31 @@ static void dpu_hw_ctl_update_pending_flush_dspp(struct > dpu_hw_ctl *ctx, > } > } > > +static void dpu_hw_ctl_update_pending_flush_dspp_subblocks( > + struct dpu_hw_ctl *ctx,
Re: [PATCH v2] dma-buf: cma_heap: Fix typo in comment
Mark-PK Tsai writes: >> [-- Attachment #1: Type: text/plain, Size: 349 bytes --] >> >> On Fri, Oct 28, 2022 at 09:44:17AM +0800, Mark-PK Tsai wrote: >> > Remove duplicated "by" from comment in cma_heap_allocate(). >> > >> >> This patch isn't typofix but duplicate word stripping, right? If so, the >> patch subject should be "dma-buf: cma_heap: Remove duplicated 'by'". > > Okay, I've update the title in v3. > Sorry for the horrible commit description. > Thanks. Your original commit description was just fine, you just had the bad luck to draw the attention of somebody who likes telling other contributors what to do. Thanks, jon
Re: [PATCH v2] drm: bridge: adv7511: use dev_err_probe in probe function
Hi, On Wed, 26 Oct 2022 14:52:46 +0200, Ahmad Fatoum wrote: > adv7511 probe may need to be attempted multiple times before no > -EPROBE_DEFER is returned. Currently, every such probe results in > an error message: > > [4.534229] adv7511 1-003d: failed to find dsi host > [4.580288] adv7511 1-003d: failed to find dsi host > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next) [1/1] drm: bridge: adv7511: use dev_err_probe in probe function https://cgit.freedesktop.org/drm/drm-misc/commit/?id=2a865248399a13bb2b2bcc50297069a7521de258 -- Neil
Re: Try to address the DMA-buf coherency problem
Am 28.10.22 um 13:42 schrieb Lucas Stach: Am Freitag, dem 28.10.2022 um 10:40 +0200 schrieb Christian König: But essentially the right thing to do. The only alternative I can see is to reverse the role of exporter and importer. I don't think that would work generally either, as buffer exporter and importer isn't always a 1:1 thing. As soon as any attached importer has a different coherency behavior than the others, things fall apart. I've just mentioned it because somebody noted that when you reverse the roles of exporter and importer with the V4L driver and i915 then the use case suddenly starts working. For DRM and most V4L2 devices I then fill in the dma_coherent flag based on the return value of dev_is_dma_coherent(). Exporting drivers are allowed to clear the flag for their buffers if special handling like the USWC flag in amdgpu or the uncached allocations for radeon/nouveau are in use. I don't think the V4L2 part works for most ARM systems. The default there is for devices to be noncoherent unless explicitly marked otherwise. I don't think any of the "devices" writing the video buffers in cached memory with the CPU do this. While we could probably mark them as coherent, I don't think this is moving in the right direction. Well why not? Those devices are coherent in the sense of the DMA API that they don't need an extra CPU copy on sync_to_cpu/sync_to_device. We could come up with a better name for coherency, e.g. snooping for example. But that is just an documentation detail. I agree that those devices copying data into a CPU cacheable buffer should be marked as coherent, just not sure right now if other things like DMA mappings are done on that device, which would require the cache maintenance. Yeah, good point. And this the exact wrong approach as far as I can see. As Daniel noted as well we absolutely need some kind of coherency between exporter and importer. I think it's important that we are very specific about the thing we are talking about here: I guess when you say coherency you mean hardware enforced coherency on cacheable memory, which is the default on x86/PCI. Well, no. What I mean with coherency is that the devices don't need insert special operation to access each others data. This can be archived by multiple approaches, e.g. by the PCI coherency requirements, device internal connections (XGMI, NVLink, CXL etc...) as well as using uncached system memory. The key point is what we certainly don't want is special operations which say: Ok, now device A can access the data, now device B. because this breaks tons of use cases. The other way to enforce coherency is to either insert cache maintenance operations, or make sure that the buffer is not cacheable by any device taking part in the sharing, including the CPU. Yes and no. When we want the devices to interact with each other without the CPU then we need some negotiated coherency between the two. This can either be provided by the PCI specification which makes it mandatory for device to snoop the caches or by platform devices agreeing that they only work on uncached memory. What you disregard here is the fact that there are many systems out there with mixed topologies, where some masters are part of the coherency domain and some are not. We have two options here: either mandate that coherency for dma-bufs need to be established by hardware, which is the option that you strongly prefer, which means forcing all buffers to be uncacheable in a system with masters that are not coherent, or allowing some form of bracketed DMA access with cache maintenance ops. Well I don't prefer that option, it's just the only one I can see. One of the main goals of DMA-buf is to allow device to share data without the need for CPU interactions. In other words we negotiate the high level properties and then the device can talk to each other without explicitly noting who is accessing what. And this concept is completely incompatible with maintenance ops. We made that mistake with SWIOTLB for the DMA API and I don't really want to repeat that stunt. Explicit cache flush operations are simple not part of the design of DMA-buf because they are not part of the design of the higher level APIs like Vulkan and OpenGL. I'm aware that some graphics APIs have been living in a universe blissfully unaware of systems without hardware enforced coherency. But that isn't the only use for dma-bufs. Yeah, but the other use cases are extremely limited. As far as I can see I totally agree that some higher level API primitives aren't possible without coherency at the hardware level and for those uses we should require either HW enforced coherency or uncachable memory. But I don't think we should make things slow deliberately on systems that allow to optimize things with the help of bracketed access. If I understood things right your amdgpu use-case even falls into this category: normally you would want to use
Re: [PATCH v1 9/9] arm64: dts: qcom: sm8350-hdk: Enable lt9611uxc dsi-hdmi bridge
On Fri, Oct 28, 2022 at 02:08:12PM +0200, Robert Foss wrote: > The sm8350-hdk ships with the LT9611 UXC DSI/HDMI bridge chip. > > In order to toggle the board to enable the HDMI output, > switch #7 & #8 on the rightmost multi-switch package have > to be toggled to On. > > Signed-off-by: Robert Foss > --- > arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 106 > 1 file changed, 106 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > index 6e07feb4b3b2..b38b58f8 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > @@ -20,6 +20,17 @@ chosen { > stdout-path = "serial0:115200n8"; > }; > > + hdmi-out { > + compatible = "hdmi-connector"; > + type = "a"; > + > + port { > + hdmi_con: endpoint { > + remote-endpoint = <_out>; > + }; > + }; > + }; > + > vph_pwr: vph-pwr-regulator { > compatible = "regulator-fixed"; > regulator-name = "vph_pwr"; > @@ -29,6 +40,32 @@ vph_pwr: vph-pwr-regulator { > regulator-always-on; > regulator-boot-on; > }; > + > + lt9611_1v2: lt9611-1v2 { > + compatible = "regulator-fixed"; > + regulator-name = "LT9611_1V2"; > + > + vin-supply = <_pwr>; > + regulator-min-microvolt = <120>; > + regulator-max-microvolt = <120>; > + gpio = < 49 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + regulator-boot-on; > + regulator-always-on; Why is this always-on? > + }; > + > + lt9611_3v3: lt9611-3v3 { > + compatible = "regulator-fixed"; > + regulator-name = "LT9611_3V3"; > + > + vin-supply = <_bob>; > + gpio = < 47 GPIO_ACTIVE_HIGH>; > + regulator-min-microvolt = <330>; > + regulator-max-microvolt = <330>; > + enable-active-high; > + regulator-boot-on; > + regulator-always-on; > + }; > }; > > { > @@ -220,6 +257,15 @@ { > { > status = "okay"; > vdda-supply = <_l6b_1p2>; > + > + ports { > + port@1 { > + endpoint { > + remote-endpoint = <_a>; > + data-lanes = <0 1 2 3>; > + }; > + }; > + }; > }; > > _phy { > @@ -231,6 +277,48 @@ _dma1 { > status = "okay"; > }; > > + { > + status = "okay"; Please keep status last. (Yes I see that it goes against the convention in this file, so let's update that at some point as well) > + clock-frequency = <40>; > + > + lt9611_codec: hdmi-bridge@2b { > + compatible = "lontium,lt9611uxc"; > + reg = <0x2b>; > + status = "okay"; This is the default, you can omit it. > + > + interrupts-extended = < 50 IRQ_TYPE_EDGE_FALLING>; > + reset-gpios = < 48 GPIO_ACTIVE_HIGH>; > + > + vdd-supply = <_1v2>; > + vcc-supply = <_3v3>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <_irq_pin _rst_pin>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + lt9611_a: endpoint { > + remote-endpoint = <_out>; > + }; > + }; > + > + port@2 { > + reg = <2>; > + > + lt9611_out: endpoint { > + remote-endpoint = <_con>; > + }; > + }; > + > + }; > + }; > +}; > + > { > status = "okay"; > }; > @@ -248,6 +336,10 @@ _id_0 { > status = "okay"; > }; > > +_id_2 { > + status = "okay"; > +}; > + > { > status = "okay"; > firmware-name = "qcom/sm8350/slpi.mbn"; > @@ -544,4 +636,18 @@ usb_hub_enabled_state: usb-hub-enabled-state { > drive-strength = <2>; > output-low; > }; > + > + lt9611_rst_pin: lt9611-rst-state { > + pins = "gpio48"; > + function = "normal"; > + > + output-high; > + input-disable; > + }; > + > + lt9611_irq_pin: lt9611-irq { pinctrl state nodes should be suffixed with "-state". And you can lump the two pins into a single -state, with rst-pins and irq-pins as subnodes, defining the two pins. Regards, Bjorn > + pins = "gpio50"; > + function = "gpio"; > + bias-disable; > + }; > }; > -- > 2.34.1 >
Re: [PATCH v1 9/9] arm64: dts: qcom: sm8350-hdk: Enable lt9611uxc dsi-hdmi bridge
On 28/10/2022 15:08, Robert Foss wrote: The sm8350-hdk ships with the LT9611 UXC DSI/HDMI bridge chip. In order to toggle the board to enable the HDMI output, switch #7 & #8 on the rightmost multi-switch package have to be toggled to On. Since this doesn't look like a default setup, it would probably make sense to move this to new sm8350-hdk-hdmi.dts with the comment regarding necessary switch changes at the top of the file. Signed-off-by: Robert Foss --- arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 106 1 file changed, 106 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts index 6e07feb4b3b2..b38b58f8 100644 --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts @@ -20,6 +20,17 @@ chosen { stdout-path = "serial0:115200n8"; }; + hdmi-out { + compatible = "hdmi-connector"; + type = "a"; + + port { + hdmi_con: endpoint { + remote-endpoint = <_out>; + }; + }; + }; + vph_pwr: vph-pwr-regulator { compatible = "regulator-fixed"; regulator-name = "vph_pwr"; @@ -29,6 +40,32 @@ vph_pwr: vph-pwr-regulator { regulator-always-on; regulator-boot-on; }; + + lt9611_1v2: lt9611-1v2 { + compatible = "regulator-fixed"; + regulator-name = "LT9611_1V2"; + + vin-supply = <_pwr>; + regulator-min-microvolt = <120>; + regulator-max-microvolt = <120>; + gpio = < 49 GPIO_ACTIVE_HIGH>; + enable-active-high; + regulator-boot-on; + regulator-always-on; + }; + + lt9611_3v3: lt9611-3v3 { + compatible = "regulator-fixed"; + regulator-name = "LT9611_3V3"; + + vin-supply = <_bob>; + gpio = < 47 GPIO_ACTIVE_HIGH>; + regulator-min-microvolt = <330>; + regulator-max-microvolt = <330>; + enable-active-high; + regulator-boot-on; + regulator-always-on; + }; }; { @@ -220,6 +257,15 @@ { { status = "okay"; vdda-supply = <_l6b_1p2>; + + ports { + port@1 { + endpoint { + remote-endpoint = <_a>; + data-lanes = <0 1 2 3>; + }; + }; + }; }; _phy { @@ -231,6 +277,48 @@ _dma1 { status = "okay"; }; + { + status = "okay"; + clock-frequency = <40>; + + lt9611_codec: hdmi-bridge@2b { + compatible = "lontium,lt9611uxc"; + reg = <0x2b>; + status = "okay"; + + interrupts-extended = < 50 IRQ_TYPE_EDGE_FALLING>; + reset-gpios = < 48 GPIO_ACTIVE_HIGH>; + + vdd-supply = <_1v2>; + vcc-supply = <_3v3>; + + pinctrl-names = "default"; + pinctrl-0 = <_irq_pin _rst_pin>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + lt9611_a: endpoint { + remote-endpoint = <_out>; + }; + }; + + port@2 { + reg = <2>; + + lt9611_out: endpoint { + remote-endpoint = <_con>; + }; + }; + + }; + }; +}; + { status = "okay"; }; @@ -248,6 +336,10 @@ _id_0 { status = "okay"; }; +_id_2 { + status = "okay"; +}; + { status = "okay"; firmware-name = "qcom/sm8350/slpi.mbn"; @@ -544,4 +636,18 @@ usb_hub_enabled_state: usb-hub-enabled-state { drive-strength = <2>; output-low; }; + + lt9611_rst_pin: lt9611-rst-state { + pins = "gpio48"; + function = "normal"; + + output-high; + input-disable; + }; + + lt9611_irq_pin: lt9611-irq { + pins = "gpio50"; + function = "gpio"; + bias-disable; + }; }; -- With best wishes Dmitry
Re: [PATCH v1 8/9] arm64: dts: qcom: sm8350-hdk: Enable display & dsi nodes
On Fri, Oct 28, 2022 at 02:08:11PM +0200, Robert Foss wrote: > Enable the display subsystem and the dsi0 output for > the sm8350-hdk board. > > Signed-off-by: Robert Foss > --- > arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 22 ++ > 1 file changed, 22 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > index e6deb08c6da0..6e07feb4b3b2 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > @@ -213,10 +213,32 @@ { > firmware-name = "qcom/sm8350/cdsp.mbn"; > }; > > + { > + status = "okay"; > +}; > + > + { If you prefix the label for the dsi controller and phy with mdss_ they sort nicely together with the other display nodes. Regards, Bjorn > + status = "okay"; > + vdda-supply = <_l6b_1p2>; > +}; > + > +_phy { > + status = "okay"; > + vdds-supply = <_l5b_0p88>; > +}; > + > _dma1 { > status = "okay"; > }; > > + { > + status = "okay"; > +}; > + > +_mdp { > + status = "okay"; > +}; > + > { > status = "okay"; > firmware-name = "qcom/sm8350/modem.mbn"; > -- > 2.34.1 >
Re: [PATCH v1 7/9] arm64: dts: qcom: sm8350: Add display system nodes
On Fri, Oct 28, 2022 at 02:08:10PM +0200, Robert Foss wrote: > Add mdss, mdss_mdp, dsi0, dsi0_phy nodes. With these > nodes the display subsystem is configured to support > one DSI output. > > Signed-off-by: Robert Foss > --- > arch/arm64/boot/dts/qcom/sm8350.dtsi | 196 ++- > 1 file changed, 192 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi > b/arch/arm64/boot/dts/qcom/sm8350.dtsi > index b6e44cd3b394..eaa3cdee1860 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi > @@ -3,6 +3,7 @@ > * Copyright (c) 2020, Linaro Limited > */ > > +#include > #include > #include > #include > @@ -2535,14 +2536,200 @@ usb_2_dwc3: usb@a80 { > }; > }; > > + mdss: mdss@ae0 { display-subsystem@ae0 please. > + compatible = "qcom,sm8350-mdss"; > + reg = <0 0x0ae0 0 0x1000>; > + reg-names = "mdss"; > + > + interconnects = <_noc MASTER_MDP0 0 _virt > SLAVE_EBI1 0>, > + <_noc MASTER_MDP1 0 _virt > SLAVE_EBI1 0>; > + interconnect-names = "mdp0-mem", "mdp1-mem"; > + > + power-domains = < MDSS_GDSC>; > + resets = < DISP_CC_MDSS_CORE_BCR>; > + > + clocks = < DISP_CC_MDSS_AHB_CLK>, > + < GCC_DISP_HF_AXI_CLK>, > + < GCC_DISP_SF_AXI_CLK>, > + < DISP_CC_MDSS_MDP_CLK>; > + clock-names = "iface", "bus", "nrt_bus", "core"; > + > + interrupts = ; > + interrupt-controller; > + #interrupt-cells = <1>; > + > + status = "ok"; You want "disabled" here, and then "okay" in the .dts. > + > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + mdss_mdp: mdp@ae01000 { display-controller@ae01000 please. > + compatible = "qcom,sm8350-dpu"; > + reg = <0 0x0ae01000 0 0x8f000>, > + <0 0x0aeb 0 0x2008>; > + reg-names = "mdp", "vbif"; > + iommus = <_smmu 0x820 0x402>; > + > + clocks = < GCC_DISP_HF_AXI_CLK>, > + < GCC_DISP_SF_AXI_CLK>, > + < DISP_CC_MDSS_AHB_CLK>, > + < DISP_CC_MDSS_MDP_LUT_CLK>, > + < DISP_CC_MDSS_MDP_CLK>, > + < DISP_CC_MDSS_VSYNC_CLK>; > + clock-names = "bus", > + "nrt_bus", > + "iface", > + "lut", > + "core", > + "vsync"; > + > + assigned-clocks = < > DISP_CC_MDSS_VSYNC_CLK>; > + assigned-clock-rates = <1920>; > + > + operating-points-v2 = <_opp_table>; > + power-domains = < SM8350_MMCX>; > + > + interrupt-parent = <>; > + interrupts = <0>; > + > + status = "ok"; "okay", but this is the default, so you can skip it in the .dtsi > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dpu_intf1_out: endpoint { > + remote-endpoint = > <_in>; > + }; > + }; > + }; > + > + mdp_opp_table: mdp-opp-table { > + compatible = "operating-points-v2"; > + > + opp-2 { > + opp-hz = /bits/ 64 <2>; > + required-opps = > <_opp_low_svs>; > + }; > + > + opp-3 { > + opp-hz = /bits/ 64 <3>; > + required-opps = > <_opp_svs>; > + }; > + > + opp-34500
Re: [PATCH v1 6/9] arm64: dts: qcom: sm8350: Use 2 interconnect cells
On Fri, Oct 28, 2022 at 02:08:09PM +0200, Robert Foss wrote: > Use two interconnect cells in order to optionally > support a path tag. > > Signed-off-by: Robert Foss > --- > arch/arm64/boot/dts/qcom/sm8350.dtsi | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi > b/arch/arm64/boot/dts/qcom/sm8350.dtsi > index 606fab087945..b6e44cd3b394 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi > @@ -1543,56 +1543,56 @@ apps_smmu: iommu@1500 { > config_noc: interconnect@150 { > compatible = "qcom,sm8350-config-noc"; > reg = <0 0x0150 0 0xa580>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; You also need amend all the interconnects references with the additional tag cell. Regards, Bjorn > qcom,bcm-voters = <_bcm_voter>; > }; > > mc_virt: interconnect@158 { > compatible = "qcom,sm8350-mc-virt"; > reg = <0 0x0158 0 0x1000>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; > qcom,bcm-voters = <_bcm_voter>; > }; > > system_noc: interconnect@168 { > compatible = "qcom,sm8350-system-noc"; > reg = <0 0x0168 0 0x1c200>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; > qcom,bcm-voters = <_bcm_voter>; > }; > > aggre1_noc: interconnect@16e { > compatible = "qcom,sm8350-aggre1-noc"; > reg = <0 0x016e 0 0x1f180>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; > qcom,bcm-voters = <_bcm_voter>; > }; > > aggre2_noc: interconnect@170 { > compatible = "qcom,sm8350-aggre2-noc"; > reg = <0 0x0170 0 0x33000>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; > qcom,bcm-voters = <_bcm_voter>; > }; > > mmss_noc: interconnect@174 { > compatible = "qcom,sm8350-mmss-noc"; > reg = <0 0x0174 0 0x1f080>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; > qcom,bcm-voters = <_bcm_voter>; > }; > > lpass_ag_noc: interconnect@3c4 { > compatible = "qcom,sm8350-lpass-ag-noc"; > reg = <0 0x03c4 0 0xf080>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; > qcom,bcm-voters = <_bcm_voter>; > }; > > compute_noc: interconnect@a0c{ > compatible = "qcom,sm8350-compute-noc"; > reg = <0 0x0a0c 0 0xa180>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; > qcom,bcm-voters = <_bcm_voter>; > }; > > @@ -2420,14 +2420,14 @@ usb_2_ssphy: phy@88ebe00 { > dc_noc: interconnect@90c { > compatible = "qcom,sm8350-dc-noc"; > reg = <0 0x090c 0 0x4200>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; > qcom,bcm-voters = <_bcm_voter>; > }; > > gem_noc: interconnect@910 { > compatible = "qcom,sm8350-gem-noc"; > reg = <0 0x0910 0 0xb4000>; > - #interconnect-cells = <1>; > + #interconnect-cells = <2>; > qcom,bcm-voters = <_bcm_voter>; > }; > > -- > 2.34.1 >
[PATCH RESEND] gpu: host1x: fix memory leak of device names
The device names allocated by dev_set_name() need be freed before module unloading, but they can not be freed because the kobject's refcount which was set in device_initialize() has not be decreased to 0. Fix the name leak by calling put_device() to give up the refcount, so the name can be freed in kobejct_cleanup(). Add a release() function to device, it's empty, because the context devices are freed together in host1x_memory_context_list_free(). Fixes: 8aa5bcb61612 ("gpu: host1x: Add context device management code") Signed-off-by: Yang Yingliang --- The fix tag is in mainline not next, so resend it for removing -next in title. --- drivers/gpu/host1x/context.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/host1x/context.c b/drivers/gpu/host1x/context.c index b08cf11f9a66..f49a7bf6afa5 100644 --- a/drivers/gpu/host1x/context.c +++ b/drivers/gpu/host1x/context.c @@ -13,6 +13,11 @@ #include "context.h" #include "dev.h" +static void host1x_memory_context_release(struct device *dev) +{ + /* context device is freed in host1x_memory_context_list_free() */ +} + int host1x_memory_context_list_init(struct host1x *host1x) { struct host1x_memory_context_list *cdl = >context_list; @@ -53,28 +58,27 @@ int host1x_memory_context_list_init(struct host1x *host1x) dev_set_name(>dev, "host1x-ctx.%d", i); ctx->dev.bus = _context_device_bus_type; ctx->dev.parent = host1x->dev; + ctx->dev.release = host1x_memory_context_release; dma_set_max_seg_size(>dev, UINT_MAX); err = device_add(>dev); if (err) { dev_err(host1x->dev, "could not add context device %d: %d\n", i, err); - goto del_devices; + goto put_dev; } err = of_dma_configure_id(>dev, node, true, ); if (err) { dev_err(host1x->dev, "IOMMU configuration failed for context device %d: %d\n", i, err); - device_del(>dev); - goto del_devices; + goto unreg_devices; } fwspec = dev_iommu_fwspec_get(>dev); if (!fwspec || !device_iommu_mapped(>dev)) { dev_err(host1x->dev, "Context device %d has no IOMMU!\n", i); - device_del(>dev); - goto del_devices; + goto unreg_devices; } ctx->stream_id = fwspec->ids[0] & 0x; @@ -82,9 +86,12 @@ int host1x_memory_context_list_init(struct host1x *host1x) return 0; -del_devices: - while (i--) - device_del(>devs[i].dev); +put_dev: + put_device(>devs[i--].dev); + +unreg_devices: + while (i >= 0) + device_unregister(>devs[i--].dev); kfree(cdl->devs); cdl->len = 0; @@ -97,7 +104,7 @@ void host1x_memory_context_list_free(struct host1x_memory_context_list *cdl) unsigned int i; for (i = 0; i < cdl->len; i++) - device_del(>devs[i].dev); + device_unregister(>devs[i].dev); kfree(cdl->devs); cdl->len = 0; -- 2.25.1
Re: [PATCH -next] gpu: host1x: fix memory leak of device names
Sorry for the noisy, it should for linux master, not next, I will resend it with right title, please ignore this patch. Thanks, Yang On 2022/10/28 20:52, Yang Yingliang wrote: The device names allocated by dev_set_name() need be freed before module unloading, but they can not be freed because the kobject's refcount which was set in device_initialize() has not be decreased to 0. Fix the name leak by calling put_device() to give up the refcount, so the name can be freed in kobejct_cleanup(). Add a release() function to device, it's empty, because the context devices are freed together in host1x_memory_context_list_free(). Fixes: 8aa5bcb61612 ("gpu: host1x: Add context device management code") Signed-off-by: Yang Yingliang --- drivers/gpu/host1x/context.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/host1x/context.c b/drivers/gpu/host1x/context.c index b08cf11f9a66..f49a7bf6afa5 100644 --- a/drivers/gpu/host1x/context.c +++ b/drivers/gpu/host1x/context.c @@ -13,6 +13,11 @@ #include "context.h" #include "dev.h" +static void host1x_memory_context_release(struct device *dev) +{ + /* context device is freed in host1x_memory_context_list_free() */ +} + int host1x_memory_context_list_init(struct host1x *host1x) { struct host1x_memory_context_list *cdl = >context_list; @@ -53,28 +58,27 @@ int host1x_memory_context_list_init(struct host1x *host1x) dev_set_name(>dev, "host1x-ctx.%d", i); ctx->dev.bus = _context_device_bus_type; ctx->dev.parent = host1x->dev; + ctx->dev.release = host1x_memory_context_release; dma_set_max_seg_size(>dev, UINT_MAX); err = device_add(>dev); if (err) { dev_err(host1x->dev, "could not add context device %d: %d\n", i, err); - goto del_devices; + goto put_dev; } err = of_dma_configure_id(>dev, node, true, ); if (err) { dev_err(host1x->dev, "IOMMU configuration failed for context device %d: %d\n", i, err); - device_del(>dev); - goto del_devices; + goto unreg_devices; } fwspec = dev_iommu_fwspec_get(>dev); if (!fwspec || !device_iommu_mapped(>dev)) { dev_err(host1x->dev, "Context device %d has no IOMMU!\n", i); - device_del(>dev); - goto del_devices; + goto unreg_devices; } ctx->stream_id = fwspec->ids[0] & 0x; @@ -82,9 +86,12 @@ int host1x_memory_context_list_init(struct host1x *host1x) return 0; -del_devices: - while (i--) - device_del(>devs[i].dev); +put_dev: + put_device(>devs[i--].dev); + +unreg_devices: + while (i >= 0) + device_unregister(>devs[i--].dev); kfree(cdl->devs); cdl->len = 0; @@ -97,7 +104,7 @@ void host1x_memory_context_list_free(struct host1x_memory_context_list *cdl) unsigned int i; for (i = 0; i < cdl->len; i++) - device_del(>devs[i].dev); + device_unregister(>devs[i].dev); kfree(cdl->devs); cdl->len = 0;
[PATCH -next] gpu: host1x: fix memory leak of device names
The device names allocated by dev_set_name() need be freed before module unloading, but they can not be freed because the kobject's refcount which was set in device_initialize() has not be decreased to 0. Fix the name leak by calling put_device() to give up the refcount, so the name can be freed in kobejct_cleanup(). Add a release() function to device, it's empty, because the context devices are freed together in host1x_memory_context_list_free(). Fixes: 8aa5bcb61612 ("gpu: host1x: Add context device management code") Signed-off-by: Yang Yingliang --- drivers/gpu/host1x/context.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/host1x/context.c b/drivers/gpu/host1x/context.c index b08cf11f9a66..f49a7bf6afa5 100644 --- a/drivers/gpu/host1x/context.c +++ b/drivers/gpu/host1x/context.c @@ -13,6 +13,11 @@ #include "context.h" #include "dev.h" +static void host1x_memory_context_release(struct device *dev) +{ + /* context device is freed in host1x_memory_context_list_free() */ +} + int host1x_memory_context_list_init(struct host1x *host1x) { struct host1x_memory_context_list *cdl = >context_list; @@ -53,28 +58,27 @@ int host1x_memory_context_list_init(struct host1x *host1x) dev_set_name(>dev, "host1x-ctx.%d", i); ctx->dev.bus = _context_device_bus_type; ctx->dev.parent = host1x->dev; + ctx->dev.release = host1x_memory_context_release; dma_set_max_seg_size(>dev, UINT_MAX); err = device_add(>dev); if (err) { dev_err(host1x->dev, "could not add context device %d: %d\n", i, err); - goto del_devices; + goto put_dev; } err = of_dma_configure_id(>dev, node, true, ); if (err) { dev_err(host1x->dev, "IOMMU configuration failed for context device %d: %d\n", i, err); - device_del(>dev); - goto del_devices; + goto unreg_devices; } fwspec = dev_iommu_fwspec_get(>dev); if (!fwspec || !device_iommu_mapped(>dev)) { dev_err(host1x->dev, "Context device %d has no IOMMU!\n", i); - device_del(>dev); - goto del_devices; + goto unreg_devices; } ctx->stream_id = fwspec->ids[0] & 0x; @@ -82,9 +86,12 @@ int host1x_memory_context_list_init(struct host1x *host1x) return 0; -del_devices: - while (i--) - device_del(>devs[i].dev); +put_dev: + put_device(>devs[i--].dev); + +unreg_devices: + while (i >= 0) + device_unregister(>devs[i--].dev); kfree(cdl->devs); cdl->len = 0; @@ -97,7 +104,7 @@ void host1x_memory_context_list_free(struct host1x_memory_context_list *cdl) unsigned int i; for (i = 0; i < cdl->len; i++) - device_del(>devs[i].dev); + device_unregister(>devs[i].dev); kfree(cdl->devs); cdl->len = 0; -- 2.25.1
Re: [PATCH v3 0/6] Add support for atomic async page-flips
On 10/13/22 13:02, Simon Ser wrote: So no tests that actually verify that the kernel properly rejects stuff stuff like modesets, gamma LUT updates, plane movement, etc.? Pondering this a bit more, it just occurred to me the current driver level checks might easily lead to confusing behaviour. Eg. is the ioctl going to succeed if you ask for an async change of some random property while the crtc disabled, but fails if you ask for the same async property change when the crtc is active? So another reason why rejecting most properties already at the uapi level might be a good idea. And just to be clear this is pretty much what I suggest: diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 79730fa1dd8e..471a2c703847 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1392,6 +1392,13 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, goto out; } + if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC && + prop != dev->mode_config.prop_fb_id) { + drm_mode_object_put(obj); + ret = -EINVAL; + goto out; + } + if (copy_from_user(_value, prop_values_ptr + copied_props, sizeof(prop_value))) { That would actively discourage people from even attempting the "just dump all the state into the ioctl" approach with async flips since even the props whose value isn't even changing would be rejected. How does this sound? diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 945761968428..ffd16bdc7b83 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -972,14 +972,26 @@ int drm_atomic_set_property(struct drm_atomic_state *state, struct drm_file *file_priv, struct drm_mode_object *obj, struct drm_property *prop, - uint64_t prop_value) + uint64_t prop_value, + bool async_flip) { struct drm_mode_object *ref; int ret; + uint64_t old_val; if (!drm_property_change_valid_get(prop, prop_value, )) return -EINVAL; + if (async_flip && prop != prop->dev->mode_config.prop_fb_id) { + ret = drm_atomic_get_property(obj, prop, _val); + if (ret != 0 || old_val != prop_value) { + drm_dbg_atomic(prop->dev, + "[PROP:%d:%s] cannot be changed during async flip\n", + prop->base.id, prop->name); + return -EINVAL; + } + } + switch (obj->type) { case DRM_MODE_OBJECT_CONNECTOR: { struct drm_connector *connector = obj_to_connector(obj); drm_atomic_get_property() needs the object lock to be used, so we need to check the property inside the switch-case like this: -- >8 -- From f3ee5a1163bfe5a88109d7084208940fe5566967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Almeida?= Date: Thu, 27 Oct 2022 17:23:09 -0300 Subject: [PATCH] drm: Check for prop changes in atomic async flip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No prop changes are allowed during an async flip via atomic DRM API, so make sure to reject them. Signed-off-by: André Almeida --- drivers/gpu/drm/drm_atomic_uapi.c | 47 +++-- drivers/gpu/drm/drm_crtc_internal.h | 2 +- drivers/gpu/drm/drm_mode_object.c | 2 +- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index ee24ed7e2edb..f63f23305621 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -964,13 +964,28 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state, return ret; } +static bool drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t prop_value, + struct drm_property *prop) +{ + if (ret != 0 || old_val != prop_value) { + drm_dbg_atomic(prop->dev, + "[PROP:%d:%s] No prop can be changed during async flip\n", + prop->base.id, prop->name); + return true; + } + + return false; +} + int drm_atomic_set_property(struct drm_atomic_state *state, struct drm_file *file_priv, struct drm_mode_object *obj, struct drm_property *prop, - uint64_t prop_value) +
Re: [PATCH v1 1/9] drm/msm: Add compatibles for SM8350 display
On 28/10/2022 15:19, Dmitry Baryshkov wrote: On 28/10/2022 15:08, Robert Foss wrote: Add compatible string for "qcom,sm8350-dpu" and "qcom,sm8350-mdss". Signed-off-by: Robert Foss --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 + drivers/gpu/drm/msm/msm_mdss.c | 1 + 2 files changed, 2 insertions(+) [skipped] diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index e13c5c12b775..fd5a95cace16 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -447,6 +447,7 @@ static const struct of_device_id mdss_dt_match[] = { { .compatible = "qcom,sc8180x-mdss" }, { .compatible = "qcom,sm8150-mdss" }, { .compatible = "qcom,sm8250-mdss" }, + { .compatible = "qcom,sm8350-mdss" }, {} }; MODULE_DEVICE_TABLE(of, mdss_dt_match); BTW: you probably also have to update the msm_mdss_enable() function with the 8350-specific code. For mdss changes you can depend on [1], I plan to merge this patch in this window. [1] https://patchwork.freedesktop.org/patch/489578/?series=105162=1 Also with the mdss changes, it would be good to split this patch into dpu and mdss parts. -- With best wishes Dmitry
Re: [PATCH v1 3/9] drm/msm/dpu: Add SM8350 to hw catalog
On 28/10/2022 15:08, Robert Foss wrote: Add compatibility for SM8350 display subsystem, including required entries in DPU hw catalog. --- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 217 ++ .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 + 2 files changed, 218 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index d0ce7612fee8..bc829d7bdd6e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -112,6 +112,15 @@ BIT(MDP_INTF3_INTR) | \ BIT(MDP_INTF4_INTR)) +#define IRQ_SM8350_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ +BIT(MDP_SSPP_TOP0_INTR2) | \ +BIT(MDP_SSPP_TOP0_HIST_INTR) | \ +BIT(MDP_INTF0_7xxx_INTR) | \ +BIT(MDP_INTF1_7xxx_INTR) | \ +BIT(MDP_INTF2_7xxx_INTR) | \ +BIT(MDP_INTF3_7xxx_INTR) | \ +0) + #define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ @@ -364,6 +373,20 @@ static const struct dpu_caps sm8250_dpu_caps = { .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, }; +static const struct dpu_caps sm8350_dpu_caps = { + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH, + .max_mixer_blendstages = 0xb, + .qseed_type = DPU_SSPP_SCALER_QSEED3LITE, + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */ + .ubwc_version = DPU_HW_UBWC_VER_40, + .has_src_split = true, + .has_dim_layer = true, + .has_idle_pc = true, + .has_3d_merge = true, + .max_linewidth = 4096, Is it 4096 or 5120? + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, +}; + static const struct dpu_caps sc7280_dpu_caps = { .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH, .max_mixer_blendstages = 0x7, @@ -501,6 +524,33 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = { }, }; +static const struct dpu_mdp_cfg sm8350_mdp[] = { + { + .name = "top_0", .id = MDP_TOP, + .base = 0x0, .len = 0x494, + .features = 0, + .highest_bank_bit = 0x3, /* TODO: 2 for LP_DDR4 */ + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { + .reg_off = 0x2AC, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_VIG1] = { + .reg_off = 0x2B4, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_VIG2] = { + .reg_off = 0x2BC, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_VIG3] = { + .reg_off = 0x2C4, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { + .reg_off = 0x2AC, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_DMA1] = { + .reg_off = 0x2B4, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { + .reg_off = 0x2BC, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = { + .reg_off = 0x2C4, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { + .reg_off = 0x2BC, .bit_off = 20}, + }, +}; + static const struct dpu_mdp_cfg sc7280_mdp[] = { { .name = "top_0", .id = MDP_TOP, @@ -659,6 +709,66 @@ static const struct dpu_ctl_cfg sm8150_ctl[] = { }, }; +static const struct dpu_ctl_cfg sm8350_ctl[] = { + { + .name = "ctl_0", .id = CTL_0, + .base = 0x15000, .len = 0x1e8, + .features = BIT(DPU_CTL_SPLIT_DISPLAY) | + BIT(DPU_CTL_PINGPONG_SPLIT) | + BIT(DPU_CTL_ACTIVE_CFG) | + BIT(DPU_CTL_FETCH_ACTIVE) | + BIT(DPU_CTL_VM_CFG) | + BIT(DPU_CTL_UNIFIED_DSPP_FLUSH), + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9), + }, + { + .name = "ctl_1", .id = CTL_1, + .base = 0x16000, .len = 0x1e8, + .features = BIT(DPU_CTL_SPLIT_DISPLAY) | + BIT(DPU_CTL_ACTIVE_CFG) | + BIT(DPU_CTL_FETCH_ACTIVE) | + BIT(DPU_CTL_VM_CFG) | + BIT(DPU_CTL_UNIFIED_DSPP_FLUSH), The UNIFIED_DSPP_FLUSH is not merged. Could you please change this to BIT(DPU_CTL_SPLIT_DISPLAY) | CTL_SC7280_MASK (for first two CTLs) and just CTL_SC7280_MASK for the rest of them? + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10), + }, + { + .name = "ctl_2", .id = CTL_2, + .base = 0x17000, .len = 0x1e8, + .features = BIT(DPU_CTL_ACTIVE_CFG) | + BIT(DPU_CTL_FETCH_ACTIVE) | + BIT(DPU_CTL_VM_CFG) | + BIT(DPU_CTL_UNIFIED_DSPP_FLUSH), + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 11), + }, + { + .name = "ctl_3", .id = CTL_3, + .base = 0x18000,
Re: [v6] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280
On 07/10/2022 17:34, Kalyan Thota wrote: -Original Message- From: Dmitry Baryshkov Sent: Tuesday, October 4, 2022 8:03 PM To: Kalyan Thota (QUIC) Cc: dri-devel@lists.freedesktop.org; linux-arm-...@vger.kernel.org; freedr...@lists.freedesktop.org; devicet...@vger.kernel.org; linux- ker...@vger.kernel.org; robdcl...@gmail.com; diand...@chromium.org; swb...@chromium.org; Vinod Polimera (QUIC) ; Abhinav Kumar (QUIC) Subject: Re: [v6] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280 WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. On Sun, 2 Oct 2022 at 06:15, Kalyan Thota wrote: Flush mechanism for DSPP blocks has changed in sc7280 family, it allows individual sub blocks to be flushed in coordination with master flush control. Representation: master_flush && (PCC_flush | IGC_flush .. etc ) This change adds necessary support for the above design. Changes in v1: - Few nits (Doug, Dmitry) - Restrict sub-block flush programming to dpu_hw_ctl file (Dmitry) Changes in v2: - Move the address offset to flush macro (Dmitry) - Seperate ops for the sub block flush (Dmitry) Changes in v3: - Reuse the DPU_DSPP_xx enum instead of a new one (Dmitry) Changes in v4: - Use shorter version for unsigned int (Stephen) Changes in v5: - Spurious patch please ignore. Changes in v6: - Add SOB tag (Doug, Dmitry) Signed-off-by: Kalyan Thota Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 35 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++-- 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 601d687..4170fbe 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -766,7 +766,7 @@ static void _dpu_crtc_setup_cp_blocks(struct drm_crtc *crtc) /* stage config flush mask */ ctl->ops.update_pending_flush_dspp(ctl, - mixer[i].hw_dspp->idx); + mixer[i].hw_dspp->idx, DPU_DSPP_PCC); } } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 27f029f..0eecb2f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -65,7 +65,10 @@ (PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2)) #define CTL_SC7280_MASK \ - (BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG)) + (BIT(DPU_CTL_ACTIVE_CFG) | \ +BIT(DPU_CTL_FETCH_ACTIVE) | \ +BIT(DPU_CTL_VM_CFG) | \ +BIT(DPU_CTL_DSPP_SUB_BLOCK_FLUSH)) #define MERGE_3D_SM8150_MASK (0) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index 38aa38a..8148e91 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -161,10 +161,12 @@ enum { * DSPP sub-blocks * @DPU_DSPP_PCC Panel color correction block * @DPU_DSPP_GC Gamma correction block + * @DPU_DSPP_IGC Inverse Gamma correction block */ enum { DPU_DSPP_PCC = 0x1, DPU_DSPP_GC, + DPU_DSPP_IGC, DPU_DSPP_MAX }; @@ -191,6 +193,7 @@ enum { * @DPU_CTL_SPLIT_DISPLAY: CTL supports video mode split display * @DPU_CTL_FETCH_ACTIVE: Active CTL for fetch HW (SSPPs) * @DPU_CTL_VM_CFG:CTL config to support multiple VMs + * @DPU_CTL_DSPP_BLOCK_FLUSH: CTL config to support dspp sub-block + flush * @DPU_CTL_MAX */ enum { @@ -198,6 +201,7 @@ enum { DPU_CTL_ACTIVE_CFG, DPU_CTL_FETCH_ACTIVE, DPU_CTL_VM_CFG, + DPU_CTL_DSPP_SUB_BLOCK_FLUSH, DPU_CTL_MAX }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c index a35ecb6..f26f484 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -33,6 +33,7 @@ #define CTL_INTF_FLUSH0x110 #define CTL_INTF_MASTER 0x134 #define CTL_FETCH_PIPE_ACTIVE 0x0FC +#define CTL_DSPP_n_FLUSH(n) ((0x13C) + ((n - 1) * 4)) #define CTL_MIXER_BORDER_OUTBIT(24) #define CTL_FLUSH_MASK_CTL BIT(17) @@ -287,8 +288,9 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx, } static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx, - enum dpu_dspp dspp) + enum dpu_dspp dspp, u32 dspp_sub_blk) { + switch (dspp) { case DSPP_0:
[PATCH v2 1/2] drm/ofdrm: Convert PCI IDs to CPU endianness for comparing
Properties of 32-bit integers are returned from the OF device tree as type __be32. Convert PCI vendor and device IDs from __be32 to host endianness before comparing them to constants. All relevant machines are old, big-endian Macintosh systems; hence the bug never happened in practice. Fixes sparse warnings shown below. drivers/gpu/drm/tiny/ofdrm.c:237:17: warning: restricted __be32 degrades to integer drivers/gpu/drm/tiny/ofdrm.c:238:18: warning: restricted __be32 degrades to integer drivers/gpu/drm/tiny/ofdrm.c:238:54: warning: restricted __be32 degrades to integer See [1] for the bug report. v2: * convert endianness (Alex) Reported-by: kernel test robot Signed-off-by: Thomas Zimmermann Link: https://lore.kernel.org/dri-devel/202210192208.d888i6x7-...@intel.com/ # [1] --- drivers/gpu/drm/tiny/ofdrm.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c index 0e1cc2369afcc..44f13a2b372be 100644 --- a/drivers/gpu/drm/tiny/ofdrm.c +++ b/drivers/gpu/drm/tiny/ofdrm.c @@ -231,7 +231,7 @@ static u64 display_get_address_of(struct drm_device *dev, struct device_node *of return address; } -static bool is_avivo(__be32 vendor, __be32 device) +static bool is_avivo(u32 vendor, u32 device) { /* This will match most R5xx */ return (vendor == PCI_VENDOR_ID_ATI) && @@ -265,8 +265,13 @@ static enum ofdrm_model display_get_model_of(struct drm_device *dev, struct devi of_parent = of_get_parent(of_node); vendor_p = of_get_property(of_parent, "vendor-id", NULL); device_p = of_get_property(of_parent, "device-id", NULL); - if (vendor_p && device_p && is_avivo(*vendor_p, *device_p)) - model = OFDRM_MODEL_AVIVO; + if (vendor_p && device_p) { + u32 vendor = be32_to_cpup(vendor_p); + u32 device = be32_to_cpup(device_p); + + if (is_avivo(vendor, device)) + model = OFDRM_MODEL_AVIVO; + } of_node_put(of_parent); } else if (of_device_is_compatible(of_node, "qemu,std-vga")) { model = OFDRM_MODEL_QEMU; -- 2.38.0
[PATCH v2 0/2] drm/ofdrm: Fix sparse warnings
Fix two types of sparse warnings in ofdrm. Reported by the LKP bot. v2: * convert PCI ID endianness (Alex) Thomas Zimmermann (2): drm/ofdrm: Convert PCI IDs to CPU endianness for comparing drm/ofdrm: Cast error pointers to void __iomem * drivers/gpu/drm/tiny/ofdrm.c | 27 --- 1 file changed, 16 insertions(+), 11 deletions(-) base-commit: 746559738f1335241ea686566cb654847c20d7a4 -- 2.38.0
[PATCH v2 2/2] drm/ofdrm: Cast error pointers to void __iomem *
Cast error pointers when returning them as void __iomem *. Fixes a number of Sparse warnings, such as the ones shown below. ../drivers/gpu/drm/tiny/ofdrm.c:439:31: warning: incorrect type in return expression (different address spaces) ../drivers/gpu/drm/tiny/ofdrm.c:439:31:expected void [noderef] __iomem * ../drivers/gpu/drm/tiny/ofdrm.c:439:31:got void * ../drivers/gpu/drm/tiny/ofdrm.c:442:31: warning: incorrect type in return expression (different address spaces) ../drivers/gpu/drm/tiny/ofdrm.c:442:31:expected void [noderef] __iomem * ../drivers/gpu/drm/tiny/ofdrm.c:442:31:got void * See [1] for the bug report. Reported-by: kernel test robot Signed-off-by: Thomas Zimmermann Link: https://lore.kernel.org/dri-devel/202210200016.yiqzpiy0-...@intel.com/ # [1] --- drivers/gpu/drm/tiny/ofdrm.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c index 44f13a2b372be..f1c301820d54b 100644 --- a/drivers/gpu/drm/tiny/ofdrm.c +++ b/drivers/gpu/drm/tiny/ofdrm.c @@ -438,21 +438,21 @@ static void __iomem *get_cmap_address_of(struct ofdrm_device *odev, struct devic if (!addr_p) addr_p = of_get_address(of_node, bar_no, _size, ); if (!addr_p) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); if ((offset + size) >= max_size) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); address = of_translate_address(of_node, addr_p); if (address == OF_BAD_ADDR) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); mem = devm_ioremap(dev->dev, address + offset, size); if (!mem) - return ERR_PTR(-ENOMEM); + return (void __iomem *)ERR_PTR(-ENOMEM); return mem; } @@ -470,7 +470,7 @@ static void __iomem *ofdrm_mach64_cmap_ioremap(struct ofdrm_device *odev, cmap_base = devm_ioremap(dev->dev, address, 0x1000); if (!cmap_base) - return ERR_PTR(-ENOMEM); + return (void __iomem *)ERR_PTR(-ENOMEM); return cmap_base; } @@ -629,11 +629,11 @@ static void __iomem *ofdrm_qemu_cmap_ioremap(struct ofdrm_device *odev, address = of_translate_address(of_node, io_of_addr); if (address == OF_BAD_ADDR) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); cmap_base = devm_ioremap(dev->dev, address + 0x3c8, 2); if (!cmap_base) - return ERR_PTR(-ENOMEM); + return (void __iomem *)ERR_PTR(-ENOMEM); return cmap_base; } -- 2.38.0
Re: [PATCH v1 1/9] drm/msm: Add compatibles for SM8350 display
On 28/10/2022 15:08, Robert Foss wrote: Add compatible string for "qcom,sm8350-dpu" and "qcom,sm8350-mdss". Signed-off-by: Robert Foss --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 + drivers/gpu/drm/msm/msm_mdss.c | 1 + 2 files changed, 2 insertions(+) [skipped] diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index e13c5c12b775..fd5a95cace16 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -447,6 +447,7 @@ static const struct of_device_id mdss_dt_match[] = { { .compatible = "qcom,sc8180x-mdss" }, { .compatible = "qcom,sm8150-mdss" }, { .compatible = "qcom,sm8250-mdss" }, + { .compatible = "qcom,sm8350-mdss" }, {} }; MODULE_DEVICE_TABLE(of, mdss_dt_match); BTW: you probably also have to update the msm_mdss_enable() function with the 8350-specific code. -- With best wishes Dmitry
Re: [PATCH v1 2/9] drm/msm/dpu: Refactor sc7280_pp location
On 28/10/2022 15:08, Robert Foss wrote: The sc7280_pp declaration is not located by the other _pp declarations, but rather hidden around the _merge_3d declarations. Let's fix this to avoid confusion. Signed-off-by: Robert Foss --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v1 1/9] drm/msm: Add compatibles for SM8350 display
On 28/10/2022 15:08, Robert Foss wrote: Add compatible string for "qcom,sm8350-dpu" and "qcom,sm8350-mdss". Signed-off-by: Robert Foss --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 + drivers/gpu/drm/msm/msm_mdss.c | 1 + 2 files changed, 2 insertions(+) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v7 01/10] drm: bridge: Add Samsung DSIM bridge driver
On Tue, Oct 18, 2022 at 8:35 AM Jagan Teki wrote: > > On Mon, Oct 17, 2022 at 2:31 PM Marek Szyprowski > wrote: > > > > Hi, > > > > On 17.10.2022 10:48, Marek Vasut wrote: > > > On 10/17/22 09:43, Jagan Teki wrote: > > >> On Mon, Oct 17, 2022 at 12:49 PM Marek Vasut wrote: > > >>> On 10/17/22 04:49, Jagan Teki wrote: > > On Sun, Oct 16, 2022 at 3:16 AM Marek Vasut wrote: > > > > > > On 10/5/22 17:13, Jagan Teki wrote: > > >> Samsung MIPI DSIM controller is common DSI IP that can be used in > > >> various > > >> SoCs like Exynos, i.MX8M Mini/Nano. > > >> > > >> In order to access this DSI controller between various platform > > >> SoCs, > > >> the ideal way to incorporate this in the drm stack is via the drm > > >> bridge > > >> driver. > > >> > > >> This patch is trying to differentiate platform-specific and > > >> bridge driver > > >> code by maintaining exynos platform glue code in exynos_drm_dsi.c > > >> driver > > >> and common bridge driver code in samsung-dsim.c providing that > > >> the new > > >> platform-specific glue should be supported in the bridge driver, > > >> unlike > > >> exynos platform drm drivers. > > >> > > >> - Add samsung_dsim_plat_data for keeping platform-specific > > >> attributes like > > >> host_ops, irq_ops, and hw_type. > > >> > > >> - Initialize the plat_data hooks for exynos platform in > > >> exynos_drm_dsi.c. > > >> > > >> - samsung_dsim_probe is the common probe call across > > >> exynos_drm_dsi.c and > > >> samsung-dsim.c. > > >> > > >> - plat_data hooks like host_ops and irq_ops are invoked during the > > >> respective bridge call chains. > > > > > > Maybe the Subject should say "Split ... driver" or "Move ... > > > driver" , > > > since it is not adding a new driver here ? > > > > Though it is not added a completely new driver, it is adding more > > infrastructure platform code to be compatible with both Exynos and > > i.MX8M. This is the prime reason for adding that commit head and > > explaining the same in the commit body. > > >>> > > >>> Diffstat looks like this: > > >>> > > >>>drivers/gpu/drm/bridge/samsung-dsim.c | 1703 > > >>> ++ > > >>>drivers/gpu/drm/exynos/Kconfig |1 + > > >>>drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1766 > > >>> ++- > > >>>include/drm/bridge/samsung-dsim.h | 113 ++ > > >>>7 files changed, 1952 insertions(+), 1653 deletions(-) > > >>> > > >>> Looks to me like most of the code is just moved from existing driver in > > >>> this patch. > > >> > > >> Yeah, as I explained (from commit) it is moved, updated, and written > > >> the plat code. How about this head? > > >> > > >> "drm: bridge: Add Samsung DSIM bridge (Split from exynos_drm_dsi)" > > > > > > I disagree with the "Add" part of the Subject, but I'll wait for > > > others' opinion here. > > > > Maybe something like a "Generalize Exynos-DSI DRM driver into a generic > > Samsung DSIM bridge"? I'm using this commit head for next version patches, hope all Okay with it. Thanks, Jagan.
[PATCH v1 9/9] arm64: dts: qcom: sm8350-hdk: Enable lt9611uxc dsi-hdmi bridge
The sm8350-hdk ships with the LT9611 UXC DSI/HDMI bridge chip. In order to toggle the board to enable the HDMI output, switch #7 & #8 on the rightmost multi-switch package have to be toggled to On. Signed-off-by: Robert Foss --- arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 106 1 file changed, 106 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts index 6e07feb4b3b2..b38b58f8 100644 --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts @@ -20,6 +20,17 @@ chosen { stdout-path = "serial0:115200n8"; }; + hdmi-out { + compatible = "hdmi-connector"; + type = "a"; + + port { + hdmi_con: endpoint { + remote-endpoint = <_out>; + }; + }; + }; + vph_pwr: vph-pwr-regulator { compatible = "regulator-fixed"; regulator-name = "vph_pwr"; @@ -29,6 +40,32 @@ vph_pwr: vph-pwr-regulator { regulator-always-on; regulator-boot-on; }; + + lt9611_1v2: lt9611-1v2 { + compatible = "regulator-fixed"; + regulator-name = "LT9611_1V2"; + + vin-supply = <_pwr>; + regulator-min-microvolt = <120>; + regulator-max-microvolt = <120>; + gpio = < 49 GPIO_ACTIVE_HIGH>; + enable-active-high; + regulator-boot-on; + regulator-always-on; + }; + + lt9611_3v3: lt9611-3v3 { + compatible = "regulator-fixed"; + regulator-name = "LT9611_3V3"; + + vin-supply = <_bob>; + gpio = < 47 GPIO_ACTIVE_HIGH>; + regulator-min-microvolt = <330>; + regulator-max-microvolt = <330>; + enable-active-high; + regulator-boot-on; + regulator-always-on; + }; }; { @@ -220,6 +257,15 @@ { { status = "okay"; vdda-supply = <_l6b_1p2>; + + ports { + port@1 { + endpoint { + remote-endpoint = <_a>; + data-lanes = <0 1 2 3>; + }; + }; + }; }; _phy { @@ -231,6 +277,48 @@ _dma1 { status = "okay"; }; + { + status = "okay"; + clock-frequency = <40>; + + lt9611_codec: hdmi-bridge@2b { + compatible = "lontium,lt9611uxc"; + reg = <0x2b>; + status = "okay"; + + interrupts-extended = < 50 IRQ_TYPE_EDGE_FALLING>; + reset-gpios = < 48 GPIO_ACTIVE_HIGH>; + + vdd-supply = <_1v2>; + vcc-supply = <_3v3>; + + pinctrl-names = "default"; + pinctrl-0 = <_irq_pin _rst_pin>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + lt9611_a: endpoint { + remote-endpoint = <_out>; + }; + }; + + port@2 { + reg = <2>; + + lt9611_out: endpoint { + remote-endpoint = <_con>; + }; + }; + + }; + }; +}; + { status = "okay"; }; @@ -248,6 +336,10 @@ _id_0 { status = "okay"; }; +_id_2 { + status = "okay"; +}; + { status = "okay"; firmware-name = "qcom/sm8350/slpi.mbn"; @@ -544,4 +636,18 @@ usb_hub_enabled_state: usb-hub-enabled-state { drive-strength = <2>; output-low; }; + + lt9611_rst_pin: lt9611-rst-state { + pins = "gpio48"; + function = "normal"; + + output-high; + input-disable; + }; + + lt9611_irq_pin: lt9611-irq { + pins = "gpio50"; + function = "gpio"; + bias-disable; + }; }; -- 2.34.1
[PATCH v1 8/9] arm64: dts: qcom: sm8350-hdk: Enable display & dsi nodes
Enable the display subsystem and the dsi0 output for the sm8350-hdk board. Signed-off-by: Robert Foss --- arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 22 ++ 1 file changed, 22 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts index e6deb08c6da0..6e07feb4b3b2 100644 --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts @@ -213,10 +213,32 @@ { firmware-name = "qcom/sm8350/cdsp.mbn"; }; + { + status = "okay"; +}; + + { + status = "okay"; + vdda-supply = <_l6b_1p2>; +}; + +_phy { + status = "okay"; + vdds-supply = <_l5b_0p88>; +}; + _dma1 { status = "okay"; }; + { + status = "okay"; +}; + +_mdp { + status = "okay"; +}; + { status = "okay"; firmware-name = "qcom/sm8350/modem.mbn"; -- 2.34.1
[PATCH v1 7/9] arm64: dts: qcom: sm8350: Add display system nodes
Add mdss, mdss_mdp, dsi0, dsi0_phy nodes. With these nodes the display subsystem is configured to support one DSI output. Signed-off-by: Robert Foss --- arch/arm64/boot/dts/qcom/sm8350.dtsi | 196 ++- 1 file changed, 192 insertions(+), 4 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi index b6e44cd3b394..eaa3cdee1860 100644 --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi @@ -3,6 +3,7 @@ * Copyright (c) 2020, Linaro Limited */ +#include #include #include #include @@ -2535,14 +2536,200 @@ usb_2_dwc3: usb@a80 { }; }; + mdss: mdss@ae0 { + compatible = "qcom,sm8350-mdss"; + reg = <0 0x0ae0 0 0x1000>; + reg-names = "mdss"; + + interconnects = <_noc MASTER_MDP0 0 _virt SLAVE_EBI1 0>, + <_noc MASTER_MDP1 0 _virt SLAVE_EBI1 0>; + interconnect-names = "mdp0-mem", "mdp1-mem"; + + power-domains = < MDSS_GDSC>; + resets = < DISP_CC_MDSS_CORE_BCR>; + + clocks = < DISP_CC_MDSS_AHB_CLK>, +< GCC_DISP_HF_AXI_CLK>, +< GCC_DISP_SF_AXI_CLK>, +< DISP_CC_MDSS_MDP_CLK>; + clock-names = "iface", "bus", "nrt_bus", "core"; + + interrupts = ; + interrupt-controller; + #interrupt-cells = <1>; + + status = "ok"; + + #address-cells = <2>; + #size-cells = <2>; + ranges; + + mdss_mdp: mdp@ae01000 { + compatible = "qcom,sm8350-dpu"; + reg = <0 0x0ae01000 0 0x8f000>, + <0 0x0aeb 0 0x2008>; + reg-names = "mdp", "vbif"; + iommus = <_smmu 0x820 0x402>; + + clocks = < GCC_DISP_HF_AXI_CLK>, + < GCC_DISP_SF_AXI_CLK>, + < DISP_CC_MDSS_AHB_CLK>, + < DISP_CC_MDSS_MDP_LUT_CLK>, + < DISP_CC_MDSS_MDP_CLK>, + < DISP_CC_MDSS_VSYNC_CLK>; + clock-names = "bus", + "nrt_bus", + "iface", + "lut", + "core", + "vsync"; + + assigned-clocks = < DISP_CC_MDSS_VSYNC_CLK>; + assigned-clock-rates = <1920>; + + operating-points-v2 = <_opp_table>; + power-domains = < SM8350_MMCX>; + + interrupt-parent = <>; + interrupts = <0>; + + status = "ok"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dpu_intf1_out: endpoint { + remote-endpoint = <_in>; + }; + }; + }; + + mdp_opp_table: mdp-opp-table { + compatible = "operating-points-v2"; + + opp-2 { + opp-hz = /bits/ 64 <2>; + required-opps = <_opp_low_svs>; + }; + + opp-3 { + opp-hz = /bits/ 64 <3>; + required-opps = <_opp_svs>; + }; + + opp-34500 { + opp-hz = /bits/ 64 <34500>; + required-opps = <_opp_svs_l1>; + }; + + opp-46000 { + opp-hz = /bits/ 64 <46000>; +
[PATCH v1 5/9] arm64: dts: qcom: sm8350: Remove mmxc power-domain-name
The mmxc power-domain-name is not required, and is not used by either earlier or later SoC versions (sm8250 / sm8450). Signed-off-by: Robert Foss --- arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi index e72a04411888..606fab087945 100644 --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi @@ -2557,7 +2557,6 @@ dispcc: clock-controller@af0 { #power-domain-cells = <1>; power-domains = < SM8350_MMCX>; - power-domain-names = "mmcx"; }; adsp: remoteproc@1730 { -- 2.34.1
[PATCH v1 6/9] arm64: dts: qcom: sm8350: Use 2 interconnect cells
Use two interconnect cells in order to optionally support a path tag. Signed-off-by: Robert Foss --- arch/arm64/boot/dts/qcom/sm8350.dtsi | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi index 606fab087945..b6e44cd3b394 100644 --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi @@ -1543,56 +1543,56 @@ apps_smmu: iommu@1500 { config_noc: interconnect@150 { compatible = "qcom,sm8350-config-noc"; reg = <0 0x0150 0 0xa580>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; mc_virt: interconnect@158 { compatible = "qcom,sm8350-mc-virt"; reg = <0 0x0158 0 0x1000>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; system_noc: interconnect@168 { compatible = "qcom,sm8350-system-noc"; reg = <0 0x0168 0 0x1c200>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; aggre1_noc: interconnect@16e { compatible = "qcom,sm8350-aggre1-noc"; reg = <0 0x016e 0 0x1f180>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; aggre2_noc: interconnect@170 { compatible = "qcom,sm8350-aggre2-noc"; reg = <0 0x0170 0 0x33000>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; mmss_noc: interconnect@174 { compatible = "qcom,sm8350-mmss-noc"; reg = <0 0x0174 0 0x1f080>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; lpass_ag_noc: interconnect@3c4 { compatible = "qcom,sm8350-lpass-ag-noc"; reg = <0 0x03c4 0 0xf080>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; compute_noc: interconnect@a0c{ compatible = "qcom,sm8350-compute-noc"; reg = <0 0x0a0c 0 0xa180>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; @@ -2420,14 +2420,14 @@ usb_2_ssphy: phy@88ebe00 { dc_noc: interconnect@90c { compatible = "qcom,sm8350-dc-noc"; reg = <0 0x090c 0 0x4200>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; gem_noc: interconnect@910 { compatible = "qcom,sm8350-gem-noc"; reg = <0 0x0910 0 0xb4000>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <_bcm_voter>; }; -- 2.34.1
[PATCH v1 1/9] drm/msm: Add compatibles for SM8350 display
Add compatible string for "qcom,sm8350-dpu" and "qcom,sm8350-mdss". Signed-off-by: Robert Foss --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 + drivers/gpu/drm/msm/msm_mdss.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 008e1420e6e5..70683dbc6b32 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1315,6 +1315,7 @@ static const struct of_device_id dpu_dt_match[] = { { .compatible = "qcom,sc8180x-dpu", }, { .compatible = "qcom,sm8150-dpu", }, { .compatible = "qcom,sm8250-dpu", }, + { .compatible = "qcom,sm8350-dpu", }, {} }; MODULE_DEVICE_TABLE(of, dpu_dt_match); diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index e13c5c12b775..fd5a95cace16 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -447,6 +447,7 @@ static const struct of_device_id mdss_dt_match[] = { { .compatible = "qcom,sc8180x-mdss" }, { .compatible = "qcom,sm8150-mdss" }, { .compatible = "qcom,sm8250-mdss" }, + { .compatible = "qcom,sm8350-mdss" }, {} }; MODULE_DEVICE_TABLE(of, mdss_dt_match); -- 2.34.1
[PATCH v1 2/9] drm/msm/dpu: Refactor sc7280_pp location
The sc7280_pp declaration is not located by the other _pp declarations, but rather hidden around the _merge_3d declarations. Let's fix this to avoid confusion. Signed-off-by: Robert Foss --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 0239a811d5ec..d0ce7612fee8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -1175,6 +1175,13 @@ static const struct dpu_pingpong_cfg sm8150_pp[] = { -1), }; +static const struct dpu_pingpong_cfg sc7280_pp[] = { + PP_BLK("pingpong_0", PINGPONG_0, 0x59000, 0, sc7280_pp_sblk, -1, -1), + PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, 0, sc7280_pp_sblk, -1, -1), + PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1, -1), + PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1), +}; + static struct dpu_pingpong_cfg qcm2290_pp[] = { PP_BLK("pingpong_0", PINGPONG_0, 0x7, 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), @@ -1198,13 +1205,6 @@ static const struct dpu_merge_3d_cfg sm8150_merge_3d[] = { MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200), }; -static const struct dpu_pingpong_cfg sc7280_pp[] = { - PP_BLK("pingpong_0", PINGPONG_0, 0x59000, 0, sc7280_pp_sblk, -1, -1), - PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, 0, sc7280_pp_sblk, -1, -1), - PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1, -1), - PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1), -}; - /* * DSC sub blocks config */ -- 2.34.1
[PATCH v1 4/9] arm64: dts: qcom: sm8350: Add gpio-line-names
Add GPIO line names as described by the sm8350-hdk schematic. Signed-off-by: Robert Foss --- arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 205 1 file changed, 205 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts index 0fcf5bd88fc7..e6deb08c6da0 100644 --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts @@ -233,6 +233,211 @@ { { gpio-reserved-ranges = <52 8>; + + gpio-line-names = + "APPS_I2C_SDA", /* GPIO_0 */ + "APPS_I2C_SCL", + "FSA_INT_N", + "USER_LED3_EN", + "SMBUS_SDA_1P8", + "SMBUS_SCL_1P8", + "2M2_3P3_EN", + "ALERT_DUAL_M2_N", + "EXP_UART_CTS", + "EXP_UART_RFR", + "EXP_UART_TX", /* GPIO_10 */ + "EXP_UART_RX", + "NC", + "NC", + "RCM_MARKER1", + "WSA0_EN", + "CAM1_RESET_N", + "CAM0_RESET_N", + "DEBUG_UART_TX", + "DEBUG_UART_RX", + "TS_I2C_SDA", /* GPIO_20 */ + "TS_I2C_SCL", + "TS_RESET_N", + "TS_INT_N", + "DISP0_RESET_N", + "DISP1_RESET_N", + "ETH_RESET", + "RCM_MARKER2", + "CAM_DC_MIPI_MUX_EN", + "CAM_DC_MIPI_MUX_SEL", + "AFC_PHY_TA_D_PLUS", /* GPIO_30 */ + "AFC_PHY_TA_D_MINUS", + "PM8008_1_IRQ", + "PM8008_1_RESET_N", + "PM8008_2_IRQ", + "PM8008_2_RESET_N", + "CAM_DC_I3C_SDA", + "CAM_DC_I3C_SCL", + "FP_INT_N", + "FP_WUHB_INT_N", + "SMB_SPMI_DATA", /* GPIO_40 */ + "SMB_SPMI_CLK", + "USB_HUB_RESET", + "FORCE_USB_BOOT", + "LRF_IRQ", + "NC", + "IMU2_INT", + "HDMI_3P3_EN", + "HDMI_RSTN", + "HDMI_1P2_EN", + "HDMI_INT", /* GPIO_50 */ + "USB1_ID", + "FP_SPI_MISO", + "FP_SPI_MOSI", + "FP_SPI_CLK", + "FP_SPI_CS_N", + "NFC_ESE_SPI_MISO", + "NFC_ESE_SPI_MOSI", + "NFC_ESE_SPI_CLK", + "NFC_ESE_SPI_CS", + "NFC_I2C_SDA", /* GPIO_60 */ + "NFC_I2C_SCLC", + "NFC_EN", + "NFC_CLK_REQ", + "HST_WLAN_EN", + "HST_BT_EN", + "HST_SW_CTRL", + "NC", + "HST_BT_UART_CTS", + "HST_BT_UART_RFR", + "HST_BT_UART_TX", /* GPIO_70 */ + "HST_BT_UART_RX", + "CAM_DC_SPI0_MISO", + "CAM_DC_SPI0_MOSI", + "CAM_DC_SPI0_CLK", + "CAM_DC_SPI0_CS_N", + "CAM_DC_SPI1_MISO", + "CAM_DC_SPI1_MOSI", + "CAM_DC_SPI1_CLK", + "CAM_DC_SPI1_CS_N", + "HALL_INT_N", /* GPIO_80 */ + "USB_PHY_PS", + "MDP_VSYNC_P", + "MDP_VSYNC_S", + "ETH_3P3_EN", + "RADAR_INT", + "NFC_DWL_REQ", + "SM_GPIO_87", + "WCD_RESET_N", + "ALSP_INT_N", + "PRESS_INT", /* GPIO_90 */ + "SAR_INT_N", + "SD_CARD_DET_N", + "NC", + "PCIE0_RESET_N", + "PCIE0_CLK_REQ_N", + "PCIE0_WAKE_N", + "PCIE1_RESET_N", + "PCIE1_CLK_REQ_N", + "PCIE1_WAKE_N", + "CAM_MCLK0", /* GPIO_100 */ + "CAM_MCLK1", + "CAM_MCLK2", + "CAM_MCLK3", + "CAM_MCLK4", + "CAM_MCLK5", + "CAM2_RESET_N", + "CCI_I2C0_SDA", + "CCI_I2C0_SCL", + "CCI_I2C1_SDA", + "CCI_I2C1_SCL", /* GPIO_110 */ + "CCI_I2C2_SDA", + "CCI_I2C2_SCL", + "CCI_I2C3_SDA", + "CCI_I2C3_SCL", + "CAM5_RESET_N", + "CAM4_RESET_N", + "CAM3_RESET_N", + "IMU1_INT", + "MAG_INT_N", + "MI2S2_I2S_SCK", /* GPIO_120 */ + "MI2S2_I2S_DAT0", + "MI2S2_I2S_WS", + "HIFI_DAC_I2S_MCLK", + "MI2S2_I2S_DAT1", + "HIFI_DAC_I2S_SCK", + "HIFI_DAC_I2S_DAT0", + "NC", + "HIFI_DAC_I2S_WS", + "HST_BT_WLAN_SLIMBUS_CLK", + "HST_BT_WLAN_SLIMBUS_DAT0", /* GPIO_130 */ + "BT_LED_EN", + "WLAN_LED_EN", + "NC", +
[PATCH v1 3/9] drm/msm/dpu: Add SM8350 to hw catalog
Add compatibility for SM8350 display subsystem, including required entries in DPU hw catalog. --- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 217 ++ .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 + 2 files changed, 218 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index d0ce7612fee8..bc829d7bdd6e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -112,6 +112,15 @@ BIT(MDP_INTF3_INTR) | \ BIT(MDP_INTF4_INTR)) +#define IRQ_SM8350_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ +BIT(MDP_SSPP_TOP0_INTR2) | \ +BIT(MDP_SSPP_TOP0_HIST_INTR) | \ +BIT(MDP_INTF0_7xxx_INTR) | \ +BIT(MDP_INTF1_7xxx_INTR) | \ +BIT(MDP_INTF2_7xxx_INTR) | \ +BIT(MDP_INTF3_7xxx_INTR) | \ +0) + #define IRQ_SC8180X_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ @@ -364,6 +373,20 @@ static const struct dpu_caps sm8250_dpu_caps = { .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, }; +static const struct dpu_caps sm8350_dpu_caps = { + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH, + .max_mixer_blendstages = 0xb, + .qseed_type = DPU_SSPP_SCALER_QSEED3LITE, + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */ + .ubwc_version = DPU_HW_UBWC_VER_40, + .has_src_split = true, + .has_dim_layer = true, + .has_idle_pc = true, + .has_3d_merge = true, + .max_linewidth = 4096, + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, +}; + static const struct dpu_caps sc7280_dpu_caps = { .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH, .max_mixer_blendstages = 0x7, @@ -501,6 +524,33 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = { }, }; +static const struct dpu_mdp_cfg sm8350_mdp[] = { + { + .name = "top_0", .id = MDP_TOP, + .base = 0x0, .len = 0x494, + .features = 0, + .highest_bank_bit = 0x3, /* TODO: 2 for LP_DDR4 */ + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { + .reg_off = 0x2AC, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_VIG1] = { + .reg_off = 0x2B4, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_VIG2] = { + .reg_off = 0x2BC, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_VIG3] = { + .reg_off = 0x2C4, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { + .reg_off = 0x2AC, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_DMA1] = { + .reg_off = 0x2B4, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { + .reg_off = 0x2BC, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = { + .reg_off = 0x2C4, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { + .reg_off = 0x2BC, .bit_off = 20}, + }, +}; + static const struct dpu_mdp_cfg sc7280_mdp[] = { { .name = "top_0", .id = MDP_TOP, @@ -659,6 +709,66 @@ static const struct dpu_ctl_cfg sm8150_ctl[] = { }, }; +static const struct dpu_ctl_cfg sm8350_ctl[] = { + { + .name = "ctl_0", .id = CTL_0, + .base = 0x15000, .len = 0x1e8, + .features = BIT(DPU_CTL_SPLIT_DISPLAY) | + BIT(DPU_CTL_PINGPONG_SPLIT) | + BIT(DPU_CTL_ACTIVE_CFG) | + BIT(DPU_CTL_FETCH_ACTIVE) | + BIT(DPU_CTL_VM_CFG) | + BIT(DPU_CTL_UNIFIED_DSPP_FLUSH), + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9), + }, + { + .name = "ctl_1", .id = CTL_1, + .base = 0x16000, .len = 0x1e8, + .features = BIT(DPU_CTL_SPLIT_DISPLAY) | + BIT(DPU_CTL_ACTIVE_CFG) | + BIT(DPU_CTL_FETCH_ACTIVE) | + BIT(DPU_CTL_VM_CFG) | + BIT(DPU_CTL_UNIFIED_DSPP_FLUSH), + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10), + }, + { + .name = "ctl_2", .id = CTL_2, + .base = 0x17000, .len = 0x1e8, + .features = BIT(DPU_CTL_ACTIVE_CFG) | + BIT(DPU_CTL_FETCH_ACTIVE) | + BIT(DPU_CTL_VM_CFG) | + BIT(DPU_CTL_UNIFIED_DSPP_FLUSH), + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 11), + }, + { + .name = "ctl_3", .id = CTL_3, + .base = 0x18000, .len = 0x1e8, + .features = BIT(DPU_CTL_ACTIVE_CFG) | + BIT(DPU_CTL_FETCH_ACTIVE) | + BIT(DPU_CTL_VM_CFG) | + BIT(DPU_CTL_UNIFIED_DSPP_FLUSH), + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 12), + }, +
[PATCH v1 0/9] Enable Display for SM8350
This series implements display support for SM8350 and enables HDMI output for the SM8350-HDK platform. Robert Foss (9): drm/msm: Add compatibles for SM8350 display drm/msm/dpu: Refactor sc7280_pp location drm/msm/dpu: Add SM8350 to hw catalog arm64: dts: qcom: sm8350: Add gpio-line-names arm64: dts: qcom: sm8350: Remove mmxc power-domain-name arm64: dts: qcom: sm8350: Use 2 interconnect cells arm64: dts: qcom: sm8350: Add display system nodes arm64: dts: qcom: sm8350-hdk: Enable display & dsi nodes arm64: dts: qcom: sm8350-hdk: Enable lt9611uxc dsi-hdmi bridge arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 333 ++ arch/arm64/boot/dts/qcom/sm8350.dtsi | 217 +++- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 227 +++- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 + drivers/gpu/drm/msm/msm_mdss.c| 1 + 6 files changed, 760 insertions(+), 20 deletions(-) -- 2.34.1
Re: Try to address the DMA-buf coherency problem
Am Freitag, dem 28.10.2022 um 10:40 +0200 schrieb Christian König: > Hi Lucas, > > Am 28.10.22 um 10:09 schrieb Lucas Stach: > > Hi Christian, > > > > Am Donnerstag, dem 20.10.2022 um 14:13 +0200 schrieb Christian König: > > > Hi guys, > > > > > > after finding that we essentially have two separate worlds for coherent > > > sharing > > > of buffer through DMA-buf I thought I will tackle that problem a bit and > > > at > > > least allow the framework to reject attachments which won't work. > > > > > > So those patches here add a new dma_coherent flag to each DMA-buf object > > > telling the framework that dev_is_dma_coherent() needs to return true for > > > an > > > importing device to be able to attach. Since we should always have a > > > fallback > > > path this should give userspace the chance to still keep the use case > > > working, > > > either by doing a CPU copy instead or reversing the roles of exporter and > > > importer. > > > > > The fallback would likely be a CPU copy with the appropriate cache > > flushes done by the device driver, which would be a massiv performance > > issue. > > But essentially the right thing to do. The only alternative I can see is > to reverse the role of exporter and importer. > I don't think that would work generally either, as buffer exporter and importer isn't always a 1:1 thing. As soon as any attached importer has a different coherency behavior than the others, things fall apart. > > > For DRM and most V4L2 devices I then fill in the dma_coherent flag based > > > on the > > > return value of dev_is_dma_coherent(). Exporting drivers are allowed to > > > clear > > > the flag for their buffers if special handling like the USWC flag in > > > amdgpu or > > > the uncached allocations for radeon/nouveau are in use. > > > > > I don't think the V4L2 part works for most ARM systems. The default > > there is for devices to be noncoherent unless explicitly marked > > otherwise. I don't think any of the "devices" writing the video buffers > > in cached memory with the CPU do this. While we could probably mark > > them as coherent, I don't think this is moving in the right direction. > > Well why not? Those devices are coherent in the sense of the DMA API > that they don't need an extra CPU copy on sync_to_cpu/sync_to_device. > > We could come up with a better name for coherency, e.g. snooping for > example. But that is just an documentation detail. > I agree that those devices copying data into a CPU cacheable buffer should be marked as coherent, just not sure right now if other things like DMA mappings are done on that device, which would require the cache maintenance. > > > Additional to that importers can also check the flag if they have some > > > non-snooping operations like the special scanout case for amdgpu for > > > example. > > > > > > The patches are only smoke tested and the solution isn't ideal, but as > > > far as > > > I can see should at least keep things working. > > > > > I would like to see this solved properly. Where I think properly means > > we make things work on systems with mixed coherent/noncoherent masters > > in the same way the DMA API does on such systems: by inserting the > > proper cache maintenance operations. > > And this the exact wrong approach as far as I can see. As Daniel noted > as well we absolutely need some kind of coherency between exporter and > importer. > I think it's important that we are very specific about the thing we are talking about here: I guess when you say coherency you mean hardware enforced coherency on cacheable memory, which is the default on x86/PCI. The other way to enforce coherency is to either insert cache maintenance operations, or make sure that the buffer is not cacheable by any device taking part in the sharing, including the CPU. > This can either be provided by the PCI specification which makes it > mandatory for device to snoop the caches or by platform devices agreeing > that they only work on uncached memory. What you disregard here is the fact that there are many systems out there with mixed topologies, where some masters are part of the coherency domain and some are not. We have two options here: either mandate that coherency for dma-bufs need to be established by hardware, which is the option that you strongly prefer, which means forcing all buffers to be uncacheable in a system with masters that are not coherent, or allowing some form of bracketed DMA access with cache maintenance ops. > > Explicit cache flush operations are simple not part of the design of > DMA-buf because they are not part of the design of the higher level APIs > like Vulkan and OpenGL. I'm aware that some graphics APIs have been living in a universe blissfully unaware of systems without hardware enforced coherency. But that isn't the only use for dma-bufs. I totally agree that some higher level API primitives aren't possible without coherency at the hardware level and for those uses we