Re: [PATCH v2 2/9] drm/sched: Move schedule policy to scheduler / entity
On Fri, Aug 11, 2023 at 06:43:22PM -0300, Maira Canal wrote:
> Hi Matthew,
>
> I'm not sure in which tree you plan to apply this series, but if you
> plan to apply it on drm-misc-next, it would be nice to rebase on top of
> it. It would make it easier for driver maintainers to review it.
>
I rebased this on drm-tip but forgot the first patch in the series.
Let me make sure I get this correct and will send a rev3 early next week.
> Apart from the small nit below it, I tested the Xe tree on v3d and things
> seems to be running smoothly.
>
> On 8/10/23 23:31, Matthew Brost wrote:
> > Rather than a global modparam for scheduling policy, move the scheduling
> > policy to scheduler / entity so user can control each scheduler / entity
> > policy.
> >
> > v2:
> >- s/DRM_SCHED_POLICY_MAX/DRM_SCHED_POLICY_COUNT (Luben)
> >- Only include policy in scheduler (Luben)
> >
> > Signed-off-by: Matthew Brost
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
> > drivers/gpu/drm/etnaviv/etnaviv_sched.c| 3 ++-
> > drivers/gpu/drm/lima/lima_sched.c | 3 ++-
> > drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++-
> > drivers/gpu/drm/nouveau/nouveau_sched.c| 3 ++-
> > drivers/gpu/drm/panfrost/panfrost_job.c| 3 ++-
> > drivers/gpu/drm/scheduler/sched_entity.c | 24 ++
> > drivers/gpu/drm/scheduler/sched_main.c | 23 +++--
> > drivers/gpu/drm/v3d/v3d_sched.c| 15 +-
> > include/drm/gpu_scheduler.h| 20 --
> > 10 files changed, 72 insertions(+), 26 deletions(-)
> >
>
> [...]
>
> > diff --git a/drivers/gpu/drm/v3d/v3d_sched.c
> > b/drivers/gpu/drm/v3d/v3d_sched.c
> > index 38e092ea41e6..5e3fe77fa991 100644
> > --- a/drivers/gpu/drm/v3d/v3d_sched.c
> > +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> > @@ -391,7 +391,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _bin_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_bin", v3d->drm.dev);
> > +NULL, "v3d_bin", DRM_SCHED_POLICY_DEFAULT,
> > +v3d->drm.dev);
> > if (ret)
> > return ret;
> > @@ -399,7 +400,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _render_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_render", v3d->drm.dev);
> > +ULL, "v3d_render", DRM_SCHED_POLICY_DEFAULT,
>
> Small nit: s/ULL/NULL
>
Yep, will fix.
Matt
> Best Regards,
> - Maíra
>
> > +v3d->drm.dev);
> > if (ret)
> > goto fail;
> > @@ -407,7 +409,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _tfu_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_tfu", v3d->drm.dev);
> > +NULL, "v3d_tfu", DRM_SCHED_POLICY_DEFAULT,
> > +v3d->drm.dev);
> > if (ret)
> > goto fail;
> > @@ -416,7 +419,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _csd_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_csd", v3d->drm.dev);
> > +NULL, "v3d_csd", DRM_SCHED_POLICY_DEFAULT,
> > +v3d->drm.dev);
> > if (ret)
> > goto fail;
> > @@ -424,7 +428,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _cache_clean_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_cache_clean", v3d->drm.dev);
> > +NULL, "v3d_cache_clean",
> > +DRM_SCHED_POLICY_DEFAULT, v3d->drm.dev);
> > if (ret)
> > goto fail;
> > }
> > diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> > index 278106e358a8..897d52a4ff4f 100644
> > --- a/include/drm/gpu_scheduler.h
> > +++ b/include/drm/gpu_scheduler.h
> > @@ -72,11 +72,15 @@ enum drm_sched_priority {
> > DRM_SCHED_PRIORITY_UNSET = -2
> > };
> > -/* Used to chose between FIFO and RR jobs scheduling */
> > -extern int drm_sched_policy;
> > -
> > -#define DRM_SCHED_POLICY_RR0
> > -#define DRM_SCHED_POLICY_FIFO 1
> > +/* Used to chose default scheduling policy*/
> > +extern int default_drm_sched_policy;
> > +
> > +enum
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/gt/pxp: User PXP contexts requires runalone bit in lrc
Hi Alan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 51fec314404b6a360493f225481083b2a664e3e1]
url:
https://github.com/intel-lab-lkp/linux/commits/Alan-Previn/drm-i915-pxp-mtl-Update-pxp-firmware-response-timeout/20230811-124534
base: 51fec314404b6a360493f225481083b2a664e3e1
patch link:
https://lore.kernel.org/r/20230811044310.944883-4-alan.previn.teres.alexis%40intel.com
patch subject: [Intel-gfx] [PATCH v2 3/3] drm/i915/gt/pxp: User PXP contexts
requires runalone bit in lrc
config: i386-randconfig-r082-20230812
(https://download.01.org/0day-ci/archive/20230812/202308121007.ju4nvvxe-...@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce:
(https://download.01.org/0day-ci/archive/20230812/202308121007.ju4nvvxe-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202308121007.ju4nvvxe-...@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/i915/gt/intel_lrc.c:851:34: sparse: sparse: dereference of
>> noderef expression
vim +851 drivers/gpu/drm/i915/gt/intel_lrc.c
847
848 static bool ctx_needs_runalone(const struct intel_context *ce)
849 {
850 if (GRAPHICS_VER_FULL(ce->engine->i915) >= IP_VER(12, 70) &&
> 851 ce->gem_context && ce->gem_context->uses_protected_content
> &&
852 (ce->engine->class == COMPUTE_CLASS || ce->engine->class ==
RENDER_CLASS))
853 return true;
854
855 return false;
856 }
857
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH -next 3/7] drm/msm: Remove unnecessary NULL values
On 8/8/2023 8:44 PM, Ruan Jinjie wrote: The NULL initialization of the pointers assigned by kzalloc() first is not necessary, because if the kzalloc() failed, the pointers will be assigned NULL, otherwise it works as usual. so remove it. Signed-off-by: Ruan Jinjie --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH] PCI/VGA: Make the vga_is_firmware_default() less arch-independent
On Tue, Aug 08, 2023 at 10:58:59PM +0800, Sui Jingfeng wrote:
> Currently, the vga_is_firmware_default() function works only on x86 and
> IA64 architectures, but it is a no-op on ARM64, PPC, RISC-V, etc. This
> patch completes the implementation by tracking the firmware framebuffer's
> address range. The added code is trying to identify the VRAM aperture that
> contains the firmware framebuffer. Once found, related information about
> the VRAM aperture will be tracked.
>
> Note that we need to identify the VRAM aperture before it get moved. We
> achieve this by using DECLARE_PCI_FIXUP_CLASS_HEADER(), which ensures that
> vga_arb_firmware_fb_addr_tracker() gets called before PCI resource
> allocation. Once we found the VRAM aperture that contains firmware fb, we
> are able to monitor the address changes of it. If the VRAM aperture of the
> primary GPU do moved, we will update our cached firmware framebuffer's
> address range accordingly. This approach overcomes the VRAM bar relocation
> issue successfully. Hence, this patch make the vga_is_firmware_default()
> function works on whatever arch that has UEFI GOP support, including x86
> and IA64. But, at the first step, we make it available only on platforms
> which PCI resource relocation do happens. Once provided method proved to
> be effective and reliable, it can be expanded to other arch easily.
I think this patch tries to solve two problems, and it should be split
into two patches:
1) Identify firmware framebuffer on arches other than x86 and ia64
2) Deal with VGA devices where the PCI core has moved the BAR
containing the framebuffer
For x86 and ia64, vga_is_firmware_default() currently gets the
framebuffer base and size from screen_info. Whenever
vga_arbiter_add_pci_device() adds a VGA device, we check to see if it
has a BAR containing the framebuffer.
It looks like this patch retains that for x86 and ia64, but only if
CONFIG_EFI=y. I think CONFIG_EFI is optional for both x86 and ia64,
so it looks like this will break systems where CONFIG_X86=y and
CONFIG_EFI is not set.
> This patch is tested on
> 1) LS3A5000+LS7A2000 and LS3A5000+LS7A1000 platform.
> 2) Intel i3-8100 CPU + H110 D4L motherboard with triple video cards:
>
> $ lspci | grep VGA
>
> Intel Corporation CoffeeLake-S GT2 [UHD Graphics 630]
> Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Radeon RX 470] (rev cf)
> ASPEED Technology, Inc. ASPEED Graphics Family (rev 52)
>
> Note that on x86, in order to testing the new approach this patch provided,
> we remove the vga_arb_get_fb_range_from_screen_info() call in
> vga_is_firmware_default() function, as following.
>
> -#if defined(CONFIG_X86) || defined(CONFIG_IA64)
> - ret = vga_arb_get_fb_range_from_screen_info(_start, _end);
> -#else
> ret = vga_arb_get_fb_range_from_tracker(_start, _end);
> -#endif
>
> It is just that we don't observe the case which VRAM Bar of VGA compatible
> controller moves, so there just no need to unify it. But on LoongArch,
> the VRAM Bar of AMDGPU do moves.
>
> v2:
> * Fix test robot warnnings and fix typos
>
> v3:
> * Fix linkage problems if the global screen_info is not exported
This doesn't build on x86:
$ git checkout -b wip/sui-vga-default-arch-independent v6.5-rc1
$ b4 am 20230808145859.1590625-1-suijingf...@loongson.cn
$ git am
./20230808_suijingfeng_pci_vga_make_the_vga_is_firmware_default_less_arch_independent.mbx
$ make drivers/pci/vgaarb.o
CC drivers/pci/vgaarb.o
drivers/pci/vgaarb.c:114:13: error: ‘vga_arb_get_fb_range_from_tracker’
defined but not used [-Werror=unused-function]
114 | static bool vga_arb_get_fb_range_from_tracker(resource_size_t *start,
| ^
> Signed-off-by: Sui Jingfeng
> ---
> drivers/pci/vgaarb.c | 154 ++-
> 1 file changed, 139 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index 5a696078b382..e0919a70af3e 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -61,6 +61,92 @@ static bool vga_arbiter_used;
> static DEFINE_SPINLOCK(vga_lock);
> static DECLARE_WAIT_QUEUE_HEAD(vga_wait_queue);
>
> +static struct firmware_fb_tracker {
> + /* The PCI(e) device who owns the firmware framebuffer */
> + struct pci_dev *pdev;
> + /* The index of the VRAM Bar */
> + unsigned int bar;
> + /* Firmware fb's offset from the VRAM aperture start */
> + resource_size_t offset;
> + /* The firmware fb's size, in bytes */
> + resource_size_t size;
> +
> + /* Firmware fb's address range, suffer from change */
> + resource_size_t start;
> + resource_size_t end;
It's redundant to save start, size, and end. Start and end should be
enough, and maybe you could use a struct resource for that.
It's not clear to me why you need to save the start/end/etc anyway.
All we need to know is which pci_dev is the firmware device. Doesn't
Re: [PATCH v2 2/9] drm/sched: Move schedule policy to scheduler / entity
Hi Matthew,
I'm not sure in which tree you plan to apply this series, but if you
plan to apply it on drm-misc-next, it would be nice to rebase on top of
it. It would make it easier for driver maintainers to review it.
Apart from the small nit below it, I tested the Xe tree on v3d and
things seems to be running smoothly.
On 8/10/23 23:31, Matthew Brost wrote:
Rather than a global modparam for scheduling policy, move the scheduling
policy to scheduler / entity so user can control each scheduler / entity
policy.
v2:
- s/DRM_SCHED_POLICY_MAX/DRM_SCHED_POLICY_COUNT (Luben)
- Only include policy in scheduler (Luben)
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
drivers/gpu/drm/etnaviv/etnaviv_sched.c| 3 ++-
drivers/gpu/drm/lima/lima_sched.c | 3 ++-
drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_sched.c| 3 ++-
drivers/gpu/drm/panfrost/panfrost_job.c| 3 ++-
drivers/gpu/drm/scheduler/sched_entity.c | 24 ++
drivers/gpu/drm/scheduler/sched_main.c | 23 +++--
drivers/gpu/drm/v3d/v3d_sched.c| 15 +-
include/drm/gpu_scheduler.h| 20 --
10 files changed, 72 insertions(+), 26 deletions(-)
[...]
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 38e092ea41e6..5e3fe77fa991 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -391,7 +391,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_bin_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_bin", v3d->drm.dev);
+NULL, "v3d_bin", DRM_SCHED_POLICY_DEFAULT,
+v3d->drm.dev);
if (ret)
return ret;
@@ -399,7 +400,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_render_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_render", v3d->drm.dev);
+ULL, "v3d_render", DRM_SCHED_POLICY_DEFAULT,
Small nit: s/ULL/NULL
Best Regards,
- Maíra
+v3d->drm.dev);
if (ret)
goto fail;
@@ -407,7 +409,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_tfu_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_tfu", v3d->drm.dev);
+NULL, "v3d_tfu", DRM_SCHED_POLICY_DEFAULT,
+v3d->drm.dev);
if (ret)
goto fail;
@@ -416,7 +419,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_csd_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_csd", v3d->drm.dev);
+NULL, "v3d_csd", DRM_SCHED_POLICY_DEFAULT,
+v3d->drm.dev);
if (ret)
goto fail;
@@ -424,7 +428,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_cache_clean_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_cache_clean", v3d->drm.dev);
+NULL, "v3d_cache_clean",
+DRM_SCHED_POLICY_DEFAULT, v3d->drm.dev);
if (ret)
goto fail;
}
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 278106e358a8..897d52a4ff4f 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -72,11 +72,15 @@ enum drm_sched_priority {
DRM_SCHED_PRIORITY_UNSET = -2
};
-/* Used to chose between FIFO and RR jobs scheduling */
-extern int drm_sched_policy;
-
-#define DRM_SCHED_POLICY_RR0
-#define DRM_SCHED_POLICY_FIFO 1
+/* Used to chose default scheduling policy*/
+extern int default_drm_sched_policy;
+
+enum drm_sched_policy {
+ DRM_SCHED_POLICY_DEFAULT,
+ DRM_SCHED_POLICY_RR,
+ DRM_SCHED_POLICY_FIFO,
+ DRM_SCHED_POLICY_COUNT,
+};
/**
* struct drm_sched_entity - A wrapper around a job queue (typically
@@ -489,6 +493,7 @@ struct drm_sched_backend_ops {
* guilty and it will no longer be considered for scheduling.
* @score: score to help loadbalancer pick a idle sched
* @_score:
[pull] amdgpu, amdkfd, radeon, drm_buddy drm-next-6.6
Hi Dave, Daniel, New stuff for 6.6. The following changes since commit d9aa1da9a8cfb0387eb5703c15bd1f54421460ac: Merge tag 'drm-intel-gt-next-2023-08-04' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2023-08-07 13:49:25 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-next-6.6-2023-08-11 for you to fetch changes up to 6be2ad4f0073c541146caa66c5ae936c955a8224: drm/amdgpu: don't allow userspace to create a doorbell BO (2023-08-11 14:48:07 -0400) amdgpu: - SDMA 6.1.0 support - SMU 13.x fixes - PSP 13.x fixes - HDP 6.1 support - SMUIO 14.0 support - IH 6.1 support - Coding style cleanups - Misc display fixes - Initial Freesync panel replay support - RAS fixes - SDMA 5.2 MGCG updates - SR-IOV fixes - DCN3+ gamma fix - Revert zpos properly until IGT regression is fixed - NBIO 7.9 fixes - Use TTM to manage the doorbell BAR - Async flip fix - DPIA tracing support - DCN 3.x TMDS HDMI fixes - FRU fixes amdkfd: - Coding style cleanups - SVM fixes - Trap handler fixes - Convert older APUs to use dGPU path like newer APUs - Drop IOMMUv2 path as it is no longer used radeon: - Coding style cleanups drm buddy: - Fix debugging output UAPI: - A new memory pool was added to amdgpu_drm.h since we converted doorbell BAR management to use TTM, but userspace is blocked from allocating from it at this point, so kind of not really anything new here per se Alex Deucher (10): drm/amdgpu: add UAPI for allocating doorbell memory drm/amdgpu: accommodate DOMAIN/PL_DOORBELL drm/amdgpu: fix possible UAF in amdgpu_cs_pass1() drm/amdgpu/gfx11: only enable CP GFX shadowing on SR-IOV drm/amdkfd: ignore crat by default drm/amdkfd: disable IOMMUv2 support for KV/CZ drm/amdkfd: disable IOMMUv2 support for Raven drm/amdkfd: drop IOMMUv2 support drm/amdkfd: rename device_queue_manager_init_v10_navi10() drm/amdgpu: don't allow userspace to create a doorbell BO Alex Sierra (1): drm/amdkfd: avoid unmap dma address when svm_ranges are split Alvin Lee (2): drm/amd/display: Implement interface for notify cursor support change drm/amd/display: Use max memclk variable when setting max memclk Aric Cyr (1): drm/amd/display: Promote DC to 2.3.245 Asad Kamal (2): drm/amdgpu: Add pcie usage callback to nbio drm/amdgpu: Add pci usage to nbio v7.9 Austin Zheng (2): drm/amd/display: Set Stream Update Flags in commit_state_no_check drm/amd/display: Use DTBCLK as refclk instead of DPREFCLK Ben Li (2): drm/amdgpu: add ih 6.1 registers drm/amdgpu: add ih 6.1 support Bhawanpreet Lakha (6): drm/amd/display: Add structs for Freesync Panel Replay drm/amd/display: Add Functions to enable Freesync Panel Replay drm/amd/display: Add Freesync Panel DM code drm/amd/display: Read replay data from sink drm/amd/display: Change HDCP update sequence for DM drm/amd/display: Get replay info from VSDB Candice Li (1): drm/amdgpu: Extend poison mode check to SDMA/VCN/JPEG Denis Arefev (1): drm/amd/display: Return value of function Emily Deng (1): drm/amdgpu/irq: Move irq resume to the beginning Evan Quan (1): drm/amd/pm: disable the SMU13 OD feature support temporarily Fangzhi Zuo (1): drm/amd/display: Temporary Disable MST DP Colorspace Property Hamza Mahfooz (1): drm/amd/display: ensure async flips are only accepted for fast updates Hawking Zhang (4): drm/amdgpu: Only create err_count sysfs when hw_op is supported drm/amdgpu: Apply poison mode check to GFX IP only drm/amdgpu: Remove gfx v11_0_3 ras_late_init call drm/amdgpu: Issue ras enable_feature for gfx ip only Jay Cornwall (3): drm/amdkfd: Sync trap handler binaries with source drm/amdkfd: Relocate TBA/TMA to opposite side of VM hole drm/amdkfd: Sign-extend TMA address in trap handler Joshua Aberback (1): drm/amd/display: Clean up flip pending timeout handling Kenneth Feng (1): drm/amd/pm: correct the pcie width for smu 13.0.0 Leo Chen (3): drm/amd/display: Revert "drm/amd/display: Read down-spread percentage from lut to adjust dprefclk." drm/amd/display: Blocking invalid 420 modes on HDMI TMDS for DCN31 drm/amd/display: Blocking invalid 420 modes on HDMI TMDS for DCN314 Lijo Lazar (7): drm/amdgpu: Match against exact bootloader status drm/amdgpu: Add pcie replay count callback to nbio drm/amdgpu: Add pci replay count to nbio v7.9 drm/amdgpu: Use nbio callback for nv and soc21 drm/amd/pm: Fix SMU v13.0.6 energy reporting drm/amdgpu: Report vbios version instead of PN drm/amdgpu: Add FRU sysfs nodes only if needed Ma Jun (1): drm/buddy: Fix drm buddy info output format
Re: [PATCH v6] drm/doc: Document DRM device reset expectations
Hi, On 8/11/23 11:55, André Almeida wrote: > Create a section that specifies how to deal with DRM device resets for > kernel and userspace drivers. > > Signed-off-by: André Almeida > > --- > > Changes: > - Due to substantial changes in the content, dropped Pekka's Acked-by > - Grammar fixes (Randy) > - Add paragraph about disabling device resets > - Add note about integrating reset tracking in drm/sched > - Add note that KMD should return failure for contexts affected by >resets and UMD should check for this > - Add note about lack of consensus around what to do about non-robust >apps > > v5: > https://lore.kernel.org/dri-devel/20230627132323.115440-1-andrealm...@igalia.com/ > --- > Documentation/gpu/drm-uapi.rst | 77 ++ > 1 file changed, 77 insertions(+) > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 65fb3036a580..6bfaf7d4c074 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -285,6 +285,83 @@ for GPU1 and GPU2 from different vendors, and a third > handler for > mmapped regular files. Threads cause additional pain with signal > handling as well. > > +Device reset > + > + > +The GPU stack is really complex and is prone to errors, from hardware bugs, > +faulty applications and everything in between the many layers. Some errors > +require resetting the device in order to make the device usable again. This > +section describes the expectations for DRM and usermode drivers when a > +device resets and how to propagate the reset status. > + > +Device resets can not be disabled without tainting the kernel, which can > lead to > +hanging the entire kernel through shrinkers/mmu_notifiers. Userspace role in > +device resets is to propagate the message to the application and apply any > +special policy for blocking guilty applications, if any. Corollary is that > +debugging a hung GPU context require hardware support to be able to preempt > such > +a GPU context while it's stopped. > + > +Kernel Mode Driver > +-- > + > +The KMD is responsible for checking if the device needs a reset, and to > perform > +it as needed. Usually a hang is detected when a job gets stuck executing. KMD > +should keep track of resets, because userspace can query any time about the > +reset status for a specific context. This is needed to propagate to the rest > of > +the stack that a reset has happened. Currently, this is implemented by each > +driver separately, with no common DRM interface. Ideally this should be > properly > +integrated at DRM scheduler to provide a common ground for all drivers. > After a > +reset, KMD should reject new command submissions for affected contexts. > + > +User Mode Driver > + > + > +After command submission, UMD should check if the submission was accepted or > +rejected. After a reset, KMD should reject submissions, and UMD can issue an > +ioclt to the KMD to check the reset status, and this can be checked more > often ioctl > +if the UMD requires it. After detecting a reset, UMD will then proceed to > report > +it to the application using the appropriate API error code, as explained in > the > +section below about robustness. > + > +Robustness > +-- > + > +The only way to try to keep an application working after a reset is if it > +complies with the robustness aspects of the graphical API that it is using. > + > +Graphical APIs provide ways to applications to deal with device resets. > However, > +there is no guarantee that the app will use such features correctly, and a > +userspace that doesn't support robust interfaces (like an non-robust like a > +OpenGL context or API without any robustness support like libva) leave the > +robustness handling entirely to the userspace driver. There is no strong > +community consensus on what the userspace driver should do in that case, > +since all reasonable approaches have some clear downsides. > + > +OpenGL > +~~ > + > +Apps using OpenGL should use the available robust interfaces, like the > +extension ``GL_ARB_robustness`` (or ``GL_EXT_robustness`` for OpenGL ES). > This > +interface tells if a reset has happened, and if so, all the context state is > +considered lost and the app proceeds by creating new ones. There's no > consensus > +on what to do to if robustness is not in use. > + > +Vulkan > +~~ > + > +Apps using Vulkan should check for ``VK_ERROR_DEVICE_LOST`` for submissions. > +This error code means, among other things, that a device reset has happened > and > +it needs to recreate the contexts to keep going. > + > +Reporting causes of resets > +-- > + > +Apart from propagating the reset through the stack so apps can recover, it's > +really useful for driver developers to learn more about what caused the > reset in > +the first place. DRM devices should make
Re: [PATCH v2 13/15] drm/panthor: Allow driver compilation
On 2023-08-11 17:56, Daniel Stone wrote: Hi, On 11/08/2023 17:35, Robin Murphy wrote: On 2023-08-09 17:53, Boris Brezillon wrote: +obj-$(CONFIG_DRM_PANTHOR) += panthor.o FWIW I still think it would be nice to have a minor directory/Kconfig/Makefile reshuffle and a trivial bit of extra registration glue to build both drivers into a single module. It seems like it could be a perpetual source of confusion to end users where Mesa "panfrost" is the right option but kernel "panfrost" is the wrong one. Especially when pretty much every other GPU driver is also just one big top-level module to load for many different generations of hardware. Plus it would mean that if someone did want to have a go at deduplicating the resource-wrangling boilerplate for OPPs etc. in future, there's more chance of being able to do so meaningfully. It might be nice to point it out, but to be fair Intel and AMD both have two (or more) drivers, as does Broadcom/RPi. As does, err ... Mali. Indeed, I didn't mean to imply that I'm not aware that e.g. gma500 is to i915 what lima is to panfrost. It was more that unlike the others where there's a pretty clear line in the sand between "driver for old hardware" and "driver for the majority of recent hardware", this one happens to fall splat in the middle of the current major generation such that panfrost is the correct module for Mali Bifrost but also the wrong one for Mali Bifrost... :/ I can see the point, but otoh if someone's managed to build all the right regulator/clock/etc modules to get a working system, they'll probably manage to figure teh GPU side out? Maybe; either way I guess it's not really my concern, since I'm the only user that *I* have to support, and I do already understand it. From the upstream perspective I mostly just want to hold on to the hope of not having to write my io-pgtable bugs twice over if at all possible :) Cheers, Robin.
Re: [PATCH v3] drm/msm/a2xx: Call adreno_gpu_init() earlier
It is in https://gitlab.freedesktop.org/drm/msm/-/merge_requests/75 so
should be in msm-next for v6.6
BR,
-R
On Fri, Aug 11, 2023 at 9:11 AM Fabio Estevam wrote:
>
> Hi Rob,
>
> Any comments, please?
>
> On Mon, Jul 24, 2023 at 5:28 PM Fabio Estevam wrote:
> >
> > Hi Rob,
> >
> > A gentle ping.
> >
> > On Thu, Jun 22, 2023 at 3:37 PM Dmitry Baryshkov
> > wrote:
> > >
> > > On 21/06/2023 02:23, Fabio Estevam wrote:
> > > > From: Fabio Estevam
> > > >
> > > > The adreno_is_a20x() and adreno_is_a225() functions rely on the
> > > > GPU revision, but such information is retrieved inside
> > > > adreno_gpu_init(),
> > > > which is called afterwards.
> > > >
> > > > Fix this problem by caling adreno_gpu_init() earlier, so that
> > > > the GPU information revision is available when adreno_is_a20x()
> > > > and adreno_is_a225() run.
> > > >
> > > > Tested on a imx53-qsb board.
> > > >
> > > > Fixes: 21af872cd8c6 ("drm/msm/adreno: add a2xx")
> > > > Signed-off-by: Fabio Estevam
> > > > ---
> > > > Hi,
> > > >
> > > > This error has been exposed by a recent commit in linux-next:
> > > >
> > > > cc943f43ece7 ("drm/msm/adreno: warn if chip revn is verified before
> > > > being set")
> > > >
> > > > , but the error has been present since the very first a2xx commit.
> > > >
> > > > Changes since v2:
> > > > - Call adreno_gpu_init() earlier.
> > >
> > > Reviewed-by: Dmitry Baryshkov
> > >
> > >
> > > >
> > > > drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 8
> > > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
> > >
[PATCH v6] drm/doc: Document DRM device reset expectations
Create a section that specifies how to deal with DRM device resets for kernel and userspace drivers. Signed-off-by: André Almeida --- Changes: - Due to substantial changes in the content, dropped Pekka's Acked-by - Grammar fixes (Randy) - Add paragraph about disabling device resets - Add note about integrating reset tracking in drm/sched - Add note that KMD should return failure for contexts affected by resets and UMD should check for this - Add note about lack of consensus around what to do about non-robust apps v5: https://lore.kernel.org/dri-devel/20230627132323.115440-1-andrealm...@igalia.com/ --- Documentation/gpu/drm-uapi.rst | 77 ++ 1 file changed, 77 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 65fb3036a580..6bfaf7d4c074 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -285,6 +285,83 @@ for GPU1 and GPU2 from different vendors, and a third handler for mmapped regular files. Threads cause additional pain with signal handling as well. +Device reset + + +The GPU stack is really complex and is prone to errors, from hardware bugs, +faulty applications and everything in between the many layers. Some errors +require resetting the device in order to make the device usable again. This +section describes the expectations for DRM and usermode drivers when a +device resets and how to propagate the reset status. + +Device resets can not be disabled without tainting the kernel, which can lead to +hanging the entire kernel through shrinkers/mmu_notifiers. Userspace role in +device resets is to propagate the message to the application and apply any +special policy for blocking guilty applications, if any. Corollary is that +debugging a hung GPU context require hardware support to be able to preempt such +a GPU context while it's stopped. + +Kernel Mode Driver +-- + +The KMD is responsible for checking if the device needs a reset, and to perform +it as needed. Usually a hang is detected when a job gets stuck executing. KMD +should keep track of resets, because userspace can query any time about the +reset status for a specific context. This is needed to propagate to the rest of +the stack that a reset has happened. Currently, this is implemented by each +driver separately, with no common DRM interface. Ideally this should be properly +integrated at DRM scheduler to provide a common ground for all drivers. After a +reset, KMD should reject new command submissions for affected contexts. + +User Mode Driver + + +After command submission, UMD should check if the submission was accepted or +rejected. After a reset, KMD should reject submissions, and UMD can issue an +ioclt to the KMD to check the reset status, and this can be checked more often +if the UMD requires it. After detecting a reset, UMD will then proceed to report +it to the application using the appropriate API error code, as explained in the +section below about robustness. + +Robustness +-- + +The only way to try to keep an application working after a reset is if it +complies with the robustness aspects of the graphical API that it is using. + +Graphical APIs provide ways to applications to deal with device resets. However, +there is no guarantee that the app will use such features correctly, and a +userspace that doesn't support robust interfaces (like an non-robust +OpenGL context or API without any robustness support like libva) leave the +robustness handling entirely to the userspace driver. There is no strong +community consensus on what the userspace driver should do in that case, +since all reasonable approaches have some clear downsides. + +OpenGL +~~ + +Apps using OpenGL should use the available robust interfaces, like the +extension ``GL_ARB_robustness`` (or ``GL_EXT_robustness`` for OpenGL ES). This +interface tells if a reset has happened, and if so, all the context state is +considered lost and the app proceeds by creating new ones. There's no consensus +on what to do to if robustness is not in use. + +Vulkan +~~ + +Apps using Vulkan should check for ``VK_ERROR_DEVICE_LOST`` for submissions. +This error code means, among other things, that a device reset has happened and +it needs to recreate the contexts to keep going. + +Reporting causes of resets +-- + +Apart from propagating the reset through the stack so apps can recover, it's +really useful for driver developers to learn more about what caused the reset in +the first place. DRM devices should make use of devcoredump to store relevant +information about the reset, so this information can be added to user bug +reports. + .. _drm_driver_ioctl: IOCTL Support on Device Nodes -- 2.41.0
[PATCH v5] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
This attempts to avoid circular locking dependency between flush delayed
work and intel_gt_reset.
When intel_gt_reset was called, task will hold a lock.
To cacel delayed work here, the _sync version will also acquire a lock,
which might trigger the possible cirular locking dependency warning.
When intel_gt_reset called, reset_in_progress flag will be set, add code
to check the flag, call async verion if reset is in progress.
Signed-off-by: Zhanjun Dong
Cc: John Harrison
Cc: Andi Shyti
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
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 a0e3ef1c65d2..600388c849f7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1359,7 +1359,16 @@ static void guc_enable_busyness_worker(struct intel_guc
*guc)
static void guc_cancel_busyness_worker(struct intel_guc *guc)
{
- cancel_delayed_work_sync(>timestamp.work);
+ /*
+* When intel_gt_reset was called, task will hold a lock.
+* To cacel delayed work here, the _sync version will also acquire a
lock, which might
+* trigger the possible cirular locking dependency warning.
+* Check the reset_in_progress flag, call async verion if reset is in
progress.
+*/
+ if (guc_to_gt(guc)->uc.reset_in_progress)
+ cancel_delayed_work(>timestamp.work);
+ else
+ cancel_delayed_work_sync(>timestamp.work);
}
static void __reset_guc_busyness_stats(struct intel_guc *guc)
--
2.34.1
[PATCH v2 6/6] drm/tests: Add KUnit tests for drm_fb_memcpy()
Insert parameterized test for the drm_fb_memcpy() to ensure correctness
and prevent future regressions. The test case can accept different
formats.
Signed-off-by: Arthur Grillo
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 391 +
1 file changed, 391 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 08071b6c00f8..09214ae65091 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -1188,6 +1188,396 @@ static void drm_test_fb_build_fourcc_list(struct kunit
*test)
KUNIT_EXPECT_MEMEQ(test, fourccs_out, params->expected, TEST_BUF_SIZE);
}
+struct fb_memcpy_result {
+ unsigned int dst_pitches[DRM_FORMAT_MAX_PLANES];
+ const u32 expected[DRM_FORMAT_MAX_PLANES][TEST_BUF_SIZE];
+};
+
+struct multi_plane_op_case {
+ const char *name;
+ u32 format;
+ struct drm_rect clip;
+ unsigned int src_pitches[DRM_FORMAT_MAX_PLANES];
+ const u32 src[DRM_FORMAT_MAX_PLANES][TEST_BUF_SIZE];
+ struct fb_memcpy_result memcpy_result;
+};
+
+/* The `src` and `expected` buffers are u32 arrays. To deal with planes that
+ * have a cpp != 4 the values are stored together on the same u32 number in a
+ * way so the order in memory is correct in a little-endian machine.
+ *
+ * Because of that, on some occasions, parts of a u32 will not be part of the
+ * test, to make this explicit the 0xFF byte is used on those parts.
+ */
+
+static struct multi_plane_op_case multi_plane_op_cases[] = {
+ {
+ .name = "single_pixel_source_buffer",
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(0, 0, 1, 1),
+ .src_pitches = { 1 * 4 },
+ .src = {{ 0x01020304 }},
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {{ 0x01020304 }},
+ }
+ },
+ {
+ .name = "single_pixel_source_buffer",
+ .format = DRM_FORMAT_XRGB_A8,
+ .clip = DRM_RECT_INIT(0, 0, 1, 1),
+ .src_pitches = { 1 * 4, 1 },
+ .src = {
+ { 0x01020304 },
+ { 0xFF01 },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x01020304 },
+ { 0x0001 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_source_buffer",
+ .format = DRM_FORMAT_YUV444,
+ .clip = DRM_RECT_INIT(0, 0, 1, 1),
+ .src_pitches = { 1, 1, 1 },
+ .src = {
+ { 0xFF01 },
+ { 0xFF01 },
+ { 0xFF01 },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x0001 },
+ { 0x0001 },
+ { 0x0001 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_clip_rectangle",
+ .format = DRM_FORMAT_XBGR,
+ .clip = DRM_RECT_INIT(1, 1, 1, 1),
+ .src_pitches = { 2 * 4 },
+ .src = {
+ {
+ 0x, 0x,
+ 0x, 0x01020304,
+ },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x01020304 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_clip_rectangle",
+ .format = DRM_FORMAT_XRGB_A8,
+ .clip = DRM_RECT_INIT(1, 1, 1, 1),
+ .src_pitches = { 2 * 4, 2 * 1 },
+ .src = {
+ {
+ 0x, 0x,
+ 0x, 0x01020304,
+ },
+ { 0x0100 },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x01020304 },
+ { 0x0001 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_clip_rectangle",
+ .format = DRM_FORMAT_YUV444,
+ .clip = DRM_RECT_INIT(1, 1, 1, 1),
+ .src_pitches = { 2 * 1, 2 * 1,
[PATCH v2 5/6] drm/tests: Add multi-plane support to conversion_buf_size()
The drm_fb_memcpy() supports multi-plane formats. To fully test it in
the future, add multi-plane support to the conversion_buf_size() helper.
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 28 +-
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 2b55d9f025f9..08071b6c00f8 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -472,7 +472,7 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
* The size of the destination buffer or negative value on error.
*/
static size_t conversion_buf_size(u32 dst_format, unsigned int dst_pitch,
- const struct drm_rect *clip)
+ const struct drm_rect *clip, int plane)
{
const struct drm_format_info *dst_fi = drm_format_info(dst_format);
@@ -480,7 +480,7 @@ static size_t conversion_buf_size(u32 dst_format, unsigned
int dst_pitch,
return -EINVAL;
if (!dst_pitch)
- dst_pitch = drm_format_info_min_pitch(dst_fi, 0,
drm_rect_width(clip));
+ dst_pitch = drm_format_info_min_pitch(dst_fi, plane,
drm_rect_width(clip));
return dst_pitch * drm_rect_height(clip);
}
@@ -554,7 +554,7 @@ static void drm_test_fb_xrgb_to_gray8(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_R8, result->dst_pitch,
- >clip);
+ >clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -588,7 +588,7 @@ static void drm_test_fb_xrgb_to_rgb332(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_RGB332, result->dst_pitch,
- >clip);
+ >clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -621,7 +621,7 @@ static void drm_test_fb_xrgb_to_rgb565(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_RGB565, result->dst_pitch,
- >clip);
+ >clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -660,7 +660,7 @@ static void drm_test_fb_xrgb_to_xrgb1555(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_XRGB1555, result->dst_pitch,
- >clip);
+ >clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -694,7 +694,7 @@ static void drm_test_fb_xrgb_to_argb1555(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_ARGB1555, result->dst_pitch,
- >clip);
+ >clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -728,7 +728,7 @@ static void drm_test_fb_xrgb_to_rgba5551(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_RGBA5551, result->dst_pitch,
- >clip);
+ >clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -762,7 +762,7 @@ static void drm_test_fb_xrgb_to_rgb888(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_RGB888, result->dst_pitch,
- >clip);
+ >clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -799,7 +799,7 @@ static void drm_test_fb_xrgb_to_argb(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_ARGB,
- result->dst_pitch, >clip);
+ result->dst_pitch, >clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -833,7 +833,7 @@ static void drm_test_fb_xrgb_to_xrgb2101010(struct
kunit *test)
};
dst_size = conversion_buf_size(DRM_FORMAT_XRGB2101010,
- result->dst_pitch, >clip);
+ result->dst_pitch, >clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -867,7 +867,7 @@ static void drm_test_fb_xrgb_to_argb2101010(struct
kunit *test)
};
dst_size =
[PATCH v2 2/6] drm/tests: Add KUnit tests for drm_fb_swab()
Insert parameterized test for the drm_fb_swab() to ensure correctness
and prevent future regressions.
Each expected color has it bytes reversed in order, so xrgb would be
bgrx.
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 66 ++
1 file changed, 66 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 938d4fdb4291..d10e18fcb994 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -74,6 +74,11 @@ struct convert_to_mono_result {
const u8 expected[TEST_BUF_SIZE];
};
+struct fb_swab_result {
+ unsigned int dst_pitch;
+ const u32 expected[TEST_BUF_SIZE];
+};
+
struct convert_xrgb_case {
const char *name;
unsigned int pitch;
@@ -90,6 +95,7 @@ struct convert_xrgb_case {
struct convert_to_xrgb2101010_result xrgb2101010_result;
struct convert_to_argb2101010_result argb2101010_result;
struct convert_to_mono_result mono_result;
+ struct fb_swab_result swab_result;
};
static struct convert_xrgb_case convert_xrgb_cases[] = {
@@ -143,6 +149,10 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0b0 },
},
+ .swab_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0xFF01 },
+ },
},
{
.name = "single_pixel_clip_rectangle",
@@ -197,6 +207,10 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0b0 },
},
+ .swab_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0xFF10 },
+ },
},
{
/* Well known colors: White, black, red, green, blue, magenta,
@@ -318,6 +332,15 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0b11,
},
},
+ .swab_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = {
+ 0xFF11, 0x0022,
+ 0xFF33, 0x00FF0044,
+ 0xFF55, 0xFF00FF66,
+ 0x0077, 0x0088,
+ },
+ },
},
{
/* Randomly picked colors. Full buffer within the clip area. */
@@ -425,6 +448,14 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0b010, 0b000,
},
},
+ .swab_result = {
+ .dst_pitch = 20,
+ .expected = {
+ 0x9C440EA1, 0x054D11B1, 0x03F3A8C1, 0x,
0x,
+ 0x73F06CD1, 0x9C440EA2, 0x054D11B2, 0x,
0x,
+ 0x0303A8C2, 0x73F06CD2, 0x9C440EA3, 0x,
0x,
+ },
+ },
},
};
@@ -886,6 +917,40 @@ static void drm_test_fb_xrgb_to_mono(struct kunit
*test)
KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
}
+static void drm_test_fb_swab(struct kunit *test)
+{
+ const struct convert_xrgb_case *params = test->param_value;
+ const struct fb_swab_result *result = >swab_result;
+ size_t dst_size;
+ u32 *buf = NULL;
+ __le32 *xrgb = NULL;
+ struct iosys_map dst, src;
+
+ struct drm_framebuffer fb = {
+ .format = drm_format_info(DRM_FORMAT_XRGB),
+ .pitches = { params->pitch, 0, 0 },
+ };
+
+ dst_size = conversion_buf_size(DRM_FORMAT_XRGB, result->dst_pitch,
>clip);
+
+ KUNIT_ASSERT_GT(test, dst_size, 0);
+
+ buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+ iosys_map_set_vaddr(, buf);
+
+ xrgb = cpubuf_to_le32(test, params->xrgb, TEST_BUF_SIZE);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb);
+ iosys_map_set_vaddr(, xrgb);
+
+ const unsigned int *dst_pitch = (result->dst_pitch ==
TEST_USE_DEFAULT_PITCH) ?
+ NULL : >dst_pitch;
+
+ drm_fb_swab(, dst_pitch, , , >clip, false);
+ buf = le32buf_to_cpu(test, (__force const __le32 *)buf, dst_size /
sizeof(u32));
+ KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
+}
+
static struct kunit_case drm_format_helper_test_cases[] = {
[PATCH v2 3/6] drm/tests: Add KUnit tests for drm_fb_clip_offset()
Insert parameterized test for the drm_fb_clip_offset() to ensure
correctness and prevent future regressions.
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 91 ++
1 file changed, 91 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index d10e18fcb994..7f24da0b1e00 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -951,6 +951,96 @@ static void drm_test_fb_swab(struct kunit *test)
KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
}
+struct clip_offset_case {
+ const char *name;
+ unsigned int pitch;
+ u32 format;
+ struct drm_rect clip;
+ unsigned int expected_offset;
+};
+
+static struct clip_offset_case clip_offset_cases[] = {
+ {
+ .name = "pass through",
+ .pitch = TEST_USE_DEFAULT_PITCH,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(0, 0, 3, 3),
+ .expected_offset = 0
+ },
+ {
+ .name = "horizontal offset",
+ .pitch = TEST_USE_DEFAULT_PITCH,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(1, 0, 3, 3),
+ .expected_offset = 4,
+ },
+ {
+ .name = "vertical offset",
+ .pitch = TEST_USE_DEFAULT_PITCH,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(0, 1, 3, 3),
+ .expected_offset = 12,
+ },
+ {
+ .name = "horizontal and vertical offset",
+ .pitch = TEST_USE_DEFAULT_PITCH,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(1, 1, 3, 3),
+ .expected_offset = 16,
+ },
+ {
+ .name = "horizontal offset (custom pitch)",
+ .pitch = 20,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(1, 0, 3, 3),
+ .expected_offset = 4,
+ },
+ {
+ .name = "vertical offset (custom pitch)",
+ .pitch = 20,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(0, 1, 3, 3),
+ .expected_offset = 20,
+ },
+ {
+ .name = "horizontal and vertical offset (custom pitch)",
+ .pitch = 20,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(1, 1, 3, 3),
+ .expected_offset = 24,
+ },
+};
+
+static void clip_offset_case_desc(struct clip_offset_case *t, char *desc)
+{
+ strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(clip_offset, clip_offset_cases, clip_offset_case_desc);
+
+static void drm_test_fb_clip_offset(struct kunit *test)
+{
+ const struct clip_offset_case *params = test->param_value;
+ const struct drm_format_info *format_info =
drm_format_info(params->format);
+
+ unsigned int offset;
+ unsigned int pitch = params->pitch;
+
+ if (pitch == TEST_USE_DEFAULT_PITCH)
+ pitch = drm_format_info_min_pitch(format_info, 0,
+
drm_rect_width(>clip));
+
+ /*
+* Assure that the pitch is not zero, because this will inevitable
cause the
+* wrong expected result
+*/
+ KUNIT_ASSERT_NE(test, pitch, 0);
+
+ offset = drm_fb_clip_offset(pitch, format_info, >clip);
+
+ KUNIT_EXPECT_EQ(test, offset, params->expected_offset);
+}
+
static struct kunit_case drm_format_helper_test_cases[] = {
KUNIT_CASE_PARAM(drm_test_fb_xrgb_to_gray8,
convert_xrgb_gen_params),
KUNIT_CASE_PARAM(drm_test_fb_xrgb_to_rgb332,
convert_xrgb_gen_params),
@@ -964,6 +1054,7 @@ static struct kunit_case drm_format_helper_test_cases[] = {
KUNIT_CASE_PARAM(drm_test_fb_xrgb_to_argb2101010,
convert_xrgb_gen_params),
KUNIT_CASE_PARAM(drm_test_fb_xrgb_to_mono,
convert_xrgb_gen_params),
KUNIT_CASE_PARAM(drm_test_fb_swab, convert_xrgb_gen_params),
+ KUNIT_CASE_PARAM(drm_test_fb_clip_offset, clip_offset_gen_params),
{}
};
--
2.41.0
[PATCH v2 4/6] drm/tests: Add KUnit tests for drm_fb_build_fourcc_list()
Insert parameterized test for the drm_fb_build_fourcc_list() to ensure
correctness and prevent future regressions.
Signed-off-by: Arthur Grillo
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 148 +
1 file changed, 148 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 7f24da0b1e00..2b55d9f025f9 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -3,11 +3,13 @@
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -1041,6 +1043,151 @@ static void drm_test_fb_clip_offset(struct kunit *test)
KUNIT_EXPECT_EQ(test, offset, params->expected_offset);
}
+struct fb_build_fourcc_list_case {
+ const char *name;
+ u32 native_fourccs[TEST_BUF_SIZE];
+ u32 expected[TEST_BUF_SIZE];
+ size_t fourccs_size;
+};
+
+static struct fb_build_fourcc_list_case fb_build_fourcc_list_cases[] = {
+ {
+ .name = "no native formats",
+ .native_fourccs = { },
+ .expected = { DRM_FORMAT_XRGB },
+ .fourccs_size = 1,
+ },
+ {
+ .name = "XRGB as native format",
+ .native_fourccs = { DRM_FORMAT_XRGB },
+ .expected = { DRM_FORMAT_XRGB },
+ .fourccs_size = 1,
+ },
+ {
+ .name = "remove duplicates",
+ .native_fourccs = {
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_XRGB,
+ },
+ .expected = {
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB565,
+ },
+ .fourccs_size = 3,
+ },
+ {
+ .name = "convert alpha formats",
+ .native_fourccs = {
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_ABGR1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_BGRA5551,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_ARGB2101010,
+ DRM_FORMAT_ABGR2101010,
+ DRM_FORMAT_RGBA1010102,
+ DRM_FORMAT_BGRA1010102,
+ },
+ .expected = {
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_XBGR1555,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_BGRX5551,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_RGBX1010102,
+ DRM_FORMAT_BGRX1010102,
+ },
+ .fourccs_size = 12,
+ },
+ {
+ .name = "random formats",
+ .native_fourccs = {
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_ABGR16161616F,
+ DRM_FORMAT_C8,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_BGR565_A8,
+ DRM_FORMAT_R10,
+ DRM_FORMAT_XYUV,
+ },
+ .expected = {
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_ABGR16161616F,
+ DRM_FORMAT_C8,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_BGR565_A8,
+ DRM_FORMAT_R10,
+ DRM_FORMAT_XYUV,
+ DRM_FORMAT_XRGB,
+ },
+ .fourccs_size = 10,
+ },
+};
+
+static void fb_build_fourcc_list_case_desc(struct fb_build_fourcc_list_case
*t, char *desc)
+{
+ strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(fb_build_fourcc_list,
[PATCH v2 1/6] drm/tests: Test default pitch fallback
Test the default pitch fallback when NULL is passed as the dst_pitch on
the conversion procedures.
Signed-off-by: Arthur Grillo
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 126 -
1 file changed, 81 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 474bb7a1c4ee..938d4fdb4291 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -16,6 +16,8 @@
#define TEST_BUF_SIZE 50
+#define TEST_USE_DEFAULT_PITCH 0
+
struct convert_to_gray8_result {
unsigned int dst_pitch;
const u8 expected[TEST_BUF_SIZE];
@@ -97,48 +99,48 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.clip = DRM_RECT_INIT(0, 0, 1, 1),
.xrgb = { 0x01FF },
.gray8_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x4C },
},
.rgb332_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xE0 },
},
.rgb565_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xF800 },
.expected_swab = { 0x00F8 },
},
.xrgb1555_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x7C00 },
},
.argb1555_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFC00 },
},
.rgba5551_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xF801 },
},
.rgb888_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x00, 0x00, 0xFF },
},
.argb_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x },
},
.xrgb2101010_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x3FF0 },
},
.argb2101010_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFFF0 },
},
.mono_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0b0 },
},
},
@@ -151,48 +153,48 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0x, 0x10FF,
},
.gray8_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x4C },
},
.rgb332_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xE0 },
},
.rgb565_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xF800 },
.expected_swab = { 0x00F8 },
},
.xrgb1555_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x7C00 },
},
.argb1555_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFC00 },
},
.rgba5551_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xF801 },
},
.rgb888_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x00, 0x00, 0xFF },
},
.argb_result = {
- .dst_pitch = 0,
+ .dst_pitch =
[PATCH v2 0/6] Increase code coverage on drm_format_helper.c
The following series include improvements and new KUnit tests to some functions on drm_format_helper.c. The first patch improves existing conversion tests to assure that the default pitch is used when NULL is used on the `dst_pitch` argument. Patches 2, 3, 4, and 6 add the new parametrized tests to the following functions: - drm_fb_swab() - drm_fb_clip_offset() - drm_fb_build_fourcc_list() - drm_fb_memcpy() The 5th patch is a change to the conversion_buf_size() helper used on the tests, this change was needed to make the patch 6. a coverage report for the file can be found below: https://grillo-0.github.io/coverage-reports/gsoc-drm-format-test/drivers/gpu/drm/drm_format_helper.c.gcov.html Signed-off-by: Arthur Grillo --- v1->v2: https://lore.kernel.org/r/20230721182316.560649-1-arthurgri...@riseup.net - Change patch prefix to "drm/tests" (Maíra Canal) - Simplify the code by changing to an ternary operator on the pitch (Maíra Canal) - Explain how the expected swab colors were obtained (André Almeida) - Fix multi-line comment style (André Almeida) - Remove unnecessary use of drm_kunit_helper_free_device() (Maíra Canal) - Hard-code the expected number of fourcss (Maíra Canal & Andre Almeida) - Fix some sparce warnings (kernel test robot) --- Arthur Grillo (6): drm/tests: Test default pitch fallback drm/tests: Add KUnit tests for drm_fb_swab() drm/tests: Add KUnit tests for drm_fb_clip_offset() drm/tests: Add KUnit tests for drm_fb_build_fourcc_list() drm/tests: Add multi-plane support to conversion_buf_size() drm/tests: Add KUnit tests for drm_fb_memcpy() drivers/gpu/drm/tests/drm_format_helper_test.c | 848 +++-- 1 file changed, 790 insertions(+), 58 deletions(-) --- base-commit: b31f78496643fa6dec31b182a3466cf4139e change-id: 20230810-gsoc-drm-format-test-v2-1989f08e115b Best regards, -- Arthur Grillo
Re: [PATCH drm-misc-next] drm/nouveau: sched: avoid job races between entities
On Thu, Aug 10, 2023 at 8:06 PM Danilo Krummrich wrote: > If a sched job depends on a dma-fence from a job from the same GPU > scheduler instance, but a different scheduler entity, the GPU scheduler > does only wait for the particular job to be scheduled, rather than for > the job to fully complete. This is due to the GPU scheduler assuming > that there is a scheduler instance per ring. However, the current > implementation, in order to avoid arbitrary amounts of kthreads, has a > single scheduler instance while scheduler entities represent rings. > > As a workaround, set the DRM_SCHED_FENCE_DONT_PIPELINE for all > out-fences in order to force the scheduler to wait for full job > completion for dependent jobs from different entities and same scheduler > instance. > > There is some work in progress [1] to address the issues of firmware > schedulers; once it is in-tree the scheduler topology in Nouveau should > be re-worked accordingly. > > [1] > https://lore.kernel.org/dri-devel/20230801205103.627779-1-matthew.br...@intel.com/ > > Signed-off-by: Danilo Krummrich > --- > drivers/gpu/drm/nouveau/nouveau_sched.c | 22 ++ > 1 file changed, 22 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c > b/drivers/gpu/drm/nouveau/nouveau_sched.c > index 3424a1bf6af3..88217185e0f3 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_sched.c > +++ b/drivers/gpu/drm/nouveau/nouveau_sched.c > @@ -292,6 +292,28 @@ nouveau_job_submit(struct nouveau_job *job) > if (job->sync) > done_fence = dma_fence_get(job->done_fence); > > + /* If a sched job depends on a dma-fence from a job from the same > GPU > +* scheduler instance, but a different scheduler entity, the GPU > +* scheduler does only wait for the particular job to be scheduled, > s/does only wait/only waits/ Reviewed-by: Faith Ekstrand +* rather than for the job to fully complete. This is due to the GPU > +* scheduler assuming that there is a scheduler instance per ring. > +* However, the current implementation, in order to avoid arbitrary > +* amounts of kthreads, has a single scheduler instance while > scheduler > +* entities represent rings. > +* > +* As a workaround, set the DRM_SCHED_FENCE_DONT_PIPELINE for all > +* out-fences in order to force the scheduler to wait for full job > +* completion for dependent jobs from different entities and same > +* scheduler instance. > +* > +* There is some work in progress [1] to address the issues of > firmware > +* schedulers; once it is in-tree the scheduler topology in Nouveau > +* should be re-worked accordingly. > +* > +* [1] > https://lore.kernel.org/dri-devel/20230801205103.627779-1-matthew.br...@intel.com/ > +*/ > + set_bit(DRM_SCHED_FENCE_DONT_PIPELINE, >done_fence->flags); > + > if (job->ops->armed_submit) > job->ops->armed_submit(job); > > > base-commit: 68132cc6d1bcbc78ade524c6c6c226de42139f0e > -- > 2.41.0 > >
Re: [PATCH 05/11] drm/bridge: tc358768: Print logical values, not raw register values
On 11/08/2023 19:31, Péter Ujfalusi wrote: On 04/08/2023 13:44, Tomi Valkeinen wrote: The driver debug prints DSI related timings as raw register values in hex. It is much more useful to see the "logical" value of the timing, not the register value. I'm a bit confused by the term 'logical' value, I think you meant decimal, easier to read by humans numbers. Not just decimal. Previously the code printed the register values, which e.g. could contain two values ORed together. So, with "logical" I just meant the "real" value, instead of "register-encoded". Change the prints to print the values separately, in case a single register contains multiple values, and use %u to have it in a more human consumable form. But, yes, decimal is better for the dmesg, as I recall I had a tool which was using hex numbers so it was better to have the prints also in hex. Reviewed-by: Peter Ujfalusi Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index 9b633038af33..0ef51d04bb21 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -739,57 +739,59 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) /* LP11 > 100us for D-PHY Rx Init */ val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1; - dev_dbg(priv->dev, "LINEINITCNT: 0x%x\n", val); + dev_dbg(priv->dev, "LINEINITCNT: %u\n", val); tc358768_write(priv, TC358768_LINEINITCNT, val); /* LPTimeCnt > 50ns */ val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1; lptxcnt = val; - dev_dbg(priv->dev, "LPTXTIMECNT: 0x%x\n", val); + dev_dbg(priv->dev, "LPTXTIMECNT: %u\n", val); tc358768_write(priv, TC358768_LPTXTIMECNT, val); /* 38ns < TCLK_PREPARE < 95ns */ val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1; + dev_dbg(priv->dev, "TCLK_PREPARECNT %u\n", val); /* TCLK_PREPARE + TCLK_ZERO > 300ns */ val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 2; + dev_dbg(priv->dev, "TCLK_ZEROCNT %u\n", val2); val |= val2 << 8; - dev_dbg(priv->dev, "TCLK_HEADERCNT: 0x%x\n", val); tc358768_write(priv, TC358768_TCLK_HEADERCNT, val); /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */ raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 5; val = clamp(raw_val, 0, 127); - dev_dbg(priv->dev, "TCLK_TRAILCNT: 0x%x\n", val); + dev_dbg(priv->dev, "TCLK_TRAILCNT: %u\n", val); tc358768_write(priv, TC358768_TCLK_TRAILCNT, val); /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */ val = 50 + tc358768_to_ns(4 * ui_nsk); val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1; + dev_dbg(priv->dev, "THS_PREPARECNT %u\n", val); /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */ raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), dsibclk_nsk) - 10; val2 = clamp(raw_val, 0, 127); + dev_dbg(priv->dev, "THS_ZEROCNT %u\n", val2); val |= val2 << 8; - dev_dbg(priv->dev, "THS_HEADERCNT: 0x%x\n", val); tc358768_write(priv, TC358768_THS_HEADERCNT, val); /* TWAKEUP > 1ms in lptxcnt steps */ val = tc358768_ns_to_cnt(102, dsibclk_nsk); val = val / (lptxcnt + 1) - 1; - dev_dbg(priv->dev, "TWAKEUP: 0x%x\n", val); + dev_dbg(priv->dev, "TWAKEUP: %u\n", val); tc358768_write(priv, TC358768_TWAKEUP, val); /* TCLK_POSTCNT > 60ns + 52*UI */ val = tc358768_ns_to_cnt(60 + tc358768_to_ns(52 * ui_nsk), dsibclk_nsk) - 3; - dev_dbg(priv->dev, "TCLK_POSTCNT: 0x%x\n", val); + dev_dbg(priv->dev, "TCLK_POSTCNT: %u\n", val); tc358768_write(priv, TC358768_TCLK_POSTCNT, val); /* max(60ns + 4*UI, 8*UI) < THS_TRAILCNT < 105ns + 12*UI */ raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(18 * ui_nsk), dsibclk_nsk) - 4; val = clamp(raw_val, 0, 15); - dev_dbg(priv->dev, "THS_TRAILCNT: 0x%x\n", val); + dev_dbg(priv->dev, "THS_TRAILCNT: %u\n", val); tc358768_write(priv, TC358768_THS_TRAILCNT, val); val = BIT(0); @@ -803,10 +805,11 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) /* TXTAGOCNT[26:16] RXTASURECNT[10:0] */ val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4); val = tc358768_ns_to_cnt(val, dsibclk_nsk) / 4 - 1; + dev_dbg(priv->dev, "TXTAGOCNT: %u\n", val); val2 = tc358768_ns_to_cnt(tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk), dsibclk_nsk) - 2; + dev_dbg(priv->dev, "RXTASURECNT: %u\n", val2); val = val << 16 | val2; - dev_dbg(priv->dev, "BTACNTRL1: 0x%x\n", val);
Re: [PATCH v2] dt-bindings: qcom: Update RPMHPD entries for some SoCs
On Thu, 27 Jul 2023 18:13:33 +0530, Rohit Agarwal wrote: > Update the RPMHPD references with new bindings defined in rpmhpd.h > for Qualcomm SoCs SM8[2345]50. > > Signed-off-by: Rohit Agarwal > --- > > Changes in v2: > - Removed the unnecessary inclusion of header rpmpd.h. > > This patch is dependent on the series that includes the new rpmhpd.h header > https://lore.kernel.org/all/1689744162-9421-1-git-send-email-quic_rohia...@quicinc.com/ > > Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 4 ++-- > Documentation/devicetree/bindings/clock/qcom,sm8350-videocc.yaml | 4 ++-- > Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml| 4 ++-- > Documentation/devicetree/bindings/clock/qcom,sm8450-dispcc.yaml | 4 ++-- > Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 4 ++-- > Documentation/devicetree/bindings/clock/qcom,sm8550-dispcc.yaml | 4 ++-- > Documentation/devicetree/bindings/clock/qcom,videocc.yaml | 4 ++-- > .../devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml | 4 ++-- > .../devicetree/bindings/display/msm/qcom,sm8250-mdss.yaml | 8 > > .../devicetree/bindings/display/msm/qcom,sm8350-dpu.yaml | 4 ++-- > .../devicetree/bindings/display/msm/qcom,sm8350-mdss.yaml | 6 +++--- > .../devicetree/bindings/display/msm/qcom,sm8450-dpu.yaml | 4 ++-- > .../devicetree/bindings/display/msm/qcom,sm8450-mdss.yaml | 8 > > .../devicetree/bindings/display/msm/qcom,sm8550-dpu.yaml | 4 ++-- > .../devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml | 8 > > Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml| 4 ++-- > Documentation/devicetree/bindings/mmc/sdhci-msm.yaml | 4 ++-- > Documentation/devicetree/bindings/remoteproc/qcom,sm8350-pas.yaml | 6 +++--- > 18 files changed, 44 insertions(+), 44 deletions(-) > Acked-by: Rob Herring
Re: [PATCH 02/11] drm/bridge: tc358768: Fix bit updates
On 11/08/2023 19:23, Péter Ujfalusi wrote:
On 04/08/2023 13:44, Tomi Valkeinen wrote:
The driver has a few places where it does:
if (thing_is_enabled_in_config)
update_thing_bit_in_hw()
This means that if the thing is _not_ enabled, the bit never gets
cleared. This affects the h/vsyncs and continuous DSI clock bits.
I guess the idea was to keep the reset value unless it needs to be flipped.
Fix the driver to always update the bit.
Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
Signed-off-by: Tomi Valkeinen
---
drivers/gpu/drm/bridge/tc358768.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358768.c
b/drivers/gpu/drm/bridge/tc358768.c
index bc97a837955b..b668f77673c3 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -794,8 +794,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
*bridge)
val |= BIT(i + 1);
tc358768_write(priv, TC358768_HSTXVREGEN, val);
- if (!(mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS))
- tc358768_write(priv, TC358768_TXOPTIONCNTRL, 0x1);
+ tc358768_write(priv, TC358768_TXOPTIONCNTRL,
+ (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) ? 0 :
BIT(0));
/* TXTAGOCNT[26:16] RXTASURECNT[10:0] */
val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4);
@@ -861,11 +861,12 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
*bridge)
tc358768_write(priv, TC358768_DSI_HACT, hact);
/* VSYNC polarity */
- if (!(mode->flags & DRM_MODE_FLAG_NVSYNC))
- tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5), BIT(5));
+ tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5),
+(mode->flags & DRM_MODE_FLAG_PVSYNC) ? BIT(5) : 0);
Was this the reverse before and should be:
(mode->flags & DRM_MODE_FLAG_PVSYNC) ? 0 : BIT(5)
Bit 5 is 1 for active high vsync polarity. The test was previously
!nvsync, i.e. the same as pvsync.
Tomi
Re: [PATCH 11/11] drm/bridge: tc358768: Add DRM_BRIDGE_ATTACH_NO_CONNECTOR support
On 11/08/2023 19:44, Péter Ujfalusi wrote:
On 04/08/2023 13:44, Tomi Valkeinen wrote:
I would rather have a commit message than a blank one.
Oops...
Signed-off-by: Tomi Valkeinen
---
drivers/gpu/drm/bridge/tc358768.c | 64 +++
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358768.c
b/drivers/gpu/drm/bridge/tc358768.c
index ea19de5509ed..a567f136ddc7 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -131,8 +131,17 @@ static const char * const tc358768_supplies[] = {
struct tc358768_dsi_output {
struct mipi_dsi_device *dev;
+
+ /* Legacy field if DRM_BRIDGE_ATTACH_NO_CONNECTOR is not used */
struct drm_panel *panel;
- struct drm_bridge *bridge;
+
+ /*
+* If DRM_BRIDGE_ATTACH_NO_CONNECTOR is not used and a panel is attached
+* to tc358768, 'next_bridge' contains the bridge the driver created
+* with drm_panel_bridge_add_typed(). Otherwise 'next_bridge' contains
+* the next bridge the driver found.
+*/
+ struct drm_bridge *next_bridge;
why it is better to call it next_bridge than just bridge? Is there a
prev_bridge also?
There is, prev bridge would be the bridge behind tc358768 in the chain.
Bridge is tc358768. Next bridge is the following one.
Here, it's in the tc358768_dsi_output struct, so bridge is perhaps ok. I
just wanted to be extra clear here, as I think it's often called
next_bridge in other drivers.
Tomi
Re: [PATCH v2 13/15] drm/panthor: Allow driver compilation
Hi, On 11/08/2023 17:35, Robin Murphy wrote: On 2023-08-09 17:53, Boris Brezillon wrote: +obj-$(CONFIG_DRM_PANTHOR) += panthor.o FWIW I still think it would be nice to have a minor directory/Kconfig/Makefile reshuffle and a trivial bit of extra registration glue to build both drivers into a single module. It seems like it could be a perpetual source of confusion to end users where Mesa "panfrost" is the right option but kernel "panfrost" is the wrong one. Especially when pretty much every other GPU driver is also just one big top-level module to load for many different generations of hardware. Plus it would mean that if someone did want to have a go at deduplicating the resource-wrangling boilerplate for OPPs etc. in future, there's more chance of being able to do so meaningfully. It might be nice to point it out, but to be fair Intel and AMD both have two (or more) drivers, as does Broadcom/RPi. As does, err ... Mali. I can see the point, but otoh if someone's managed to build all the right regulator/clock/etc modules to get a working system, they'll probably manage to figure teh GPU side out? Cheers, Daniel
Re: [git pull] drm fixes for 6.4-rc6
The pull request you sent on Fri, 11 Aug 2023 16:27:34 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2023-08-11 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/9b1b1b74ddb236e7ccf6d11d4c0b642fbe0c66c6 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH 11/11] drm/bridge: tc358768: Add DRM_BRIDGE_ATTACH_NO_CONNECTOR support
On 04/08/2023 13:44, Tomi Valkeinen wrote:
I would rather have a commit message than a blank one.
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/tc358768.c | 64
> +++
> 1 file changed, 45 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c
> b/drivers/gpu/drm/bridge/tc358768.c
> index ea19de5509ed..a567f136ddc7 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -131,8 +131,17 @@ static const char * const tc358768_supplies[] = {
>
> struct tc358768_dsi_output {
> struct mipi_dsi_device *dev;
> +
> + /* Legacy field if DRM_BRIDGE_ATTACH_NO_CONNECTOR is not used */
> struct drm_panel *panel;
> - struct drm_bridge *bridge;
> +
> + /*
> + * If DRM_BRIDGE_ATTACH_NO_CONNECTOR is not used and a panel is attached
> + * to tc358768, 'next_bridge' contains the bridge the driver created
> + * with drm_panel_bridge_add_typed(). Otherwise 'next_bridge' contains
> + * the next bridge the driver found.
> + */
> + struct drm_bridge *next_bridge;
why it is better to call it next_bridge than just bridge? Is there a
prev_bridge also?
> };
>
> struct tc358768_priv {
> @@ -391,8 +400,6 @@ static int tc358768_dsi_host_attach(struct mipi_dsi_host
> *host,
> struct mipi_dsi_device *dev)
> {
> struct tc358768_priv *priv = dsi_host_to_tc358768(host);
> - struct drm_bridge *bridge;
> - struct drm_panel *panel;
> struct device_node *ep;
> int ret;
>
> @@ -420,21 +427,7 @@ static int tc358768_dsi_host_attach(struct mipi_dsi_host
> *host,
> return -ENOTSUPP;
> }
>
> - ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0, ,
> - );
> - if (ret)
> - return ret;
> -
> - if (panel) {
> - bridge = drm_panel_bridge_add_typed(panel,
> - DRM_MODE_CONNECTOR_DSI);
> - if (IS_ERR(bridge))
> - return PTR_ERR(bridge);
> - }
> -
> priv->output.dev = dev;
> - priv->output.bridge = bridge;
> - priv->output.panel = panel;
>
> priv->dsi_lanes = dev->lanes;
> priv->dsi_bpp = mipi_dsi_pixel_format_to_bpp(dev->format);
> @@ -463,7 +456,7 @@ static int tc358768_dsi_host_detach(struct mipi_dsi_host
> *host,
>
> drm_bridge_remove(>bridge);
> if (priv->output.panel)
> - drm_panel_bridge_remove(priv->output.bridge);
> + drm_panel_bridge_remove(priv->output.next_bridge);
>
> return 0;
> }
> @@ -544,7 +537,40 @@ static int tc358768_bridge_attach(struct drm_bridge
> *bridge,
> return -ENOTSUPP;
> }
>
> - return drm_bridge_attach(bridge->encoder, priv->output.bridge, bridge,
> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> + struct device_node *node;
> +
> + /* Get the next bridge, connected to port@1. */
> + node = of_graph_get_remote_node(priv->dev->of_node, 1, -1);
> + if (!node)
> + return -ENODEV;
> +
> + priv->output.next_bridge = of_drm_find_bridge(node);
> + of_node_put(node);
> + if (!priv->output.next_bridge)
> + return -EPROBE_DEFER;
> + } else {
> + struct drm_bridge *bridge;
> + struct drm_panel *panel;
> + int ret;
> +
> + ret = drm_of_find_panel_or_bridge(priv->dev->of_node, 1, 0,
> + , );
> + if (ret)
> + return ret;
> +
> + if (panel) {
> + bridge = drm_panel_bridge_add_typed(panel,
> + DRM_MODE_CONNECTOR_DSI);
> + if (IS_ERR(bridge))
> + return PTR_ERR(bridge);
> + }
> +
> + priv->output.next_bridge = bridge;
> + priv->output.panel = panel;
> + }
> +
> + return drm_bridge_attach(bridge->encoder, priv->output.next_bridge,
> bridge,
>flags);
> }
>
>
--
Péter
Re: [PATCH] drm/msm/a690: Switch to a660_gmu.bin
On Fri, Aug 11, 2023 at 9:31 AM Konrad Dybcio wrote:
>
> On 11.08.2023 18:21, Rob Clark wrote:
> > On Fri, Aug 11, 2023 at 9:11 AM Konrad Dybcio
> > wrote:
> >>
> >> On 11.08.2023 18:09, Rob Clark wrote:
> >>> On Fri, Aug 11, 2023 at 9:05 AM Rob Clark wrote:
>
> From: Rob Clark
>
> There isn't actually a a690_gmu.bin. But it appears that the normal
> a660_gmu.bin works fine. Normally all the devices within a sub-
> generation (or "family") will use the same fw, and a690 is in the a660
> family.
>
> >>>
> >>> possibly this could be considered as:
> >>>
> >>> Fixes: 5e7665b5e484 ("drm/msm/adreno: Add Adreno A690 support")
> >> For a lack of a better response, "meh"
> >
> > It would help route the change back to 6.4.y so we can stop explaining
> > to folks that they should create a symlink ;-)
> Ok fair!
>
> >
> >> Other than that:
> >>
> >> Reviewed-by: Konrad Dybcio
> >>
> >> Perhaps we could take this further and do something like
> >>
> >> if (failed to load gmu fw)
> >> try loading "gmu.bin"
> >
> > that (loading random fw) sounds like a bad idea.. plus gmu.bin doesn't exist
> I guess we still should hear from Qualcomm on this one, but I
> do believe that "aXYZ_gmu.bin" is really "gmu firmware, tagged
> at aXYZ release". They're even versioned and adreno-gpulist.h
> downstream contains the minimum version for each SKU.
We definitely can't use a660_gmu.bin as is on a630 family devices, for
example. Maybe that is just down to sw <-> fw interface changes but
it means linux-firmware would still need to keep separate a630_gmu.bin
and a660_gmu.bin, otherwise new l-f with old kernel would explode.
That doesn't mean they are unrelated.. I think they probably just
branch the fw for each family.
BR,
-R
> Konrad
Re: [PATCH 10/11] drm/bridge: tc358768: Attempt to fix DSI horizontal timings
On 04/08/2023 13:44, Tomi Valkeinen wrote:
> The DSI horizontal timing calculations done by the driver seem to often
> lead to underflows or overflows, depending on the videomode.
>
> There are two main things the current driver doesn't seem to get right:
> DSI HSW and HFP, and VSDly. However, even following Toshiba's
> documentation it seems we don't always get a working display.
>
> This patch attempts to fix the horizontal timings for DSI event mode, and
> on a system with a DSI->HDMI encoder, a lot of standard HDMI modes now
> seem to work. The work relies on Toshiba's documentation, but also quite
> a bit on empirical testing.
>
> This also adds timing related debug prints to make it easier to improve
> on this later.
>
> The DSI pulse mode has only been tested with a fixed-resolution panel,
> which limits the testing of different modes on DSI pulse mode. However,
> as the VSDly calculation also affects pulse mode, so this might cause a
> regression.
If my memory serves me right we only had this used in a sinlge, static
configuration and again, it might be mentioned in a comment somewhere?
Nice work!
Reviewed-by: Peter Ujfalusi
>
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/tc358768.c | 211
> +-
> 1 file changed, 183 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c
> b/drivers/gpu/drm/bridge/tc358768.c
> index dc2241c18dde..ea19de5509ed 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -9,6 +9,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -157,6 +158,7 @@ struct tc358768_priv {
> u32 frs;/* PLL Freqency range for HSCK (post divider) */
>
> u32 dsiclk; /* pll_clk / 2 */
> + u32 pclk; /* incoming pclk rate */
> };
>
> static inline struct tc358768_priv *dsi_host_to_tc358768(struct mipi_dsi_host
> @@ -380,6 +382,7 @@ static int tc358768_calc_pll(struct tc358768_priv *priv,
> priv->prd = best_prd;
> priv->frs = frs;
> priv->dsiclk = best_pll / 2;
> + priv->pclk = mode->clock * 1000;
>
> return 0;
> }
> @@ -638,6 +641,28 @@ static u32 tc358768_ps_to_ns(u32 ps)
> return ps / 1000;
> }
>
> +static u32 tc358768_dpi_to_ns(u32 val, u32 pclk)
> +{
> + return (u32)div_u64((u64)val * NANO, pclk);
> +}
> +
> +/* Convert value in DPI pixel clock units to DSI byte count */
> +static u32 tc358768_dpi_to_dsi_bytes(struct tc358768_priv *priv, u32 val)
> +{
> + u64 m = (u64)val * priv->dsiclk / 4 * priv->dsi_lanes;
> + u64 n = priv->pclk;
> +
> + return (u32)div_u64(m + n - 1, n);
> +}
> +
> +static u32 tc358768_dsi_bytes_to_ns(struct tc358768_priv *priv, u32 val)
> +{
> + u64 m = (u64)val * NANO;
> + u64 n = priv->dsiclk / 4 * priv->dsi_lanes;
> +
> + return (u32)div_u64(m, n);
> +}
> +
> static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> {
> struct tc358768_priv *priv = bridge_to_tc358768(bridge);
> @@ -647,11 +672,19 @@ static void tc358768_bridge_pre_enable(struct
> drm_bridge *bridge)
> s32 raw_val;
> const struct drm_display_mode *mode;
> u32 hsbyteclk_ps, dsiclk_ps, ui_ps;
> - u32 dsiclk, hsbyteclk, video_start;
> - const u32 internal_delay = 40;
> + u32 dsiclk, hsbyteclk;
> int ret, i;
> struct videomode vm;
> struct device *dev = priv->dev;
> + /* In pixelclock units */
> + u32 dpi_htot, dpi_data_start;
> + /* In byte units */
> + u32 dsi_dpi_htot, dsi_dpi_data_start;
> + u32 dsi_hsw, dsi_hbp, dsi_hact, dsi_hfp;
> + const u32 dsi_hss = 4; /* HSS is a short packet (4 bytes) */
> + /* In hsbyteclk units */
> + u32 dsi_vsdly;
> + const u32 internal_dly = 40;
>
> if (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
> dev_warn_once(dev, "Non-continuous mode unimplemented, falling
> back to continuous\n");
> @@ -686,27 +719,23 @@ static void tc358768_bridge_pre_enable(struct
> drm_bridge *bridge)
> case MIPI_DSI_FMT_RGB888:
> val |= (0x3 << 4);
> hact = vm.hactive * 3;
> - video_start = (vm.hsync_len + vm.hback_porch) * 3;
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> break;
> case MIPI_DSI_FMT_RGB666:
> val |= (0x4 << 4);
> hact = vm.hactive * 3;
> - video_start = (vm.hsync_len + vm.hback_porch) * 3;
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
> break;
>
> case MIPI_DSI_FMT_RGB666_PACKED:
> val |= (0x4 << 4) | BIT(3);
> hact = vm.hactive * 18 / 8;
> - video_start = (vm.hsync_len + vm.hback_porch) * 18 / 8;
> data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
> break;
>
> case MIPI_DSI_FMT_RGB565:
> val |= (0x5 << 4);
> hact
Re: [PATCH v2 13/15] drm/panthor: Allow driver compilation
On 2023-08-09 17:53, Boris Brezillon wrote: Now that all blocks are available, we can add/update Kconfig/Makefile files to allow compilation. v2: - Rename the driver (pancsf -> panthor) - Change the license (GPL2 -> MIT + GPL2) - Split the driver addition commit - Add new dependencies on GPUVA and DRM_SCHED Signed-off-by: Boris Brezillon --- drivers/gpu/drm/Kconfig | 2 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/panthor/Kconfig | 16 drivers/gpu/drm/panthor/Makefile | 15 +++ 4 files changed, 34 insertions(+) create mode 100644 drivers/gpu/drm/panthor/Kconfig create mode 100644 drivers/gpu/drm/panthor/Makefile diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 2a44b9419d4d..bddfbdb2ffee 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -358,6 +358,8 @@ source "drivers/gpu/drm/lima/Kconfig" source "drivers/gpu/drm/panfrost/Kconfig" +source "drivers/gpu/drm/panthor/Kconfig" + source "drivers/gpu/drm/aspeed/Kconfig" source "drivers/gpu/drm/mcde/Kconfig" diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 215e78e79125..0a260727505f 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -188,6 +188,7 @@ obj-$(CONFIG_DRM_TVE200) += tve200/ obj-$(CONFIG_DRM_XEN) += xen/ obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/ obj-$(CONFIG_DRM_LIMA) += lima/ +obj-$(CONFIG_DRM_PANTHOR) += panthor/ obj-$(CONFIG_DRM_PANFROST) += panfrost/ obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/ obj-$(CONFIG_DRM_MCDE) += mcde/ diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig new file mode 100644 index ..a9d17b1bbb75 --- /dev/null +++ b/drivers/gpu/drm/panthor/Kconfig @@ -0,0 +1,16 @@ +# SPDX-License-Identifier: GPL-2.0 or MIT + +config DRM_PANTHOR + tristate "Panthor (DRM support for ARM Mali CSF-based GPUs)" + depends on DRM + depends on ARM || ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64) + depends on MMU + select DRM_EXEC + select DRM_SCHED + select IOMMU_SUPPORT + select IOMMU_IO_PGTABLE_LPAE + select DRM_GEM_SHMEM_HELPER + select PM_DEVFREQ + select DEVFREQ_GOV_SIMPLE_ONDEMAND + help + DRM driver for ARM Mali CSF-based GPUs. diff --git a/drivers/gpu/drm/panthor/Makefile b/drivers/gpu/drm/panthor/Makefile new file mode 100644 index ..64193a484879 --- /dev/null +++ b/drivers/gpu/drm/panthor/Makefile @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: GPL-2.0 or MIT + +panthor-y := \ + panthor_devfreq.o \ + panthor_device.o \ + panthor_drv.o \ + panthor_gem.o \ + panthor_gpu.o \ + panthor_heap.o \ + panthor_heap.o \ + panthor_fw.o \ + panthor_mmu.o \ + panthor_sched.o + +obj-$(CONFIG_DRM_PANTHOR) += panthor.o FWIW I still think it would be nice to have a minor directory/Kconfig/Makefile reshuffle and a trivial bit of extra registration glue to build both drivers into a single module. It seems like it could be a perpetual source of confusion to end users where Mesa "panfrost" is the right option but kernel "panfrost" is the wrong one. Especially when pretty much every other GPU driver is also just one big top-level module to load for many different generations of hardware. Plus it would mean that if someone did want to have a go at deduplicating the resource-wrangling boilerplate for OPPs etc. in future, there's more chance of being able to do so meaningfully. Cheers, Robin.
Re: [PATCH 09/11] drm/bridge: tc358768: Fix tc358768_ns_to_cnt()
On 04/08/2023 13:44, Tomi Valkeinen wrote:
> The tc358768_ns_to_cnt() is, most likely, supposed to do a div-round-up
> operation, but it misses subtracting one from the dividend.
>
> Fix this by just using DIV_ROUND_UP().
Reviewed-by: Peter Ujfalusi
> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/tc358768.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c
> b/drivers/gpu/drm/bridge/tc358768.c
> index 9411b0fb471e..dc2241c18dde 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -630,7 +630,7 @@ static int tc358768_setup_pll(struct tc358768_priv *priv,
>
> static u32 tc358768_ns_to_cnt(u32 ns, u32 period_ps)
> {
> - return (ns * 1000 + period_ps) / period_ps;
> + return DIV_ROUND_UP(ns * 1000, period_ps);
> }
>
> static u32 tc358768_ps_to_ns(u32 ps)
>
--
Péter
Re: [PATCH 08/11] drm/bridge: tc358768: Clean up clock period code
On 04/08/2023 13:44, Tomi Valkeinen wrote:
> The driver defines TC358768_PRECISION as 1000, and uses "nsk" to refer
> to clock periods. The original author does not remember where all this
> came from.
I can confirm this!
> Effectively the driver is using picoseconds as the unit for
> clock periods, yet referring to them by "nsk".
>
> Clean this up by just saying the periods are in picoseconds.
Thanks,
Reviewed-by: Peter Ujfalusi
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/tc358768.c | 60
> +++
> 1 file changed, 29 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c
> b/drivers/gpu/drm/bridge/tc358768.c
> index db45b4a982c0..9411b0fb471e 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -627,15 +628,14 @@ static int tc358768_setup_pll(struct tc358768_priv
> *priv,
> return tc358768_clear_error(priv);
> }
>
> -#define TC358768_PRECISION 1000
> -static u32 tc358768_ns_to_cnt(u32 ns, u32 period_nsk)
> +static u32 tc358768_ns_to_cnt(u32 ns, u32 period_ps)
> {
> - return (ns * TC358768_PRECISION + period_nsk) / period_nsk;
> + return (ns * 1000 + period_ps) / period_ps;
> }
>
> -static u32 tc358768_to_ns(u32 nsk)
> +static u32 tc358768_ps_to_ns(u32 ps)
> {
> - return (nsk / TC358768_PRECISION);
> + return ps / 1000;
> }
>
> static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
> @@ -646,7 +646,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
> *bridge)
> u32 val, val2, lptxcnt, hact, data_type;
> s32 raw_val;
> const struct drm_display_mode *mode;
> - u32 hsbyteclk_nsk, dsiclk_nsk, ui_nsk;
> + u32 hsbyteclk_ps, dsiclk_ps, ui_ps;
> u32 dsiclk, hsbyteclk, video_start;
> const u32 internal_delay = 40;
> int ret, i;
> @@ -730,67 +730,65 @@ static void tc358768_bridge_pre_enable(struct
> drm_bridge *bridge)
> tc358768_write(priv, TC358768_D0W_CNTRL + i * 4, 0x);
>
> /* DSI Timings */
> - hsbyteclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION,
> - hsbyteclk);
> - dsiclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, dsiclk);
> - ui_nsk = dsiclk_nsk / 2;
> - dev_dbg(dev, "dsiclk_nsk: %u\n", dsiclk_nsk);
> - dev_dbg(dev, "ui_nsk: %u\n", ui_nsk);
> - dev_dbg(dev, "hsbyteclk_nsk: %u\n", hsbyteclk_nsk);
> + hsbyteclk_ps = (u32)div_u64(PICO, hsbyteclk);
> + dsiclk_ps = (u32)div_u64(PICO, dsiclk);
> + ui_ps = dsiclk_ps / 2;
> + dev_dbg(dev, "dsiclk: %u ps, ui %u ps, hsbyteclk %u ps\n", dsiclk_ps,
> + ui_ps, hsbyteclk_ps);
>
> /* LP11 > 100us for D-PHY Rx Init */
> - val = tc358768_ns_to_cnt(100 * 1000, hsbyteclk_nsk) - 1;
> + val = tc358768_ns_to_cnt(100 * 1000, hsbyteclk_ps) - 1;
> dev_dbg(dev, "LINEINITCNT: %u\n", val);
> tc358768_write(priv, TC358768_LINEINITCNT, val);
>
> /* LPTimeCnt > 50ns */
> - val = tc358768_ns_to_cnt(50, hsbyteclk_nsk) - 1;
> + val = tc358768_ns_to_cnt(50, hsbyteclk_ps) - 1;
> lptxcnt = val;
> dev_dbg(dev, "LPTXTIMECNT: %u\n", val);
> tc358768_write(priv, TC358768_LPTXTIMECNT, val);
>
> /* 38ns < TCLK_PREPARE < 95ns */
> - val = tc358768_ns_to_cnt(65, hsbyteclk_nsk) - 1;
> + val = tc358768_ns_to_cnt(65, hsbyteclk_ps) - 1;
> dev_dbg(dev, "TCLK_PREPARECNT %u\n", val);
> /* TCLK_PREPARE + TCLK_ZERO > 300ns */
> - val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk),
> - hsbyteclk_nsk) - 2;
> + val2 = tc358768_ns_to_cnt(300 - tc358768_ps_to_ns(2 * ui_ps),
> + hsbyteclk_ps) - 2;
> dev_dbg(dev, "TCLK_ZEROCNT %u\n", val2);
> val |= val2 << 8;
> tc358768_write(priv, TC358768_TCLK_HEADERCNT, val);
>
> /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */
> - raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk),
> hsbyteclk_nsk) - 5;
> + raw_val = tc358768_ns_to_cnt(60 + tc358768_ps_to_ns(2 * ui_ps),
> hsbyteclk_ps) - 5;
> val = clamp(raw_val, 0, 127);
> dev_dbg(dev, "TCLK_TRAILCNT: %u\n", val);
> tc358768_write(priv, TC358768_TCLK_TRAILCNT, val);
>
> /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */
> - val = 50 + tc358768_to_ns(4 * ui_nsk);
> - val = tc358768_ns_to_cnt(val, hsbyteclk_nsk) - 1;
> + val = 50 + tc358768_ps_to_ns(4 * ui_ps);
> + val = tc358768_ns_to_cnt(val, hsbyteclk_ps) - 1;
> dev_dbg(dev, "THS_PREPARECNT %u\n", val);
> /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */
> - raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk),
> hsbyteclk_nsk) - 10;
> + raw_val = tc358768_ns_to_cnt(145 - tc358768_ps_to_ns(3 * ui_ps),
> hsbyteclk_ps) - 10;
>
Re: [PATCH 07/11] drm/bridge: tc358768: Rename dsibclk to hsbyteclk
On 04/08/2023 13:44, Tomi Valkeinen wrote: > The Toshiba documentation talks about HSByteClk when referring to the > DSI HS byte clock, whereas the driver uses 'dsibclk' name. Also, in a > few places the driver calculates the byte clock from the DSI clock, even > if the byte clock is already available in a variable. If you say so ;) I don't have access to the documentation anymore. > > To align the driver with the documentation, change the 'dsibclk' > variable to 'hsbyteclk'. This also make it easier to visually separate > 'dsibclk' and 'dsiclk' variables. Reviewed-by: Peter Ujfalusi > > Signed-off-by: Tomi Valkeinen > --- > drivers/gpu/drm/bridge/tc358768.c | 48 > +++ > 1 file changed, 24 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358768.c > b/drivers/gpu/drm/bridge/tc358768.c > index 3266c08d9bf1..db45b4a982c0 100644 > --- a/drivers/gpu/drm/bridge/tc358768.c > +++ b/drivers/gpu/drm/bridge/tc358768.c > @@ -604,7 +604,7 @@ static int tc358768_setup_pll(struct tc358768_priv *priv, > > dev_dbg(priv->dev, "PLL: refclk %lu, fbd %u, prd %u, frs %u\n", > clk_get_rate(priv->refclk), fbd, prd, frs); > - dev_dbg(priv->dev, "PLL: pll_clk: %u, DSIClk %u, DSIByteClk %u\n", > + dev_dbg(priv->dev, "PLL: pll_clk: %u, DSIClk %u, HSByteClk %u\n", > priv->dsiclk * 2, priv->dsiclk, priv->dsiclk / 4); > dev_dbg(priv->dev, "PLL: pclk %u (panel: %u)\n", > tc358768_pll_to_pclk(priv, priv->dsiclk * 2), > @@ -646,8 +646,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge > *bridge) > u32 val, val2, lptxcnt, hact, data_type; > s32 raw_val; > const struct drm_display_mode *mode; > - u32 dsibclk_nsk, dsiclk_nsk, ui_nsk; > - u32 dsiclk, dsibclk, video_start; > + u32 hsbyteclk_nsk, dsiclk_nsk, ui_nsk; > + u32 dsiclk, hsbyteclk, video_start; > const u32 internal_delay = 40; > int ret, i; > struct videomode vm; > @@ -678,7 +678,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge > *bridge) > drm_display_mode_to_videomode(mode, ); > > dsiclk = priv->dsiclk; > - dsibclk = dsiclk / 4; > + hsbyteclk = dsiclk / 4; > > /* Data Format Control Register */ > val = BIT(2) | BIT(1) | BIT(0); /* rdswap_en | dsitx_en | txdt_en */ > @@ -730,67 +730,67 @@ static void tc358768_bridge_pre_enable(struct > drm_bridge *bridge) > tc358768_write(priv, TC358768_D0W_CNTRL + i * 4, 0x); > > /* DSI Timings */ > - dsibclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, > - dsibclk); > + hsbyteclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, > + hsbyteclk); > dsiclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, dsiclk); > ui_nsk = dsiclk_nsk / 2; > dev_dbg(dev, "dsiclk_nsk: %u\n", dsiclk_nsk); > dev_dbg(dev, "ui_nsk: %u\n", ui_nsk); > - dev_dbg(dev, "dsibclk_nsk: %u\n", dsibclk_nsk); > + dev_dbg(dev, "hsbyteclk_nsk: %u\n", hsbyteclk_nsk); > > /* LP11 > 100us for D-PHY Rx Init */ > - val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1; > + val = tc358768_ns_to_cnt(100 * 1000, hsbyteclk_nsk) - 1; > dev_dbg(dev, "LINEINITCNT: %u\n", val); > tc358768_write(priv, TC358768_LINEINITCNT, val); > > /* LPTimeCnt > 50ns */ > - val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1; > + val = tc358768_ns_to_cnt(50, hsbyteclk_nsk) - 1; > lptxcnt = val; > dev_dbg(dev, "LPTXTIMECNT: %u\n", val); > tc358768_write(priv, TC358768_LPTXTIMECNT, val); > > /* 38ns < TCLK_PREPARE < 95ns */ > - val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1; > + val = tc358768_ns_to_cnt(65, hsbyteclk_nsk) - 1; > dev_dbg(dev, "TCLK_PREPARECNT %u\n", val); > /* TCLK_PREPARE + TCLK_ZERO > 300ns */ > val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk), > - dsibclk_nsk) - 2; > + hsbyteclk_nsk) - 2; > dev_dbg(dev, "TCLK_ZEROCNT %u\n", val2); > val |= val2 << 8; > tc358768_write(priv, TC358768_TCLK_HEADERCNT, val); > > /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */ > - raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), > dsibclk_nsk) - 5; > + raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), > hsbyteclk_nsk) - 5; > val = clamp(raw_val, 0, 127); > dev_dbg(dev, "TCLK_TRAILCNT: %u\n", val); > tc358768_write(priv, TC358768_TCLK_TRAILCNT, val); > > /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */ > val = 50 + tc358768_to_ns(4 * ui_nsk); > - val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1; > + val = tc358768_ns_to_cnt(val, hsbyteclk_nsk) - 1; > dev_dbg(dev, "THS_PREPARECNT %u\n", val); > /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */ > - raw_val =
Re: [PATCH] drm/msm/a690: Switch to a660_gmu.bin
On 11.08.2023 18:21, Rob Clark wrote:
> On Fri, Aug 11, 2023 at 9:11 AM Konrad Dybcio
> wrote:
>>
>> On 11.08.2023 18:09, Rob Clark wrote:
>>> On Fri, Aug 11, 2023 at 9:05 AM Rob Clark wrote:
From: Rob Clark
There isn't actually a a690_gmu.bin. But it appears that the normal
a660_gmu.bin works fine. Normally all the devices within a sub-
generation (or "family") will use the same fw, and a690 is in the a660
family.
>>>
>>> possibly this could be considered as:
>>>
>>> Fixes: 5e7665b5e484 ("drm/msm/adreno: Add Adreno A690 support")
>> For a lack of a better response, "meh"
>
> It would help route the change back to 6.4.y so we can stop explaining
> to folks that they should create a symlink ;-)
Ok fair!
>
>> Other than that:
>>
>> Reviewed-by: Konrad Dybcio
>>
>> Perhaps we could take this further and do something like
>>
>> if (failed to load gmu fw)
>> try loading "gmu.bin"
>
> that (loading random fw) sounds like a bad idea.. plus gmu.bin doesn't exist
I guess we still should hear from Qualcomm on this one, but I
do believe that "aXYZ_gmu.bin" is really "gmu firmware, tagged
at aXYZ release". They're even versioned and adreno-gpulist.h
downstream contains the minimum version for each SKU.
Konrad
Re: [PATCH 06/11] drm/bridge: tc358768: Use dev for dbg prints, not priv->dev
On 04/08/2023 13:44, Tomi Valkeinen wrote:
> Simplify the code by capturing the priv->dev value to dev variable, and
> use it.
Reviewed-by: Peter Ujfalusi
>
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/tc358768.c | 41
> ---
> 1 file changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c
> b/drivers/gpu/drm/bridge/tc358768.c
> index 0ef51d04bb21..3266c08d9bf1 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -651,9 +651,10 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
> *bridge)
> const u32 internal_delay = 40;
> int ret, i;
> struct videomode vm;
> + struct device *dev = priv->dev;
>
> if (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
> - dev_warn_once(priv->dev, "Non-continuous mode unimplemented,
> falling back to continuous\n");
> + dev_warn_once(dev, "Non-continuous mode unimplemented, falling
> back to continuous\n");
> mode_flags &= ~MIPI_DSI_CLOCK_NON_CONTINUOUS;
> }
>
> @@ -661,7 +662,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
> *bridge)
>
> ret = tc358768_sw_reset(priv);
> if (ret) {
> - dev_err(priv->dev, "Software reset failed: %d\n", ret);
> + dev_err(dev, "Software reset failed: %d\n", ret);
> tc358768_hw_disable(priv);
> return;
> }
> @@ -669,7 +670,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
> *bridge)
> mode = >encoder->crtc->state->adjusted_mode;
> ret = tc358768_setup_pll(priv, mode);
> if (ret) {
> - dev_err(priv->dev, "PLL setup failed: %d\n", ret);
> + dev_err(dev, "PLL setup failed: %d\n", ret);
> tc358768_hw_disable(priv);
> return;
> }
> @@ -709,7 +710,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
> *bridge)
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
> break;
> default:
> - dev_err(priv->dev, "Invalid data format (%u)\n",
> + dev_err(dev, "Invalid data format (%u)\n",
> dsi_dev->format);
> tc358768_hw_disable(priv);
> return;
> @@ -733,65 +734,65 @@ static void tc358768_bridge_pre_enable(struct
> drm_bridge *bridge)
> dsibclk);
> dsiclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, dsiclk);
> ui_nsk = dsiclk_nsk / 2;
> - dev_dbg(priv->dev, "dsiclk_nsk: %u\n", dsiclk_nsk);
> - dev_dbg(priv->dev, "ui_nsk: %u\n", ui_nsk);
> - dev_dbg(priv->dev, "dsibclk_nsk: %u\n", dsibclk_nsk);
> + dev_dbg(dev, "dsiclk_nsk: %u\n", dsiclk_nsk);
> + dev_dbg(dev, "ui_nsk: %u\n", ui_nsk);
> + dev_dbg(dev, "dsibclk_nsk: %u\n", dsibclk_nsk);
>
> /* LP11 > 100us for D-PHY Rx Init */
> val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1;
> - dev_dbg(priv->dev, "LINEINITCNT: %u\n", val);
> + dev_dbg(dev, "LINEINITCNT: %u\n", val);
> tc358768_write(priv, TC358768_LINEINITCNT, val);
>
> /* LPTimeCnt > 50ns */
> val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1;
> lptxcnt = val;
> - dev_dbg(priv->dev, "LPTXTIMECNT: %u\n", val);
> + dev_dbg(dev, "LPTXTIMECNT: %u\n", val);
> tc358768_write(priv, TC358768_LPTXTIMECNT, val);
>
> /* 38ns < TCLK_PREPARE < 95ns */
> val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1;
> - dev_dbg(priv->dev, "TCLK_PREPARECNT %u\n", val);
> + dev_dbg(dev, "TCLK_PREPARECNT %u\n", val);
> /* TCLK_PREPARE + TCLK_ZERO > 300ns */
> val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk),
> dsibclk_nsk) - 2;
> - dev_dbg(priv->dev, "TCLK_ZEROCNT %u\n", val2);
> + dev_dbg(dev, "TCLK_ZEROCNT %u\n", val2);
> val |= val2 << 8;
> tc358768_write(priv, TC358768_TCLK_HEADERCNT, val);
>
> /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */
> raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk),
> dsibclk_nsk) - 5;
> val = clamp(raw_val, 0, 127);
> - dev_dbg(priv->dev, "TCLK_TRAILCNT: %u\n", val);
> + dev_dbg(dev, "TCLK_TRAILCNT: %u\n", val);
> tc358768_write(priv, TC358768_TCLK_TRAILCNT, val);
>
> /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */
> val = 50 + tc358768_to_ns(4 * ui_nsk);
> val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
> - dev_dbg(priv->dev, "THS_PREPARECNT %u\n", val);
> + dev_dbg(dev, "THS_PREPARECNT %u\n", val);
> /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */
> raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk),
> dsibclk_nsk) - 10;
> val2 = clamp(raw_val, 0, 127);
> - dev_dbg(priv->dev, "THS_ZEROCNT %u\n", val2);
> + dev_dbg(dev, "THS_ZEROCNT %u\n", val2);
> val |= val2 << 8;
> tc358768_write(priv,
Re: [PATCH 05/11] drm/bridge: tc358768: Print logical values, not raw register values
On 04/08/2023 13:44, Tomi Valkeinen wrote: > The driver debug prints DSI related timings as raw register values in > hex. It is much more useful to see the "logical" value of the timing, > not the register value. I'm a bit confused by the term 'logical' value, I think you meant decimal, easier to read by humans numbers. > Change the prints to print the values separately, in case a single > register contains multiple values, and use %u to have it in a more human > consumable form. But, yes, decimal is better for the dmesg, as I recall I had a tool which was using hex numbers so it was better to have the prints also in hex. Reviewed-by: Peter Ujfalusi > > Signed-off-by: Tomi Valkeinen > --- > drivers/gpu/drm/bridge/tc358768.c | 21 - > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358768.c > b/drivers/gpu/drm/bridge/tc358768.c > index 9b633038af33..0ef51d04bb21 100644 > --- a/drivers/gpu/drm/bridge/tc358768.c > +++ b/drivers/gpu/drm/bridge/tc358768.c > @@ -739,57 +739,59 @@ static void tc358768_bridge_pre_enable(struct > drm_bridge *bridge) > > /* LP11 > 100us for D-PHY Rx Init */ > val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1; > - dev_dbg(priv->dev, "LINEINITCNT: 0x%x\n", val); > + dev_dbg(priv->dev, "LINEINITCNT: %u\n", val); > tc358768_write(priv, TC358768_LINEINITCNT, val); > > /* LPTimeCnt > 50ns */ > val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1; > lptxcnt = val; > - dev_dbg(priv->dev, "LPTXTIMECNT: 0x%x\n", val); > + dev_dbg(priv->dev, "LPTXTIMECNT: %u\n", val); > tc358768_write(priv, TC358768_LPTXTIMECNT, val); > > /* 38ns < TCLK_PREPARE < 95ns */ > val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1; > + dev_dbg(priv->dev, "TCLK_PREPARECNT %u\n", val); > /* TCLK_PREPARE + TCLK_ZERO > 300ns */ > val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk), > dsibclk_nsk) - 2; > + dev_dbg(priv->dev, "TCLK_ZEROCNT %u\n", val2); > val |= val2 << 8; > - dev_dbg(priv->dev, "TCLK_HEADERCNT: 0x%x\n", val); > tc358768_write(priv, TC358768_TCLK_HEADERCNT, val); > > /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */ > raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), > dsibclk_nsk) - 5; > val = clamp(raw_val, 0, 127); > - dev_dbg(priv->dev, "TCLK_TRAILCNT: 0x%x\n", val); > + dev_dbg(priv->dev, "TCLK_TRAILCNT: %u\n", val); > tc358768_write(priv, TC358768_TCLK_TRAILCNT, val); > > /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */ > val = 50 + tc358768_to_ns(4 * ui_nsk); > val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1; > + dev_dbg(priv->dev, "THS_PREPARECNT %u\n", val); > /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */ > raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), > dsibclk_nsk) - 10; > val2 = clamp(raw_val, 0, 127); > + dev_dbg(priv->dev, "THS_ZEROCNT %u\n", val2); > val |= val2 << 8; > - dev_dbg(priv->dev, "THS_HEADERCNT: 0x%x\n", val); > tc358768_write(priv, TC358768_THS_HEADERCNT, val); > > /* TWAKEUP > 1ms in lptxcnt steps */ > val = tc358768_ns_to_cnt(102, dsibclk_nsk); > val = val / (lptxcnt + 1) - 1; > - dev_dbg(priv->dev, "TWAKEUP: 0x%x\n", val); > + dev_dbg(priv->dev, "TWAKEUP: %u\n", val); > tc358768_write(priv, TC358768_TWAKEUP, val); > > /* TCLK_POSTCNT > 60ns + 52*UI */ > val = tc358768_ns_to_cnt(60 + tc358768_to_ns(52 * ui_nsk), >dsibclk_nsk) - 3; > - dev_dbg(priv->dev, "TCLK_POSTCNT: 0x%x\n", val); > + dev_dbg(priv->dev, "TCLK_POSTCNT: %u\n", val); > tc358768_write(priv, TC358768_TCLK_POSTCNT, val); > > /* max(60ns + 4*UI, 8*UI) < THS_TRAILCNT < 105ns + 12*UI */ > raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(18 * ui_nsk), >dsibclk_nsk) - 4; > val = clamp(raw_val, 0, 15); > - dev_dbg(priv->dev, "THS_TRAILCNT: 0x%x\n", val); > + dev_dbg(priv->dev, "THS_TRAILCNT: %u\n", val); > tc358768_write(priv, TC358768_THS_TRAILCNT, val); > > val = BIT(0); > @@ -803,10 +805,11 @@ static void tc358768_bridge_pre_enable(struct > drm_bridge *bridge) > /* TXTAGOCNT[26:16] RXTASURECNT[10:0] */ > val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4); > val = tc358768_ns_to_cnt(val, dsibclk_nsk) / 4 - 1; > + dev_dbg(priv->dev, "TXTAGOCNT: %u\n", val); > val2 = tc358768_ns_to_cnt(tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk), > dsibclk_nsk) - 2; > + dev_dbg(priv->dev, "RXTASURECNT: %u\n", val2); > val = val << 16 | val2; > - dev_dbg(priv->dev, "BTACNTRL1: 0x%x\n", val); > tc358768_write(priv, TC358768_BTACNTRL1, val); > > /* START[0] */ > -- Péter
Re: [PATCH 04/11] drm/bridge: tc358768: Use struct videomode
On 04/08/2023 13:44, Tomi Valkeinen wrote:
> The TC358768 documentation uses HFP, HBP, etc. values to deal with the
> video mode, while the driver currently uses the DRM display mode
> (htotal, hsync_start, etc).
>
> Change the driver to convert the DRM display mode to struct videomode,
> which then allows us to use the same units the documentation uses. This
> makes it much easier to work on the code when using the TC358768
> documentation as a reference.
Reviewed-by: Peter Ujfalusi
>
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/tc358768.c | 45
> +--
> 1 file changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c
> b/drivers/gpu/drm/bridge/tc358768.c
> index d5831a1236e9..9b633038af33 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -650,6 +650,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
> *bridge)
> u32 dsiclk, dsibclk, video_start;
> const u32 internal_delay = 40;
> int ret, i;
> + struct videomode vm;
>
> if (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
> dev_warn_once(priv->dev, "Non-continuous mode unimplemented,
> falling back to continuous\n");
> @@ -673,6 +674,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
> *bridge)
> return;
> }
>
> + drm_display_mode_to_videomode(mode, );
> +
> dsiclk = priv->dsiclk;
> dsibclk = dsiclk / 4;
>
> @@ -681,28 +684,28 @@ static void tc358768_bridge_pre_enable(struct
> drm_bridge *bridge)
> switch (dsi_dev->format) {
> case MIPI_DSI_FMT_RGB888:
> val |= (0x3 << 4);
> - hact = mode->hdisplay * 3;
> - video_start = (mode->htotal - mode->hsync_start) * 3;
> + hact = vm.hactive * 3;
> + video_start = (vm.hsync_len + vm.hback_porch) * 3;
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> break;
> case MIPI_DSI_FMT_RGB666:
> val |= (0x4 << 4);
> - hact = mode->hdisplay * 3;
> - video_start = (mode->htotal - mode->hsync_start) * 3;
> + hact = vm.hactive * 3;
> + video_start = (vm.hsync_len + vm.hback_porch) * 3;
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
> break;
>
> case MIPI_DSI_FMT_RGB666_PACKED:
> val |= (0x4 << 4) | BIT(3);
> - hact = mode->hdisplay * 18 / 8;
> - video_start = (mode->htotal - mode->hsync_start) * 18 / 8;
> + hact = vm.hactive * 18 / 8;
> + video_start = (vm.hsync_len + vm.hback_porch) * 18 / 8;
> data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
> break;
>
> case MIPI_DSI_FMT_RGB565:
> val |= (0x5 << 4);
> - hact = mode->hdisplay * 2;
> - video_start = (mode->htotal - mode->hsync_start) * 2;
> + hact = vm.hactive * 2;
> + video_start = (vm.hsync_len + vm.hback_porch) * 2;
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
> break;
> default:
> @@ -814,43 +817,43 @@ static void tc358768_bridge_pre_enable(struct
> drm_bridge *bridge)
> tc358768_write(priv, TC358768_DSI_EVENT, 0);
>
> /* vact */
> - tc358768_write(priv, TC358768_DSI_VACT, mode->vdisplay);
> + tc358768_write(priv, TC358768_DSI_VACT, vm.vactive);
>
> /* vsw */
> - tc358768_write(priv, TC358768_DSI_VSW,
> -mode->vsync_end - mode->vsync_start);
> + tc358768_write(priv, TC358768_DSI_VSW, vm.vsync_len);
> +
> /* vbp */
> - tc358768_write(priv, TC358768_DSI_VBPR,
> -mode->vtotal - mode->vsync_end);
> + tc358768_write(priv, TC358768_DSI_VBPR, vm.vback_porch);
>
> /* hsw * byteclk * ndl / pclk */
> - val = (u32)div_u64((mode->hsync_end - mode->hsync_start) *
> + val = (u32)div_u64(vm.hsync_len *
> ((u64)priv->dsiclk / 4) * priv->dsi_lanes,
> -mode->clock * 1000);
> +vm.pixelclock);
> tc358768_write(priv, TC358768_DSI_HSW, val);
>
> /* hbp * byteclk * ndl / pclk */
> - val = (u32)div_u64((mode->htotal - mode->hsync_end) *
> + val = (u32)div_u64(vm.hback_porch *
> ((u64)priv->dsiclk / 4) * priv->dsi_lanes,
> -mode->clock * 1000);
> +vm.pixelclock);
> tc358768_write(priv, TC358768_DSI_HBPR, val);
> } else {
> /* Set event mode */
> tc358768_write(priv, TC358768_DSI_EVENT, 1);
>
> /* vact */
> -
Re: [PATCH 03/11] drm/bridge: tc358768: Cleanup PLL calculations
On 04/08/2023 13:44, Tomi Valkeinen wrote:
> As is quite common, some of TC358768's PLL register fields are to be
> programmed with (value - 1). Specifically, the FBD and PRD, multiplier
> and divider, are such fields.
>
> However, what the driver currently does is that it considers that the
> formula used for PLL rate calculation is:
>
> RefClk * [(FBD + 1)/ (PRD + 1)] * [1 / (2^FRS)]
>
> where FBD and PRD are values directly from the registers, while a more
> sensible way to look at it is:
>
> RefClk * FBD / PRD * (1 / (2^FRS))
>
> and when the FBD and PRD values are written to the registers, they will
> be subtracted by one.
>
> Change the driver accordingly, as it simplifies the PLL code.
Reviewed-by: Peter Ujfalusi
>
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/tc358768.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c
> b/drivers/gpu/drm/bridge/tc358768.c
> index b668f77673c3..d5831a1236e9 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -316,7 +316,7 @@ static int tc358768_calc_pll(struct tc358768_priv *priv,
>
> target_pll = tc358768_pclk_to_pll(priv, mode->clock * 1000);
>
> - /* pll_clk = RefClk * [(FBD + 1)/ (PRD + 1)] * [1 / (2^FRS)] */
> + /* pll_clk = RefClk * FBD / PRD * (1 / (2^FRS)) */
>
> for (i = 0; i < ARRAY_SIZE(frs_limits); i++)
> if (target_pll >= frs_limits[i])
> @@ -336,19 +336,19 @@ static int tc358768_calc_pll(struct tc358768_priv *priv,
> best_prd = 0;
> best_fbd = 0;
>
> - for (prd = 0; prd < 16; ++prd) {
> - u32 divisor = (prd + 1) * (1 << frs);
> + for (prd = 1; prd <= 16; ++prd) {
> + u32 divisor = prd * (1 << frs);
> u32 fbd;
>
> - for (fbd = 0; fbd < 512; ++fbd) {
> + for (fbd = 1; fbd <= 512; ++fbd) {
> u32 pll, diff, pll_in;
>
> - pll = (u32)div_u64((u64)refclk * (fbd + 1), divisor);
> + pll = (u32)div_u64((u64)refclk * fbd, divisor);
>
> if (pll >= max_pll || pll < min_pll)
> continue;
>
> - pll_in = (u32)div_u64((u64)refclk, prd + 1);
> + pll_in = (u32)div_u64((u64)refclk, prd);
> if (pll_in < 400)
> continue;
>
> @@ -611,7 +611,7 @@ static int tc358768_setup_pll(struct tc358768_priv *priv,
> mode->clock * 1000);
>
> /* PRD[15:12] FBD[8:0] */
> - tc358768_write(priv, TC358768_PLLCTL0, (prd << 12) | fbd);
> + tc358768_write(priv, TC358768_PLLCTL0, ((prd - 1) << 12) | (fbd - 1));
>
> /* FRS[11:10] LBWS[9:8] CKEN[4] RESETB[1] EN[0] */
> tc358768_write(priv, TC358768_PLLCTL1,
>
--
Péter
Re: [PATCH 02/11] drm/bridge: tc358768: Fix bit updates
On 04/08/2023 13:44, Tomi Valkeinen wrote:
> The driver has a few places where it does:
>
> if (thing_is_enabled_in_config)
> update_thing_bit_in_hw()
>
> This means that if the thing is _not_ enabled, the bit never gets
> cleared. This affects the h/vsyncs and continuous DSI clock bits.
I guess the idea was to keep the reset value unless it needs to be flipped.
>
> Fix the driver to always update the bit.
>
> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/tc358768.c | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c
> b/drivers/gpu/drm/bridge/tc358768.c
> index bc97a837955b..b668f77673c3 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -794,8 +794,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge
> *bridge)
> val |= BIT(i + 1);
> tc358768_write(priv, TC358768_HSTXVREGEN, val);
>
> - if (!(mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS))
> - tc358768_write(priv, TC358768_TXOPTIONCNTRL, 0x1);
> + tc358768_write(priv, TC358768_TXOPTIONCNTRL,
> +(mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) ? 0 :
> BIT(0));
>
> /* TXTAGOCNT[26:16] RXTASURECNT[10:0] */
> val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4);
> @@ -861,11 +861,12 @@ static void tc358768_bridge_pre_enable(struct
> drm_bridge *bridge)
> tc358768_write(priv, TC358768_DSI_HACT, hact);
>
> /* VSYNC polarity */
> - if (!(mode->flags & DRM_MODE_FLAG_NVSYNC))
> - tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5), BIT(5));
> + tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5),
> + (mode->flags & DRM_MODE_FLAG_PVSYNC) ? BIT(5) : 0);
Was this the reverse before and should be:
(mode->flags & DRM_MODE_FLAG_PVSYNC) ? 0 : BIT(5)
> +
> /* HSYNC polarity */
> - if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> - tc358768_update_bits(priv, TC358768_PP_MISC, BIT(0), BIT(0));
> + tc358768_update_bits(priv, TC358768_PP_MISC, BIT(0),
> + (mode->flags & DRM_MODE_FLAG_PHSYNC) ? BIT(0) : 0);
>
> /* Start DSI Tx */
> tc358768_write(priv, TC358768_DSI_START, 0x1);
>
--
Péter
Re: [PATCH] drm/msm/a690: Switch to a660_gmu.bin
On Fri, Aug 11, 2023 at 9:11 AM Konrad Dybcio wrote:
>
> On 11.08.2023 18:09, Rob Clark wrote:
> > On Fri, Aug 11, 2023 at 9:05 AM Rob Clark wrote:
> >>
> >> From: Rob Clark
> >>
> >> There isn't actually a a690_gmu.bin. But it appears that the normal
> >> a660_gmu.bin works fine. Normally all the devices within a sub-
> >> generation (or "family") will use the same fw, and a690 is in the a660
> >> family.
> >>
> >
> > possibly this could be considered as:
> >
> > Fixes: 5e7665b5e484 ("drm/msm/adreno: Add Adreno A690 support")
> For a lack of a better response, "meh"
It would help route the change back to 6.4.y so we can stop explaining
to folks that they should create a symlink ;-)
> Other than that:
>
> Reviewed-by: Konrad Dybcio
>
> Perhaps we could take this further and do something like
>
> if (failed to load gmu fw)
> try loading "gmu.bin"
that (loading random fw) sounds like a bad idea.. plus gmu.bin doesn't exist
BR,
-R
>
> Konrad
> >
> >> Signed-off-by: Rob Clark
> >> ---
> >> drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> >> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> >> index 1ed270dae148..756a9cfe1cbf 100644
> >> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> >> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> >> @@ -478,7 +478,7 @@ static const struct adreno_info gpulist[] = {
> >> .family = ADRENO_6XX_GEN4,
> >> .fw = {
> >> [ADRENO_FW_SQE] = "a660_sqe.fw",
> >> - [ADRENO_FW_GMU] = "a690_gmu.bin",
> >> + [ADRENO_FW_GMU] = "a660_gmu.bin",
> >> },
> >> .gmem = SZ_4M,
> >> .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> >> --
> >> 2.41.0
> >>
Re: [PATCH 01/11] drm/bridge: tc358768: Fix use of uninitialized variable
On 04/08/2023 13:44, Tomi Valkeinen wrote:
> smatch reports:
>
> drivers/gpu/drm/bridge/tc358768.c:223 tc358768_update_bits() error:
> uninitialized symbol 'orig'.
>
> Fix this by bailing out from tc358768_update_bits() if the
> tc358768_read() produces an error.
>
> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/bridge/tc358768.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/tc358768.c
> b/drivers/gpu/drm/bridge/tc358768.c
> index 819a4b6ec2a0..bc97a837955b 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -216,6 +216,10 @@ static void tc358768_update_bits(struct tc358768_priv
> *priv, u32 reg, u32 mask,
> u32 tmp, orig;
>
> tc358768_read(priv, reg, );
> +
no need for blank line
> + if (priv->error)
> + return;
> +
> tmp = orig & ~mask;
> tmp |= val & mask;
> if (tmp != orig)
>
--
Péter
Re: [PATCH 1/1] drm/fourcc: Add documentation about software color conversion.
Jocelyn Falempe writes:
Hello Jocelyn,
> On 10/08/2023 09:45, Maxime Ripard wrote:
>> Hi
>>
>> On Mon, Aug 07, 2023 at 03:45:15PM +0200, Jocelyn Falempe wrote:
>>> After discussions on IRC, the consensus is that the DRM drivers should
>>> not do software color conversion, and only advertise the supported formats.
>>> Update the doc accordingly so that the rule and exceptions are clear for
>>> everyone.
>>>
>>> Signed-off-by: Jocelyn Falempe
>>> ---
>>> include/uapi/drm/drm_fourcc.h | 7 +++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>>> index 8db7fd3f743e..00a29152da9f 100644
>>> --- a/include/uapi/drm/drm_fourcc.h
>>> +++ b/include/uapi/drm/drm_fourcc.h
>>> @@ -38,6 +38,13 @@ extern "C" {
>>>* fourcc code, a Format Modifier may optionally be provided, in order to
>>>* further describe the buffer's format - for example tiling or
>>> compression.
>>>*
>>> + * DRM drivers should not do software color conversion, and only advertise
>>> the
>>> + * format they support in hardware. But there are two exceptions:
>>
>> I would do a bullet list here:
>> https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#lists-and-quote-like-blocks
>>
> ok, that would look better.
>
>>> + * The first is to support XRGB if the hardware doesn't support it,
>>> because
>>> + * it's the de facto standard for userspace applications.
>>
>> We can also provide a bit more context here, something like:
>>
>> All drivers must support XRGB, even if the hardware cannot support
>> it. This has become the de-facto standard and a lot of user-space assume
>> it will be present.
>
> ok, I can add this before the first paragraph.
>>
Agreed with Maxime's suggestion but I would also mention that if XRGB
is not supported, the native formats should be the default and XRGB be
only used as a fallback for user-space compatiblity. In other words, that
XRGB must not be the default, e.g: mode_config.preferred_depth = 24 or
drm_fbdev_generic_setup(drm, 32) only if is a supported native format.
I agree with the general content of the patch, so if you post a v2 feel
free to add my
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH] drm/msm/a690: Switch to a660_gmu.bin
On 11.08.2023 18:09, Rob Clark wrote:
> On Fri, Aug 11, 2023 at 9:05 AM Rob Clark wrote:
>>
>> From: Rob Clark
>>
>> There isn't actually a a690_gmu.bin. But it appears that the normal
>> a660_gmu.bin works fine. Normally all the devices within a sub-
>> generation (or "family") will use the same fw, and a690 is in the a660
>> family.
>>
>
> possibly this could be considered as:
>
> Fixes: 5e7665b5e484 ("drm/msm/adreno: Add Adreno A690 support")
For a lack of a better response, "meh"
Other than that:
Reviewed-by: Konrad Dybcio
Perhaps we could take this further and do something like
if (failed to load gmu fw)
try loading "gmu.bin"
Konrad
>
>> Signed-off-by: Rob Clark
>> ---
>> drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> index 1ed270dae148..756a9cfe1cbf 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> @@ -478,7 +478,7 @@ static const struct adreno_info gpulist[] = {
>> .family = ADRENO_6XX_GEN4,
>> .fw = {
>> [ADRENO_FW_SQE] = "a660_sqe.fw",
>> - [ADRENO_FW_GMU] = "a690_gmu.bin",
>> + [ADRENO_FW_GMU] = "a660_gmu.bin",
>> },
>> .gmem = SZ_4M,
>> .inactive_period = DRM_MSM_INACTIVE_PERIOD,
>> --
>> 2.41.0
>>
Re: [PATCH v3] drm/msm/a2xx: Call adreno_gpu_init() earlier
Hi Rob,
Any comments, please?
On Mon, Jul 24, 2023 at 5:28 PM Fabio Estevam wrote:
>
> Hi Rob,
>
> A gentle ping.
>
> On Thu, Jun 22, 2023 at 3:37 PM Dmitry Baryshkov
> wrote:
> >
> > On 21/06/2023 02:23, Fabio Estevam wrote:
> > > From: Fabio Estevam
> > >
> > > The adreno_is_a20x() and adreno_is_a225() functions rely on the
> > > GPU revision, but such information is retrieved inside adreno_gpu_init(),
> > > which is called afterwards.
> > >
> > > Fix this problem by caling adreno_gpu_init() earlier, so that
> > > the GPU information revision is available when adreno_is_a20x()
> > > and adreno_is_a225() run.
> > >
> > > Tested on a imx53-qsb board.
> > >
> > > Fixes: 21af872cd8c6 ("drm/msm/adreno: add a2xx")
> > > Signed-off-by: Fabio Estevam
> > > ---
> > > Hi,
> > >
> > > This error has been exposed by a recent commit in linux-next:
> > >
> > > cc943f43ece7 ("drm/msm/adreno: warn if chip revn is verified before being
> > > set")
> > >
> > > , but the error has been present since the very first a2xx commit.
> > >
> > > Changes since v2:
> > > - Call adreno_gpu_init() earlier.
> >
> > Reviewed-by: Dmitry Baryshkov
> >
> >
> > >
> > > drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 8
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> >
> > --
> > With best wishes
> > Dmitry
> >
Re: [PATCH] drm/msm/a690: Switch to a660_gmu.bin
On Fri, Aug 11, 2023 at 9:05 AM Rob Clark wrote:
>
> From: Rob Clark
>
> There isn't actually a a690_gmu.bin. But it appears that the normal
> a660_gmu.bin works fine. Normally all the devices within a sub-
> generation (or "family") will use the same fw, and a690 is in the a660
> family.
>
possibly this could be considered as:
Fixes: 5e7665b5e484 ("drm/msm/adreno: Add Adreno A690 support")
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 1ed270dae148..756a9cfe1cbf 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -478,7 +478,7 @@ static const struct adreno_info gpulist[] = {
> .family = ADRENO_6XX_GEN4,
> .fw = {
> [ADRENO_FW_SQE] = "a660_sqe.fw",
> - [ADRENO_FW_GMU] = "a690_gmu.bin",
> + [ADRENO_FW_GMU] = "a660_gmu.bin",
> },
> .gmem = SZ_4M,
> .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> --
> 2.41.0
>
Re: [PATCH v2 15/15] drm/panthor: Add an entry to MAINTAINERS
On 09/08/2023 17:53, Boris Brezillon wrote: > Add an entry for the Panthor driver to the MAINTAINERS file. > > v2: > - New commit > > Signed-off-by: Boris Brezillon > --- > > If anyone from Arm wants to volunteer to become a co-maintainer, that > would be highly appreciated *sticks his hand up* me me! ;) Seriously though I'm happy to help out with the maintenance. And I'll try to finish reviewing the patches next week. I gave it a quick spin on my Rock 5B and the GPU seems to work fine. I also need to rebase my user space submission work. And recover from coming back from holiday! Plus I'm sure I wasn't full-time on GPU related things before I went on holiday... ;) Thanks, Steve > --- > MAINTAINERS | 8 > 1 file changed, 8 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index cd882b87a3c6..6149ab68d461 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1624,6 +1624,14 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > F: drivers/gpu/drm/panfrost/ > F: include/uapi/drm/panfrost_drm.h > > +ARM MALI PANTHOR DRM DRIVER > +M: Boris Brezillon > +L: dri-devel@lists.freedesktop.org > +S: Supported > +T: git git://anongit.freedesktop.org/drm/drm-misc > +F: drivers/gpu/drm/panthor/ > +F: include/uapi/drm/panthor_drm.h > + > ARM MALI-DP DRM DRIVER > M: Liviu Dudau > S: Supported
[PATCH] drm/msm/a690: Switch to a660_gmu.bin
From: Rob Clark
There isn't actually a a690_gmu.bin. But it appears that the normal
a660_gmu.bin works fine. Normally all the devices within a sub-
generation (or "family") will use the same fw, and a690 is in the a660
family.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 1ed270dae148..756a9cfe1cbf 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -478,7 +478,7 @@ static const struct adreno_info gpulist[] = {
.family = ADRENO_6XX_GEN4,
.fw = {
[ADRENO_FW_SQE] = "a660_sqe.fw",
- [ADRENO_FW_GMU] = "a690_gmu.bin",
+ [ADRENO_FW_GMU] = "a660_gmu.bin",
},
.gmem = SZ_4M,
.inactive_period = DRM_MSM_INACTIVE_PERIOD,
--
2.41.0
Re: [PATCH 1/6] dt-bindings: trivial-devices: Remove national,lm75
On Thu, 10 Aug 2023 16:44:46 +0200, Alexander Stein wrote:
> Starting with commit 3e37c9d48f7a ("dt-bindings: hwmon: Convert lm75
> bindings to yaml") 'national,lm75' has it's own dedicated (YAML) binding.
> If kept in this file device specific properties as 'vs-supply' are
> considered excessive. Remove compatible here so it can be checked with
> more specific binding.
> arch/arm/boot/dts/nxp/imx/imx6q-mba6a.dtb: sensor@48: 'vs-supply' does not
> match any of the regexes: 'pinctrl-[0-9]+'
> From schema: Documentation/devicetree/bindings/trivial-devices.yaml
>
> Signed-off-by: Alexander Stein
> ---
> Documentation/devicetree/bindings/trivial-devices.yaml | 2 --
> 1 file changed, 2 deletions(-)
>
Applied, thanks!
Re: [PATCH v2 04/15] drm/panthor: Add the device logical block
On 09/08/2023 17:53, Boris Brezillon wrote:
> The panthor driver is designed in a modular way, where each logical
> block is dealing with a specific HW-block or software feature. In order
> for those blocks to communicate with each other, we need a central
> panthor_device collecting all the blocks, and exposing some common
> features, like interrupt handling, power management, reset, ...
>
> This what this panthor_device logical block is about.
>
> v2:
> - Rename the driver (pancsf -> panthor)
> - Change the license (GPL2 -> MIT + GPL2)
> - Split the driver addition commit
> - Add devfreq/PM support
> - Use drm_dev_{unplug,enter,exit}() to provide safe device removal
>
> Signed-off-by: Boris Brezillon
> ---
> drivers/gpu/drm/panthor/panthor_device.c | 479 +++
> drivers/gpu/drm/panthor/panthor_device.h | 354 +
> 2 files changed, 833 insertions(+)
> create mode 100644 drivers/gpu/drm/panthor/panthor_device.c
> create mode 100644 drivers/gpu/drm/panthor/panthor_device.h
>
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c
> b/drivers/gpu/drm/panthor/panthor_device.c
> new file mode 100644
> index ..15f102116fa0
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -0,0 +1,479 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +/* Copyright 2018 Marty E. Plummer */
> +/* Copyright 2019 Linaro, Ltd, Rob Herring */
> +/* Copyright 2023 Collabora ltd. */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include "panthor_sched.h"
> +#include "panthor_device.h"
> +#include "panthor_devfreq.h"
> +#include "panthor_gpu.h"
> +#include "panthor_fw.h"
> +#include "panthor_mmu.h"
> +#include "panthor_regs.h"
> +
> +static int panthor_clk_init(struct panthor_device *ptdev)
> +{
> + ptdev->clks.core = devm_clk_get(ptdev->base.dev, NULL);
> + if (IS_ERR(ptdev->clks.core)) {
> + drm_err(>base, "get 'core' clock failed %ld\n",
> + PTR_ERR(ptdev->clks.core));
I suspect it would be a good idea to use dev_err_probe() here (and
below) as I believe devm_clk_get can return -EPROBE_DEFER.
> + return PTR_ERR(ptdev->clks.core);
> + }
> +
> + ptdev->clks.stacks = devm_clk_get_optional(ptdev->base.dev, "stacks");
> + if (IS_ERR(ptdev->clks.stacks)) {
> + drm_err(>base, "get 'stacks' clock failed %ld\n",
> + PTR_ERR(ptdev->clks.stacks));
> + return PTR_ERR(ptdev->clks.stacks);
> + }
> +
> + ptdev->clks.coregroup = devm_clk_get_optional(ptdev->base.dev,
> "coregroup");
> + if (IS_ERR(ptdev->clks.coregroup)) {
> + drm_err(>base, "get 'coregroup' clock failed %ld\n",
> + PTR_ERR(ptdev->clks.coregroup));
> + return PTR_ERR(ptdev->clks.coregroup);
> + }
> +
> + drm_info(>base, "clock rate = %lu\n",
> clk_get_rate(ptdev->clks.core));
> + return 0;
> +}
> +
> +void panthor_device_unplug(struct panthor_device *ptdev)
> +{
> + /* FIXME: This is racy. */
Can we fix this? From a quick look it seems like a sequence like below
should avoid the race.
if (!drm_dev_enter())
/* Already unplugged */
return;
ptdev->base.unplugged = true;
drm_dev_exit();
Although possibly that should be in the DRM core rather than open-coded
here.
> + if (drm_dev_is_unplugged(>base))
> + return;
> +
> + drm_WARN_ON(>base, pm_runtime_get_sync(ptdev->base.dev) < 0);
> +
> + /* Call drm_dev_unplug() so any access to HW block happening after
> + * that point get rejected.
> + */
> + drm_dev_unplug(>base);
> +
> + /* Now, try to cleanly shutdown the GPU before the device resources
> + * get reclaimed.
> + */
> + panthor_sched_unplug(ptdev);
> + panthor_fw_unplug(ptdev);
> + panthor_mmu_unplug(ptdev);
> + panthor_gpu_unplug(ptdev);
> +
> + pm_runtime_dont_use_autosuspend(ptdev->base.dev);
> + pm_runtime_put_sync_suspend(ptdev->base.dev);
> +}
> +
> +static void panthor_device_reset_cleanup(struct drm_device *ddev, void *data)
> +{
> + struct panthor_device *ptdev = container_of(ddev, struct
> panthor_device, base);
> +
> + cancel_work_sync(>reset.work);
> + destroy_workqueue(ptdev->reset.wq);
> +}
> +
> +static void panthor_device_reset_work(struct work_struct *work)
> +{
> + struct panthor_device *ptdev = container_of(work, struct
> panthor_device, reset.work);
> + int ret, cookie;
> +
> + if (!drm_dev_enter(>base, ))
> + return;
> +
> + panthor_sched_pre_reset(ptdev);
> + panthor_fw_pre_reset(ptdev, true);
> + panthor_mmu_pre_reset(ptdev);
> + panthor_gpu_soft_reset(ptdev);
> + panthor_gpu_l2_power_on(ptdev);
> + panthor_mmu_post_reset(ptdev);
> + ret = panthor_fw_post_reset(ptdev);
> + if (ret)
> + goto out;
> +
> +
Re: [PATCH V2 1/2] dt-bindings: display: newvision,nv3051d: Add Anbernic 351V Support
On Thu, Aug 10, 2023 at 05:24:09PM -0600, Rob Herring wrote: > On Wed, Aug 09, 2023 at 10:39:40AM -0500, Chris Morgan wrote: > > From: Chris Morgan > > > > Document the Anbernic RG351V panel, which appears to be identical to > > the panel used in their 353 series except for in inclusion of an > > additional DSI format flag. > > > > Signed-off-by: Chris Morgan > > --- > > .../display/panel/newvision,nv3051d.yaml | 18 ++ > > 1 file changed, 10 insertions(+), 8 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > > b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > > index 116c1b6030a2..576f3640cb33 100644 > > --- a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > > +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > > @@ -7,9 +7,7 @@ $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. > > + The NewVision NV3051D is a driver chip used to drive DSI panels. > > > > maintainers: > >- Chris Morgan > > @@ -19,11 +17,15 @@ allOf: > > > > properties: > >compatible: > > -items: > > - - enum: > > - - anbernic,rg353p-panel > > - - anbernic,rg353v-panel > > - - const: newvision,nv3051d > > +oneOf: > > + - items: > > + - enum: > > + - anbernic,rg353p-panel > > + - anbernic,rg353v-panel > > + - const: newvision,nv3051d > > + > > + - items: > > + - const: anbernic,rg351v-panel > > I don't understand. Is this panel not based on newvision,nv3051d? If > not, then it probably should be a different binding. Lot's of panel > bindings have similar properties. It appears to be the same panel (or extremely similar, honestly I don't know because there are no external markings on it). However, this specific implementation seems to require MIPI_DSI_CLOCK_NON_CONTINUOUS, not using it prevents the panel from working. As for the existing panel MIPI_DSI_CLOCK_NON_CONTINUOUS stops it from working. The different binding essentially determines whether or not that flag is present, but otherwise everything else is identical. Chris > > Rob
Re: [PATCH drm-misc-next] drm/ttm/tests: Require MMU when testing
Am 11.08.23 um 13:36 schrieb Karolina Stolarek: Satisfy MMU dependency when testing TTM with KUnit. This fixes compilation errors on platforms that don't select this option by default. Signed-off-by: Karolina Stolarek Cc: Christian König Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202308110133.f0lhfwmv-...@intel.com/ Closes: https://lore.kernel.org/oe-kbuild-all/202308111032.enu8iisr-...@intel.com/ Reviewed and pushed to drm-misc-next. --- drivers/gpu/drm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 9d1f0e04fd56..ab9ef1c20349 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -198,7 +198,7 @@ config DRM_TTM config DRM_TTM_KUNIT_TEST tristate "KUnit tests for TTM" if !KUNIT_ALL_TESTS default n -depends on DRM && KUNIT +depends on DRM && KUNIT && MMU select DRM_TTM select DRM_EXPORT_FOR_TESTS if m select DRM_KUNIT_TEST_HELPERS
Re: [PATCH v2 02/15] drm/panthor: Add uAPI
On 09/08/2023 17:53, Boris Brezillon wrote:
> Panthor follows the lead of other recently submitted drivers with
> ioctls allowing us to support modern Vulkan features, like sparse memory
> binding:
>
> - Pretty standard GEM management ioctls (BO_CREATE and BO_MMAP_OFFSET),
> with the 'exclusive-VM' bit to speed-up BO reservation on job submission
> - VM management ioctls (VM_CREATE, VM_DESTROY and VM_BIND). The VM_BIND
> ioctl is loosely based on the Xe model, and can handle both
> asynchronous and synchronous requests
> - GPU execution context creation/destruction, tiler heap context creation
> and job submission. Those ioctls reflect how the hardware/scheduler
> works and are thus driver specific.
>
> We also have a way to expose IO regions, such that the usermode driver
> can directly access specific/well-isolate registers, like the
> LATEST_FLUSH register used to implement cache-flush reduction.
>
> This uAPI intentionally keeps usermode queues out of the scope, which
> explains why doorbell registers and command stream ring-buffers are not
> directly exposed to userspace.
>
> v2:
> - Rename the driver (pancsf -> panthor)
> - Change the license (GPL2 -> MIT + GPL2)
> - Split the driver addition commit
> - Turn the VM_{MAP,UNMAP} ioctls into a VM_BIND ioctl
> - Add the concept of exclusive_vm at BO creation time
> - Add missing padding fields
> - Add documentation
>
> Signed-off-by: Boris Brezillon
Looks good, just documentation typos/corrections below. With those fixed
Reviewed-by: Steven Price
> ---
> Documentation/gpu/driver-uapi.rst | 5 +
> include/uapi/drm/panthor_drm.h| 862 ++
> 2 files changed, 867 insertions(+)
> create mode 100644 include/uapi/drm/panthor_drm.h
>
> diff --git a/Documentation/gpu/driver-uapi.rst
> b/Documentation/gpu/driver-uapi.rst
> index c08bcbb95fb3..7a667901830f 100644
> --- a/Documentation/gpu/driver-uapi.rst
> +++ b/Documentation/gpu/driver-uapi.rst
> @@ -17,3 +17,8 @@ VM_BIND / EXEC uAPI
> :doc: Overview
>
> .. kernel-doc:: include/uapi/drm/nouveau_drm.h
> +
> +drm/panthor uAPI
> +
> +
> +.. kernel-doc:: include/uapi/drm/panthor_drm.h
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> new file mode 100644
> index ..e217eb5ad198
> --- /dev/null
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -0,0 +1,862 @@
> +/* SPDX-License-Identifier: MIT */
> +/* Copyright (C) 2023 Collabora ltd. */
> +#ifndef _PANTHOR_DRM_H_
> +#define _PANTHOR_DRM_H_
> +
> +#include "drm.h"
> +
> +#if defined(__cplusplus)
> +extern "C" {
> +#endif
> +
> +/**
> + * DOC: Introduction
> + *
> + * This documentation decribes the Panthor IOCTLs.
describes
> + *
> + * Just a few generic rules about the data passed to the Panthor IOCTLs:
> + *
> + * - Structures must be aligned on 64-bit/8-byte. If the object is not
> + * naturally aligned, a padding field must be added.
> + * - Fields must be explicity aligned to their natural type alignment with
^ explicitly
> + * pad[0..N] fields.
> + * - All padding fields will be checked by the driver to make sure they are
> + * zeroed.
> + * - Flags can be added, but not removed/replaced.
> + * - New fields can be added to the main structures (the structures
> + * directly passed to the ioctl). Those fiels can be added at the end of
^ fields
> + * the structure, or replace existing padding fields. Any new field being
> + * added must preserve the behavior that existed before those fields were
> + * added when a value of zero is passed.
> + * - New fields can be added to indirect objects (objects pointed by the
> + * main structure), iff those objects are passed a size to reflect the
> + * size known by the userspace driver (see drm_panthor_obj_array::stride
> + * or drm_panthor_dev_query::size).
> + * - If the kernel driver is too old to know some fields, those will
> + * be ignored (input) and set back to zero (output).
I presume this should be "will be ignored if zero (input)" and rejected
if non-zero?
> + * - If userspace is too old to know some fields, those will be zeroed
> + * (input) before the structure is parsed by the kernel driver.
> + * - Each new flag/field addition must come with a driver version update so
> + * the userspace driver doesn't have to trial and error to know which
> + * flags are supported.
> + * - Structures should not contain unions, as this would defeat the
> + * extensibility of such structures.
> + * - IOCTLs can't be removed or replaced. New IOCTL IDs should be placed
> + * at the end of the drm_panthor_ioctl_id enum.
> + */
> +
> +/**
> + * DOC: MMIO regions exposed to userspace.
> + *
> + * .. c:macro:: DRM_PANTHOR_USER_MMIO_OFFSET
> + *
> + * File offset for all MMIO regions being exposed to userspace. Don't use
> + * this value directly, use
Re: [PATCH v2 03/15] drm/panthor: Add GPU register definitions
On 09/08/2023 17:53, Boris Brezillon wrote: > Those are the registers directly accessible through the MMIO range. > > FW registers are exposed in panthor_fw.h. > > v2: > - Rename the driver (pancsf -> panthor) > - Change the license (GPL2 -> MIT + GPL2) > - Split the driver addition commit > > Signed-off-by: Boris Brezillon Two possible redundant defines (see below), but otherwise: Reviewed-by: Steven Price > --- > drivers/gpu/drm/panthor/panthor_regs.h | 229 + > 1 file changed, 229 insertions(+) > create mode 100644 drivers/gpu/drm/panthor/panthor_regs.h > > diff --git a/drivers/gpu/drm/panthor/panthor_regs.h > b/drivers/gpu/drm/panthor/panthor_regs.h > new file mode 100644 > index ..00e149cf9eab > --- /dev/null > +++ b/drivers/gpu/drm/panthor/panthor_regs.h > @@ -0,0 +1,229 @@ > +/* SPDX-License-Identifier: GPL-2.0 or MIT */ > +/* Copyright 2018 Marty E. Plummer */ > +/* Copyright 2019 Linaro, Ltd, Rob Herring */ > +/* Copyright 2023 Collabora ltd. */ > +/* > + * Register definitions based on mali_kbase_gpu_regmap.h and > + * mali_kbase_gpu_regmap_csf.h > + * (C) COPYRIGHT 2010-2022 ARM Limited. All rights reserved. > + */ > +#ifndef __PANTHOR_REGS_H__ > +#define __PANTHOR_REGS_H__ > + > +#define GPU_ID 0x00 > +#define GPU_L2_FEATURES 0x004 > +#define GPU_TILER_FEATURES 0x00C > +#define GPU_MEM_FEATURES 0x010 > +#define GROUPS_L2_COHERENT BIT(0) > + > +#define GPU_MMU_FEATURES 0x014 > +#define GPU_MMU_FEATURES_VA_BITS(x) ((x) & GENMASK(7, 0)) > +#define GPU_MMU_FEATURES_PA_BITS(x) (((x) >> 8) & > GENMASK(7, 0)) > +#define GPU_AS_PRESENT 0x018 > +#define GPU_CSF_ID 0x01C > + > +#define GPU_INT_RAWSTAT 0x20 > +#define GPU_INT_CLEAR0x24 > +#define GPU_INT_MASK 0x28 > +#define GPU_INT_STAT 0x2c > +#define GPU_IRQ_FAULT BIT(0) > +#define GPU_IRQ_PROTM_FAULTBIT(1) > +#define GPU_IRQ_RESET_COMPLETEDBIT(8) > +#define GPU_IRQ_POWER_CHANGED BIT(9) > +#define GPU_IRQ_POWER_CHANGED_ALL BIT(10) > +#define GPU_IRQ_CLEAN_CACHES_COMPLETED BIT(17) > +#define GPU_IRQ_DOORBELL_MIRRORBIT(18) > +#define GPU_IRQ_MCU_STATUS_CHANGED BIT(19) > +#define GPU_CMD 0x30 > +#define GPU_CMD_DEF(type, payload) ((type) | ((payload) << > 8)) > +#define GPU_SOFT_RESET GPU_CMD_DEF(1, 1) > +#define GPU_HARD_RESET GPU_CMD_DEF(1, 2) > +#define CACHE_CLEANBIT(0) > +#define CACHE_INV BIT(1) > +#define GPU_FLUSH_CACHES(l2, lsc, oth) \ > + GPU_CMD_DEF(4, ((l2) << 0) | ((lsc) << 4) | ((oth) << 8)) > + > +#define GPU_STATUS 0x34 > +#define GPU_STATUS_ACTIVE BIT(0) > +#define GPU_STATUS_PWR_ACTIVE BIT(1) > +#define GPU_STATUS_PAGE_FAULT BIT(4) > +#define GPU_STATUS_PROTM_ACTIVEBIT(7) > +#define GPU_STATUS_DBG_ENABLED BIT(8) > + > +#define GPU_FAULT_STATUS 0x3C > +#define GPU_FAULT_ADDR_LO0x40 > +#define GPU_FAULT_ADDR_HI0x44 > + > +#define GPU_PWR_KEY 0x50 > +#define GPU_PWR_KEY_UNLOCK 0x2968A819 > +#define GPU_PWR_OVERRIDE00x54 > +#define GPU_PWR_OVERRIDE10x58 > + > +#define GPU_TIMESTAMP_OFFSET_LO 0x88 > +#define GPU_TIMESTAMP_OFFSET_HI 0x8C > +#define GPU_CYCLE_COUNT_LO 0x90 > +#define GPU_CYCLE_COUNT_HI 0x94 > +#define GPU_TIMESTAMP_LO 0x98 > +#define GPU_TIMESTAMP_HI 0x9C > + > +#define GPU_THREAD_MAX_THREADS 0xA0 > +#define GPU_THREAD_MAX_WORKGROUP_SIZE0xA4 > +#define GPU_THREAD_MAX_BARRIER_SIZE 0xA8 > +#define GPU_THREAD_FEATURES 0xAC > + > +#define GPU_TEXTURE_FEATURES(n) (0xB0 + ((n) * > 4)) > + > +#define GPU_SHADER_PRESENT_LO0x100 > +#define GPU_SHADER_PRESENT_HI
Re: [PATCH 2/6] dt-bindings: imx-thermal: Add #thermal-sensor-cells property
On Thu, 10 Aug 2023 16:44:47 +0200, Alexander Stein wrote: > This property is defined in thermal-sensor.yaml. Reference this file and > constraint '#thermal-sensor-cells' to 0 for imx-thermal. > Fixes the warning: > arch/arm/boot/dts/nxp/imx/imx6q-mba6a.dtb: tempmon: > '#thermal-sensor-cells' does not match any of the regexes: 'pinctrl-[0-9]+' > From schema: Documentation/devicetree/bindings/thermal/imx-thermal.yaml > > Signed-off-by: Alexander Stein > --- > Documentation/devicetree/bindings/thermal/imx-thermal.yaml | 6 ++ > 1 file changed, 6 insertions(+) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/imx-thermal.example.dtb: tempmon: '#thermal-sensor-cells' is a required property from schema $id: http://devicetree.org/schemas/thermal/imx-thermal.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230810144451.1459985-3-alexander.st...@ew.tq-group.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [Intel-xe] [RFC PATCH] Documentation/gpu: Draft VM_BIND locking document
Thanks for reviewing, Rodrigo! On 8/4/23 22:15, Rodrigo Vivi wrote: On Fri, Jun 30, 2023 at 06:44:52PM +0200, Thomas Hellström wrote: Add the first version of the VM_BIND locking document which is intended to be part of the xe driver upstreaming agreement. The document describes and discuss the locking used during exec- functions, evicton and for userptr gmvas. Intention is to be using the same nomenclature as the drm-vm-bind-async.rst, but to keep naming a little shorter, use gvm and gmva instead of gpu_vm and gpu_vma which is used in the previous document, with an intention to modify also that document. I preferred the gpu_vm and gpu_vma as written in the async doc. Much easier to read imho. OK. I'll keep that naming then. Signed-off-by: Thomas Hellström --- Documentation/gpu/drm-vm-bind-locking.rst | 339 ++ 1 file changed, 339 insertions(+) create mode 100644 Documentation/gpu/drm-vm-bind-locking.rst diff --git a/Documentation/gpu/drm-vm-bind-locking.rst b/Documentation/gpu/drm-vm-bind-locking.rst new file mode 100644 index ..f5d1a40a2906 --- /dev/null +++ b/Documentation/gpu/drm-vm-bind-locking.rst @@ -0,0 +1,339 @@ +=== +VM_BIND lockinghttps://jira.devtools.intel.com/browse/VLK-50299a +=== + +This document attempts to describe what's needed to get VM_BIND locking right, +including the userptr mmu_notifier locking and it will also discuss some +optimizations to get rid of the looping through of all userptr mappings and +external / shared object mappings that is needed in the simplest +implementation. It will also discuss some implications for faulting gvms. + +Nomenclature + + +* ``Context``: GPU execution context. +* ``gvm``: Abstraction of a GPU address space with meta-data. Typically + one per client (DRM file-private), or one per context. +* ``gvma``: Abstraction of a GPU address range within a gvma with within a gpu_vm you meant? Yes. + associated meta-data. The backing storage of a gvma can either be + a gem buffer object or anonymous pages mapped also into the CPU + address space for the process. +* ``userptr gvma or just userptr``: A gvma, the backing store of + which is anonymous pages as described above. +* ``revalidating``: Revalidating a gvma means making the latest version + of the backing store resident and making sure the gvma's + page-table entries point to that backing store. +* ``dma_fence``: A struct dma_fence that is similar to a struct completion + and which tracks GPU activity. When the GPU activity is finished, + the dma_fence signals. +* ``dma_resv``: A struct dma_resv (AKA reservation object) that is used + to track GPU activity in the form of multiple dma_fences on a + gvm or a gem buffer object. The dma_resv contains an array / list + of dma_fences and a lock that needs to be held when adding + additional dma_fences to the dma_resv. The lock is of a type that + allows deadlock-safe locking of multiple dma_resvs in arbitrary order. +* ``exec function``: An exec function is a function that revalidates all + affected gvmas, submits a GPU command batch and registers the + dma_fence representing the GPU command's activity with all affected + dma_resvs. For completeness, although not covered by this document, + it's worth mentioning that an exec function may also be the + revalidation worker that is used by some drivers in compute / + long-running mode. +* ``local object``: A GEM object which is local to a gvm. Shared gem + objects also share the gvm's dma_resv. +* ``shared object``: AKA external object: A GEM object which may be shared + by multiple gvms and whose backing storage may be shared with + other drivers. + + +Introducing the locks += + +One of the benefits of VM_BIND is that local GEM objects share the gvm's +dma_resv object and hence the dma_resv lock. So even with a huge +number of local GEM objects, only one lock is needed to make the exec +sequence atomic. + +The following locks and locking orders are used: + +* The ``gvm->lock`` (optionally an rwsem). Protects how the gvm is + partitioned into gvmas, protects the gvm's list of external objects, + and can also with some simplification protect the gvm's list of + userptr gvmas. With the CPU mm analogy this would correspond to the + mmap_lock. +* The ``userptr_seqlock``. This lock is taken in read mode for each + userptr gvma on the gvm's userptr list, and in write mode during mmu + notifier invalidation. is this something that exists withing the mmu_notifier or a new lock when handling the notifier? No it's the MMU interval notifier functionality that acts like a seqlock: The read side waits until there are no writers (invalidators) until entering a critical section and reruns the read-side critical section if there was a writer sneaking in before leaving the read-side critical section. I'll add a pointer to that documentation. +* The ``gvm->resv`` lock.
[PATCH] backlight: lp855x: fix unintialized function return
From: Arnd Bergmann
The function now returns an error code in some cases, but fails to initialize
it in others:
drivers/video/backlight/lp855x_bl.c:252:11: error: variable 'ret' is used
uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
else if (lp->mode == REGISTER_BASED)
^~
drivers/video/backlight/lp855x_bl.c:256:9: note: uninitialized use occurs here
return ret;
^~~
drivers/video/backlight/lp855x_bl.c:252:7: note: remove the 'if' if its
condition is always true
else if (lp->mode == REGISTER_BASED)
^~~
Since this case should not actually happen, return the -EINVAL code.
Fixes: 5145531be5fba ("backlight: lp855x: Catch errors when changing
brightness")
Signed-off-by: Arnd Bergmann
---
drivers/video/backlight/lp855x_bl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/backlight/lp855x_bl.c
b/drivers/video/backlight/lp855x_bl.c
index 61a7f45bfad84..ea4fa69e49a70 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -252,6 +252,8 @@ static int lp855x_bl_update_status(struct backlight_device
*bl)
else if (lp->mode == REGISTER_BASED)
ret = lp855x_write_byte(lp, lp->cfg->reg_brightness,
(u8)brightness);
+ else
+ ret = -EINVAL;
return ret;
}
--
2.39.2
Re: [PATCH 1/2] drm/bridge: adv7511: fix Wvoid-pointer-to-enum-cast warning
On Thu, Aug 10, 2023 at 12:12 PM Laurent Pinchart wrote: > > Hi Krzysztof, > > Thank you for the patch. > > On Thu, Aug 10, 2023 at 11:59:22AM +0200, Krzysztof Kozlowski wrote: > > 'type' is an enum, thus cast of pointer on 64-bit compile test with W=1 > > causes: > > > > adv7511_drv.c:1214:19: error: cast to smaller integer type 'enum > > adv7511_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast] > > > > Signed-off-by: Krzysztof Kozlowski > > Reviewed-by: Laurent Pinchart > > > --- > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > index 2611afd2c1c1..0e284f379602 100644 > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > @@ -1211,7 +1211,7 @@ static int adv7511_probe(struct i2c_client *i2c) > > adv7511->status = connector_status_disconnected; > > > > if (dev->of_node) > > - adv7511->type = (enum > > adv7511_type)of_device_get_match_data(dev); > > + adv7511->type = (uintptr_t)of_device_get_match_data(dev); > > else > > adv7511->type = id->driver_data; > > > > -- > Regards, > > Laurent Pinchart > Ack for drm/bridge part. Acked-by: Robert Foss
Re: [PATCH 1/1] drm/bridge: lvds-codec: Implement atomic_get_input_bus_fmts for LVDS encoder
On Thu, 10 Aug 2023 08:32:29 +0200, Alexander Stein wrote: > atomic_get_input_bus_fmts is only implemented for LVDS decoders, resulting > that LVDS encoders only support bus format MEDIA_BUS_FMT_FIXED. This > results in warnings like: > mxsfb 21c8000.lcdif: Bridge does not provide bus format, assuming > MEDIA_BUS_FMT_RGB888_1X24. > Please fix bridge driver by handling atomic_get_input_bus_fmts. > > [...] Applied, thanks! [1/1] drm/bridge: lvds-codec: Implement atomic_get_input_bus_fmts for LVDS encoder https://cgit.freedesktop.org/drm/drm-misc/commit/?id=b31f7849 Rob
Re: [PATCH 1/1] drm/bridge: lvds-codec: Implement atomic_get_input_bus_fmts for LVDS encoder
On Thu, Aug 10, 2023 at 8:32 AM Alexander Stein
wrote:
>
> atomic_get_input_bus_fmts is only implemented for LVDS decoders, resulting
> that LVDS encoders only support bus format MEDIA_BUS_FMT_FIXED. This
> results in warnings like:
> mxsfb 21c8000.lcdif: Bridge does not provide bus format, assuming
> MEDIA_BUS_FMT_RGB888_1X24.
> Please fix bridge driver by handling atomic_get_input_bus_fmts.
>
> Fix this by reusing lvds_codec_atomic_get_input_bus_fmts currently used
> for LVDS decoders. Use RGB888_1X24 for LVDS encoders. This also allows
> removing the dedicated struct drm_bridge_funcs for decoders.
>
> Signed-off-by: Alexander Stein
> ---
>
> Notes:
> Currently all supported LVDS encoders (ti,ds90c185, ti,ds90c187 and
> ti,sn75lvds83) only support RGB888_1X24.
>
> drivers/gpu/drm/bridge/lvds-codec.c | 12
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/lvds-codec.c
> b/drivers/gpu/drm/bridge/lvds-codec.c
> index 8c5668dca0c4b..991732c4b6298 100644
> --- a/drivers/gpu/drm/bridge/lvds-codec.c
> +++ b/drivers/gpu/drm/bridge/lvds-codec.c
> @@ -5,6 +5,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -71,12 +72,6 @@ static void lvds_codec_disable(struct drm_bridge *bridge)
> "Failed to disable regulator \"vcc\": %d\n", ret);
> }
>
> -static const struct drm_bridge_funcs funcs = {
> - .attach = lvds_codec_attach,
> - .enable = lvds_codec_enable,
> - .disable = lvds_codec_disable,
> -};
> -
> #define MAX_INPUT_SEL_FORMATS 1
> static u32 *
> lvds_codec_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> @@ -102,7 +97,7 @@ lvds_codec_atomic_get_input_bus_fmts(struct drm_bridge
> *bridge,
> return input_fmts;
> }
>
> -static const struct drm_bridge_funcs funcs_decoder = {
> +static const struct drm_bridge_funcs funcs = {
> .attach = lvds_codec_attach,
> .enable = lvds_codec_enable,
> .disable = lvds_codec_disable,
> @@ -184,8 +179,9 @@ static int lvds_codec_probe(struct platform_device *pdev)
> return ret;
> } else {
> lvds_codec->bus_format = ret;
> - lvds_codec->bridge.funcs = _decoder;
> }
> + } else {
> + lvds_codec->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> }
>
> /*
Reviewed-by: Robert Foss
Re: [PATCH v2 01/15] drm/shmem-helper: Make pages_use_count an atomic_t
On 09/08/2023 17:53, Boris Brezillon wrote:
> This way we can grab a pages ref without acquiring the resv lock when
> pages_use_count > 0. Need to implement asynchronous map using the
NIT: s/Need/This is needed/
> drm_gpuva_mgr when the map/unmap operation triggers a mapping split,
> requiring the new left/right regions to grab an additional page ref
> to guarantee that the pages stay pinned when the middle section is
> unmapped.
>
> Signed-off-by: Boris Brezillon
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 28 +
> drivers/gpu/drm/lima/lima_gem.c | 2 +-
> drivers/gpu/drm/panfrost/panfrost_mmu.c | 2 +-
> include/drm/drm_gem_shmem_helper.h | 2 +-
> 4 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index a783d2245599..ca6938ea1b82 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -155,7 +155,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
> *shmem)
> if (shmem->pages)
> drm_gem_shmem_put_pages(shmem);
>
> - drm_WARN_ON(obj->dev, shmem->pages_use_count);
> + drm_WARN_ON(obj->dev, atomic_read(>pages_use_count));
>
> dma_resv_unlock(shmem->base.resv);
> }
> @@ -172,14 +172,14 @@ static int drm_gem_shmem_get_pages(struct
> drm_gem_shmem_object *shmem)
>
> dma_resv_assert_held(shmem->base.resv);
>
> - if (shmem->pages_use_count++ > 0)
> + if (atomic_inc_return(>pages_use_count) > 1)
> return 0;
>
> pages = drm_gem_get_pages(obj);
> if (IS_ERR(pages)) {
> drm_dbg_kms(obj->dev, "Failed to get pages (%ld)\n",
> PTR_ERR(pages));
> - shmem->pages_use_count = 0;
> + atomic_set(>pages_use_count, 0);
> return PTR_ERR(pages);
> }
>
> @@ -210,10 +210,10 @@ void drm_gem_shmem_put_pages(struct
> drm_gem_shmem_object *shmem)
>
> dma_resv_assert_held(shmem->base.resv);
>
> - if (drm_WARN_ON_ONCE(obj->dev, !shmem->pages_use_count))
> + if (drm_WARN_ON_ONCE(obj->dev, !atomic_read(>pages_use_count)))
> return;
>
> - if (--shmem->pages_use_count > 0)
> + if (atomic_dec_return(>pages_use_count) > 0)
> return;
>
> #ifdef CONFIG_X86
> @@ -263,6 +263,10 @@ int drm_gem_shmem_pin(struct drm_gem_shmem_object *shmem)
>
> drm_WARN_ON(obj->dev, obj->import_attach);
>
> + /* If we are the first owner, we need to grab the lock. */
> + if (atomic_inc_not_zero(>pages_use_count))
> + return 0;
> +
Unless I'm misunderstanding I think this introduces a race where two
threads call drm_gem_shmem_pin() at the same time:
Thread1 | Thread 2
+--
drm_gem_shmem_pin() |
- pages_use_count == 0 so not |
incremented |
- lock taken |
drm_gem_shmem_pin_locked() |
drm_gem_shmem_get_pages() |
- pages_use_count incremented |
| drm_gem_shmem_pin()
| - pages_use_count == 1 so is it
|incremented and returns early
|without taking the lock
| Code tries to use shmem->pages
| and blows up
drm_gem_get_pages() |
shmem->pages populated |
lock released |
I think you need to modify drm_gem_shmem_get_pages() to only increment
pages_use_count when shmem->pages has been populated. That also gets rid
of the atomic_set() in that function which scares me.
Steve
> ret = dma_resv_lock_interruptible(shmem->base.resv, NULL);
> if (ret)
> return ret;
> @@ -286,6 +290,10 @@ void drm_gem_shmem_unpin(struct drm_gem_shmem_object
> *shmem)
>
> drm_WARN_ON(obj->dev, obj->import_attach);
>
> + /* If we are the last owner, we need to grab the lock. */
> + if (atomic_add_unless(>pages_use_count, -1, 1))
> + return;
> +
> dma_resv_lock(shmem->base.resv, NULL);
> drm_gem_shmem_unpin_locked(shmem);
> dma_resv_unlock(shmem->base.resv);
> @@ -543,18 +551,12 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct
> *vma)
>
> drm_WARN_ON(obj->dev, obj->import_attach);
>
> - dma_resv_lock(shmem->base.resv, NULL);
> -
> /*
>* We should have already pinned the pages when the buffer was first
>* mmap'd, vm_open() just grabs an additional reference for the new
>* mm the vma is getting copied into (ie. on fork()).
>*/
> - if (!drm_WARN_ON_ONCE(obj->dev, !shmem->pages_use_count))
> - shmem->pages_use_count++;
> -
> - dma_resv_unlock(shmem->base.resv);
> -
> + drm_WARN_ON_ONCE(obj->dev,
Re: [PATCH v4] drm: bridge: samsung-dsim: Fix waiting for empty cmd transfer FIFO on older Exynos
On Wed, 9 Aug 2023 16:56:41 +0200, Marek Szyprowski wrote:
> Samsung DSIM used in older Exynos SoCs (like Exynos 4210, 4x12, 3250)
> doesn't report empty level of packer header FIFO. In case of those SoCs,
> use the old way of waiting for empty command tranfsfer FIFO, removed
> recently by commit 14806c641582 ("Drain command transfer FIFO before
> transfer").
>
>
> [...]
Fixed formatting warning related to commit message syntax.
Applied, thanks!
[1/1] drm: bridge: samsung-dsim: Fix waiting for empty cmd transfer FIFO on
older Exynos
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=15f389da1125
Rob
Re: [PATCH] drm: bridge: for GENERIC_PHY_MIPI_DPHY also select GENERIC_PHY
On Thu, 3 Aug 2023 20:01:37 -0700, Randy Dunlap wrote: > Three DRM bridge drivers select GENERIC_PHY_MIPI_DPHY when GENERIC_PHY > might not be set. This causes Kconfig warnings and a build error. > > WARNING: unmet direct dependencies detected for GENERIC_PHY_MIPI_DPHY > Depends on [n]: GENERIC_PHY [=n] > Selected by [y]: > - DRM_NWL_MIPI_DSI [=y] && DRM_BRIDGE [=y] && DRM [=y] && COMMON_CLK [=y] > && OF [=y] && HAS_IOMEM [=y] > - DRM_SAMSUNG_DSIM [=y] && DRM [=y] && DRM_BRIDGE [=y] && COMMON_CLK [=y] > && OF [=y] && HAS_IOMEM [=y] > > [...] Applied, thanks! [1/1] drm: bridge: for GENERIC_PHY_MIPI_DPHY also select GENERIC_PHY https://cgit.freedesktop.org/drm/drm-misc/commit/?id=96413b355a49 Rob
Re: [PATCH] drm: bridge: samsung-dsim: Initialize ULPS EXIT for i.MX8M DSIM
On Sun, 9 Jul 2023 15:48:27 +0200, Marek Vasut wrote: > The ULPS EXIT is initialized to 0xaf in downstream BSP as well as older > revisions of this patchset, in newer revisions of the DSIM patchset it > was left out and set to 0. Fix it. > > Applied, thanks! [1/1] drm: bridge: samsung-dsim: Initialize ULPS EXIT for i.MX8M DSIM https://cgit.freedesktop.org/drm/drm-misc/commit/?id=192948f6a923 Rob
[PATCH drm-misc-next] drm/ttm/tests: Require MMU when testing
Satisfy MMU dependency when testing TTM with KUnit. This fixes compilation errors on platforms that don't select this option by default. Signed-off-by: Karolina Stolarek Cc: Christian König Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202308110133.f0lhfwmv-...@intel.com/ Closes: https://lore.kernel.org/oe-kbuild-all/202308111032.enu8iisr-...@intel.com/ --- drivers/gpu/drm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 9d1f0e04fd56..ab9ef1c20349 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -198,7 +198,7 @@ config DRM_TTM config DRM_TTM_KUNIT_TEST tristate "KUnit tests for TTM" if !KUNIT_ALL_TESTS default n -depends on DRM && KUNIT +depends on DRM && KUNIT && MMU select DRM_TTM select DRM_EXPORT_FOR_TESTS if m select DRM_KUNIT_TEST_HELPERS -- 2.25.1
Re: [RFC PATCH v2 00/11] Device Memory TCP
Am 10.08.23 um 20:44 schrieb Mina Almasry: On Thu, Aug 10, 2023 at 3:29 AM Christian König wrote: Am 10.08.23 um 03:57 schrieb Mina Almasry: Changes in RFC v2: -- The sticking point in RFC v1[1] was the dma-buf pages approach we used to deliver the device memory to the TCP stack. RFC v2 is a proof-of-concept that attempts to resolve this by implementing scatterlist support in the networking stack, such that we can import the dma-buf scatterlist directly. Impressive work, I didn't thought that this would be possible that "easily". Please note that we have considered replacing scatterlists with simple arrays of DMA-addresses in the DMA-buf framework to avoid people trying to access the struct page inside the scatterlist. FWIW, I'm not doing anything with the struct pages inside the scatterlist. All I need from the scatterlist are the sg_dma_address(sg) and the sg_dma_len(sg), and I'm guessing the array you're describing will provide exactly those, but let me know if I misunderstood. Your understanding is perfectly correct. It might be a good idea to push for that first before this here is finally implemented. GPU drivers already convert the scatterlist used to arrays of DMA-addresses as soon as they get them. This leaves RDMA and V4L as the other two main users which would need to be converted. This is the approach proposed at a high level here[2]. Detailed changes: 1. Replaced dma-buf pages approach with importing scatterlist into the page pool. 2. Replace the dma-buf pages centric API with a netlink API. 3. Removed the TX path implementation - there is no issue with implementing the TX path with scatterlist approach, but leaving out the TX path makes it easier to review. 4. Functionality is tested with this proposal, but I have not conducted perf testing yet. I'm not sure there are regressions, but I removed perf claims from the cover letter until they can be re-confirmed. 5. Added Signed-off-by: contributors to the implementation. 6. Fixed some bugs with the RX path since RFC v1. Any feedback welcome, but specifically the biggest pending questions needing feedback IMO are: 1. Feedback on the scatterlist-based approach in general. As far as I can see this sounds like the right thing to do in general. Question is rather if we should stick with scatterlist, use array of DMA-addresses or maybe even come up with a completely new structure. As far as I can tell, it should be trivial to switch this device memory TCP implementation to anything that provides: 1. DMA-addresses (sg_dma_address() equivalent) 2. lengths (sg_dma_len() equivalent) if you go that route. Specifically, I think it will be pretty much a localized change to netdev_bind_dmabuf_to_queue() implemented in this patch: https://lore.kernel.org/netdev/znulidzuvvyfy...@ziepe.ca/T/#m2d344b08f54562cc9155c3f5b018cbfaed96036f Thanks, that's exactly what I wanted to hear. 2. Netlink API (Patch 1 & 2). How does netlink manage the lifetime of objects? Netlink itself doesn't handle the lifetime of the binding. However, the API I implemented unbinds the dma-buf when the netlink socket is destroyed. I do this so that even if the user process crashes or forgets to unbind, the dma-buf will still be unbound once the netlink socket is closed on the process exit. Details in this patch: https://lore.kernel.org/netdev/znulidzuvvyfy...@ziepe.ca/T/#m2d344b08f54562cc9155c3f5b018cbfaed96036f I need to double check the details, but at least of hand that sounds sufficient for the lifetime requirements of DMA-buf. Thanks, Christian. On Thu, Aug 10, 2023 at 9:07 AM Jason Gunthorpe wrote: On Thu, Aug 10, 2023 at 12:29:08PM +0200, Christian König wrote: Am 10.08.23 um 03:57 schrieb Mina Almasry: Changes in RFC v2: -- The sticking point in RFC v1[1] was the dma-buf pages approach we used to deliver the device memory to the TCP stack. RFC v2 is a proof-of-concept that attempts to resolve this by implementing scatterlist support in the networking stack, such that we can import the dma-buf scatterlist directly. Impressive work, I didn't thought that this would be possible that "easily". Please note that we have considered replacing scatterlists with simple arrays of DMA-addresses in the DMA-buf framework to avoid people trying to access the struct page inside the scatterlist. It might be a good idea to push for that first before this here is finally implemented. GPU drivers already convert the scatterlist used to arrays of DMA-addresses as soon as they get them. This leaves RDMA and V4L as the other two main users which would need to be converted. Oh that would be a nightmare for RDMA. We need a standard based way to have scalable lists of DMA addresses :( 2. Netlink API (Patch 1 & 2). How does netlink manage the lifetime of objects? And access control.. Someone will correct me if I'm wrong but I'm not sure netlink itself will do (sufficient) access control.
[PULL] drm-intel-gt-next
Hi Dave & Daniel, Here's the final drm-intel-gt-next PR for v6.6. Not too many patches as the previous PR was later than usual. Just one more workaround fix for MTL, some object coherency refactoring and selftest fix. Note that there is a backmerge of drm-next[1], too. Regards, Joonas [1] As prep for https://patchwork.freedesktop.org/series/121735/ but the series started failing CI after rebasing and continues to be investigated so not landing here yet. *** drm-intel-gt-next-2023-08-11: Cross-subsystem Changes: - Backmerge of drm-next Driver Changes: - Apply workaround 22016122933 correctly (Jonathan, Matt R) - Simplify shmem_create_from_object map_type selection (Jonathan, Tvrtko) - Make i915_coherent_map_type GT-centric (Jonathan, Matt R) - Selftest improvements (John) The following changes since commit d9aa1da9a8cfb0387eb5703c15bd1f54421460ac: Merge tag 'drm-intel-gt-next-2023-08-04' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2023-08-07 13:49:25 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-gt-next-2023-08-11 for you to fetch changes up to 788568fad4015406fa84fc86cefbef7c470c7c1f: drm/i915/guc: Fix potential null pointer deref in GuC 'steal id' test (2023-08-10 16:02:01 -0700) Cross-subsystem Changes: - Backmerge of drm-next Driver Changes: - Apply workaround 22016122933 correctly (Jonathan, Matt R) - Simplify shmem_create_from_object map_type selection (Jonathan, Tvrtko) - Make i915_coherent_map_type GT-centric (Jonathan, Matt R) - Selftest improvements (John) John Harrison (1): drm/i915/guc: Fix potential null pointer deref in GuC 'steal id' test Jonathan Cavitt (3): drm/i915/gt: Simplify shmem_create_from_object map_type selection drm/i915: Make i915_coherent_map_type GT-centric drm/i915/gt: Apply workaround 22016122933 correctly Joonas Lahtinen (1): Merge drm/drm-next into drm-intel-gt-next drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- drivers/gpu/drm/i915/gem/i915_gem_object.h| 4 drivers/gpu/drm/i915/gem/i915_gem_pages.c | 15 --- drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c | 12 ++-- drivers/gpu/drm/i915/gt/intel_engine_pm.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt.c| 16 drivers/gpu/drm/i915/gt/intel_gt.h| 10 ++ drivers/gpu/drm/i915/gt/intel_gtt.c | 4 ++-- drivers/gpu/drm/i915/gt/intel_lrc.c | 13 +++-- drivers/gpu/drm/i915/gt/intel_ring.c | 3 ++- drivers/gpu/drm/i915/gt/selftest_context.c| 5 +++-- drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++-- drivers/gpu/drm/i915/gt/selftest_lrc.c| 6 +++--- drivers/gpu/drm/i915/gt/shmem_utils.c | 3 +-- drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c | 7 +-- drivers/gpu/drm/i915/gt/uc/intel_guc.c| 11 ++- drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 4 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 3 +-- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 ++- drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 6 +++--- drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c| 3 ++- drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 5 - drivers/gpu/drm/i915/selftests/igt_spinner.c | 2 +- 23 files changed, 75 insertions(+), 69 deletions(-)
[PATCH -next] drm/virtio: Remove unused function declarations
Commit dc5698e80cf7 ("Add virtio gpu driver.") declared but never
implemented virtio_gpu_attach_status_page()/virtio_gpu_detach_status_page()
Also commit 62fb7a5e1096 ("virtio-gpu: add 3d/virgl support")
declared but never implemented virtio_gpu_fence_ack() and
virtio_gpu_dequeue_fence_func().
Commit c84adb304c10 ("drm/virtio: Support virtgpu exported resources")
declared but never implemented virtgpu_gem_prime_get_uuid().
Signed-off-by: Yue Haibing
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 4126c384286b..8513b671f871 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -344,8 +344,6 @@ void virtio_gpu_object_attach(struct virtio_gpu_device
*vgdev,
struct virtio_gpu_object *obj,
struct virtio_gpu_mem_entry *ents,
unsigned int nents);
-int virtio_gpu_attach_status_page(struct virtio_gpu_device *vgdev);
-int virtio_gpu_detach_status_page(struct virtio_gpu_device *vgdev);
void virtio_gpu_cursor_ping(struct virtio_gpu_device *vgdev,
struct virtio_gpu_output *output);
int virtio_gpu_cmd_get_display_info(struct virtio_gpu_device *vgdev);
@@ -394,11 +392,8 @@ virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device
*vgdev,
struct virtio_gpu_fence *fence);
void virtio_gpu_ctrl_ack(struct virtqueue *vq);
void virtio_gpu_cursor_ack(struct virtqueue *vq);
-void virtio_gpu_fence_ack(struct virtqueue *vq);
void virtio_gpu_dequeue_ctrl_func(struct work_struct *work);
void virtio_gpu_dequeue_cursor_func(struct work_struct *work);
-void virtio_gpu_dequeue_fence_func(struct work_struct *work);
-
void virtio_gpu_notify(struct virtio_gpu_device *vgdev);
int
@@ -465,8 +460,6 @@ struct dma_buf *virtgpu_gem_prime_export(struct
drm_gem_object *obj,
int flags);
struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev,
struct dma_buf *buf);
-int virtgpu_gem_prime_get_uuid(struct drm_gem_object *obj,
- uuid_t *uuid);
struct drm_gem_object *virtgpu_gem_prime_import_sg_table(
struct drm_device *dev, struct dma_buf_attachment *attach,
struct sg_table *sgt);
--
2.34.1
Re: [Intel-xe] [PATCH v5] Documentation/gpu: Add a VM_BIND async draft document
On 8/10/23 22:51, Zanoni, Paulo R wrote: On Thu, 2023-08-10 at 21:49 +0200, Thomas Hellström wrote: Hi, Paulo. Thanks for reviewing. I'd like to try give some answers below. On 8/4/23 23:30, Zanoni, Paulo R wrote: On Fri, 2023-08-04 at 16:02 -0400, Rodrigo Vivi wrote: On Wed, Jul 19, 2023 at 07:50:21PM +, Zanoni, Paulo R wrote: On Sat, 2023-07-15 at 17:45 +0200, Thomas Hellström wrote: Add a motivation for and description of asynchronous VM_BIND operation I think I may have missed some other documentation, which would explain some of my questions below, so please be patient with my misunderstandings. But here's a review from the POV of a UMD person. v2: - Fix typos (Nirmoy Das) - Improve the description of a memory fence (Oak Zeng) - Add a reference to the document in the Xe RFC. - Add pointers to sample uAPI suggestions v3: - Address review comments (Danilo Krummrich) - Formatting fixes v4: - Address typos (Francois Dugast) - Explain why in-fences are not allowed for VM_BIND operations for long- running workloads (Matthew Brost) v5: - More typo- and style fixing - Further clarify the implications of disallowing in-fences for VM_BIND operations for long-running workloads (Matthew Brost) Signed-off-by: Thomas Hellström Acked-by: Nirmoy Das --- Documentation/gpu/drm-vm-bind-async.rst | 171 Documentation/gpu/rfc/xe.rst| 4 +- 2 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 Documentation/gpu/drm-vm-bind-async.rst diff --git a/Documentation/gpu/drm-vm-bind-async.rst b/Documentation/gpu/drm-vm-bind-async.rst new file mode 100644 index ..d2b02a38198a --- /dev/null +++ b/Documentation/gpu/drm-vm-bind-async.rst @@ -0,0 +1,171 @@ + +Asynchronous VM_BIND + + +Nomenclature: += + +* ``VRAM``: On-device memory. Sometimes referred to as device local memory. + +* ``gpu_vm``: A GPU address space. Typically per process, but can be shared by + multiple processes. + +* ``VM_BIND``: An operation or a list of operations to modify a gpu_vm using + an IOCTL. The operations include mapping and unmapping system- or + VRAM memory. + +* ``syncobj``: A container that abstracts synchronization objects. The + synchronization objects can be either generic, like dma-fences or + driver specific. A syncobj typically indicates the type of the + underlying synchronization object. + +* ``in-syncobj``: Argument to a VM_BIND IOCTL, the VM_BIND operation waits + for these before starting. + +* ``out-syncobj``: Argument to a VM_BIND_IOCTL, the VM_BIND operation + signals these when the bind operation is complete. + +* ``memory fence``: A synchronization object, different from a dma-fence. Since you've mentioned it twice in this document already, for completeness would you mind also giving a definition for dma-fence in what it relates/contrasts to the rest of the text? Maybe worth a link to the dma-fence doc itself? (somehow making sphinx to point out to driver-api/dma-buf.html#dma-fences) Will respin and add a link to this. This document indeed assumes the reader is somewhat familiar with the dma-fence concept. But the differences are written below Paulo: + A memory fence uses the value of a specified memory location to determine + signaled status. A memory fence can be awaited and signaled by both + the GPU and CPU. Memory fences are sometimes referred to as + user-fences, userspace-fences or gpu futexes and do not necessarily obey + the dma-fence rule of signaling within a "reasonable amount of time". + The kernel should thus avoid waiting for memory fences with locks held. ^ + +* ``long-running workload``: A workload that may take more than the + current stipulated dma-fence maximum signal delay to complete and Where is this delay defined? Is this the same as the gpuhang timer? dma-fence defines it in a very "cool" way: "reasonable amount of time": https://www.kernel.org/doc/html/latest/driver-api/dma-buf.html#dma-fences so, in contrast, long-running workload is *anything* above that "reasonable amount of time". That answers it but doesn't exactly answer it either. In practice, how much is that "reasonable amount of time"? This is documentation, it should avoid using vague statements or hypothetical cases. The documentation you posted suggested this may be the same as the GPU hang timeout, but doesn't give a definitive answer (because multiple drivers may define it differently, apparently). In practice, what do drivers do? As a user-space developer, how long can I wait before things fail? Is there a way to figure out, maybe query a parameter somewhere? Which driver waits the least? Which driver waits the most? Is 10 seconds "reasonable amount of time"? My grandma thinks 2 weeks is a reasonable amount of time when waiting for things. We can't do much more than point to the dma-fence document here, since we can't have a vague definition in the main
Re: [PATCH V8 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD
On Thu, Aug 10, 2023 at 03:37:56PM +0800, Evan Quan wrote:
> AMD has introduced an ACPI based mechanism to support WBRF for some
> platforms with AMD dGPU + WLAN. This needs support from BIOS equipped
> with necessary AML implementations and dGPU firmwares.
>
> For those systems without the ACPI mechanism and developing solutions,
> user can use/fall-back the generic WBRF solution for diagnosing potential
> interference issues.
>
> And for the platform which does not equip with the necessary AMD ACPI
> implementations but with CONFIG_WBRF_AMD_ACPI built as 'y', it will
> fall back to generic WBRF solution if the `wbrf` is set as "on".
>
> Co-developed-by: Mario Limonciello
> Signed-off-by: Mario Limonciello
> Co-developed-by: Evan Quan
> Signed-off-by: Evan Quan
...
> diff --git a/drivers/acpi/amd_wbrf.c b/drivers/acpi/amd_wbrf.c
...
> +static bool check_acpi_wbrf(acpi_handle handle, u64 rev, u64 funcs)
> +{
> + int i;
> + u64 mask = 0;
> + union acpi_object *obj;
> +
> + if (funcs == 0)
> + return false;
> +
> + obj = acpi_evaluate_wbrf(handle, rev, 0);
> + if (!obj)
> + return false;
> +
> + if (obj->type != ACPI_TYPE_BUFFER)
> + return false;
> +
> + /*
> + * Bit vector providing supported functions information.
> + * Each bit marks support for one specific function of the WBRF method.
> + */
> + for (i = 0; i < obj->buffer.length && i < 8; i++)
> + mask |= (((u64)obj->buffer.pointer[i]) << (i * 8));
> +
> + ACPI_FREE(obj);
> +
> + if ((mask & BIT(WBRF_ENABLED)) &&
> + (mask & funcs) == funcs)
Hi Evan,
a minor nit from my side: the indentation of the line above seems odd.
if ((mask & BIT(WBRF_ENABLED)) &&
(mask & funcs) == funcs)
> + return true;
> +
> + return false;
> +}
...
Re: [PATCH V8 6/9] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature
On Thu, Aug 10, 2023 at 03:38:00PM +0800, Evan Quan wrote:
> With WBRF feature supported, as a driver responding to the frequencies,
> amdgpu driver is able to do shadow pstate switching to mitigate possible
> interference(between its (G-)DDR memory clocks and local radio module
> frequency bands used by Wifi 6/6e/7).
>
> Signed-off-by: Evan Quan
> Reviewed-by: Mario Limonciello
...
> +/**
> + * smu_wbrf_event_handler - handle notify events
> + *
> + * @nb: notifier block
> + * @action: event type
> + * @data: event data
Hi Evan,
a minor nit from my side: although it is documented here,
smu_wbrf_event_handler has no @data parameter, while
it does have an undocumented _arg parameter.
> + *
> + * Calls relevant amdgpu function in response to wbrf event
> + * notification from kernel.
> + */
> +static int smu_wbrf_event_handler(struct notifier_block *nb,
> + unsigned long action, void *_arg)
> +{
> + struct smu_context *smu = container_of(nb, struct smu_context,
> +wbrf_notifier);
> +
> + switch (action) {
> + case WBRF_CHANGED:
> + smu_wbrf_handle_exclusion_ranges(smu);
> + break;
> + default:
> + return NOTIFY_DONE;
> + };
> +
> + return NOTIFY_OK;
> +}
...
Re: [PATCH] accel/ivpu: Document DRM_IVPU_PARAM_CAPABILITIES
On Thu, Aug 10, 2023 at 08:49:00AM -0600, Jeffrey Hugo wrote:
> On 8/10/2023 8:32 AM, Stanislaw Gruszka wrote:
> > On Thu, Aug 10, 2023 at 08:27:08AM -0600, Jeffrey Hugo wrote:
> > > On 8/10/2023 2:07 AM, Stanislaw Gruszka wrote:
> > > > Add comments regarding new DRM_IVPU_PARAM_CAPABILITIES param.
> > > >
> > > > Signed-off-by: Stanislaw Gruszka
> > > > ---
> > > >include/uapi/drm/ivpu_accel.h | 19 +--
> > > >1 file changed, 17 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/uapi/drm/ivpu_accel.h
> > > > b/include/uapi/drm/ivpu_accel.h
> > > > index a58a14c9f222..7d1ecc5bbac3 100644
> > > > --- a/include/uapi/drm/ivpu_accel.h
> > > > +++ b/include/uapi/drm/ivpu_accel.h
> > > > @@ -69,8 +69,21 @@ extern "C" {
> > > >#define DRM_IVPU_CONTEXT_PRIORITY_FOCUS 2
> > > >#define DRM_IVPU_CONTEXT_PRIORITY_REALTIME 3
> > > > -#define DRM_IVPU_CAP_METRIC_STREAMER 1
> > > > -#define DRM_IVPU_CAP_DMA_MEMORY_RANGE 2
> > > > +/**
> > > > + * DRM_IVPU_CAP_METRIC_STREAMER
> > > > + *
> > > > + * Metric streamer support, provides sampling of various hardware
> > > > + * performance metrics (like DMA bandwidth, cache miss/hits),
> > > > + * can be used for profiling.
> > >
> > > Feels like this is a run-on sentence. I think this reads better -
> > >
> > > Metric streamer support. Provides sampling of various hardware
> > > performance
> > > metrics like DMA bandwidth and cache miss/hits. Can be used for
> > > profiling.
> > >
> > > What do you think?
> >
> > Looks better :-)
>
> Ok. I guess with that
> Reviewed-by: Jeffrey Hugo
Thanks, applied to drm-misc-next
Regards
Stanislaw
[PATCH -next] fbdev: atmel_lcdfb: Remove redundant of_match_ptr()
The driver depends on CONFIG_OF, it is not necessary to use
of_match_ptr() here.
Signed-off-by: Ruan Jinjie
---
drivers/video/fbdev/atmel_lcdfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/atmel_lcdfb.c
b/drivers/video/fbdev/atmel_lcdfb.c
index c75a62287ec4..a908db233409 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1306,7 +1306,7 @@ static struct platform_driver atmel_lcdfb_driver = {
.resume = atmel_lcdfb_resume,
.driver = {
.name = "atmel_lcdfb",
- .of_match_table = of_match_ptr(atmel_lcdfb_dt_ids),
+ .of_match_table = atmel_lcdfb_dt_ids,
},
};
--
2.34.1
Re: [PATCH v2 07/11] PCI/VGA: vga_client_register() return -ENODEV on failure, not -1
On Thu, 10 Aug 2023, suijingfeng wrote:
> On 2023/8/9 21:52, Ilpo Järvinen wrote:
> > On Wed, 9 Aug 2023, Sui Jingfeng wrote:
> >
> > > From: Sui Jingfeng
> > >
> > Changelog body is missing.
>
>
> I thought that probably the Fixes tag could be taken as the body of this
> commit,
> since there are no warnings when I check the whole series with checkpatch.pl.
>
>
> > > Fixes: 934f992c763a ("drm/i915: Recognise non-VGA display devices")
> > > Signed-off-by: Sui Jingfeng
> > > ---
> > > drivers/pci/vgaarb.c | 15 ++-
> > > 1 file changed, 6 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> > > index 811510253553..a6b8c0def35d 100644
> > > --- a/drivers/pci/vgaarb.c
> > > +++ b/drivers/pci/vgaarb.c
> > > @@ -964,7 +964,7 @@ EXPORT_SYMBOL(vga_set_legacy_decoding);
> > >*
> > >* To unregister just call vga_client_unregister().
> > >*
> > > - * Returns: 0 on success, -1 on failure
> > > + * Returns: 0 on success, -ENODEV on failure
> > So this is the true substance of this change??
> >
> Yes.
>
>
> > It doesn't warrant Fixes tag which requires a real problem to fix. An
> > incorrect comment is not enough.
> >
> > I think the shortlog is a bit misleading as is because it doesn't in any
> > way indicate the problem is only in a comment.
>
> But it's that commit(934f992c763a) alter the return value of
> vga_client_register(),
> which make the commit and code don't match anymore.
This is useful information, no point in withholding it which forces
others to figure it out by looking that commit up so put that detail into
the changelog body.
> > I'd prefer to
> > initialize ret = 0 instead:
> >
> > int ret = 0;
> > ...
> > if (!vgadev) {
> > err = -ENODEV;
> > goto unlock;
> > }
> > ...
> > unlock:
> > ...
> >
>
> But this is same as the original coding style, no fundamental improve.
> The key point is to make the wrapped code between the spin_lock_irqsave() and
> spin_unlock_irqrestore() compact.
> my patch remove the necessary 'goto' statement and the 'bail' label.
> After apply my patch, the vga_client_register() function became as this:
>
> int vga_client_register(struct pci_dev *pdev,
> unsigned int (*set_decode)(struct pci_dev *pdev, bool decode))
> {
> int ret = -ENODEV;
> struct vga_device *vgadev;
> unsigned long flags;
>
> spin_lock_irqsave(_lock, flags);
> vgadev = vgadev_find(pdev);
> if (vgadev) {
> vgadev->set_decode = set_decode;
> ret = 0;
> }
> spin_unlock_irqrestore(_lock, flags);
>
> return ret;
> }
I'm not too attached to either of the ways around since there's no
correctness issues here. Feel free to ignore my alternative suggestion
(make the separate patch out of it in anycase).
--
i.
[REGRESSION] HDMI connector detection broken in 6.3 on Intel(R) Celeron(R) N3060 integrated graphics
The following is a copy an issue I posted to drm/i915 gitlab [1] two
months ago. I repost it to the mailing lists in hope that it will help
the right people pay attention to it.
After kernel upgrade from 6.2.13 to 6.3 HDMI connector detection is
broken for me. Issue is 100% reproducible:
1. Start system as usual with HDMI connected.
2. Disconnect HDMI
3. Connect HDMI back
4. Get "no signal" on display, connector status in sysfs is disconnected
Curiously, running xrandr over ssh like
ssh qnap251.local env DISPLAY=:0 xrandr
makes display come back. drm-tip tip is affected as well (last test
2023-08-02).
Bisecting points at a4e771729a51 ("drm/probe_helper: sort out poll_running vs
poll_enabled").
Reverting that commit on top of 6.3 fixes the issue for me.
System information:
* System architecture: x86_64
* Kernel version: 6.3.arch1
* Linux distribution: Arch Linux
* Machine: QNAP TS-251A, CPU: Intel(R) Celeron(R) CPU N3060 @ 1.60GHz
* Display connector: single HDMI display
* dmesg with debug information (captured on drm-tip, following above 4 steps):
[2]
* xrandr output:
Screen 0: minimum 320 x 200, current 1920 x 1080, maximum 16384 x 16384
DP-1 disconnected (normal left inverted right x axis y axis)
HDMI-1 connected primary 1920x1080+0+0 (normal left inverted right x axis y
axis) 708mm x 398mm
1920x1080 60.00*+ 50.0059.9430.0025.0024.00
29.9723.98
1920x1080i60.0050.0059.94
1360x768 59.80
1280x768 60.35
1280x720 60.0050.0059.94
1024x768 75.0370.0760.00
832x624 74.55
800x600 75.0060.32
720x576 50.00
720x480 60.0059.94
640x480 75.0060.0059.94
720x400 70.08
DP-2 disconnected (normal left inverted right x axis y axis)
HDMI-2 disconnected (normal left inverted right x axis y axis)```
I'm willing to provide additional information and/or test fixes.
[1] https://gitlab.freedesktop.org/drm/intel/-/issues/8451
[2]
https://gitlab.freedesktop.org/drm/intel/uploads/fda7aff0b13ef20962856c2c7be51544/dmesg.txt
#regzbot introduced: a4e771729a51
--
Best regards,
Mikhail Rudenko
Re: [PATCH] accel/qaic: Clean up integer overflow checking in map_user_pages()
On 8/10/2023 5:23 AM, Dan Carpenter wrote:
The encode_dma() function has some validation on in_trans->size but it
would be more clear to move those checks to find_and_map_user_pages().
The encode_dma() had two checks:
if (in_trans->addr + in_trans->size < in_trans->addr || !in_trans->size)
return -EINVAL;
The in_trans->addr variable is the starting address. The in_trans->size
variable is the total size of the transfer. The transfer can occur in
parts and the resources->xferred_dma_size tracks how many bytes we have
already transferred.
This patch introduces a new variable "remaining" which represents the
amount we want to transfer (in_trans->size) minus the amount we have
already transferred (resources->xferred_dma_size).
I have modified the check for if in_trans->size is zero to instead check
if in_trans->size is less than resources->xferred_dma_size. If we have
already transferred more bytes than in_trans->size then there are negative
bytes remaining which doesn't make sense. If there are zero bytes
remaining to be copied, just return success.
The check in encode_dma() checked that "addr + size" could not overflow
and barring a driver bug that should work, but it's easier to check if
we do this in parts. First check that "in_trans->addr +
resources->xferred_dma_size" is safe. Then check that "xfer_start_addr +
remaining" is safe.
My final concern was that we are dealing with u64 values but on 32bit
systems the kmalloc() function will truncate the sizes to 32 bits. So
I calculated "total = in_trans->size + offset_in_page(xfer_start_addr);"
and returned -EINVAL if it were >= SIZE_MAX. This will not affect 64bit
systems.
Fixes: 129776ac2e38 ("accel/qaic: Add control path")
Signed-off-by: Dan Carpenter
Looks good to me.
Reviewed-by: Carl Vanderlip
[PATCH] drm/i915/gmch: fix build error var set but not used
When CONFIG_PNP is not defined, i915 will fail to compile with error bellow:
drivers/gpu/drm/i915/soc/intel_gmch.c:43:13: error: variable
‘mchbar_addr’ set but not used
Fix it by surrounding variable declaration and assignment with ifdef
Signed-off-by: Wang Jinchao
---
drivers/gpu/drm/i915/soc/intel_gmch.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/soc/intel_gmch.c
b/drivers/gpu/drm/i915/soc/intel_gmch.c
index 6d0204942f7a..d2c442b0b4eb 100644
--- a/drivers/gpu/drm/i915/soc/intel_gmch.c
+++ b/drivers/gpu/drm/i915/soc/intel_gmch.c
@@ -38,16 +38,17 @@ intel_alloc_mchbar_resource(struct drm_i915_private *i915)
{
int reg = GRAPHICS_VER(i915) >= 4 ? MCHBAR_I965 : MCHBAR_I915;
u32 temp_lo, temp_hi = 0;
- u64 mchbar_addr;
int ret;
-
+#ifdef CONFIG_PNP
+ u64 mchbar_addr;
+#endif
if (GRAPHICS_VER(i915) >= 4)
pci_read_config_dword(i915->gmch.pdev, reg + 4, _hi);
pci_read_config_dword(i915->gmch.pdev, reg, _lo);
- mchbar_addr = ((u64)temp_hi << 32) | temp_lo;
/* If ACPI doesn't have it, assume we need to allocate it ourselves */
#ifdef CONFIG_PNP
+ mchbar_addr = ((u64)temp_hi << 32) | temp_lo;
if (mchbar_addr &&
pnp_range_reserved(mchbar_addr, mchbar_addr + MCHBAR_SIZE))
return 0;
--
2.40.0
[PATCH -next] drm/nouveau/fifo: Remove duplicated include
Remove duplicated include of chid.h. Resolves checkincludes message. Signed-off-by: GUO Zihua --- drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c index b7c9d6115bce..b19a3612b62e 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c @@ -24,7 +24,6 @@ #include "chan.h" #include "chid.h" #include "cgrp.h" -#include "chid.h" #include "runl.h" #include "priv.h" -- 2.17.1
[PATCH -next] drm/shmem-helper: Remove duplicated include
Remove duplicated include of linux/module.h. Resolves checkincludes message. Signed-off-by: GUO Zihua --- drivers/gpu/drm/drm_gem_shmem_helper.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index a783d2245599..526226e615f9 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -10,7 +10,6 @@ #include #include #include -#include #ifdef CONFIG_X86 #include -- 2.17.1
[PATCH] drm: xlnx: Use devm_platform_ioremap_resource_byname() helper function
platform_get_resource_byname() and devm_ioremap_resource() can be
replaced by devm_platform_ioremap_resource_byname(), which can
simplify the code logic a bit, No functional change here.
Signed-off-by: Zhang Zekun
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 3b87eebddc97..2b731f3eea54 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -1228,7 +1228,6 @@ int zynqmp_disp_probe(struct zynqmp_dpsub *dpsub)
{
struct platform_device *pdev = to_platform_device(dpsub->dev);
struct zynqmp_disp *disp;
- struct resource *res;
int ret;
disp = kzalloc(sizeof(*disp), GFP_KERNEL);
@@ -1238,22 +1237,19 @@ int zynqmp_disp_probe(struct zynqmp_dpsub *dpsub)
disp->dev = >dev;
disp->dpsub = dpsub;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "blend");
- disp->blend.base = devm_ioremap_resource(disp->dev, res);
+ disp->blend.base = devm_platform_ioremap_resource_byname(pdev, "blend");
if (IS_ERR(disp->blend.base)) {
ret = PTR_ERR(disp->blend.base);
goto error;
}
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "av_buf");
- disp->avbuf.base = devm_ioremap_resource(disp->dev, res);
+ disp->avbuf.base = devm_platform_ioremap_resource_byname(pdev,
"av_buf");
if (IS_ERR(disp->avbuf.base)) {
ret = PTR_ERR(disp->avbuf.base);
goto error;
}
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aud");
- disp->audio.base = devm_ioremap_resource(disp->dev, res);
+ disp->audio.base = devm_platform_ioremap_resource_byname(pdev, "aud");
if (IS_ERR(disp->audio.base)) {
ret = PTR_ERR(disp->audio.base);
goto error;
--
2.17.1
Re: [Nouveau] Fwd: System (Xeon Nvidia) hangs at boot terminal after kernel 6.4.7
6.4.9 built with unconn.c from 6.4.6 builds and boots and runs fine. Thanks everybody.
Re: [PATCH] drm: xlnx: Use devm_platform_ioremap_resource_byname() helper function
I am sorry for make noise, this patch has been sent before:
https://lore.kernel.org/linux-arm-kernel/20230801214729.gf8...@pendragon.ideasonboard.com/T/
Please ignore this patch.
在 2023/8/11 10:11, Zhang Zekun 写道:
platform_get_resource_byname() and devm_ioremap_resource() can be
replaced by devm_platform_ioremap_resource_byname(), which can
simplify the code logic a bit, No functional change here.
Signed-off-by: Zhang Zekun
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 3b87eebddc97..2b731f3eea54 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -1228,7 +1228,6 @@ int zynqmp_disp_probe(struct zynqmp_dpsub *dpsub)
{
struct platform_device *pdev = to_platform_device(dpsub->dev);
struct zynqmp_disp *disp;
- struct resource *res;
int ret;
disp = kzalloc(sizeof(*disp), GFP_KERNEL);
@@ -1238,22 +1237,19 @@ int zynqmp_disp_probe(struct zynqmp_dpsub *dpsub)
disp->dev = >dev;
disp->dpsub = dpsub;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "blend");
- disp->blend.base = devm_ioremap_resource(disp->dev, res);
+ disp->blend.base = devm_platform_ioremap_resource_byname(pdev, "blend");
if (IS_ERR(disp->blend.base)) {
ret = PTR_ERR(disp->blend.base);
goto error;
}
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "av_buf");
- disp->avbuf.base = devm_ioremap_resource(disp->dev, res);
+ disp->avbuf.base = devm_platform_ioremap_resource_byname(pdev,
"av_buf");
if (IS_ERR(disp->avbuf.base)) {
ret = PTR_ERR(disp->avbuf.base);
goto error;
}
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aud");
- disp->audio.base = devm_ioremap_resource(disp->dev, res);
+ disp->audio.base = devm_platform_ioremap_resource_byname(pdev, "aud");
if (IS_ERR(disp->audio.base)) {
ret = PTR_ERR(disp->audio.base);
goto error;
drm/nouveau: switch over to vmemdup_user()
Use vmemdup_user() rather than duplicating its implementation.
This patch fixes the following Coccinelle warning:
./drivers/gpu/drm/nouveau/nouveau_gem.c:630:7-15: WARNING opportunity for
vmemdup_user
Signed-off-by: Atul Raut
---
drivers/gpu/drm/nouveau/nouveau_gem.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index ab9062e50977..a244b39df213 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -627,15 +627,10 @@ u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
size *= nmemb;
- mem = kvmalloc(size, GFP_KERNEL);
- if (!mem)
+ mem = vmemdup_user(userptr, size);
+ if (IS_ERR(mem))
return ERR_PTR(-ENOMEM);
- if (copy_from_user(mem, userptr, size)) {
- u_free(mem);
- return ERR_PTR(-EFAULT);
- }
-
return mem;
}
--
2.34.1
Re: [PATCH v2 02/11] PCI: Add the pci_get_class_masked() helper
On Thu, 10 Aug 2023, suijingfeng wrote:
> Hi,
>
>
> On 2023/8/9 22:01, Ilpo Järvinen wrote:
> > On Wed, 9 Aug 2023, Sui Jingfeng wrote:
> >
> > > From: Sui Jingfeng
> > >
> > > Because there is no good way to get the mask member used to searching for
> > > devices that conform to a specific PCI class code, an application needs to
> > > process all PCI display devices can achieve its goal as follows:
> > This is mixing old and new way in a single sentence (which is confusing)?
>
>
> Thanks for reviewing, but I can't understand this sentence.
> Are you telling me that my description have grammar problem or something else?
I think it's a bit of both.
> I means that before apply this patch, we don't have a function can be used
> to get all PCI(e) devices in a system by matching against its the PCI base
> class code only,
> while keep the Sub-Class code and the Programming Interface ignored.
> By supply a mask as argument, such thing become possible.
This explanation you put into this reply is much easier to follow and
understand. I recommend you'd use it to replace the unclear fragment
above. So something along the lines of:
There is no function that can be used to get all PCI(e) devices in a
system by matching against its the PCI base class code only, while keep
the Sub-Class code and the Programming Interface ignored.
Add pci_get_class_masked() to allow supplying a mask for the get.
[After this you can put the explanining code block+its intro if you
want]
--
i.
> If an application want to process all PCI display devices in the system,
> then it can achieve its goal by calling pci_get_class_masked() function.
>
>
> > > pdev = NULL;
> > > do {
> > > pdev = pci_get_class_masked(PCI_BASE_CLASS_DISPLAY << 16, 0xFF,
> > > pdev);
> > > if (pdev)
> > > do_something_for_pci_display_device(pdev);
> > > } while (pdev);
> > >
> > > While previously, we just can not ignore Sub-Class code and the
> > > Programming
> > cannot
> >
> > > Interface byte when do the searching.
> > doing the search.
>
>
> OK, will be fixed at the next version.
>
>
> > > Signed-off-by: Sui Jingfeng
> > > ---
> > > drivers/pci/search.c | 30 ++
> > > include/linux/pci.h | 7 +++
> > > 2 files changed, 37 insertions(+)
> > >
> > > diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> > > index b4c138a6ec02..f1c15aea868b 100644
> > > --- a/drivers/pci/search.c
> > > +++ b/drivers/pci/search.c
> > > @@ -334,6 +334,36 @@ struct pci_dev *pci_get_device(unsigned int vendor,
> > > unsigned int device,
> > > }
> > > EXPORT_SYMBOL(pci_get_device);
> > > +/**
> > > + * pci_get_class_masked - begin or continue searching for a PCI device by
> > > class and mask
> > > + * @class: search for a PCI device with this class designation
> > > + * @from: Previous PCI device found in search, or %NULL for new search.
> > > + *
> > > + * Iterates through the list of known PCI devices. If a PCI device is
> > No double spaces in kernel comments. Perhaps your editor might be adding
> > them on reflow (might be configurable to not do that).
> >
> > > + * found with a matching @class, the reference count to the device is
> > > + * incremented and a pointer to its device structure is returned.
> > > + * Otherwise, %NULL is returned.
> > > + * A new search is initiated by passing %NULL as the @from argument.
> > > + * Otherwise if @from is not %NULL, searches continue from next device
> > > + * on the global list. The reference count for @from is always
> > > decremented
> > > + * if it is not %NULL.
> > Use kerneldoc's Return: section for describing return value.
> >
> > > + */
> > > +struct pci_dev *pci_get_class_masked(unsigned int class, unsigned int
> > > mask,
> > > + struct pci_dev *from)
> > > +{
> > > + struct pci_device_id id = {
> > > + .vendor = PCI_ANY_ID,
> > > + .device = PCI_ANY_ID,
> > > + .subvendor = PCI_ANY_ID,
> > > + .subdevice = PCI_ANY_ID,
> > > + .class_mask = mask,
> > > + .class = class,
> > > + };
> > > +
> > > + return pci_get_dev_by_id(, from);
> > > +}
> > > +EXPORT_SYMBOL(pci_get_class_masked);
> > > +
> > > /**
> > >* pci_get_class - begin or continue searching for a PCI device by class
> > >* @class: search for a PCI device with this class designation
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > index 0ff7500772e6..b20e7ba844bf 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -1180,6 +1180,9 @@ struct pci_dev *pci_get_slot(struct pci_bus *bus,
> > > unsigned int devfn);
> > > struct pci_dev *pci_get_domain_bus_and_slot(int domain, unsigned int
> > > bus,
> > > unsigned int devfn);
> > > struct pci_dev *pci_get_class(unsigned int class, struct pci_dev *from);
> > > +struct pci_dev *pci_get_class_masked(unsigned int class, unsigned int
> > >
Re: [REGRESSION] HDMI connector detection broken in 6.3 on Intel(R) Celeron(R) N3060 integrated graphics
On Fri, 11 Aug 2023, Thorsten Leemhuis wrote:
> [CCing the i915 maintainers and the dri maintainers]
>
> Hi, Thorsten here, the Linux kernel's regression tracker.
>
> On 10.08.23 21:33, Mikhail Rudenko wrote:
>> The following is a copy an issue I posted to drm/i915 gitlab [1] two
>> months ago. I repost it to the mailing lists in hope that it will help
>> the right people pay attention to it.
>
> Thx for your report. Wonder why Dmitry (who authored a4e771729a51) or
> Thomas (who committed it) it didn't look into this, but maybe the i915
> devs didn't forward the report to them.
It was [1], but never followed up on, and fell between the cracks. :(
BR,
Jani.
[1] https://lore.kernel.org/r/zgiraoocr1glc...@intel.com
> Let's see if these mails help. Just wondering: does reverting
> a4e771729a51 from 6.5-rc5 or drm-tip help as well?
>
> BTW, there was an earlier report about a problem with a4e771729a51 that
> afaics was never addressed, but it might be unrelated.
>
> https://lore.kernel.org/all/20230328023129.3596968-1-zhouzong...@kylinos.cn/
>
> Ciao, Thorsten
>
>> After kernel upgrade from 6.2.13 to 6.3 HDMI connector detection is
>> broken for me. Issue is 100% reproducible:
>>
>> 1. Start system as usual with HDMI connected.
>> 2. Disconnect HDMI
>> 3. Connect HDMI back
>> 4. Get "no signal" on display, connector status in sysfs is disconnected
>>
>> Curiously, running xrandr over ssh like
>>
>> ssh qnap251.local env DISPLAY=:0 xrandr
>>
>> makes display come back. drm-tip tip is affected as well (last test
>> 2023-08-02).
>>
>> Bisecting points at a4e771729a51 ("drm/probe_helper: sort out poll_running
>> vs poll_enabled").
>> Reverting that commit on top of 6.3 fixes the issue for me.
>>
>> System information:
>> * System architecture: x86_64
>> * Kernel version: 6.3.arch1
>> * Linux distribution: Arch Linux
>> * Machine: QNAP TS-251A, CPU: Intel(R) Celeron(R) CPU N3060 @ 1.60GHz
>> * Display connector: single HDMI display
>> * dmesg with debug information (captured on drm-tip, following above 4
>> steps): [2]
>> * xrandr output:
>>
>> Screen 0: minimum 320 x 200, current 1920 x 1080, maximum 16384 x 16384
>> DP-1 disconnected (normal left inverted right x axis y axis)
>> HDMI-1 connected primary 1920x1080+0+0 (normal left inverted right x
>> axis y axis) 708mm x 398mm
>>1920x1080 60.00*+ 50.0059.9430.0025.0024.00
>> 29.9723.98
>>1920x1080i60.0050.0059.94
>>1360x768 59.80
>>1280x768 60.35
>>1280x720 60.0050.0059.94
>>1024x768 75.0370.0760.00
>>832x624 74.55
>>800x600 75.0060.32
>>720x576 50.00
>>720x480 60.0059.94
>>640x480 75.0060.0059.94
>>720x400 70.08
>> DP-2 disconnected (normal left inverted right x axis y axis)
>> HDMI-2 disconnected (normal left inverted right x axis y axis)```
>>
>> I'm willing to provide additional information and/or test fixes.
>>
>> [1] https://gitlab.freedesktop.org/drm/intel/-/issues/8451
>> [2]
>> https://gitlab.freedesktop.org/drm/intel/uploads/fda7aff0b13ef20962856c2c7be51544/dmesg.txt
>>
>> #regzbot introduced: a4e771729a51
>>
>> --
>> Best regards,
>> Mikhail Rudenko
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] dt-bindings: mxsfb: Exclude i.MX8MQ from power-domains requirement
On Thu, Aug 10, 2023 at 08:30:21PM +0100, Conor Dooley wrote:
> On Wed, Aug 02, 2023 at 11:51:17AM +0200, Guido Günther wrote:
> > Hi,
> > On Sun, Jul 30, 2023 at 09:41:20PM +0300, David Heidelberg wrote:
> > > i.MX8MQ uses as secondary compatible fsl,imx6sx-lcdif, which triggers
> > > requirement of power-domains, thou it's not required.
> > >
> > > Fixes: f62678a77d58 ("dt-bindings: mxsfb: Document i.MX8M/i.MX6SX/i.MX6SL
> > > power-domains property")
> > >
> > > Signed-off-by: David Heidelberg
> > > ---
> > > .../devicetree/bindings/display/fsl,lcdif.yaml| 11 +--
> > > 1 file changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> > > b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> > > index fc11ab5fc465..2d868276b0f9 100644
> > > --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> > > +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> > > @@ -129,8 +129,15 @@ allOf:
> > >- fsl,imx8mp-lcdif
> > >- fsl,imx93-lcdif
> > > then:
> > > - required:
> > > -- power-domains
> > > + if:
> > > +properties:
> > > + compatible:
> > > +not:
> > > + contains:
> > > +const: fsl,imx8mq-lcdif
> > > + then:
> > > +required:
> > > + - power-domains
> >
> > We currently enable the mipi power domain for the mipi dphy and nwl
> > bridge only but not for LCDIF itself assuming it's not needed (as there's
> > other outputs LCDIF can drive). I *think* this is correct but maybe
> > Marek or Liu can confirm?
>
> I'm happy to Ack this, but I've been sitting on it waiting to see if
> Marek or Liu popped up..
Well, I got two positive responses, so
Acked-by: Conor Dooley
Thanks,
Conor.
signature.asc
Description: PGP signature
Re: [REGRESSION] HDMI connector detection broken in 6.3 on Intel(R) Celeron(R) N3060 integrated graphics
[CCing the i915 maintainers and the dri maintainers]
Hi, Thorsten here, the Linux kernel's regression tracker.
On 10.08.23 21:33, Mikhail Rudenko wrote:
> The following is a copy an issue I posted to drm/i915 gitlab [1] two
> months ago. I repost it to the mailing lists in hope that it will help
> the right people pay attention to it.
Thx for your report. Wonder why Dmitry (who authored a4e771729a51) or
Thomas (who committed it) it didn't look into this, but maybe the i915
devs didn't forward the report to them.
Let's see if these mails help. Just wondering: does reverting
a4e771729a51 from 6.5-rc5 or drm-tip help as well?
BTW, there was an earlier report about a problem with a4e771729a51 that
afaics was never addressed, but it might be unrelated.
https://lore.kernel.org/all/20230328023129.3596968-1-zhouzong...@kylinos.cn/
Ciao, Thorsten
> After kernel upgrade from 6.2.13 to 6.3 HDMI connector detection is
> broken for me. Issue is 100% reproducible:
>
> 1. Start system as usual with HDMI connected.
> 2. Disconnect HDMI
> 3. Connect HDMI back
> 4. Get "no signal" on display, connector status in sysfs is disconnected
>
> Curiously, running xrandr over ssh like
>
> ssh qnap251.local env DISPLAY=:0 xrandr
>
> makes display come back. drm-tip tip is affected as well (last test
> 2023-08-02).
>
> Bisecting points at a4e771729a51 ("drm/probe_helper: sort out poll_running vs
> poll_enabled").
> Reverting that commit on top of 6.3 fixes the issue for me.
>
> System information:
> * System architecture: x86_64
> * Kernel version: 6.3.arch1
> * Linux distribution: Arch Linux
> * Machine: QNAP TS-251A, CPU: Intel(R) Celeron(R) CPU N3060 @ 1.60GHz
> * Display connector: single HDMI display
> * dmesg with debug information (captured on drm-tip, following above 4
> steps): [2]
> * xrandr output:
>
> Screen 0: minimum 320 x 200, current 1920 x 1080, maximum 16384 x 16384
> DP-1 disconnected (normal left inverted right x axis y axis)
> HDMI-1 connected primary 1920x1080+0+0 (normal left inverted right x axis
> y axis) 708mm x 398mm
>1920x1080 60.00*+ 50.0059.9430.0025.0024.00
> 29.9723.98
>1920x1080i60.0050.0059.94
>1360x768 59.80
>1280x768 60.35
>1280x720 60.0050.0059.94
>1024x768 75.0370.0760.00
>832x624 74.55
>800x600 75.0060.32
>720x576 50.00
>720x480 60.0059.94
>640x480 75.0060.0059.94
>720x400 70.08
> DP-2 disconnected (normal left inverted right x axis y axis)
> HDMI-2 disconnected (normal left inverted right x axis y axis)```
>
> I'm willing to provide additional information and/or test fixes.
>
> [1] https://gitlab.freedesktop.org/drm/intel/-/issues/8451
> [2]
> https://gitlab.freedesktop.org/drm/intel/uploads/fda7aff0b13ef20962856c2c7be51544/dmesg.txt
>
> #regzbot introduced: a4e771729a51
>
> --
> Best regards,
> Mikhail Rudenko
[git pull] drm fixes for 6.4-rc6
Hey Linus, This week's fixes, as expected amdgpu is probably a little larger since it skipped a week, but otherwise a few nouveau fixes, a couple of bridge, rockchip and ivpu fixes. Dave. drm-fixes-2023-08-11: drm fixes for 6.4-rc6 amdgpu: - S/G display workaround for platforms with >= 64G of memory - S0i3 fix - SMU 13.0.0 fixes - Disable SMU 13.x OD features temporarily while the interface is reworked to enable additional functionality - Fix cursor gamma issues on DCN3+ - SMU 13.0.6 fixes - Fix possible UAF in CS IOCTL - Polaris display regression fix - Only enable CP GFX shadowing on SR-IOV amdkfd: - Raven/Picasso KFD regression fix bridge: - it6505: runtime PM fix - lt9611: revert Do not generate HFP/HBP/HSA and EOT packet nouveau: - enable global memory loads for helper invocations for userspace driver - dp 1.3 dpcd+ workaround fix - remove unused function - revert incorrect NULL check accel/ivpu: - Add set_pages_array_wc/uc for internal buffers rockchip: - Don't spam logs in atomic check The following changes since commit 52a93d39b17dc7eb98b6aa3edb93943248e03b2f: Linux 6.5-rc5 (2023-08-06 15:07:51 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2023-08-11 for you to fetch changes up to fbe8ff726a1de82d87524f306b0f6491e13d7dfa: Merge tag 'amd-drm-fixes-6.5-2023-08-09' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes (2023-08-11 14:49:20 +1000) drm fixes for 6.4-rc6 amdgpu: - S/G display workaround for platforms with >= 64G of memory - S0i3 fix - SMU 13.0.0 fixes - Disable SMU 13.x OD features temporarily while the interface is reworked to enable additional functionality - Fix cursor gamma issues on DCN3+ - SMU 13.0.6 fixes - Fix possible UAF in CS IOCTL - Polaris display regression fix - Only enable CP GFX shadowing on SR-IOV amdkfd: - Raven/Picasso KFD regression fix bridge: - it6505: runtime PM fix - lt9611: revert Do not generate HFP/HBP/HSA and EOT packet nouveau: - enable global memory loads for helper invocations for userspace driver - dp 1.3 dpcd+ workaround fix - remove unused function - revert incorrect NULL check accel/ivpu: - Add set_pages_array_wc/uc for internal buffers rockchip: - Don't spam logs in atomic check Alex Deucher (5): drm/amdgpu: fix possible UAF in amdgpu_cs_pass1() drm/amdgpu/gfx11: only enable CP GFX shadowing on SR-IOV drm/amdkfd: ignore crat by default drm/amdkfd: disable IOMMUv2 support for KV/CZ drm/amdkfd: disable IOMMUv2 support for Raven Arnd Bergmann (1): drm/nouveau: remove unused tu102_gr_load() function Boris Brezillon (1): drm/shmem-helper: Reset vma->vm_ops before calling dma_buf_mmap() Daniel Stone (1): drm/rockchip: Don't spam logs in atomic check Dave Airlie (2): Merge tag 'drm-misc-fixes-2023-08-10' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'amd-drm-fixes-6.5-2023-08-09' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Evan Quan (1): drm/amd/pm: disable the SMU13 OD feature support temporarily Karol Herbst (2): drm/nouveau/gr: enable memory loads on helper invocation on all channels drm/nouveau/disp: Revert a NULL check inside nouveau_connector_get_modes Karol Wachowski (1): accel/ivpu: Add set_pages_array_wc/uc for internal buffers Kenneth Feng (1): drm/amd/pm: correct the pcie width for smu 13.0.0 Lijo Lazar (2): drm/amdgpu: Match against exact bootloader status drm/amd/pm: Fix SMU v13.0.6 energy reporting Lyude Paul (1): drm/nouveau/nvkm/dp: Add workaround to fix DP 1.3+ DPCD issues Mario Limonciello (3): drm/amd: Disable S/G for APUs when 64GB or more host memory drm/amd/display: Don't show stack trace for missing eDP drm/amd/display: Fix a regression on Polaris cards Melissa Wen (1): drm/amd/display: check attr flag before set cursor degamma on DCN3+ Neil Armstrong (1): Revert "drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet" Pin-yen Lin (1): drm/bridge: it6505: Check power state with it6505->powered in IRQ handler Tim Huang (1): drm/amd/pm: skip the RLC stop when S0i3 suspend for SMU v13.0.4/11 drivers/accel/ivpu/ivpu_gem.c | 8 drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 26 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 8 +++- drivers/gpu/drm/amd/amdgpu/psp_v13_0.c | 15 +++ drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 4 -- drivers/gpu/drm/amd/amdkfd/kfd_device.c| 13 -- .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 +--- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +--
