Re: [PATCH] drm/i915/gem: Allow users to disable waitboost
Hello,
kernel test robot noticed "assertion_failure" on:
commit: 54fef7ea35dadd66193b98805b0bc42ef2b279db ("[PATCH] drm/i915/gem: Allow
users to disable waitboost")
url:
https://github.com/intel-lab-lkp/linux/commits/Vinay-Belgaumkar/drm-i915-gem-Allow-users-to-disable-waitboost/20230921-060357
base: git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:
https://lore.kernel.org/all/20230920215624.3482244-1-vinay.belgaum...@intel.com/
patch subject: [PATCH] drm/i915/gem: Allow users to disable waitboost
in testcase: igt
version: igt-x86_64-0f075441-1_20230520
with following parameters:
group: group-23
compiler: gcc-12
test machine: 20 threads 1 sockets (Commet Lake) with 16G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
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-lkp/202310271326.abb748d0-oliver.s...@intel.com
user :notice: [ 42.847071] Starting subtest: engines-cleanup
user :notice: [ 42.854776] Starting dynamic subtest: rcs0
user :notice: [ 42.863938] (gem_ctx_persistence:833) CRITICAL: Test
assertion failure function test_nonpersistent_cleanup, file
../tests/i915/gem_ctx_persistence.c:283:
user :notice: [ 42.882029] (gem_ctx_persistence:833) CRITICAL: Failed
assertion: gem_wait(i915, spin->handle, ) == 0
user :notice: [ 42.895541] (gem_ctx_persistence:833) CRITICAL: error: -22 != 0
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20231027/202310271326.abb748d0-oliver.s...@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[drm:topic/nvidia-gsp 41/48] disp.c:undefined reference to `__udivdi3'
tree: git://anongit.freedesktop.org/drm/drm topic/nvidia-gsp head: c09ddadcb05445b46e1dd0554a3ee1a96328 commit: e9ad7a2f99667b6ba6ac966050e5d7d6b5e485dd [41/48] drm/nouveau/disp/r535: initial support config: powerpc-randconfig-003-20231026 (https://download.01.org/0day-ci/archive/20231027/202310271316.3nkc9uv0-...@intel.com/config) compiler: powerpc-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231027/202310271316.3nkc9uv0-...@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/202310271316.3nkc9uv0-...@intel.com/ All errors (new ones prefixed by >>): powerpc-linux-ld: drivers/gpu/drm/nouveau/dispnv50/disp.o: in function `nv50_sor_dp_watermark_sst.isra.0': >> disp.c:(.text+0x1710): undefined reference to `__udivdi3' >> powerpc-linux-ld: disp.c:(.text+0x1768): undefined reference to `__udivdi3' powerpc-linux-ld: disp.c:(.text+0x1790): undefined reference to `__udivdi3' powerpc-linux-ld: disp.c:(.text+0x17b4): undefined reference to `__udivdi3' powerpc-linux-ld: disp.c:(.text+0x1864): undefined reference to `__udivdi3' powerpc-linux-ld: drivers/gpu/drm/nouveau/dispnv50/disp.o:disp.c:(.text+0x1928): more undefined references to `__udivdi3' follow -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
[PATCH v2] drm/atomic-helper: Fix spelling mistake "preceeding" -> "preceding"
From: Kunwu Chan There is a typo in the kernel documentation for function drm_atomic_helper_wait_for_dependencies. Fix it. Signed-off-by: Kunwu Chan --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2444fc33dd7c..c3f677130def 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2382,10 +2382,10 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, EXPORT_SYMBOL(drm_atomic_helper_setup_commit); /** - * drm_atomic_helper_wait_for_dependencies - wait for required preceeding commits + * drm_atomic_helper_wait_for_dependencies - wait for required preceding commits * @old_state: atomic state object with old state structures * - * This function waits for all preceeding commits that touch the same CRTC as + * This function waits for all preceding commits that touch the same CRTC as * @old_state to both be committed to the hardware (as signalled by * drm_atomic_helper_commit_hw_done()) and executed by the hardware (as signalled * by calling drm_crtc_send_vblank_event() on the _crtc_state.event). -- 2.34.1
[drm:topic/nvidia-gsp 41/48] ld: disp.c:undefined reference to `__udivdi3'
tree: git://anongit.freedesktop.org/drm/drm topic/nvidia-gsp head: c09ddadcb05445b46e1dd0554a3ee1a96328 commit: e9ad7a2f99667b6ba6ac966050e5d7d6b5e485dd [41/48] drm/nouveau/disp/r535: initial support config: i386-buildonly-randconfig-002-20231027 (https://download.01.org/0day-ci/archive/20231027/202310271011.kl4e5dy2-...@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231027/202310271011.kl4e5dy2-...@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/202310271011.kl4e5dy2-...@intel.com/ All errors (new ones prefixed by >>): ld: drivers/gpu/drm/nouveau/dispnv50/disp.o: in function `nv50_sor_dp_watermark_sst.isra.0': disp.c:(.text+0x18c3): undefined reference to `__udivdi3' >> ld: disp.c:(.text+0x1943): undefined reference to `__udivdi3' ld: disp.c:(.text+0x195f): undefined reference to `__udivdi3' ld: disp.c:(.text+0x197d): undefined reference to `__udivdi3' ld: disp.c:(.text+0x19eb): undefined reference to `__udivdi3' ld: drivers/gpu/drm/nouveau/dispnv50/disp.o:disp.c:(.text+0x1a09): more undefined references to `__udivdi3' follow -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH] drm/atomic: Spelling fix in comments
Hi Hamza, Thank you very much for your guidance and advice to me, I have revised it according to your suggestion. On 2023/10/25 22:50, Hamza Mahfooz wrote: Hi Kunwu, Can you make the tagline something along the lines of `drm/atomic helper: fix spelling mistake "preceeding" -> "preceding"`, in general to determine an appropriate prefix, you can see what previous commits used when making changes to files in your particular patch, e.g. using the following: $ git log drivers/gpu/drm/drm_atomic_helper.c On 10/25/23 04:26, Kunwu Chan wrote: fix a typo in a comments. For patch descriptions you should try to more descriptive. So, something like "There is a spelling mistake in drm_atomic_helper_wait_for_dependencies()'s kernel doc. Fix it." would be better. Signed-off-by: Kunwu Chan --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2444fc33dd7c..c3f677130def 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2382,10 +2382,10 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, EXPORT_SYMBOL(drm_atomic_helper_setup_commit); /** - * drm_atomic_helper_wait_for_dependencies - wait for required preceeding commits + * drm_atomic_helper_wait_for_dependencies - wait for required preceding commits * @old_state: atomic state object with old state structures * - * This function waits for all preceeding commits that touch the same CRTC as + * This function waits for all preceding commits that touch the same CRTC as * @old_state to both be committed to the hardware (as signalled by * drm_atomic_helper_commit_hw_done()) and executed by the hardware (as signalled * by calling drm_crtc_send_vblank_event() on the _crtc_state.event).
Re: [PATCH 1/1] drm/mediatek: Fix errors when reporting rotation capability
Re: [PATCH drm-misc-next v3] drm/sched: implement dynamic job-flow control
On 2023-10-26 17:13, Luben Tuikov wrote: > On 2023-10-26 12:13, Danilo Krummrich wrote: >> Currently, job flow control is implemented simply by limiting the number >> of jobs in flight. Therefore, a scheduler is initialized with a credit >> limit that corresponds to the number of jobs which can be sent to the >> hardware. >> >> This implies that for each job, drivers need to account for the maximum >> job size possible in order to not overflow the ring buffer. >> >> However, there are drivers, such as Nouveau, where the job size has a >> rather large range. For such drivers it can easily happen that job >> submissions not even filling the ring by 1% can block subsequent >> submissions, which, in the worst case, can lead to the ring run dry. >> >> In order to overcome this issue, allow for tracking the actual job size >> instead of the number of jobs. Therefore, add a field to track a job's >> credit count, which represents the number of credits a job contributes >> to the scheduler's credit limit. >> >> Signed-off-by: Danilo Krummrich >> --- >> Changes in V2: >> == >> - fixed up influence on scheduling fairness due to consideration of a job's >> size >> - If we reach a ready entity in drm_sched_select_entity() but can't >> actually >> queue a job from it due to size limitations, just give up and go to >> sleep >> until woken up due to a pending job finishing, rather than continue to >> try >> other entities. >> - added a callback to dynamically update a job's credits (Boris) >> - renamed 'units' to 'credits' >> - fixed commit message and comments as requested by Luben >> >> Changes in V3: >> == >> - rebased onto V7 of the "DRM scheduler changes for Xe" series by Matt >> - move up drm_sched_can_queue() instead of adding a forward declaration >> (Boris) >> - add a drm_sched_available_credits() helper (Boris) >> - adjust control flow in drm_sched_rq_select_entity_fifo() to Luben's >> proposal >> - re-phrase a few comments and fix a typo (Luben) >> - change naming of all structures credit fields and function parameters to >> the >> following scheme >> - drm_sched_job::credits >> - drm_gpu_scheduler::credit_limit >> - drm_gpu_scheduler::credit_count >> - drm_sched_init(..., u32 credit_limit, ...) >> - drm_sched_job_init(..., u32 credits, ...) >> - add proper documentation for the scheduler's job-flow control mechanism >> >> This patch is based on V7 of the "DRM scheduler changes for Xe" series. [1] >> provides a branch based on drm-misc-next, with the named series and this >> patch >> on top of it. >> >> [1] https://gitlab.freedesktop.org/nouvelles/kernel/-/commits/sched-credits/ >> --- >> Documentation/gpu/drm-mm.rst | 6 + >> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- >> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- >> drivers/gpu/drm/lima/lima_sched.c | 2 +- >> drivers/gpu/drm/msm/msm_gem_submit.c | 2 +- >> drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +- >> drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- >> .../gpu/drm/scheduler/gpu_scheduler_trace.h | 2 +- >> drivers/gpu/drm/scheduler/sched_entity.c | 4 +- >> drivers/gpu/drm/scheduler/sched_main.c| 142 ++ >> drivers/gpu/drm/v3d/v3d_gem.c | 2 +- >> include/drm/gpu_scheduler.h | 33 +++- >> 12 files changed, 152 insertions(+), 49 deletions(-) >> >> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst >> index 602010cb6894..acc5901ac840 100644 >> --- a/Documentation/gpu/drm-mm.rst >> +++ b/Documentation/gpu/drm-mm.rst >> @@ -552,6 +552,12 @@ Overview >> .. kernel-doc:: drivers/gpu/drm/scheduler/sched_main.c >> :doc: Overview >> >> +Flow Control >> + >> + >> +.. kernel-doc:: drivers/gpu/drm/scheduler/sched_main.c >> + :doc: Flow Control >> + >> Scheduler Function References >> - >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c >> index 1f357198533f..62bb7fc7448a 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c >> @@ -115,7 +115,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct >> amdgpu_vm *vm, >> if (!entity) >> return 0; >> >> -return drm_sched_job_init(&(*job)->base, entity, owner); >> +return drm_sched_job_init(&(*job)->base, entity, 1, owner); >> } >> >> int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, >> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c >> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c >> index 2416c526f9b0..3d0f8d182506 100644 >> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c >> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c >> @@ -535,7 +535,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, >> void *data, >> >>
Re: drm/panel: panel-simple power-off sequencing
Hi, On Thu, Oct 26, 2023 at 7:37 AM Jonas Mark (BT-FS/ENG1-GRB) wrote: > > Hi, > > We have a parallel LCD panel which is driven by panel/panel-simple. The > power-off sequence specified in the datasheet requires that the enable-gpio > must be deasserted for a number of VSYNC cycles before shutting down all > other control signals. See the diagram below: > __ __ __ __ __ > CLK, VSYNC, DE, HSYNC: __><__><__><__><__\_ > __ > enable-gpio :\_ > > So far, in kernel 5.4 we relied on the unprepare delay time for making sure > that the enable-gpio timing requirements are fulfilled. That is, the > panel_simple_unprepare() would: > > 1. Deassert the enable-gpio > 2. Switch off the voltage regulator > 3. Wait display_desc.delay.unprepare milliseconds > > Afterwards the IPU was shutdown, and all the control signals stopped. > > But with the below commits: > > - 3235b0f20a0a4135e9053f1174d096eff166d0fb >"drm/panel: panel-simple: Use runtime pm to avoid excessive unprepare / > prepare" > - e5e30dfcf3db1534019c40d94ed58fd2869f9359 >"drm: panel: simple: Defer unprepare delay till next prepare to shorten it" > > The enable-gpio is now deasserted in panel_simple_suspend(), which is called > some time after the disablement of control signals are stopped: > __ __ __ __ __ > CLK, VSYNC, DE, HSYNC: __><__><__><__><__\_ > __ > enable-gpio :\_ > > With the latest panel-simple, is there a way which allows us to deassert > enable-gpio before the control signals stop? As I understood it, the "unprepare" time was originally intended to meet minimum power off timings and that's how I always saw it used, but it doesn't totally surprise me that there was someone relying on the old behavior. I personally wouldn't object to adding another field to panel-simple that allowed you to get the delay you needed and then change your panel details to use that new field instead of the "unprepare" milliseconds. ...or you could rename the current "unprepare" delay to something like "min_poweroff" and then re-introduce an "unprepare" delay that does what you want. Oh! ...but even this won't _really_ do what you want, right? The bigger issue here is that panel-simple is using auto-suspend now and thus the enable line can go off much, much later. What it kind-of sounds like is that you want the "enable" GPIO to be controlled by the "enable" and "disable" functions of panel-simple. Then you could use the "disable" delay, right? I think I've looked at this exact case before and then realized that there's a better solution. At least in all cases I looked at the "enable-gpio" you're talking about was actually better modeled as a _backlight_ enable GPIO. The "backlight" is turned off before panel-simple's disable() function is called (see drm_panel_disable(). So if you move the GPIO to the backlight and add a "disable" delay then you're all set. Does that work for you? Does it make sense for this GPIO to be modeled as a backlight GPIO? -Doug
[PATCH] drm/i915: Skip pxp init if gt is wedged
gt wedged is fatal error, skip the pxp init on this situation. Signed-off-by: Zhanjun Dong --- drivers/gpu/drm/i915/pxp/intel_pxp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index dc327cf40b5a..923f233c91e1 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -212,6 +212,9 @@ int intel_pxp_init(struct drm_i915_private *i915) if (!gt) return -ENODEV; + if (intel_gt_is_wedged(gt)) + return -ENODEV; + /* * At this point, we will either enable full featured PXP capabilities * including session and object management, or we will init the backend tee -- 2.34.1
[PATCH] staging: fbtft: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
The function fbtft_driver_remove_pdev() (that exists several times as it's
part of a macro expansion) returns zero unconditionally, so it can be
trivially converted to return void without semantic changes.
Signed-off-by: Uwe Kleine-König
---
drivers/staging/fbtft/fbtft.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 2c2b5f1c1df3..f86ed9d470b8 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -310,12 +310,11 @@ static int fbtft_driver_probe_pdev(struct platform_device
*pdev) \
return fbtft_probe_common(_display, NULL, pdev); \
} \
\
-static int fbtft_driver_remove_pdev(struct platform_device *pdev) \
+static void fbtft_driver_remove_pdev(struct platform_device *pdev)\
{ \
struct fb_info *info = platform_get_drvdata(pdev); \
\
fbtft_remove_common(>dev, info); \
- return 0; \
} \
\
FBTFT_DT_TABLE(_compatible) \
@@ -329,7 +328,7 @@ static struct platform_driver fbtft_driver_platform_driver
= { \
.of_match_table = dt_ids, \
}, \
.probe = fbtft_driver_probe_pdev, \
- .remove = fbtft_driver_remove_pdev,\
+ .remove_new = fbtft_driver_remove_pdev,\
}; \
\
static int __init fbtft_driver_module_init(void) \
base-commit: 2ef7141596eed0b4b45ef18b3626f428a6b0a822
--
2.42.0.482.g2e8e77cbac8a.dirty
[PATCH] drm/amdgpu: Fixes uninitialized variable usage in amdgpu_dm_setup_replay
Since `pr_config` is not initialized after its declaration, the
following operations with `replay_enable_option` may be performed
when `replay_enable_option` is holding junk values which could
possibly lead to undefined behaviour
```
...
pr_config.replay_enable_option |= pr_enable_option_static_screen;
...
if (!pr_config.replay_timing_sync_supported)
pr_config.replay_enable_option &= ~pr_enable_option_general_ui;
...
```
This patch initializes `pr_config` after its declaration to ensure that
it doesn't contain junk data, and prevent any undefined behaviour
Addresses-Coverity-ID: 1544428 ("Uninitialized scalar variable")
Fixes: dede1fea4460 ("drm/amd/display: Add Freesync Panel DM code")
Signed-off-by: Yuran Pereira
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
index 32d3086c4cb7..40526507f50b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
@@ -23,6 +23,7 @@
*
*/
+#include
#include "amdgpu_dm_replay.h"
#include "dc.h"
#include "dm_helpers.h"
@@ -74,6 +75,8 @@ bool amdgpu_dm_setup_replay(struct dc_link *link, struct
amdgpu_dm_connector *ac
struct replay_config pr_config;
union replay_debug_flags *debug_flags = NULL;
+ memset(_config, 0, sizeof(pr_config));
+
// For eDP, if Replay is supported, return true to skip checks
if (link->replay_settings.config.replay_supported)
return true;
--
2.25.1
Re: [PATCH drm-misc-next v3] drm/sched: implement dynamic job-flow control
On 2023-10-26 12:13, Danilo Krummrich wrote: > Currently, job flow control is implemented simply by limiting the number > of jobs in flight. Therefore, a scheduler is initialized with a credit > limit that corresponds to the number of jobs which can be sent to the > hardware. > > This implies that for each job, drivers need to account for the maximum > job size possible in order to not overflow the ring buffer. > > However, there are drivers, such as Nouveau, where the job size has a > rather large range. For such drivers it can easily happen that job > submissions not even filling the ring by 1% can block subsequent > submissions, which, in the worst case, can lead to the ring run dry. > > In order to overcome this issue, allow for tracking the actual job size > instead of the number of jobs. Therefore, add a field to track a job's > credit count, which represents the number of credits a job contributes > to the scheduler's credit limit. > > Signed-off-by: Danilo Krummrich > --- > Changes in V2: > == > - fixed up influence on scheduling fairness due to consideration of a job's > size > - If we reach a ready entity in drm_sched_select_entity() but can't > actually > queue a job from it due to size limitations, just give up and go to > sleep > until woken up due to a pending job finishing, rather than continue to > try > other entities. > - added a callback to dynamically update a job's credits (Boris) > - renamed 'units' to 'credits' > - fixed commit message and comments as requested by Luben > > Changes in V3: > == > - rebased onto V7 of the "DRM scheduler changes for Xe" series by Matt > - move up drm_sched_can_queue() instead of adding a forward declaration > (Boris) > - add a drm_sched_available_credits() helper (Boris) > - adjust control flow in drm_sched_rq_select_entity_fifo() to Luben's > proposal > - re-phrase a few comments and fix a typo (Luben) > - change naming of all structures credit fields and function parameters to > the > following scheme > - drm_sched_job::credits > - drm_gpu_scheduler::credit_limit > - drm_gpu_scheduler::credit_count > - drm_sched_init(..., u32 credit_limit, ...) > - drm_sched_job_init(..., u32 credits, ...) > - add proper documentation for the scheduler's job-flow control mechanism > > This patch is based on V7 of the "DRM scheduler changes for Xe" series. [1] > provides a branch based on drm-misc-next, with the named series and this patch > on top of it. > > [1] https://gitlab.freedesktop.org/nouvelles/kernel/-/commits/sched-credits/ > --- > Documentation/gpu/drm-mm.rst | 6 + > drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- > drivers/gpu/drm/lima/lima_sched.c | 2 +- > drivers/gpu/drm/msm/msm_gem_submit.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- > .../gpu/drm/scheduler/gpu_scheduler_trace.h | 2 +- > drivers/gpu/drm/scheduler/sched_entity.c | 4 +- > drivers/gpu/drm/scheduler/sched_main.c| 142 ++ > drivers/gpu/drm/v3d/v3d_gem.c | 2 +- > include/drm/gpu_scheduler.h | 33 +++- > 12 files changed, 152 insertions(+), 49 deletions(-) > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > index 602010cb6894..acc5901ac840 100644 > --- a/Documentation/gpu/drm-mm.rst > +++ b/Documentation/gpu/drm-mm.rst > @@ -552,6 +552,12 @@ Overview > .. kernel-doc:: drivers/gpu/drm/scheduler/sched_main.c > :doc: Overview > > +Flow Control > + > + > +.. kernel-doc:: drivers/gpu/drm/scheduler/sched_main.c > + :doc: Flow Control > + > Scheduler Function References > - > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c > index 1f357198533f..62bb7fc7448a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c > @@ -115,7 +115,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct > amdgpu_vm *vm, > if (!entity) > return 0; > > - return drm_sched_job_init(&(*job)->base, entity, owner); > + return drm_sched_job_init(&(*job)->base, entity, 1, owner); > } > > int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > index 2416c526f9b0..3d0f8d182506 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > @@ -535,7 +535,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void > *data, > > ret = drm_sched_job_init(>sched_job, >>sched_entity[args->pipe], > - submit->ctx);
Re: [PATCH v3 05/15] phy: qualcomm: add legacy HDMI PHY driver
On 9/28/23 13:16, Dmitry Baryshkov wrote:
Add the driver for pre-QMP Qualcomm HDMI PHYs. Currently it suppports
Qualcomm MSM8960 / APQ8064 platforms, other platforms will come later.
Signed-off-by: Dmitry Baryshkov
---
[...]
+{
+ unsigned int pixclk = hdmi_phy->hdmi_opts.pixel_clk_rate;
+unsigned int int_ref_freq;
there's some misaligned things
[...]
+ lf_cfg0 = dc_offset >= 30 ? 0 : (dc_offset >= 16 ? 0x10 : 0x20);
some unreadable things
[...]
+ hdmi_pll_write(hdmi_phy, REG_HDMI_8960_PHY_PLL_SDM_CFG2,
+ sdm_freq_seed & 0xff);
+
+ hdmi_pll_write(hdmi_phy, REG_HDMI_8960_PHY_PLL_SDM_CFG3,
+ (sdm_freq_seed >> 8) & 0xff);
+
+ hdmi_pll_write(hdmi_phy, REG_HDMI_8960_PHY_PLL_SDM_CFG4,
+ sdm_freq_seed >> 16);
and a lot of magic bits in this submission
and some non-reverse-Christmas-trees
and some leftover comments like /* XXX: 19.2 for qcs404 */
I see a lot of RMW, maybe regmap could help make this more readable
on the !style front, it looks sane to an unknowing eye, so I guess
that's the end of my complaints
Konrad
Re: [PATCH v3 15/15] drm/msm/hdmi: drop old HDMI PHY code
On 10/26/23 23:03, Dmitry Baryshkov wrote: On Fri, 27 Oct 2023 at 00:00, Konrad Dybcio wrote: On 9/28/23 13:16, Dmitry Baryshkov wrote: Drop source files used by old HDMI PHY and HDMI PLL drivers. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 216 --- drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c | 51 -- drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c | 765 --- drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c | 141 - drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c | 44 -- drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c | 458 -- 6 files changed, 1675 deletions(-) delete mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy.c delete mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c delete mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c Uh-oh, is the 8996 HDMI phy accounted for somwhere else? Yes, it is the QMP PHY now. Right, I realized that as soon as I've seen that you replied :D Konrad
Re: [PATCH v3 02/15] phy: qualcomm: add QMP HDMI PHY driver
On 9/28/23 13:16, Dmitry Baryshkov wrote: Port Qualcomm QMP HDMI PHY to the generic PHY framework. Split the generic part and the msm8996 part. When adding support for msm8992/4 and msm8998 (which also employ QMP for HDMI PHY), one will have to provide the PLL programming part only. Signed-off-by: Dmitry Baryshkov --- Taking a quick look, my comments from v2 were not taken into account https://lore.kernel.org/linux-arm-msm/1513ea17-2807-4f7c-30f2-6158b5f3e...@linaro.org/ Konrad
Re: [PATCH v3 15/15] drm/msm/hdmi: drop old HDMI PHY code
On Fri, 27 Oct 2023 at 00:00, Konrad Dybcio wrote: > > > > On 9/28/23 13:16, Dmitry Baryshkov wrote: > > Drop source files used by old HDMI PHY and HDMI PLL drivers. > > > > Signed-off-by: Dmitry Baryshkov > > --- > > drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 216 --- > > drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c | 51 -- > > drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c | 765 --- > > drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c | 141 - > > drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c | 44 -- > > drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c | 458 -- > > 6 files changed, 1675 deletions(-) > > delete mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy.c > > delete mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c > > delete mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c > Uh-oh, is the 8996 HDMI phy accounted for somwhere else? Yes, it is the QMP PHY now. -- With best wishes Dmitry
Re: [PATCH v3 15/15] drm/msm/hdmi: drop old HDMI PHY code
On 9/28/23 13:16, Dmitry Baryshkov wrote: Drop source files used by old HDMI PHY and HDMI PLL drivers. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 216 --- drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c | 51 -- drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c | 765 --- drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c | 141 - drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c | 44 -- drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c | 458 -- 6 files changed, 1675 deletions(-) delete mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy.c delete mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c delete mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c Uh-oh, is the 8996 HDMI phy accounted for somwhere else? Konrad
Re: [PATCH v3 14/15] drm/msm/hdmi: switch to generic PHY subsystem
On 9/28/23 13:16, Dmitry Baryshkov wrote: Change the MSM HDMI driver to use generic PHY subsystem. Moving PHY drivers allows better code sharing with the rest of the PHY system. Signed-off-by: Dmitry Baryshkov --- Looks like this will require some atomicity with the phy changes Acked-by: Konrad Dybcio Konrad
Re: [PATCH v3 13/15] drm/msm/hdmi: pair msm_hdmi_phy_powerup with msm_hdmi_phy_powerdown
On Thu, 26 Oct 2023 at 22:54, Konrad Dybcio wrote: > > > > On 9/28/23 13:16, Dmitry Baryshkov wrote: > > In preparation to converting MSM HDMI driver to use PHY framework, which > > requires phy_power_on() calls to be paired with phy_power_off(), add a > > conditional call to msm_hdmi_phy_powerdown() before the call to > > msm_hdmi_phy_powerup(). > > > > Signed-off-by: Dmitry Baryshkov > > --- > Is this a conversion artifact that will be undone, or does the > framework actually expect that refcounting may not be enough and > phy resetting will have to take place? I don't remember why I did it this way. Let me check, most likely this patch can be completely dropped as the enable / disable operations are paired by the DRM core. -- With best wishes Dmitry
Re: [PATCH v2] drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full
Hi Chris,
On 2023-10-26 22:02, Christopher Obbard wrote:
> Hi Jonas,
>
> On Thu, 2023-10-26 at 19:14 +, Jonas Karlman wrote:
>> Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328
>> and RK3399 result in wrong colors being displayed.
>>
>> The issue can be observed using modetest:
>>
>> modetest -s @:1920x1080-60@RG24
>> modetest -s @:1920x1080-60@BG24
>>
>> Vendor 4.4 kernel apply an inverted rb swap for these formats on VOP
>> full framework (IP version 3.x) compared to VOP little framework (2.x).
>>
>> Fix colors by applying different rb swap for VOP full framework (3.x)
>> and VOP little framework (2.x) similar to vendor 4.4 kernel.
>>
>> Fixes: 85a359f25388 ("drm/rockchip: Add BGR formats to VOP")
>> Signed-off-by: Jonas Karlman
>
> Reviewed-by: Christopher Obbard
> Tested-by: Christopher Obbard
>
> Since you missed adding my *-by tags in v2.
>
Thanks, and sorry about that ;-)
Regards,
Jonas
Re: [RFC] drm: bridge: samsung-dsim: Recalculate timings when rounding HFP up
On Thu, Oct 26, 2023 at 1:12 PM Frieder Schrempf wrote: > > Hi Adam, > > On 13.10.23 05:10, Adam Ford wrote: > > When using video sync pulses, the HFP, HBP, and HSA are divided between > > the available lanes if there is more than one lane. For certain > > timings and lane configurations, the HFP may not be evenly divisible > > and it gets rounded down which can cause certain resolutions and > > refresh rates to not sync. > > > > ie. 720p60 on some monitors show hss of 1390 and hdisplay of 1280. This > > yields an HFP of 110. When taking the HFP of 110 divides along 4 lanes, > > the result is 27.5 which rounds down to 27 and causes some monitors not > > to sync. > > > > The solultion is to round HFP up by finding the remainder of HFP / > > the number of lanes and increasing the hsync_start, hsync_end, and > > htotal to compensate when there is a remainder. > > > > Signed-off-by: Adam Ford > > --- > > This adds support for 720p60 in the i.MX8M Plus. > > > > NXP uses a look-up table in their downstream code to accomplish this. > > Using this calculation, the driver can adjust without the need for a > > complicated table and should be flexible for different timings and > > resolutions depending on the monitor. > > > > I don't have a DSI analyzer, and this appears to only work on > > i.MX8M Plus but not Mini and Nano for some reason, despite their > > having a similar DSI bridge. > > I just want to report that I have tested this patch (on top of current > linux-next) on our Kontron BL i.MX8MM board with the ADV7535 bridge and > I don't see any change when trying the 30 different modes the monitor > reports as supported. 18 of those work and 12 don't work. Thanks for testing this. I didn't see any regressions on my Mini or Nano either, but I did see the 720p60 now works on i.MX8M Plus (but not on Mini/Nano). I am not sure why, and I don't have a DSI analyzer. > > So at least there is no negative impact in this case. At least nothing broke. :-) > > Tested-by: Frieder Schrempf # Kontron BL > i.MX8MM with HDMI monitor I'll add your T-b when I post it again. > > Thanks > Frieder
Re: [PATCH v2] drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full
Hi Jonas,
On Thu, 2023-10-26 at 19:14 +, Jonas Karlman wrote:
> Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328
> and RK3399 result in wrong colors being displayed.
>
> The issue can be observed using modetest:
>
> modetest -s @:1920x1080-60@RG24
> modetest -s @:1920x1080-60@BG24
>
> Vendor 4.4 kernel apply an inverted rb swap for these formats on VOP
> full framework (IP version 3.x) compared to VOP little framework (2.x).
>
> Fix colors by applying different rb swap for VOP full framework (3.x)
> and VOP little framework (2.x) similar to vendor 4.4 kernel.
>
> Fixes: 85a359f25388 ("drm/rockchip: Add BGR formats to VOP")
> Signed-off-by: Jonas Karlman
Reviewed-by: Christopher Obbard
Tested-by: Christopher Obbard
Since you missed adding my *-by tags in v2.
> ---
> Changes in v2:
> - Add comment about different rb swap for IP version 3.x and 2.x
> - Add fixes tag
>
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index b3d0b6ae9294..ed2ed25959a2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -247,14 +247,22 @@ static inline void vop_cfg_done(struct vop *vop)
> VOP_REG_SET(vop, common, cfg_done, 1);
> }
>
> -static bool has_rb_swapped(uint32_t format)
> +static bool has_rb_swapped(uint32_t version, uint32_t format)
> {
> switch (format) {
> case DRM_FORMAT_XBGR:
> case DRM_FORMAT_ABGR:
> - case DRM_FORMAT_BGR888:
> case DRM_FORMAT_BGR565:
> return true;
> + /*
> + * full framework (IP version 3.x) only need rb swapped for RGB888
> and
> + * little framework (IP version 2.x) only need rb swapped for
> BGR888,
> + * check for 3.x to also only rb swap BGR888 for unknown vop
> version
> + */
> + case DRM_FORMAT_RGB888:
> + return VOP_MAJOR(version) == 3;
> + case DRM_FORMAT_BGR888:
> + return VOP_MAJOR(version) != 3;
> default:
> return false;
> }
> @@ -1035,7 +1043,7 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> VOP_WIN_SET(vop, win, dsp_info, dsp_info);
> VOP_WIN_SET(vop, win, dsp_st, dsp_st);
>
> - rb_swap = has_rb_swapped(fb->format->format);
> + rb_swap = has_rb_swapped(vop->data->version, fb->format->format);
> VOP_WIN_SET(vop, win, rb_swap, rb_swap);
>
> /*
Re: [PATCH v3 13/15] drm/msm/hdmi: pair msm_hdmi_phy_powerup with msm_hdmi_phy_powerdown
On 9/28/23 13:16, Dmitry Baryshkov wrote: In preparation to converting MSM HDMI driver to use PHY framework, which requires phy_power_on() calls to be paired with phy_power_off(), add a conditional call to msm_hdmi_phy_powerdown() before the call to msm_hdmi_phy_powerup(). Signed-off-by: Dmitry Baryshkov --- Is this a conversion artifact that will be undone, or does the framework actually expect that refcounting may not be enough and phy resetting will have to take place? Konrad
Re: [PATCH v3 11/15] drm/msm/hdmi: switch to atomic_pre_enable/post_disable
On 9/28/23 13:16, Dmitry Baryshkov wrote: In preparation of reworking the HDMI mode setting, switch pre_enable and post_disable callbacks to their atomic variants. Signed-off-by: Dmitry Baryshkov --- This looks good, but I'm far from knowledgeable in terms of drm, so: Acked-by: Konrad Dybcio Konrad
Re: [PATCH v3 10/15] drm/msm/hdmi: correct indentation of HDMI bridge functions
On 9/28/23 13:16, Dmitry Baryshkov wrote: Signed-off-by: Dmitry Baryshkov --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH v3 09/15] drm/msm/hdmi: simplify extp clock handling
On 9/28/23 13:16, Dmitry Baryshkov wrote: With the extp being the only "power" clock left, remove the surrounding loops and handle the extp clock directly. Signed-off-by: Dmitry Baryshkov ---Reviewed-by: Konrad Dybcio Konrad
[PATCH v2] drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full
Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328
and RK3399 result in wrong colors being displayed.
The issue can be observed using modetest:
modetest -s @:1920x1080-60@RG24
modetest -s @:1920x1080-60@BG24
Vendor 4.4 kernel apply an inverted rb swap for these formats on VOP
full framework (IP version 3.x) compared to VOP little framework (2.x).
Fix colors by applying different rb swap for VOP full framework (3.x)
and VOP little framework (2.x) similar to vendor 4.4 kernel.
Fixes: 85a359f25388 ("drm/rockchip: Add BGR formats to VOP")
Signed-off-by: Jonas Karlman
---
Changes in v2:
- Add comment about different rb swap for IP version 3.x and 2.x
- Add fixes tag
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index b3d0b6ae9294..ed2ed25959a2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -247,14 +247,22 @@ static inline void vop_cfg_done(struct vop *vop)
VOP_REG_SET(vop, common, cfg_done, 1);
}
-static bool has_rb_swapped(uint32_t format)
+static bool has_rb_swapped(uint32_t version, uint32_t format)
{
switch (format) {
case DRM_FORMAT_XBGR:
case DRM_FORMAT_ABGR:
- case DRM_FORMAT_BGR888:
case DRM_FORMAT_BGR565:
return true;
+ /*
+* full framework (IP version 3.x) only need rb swapped for RGB888 and
+* little framework (IP version 2.x) only need rb swapped for BGR888,
+* check for 3.x to also only rb swap BGR888 for unknown vop version
+*/
+ case DRM_FORMAT_RGB888:
+ return VOP_MAJOR(version) == 3;
+ case DRM_FORMAT_BGR888:
+ return VOP_MAJOR(version) != 3;
default:
return false;
}
@@ -1035,7 +1043,7 @@ static void vop_plane_atomic_update(struct drm_plane
*plane,
VOP_WIN_SET(vop, win, dsp_info, dsp_info);
VOP_WIN_SET(vop, win, dsp_st, dsp_st);
- rb_swap = has_rb_swapped(fb->format->format);
+ rb_swap = has_rb_swapped(vop->data->version, fb->format->format);
VOP_WIN_SET(vop, win, rb_swap, rb_swap);
/*
--
2.42.0
Re: [PATCH v2 2/2] drm/todo: Add entry to clean up former seltests suites
Hi Maira, On Wed, Oct 25, 2023 at 02:26:44PM -0300, Maira Canal wrote: > Hi Maxime, > > Wouldn't be nice to add to the TODO list an item regarding the deleted > drm_mm tests? Something just to remember us to develop new tests for it > in the future. I guess we could, but it's really not clear to me what these were testing in the first place. So the scope of the work would effectively be "increase our test coverage" which I believe is already covered by the todo task just above. Maxime > On 10/25/23 10:24, Maxime Ripard wrote: > > Most of those suites are undocumented and aren't really clear about what > > they are testing. Let's add a TODO entry as a future task to get started > > into KUnit and DRM. > > > > Signed-off-by: Maxime Ripard > > --- > > Documentation/gpu/todo.rst | 17 + > > 1 file changed, 17 insertions(+) > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 03fe5d1247be..b62c7fa0c2bc 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -621,6 +621,23 @@ Contact: Javier Martinez Canillas > > Level: Intermediate > > +Clean up and document former selftests suites > > +- > > + > > +Some KUnit test suites (drm_buddy, drm_cmdline_parser, drm_damage_helper, > > +drm_format, drm_framebuffer, drm_dp_mst_helper, drm_mm, drm_plane_helper > > and > > +drm_rect) are former selftests suites that have been converted over when > > KUnit > > +was first introduced. > > + > > +These suites were fairly undocumented, and with different goals than what > > unit > > +tests can be. Trying to identify what each test in these suites actually > > test > > +for, whether that makes sense for a unit test, and either remove it if it > > +doesn't or document it if it does would be of great help. > > + > > +Contact: Maxime Ripard > > + > > +Level: Intermediate > > + > > Enable trinity for DRM > > -- > signature.asc Description: PGP signature
Re: [PATCH] MAINTAINERS: Update the GPU Scheduler email
On Thu, Oct 26, 2023 at 1:45 PM Luben Tuikov wrote: > > Update the GPU Scheduler maintainer email. > > Cc: Alex Deucher > Cc: Christian König > Cc: Daniel Vetter > Cc: Dave Airlie > Cc: AMD Graphics > Cc: Direct Rendering Infrastructure - Development > > Signed-off-by: Luben Tuikov Acked-by: Alex Deucher > --- > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4452508bc1b040..f13e476ed8038b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7153,7 +7153,7 @@ F: > Documentation/devicetree/bindings/display/xlnx/ > F: drivers/gpu/drm/xlnx/ > > DRM GPU SCHEDULER > -M: Luben Tuikov > +M: Luben Tuikov > L: dri-devel@lists.freedesktop.org > S: Maintained > T: git git://anongit.freedesktop.org/drm/drm-misc > > base-commit: 56e449603f0ac580700621a356d35d5716a62ce5 > -- > 2.42.0 >
Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On Thu, 26 Oct 2023, Sebastian Wick wrote:
> On Thu, Oct 26, 2023 at 11:57:47AM +0300, Pekka Paalanen wrote:
> > On Wed, 25 Oct 2023 15:16:08 -0500 (CDT)
> > Alex Goins wrote:
> >
> > > Thank you Harry and all other contributors for your work on this.
> > > Responses
> > > inline -
> > >
> > > On Mon, 23 Oct 2023, Pekka Paalanen wrote:
> > >
> > > > On Fri, 20 Oct 2023 11:23:28 -0400
> > > > Harry Wentland wrote:
> > > >
> > > > > On 2023-10-20 10:57, Pekka Paalanen wrote:
> > > > > > On Fri, 20 Oct 2023 16:22:56 +0200
> > > > > > Sebastian Wick wrote:
> > > > > >
> > > > > >> Thanks for continuing to work on this!
> > > > > >>
> > > > > >> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:
> > > > > >>> v2:
> > > > > >>> - Update colorop visualizations to match reality (Sebastian,
> > > > > >>> Alex Hung)
> > > > > >>> - Updated wording (Pekka)
> > > > > >>> - Change BYPASS wording to make it non-mandatory (Sebastian)
> > > > > >>> - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane
> > > > > >>> Property
> > > > > >>>section (Pekka)
> > > > > >>> - Use PQ EOTF instead of its inverse in Pipeline Programming
> > > > > >>> example (Melissa)
> > > > > >>> - Add "Driver Implementer's Guide" section (Pekka)
> > > > > >>> - Add "Driver Forward/Backward Compatibility" section
> > > > > >>> (Sebastian, Pekka)
> > > > > >
> > > > > > ...
> > > > > >
> > > > > >>> +An example of a drm_colorop object might look like one of these::
> > > > > >>> +
> > > > > >>> +/* 1D enumerated curve */
> > > > > >>> +Color operation 42
> > > > > >>> +├─ "TYPE": immutable enum {1D enumerated curve, 1D LUT, 3x3
> > > > > >>> matrix, 3x4 matrix, 3D LUT, etc.} = 1D enumerated curve
> > > > > >>> +├─ "BYPASS": bool {true, false}
> > > > > >>> +├─ "CURVE_1D_TYPE": enum {sRGB EOTF, sRGB inverse EOTF, PQ
> > > > > >>> EOTF, PQ inverse EOTF, …}
> > > > > >>> +└─ "NEXT": immutable color operation ID = 43
> > >
> > > I know these are just examples, but I would also like to suggest the
> > > possibility
> > > of an "identity" CURVE_1D_TYPE. BYPASS = true might get different results
> > > compared to setting an identity in some cases depending on the hardware.
> > > See
> > > below for more on this, RE: implicit format conversions.
> > >
> > > Although NVIDIA hardware doesn't use a ROM for enumerated curves, it came
> > > up in
> > > offline discussions that it would nonetheless be helpful to expose
> > > enumerated
> > > curves in order to hide the vendor-specific complexities of programming
> > > segmented LUTs from clients. In that case, we would simply refer to the
> > > enumerated curve when calculating/choosing segmented LUT entries.
> >
> > That's a good idea.
> >
> > > Another thing that came up in offline discussions is that we could use
> > > multiple
> > > color operations to program a single operation in hardware. As I
> > > understand it,
> > > AMD has a ROM-defined LUT, followed by a custom 4K entry LUT, followed by
> > > an
> > > "HDR Multiplier". On NVIDIA we don't have these as separate hardware
> > > stages, but
> > > we could combine them into a singular LUT in software, such that you can
> > > combine
> > > e.g. segmented PQ EOTF with night light. One caveat is that you will lose
> > > precision from the custom LUT where it overlaps with the linear section
> > > of the
> > > enumerated curve, but that is unavoidable and shouldn't be an issue in
> > > most
> > > use-cases.
> >
> > Indeed.
> >
> > > Actually, the current examples in the proposal don't include a multiplier
> > > color
> > > op, which might be useful. For AMD as above, but also for NVIDIA as the
> > > following issue arises:
> > >
> > > As discussed further below, the NVIDIA "degamma" LUT performs an implicit
> > > fixed
> > > point to FP16 conversion. In that conversion, what fixed point 0x
> > > maps
> > > to in floating point varies depending on the source content. If it's SDR
> > > content, we want the max value in FP16 to be 1.0 (80 nits), subject to a
> > > potential boost multiplier if we want SDR content to be brighter. If it's
> > > HDR PQ
> > > content, we want the max value in FP16 to be 125.0 (10,000 nits). My
> > > assumption
> > > is that this is also what AMD's "HDR Multiplier" stage is used for, is
> > > that
> > > correct?
> >
> > It would be against the UAPI design principles to tag content as HDR or
> > SDR. What you can do instead is to expose a colorop with a multiplier of
> > 1.0 or 125.0 to match your hardware behaviour, then tell your hardware
> > that the input is SDR or HDR to get the expected multiplier. You will
> > never know what the content actually is, anyway.
Right, I didn't mean to suggest that we should tag content as HDR or SDR in the
UAPI, just relating to the end result in the pipe, ultimately it would be
determined by the multiplier color op.
> >
> > Of course, if we want to have a arbitrary multiplier colorop that is
> >
Re: [PATCH v2] drm/syncobj: fix DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE
On Thursday, October 26th, 2023 at 21:16, Simon Ser wrote: > On Thursday, October 26th, 2023 at 19:55, Erik Kurzinger > ekurzin...@nvidia.com wrote: > > > Is there anything else needed for this fix to be merged? I have shared > > an accompanying patch for the IGT test suite here > > https://lists.freedesktop.org/archives/igt-dev/2023-August/060154.html > > Do you also happen to have user-space patches to use this? That's also > a requirement for new uAPI. Oh my, scratch that. This is not new uAPI, this is a fix. I got things mixed up. I'll push your patch now. Thanks!
Re: [PATCH] drm/msm/adreno: Drop WARN_ON from patchid lookup for new GPUs
On 10/23/23 22:20, Rob Clark wrote:
On Mon, Oct 23, 2023 at 12:56 PM Konrad Dybcio wrote:
On 10/23/23 21:42, Rob Clark wrote:
On Mon, Oct 23, 2023 at 7:29 AM Konrad Dybcio wrote:
New GPUs still use the lower 2 bytes of the chip id (in whatever form
it comes) to signify silicon revision. Drop the warning that makes it
sound as if that was unintended.
Fixes: 90b593ce1c9e ("drm/msm/adreno: Switch to chip-id for identifying GPU")
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 80b3f6312116..9a1ec42155fd 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -203,11 +203,6 @@ struct adreno_platform_config {
static inline uint8_t adreno_patchid(const struct adreno_gpu *gpu)
{
- /* It is probably ok to assume legacy "adreno_rev" format
-* for all a6xx devices, but probably best to limit this
-* to older things.
-*/
- WARN_ON_ONCE(gpu->info->family >= ADRENO_6XX_GEN1);
Maybe just change it to ADRENO_6XX_GEN4?
That also applies to 700
Then the warn is warning about what it is supposed to ;-)
I guess this is coming from a6xx_gmu_fw_start()? I think we need a
different way to construct the gmu chipid, since the point of this was
to not depend on the low 8b having any particular meaning. Perhaps we
should just get the gmu chipid from the device table.
Guess that could work as well..
Konrad
Re: [PATCH v2] drm/syncobj: fix DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE
On Thursday, October 26th, 2023 at 19:55, Erik Kurzinger wrote: > Is there anything else needed for this fix to be merged? I have shared > an accompanying patch for the IGT test suite here > https://lists.freedesktop.org/archives/igt-dev/2023-August/060154.html Do you also happen to have user-space patches to use this? That's also a requirement for new uAPI.
Re: [PATCH] drm: bridge: adv7511: fix reading edid segments
On 26.10.23 13:30, Emil Abildgaard Svendsen wrote: > [Sie erhalten nicht häufig E-Mails von e...@bang-olufsen.dk. Weitere > Informationen, warum dies wichtig ist, finden Sie unter > https://aka.ms/LearnAboutSenderIdentification ] > > Currently reading EDID only works because usually only two EDID blocks > of 128 bytes is used. Where an EDID segment holds 256 bytes or two EDID > blocks. And the first EDID segment read works fine but E-EDID specifies > up to 128 segments. > > The logic is broken so change EDID segment index to multiple of 256 > bytes and not 128 (block size). > > Also change check of DDC status. Instead of silently not reading EDID > when in "IDLE" state [1]. Check if the DDC controller is in reset > instead (no HPD). > > [1] > ADV7511 Programming Guide: Table 11: DDCController Status: > > 0xC8 [3:0] DDC Controller State > > In Reset (No Hot Plug Detected) > 0001Reading EDID > 0010IDLE (Waiting for HDCP Requested) > 0011Initializing HDCP > 0100HDCP Enabled > 0101Initializing HDCP Repeater > > Signed-off-by: Emil Svendsen This patch somehow seems to break the reported display modes with my setup (i.MX8MM DSI -> ADV7535 -> FullHD screen) when I test on current linux-next. Without this patch I get 30 valid modes reported by modetest. With this patch applied I only get 5 valid modes. The screen still comes up with the 1080p default mode though, that is now not even in the list anymore.
Re: [REGRESSION] rx7600 stopped working after "1cfb4d612127 drm/amdgpu: put MQDs in VRAM"
On Thu, Oct 26, 2023 at 1:33 PM Alexey Klimov wrote:
>
> #regzbot introduced: 1cfb4d612127
> #regzbot title: rx7600 stopped working after "1cfb4d612127 drm/amdgpu: put
> MQDs in VRAM"
>
> Hi all,
>
> I've been playing with RX7600 and it was observed that amdgpu stopped working
> between kernel 6.2 and 6.5.
> Then I narrowed it down to 6.4 <-> 6.5-rc1 and finally bisect pointed at
> 1cfb4d6121276a829aa94d0e32a7f5e1830ebc21
> And I manually checked if it boots/works on the previous commit and the
> mentioned one.
>
> I guess the log also reveals warning in error path. Please see below.
>
> I didn't check any further. This is simple debian testing system with the
> following cmdline options:
> root@avadebian:~# cat /proc/cmdline
> BOOT_IMAGE=/boot/vmlinuz-6.6-rc7+ ignore_loglevel root=/dev/nvme1n1p2 ro
> nr_cpus=32
>
> So far simple revert (patch is below) returns things back to normal-ish:
> there are huge graphics artifacts on Xorg/X11 under 6.1 to upstream kernel.
> Wayland-based sway works great without issues. Not sure where should I report
> this.
>
> Please let me know if I can help debugging, testing or provide some other
> logs regarding 1cfb4d612127? Any cmdline options to collect more info?
Please make sure you have this patch as well:
e602157ec089240861cd641ee2c7c64eeaec09bf ("drm/amdgpu: fix S3 issue if
MQD in VRAM")
Please open a ticket here so we can track this:
https://gitlab.freedesktop.org/drm/amd/-/issues/
I think I see the problem. Please see if attached patch 1 fixes the
issue. If this fixes it, that would also explain the issues you are
seeing with Xorg. It would appear there are limitations around MMIO
access on your platform and unfortunately most graphics APIs require
unaligned access to MMIO space with the CPU. We can fix the kernel
side pretty easily, but userspace will be a problem.
More below.
>
> Thanks,
> Alexey
>
>
>
> From 214372d5cedcf8757dd80d5f4d058377a3d92c52 Mon Sep 17 00:00:00 2001
> From: Alexey Klimov
> Date: Thu, 26 Oct 2023 17:01:02 +0100
> Subject: [PATCH] drm/amdgpu: Revert "drm/amdgpu: put MQDs in VRAM"
>
> This reverts commit 1cfb4d6121276a829aa94d0e32a7f5e1830ebc21.
>
> amdgpu driver fails during initialisation with RX7600/gfx11 on
> ADLINK Ampere Altra Developer Platform (AVA developer platform)
> with mentioned commit:
>
> [ 12.559893] [drm] Display Core v3.2.247 initialized on DCN 3.2.1
> [ 12.565906] [drm] DP-HDMI FRL PCON supported
> [ 12.572192] [drm] DMUB hardware initialized: version=0x07000C00
> [ 12.582541] snd_hda_intel 000d:03:00.1: bound 000d:03:00.0 (ops
> amdgpu_dm_audio_component_bind_ops [amdgpu])
> [ 12.625357] [drm] kiq ring mec 3 pipe 1 q 0
> [ 12.857087] amdgpu 000d:03:00.0: [drm:amdgpu_ring_test_helper [amdgpu]]
> *ERROR* ring comp_1.0.0 test failed (-110)
> [ 12.867930] [drm:amdgpu_device_init [amdgpu]] *ERROR* hw_init of IP block
> failed -110
> [ 12.877289] amdgpu 000d:03:00.0: amdgpu: amdgpu_device_ip_init failed
> [ 12.883723] amdgpu 000d:03:00.0: amdgpu: Fatal error during GPU init
> [ 12.890070] amdgpu 000d:03:00.0: amdgpu: amdgpu: finishing device.
> [ 12.896586] [drm] DSC precompute is not needed.
> [ 12.901142] [ cut here ]
> [ 12.905747] WARNING: CPU: 0 PID: 212 at
> drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:615 amdgpu_irq_put+0xa8/0xc8 [amdgpu]
> [ 12.916841] Modules linked in: hid_generic(E) usbhid(E) hid(E) qrtr(E)
> iptable_nat(E) amdgpu(E+) nf_nat(E) nf_conntrack(E) snd_hda_codec_hdmi(E)
> nf_defrag_ipv6(E) nf_defrag_ipv4(E) libcrc32c(E) iptable_mangle(E)
> iptable_filter(E) amdxcp(E) drm_exec(E) gpu_sched(E) snd_hda_intel(E)
> aes_ce_blk(E) snd_intel_dspcfg(E) drm_buddy(E) aes_ce_cipher(E)
> snd_hda_codec(E) xhci_pci(E) video(E) crct10dif_ce(E) polyval_ce(E)
> snd_hda_core(E) xhci_hcd(E) drm_suballoc_helper(E) snd_hwdep(E)
> polyval_generic(E) drm_ttm_helper(E) snd_pcm(E) ghash_ce(E) ast(E) ttm(E)
> gf128mul(E) snd_timer(E) ipmi_ssif(E) drm_display_helper(E)
> drm_shmem_helper(E) sha2_ce(E) sha256_arm64(E) ipmi_devintf(E) usbcore(E)
> snd(E) drm_kms_helper(E) igb(E) sha1_ce(E) sbsa_gwdt(E) ipmi_msghandler(E)
> arm_spe_pmu(E) soundcore(E) usb_common(E) i2c_algo_bit(E) cppc_cpufreq(E)
> i2c_designware_platform(E) arm_dsu_pmu(E) arm_cmn(E) xgene_hwmon(E)
> i2c_designware_core(E) evdev(E) binfmt_misc(E) loop(E) fuse(E) efi_pstore(E)
> drm(E) dm_mod(E) dax(E) configfs(E) efivarfs(E)
> [ 12.916916] ip_tables(E) x_tables(E) autofs4(E)
> [ 13.01] CPU: 0 PID: 212 Comm: kworker/0:2 Tainted: GE
> 6.6.0-rc7+ #23
> [ 13.019277] Hardware name: ADLINK Ampere Altra Developer Platform/Ampere
> Altra Developer Platform, BIOS TianoCore 2.04.100.10 (SYS: 2.06.20220308)
> 04/18/2
> [ 13.033084] Workqueue: events work_for_cpu_fn
> [ 13.037434] pstate: 2049 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 13.044384] pc : amdgpu_irq_put+0xa8/0xc8 [amdgpu]
> [ 13.049652] lr : amdgpu_fence_driver_hw_fini+0x118/0x160
Re: [RFC] drm: bridge: samsung-dsim: Recalculate timings when rounding HFP up
Hi Adam, On 13.10.23 05:10, Adam Ford wrote: > When using video sync pulses, the HFP, HBP, and HSA are divided between > the available lanes if there is more than one lane. For certain > timings and lane configurations, the HFP may not be evenly divisible > and it gets rounded down which can cause certain resolutions and > refresh rates to not sync. > > ie. 720p60 on some monitors show hss of 1390 and hdisplay of 1280. This > yields an HFP of 110. When taking the HFP of 110 divides along 4 lanes, > the result is 27.5 which rounds down to 27 and causes some monitors not > to sync. > > The solultion is to round HFP up by finding the remainder of HFP / > the number of lanes and increasing the hsync_start, hsync_end, and > htotal to compensate when there is a remainder. > > Signed-off-by: Adam Ford > --- > This adds support for 720p60 in the i.MX8M Plus. > > NXP uses a look-up table in their downstream code to accomplish this. > Using this calculation, the driver can adjust without the need for a > complicated table and should be flexible for different timings and > resolutions depending on the monitor. > > I don't have a DSI analyzer, and this appears to only work on > i.MX8M Plus but not Mini and Nano for some reason, despite their > having a similar DSI bridge. I just want to report that I have tested this patch (on top of current linux-next) on our Kontron BL i.MX8MM board with the ADV7535 bridge and I don't see any change when trying the 30 different modes the monitor reports as supported. 18 of those work and 12 don't work. So at least there is no negative impact in this case. Tested-by: Frieder Schrempf # Kontron BL i.MX8MM with HDMI monitor Thanks Frieder
Re: [PATCH v2] drm/syncobj: fix DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE
Is there anything else needed for this fix to be merged? I have shared an
accompanying patch for the IGT test suite here
https://lists.freedesktop.org/archives/igt-dev/2023-August/060154.html
On 8/16/23 09:26, Erik Kurzinger wrote:
> If DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT is invoked with the
> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE flag set but no fence has yet been
> submitted for the given timeline point the call will fail immediately
> with EINVAL. This does not match the intended behavior where the call
> should wait until the fence has been submitted (or the timeout expires).
>
> The following small example program illustrates the issue. It should
> wait for 5 seconds and then print ETIME, but instead it terminates right
> away after printing EINVAL.
>
> #include
> #include
> #include
> #include
> #include
> int main(void)
> {
> int fd = open("/dev/dri/card0", O_RDWR);
> uint32_t syncobj;
> drmSyncobjCreate(fd, 0, );
> struct timespec ts;
> clock_gettime(CLOCK_MONOTONIC, );
> uint64_t point = 1;
> if (drmSyncobjTimelineWait(fd, , , 1,
> ts.tv_sec * 10 + ts.tv_nsec +
> 50, // 5s
> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE,
> NULL)) {
> printf("drmSyncobjTimelineWait failed %d\n", errno);
> }
> }
>
> Fixes: 01d6c3578379 ("drm/syncobj: add support for timeline point wait v8")
> Signed-off-by: Erik Kurzinger
> Reviewed by: Simon Ser
> ---
> drivers/gpu/drm/drm_syncobj.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index add45001e939..a8e6b61a188c 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1087,7 +1087,8 @@ static signed long
> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
> fence = drm_syncobj_fence_get(syncobjs[i]);
> if (!fence || dma_fence_chain_find_seqno(, points[i])) {
> dma_fence_put(fence);
> - if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
> + if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) {
> continue;
> } else {
> timeout = -EINVAL;
[PATCH] MAINTAINERS: Update the GPU Scheduler email
Update the GPU Scheduler maintainer email. Cc: Alex Deucher Cc: Christian König Cc: Daniel Vetter Cc: Dave Airlie Cc: AMD Graphics Cc: Direct Rendering Infrastructure - Development Signed-off-by: Luben Tuikov --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 4452508bc1b040..f13e476ed8038b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7153,7 +7153,7 @@ F:Documentation/devicetree/bindings/display/xlnx/ F: drivers/gpu/drm/xlnx/ DRM GPU SCHEDULER -M: Luben Tuikov +M: Luben Tuikov L: dri-devel@lists.freedesktop.org S: Maintained T: git git://anongit.freedesktop.org/drm/drm-misc base-commit: 56e449603f0ac580700621a356d35d5716a62ce5 -- 2.42.0
[REGRESSION] rx7600 stopped working after "1cfb4d612127 drm/amdgpu: put MQDs in VRAM"
#regzbot introduced: 1cfb4d612127 #regzbot title: rx7600 stopped working after "1cfb4d612127 drm/amdgpu: put MQDs in VRAM" Hi all, I've been playing with RX7600 and it was observed that amdgpu stopped working between kernel 6.2 and 6.5. Then I narrowed it down to 6.4 <-> 6.5-rc1 and finally bisect pointed at 1cfb4d6121276a829aa94d0e32a7f5e1830ebc21 And I manually checked if it boots/works on the previous commit and the mentioned one. I guess the log also reveals warning in error path. Please see below. I didn't check any further. This is simple debian testing system with the following cmdline options: root@avadebian:~# cat /proc/cmdline BOOT_IMAGE=/boot/vmlinuz-6.6-rc7+ ignore_loglevel root=/dev/nvme1n1p2 ro nr_cpus=32 So far simple revert (patch is below) returns things back to normal-ish: there are huge graphics artifacts on Xorg/X11 under 6.1 to upstream kernel. Wayland-based sway works great without issues. Not sure where should I report this. Please let me know if I can help debugging, testing or provide some other logs regarding 1cfb4d612127? Any cmdline options to collect more info? Thanks, Alexey >From 214372d5cedcf8757dd80d5f4d058377a3d92c52 Mon Sep 17 00:00:00 2001 From: Alexey Klimov Date: Thu, 26 Oct 2023 17:01:02 +0100 Subject: [PATCH] drm/amdgpu: Revert "drm/amdgpu: put MQDs in VRAM" This reverts commit 1cfb4d6121276a829aa94d0e32a7f5e1830ebc21. amdgpu driver fails during initialisation with RX7600/gfx11 on ADLINK Ampere Altra Developer Platform (AVA developer platform) with mentioned commit: [ 12.559893] [drm] Display Core v3.2.247 initialized on DCN 3.2.1 [ 12.565906] [drm] DP-HDMI FRL PCON supported [ 12.572192] [drm] DMUB hardware initialized: version=0x07000C00 [ 12.582541] snd_hda_intel 000d:03:00.1: bound 000d:03:00.0 (ops amdgpu_dm_audio_component_bind_ops [amdgpu]) [ 12.625357] [drm] kiq ring mec 3 pipe 1 q 0 [ 12.857087] amdgpu 000d:03:00.0: [drm:amdgpu_ring_test_helper [amdgpu]] *ERROR* ring comp_1.0.0 test failed (-110) [ 12.867930] [drm:amdgpu_device_init [amdgpu]] *ERROR* hw_init of IP block failed -110 [ 12.877289] amdgpu 000d:03:00.0: amdgpu: amdgpu_device_ip_init failed [ 12.883723] amdgpu 000d:03:00.0: amdgpu: Fatal error during GPU init [ 12.890070] amdgpu 000d:03:00.0: amdgpu: amdgpu: finishing device. [ 12.896586] [drm] DSC precompute is not needed. [ 12.901142] [ cut here ] [ 12.905747] WARNING: CPU: 0 PID: 212 at drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:615 amdgpu_irq_put+0xa8/0xc8 [amdgpu] [ 12.916841] Modules linked in: hid_generic(E) usbhid(E) hid(E) qrtr(E) iptable_nat(E) amdgpu(E+) nf_nat(E) nf_conntrack(E) snd_hda_codec_hdmi(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) libcrc32c(E) iptable_mangle(E) iptable_filter(E) amdxcp(E) drm_exec(E) gpu_sched(E) snd_hda_intel(E) aes_ce_blk(E) snd_intel_dspcfg(E) drm_buddy(E) aes_ce_cipher(E) snd_hda_codec(E) xhci_pci(E) video(E) crct10dif_ce(E) polyval_ce(E) snd_hda_core(E) xhci_hcd(E) drm_suballoc_helper(E) snd_hwdep(E) polyval_generic(E) drm_ttm_helper(E) snd_pcm(E) ghash_ce(E) ast(E) ttm(E) gf128mul(E) snd_timer(E) ipmi_ssif(E) drm_display_helper(E) drm_shmem_helper(E) sha2_ce(E) sha256_arm64(E) ipmi_devintf(E) usbcore(E) snd(E) drm_kms_helper(E) igb(E) sha1_ce(E) sbsa_gwdt(E) ipmi_msghandler(E) arm_spe_pmu(E) soundcore(E) usb_common(E) i2c_algo_bit(E) cppc_cpufreq(E) i2c_designware_platform(E) arm_dsu_pmu(E) arm_cmn(E) xgene_hwmon(E) i2c_designware_core(E) evdev(E) binfmt_misc(E) loop(E) fuse(E) efi_pstore(E) drm(E) dm_mod(E) dax(E) configfs(E) efivarfs(E) [ 12.916916] ip_tables(E) x_tables(E) autofs4(E) [ 13.01] CPU: 0 PID: 212 Comm: kworker/0:2 Tainted: GE 6.6.0-rc7+ #23 [ 13.019277] Hardware name: ADLINK Ampere Altra Developer Platform/Ampere Altra Developer Platform, BIOS TianoCore 2.04.100.10 (SYS: 2.06.20220308) 04/18/2 [ 13.033084] Workqueue: events work_for_cpu_fn [ 13.037434] pstate: 2049 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 13.044384] pc : amdgpu_irq_put+0xa8/0xc8 [amdgpu] [ 13.049652] lr : amdgpu_fence_driver_hw_fini+0x118/0x160 [amdgpu] [ 13.056220] sp : 80008012bc10 [ 13.059522] x29: 80008012bc20 x28: x27: [ 13.066647] x26: x25: 07ff98580010 x24: 07ff9858 [ 13.073772] x23: 07ff985a78f0 x22: 07ff98580010 x21: 07ff985904c8 [ 13.080896] x20: 07ff985900e8 x19: 07ff98598580 x18: 0006 [ 13.088020] x17: 0020 x16: bb510d0d7140 x15: fefb [ 13.095145] x14: x13: 2e64656465656e20 x12: 07ff8c7fd9e0 [ 13.102268] x11: 03e8 x10: 07ff8c7fd9e0 x9 : bb50ac3345e0 [ 13.109392] x8 : bb50abf18000 x7 : x6 : 7a456104 [ 13.116516] x5 : x4 : 07ff9858 x3 : [ 13.123641] x2 : x1 : 07ff985a78f0 x0 : 07ffc5fd4000 [
Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On Thu, Oct 26, 2023 at 11:57:47AM +0300, Pekka Paalanen wrote:
> On Wed, 25 Oct 2023 15:16:08 -0500 (CDT)
> Alex Goins wrote:
>
> > Thank you Harry and all other contributors for your work on this. Responses
> > inline -
> >
> > On Mon, 23 Oct 2023, Pekka Paalanen wrote:
> >
> > > On Fri, 20 Oct 2023 11:23:28 -0400
> > > Harry Wentland wrote:
> > >
> > > > On 2023-10-20 10:57, Pekka Paalanen wrote:
> > > > > On Fri, 20 Oct 2023 16:22:56 +0200
> > > > > Sebastian Wick wrote:
> > > > >
> > > > >> Thanks for continuing to work on this!
> > > > >>
> > > > >> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:
> > > > >>> v2:
> > > > >>> - Update colorop visualizations to match reality (Sebastian, Alex
> > > > >>> Hung)
> > > > >>> - Updated wording (Pekka)
> > > > >>> - Change BYPASS wording to make it non-mandatory (Sebastian)
> > > > >>> - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane
> > > > >>> Property
> > > > >>>section (Pekka)
> > > > >>> - Use PQ EOTF instead of its inverse in Pipeline Programming
> > > > >>> example (Melissa)
> > > > >>> - Add "Driver Implementer's Guide" section (Pekka)
> > > > >>> - Add "Driver Forward/Backward Compatibility" section (Sebastian,
> > > > >>> Pekka)
> > > > >
> > > > > ...
> > > > >
> > > > >>> +An example of a drm_colorop object might look like one of these::
> > > > >>> +
> > > > >>> +/* 1D enumerated curve */
> > > > >>> +Color operation 42
> > > > >>> +├─ "TYPE": immutable enum {1D enumerated curve, 1D LUT, 3x3
> > > > >>> matrix, 3x4 matrix, 3D LUT, etc.} = 1D enumerated curve
> > > > >>> +├─ "BYPASS": bool {true, false}
> > > > >>> +├─ "CURVE_1D_TYPE": enum {sRGB EOTF, sRGB inverse EOTF, PQ
> > > > >>> EOTF, PQ inverse EOTF, …}
> > > > >>> +└─ "NEXT": immutable color operation ID = 43
> >
> > I know these are just examples, but I would also like to suggest the
> > possibility
> > of an "identity" CURVE_1D_TYPE. BYPASS = true might get different results
> > compared to setting an identity in some cases depending on the hardware. See
> > below for more on this, RE: implicit format conversions.
> >
> > Although NVIDIA hardware doesn't use a ROM for enumerated curves, it came
> > up in
> > offline discussions that it would nonetheless be helpful to expose
> > enumerated
> > curves in order to hide the vendor-specific complexities of programming
> > segmented LUTs from clients. In that case, we would simply refer to the
> > enumerated curve when calculating/choosing segmented LUT entries.
>
> That's a good idea.
>
> > Another thing that came up in offline discussions is that we could use
> > multiple
> > color operations to program a single operation in hardware. As I understand
> > it,
> > AMD has a ROM-defined LUT, followed by a custom 4K entry LUT, followed by an
> > "HDR Multiplier". On NVIDIA we don't have these as separate hardware
> > stages, but
> > we could combine them into a singular LUT in software, such that you can
> > combine
> > e.g. segmented PQ EOTF with night light. One caveat is that you will lose
> > precision from the custom LUT where it overlaps with the linear section of
> > the
> > enumerated curve, but that is unavoidable and shouldn't be an issue in most
> > use-cases.
>
> Indeed.
>
> > Actually, the current examples in the proposal don't include a multiplier
> > color
> > op, which might be useful. For AMD as above, but also for NVIDIA as the
> > following issue arises:
> >
> > As discussed further below, the NVIDIA "degamma" LUT performs an implicit
> > fixed
> > point to FP16 conversion. In that conversion, what fixed point 0x
> > maps
> > to in floating point varies depending on the source content. If it's SDR
> > content, we want the max value in FP16 to be 1.0 (80 nits), subject to a
> > potential boost multiplier if we want SDR content to be brighter. If it's
> > HDR PQ
> > content, we want the max value in FP16 to be 125.0 (10,000 nits). My
> > assumption
> > is that this is also what AMD's "HDR Multiplier" stage is used for, is that
> > correct?
>
> It would be against the UAPI design principles to tag content as HDR or
> SDR. What you can do instead is to expose a colorop with a multiplier of
> 1.0 or 125.0 to match your hardware behaviour, then tell your hardware
> that the input is SDR or HDR to get the expected multiplier. You will
> never know what the content actually is, anyway.
>
> Of course, if we want to have a arbitrary multiplier colorop that is
> somewhat standard, as in, exposed by many drivers to ease userspace
> development, you can certainly use any combination of your hardware
> features you need to realize the UAPI prescribed mathematical operation.
>
> Since we are talking about floating-point in hardware, a multiplier
> does not significantly affect precision.
>
> In order to mathematically define all colorops, I believe it is
> necessary to define all colorops in terms of floating-point
Re: [PATCH] drm/sched: Convert the GPU scheduler to variable number of run-queues
On 2023-10-26 12:39, Danilo Krummrich wrote: > On 10/23/23 05:22, Luben Tuikov wrote: >> The GPU scheduler has now a variable number of run-queues, which are set up >> at >> drm_sched_init() time. This way, each driver announces how many run-queues it >> requires (supports) per each GPU scheduler it creates. Note, that run-queues >> correspond to scheduler "priorities", thus if the number of run-queues is set >> to 1 at drm_sched_init(), then that scheduler supports a single run-queue, >> i.e. single "priority". If a driver further sets a single entity per >> run-queue, then this creates a 1-to-1 correspondence between a scheduler and >> a scheduled entity. > > Generally, I'm fine with this patch and how it replaces / generalizes the > single > entity approach. Great! > However, I'm not quite sure how to properly use this. What is a driver > supposed to > do, which previously took advantage of DRM_SCHED_POLICY_SINGLE_ENTITY? > > Is it supposed to call drm_sched_init() with num_rqs=1? If so, what's the > correct way Yes, you call drm_sched_init() with num_rqs set to 1. > to initialize the drm_sched_entity then? Calling drm_sched_entity_init() with > priority=0? Yes, with priority set to 0. One unfortunate fact I noticed when doing this patch is that the numerical values assigned to enum drm_sched_priority is that the names to values are upside down. Instead of min being 0, normal:1, high:2, kernel:3, it should've been kernel:0 (highest), high:1, normal:2, low:4, and so on. The reason for this is absolutely clear: if you had a single priority, it would be 0, the kernel, one, highest one. This is similar to how lanes in a highway are counted: you always have lane 1. Similarly to nice(1) and kernel priorities... > Any other priority consequently faults in drm_sched_job_arm(). drm_sched_job_arm() faults on !ENTITY, but the "priority" is just assigned to s_priority: job->s_priority = entity->priority; > While I might sound like a broken record (sorry for that), I really think > everything > related to Matt's series needs documentation, as in: Yes, I agree. > - What is the typical application of the single entity / variable run queue > design? >How do drivers make use of it? I believe most drivers in the future, would want to have a single sched_rq (i.e. single "priority", and then would tack a single entity to this, and then do prioritization internally in their firmware/hardware. > - How to tear down a scheduler instance properly? > - Motivation and implications of the workqueue topology (default workqueue, > external >driver workqueue, free job work, run job work, etc.). > > But also the existing scheduler infrastructure requires more documentation. > > The scheduler barely has some documentation to describe its basic topology of > struct drm_gpu_scheduler, struct drm_sched_entity and struct drm_sched_job. > Plus a few hints regarding run queue priorities, which, with this patch, seem > to be > (partially) out-dated or at least incomplete. > > I think Sima also mentioned that we really need to put some efforts here. [1] Yes, that's true. Regards, Luben > > - Danilo > > [1] > https://lore.kernel.org/all/20231017150958.838613-1-matthew.br...@intel.com/T/#m330335b44bdb7ae93ac6ebdedd65706df5a0f03d > >> >> Cc: Lucas Stach >> Cc: Russell King >> Cc: Qiang Yu >> Cc: Rob Clark >> Cc: Abhinav Kumar >> Cc: Dmitry Baryshkov >> Cc: Danilo Krummrich >> Cc: Matthew Brost >> Cc: Boris Brezillon >> Cc: Alex Deucher >> Cc: Christian König >> Cc: Emma Anholt >> Cc: etna...@lists.freedesktop.org >> Cc: l...@lists.freedesktop.org >> Cc: linux-arm-...@vger.kernel.org >> Cc: freedr...@lists.freedesktop.org >> Cc: nouv...@lists.freedesktop.org >> Cc: dri-devel@lists.freedesktop.org >> Signed-off-by: Luben Tuikov >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 + >> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c| 4 +- >> drivers/gpu/drm/etnaviv/etnaviv_sched.c| 1 + >> drivers/gpu/drm/lima/lima_sched.c | 4 +- >> drivers/gpu/drm/msm/msm_ringbuffer.c | 5 +- >> drivers/gpu/drm/nouveau/nouveau_sched.c| 1 + >> drivers/gpu/drm/panfrost/panfrost_job.c| 1 + >> drivers/gpu/drm/scheduler/sched_entity.c | 18 +- >> drivers/gpu/drm/scheduler/sched_main.c | 74 ++ >> drivers/gpu/drm/v3d/v3d_sched.c| 5 ++ >> include/drm/gpu_scheduler.h| 9 ++- >> 11 files changed, 98 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index 2b8356699f235d..251995a90bbe69 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -2280,6 +2280,7 @@ static int amdgpu_device_init_schedulers(struct >> amdgpu_device *adev) >> } >> >> r = drm_sched_init(>sched, _sched_ops, >> +
[PATCH] drm/nouveau: Prevents NULL pointer dereference in nouveau_uvmm_sm_prepare
There are instances where the "args" argument passed to
nouveau_uvmm_sm_prepare() is NULL.
I.e. when nouveau_uvmm_sm_prepare() is called from
nouveau_uvmm_sm_unmap_prepare()
```
static int
nouveau_uvmm_sm_unmap_prepare(struct nouveau_uvmm *uvmm,
...
{
return nouveau_uvmm_sm_prepare(uvmm, new, ops, NULL);
}
```
The problem is that op_map_prepare() which nouveau_uvmm_sm_prepare
calls, dereferences this value, which can lead to a NULL pointer
dereference.
```
static int
op_map_prepare(struct nouveau_uvmm *uvmm,
...
{
...
uvma->region = args->region; <-- Dereferencing of possibly NULL pointer
uvma->kind = args->kind; <--
...
}
```
```
static int
nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
...
struct uvmm_map_args *args)
{
struct drm_gpuva_op *op;
u64 vmm_get_start = args ? args->addr : 0;
u64 vmm_get_end = args ? args->addr + args->range : 0;
int ret;
drm_gpuva_for_each_op(op, ops) {
switch (op->op) {
case DRM_GPUVA_OP_MAP: {
u64 vmm_get_range = vmm_get_end - vmm_get_start;
ret = op_map_prepare(uvmm, >map, >map, args); <---
if (ret)
goto unwind;
if (args && vmm_get_range) {
ret = nouveau_uvmm_vmm_get(uvmm, vmm_get_start,
vmm_get_range);
if (ret) {
op_map_prepare_unwind(new->map);
goto unwind;
}
}
...
```
Since the switch "case DRM_GPUVA_OP_MAP", also NULL checks "args"
after the call to op_map_prepare(), my guess is that we should
probably relocate this check to a point before op_map_prepare()
is called.
This patch ensures that the value of args is checked before
calling op_map_prepare()
Addresses-Coverity-ID: 1544574 ("Dereference after null check")
Signed-off-by: Yuran Pereira
---
drivers/gpu/drm/nouveau/nouveau_uvmm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index aae780e4a4aa..6baa481eb2c8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -620,11 +620,14 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
case DRM_GPUVA_OP_MAP: {
u64 vmm_get_range = vmm_get_end - vmm_get_start;
+ if (!args)
+ goto unwind;
+
ret = op_map_prepare(uvmm, >map, >map, args);
if (ret)
goto unwind;
- if (args && vmm_get_range) {
+ if (vmm_get_range) {
ret = nouveau_uvmm_vmm_get(uvmm, vmm_get_start,
vmm_get_range);
if (ret) {
--
2.25.1
Re: [PATCH] drm/sched: Convert the GPU scheduler to variable number of run-queues
On 10/23/23 05:22, Luben Tuikov wrote:
The GPU scheduler has now a variable number of run-queues, which are set up at
drm_sched_init() time. This way, each driver announces how many run-queues it
requires (supports) per each GPU scheduler it creates. Note, that run-queues
correspond to scheduler "priorities", thus if the number of run-queues is set
to 1 at drm_sched_init(), then that scheduler supports a single run-queue,
i.e. single "priority". If a driver further sets a single entity per
run-queue, then this creates a 1-to-1 correspondence between a scheduler and
a scheduled entity.
Generally, I'm fine with this patch and how it replaces / generalizes the single
entity approach.
However, I'm not quite sure how to properly use this. What is a driver supposed
to
do, which previously took advantage of DRM_SCHED_POLICY_SINGLE_ENTITY?
Is it supposed to call drm_sched_init() with num_rqs=1? If so, what's the
correct way
to initialize the drm_sched_entity then? Calling drm_sched_entity_init() with
priority=0?
Any other priority consequently faults in drm_sched_job_arm().
While I might sound like a broken record (sorry for that), I really think
everything
related to Matt's series needs documentation, as in:
- What is the typical application of the single entity / variable run queue
design?
How do drivers make use of it?
- How to tear down a scheduler instance properly?
- Motivation and implications of the workqueue topology (default workqueue,
external
driver workqueue, free job work, run job work, etc.).
But also the existing scheduler infrastructure requires more documentation.
The scheduler barely has some documentation to describe its basic topology of
struct drm_gpu_scheduler, struct drm_sched_entity and struct drm_sched_job.
Plus a few hints regarding run queue priorities, which, with this patch, seem
to be
(partially) out-dated or at least incomplete.
I think Sima also mentioned that we really need to put some efforts here. [1]
- Danilo
[1]
https://lore.kernel.org/all/20231017150958.838613-1-matthew.br...@intel.com/T/#m330335b44bdb7ae93ac6ebdedd65706df5a0f03d
Cc: Lucas Stach
Cc: Russell King
Cc: Qiang Yu
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Danilo Krummrich
Cc: Matthew Brost
Cc: Boris Brezillon
Cc: Alex Deucher
Cc: Christian König
Cc: Emma Anholt
Cc: etna...@lists.freedesktop.org
Cc: l...@lists.freedesktop.org
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Luben Tuikov
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c| 4 +-
drivers/gpu/drm/etnaviv/etnaviv_sched.c| 1 +
drivers/gpu/drm/lima/lima_sched.c | 4 +-
drivers/gpu/drm/msm/msm_ringbuffer.c | 5 +-
drivers/gpu/drm/nouveau/nouveau_sched.c| 1 +
drivers/gpu/drm/panfrost/panfrost_job.c| 1 +
drivers/gpu/drm/scheduler/sched_entity.c | 18 +-
drivers/gpu/drm/scheduler/sched_main.c | 74 ++
drivers/gpu/drm/v3d/v3d_sched.c| 5 ++
include/drm/gpu_scheduler.h| 9 ++-
11 files changed, 98 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2b8356699f235d..251995a90bbe69 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2280,6 +2280,7 @@ static int amdgpu_device_init_schedulers(struct
amdgpu_device *adev)
}
r = drm_sched_init(>sched, _sched_ops,
+ DRM_SCHED_PRIORITY_COUNT,
ring->num_hw_submission, 0,
timeout, adev->reset_domain->wq,
ring->sched_score, ring->name,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 78476bc75b4e1d..1f357198533f3e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -325,8 +325,8 @@ void amdgpu_job_stop_all_jobs_on_sched(struct
drm_gpu_scheduler *sched)
int i;
/* Signal all jobs not yet scheduled */
- for (i = DRM_SCHED_PRIORITY_COUNT - 1; i >= DRM_SCHED_PRIORITY_MIN;
i--) {
- struct drm_sched_rq *rq = >sched_rq[i];
+ for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_MIN; i--) {
+ struct drm_sched_rq *rq = sched->sched_rq[i];
spin_lock(>lock);
list_for_each_entry(s_entity, >entities, list) {
while ((s_job =
to_drm_sched_job(spsc_queue_pop(_entity->job_queue {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 345fec6cb1a4c1..9b79f218e21afc 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@
[PATCH drm-misc-next v3] drm/sched: implement dynamic job-flow control
Currently, job flow control is implemented simply by limiting the number of jobs in flight. Therefore, a scheduler is initialized with a credit limit that corresponds to the number of jobs which can be sent to the hardware. This implies that for each job, drivers need to account for the maximum job size possible in order to not overflow the ring buffer. However, there are drivers, such as Nouveau, where the job size has a rather large range. For such drivers it can easily happen that job submissions not even filling the ring by 1% can block subsequent submissions, which, in the worst case, can lead to the ring run dry. In order to overcome this issue, allow for tracking the actual job size instead of the number of jobs. Therefore, add a field to track a job's credit count, which represents the number of credits a job contributes to the scheduler's credit limit. Signed-off-by: Danilo Krummrich --- Changes in V2: == - fixed up influence on scheduling fairness due to consideration of a job's size - If we reach a ready entity in drm_sched_select_entity() but can't actually queue a job from it due to size limitations, just give up and go to sleep until woken up due to a pending job finishing, rather than continue to try other entities. - added a callback to dynamically update a job's credits (Boris) - renamed 'units' to 'credits' - fixed commit message and comments as requested by Luben Changes in V3: == - rebased onto V7 of the "DRM scheduler changes for Xe" series by Matt - move up drm_sched_can_queue() instead of adding a forward declaration (Boris) - add a drm_sched_available_credits() helper (Boris) - adjust control flow in drm_sched_rq_select_entity_fifo() to Luben's proposal - re-phrase a few comments and fix a typo (Luben) - change naming of all structures credit fields and function parameters to the following scheme - drm_sched_job::credits - drm_gpu_scheduler::credit_limit - drm_gpu_scheduler::credit_count - drm_sched_init(..., u32 credit_limit, ...) - drm_sched_job_init(..., u32 credits, ...) - add proper documentation for the scheduler's job-flow control mechanism This patch is based on V7 of the "DRM scheduler changes for Xe" series. [1] provides a branch based on drm-misc-next, with the named series and this patch on top of it. [1] https://gitlab.freedesktop.org/nouvelles/kernel/-/commits/sched-credits/ --- Documentation/gpu/drm-mm.rst | 6 + drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- drivers/gpu/drm/lima/lima_sched.c | 2 +- drivers/gpu/drm/msm/msm_gem_submit.c | 2 +- drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- .../gpu/drm/scheduler/gpu_scheduler_trace.h | 2 +- drivers/gpu/drm/scheduler/sched_entity.c | 4 +- drivers/gpu/drm/scheduler/sched_main.c| 142 ++ drivers/gpu/drm/v3d/v3d_gem.c | 2 +- include/drm/gpu_scheduler.h | 33 +++- 12 files changed, 152 insertions(+), 49 deletions(-) diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index 602010cb6894..acc5901ac840 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -552,6 +552,12 @@ Overview .. kernel-doc:: drivers/gpu/drm/scheduler/sched_main.c :doc: Overview +Flow Control + + +.. kernel-doc:: drivers/gpu/drm/scheduler/sched_main.c + :doc: Flow Control + Scheduler Function References - diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index 1f357198533f..62bb7fc7448a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -115,7 +115,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct amdgpu_vm *vm, if (!entity) return 0; - return drm_sched_job_init(&(*job)->base, entity, owner); + return drm_sched_job_init(&(*job)->base, entity, 1, owner); } int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 2416c526f9b0..3d0f8d182506 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -535,7 +535,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, ret = drm_sched_job_init(>sched_job, >sched_entity[args->pipe], -submit->ctx); +1, submit->ctx); if (ret) goto err_submit_put; diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c index 23a6276f1332..cdcb37ff62c3 100644 --- a/drivers/gpu/drm/lima/lima_sched.c
Re: [PATCH] drm/sched: Convert the GPU scheduler to variable number of run-queues
Hi,
I've pushed this commit as I got a verbal Acked-by from Christian in our kernel
meeting this morning.
Matt, please rebase your patches to drm-misc-next.
Regards,
Luben
On 2023-10-26 11:20, Luben Tuikov wrote:
> Ping!
>
> On 2023-10-22 23:22, Luben Tuikov wrote:
>> The GPU scheduler has now a variable number of run-queues, which are set up
>> at
>> drm_sched_init() time. This way, each driver announces how many run-queues it
>> requires (supports) per each GPU scheduler it creates. Note, that run-queues
>> correspond to scheduler "priorities", thus if the number of run-queues is set
>> to 1 at drm_sched_init(), then that scheduler supports a single run-queue,
>> i.e. single "priority". If a driver further sets a single entity per
>> run-queue, then this creates a 1-to-1 correspondence between a scheduler and
>> a scheduled entity.
>>
>> Cc: Lucas Stach
>> Cc: Russell King
>> Cc: Qiang Yu
>> Cc: Rob Clark
>> Cc: Abhinav Kumar
>> Cc: Dmitry Baryshkov
>> Cc: Danilo Krummrich
>> Cc: Matthew Brost
>> Cc: Boris Brezillon
>> Cc: Alex Deucher
>> Cc: Christian König
>> Cc: Emma Anholt
>> Cc: etna...@lists.freedesktop.org
>> Cc: l...@lists.freedesktop.org
>> Cc: linux-arm-...@vger.kernel.org
>> Cc: freedr...@lists.freedesktop.org
>> Cc: nouv...@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: Luben Tuikov
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c| 4 +-
>> drivers/gpu/drm/etnaviv/etnaviv_sched.c| 1 +
>> drivers/gpu/drm/lima/lima_sched.c | 4 +-
>> drivers/gpu/drm/msm/msm_ringbuffer.c | 5 +-
>> drivers/gpu/drm/nouveau/nouveau_sched.c| 1 +
>> drivers/gpu/drm/panfrost/panfrost_job.c| 1 +
>> drivers/gpu/drm/scheduler/sched_entity.c | 18 +-
>> drivers/gpu/drm/scheduler/sched_main.c | 74 ++
>> drivers/gpu/drm/v3d/v3d_sched.c| 5 ++
>> include/drm/gpu_scheduler.h| 9 ++-
>> 11 files changed, 98 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 2b8356699f235d..251995a90bbe69 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2280,6 +2280,7 @@ static int amdgpu_device_init_schedulers(struct
>> amdgpu_device *adev)
>> }
>>
>> r = drm_sched_init(>sched, _sched_ops,
>> + DRM_SCHED_PRIORITY_COUNT,
>> ring->num_hw_submission, 0,
>> timeout, adev->reset_domain->wq,
>> ring->sched_score, ring->name,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> index 78476bc75b4e1d..1f357198533f3e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> @@ -325,8 +325,8 @@ void amdgpu_job_stop_all_jobs_on_sched(struct
>> drm_gpu_scheduler *sched)
>> int i;
>>
>> /* Signal all jobs not yet scheduled */
>> -for (i = DRM_SCHED_PRIORITY_COUNT - 1; i >= DRM_SCHED_PRIORITY_MIN;
>> i--) {
>> -struct drm_sched_rq *rq = >sched_rq[i];
>> +for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_MIN; i--) {
>> +struct drm_sched_rq *rq = sched->sched_rq[i];
>> spin_lock(>lock);
>> list_for_each_entry(s_entity, >entities, list) {
>> while ((s_job =
>> to_drm_sched_job(spsc_queue_pop(_entity->job_queue {
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
>> b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
>> index 345fec6cb1a4c1..9b79f218e21afc 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
>> @@ -135,6 +135,7 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
>> int ret;
>>
>> ret = drm_sched_init(>sched, _sched_ops,
>> + DRM_SCHED_PRIORITY_COUNT,
>> etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
>> msecs_to_jiffies(500), NULL, NULL,
>> dev_name(gpu->dev), gpu->dev);
>> diff --git a/drivers/gpu/drm/lima/lima_sched.c
>> b/drivers/gpu/drm/lima/lima_sched.c
>> index ffd91a5ee29901..295f0353a02e58 100644
>> --- a/drivers/gpu/drm/lima/lima_sched.c
>> +++ b/drivers/gpu/drm/lima/lima_sched.c
>> @@ -488,7 +488,9 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe,
>> const char *name)
>>
>> INIT_WORK(>recover_work, lima_sched_recover_work);
>>
>> -return drm_sched_init(>base, _sched_ops, 1,
>> +return drm_sched_init(>base, _sched_ops,
>> + DRM_SCHED_PRIORITY_COUNT,
>> + 1,
>>lima_job_hang_limit,
>>msecs_to_jiffies(timeout), NULL,
>>
Re: [PATCH] MAINTAINERS: drm/ci: add entries for xfail files
On Tue, 19 Sep 2023 15:22:49 -0300, Helen Koike wrote: > DRM CI keeps track of which tests are failing, flaking or being skipped > by the ci in the expectations files. Add entries for those files to the > corresponding driver maintainer, so they can be notified when they > change. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH v2] drm/doc: ci: Require more context for flaky tests
Hi, On Thu, Oct 26, 2023 at 11:27:18AM -0300, Helen Koike wrote: > On 26/10/2023 10:27, Maxime Ripard wrote: > > On Thu, Oct 26, 2023 at 09:08:03AM -0300, Helen Koike wrote: > > > > > > > > > On 26/10/2023 09:01, Helen Koike wrote: > > > > > > > > > > > > On 26/10/2023 07:58, Maxime Ripard wrote: > > > > > On Wed, 25 Oct 2023 16:24:41 +0200, Maxime Ripard wrote: > > > > > > Flaky tests can be very difficult to reproduce after the facts, > > > > > > which > > > > > > will make it even harder to ever fix. > > > > > > > > > > > > Let's document the metadata we agreed on to provide more context to > > > > > > anyone trying to address these fixes. > > > > > > > > > > > > > > > > > > [...] > > > > > > > > > > Applied to drm/drm-misc (drm-misc-next). > > > > > > > > Thanks! > > > > > > > > Could you also apply > > > > https://patchwork.kernel.org/project/dri-devel/cover/20231024004525.169002-1-helen.ko...@collabora.com/ > > > > (and the dependencies listed on it). > > > > > > For some reason, commit message 7/10 (drm/ci: increase i915 job timeout to > > > 1h30m) looks incomplete in patchwork, but it looks fine in my branch: > > > > > > https://gitlab.freedesktop.org/helen.fornazier/linux/-/commits/for-drm-misc-wip/ > > > > > > Let me know if you prefer that I send it again or if you could pull from > > > the > > > branch. > > > > It was fine on lore.kernel.org and that's where I'm pulling from, so it all > > worked out :) > > > > Everything you asked for should be applied now > > > > Maxime > > Awesome, thank you! > > Sorry, just another request, could you please pull this other one updating > MAINTAINERS? > > https://patchwork.kernel.org/project/linux-arm-msm/patch/20230919182249.153499-1-helen.ko...@collabora.com/ I don't mind, but the expectation (the one I had at least) was that you would do it :) If you don't have drm-misc access, please create an account, you have done way more than expected to get one already Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/sched: Convert the GPU scheduler to variable number of run-queues
Ping!
On 2023-10-22 23:22, Luben Tuikov wrote:
> The GPU scheduler has now a variable number of run-queues, which are set up at
> drm_sched_init() time. This way, each driver announces how many run-queues it
> requires (supports) per each GPU scheduler it creates. Note, that run-queues
> correspond to scheduler "priorities", thus if the number of run-queues is set
> to 1 at drm_sched_init(), then that scheduler supports a single run-queue,
> i.e. single "priority". If a driver further sets a single entity per
> run-queue, then this creates a 1-to-1 correspondence between a scheduler and
> a scheduled entity.
>
> Cc: Lucas Stach
> Cc: Russell King
> Cc: Qiang Yu
> Cc: Rob Clark
> Cc: Abhinav Kumar
> Cc: Dmitry Baryshkov
> Cc: Danilo Krummrich
> Cc: Matthew Brost
> Cc: Boris Brezillon
> Cc: Alex Deucher
> Cc: Christian König
> Cc: Emma Anholt
> Cc: etna...@lists.freedesktop.org
> Cc: l...@lists.freedesktop.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Luben Tuikov
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c| 4 +-
> drivers/gpu/drm/etnaviv/etnaviv_sched.c| 1 +
> drivers/gpu/drm/lima/lima_sched.c | 4 +-
> drivers/gpu/drm/msm/msm_ringbuffer.c | 5 +-
> drivers/gpu/drm/nouveau/nouveau_sched.c| 1 +
> drivers/gpu/drm/panfrost/panfrost_job.c| 1 +
> drivers/gpu/drm/scheduler/sched_entity.c | 18 +-
> drivers/gpu/drm/scheduler/sched_main.c | 74 ++
> drivers/gpu/drm/v3d/v3d_sched.c| 5 ++
> include/drm/gpu_scheduler.h| 9 ++-
> 11 files changed, 98 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2b8356699f235d..251995a90bbe69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2280,6 +2280,7 @@ static int amdgpu_device_init_schedulers(struct
> amdgpu_device *adev)
> }
>
> r = drm_sched_init(>sched, _sched_ops,
> +DRM_SCHED_PRIORITY_COUNT,
> ring->num_hw_submission, 0,
> timeout, adev->reset_domain->wq,
> ring->sched_score, ring->name,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 78476bc75b4e1d..1f357198533f3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -325,8 +325,8 @@ void amdgpu_job_stop_all_jobs_on_sched(struct
> drm_gpu_scheduler *sched)
> int i;
>
> /* Signal all jobs not yet scheduled */
> - for (i = DRM_SCHED_PRIORITY_COUNT - 1; i >= DRM_SCHED_PRIORITY_MIN;
> i--) {
> - struct drm_sched_rq *rq = >sched_rq[i];
> + for (i = sched->num_rqs - 1; i >= DRM_SCHED_PRIORITY_MIN; i--) {
> + struct drm_sched_rq *rq = sched->sched_rq[i];
> spin_lock(>lock);
> list_for_each_entry(s_entity, >entities, list) {
> while ((s_job =
> to_drm_sched_job(spsc_queue_pop(_entity->job_queue {
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> index 345fec6cb1a4c1..9b79f218e21afc 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> @@ -135,6 +135,7 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
> int ret;
>
> ret = drm_sched_init(>sched, _sched_ops,
> + DRM_SCHED_PRIORITY_COUNT,
>etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
>msecs_to_jiffies(500), NULL, NULL,
>dev_name(gpu->dev), gpu->dev);
> diff --git a/drivers/gpu/drm/lima/lima_sched.c
> b/drivers/gpu/drm/lima/lima_sched.c
> index ffd91a5ee29901..295f0353a02e58 100644
> --- a/drivers/gpu/drm/lima/lima_sched.c
> +++ b/drivers/gpu/drm/lima/lima_sched.c
> @@ -488,7 +488,9 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe,
> const char *name)
>
> INIT_WORK(>recover_work, lima_sched_recover_work);
>
> - return drm_sched_init(>base, _sched_ops, 1,
> + return drm_sched_init(>base, _sched_ops,
> + DRM_SCHED_PRIORITY_COUNT,
> + 1,
> lima_job_hang_limit,
> msecs_to_jiffies(timeout), NULL,
> NULL, name, pipe->ldev->dev);
> diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c
> b/drivers/gpu/drm/msm/msm_ringbuffer.c
> index 40c0bc35a44cee..95257ab0185dc4 100644
> --- a/drivers/gpu/drm/msm/msm_ringbuffer.c
> +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
> @@ -95,8 +95,9 @@
Re: [PATCH v2 1/1] dt-bindings: backlight: mp3309c: remove two required properties
On Wed, Oct 25, 2023 at 05:50:57PM +0200, Flavio Suligoi wrote:
> The two properties:
>
> - max-brightness
> - default brightness
>
> are not really required, so they can be removed from the "required"
> section.
> The "max-brightness" is no longer used in the current version
> of the driver (it was used only in the first version).
> The "default-brightness", if omitted in the DT, is managed by the
> device driver, using a default value. This value depends on the dimming
> mode used:
>
> - for the "analog mode", via I2C commands, this value is fixed by
> hardware (=31)
> - while in case of pwm mode the default used is the last value of the
> brightness-levels array.
>
> Also the brightness-levels array is not required:
>
> - in "analog mode", via I2C commands, the brightness-level array is
> fixed by hardware (0..31).;
> - in pwm dimming mode, the driver uses a default array of 0..255 and
> the "default-brightness" is the last one, which is "255"
>
> NOTE: there are no compatibility problems with the previous version,
> since the device driver has not yet been included in any kernel.
> Only this dt-binding yaml file is already included in the
> "for-backlight-next" branch of the "backlight" kernel repository.
> No developer may have used it.
I'd argue
Fixes: 02c4e661658f ("dt-bindings: backlight: Add MPS MP3309C")
but that's up to Lee.
Acked-by: Conor Dooley
Thanks,
Conor.
>
> Other changes:
>
> - improve the backlight working mode description, in the "description"
> section
> - update the example, removing the "max-brightness" and introducing the
> "brightess-levels" property
>
> Signed-off-by: Flavio Suligoi
> ---
> .../bindings/leds/backlight/mps,mp3309c.yaml | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> index 4191e33626f5..527a37368ed7 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> +++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> @@ -14,8 +14,8 @@ description: |
>programmable switching frequency to optimize efficiency.
>It supports two different dimming modes:
>
> - - analog mode, via I2C commands (default)
> - - PWM controlled mode.
> + - analog mode, via I2C commands, as default mode (32 dimming levels)
> + - PWM controlled mode (optional)
>
>The datasheet is available at:
>https://www.monolithicpower.com/en/mp3309c.html
> @@ -50,8 +50,6 @@ properties:
> required:
>- compatible
>- reg
> - - max-brightness
> - - default-brightness
>
> unevaluatedProperties: false
>
> @@ -66,8 +64,8 @@ examples:
> compatible = "mps,mp3309c";
> reg = <0x17>;
> pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
> -max-brightness = <100>;
> -default-brightness = <80>;
> +brightness-levels = <0 4 8 16 32 64 128 255>;
> +default-brightness = <6>;
> mps,overvoltage-protection-microvolt = <2400>;
> };
> };
> --
> 2.34.1
>
signature.asc
Description: PGP signature
Re: [PATCH] drm: bridge: adv7511: fix reading edid segments
Hi Emil, On Thu, Oct 26, 2023 at 11:47 AM Emil Abildgaard Svendsen wrote: > > Currently reading EDID only works because usually only two EDID blocks > of 128 bytes is used. Where an EDID segment holds 256 bytes or two EDID > blocks. And the first EDID segment read works fine but E-EDID specifies > up to 128 segments. > > The logic is broken so change EDID segment index to multiple of 256 > bytes and not 128 (block size). > > Also change check of DDC status. Instead of silently not reading EDID > when in "IDLE" state [1]. Check if the DDC controller is in reset > instead (no HPD). > > [1] > ADV7511 Programming Guide: Table 11: DDCController Status: > > 0xC8 [3:0] DDC Controller State > > In Reset (No Hot Plug Detected) > 0001Reading EDID > 0010IDLE (Waiting for HDCP Requested) > 0011Initializing HDCP > 0100HDCP Enabled > 0101Initializing HDCP Repeater > > Signed-off-by: Emil Svendsen What about passing a Fixes tag?
[PULL] drm-intel-fixes
Hi Dave and Daniel, Here goes drm-intel-fixes-2023-10-26: - Determine context valid in OA reports (Umesh) - Hold GT forcewake during steering operations (Matt Roper) - Check if PMU is closed before stopping event (Umesh) Thanks, Rodrigo. The following changes since commit 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1: Linux 6.6-rc7 (2023-10-22 12:11:21 -1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2023-10-26 for you to fetch changes up to 4cbed7702eb775cca22fff6827a549092cb59f61: drm/i915/pmu: Check if pmu is closed before stopping event (2023-10-25 08:44:30 -0400) - Determine context valid in OA reports (Umesh) - Hold GT forcewake during steering operations (Matt Roper) - Check if PMU is closed before stopping event (Umesh) Matt Roper (1): drm/i915/mcr: Hold GT forcewake during steering operations Umesh Nerlige Ramappa (2): drm/i915/perf: Determine context valid in OA reports drm/i915/pmu: Check if pmu is closed before stopping event drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 24 ++-- drivers/gpu/drm/i915/i915_perf.c | 4 ++-- drivers/gpu/drm/i915/i915_pmu.c| 9 + 3 files changed, 33 insertions(+), 4 deletions(-)
Re: [PATCH 1/7] drm: Do not round to megabytes for greater than 1MiB sizes in fdinfo stats
On 28/09/2023 13:47, Tvrtko Ursulin wrote:
On 27/09/2023 14:48, Steven Price wrote:
On 27/09/2023 14:38, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
It is better not to lose precision and not revert to 1 MiB size
granularity for every size greater than 1 MiB.
Sizes in KiB should not be so troublesome to read (and in fact machine
parsing is I expect the norm here), they align with other api like
/proc/meminfo, and they allow writing tests for the interface without
having to embed drm.ko implementation knowledge into them. (Like knowing
that minimum buffer size one can use for successful verification has
to be
1MiB aligned, and on top account for any pre-existing memory utilisation
outside of driver's control.)
But probably even more importantly I think that it is just better to
show
the accurate sizes and not arbitrary lose precision for a little bit
of a
stretched use case of eyeballing fdinfo text directly.
Signed-off-by: Tvrtko Ursulin
Cc: Rob Clark
Cc: Adrián Larumbe
Cc: steven.pr...@arm.com
Reviewed-by: Steven Price
Thanks! Rob? Can we have your blessing? Could you live with KiBs? :)
Acked received on #dri-devel:
[12:15] robclark: ping on
https://lists.freedesktop.org/archives/dri-devel/2023-September/424905.html - can you
live with it or object?
[14:41] tursulin: a-b
Adding the drm-misc maintainers with an ask to merge please.
Regards,
Tvrtko
Regards,
Tvrtko
---
drivers/gpu/drm/drm_file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index e692770ef6d3..ecb5038009e7 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -913,7 +913,7 @@ static void print_size(struct drm_printer *p,
const char *stat,
unsigned u;
for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
- if (sz < SZ_1K)
+ if (sz == 0 || !IS_ALIGNED(sz, SZ_1K))
break;
sz = div_u64(sz, SZ_1K);
}
drm/panel: panel-simple power-off sequencing
Hi, We have a parallel LCD panel which is driven by panel/panel-simple. The power-off sequence specified in the datasheet requires that the enable-gpio must be deasserted for a number of VSYNC cycles before shutting down all other control signals. See the diagram below: __ __ __ __ __ CLK, VSYNC, DE, HSYNC: __><__><__><__><__\_ __ enable-gpio :\_ So far, in kernel 5.4 we relied on the unprepare delay time for making sure that the enable-gpio timing requirements are fulfilled. That is, the panel_simple_unprepare() would: 1. Deassert the enable-gpio 2. Switch off the voltage regulator 3. Wait display_desc.delay.unprepare milliseconds Afterwards the IPU was shutdown, and all the control signals stopped. But with the below commits: - 3235b0f20a0a4135e9053f1174d096eff166d0fb "drm/panel: panel-simple: Use runtime pm to avoid excessive unprepare / prepare" - e5e30dfcf3db1534019c40d94ed58fd2869f9359 "drm: panel: simple: Defer unprepare delay till next prepare to shorten it" The enable-gpio is now deasserted in panel_simple_suspend(), which is called some time after the disablement of control signals are stopped: __ __ __ __ __ CLK, VSYNC, DE, HSYNC: __><__><__><__><__\_ __ enable-gpio :\_ With the latest panel-simple, is there a way which allows us to deassert enable-gpio before the control signals stop? Mit freundlichen Grüßen / Best regards Mark Jonas Building Technologies, Panel Software Fire (BT-FIR/ENG1-Grb) Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | www.boschsecurity.com Tel. +49 89 6290-1233 | Telefax +49 89 6290-281233 | mark.jo...@de.bosch.com Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118 Aufsichtsratsvorsitzender: Christian Fischer; Geschäftsführung: Thomas Quante, Peter Löffler, Henrik Siegle
Re: (subset) [PATCH v2] drm/doc: ci: Require more context for flaky tests
On 26/10/2023 10:27, Maxime Ripard wrote: On Thu, Oct 26, 2023 at 09:08:03AM -0300, Helen Koike wrote: On 26/10/2023 09:01, Helen Koike wrote: On 26/10/2023 07:58, Maxime Ripard wrote: On Wed, 25 Oct 2023 16:24:41 +0200, Maxime Ripard wrote: Flaky tests can be very difficult to reproduce after the facts, which will make it even harder to ever fix. Let's document the metadata we agreed on to provide more context to anyone trying to address these fixes. [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Could you also apply https://patchwork.kernel.org/project/dri-devel/cover/20231024004525.169002-1-helen.ko...@collabora.com/ (and the dependencies listed on it). For some reason, commit message 7/10 (drm/ci: increase i915 job timeout to 1h30m) looks incomplete in patchwork, but it looks fine in my branch: https://gitlab.freedesktop.org/helen.fornazier/linux/-/commits/for-drm-misc-wip/ Let me know if you prefer that I send it again or if you could pull from the branch. It was fine on lore.kernel.org and that's where I'm pulling from, so it all worked out :) Everything you asked for should be applied now Maxime Awesome, thank you! Sorry, just another request, could you please pull this other one updating MAINTAINERS? https://patchwork.kernel.org/project/linux-arm-msm/patch/20230919182249.153499-1-helen.ko...@collabora.com/ So get_maintainers.pl returns the right people and we don't forget to notify maintainers when flakes/fails/skips are updated. Thank again. Helen
[PATCH] drm: bridge: adv7511: fix reading edid segments
Currently reading EDID only works because usually only two EDID blocks
of 128 bytes is used. Where an EDID segment holds 256 bytes or two EDID
blocks. And the first EDID segment read works fine but E-EDID specifies
up to 128 segments.
The logic is broken so change EDID segment index to multiple of 256
bytes and not 128 (block size).
Also change check of DDC status. Instead of silently not reading EDID
when in "IDLE" state [1]. Check if the DDC controller is in reset
instead (no HPD).
[1]
ADV7511 Programming Guide: Table 11: DDCController Status:
0xC8 [3:0] DDC Controller State
In Reset (No Hot Plug Detected)
0001Reading EDID
0010IDLE (Waiting for HDCP Requested)
0011Initializing HDCP
0100HDCP Enabled
0101Initializing HDCP Repeater
Signed-off-by: Emil Svendsen
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 24
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 8be235144f6d..c641c2ccd412 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -537,6 +537,8 @@ static int adv7511_get_edid_block(void *data, u8 *buf,
unsigned int block,
size_t len)
{
struct adv7511 *adv7511 = data;
+ struct device* dev = >i2c_edid->dev;
+ int edid_segment = block / 2;
struct i2c_msg xfer[2];
uint8_t offset;
unsigned int i;
@@ -545,7 +547,7 @@ static int adv7511_get_edid_block(void *data, u8 *buf,
unsigned int block,
if (len > 128)
return -EINVAL;
- if (adv7511->current_edid_segment != block / 2) {
+ if (adv7511->current_edid_segment != edid_segment) {
unsigned int status;
ret = regmap_read(adv7511->regmap, ADV7511_REG_DDC_STATUS,
@@ -553,15 +555,19 @@ static int adv7511_get_edid_block(void *data, u8 *buf,
unsigned int block,
if (ret < 0)
return ret;
- if (status != 2) {
- adv7511->edid_read = false;
- regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT,
-block);
- ret = adv7511_wait_for_edid(adv7511, 200);
- if (ret < 0)
- return ret;
+ if (!(status & 0x0F)) {
+ dev_dbg(dev, "DDC in reset no hot plug detected %x\n",
+status);
+ return -EIO;
}
+ adv7511->edid_read = false;
+ regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT,
+edid_segment);
+ ret = adv7511_wait_for_edid(adv7511, 200);
+ if (ret < 0)
+ return ret;
+
/* Break this apart, hopefully more I2C controllers will
* support 64 byte transfers than 256 byte transfers
*/
@@ -589,7 +595,7 @@ static int adv7511_get_edid_block(void *data, u8 *buf,
unsigned int block,
offset += 64;
}
- adv7511->current_edid_segment = block / 2;
+ adv7511->current_edid_segment = edid_segment;
}
if (block % 2 == 0)
--
2.34.1
Re: (subset) [PATCH v2] drm/doc: ci: Require more context for flaky tests
On Thu, Oct 26, 2023 at 09:08:03AM -0300, Helen Koike wrote: > > > On 26/10/2023 09:01, Helen Koike wrote: > > > > > > On 26/10/2023 07:58, Maxime Ripard wrote: > > > On Wed, 25 Oct 2023 16:24:41 +0200, Maxime Ripard wrote: > > > > Flaky tests can be very difficult to reproduce after the facts, which > > > > will make it even harder to ever fix. > > > > > > > > Let's document the metadata we agreed on to provide more context to > > > > anyone trying to address these fixes. > > > > > > > > > > > > [...] > > > > > > Applied to drm/drm-misc (drm-misc-next). > > > > Thanks! > > > > Could you also apply > > https://patchwork.kernel.org/project/dri-devel/cover/20231024004525.169002-1-helen.ko...@collabora.com/ > > (and the dependencies listed on it). > > For some reason, commit message 7/10 (drm/ci: increase i915 job timeout to > 1h30m) looks incomplete in patchwork, but it looks fine in my branch: > > https://gitlab.freedesktop.org/helen.fornazier/linux/-/commits/for-drm-misc-wip/ > > Let me know if you prefer that I send it again or if you could pull from the > branch. It was fine on lore.kernel.org and that's where I'm pulling from, so it all worked out :) Everything you asked for should be applied now Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/ci: Enable CONFIG_BACKLIGHT_CLASS_DEVICE
On Mon, 02 Oct 2023 09:47:14 -0700, Rob Clark wrote: > Dependency for CONFIG_DRM_PANEL_EDP. Missing this was causing the drm > driver to not probe on devices that use panel-edp. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH 1/2] drm/ci: pick up -external-fixes from the merge target repo
On Sun, 08 Oct 2023 16:23:19 +0300, Dmitry Baryshkov wrote: > In case of the merge requests it might be useful to push repo-specific > fixes which have not yet propagated to the -external-fixes branch in the > main UPSTREAM_REPO. For example, in case of drm/msm development, we are > staging fixes locally for testing, before pushing them to the drm/drm > repo. Thus, if the CI run was triggered by merge request, also pick up > the -external fixes basing on the the CI_MERGE target repo / and branch. > > [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH v3 00/10] drm/ci: fixes and improvements
On Mon, 23 Oct 2023 21:45:15 -0300, Helen Koike wrote: > This series contains the several fixes, making drm/ci much > more reliable and useful. > > Highlights: > > * Current DRM/CI in drm-misc is broken, this series fixes it with mesa > uprev (commit 1/9). > > [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH 2/2] fbdev/simplefb: Add support for generic power-domains
Hi,
Thank you for your patches.
On 10/11/23 16:38, Thierry Reding wrote:
> From: Thierry Reding
>
> The simple-framebuffer device tree bindings document the power-domains
> property, so make sure that simplefb supports it. This ensures that the
> power domains remain enabled as long as simplefb is active.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/video/fbdev/simplefb.c | 93 +-
> 1 file changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index 18025f34fde7..e69fb0ad2d54 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> static const struct fb_fix_screeninfo simplefb_fix = {
> @@ -78,6 +79,11 @@ struct simplefb_par {
> unsigned int clk_count;
> struct clk **clks;
> #endif
> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> + unsigned int num_genpds;
> + struct device **genpds;
> + struct device_link **genpd_links;
> +#endif
> #if defined CONFIG_OF && defined CONFIG_REGULATOR
> bool regulators_enabled;
> u32 regulator_count;
> @@ -432,6 +438,83 @@ static void simplefb_regulators_enable(struct
> simplefb_par *par,
> static void simplefb_regulators_destroy(struct simplefb_par *par) { }
> #endif
>
> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> +static void simplefb_detach_genpds(void *res)
> +{
> + struct simplefb_par *par = res;
> + unsigned int i = par->num_genpds;
> +
> + if (par->num_genpds <= 1)
> + return;
> +
> + while (i--) {
> + if (par->genpd_links[i])
> + device_link_del(par->genpd_links[i]);
> +
> + if (!IS_ERR_OR_NULL(par->genpds[i]))
> + dev_pm_domain_detach(par->genpds[i], true);
> + }
Using this i-- construct means that genpd at index 0 will
not be cleaned up.
I think it would be best to instead use the same construct
as the simpledrm version of this which makes i signed
(and does not initialize it) and then does:
for (i = sdev->pwr_dom_count - 1; i >= 0; i--) { .. }
> +}
> +
> +static int simplefb_attach_genpd(struct simplefb_par *par,
> + struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + unsigned int i;
> + int err;
> +
> + par->num_genpds = of_count_phandle_with_args(dev->of_node,
> + "power-domains",
> + "#power-domain-cells");
> + /*
> + * Single power-domain devices are handled by the driver core, so
> + * nothing to do here.
> + */
> + if (par->num_genpds <= 1)
> + return 0;
> +
> + par->genpds = devm_kcalloc(dev, par->num_genpds, sizeof(*par->genpds),
> +GFP_KERNEL);
> + if (!par->genpds)
> + return -ENOMEM;
> +
> + par->genpd_links = devm_kcalloc(dev, par->num_genpds,
> + sizeof(*par->genpd_links),
> + GFP_KERNEL);
> + if (!par->genpd_links)
> + return -ENOMEM;
> +
> + for (i = 0; i < par->num_genpds; i++) {
> + par->genpds[i] = dev_pm_domain_attach_by_id(dev, i);
> + if (IS_ERR(par->genpds[i])) {
> + err = PTR_ERR(par->genpds[i]);
> + if (err == -EPROBE_DEFER) {
> + simplefb_detach_genpds(par);
> + return err;
> + }
> +
> + dev_warn(dev, "failed to attach domain %u: %d\n", i,
> err);
> + continue;
> + }
> +
> + par->genpd_links[i] = device_link_add(dev, par->genpds[i],
> + DL_FLAG_STATELESS |
> + DL_FLAG_PM_RUNTIME |
> + DL_FLAG_RPM_ACTIVE);
> + if (!par->genpd_links[i])
> + dev_warn(dev, "failed to link power-domain %u\n", i);
> + }
> +
> + return devm_add_action_or_reset(dev, simplefb_detach_genpds, par);
> +}
> +#else
> +static int simplefb_attach_genpd(struct simplefb_par *par,
> + struct platform_device *pdev)
> +{
> + return 0;
> +}
> +#endif
> +
> static int simplefb_probe(struct platform_device *pdev)
> {
> int ret;
> @@ -518,6 +601,10 @@ static int simplefb_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_clocks;
>
> + ret = simplefb_attach_genpd(par, pdev);
> + if (ret < 0)
> + goto error_regulators;
> +
> simplefb_clocks_enable(par, pdev);
> simplefb_regulators_enable(par, pdev);
>
> @@ -534,18 +621,20 @@ static int
Re: [PATCH 1/2] fbdev/simplefb: Support memory-region property
Hi,
On 10/11/23 16:38, Thierry Reding wrote:
> From: Thierry Reding
>
> The simple-framebuffer bindings specify that the "memory-region"
> property can be used as an alternative to the "reg" property to define
> the framebuffer memory used by the display hardware. Implement support
> for this in the simplefb driver.
>
> Signed-off-by: Thierry Reding
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
> ---
> drivers/video/fbdev/simplefb.c | 35 +-
> 1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index 62f99f6fccd3..18025f34fde7 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -121,12 +122,13 @@ struct simplefb_params {
> u32 height;
> u32 stride;
> struct simplefb_format *format;
> + struct resource memory;
> };
>
> static int simplefb_parse_dt(struct platform_device *pdev,
> struct simplefb_params *params)
> {
> - struct device_node *np = pdev->dev.of_node;
> + struct device_node *np = pdev->dev.of_node, *mem;
> int ret;
> const char *format;
> int i;
> @@ -166,6 +168,23 @@ static int simplefb_parse_dt(struct platform_device
> *pdev,
> return -EINVAL;
> }
>
> + mem = of_parse_phandle(np, "memory-region", 0);
> + if (mem) {
> + ret = of_address_to_resource(mem, 0, >memory);
> + if (ret < 0) {
> + dev_err(>dev, "failed to parse memory-region\n");
> + of_node_put(mem);
> + return ret;
> + }
> +
> + if (of_property_present(np, "reg"))
> + dev_warn(>dev, "preferring \"memory-region\" over
> \"reg\" property\n");
> +
> + of_node_put(mem);
> + } else {
> + memset(>memory, 0, sizeof(params->memory));
> + }
> +
> return 0;
> }
>
> @@ -193,6 +212,8 @@ static int simplefb_parse_pd(struct platform_device *pdev,
> return -EINVAL;
> }
>
> + memset(>memory, 0, sizeof(params->memory));
> +
> return 0;
> }
>
> @@ -431,10 +452,14 @@ static int simplefb_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res) {
> - dev_err(>dev, "No memory resource\n");
> - return -EINVAL;
> + if (params.memory.start == 0 && params.memory.end == 0) {
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(>dev, "No memory resource\n");
> + return -EINVAL;
> + }
> + } else {
> + res =
> }
>
> mem = request_mem_region(res->start, resource_size(res), "simplefb");
Re: [Intel-gfx] [PATCH] drm/i915/mtl: avoid stringop-overflow warning
On Mon, 23 Oct 2023, Jani Nikula wrote:
> On Mon, 16 Oct 2023, Arnd Bergmann wrote:
>> From: Arnd Bergmann
>>
>> The newly added memset() causes a warning for some reason I could not figure
>> out:
>>
>> In file included from arch/x86/include/asm/string.h:3,
>> from drivers/gpu/drm/i915/gt/intel_rc6.c:6:
>> In function 'rc6_res_reg_init',
>> inlined from 'intel_rc6_init' at
>> drivers/gpu/drm/i915/gt/intel_rc6.c:610:2:
>> arch/x86/include/asm/string_32.h:195:29: error: '__builtin_memset' writing
>> 16 bytes into a region of size 0 overflows the destination
>> [-Werror=stringop-overflow=]
>> 195 | #define memset(s, c, count) __builtin_memset(s, c, count)
>> | ^
>> drivers/gpu/drm/i915/gt/intel_rc6.c:584:9: note: in expansion of macro
>> 'memset'
>> 584 | memset(rc6->res_reg, INVALID_MMIO_REG.reg,
>> sizeof(rc6->res_reg));
>> | ^~
>> In function 'intel_rc6_init':
>>
>> Change it to an normal initializer and an added memcpy() that does not have
>> this problem.
>>
>> Fixes: 4bb9ca7ee0745 ("drm/i915/mtl: C6 residency and C state type for MTL
>> SAMedia")
>> Signed-off-by: Arnd Bergmann
>> ---
>> drivers/gpu/drm/i915/gt/intel_rc6.c | 16 ++--
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c
>> b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> index 8b67abd720be8..7090e4be29cb6 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> @@ -581,19 +581,23 @@ static void __intel_rc6_disable(struct intel_rc6 *rc6)
>>
>> static void rc6_res_reg_init(struct intel_rc6 *rc6)
>> {
>> -memset(rc6->res_reg, INVALID_MMIO_REG.reg, sizeof(rc6->res_reg));
>
> That's just bollocks. memset() is byte granularity, while
> INVALID_MMIO_REG.reg is u32. If the value was anything other than 0,
> this would break.
>
> And you're not supposed to look at the guts of i915_reg_t to begin with,
> that's why it's a typedef. Basically any code that accesses the members
> of i915_reg_t outside of its implementation are doing it wrong.
>
> Reviewed-by: Jani Nikula
Thanks for the patch, pushed to drm-intel-gt-next.
BR,
Jani.
>
>
>> +i915_reg_t res_reg[INTEL_RC6_RES_MAX] = {
>> +[0 ... INTEL_RC6_RES_MAX - 1] = INVALID_MMIO_REG,
>> +};
>>
>> switch (rc6_to_gt(rc6)->type) {
>> case GT_MEDIA:
>> -rc6->res_reg[INTEL_RC6_RES_RC6] = MTL_MEDIA_MC6;
>> +res_reg[INTEL_RC6_RES_RC6] = MTL_MEDIA_MC6;
>> break;
>> default:
>> -rc6->res_reg[INTEL_RC6_RES_RC6_LOCKED] = GEN6_GT_GFX_RC6_LOCKED;
>> -rc6->res_reg[INTEL_RC6_RES_RC6] = GEN6_GT_GFX_RC6;
>> -rc6->res_reg[INTEL_RC6_RES_RC6p] = GEN6_GT_GFX_RC6p;
>> -rc6->res_reg[INTEL_RC6_RES_RC6pp] = GEN6_GT_GFX_RC6pp;
>> +res_reg[INTEL_RC6_RES_RC6_LOCKED] = GEN6_GT_GFX_RC6_LOCKED;
>> +res_reg[INTEL_RC6_RES_RC6] = GEN6_GT_GFX_RC6;
>> +res_reg[INTEL_RC6_RES_RC6p] = GEN6_GT_GFX_RC6p;
>> +res_reg[INTEL_RC6_RES_RC6pp] = GEN6_GT_GFX_RC6pp;
>> break;
>> }
>> +
>> +memcpy(rc6->res_reg, res_reg, sizeof(res_reg));
>> }
>>
>> void intel_rc6_init(struct intel_rc6 *rc6)
--
Jani Nikula, Intel
Re: (subset) [PATCH v2] drm/doc: ci: Require more context for flaky tests
On 26/10/2023 09:01, Helen Koike wrote: On 26/10/2023 07:58, Maxime Ripard wrote: On Wed, 25 Oct 2023 16:24:41 +0200, Maxime Ripard wrote: Flaky tests can be very difficult to reproduce after the facts, which will make it even harder to ever fix. Let's document the metadata we agreed on to provide more context to anyone trying to address these fixes. [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Could you also apply https://patchwork.kernel.org/project/dri-devel/cover/20231024004525.169002-1-helen.ko...@collabora.com/ (and the dependencies listed on it). For some reason, commit message 7/10 (drm/ci: increase i915 job timeout to 1h30m) looks incomplete in patchwork, but it looks fine in my branch: https://gitlab.freedesktop.org/helen.fornazier/linux/-/commits/for-drm-misc-wip/ Let me know if you prefer that I send it again or if you could pull from the branch. Thanks Helen Drm/ci in drm-misc is broken atm, there are some people asking me how to run it, and this unbreaks it. Thanks again Helen Thanks! Maxime
Re: (subset) [PATCH v2] drm/doc: ci: Require more context for flaky tests
On 26/10/2023 07:58, Maxime Ripard wrote: On Wed, 25 Oct 2023 16:24:41 +0200, Maxime Ripard wrote: Flaky tests can be very difficult to reproduce after the facts, which will make it even harder to ever fix. Let's document the metadata we agreed on to provide more context to anyone trying to address these fixes. [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Could you also apply https://patchwork.kernel.org/project/dri-devel/cover/20231024004525.169002-1-helen.ko...@collabora.com/ (and the dependencies listed on it). Drm/ci in drm-misc is broken atm, there are some people asking me how to run it, and this unbreaks it. Thanks again Helen Thanks! Maxime
Re: [PATCH v2 6/6] drm/vs: Add hdmi driver
On Thu, Oct 26, 2023 at 11:57:22AM +0300, Dmitry Baryshkov wrote:
> On Thu, 26 Oct 2023 at 11:07, Maxime Ripard wrote:
> >
> > On Thu, Oct 26, 2023 at 01:23:53AM +0300, Dmitry Baryshkov wrote:
> > > > +static int starfive_hdmi_register(struct drm_device *drm, struct
> > > > starfive_hdmi *hdmi)
> > > > +{
> > > > + struct drm_encoder *encoder = >encoder;
> > > > + struct device *dev = hdmi->dev;
> > > > +
> > > > + encoder->possible_crtcs = drm_of_find_possible_crtcs(drm,
> > > > dev->of_node);
> > > > +
> > > > + /*
> > > > +* If we failed to find the CRTC(s) which this encoder is
> > > > +* supposed to be connected to, it's because the CRTC has
> > > > +* not been registered yet. Defer probing, and hope that
> > > > +* the required CRTC is added later.
> > > > +*/
> > > > + if (encoder->possible_crtcs == 0)
> > > > + return -EPROBE_DEFER;
> > > > +
> > > > + drm_encoder_helper_add(encoder,
> > > > _hdmi_encoder_helper_funcs);
> > > > +
> > > > + hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
> > > > +
> > > > + drm_connector_helper_add(>connector,
> > > > +_hdmi_connector_helper_funcs);
> > > > + drmm_connector_init(drm, >connector,
> > > > + _hdmi_connector_funcs,
> > > > + DRM_MODE_CONNECTOR_HDMIA,
> > >
> > > On an embedded device one can not be so sure. There can be MHL or HDMI
> > > Alternative Mode. Usually we use drm_bridge here and drm_bridge_connector.
> >
> > On an HDMI driver, it's far from being a requirement, especially given
> > the limitations bridges have.
>
> It's a blessing that things like MHL / HDMI-in-USB-C / HDMI-to-MyDP
> are not widely used in the wild and are mostly non-existing except
> several phones that preate wide DP usage.
And those can be supported without relying on bridges.
> Using drm_connector directly prevents one from handling possible
> modifications on the board level. For example, with the DRM connector
> in place, handling a separate HPD GPIO will result in code duplication
> from the hdmi-connector driver. Handling any other variations in the
> board design (which are pretty common in the embedded world) will also
> require changing the driver itself. drm_bridge / drm_bridge_connector
> save us from those issues.
And we have other solutions there too. Like, EDIDs are pretty much in
the same spot with a lot of device variations, but it also works without
a common driver. I'd really wish we were having less bridges and more
helpers, but here we are.
> BTW: what are the limitations of the drm_bridge wrt. HDMI output? I'm
> asking because we heavily depend on the bridge infrastructure for HDMI
> output. Maybe we are missing something there, which went unnoticed to
> me and my colleagues.
A bridge cannot extend the connector state or use properties, for
example. It works for basic stuff but falls apart as soon as you're
trying to do something slightly advanced.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v5 4/6] drm/bridge: implement generic DP HPD bridge
Hi Dmitry,
kernel test robot noticed the following build errors:
[auto build test ERROR on next-20231025]
[also build test ERROR on v6.6-rc7]
[cannot apply to drm-misc/drm-misc-next usb/usb-testing usb/usb-next
usb/usb-linus drm/drm-next linus/master v6.6-rc7 v6.6-rc6 v6.6-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Dmitry-Baryshkov/drm-bridge-add-transparent-bridge-helper/20231026-063135
base: next-20231025
patch link:
https://lore.kernel.org/r/20231025223027.943563-5-dmitry.baryshkov%40linaro.org
patch subject: [PATCH v5 4/6] drm/bridge: implement generic DP HPD bridge
config: csky-randconfig-002-20231026
(https://download.01.org/0day-ci/archive/20231026/202310261906.c62l4fc2-...@intel.com/config)
compiler: csky-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20231026/202310261906.c62l4fc2-...@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/202310261906.c62l4fc2-...@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from drivers/phy/qualcomm/phy-qcom-qmp-combo.c:24:
>> include/drm/bridge/aux-bridge.h:28:1: error: expected identifier or '('
>> before '{' token
28 | {
| ^
>> include/drm/bridge/aux-bridge.h:26:30: warning: 'drm_dp_hpd_bridge_register'
>> declared 'static' but never defined [-Wunused-function]
26 | static inline struct device *drm_dp_hpd_bridge_register(struct
device *parent,
| ^~
vim +28 include/drm/bridge/aux-bridge.h
20
21 #if IS_ENABLED(CONFIG_DRM_AUX_HPD_BRIDGE)
22 struct device *drm_dp_hpd_bridge_register(struct device *parent,
23struct device_node *np);
24 void drm_aux_hpd_bridge_notify(struct device *dev, enum
drm_connector_status status);
25 #else
> 26 static inline struct device *drm_dp_hpd_bridge_register(struct device
*parent,
27 struct
device_node *np);
> 28 {
29 return 0;
30 }
31
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: (subset) [PATCH] drm/vc4: tests: Fix UAF in the mock helpers
On Tue, 24 Oct 2023 12:56:40 +0200, Maxime Ripard wrote: > The VC4 mock helpers allocate the CRTC, encoders and connectors using a > call to kunit_kzalloc(), but the DRM device they are attache to survives > for longer than the test itself which leads to use-after-frees reported > by KASAN. > > Switch to drmm_kzalloc to tie the lifetime of these objects to the main > DRM device. > > [...] Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
Re: [PATCH] drm/doc: ci: Require more context for flaky tests
On Thu, Oct 26, 2023 at 12:58:48PM +0200, Maxime Ripard wrote: > On Thu, 19 Oct 2023 11:46:09 +0200, Maxime Ripard wrote: > > Flaky tests can be very difficult to reproduce after the facts, which > > will make it even harder to ever fix. > > > > Let's document the metadata we agreed on to provide more context to > > anyone trying to address these fixes. > > > > > > [...] > > Applied to drm/drm-misc (drm-misc-next). b4 might have been confused, but I only applied the v2. Maxime signature.asc Description: PGP signature
[PULL] drm-misc-fixes
Hi, this is the week's PR for drm-misc-fixes. Best regards Thomas drm-misc-fixes-2023-10-26: Short summary of fixes pull: amdgpu: - ignore duplicated BOs in CS parser - remove redundant call to amdgpu_ctx_priority_is_valid() amdkfd: - reserve fence slot while locking BO dp_mst: - Fix NULL deref in get_mst_branch_device_by_guid_helper() logicvc: - Kconfig: Select REGMAP and REGMAP_MMIO ivpu: - Fix missing VPUIP interrupts The following changes since commit 8f5ad367e8b884772945c6c9fb622ac94b7d3e32: accel/ivpu: Extend address range for MMU mmap (2023-10-19 08:01:20 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2023-10-26 for you to fetch changes up to b132ac51d7a50c37683be56c96ff64f8c887930f: accel/ivpu/37xx: Fix missing VPUIP interrupts (2023-10-26 07:43:28 +0200) Short summary of fixes pull: amdgpu: - ignore duplicated BOs in CS parser - remove redundant call to amdgpu_ctx_priority_is_valid() amdkfd: - reserve fence slot while locking BO dp_mst: - Fix NULL deref in get_mst_branch_device_by_guid_helper() logicvc: - Kconfig: Select REGMAP and REGMAP_MMIO ivpu: - Fix missing VPUIP interrupts Christian König (2): drm/amdgpu: ignore duplicate BOs again drm/amdkfd: reserve a fence slot while locking the BO Karol Wachowski (1): accel/ivpu/37xx: Fix missing VPUIP interrupts Luben Tuikov (1): drm/amdgpu: Remove redundant call to priority_is_valid() Lukasz Majczak (1): drm/dp_mst: Fix NULL deref in get_mst_branch_device_by_guid_helper() Sui Jingfeng (1): drm/logicvc: Kconfig: select REGMAP and REGMAP_MMIO drivers/accel/ivpu/ivpu_hw_37xx.c| 11 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 15 --- drivers/gpu/drm/display/drm_dp_mst_topology.c| 6 +++--- drivers/gpu/drm/logicvc/Kconfig | 2 ++ 6 files changed, 21 insertions(+), 18 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Re: [PATCH] drm/doc: ci: Require more context for flaky tests
On Thu, 19 Oct 2023 11:46:09 +0200, Maxime Ripard wrote: > Flaky tests can be very difficult to reproduce after the facts, which > will make it even harder to ever fix. > > Let's document the metadata we agreed on to provide more context to > anyone trying to address these fixes. > > > [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH v2] drm/doc: ci: Require more context for flaky tests
On Wed, 25 Oct 2023 16:24:41 +0200, Maxime Ripard wrote: > Flaky tests can be very difficult to reproduce after the facts, which > will make it even harder to ever fix. > > Let's document the metadata we agreed on to provide more context to > anyone trying to address these fixes. > > > [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH] drm/vc4: tests: Fix UAF in the mock helpers
On Tue, 24 Oct 2023 at 18:38, Maíra Canal wrote:
>
> On 10/24/23 07:56, Maxime Ripard wrote:
> > The VC4 mock helpers allocate the CRTC, encoders and connectors using a
> > call to kunit_kzalloc(), but the DRM device they are attache to survives
> > for longer than the test itself which leads to use-after-frees reported
> > by KASAN.
> >
> > Switch to drmm_kzalloc to tie the lifetime of these objects to the main
> > DRM device.
> >
> > Fixes: f759f5b53f1c ("drm/vc4: tests: Introduce a mocking infrastructure")
> > Closes:
> > https://lore.kernel.org/all/ca+g9fyvja2hgqzr9lggq63v0skauejhae6f7+z9cwwn-our...@mail.gmail.com/
> > Reported-by: Linux Kernel Functional Testing
> > Signed-off-by: Maxime Ripard
>
> Reviewed-by: Maíra Canal
Tested-by: Anders Roxell
I tested this patch ontop of next-20231025 and on next-20231011 when
we first saw the issue,
both kernels booted fine on a RPI4(bcm2711-rpi-4-b)
Cheers,
Anders
>
> Best Regards,
> - Maíra Canal
>
> >
> > ---
> >
> > Cc: Naresh Kamboju
> > Cc: Dan Carpenter
> > ---
> > drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c | 2 +-
> > drivers/gpu/drm/vc4/tests/vc4_mock_output.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> > b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> > index 5d12d7beef0e..ade3309ae042 100644
> > --- a/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> > +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> > @@ -26,7 +26,7 @@ struct vc4_dummy_crtc *vc4_mock_pv(struct kunit *test,
> > struct vc4_crtc *vc4_crtc;
> > int ret;
> >
> > - dummy_crtc = kunit_kzalloc(test, sizeof(*dummy_crtc), GFP_KERNEL);
> > + dummy_crtc = drmm_kzalloc(drm, sizeof(*dummy_crtc), GFP_KERNEL);
> > KUNIT_ASSERT_NOT_NULL(test, dummy_crtc);
> >
> > vc4_crtc = _crtc->crtc;
> > diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > index 6e11fcc9ef45..e70d7c3076ac 100644
> > --- a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > @@ -32,7 +32,7 @@ struct vc4_dummy_output *vc4_dummy_output(struct kunit
> > *test,
> > struct drm_encoder *enc;
> > int ret;
> >
> > - dummy_output = kunit_kzalloc(test, sizeof(*dummy_output), GFP_KERNEL);
> > + dummy_output = drmm_kzalloc(drm, sizeof(*dummy_output), GFP_KERNEL);
> > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_output);
> > dummy_output->encoder.type = vc4_encoder_type;
> >
Re: [PATCH v2 5/6] drm/vs: Add KMS crtc
sorry
Dmitry ,accidentally wrote the wrong name
Take no offense
On 2023/10/26 17:42, Keith Zhao wrote:
> hi Ville:
> very glad to receive your feedback
> Some of them are very good ideas.
> Some are not very clear and hope to get your further reply!
>
>
> On 2023/10/26 3:49, Ville Syrjälä wrote:
>> On Wed, Oct 25, 2023 at 10:28:56PM +0300, Dmitry Baryshkov wrote:
>>> On 25/10/2023 13:39, Keith Zhao wrote:
>>> > add 2 crtcs and 8 planes in vs-drm
>>> >
>>> > Signed-off-by: Keith Zhao
>>> > ---
>>> > drivers/gpu/drm/verisilicon/Makefile |8 +-
>>> > drivers/gpu/drm/verisilicon/vs_crtc.c | 257
>>> > drivers/gpu/drm/verisilicon/vs_crtc.h | 43 +
>>> > drivers/gpu/drm/verisilicon/vs_dc.c| 1002
>>> > drivers/gpu/drm/verisilicon/vs_dc.h| 80 +
>>> > drivers/gpu/drm/verisilicon/vs_dc_hw.c | 1959
>>> > drivers/gpu/drm/verisilicon/vs_dc_hw.h | 492 ++
>>> > drivers/gpu/drm/verisilicon/vs_drv.c |2 +
>>> > drivers/gpu/drm/verisilicon/vs_plane.c | 526 +++
>>> > drivers/gpu/drm/verisilicon/vs_plane.h | 58 +
>>> > drivers/gpu/drm/verisilicon/vs_type.h | 69 +
>>> > 11 files changed, 4494 insertions(+), 2 deletions(-)
>>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.c
>>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.h
>>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.c
>>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.h
>>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.c
>>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.h
>>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.c
>>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.h
>>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_type.h
>>> >
>>> > diff --git a/drivers/gpu/drm/verisilicon/Makefile
>>> > b/drivers/gpu/drm/verisilicon/Makefile
>>> > index 7d3be305b..1d48016ca 100644
>>> > --- a/drivers/gpu/drm/verisilicon/Makefile
>>> > +++ b/drivers/gpu/drm/verisilicon/Makefile
>>> > @@ -1,7 +1,11 @@
>>> > # SPDX-License-Identifier: GPL-2.0
>>> >
>>> > -vs_drm-objs := vs_drv.o \
>>> > - vs_modeset.o
>>> > +vs_drm-objs := vs_dc_hw.o \
>>> > + vs_dc.o \
>>> > + vs_crtc.o \
>>> > + vs_drv.o \
>>> > + vs_modeset.o \
>>> > + vs_plane.o
>>> >
>>> > obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
>>> >
>>> > diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c
>>> > b/drivers/gpu/drm/verisilicon/vs_crtc.c
>>> > new file mode 100644
>>> > index 0..8a658ea77
>>> > --- /dev/null
>>> > +++ b/drivers/gpu/drm/verisilicon/vs_crtc.c
>>> > @@ -0,0 +1,257 @@
>>> > +// SPDX-License-Identifier: GPL-2.0
>>> > +/*
>>> > + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
>>> > + *
>>> > + */
>>> > +
>>> > +#include
>>> > +#include
>>> > +#include
>>> > +
>>> > +#include
>>> > +#include
>>> > +#include
>>> > +#include
>>> > +#include
>>> > +#include
>>> > +
>>> > +#include "vs_crtc.h"
>>> > +#include "vs_dc.h"
>>> > +#include "vs_drv.h"
>>> > +
>>> > +static void vs_crtc_reset(struct drm_crtc *crtc)
>>> > +{
>>> > + struct vs_crtc_state *state;
>>> > +
>>> > + if (crtc->state) {
>>> > + __drm_atomic_helper_crtc_destroy_state(crtc->state);
>>> > +
>>> > + state = to_vs_crtc_state(crtc->state);
>>> > + kfree(state);
>>> > + crtc->state = NULL;
>>> > + }
>>>
>>> You can call your crtc_destroy_state function directly here.
> ok got it !
>>>
>>> > +
>>> > + state = kzalloc(sizeof(*state), GFP_KERNEL);
>>> > + if (!state)
>>> > + return;
>>> > +
>>> > + __drm_atomic_helper_crtc_reset(crtc, >base);
>>> > +}
>>> > +
>>> > +static struct drm_crtc_state *
>>> > +vs_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
>>> > +{
>>> > + struct vs_crtc_state *ori_state;
>>>
>>> It might be a matter of taste, but it is usually old_state.
>>>
>>> > + struct vs_crtc_state *state;
>>> > +
>>> > + if (!crtc->state)
>>> > + return NULL;
>>> > +
>>> > + ori_state = to_vs_crtc_state(crtc->state);
>>> > + state = kzalloc(sizeof(*state), GFP_KERNEL);
>>> > + if (!state)
>>> > + return NULL;
>>> > +
>>> > + __drm_atomic_helper_crtc_duplicate_state(crtc, >base);
>>> > +
>>> > + state->output_fmt = ori_state->output_fmt;
>>> > + state->encoder_type = ori_state->encoder_type;
>>> > + state->bpp = ori_state->bpp;
>>> > + state->underflow = ori_state->underflow;
>>>
>>> Can you use kmemdup instead?
> ok
>>>
>>> > +
>>> > + return >base;
>>> > +}
>>> > +
>>> > +static void vs_crtc_atomic_destroy_state(struct drm_crtc *crtc,
>>> > + struct drm_crtc_state *state)
>>> > +{
>>> > + __drm_atomic_helper_crtc_destroy_state(state);
>>> > + kfree(to_vs_crtc_state(state));
>>> > +}
>>> > +
>>> > +static int vs_crtc_enable_vblank(struct drm_crtc *crtc)
>>> > +{
>>> > + struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
>>> > + struct vs_dc *dc =
Re: [PATCH v2 07/11] drm/mediatek: Add secure layer config support for ovl
Re: [RFC v4 0/5] Proposal to use netlink for RAS and Telemetry across drm subsystem
On 10/23/2023 8:59 PM, Alex Deucher wrote: On Fri, Oct 20, 2023 at 7:42 PM Aravind Iddamsetty wrote: Our hardware supports RAS(Reliability, Availability, Serviceability) by reporting the errors to the host, which the KMD processes and exposes a set of error counters which can be used by observability tools to take corrective actions or repairs. Traditionally there were being exposed via PMU (for relative counters) and sysfs interface (for absolute value) in our internal branch. But, due to the limitations in this approach to use two interfaces and also not able to have an event based reporting or configurability, an alternative approach to try netlink was suggested by community for drm subsystem wide UAPI for RAS and telemetry as discussed in [1]. This [1] is the inspiration to this series. It uses the generic netlink(genl) family subsystem and exposes a set of commands that can be used by every drm driver, the framework provides a means to have custom commands too. Each drm driver instance in this example xe driver instance registers a family and operations to the genl subsystem through which it enumerates and reports the error counters. An event based notification is also supported to which userpace can subscribe to and be notified when any error occurs and read the error counter this avoids continuous polling on error counter. This can also be extended to threshold based notification. The commands used seems very limited. In AMD SOCs, IP blocks, instances of IP blocks, block types which support RAS will change across generations. This series has a single command to query the counters supported. Within that it seems to assign unique ids for every combination of error type, IP block type and then another for each instance. Not sure how good this kind of approach is for an end user. The Ids won't necessarily the stay the same across multiple generations. Users will generally be interested in specific IP blocks. For ex: to get HBM errors, it looks like the current patch series supports READALL which dumps the whole set of errors. Or, users have to figure out the ids of HBM stack instance (whose capacity can change depending on the SOC and within a single family multiple configurations can exist) errors and do multiple READ_ONE calls. Both don't look good. It would be better if the command argument format can be well defined so that it can be queried based on IP block type, instance, and error types supported (CE/UE/fatal/parity/deferred etc.). Thanks, Lijo @Hawking Zhang, @Lazar, Lijo Can you take a look at this series and API and see if it would align with our RAS requirements going forward? Alex [1]: https://airlied.blogspot.com/2022/09/accelerators-bof-outcomes-summary.html this series is on top of https://patchwork.freedesktop.org/series/125373/, v4: 1. Rebase 2. rename drm_genl_send to drm_genl_reply 3. catch error from xa_store and handle appropriately 4. presently xe_list_errors fills blank data for IGFX, prevent it by having an early check of IS_DGFX (Michael J. Ruhl) v3: 1. Rebase on latest RAS series for XE 2. drop DRIVER_NETLINK flag and use the driver_genl_ops structure to register to netlink subsystem v2: define common interfaces to genl netlink subsystem that all drm drivers can leverage. Below is an example tool drm_ras which demonstrates the use of the supported commands. The tool will be sent to ML with the subject "[RFC i-g-t v2 0/1] A tool to demonstrate use of netlink sockets to read RAS error counters" https://patchwork.freedesktop.org/series/118437/#rev2 read single error counter: $ ./drm_ras READ_ONE --device=drm:/dev/dri/card1 --error_id=0x0005 counter value 0 read all error counters: $ ./drm_ras READ_ALL --device=drm:/dev/dri/card1 nameconfig-id counter error-gt0-correctable-guc 0x0001 0 error-gt0-correctable-slm 0x0003 0 error-gt0-correctable-eu-ic 0x0004 0 error-gt0-correctable-eu-grf0x0005 0 error-gt0-fatal-guc 0x0009 0 error-gt0-fatal-slm 0x000d 0 error-gt0-fatal-eu-grf 0x000f 0 error-gt0-fatal-fpu 0x0010 0 error-gt0-fatal-tlb 0x0011 0 error-gt0-fatal-l3-fabric 0x0012 0 error-gt0-correctable-subslice 0x0013 0 error-gt0-correctable-l3bank0x0014 0 error-gt0-fatal-subslice0x0015 0 error-gt0-fatal-l3bank
Re: [PATCH 7/8] drm/msm: dsi: add support for DSI-PHY on SM8650
On 25/10/2023 10:03, Dmitry Baryshkov wrote:
On Wed, 25 Oct 2023 at 10:35, Neil Armstrong wrote:
Add DSI PHY support for the SM8650 platform.
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 ++
drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 1 +
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 27 +++
3 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index 05621e5e7d63..7612be6c3618 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -585,6 +585,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
.data = _phy_5nm_8450_cfgs },
{ .compatible = "qcom,sm8550-dsi-phy-4nm",
.data = _phy_4nm_8550_cfgs },
+ { .compatible = "qcom,sm8650-dsi-phy-4nm",
+ .data = _phy_4nm_8650_cfgs },
#endif
{}
};
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index 8b640d174785..e4275d3ad581 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -62,6 +62,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs;
extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8350_cfgs;
extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs;
extern const struct msm_dsi_phy_cfg dsi_phy_4nm_8550_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_4nm_8650_cfgs;
struct msm_dsi_dphy_timing {
u32 clk_zero;
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 3b1ed02f644d..c66193f2dc0d 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -1121,6 +1121,10 @@ static const struct regulator_bulk_data
dsi_phy_7nm_37750uA_regulators[] = {
{ .supply = "vdds", .init_load_uA = 37550 },
};
+static const struct regulator_bulk_data dsi_phy_7nm_98000uA_regulators[] = {
+ { .supply = "vdds", .init_load_uA = 98000 },
+};
+
static const struct regulator_bulk_data dsi_phy_7nm_97800uA_regulators[] = {
{ .supply = "vdds", .init_load_uA = 97800 },
};
@@ -1281,3 +1285,26 @@ const struct msm_dsi_phy_cfg dsi_phy_4nm_8550_cfgs = {
.num_dsi_phy = 2,
.quirks = DSI_PHY_7NM_QUIRK_V5_2,
};
+
+const struct msm_dsi_phy_cfg dsi_phy_4nm_8650_cfgs = {
So, this is the same as sm8550 config, just using 400 uA less? I
wonder if it makes sense to go for setting the regulator mode instead
of setting the load.
I have no idea, we keep changing this but indeed we should instead change
the regulator mode, it's safer to keep it that way until we figure that out.
I'll double check anyway
Nevertheless (unless you'd like to reuse sm8550 config entry):
Reviewed-by: Dmitry Baryshkov
Thanks,
Neil
+ .has_phy_lane = true,
+ .regulator_data = dsi_phy_7nm_98000uA_regulators,
+ .num_regulators = ARRAY_SIZE(dsi_phy_7nm_98000uA_regulators),
+ .ops = {
+ .enable = dsi_7nm_phy_enable,
+ .disable = dsi_7nm_phy_disable,
+ .pll_init = dsi_pll_7nm_init,
+ .save_pll_state = dsi_7nm_pll_save_state,
+ .restore_pll_state = dsi_7nm_pll_restore_state,
+ .set_continuous_clock = dsi_7nm_set_continuous_clock,
+ },
+ .min_pll_rate = 6UL,
+#ifdef CONFIG_64BIT
+ .max_pll_rate = 50UL,
+#else
+ .max_pll_rate = ULONG_MAX,
+#endif
+ .io_start = { 0xae95000, 0xae97000 },
+ .num_dsi_phy = 2,
+ .quirks = DSI_PHY_7NM_QUIRK_V5_2,
+};
--
2.34.1
Re: [PATCH v2 5/6] drm/vs: Add KMS crtc
hi Ville:
very glad to receive your feedback
Some of them are very good ideas.
Some are not very clear and hope to get your further reply!
On 2023/10/26 3:49, Ville Syrjälä wrote:
> On Wed, Oct 25, 2023 at 10:28:56PM +0300, Dmitry Baryshkov wrote:
>> On 25/10/2023 13:39, Keith Zhao wrote:
>> > add 2 crtcs and 8 planes in vs-drm
>> >
>> > Signed-off-by: Keith Zhao
>> > ---
>> > drivers/gpu/drm/verisilicon/Makefile |8 +-
>> > drivers/gpu/drm/verisilicon/vs_crtc.c | 257
>> > drivers/gpu/drm/verisilicon/vs_crtc.h | 43 +
>> > drivers/gpu/drm/verisilicon/vs_dc.c| 1002
>> > drivers/gpu/drm/verisilicon/vs_dc.h| 80 +
>> > drivers/gpu/drm/verisilicon/vs_dc_hw.c | 1959
>> > drivers/gpu/drm/verisilicon/vs_dc_hw.h | 492 ++
>> > drivers/gpu/drm/verisilicon/vs_drv.c |2 +
>> > drivers/gpu/drm/verisilicon/vs_plane.c | 526 +++
>> > drivers/gpu/drm/verisilicon/vs_plane.h | 58 +
>> > drivers/gpu/drm/verisilicon/vs_type.h | 69 +
>> > 11 files changed, 4494 insertions(+), 2 deletions(-)
>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.c
>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.h
>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.c
>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.h
>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.c
>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.h
>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.c
>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.h
>> > create mode 100644 drivers/gpu/drm/verisilicon/vs_type.h
>> >
>> > diff --git a/drivers/gpu/drm/verisilicon/Makefile
>> > b/drivers/gpu/drm/verisilicon/Makefile
>> > index 7d3be305b..1d48016ca 100644
>> > --- a/drivers/gpu/drm/verisilicon/Makefile
>> > +++ b/drivers/gpu/drm/verisilicon/Makefile
>> > @@ -1,7 +1,11 @@
>> > # SPDX-License-Identifier: GPL-2.0
>> >
>> > -vs_drm-objs := vs_drv.o \
>> > - vs_modeset.o
>> > +vs_drm-objs := vs_dc_hw.o \
>> > + vs_dc.o \
>> > + vs_crtc.o \
>> > + vs_drv.o \
>> > + vs_modeset.o \
>> > + vs_plane.o
>> >
>> > obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
>> >
>> > diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c
>> > b/drivers/gpu/drm/verisilicon/vs_crtc.c
>> > new file mode 100644
>> > index 0..8a658ea77
>> > --- /dev/null
>> > +++ b/drivers/gpu/drm/verisilicon/vs_crtc.c
>> > @@ -0,0 +1,257 @@
>> > +// SPDX-License-Identifier: GPL-2.0
>> > +/*
>> > + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
>> > + *
>> > + */
>> > +
>> > +#include
>> > +#include
>> > +#include
>> > +
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +
>> > +#include "vs_crtc.h"
>> > +#include "vs_dc.h"
>> > +#include "vs_drv.h"
>> > +
>> > +static void vs_crtc_reset(struct drm_crtc *crtc)
>> > +{
>> > + struct vs_crtc_state *state;
>> > +
>> > + if (crtc->state) {
>> > + __drm_atomic_helper_crtc_destroy_state(crtc->state);
>> > +
>> > + state = to_vs_crtc_state(crtc->state);
>> > + kfree(state);
>> > + crtc->state = NULL;
>> > + }
>>
>> You can call your crtc_destroy_state function directly here.
ok got it !
>>
>> > +
>> > + state = kzalloc(sizeof(*state), GFP_KERNEL);
>> > + if (!state)
>> > + return;
>> > +
>> > + __drm_atomic_helper_crtc_reset(crtc, >base);
>> > +}
>> > +
>> > +static struct drm_crtc_state *
>> > +vs_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
>> > +{
>> > + struct vs_crtc_state *ori_state;
>>
>> It might be a matter of taste, but it is usually old_state.
>>
>> > + struct vs_crtc_state *state;
>> > +
>> > + if (!crtc->state)
>> > + return NULL;
>> > +
>> > + ori_state = to_vs_crtc_state(crtc->state);
>> > + state = kzalloc(sizeof(*state), GFP_KERNEL);
>> > + if (!state)
>> > + return NULL;
>> > +
>> > + __drm_atomic_helper_crtc_duplicate_state(crtc, >base);
>> > +
>> > + state->output_fmt = ori_state->output_fmt;
>> > + state->encoder_type = ori_state->encoder_type;
>> > + state->bpp = ori_state->bpp;
>> > + state->underflow = ori_state->underflow;
>>
>> Can you use kmemdup instead?
ok
>>
>> > +
>> > + return >base;
>> > +}
>> > +
>> > +static void vs_crtc_atomic_destroy_state(struct drm_crtc *crtc,
>> > + struct drm_crtc_state *state)
>> > +{
>> > + __drm_atomic_helper_crtc_destroy_state(state);
>> > + kfree(to_vs_crtc_state(state));
>> > +}
>> > +
>> > +static int vs_crtc_enable_vblank(struct drm_crtc *crtc)
>> > +{
>> > + struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
>> > + struct vs_dc *dc = dev_get_drvdata(vs_crtc->dev);
>> > +
>> > + vs_dc_enable_vblank(dc, true);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void vs_crtc_disable_vblank(struct drm_crtc *crtc)
>> > +{
>> > + struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
[PATCH 2/2] drm/bridge: switch to drm_bridge_read_edid()
Prefer using the struct drm_edid based functions.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_bridge_connector.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index 8239ad43aed5..b7d7092aad9f 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -245,27 +245,27 @@ static int drm_bridge_connector_get_modes_edid(struct
drm_connector *connector,
struct drm_bridge *bridge)
{
enum drm_connector_status status;
- struct edid *edid;
+ const struct drm_edid *drm_edid;
int n;
status = drm_bridge_connector_detect(connector, false);
if (status != connector_status_connected)
goto no_edid;
- edid = drm_bridge_get_edid(bridge, connector);
- if (!drm_edid_is_valid(edid)) {
- kfree(edid);
+ drm_edid = drm_bridge_edid_read(bridge, connector);
+ if (!drm_edid_valid(drm_edid)) {
+ drm_edid_free(drm_edid);
goto no_edid;
}
- drm_connector_update_edid_property(connector, edid);
- n = drm_add_edid_modes(connector, edid);
+ drm_edid_connector_update(connector, drm_edid);
+ n = drm_edid_connector_add_modes(connector);
- kfree(edid);
+ drm_edid_free(drm_edid);
return n;
no_edid:
- drm_connector_update_edid_property(connector, NULL);
+ drm_edid_connector_update(connector, NULL);
return 0;
}
--
2.39.2
[PATCH 0/2] drm/bridge: start moving towards struct drm_edid
This is just the first two patches of a lengthy series that I'm not really sure how to proceed with. Basically the series converts all of drm/bridge to the new struct drm_edid infrastructure. It's safer than struct edid, because it contains meta information about the allocated size of the EDID, instead of relying on the size (number of extensions) originating from outside of the kernel. The rest is at [1]. The commit messages are lacking, and I don't really have the toolchain to even build test most of it. But I think this is where drm/bridge should go. Among all of drm, I think bridge has the most uses of struct edid that do not originate from the drm_get_edid() family of functions, which means the validity checks are somewhat inconsistent, and having the meta information is more crucial. Bridge maintainers, please instruct how to best proceed with this. Thanks, Jani. [1] https://gitlab.freedesktop.org/jani/linux/-/commits/drm-edid-bridge Jani Nikula (2): drm/bridge: add ->edid_read hook and drm_bridge_edid_read() drm/bridge: switch to drm_bridge_read_edid() drivers/gpu/drm/drm_bridge.c | 46 +- drivers/gpu/drm/drm_bridge_connector.c | 16 - include/drm/drm_bridge.h | 33 ++ 3 files changed, 86 insertions(+), 9 deletions(-) -- 2.39.2
[PATCH 1/2] drm/bridge: add ->edid_read hook and drm_bridge_edid_read()
Add new struct drm_edid based ->edid_read hook and
drm_bridge_edid_read() function to call the hook.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_bridge.c | 46 +++-
include/drm/drm_bridge.h | 33 ++
2 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 30d66bee0ec6..e1cfba2ff583 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -27,8 +27,9 @@
#include
#include
-#include
#include
+#include
+#include
#include
#include
#include
@@ -1206,6 +1207,47 @@ int drm_bridge_get_modes(struct drm_bridge *bridge,
}
EXPORT_SYMBOL_GPL(drm_bridge_get_modes);
+/**
+ * drm_bridge_edid_read - read the EDID data of the connected display
+ * @bridge: bridge control structure
+ * @connector: the connector to read EDID for
+ *
+ * If the bridge supports output EDID retrieval, as reported by the
+ * DRM_BRIDGE_OP_EDID bridge ops flag, call _bridge_funcs.edid_read to get
+ * the EDID and return it. Otherwise return NULL.
+ *
+ * If _bridge_funcs.edid_read is not set, fall back to using
+ * drm_bridge_get_edid() and wrapping it in struct drm_edid.
+ *
+ * RETURNS:
+ * The retrieved EDID on success, or NULL otherwise.
+ */
+const struct drm_edid *drm_bridge_edid_read(struct drm_bridge *bridge,
+ struct drm_connector *connector)
+{
+ if (!(bridge->ops & DRM_BRIDGE_OP_EDID))
+ return NULL;
+
+ /* Transitional: Fall back to ->get_edid. */
+ if (!bridge->funcs->edid_read) {
+ const struct drm_edid *drm_edid;
+ struct edid *edid;
+
+ edid = drm_bridge_get_edid(bridge, connector);
+ if (!edid)
+ return NULL;
+
+ drm_edid = drm_edid_alloc(edid, (edid->extensions + 1) *
EDID_LENGTH);
+
+ kfree(edid);
+
+ return drm_edid;
+ }
+
+ return bridge->funcs->edid_read(bridge, connector);
+}
+EXPORT_SYMBOL_GPL(drm_bridge_edid_read);
+
/**
* drm_bridge_get_edid - get the EDID data of the connected display
* @bridge: bridge control structure
@@ -1215,6 +1257,8 @@ EXPORT_SYMBOL_GPL(drm_bridge_get_modes);
* DRM_BRIDGE_OP_EDID bridge ops flag, call _bridge_funcs.get_edid to
* get the EDID and return it. Otherwise return NULL.
*
+ * Deprecated. Prefer using drm_bridge_edid_read().
+ *
* RETURNS:
* The retrieved EDID on success, or NULL otherwise.
*/
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index cfb7dcdb66c4..1a8ba92c889c 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -557,6 +557,37 @@ struct drm_bridge_funcs {
int (*get_modes)(struct drm_bridge *bridge,
struct drm_connector *connector);
+ /**
+* @edid_read:
+*
+* Read the EDID data of the connected display.
+*
+* The @edid_read callback is the preferred way of reporting mode
+* information for a display connected to the bridge output. Bridges
+* that support reading EDID shall implement this callback and leave
+* the @get_modes callback unimplemented.
+*
+* The caller of this operation shall first verify the output
+* connection status and refrain from reading EDID from a disconnected
+* output.
+*
+* This callback is optional. Bridges that implement it shall set the
+* DRM_BRIDGE_OP_EDID flag in their _bridge->ops.
+*
+* The connector parameter shall be used for the sole purpose of EDID
+* retrieval, and shall not be stored internally by bridge drivers for
+* future usage.
+*
+* RETURNS:
+*
+* An edid structure newly allocated with drm_edid_alloc() or returned
+* from drm_edid_read() family of functions on success, or NULL
+* otherwise. The caller is responsible for freeing the returned edid
+* structure with drm_edid_free().
+*/
+ const struct drm_edid *(*edid_read)(struct drm_bridge *bridge,
+ struct drm_connector *connector);
+
/**
* @get_edid:
*
@@ -888,6 +919,8 @@ drm_atomic_helper_bridge_propagate_bus_fmt(struct
drm_bridge *bridge,
enum drm_connector_status drm_bridge_detect(struct drm_bridge *bridge);
int drm_bridge_get_modes(struct drm_bridge *bridge,
struct drm_connector *connector);
+const struct drm_edid *drm_bridge_edid_read(struct drm_bridge *bridge,
+ struct drm_connector *connector);
struct edid *drm_bridge_get_edid(struct drm_bridge *bridge,
struct drm_connector *connector);
void drm_bridge_hpd_enable(struct drm_bridge *bridge,
--
2.39.2
Re: [PATCH v7 3/6] drm/sched: Convert the GPU scheduler to variable number of run-queues
Also note that there were no complaints from "kernel test robot " when I posted my patch (this patch), but there is now, which further shows that there's unwarranted changes. Just follow the steps I outlined below, and we should all be good. Thanks! Regards, Luben On 2023-10-26 05:36, Luben Tuikov wrote: > Hi, > > On 2023-10-26 02:33, kernel test robot wrote: >> Hi Matthew, >> >> kernel test robot noticed the following build warnings: >> >> [auto build test WARNING on 201c8a7bd1f3f415920a2df4b8a8817e973f42fe] >> >> url: >> https://github.com/intel-lab-lkp/linux/commits/Matthew-Brost/drm-sched-Add-drm_sched_wqueue_-helpers/20231026-121313 >> base: 201c8a7bd1f3f415920a2df4b8a8817e973f42fe >> patch link: >> https://lore.kernel.org/r/20231026041236.1273694-4-matthew.brost%40intel.com >> patch subject: [PATCH v7 3/6] drm/sched: Convert the GPU scheduler to >> variable number of run-queues >> config: m68k-allyesconfig >> (https://download.01.org/0day-ci/archive/20231026/202310261439.3rbateob-...@intel.com/config) >> compiler: m68k-linux-gcc (GCC) 13.2.0 >> reproduce (this is a W=1 build): >> (https://download.01.org/0day-ci/archive/20231026/202310261439.3rbateob-...@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/202310261439.3rbateob-...@intel.com/ >> >> All warnings (new ones prefixed by >>): >> >>drivers/gpu/drm/etnaviv/etnaviv_sched.c: In function 'etnaviv_sched_init': >>>> drivers/gpu/drm/etnaviv/etnaviv_sched.c:138:30: warning: passing argument >>>> 3 of 'drm_sched_init' makes pointer from integer without a cast >>>> [-Wint-conversion] >> 138 | DRM_SCHED_PRIORITY_COUNT, NULL, >> | ^~~~ >> | | >> | int >>In file included from drivers/gpu/drm/etnaviv/etnaviv_drv.h:20, >> from drivers/gpu/drm/etnaviv/etnaviv_sched.c:8: >>include/drm/gpu_scheduler.h:530:45: note: expected 'struct >> workqueue_struct *' but argument is of type 'int' >> 530 |struct workqueue_struct *submit_wq, >> |~^ >>In file included from include/uapi/linux/posix_types.h:5, >> from include/uapi/linux/types.h:14, >> from include/linux/types.h:6, >> from include/linux/kasan-checks.h:5, >> from include/asm-generic/rwonce.h:26, >> from ./arch/m68k/include/generated/asm/rwonce.h:1, >> from include/linux/compiler.h:246, >> from include/linux/build_bug.h:5, >> from include/linux/init.h:5, >> from include/linux/moduleparam.h:5, >> from drivers/gpu/drm/etnaviv/etnaviv_sched.c:6: > > The reason for this compilation failure is that this patch is completely > mangled and nothing like the patch I posted. > > My patch is: > https://lore.kernel.org/all/20231023032251.164775-1-luben.tui...@amd.com/ > > Save it raw to your disk from this link: > https://lore.kernel.org/all/20231023032251.164775-1-luben.tui...@amd.com/raw > > And apply it with "git am " on top of your clean tree, e.g. > drm-misc-next. THEN, after that, > apply your patches. > > It should then compile without any problems. > > Just looking at the first hunk in my patch: > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index 2b8356699f235d..251995a90bbe69 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -2280,6 +2280,7 @@ static int amdgpu_device_init_schedulers(struct >> amdgpu_device *adev) >> } >> >> r = drm_sched_init(>sched, _sched_ops, >> + DRM_SCHED_PRIORITY_COUNT, >>ring->num_hw_submission, 0, >>timeout, adev->reset_domain->wq, >>ring->sched_score, ring->name, > > While this looks like this in the version you posted of my patch: > &
Re: [PATCH v7 3/6] drm/sched: Convert the GPU scheduler to variable number of run-queues
Hi, On 2023-10-26 02:33, kernel test robot wrote: > Hi Matthew, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on 201c8a7bd1f3f415920a2df4b8a8817e973f42fe] > > url: > https://github.com/intel-lab-lkp/linux/commits/Matthew-Brost/drm-sched-Add-drm_sched_wqueue_-helpers/20231026-121313 > base: 201c8a7bd1f3f415920a2df4b8a8817e973f42fe > patch link: > https://lore.kernel.org/r/20231026041236.1273694-4-matthew.brost%40intel.com > patch subject: [PATCH v7 3/6] drm/sched: Convert the GPU scheduler to > variable number of run-queues > config: m68k-allyesconfig > (https://download.01.org/0day-ci/archive/20231026/202310261439.3rbateob-...@intel.com/config) > compiler: m68k-linux-gcc (GCC) 13.2.0 > reproduce (this is a W=1 build): > (https://download.01.org/0day-ci/archive/20231026/202310261439.3rbateob-...@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/202310261439.3rbateob-...@intel.com/ > > All warnings (new ones prefixed by >>): > >drivers/gpu/drm/etnaviv/etnaviv_sched.c: In function 'etnaviv_sched_init': >>> drivers/gpu/drm/etnaviv/etnaviv_sched.c:138:30: warning: passing argument 3 >>> of 'drm_sched_init' makes pointer from integer without a cast >>> [-Wint-conversion] > 138 | DRM_SCHED_PRIORITY_COUNT, NULL, > | ^~~~ > | | > | int >In file included from drivers/gpu/drm/etnaviv/etnaviv_drv.h:20, > from drivers/gpu/drm/etnaviv/etnaviv_sched.c:8: >include/drm/gpu_scheduler.h:530:45: note: expected 'struct > workqueue_struct *' but argument is of type 'int' > 530 |struct workqueue_struct *submit_wq, > |~^ >In file included from include/uapi/linux/posix_types.h:5, > from include/uapi/linux/types.h:14, > from include/linux/types.h:6, > from include/linux/kasan-checks.h:5, > from include/asm-generic/rwonce.h:26, > from ./arch/m68k/include/generated/asm/rwonce.h:1, > from include/linux/compiler.h:246, > from include/linux/build_bug.h:5, > from include/linux/init.h:5, > from include/linux/moduleparam.h:5, > from drivers/gpu/drm/etnaviv/etnaviv_sched.c:6: The reason for this compilation failure is that this patch is completely mangled and nothing like the patch I posted. My patch is: https://lore.kernel.org/all/20231023032251.164775-1-luben.tui...@amd.com/ Save it raw to your disk from this link: https://lore.kernel.org/all/20231023032251.164775-1-luben.tui...@amd.com/raw And apply it with "git am " on top of your clean tree, e.g. drm-misc-next. THEN, after that, apply your patches. It should then compile without any problems. Just looking at the first hunk in my patch: > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 2b8356699f235d..251995a90bbe69 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2280,6 +2280,7 @@ static int amdgpu_device_init_schedulers(struct > amdgpu_device *adev) > } > > r = drm_sched_init(>sched, _sched_ops, > + DRM_SCHED_PRIORITY_COUNT, >ring->num_hw_submission, 0, >timeout, adev->reset_domain->wq, >ring->sched_score, ring->name, While this looks like this in the version you posted of my patch: > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index b54c4d771104..94d073bfbd13 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2280,6 +2280,7 @@ static int amdgpu_device_init_schedulers(struct > amdgpu_device *adev) > } > > r = drm_sched_init(>sched, _sched_ops, NULL, > +DRM_SCHED_PRIORITY_COUNT, > ring->num_hw_submission, 0, > timeout, adev->reset_domain->wq, >
Re: [PATCH v2] vga_switcheroo: Fix impossible judgment condition
On Thu, Oct 26, 2023 at 05:18:04PM +0800, Su Hui wrote:
> 'id' is enum type like unsigned int, so it will never be less than zero.
> It's better to check VGA_SWITCHEROO_UNKNOWN_ID too.
>
> Fixes: 4aaf448fa975 ("vga_switcheroo: set audio client id according to bound
> GPU id")
> Signed-off-by: Su Hui
> ---
> v2:
> - add check of VGA_SWITCHEROO_UNKNOWN_ID(Dan's suggestion).
>
> By the way, all functions of 'get_client_id' will never return error code
> or VGA_SWITCHEROO_UNKNOWN_ID，should we remove this check or keep it for
> future.
>
Thanks!
Reviewed-by: Dan Carpenter
regards,
dan carpenter
Re: [RFC v4 0/5] Proposal to use netlink for RAS and Telemetry across drm subsystem
On 24/10/23 14:29, Zhang, Hawking wrote: Hi Hawking, Thank you for your comment. > [AMD Official Use Only - General] > > Hi Aravind, > > Is it allowed to register multiple genl families per drm_device? Also, is it > allowed to customize error type and even error counter (status)? In the present series it registers only one genl family per device, but genl framework shouldn't impose any restriction on multiple family registration as along as the family names are unique, but what is the purpose of it? for the second part of the question IIUC an error can have different severity, like hbm-ss0-0 can be of fatal or non fatal, so then we could have two entries for each like how it is done in this series for the same error type which can have different severities, so for hbm-ss0-0 it would enumerate error-gt0-soc-fatal-hbm-ss0-0 and error-gt0-soc-nonfatal-hbm-ss0-0 counters as our HW reports both of these kinds. Also, to highlight the error management is left to the driver, the drm_netlink doesn't handle any of those it just reports whatever the driver exposes. please let me know if I didn't get your question right. Thanks, Aravind. > > SOC might integrate different type of controllers that report error in > different types. Also, the controllers are capable of convert the error, or > change its severity in some circumstances. Mixing severity and error type in > a single array may not be the best practice. for example, > error-gt0-soc-fatal-hbm-ss0-0 might be converted to non-fatal or deferred > error, so driver doesn't need to be response immediately. > > Regards, > Hawking > > -Original Message- > From: Alex Deucher > Sent: Monday, October 23, 2023 23:29 > To: Aravind Iddamsetty ; Lazar, Lijo > > Cc: intel...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Deucher, > Alexander ; airl...@gmail.com; dan...@ffwll.ch; > joonas.lahti...@linux.intel.com; ogab...@kernel.org; tta...@habana.ai; Zhang, > Hawking ; Kasiviswanathan, Harish > ; Kuehling, Felix ; > Tuikov, Luben ; michael.j.r...@intel.com > Subject: Re: [RFC v4 0/5] Proposal to use netlink for RAS and Telemetry > across drm subsystem > > On Fri, Oct 20, 2023 at 7:42 PM Aravind Iddamsetty > wrote: >> Our hardware supports RAS(Reliability, Availability, Serviceability) >> by reporting the errors to the host, which the KMD processes and >> exposes a set of error counters which can be used by observability >> tools to take corrective actions or repairs. Traditionally there were >> being exposed via PMU (for relative counters) and sysfs interface (for >> absolute >> value) in our internal branch. But, due to the limitations in this >> approach to use two interfaces and also not able to have an event >> based reporting or configurability, an alternative approach to try >> netlink was suggested by community for drm subsystem wide UAPI for RAS >> and telemetry as discussed in [1]. >> >> This [1] is the inspiration to this series. It uses the generic >> netlink(genl) family subsystem and exposes a set of commands that can >> be used by every drm driver, the framework provides a means to have >> custom commands too. Each drm driver instance in this example xe >> driver instance registers a family and operations to the genl >> subsystem through which it enumerates and reports the error counters. >> An event based notification is also supported to which userpace can >> subscribe to and be notified when any error occurs and read the error >> counter this avoids continuous polling on error counter. This can also >> be extended to threshold based notification. > @Hawking Zhang, @Lazar, Lijo > > Can you take a look at this series and API and see if it would align with our > RAS requirements going forward? > > Alex > > >> [1]: >> https://airlied.blogspot.com/2022/09/accelerators-bof-outcomes-summary >> .html >> >> this series is on top of >> https://patchwork.freedesktop.org/series/125373/, >> >> v4: >> 1. Rebase >> 2. rename drm_genl_send to drm_genl_reply 3. catch error from xa_store >> and handle appropriately 4. presently xe_list_errors fills blank data >> for IGFX, prevent it by having an early check of IS_DGFX (Michael J. >> Ruhl) >> >> v3: >> 1. Rebase on latest RAS series for XE >> 2. drop DRIVER_NETLINK flag and use the driver_genl_ops structure to >> register to netlink subsystem >> >> v2: define common interfaces to genl netlink subsystem that all drm >> drivers can leverage. >> >> Below is an example tool drm_ras which demonstrates the use of the >> supported commands. The tool will be sent to ML with the subject "[RFC >> i-g-t v2 0/1] A tool to demonstrate use of netlink sockets to read RAS error >> counters" >> https://patchwork.freedesktop.org/series/118437/#rev2 >> >> read single error counter: >> >> $ ./drm_ras READ_ONE --device=drm:/dev/dri/card1 >> --error_id=0x0005 counter value 0 >> >> read all error counters: >> >> $ ./drm_ras READ_ALL --device=drm:/dev/dri/card1 >> name
[PATCH v2] vga_switcheroo: Fix impossible judgment condition
'id' is enum type like unsigned int, so it will never be less than zero.
It's better to check VGA_SWITCHEROO_UNKNOWN_ID too.
Fixes: 4aaf448fa975 ("vga_switcheroo: set audio client id according to bound
GPU id")
Signed-off-by: Su Hui
---
v2:
- add check of VGA_SWITCHEROO_UNKNOWN_ID(Dan's suggestion).
By the way, all functions of 'get_client_id' will never return error code
or VGA_SWITCHEROO_UNKNOWN_ID，should we remove this check or keep it for
future.
drivers/gpu/vga/vga_switcheroo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 365e6ddbe90f..cf530094f929 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -375,7 +375,7 @@ int vga_switcheroo_register_audio_client(struct pci_dev
*pdev,
mutex_lock(_mutex);
if (vgasr_priv.active) {
id = vgasr_priv.handler->get_client_id(vga_dev);
- if (id < 0) {
+ if ((int)id < 0 || id == VGA_SWITCHEROO_UNKNOWN_ID) {
mutex_unlock(_mutex);
return -EINVAL;
}
--
2.30.2
Re: [PATCH] vga_switcheroo: Fix impossible judgment condition
On Thu, Oct 26, 2023 at 04:46:29PM +0800, Su Hui wrote:
> On 2023/10/26 12:44, Dan Carpenter wrote:
> > On Thu, Oct 26, 2023 at 10:10:57AM +0800, Su Hui wrote:
> > > 'id' is enum type like unsigned int, so it will never be less than zero.
> > >
> > > Fixes: 4aaf448fa975 ("vga_switcheroo: set audio client id according to
> > > bound GPU id")
> > > Signed-off-by: Su Hui
> > > ---
> > > drivers/gpu/vga/vga_switcheroo.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/vga/vga_switcheroo.c
> > > b/drivers/gpu/vga/vga_switcheroo.c
> > > index 365e6ddbe90f..d3064466fd3a 100644
> > > --- a/drivers/gpu/vga/vga_switcheroo.c
> > > +++ b/drivers/gpu/vga/vga_switcheroo.c
> > > @@ -375,7 +375,7 @@ int vga_switcheroo_register_audio_client(struct
> > > pci_dev *pdev,
> > > mutex_lock(_mutex);
> > > if (vgasr_priv.active) {
> > > id = vgasr_priv.handler->get_client_id(vga_dev);
> > > - if (id < 0) {
> > > + if ((int)id < 0) {
> > Hi,
> >
> > I feel like you're using Smatch? Which is great! Fantastic!
> Yep, Smatch helps me a lot to find these bugs! I really like this excellent
> tool!
> >
> > Have you built the cross function database? If you have there is a
> > command that's useful.
> Not yet, bu I want to build this.
Yeah. It's super useful for kernel development. It helps to understand
how functions are called and where variables are set etc. The smatch
documentation is crap, I know. But I did write a short blog about the
cross function DB.
https://staticthinking.wordpress.com/2023/05/02/the-cross-function-db/
It's simple to build, but it takes a long time. Just run
smatch_scripts/build_kernel_data.sh
regards,
dan carpenter
Re: [Intel-gfx] [PATCH] drm/i915/gt: Remove {} from if-else
Hi Soumya, On Wed, Oct 25, 2023 at 09:43:08PM -0700, Soumya Negi wrote: > In accordance to Linux coding style(Documentation/process/4.Coding.rst), > remove unneeded braces from if-else block as all arms of this block > contain single statements. > > Suggested-by: Andi Shyti > Signed-off-by: Soumya Negi Acked-by: Karolina Stolarek Reviewed-by: Andi Shyti Thanks, Andi
Re: [PATCH] drm/rockchip: vop2: Add NV20 and NV30 support
Hi Jonas,
On Wed, 2023-10-25 at 21:32 +, Jonas Karlman wrote:
> Add support for the 10-bit 4:2:2 and 4:4:4 formats NV20 and NV30.
>
> These formats can be tested using modetest [1]:
>
> modetest -P @:1920x1080@
>
> e.g. on a ROCK 3 Model A (rk3568):
>
> modetest -P 43@67:1920x1080@NV20 -F tiles,tiles
> modetest -P 43@67:1920x1080@NV30 -F smpte,smpte
>
> [1] https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/329
>
> Signed-off-by: Jonas Karlman
Reviewed-by: Christopher Obbard
Tested-by: Christopher Obbard
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 5 +
> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 2 ++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index ab944010fe14..592f9d726f2e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -326,11 +326,14 @@ static enum vop2_data_format vop2_convert_format(u32
> format)
> case DRM_FORMAT_NV16:
> case DRM_FORMAT_NV61:
> return VOP2_FMT_YUV422SP;
> + case DRM_FORMAT_NV20:
> case DRM_FORMAT_Y210:
> return VOP2_FMT_YUV422SP_10;
> case DRM_FORMAT_NV24:
> case DRM_FORMAT_NV42:
> return VOP2_FMT_YUV444SP;
> + case DRM_FORMAT_NV30:
> + return VOP2_FMT_YUV444SP_10;
> case DRM_FORMAT_YUYV:
> case DRM_FORMAT_YVYU:
> return VOP2_FMT_VYUY422;
> @@ -415,6 +418,8 @@ static bool vop2_win_uv_swap(u32 format)
> case DRM_FORMAT_NV16:
> case DRM_FORMAT_NV24:
> case DRM_FORMAT_NV15:
> + case DRM_FORMAT_NV20:
> + case DRM_FORMAT_NV30:
> case DRM_FORMAT_YUYV:
> case DRM_FORMAT_UYVY:
> return true;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> index fdb48571efce..0b4280218a59 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> @@ -48,8 +48,10 @@ static const uint32_t formats_rk356x_esmart[] = {
> DRM_FORMAT_NV15, /* yuv420_10bit linear mode, 2 plane, no padding
> */
> DRM_FORMAT_NV16, /* yuv422_8bit linear mode, 2 plane */
> DRM_FORMAT_NV61, /* yuv422_8bit linear mode, 2 plane */
> + DRM_FORMAT_NV20, /* yuv422_10bit linear mode, 2 plane, no padding
> */
> DRM_FORMAT_NV24, /* yuv444_8bit linear mode, 2 plane */
> DRM_FORMAT_NV42, /* yuv444_8bit linear mode, 2 plane */
> + DRM_FORMAT_NV30, /* yuv444_10bit linear mode, 2 plane, no padding
> */
> DRM_FORMAT_YVYU, /* yuv422_8bit[YVYU] linear mode */
> DRM_FORMAT_VYUY, /* yuv422_8bit[VYUY] linear mode */
> };
Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On Wed, 25 Oct 2023 15:16:08 -0500 (CDT)
Alex Goins wrote:
> Thank you Harry and all other contributors for your work on this. Responses
> inline -
>
> On Mon, 23 Oct 2023, Pekka Paalanen wrote:
>
> > On Fri, 20 Oct 2023 11:23:28 -0400
> > Harry Wentland wrote:
> >
> > > On 2023-10-20 10:57, Pekka Paalanen wrote:
> > > > On Fri, 20 Oct 2023 16:22:56 +0200
> > > > Sebastian Wick wrote:
> > > >
> > > >> Thanks for continuing to work on this!
> > > >>
> > > >> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:
> > > >>> v2:
> > > >>> - Update colorop visualizations to match reality (Sebastian, Alex
> > > >>> Hung)
> > > >>> - Updated wording (Pekka)
> > > >>> - Change BYPASS wording to make it non-mandatory (Sebastian)
> > > >>> - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane Property
> > > >>>section (Pekka)
> > > >>> - Use PQ EOTF instead of its inverse in Pipeline Programming example
> > > >>> (Melissa)
> > > >>> - Add "Driver Implementer's Guide" section (Pekka)
> > > >>> - Add "Driver Forward/Backward Compatibility" section (Sebastian,
> > > >>> Pekka)
> > > >
> > > > ...
> > > >
> > > >>> +An example of a drm_colorop object might look like one of these::
> > > >>> +
> > > >>> +/* 1D enumerated curve */
> > > >>> +Color operation 42
> > > >>> +├─ "TYPE": immutable enum {1D enumerated curve, 1D LUT, 3x3
> > > >>> matrix, 3x4 matrix, 3D LUT, etc.} = 1D enumerated curve
> > > >>> +├─ "BYPASS": bool {true, false}
> > > >>> +├─ "CURVE_1D_TYPE": enum {sRGB EOTF, sRGB inverse EOTF, PQ EOTF,
> > > >>> PQ inverse EOTF, …}
> > > >>> +└─ "NEXT": immutable color operation ID = 43
>
> I know these are just examples, but I would also like to suggest the
> possibility
> of an "identity" CURVE_1D_TYPE. BYPASS = true might get different results
> compared to setting an identity in some cases depending on the hardware. See
> below for more on this, RE: implicit format conversions.
>
> Although NVIDIA hardware doesn't use a ROM for enumerated curves, it came up
> in
> offline discussions that it would nonetheless be helpful to expose enumerated
> curves in order to hide the vendor-specific complexities of programming
> segmented LUTs from clients. In that case, we would simply refer to the
> enumerated curve when calculating/choosing segmented LUT entries.
That's a good idea.
> Another thing that came up in offline discussions is that we could use
> multiple
> color operations to program a single operation in hardware. As I understand
> it,
> AMD has a ROM-defined LUT, followed by a custom 4K entry LUT, followed by an
> "HDR Multiplier". On NVIDIA we don't have these as separate hardware stages,
> but
> we could combine them into a singular LUT in software, such that you can
> combine
> e.g. segmented PQ EOTF with night light. One caveat is that you will lose
> precision from the custom LUT where it overlaps with the linear section of the
> enumerated curve, but that is unavoidable and shouldn't be an issue in most
> use-cases.
Indeed.
> Actually, the current examples in the proposal don't include a multiplier
> color
> op, which might be useful. For AMD as above, but also for NVIDIA as the
> following issue arises:
>
> As discussed further below, the NVIDIA "degamma" LUT performs an implicit
> fixed
> point to FP16 conversion. In that conversion, what fixed point 0x maps
> to in floating point varies depending on the source content. If it's SDR
> content, we want the max value in FP16 to be 1.0 (80 nits), subject to a
> potential boost multiplier if we want SDR content to be brighter. If it's HDR
> PQ
> content, we want the max value in FP16 to be 125.0 (10,000 nits). My
> assumption
> is that this is also what AMD's "HDR Multiplier" stage is used for, is that
> correct?
It would be against the UAPI design principles to tag content as HDR or
SDR. What you can do instead is to expose a colorop with a multiplier of
1.0 or 125.0 to match your hardware behaviour, then tell your hardware
that the input is SDR or HDR to get the expected multiplier. You will
never know what the content actually is, anyway.
Of course, if we want to have a arbitrary multiplier colorop that is
somewhat standard, as in, exposed by many drivers to ease userspace
development, you can certainly use any combination of your hardware
features you need to realize the UAPI prescribed mathematical operation.
Since we are talking about floating-point in hardware, a multiplier
does not significantly affect precision.
In order to mathematically define all colorops, I believe it is
necessary to define all colorops in terms of floating-point values (as
in math), even if they operate on fixed-point or integer. By this I
mean that if the input is 8 bpc unsigned integer pixel format for
instance, 0 raw pixel channel value is mapped to 0.0 and 255 is mapped
to 1.0, and the color pipeline starts with [0.0, 1.0], not [0, 255]
domain. We have to
Re: [PATCH v2 6/6] drm/vs: Add hdmi driver
On Thu, 26 Oct 2023 at 11:07, Maxime Ripard wrote:
>
> On Thu, Oct 26, 2023 at 01:23:53AM +0300, Dmitry Baryshkov wrote:
> > > +static int starfive_hdmi_register(struct drm_device *drm, struct
> > > starfive_hdmi *hdmi)
> > > +{
> > > + struct drm_encoder *encoder = >encoder;
> > > + struct device *dev = hdmi->dev;
> > > +
> > > + encoder->possible_crtcs = drm_of_find_possible_crtcs(drm,
> > > dev->of_node);
> > > +
> > > + /*
> > > +* If we failed to find the CRTC(s) which this encoder is
> > > +* supposed to be connected to, it's because the CRTC has
> > > +* not been registered yet. Defer probing, and hope that
> > > +* the required CRTC is added later.
> > > +*/
> > > + if (encoder->possible_crtcs == 0)
> > > + return -EPROBE_DEFER;
> > > +
> > > + drm_encoder_helper_add(encoder, _hdmi_encoder_helper_funcs);
> > > +
> > > + hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
> > > +
> > > + drm_connector_helper_add(>connector,
> > > +_hdmi_connector_helper_funcs);
> > > + drmm_connector_init(drm, >connector,
> > > + _hdmi_connector_funcs,
> > > + DRM_MODE_CONNECTOR_HDMIA,
> >
> > On an embedded device one can not be so sure. There can be MHL or HDMI
> > Alternative Mode. Usually we use drm_bridge here and drm_bridge_connector.
>
> On an HDMI driver, it's far from being a requirement, especially given
> the limitations bridges have.
It's a blessing that things like MHL / HDMI-in-USB-C / HDMI-to-MyDP
are not widely used in the wild and are mostly non-existing except
several phones that preate wide DP usage.
Using drm_connector directly prevents one from handling possible
modifications on the board level. For example, with the DRM connector
in place, handling a separate HPD GPIO will result in code duplication
from the hdmi-connector driver. Handling any other variations in the
board design (which are pretty common in the embedded world) will also
require changing the driver itself. drm_bridge / drm_bridge_connector
save us from those issues.
BTW: what are the limitations of the drm_bridge wrt. HDMI output? I'm
asking because we heavily depend on the bridge infrastructure for HDMI
output. Maybe we are missing something there, which went unnoticed to
me and my colleagues.
--
With best wishes
Dmitry
Re: [PATCH] vga_switcheroo: Fix impossible judgment condition
On 2023/10/26 12:44, Dan Carpenter wrote:
On Thu, Oct 26, 2023 at 10:10:57AM +0800, Su Hui wrote:
'id' is enum type like unsigned int, so it will never be less than zero.
Fixes: 4aaf448fa975 ("vga_switcheroo: set audio client id according to bound GPU
id")
Signed-off-by: Su Hui
---
drivers/gpu/vga/vga_switcheroo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 365e6ddbe90f..d3064466fd3a 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -375,7 +375,7 @@ int vga_switcheroo_register_audio_client(struct pci_dev
*pdev,
mutex_lock(_mutex);
if (vgasr_priv.active) {
id = vgasr_priv.handler->get_client_id(vga_dev);
- if (id < 0) {
+ if ((int)id < 0) {
Hi,
I feel like you're using Smatch? Which is great! Fantastic!
Yep, Smatch helps me a lot to find these bugs! I really like this
excellent tool!
Have you built the cross function database? If you have there is a
command that's useful.
Not yet, bu I want to build this.
$ ~/smatch/smatch_db/smdb.py functions vga_switcheroo_handler get_client_id |
tee where
drivers/gpu/drm/nouveau/nouveau_acpi.c | (struct
vga_switcheroo_handler)->get_client_id | nouveau_dsm_get_client_id | 1
drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | (struct
vga_switcheroo_handler)->get_client_id | amdgpu_atpx_get_client_id | 1
drivers/gpu/drm/radeon/radeon_atpx_handler.c | (struct
vga_switcheroo_handler)->get_client_id | radeon_atpx_get_client_id | 1
drivers/platform/x86/apple-gmux.c | (struct
vga_switcheroo_handler)->get_client_id | gmux_get_client_id | 1
$ make cscope
$ vim where
Use cscope to jump to each of those four functions. Move the cursor to
the nouveau_dsm_get_client_id and hit CTRL-].
Sounds great! I must try this!
They never return negatives. The enum vga_switcheroo_client_id has a
VGA_SWITCHEROO_UNKNOWN_ID define which I guess these functions are
supposed to return on error. They never do return that, but I bet
that's what we are supposed to check for. It honestly might be good
to check for both...
if ((int)id < 0 || id == VGA_SWITCHEROO_UNKNOWN_ID) {
mutex_unlock(_mutex);
return -EINVAL;
}
Agreed, I will send v2 patch soon.
Really thanks for your wonderful suggestion! :)
Su Hui
regards,
dan carpenter
Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
On Thu, 26 Oct 2023 at 11:04, Maxime Ripard wrote:
>
> On Wed, Oct 25, 2023 at 06:16:14PM +0300, Dmitry Baryshkov wrote:
> > On 25/10/2023 15:44, Maxime Ripard wrote:
> > > On Thu, Oct 19, 2023 at 02:19:51PM +0300, Dmitry Baryshkov wrote:
> > > > On Thu, 19 Oct 2023 at 12:26, Maxime Ripard wrote:
> > > > >
> > > > > On Mon, Oct 16, 2023 at 07:53:48PM +0300, Dmitry Baryshkov wrote:
> > > > > > The MIPI DSI links do not fully fall into the DRM callbacks model.
> > > > >
> > > > > Explaining why would help
> > > >
> > > > A kind of explanation comes afterwards, but probably I should change
> > > > the order of the phrases and expand it:
> > > >
> > > > The atomic_pre_enable / atomic_enable and correspondingly
> > > > atomic_disable / atomic_post_disable expect that the bridge links
> > > > follow a simple paradigm: either it is off, or it is on and streaming
> > > > video. Thus, it is fine to just enable the link at the enable time,
> > > > doing some preparations during the pre_enable.
> > > >
> > > > But then it causes several issues with DSI. First, some of the DSI
> > > > bridges and most of the DSI panels would like to send commands over
> > > > the DSI link to setup the device.
> > >
> > > What prevent them from doing it in enable when the link is enabled?
> > >
> > > > Next, some of the DSI hosts have limitations on sending the commands.
> > > > The proverbial sunxi DSI host can not send DSI commands after the
> > > > video stream has started. Thus most of the panels have opted to send
> > > > all DSI commands from pre_enable (or prepare) callback (before the
> > > > video stream has started).
> > >
> > > I'm not sure we should account for a single driver when designing a
> > > framework. We should focus on designing something sound, and then making
> > > that driver work with whatever we designed, but not the other way
> > > around. And if we can't, we should get rid of that driver because it's
> > > de-facto unmaintainable. And I'm saying that as the author of that
> > > driver.
> >
> > That's not the only driver with strange peculiarities. For example, see
> > commit 8a4b2fc9c91a ("drm/bridge: tc358762: Split register programming from
> > pre-enable to enable"), which was one of the issues that actually prompted
> > me to send this this patchset (after my previous version of this patch being
> > rejected because of sunxi).
>
> The datasheet for that bridge is available so at least we can try to fix
> it (and bridges are much simpler than controllers anyway). It's not
> something we can do with the sunxi driver.
>
> > > > However this leaves no good place for the DSI host to power up the DSI
> > > > link. By default the host's pre_enable callback is called after the
> > > > DSI bridge's pre_enable. For quite some time we were powering up the
> > > > DSI link from mode_set. This doesn't look fully correct.
> > >
> > > Yeah, it's not.
> > >
> > > > And also we got into the issue with ps8640 bridge, which requires for
> > > > the DSI link to be quiet / unpowered at the bridge's reset time.
> > > >
> > > > Dave has come with the idea of pre_enable_prev_first /
> > > > prepare_prev_first flags, which attempt to solve the issue by
> > > > reversing the order of pre_enable callbacks. This mostly solves the
> > > > issue. However during this cycle it became obvious that this approach
> > > > is not ideal too. There is no way for the DSI host to know whether the
> > > > DSI panel / bridge has been updated to use this flag or not, see the
> > > > discussion at [1].
> > >
> > > Yeah. Well, that happens. I kind of disagree with Neil here though when
> > > he says that "A panel driver should not depend on features of a DSI
> > > controller". Panels definitely rely on particular features, like the
> > > number of lanes, the modes supported, etc.
> >
> > In the mentioned discussion it was more about 'DSI host should not assume
> > panel driver features', like the panel sending commands in pre_enable or
> > not, or having pre_enable_prev_first.
> >
> > So the pre_enable_prev_first clearly lacks feature negotiation.
> >
> > > Panels shouldn't depend on a particular driver *behaviour*. But the
> > > features are fine.
> > >
> > > For our particular discussion, I think that that kind of discussion is a
> > > dead-end, and we just shouldn't worry about it. Yes, some panels have
> > > not yet been updated to take the new flags into account. However, the
> > > proper thing to do is to update them if we see a problem with that (and
> > > thus move forward to the ideal solution), not revert the beginning of
> > > that feature enablement (thus moving away from where we want to end up
> > > in).
> > >
> > > > Thus comes this proposal. It allows for the panels to explicitly bring
> > > > the link up and down at the correct time, it supports automatic use
> > > > case, where no special handling is required. And last, but not least,
> > > > it allows the DSI host to note that the bridge / panel were not
> > > > updated to
Re: [PATCH 1/1] drm/mediatek: Fix errors when reporting rotation capability
Re: [PATCH 1/1] drm/mediatek: Fix errors when reporting rotation capability
Re: [PATCH v2 6/6] drm/vs: Add hdmi driver
On Thu, Oct 26, 2023 at 01:23:53AM +0300, Dmitry Baryshkov wrote:
> > +static int starfive_hdmi_register(struct drm_device *drm, struct
> > starfive_hdmi *hdmi)
> > +{
> > + struct drm_encoder *encoder = >encoder;
> > + struct device *dev = hdmi->dev;
> > +
> > + encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
> > +
> > + /*
> > +* If we failed to find the CRTC(s) which this encoder is
> > +* supposed to be connected to, it's because the CRTC has
> > +* not been registered yet. Defer probing, and hope that
> > +* the required CRTC is added later.
> > +*/
> > + if (encoder->possible_crtcs == 0)
> > + return -EPROBE_DEFER;
> > +
> > + drm_encoder_helper_add(encoder, _hdmi_encoder_helper_funcs);
> > +
> > + hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
> > +
> > + drm_connector_helper_add(>connector,
> > +_hdmi_connector_helper_funcs);
> > + drmm_connector_init(drm, >connector,
> > + _hdmi_connector_funcs,
> > + DRM_MODE_CONNECTOR_HDMIA,
>
> On an embedded device one can not be so sure. There can be MHL or HDMI
> Alternative Mode. Usually we use drm_bridge here and drm_bridge_connector.
On an HDMI driver, it's far from being a requirement, especially given
the limitations bridges have.
Maxime
signature.asc
Description: PGP signature
Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
On Wed, Oct 25, 2023 at 06:16:14PM +0300, Dmitry Baryshkov wrote:
> On 25/10/2023 15:44, Maxime Ripard wrote:
> > On Thu, Oct 19, 2023 at 02:19:51PM +0300, Dmitry Baryshkov wrote:
> > > On Thu, 19 Oct 2023 at 12:26, Maxime Ripard wrote:
> > > >
> > > > On Mon, Oct 16, 2023 at 07:53:48PM +0300, Dmitry Baryshkov wrote:
> > > > > The MIPI DSI links do not fully fall into the DRM callbacks model.
> > > >
> > > > Explaining why would help
> > >
> > > A kind of explanation comes afterwards, but probably I should change
> > > the order of the phrases and expand it:
> > >
> > > The atomic_pre_enable / atomic_enable and correspondingly
> > > atomic_disable / atomic_post_disable expect that the bridge links
> > > follow a simple paradigm: either it is off, or it is on and streaming
> > > video. Thus, it is fine to just enable the link at the enable time,
> > > doing some preparations during the pre_enable.
> > >
> > > But then it causes several issues with DSI. First, some of the DSI
> > > bridges and most of the DSI panels would like to send commands over
> > > the DSI link to setup the device.
> >
> > What prevent them from doing it in enable when the link is enabled?
> >
> > > Next, some of the DSI hosts have limitations on sending the commands.
> > > The proverbial sunxi DSI host can not send DSI commands after the
> > > video stream has started. Thus most of the panels have opted to send
> > > all DSI commands from pre_enable (or prepare) callback (before the
> > > video stream has started).
> >
> > I'm not sure we should account for a single driver when designing a
> > framework. We should focus on designing something sound, and then making
> > that driver work with whatever we designed, but not the other way
> > around. And if we can't, we should get rid of that driver because it's
> > de-facto unmaintainable. And I'm saying that as the author of that
> > driver.
>
> That's not the only driver with strange peculiarities. For example, see
> commit 8a4b2fc9c91a ("drm/bridge: tc358762: Split register programming from
> pre-enable to enable"), which was one of the issues that actually prompted
> me to send this this patchset (after my previous version of this patch being
> rejected because of sunxi).
The datasheet for that bridge is available so at least we can try to fix
it (and bridges are much simpler than controllers anyway). It's not
something we can do with the sunxi driver.
> > > However this leaves no good place for the DSI host to power up the DSI
> > > link. By default the host's pre_enable callback is called after the
> > > DSI bridge's pre_enable. For quite some time we were powering up the
> > > DSI link from mode_set. This doesn't look fully correct.
> >
> > Yeah, it's not.
> >
> > > And also we got into the issue with ps8640 bridge, which requires for
> > > the DSI link to be quiet / unpowered at the bridge's reset time.
> > >
> > > Dave has come with the idea of pre_enable_prev_first /
> > > prepare_prev_first flags, which attempt to solve the issue by
> > > reversing the order of pre_enable callbacks. This mostly solves the
> > > issue. However during this cycle it became obvious that this approach
> > > is not ideal too. There is no way for the DSI host to know whether the
> > > DSI panel / bridge has been updated to use this flag or not, see the
> > > discussion at [1].
> >
> > Yeah. Well, that happens. I kind of disagree with Neil here though when
> > he says that "A panel driver should not depend on features of a DSI
> > controller". Panels definitely rely on particular features, like the
> > number of lanes, the modes supported, etc.
>
> In the mentioned discussion it was more about 'DSI host should not assume
> panel driver features', like the panel sending commands in pre_enable or
> not, or having pre_enable_prev_first.
>
> So the pre_enable_prev_first clearly lacks feature negotiation.
>
> > Panels shouldn't depend on a particular driver *behaviour*. But the
> > features are fine.
> >
> > For our particular discussion, I think that that kind of discussion is a
> > dead-end, and we just shouldn't worry about it. Yes, some panels have
> > not yet been updated to take the new flags into account. However, the
> > proper thing to do is to update them if we see a problem with that (and
> > thus move forward to the ideal solution), not revert the beginning of
> > that feature enablement (thus moving away from where we want to end up
> > in).
> >
> > > Thus comes this proposal. It allows for the panels to explicitly bring
> > > the link up and down at the correct time, it supports automatic use
> > > case, where no special handling is required. And last, but not least,
> > > it allows the DSI host to note that the bridge / panel were not
> > > updated to follow new protocol and thus the link should be powered on
> > > at the mode_set time. This leaves us with the possibility of dropping
> > > support for this workaround once all in-kernel drivers are updated.
> >
> > I'm
Re: [PATCH] drm/i915/gt: Remove {} from if-else
On 26.10.2023 06:43, Soumya Negi wrote:
In accordance to Linux coding style(Documentation/process/4.Coding.rst),
remove unneeded braces from if-else block as all arms of this block
contain single statements.
I'd just keep the description simple, and say that braces are not needed
for single line statements.
The patch looks fine to me. Andi, if you decide to merge it, feel free
to add my ack.
While we're here, I wanted briefly discuss how to construct To and CC
when working on i915 code. These are not hard rules (and some developers
might disagree with me), but suggestions on how to get the right people
look at your code and reduce the noise (decided to drop maintainers;
they'll be able to join the conversation from their subscription to ML)
First of all, if you work on something in i915 that only touches this
driver, you should submit it to intel-gfx, and there's no need to
include dri-devel. You can, but that mailing list is mostly used for
changes that are either for DRM or impact other drivers.
Secondly, try to include only people who are directly involved and
potential reviewers. You can CC maintainers for bigger changes that
require their involvement, but here, it's enough to include Andi, myself
and someone who added this piece of code.
So, if it was my patch, I'd have intel-gfx in To: and Andi, Prathap and
myself in Cc:. get_maintainer.pl script might've added a lot more people
there, so I'd move away from using it, and only include developers that
are involved or interested in your work. You can always reach out to
Andi and me before sending your patches, if you have any doubts.
All the best,
Karolina
Suggested-by: Andi Shyti
Signed-off-by: Soumya Negi
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 1c93e84278a0..9f6f9e138532 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -226,16 +226,15 @@ static void guc_ggtt_invalidate(struct i915_ggtt *ggtt)
gen8_ggtt_invalidate(ggtt);
list_for_each_entry(gt, >gt_list, ggtt_link) {
- if (intel_guc_tlb_invalidation_is_available(>uc.guc)) {
+ if (intel_guc_tlb_invalidation_is_available(>uc.guc))
guc_ggtt_ct_invalidate(gt);
- } else if (GRAPHICS_VER(i915) >= 12) {
+ else if (GRAPHICS_VER(i915) >= 12)
intel_uncore_write_fw(gt->uncore,
GEN12_GUC_TLB_INV_CR,
GEN12_GUC_TLB_INV_CR_INVALIDATE);
- } else {
+ else
intel_uncore_write_fw(gt->uncore,
GEN8_GTCR, GEN8_GTCR_INVALIDATE);
- }
}
}
Re: [PATCH 4/9] dma-buf: heaps: Initialise MediaTek secure heap
On 10/20/2023 3:29 PM, Yong Wu (吴勇) wrote:
> On Thu, 2023-10-19 at 10:15 +0530, Vijayanand Jitta wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On 9/11/2023 8:00 AM, Yong Wu wrote:
>>> Initialise a mtk_svp heap. Currently just add a null heap, Prepare
>> for
>>> the later patches.
>>>
>>> Signed-off-by: Yong Wu
>>> ---
>>> drivers/dma-buf/heaps/Kconfig | 8 ++
>>> drivers/dma-buf/heaps/Makefile | 1 +
>>> drivers/dma-buf/heaps/mtk_secure_heap.c | 99
>> +
>>> 3 files changed, 108 insertions(+)
>>> create mode 100644 drivers/dma-buf/heaps/mtk_secure_heap.c
>>>
>>> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-
>> buf/heaps/Kconfig
>>> index a5eef06c4226..729c0cf3eb7c 100644
>>> --- a/drivers/dma-buf/heaps/Kconfig
>>> +++ b/drivers/dma-buf/heaps/Kconfig
>>> @@ -12,3 +12,11 @@ config DMABUF_HEAPS_CMA
>>>Choose this option to enable dma-buf CMA heap. This heap is
>> backed
>>>by the Contiguous Memory Allocator (CMA). If your system has
>> these
>>>regions, you should say Y here.
>>> +
>>> +config DMABUF_HEAPS_MTK_SECURE
>>> +bool "DMA-BUF MediaTek Secure Heap"
>>> +depends on DMABUF_HEAPS && TEE
>>> +help
>>> + Choose this option to enable dma-buf MediaTek secure heap for
>> Secure
>>> + Video Path. This heap is backed by TEE client interfaces. If in
>>> + doubt, say N.
>>> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-
>> buf/heaps/Makefile
>>> index 974467791032..df559dbe33fe 100644
>>> --- a/drivers/dma-buf/heaps/Makefile
>>> +++ b/drivers/dma-buf/heaps/Makefile
>>> @@ -1,3 +1,4 @@
>>> # SPDX-License-Identifier: GPL-2.0
>>> obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)+= system_heap.o
>>> obj-$(CONFIG_DMABUF_HEAPS_CMA)+= cma_heap.o
>>> +obj-$(CONFIG_DMABUF_HEAPS_MTK_SECURE)+= mtk_secure_heap.o
>>> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c b/drivers/dma-
>> buf/heaps/mtk_secure_heap.c
>>> new file mode 100644
>>> index ..bbf1c8dce23e
>>> --- /dev/null
>>> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c
>>> @@ -0,0 +1,99 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * DMABUF mtk_secure_heap exporter
>>> + *
>>> + * Copyright (C) 2023 MediaTek Inc.
>>> + */
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +/*
>>> + * MediaTek secure (chunk) memory type
>>> + *
>>> + * @KREE_MEM_SEC_CM_TZ: static chunk memory carved out for
>> trustzone.
>>> + */
>>> +enum kree_mem_type {
>>> +KREE_MEM_SEC_CM_TZ = 1,
>>> +};
>>> +
>>> +struct mtk_secure_heap_buffer {
>>> +struct dma_heap*heap;
>>> +size_tsize;
>>> +};
>>> +
>>> +struct mtk_secure_heap {
>>> +const char*name;
>>> +const enum kree_mem_type mem_type;
>>> +};
>>> +
>>> +static struct dma_buf *
>>> +mtk_sec_heap_allocate(struct dma_heap *heap, size_t size,
>>> + unsigned long fd_flags, unsigned long heap_flags)
>>> +{
>>> +struct mtk_secure_heap_buffer *sec_buf;
>>> +DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>>> +struct dma_buf *dmabuf;
>>> +int ret;
>>> +
>>> +sec_buf = kzalloc(sizeof(*sec_buf), GFP_KERNEL);
>>
>> As we know, kzalloc can only allocate 4MB at max. So, secure heap has
>> this limitation.
>> can we have a way to allocate more memory in secure heap ? maybe
>> similar to how system heap does?
>
> This is just the size of a internal structure. I guess you mean the
> secure memory size here. Regarding secure memory allocating flow, our
> flow may be different with yours.
>
> Let me explain our flow, we have two secure buffer types(heaps).
> a) mtk_svp
> b) mtk_svp_cma which requires the cma binding.
>
> The memory management of both is inside the TEE. We only need to tell
> the TEE which type and size of buffer we want, and then the TEE will
> perform and return the memory handle to the kernel. The
> kzalloc/alloc_pages is for the normal buffers.
>
> Regarding the CMA buffer, we only call cma_alloc once, and its
> management is also within the TEE.
>
Thanks for the details.
I see for mvp_svp, allocation is also specific to TEE, as TEE takes
care of allocation as well.
I was thinking if allocation path can also be made generic ? without having
dependency on TEE.
For eg : A case where we want to allocate from kernel and secure that memory,
the current secure heap design can't be used.
Also i suppose TEE allocates contiguous memory for mtk_svp ? or does it support
scattered memory ?
>>
>> Thanks,
>> Vijay
>>
[PATCH] drm/i915/gt: Remove {} from if-else
In accordance to Linux coding style(Documentation/process/4.Coding.rst),
remove unneeded braces from if-else block as all arms of this block
contain single statements.
Suggested-by: Andi Shyti
Signed-off-by: Soumya Negi
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 1c93e84278a0..9f6f9e138532 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -226,16 +226,15 @@ static void guc_ggtt_invalidate(struct i915_ggtt *ggtt)
gen8_ggtt_invalidate(ggtt);
list_for_each_entry(gt, >gt_list, ggtt_link) {
- if (intel_guc_tlb_invalidation_is_available(>uc.guc)) {
+ if (intel_guc_tlb_invalidation_is_available(>uc.guc))
guc_ggtt_ct_invalidate(gt);
- } else if (GRAPHICS_VER(i915) >= 12) {
+ else if (GRAPHICS_VER(i915) >= 12)
intel_uncore_write_fw(gt->uncore,
GEN12_GUC_TLB_INV_CR,
GEN12_GUC_TLB_INV_CR_INVALIDATE);
- } else {
+ else
intel_uncore_write_fw(gt->uncore,
GEN8_GTCR, GEN8_GTCR_INVALIDATE);
- }
}
}
--
2.42.0
Re: [PATCH v7 3/6] drm/sched: Convert the GPU scheduler to variable number of run-queues
Hi Matthew,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 201c8a7bd1f3f415920a2df4b8a8817e973f42fe]
url:
https://github.com/intel-lab-lkp/linux/commits/Matthew-Brost/drm-sched-Add-drm_sched_wqueue_-helpers/20231026-121313
base: 201c8a7bd1f3f415920a2df4b8a8817e973f42fe
patch link:
https://lore.kernel.org/r/20231026041236.1273694-4-matthew.brost%40intel.com
patch subject: [PATCH v7 3/6] drm/sched: Convert the GPU scheduler to variable
number of run-queues
config: m68k-allyesconfig
(https://download.01.org/0day-ci/archive/20231026/202310261439.3rbateob-...@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20231026/202310261439.3rbateob-...@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/202310261439.3rbateob-...@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/etnaviv/etnaviv_sched.c: In function 'etnaviv_sched_init':
>> drivers/gpu/drm/etnaviv/etnaviv_sched.c:138:30: warning: passing argument 3
>> of 'drm_sched_init' makes pointer from integer without a cast
>> [-Wint-conversion]
138 | DRM_SCHED_PRIORITY_COUNT, NULL,
| ^~~~
| |
| int
In file included from drivers/gpu/drm/etnaviv/etnaviv_drv.h:20,
from drivers/gpu/drm/etnaviv/etnaviv_sched.c:8:
include/drm/gpu_scheduler.h:530:45: note: expected 'struct workqueue_struct
*' but argument is of type 'int'
530 |struct workqueue_struct *submit_wq,
|~^
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from ./arch/m68k/include/generated/asm/rwonce.h:1,
from include/linux/compiler.h:246,
from include/linux/build_bug.h:5,
from include/linux/init.h:5,
from include/linux/moduleparam.h:5,
from drivers/gpu/drm/etnaviv/etnaviv_sched.c:6:
>> include/linux/stddef.h:8:14: warning: passing argument 4 of 'drm_sched_init'
>> makes integer from pointer without a cast [-Wint-conversion]
8 | #define NULL ((void *)0)
| ^~~
| |
| void *
drivers/gpu/drm/etnaviv/etnaviv_sched.c:138:56: note: in expansion of macro
'NULL'
138 | DRM_SCHED_PRIORITY_COUNT, NULL,
|^~~~
include/drm/gpu_scheduler.h:531:24: note: expected 'u32' {aka 'unsigned
int'} but argument is of type 'void *'
531 |u32 num_rqs, uint32_t hw_submission, unsigned
hang_limit,
|^~~
--
drivers/gpu/drm/lima/lima_sched.c: In function 'lima_sched_pipe_init':
>> drivers/gpu/drm/lima/lima_sched.c:492:31: warning: passing argument 3 of
>> 'drm_sched_init' makes pointer from integer without a cast [-Wint-conversion]
492 | DRM_SCHED_PRIORITY_COUNT, NULL, 1,
| ^~~~
| |
| int
In file included from drivers/gpu/drm/lima/lima_sched.h:7,
from drivers/gpu/drm/lima/lima_device.h:12,
from drivers/gpu/drm/lima/lima_ctx.h:10,
from drivers/gpu/drm/lima/lima_drv.h:9,
from drivers/gpu/drm/lima/lima_sched.c:11:
include/drm/gpu_scheduler.h:530:45: note: expected 'struct workqueue_struct
*' but argument is of type 'int'
530 |struct workqueue_struct *submit_wq,
|~^
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/io.h:9,
from include/linux/iosys-map.h:10,
from drivers/gpu/drm/lima/lima_sched.c:4:
>> include/linux/stddef.h:8:14: warning: passing argument 4 of 'drm_sched_init'
>> makes integer from pointer without a cast [-Wint-conversion]
8 | #define NULL ((void *)0)
| ^~~
| |
|
