Re: [PATCH v5 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path
Il 11/06/24 08:48, CK Hu (胡俊光) ha scritto:
On Mon, 2024-06-10 at 10:28 +0200, AngeloGioacchino Del Regno wrote:
Il 06/06/24 07:29, CK Hu (胡俊光) ha scritto:
Hi, Angelo:
On Wed, 2024-06-05 at 13:15 +0200, AngeloGioacchino Del Regno wrote:
Il 05/06/24 03:38, CK Hu (胡俊光) ha scritto:
Hi, Angelo:
On Tue, 2024-05-21 at 09:57 +0200, AngeloGioacchino Del Regno wrote:
Document OF graph on MMSYS/VDOSYS: this supports up to three DDP paths
per HW instance (so potentially up to six displays for multi-vdo SoCs).
The MMSYS or VDOSYS is always the first component in the DDP pipeline,
so it only supports an output port with multiple endpoints - where each
endpoint defines the starting point for one of the (currently three)
possible hardware paths.
Reviewed-by: Rob Herring (Arm)
Reviewed-by: Alexandre Mergnat
Tested-by: Alexandre Mergnat
Signed-off-by: AngeloGioacchino Del Regno
---
.../bindings/arm/mediatek/mediatek,mmsys.yaml | 28 +++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
index b3c6888c1457..0ef67ca4122b 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
@@ -93,6 +93,34 @@ properties:
'#reset-cells':
const: 1
+ port:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+ Output port node. This port connects the MMSYS/VDOSYS output to
+ the first component of one display pipeline, for example one of
+ the available OVL or RDMA blocks.
+ Some MediaTek SoCs support multiple display outputs per MMSYS.
This patch looks good to me. Just want to share another information for you.
Here is an example that mmsys/vdosys could point to the display interface node.
vdosys0: syscon@1c01a000 {
mmsys-display-interface = <&dsi0>, <&dsi1>, <&dp_intf0>;
};
vdosys1: syscon@1c10 {
mmsys-display-interface = <&dp_intf1>;
};
There is no conflict that mmsys/vdosys point to first component of one display
pipeline or point to display interface.
Both could co-exist.
Hey CK,
yes, this could be an alternative to the OF graphs, and I'm sure that it'd work,
even though this kind of solution would still require partial hardcoding of the
display paths up until mmsys-display-interface (so, up until DSI0, or DSI1,
etc).
The problem with a solution like this is that, well, even though it would work,
even if we ignore the suboptimal partial hardcoding, OF graphs are something
generic, while the mmsys-display-interface would be a MediaTek specific/custom
property.
In the end, reusing generic kernel apis/interfaces/etc is always preferred
compared to custom solutions, especially in this case, in which the generic
stuff is on-par (or actually, depending purely on personal opinions, superior).
As for the two to co-exist, I'm not sure that this is actually needed, as the
OF graphs are already (at the end of the graph) pointing to the display
interface.
In any case, just as a reminder: if there will be any need to add any custom
MediaTek specific properties later, it's ok and we can do that at any time.
The alternative solution is using OF graphs to point display interface and use
MediaTek specific property to first component:
vdosys0: syscon@1c01a000 {
ports {
port@0 {
endpoint {
remote-endpoint = <&dsi0_endpoint>;
};
};
port@1 {
endpoint {
remote-endpoint = <&dsi1_endpoint>;
};
};
port@2 {
endpoint {
remote-endpoint = <&dp_intf0_endpoint>;
};
};
};
display-first-component = <&ovl0>;
};
And I agree to it's better to keep only OF graphs property, so it would be
vdosys0: syscon@1c01a000 {
ports {
port@0 {
endpoint {
remote-endpoint = <&dsi0_endpoint>;
};
};
port@1 {
endpoint {
remote-endpoint = <&dsi1_endpoint>;
};
};
port@2 {
endpoint {
remote-endpoint = <&dp_intf0_endpoint>;
}
;
};
};
};
Maybe we could use OF graphs for both first comp
Re: [PATCH v5 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path
Re: [PATCH] drivers/base/devres.c: refactor using guards
Hello,
kernel test robot noticed "WARNING:at_drivers/pinctrl/core.c:#devm_pinctrl_put"
on:
commit: ce2701911ab8b371b9a93b6f9482f0577729d8aa ("[PATCH]
drivers/base/devres.c: refactor using guards")
url:
https://github.com/intel-lab-lkp/linux/commits/Andrea-Calabrese/drivers-base-devres-c-refactor-using-guards/20240606-194831
base: https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git
1968845d358e108cfbfba45538d64b3cbdf04ac2
patch link:
https://lore.kernel.org/all/20240606091744.1115656-1-andrea.calabr...@amarulasolutions.com/
patch subject: [PATCH] drivers/base/devres.c: refactor using guards
in testcase: boot
compiler: gcc-13
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
+-+++
| | 1968845d35 | ce2701911a
|
+-+++
| WARNING:at_drivers/pinctrl/core.c:#devm_pinctrl_put | 0 | 8
|
| RIP:devm_pinctrl_put| 0 | 8
|
| WARNING:at_drivers/base/devres.c:#devm_kfree| 0 | 8
|
| RIP:devm_kfree | 0 | 8
|
+-+++
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/202406111401.915dd40c-oliver.s...@intel.com
[6.612845][T1] [ cut here ]
[ 6.613979][ T1] WARNING: CPU: 1 PID: 1 at drivers/pinctrl/core.c:1421
devm_pinctrl_put (drivers/pinctrl/core.c:1421 (discriminator 1))
[6.615797][T1] Modules linked in:
[6.616639][T1] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
6.10.0-rc2-3-gce2701911ab8 #1
[6.618361][T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 6.620369][ T1] RIP: 0010:devm_pinctrl_put (drivers/pinctrl/core.c:1421
(discriminator 1))
[ 6.621474][ T1] Code: 1e fa 0f 1f 44 00 00 48 89 f9 48 8b 7f 10 48 c7 c2 10 31
a7 af 48 c7 c6 d0 52 a7 af e8 dd fc 1d 00 85 c0 75 05 c3 cc cc cc cc <0f> 0b c3
cc cc cc cc 66 66 2e 0f 1f 84 00 00 00 00 00 66 66 2e 0f
All code
0: 1e (bad)
1: fa cli
2: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
7: 48 89 f9mov%rdi,%rcx
a: 48 8b 7f 10 mov0x10(%rdi),%rdi
e: 48 c7 c2 10 31 a7 afmov$0xafa73110,%rdx
15: 48 c7 c6 d0 52 a7 afmov$0xafa752d0,%rsi
1c: e8 dd fc 1d 00 callq 0x1dfcfe
21: 85 c0 test %eax,%eax
23: 75 05 jne0x2a
25: c3 retq
26: cc int3
27: cc int3
28: cc int3
29: cc int3
2a:* 0f 0b ud2 <-- trapping instruction
2c: c3 retq
2d: cc int3
2e: cc int3
2f: cc int3
30: cc int3
31: 66 66 2e 0f 1f 84 00data16 nopw %cs:0x0(%rax,%rax,1)
38: 00 00 00 00
3c: 66 data16
3d: 66 data16
3e: 2e cs
3f: 0f .byte 0xf
Code starting with the faulting instruction
===
0: 0f 0b ud2
2: c3 retq
3: cc int3
4: cc int3
5: cc int3
6: cc int3
7: 66 66 2e 0f 1f 84 00data16 nopw %cs:0x0(%rax,%rax,1)
e: 00 00 00 00
12: 66 data16
13: 66 data16
14: 2e cs
15: 0f .byte 0xf
[6.625180][T1] RSP: :a68c80013cc8 EFLAGS: 00010282
[6.626437][T1] RAX: fffe RBX: 904e3c9df268 RCX:
904dc2ff1de0
[6.628077][T1] RDX: 0001 RSI: 904dc2cc0da8 RDI:
904e3c9df4fc
[6.629687][T1] RBP: R08: b0b25f14 R09:
0008
[6.631332][T1] R10: a68c80013c70 R11: fefefefefefefeff R12:
904dc2ff1448
[6.633008][T1] R13: b12dfea8 R14: 904dc316c2a8 R15:
[6.634594][T1] FS: () GS:9050efd0()
knlGS:
[6.640486][T1] CS: 0010 DS: ES: CR0: 80050033
[6.64
[Bug 211807] [drm:drm_dp_mst_dpcd_read] *ERROR* mstb 000000004e6288dd port 3: DPCD read on addr 0x60 for 1 bytes NAKed
https://bugzilla.kernel.org/show_bug.cgi?id=211807 Felix Andrea (felixandre...@gmail.com) changed: What|Removed |Added CC||felixandre...@gmail.com --- Comment #24 from Felix Andrea (felixandre...@gmail.com) --- I have tried updating the driver and related firmware but it still doesn't seem to solve the problem. Does disabling fwupd checking DP aux ports help? I found this thread: https://lists.freedesktop.org/archives/dri-devel/2022-February/342776.html https://ricepurity-test.io/ but not really know how to fix it. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
RE: [PATCH] drm/fbdev-dma: fix getting smem_start
> Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start > > Hi > > Am 04.06.24 um 10:03 schrieb Peng Fan (OSS): > > From: Peng Fan > > > > If 'info->screen_buffer' locates in vmalloc address space, > > virt_to_page will not be able to get correct results. With > > CONFIG_DEBUG_VM and CONFIG_DEBUG_VIRTUAL enabled on ARM64, > there is dump below: > > Which graphics driver triggers this bug? It is NXP i.MX95 DPU driver which is still in NXP downstream repo. Thanks, Peng.
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On Mon, Jun 10, 2024 at 08:20:08PM +0100, Pavel Begunkov wrote: > On 6/10/24 16:16, David Ahern wrote: > > > There is no reason you shouldn't be able to use your fast io_uring > > > completion and lifecycle flow with DMABUF backed memory. Those are not > > > widly different things and there is good reason they should work > > > together. > > Let's not mix up devmem TCP and dmabuf specifically, as I see it > your question was concerning the latter: "... DMABUF memory registered > through Mina's mechanism". io_uring's zcrx can trivially get dmabuf > support in future, as mentioned it's mostly the setup side. ABI, > buffer workflow and some details is a separate issue, and I don't > see how further integration aside from what we're already sharing > is beneficial, on opposite it'll complicate things. Again, I am talking about composability here, duplicating the DMABUF stuff into io_uring is not composable, it is just duplicating things. It does not match the view that there should be two distinct layers here, one that provides the pages and one that manages the lifecycle. As HCH pushes for pages either come from the allocator and get to use the struct folio or the come from a dmabuf and they don't. That is it, the only two choices. The iouring stuff is trying to confuse the source of the pages with the lifecycle - which is surely convenient, but is why Christoph is opposing it. Jason
Re: 6.10/bisected/regression - commits bc87d666c05 and 6d4279cb99ac cause appearing green flashing bar on top of screen on Radeon 6900XT and 120Hz
On Fri, Jun 7, 2024 at 5:29 PM Linux regression tracking (Thorsten Leemhuis) wrote: > > [CCing the other amd drm maintainers] > > Mikhail: are those details in any way relevant? Then in the future best > leave them out (or make things easier to follow), they make the bug > report confusing and sounds like this is just a bug, when it fact from > your bisection is sounds like this is a regression. Apologies if my pre-story is confused. I just wanna say I completely moved to the 7900XTX more than a year ago and I was surprised to see this regression on the old 6900XT. An accident helped me find this issue because I didn't plan to use old hardware. -- Best Regards, Mike Gavrilov.
Re: [PATCH v2] drm/nouveau: don't attempt to schedule hpd_work on headless cards
On 8/6/24 08:09, Vasily Khoruzhick wrote:
If the card doesn't have display hardware, hpd_work and hpd_lock are
left uninitialized which causes BUG when attempting to schedule hpd_work
on runtime PM resume.
Fix it by adding headless flag to DRM and skip any hpd if it's set.
Fixes: ae1aadb1eb8d ("nouveau: don't fail driver load if no display hw
present.")
Link: https://gitlab.freedesktop.org/drm/nouveau/-/issues/337
Signed-off-by: Vasily Khoruzhick
Reviewed-by: Ben Skeggs
---
v2: drop extra checks in nouveau_display_hpd_work() and
nouveau_connector_hpd()
drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_display.c | 6 +-
drivers/gpu/drm/nouveau/nouveau_drv.h | 1 +
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c
b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index 13705c5f1497..4b7497a8755c 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -68,7 +68,7 @@ nv04_display_fini(struct drm_device *dev, bool runtime, bool
suspend)
if (nv_two_heads(dev))
NVWriteCRTC(dev, 1, NV_PCRTC_INTR_EN_0, 0);
- if (!runtime)
+ if (!runtime && !drm->headless)
cancel_work_sync(&drm->hpd_work);
if (!suspend)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 88728a0b2c25..674dc567e179 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2680,7 +2680,7 @@ nv50_display_fini(struct drm_device *dev, bool runtime,
bool suspend)
nv50_mstm_fini(nouveau_encoder(encoder));
}
- if (!runtime)
+ if (!runtime && !drm->headless)
cancel_work_sync(&drm->hpd_work);
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index aed5d5b51b43..d4725a968827 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -450,6 +450,9 @@ nouveau_display_hpd_resume(struct drm_device *dev)
{
struct nouveau_drm *drm = nouveau_drm(dev);
+ if (drm->headless)
+ return;
+
spin_lock_irq(&drm->hpd_lock);
drm->hpd_pending = ~0;
spin_unlock_irq(&drm->hpd_lock);
@@ -635,7 +638,7 @@ nouveau_display_fini(struct drm_device *dev, bool suspend,
bool runtime)
}
drm_connector_list_iter_end(&conn_iter);
- if (!runtime)
+ if (!runtime && !drm->headless)
cancel_work_sync(&drm->hpd_work);
drm_kms_helper_poll_disable(dev);
@@ -729,6 +732,7 @@ nouveau_display_create(struct drm_device *dev)
/* no display hw */
if (ret == -ENODEV) {
ret = 0;
+ drm->headless = true;
goto disp_create_err;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index e239c6bf4afa..25fca98a20bc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -276,6 +276,7 @@ struct nouveau_drm {
/* modesetting */
struct nvbios vbios;
struct nouveau_display *display;
+ bool headless;
struct work_struct hpd_work;
spinlock_t hpd_lock;
u32 hpd_pending;
Re: [PATCH v4 08/13] drm/msm/dpu: add support for virtual planes
On 6/7/2024 7:45 PM, Abhinav Kumar wrote:
On 6/7/2024 5:57 PM, Dmitry Baryshkov wrote:
On Sat, 8 Jun 2024 at 02:55, Abhinav Kumar
wrote:
On 6/7/2024 3:26 PM, Dmitry Baryshkov wrote:
On Sat, 8 Jun 2024 at 00:39, Abhinav Kumar
wrote:
On 6/7/2024 2:10 PM, Dmitry Baryshkov wrote:
On Fri, Jun 07, 2024 at 12:22:16PM -0700, Abhinav Kumar wrote:
On 6/7/2024 12:16 AM, Dmitry Baryshkov wrote:
On Thu, Jun 06, 2024 at 03:21:11PM -0700, Abhinav Kumar wrote:
On 3/13/2024 5:02 PM, Dmitry Baryshkov wrote:
Only several SSPP blocks support such features as YUV output
or scaling,
thus different DRM planes have different features. Properly
utilizing
all planes requires the attention of the compositor, who should
prefer simpler planes to YUV-supporting ones. Otherwise it is
very easy
to end up in a situation when all featureful planes are already
allocated for simple windows, leaving no spare plane for YUV
playback.
To solve this problem make all planes virtual. Each plane is
registered
as if it supports all possible features, but then at the
runtime during
the atomic_check phase the driver selects backing SSPP block
for each
plane.
Note, this does not provide support for using two different
SSPP blocks
for a single plane or using two rectangles of an SSPP to drive
two
planes. Each plane still gets its own SSPP and can utilize
either a solo
rectangle or both multirect rectangles depending on the
resolution.
Note #2: By default support for virtual planes is turned off
and the
driver still uses old code path with preallocated SSPP block
for each
plane. To enable virtual planes, pass
'msm.dpu_use_virtual_planes=1'
kernel parameter.
While posting the next revision, can you pls leave a note in the commit
text on the reason behind picking crtc_id for sspp allocation and not
encoder_id?
I recall you mentioned that, two rects of the smartDMA cannot goto two
LMs. This is true. But crtc mapping need not goto 1:1 with LM mapping.
It depends on topology. I think I forgot the full explanation for this
aspect of it. Hence it will be better to note that in the commit text.
I like the overall approach in this patch. Some comments below.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 50 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 +
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 230
+++---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 19 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 77
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 28 +++
7 files changed, 390 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 88c2e51ab166..794c5643584f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1168,6 +1168,49 @@ static bool
dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)
return false;
}
+static int dpu_crtc_reassign_planes(struct drm_crtc *crtc,
struct drm_crtc_state *crtc_state)
+{
+ int total_planes = crtc->dev->mode_config.num_total_plane;
+ struct drm_atomic_state *state = crtc_state->state;
+ struct dpu_global_state *global_state;
+ struct drm_plane_state **states;
+ struct drm_plane *plane;
+ int ret;
+
+ global_state = dpu_kms_get_global_state(crtc_state->state);
+ if (IS_ERR(global_state))
+ return PTR_ERR(global_state);
+
+ dpu_rm_release_all_sspp(global_state, crtc);
+
Do we need to call dpu_rm_release_all_sspp() even in the
_dpu_plane_atomic_disable()?
It allows the driver to optimize the usage of the SSPP rectangles.
No, what I meant was that we should call
dpu_rm_release_all_sspp() in
dpu_plane_atomic_update() as well because in the atomic_check()
path where
its called today, its being called only for zpos_changed and
planes_changed
but during disable we must call this for sure.
No. the dpu_rm_release_all_sspp() should only be called during check.
When dpu_plane_atomic_update() is called, the state should already be
finalised. The atomic_check() callback is called when a plane is
going
to be disabled.
atomic_check() will be called when plane is disabled but
dpu_rm_release_all_sspp() may not be called as it is protected by
zpos_changed and planes_changed. OR you need to add a !visible check
here to call dpu_rm_release_all_sspp() that time. Thats whay I wrote
previously.
Unless I miss something, if a plane gets disabled, then obviously
planes_changed is true.
[trimmed]
Do you mean DRM fwk sets planes_changed correctly for this case?
Currently we have
if (!new_state->visible) {
_dpu_plane_atomic_disable(plane);
} else {
dpu_plane_sspp_atomic_update(plane);
}
So I wanted to ensure that when plane gets disabled, its SSPP is freed
too. If this is confir
Re: [PATCH] drm/amd: force min_input_signal to 0 on Framework AMD 13/16
On 6/10/2024 15:12, Thomas Weißschuh wrote:
On 2024-06-10 14:58:02+, Mario Limonciello wrote:
+Kieran
On 6/10/2024 14:26, Thomas Weißschuh wrote:
The value of "min_input_signal" returned from ATIF on a Framework AMD 13
is "12". This leads to a fairly bright minimum display backlight.
Introduce a quirk to override "min_input_signal" to "0" which leads to a
much lower minimum brightness, which is still readable even in daylight.
Tested on a Framework AMD 13 BIOS 3.05 and Framework AMD 16.
Link: https://community.frame.work/t/25711/9
Link: https://community.frame.work/t/47036
Signed-off-by: Thomas Weißschuh
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 35
1 file changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 7099ff9cf8c5..b481889f7491 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -130,6 +131,35 @@ static struct amdgpu_acpi_priv {
struct amdgpu_atcs atcs;
} amdgpu_acpi_priv;
+struct amdgpu_acpi_quirks {
+ bool ignore_min_input_signal;
+};
+
+static const struct dmi_system_id amdgpu_acpi_quirk_table[] = {
+ {
+ /* the Framework Laptop 13 (AMD Ryzen) and 16 (AMD Ryzen) */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "AMD Ryzen"),
+ DMI_MATCH(DMI_PRODUCT_FAMILY, "Laptop"),
+ },
Two problems I see:
1) This really "should" be fixed in the BIOS. I added Kieran to the thread
for comments if that's viable.
Agreed!
2) IMO this is going to match too liberally across all potential Framework
models. If they introduce a refreshed motherboard for either product then
the quirk would apply to both products when we don't know that such a
deficiency would exist.
Also agreed.
In addition to be really specific this should also match by display type
(via EDID?).
So far this was only tested with the matte panel.
(I forgot to mention that, sorry)
Yeah; I would expect this also matters for the new high res panel that
they announced whether this value can work.
You can reference drivers/platform/x86/amd/pmc/pmc-quirks.c for what we used
for a quirk that was matching against a single product and single BIOS.
Will do for the next revision, but let's gather some feedback first.
👍
But FWIW if that issue isn't fixed in the next BIOS I think we'll end up
needing to tear out the BIOS string match and match just the platform.
I'm wondering what the longterm strategy will have to be.
Given that there are different kinds of displays, and new ones will be
released, each new display type will require an update to the firmware.
When there are no firmware updates for a device anymore, but new,
compatible displays are released, then the kernel will need the quirks
again.
Yeah I think all this points to the 'best' home for this is BIOS.
Framework can test whether the 0 value works on all the displays they
want to support and look for negative impacts for all OSes they support.
+ .driver_data = &(struct amdgpu_acpi_quirks) {
+ .ignore_min_input_signal = true,
+ },
+ },
+ {}
+};
+
+static const struct amdgpu_acpi_quirks *amdgpu_acpi_get_quirks(void)
+{
+ const struct dmi_system_id *dmi_id;
+
+ dmi_id = dmi_first_match(amdgpu_acpi_quirk_table);
+ if (!dmi_id)
+ return NULL;
+ return dmi_id->driver_data;
+}
+
/* Call the ATIF method
*/
/**
@@ -1388,6 +1418,7 @@ bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device
*adev)
*/
void amdgpu_acpi_detect(void)
{
+ const struct amdgpu_acpi_quirks *quirks = amdgpu_acpi_get_quirks();
struct amdgpu_atif *atif = &amdgpu_acpi_priv.atif;
struct amdgpu_atcs *atcs = &amdgpu_acpi_priv.atcs;
struct pci_dev *pdev = NULL;
@@ -1429,6 +1460,10 @@ void amdgpu_acpi_detect(void)
ret);
atif->backlight_caps.caps_valid = false;
}
+ if (quirks && quirks->ignore_min_input_signal) {
+ DRM_INFO("amdgpu_acpi quirk: min_input_signal=0\n");
+ atif->backlight_caps.min_input_signal = 0;
+ }
} else {
atif->backlight_caps.caps_valid = false;
}
---
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
change-id: 20240610-amdgpu-min-backlight-quirk-8402fd8e736a
Best regards,
Re: [PATCH v4 09/13] drm/msm/dpu: allow using two SSPP blocks for a single plane
On 3/13/2024 5:02 PM, Dmitry Baryshkov wrote:
Virtual wide planes give high amount of flexibility, but it is not
always enough:
In parallel multirect case only the half of the usual width is supported
for tiled formats. Thus the whole width of two tiled multirect
rectangles can not be greater than max_linewidth, which is not enough
for some platforms/compositors.
Another example is as simple as wide YUV plane. YUV planes can not use
multirect, so currently they are limited to max_linewidth too.
Now that the planes are fully virtualized, add support for allocating
two SSPP blocks to drive a single DRM plane. This fixes both mentioned
cases and allows all planes to go up to 2*max_linewidth (at the cost of
making some of the planes unavailable to the user).
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 172 --
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 8 +
2 files changed, 131 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 2961b809ccf3..cde20c1fa90d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -886,6 +886,28 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane
*plane,
return 0;
}
+static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
+ struct dpu_sw_pipe_cfg
*pipe_cfg,
+ const struct dpu_format *fmt,
+ uint32_t max_linewidth)
+{
+ if (drm_rect_width(&pipe_cfg->src_rect) !=
drm_rect_width(&pipe_cfg->dst_rect) ||
+ drm_rect_height(&pipe_cfg->src_rect) !=
drm_rect_height(&pipe_cfg->dst_rect))
+ return false;
+
+ if (pipe_cfg->rotation & DRM_MODE_ROTATE_90)
+ return false;
+
+ if (DPU_FORMAT_IS_YUV(fmt))
+ return false;
+
+ if (DPU_FORMAT_IS_UBWC(fmt) &&
+ drm_rect_width(&pipe_cfg->src_rect) > max_linewidth / 2)
+ return false;
+
+ return true;
+}
+
This is a good idea to separate out multirect checks to a separate API.
I think can push this part of the change even today.
static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
struct drm_atomic_state *state,
const struct drm_crtc_state *crtc_state)
@@ -899,7 +921,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
const struct dpu_format *fmt;
struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
- uint32_t max_linewidth;
uint32_t supported_rotations;
const struct dpu_sspp_cfg *pipe_hw_caps;
const struct dpu_sspp_sub_blks *sblk;
@@ -919,15 +940,8 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
drm_rect_height(&new_plane_state->dst
return -ERANGE;
- pipe->multirect_index = DPU_SSPP_RECT_SOLO;
- pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
- r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
- r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-
fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
- max_linewidth = pdpu->catalog->caps->max_linewidth;
-
supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
@@ -943,41 +957,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
return ret;
if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
- /*
-* In parallel multirect case only the half of the usual width
-* is supported for tiled formats. If we are here, we know that
-* full width is more than max_linewidth, thus each rect is
-* wider than allowed.
-*/
- if (DPU_FORMAT_IS_UBWC(fmt) &&
- drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
- DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u,
tiled format\n",
- DRM_RECT_ARG(&pipe_cfg->src_rect),
max_linewidth);
- return -E2BIG;
- }
-
- if (drm_rect_width(&pipe_cfg->src_rect) !=
drm_rect_width(&pipe_cfg->dst_rect) ||
- drm_rect_height(&pipe_cfg->src_rect) !=
drm_rect_height(&pipe_cfg->dst_rect) ||
- (!test_bit(DPU_SSPP_SMART_DMA_V1, &pipe->sspp->cap->features)
&&
-!test_bit(DPU_SSPP_SMART_DMA_V2,
&pipe->sspp->cap->features)) ||
- pipe_cfg->rotation & DRM_MODE_ROTATE_90 ||
- DPU_FORMAT_IS_YUV(fmt)) {
-
Re: [PATCH] drm/amd: force min_input_signal to 0 on Framework AMD 13/16
On 2024-06-10 14:58:02+, Mario Limonciello wrote:
> +Kieran
>
> On 6/10/2024 14:26, Thomas Weißschuh wrote:
> > The value of "min_input_signal" returned from ATIF on a Framework AMD 13
> > is "12". This leads to a fairly bright minimum display backlight.
> >
> > Introduce a quirk to override "min_input_signal" to "0" which leads to a
> > much lower minimum brightness, which is still readable even in daylight.
> >
> > Tested on a Framework AMD 13 BIOS 3.05 and Framework AMD 16.
> >
> > Link: https://community.frame.work/t/25711/9
> > Link: https://community.frame.work/t/47036
> > Signed-off-by: Thomas Weißschuh
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 35
> >
> > 1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > index 7099ff9cf8c5..b481889f7491 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > @@ -25,6 +25,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -130,6 +131,35 @@ static struct amdgpu_acpi_priv {
> > struct amdgpu_atcs atcs;
> > } amdgpu_acpi_priv;
> > +struct amdgpu_acpi_quirks {
> > + bool ignore_min_input_signal;
> > +};
> > +
> > +static const struct dmi_system_id amdgpu_acpi_quirk_table[] = {
> > + {
> > + /* the Framework Laptop 13 (AMD Ryzen) and 16 (AMD Ryzen) */
> > + .matches = {
> > + DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "AMD Ryzen"),
> > + DMI_MATCH(DMI_PRODUCT_FAMILY, "Laptop"),
> > + },
>
> Two problems I see:
>
> 1) This really "should" be fixed in the BIOS. I added Kieran to the thread
> for comments if that's viable.
Agreed!
> 2) IMO this is going to match too liberally across all potential Framework
> models. If they introduce a refreshed motherboard for either product then
> the quirk would apply to both products when we don't know that such a
> deficiency would exist.
Also agreed.
In addition to be really specific this should also match by display type
(via EDID?).
So far this was only tested with the matte panel.
(I forgot to mention that, sorry)
> You can reference drivers/platform/x86/amd/pmc/pmc-quirks.c for what we used
> for a quirk that was matching against a single product and single BIOS.
Will do for the next revision, but let's gather some feedback first.
> But FWIW if that issue isn't fixed in the next BIOS I think we'll end up
> needing to tear out the BIOS string match and match just the platform.
I'm wondering what the longterm strategy will have to be.
Given that there are different kinds of displays, and new ones will be
released, each new display type will require an update to the firmware.
When there are no firmware updates for a device anymore, but new,
compatible displays are released, then the kernel will need the quirks
again.
> > + .driver_data = &(struct amdgpu_acpi_quirks) {
> > + .ignore_min_input_signal = true,
> > + },
> > + },
> > + {}
> > +};
> > +
> > +static const struct amdgpu_acpi_quirks *amdgpu_acpi_get_quirks(void)
> > +{
> > + const struct dmi_system_id *dmi_id;
> > +
> > + dmi_id = dmi_first_match(amdgpu_acpi_quirk_table);
> > + if (!dmi_id)
> > + return NULL;
> > + return dmi_id->driver_data;
> > +}
> > +
> > /* Call the ATIF method
> >*/
> > /**
> > @@ -1388,6 +1418,7 @@ bool amdgpu_acpi_should_gpu_reset(struct
> > amdgpu_device *adev)
> >*/
> > void amdgpu_acpi_detect(void)
> > {
> > + const struct amdgpu_acpi_quirks *quirks = amdgpu_acpi_get_quirks();
> > struct amdgpu_atif *atif = &amdgpu_acpi_priv.atif;
> > struct amdgpu_atcs *atcs = &amdgpu_acpi_priv.atcs;
> > struct pci_dev *pdev = NULL;
> > @@ -1429,6 +1460,10 @@ void amdgpu_acpi_detect(void)
> > ret);
> > atif->backlight_caps.caps_valid = false;
> > }
> > + if (quirks && quirks->ignore_min_input_signal) {
> > + DRM_INFO("amdgpu_acpi quirk: min_input_signal=0\n");
> > + atif->backlight_caps.min_input_signal = 0;
> > + }
> > } else {
> > atif->backlight_caps.caps_valid = false;
> > }
> >
> > ---
> > base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
> > change-id: 20240610-amdgpu-min-backlight-quirk-8402fd8e736a
> >
> > Best regards,
>
Re: [PATCH] drm/amd: force min_input_signal to 0 on Framework AMD 13/16
+Kieran
On 6/10/2024 14:26, Thomas Weißschuh wrote:
The value of "min_input_signal" returned from ATIF on a Framework AMD 13
is "12". This leads to a fairly bright minimum display backlight.
Introduce a quirk to override "min_input_signal" to "0" which leads to a
much lower minimum brightness, which is still readable even in daylight.
Tested on a Framework AMD 13 BIOS 3.05 and Framework AMD 16.
Link: https://community.frame.work/t/25711/9
Link: https://community.frame.work/t/47036
Signed-off-by: Thomas Weißschuh
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 35
1 file changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 7099ff9cf8c5..b481889f7491 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -130,6 +131,35 @@ static struct amdgpu_acpi_priv {
struct amdgpu_atcs atcs;
} amdgpu_acpi_priv;
+struct amdgpu_acpi_quirks {
+ bool ignore_min_input_signal;
+};
+
+static const struct dmi_system_id amdgpu_acpi_quirk_table[] = {
+ {
+ /* the Framework Laptop 13 (AMD Ryzen) and 16 (AMD Ryzen) */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "AMD Ryzen"),
+ DMI_MATCH(DMI_PRODUCT_FAMILY, "Laptop"),
+ },
Two problems I see:
1) This really "should" be fixed in the BIOS. I added Kieran to the
thread for comments if that's viable.
2) IMO this is going to match too liberally across all potential
Framework models. If they introduce a refreshed motherboard for either
product then the quirk would apply to both products when we don't know
that such a deficiency would exist.
You can reference drivers/platform/x86/amd/pmc/pmc-quirks.c for what we
used for a quirk that was matching against a single product and single
BIOS.
But FWIW if that issue isn't fixed in the next BIOS I think we'll end up
needing to tear out the BIOS string match and match just the platform.
+ .driver_data = &(struct amdgpu_acpi_quirks) {
+ .ignore_min_input_signal = true,
+ },
+ },
+ {}
+};
+
+static const struct amdgpu_acpi_quirks *amdgpu_acpi_get_quirks(void)
+{
+ const struct dmi_system_id *dmi_id;
+
+ dmi_id = dmi_first_match(amdgpu_acpi_quirk_table);
+ if (!dmi_id)
+ return NULL;
+ return dmi_id->driver_data;
+}
+
/* Call the ATIF method
*/
/**
@@ -1388,6 +1418,7 @@ bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device
*adev)
*/
void amdgpu_acpi_detect(void)
{
+ const struct amdgpu_acpi_quirks *quirks = amdgpu_acpi_get_quirks();
struct amdgpu_atif *atif = &amdgpu_acpi_priv.atif;
struct amdgpu_atcs *atcs = &amdgpu_acpi_priv.atcs;
struct pci_dev *pdev = NULL;
@@ -1429,6 +1460,10 @@ void amdgpu_acpi_detect(void)
ret);
atif->backlight_caps.caps_valid = false;
}
+ if (quirks && quirks->ignore_min_input_signal) {
+ DRM_INFO("amdgpu_acpi quirk: min_input_signal=0\n");
+ atif->backlight_caps.min_input_signal = 0;
+ }
} else {
atif->backlight_caps.caps_valid = false;
}
---
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
change-id: 20240610-amdgpu-min-backlight-quirk-8402fd8e736a
Best regards,
[PATCH] drm/amd: force min_input_signal to 0 on Framework AMD 13/16
The value of "min_input_signal" returned from ATIF on a Framework AMD 13
is "12". This leads to a fairly bright minimum display backlight.
Introduce a quirk to override "min_input_signal" to "0" which leads to a
much lower minimum brightness, which is still readable even in daylight.
Tested on a Framework AMD 13 BIOS 3.05 and Framework AMD 16.
Link: https://community.frame.work/t/25711/9
Link: https://community.frame.work/t/47036
Signed-off-by: Thomas Weißschuh
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 35
1 file changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 7099ff9cf8c5..b481889f7491 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -130,6 +131,35 @@ static struct amdgpu_acpi_priv {
struct amdgpu_atcs atcs;
} amdgpu_acpi_priv;
+struct amdgpu_acpi_quirks {
+ bool ignore_min_input_signal;
+};
+
+static const struct dmi_system_id amdgpu_acpi_quirk_table[] = {
+ {
+ /* the Framework Laptop 13 (AMD Ryzen) and 16 (AMD Ryzen) */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "AMD Ryzen"),
+ DMI_MATCH(DMI_PRODUCT_FAMILY, "Laptop"),
+ },
+ .driver_data = &(struct amdgpu_acpi_quirks) {
+ .ignore_min_input_signal = true,
+ },
+ },
+ {}
+};
+
+static const struct amdgpu_acpi_quirks *amdgpu_acpi_get_quirks(void)
+{
+ const struct dmi_system_id *dmi_id;
+
+ dmi_id = dmi_first_match(amdgpu_acpi_quirk_table);
+ if (!dmi_id)
+ return NULL;
+ return dmi_id->driver_data;
+}
+
/* Call the ATIF method
*/
/**
@@ -1388,6 +1418,7 @@ bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device
*adev)
*/
void amdgpu_acpi_detect(void)
{
+ const struct amdgpu_acpi_quirks *quirks = amdgpu_acpi_get_quirks();
struct amdgpu_atif *atif = &amdgpu_acpi_priv.atif;
struct amdgpu_atcs *atcs = &amdgpu_acpi_priv.atcs;
struct pci_dev *pdev = NULL;
@@ -1429,6 +1460,10 @@ void amdgpu_acpi_detect(void)
ret);
atif->backlight_caps.caps_valid = false;
}
+ if (quirks && quirks->ignore_min_input_signal) {
+ DRM_INFO("amdgpu_acpi quirk: min_input_signal=0\n");
+ atif->backlight_caps.min_input_signal = 0;
+ }
} else {
atif->backlight_caps.caps_valid = false;
}
---
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
change-id: 20240610-amdgpu-min-backlight-quirk-8402fd8e736a
Best regards,
--
Thomas Weißschuh
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On 6/10/24 16:41, Mina Almasry wrote: On Mon, Jun 10, 2024 at 5:38 AM Christian König wrote: Am 10.06.24 um 14:16 schrieb Jason Gunthorpe: On Mon, Jun 10, 2024 at 02:07:01AM +0100, Pavel Begunkov wrote: On 6/10/24 01:37, David Wei wrote: On 2024-06-07 17:52, Jason Gunthorpe wrote: IMHO it seems to compose poorly if you can only use the io_uring lifecycle model with io_uring registered memory, and not with DMABUF memory registered through Mina's mechanism. By this, do you mean io_uring must be exclusively used to use this feature? And you'd rather see the two decoupled, so userspace can register w/ say dmabuf then pass it to io_uring? Personally, I have no clue what Jason means. You can just as well say that it's poorly composable that write(2) to a disk cannot post a completion into a XDP ring, or a netlink socket, or io_uring's main completion queue, or name any other API. There is no reason you shouldn't be able to use your fast io_uring completion and lifecycle flow with DMABUF backed memory. Those are not widly different things and there is good reason they should work together. Well there is the fundamental problem that you can't use io_uring to implement the semantics necessary for a dma_fence. That's why we had to reject the io_uring work on DMA-buf sharing from Google a few years ago. Any chance someone can link me to this? io_uring, as far as my primitive understanding goes, is not yet very adopted at Google, and I'm curious what this effort is. I'm curious as well, I don't remember it floating anywhere in mailing lists. The only discussion I recall was about DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, but it didn't get through only because someone pushed for evenfds. -- Pavel Begunkov
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On 6/10/24 16:16, David Ahern wrote: On 6/10/24 6:16 AM, Jason Gunthorpe wrote: On Mon, Jun 10, 2024 at 02:07:01AM +0100, Pavel Begunkov wrote: On 6/10/24 01:37, David Wei wrote: On 2024-06-07 17:52, Jason Gunthorpe wrote: IMHO it seems to compose poorly if you can only use the io_uring lifecycle model with io_uring registered memory, and not with DMABUF memory registered through Mina's mechanism. By this, do you mean io_uring must be exclusively used to use this feature? And you'd rather see the two decoupled, so userspace can register w/ say dmabuf then pass it to io_uring? Personally, I have no clue what Jason means. You can just as well say that it's poorly composable that write(2) to a disk cannot post a completion into a XDP ring, or a netlink socket, or io_uring's main completion queue, or name any other API. There is no reason you shouldn't be able to use your fast io_uring completion and lifecycle flow with DMABUF backed memory. Those are not widly different things and there is good reason they should work together. Let's not mix up devmem TCP and dmabuf specifically, as I see it your question was concerning the latter: "... DMABUF memory registered through Mina's mechanism". io_uring's zcrx can trivially get dmabuf support in future, as mentioned it's mostly the setup side. ABI, buffer workflow and some details is a separate issue, and I don't see how further integration aside from what we're already sharing is beneficial, on opposite it'll complicate things. Pretending they are totally different just because two different people wrote them is a very siloed view. io_uring zcrx and devmem? They are not, nobody is saying otherwise, _very_ similar approaches if anything but with different API, which is the reason we already use common infra. The devmem TCP callback can implement it in a way feasible to the project, but it cannot directly post events to an unrelated API like io_uring. And devmem attaches buffers to a socket, for which a ring for returning buffers might even be a nuisance. If you can't compose your io_uring completion mechanism with a DMABUF provided backing store then I think it needs more work. As per above, it conflates devmem TCP with dmabuf. exactly. io_uring, page_pool, dmabuf - all kernel building blocks for solutions. This why I was pushing for Mina's set not to be using the name `devmem` - it is but one type of memory and with dmabuf it should not matter if it is gpu or host (or something else later on - cxl?). -- Pavel Begunkov
Re: [PATCH v1 1/1] treewide: Align match_string() with sysfs_match_string()
On Tue, Jun 4, 2024 at 9:46 AM Jani Nikula wrote: [Maybe slightly off-topic, ranty] > Why do we think it's a good idea to increase and normalize the use of > double-underscore function names across the kernel, like > __match_string() in this case? It should mean "reserved for the > implementation, not to be called directly". > > If it's to be used directly, it should be named accordingly, right? It's a huge mess. "__" prefix is just so ambiguous I think it just shouldn't be used or prolifierated, and it usually breaks Rusty Russells API rules times over. Consider __set_bit() from , used all over the place, in contrast with set_bit() for example, what does "__" represent in this context that makes __set_bit() different from set_bit()? It means "non-atomic"... How does a random contributor know this? Yeah, you guess it. By the token of "everybody knows that". (Grep, google, repeat for the number of contributors to the kernel.) I was considering to send a script to Torvalds to just change all this to set_bit_nonatomic() (etc) but was hesitating because that makes the name unambiguous but long. I think I stayed off it because changing stuff like that all over the place creates churn and churn is bad. Yours, Linus Walleij
[PATCH] drm/bridge/panel: Fix runtime warning on panel bridge release
Device managed panel bridge wrappers are created by calling to
drm_panel_bridge_add_typed() and registering a release handler for
clean-up when the device gets unbound.
Since the memory for this bridge is also managed and linked to the panel
device, the release function should not try to free that memory.
Moreover, the call to devm_kfree() inside drm_panel_bridge_remove() will
fail in this case and emit a warning because the panel bridge resource
is no longer on the device resources list (it has been removed from
there before the call to release handlers).
Signed-off-by: Adam Miotk
---
drivers/gpu/drm/bridge/panel.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 32506524d9a2..fe5fb08c9fc4 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -360,9 +360,12 @@ EXPORT_SYMBOL(drm_panel_bridge_set_orientation);
static void devm_drm_panel_bridge_release(struct device *dev, void *res)
{
- struct drm_bridge **bridge = res;
+ struct drm_bridge *bridge = *(struct drm_bridge **)res;
- drm_panel_bridge_remove(*bridge);
+ if (!bridge)
+ return;
+
+ drm_bridge_remove(bridge);
}
/**
--
2.25.1
[PATCH] drm/komeda: check for error-valued pointer
komeda_pipeline_get_state() may return an error-valued pointer, thus check the pointer for negative or null value before dereferencing. Signed-off-by: Amjad Ouled-Ameur --- drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c index f3e744172673..f4e76b46ca32 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c @@ -259,7 +259,7 @@ komeda_component_get_avail_scaler(struct komeda_component *c, u32 avail_scalers; pipe_st = komeda_pipeline_get_state(c->pipeline, state); - if (!pipe_st) + if (IS_ERR_OR_NULL(pipe_st)) return NULL; avail_scalers = (pipe_st->active_comps & KOMEDA_PIPELINE_SCALERS) ^ -- 2.25.1
Re: [PATCH] drm/connector: hdmi: Fix kerneldoc warnings
On Mon, Jun 10, 2024 at 01:12:00PM +0200, Maxime Ripard wrote:
> It looks like the documentation for the HDMI-related fields recently
> added to both the drm_connector and drm_connector_state structures
> trigger some warnings because of their use of anonymous structures:
>
> $ scripts/kernel-doc -none include/drm/drm_connector.h
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'broadcast_rgb' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'infoframes' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member 'avi'
> description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member 'hdr_drm'
> description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member 'spd'
> description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member 'vendor'
> description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'is_limited_range' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'output_bpc' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'output_format' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'tmds_char_rate' description in 'drm_connector_state'
> include/drm/drm_connector.h:2112: warning: Excess struct member 'vendor'
> description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member 'product'
> description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member
> 'supported_formats' description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member
> 'infoframes' description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member 'lock'
> description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member 'audio'
> description in 'drm_connector'
>
> Create some intermediate structures instead of anonymous ones to silence
> the warnings.
>
> Reported-by: Jani Nikula
> Suggested-by: Jani Nikula
> Fixes: 54cb39e2293b ("drm/connector: hdmi: Create an HDMI sub-state")
> Fixes: 948f01d5e559 ("drm/connector: hdmi: Add support for output format")
> Signed-off-by: Maxime Ripard
> ---
> include/drm/drm_connector.h | 207 +++-
> 1 file changed, 109 insertions(+), 98 deletions(-)
>
With the semicolon fixed:
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH v5 2/9] drm/bridge-connector: switch to using drmm allocations
On Mon, Jun 10, 2024 at 02:07:06PM +0200, Maxime Ripard wrote:
> Hi,
>
> +Hans
>
> On Mon, Jun 10, 2024 at 02:46:03PM GMT, Dmitry Baryshkov wrote:
> > On Mon, 10 Jun 2024 at 11:04, Maxime Ripard wrote:
> > >
> > > Hi,
> > >
> > > On Fri, Jun 07, 2024 at 04:22:59PM GMT, Dmitry Baryshkov wrote:
> > > > Turn drm_bridge_connector to using drmm_kzalloc() and
> > > > drmm_connector_init() and drop the custom destroy function. The
> > > > drm_connector_unregister() and fwnode_handle_put() are already handled
> > > > by the drm_connector_cleanup() and so are safe to be dropped.
> > > >
> > > > Acked-by: Maxime Ripard
> > > > Signed-off-by: Dmitry Baryshkov
> > > > ---
> > > > drivers/gpu/drm/drm_bridge_connector.c | 23 +--
> > > > 1 file changed, 5 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c
> > > > b/drivers/gpu/drm/drm_bridge_connector.c
> > > > index 982552c9f92c..e093fc8928dc 100644
> > > > --- a/drivers/gpu/drm/drm_bridge_connector.c
> > > > +++ b/drivers/gpu/drm/drm_bridge_connector.c
> > > > @@ -15,6 +15,7 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > >
> > > > @@ -193,19 +194,6 @@ drm_bridge_connector_detect(struct drm_connector
> > > > *connector, bool force)
> > > > return status;
> > > > }
> > > >
> > > > -static void drm_bridge_connector_destroy(struct drm_connector
> > > > *connector)
> > > > -{
> > > > - struct drm_bridge_connector *bridge_connector =
> > > > - to_drm_bridge_connector(connector);
> > > > -
> > > > - drm_connector_unregister(connector);
> > > > - drm_connector_cleanup(connector);
> > > > -
> > > > - fwnode_handle_put(connector->fwnode);
> > > > -
> > > > - kfree(bridge_connector);
> > > > -}
> > > > -
> > > > static void drm_bridge_connector_debugfs_init(struct drm_connector
> > > > *connector,
> > > > struct dentry *root)
> > > > {
> > > > @@ -224,7 +212,6 @@ static const struct drm_connector_funcs
> > > > drm_bridge_connector_funcs = {
> > > > .reset = drm_atomic_helper_connector_reset,
> > > > .detect = drm_bridge_connector_detect,
> > > > .fill_modes = drm_helper_probe_single_connector_modes,
> > > > - .destroy = drm_bridge_connector_destroy,
> > > > .atomic_duplicate_state =
> > > > drm_atomic_helper_connector_duplicate_state,
> > > > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > > > .debugfs_init = drm_bridge_connector_debugfs_init,
> > > > @@ -328,7 +315,7 @@ struct drm_connector
> > > > *drm_bridge_connector_init(struct drm_device *drm,
> > > > int connector_type;
> > > > int ret;
> > > >
> > > > - bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL);
> > > > + bridge_connector = drmm_kzalloc(drm, sizeof(*bridge_connector),
> > > > GFP_KERNEL);
> > >
> > > So you make destroy's kfree call unnecessary here ...
> > >
> > > > if (!bridge_connector)
> > > > return ERR_PTR(-ENOMEM);
> > > >
> > > > @@ -383,9 +370,9 @@ struct drm_connector
> > > > *drm_bridge_connector_init(struct drm_device *drm,
> > > > return ERR_PTR(-EINVAL);
> > > > }
> > > >
> > > > - ret = drm_connector_init_with_ddc(drm, connector,
> > > > - &drm_bridge_connector_funcs,
> > > > - connector_type, ddc);
> > > > + ret = drmm_connector_init(drm, connector,
> > > > + &drm_bridge_connector_funcs,
> > > > + connector_type, ddc);
> > >
> > > ... and here of drm_connector_cleanup.
> > >
> > > drm_connector_unregister wasn't needed, so can ignore it, but you leak a
> > > reference to
> > > connector->fwnode since you don't call fwnode_handle_put anymore.
> > >
> > > We should register a drmm action right below the call to
> > > fwnode_handle_get too.
> >
> > But drm_connector_cleanup() already contains
> > fwnode_handle_put(connector->fwnode). Isn't that enough?
>
> It does, but now I'm confused.
>
> drm_bridge_connector_init takes a reference, drm_connector_init doesn't.
> It will call drm_bridge_connector_destroy() that gives back its
> reference (which makes sense to me), but then why do
> drm_connector_cleanup() does? None of the drm_connector code even took
> that reference, and we end up with a double-put.
>
> It looks like it was introduced by commit 48c429c6d18d ("drm/connector:
> Add a fwnode pointer to drm_connector and register with ACPI (v2)") from
> Hans, which does call put, but never gets that reference.
The mentioned patch documents that pretty clearly:
* Drivers can set this to associate a fwnode with a connector, drivers
* are expected to get a reference on the fwnode when setting this.
* drm_connector_cleanup() will call fwnode_handle_put() on this.
This is logical.
Re: [PATCH v2 7/9] drm/bridge: cdns-dsi: Support atomic bridge APIs
On Mon, Jun 10, 2024 at 06:02:41PM +0530, Aradhya Bhatia wrote:
> Hi Dmitry,
>
> Thank you for reviewing the patches.
>
> On 31/05/24 04:51, Dmitry Baryshkov wrote:
> > On Thu, May 30, 2024 at 03:06:19PM +0530, Aradhya Bhatia wrote:
> >> Change the existing (and deprecated) bridge hooks, to the bridge
> >> atomic APIs.
> >>
> >> Add drm helpers for duplicate_state, destroy_state, and bridge_reset
> >> bridge hooks.
> >>
> >> Further add support for the input format negotiation hook.
> >>
> >> Signed-off-by: Aradhya Bhatia
> >> ---
> >> .../gpu/drm/bridge/cadence/cdns-dsi-core.c| 70 ---
> >> 1 file changed, 62 insertions(+), 8 deletions(-)
> >
> > Reviewed-by: Dmitry Baryshkov
> >
> > Minor nit below.
> >
> >>
> >> @@ -915,13 +920,62 @@ static void cdns_dsi_bridge_pre_enable(struct
> >> drm_bridge *bridge)
> >>cdns_dsi_hs_init(dsi);
> >> }
> >>
> >> +static u32 *cdns_dsi_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
> >> + struct drm_bridge_state
> >> *bridge_state,
> >> + struct drm_crtc_state
> >> *crtc_state,
> >> + struct drm_connector_state
> >> *conn_state,
> >> + u32 output_fmt,
> >> + unsigned int *num_input_fmts)
> >> +{
> >
> > This code below looks pretty generic. Would be logical to extract it to
> > a helper and allow it to be used by other DSI host bridges?
>
> I agree, it would indeed make sense.
>
> I just tried to make a helper function that could directly be passed to
> the drm_bridge_funcs list - like we do with
> "drm_atomic_helper_bridge_propagate_bus_fmt". This would have been ideal
> in my opinion.
>
> But, that doesn't seem possible today. The parameter "output_fmt"
> doesn't describe any of the DSI pixel formats from "enum
> mipi_dsi_pixel_format", which is what's required to ascertain the input
> bus format for dsi hosts. Neither do the drm_bridge{_state} help with
> that.
>
> For now, I am thinking to just add a common function that accepts the
> dsi bus output format and returns the corresponding input bus format.
> With this, every dsi host will still need to implement their own
> get_input_fmt hook and call that function.
>
> If you had something else in mind, do let me know! =)
No, the code that you have described looks pretty good. Please proceed
with the implementation!
>
> Regards
> Aradhya
>
> >
> >> + struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
> >> + struct cdns_dsi *dsi = input_to_dsi(input);
> >> + struct cdns_dsi_output *output = &dsi->output;
> >> + u32 *input_fmts;
> >> +
> >> + *num_input_fmts = 0;
> >> +
> >> + input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
> >> + if (!input_fmts)
> >> + return NULL;
> >> +
> >> + switch (output->dev->format) {
> >> + case MIPI_DSI_FMT_RGB888:
> >> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> >> + break;
> >> +
> >> + case MIPI_DSI_FMT_RGB666:
> >> + input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
> >> + break;
> >> +
> >> + case MIPI_DSI_FMT_RGB666_PACKED:
> >> + input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X18;
> >> + break;
> >> +
> >> + case MIPI_DSI_FMT_RGB565:
> >> + input_fmts[0] = MEDIA_BUS_FMT_RGB565_1X16;
> >> + break;
> >> +
> >> + default:
> >> + /* Unsupported DSI Format */
> >> + return NULL;
> >> + }
> >> +
> >> + *num_input_fmts = 1;
> >> +
> >> + return input_fmts;
> >> +}
> >> +
> >
--
With best wishes
Dmitry
Re: [PATCH] dma-buf/heaps: Correct the types of fd_flags and heap_flags
On Thu, Jun 06, 2024 at 02:02:13PM +1200, Barry Song wrote:
> From: Barry Song
>
> dma_heap_allocation_data defines the UAPI as follows:
>
> struct dma_heap_allocation_data {
> __u64 len;
> __u32 fd;
> __u32 fd_flags;
> __u64 heap_flags;
> };
>
> But dma heaps are casting both fd_flags and heap_flags into
> unsigned long. This patch makes dma heaps - cma heap and
> system heap have consistent types with UAPI.
>
> Signed-off-by: Barry Song
> ---
Looks good to me, thanks!
Reviewed-by: Carlos Llamas
Re: [PATCH RFC 1/8] dt-bindings: bus: allwinner: add H616 DE33 bindings
On Sun, Jun 09, 2024 at 03:19:55PM +1200, Ryan Walklin wrote: > On Sat, 8 Jun 2024, at 2:23 AM, Conor Dooley wrote: > >> + - const: allwinner,sun50i-h616-de33-clk > > > > I think this is not right, as a corresponding driver change is missing. > > Either you're missing a clock driver patch or you didn't test your dts. > > The clock driver patch with this compatible string is in patch 8/8. Ahh, I didn't notice that " drm: sun4i: add Display Engine 3.3 (DE33) support" had a clk driver. That needs to go into a patch of its own. signature.asc Description: PGP signature
Re: [PATCH v4 0/3] Improve gpu_scheduler trace events
On Mon, Jun 10, 2024 at 03:26:53PM +0200, Pierre-Eric Pelloux-Prayer wrote: > v3: https://lists.freedesktop.org/archives/dri-devel/2024-June/456792.html > > Changes since v3: > * trace device name instead of drm_device primary index > * no pointer deref in the TP_printk anymore. Instead the fence context/seqno > are saved in TP_fast_assign Some high-level comments: - Quick summary of the what, why and how in the cover letter would be great. - Link to the userspace, once you have that. At least last time we chatted that was still wip. - Maybe most important to make this actually work, work well, and work long-term: I think we should clearly commit to these tracepoints being stable uapi, and document that by adding a stable tracepoint section in the drm uapi book. And then get acks from a pile of driver maintainers that they really think this is a good idea and has a future. Should also help with getting good review on the tracepoints themselves. Otherwise I fear we'll miss the mark again and still force userspace to hand-roll tracing for every driver, or maybe worse, even specific kernel versions. Cheers, Sima > > Pierre-Eric Pelloux-Prayer (3): > drm/sched: add device name to the drm_sched_process_job event > drm/sched: cleanup gpu_scheduler trace events > drm/sched: trace dependencies for gpu jobs > > .../gpu/drm/scheduler/gpu_scheduler_trace.h | 97 +++ > drivers/gpu/drm/scheduler/sched_entity.c | 8 +- > drivers/gpu/drm/scheduler/sched_main.c| 2 +- > 3 files changed, 84 insertions(+), 23 deletions(-) > > -- > 2.40.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On Mon, Jun 10, 2024 at 02:38:18PM +0200, Christian König wrote: > Am 10.06.24 um 14:16 schrieb Jason Gunthorpe: > > On Mon, Jun 10, 2024 at 02:07:01AM +0100, Pavel Begunkov wrote: > > > On 6/10/24 01:37, David Wei wrote: > > > > On 2024-06-07 17:52, Jason Gunthorpe wrote: > > > > > IMHO it seems to compose poorly if you can only use the io_uring > > > > > lifecycle model with io_uring registered memory, and not with DMABUF > > > > > memory registered through Mina's mechanism. > > > > By this, do you mean io_uring must be exclusively used to use this > > > > feature? > > > > > > > > And you'd rather see the two decoupled, so userspace can register w/ say > > > > dmabuf then pass it to io_uring? > > > Personally, I have no clue what Jason means. You can just as > > > well say that it's poorly composable that write(2) to a disk > > > cannot post a completion into a XDP ring, or a netlink socket, > > > or io_uring's main completion queue, or name any other API. > > There is no reason you shouldn't be able to use your fast io_uring > > completion and lifecycle flow with DMABUF backed memory. Those are not > > widly different things and there is good reason they should work > > together. > > Well there is the fundamental problem that you can't use io_uring to > implement the semantics necessary for a dma_fence. > > That's why we had to reject the io_uring work on DMA-buf sharing from Google > a few years ago. > > But this only affects the dma_fence synchronization part of DMA-buf, but > *not* the general buffer sharing. More precisely, it only impacts the userspace/data access implicit synchronization part of dma-buf. For tracking buffer movements like on invalidations/refault with a dynamic dma-buf importer/exporter I think the dma-fence rules are acceptable. At least they've been for rdma drivers. But the escape hatch is to (temporarily) pin the dma-buf, which is exactly what direct I/O also does when accessing pages. So aside from the still unsolved question on how we should account/track pinned dma-buf, there shouldn't be an issue. Or at least I'm failing to see one. And for synchronization to data access the dma-fence stuff on dma-buf is anyway rather deprecated on the gpu side too, exactly because of all these limitations. On the gpu side we've been moving to free-standing drm_syncobj instead, but those are fairly gpu specific and any other subsystem should be able to just reuse what they have already to signal transaction completions. Cheers, Sima > > Regards, > Christian. > > > > > Pretending they are totally different just because two different > > people wrote them is a very siloed view. > > > > > The devmem TCP callback can implement it in a way feasible to > > > the project, but it cannot directly post events to an unrelated > > > API like io_uring. And devmem attaches buffers to a socket, > > > for which a ring for returning buffers might even be a nuisance. > > If you can't compose your io_uring completion mechanism with a DMABUF > > provided backing store then I think it needs more work. > > > > Jason > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] drm/connector: hdmi: Fix kerneldoc warnings
Hi Maxime,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm-tip/drm-tip]
[cannot apply to linus/master v6.10-rc3 next-20240607]
[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/Maxime-Ripard/drm-connector-hdmi-Fix-kerneldoc-warnings/20240610-191427
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20240610111200.428224-1-mripard%40kernel.org
patch subject: [PATCH] drm/connector: hdmi: Fix kerneldoc warnings
config: parisc-defconfig
(https://download.01.org/0day-ci/archive/20240610/202406102348.tvih790u-...@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240610/202406102348.tvih790u-...@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/202406102348.tvih790u-...@intel.com/
All errors (new ones prefixed by >>):
In file included from include/drm/drm_modes.h:33,
from include/drm/drm_crtc.h:32,
from include/drm/drm_atomic.h:31,
from drivers/gpu/drm/drm_atomic.c:32:
>> include/drm/drm_connector.h:997:1: error: expected ';', identifier or '('
>> before 'struct'
997 | struct drm_connector_state {
| ^~
vim +997 include/drm/drm_connector.h
5b530587b3eb3e Maxime Ripard2024-06-10 993
52217195176115 Daniel Vetter2016-08-12 994 /**
52217195176115 Daniel Vetter2016-08-12 995 * struct
drm_connector_state - mutable connector state
52217195176115 Daniel Vetter2016-08-12 996 */
52217195176115 Daniel Vetter2016-08-12 @997 struct
drm_connector_state {
aab999a66e4bc4 Daniel Vetter2018-07-09 998/** @connector:
backpointer to the connector */
52217195176115 Daniel Vetter2016-08-12 999struct drm_connector
*connector;
52217195176115 Daniel Vetter2016-08-12 1000
afb21ea63d815d Daniel Vetter2016-08-31 1001/**
afb21ea63d815d Daniel Vetter2016-08-31 1002 * @crtc: CRTC to
connect connector to, NULL if disabled.
afb21ea63d815d Daniel Vetter2016-08-31 1003 *
afb21ea63d815d Daniel Vetter2016-08-31 1004 * Do not change this
directly, use drm_atomic_set_crtc_for_connector()
afb21ea63d815d Daniel Vetter2016-08-31 1005 * instead.
afb21ea63d815d Daniel Vetter2016-08-31 1006 */
afb21ea63d815d Daniel Vetter2016-08-31 1007struct drm_crtc *crtc;
52217195176115 Daniel Vetter2016-08-12 1008
aab999a66e4bc4 Daniel Vetter2018-07-09 1009/**
aab999a66e4bc4 Daniel Vetter2018-07-09 1010 * @best_encoder:
aab999a66e4bc4 Daniel Vetter2018-07-09 1011 *
aab999a66e4bc4 Daniel Vetter2018-07-09 1012 * Used by the atomic
helpers to select the encoder, through the
aab999a66e4bc4 Daniel Vetter2018-07-09 1013 *
&drm_connector_helper_funcs.atomic_best_encoder or
aab999a66e4bc4 Daniel Vetter2018-07-09 1014 *
&drm_connector_helper_funcs.best_encoder callbacks.
27edadf6df811b Daniel Vetter2019-05-06 1015 *
1b27fbdde1df17 Laurent Pinchart 2019-06-11 1016 * This is also used in
the atomic helpers to map encoders to their
1b27fbdde1df17 Laurent Pinchart 2019-06-11 1017 * current and previous
connectors, see
12db36bc3cec76 Sean Paul2019-08-12 1018 *
drm_atomic_get_old_connector_for_encoder() and
12db36bc3cec76 Sean Paul2019-08-12 1019 *
drm_atomic_get_new_connector_for_encoder().
1b27fbdde1df17 Laurent Pinchart 2019-06-11 1020 *
27edadf6df811b Daniel Vetter2019-05-06 1021 * NOTE: Atomic drivers
must fill this out (either themselves or through
27edadf6df811b Daniel Vetter2019-05-06 1022 * helpers), for
otherwise the GETCONNECTOR and GETENCODER IOCTLs will
27edadf6df811b Daniel Vetter2019-05-06 1023 * not return correct
data to userspace.
aab999a66e4bc4 Daniel Vetter2018-07-09 1024 */
52217195176115 Daniel Vetter2016-08-12 1025struct drm_encoder
*best_encoder;
52217195176115 Daniel Vetter2016-08-12 1026
40ee6fbef75fe6 Manasi Navare2016-12-16 1027/**
40ee6fbef75fe6 Manasi Navare2016-12-16 1028 * @link_status:
Connector link_status to keep track of whether link is
40ee6fbef75fe6 Manasi Navare2016-12-16 1029 * GOOD or BAD to
notify
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On Mon, Jun 10, 2024 at 5:38 AM Christian König wrote: > > Am 10.06.24 um 14:16 schrieb Jason Gunthorpe: > > On Mon, Jun 10, 2024 at 02:07:01AM +0100, Pavel Begunkov wrote: > >> On 6/10/24 01:37, David Wei wrote: > >>> On 2024-06-07 17:52, Jason Gunthorpe wrote: > IMHO it seems to compose poorly if you can only use the io_uring > lifecycle model with io_uring registered memory, and not with DMABUF > memory registered through Mina's mechanism. > >>> By this, do you mean io_uring must be exclusively used to use this > >>> feature? > >>> > >>> And you'd rather see the two decoupled, so userspace can register w/ say > >>> dmabuf then pass it to io_uring? > >> Personally, I have no clue what Jason means. You can just as > >> well say that it's poorly composable that write(2) to a disk > >> cannot post a completion into a XDP ring, or a netlink socket, > >> or io_uring's main completion queue, or name any other API. > > There is no reason you shouldn't be able to use your fast io_uring > > completion and lifecycle flow with DMABUF backed memory. Those are not > > widly different things and there is good reason they should work > > together. > > Well there is the fundamental problem that you can't use io_uring to > implement the semantics necessary for a dma_fence. > > That's why we had to reject the io_uring work on DMA-buf sharing from > Google a few years ago. > Any chance someone can link me to this? io_uring, as far as my primitive understanding goes, is not yet very adopted at Google, and I'm curious what this effort is. -- Thanks, Mina
Re: [PATCH] drm/connector: hdmi: Fix kerneldoc warnings
Hi Maxime,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm-tip/drm-tip]
[cannot apply to linus/master v6.10-rc3 next-20240607]
[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/Maxime-Ripard/drm-connector-hdmi-Fix-kerneldoc-warnings/20240610-191427
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20240610111200.428224-1-mripard%40kernel.org
patch subject: [PATCH] drm/connector: hdmi: Fix kerneldoc warnings
config: riscv-defconfig
(https://download.01.org/0day-ci/archive/20240610/202406102334.csol5g2p-...@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project
4403cdbaf01379de96f8d0d6ea4f51a085e37766)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240610/202406102334.csol5g2p-...@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/202406102334.csol5g2p-...@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/gpu/drm/radeon/radeon_drv.c:36:
In file included from include/linux/vga_switcheroo.h:34:
In file included from include/linux/fb.h:5:
In file included from include/uapi/linux/fb.h:6:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:21:
In file included from arch/riscv/include/asm/sections.h:9:
In file included from include/linux/mm.h:2253:
include/linux/vmstat.h:514:36: warning: arithmetic between different
enumeration types ('enum node_stat_item' and 'enum lru_list')
[-Wenum-enum-conversion]
514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~ ^ ~~~
In file included from drivers/gpu/drm/radeon/radeon_drv.c:46:
In file included from include/drm/drm_probe_helper.h:6:
In file included from include/drm/drm_modes.h:33:
>> include/drm/drm_connector.h:992:2: error: expected ';' after struct
992 | }
| ^
| ;
drivers/gpu/drm/radeon/radeon_drv.c:251:2: warning: bitwise operation
between different enumeration types ('enum radeon_family' and 'enum
radeon_chip_flags') [-Wenum-enum-conversion]
251 | radeon_PCI_IDS
| ^~
include/drm/drm_pciids.h:3:60: note: expanded from macro 'radeon_PCI_IDS'
3 | {0x1002, 0x1304, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
|
~~~^~~
drivers/gpu/drm/radeon/radeon_drv.c:251:2: warning: bitwise operation
between different enumeration types ('enum radeon_family' and 'enum
radeon_chip_flags') [-Wenum-enum-conversion]
251 | radeon_PCI_IDS
| ^~
include/drm/drm_pciids.h:4:60: note: expanded from macro 'radeon_PCI_IDS'
4 | {0x1002, 0x1305, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
|
~~~^~
drivers/gpu/drm/radeon/radeon_drv.c:251:2: warning: bitwise operation
between different enumeration types ('enum radeon_family' and 'enum
radeon_chip_flags') [-Wenum-enum-conversion]
251 | radeon_PCI_IDS
| ^~
include/drm/drm_pciids.h:5:60: note: expanded from macro 'radeon_PCI_IDS'
5 | {0x1002, 0x1306, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
|
~~~^~~
drivers/gpu/drm/radeon/radeon_drv.c:251:2: warning: bitwise operation
between different enumeration types ('enum radeon_family' and 'enum
radeon_chip_flags') [-Wenum-enum-conversion]
251 | radeon_PCI_IDS
| ^~
include/drm/
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On 6/10/24 6:16 AM, Jason Gunthorpe wrote: > On Mon, Jun 10, 2024 at 02:07:01AM +0100, Pavel Begunkov wrote: >> On 6/10/24 01:37, David Wei wrote: >>> On 2024-06-07 17:52, Jason Gunthorpe wrote: IMHO it seems to compose poorly if you can only use the io_uring lifecycle model with io_uring registered memory, and not with DMABUF memory registered through Mina's mechanism. >>> >>> By this, do you mean io_uring must be exclusively used to use this >>> feature? >>> >>> And you'd rather see the two decoupled, so userspace can register w/ say >>> dmabuf then pass it to io_uring? >> >> Personally, I have no clue what Jason means. You can just as >> well say that it's poorly composable that write(2) to a disk >> cannot post a completion into a XDP ring, or a netlink socket, >> or io_uring's main completion queue, or name any other API. > > There is no reason you shouldn't be able to use your fast io_uring > completion and lifecycle flow with DMABUF backed memory. Those are not > widly different things and there is good reason they should work > together. > > Pretending they are totally different just because two different > people wrote them is a very siloed view. > >> The devmem TCP callback can implement it in a way feasible to >> the project, but it cannot directly post events to an unrelated >> API like io_uring. And devmem attaches buffers to a socket, >> for which a ring for returning buffers might even be a nuisance. > > If you can't compose your io_uring completion mechanism with a DMABUF > provided backing store then I think it needs more work. > exactly. io_uring, page_pool, dmabuf - all kernel building blocks for solutions. This why I was pushing for Mina's set not to be using the name `devmem` - it is but one type of memory and with dmabuf it should not matter if it is gpu or host (or something else later on - cxl?).
Re: Subject: [PATCH] drm/panel : truly-nt35521: transition to mipi_dsi wrapped functions
Hi,
On Sat, Jun 8, 2024 at 3:57 AM Tejas Vipin wrote:
>
> Use functions introduced in 966e397e4f603 ("drm/mipi-dsi: Introduce
> mipi_dsi_*_write_seq_multi()") and f79d6d28d8fe
> ("drm/mipi-dsi: wrap more functions for streamline handling") for the
> sony tulip truly nt35521 panel.
Running "scripts/checkpatch.pl" will yell about the above. You're
supposed to write the word "commit" before the git hash. AKA:
Use functions introduced in commit 966e397e4f603 ("drm/mipi-dsi:
Introduce mipi_dsi_*_write_seq_multi()") and commit f79d6d28d8fe
("drm/mipi-dsi: wrap more functions for streamline handling") for
the...
> Signed-off-by: Tejas Vipin
> ---
> .../panel/panel-sony-tulip-truly-nt35521.c| 383 +-
> 1 file changed, 183 insertions(+), 200 deletions(-)
The subject of your patch has an extra "Subject:" prefix. See:
https://lore.kernel.org/r/485eef24-ddad-466a-a89f-f9f226801...@gmail.com
...where you can see "Subject: Subject:". Maybe use "b4" or "patman"
to help you send your patch?
> diff --git a/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> b/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> index 6d44970dccd9..13472c7c37f5 100644
> --- a/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> +++ b/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> @@ -44,248 +44,231 @@ static void truly_nt35521_reset(struct truly_nt35521
> *ctx)
> static int truly_nt35521_on(struct truly_nt35521 *ctx)
> {
> struct mipi_dsi_device *dsi = ctx->dsi;
> - struct device *dev = &dsi->dev;
> - int ret;
>
> dsi->mode_flags |= MIPI_DSI_MODE_LPM;
>
> - mipi_dsi_generic_write_seq(dsi, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
> - mipi_dsi_generic_write_seq(dsi, 0xff, 0xaa, 0x55, 0xa5, 0x80);
> - mipi_dsi_generic_write_seq(dsi, 0x6f, 0x11, 0x00);
> - mipi_dsi_generic_write_seq(dsi, 0xf7, 0x20, 0x00);
> - mipi_dsi_generic_write_seq(dsi, 0x6f, 0x01);
> - mipi_dsi_generic_write_seq(dsi, 0xb1, 0x21);
> - mipi_dsi_generic_write_seq(dsi, 0xbd, 0x01, 0xa0, 0x10, 0x08, 0x01);
> - mipi_dsi_generic_write_seq(dsi, 0xb8, 0x01, 0x02, 0x0c, 0x02);
> - mipi_dsi_generic_write_seq(dsi, 0xbb, 0x11, 0x11);
> - mipi_dsi_generic_write_seq(dsi, 0xbc, 0x00, 0x00);
> - mipi_dsi_generic_write_seq(dsi, 0xb6, 0x02);
> - mipi_dsi_generic_write_seq(dsi, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x01);
> - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x09, 0x09);
> - mipi_dsi_generic_write_seq(dsi, 0xb1, 0x09, 0x09);
> - mipi_dsi_generic_write_seq(dsi, 0xbc, 0x8c, 0x00);
> - mipi_dsi_generic_write_seq(dsi, 0xbd, 0x8c, 0x00);
> - mipi_dsi_generic_write_seq(dsi, 0xca, 0x00);
> - mipi_dsi_generic_write_seq(dsi, 0xc0, 0x04);
> - mipi_dsi_generic_write_seq(dsi, 0xbe, 0xb5);
> - mipi_dsi_generic_write_seq(dsi, 0xb3, 0x35, 0x35);
> - mipi_dsi_generic_write_seq(dsi, 0xb4, 0x25, 0x25);
> - mipi_dsi_generic_write_seq(dsi, 0xb9, 0x43, 0x43);
> - mipi_dsi_generic_write_seq(dsi, 0xba, 0x24, 0x24);
> - mipi_dsi_generic_write_seq(dsi, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x02);
> - mipi_dsi_generic_write_seq(dsi, 0xee, 0x03);
> - mipi_dsi_generic_write_seq(dsi, 0xb0,
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
Please move the variable declaration above the line "dsi->mode_flags
|= MIPI_DSI_MODE_LPM;"
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52,
> 0x08, 0x00);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5,
> 0x80);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x6f, 0x11, 0x00);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xf7, 0x20, 0x00);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x6f, 0x01);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb1, 0x21);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbd, 0x01, 0xa0, 0x10,
> 0x08, 0x01);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb8, 0x01, 0x02, 0x0c,
> 0x02);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbb, 0x11, 0x11);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbc, 0x00, 0x00);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb6, 0x02);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52,
> 0x08, 0x01);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x09, 0x09);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb1, 0x09, 0x09);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbc, 0x8c, 0x00);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbd, 0x8c, 0x00);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xca, 0x00);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xc0, 0x04);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbe, 0xb5);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb3, 0x35, 0x35);
> + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb4, 0x25, 0x25);
> +
[Bug 218900] amdgpu: Fatal error during GPU init
https://bugzilla.kernel.org/show_bug.cgi?id=218900 --- Comment #16 from Vasant Hegde (vasant.he...@amd.com) --- (In reply to Hanabishi from comment #15) > (In reply to Vasant Hegde from comment #5) > > Created attachment 306364 [details] > > Check Enhanced PPR support before enabling PPR > > I applied your patch on top of rc2 and also confirm that it works. > Thank you. Thanks Hanabishi for testing. FYI. Patches merged into -rc3. -Vasant -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v1] drm/bridge: it6505: update usleep_range for RC circuit charge time
On Tue, 4 Jun 2024 10:44:05 +0800, kuro wrote: > From: Kuro Chung > > The spec of timing between IVDD/OVDD and SYSRTEN is 10ms, but SYSRSTN RC > circuit need at least 25ms for rising time, update for match spec > > Applied, thanks! [1/1] drm/bridge: it6505: update usleep_range for RC circuit charge time https://cgit.freedesktop.org/drm/drm-misc/commit/?id=881e62b8 Rob
Re: [PATCH V3 1/2] drm/bridge: samsung-dsim: Set P divider based on min/max of fin pll
On Sat, 1 Jun 2024 09:41:01 -0500, Adam Ford wrote: > The P divider should be set based on the min and max values of > the fin pll which may vary between different platforms. > These ranges are defined per platform, but hard-coded values > were used instead which resulted in a smaller range available > on the i.MX8M[MNP] than what was possible. > > As noted by Frieder, there are descripencies between the reference > manuals of the Mini, Nano and Plus, so I reached out to my NXP > rep and got the following response regarding the varing notes > in the documentation. > > [...] Applied, thanks! [1/2] drm/bridge: samsung-dsim: Set P divider based on min/max of fin pll (no commit info) [2/2] drm/bridge: samsung-dsim: Fix porch calcalcuation rounding (no commit info) Rob
Re: [PATCH] Revert "drm/amdgpu: init iommu after amdkfd device init"
Am 04.06.24 um 20:28 schrieb Deucher, Alexander: [AMD Official Use Only - AMD Internal Distribution Only] -Original Message- From: Kuehling, Felix Sent: Tuesday, June 4, 2024 2:25 PM To: Armin Wolf ; Deucher, Alexander ; Koenig, Christian ; Pan, Xinhui ; gre...@linuxfoundation.org; sas...@kernel.org Cc: sta...@vger.kernel.org; bkau...@gmail.com; Zhang, Yifan ; Liang, Prike ; dri- de...@lists.freedesktop.org; amd-...@lists.freedesktop.org Subject: Re: [PATCH] Revert "drm/amdgpu: init iommu after amdkfd device init" On 2024-06-03 18:19, Armin Wolf wrote: Am 23.05.24 um 19:30 schrieb Armin Wolf: This reverts commit 56b522f4668167096a50c39446d6263c96219f5f. A user reported that this commit breaks the integrated gpu of his notebook, causing a black screen. He was able to bisect the problematic commit and verified that by reverting it the notebook works again. He also confirmed that kernel 6.8.1 also works on his device, so the upstream commit itself seems to be ok. An amdgpu developer (Alex Deucher) confirmed that this patch should have never been ported to 5.15 in the first place, so revert this commit from the 5.15 stable series. Hi, what is the status of this? Which branch is this for? This patch won't apply to anything after Linux 6.5. It's applicable to 5.15 stable only. The original patch caused a regression on 5.15 so probably should not have been applied there. Alex Correct, and i would be very grateful if this regression could be resolved in the near future. The user already wrote a blog post about the whole issue, see here: https://bkhome.org/news/202405/kernel-amd-gpu-disaster-fixed.html Thanks, Armin Wolf Support for IOMMUv2 was removed from amdgpu in Linux 6.6 by: commit c99a2e7ae291e5b19b60443eb6397320ef9e8571 Author: Alex Deucher Date: Fri Jul 28 12:20:12 2023 -0400 drm/amdkfd: drop IOMMUv2 support Now that we use the dGPU path for all APUs, drop the IOMMUv2 support. v2: drop the now unused queue manager functions for gfx7/8 APUs Reviewed-by: Felix Kuehling Acked-by: Christian König Tested-by: Mike Lothian Signed-off-by: Alex Deucher Regards, Felix Armin Wolf Reported-by: Barry Kauler Signed-off-by: Armin Wolf --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 222a1d9ecf16..5f6c32ec674d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2487,6 +2487,10 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) if (r) goto init_failed; +r = amdgpu_amdkfd_resume_iommu(adev); +if (r) +goto init_failed; + r = amdgpu_device_ip_hw_init_phase1(adev); if (r) goto init_failed; @@ -2525,10 +2529,6 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) if (!adev->gmc.xgmi.pending_reset) amdgpu_amdkfd_device_init(adev); -r = amdgpu_amdkfd_resume_iommu(adev); -if (r) -goto init_failed; - amdgpu_fru_get_product_info(adev); init_failed: -- 2.39.2
Re: [PATCH] drm/bridge: tc358767: Check if fully initialized before signalling HPD event via IRQ
On Fri, 31 May 2024 22:33:12 +0200, Marek Vasut wrote: > Make sure the connector is fully initialized before signalling any > HPD events via drm_kms_helper_hotplug_event(), otherwise this may > lead to NULL pointer dereference. > > Applied, thanks! [1/1] drm/bridge: tc358767: Check if fully initialized before signalling HPD event via IRQ https://cgit.freedesktop.org/drm/drm-misc/commit/?id=162e48cb1d84 Rob
Re: [PATCH] drm/bridge: tc358767: Fix comment in tc_edp_mode_valid
On Fri, 31 May 2024 22:32:01 +0200, Marek Vasut wrote: > Fix comment copy-paste error in tc_edp_mode_valid(), this function > is validating DP/eDP clock, not DPI clock frequency. Update the > comment to match. No functional change. > > Applied, thanks! [1/1] drm/bridge: tc358767: Fix comment in tc_edp_mode_valid https://cgit.freedesktop.org/drm/drm-misc/commit/?id=004370a82ae1 Rob
[PATCH v2 1/3] drm/mgag200: Consolidate VGA output
The various models have common code for the VGA output's encoder and
connector. Move everything into a single shared source file. Remove some
obsolete initializer macros. No functional changes.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/mgag200/Makefile | 3 +-
drivers/gpu/drm/mgag200/mgag200_drv.h | 24 +++-
drivers/gpu/drm/mgag200/mgag200_g200.c| 46 +--
drivers/gpu/drm/mgag200/mgag200_g200eh.c | 46 +--
drivers/gpu/drm/mgag200/mgag200_g200eh3.c | 46 +--
drivers/gpu/drm/mgag200/mgag200_g200er.c | 46 +--
drivers/gpu/drm/mgag200/mgag200_g200ev.c | 46 +--
drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 46 +--
drivers/gpu/drm/mgag200/mgag200_g200se.c | 46 +--
drivers/gpu/drm/mgag200/mgag200_g200wb.c | 46 +--
drivers/gpu/drm/mgag200/mgag200_vga.c | 68 +++
11 files changed, 95 insertions(+), 368 deletions(-)
create mode 100644 drivers/gpu/drm/mgag200/mgag200_vga.c
diff --git a/drivers/gpu/drm/mgag200/Makefile b/drivers/gpu/drm/mgag200/Makefile
index 0b919352046eb..d1b25f9f6586e 100644
--- a/drivers/gpu/drm/mgag200/Makefile
+++ b/drivers/gpu/drm/mgag200/Makefile
@@ -11,6 +11,7 @@ mgag200-y := \
mgag200_g200ew3.o \
mgag200_g200se.o \
mgag200_g200wb.o \
- mgag200_mode.o
+ mgag200_mode.o \
+ mgag200_vga.o
obj-$(CONFIG_DRM_MGAG200) += mgag200.o
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 20e3710e056b3..db89fddc26dcb 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -283,8 +283,12 @@ struct mga_device {
struct drm_plane primary_plane;
struct drm_crtc crtc;
- struct drm_encoder encoder;
- struct drm_connector connector;
+ struct {
+ struct {
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+ } vga;
+ } output;
};
static inline struct mga_device *to_mga_device(struct drm_device *dev)
@@ -417,25 +421,15 @@ void mgag200_crtc_atomic_destroy_state(struct drm_crtc
*crtc, struct drm_crtc_st
.atomic_duplicate_state = mgag200_crtc_atomic_duplicate_state, \
.atomic_destroy_state = mgag200_crtc_atomic_destroy_state
-#define MGAG200_DAC_ENCODER_FUNCS \
- .destroy = drm_encoder_cleanup
-
-#define MGAG200_VGA_CONNECTOR_HELPER_FUNCS \
- .get_modes = drm_connector_helper_get_modes
-
-#define MGAG200_VGA_CONNECTOR_FUNCS \
- .reset = drm_atomic_helper_connector_reset, \
- .fill_modes = drm_helper_probe_single_connector_modes, \
- .destroy= drm_connector_cleanup, \
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, \
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state
-
void mgag200_set_mode_regs(struct mga_device *mdev, const struct
drm_display_mode *mode);
void mgag200_set_format_regs(struct mga_device *mdev, const struct
drm_format_info *format);
void mgag200_enable_display(struct mga_device *mdev);
void mgag200_init_registers(struct mga_device *mdev);
int mgag200_mode_config_init(struct mga_device *mdev, resource_size_t
vram_available);
+/* mgag200_vga.c */
+int mgag200_vga_output_init(struct mga_device *mdev);
+
/* mgag200_bmc.c */
void mgag200_bmc_disable_vidrst(struct mga_device *mdev);
void mgag200_bmc_enable_vidrst(struct mga_device *mdev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200.c
b/drivers/gpu/drm/mgag200/mgag200_g200.c
index 39a29d8ffca6e..ff467b0f9cbf3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200.c
@@ -9,7 +9,6 @@
#include
#include
-#include "mgag200_ddc.h"
#include "mgag200_drv.h"
static int mgag200_g200_init_pci_options(struct pci_dev *pdev)
@@ -184,26 +183,11 @@ static const struct drm_crtc_funcs
mgag200_g200_crtc_funcs = {
MGAG200_CRTC_FUNCS,
};
-static const struct drm_encoder_funcs mgag200_g200_dac_encoder_funcs = {
- MGAG200_DAC_ENCODER_FUNCS,
-};
-
-static const struct drm_connector_helper_funcs
mgag200_g200_vga_connector_helper_funcs = {
- MGAG200_VGA_CONNECTOR_HELPER_FUNCS,
-};
-
-static const struct drm_connector_funcs mgag200_g200_vga_connector_funcs = {
- MGAG200_VGA_CONNECTOR_FUNCS,
-};
-
static int mgag200_g200_pipeline_init(struct mga_device *mdev)
{
struct drm_device *dev = &mdev->base;
struct drm_plane *primary_plane = &mdev->primary_plane;
struct drm_crtc *crtc = &mdev->crtc;
- struct drm_encoder *encoder = &mdev->encoder;
- struct drm_connector *connector = &mdev->connector;
- struct i2c_adapter *ddc;
int ret;
ret = drm_universal_plane_init(dev, primary_plane, 0,
@@ -231,35 +215,9 @@ static int mgag200_g200_pipeline_ini
[PATCH v2 2/3] drm/mgag200: Add BMC output
The BMC output can be viewed via the BMC's web interface or a
similar client. Represent it as virtual encoder and connector.
It's attached to the same CRTC as the VGA connector.
The connector's status depends on the physical connector's status.
The BMC is only connected if the physical connector is not. This
is necessary to support userspace clients that can only handle a
single output per CRTC.
The BMC is a server feature. Add a BMC output for all server chips,
but not the desktop models.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/mgag200/mgag200_bmc.c | 107 ++
drivers/gpu/drm/mgag200/mgag200_drv.h | 10 ++
drivers/gpu/drm/mgag200/mgag200_g200eh.c | 4 +
drivers/gpu/drm/mgag200/mgag200_g200eh3.c | 4 +
drivers/gpu/drm/mgag200/mgag200_g200er.c | 4 +
drivers/gpu/drm/mgag200/mgag200_g200ev.c | 4 +
drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 4 +
drivers/gpu/drm/mgag200/mgag200_g200se.c | 4 +
drivers/gpu/drm/mgag200/mgag200_g200wb.c | 4 +
9 files changed, 145 insertions(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_bmc.c
b/drivers/gpu/drm/mgag200/mgag200_bmc.c
index 2ba2e3c5086a5..23ef85aa7e378 100644
--- a/drivers/gpu/drm/mgag200/mgag200_bmc.c
+++ b/drivers/gpu/drm/mgag200/mgag200_bmc.c
@@ -2,8 +2,18 @@
#include
+#include
+#include
+#include
+#include
+
#include "mgag200_drv.h"
+static struct mgag200_bmc_connector *to_mgag200_bmc_connector(struct
drm_connector *connector)
+{
+ return container_of(connector, struct mgag200_bmc_connector, base);
+}
+
void mgag200_bmc_disable_vidrst(struct mga_device *mdev)
{
u8 tmp;
@@ -97,3 +107,100 @@ void mgag200_bmc_enable_vidrst(struct mga_device *mdev)
tmp &= ~0x10;
WREG_DAC(MGA1064_GEN_IO_DATA, tmp);
}
+
+static const struct drm_encoder_funcs mgag200_bmc_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static int mgag200_bmc_connector_helper_detect_ctx(struct drm_connector
*connector,
+ struct
drm_modeset_acquire_ctx *ctx,
+ bool force)
+{
+ struct mgag200_bmc_connector *bmc_connector =
to_mgag200_bmc_connector(connector);
+ struct drm_connector *physical_connector =
bmc_connector->physical_connector;
+
+ /*
+* Most user-space compositors cannot handle more than one connected
+* connector per CRTC. Hence, we only mark the BMC as connected if the
+* physical connector is disconnected. If the physical connector's
status
+* is connected or unknown, the BMC remains disconnected. This has no
+* effect on the output of the BMC.
+*
+* FIXME: Remove this logic once user-space compositors can handle more
+*than one connector per CRTC. The BMC should always be
connected.
+*/
+
+ if (physical_connector && physical_connector->status ==
connector_status_disconnected)
+ return connector_status_connected;
+
+ return connector_status_disconnected;
+}
+
+static int mgag200_bmc_connector_helper_get_modes(struct drm_connector
*connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct mga_device *mdev = to_mga_device(dev);
+ const struct mgag200_device_info *minfo = mdev->info;
+
+ return drm_add_modes_noedid(connector, minfo->max_hdisplay,
minfo->max_vdisplay);
+}
+
+static const struct drm_connector_helper_funcs
mgag200_bmc_connector_helper_funcs = {
+ .get_modes = mgag200_bmc_connector_helper_get_modes,
+ .detect_ctx = mgag200_bmc_connector_helper_detect_ctx,
+};
+
+static const struct drm_connector_funcs mgag200_bmc_connector_funcs = {
+ .reset = drm_atomic_helper_connector_reset,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int mgag200_bmc_connector_init(struct drm_device *dev,
+ struct mgag200_bmc_connector
*bmc_connector,
+ struct drm_connector *physical_connector)
+{
+ struct drm_connector *connector = &bmc_connector->base;
+ int ret;
+
+ ret = drm_connector_init(dev, connector, &mgag200_bmc_connector_funcs,
+DRM_MODE_CONNECTOR_VIRTUAL);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector,
&mgag200_bmc_connector_helper_funcs);
+
+ bmc_connector->physical_connector = physical_connector;
+
+ return 0;
+}
+
+int mgag200_bmc_output_init(struct mga_device *mdev, struct drm_connector
*physical_connector)
+{
+ struct drm_device *dev = &mdev->base;
+ struct drm_crtc *crtc = &mdev->crtc;
+ struct drm_encoder *encoder;
+ struct mgag200_bmc_connector *bmc_connector;
[PATCH v2 3/3] drm/mgag200: Set .detect_ctx() and enable connector polling
Set .detect_ctx() in struct drm_connector_helper_funcs to the
common helper drm_connector_helper_detect_from_ddc() and enable
polling for the connector. Mgag200 will now test for the monitor's
presence by probing the DDC in regular intervals.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/mgag200/mgag200_g200.c| 1 +
drivers/gpu/drm/mgag200/mgag200_g200eh.c | 1 +
drivers/gpu/drm/mgag200/mgag200_g200eh3.c | 1 +
drivers/gpu/drm/mgag200/mgag200_g200er.c | 1 +
drivers/gpu/drm/mgag200/mgag200_g200ev.c | 1 +
drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 1 +
drivers/gpu/drm/mgag200/mgag200_g200se.c | 1 +
drivers/gpu/drm/mgag200/mgag200_g200wb.c | 1 +
drivers/gpu/drm/mgag200/mgag200_vga.c | 6 +-
9 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200.c
b/drivers/gpu/drm/mgag200/mgag200_g200.c
index ff467b0f9cbf3..f874e29498409 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200.c
@@ -401,6 +401,7 @@ struct mga_device *mgag200_g200_device_create(struct
pci_dev *pdev, const struct
return ERR_PTR(ret);
drm_mode_config_reset(dev);
+ drm_kms_helper_poll_init(dev);
return mdev;
}
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh.c
b/drivers/gpu/drm/mgag200/mgag200_g200eh.c
index 6f31c5249f0b1..52bf49ead5c50 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200eh.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh.c
@@ -277,6 +277,7 @@ struct mga_device *mgag200_g200eh_device_create(struct
pci_dev *pdev, const stru
return ERR_PTR(ret);
drm_mode_config_reset(dev);
+ drm_kms_helper_poll_init(dev);
return mdev;
}
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
b/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
index 5befe8da4beb2..e7f89b2a59fd0 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
@@ -182,6 +182,7 @@ struct mga_device *mgag200_g200eh3_device_create(struct
pci_dev *pdev,
return ERR_PTR(ret);
drm_mode_config_reset(dev);
+ drm_kms_helper_poll_init(dev);
return mdev;
}
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c
b/drivers/gpu/drm/mgag200/mgag200_g200er.c
index 55c275180cde2..4e8a1756138d7 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
@@ -316,6 +316,7 @@ struct mga_device *mgag200_g200er_device_create(struct
pci_dev *pdev, const stru
return ERR_PTR(ret);
drm_mode_config_reset(dev);
+ drm_kms_helper_poll_init(dev);
return mdev;
}
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
index 2466126140db6..d884f3cb0ec79 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
@@ -321,6 +321,7 @@ struct mga_device *mgag200_g200ev_device_create(struct
pci_dev *pdev, const stru
return ERR_PTR(ret);
drm_mode_config_reset(dev);
+ drm_kms_helper_poll_init(dev);
return mdev;
}
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
index a52e60609c3de..839401e8b4654 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
@@ -202,6 +202,7 @@ struct mga_device *mgag200_g200ew3_device_create(struct
pci_dev *pdev,
return ERR_PTR(ret);
drm_mode_config_reset(dev);
+ drm_kms_helper_poll_init(dev);
return mdev;
}
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c
b/drivers/gpu/drm/mgag200/mgag200_g200se.c
index 212770acdd477..a824bb8ad5791 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
@@ -521,6 +521,7 @@ struct mga_device *mgag200_g200se_device_create(struct
pci_dev *pdev, const stru
return ERR_PTR(ret);
drm_mode_config_reset(dev);
+ drm_kms_helper_poll_init(dev);
return mdev;
}
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200wb.c
b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
index cb6daa0426fbc..835df0f4fc13d 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200wb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
@@ -326,6 +326,7 @@ struct mga_device *mgag200_g200wb_device_create(struct
pci_dev *pdev, const stru
return ERR_PTR(ret);
drm_mode_config_reset(dev);
+ drm_kms_helper_poll_init(dev);
return mdev;
}
diff --git a/drivers/gpu/drm/mgag200/mgag200_vga.c
b/drivers/gpu/drm/mgag200/mgag200_vga.c
index 6d8982990c2c3..60568f32736dd 100644
--- a/drivers/gpu/drm/mgag200/mgag200_vga.c
+++ b/drivers/gpu/drm/mgag200/mgag200_vga.c
@@ -12,7 +12,8 @@ static const struct drm_encoder_funcs
mgag200_dac_encoder_funcs = {
};
static const struct drm_connector_helper_funcs
mgag200_vga_connector_helper_funcs = {
- .get_modes = drm_conn
[PATCH v2 0/3] drm/mgag200: Detect connector status
Detect the connector status by polling the DDC. Update the status at runtime. Add a dedicated BMC output to still display to the BMC while the VGA connector is not attached. This patchset fixes a long-standing problem where attaching the VGA display a runtime resulted in incorrect display modes. Tested on various Matrox hardware. v2: - move the DDC clean up into a separate patchset [1] - add dedicated BMC support (Jocelyn) [1] https://patchwork.freedesktop.org/series/133537/ Thomas Zimmermann (3): drm/mgag200: Consolidate VGA output drm/mgag200: Add BMC output drm/mgag200: Set .detect_ctx() and enable connector polling drivers/gpu/drm/mgag200/Makefile | 3 +- drivers/gpu/drm/mgag200/mgag200_bmc.c | 107 ++ drivers/gpu/drm/mgag200/mgag200_drv.h | 34 --- drivers/gpu/drm/mgag200/mgag200_g200.c| 47 +- drivers/gpu/drm/mgag200/mgag200_g200eh.c | 47 +- drivers/gpu/drm/mgag200/mgag200_g200eh3.c | 47 +- drivers/gpu/drm/mgag200/mgag200_g200er.c | 47 +- drivers/gpu/drm/mgag200/mgag200_g200ev.c | 47 +- drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 47 +- drivers/gpu/drm/mgag200/mgag200_g200se.c | 47 +- drivers/gpu/drm/mgag200/mgag200_g200wb.c | 47 +- drivers/gpu/drm/mgag200/mgag200_vga.c | 72 +++ 12 files changed, 238 insertions(+), 354 deletions(-) create mode 100644 drivers/gpu/drm/mgag200/mgag200_vga.c base-commit: 2bea08bd31298d60d416b2a6ed346bb53dd28037 -- 2.45.2
[drm-misc:topic/rust-drm 9/20] warning: unresolved link to `new_device_data`
tree: git://anongit.freedesktop.org/drm/drm-misc topic/rust-drm head: 508348922df19178b613531fb6cc7beb624642ae commit: 28ea76b321b25ee422d9c9bd08f1bf605a9ae422 [9/20] rust: add device::Data config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240610/202406102138.eekfigxb-...@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240610/202406102138.eekfigxb-...@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/202406102138.eekfigxb-...@intel.com/ All warnings (new ones prefixed by >>): >> warning: unresolved link to `new_device_data` --> rust/kernel/device.rs:106:38 | 106 | /// It is recommended that the [`new_device_data`] macro be used as it automatically creates | ^^^ no item named `new_device_data` in scope | = help: to escape `[` and `]` characters, add '' before them like `[` or `]` = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
[PATCH v4 3/3] drm/sched: trace dependencies for gpu jobs
Trace the fence dependencies similarly to how we print fences:
... , dependencies:{(context:606, seqno:38006)}
This allows tools to analyze the dependencies between the jobs (previously
it was only possible for fences traced by drm_sched_job_wait_dep).
Since drm_sched_job and drm_run_job use the same base event class,
the caller of trace_drm_run_job have to pass a dep_count of 0 (which
is ignored because dependencies are only considered at submit time).
Signed-off-by: Pierre-Eric Pelloux-Prayer
---
.../gpu/drm/scheduler/gpu_scheduler_trace.h | 59 ---
drivers/gpu/drm/scheduler/sched_entity.c | 8 ++-
drivers/gpu/drm/scheduler/sched_main.c| 2 +-
3 files changed, 58 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
index 6541e161962f..702d1f709bcf 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
@@ -26,15 +26,41 @@
#include
#include
+#include
#include
#undef TRACE_SYSTEM
#define TRACE_SYSTEM gpu_scheduler
#define TRACE_INCLUDE_FILE gpu_scheduler_trace
+#ifndef __TRACE_EVENT_GPU_SCHEDULER_PRINT_FN
+#define __TRACE_EVENT_GPU_SCHEDULER_PRINT_FN
+/* Similar to trace_print_array_seq but for fences. */
+static inline const char *__print_dma_fence_array(struct trace_seq *p, const
void *buf, int count)
+{
+ const char *ret = trace_seq_buffer_ptr(p);
+ u64 *fences = (u64 *) buf;
+ const char *prefix = "";
+
+ trace_seq_putc(p, '{');
+ for (int i = 0; i < count; i++) {
+ u64 context = fences[2 * i], seqno = fences[2 * i + 1];
+
+ trace_seq_printf(p, "%s(context:%llu, seqno:%lld)",
+prefix, context, seqno);
+ prefix = ",";
+ }
+ trace_seq_putc(p, '}');
+ trace_seq_putc(p, 0);
+
+ return ret;
+}
+#endif
+
DECLARE_EVENT_CLASS(drm_sched_job,
- TP_PROTO(struct drm_sched_job *sched_job, struct drm_sched_entity
*entity),
- TP_ARGS(sched_job, entity),
+ TP_PROTO(struct drm_sched_job *sched_job, struct drm_sched_entity
*entity,
+unsigned int dep_count),
+ TP_ARGS(sched_job, entity, dep_count),
TP_STRUCT__entry(
__field(struct drm_sched_entity *, entity)
__string(name, sched_job->sched->name)
@@ -44,9 +70,14 @@ DECLARE_EVENT_CLASS(drm_sched_job,
__string(dev, dev_name(sched_job->sched->dev))
__field(uint64_t, fence_context)
__field(uint64_t, fence_seqno)
+__field(int, n_deps)
+__dynamic_array(u64, deps, dep_count * 2)
),
TP_fast_assign(
+ unsigned long idx;
+ struct dma_fence *fence;
+ u64 *dyn_arr;
__entry->entity = entity;
__entry->id = sched_job->id;
__assign_str(name);
@@ -56,22 +87,32 @@ DECLARE_EVENT_CLASS(drm_sched_job,
__assign_str(dev);
__entry->fence_context =
sched_job->s_fence->finished.context;
__entry->fence_seqno =
sched_job->s_fence->finished.seqno;
-
+ __entry->n_deps = dep_count;
+ if (dep_count) {
+ dyn_arr = __get_dynamic_array(deps);
+ xa_for_each(&sched_job->dependencies, idx,
fence) {
+ dyn_arr[2 * idx] = fence->context;
+ dyn_arr[2 * idx + 1] = fence->seqno;
+ }
+ }
),
- TP_printk("id=%llu, fence=(context:%llu, seqno:%lld), ring=%s, job
count:%u, hw job count:%d",
+ TP_printk("id=%llu, fence=(context:%llu, seqno:%lld), ring=%s, job
count:%u, hw job count:%d, dependencies:%s",
__entry->id,
__entry->fence_context, __entry->fence_seqno,
__get_str(name),
- __entry->job_count, __entry->hw_job_count)
+ __entry->job_count, __entry->hw_job_count,
+ __print_dma_fence_array(p, __get_dynamic_array(deps),
__entry->n_deps))
);
DEFINE_EVENT(drm_sched_job, drm_sched_job,
- TP_PROTO(struct drm_sched_job *sched_job, struct drm_sched_entity
*entity),
- TP_ARGS(sched_job, entity)
+ TP_PROTO(struct drm_sched_job *sched_job, struct drm_sched_entity
*entity,
+unsigned int dep_count),
+ TP_ARGS(sched_job, entity, dep_count)
);
DEFINE_EVENT_PRINT(drm_sched_job, drm_run_job,
-
[PATCH v4 2/3] drm/sched: cleanup gpu_scheduler trace events
Print identifiers instead of pointers:
* "fence=%p" is replaced by "fence=(context:%llu, seqno:%lld)" to have a
coherent way to print the fence. A possible follow up change would be
to use the same format in traces/../dma-fence.h.
* "entity=%p" is removed because the fence's context is already an
identifier of the job owner.
For drm_sched_job_wait_dep, we also print the hardware exec context of
the fence that's initiating the wait (the scheduled fence ctx is not
relevant here, since it's not traced in other events).
Signed-off-by: Pierre-Eric Pelloux-Prayer
---
.../gpu/drm/scheduler/gpu_scheduler_trace.h | 40 +++
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
index 1f9c5a884487..6541e161962f 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
@@ -37,27 +37,30 @@ DECLARE_EVENT_CLASS(drm_sched_job,
TP_ARGS(sched_job, entity),
TP_STRUCT__entry(
__field(struct drm_sched_entity *, entity)
-__field(struct dma_fence *, fence)
__string(name, sched_job->sched->name)
__field(uint64_t, id)
__field(u32, job_count)
__field(int, hw_job_count)
__string(dev, dev_name(sched_job->sched->dev))
+__field(uint64_t, fence_context)
+__field(uint64_t, fence_seqno)
),
TP_fast_assign(
__entry->entity = entity;
__entry->id = sched_job->id;
- __entry->fence = &sched_job->s_fence->finished;
__assign_str(name);
__entry->job_count =
spsc_queue_count(&entity->job_queue);
__entry->hw_job_count = atomic_read(
&sched_job->sched->credit_count);
__assign_str(dev);
+ __entry->fence_context =
sched_job->s_fence->finished.context;
+ __entry->fence_seqno =
sched_job->s_fence->finished.seqno;
+
),
- TP_printk("entity=%p, id=%llu, fence=%p, ring=%s, job count:%u, hw
job count:%d",
- __entry->entity, __entry->id,
- __entry->fence, __get_str(name),
+ TP_printk("id=%llu, fence=(context:%llu, seqno:%lld), ring=%s, job
count:%u, hw job count:%d",
+ __entry->id,
+ __entry->fence_context, __entry->fence_seqno,
__get_str(name),
__entry->job_count, __entry->hw_job_count)
);
@@ -69,9 +72,9 @@ DEFINE_EVENT(drm_sched_job, drm_sched_job,
DEFINE_EVENT_PRINT(drm_sched_job, drm_run_job,
TP_PROTO(struct drm_sched_job *sched_job, struct drm_sched_entity
*entity),
TP_ARGS(sched_job, entity),
- TP_printk("dev=%s, entity=%p id=%llu, fence=%p, ring=%s, job
count:%u, hw job count:%d",
- __get_str(dev), __entry->entity, __entry->id,
- __entry->fence, __get_str(name),
+ TP_printk("dev=%s, id=%llu, fence=(context:%llu, seqno:%lld),
ring=%s, job count:%u, hw job count:%d",
+ __get_str(dev), __entry->id,
+ __entry->fence_context, __entry->fence_seqno,
__get_str(name),
__entry->job_count, __entry->hw_job_count)
);
@@ -79,13 +82,16 @@ TRACE_EVENT(drm_sched_process_job,
TP_PROTO(struct drm_sched_fence *fence),
TP_ARGS(fence),
TP_STRUCT__entry(
- __field(struct dma_fence *, fence)
+ __field(uint64_t, fence_context)
+ __field(uint64_t, fence_seqno)
),
TP_fast_assign(
- __entry->fence = &fence->finished;
+ __entry->fence_context = fence->finished.context;
+ __entry->fence_seqno = fence->finished.seqno;
),
- TP_printk("fence=%p signaled", __entry->fence)
+ TP_printk("fence=(context:%llu, seqno:%lld) signaled",
+ __entry->fence_context, __entry->fence_seqno)
);
TRACE_EVENT(drm_sched_job_wait_dep,
@@ -93,23 +99,25 @@ TRACE_EVENT(drm_sched_job_wait_dep,
TP_ARGS(sched_job, fence),
TP_STRUCT__entry(
__string(name, sched_job->sched->name)
+__field(uint64_t, fence_context)
__field(uint64_t, id)
__field(struct dma_fence *, fence)
__field(uint64_t, ctx)
-__field(unsigned, s
[PATCH v4 1/3] drm/sched: add device name to the drm_sched_process_job event
Until the switch from kthread to workqueue, a userspace application could
determine the source device from the pid of the thread sending the event.
With workqueues this is not possible anymore, so the event needs to contain
the dev_name() to identify the device.
Signed-off-by: Pierre-Eric Pelloux-Prayer
---
drivers/gpu/drm/scheduler/gpu_scheduler_trace.h | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
index c75302ca3427..1f9c5a884487 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
@@ -42,6 +42,7 @@ DECLARE_EVENT_CLASS(drm_sched_job,
__field(uint64_t, id)
__field(u32, job_count)
__field(int, hw_job_count)
+__string(dev, dev_name(sched_job->sched->dev))
),
TP_fast_assign(
@@ -52,6 +53,7 @@ DECLARE_EVENT_CLASS(drm_sched_job,
__entry->job_count =
spsc_queue_count(&entity->job_queue);
__entry->hw_job_count = atomic_read(
&sched_job->sched->credit_count);
+ __assign_str(dev);
),
TP_printk("entity=%p, id=%llu, fence=%p, ring=%s, job count:%u, hw
job count:%d",
__entry->entity, __entry->id,
@@ -64,9 +66,13 @@ DEFINE_EVENT(drm_sched_job, drm_sched_job,
TP_ARGS(sched_job, entity)
);
-DEFINE_EVENT(drm_sched_job, drm_run_job,
+DEFINE_EVENT_PRINT(drm_sched_job, drm_run_job,
TP_PROTO(struct drm_sched_job *sched_job, struct drm_sched_entity
*entity),
- TP_ARGS(sched_job, entity)
+ TP_ARGS(sched_job, entity),
+ TP_printk("dev=%s, entity=%p id=%llu, fence=%p, ring=%s, job
count:%u, hw job count:%d",
+ __get_str(dev), __entry->entity, __entry->id,
+ __entry->fence, __get_str(name),
+ __entry->job_count, __entry->hw_job_count)
);
TRACE_EVENT(drm_sched_process_job,
--
2.40.1
[PATCH v4 0/3] Improve gpu_scheduler trace events
v3: https://lists.freedesktop.org/archives/dri-devel/2024-June/456792.html Changes since v3: * trace device name instead of drm_device primary index * no pointer deref in the TP_printk anymore. Instead the fence context/seqno are saved in TP_fast_assign Pierre-Eric Pelloux-Prayer (3): drm/sched: add device name to the drm_sched_process_job event drm/sched: cleanup gpu_scheduler trace events drm/sched: trace dependencies for gpu jobs .../gpu/drm/scheduler/gpu_scheduler_trace.h | 97 +++ drivers/gpu/drm/scheduler/sched_entity.c | 8 +- drivers/gpu/drm/scheduler/sched_main.c| 2 +- 3 files changed, 84 insertions(+), 23 deletions(-) -- 2.40.1
Re: [RFC PATCH 7/8] rust: add firmware abstractions
On Sat, Jun 08, 2024 at 08:28:06AM +0900, FUJITA Tomonori wrote: > On Fri, 7 Jun 2024 19:55:49 +0200 > Danilo Krummrich wrote: > > > On Fri, Jun 07, 2024 at 05:41:11PM +0200, Greg KH wrote: > >> On Fri, Jun 07, 2024 at 03:33:39PM +0200, Danilo Krummrich wrote: > >> > On Fri, Jun 07, 2024 at 02:36:50PM +0200, Greg KH wrote: > >> > > Anyway, that's all hand-wavy right now, sorry, to get back to the point > >> > > here, again, let's take this, which will allow the firmware bindings to > >> > > be resubmitted and hopefully accepted, and we can move forward from > >> > > there to "real" things like a USB or PCI or even platform device and > >> > > driver binding stuff. > >> > > >> > In order to continue I propose to send out the following series: > >> > > >> > 1) minimal device and firmware abstractions only > >> > >> Sounds good. > > > > Just a heads-up, I'll probably send this one quite a bit earlier than the > > other > > two to make sure to unblock Fujita on their PHY driver. > > Please. The sooner, the better. I need to send the PHY driver with > these patchse to netdev. Why do you want to send those patches to netdev? I think nothing prevents you from sending your PHY driver to netdev. Just add a note to your series that it depends on those two patches. How and through which trees things are merged in the end can be figured out by the corresponding maintainers in the end. > > I'm not sure what the above "minimal device" means. If you send the > original patch again instead of the patch that Greg already approved > and the discussion continues, then I proceed with the approved patch. > I'm honestly getting a bit tired of this... 1) I fundamentally disagree that it's a good thing to fork off patches that are actively discussed and reviewed on the mailing list with the objective to bypass the discussion and the review process. Especially without agreement of all involved parties. 2) It's at least questionable to claim that your forked-off patch can be considered to be "approved". 3) I really try to help and already confirmed to send out a separate series with only the patches you need as well to accelerate things for you. If you really want to help with that, you are very welcome to get involved in the discussion and review process. If you don't want to, that is fine too. But please stop adding confusion to those series by forking off patches. Besides that, I also don't appreciate your attitude, trying to put some kind of "ultimatum" on me. - Danilo
Re: [PATCH] drm/i915/gt: Delete the live_hearbeat_fast selftest
Hi Andi, On 10/06/2024 13:10, Andi Shyti wrote: Hi Tvrtko, On Mon, Jun 10, 2024 at 12:42:31PM +0100, Tvrtko Ursulin wrote: On 03/06/2024 17:20, Niemiec, Krzysztof wrote: The test is trying to push the heartbeat frequency to the limit, which might sometimes fail. Such a failure does not provide valuable information, because it does not indicate that there is something necessarily wrong with either the driver or the hardware. Remove the test to prevent random, unnecessary failures from appearing in CI. Suggested-by: Chris Wilson Signed-off-by: Niemiec, Krzysztof Just a note in passing that comma in the email display name is I believe not RFC 5322 compliant and there might be tools which barf on it(*). If you can put it in double quotes, it would be advisable. yes, we discussed it with Krzysztof, I noticed it right after I submitted the code. Regards, Tvrtko *) Such as my internal pull request generator which uses CPAN's Email::Address::XS. :) If we are in time, we can fix it as Krzysztof Niemiec Sorry about this oversight, It's not a big deal (it isn't the first and only occurence) and no need to do anything more than correct the display name going forward. Regards, Tvrtko
Re: [PATCH 0/2] drm bridge: drop drm_bridge_chain_mode_fixup.
On Fri, 31 May 2024 22:37:44 +0200, Sam Ravnborg wrote: > I had a few bridge related patches in an old branch. > > They were last posted here almost one year ago: > https://lore.kernel.org/dri-devel/20220717174454.46616-1-...@ravnborg.org/ > > The following two patches gets rid of drm_bridge_chain_mode_fixup. > The patches was already rb / ab - but due to the age a repost is > due before applying the patches. > > [...] Applied, thanks! [1/2] drm/mediatek: Drop chain_mode_fixup call in mode_valid() https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ac4be1e50165 [2/2] drm/bridge: Drop drm_bridge_chain_mode_fixup https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1f0204954583 Rob
Re: [PATCH v2 0/3] Move blender setup from individual planes to crtc commit in sun4i-drm
On Mon, Jun 10, 2024 at 12:46:11PM GMT, Maxime Ripard wrote: > On Sat, 24 Feb 2024 16:05:57 +0100, Ondřej Jirman wrote: > > From: Ondrej Jirman > > > > This series refactors blender setup from individual planes to a common > > place where it can be performed at once and is easier to reason about. > > > > In the process this fixes a few bugs that allowed blender pipes to be > > disabled while corresponding DRM planes were requested to be enabled. > > > > [...] > > Applied to misc/kernel.git (drm-misc-next). Thank you. :) Kind regards, o. > Thanks! > Maxime >
Re: [PATCH 00/14] improve Analogix DP AUX channel handling
Am Freitag, 3. Mai 2024, 17:11:15 CEST schrieb Lucas Stach: > Currently the AUX channel support in the Analogix DP driver is severely > limited as the AUX block of the bridge is only initialized when the video > link is to be enabled. This is okay for the purposes of link training, > but does not allow to detect displays by probing for EDID. This series > reworks the driver to allow AUX transactions before the video link is > active. > > As this requires to rework some of the controller initialization and > also handling of both internal and external clocks, the series includes > quite a few changes to add better runtime PM handling. > > Lucas Stach (14): > drm/bridge: analogix_dp: remove unused platform power_on_end callback > drm/rockchip: analogix_dp: add runtime PM handling > drm/bridge: analogix_dp: register AUX bus after enabling runtime PM > drm/bridge: analogix_dp: handle clock via runtime PM > drm/bridge: analogix_dp: remove unused analogix_dp_remove > drm/bridge: analogix_dp: remove clk handling from > analogix_dp_set_bridge > drm/bridge: analogix_dp: move platform and PHY power handling into > runtime PM > drm/bridge: analogix_dp: move basic controller init into runtime PM > drm/bridge: analogix_dp: remove PLL lock check from > analogix_dp_config_video > drm/bridge: analogix_dp: move macro reset after link bandwidth setting > drm/bridge: analogix_dp: don't wait for PLL lock too early > drm/bridge: analogix_dp: simplify and correct PLL lock checks > drm/bridge: analogix_dp: only read AUX status when an error occured > drm/bridge: analogix_dp: handle AUX transfer timeouts my knowledge of Analgix-dp internals is limited, but at least both rk3288-veyron and rk3399 gru still had working displays with this series applied and both device classes using the analogix-dp for their main display. So on rk3288-veyron and rk3399-gru Tested-by: Heiko Stuebner
Re: [PATCH 02/14] drm/rockchip: analogix_dp: add runtime PM handling
Am Freitag, 3. Mai 2024, 17:11:17 CEST schrieb Lucas Stach: > Hook up the runtime PM suspend/resume paths to make the rockchip > glue behave more like the exynos one. The same suspend/resume > functions are used for system sleep via the runtime PM force > suspend/resume. > > Signed-off-by: Lucas Stach Reviewed-by: Heiko Stuebner A rk3399-gru Chromebook was able to suspend and wake up again with working display both before and after. Heiko
[syzbot] Monthly dri report (Jun 2024)
Hello dri maintainers/developers, This is a 31-day syzbot report for the dri subsystem. All related reports/information can be found at: https://syzkaller.appspot.com/upstream/s/dri During the period, 2 new issues were detected and 0 were fixed. In total, 18 issues are still open and 31 have been fixed so far. Some of the still happening issues: Ref Crashes Repro Title <1> 462 Yes WARNING in drm_syncobj_array_find https://syzkaller.appspot.com/bug?extid=95416f957d84e858b377 <2> 335 Yes inconsistent lock state in sync_timeline_debug_remove https://syzkaller.appspot.com/bug?extid=7dcd254b8987a29f6450 <3> 277 Yes inconsistent lock state in sync_info_debugfs_show https://syzkaller.appspot.com/bug?extid=007bfe0f3330f6e1e7d1 <4> 265 Yes WARNING in vkms_get_vblank_timestamp (2) https://syzkaller.appspot.com/bug?extid=93bd128a383695391534 <5> 17 Yes WARNING in drm_gem_prime_fd_to_handle https://syzkaller.appspot.com/bug?extid=268d319a7bfd92f4ae01 <6> 10 Yes divide error in drm_mode_vrefresh https://syzkaller.appspot.com/bug?extid=622bba18029bcde672e1 <7> 9 Yes general protection fault in udmabuf_create (2) https://syzkaller.appspot.com/bug?extid=40c7dad27267f61839d4 <8> 6 NoWARNING in drm_atomic_helper_wait_for_vblanks (3) https://syzkaller.appspot.com/bug?extid=0ac28002caff799b9e57 <9> 3 Yes divide error in drm_mode_convert_to_umode https://syzkaller.appspot.com/bug?extid=0d7a3627fb6a42cf0863 --- This report is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. To disable reminders for individual bugs, reply with the following command: #syz set no-reminders To change bug's subsystems, reply with: #syz set subsystems: new-subsystem You may send multiple commands in a single email message.
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
Am 10.06.24 um 14:16 schrieb Jason Gunthorpe: On Mon, Jun 10, 2024 at 02:07:01AM +0100, Pavel Begunkov wrote: On 6/10/24 01:37, David Wei wrote: On 2024-06-07 17:52, Jason Gunthorpe wrote: IMHO it seems to compose poorly if you can only use the io_uring lifecycle model with io_uring registered memory, and not with DMABUF memory registered through Mina's mechanism. By this, do you mean io_uring must be exclusively used to use this feature? And you'd rather see the two decoupled, so userspace can register w/ say dmabuf then pass it to io_uring? Personally, I have no clue what Jason means. You can just as well say that it's poorly composable that write(2) to a disk cannot post a completion into a XDP ring, or a netlink socket, or io_uring's main completion queue, or name any other API. There is no reason you shouldn't be able to use your fast io_uring completion and lifecycle flow with DMABUF backed memory. Those are not widly different things and there is good reason they should work together. Well there is the fundamental problem that you can't use io_uring to implement the semantics necessary for a dma_fence. That's why we had to reject the io_uring work on DMA-buf sharing from Google a few years ago. But this only affects the dma_fence synchronization part of DMA-buf, but *not* the general buffer sharing. Regards, Christian. Pretending they are totally different just because two different people wrote them is a very siloed view. The devmem TCP callback can implement it in a way feasible to the project, but it cannot directly post events to an unrelated API like io_uring. And devmem attaches buffers to a socket, for which a ring for returning buffers might even be a nuisance. If you can't compose your io_uring completion mechanism with a DMABUF provided backing store then I think it needs more work. Jason
Re: [PATCH v2 7/9] drm/bridge: cdns-dsi: Support atomic bridge APIs
Hi Dmitry,
Thank you for reviewing the patches.
On 31/05/24 04:51, Dmitry Baryshkov wrote:
> On Thu, May 30, 2024 at 03:06:19PM +0530, Aradhya Bhatia wrote:
>> Change the existing (and deprecated) bridge hooks, to the bridge
>> atomic APIs.
>>
>> Add drm helpers for duplicate_state, destroy_state, and bridge_reset
>> bridge hooks.
>>
>> Further add support for the input format negotiation hook.
>>
>> Signed-off-by: Aradhya Bhatia
>> ---
>> .../gpu/drm/bridge/cadence/cdns-dsi-core.c| 70 ---
>> 1 file changed, 62 insertions(+), 8 deletions(-)
>
> Reviewed-by: Dmitry Baryshkov
>
> Minor nit below.
>
>>
>> @@ -915,13 +920,62 @@ static void cdns_dsi_bridge_pre_enable(struct
>> drm_bridge *bridge)
>> cdns_dsi_hs_init(dsi);
>> }
>>
>> +static u32 *cdns_dsi_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
>> + struct drm_bridge_state
>> *bridge_state,
>> + struct drm_crtc_state
>> *crtc_state,
>> + struct drm_connector_state
>> *conn_state,
>> + u32 output_fmt,
>> + unsigned int *num_input_fmts)
>> +{
>
> This code below looks pretty generic. Would be logical to extract it to
> a helper and allow it to be used by other DSI host bridges?
I agree, it would indeed make sense.
I just tried to make a helper function that could directly be passed to
the drm_bridge_funcs list - like we do with
"drm_atomic_helper_bridge_propagate_bus_fmt". This would have been ideal
in my opinion.
But, that doesn't seem possible today. The parameter "output_fmt"
doesn't describe any of the DSI pixel formats from "enum
mipi_dsi_pixel_format", which is what's required to ascertain the input
bus format for dsi hosts. Neither do the drm_bridge{_state} help with
that.
For now, I am thinking to just add a common function that accepts the
dsi bus output format and returns the corresponding input bus format.
With this, every dsi host will still need to implement their own
get_input_fmt hook and call that function.
If you had something else in mind, do let me know! =)
Regards
Aradhya
>
>> +struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
>> +struct cdns_dsi *dsi = input_to_dsi(input);
>> +struct cdns_dsi_output *output = &dsi->output;
>> +u32 *input_fmts;
>> +
>> +*num_input_fmts = 0;
>> +
>> +input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
>> +if (!input_fmts)
>> +return NULL;
>> +
>> +switch (output->dev->format) {
>> +case MIPI_DSI_FMT_RGB888:
>> +input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>> +break;
>> +
>> +case MIPI_DSI_FMT_RGB666:
>> +input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
>> +break;
>> +
>> +case MIPI_DSI_FMT_RGB666_PACKED:
>> +input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X18;
>> +break;
>> +
>> +case MIPI_DSI_FMT_RGB565:
>> +input_fmts[0] = MEDIA_BUS_FMT_RGB565_1X16;
>> +break;
>> +
>> +default:
>> +/* Unsupported DSI Format */
>> +return NULL;
>> +}
>> +
>> +*num_input_fmts = 1;
>> +
>> +return input_fmts;
>> +}
>> +
>
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On Mon, Jun 10, 2024 at 02:07:01AM +0100, Pavel Begunkov wrote: > On 6/10/24 01:37, David Wei wrote: > > On 2024-06-07 17:52, Jason Gunthorpe wrote: > > > IMHO it seems to compose poorly if you can only use the io_uring > > > lifecycle model with io_uring registered memory, and not with DMABUF > > > memory registered through Mina's mechanism. > > > > By this, do you mean io_uring must be exclusively used to use this > > feature? > > > > And you'd rather see the two decoupled, so userspace can register w/ say > > dmabuf then pass it to io_uring? > > Personally, I have no clue what Jason means. You can just as > well say that it's poorly composable that write(2) to a disk > cannot post a completion into a XDP ring, or a netlink socket, > or io_uring's main completion queue, or name any other API. There is no reason you shouldn't be able to use your fast io_uring completion and lifecycle flow with DMABUF backed memory. Those are not widly different things and there is good reason they should work together. Pretending they are totally different just because two different people wrote them is a very siloed view. > The devmem TCP callback can implement it in a way feasible to > the project, but it cannot directly post events to an unrelated > API like io_uring. And devmem attaches buffers to a socket, > for which a ring for returning buffers might even be a nuisance. If you can't compose your io_uring completion mechanism with a DMABUF provided backing store then I think it needs more work. Jason
Re: [PATCH v2 3/3] drm/panic: Add a kmsg panic screen
On 07/06/2024 11:16, Javier Martinez Canillas wrote:
Jocelyn Falempe writes:
Add a kmsg option, which will display the last lines of kmsg,
and should be similar to fbcon.
Add a drm.panic_screen module parameter, so you can choose between
the different panic screens available.
two options currently, but more will be added later:
* "user": a short message telling the user to reboot the machine.
* "kmsg": fill the screen with the last lines of kmsg.
You can even change it at runtime by writing to
/sys/module/drm/parameters/panic_screen
Great!
v2:
* use module parameter instead of Kconfig choice
(Javier Martinez Canillas)
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/Kconfig | 11
drivers/gpu/drm/drm_panic.c | 108
2 files changed, 109 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 9703429de6b9..944815cee080 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -137,6 +137,17 @@ config DRM_PANIC_DEBUG
This is unsafe and should not be enabled on a production build.
If in doubt, say "N".
+config DRM_PANIC_SCREEN
+ string "Panic screen formater"
+ default "user"
+ depends on DRM_PANIC
+ help
+ This option enable to choose what will be displayed when a kernel
+ panic occurs. You can choose between "user", a short message telling
+ the user to reboot the system, or "kmsg" which will display the last
+ lines of kmsg.
Maybe I would mention here that this is only about the default, but that
can be changed using the "drm.panic_screen=" kernel cmdline parameter or
writting to the /sys/module/drm/parameters/panic_screen sysfs entry.
[...]
Done
+static void draw_panic_static_kmsg(struct drm_scanout_buffer *sb)
+{
+ u32 fg_color = convert_from_xrgb(CONFIG_DRM_PANIC_FOREGROUND_COLOR,
sb->format->format);
+ u32 bg_color = convert_from_xrgb(CONFIG_DRM_PANIC_BACKGROUND_COLOR,
sb->format->format);
+ const struct font_desc *font = get_default_font(sb->width, sb->height,
NULL, NULL);
Dan reported that get_default_font() can return NULL
+ struct drm_rect r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
+ struct kmsg_dump_iter iter;
+ char kmsg_buf[512];
+ size_t kmsg_len;
+ struct drm_panic_line line;
+ int yoffset = sb->height - font->height - (sb->height % font->height) /
2;
+
+ if (!font)
+ return;
+
... so you have to calculate yoffset after checking if the font is not NULL.
Yes I fixed that too.
with that fixed:
Reviewed-by: Javier Martinez Canillas
Thanks a lot.
I just pushed this series to drm-misc-next.
Best regards,
--
Jocelyn
Re: [PATCH] drm/i915/gt: Delete the live_hearbeat_fast selftest
Hi Tvrtko, On Mon, Jun 10, 2024 at 12:42:31PM +0100, Tvrtko Ursulin wrote: > On 03/06/2024 17:20, Niemiec, Krzysztof wrote: > > The test is trying to push the heartbeat frequency to the limit, which > > might sometimes fail. Such a failure does not provide valuable > > information, because it does not indicate that there is something > > necessarily wrong with either the driver or the hardware. > > > > Remove the test to prevent random, unnecessary failures from appearing > > in CI. > > > > Suggested-by: Chris Wilson > > Signed-off-by: Niemiec, Krzysztof > > Just a note in passing that comma in the email display name is I believe not > RFC 5322 compliant and there might be tools which barf on it(*). If you can > put it in double quotes, it would be advisable. yes, we discussed it with Krzysztof, I noticed it right after I submitted the code. > Regards, > > Tvrtko > > *) Such as my internal pull request generator which uses CPAN's > Email::Address::XS. :) If we are in time, we can fix it as Krzysztof Niemiec Sorry about this oversight, Andi
Re: [PATCH v5 2/9] drm/bridge-connector: switch to using drmm allocations
Hi,
+Hans
On Mon, Jun 10, 2024 at 02:46:03PM GMT, Dmitry Baryshkov wrote:
> On Mon, 10 Jun 2024 at 11:04, Maxime Ripard wrote:
> >
> > Hi,
> >
> > On Fri, Jun 07, 2024 at 04:22:59PM GMT, Dmitry Baryshkov wrote:
> > > Turn drm_bridge_connector to using drmm_kzalloc() and
> > > drmm_connector_init() and drop the custom destroy function. The
> > > drm_connector_unregister() and fwnode_handle_put() are already handled
> > > by the drm_connector_cleanup() and so are safe to be dropped.
> > >
> > > Acked-by: Maxime Ripard
> > > Signed-off-by: Dmitry Baryshkov
> > > ---
> > > drivers/gpu/drm/drm_bridge_connector.c | 23 +--
> > > 1 file changed, 5 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c
> > > b/drivers/gpu/drm/drm_bridge_connector.c
> > > index 982552c9f92c..e093fc8928dc 100644
> > > --- a/drivers/gpu/drm/drm_bridge_connector.c
> > > +++ b/drivers/gpu/drm/drm_bridge_connector.c
> > > @@ -15,6 +15,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > >
> > > @@ -193,19 +194,6 @@ drm_bridge_connector_detect(struct drm_connector
> > > *connector, bool force)
> > > return status;
> > > }
> > >
> > > -static void drm_bridge_connector_destroy(struct drm_connector *connector)
> > > -{
> > > - struct drm_bridge_connector *bridge_connector =
> > > - to_drm_bridge_connector(connector);
> > > -
> > > - drm_connector_unregister(connector);
> > > - drm_connector_cleanup(connector);
> > > -
> > > - fwnode_handle_put(connector->fwnode);
> > > -
> > > - kfree(bridge_connector);
> > > -}
> > > -
> > > static void drm_bridge_connector_debugfs_init(struct drm_connector
> > > *connector,
> > > struct dentry *root)
> > > {
> > > @@ -224,7 +212,6 @@ static const struct drm_connector_funcs
> > > drm_bridge_connector_funcs = {
> > > .reset = drm_atomic_helper_connector_reset,
> > > .detect = drm_bridge_connector_detect,
> > > .fill_modes = drm_helper_probe_single_connector_modes,
> > > - .destroy = drm_bridge_connector_destroy,
> > > .atomic_duplicate_state =
> > > drm_atomic_helper_connector_duplicate_state,
> > > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > > .debugfs_init = drm_bridge_connector_debugfs_init,
> > > @@ -328,7 +315,7 @@ struct drm_connector
> > > *drm_bridge_connector_init(struct drm_device *drm,
> > > int connector_type;
> > > int ret;
> > >
> > > - bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL);
> > > + bridge_connector = drmm_kzalloc(drm, sizeof(*bridge_connector),
> > > GFP_KERNEL);
> >
> > So you make destroy's kfree call unnecessary here ...
> >
> > > if (!bridge_connector)
> > > return ERR_PTR(-ENOMEM);
> > >
> > > @@ -383,9 +370,9 @@ struct drm_connector
> > > *drm_bridge_connector_init(struct drm_device *drm,
> > > return ERR_PTR(-EINVAL);
> > > }
> > >
> > > - ret = drm_connector_init_with_ddc(drm, connector,
> > > - &drm_bridge_connector_funcs,
> > > - connector_type, ddc);
> > > + ret = drmm_connector_init(drm, connector,
> > > + &drm_bridge_connector_funcs,
> > > + connector_type, ddc);
> >
> > ... and here of drm_connector_cleanup.
> >
> > drm_connector_unregister wasn't needed, so can ignore it, but you leak a
> > reference to
> > connector->fwnode since you don't call fwnode_handle_put anymore.
> >
> > We should register a drmm action right below the call to fwnode_handle_get
> > too.
>
> But drm_connector_cleanup() already contains
> fwnode_handle_put(connector->fwnode). Isn't that enough?
It does, but now I'm confused.
drm_bridge_connector_init takes a reference, drm_connector_init doesn't.
It will call drm_bridge_connector_destroy() that gives back its
reference (which makes sense to me), but then why do
drm_connector_cleanup() does? None of the drm_connector code even took
that reference, and we end up with a double-put.
It looks like it was introduced by commit 48c429c6d18d ("drm/connector:
Add a fwnode pointer to drm_connector and register with ACPI (v2)") from
Hans, which does call put, but never gets that reference.
It has nothing to do with this series anymore, but that's super fishy to
me, and the source of bugs as we can see here.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH] drm/edid: reduce DisplayID log spamming
Am 06.06.24 um 14:35 schrieb Jani Nikula:
Debug printing at DisplayID validation leads to lots of log spamming as
it's called at DisplayID iterators during EDID parsing. Remove it, and
replace with a less noisy message at connector EDID update.
Signed-off-by: Jani Nikula
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_displayid.c | 3 ---
drivers/gpu/drm/drm_edid.c | 5 +
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_displayid.c b/drivers/gpu/drm/drm_displayid.c
index 9d01d762801f..b4fd43783c50 100644
--- a/drivers/gpu/drm/drm_displayid.c
+++ b/drivers/gpu/drm/drm_displayid.c
@@ -33,9 +33,6 @@ validate_displayid(const u8 *displayid, int length, int idx)
if (IS_ERR(base))
return base;
- DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
- base->rev, base->bytes, base->prod_id, base->ext_count);
-
/* +1 for DispID checksum */
dispid_length = sizeof(*base) + base->bytes + 1;
if (dispid_length > length - idx)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f68a41eeb1fa..9fc7292f5382 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6629,6 +6629,11 @@ static void update_displayid_info(struct drm_connector
*connector,
displayid_iter_edid_begin(drm_edid, &iter);
displayid_iter_for_each(block, &iter) {
+ drm_dbg_kms(connector->dev,
+ "[CONNECTOR:%d:%s] DisplayID extension version 0x%02x,
primary use 0x%02x\n",
+ connector->base.id, connector->name,
+ displayid_version(&iter),
+ displayid_primary_use(&iter));
if (displayid_version(&iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
(displayid_primary_use(&iter) ==
PRIMARY_USE_HEAD_MOUNTED_VR ||
displayid_primary_use(&iter) ==
PRIMARY_USE_HEAD_MOUNTED_AR))
--
--
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 14/14] drm/bridge: analogix_dp: handle AUX transfer timeouts
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote: > > Timeouts on the AUX bus are to be expected in certain normal operating > conditions. There is no need to raise an error log or re-initialize the > whole AUX state machine. Simply acknowledge the AUX_ERR interrupt and > let upper layers know about the timeout. > > Signed-off-by: Lucas Stach > --- > drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 3 +++ > drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 9 + > 2 files changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > index 0f016dca9f3d..3afc73c858c4 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > @@ -1016,6 +1016,9 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device > *dp, > > writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA); > > + if (aux_status == AUX_STATUS_TIMEOUT_ERROR) > + return -ETIMEDOUT; > + > dev_warn(dp->dev, "AUX CH error happened: %#x (%d)\n", > aux_status, !!(reg & AUX_ERR)); > goto aux_error; > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h > b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h > index e284ee8da58b..12735139046c 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h > @@ -361,6 +361,15 @@ > /* ANALOGIX_DP_AUX_CH_STA */ > #define AUX_BUSY (0x1 << 4) > #define AUX_STATUS_MASK(0xf << 0) > +#define AUX_STATUS_OK (0x0 << 0) > +#define AUX_STATUS_NACK_ERROR (0x1 << 0) > +#define AUX_STATUS_TIMEOUT_ERROR (0x2 << 0) > +#define AUX_STATUS_UNKNOWN_ERROR (0x3 << 0) > +#define AUX_STATUS_MUCH_DEFER_ERROR(0x4 << 0) > +#define AUX_STATUS_TX_SHORT_ERROR (0x5 << 0) > +#define AUX_STATUS_RX_SHORT_ERROR (0x6 << 0) > +#define AUX_STATUS_NACK_WITHOUT_M_ERROR(0x7 << 0) > +#define AUX_STATUS_I2C_NACK_ERROR (0x8 << 0) > > /* ANALOGIX_DP_AUX_CH_DEFER_CTL */ > #define DEFER_CTRL_EN (0x1 << 7) > -- > 2.39.2 > Reviewed-by: Robert Foss This series has a few checkpath --strict warnings, could you fix those and I'll merge v2? Rob.
Re: [PATCH 13/14] drm/bridge: analogix_dp: only read AUX status when an error occured
On Fri, May 3, 2024 at 5:13 PM Lucas Stach wrote:
>
> All AUX error responses raise the AUX_ERR interrupt, so there is no
> need to read the AUX status register in normal operation. Only read
> the status when an error occured and we can expect a different status
> than OK.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 143a78b1d156..0f016dca9f3d 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -924,7 +924,6 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device
> *dp,
> struct drm_dp_aux_msg *msg)
> {
> u32 reg;
> - u32 status_reg;
> u8 *buffer = msg->buffer;
> unsigned int i;
> int ret;
> @@ -1011,12 +1010,14 @@ ssize_t analogix_dp_transfer(struct
> analogix_dp_device *dp,
>
> /* Clear interrupt source for AUX CH access error */
> reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> - status_reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA);
> - if ((reg & AUX_ERR) || (status_reg & AUX_STATUS_MASK)) {
> + if ((reg & AUX_ERR)) {
> + u32 aux_status = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA)
> &
> +AUX_STATUS_MASK;
> +
> writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA);
>
> dev_warn(dp->dev, "AUX CH error happened: %#x (%d)\n",
> -status_reg & AUX_STATUS_MASK, !!(reg & AUX_ERR));
> +aux_status, !!(reg & AUX_ERR));
> goto aux_error;
> }
>
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH 12/14] drm/bridge: analogix_dp: simplify and correct PLL lock checks
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote:
>
> Move the wait loop into its own function, so it doesn't need to be
> replicated in multiple locations. Also move the PLL lock checks between
> setting the link bandwidth, which may cause the PLL to unlock, and the
> MACRO_RST, which needs the PLL to be locked.
>
> Signed-off-by: Lucas Stach
> ---
> .../drm/bridge/analogix/analogix_dp_core.c| 34 +++
> .../drm/bridge/analogix/analogix_dp_core.h| 7 +---
> .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 12 +++
> 3 files changed, 18 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 736b2ed745e1..7bbc3d8a85df 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -231,7 +231,7 @@ static int analogix_dp_training_pattern_dis(struct
> analogix_dp_device *dp)
> static int analogix_dp_link_start(struct analogix_dp_device *dp)
> {
> u8 buf[4];
> - int lane, lane_count, pll_tries, retval;
> + int lane, lane_count, retval;
>
> lane_count = dp->link_train.lane_count;
>
> @@ -243,6 +243,11 @@ static int analogix_dp_link_start(struct
> analogix_dp_device *dp)
>
> /* Set link rate and count as you want to establish*/
> analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
> + retval = analogix_dp_wait_pll_locked(dp);
> + if (retval) {
> + DRM_DEV_ERROR(dp->dev, "Wait for pll lock failed %d\n",
> retval);
> + return retval;
> + }
> /*
> * MACRO_RST must be applied after the PLL_LOCK to avoid
> * the DP inter pair skew issue for at least 10 us
> @@ -270,18 +275,6 @@ static int analogix_dp_link_start(struct
> analogix_dp_device *dp)
> DP_TRAIN_PRE_EMPH_LEVEL_0;
> analogix_dp_set_lane_link_training(dp);
>
> - /* Wait for PLL lock */
> - pll_tries = 0;
> - while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
> - if (pll_tries == DP_TIMEOUT_LOOP_COUNT) {
> - dev_err(dp->dev, "Wait for PLL lock timed out\n");
> - return -ETIMEDOUT;
> - }
> -
> - pll_tries++;
> - usleep_range(90, 120);
> - }
> -
> /* Set training pattern 1 */
> analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
>
> @@ -634,9 +627,14 @@ static int analogix_dp_fast_link_train(struct
> analogix_dp_device *dp)
> {
> int ret;
> u8 link_align, link_status[2];
> - enum pll_status status;
>
> analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
> + ret = analogix_dp_wait_pll_locked(dp);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev, "Wait for pll lock failed %d\n", ret);
> + return ret;
> + }
> +
> /*
> * MACRO_RST must be applied after the PLL_LOCK to avoid
> * the DP inter pair skew issue for at least 10 us
> @@ -645,14 +643,6 @@ static int analogix_dp_fast_link_train(struct
> analogix_dp_device *dp)
> analogix_dp_set_lane_count(dp, dp->link_train.lane_count);
> analogix_dp_set_lane_link_training(dp);
>
> - ret = readx_poll_timeout(analogix_dp_get_pll_lock_status, dp, status,
> -status != PLL_UNLOCKED, 120,
> -120 * DP_TIMEOUT_LOOP_COUNT);
> - if (ret) {
> - DRM_DEV_ERROR(dp->dev, "Wait for pll lock failed %d\n", ret);
> - return ret;
> - }
> -
> /* source Set training pattern 1 */
> analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
> /* From DP spec, pattern must be on-screen for a minimum 500us */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 382b2f068ab9..774d11574b09 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -95,11 +95,6 @@ enum dynamic_range {
> CEA
> };
>
> -enum pll_status {
> - PLL_UNLOCKED,
> - PLL_LOCKED
> -};
> -
> enum clock_recovery_m_value_type {
> CALCULATED_M,
> REGISTER_M
> @@ -191,7 +186,7 @@ void analogix_dp_swreset(struct analogix_dp_device *dp);
> void analogix_dp_config_interrupt(struct analogix_dp_device *dp);
> void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp);
> void analogix_dp_unmute_hpd_interrupt(struct analogix_dp_device *dp);
> -enum pll_status analogix_dp_get_pll_lock_status(struct analogix_dp_device
> *dp);
> +int analogix_dp_wait_pll_locked(struct analogix_dp_device *dp);
> void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool
> enable);
> void analogix_dp_set_analog_power_down(struct analo
Re: [PATCH 11/14] drm/bridge: analogix_dp: don't wait for PLL lock too early
On Fri, May 3, 2024 at 5:12 PM Lucas Stach wrote:
>
> The PLL will be reconfigured later, which may cause it to go out of lock
> anyways, so there is no point in waiting for the PLL to lock here. Instead
> we can continue execution of the link setup, which will properly set the
> PLL parameters and will wait for the PLL to lock at the appropriate times.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 14 +-
> 1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index d267cf05cbca..e9c643a8b6fc 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -356,7 +356,6 @@ void analogix_dp_set_analog_power_down(struct
> analogix_dp_device *dp,
> int analogix_dp_init_analog_func(struct analogix_dp_device *dp)
> {
> u32 reg;
> - int timeout_loop = 0;
>
> analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
>
> @@ -368,18 +367,7 @@ int analogix_dp_init_analog_func(struct
> analogix_dp_device *dp)
> writel(reg, dp->reg_base + ANALOGIX_DP_DEBUG_CTL);
>
> /* Power up PLL */
> - if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
> - analogix_dp_set_pll_power_down(dp, 0);
> -
> - while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
> - timeout_loop++;
> - if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> - dev_err(dp->dev, "failed to get pll lock
> status\n");
> - return -ETIMEDOUT;
> - }
> - usleep_range(10, 20);
> - }
> - }
> + analogix_dp_set_pll_power_down(dp, 0);
>
> /* Enable Serdes FIFO function and Link symbol clock domain module */
> reg = readl(dp->reg_base + ANALOGIX_DP_FUNC_EN_2);
> --
> 2.39.2
>
Reviewed-by: Robert Foss
Re: [PATCH v5 2/9] drm/bridge-connector: switch to using drmm allocations
On Mon, 10 Jun 2024 at 11:04, Maxime Ripard wrote:
>
> Hi,
>
> On Fri, Jun 07, 2024 at 04:22:59PM GMT, Dmitry Baryshkov wrote:
> > Turn drm_bridge_connector to using drmm_kzalloc() and
> > drmm_connector_init() and drop the custom destroy function. The
> > drm_connector_unregister() and fwnode_handle_put() are already handled
> > by the drm_connector_cleanup() and so are safe to be dropped.
> >
> > Acked-by: Maxime Ripard
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/drm_bridge_connector.c | 23 +--
> > 1 file changed, 5 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_bridge_connector.c
> > b/drivers/gpu/drm/drm_bridge_connector.c
> > index 982552c9f92c..e093fc8928dc 100644
> > --- a/drivers/gpu/drm/drm_bridge_connector.c
> > +++ b/drivers/gpu/drm/drm_bridge_connector.c
> > @@ -15,6 +15,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -193,19 +194,6 @@ drm_bridge_connector_detect(struct drm_connector
> > *connector, bool force)
> > return status;
> > }
> >
> > -static void drm_bridge_connector_destroy(struct drm_connector *connector)
> > -{
> > - struct drm_bridge_connector *bridge_connector =
> > - to_drm_bridge_connector(connector);
> > -
> > - drm_connector_unregister(connector);
> > - drm_connector_cleanup(connector);
> > -
> > - fwnode_handle_put(connector->fwnode);
> > -
> > - kfree(bridge_connector);
> > -}
> > -
> > static void drm_bridge_connector_debugfs_init(struct drm_connector
> > *connector,
> > struct dentry *root)
> > {
> > @@ -224,7 +212,6 @@ static const struct drm_connector_funcs
> > drm_bridge_connector_funcs = {
> > .reset = drm_atomic_helper_connector_reset,
> > .detect = drm_bridge_connector_detect,
> > .fill_modes = drm_helper_probe_single_connector_modes,
> > - .destroy = drm_bridge_connector_destroy,
> > .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > .debugfs_init = drm_bridge_connector_debugfs_init,
> > @@ -328,7 +315,7 @@ struct drm_connector *drm_bridge_connector_init(struct
> > drm_device *drm,
> > int connector_type;
> > int ret;
> >
> > - bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL);
> > + bridge_connector = drmm_kzalloc(drm, sizeof(*bridge_connector),
> > GFP_KERNEL);
>
> So you make destroy's kfree call unnecessary here ...
>
> > if (!bridge_connector)
> > return ERR_PTR(-ENOMEM);
> >
> > @@ -383,9 +370,9 @@ struct drm_connector *drm_bridge_connector_init(struct
> > drm_device *drm,
> > return ERR_PTR(-EINVAL);
> > }
> >
> > - ret = drm_connector_init_with_ddc(drm, connector,
> > - &drm_bridge_connector_funcs,
> > - connector_type, ddc);
> > + ret = drmm_connector_init(drm, connector,
> > + &drm_bridge_connector_funcs,
> > + connector_type, ddc);
>
> ... and here of drm_connector_cleanup.
>
> drm_connector_unregister wasn't needed, so can ignore it, but you leak a
> reference to
> connector->fwnode since you don't call fwnode_handle_put anymore.
>
> We should register a drmm action right below the call to fwnode_handle_get
> too.
But drm_connector_cleanup() already contains
fwnode_handle_put(connector->fwnode). Isn't that enough?
--
With best wishes
Dmitry
Re: [PATCH 10/14] drm/bridge: analogix_dp: move macro reset after link bandwidth setting
On Tue, May 7, 2024 at 3:07 PM Robert Foss wrote: > > On Fri, May 3, 2024 at 5:13 PM Lucas Stach wrote: > > > > Setting the link bandwidth may change the PLL parameters, which will cause > > the PLL to go out of lock, so make sure to apply the MACRO_RST, which > > according to the comment is required to be pulsed after the PLL is locked. > > > > Signed-off-by: Lucas Stach > > --- > > .../gpu/drm/bridge/analogix/analogix_dp_core.c | 18 ++ > > 1 file changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > index b4a47311cfe8..736b2ed745e1 100644 > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > @@ -243,6 +243,11 @@ static int analogix_dp_link_start(struct > > analogix_dp_device *dp) > > > > /* Set link rate and count as you want to establish*/ > > analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate); > > + /* > > +* MACRO_RST must be applied after the PLL_LOCK to avoid > > +* the DP inter pair skew issue for at least 10 us > > +*/ > > + analogix_dp_reset_macro(dp); > > analogix_dp_set_lane_count(dp, dp->link_train.lane_count); > > > > /* Setup RX configuration */ > > @@ -565,12 +570,6 @@ static int analogix_dp_full_link_train(struct > > analogix_dp_device *dp, > > int retval = 0; > > bool training_finished = false; > > > > - /* > > -* MACRO_RST must be applied after the PLL_LOCK to avoid > > -* the DP inter pair skew issue for at least 10 us > > -*/ > > - analogix_dp_reset_macro(dp); > > - > > /* Initialize by reading RX's DPCD */ > > analogix_dp_get_max_rx_bandwidth(dp, &dp->link_train.link_rate); > > analogix_dp_get_max_rx_lane_count(dp, &dp->link_train.lane_count); > > @@ -637,9 +636,12 @@ static int analogix_dp_fast_link_train(struct > > analogix_dp_device *dp) > > u8 link_align, link_status[2]; > > enum pll_status status; > > > > - analogix_dp_reset_macro(dp); > > - > > analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate); > > + /* > > +* MACRO_RST must be applied after the PLL_LOCK to avoid > > +* the DP inter pair skew issue for at least 10 us > > +*/ > > + analogix_dp_reset_macro(dp); > > analogix_dp_set_lane_count(dp, dp->link_train.lane_count); > > analogix_dp_set_lane_link_training(dp); > > > > -- > > 2.39.2 > > > > This patch does not apply on drm-misc/drm-misc-next as far as I can tell. Reviewed-by: Robert Foss
Re: [PATCH] drm/i915/gt: Delete the live_hearbeat_fast selftest
On 03/06/2024 17:20, Niemiec, Krzysztof wrote:
The test is trying to push the heartbeat frequency to the limit, which
might sometimes fail. Such a failure does not provide valuable
information, because it does not indicate that there is something
necessarily wrong with either the driver or the hardware.
Remove the test to prevent random, unnecessary failures from appearing
in CI.
Suggested-by: Chris Wilson
Signed-off-by: Niemiec, Krzysztof
Just a note in passing that comma in the email display name is I believe
not RFC 5322 compliant and there might be tools which barf on it(*). If
you can put it in double quotes, it would be advisable.
Regards,
Tvrtko
*) Such as my internal pull request generator which uses CPAN's
Email::Address::XS. :)
---
.../drm/i915/gt/selftest_engine_heartbeat.c | 110 --
1 file changed, 110 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index ef014df4c4fc..9e4f0e417b3b 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -193,115 +193,6 @@ static int live_idle_pulse(void *arg)
return err;
}
-static int cmp_u32(const void *_a, const void *_b)
-{
- const u32 *a = _a, *b = _b;
-
- return *a - *b;
-}
-
-static int __live_heartbeat_fast(struct intel_engine_cs *engine)
-{
- const unsigned int error_threshold = max(2u, jiffies_to_usecs(6));
- struct intel_context *ce;
- struct i915_request *rq;
- ktime_t t0, t1;
- u32 times[5];
- int err;
- int i;
-
- ce = intel_context_create(engine);
- if (IS_ERR(ce))
- return PTR_ERR(ce);
-
- intel_engine_pm_get(engine);
-
- err = intel_engine_set_heartbeat(engine, 1);
- if (err)
- goto err_pm;
-
- for (i = 0; i < ARRAY_SIZE(times); i++) {
- do {
- /* Manufacture a tick */
- intel_engine_park_heartbeat(engine);
- GEM_BUG_ON(engine->heartbeat.systole);
- engine->serial++; /* pretend we are not idle! */
- intel_engine_unpark_heartbeat(engine);
-
- flush_delayed_work(&engine->heartbeat.work);
- if (!delayed_work_pending(&engine->heartbeat.work)) {
- pr_err("%s: heartbeat %d did not start\n",
- engine->name, i);
- err = -EINVAL;
- goto err_pm;
- }
-
- rcu_read_lock();
- rq = READ_ONCE(engine->heartbeat.systole);
- if (rq)
- rq = i915_request_get_rcu(rq);
- rcu_read_unlock();
- } while (!rq);
-
- t0 = ktime_get();
- while (rq == READ_ONCE(engine->heartbeat.systole))
- yield(); /* work is on the local cpu! */
- t1 = ktime_get();
-
- i915_request_put(rq);
- times[i] = ktime_us_delta(t1, t0);
- }
-
- sort(times, ARRAY_SIZE(times), sizeof(times[0]), cmp_u32, NULL);
-
- pr_info("%s: Heartbeat delay: %uus [%u, %u]\n",
- engine->name,
- times[ARRAY_SIZE(times) / 2],
- times[0],
- times[ARRAY_SIZE(times) - 1]);
-
- /*
-* Ideally, the upper bound on min work delay would be something like
-* 2 * 2 (worst), +1 for scheduling, +1 for slack. In practice, we
-* are, even with system_wq_highpri, at the mercy of the CPU scheduler
-* and may be stuck behind some slow work for many millisecond. Such
-* as our very own display workers.
-*/
- if (times[ARRAY_SIZE(times) / 2] > error_threshold) {
- pr_err("%s: Heartbeat delay was %uus, expected less than
%dus\n",
- engine->name,
- times[ARRAY_SIZE(times) / 2],
- error_threshold);
- err = -EINVAL;
- }
-
- reset_heartbeat(engine);
-err_pm:
- intel_engine_pm_put(engine);
- intel_context_put(ce);
- return err;
-}
-
-static int live_heartbeat_fast(void *arg)
-{
- struct intel_gt *gt = arg;
- struct intel_engine_cs *engine;
- enum intel_engine_id id;
- int err = 0;
-
- /* Check that the heartbeat ticks at the desired rate. */
- if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
- return 0;
-
- for_each_engine(engine, gt, id) {
- err = __live_heartbeat_fast(engine);
- if (err)
- break;
- }
-
- return err;
-}
-
static int __live_heartbeat_off(struct intel_engine_cs *engine)
{
int err;
@@ -372,7 +263,6 @@ i
Re: [PATCH v2] drm/bridge: it6505: remove unnecessary NULL checks
On Mon, 10 Jun 2024 13:50:26 +0300, Dan Carpenter wrote: > Smatch complains correctly that the NULL checking isn't consistent: > > drivers/gpu/drm/bridge/ite-it6505.c:2583 it6505_poweron() > error: we previously assumed 'pdata->pwr18' could be null > (see line 2569) > > These the ->pwr18 pointer is allocated during probe using the > devm_regulator_get() function. If CONFIG_REGULATOR is disabled then, > yes, it can return NULL but in that case regulator_enable() and > disable() functions are no-ops so passing a NULL pointer is okay. > > [...] Applied, thanks! [1/1] drm/bridge: it6505: remove unnecessary NULL checks (no commit info) Rob
Re: [PATCH] drm/bridge/panel: Fix runtime warning on panel bridge release
On Mon, Jun 10, 2024 at 11:27:39AM GMT, Adam Miotk wrote: > Device managed panel bridge wrappers are created by calling to > drm_panel_bridge_add_typed() and registering a release handler for > clean-up when the device gets unbound. > > Since the memory for this bridge is also managed and linked to the panel > device, the release function should not try to free that memory. > Moreover, the call to devm_kfree() inside drm_panel_bridge_remove() will > fail in this case and emit a warning because the panel bridge resource > is no longer on the device resources list (it has been removed from > there before the call to release handlers). > > Signed-off-by: Adam Miotk I've added a Fixes tag and applied to drm-misc-fixes, thanks! Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/komeda: check for error-valued pointer
On Mon, Jun 10, 2024 at 11:20:56AM GMT, Amjad Ouled-Ameur wrote: > komeda_pipeline_get_state() may return an error-valued pointer, thus > check the pointer for negative or null value before dereferencing. > > Signed-off-by: Amjad Ouled-Ameur I've added a Fixes tag and applied to drm-misc-fixes, thanks! Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/connector: hdmi: Fix kerneldoc warnings
Hi,
On Mon, Jun 10, 2024 at 01:12:00PM GMT, Maxime Ripard wrote:
> It looks like the documentation for the HDMI-related fields recently
> added to both the drm_connector and drm_connector_state structures
> trigger some warnings because of their use of anonymous structures:
>
> $ scripts/kernel-doc -none include/drm/drm_connector.h
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'broadcast_rgb' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'infoframes' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member 'avi'
> description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member 'hdr_drm'
> description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member 'spd'
> description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member 'vendor'
> description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'is_limited_range' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'output_bpc' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'output_format' description in 'drm_connector_state'
> include/drm/drm_connector.h:1138: warning: Excess struct member
> 'tmds_char_rate' description in 'drm_connector_state'
> include/drm/drm_connector.h:2112: warning: Excess struct member 'vendor'
> description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member 'product'
> description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member
> 'supported_formats' description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member
> 'infoframes' description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member 'lock'
> description in 'drm_connector'
> include/drm/drm_connector.h:2112: warning: Excess struct member 'audio'
> description in 'drm_connector'
>
> Create some intermediate structures instead of anonymous ones to silence
> the warnings.
>
> Reported-by: Jani Nikula
> Suggested-by: Jani Nikula
> Fixes: 54cb39e2293b ("drm/connector: hdmi: Create an HDMI sub-state")
> Fixes: 948f01d5e559 ("drm/connector: hdmi: Add support for output format")
> Signed-off-by: Maxime Ripard
> ---
> include/drm/drm_connector.h | 207 +++-
> 1 file changed, 109 insertions(+), 98 deletions(-)
>
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index e04a8a0d1bbd..1afee2939b71 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -927,10 +927,72 @@ struct drm_connector_hdmi_infoframe {
>* @set: Is the content of @data valid?
>*/
> bool set;
> };
>
> +/*
> + * struct drm_connector_hdmi_state - HDMI state container
> + */
> +struct drm_connector_hdmi_state {
> + /**
> + * @broadcast_rgb: Connector property to pass the
> + * Broadcast RGB selection value.
> + */
> + enum drm_hdmi_broadcast_rgb broadcast_rgb;
> +
> + /**
> + * @infoframes: HDMI Infoframes matching that state
> + */
> + struct {
> + /**
> + * @avi: AVI Infoframes structure matching our
> + * state.
> + */
> + struct drm_connector_hdmi_infoframe avi;
> +
> + /**
> + * @hdr_drm: DRM (Dynamic Range and Mastering)
> + * Infoframes structure matching our state.
> + */
> + struct drm_connector_hdmi_infoframe hdr_drm;
> +
> + /**
> + * @spd: SPD Infoframes structure matching our
> + * state.
> + */
> + struct drm_connector_hdmi_infoframe spd;
> +
> + /**
> + * @vendor: HDMI Vendor Infoframes structure
> + * matching our state.
> + */
> + struct drm_connector_hdmi_infoframe hdmi;
> + } infoframes;
> +
> + /**
> + * @is_limited_range: Is the output supposed to use a limited
> + * RGB Quantization Range or not?
> + */
> + bool is_limited_range;
> +
> + /**
> + * @output_bpc: Bits per color channel to output.
> + */
> + unsigned int output_bpc;
> +
> + /**
> + * @output_format: Pixel format to output in.
> + */
> + enum hdmi_colorspace output_format;
> +
> + /**
> + * @tmds_char_rate: TMDS Character Rate, in Hz.
> + */
> + unsigned long long tmds_char_rate;
> +
> +}
FTR, there's a missing ; here
Maxime
signature.asc
Description: PGP signature
[PATCH] drm/connector: hdmi: Fix kerneldoc warnings
It looks like the documentation for the HDMI-related fields recently
added to both the drm_connector and drm_connector_state structures
trigger some warnings because of their use of anonymous structures:
$ scripts/kernel-doc -none include/drm/drm_connector.h
include/drm/drm_connector.h:1138: warning: Excess struct member
'broadcast_rgb' description in 'drm_connector_state'
include/drm/drm_connector.h:1138: warning: Excess struct member 'infoframes'
description in 'drm_connector_state'
include/drm/drm_connector.h:1138: warning: Excess struct member 'avi'
description in 'drm_connector_state'
include/drm/drm_connector.h:1138: warning: Excess struct member 'hdr_drm'
description in 'drm_connector_state'
include/drm/drm_connector.h:1138: warning: Excess struct member 'spd'
description in 'drm_connector_state'
include/drm/drm_connector.h:1138: warning: Excess struct member 'vendor'
description in 'drm_connector_state'
include/drm/drm_connector.h:1138: warning: Excess struct member
'is_limited_range' description in 'drm_connector_state'
include/drm/drm_connector.h:1138: warning: Excess struct member 'output_bpc'
description in 'drm_connector_state'
include/drm/drm_connector.h:1138: warning: Excess struct member
'output_format' description in 'drm_connector_state'
include/drm/drm_connector.h:1138: warning: Excess struct member
'tmds_char_rate' description in 'drm_connector_state'
include/drm/drm_connector.h:2112: warning: Excess struct member 'vendor'
description in 'drm_connector'
include/drm/drm_connector.h:2112: warning: Excess struct member 'product'
description in 'drm_connector'
include/drm/drm_connector.h:2112: warning: Excess struct member
'supported_formats' description in 'drm_connector'
include/drm/drm_connector.h:2112: warning: Excess struct member 'infoframes'
description in 'drm_connector'
include/drm/drm_connector.h:2112: warning: Excess struct member 'lock'
description in 'drm_connector'
include/drm/drm_connector.h:2112: warning: Excess struct member 'audio'
description in 'drm_connector'
Create some intermediate structures instead of anonymous ones to silence
the warnings.
Reported-by: Jani Nikula
Suggested-by: Jani Nikula
Fixes: 54cb39e2293b ("drm/connector: hdmi: Create an HDMI sub-state")
Fixes: 948f01d5e559 ("drm/connector: hdmi: Add support for output format")
Signed-off-by: Maxime Ripard
---
include/drm/drm_connector.h | 207 +++-
1 file changed, 109 insertions(+), 98 deletions(-)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index e04a8a0d1bbd..1afee2939b71 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -927,10 +927,72 @@ struct drm_connector_hdmi_infoframe {
* @set: Is the content of @data valid?
*/
bool set;
};
+/*
+ * struct drm_connector_hdmi_state - HDMI state container
+ */
+struct drm_connector_hdmi_state {
+ /**
+* @broadcast_rgb: Connector property to pass the
+* Broadcast RGB selection value.
+*/
+ enum drm_hdmi_broadcast_rgb broadcast_rgb;
+
+ /**
+* @infoframes: HDMI Infoframes matching that state
+*/
+ struct {
+ /**
+* @avi: AVI Infoframes structure matching our
+* state.
+*/
+ struct drm_connector_hdmi_infoframe avi;
+
+ /**
+* @hdr_drm: DRM (Dynamic Range and Mastering)
+* Infoframes structure matching our state.
+*/
+ struct drm_connector_hdmi_infoframe hdr_drm;
+
+ /**
+* @spd: SPD Infoframes structure matching our
+* state.
+*/
+ struct drm_connector_hdmi_infoframe spd;
+
+ /**
+* @vendor: HDMI Vendor Infoframes structure
+* matching our state.
+*/
+ struct drm_connector_hdmi_infoframe hdmi;
+ } infoframes;
+
+ /**
+* @is_limited_range: Is the output supposed to use a limited
+* RGB Quantization Range or not?
+*/
+ bool is_limited_range;
+
+ /**
+* @output_bpc: Bits per color channel to output.
+*/
+ unsigned int output_bpc;
+
+ /**
+* @output_format: Pixel format to output in.
+*/
+ enum hdmi_colorspace output_format;
+
+ /**
+* @tmds_char_rate: TMDS Character Rate, in Hz.
+*/
+ unsigned long long tmds_char_rate;
+
+}
+
/**
* struct drm_connector_state - mutable connector state
*/
struct drm_connector_state {
/** @connector: backpointer to the connector */
@@ -1076,67 +1138,11 @@ struct drm_connector_state {
/**
* @hdmi: HDMI-related variable and properties. Filled by
* @drm_atomic_helper_connector_hdmi_check().
*/
- struct {
- /**
-
[PATCH v2] drm/bridge: it6505: remove unnecessary NULL checks
Smatch complains correctly that the NULL checking isn't consistent:
drivers/gpu/drm/bridge/ite-it6505.c:2583 it6505_poweron()
error: we previously assumed 'pdata->pwr18' could be null
(see line 2569)
These the ->pwr18 pointer is allocated during probe using the
devm_regulator_get() function. If CONFIG_REGULATOR is disabled then,
yes, it can return NULL but in that case regulator_enable() and
disable() functions are no-ops so passing a NULL pointer is okay.
Smatch is correct that the NULL checks are inconsistent but the
fix is to delete the unnecessary NULL checking. Do the same for
"pdata->ovdd" as well.
Signed-off-by: Dan Carpenter
---
v2: In the first patch, I added a NULL check but that wasn't necessary
or correct.
drivers/gpu/drm/bridge/ite-it6505.c | 43 -
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
b/drivers/gpu/drm/bridge/ite-it6505.c
index 3f68c82888c2..d89d1bb9a8ec 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2566,24 +2566,21 @@ static int it6505_poweron(struct it6505 *it6505)
return 0;
}
- if (pdata->pwr18) {
- err = regulator_enable(pdata->pwr18);
- if (err) {
- DRM_DEV_DEBUG_DRIVER(dev, "Failed to enable VDD18: %d",
-err);
- return err;
- }
+ err = regulator_enable(pdata->pwr18);
+ if (err) {
+ DRM_DEV_DEBUG_DRIVER(dev, "Failed to enable VDD18: %d",
+err);
+ return err;
}
- if (pdata->ovdd) {
- /* time interval between IVDD and OVDD at least be 1ms */
- usleep_range(1000, 2000);
- err = regulator_enable(pdata->ovdd);
- if (err) {
- regulator_disable(pdata->pwr18);
- return err;
- }
+ /* time interval between IVDD and OVDD at least be 1ms */
+ usleep_range(1000, 2000);
+ err = regulator_enable(pdata->ovdd);
+ if (err) {
+ regulator_disable(pdata->pwr18);
+ return err;
}
+
/* time interval between OVDD and SYSRSTN at least be 10ms */
if (pdata->gpiod_reset) {
usleep_range(1, 2);
@@ -2618,17 +2615,13 @@ static int it6505_poweroff(struct it6505 *it6505)
if (pdata->gpiod_reset)
gpiod_set_value_cansleep(pdata->gpiod_reset, 0);
- if (pdata->pwr18) {
- err = regulator_disable(pdata->pwr18);
- if (err)
- return err;
- }
+ err = regulator_disable(pdata->pwr18);
+ if (err)
+ return err;
- if (pdata->ovdd) {
- err = regulator_disable(pdata->ovdd);
- if (err)
- return err;
- }
+ err = regulator_disable(pdata->ovdd);
+ if (err)
+ return err;
it6505->powered = false;
it6505->sink_count = 0;
--
2.39.2
Re: [PATCH v2 0/3] Move blender setup from individual planes to crtc commit in sun4i-drm
On Sat, 24 Feb 2024 16:05:57 +0100, Ondřej Jirman wrote: > From: Ondrej Jirman > > This series refactors blender setup from individual planes to a common > place where it can be performed at once and is easier to reason about. > > In the process this fixes a few bugs that allowed blender pipes to be > disabled while corresponding DRM planes were requested to be enabled. > > [...] Applied to misc/kernel.git (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm: add missing MODULE_DESCRIPTION() macros
On Sun, 09 Jun 2024 11:42:53 -0700, Jeff Johnson wrote: > On x86, make allmodconfig && make W=1 C=1 reports: > WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/gpu/drm/gud/gud.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/drm_panel_orientation_quirks.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/drm_mipi_dbi.o > WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/gpu/drm/i915/kvmgt.o > WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/gpu/drm/udl/udl.o > > [...] Applied to misc/kernel.git (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/bridge: add missing MODULE_DESCRIPTION() macros
On Sun, 09 Jun 2024 10:06:17 -0700, Jeff Johnson wrote: > make allmodconfig && make W=1 C=1 reports: > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/bridge/lontium-lt9611.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/bridge/lontium-lt9611uxc.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/bridge/sil-sii8620.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/bridge/sii9234.o > > Add the missing invocations of the MODULE_DESCRIPTION() macro. > > [...] Applied to misc/kernel.git (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/tiny: add missing MODULE_DESCRIPTION() macros
On Sun, 09 Jun 2024 10:20:27 -0700, Jeff Johnson wrote: > make allmodconfig && make W=1 C=1 reports: > WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/gpu/drm/tiny/bochs.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tiny/cirrus.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tiny/gm12u320.o > > Add the missing invocations of the MODULE_DESCRIPTION() macro. > > [...] Applied to misc/kernel.git (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/panel: add missing MODULE_DESCRIPTION() macros
On Sun, 09 Jun 2024 09:53:21 -0700, Jeff Johnson wrote: > make allmodconfig && make W=1 C=1 reports: > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/panel/panel-abt-y030xx067a.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/panel/panel-auo-a030jtn01.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/panel/panel-innolux-ej030na.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/panel/panel-newvision-nv3052c.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/panel/panel-novatek-nt39016.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/panel/panel-orisetech-ota5601a.o > > [...] Applied to misc/kernel.git (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH] drm/tests: add missing MODULE_DESCRIPTION() macros
On Thu, 06 Jun 2024 21:42:46 -0700, Jeff Johnson wrote: > make allmodconfig && make W=1 C=1 reports: > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_kunit_helpers.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_buddy_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_cmdline_parser_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_connector_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_damage_helper_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_dp_mst_helper_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_exec_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_format_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_framebuffer_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_gem_shmem_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_managed_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_mm_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_modes_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_plane_helper_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_probe_helper_test.o > WARNING: modpost: missing MODULE_DESCRIPTION() in > drivers/gpu/drm/tests/drm_rect_test.o > > [...] Applied to misc/kernel.git (drm-misc-next). Thanks! Maxime
Re: [PATCH v8 08/13] PCI: Move pinned status bit to struct pci_dev
On Mon, 2024-06-10 at 11:31 +0200, Philipp Stanner wrote:
> The bit describing whether the PCI device is currently pinned is stored
> in struct pci_devres. To clean up and simplify the PCI devres API, it's
> better if this information is stored in struct pci_dev.
>
> This will later permit simplifying pcim_enable_device().
>
> Move the 'pinned' boolean bit to struct pci_dev.
>
> Restructure bits in struct pci_dev so the pm / pme fields are next to
> each other.
>
> Signed-off-by: Philipp Stanner
> ---
> drivers/pci/devres.c | 14 --
> drivers/pci/pci.h | 1 -
> include/linux/pci.h | 4 +++-
> 3 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
> index 9d25940ce260..2696baef5c2c 100644
> --- a/drivers/pci/devres.c
> +++ b/drivers/pci/devres.c
> @@ -403,7 +403,7 @@ static void pcim_release(struct device *gendev, void *res)
> if (this->restore_intx)
> pci_intx(dev, this->orig_intx);
>
> - if (pci_is_enabled(dev) && !this->pinned)
> + if (pci_is_enabled(dev) && !dev->pinned)
> pci_disable_device(dev);
> }
>
> @@ -459,18 +459,12 @@ EXPORT_SYMBOL(pcim_enable_device);
> * pcim_pin_device - Pin managed PCI device
> * @pdev: PCI device to pin
> *
> - * Pin managed PCI device @pdev. Pinned device won't be disabled on
> - * driver detach. @pdev must have been enabled with
> - * pcim_enable_device().
> + * Pin managed PCI device @pdev. Pinned device won't be disabled on driver
> + * detach. @pdev must have been enabled with pcim_enable_device().
> */
> void pcim_pin_device(struct pci_dev *pdev)
> {
> - struct pci_devres *dr;
> -
> - dr = find_pci_dr(pdev);
> - WARN_ON(!dr || !pci_is_enabled(pdev));
> - if (dr)
> - dr->pinned = 1;
> + pdev->pinned = true;
> }
> EXPORT_SYMBOL(pcim_pin_device);
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index d7f00b43b098..6e02ba1b5947 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -821,7 +821,6 @@ static inline pci_power_t mid_pci_get_power_state(struct
> pci_dev *pdev)
> * then remove them from here.
> */
> struct pci_devres {
> - unsigned int pinned:1;
> unsigned int orig_intx:1;
> unsigned int restore_intx:1;
> unsigned int mwi:1;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index fb004fd4e889..cc9247f78158 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -367,10 +367,12 @@ struct pci_dev {
> this is D0-D3, D0 being fully
> functional, and D3 being off. */
> u8 pm_cap; /* PM capability offset */
> - unsigned intimm_ready:1;/* Supports Immediate Readiness */
> unsigned intpme_support:5; /* Bitmask of states from which PME#
> can be generated */
> unsigned intpme_poll:1; /* Poll device's PME status bit */
> + unsigned intenabled:1; /* Whether this dev is enabled */
Ah crap, here it survived for some reason...
Should just be dead code and not have any effect. In any case, we
should remove it.
@Bjorn: Feel free to remove it yourself. Otherwise I could provide a v9
together with potential further feedback taken into account in a few
days
Thx,
P.
> + unsigned intpinned:1; /* Whether this dev is pinned */
> + unsigned intimm_ready:1;/* Supports Immediate Readiness */
> unsigned intd1_support:1; /* Low power state D1 is supported */
> unsigned intd2_support:1; /* Low power state D2 is supported */
> unsigned intno_d1d2:1; /* D1 and D2 are forbidden */
Re: [PATCH RESEND] drm: panel-orientation-quirks: Add quirk for Aya Neo KUN
Hi Tobias,
On 5/31/24 9:04 PM, Tobias Jakobi wrote:
> On 3/10/24 23:04, tjak...@math.uni-bielefeld.de wrote:
>
>> From: Tobias Jakobi
>>
>> Similar to the other Aya Neo devices this one features
>> again a portrait screen, here with a native resolution
>> of 1600x2560.
>>
>> Signed-off-by: Tobias Jakobi
>> ---
>> drivers/gpu/drm/drm_panel_orientation_quirks.c | 6 ++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c
>> b/drivers/gpu/drm/drm_panel_orientation_quirks.c
>> index 3d92f66e550c..5d3fb11fd45f 100644
>> --- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
>> +++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
>> @@ -196,6 +196,12 @@ static const struct dmi_system_id orientation_data[] = {
>> DMI_MATCH(DMI_BOARD_NAME, "NEXT"),
>> },
>> .driver_data = (void *)&lcd800x1280_rightside_up,
>> + }, { /* AYA NEO KUN */
>> + .matches = {
>> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
>> + DMI_MATCH(DMI_BOARD_NAME, "KUN"),
>> + },
>> + .driver_data = (void *)&lcd1600x2560_rightside_up,
>> }, { /* Chuwi HiBook (CWI514) */
>> .matches = {
>> DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
>
> Trying yet another ping! Also adding Hans to the list of recipients, as he
> committed the last quirk for an Ayaneo device. Someone pick this up, pretty
> please! :-)
Thank you for Cc-ing me and thank you for your patch.
This looks good to me:
Reviewed-by: Hans de Goede
I'll go and merge this into drm-misc-fixes now. Note I've not done a build
for drm-misc-fixes in a while and I'm on a laptop atm, so it will be a while
before this shows up as I'll do a (slow) test-build before pusing out the
changes.
Regards,
Hans
[PATCH v8 10/13] PCI: Give pci_intx() its own devres callback
pci_intx() is one of the functions that have "hybrid mode" (i.e.,
sometimes managed, sometimes not). Providing a separate pcim_intx()
function with its own device resource and cleanup callback allows for
removing further large parts of the legacy PCI devres implementation.
As in the region-request-functions, pci_intx() has to call into its
managed counterpart for backwards compatibility.
As pci_intx() is an outdated function, pcim_intx() shall not be made
visible to drivers via a public API.
Implement pcim_intx() with its own device resource.
Make pci_intx() call pcim_intx() in the managed case.
Remove the now surplus function find_pci_dr().
Signed-off-by: Philipp Stanner
---
drivers/pci/devres.c | 76
drivers/pci/pci.c| 21 ++--
drivers/pci/pci.h| 13
3 files changed, 80 insertions(+), 30 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index a0a59338cd92..0bb144fdb69b 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -42,6 +42,11 @@ struct pcim_iomap_devres {
void __iomem *table[PCI_STD_NUM_BARS];
};
+/* Used to restore the old intx state on driver detach. */
+struct pcim_intx_devres {
+ int orig_intx;
+};
+
enum pcim_addr_devres_type {
/* Default initializer. */
PCIM_ADDR_DEVRES_TYPE_INVALID,
@@ -397,32 +402,75 @@ int pcim_set_mwi(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pcim_set_mwi);
+
static inline bool mask_contains_bar(int mask, int bar)
{
return mask & BIT(bar);
}
-static void pcim_release(struct device *gendev, void *res)
+static void pcim_intx_restore(struct device *dev, void *data)
{
- struct pci_dev *dev = to_pci_dev(gendev);
- struct pci_devres *this = res;
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct pcim_intx_devres *res = data;
- if (this->restore_intx)
- pci_intx(dev, this->orig_intx);
+ pci_intx(pdev, res->orig_intx);
+}
- if (pci_is_enabled(dev) && !dev->pinned)
- pci_disable_device(dev);
+static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev)
+{
+ struct pcim_intx_devres *res;
+
+ res = devres_find(dev, pcim_intx_restore, NULL, NULL);
+ if (res)
+ return res;
+
+ res = devres_alloc(pcim_intx_restore, sizeof(*res), GFP_KERNEL);
+ if (res)
+ devres_add(dev, res);
+
+ return res;
}
-/*
- * TODO: After the last four callers in pci.c are ported, find_pci_dr()
- * needs to be made static again.
+/**
+ * pcim_intx - managed pci_intx()
+ * @pdev: the PCI device to operate on
+ * @enable: boolean: whether to enable or disable PCI INTx
+ *
+ * Returns: 0 on success, -ENOMEM on error.
+ *
+ * Enables/disables PCI INTx for device @pdev.
+ * Restores the original state on driver detach.
*/
-struct pci_devres *find_pci_dr(struct pci_dev *pdev)
+int pcim_intx(struct pci_dev *pdev, int enable)
{
- if (pci_is_managed(pdev))
- return devres_find(&pdev->dev, pcim_release, NULL, NULL);
- return NULL;
+ u16 pci_command, new;
+ struct pcim_intx_devres *res;
+
+ res = get_or_create_intx_devres(&pdev->dev);
+ if (!res)
+ return -ENOMEM;
+
+ res->orig_intx = !enable;
+
+ pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+
+ if (enable)
+ new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
+ else
+ new = pci_command | PCI_COMMAND_INTX_DISABLE;
+
+ if (new != pci_command)
+ pci_write_config_word(pdev, PCI_COMMAND, new);
+
+ return 0;
+}
+
+static void pcim_release(struct device *gendev, void *res)
+{
+ struct pci_dev *dev = to_pci_dev(gendev);
+
+ if (pci_is_enabled(dev) && !dev->pinned)
+ pci_disable_device(dev);
}
static struct pci_devres *get_pci_dr(struct pci_dev *pdev)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index db2cc48f3d63..1b4832a60047 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4443,6 +4443,16 @@ void pci_intx(struct pci_dev *pdev, int enable)
{
u16 pci_command, new;
+ /*
+* This is done for backwards compatibility, because the old PCI devres
+* API had a mode in which this function became managed if the dev had
+* been enabled with pcim_enable_device() instead of
pci_enable_device().
+*/
+ if (pci_is_managed(pdev)) {
+ WARN_ON_ONCE(pcim_intx(pdev, enable) != 0);
+ return;
+ }
+
pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
if (enable)
@@ -4450,17 +4460,8 @@ void pci_intx(struct pci_dev *pdev, int enable)
else
new = pci_command | PCI_COMMAND_INTX_DISABLE;
- if (new != pci_command) {
- struct pci_devres *dr;
-
+ if (new != pci_command)
pci_write_config_word(pdev, PCI_COMMAND, new);
-
-
[PATCH v8 04/13] PCI: Deprecate two surplus devres functions
pcim_iomap_table() should not be used anymore because it contributed to the
PCI devres API being designed contrary to devres's design goals.
pcim_iomap_regions_request_all() is a surplus, complicated function that
can easily be replaced by using a pcim_* request function in combination
with a pcim_* mapping function.
Mark pcim_iomap_table() and pcim_iomap_regions_request_all() as deprecated
in the function documentation.
Link: https://lore.kernel.org/r/20240605081605.18769-6-pstan...@redhat.com
Signed-off-by: Philipp Stanner
Signed-off-by: Bjorn Helgaas
---
drivers/pci/devres.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 82f71f5e164a..54b10f5433ab 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -507,7 +507,7 @@ static void pcim_iomap_release(struct device *gendev, void
*res)
}
/**
- * pcim_iomap_table - access iomap allocation table
+ * pcim_iomap_table - access iomap allocation table (DEPRECATED)
* @pdev: PCI device to access iomap table for
*
* Returns:
@@ -521,6 +521,11 @@ static void pcim_iomap_release(struct device *gendev, void
*res)
* This function might sleep when the table is first allocated but can
* be safely called without context and guaranteed to succeed once
* allocated.
+ *
+ * This function is DEPRECATED. Do not use it in new code. Instead, obtain a
+ * mapping's address directly from one of the pcim_* mapping functions. For
+ * example:
+ * void __iomem *mappy = pcim_iomap(pdev, bar, length);
*/
void __iomem * const *pcim_iomap_table(struct pci_dev *pdev)
{
@@ -894,6 +899,7 @@ static int pcim_request_all_regions(struct pci_dev *pdev,
const char *name)
/**
* pcim_iomap_regions_request_all - Request all BARs and iomap specified ones
+ * (DEPRECATED)
* @pdev: PCI device to map IO resources for
* @mask: Mask of BARs to iomap
* @name: Name associated with the requests
@@ -904,6 +910,10 @@ static int pcim_request_all_regions(struct pci_dev *pdev,
const char *name)
*
* To release these resources manually, call pcim_release_region() for the
* regions and pcim_iounmap() for the mappings.
+ *
+ * This function is DEPRECATED. Don't use it in new code. Instead, use one
+ * of the pcim_* region request functions in combination with a pcim_*
+ * mapping function.
*/
int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask,
const char *name)
--
2.45.0
[PATCH v8 07/13] PCI: Remove enabled status bit from pci_devres
The PCI devres implementation has a separate boolean to track whether a
device is enabled. However, that can easily be tracked through the
function pci_is_enabled() which is agnostic.
Using it allows for simplifying the PCI devres implementation.
Replace the separate 'enabled' status bit from struct pci_devres with
calls to pci_is_enabled() at the appropriate places.
Signed-off-by: Philipp Stanner
---
drivers/pci/devres.c | 11 ---
drivers/pci/pci.c| 6 --
drivers/pci/pci.h| 1 -
3 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index f2a1250c0679..9d25940ce260 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -403,7 +403,7 @@ static void pcim_release(struct device *gendev, void *res)
if (this->restore_intx)
pci_intx(dev, this->orig_intx);
- if (this->enabled && !this->pinned)
+ if (pci_is_enabled(dev) && !this->pinned)
pci_disable_device(dev);
}
@@ -446,14 +446,11 @@ int pcim_enable_device(struct pci_dev *pdev)
dr = get_pci_dr(pdev);
if (unlikely(!dr))
return -ENOMEM;
- if (dr->enabled)
- return 0;
rc = pci_enable_device(pdev);
- if (!rc) {
+ if (!rc)
pdev->is_managed = 1;
- dr->enabled = 1;
- }
+
return rc;
}
EXPORT_SYMBOL(pcim_enable_device);
@@ -471,7 +468,7 @@ void pcim_pin_device(struct pci_dev *pdev)
struct pci_devres *dr;
dr = find_pci_dr(pdev);
- WARN_ON(!dr || !dr->enabled);
+ WARN_ON(!dr || !pci_is_enabled(pdev));
if (dr)
dr->pinned = 1;
}
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 5e4f377411ec..db2cc48f3d63 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2218,12 +2218,6 @@ void pci_disable_enabled_device(struct pci_dev *dev)
*/
void pci_disable_device(struct pci_dev *dev)
{
- struct pci_devres *dr;
-
- dr = find_pci_dr(dev);
- if (dr)
- dr->enabled = 0;
-
dev_WARN_ONCE(&dev->dev, atomic_read(&dev->enable_cnt) <= 0,
"disabling already-disabled device");
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 2403c5a0ff7a..d7f00b43b098 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -821,7 +821,6 @@ static inline pci_power_t mid_pci_get_power_state(struct
pci_dev *pdev)
* then remove them from here.
*/
struct pci_devres {
- unsigned int enabled:1;
unsigned int pinned:1;
unsigned int orig_intx:1;
unsigned int restore_intx:1;
--
2.45.0
[PATCH v8 13/13] drm/vboxvideo: fix mapping leaks
When the PCI devres API was introduced to this driver, it was wrongly
assumed that initializing the device with pcim_enable_device() instead
of pci_enable_device() will make all PCI functions managed.
This is wrong and was caused by the quite confusing PCI devres API in
which some, but not all, functions become managed that way.
The function pci_iomap_range() is never managed.
Replace pci_iomap_range() with the actually managed function
pcim_iomap_range().
Fixes: 8558de401b5f ("drm/vboxvideo: use managed pci functions")
Signed-off-by: Philipp Stanner
Reviewed-by: Hans de Goede
---
drivers/gpu/drm/vboxvideo/vbox_main.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_main.c
b/drivers/gpu/drm/vboxvideo/vbox_main.c
index 42c2d8a99509..d4ade9325401 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_main.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_main.c
@@ -42,12 +42,11 @@ static int vbox_accel_init(struct vbox_private *vbox)
/* Take a command buffer for each screen from the end of usable VRAM. */
vbox->available_vram_size -= vbox->num_crtcs * VBVA_MIN_BUFFER_SIZE;
- vbox->vbva_buffers = pci_iomap_range(pdev, 0,
-vbox->available_vram_size,
-vbox->num_crtcs *
-VBVA_MIN_BUFFER_SIZE);
- if (!vbox->vbva_buffers)
- return -ENOMEM;
+ vbox->vbva_buffers = pcim_iomap_range(
+ pdev, 0, vbox->available_vram_size,
+ vbox->num_crtcs * VBVA_MIN_BUFFER_SIZE);
+ if (IS_ERR(vbox->vbva_buffers))
+ return PTR_ERR(vbox->vbva_buffers);
for (i = 0; i < vbox->num_crtcs; ++i) {
vbva_setup_buffer_context(&vbox->vbva_info[i],
@@ -116,11 +115,10 @@ int vbox_hw_init(struct vbox_private *vbox)
DRM_INFO("VRAM %08x\n", vbox->full_vram_size);
/* Map guest-heap at end of vram */
- vbox->guest_heap =
- pci_iomap_range(pdev, 0, GUEST_HEAP_OFFSET(vbox),
- GUEST_HEAP_SIZE);
- if (!vbox->guest_heap)
- return -ENOMEM;
+ vbox->guest_heap = pcim_iomap_range(pdev, 0,
+ GUEST_HEAP_OFFSET(vbox), GUEST_HEAP_SIZE);
+ if (IS_ERR(vbox->guest_heap))
+ return PTR_ERR(vbox->guest_heap);
/* Create guest-heap mem-pool use 2^4 = 16 byte chunks */
vbox->guest_pool = devm_gen_pool_create(vbox->ddev.dev, 4, -1,
--
2.45.0
[PATCH v8 12/13] PCI: Add pcim_iomap_range()
The only managed mapping function currently is pcim_iomap() which
doesn't allow for mapping an area starting at a certain offset, which
many drivers want.
Add pcim_iomap_range() as an exported function.
Signed-off-by: Philipp Stanner
---
drivers/pci/devres.c | 44
include/linux/pci.h | 2 ++
2 files changed, 46 insertions(+)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index e92a8802832f..96f18243742b 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -1015,3 +1015,47 @@ void pcim_iounmap_regions(struct pci_dev *pdev, int mask)
}
}
EXPORT_SYMBOL(pcim_iounmap_regions);
+
+/**
+ * pcim_iomap_range - Create a ranged __iomap mapping within a PCI BAR
+ * @pdev: PCI device to map IO resources for
+ * @bar: Index of the BAR
+ * @offset: Offset from the begin of the BAR
+ * @len: Length in bytes for the mapping
+ *
+ * Returns: __iomem pointer on success, an IOMEM_ERR_PTR on failure.
+ *
+ * Creates a new IO-Mapping within the specified @bar, ranging from @offset to
+ * @offset + @len.
+ *
+ * The mapping will automatically get unmapped on driver detach. If desired,
+ * release manually only with pcim_iounmap().
+ */
+void __iomem *pcim_iomap_range(struct pci_dev *pdev, int bar,
+ unsigned long offset, unsigned long len)
+{
+ void __iomem *mapping;
+ struct pcim_addr_devres *res;
+
+ res = pcim_addr_devres_alloc(pdev);
+ if (!res)
+ return IOMEM_ERR_PTR(-ENOMEM);
+
+ mapping = pci_iomap_range(pdev, bar, offset, len);
+ if (!mapping) {
+ pcim_addr_devres_free(res);
+ return IOMEM_ERR_PTR(-EINVAL);
+ }
+
+ res->type = PCIM_ADDR_DEVRES_TYPE_MAPPING;
+ res->baseaddr = mapping;
+
+ /*
+* Ranged mappings don't get added to the legacy-table, since the table
+* only ever keeps track of whole BARs.
+*/
+
+ devres_add(&pdev->dev, res);
+ return mapping;
+}
+EXPORT_SYMBOL(pcim_iomap_range);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index cc9247f78158..bee1b2754219 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2304,6 +2304,8 @@ int pcim_iomap_regions(struct pci_dev *pdev, int mask,
const char *name);
int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask,
const char *name);
void pcim_iounmap_regions(struct pci_dev *pdev, int mask);
+void __iomem *pcim_iomap_range(struct pci_dev *pdev, int bar,
+ unsigned long offset, unsigned long len);
extern int pci_pci_problems;
#define PCIPCI_FAIL1 /* No PCI PCI DMA */
--
2.45.0
[PATCH v8 11/13] PCI: Remove legacy pcim_release()
Thanks to preceding cleanup steps, pcim_release() is now not needed
anymore and can be replaced by pcim_disable_device(), which is the exact
counterpart to pcim_enable_device().
This permits removing further parts of the old PCI devres implementation.
Replace pcim_release() with pcim_disable_device().
Remove the now surplus function get_pci_dr().
Remove the struct pci_devres from pci.h.
Signed-off-by: Philipp Stanner
---
drivers/pci/devres.c | 53 +---
drivers/pci/pci.h| 16 -
2 files changed, 25 insertions(+), 44 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 0bb144fdb69b..e92a8802832f 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -465,48 +465,45 @@ int pcim_intx(struct pci_dev *pdev, int enable)
return 0;
}
-static void pcim_release(struct device *gendev, void *res)
+static void pcim_disable_device(void *pdev_raw)
{
- struct pci_dev *dev = to_pci_dev(gendev);
-
- if (pci_is_enabled(dev) && !dev->pinned)
- pci_disable_device(dev);
-}
-
-static struct pci_devres *get_pci_dr(struct pci_dev *pdev)
-{
- struct pci_devres *dr, *new_dr;
-
- dr = devres_find(&pdev->dev, pcim_release, NULL, NULL);
- if (dr)
- return dr;
+ struct pci_dev *pdev = pdev_raw;
- new_dr = devres_alloc(pcim_release, sizeof(*new_dr), GFP_KERNEL);
- if (!new_dr)
- return NULL;
- return devres_get(&pdev->dev, new_dr, NULL, NULL);
+ if (!pdev->pinned)
+ pci_disable_device(pdev);
}
/**
* pcim_enable_device - Managed pci_enable_device()
* @pdev: PCI device to be initialized
*
- * Managed pci_enable_device().
+ * Returns: 0 on success, negative error code on failure.
+ *
+ * Managed pci_enable_device(). Device will automatically be disabled on
+ * driver detach.
*/
int pcim_enable_device(struct pci_dev *pdev)
{
- struct pci_devres *dr;
- int rc;
+ int ret;
- dr = get_pci_dr(pdev);
- if (unlikely(!dr))
- return -ENOMEM;
+ ret = devm_add_action(&pdev->dev, pcim_disable_device, pdev);
+ if (ret != 0)
+ return ret;
- rc = pci_enable_device(pdev);
- if (!rc)
- pdev->is_managed = 1;
+ /*
+* We prefer removing the action in case of an error over
+* devm_add_action_or_reset() because the later could theoretically be
+* disturbed by users having pinned the device too soon.
+*/
+ ret = pci_enable_device(pdev);
+ if (ret != 0) {
+ devm_remove_action(&pdev->dev, pcim_disable_device, pdev);
+ return ret;
+ }
- return rc;
+ pdev->is_managed = true;
+
+ return ret;
}
EXPORT_SYMBOL(pcim_enable_device);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9e87528f1157..e51e6fa79fcc 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -810,22 +810,6 @@ static inline pci_power_t mid_pci_get_power_state(struct
pci_dev *pdev)
}
#endif
-/*
- * Managed PCI resources. This manages device on/off, INTx/MSI/MSI-X
- * on/off and BAR regions. pci_dev itself records MSI/MSI-X status, so
- * there's no need to track it separately. pci_devres is initialized
- * when a device is enabled using managed PCI device enable interface.
- *
- * TODO: Struct pci_devres only needs to be here because they're used in pci.c.
- * Port or move these functions to devres.c and then remove them from here.
- */
-struct pci_devres {
- /*
-* TODO:
-* This struct is now surplus. Remove it by refactoring pci/devres.c
-*/
-};
-
int pcim_intx(struct pci_dev *dev, int enable);
int pcim_request_region(struct pci_dev *pdev, int bar, const char *name);
--
2.45.0
[PATCH v8 09/13] PCI: Give pcim_set_mwi() its own devres callback
Managing pci_set_mwi() with devres can easily be done with its own
callback, without the necessity to store any state about it in a
device-related struct.
Remove the MWI state from struct pci_devres.
Give pcim_set_mwi() a separate devres-callback.
Signed-off-by: Philipp Stanner
---
drivers/pci/devres.c | 29 ++---
drivers/pci/pci.h| 1 -
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 2696baef5c2c..a0a59338cd92 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -366,24 +366,34 @@ void __iomem *devm_pci_remap_cfg_resource(struct device
*dev,
}
EXPORT_SYMBOL(devm_pci_remap_cfg_resource);
+static void __pcim_clear_mwi(void *pdev_raw)
+{
+ struct pci_dev *pdev = pdev_raw;
+
+ pci_clear_mwi(pdev);
+}
+
/**
* pcim_set_mwi - a device-managed pci_set_mwi()
- * @dev: the PCI device for which MWI is enabled
+ * @pdev: the PCI device for which MWI is enabled
*
* Managed pci_set_mwi().
*
* RETURNS: An appropriate -ERRNO error value on error, or zero for success.
*/
-int pcim_set_mwi(struct pci_dev *dev)
+int pcim_set_mwi(struct pci_dev *pdev)
{
- struct pci_devres *dr;
+ int ret;
- dr = find_pci_dr(dev);
- if (!dr)
- return -ENOMEM;
+ ret = devm_add_action(&pdev->dev, __pcim_clear_mwi, pdev);
+ if (ret != 0)
+ return ret;
+
+ ret = pci_set_mwi(pdev);
+ if (ret != 0)
+ devm_remove_action(&pdev->dev, __pcim_clear_mwi, pdev);
- dr->mwi = 1;
- return pci_set_mwi(dev);
+ return ret;
}
EXPORT_SYMBOL(pcim_set_mwi);
@@ -397,9 +407,6 @@ static void pcim_release(struct device *gendev, void *res)
struct pci_dev *dev = to_pci_dev(gendev);
struct pci_devres *this = res;
- if (this->mwi)
- pci_clear_mwi(dev);
-
if (this->restore_intx)
pci_intx(dev, this->orig_intx);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 6e02ba1b5947..c355bb6a698d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -823,7 +823,6 @@ static inline pci_power_t mid_pci_get_power_state(struct
pci_dev *pdev)
struct pci_devres {
unsigned int orig_intx:1;
unsigned int restore_intx:1;
- unsigned int mwi:1;
};
struct pci_devres *find_pci_dr(struct pci_dev *pdev);
--
2.45.0
[PATCH v8 08/13] PCI: Move pinned status bit to struct pci_dev
The bit describing whether the PCI device is currently pinned is stored
in struct pci_devres. To clean up and simplify the PCI devres API, it's
better if this information is stored in struct pci_dev.
This will later permit simplifying pcim_enable_device().
Move the 'pinned' boolean bit to struct pci_dev.
Restructure bits in struct pci_dev so the pm / pme fields are next to
each other.
Signed-off-by: Philipp Stanner
---
drivers/pci/devres.c | 14 --
drivers/pci/pci.h| 1 -
include/linux/pci.h | 4 +++-
3 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 9d25940ce260..2696baef5c2c 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -403,7 +403,7 @@ static void pcim_release(struct device *gendev, void *res)
if (this->restore_intx)
pci_intx(dev, this->orig_intx);
- if (pci_is_enabled(dev) && !this->pinned)
+ if (pci_is_enabled(dev) && !dev->pinned)
pci_disable_device(dev);
}
@@ -459,18 +459,12 @@ EXPORT_SYMBOL(pcim_enable_device);
* pcim_pin_device - Pin managed PCI device
* @pdev: PCI device to pin
*
- * Pin managed PCI device @pdev. Pinned device won't be disabled on
- * driver detach. @pdev must have been enabled with
- * pcim_enable_device().
+ * Pin managed PCI device @pdev. Pinned device won't be disabled on driver
+ * detach. @pdev must have been enabled with pcim_enable_device().
*/
void pcim_pin_device(struct pci_dev *pdev)
{
- struct pci_devres *dr;
-
- dr = find_pci_dr(pdev);
- WARN_ON(!dr || !pci_is_enabled(pdev));
- if (dr)
- dr->pinned = 1;
+ pdev->pinned = true;
}
EXPORT_SYMBOL(pcim_pin_device);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d7f00b43b098..6e02ba1b5947 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -821,7 +821,6 @@ static inline pci_power_t mid_pci_get_power_state(struct
pci_dev *pdev)
* then remove them from here.
*/
struct pci_devres {
- unsigned int pinned:1;
unsigned int orig_intx:1;
unsigned int restore_intx:1;
unsigned int mwi:1;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fb004fd4e889..cc9247f78158 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -367,10 +367,12 @@ struct pci_dev {
this is D0-D3, D0 being fully
functional, and D3 being off. */
u8 pm_cap; /* PM capability offset */
- unsigned intimm_ready:1;/* Supports Immediate Readiness */
unsigned intpme_support:5; /* Bitmask of states from which PME#
can be generated */
unsigned intpme_poll:1; /* Poll device's PME status bit */
+ unsigned intenabled:1; /* Whether this dev is enabled */
+ unsigned intpinned:1; /* Whether this dev is pinned */
+ unsigned intimm_ready:1;/* Supports Immediate Readiness */
unsigned intd1_support:1; /* Low power state D1 is supported */
unsigned intd2_support:1; /* Low power state D2 is supported */
unsigned intno_d1d2:1; /* D1 and D2 are forbidden */
--
2.45.0
[PATCH v8 01/13] PCI: Add and use devres helper for bit masks
The current derves implementation uses manual shift operations to check
whether a bit in a mask is set. The code can be made more readable by
writing a small helper function for that.
Implement mask_contains_bar() and use it where applicable.
Link: https://lore.kernel.org/r/20240605081605.18769-3-pstan...@redhat.com
Signed-off-by: Philipp Stanner
Signed-off-by: Bjorn Helgaas
---
drivers/pci/devres.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 2c562b9eaf80..f13edd4a3873 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -161,6 +161,10 @@ int pcim_set_mwi(struct pci_dev *dev)
}
EXPORT_SYMBOL(pcim_set_mwi);
+static inline bool mask_contains_bar(int mask, int bar)
+{
+ return mask & BIT(bar);
+}
static void pcim_release(struct device *gendev, void *res)
{
@@ -169,7 +173,7 @@ static void pcim_release(struct device *gendev, void *res)
int i;
for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
- if (this->region_mask & (1 << i))
+ if (mask_contains_bar(this->region_mask, i))
pci_release_region(dev, i);
if (this->mwi)
@@ -363,7 +367,7 @@ int pcim_iomap_regions(struct pci_dev *pdev, int mask,
const char *name)
for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
unsigned long len;
- if (!(mask & (1 << i)))
+ if (!mask_contains_bar(mask, i))
continue;
rc = -EINVAL;
@@ -386,7 +390,7 @@ int pcim_iomap_regions(struct pci_dev *pdev, int mask,
const char *name)
pci_release_region(pdev, i);
err_inval:
while (--i >= 0) {
- if (!(mask & (1 << i)))
+ if (!mask_contains_bar(mask, i))
continue;
pcim_iounmap(pdev, iomap[i]);
pci_release_region(pdev, i);
@@ -438,7 +442,7 @@ void pcim_iounmap_regions(struct pci_dev *pdev, int mask)
return;
for (i = 0; i < PCIM_IOMAP_MAX; i++) {
- if (!(mask & (1 << i)))
+ if (!mask_contains_bar(mask, i))
continue;
pcim_iounmap(pdev, iomap[i]);
--
2.45.0
[PATCH v8 06/13] PCI: Warn users about complicated devres nature
The PCI region-request functions become managed functions when
pcim_enable_device() has been called previously instead of
pci_enable_device().
This has already caused a bug (in 8558de401b5f) by confusing users, who
came to believe that all pci functions, such as pci_iomap_range(), suddenly
are managed that way, which is not the case.
Add comments to the relevant functions' docstrings that warn users about
this behavior.
Link: https://lore.kernel.org/r/20240605081605.18769-8-pstan...@redhat.com
Signed-off-by: Philipp Stanner
Signed-off-by: Bjorn Helgaas
---
drivers/pci/iomap.c | 16
drivers/pci/pci.c | 42 +-
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/iomap.c b/drivers/pci/iomap.c
index c9725428e387..a715a4803c95 100644
--- a/drivers/pci/iomap.c
+++ b/drivers/pci/iomap.c
@@ -23,6 +23,10 @@
*
* @maxlen specifies the maximum length to map. If you want to get access to
* the complete BAR from offset to the end, pass %0 here.
+ *
+ * NOTE:
+ * This function is never managed, even if you initialized with
+ * pcim_enable_device().
* */
void __iomem *pci_iomap_range(struct pci_dev *dev,
int bar,
@@ -63,6 +67,10 @@ EXPORT_SYMBOL(pci_iomap_range);
*
* @maxlen specifies the maximum length to map. If you want to get access to
* the complete BAR from offset to the end, pass %0 here.
+ *
+ * NOTE:
+ * This function is never managed, even if you initialized with
+ * pcim_enable_device().
* */
void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
int bar,
@@ -106,6 +114,10 @@ EXPORT_SYMBOL_GPL(pci_iomap_wc_range);
*
* @maxlen specifies the maximum length to map. If you want to get access to
* the complete BAR without checking for its length first, pass %0 here.
+ *
+ * NOTE:
+ * This function is never managed, even if you initialized with
+ * pcim_enable_device(). If you need automatic cleanup, use pcim_iomap().
* */
void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
{
@@ -127,6 +139,10 @@ EXPORT_SYMBOL(pci_iomap);
*
* @maxlen specifies the maximum length to map. If you want to get access to
* the complete BAR without checking for its length first, pass %0 here.
+ *
+ * NOTE:
+ * This function is never managed, even if you initialized with
+ * pcim_enable_device().
* */
void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen)
{
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7013699db242..5e4f377411ec 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3900,6 +3900,8 @@ EXPORT_SYMBOL(pci_release_region);
* @res_name: Name to be associated with resource.
* @exclusive: whether the region access is exclusive or not
*
+ * Returns: 0 on success, negative error code on failure.
+ *
* Mark the PCI region associated with PCI device @pdev BAR @bar as
* being reserved by owner @res_name. Do not access any
* address inside the PCI regions unless this call returns
@@ -3950,6 +3952,8 @@ static int __pci_request_region(struct pci_dev *pdev, int
bar,
* @bar: BAR to be reserved
* @res_name: Name to be associated with resource
*
+ * Returns: 0 on success, negative error code on failure.
+ *
* Mark the PCI region associated with PCI device @pdev BAR @bar as
* being reserved by owner @res_name. Do not access any
* address inside the PCI regions unless this call returns
@@ -3957,6 +3961,11 @@ static int __pci_request_region(struct pci_dev *pdev,
int bar,
*
* Returns 0 on success, or %EBUSY on error. A warning
* message is also printed on failure.
+ *
+ * NOTE:
+ * This is a "hybrid" function: It's normally unmanaged, but becomes managed
+ * when pcim_enable_device() has been called in advance. This hybrid feature is
+ * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
*/
int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
{
@@ -4007,6 +4016,13 @@ static int __pci_request_selected_regions(struct pci_dev
*pdev, int bars,
* @pdev: PCI device whose resources are to be reserved
* @bars: Bitmask of BARs to be requested
* @res_name: Name to be associated with resource
+ *
+ * Returns: 0 on success, negative error code on failure.
+ *
+ * NOTE:
+ * This is a "hybrid" function: It's normally unmanaged, but becomes managed
+ * when pcim_enable_device() has been called in advance. This hybrid feature is
+ * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
*/
int pci_request_selected_regions(struct pci_dev *pdev, int bars,
const char *res_name)
@@ -4015,6 +4031,19 @@ int pci_request_selected_regions(struct pci_dev *pdev,
int bars,
}
EXPORT_SYMBOL(pci_request_selected_regions);
+/**
+ * pci_request_selected_regions_exclusive - Request regions exclusively
+ * @pdev: PCI device to request regions from
+ * @bars: bit mask of BARs to request
[PATCH v8 05/13] PCI: Make devres region requests consistent
Now that pure managed region request functions are available, the
implementation of the hybrid-functions which are only sometimes managed can
be made more consistent and readable by wrapping those always-managed
functions.
Implement pcim_request_region_exclusive() as a PCI-internal helper. Have
the PCI request / release functions call their pcim_ counterparts. Remove
the now surplus region_mask from struct pci_devres.
Link: https://lore.kernel.org/r/20240605081605.18769-7-pstan...@redhat.com
Signed-off-by: Philipp Stanner
Signed-off-by: Bjorn Helgaas
---
drivers/pci/devres.c | 53 ++--
drivers/pci/pci.c| 47 +--
drivers/pci/pci.h| 10 -
3 files changed, 45 insertions(+), 65 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 54b10f5433ab..f2a1250c0679 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -24,18 +24,15 @@
*
*Consequently, in the new API, region requests performed by the pcim_
*functions are automatically cleaned up through the devres callback
- *pcim_addr_resource_release(), while requests performed by
- *pcim_enable_device() + pci_*region*() are automatically cleaned up
- *through the for-loop in pcim_release().
+ *pcim_addr_resource_release().
+ *Users utilizing pcim_enable_device() + pci_*region*() are redirected in
+ *pci.c to the managed functions here in this file. This isn't exactly
+ *perfect, but the only alternative way would be to port ALL drivers using
+ *said combination to pcim_ functions.
*
- * TODO 1:
+ * TODO:
* Remove the legacy table entirely once all calls to pcim_iomap_table() in
* the kernel have been removed.
- *
- * TODO 2:
- * Port everyone calling pcim_enable_device() + pci_*region*() to using the
- * pcim_ functions. Then, remove all devres functionality from pci_*region*()
- * functions and remove the associated cleanups described above in point #2.
*/
/*
@@ -399,22 +396,6 @@ static void pcim_release(struct device *gendev, void *res)
{
struct pci_dev *dev = to_pci_dev(gendev);
struct pci_devres *this = res;
- int i;
-
- /*
-* This is legacy code.
-*
-* All regions requested by a pcim_ function do get released through
-* pcim_addr_resource_release(). Thanks to the hybrid nature of the pci_
-* region-request functions, this for-loop has to release the regions
-* if they have been requested by such a function.
-*
-* TODO: Remove this once all users of pcim_enable_device() PLUS
-* pci-region-request-functions have been ported to pcim_ functions.
-*/
- for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
- if (mask_contains_bar(this->region_mask, i))
- pci_release_region(dev, i);
if (this->mwi)
pci_clear_mwi(dev);
@@ -823,11 +804,29 @@ static int _pcim_request_region(struct pci_dev *pdev, int
bar, const char *name,
* The region will automatically be released on driver detach. If desired,
* release manually only with pcim_release_region().
*/
-static int pcim_request_region(struct pci_dev *pdev, int bar, const char *name)
+int pcim_request_region(struct pci_dev *pdev, int bar, const char *name)
{
return _pcim_request_region(pdev, bar, name, 0);
}
+/**
+ * pcim_request_region_exclusive - Request a PCI BAR exclusively
+ * @pdev: PCI device to requestion region for
+ * @bar: Index of BAR to request
+ * @name: Name associated with the request
+ *
+ * Returns: 0 on success, a negative error code on failure.
+ *
+ * Request region specified by @bar exclusively.
+ *
+ * The region will automatically be released on driver detach. If desired,
+ * release manually only with pcim_release_region().
+ */
+int pcim_request_region_exclusive(struct pci_dev *pdev, int bar, const char
*name)
+{
+ return _pcim_request_region(pdev, bar, name, IORESOURCE_EXCLUSIVE);
+}
+
/**
* pcim_release_region - Release a PCI BAR
* @pdev: PCI device to operate on
@@ -836,7 +835,7 @@ static int pcim_request_region(struct pci_dev *pdev, int
bar, const char *name)
* Release a region manually that was previously requested by
* pcim_request_region().
*/
-static void pcim_release_region(struct pci_dev *pdev, int bar)
+void pcim_release_region(struct pci_dev *pdev, int bar)
{
struct pcim_addr_devres res_searched;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d94445f5f882..7013699db242 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3872,7 +3872,15 @@ EXPORT_SYMBOL(pci_enable_atomic_ops_to_root);
*/
void pci_release_region(struct pci_dev *pdev, int bar)
{
- struct pci_devres *dr;
+ /*
+* This is done for backwards compatibility, because the old PCI devres
+* API had a mode in which the function became managed if it had been
+* enabled with pcim_enabl
[PATCH v8 03/13] PCI: Reimplement plural devres functions
When the original PCI devres API was implemented, priority was given to the
creation of a set of "plural functions" such as pcim_request_regions().
These functions have bit masks as parameters to specify which BARs shall
get mapped. Most users, however, only use those to map 1-3 BARs.
A complete set of "singular functions" does not exist.
As functions mapping / requesting multiple BARs at once have (almost) no
mechanism in C to return the resources to the caller of the plural
function, the PCI devres API utilizes the iomap-table administrated by the
function pcim_iomap_table().
The entire PCI devres API was strongly tied to that table which only allows
for mapping whole, complete BARs, as the BAR's index is used as table
index. Consequently, it's not possible to, e.g., have a pcim_iomap_range()
function with that mechanism.
An additional problem is that the PCI devres API has been implemented in a
sort of "hybrid-mode": Some unmanaged functions have managed counterparts
(e.g.: pci_iomap() <-> pcim_iomap()), making their managed nature obvious
to the programmer. However, the region-request functions in pci.c, prefixed
with pci_, behave either managed or unmanaged, depending on whether
pci_enable_device() or pcim_enable_device() has been called in advance.
This hybrid API is confusing and should be more cleanly separated by
providing always-managed functions prefixed with pcim_.
Thus, the existing PCI devres API is not desirable because:
a) The vast majority of the users of the plural functions only ever sets
a single bit in the bit mask, consequently making them singular
functions anyways.
b) There is no mechanism to request / iomap only part of a BAR.
c) The iomap-table mechanism is over-engineered and complicated. Even
worse, some users index over the table administration function
directly, e.g.:
void __iomem *mapping = pcim_iomap_table(pdev)[my_index];
This can not perform bounds checks; an invalid index won't cause
return of -EINVAL or even NULL, resulting in undefined behavior.
d) region-request functions being sometimes managed and sometimes not
is bug-provoking.
Implement a set of internal helper functions that don't have the problem of
a hybrid nature that their counter parts in pci.c have. Write those helpers
in a generic manner so that they can easily be extended to, e.g., ranged
mappings and requests.
Implement a set of singular functions that use devres as it's intended and
use those singular functions to reimplement the plural functions.
Link: https://lore.kernel.org/r/20240605081605.18769-5-pstan...@redhat.com
Signed-off-by: Philipp Stanner
Signed-off-by: Bjorn Helgaas
---
drivers/pci/devres.c | 608 ++-
drivers/pci/pci.c| 22 ++
drivers/pci/pci.h| 5 +
3 files changed, 568 insertions(+), 67 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 845d6fab0ce7..82f71f5e164a 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -4,14 +4,243 @@
#include "pci.h"
/*
- * PCI iomap devres
+ * On the state of PCI's devres implementation:
+ *
+ * The older devres API for PCI has two significant problems:
+ *
+ * 1. It is very strongly tied to the statically allocated mapping table in
+ *struct pcim_iomap_devres below. This is mostly solved in the sense of the
+ *pcim_ functions in this file providing things like ranged mapping by
+ *bypassing this table, wheras the functions that were present in the old
+ *API still enter the mapping addresses into the table for users of the old
+ *API.
+ *
+ * 2. The region-request-functions in pci.c do become managed IF the device has
+ *been enabled with pcim_enable_device() instead of pci_enable_device().
+ *This resulted in the API becoming inconsistent: Some functions have an
+ *obviously managed counter-part (e.g., pci_iomap() <-> pcim_iomap()),
+ *whereas some don't and are never managed, while others don't and are
+ *_sometimes_ managed (e.g. pci_request_region()).
+ *
+ *Consequently, in the new API, region requests performed by the pcim_
+ *functions are automatically cleaned up through the devres callback
+ *pcim_addr_resource_release(), while requests performed by
+ *pcim_enable_device() + pci_*region*() are automatically cleaned up
+ *through the for-loop in pcim_release().
+ *
+ * TODO 1:
+ * Remove the legacy table entirely once all calls to pcim_iomap_table() in
+ * the kernel have been removed.
+ *
+ * TODO 2:
+ * Port everyone calling pcim_enable_device() + pci_*region*() to using the
+ * pcim_ functions. Then, remove all devres functionality from pci_*region*()
+ * functions and remove the associated cleanups described above in point #2.
*/
-#define PCIM_IOMAP_MAX PCI_STD_NUM_BARS
+/*
+ * Legacy struct storing addresses to whole mapped BARs.
+ */
struct pcim_iomap_devres {
- void __iomem *table[PCIM_IOMAP_MAX];
+ void __io
[PATCH v8 02/13] PCI: Add devres helpers for iomap table
The pcim_iomap_devres.table administrated by pcim_iomap_table() has its
entries set and unset at several places throughout devres.c using manual
iterations which are effectively code duplications.
Add pcim_add_mapping_to_legacy_table() and
pcim_remove_mapping_from_legacy_table() helper functions and use them where
possible.
Link: https://lore.kernel.org/r/20240605081605.18769-4-pstan...@redhat.com
Signed-off-by: Philipp Stanner
[bhelgaas: s/short bar/int bar/ for consistency]
Signed-off-by: Bjorn Helgaas
---
drivers/pci/devres.c | 77 +---
1 file changed, 58 insertions(+), 19 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index f13edd4a3873..845d6fab0ce7 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -297,6 +297,52 @@ void __iomem * const *pcim_iomap_table(struct pci_dev
*pdev)
}
EXPORT_SYMBOL(pcim_iomap_table);
+/*
+ * Fill the legacy mapping-table, so that drivers using the old API can
+ * still get a BAR's mapping address through pcim_iomap_table().
+ */
+static int pcim_add_mapping_to_legacy_table(struct pci_dev *pdev,
+ void __iomem *mapping, int bar)
+{
+ void __iomem **legacy_iomap_table;
+
+ if (bar >= PCI_STD_NUM_BARS)
+ return -EINVAL;
+
+ legacy_iomap_table = (void __iomem **)pcim_iomap_table(pdev);
+ if (!legacy_iomap_table)
+ return -ENOMEM;
+
+ /* The legacy mechanism doesn't allow for duplicate mappings. */
+ WARN_ON(legacy_iomap_table[bar]);
+
+ legacy_iomap_table[bar] = mapping;
+
+ return 0;
+}
+
+/*
+ * Remove a mapping. The table only contains whole-BAR mappings, so this will
+ * never interfere with ranged mappings.
+ */
+static void pcim_remove_mapping_from_legacy_table(struct pci_dev *pdev,
+ void __iomem *addr)
+{
+ int bar;
+ void __iomem **legacy_iomap_table;
+
+ legacy_iomap_table = (void __iomem **)pcim_iomap_table(pdev);
+ if (!legacy_iomap_table)
+ return;
+
+ for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
+ if (legacy_iomap_table[bar] == addr) {
+ legacy_iomap_table[bar] = NULL;
+ return;
+ }
+ }
+}
+
/**
* pcim_iomap - Managed pcim_iomap()
* @pdev: PCI device to iomap for
@@ -308,16 +354,20 @@ EXPORT_SYMBOL(pcim_iomap_table);
*/
void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen)
{
- void __iomem **tbl;
+ void __iomem *mapping;
- BUG_ON(bar >= PCIM_IOMAP_MAX);
-
- tbl = (void __iomem **)pcim_iomap_table(pdev);
- if (!tbl || tbl[bar]) /* duplicate mappings not allowed */
+ mapping = pci_iomap(pdev, bar, maxlen);
+ if (!mapping)
return NULL;
- tbl[bar] = pci_iomap(pdev, bar, maxlen);
- return tbl[bar];
+ if (pcim_add_mapping_to_legacy_table(pdev, mapping, bar) != 0)
+ goto err_table;
+
+ return mapping;
+
+err_table:
+ pci_iounmap(pdev, mapping);
+ return NULL;
}
EXPORT_SYMBOL(pcim_iomap);
@@ -330,20 +380,9 @@ EXPORT_SYMBOL(pcim_iomap);
*/
void pcim_iounmap(struct pci_dev *pdev, void __iomem *addr)
{
- void __iomem **tbl;
- int i;
-
pci_iounmap(pdev, addr);
- tbl = (void __iomem **)pcim_iomap_table(pdev);
- BUG_ON(!tbl);
-
- for (i = 0; i < PCIM_IOMAP_MAX; i++)
- if (tbl[i] == addr) {
- tbl[i] = NULL;
- return;
- }
- WARN_ON(1);
+ pcim_remove_mapping_from_legacy_table(pdev, addr);
}
EXPORT_SYMBOL(pcim_iounmap);
--
2.45.0
[PATCH v8 00/13] Make PCI's devres API more consistent
This v8 is based on [1]. It contains the already applied patches in unchanged form; just for readability and consistency. Thx for taking the first set of patches! This series provides the enabled_cnt rework and some other minor fixes. See below. [1] https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=devres P. Changes in v8: - Rebase the series on the already merged patches which were slightly modified by Bjorn Helgaas. - Reword the pci_intx() commit message so it clearly states it's about reworking pci_intx(). - Move the removal of find_pci_dr() from patch "Remove legacy pcim_release()" to patch "Give pci_intx() its own devres callback" since this later patch already removed all calls to that function. - In patch "Give pci_intx() its own devres callback": use pci_is_enabled() (and, thus, the enabled_cnt in struct pci_dev) instead of a separate enabled field. (Bjorn) Changes in v7: - Split the entire series in smaller, more atomic chunks / patches (Bjorn) - Remove functions (such as pcim_iomap_region_range()) that do not yet have a user (Bjorn) - Don't export interfaces publicly anymore, except for pcim_iomap_range(), needed by vboxvideo (Bjorn) - Mention the actual (vboxvideo) bug in "PCI: Warn users..." commit (Bjorn) - Drop docstring warnings on PCI-internal functions (Bjorn) - Rework docstring warnings - Fix spelling in a few places. Rewrapp paragraphs (Bjorn) Changes in v6: - Restructure the cleanup in pcim_iomap_regions_request_all() so that it doesn't trigger a (false positive) test robot warning. No behavior change intended. (Dan Carpenter) Changes in v5: - Add Hans's Reviewed-by to vboxvideo patch (Hans de Goede) - Remove stable-kernel from CC in vboxvideo patch (Hans de Goede) Changes in v4: - Rebase against linux-next Changes in v3: - Use the term "PCI devres API" at some forgotten places. - Fix more grammar errors in patch #3. - Remove the comment advising to call (the outdated) pcim_intx() in pci.c - Rename __pcim_request_region_range() flags-field "exclusive" to "req_flags", since this is what the int actually represents. - Remove the call to pcim_region_request() from patch #10. (Hans) Changes in v2: - Make commit head lines congruent with PCI's style (Bjorn) - Add missing error checks for devm_add_action(). (Andy) - Repair the "Returns: " marks for docu generation (Andy) - Initialize the addr_devres struct with memset(). (Andy) - Make pcim_intx() a PCI-internal function so that new drivers won't be encouraged to use the outdated pci_intx() mechanism. (Andy / Philipp) - Fix grammar and spelling (Bjorn) - Be more precise on why pcim_iomap_table() is problematic (Bjorn) - Provide the actual structs' and functions' names in the commit messages (Bjorn) - Remove redundant variable initializers (Andy) - Regroup PM bitfield members in struct pci_dev (Andy) - Make pcim_intx() visible only for the PCI subsystem so that new drivers won't use this outdated API (Andy, Myself) - Add a NOTE to pcim_iomap() to warn about this function being theonee xception that does just return NULL. - Consistently use the term "PCI devres API"; also in Patch #10 (Bjorn) ¡Hola! PCI's devres API suffers several weaknesses: 1. There are functions prefixed with pcim_. Those are always managed counterparts to never-managed functions prefixed with pci_ – or so one would like to think. There are some apparently unmanaged functions (all region-request / release functions, and pci_intx()) which suddenly become managed once the user has initialized the device with pcim_enable_device() instead of pci_enable_device(). This "sometimes yes, sometimes no" nature of those functions is confusing and therefore bug-provoking. In fact, it has already caused a bug in DRM. The last patch in this series fixes that bug. 2. iomappings: Instead of giving each mapping its own callback, the existing API uses a statically allocated struct tracking one mapping per bar. This is not extensible. Especially, you can't create _ranged_ managed mappings that way, which many drivers want. 3. Managed request functions only exist as "plural versions" with a bit-mask as a parameter. That's quite over-engineered considering that each user only ever mapps one, maybe two bars. This series: - add a set of new "singular" devres functions that use devres the way its intended, with one callback per resource. - deprecates the existing iomap-table mechanism. - deprecates the hybrid nature of pci_ functions. - preserves backwards compatibility so that drivers using the existing API won't notice any changes. - adds documentation, especially some warning users about the complicated nature of PCI's devres. Note that this series is based on my "unify pci_iounmap"-series from a few weeks ago. [1] I tested this on a x86 VM with a simple pci test-device wit
Re: [PATCH] drm/i915/gt: debugfs: Evaluate forcewake usage within locks
Hi Andi,
On 6/7/2024 4:51 PM, Andi Shyti wrote:
The forcewake count and domains listing is multi process critical
and the uncore provides a spinlock for such cases.
Lock the forcewake evaluation section in the fw_domains_show()
debugfs interface.
Signed-off-by: Andi Shyti
Needs a Fixes tag, below seems to be correct one.
Fixes: 9dd4b065446a ("drm/i915/gt: Move pm debug files into a gt aware
debugfs")
Cc: # v5.6+
Reviewed-by: Nirmoy Das
Regards,
Nirmoy
---
drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
index 4fcba42cfe34..0437fd8217e0 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
@@ -71,6 +71,8 @@ static int fw_domains_show(struct seq_file *m, void *data)
struct intel_uncore_forcewake_domain *fw_domain;
unsigned int tmp;
+ spin_lock_irq(&uncore->lock);
+
seq_printf(m, "user.bypass_count = %u\n",
uncore->user_forcewake_count);
@@ -79,6 +81,8 @@ static int fw_domains_show(struct seq_file *m, void *data)
intel_uncore_forcewake_domain_to_str(fw_domain->id),
READ_ONCE(fw_domain->wake_count));
+ spin_unlock_irq(&uncore->lock);
+
return 0;
}
DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(fw_domains);
Re: [PATCH 04/11] drm/exynos: hdmi: convert to struct drm_edid
Hi, Jani Nikula,
Thanks for your contribution and sorry for being late. Below are my comments.
2024년 5월 14일 (화) 오후 9:57, Jani Nikula 님이 작성:
>
> Prefer the struct drm_edid based functions for reading the EDID and
> updating the connector.
>
> The functional change is that the CEC physical address gets invalidated
> when the EDID could not be read.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> Cc: Inki Dae
> Cc: Seung-Woo Kim
> Cc: Kyungmin Park
> Cc: Krzysztof Kozlowski
> Cc: Alim Akhtar
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c | 23 +--
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index e968824a4c72..9033e8b66816 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -883,27 +883,30 @@ static const struct drm_connector_funcs
> hdmi_connector_funcs = {
> static int hdmi_get_modes(struct drm_connector *connector)
> {
> struct hdmi_context *hdata = connector_to_hdmi(connector);
> - struct edid *edid;
> + const struct drm_display_info *info = &connector->display_info;
> + const struct drm_edid *drm_edid;
> int ret;
>
> if (!hdata->ddc_adpt)
> return 0;
>
> - edid = drm_get_edid(connector, hdata->ddc_adpt);
> - if (!edid)
> + drm_edid = drm_edid_read_ddc(connector, hdata->ddc_adpt);
drm_edid_read_ddc function can return NULL for an error. Could you add
an exception handling?
> +
> + drm_edid_connector_update(connector, drm_edid);
Ditto. drm_edid_connector_update function can return a negative value
for an error.
> +
> + cec_notifier_set_phys_addr(hdata->notifier,
> info->source_physical_address);
> +
> + if (!drm_edid)
> return 0;
>
> - hdata->dvi_mode = !connector->display_info.is_hdmi;
> + hdata->dvi_mode = !info->is_hdmi;
Above change wouldn't be related to this patch.
> DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n",
> (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
> - edid->width_cm, edid->height_cm);
> -
> - drm_connector_update_edid_property(connector, edid);
> - cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);
> + info->width_mm / 10, info->height_mm / 10);
The purpose of this patch would be to replace edid with drm_edid so
how about updating the above change like below?
drm_edid->edid->width_cm, erm_edid->edid->height_cm);
Thanks,
Inki Dae
>
> - ret = drm_add_edid_modes(connector, edid);
> + ret = drm_edid_connector_add_modes(connector);
>
> - kfree(edid);
> + drm_edid_free(drm_edid);
>
> return ret;
> }
> --
> 2.39.2
>
>
[PATCH v3 21/21] iommu: Remove iommu_domain_alloc()
The iommu_domain_alloc() interface is no longer used in the tree anymore.
Remove it to avoid dead code.
There is increasing demand for supporting multiple IOMMU drivers, and this
is the last bus-based thing standing in the way of that.
Signed-off-by: Lu Baolu
---
include/linux/iommu.h | 6 --
drivers/iommu/iommu.c | 36
2 files changed, 42 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ba0b27afc256..bff6e7e81fa3 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -778,7 +778,6 @@ static inline void iommu_iotlb_gather_init(struct
iommu_iotlb_gather *gather)
extern int bus_iommu_probe(const struct bus_type *bus);
extern bool device_iommu_capable(struct device *dev, enum iommu_cap cap);
extern bool iommu_group_has_isolated_msi(struct iommu_group *group);
-extern struct iommu_domain *iommu_domain_alloc(const struct bus_type *bus);
struct iommu_domain *iommu_paging_domain_alloc(struct device *dev);
extern void iommu_domain_free(struct iommu_domain *domain);
extern int iommu_attach_device(struct iommu_domain *domain,
@@ -1076,11 +1075,6 @@ static inline bool device_iommu_capable(struct device
*dev, enum iommu_cap cap)
return false;
}
-static inline struct iommu_domain *iommu_domain_alloc(const struct bus_type
*bus)
-{
- return NULL;
-}
-
static inline struct iommu_domain *iommu_paging_domain_alloc(struct device
*dev)
{
return ERR_PTR(-ENODEV);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index c7ebdf96e4bc..8212fed27e18 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1975,42 +1975,6 @@ __iommu_group_domain_alloc(struct iommu_group *group,
unsigned int type)
return __iommu_domain_alloc(dev_iommu_ops(dev), dev, type);
}
-static int __iommu_domain_alloc_dev(struct device *dev, void *data)
-{
- const struct iommu_ops **ops = data;
-
- if (!dev_has_iommu(dev))
- return 0;
-
- if (WARN_ONCE(*ops && *ops != dev_iommu_ops(dev),
- "Multiple IOMMU drivers present for bus %s, which the
public IOMMU API can't fully support yet. You will still need to disable one or
more for this to work, sorry!\n",
- dev_bus_name(dev)))
- return -EBUSY;
-
- *ops = dev_iommu_ops(dev);
- return 0;
-}
-
-/*
- * The iommu ops in bus has been retired. Do not use this interface in
- * new drivers.
- */
-struct iommu_domain *iommu_domain_alloc(const struct bus_type *bus)
-{
- const struct iommu_ops *ops = NULL;
- int err = bus_for_each_dev(bus, NULL, &ops, __iommu_domain_alloc_dev);
- struct iommu_domain *domain;
-
- if (err || !ops)
- return NULL;
-
- domain = __iommu_domain_alloc(ops, NULL, IOMMU_DOMAIN_UNMANAGED);
- if (IS_ERR(domain))
- return NULL;
- return domain;
-}
-EXPORT_SYMBOL_GPL(iommu_domain_alloc);
-
/**
* iommu_paging_domain_alloc() - Allocate a paging domain
* @dev: device for which the domain is allocated
--
2.34.1
[PATCH v3 20/21] iommu: Remove iommu_present()
The iommu_present() interface is no longer used in the tree anymore.
Remove it to avoid dead code.
Signed-off-by: Lu Baolu
---
include/linux/iommu.h | 6 --
drivers/iommu/iommu.c | 25 -
2 files changed, 31 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 46f1348f1f0a..ba0b27afc256 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -776,7 +776,6 @@ static inline void iommu_iotlb_gather_init(struct
iommu_iotlb_gather *gather)
}
extern int bus_iommu_probe(const struct bus_type *bus);
-extern bool iommu_present(const struct bus_type *bus);
extern bool device_iommu_capable(struct device *dev, enum iommu_cap cap);
extern bool iommu_group_has_isolated_msi(struct iommu_group *group);
extern struct iommu_domain *iommu_domain_alloc(const struct bus_type *bus);
@@ -1072,11 +1071,6 @@ struct iommu_iotlb_gather {};
struct iommu_dirty_bitmap {};
struct iommu_dirty_ops {};
-static inline bool iommu_present(const struct bus_type *bus)
-{
- return false;
-}
-
static inline bool device_iommu_capable(struct device *dev, enum iommu_cap cap)
{
return false;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e03c71a34347..c7ebdf96e4bc 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1846,31 +1846,6 @@ int bus_iommu_probe(const struct bus_type *bus)
return 0;
}
-/**
- * iommu_present() - make platform-specific assumptions about an IOMMU
- * @bus: bus to check
- *
- * Do not use this function. You want device_iommu_mapped() instead.
- *
- * Return: true if some IOMMU is present and aware of devices on the given bus;
- * in general it may not be the only IOMMU, and it may not have anything to do
- * with whatever device you are ultimately interested in.
- */
-bool iommu_present(const struct bus_type *bus)
-{
- bool ret = false;
-
- for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++) {
- if (iommu_buses[i] == bus) {
- spin_lock(&iommu_device_lock);
- ret = !list_empty(&iommu_device_list);
- spin_unlock(&iommu_device_lock);
- }
- }
- return ret;
-}
-EXPORT_SYMBOL_GPL(iommu_present);
-
/**
* device_iommu_capable() - check for a general IOMMU capability
* @dev: device to which the capability would be relevant, if available
--
2.34.1
[PATCH v3 19/21] drm/tegra: Remove call to iommu_domain_alloc()
Commit <17de3f5fdd35> ("iommu: Retire bus ops") removes iommu ops from
the bus structure. The iommu subsystem no longer relies on bus for
operations. So iommu_domain_alloc() interface is no longer relevant.
Normally, iommu_paging_domain_alloc() could be a replacement for
iommu_domain_alloc() if the caller has the right device for IOMMU API
use. Unfortunately, this is not the case for this driver.
Iterate the devices on the platform bus and find a suitable device
whose device DMA is translated by an IOMMU. Then use this device to
allocate an iommu domain. The iommu subsystem prevents domains
allocated by one iommu driver from being attached to devices managed
by any different iommu driver.
Signed-off-by: Lu Baolu
---
drivers/gpu/drm/tegra/drm.c | 34 +-
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 03d1c76aec2d..ee391f859992 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1133,6 +1133,17 @@ static bool host1x_drm_wants_iommu(struct host1x_device
*dev)
return domain != NULL;
}
+static int iommu_mapped_device(struct device *dev, void *data)
+{
+ struct device **iommu_dev = data;
+
+ if (!device_iommu_mapped(dev))
+ return 0;
+
+ *iommu_dev = dev;
+ return 1;
+}
+
static int host1x_drm_probe(struct host1x_device *dev)
{
struct tegra_drm *tegra;
@@ -1149,16 +1160,21 @@ static int host1x_drm_probe(struct host1x_device *dev)
goto put;
}
- if (host1x_drm_wants_iommu(dev) && iommu_present(&platform_bus_type)) {
- tegra->domain = iommu_domain_alloc(&platform_bus_type);
- if (!tegra->domain) {
- err = -ENOMEM;
- goto free;
+ if (host1x_drm_wants_iommu(dev)) {
+ struct device *iommu_dev = NULL;
+
+ bus_for_each_dev(&platform_bus_type, NULL, &iommu_dev,
iommu_mapped_device);
+ if (iommu_dev) {
+ tegra->domain = iommu_paging_domain_alloc(iommu_dev);
+ if (IS_ERR(tegra->domain)) {
+ err = PTR_ERR(tegra->domain);
+ goto free;
+ }
+
+ err = iova_cache_get();
+ if (err < 0)
+ goto domain;
}
-
- err = iova_cache_get();
- if (err < 0)
- goto domain;
}
mutex_init(&tegra->clients_lock);
--
2.34.1
