date:20250306

From: Dmitry Baryshkov 


On Wed, 26 Feb 2025 10:59:23 +0200, Dmitry Baryshkov wrote:
> This patchset sits on top Maxime's HDMI connector patchset ([1]).
> 
> Currently this is an RFC exploring the interface between HDMI bridges
> and HDMI connector code. This has been lightly verified on the Qualcomm
> DB820c, which has native HDMI output. If this approach is considered to
> be acceptable, I'll finish MSM HDMI bridge conversion (reworking the
> Audio Infoframe code). Other bridges can follow the same approach (we
> have lt9611 / lt9611uxc / adv7511 on Qualcomm hardware).
> 
> [...]

Applied, thanks!

[1/7] drm/msm/hdmi: switch to atomic bridge callbacks
  https://gitlab.freedesktop.org/lumag/msm/-/commit/8ae7192e7a00
[2/7] drm/msm/hdmi: program HDMI timings during atomic_pre_enable
  https://gitlab.freedesktop.org/lumag/msm/-/commit/d309bda67172
[3/7] drm/msm/hdmi: make use of the drm_connector_hdmi framework
  https://gitlab.freedesktop.org/lumag/msm/-/commit/384d2b03d0a1
[4/7] drm/msm/hdmi: get rid of hdmi_mode
  https://gitlab.freedesktop.org/lumag/msm/-/commit/d840a2162112
[5/7] drm/msm/hdmi: update HDMI_GEN_PKT_CTRL_GENERIC0_UPDATE definition
  https://gitlab.freedesktop.org/lumag/msm/-/commit/e92573638792
[6/7] drm/msm/hdmi: also send the SPD and HDMI Vendor Specific InfoFrames
  https://gitlab.freedesktop.org/lumag/msm/-/commit/d7d57ecfcf52
[7/7] drm/msm/hdmi: use DRM HDMI Audio framework
  https://gitlab.freedesktop.org/lumag/msm/-/commit/ea54cfac0f8c

Best regards,
-- 
Dmitry Baryshkov

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

2025-03-06 Thread H. Peter Anvin

On March 6, 2025 8:25:25 AM PST, Kuan-Wei Chiu  wrote:
>Several parts of the kernel contain redundant implementations of parity
>calculations for 16/32/64-bit values. Introduces generic
>parity16/32/64() helpers in bitops.h, providing a standardized
>and optimized implementation. 
>
>Subsequent patches refactor various kernel components to replace
>open-coded parity calculations with the new helpers, reducing code
>duplication and improving maintainability.
>
>Co-developed-by: Yu-Chun Lin 
>Signed-off-by: Yu-Chun Lin 
>Signed-off-by: Kuan-Wei Chiu 
>---
>In v3, I use parityXX() instead of the parity() macro since the
>parity() macro may generate suboptimal code and requires special hacks
>to make GCC happy. If anyone still prefers a single parity() macro,
>please let me know.
>
>Additionally, I changed parityXX() << y users to !!parityXX() << y
>because, unlike C++, C does not guarantee that true casts to int as 1.
>
>Changes in v3:
>- Avoid using __builtin_parity.
>- Change return type to bool.
>- Drop parity() macro.
>- Change parityXX() << y to !!parityXX() << y.
>
>
>Changes in v2:
>- Provide fallback functions for __builtin_parity() when the compiler
>  decides not to inline it
>- Use __builtin_parity() when no architecture-specific implementation
>  is available
>- Optimize for constant folding when val is a compile-time constant
>- Add a generic parity() macro
>- Drop the x86 bootflag conversion patch since it has been merged into
>  the tip tree
>
>v1: https://lore.kernel.org/lkml/20250223164217.2139331-1-visitor...@gmail.com/
>v2: https://lore.kernel.org/lkml/20250301142409.2513835-1-visitor...@gmail.com/
>
>Kuan-Wei Chiu (16):
>  bitops: Change parity8() return type to bool
>  bitops: Add parity16(), parity32(), and parity64() helpers
>  media: media/test_drivers: Replace open-coded parity calculation with
>parity8()
>  media: pci: cx18-av-vbi: Replace open-coded parity calculation with
>parity8()
>  media: saa7115: Replace open-coded parity calculation with parity8()
>  serial: max3100: Replace open-coded parity calculation with parity8()
>  lib/bch: Replace open-coded parity calculation with parity32()
>  Input: joystick - Replace open-coded parity calculation with
>parity32()
>  net: ethernet: oa_tc6: Replace open-coded parity calculation with
>parity32()
>  wifi: brcm80211: Replace open-coded parity calculation with parity32()
>  drm/bridge: dw-hdmi: Replace open-coded parity calculation with
>parity32()
>  mtd: ssfdc: Replace open-coded parity calculation with parity32()
>  fsi: i2cr: Replace open-coded parity calculation with parity32()
>  fsi: i2cr: Replace open-coded parity calculation with parity64()
>  Input: joystick - Replace open-coded parity calculation with
>parity64()
>  nfp: bpf: Replace open-coded parity calculation with parity64()
>
> drivers/fsi/fsi-master-i2cr.c | 18 ++-
> .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  8 +--
> drivers/input/joystick/grip_mp.c  | 17 +-
> drivers/input/joystick/sidewinder.c   | 24 ++---
> drivers/media/i2c/saa7115.c   | 12 +
> drivers/media/pci/cx18/cx18-av-vbi.c  | 12 +
> .../media/test-drivers/vivid/vivid-vbi-gen.c  |  8 +--
> drivers/mtd/ssfdc.c   | 20 ++-
> drivers/net/ethernet/netronome/nfp/nfp_asm.c  |  7 +--
> drivers/net/ethernet/oa_tc6.c | 19 ++-
> .../broadcom/brcm80211/brcmsmac/dma.c | 16 +-
> drivers/tty/serial/max3100.c  |  3 +-
> include/linux/bitops.h| 52 +--
> lib/bch.c | 14 +
> 14 files changed, 77 insertions(+), 153 deletions(-)
>

(int)true most definitely is guaranteed to be 1.

Re: [PATCH 1/2] drm/bridge: imx8qxp-ldb: cleanup return value

On 03/07/2025, Luca Ceresoli wrote:
> 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> ret;'. So return 0 for clarity.
> 
> Signed-off-by: Luca Ceresoli 
> ---
>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c 
> b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> index 
> 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e
>  100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
>  
>   ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
>  
> - return ret;
> + return 0;

I guess this is not the only place across the kernel tree where this cleanup
could be done.  So, maybe use some tools to cleanup them all?

>  }
>  
>  static void imx8qxp_ldb_remove(struct platform_device *pdev)
> 

-- 
Regards,
Liu Ying

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

2025-03-06 Thread Jiri Slaby


On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:

Change return type to bool for better clarity. Update the kernel doc
comment accordingly, including fixing "@value" to "@val" and adjusting
examples. Also mark the function with __attribute_const__ to allow
potential compiler optimizations.

Co-developed-by: Yu-Chun Lin 
Signed-off-by: Yu-Chun Lin 
Signed-off-by: Kuan-Wei Chiu 
---
  include/linux/bitops.h | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index c1cb53cf2f0f..44e5765b8bec 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
  
  /**

   * parity8 - get the parity of an u8 value
- * @value: the value to be examined
+ * @val: the value to be examined
   *
   * Determine the parity of the u8 argument.
   *
   * Returns:
- * 0 for even parity, 1 for odd parity
+ * false for even parity, true for odd parity


This occurs somehow inverted to me. When something is in parity means 
that it has equal number of 1s and 0s. I.e. return true for even 
distribution. Dunno what others think? Or perhaps this should be dubbed 
odd_parity() when bool is returned? Then you'd return true for odd.


thanks,
--
js
suse labs

Re: [PATCH -next] drm/msm/dpu: Remove duplicate dpu_hw_cwb.h header

On Fri, Mar 07, 2025 at 09:50:30AM +0800, Jiapeng Chong wrote:
> ./drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c: dpu_hw_cwb.h is included more 
> than once.
> 
> Reported-by: Abaci Robot 
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=19239
> Signed-off-by: Jiapeng Chong 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 1 -
>  1 file changed, 1 deletion(-)
> 

Reported-by: kernel test robot 
Closes: https://lore.kernel.org/r/202503070155.tuungwd3-...@intel.com/


Reviewed-by: Dmitry Baryshkov 


-- 
With best wishes
Dmitry

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

2025-03-06 Thread Jiri Slaby


On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:

Several parts of the kernel contain redundant implementations of parity
calculations for 16/32/64-bit values. Introduces generic
parity16/32/64() helpers in bitops.h, providing a standardized
and optimized implementation.

Subsequent patches refactor various kernel components to replace
open-coded parity calculations with the new helpers, reducing code
duplication and improving maintainability.

Co-developed-by: Yu-Chun Lin 
Signed-off-by: Yu-Chun Lin 
Signed-off-by: Kuan-Wei Chiu 
---
In v3, I use parityXX() instead of the parity() macro since the
parity() macro may generate suboptimal code and requires special hacks
to make GCC happy. If anyone still prefers a single parity() macro,
please let me know.


What is suboptimal and where exactly it matters? Have you actually 
measured it?



Additionally, I changed parityXX() << y users to !!parityXX() << y
because, unlike C++, C does not guarantee that true casts to int as 1.


How comes? ANSI C99 exactly states:
===
true
which expands to the integer constant 1,
===

thanks,
--
js
suse labs

Re: [RFC PATCH 0/3] gpu: nova-core: add basic timer subdevice implementation

2025-03-06 Thread Jason Gunthorpe

On Thu, Mar 06, 2025 at 11:42:38AM +0100, Simona Vetter wrote:
> > Further, I just remembered, (Danilo please notice!) there is another
> > related issue here that DMA mappings *may not* outlive remove()
> > either. netdev had a bug related to this recently and it was all
> > agreed that it is not allowed. The kernel can crash in a couple of
> > different ways if you try to do this.
> > 
> > https://lore.kernel.org/lkml/8067f204-1380-4d37-8ffd-007fc6f26...@kernel.org/T/#m0c7dda0fb5981240879c5ca489176987d688844c
> 
> Hm for the physical dma I thought disabling pci bust master should put a
> stop to all this stuff?

Not in the general case. Many device classes (eg platform) don't have
something like "PCI bus master". It is also not always possible to
reset a device, even in PCI.

So the way things work today for module reload relies on the driver
duing a full quiet down so that the next driver to attach can safely
start up the device. Otherwise the next driver flips PCI bus master
back on and immediately UAFs memory through rouge DMA.

Relying on PCI Bus master also exposes a weakness we battled with in
kexec. When the new driver boots up it has to gain control of the
device and stop the DMA before flipping "PCI Bus Master" off. Almost
no drivers actually do this, and some HW can't even achieve it without
PCI reset (which is not always available). Meaning you end up with a
likely UAF flow if you rely on this technique.

> For the sw lifecycle stuff I honestly didn't know that was an issue, I
> guess that needs to be adressed in the dma-api wrappers or rust can blow
> up in funny ways. C drivers just walk all mappings and shoot them.

I wonder what people will come up with. DMA API is performance path,
people are not going to accept pointless overheads there.

IMHO whatever path the DMA API takes the MMIO design should follow
it.

> The trouble is that for real hotunplug, you need all this anyway. Because
> when you physically hotunplug the interrupts will be dead, the mmio will
> be gone any momem (not just at the beginnning of an rcu revocable
> section), so real hotunplug is worse than what we're trying to do here.

There are two kinds of real hotunplug, the friendly kind that we see
in physical PCI where you actually plonk a button on the case and wait
for the light to go off. Ie it is interactive and safe with the
OS. Very similar to module reloading.

And the hostile kind, like in thunderbolt, where it just goes away and
dies.

In the server world, other than nvme, we seem to focus on the friendly
kind.

> So randomly interrupts not happening is something you need to cope with no
> matter what.

Yes

> But for a driver unbind you _do_ have to worry about cleanly shutting down
> the hardware. For the above reasons and also in general putting hardware
> into a well-known (all off usually) state is better for then reloading a
> new driver version and binding that. Except that there's no way to tell
> whether your ->remove got called for unbinding or hotunplug.

IMHO it doesn't really matter, the driver has to support the most
difficult scenario anyhow. The only practical difference is that the
MMIO might return -1 to all reads and the interrupts are dead. If you
want to detect a gone PCI device then just do a register read and
check for -1, which some drivers like mlx5 are doing as part of their
resiliency strategy.

> pci device was physically unplugged or not, and so for developer
> convenience most pci drivers go with the "cleanly shut down everything"
> approach, which is the wrong thing to do for actual hotunplug.

I wouldn't say it is wrong. It is still the correct thing to do, and
following down the normal cleanup paths is a good way to ensure the
special case doesn't have bugs. The primary difference is you want to
understand the device is dead and stop waiting on it faster. Drivers
need to consider these things anyhow if they want resiliency against
device crashes, PCI link wobbles and so on that don't involve
remove().

Regardless, I think the point is clear that the driver author bears
alot of responsibility to sequence this stuff correctly as part of
their remove() implementation. The idea that Rust can magically make
all this safe against UAF or lockups seems incorrect.

> > Ah.. I guess rust would have to validate the function pointers and the
> > THIS_MODULE are consistent at runtime time before handing them off to
> > C to prevent this. Seems like a reasonable thing to put under some
> > CONFIG_DEBUG, also seems a bit hard to implement perhaps..
> 
> We should know the .text section of a module, so checking whether a
> pointer is within that shouldn't be too hard.

It is legal to pass a pointer to a function in a module that this
module is linked to as well. We do that sometimes.. Eg a fops having a
simple_xx pointer. So you'd need to do some graph analysis.

Jason

Re: [PATCH v6 00/14] drm/msm/dpu: Add Concurrent Writeback Support for DPU 10.x+

From: Dmitry Baryshkov 


On Fri, 14 Feb 2025 16:14:23 -0800, Jessica Zhang wrote:
> DPU supports a single writeback session running concurrently with primary
> display when the CWB mux is configured properly. This series enables
> clone mode for DPU driver and adds support for programming the CWB mux
> in cases where the hardware has dedicated CWB pingpong blocks. Currently,
> the CWB hardware blocks have only been added to the SM8650
> hardware catalog and only DSI has been exposed as a possible_clone of WB.
> 
> [...]

Applied, thanks!

[01/14] drm/msm/dpu: fill CRTC resources in dpu_crtc.c
https://gitlab.freedesktop.org/lumag/msm/-/commit/17666e764f38
[02/14] drm/msm/dpu: move resource allocation to CRTC
https://gitlab.freedesktop.org/lumag/msm/-/commit/1ce69c265a53
[03/14] drm/msm/dpu: switch RM to use crtc_id rather than enc_id for allocation
https://gitlab.freedesktop.org/lumag/msm/-/commit/cae6a13a71f7
[04/14] drm/msm/dpu: Add CWB to msm_display_topology
https://gitlab.freedesktop.org/lumag/msm/-/commit/2ea34682263b
[05/14] drm/msm/dpu: Require modeset if clone mode status changes
https://gitlab.freedesktop.org/lumag/msm/-/commit/20972609d12c
[06/14] drm/msm/dpu: Fail atomic_check if multiple outputs request CDM block
https://gitlab.freedesktop.org/lumag/msm/-/commit/f1f0379e9dd5
[07/14] drm/msm/dpu: Reserve resources for CWB
https://gitlab.freedesktop.org/lumag/msm/-/commit/5008375443ed
[08/14] drm/msm/dpu: Configure CWB in writeback encoder
https://gitlab.freedesktop.org/lumag/msm/-/commit/dd331404ac7c
[09/14] drm/msm/dpu: Support CWB in dpu_hw_ctl
https://gitlab.freedesktop.org/lumag/msm/-/commit/0f3801d666fe
[10/14] drm/msm/dpu: Adjust writeback phys encoder setup for CWB
https://gitlab.freedesktop.org/lumag/msm/-/commit/3371005e28e8
[11/14] drm/msm/dpu: Start frame done timer after encoder kickoff
https://gitlab.freedesktop.org/lumag/msm/-/commit/95bbde1d0d07
[12/14] drm/msm/dpu: Skip trigger flush and start for CWB
https://gitlab.freedesktop.org/lumag/msm/-/commit/8144d17a81d9
[13/14] drm/msm/dpu: Reorder encoder kickoff for CWB
https://gitlab.freedesktop.org/lumag/msm/-/commit/ad06972d5365
[14/14] drm/msm/dpu: Set possible clones for all encoders
https://gitlab.freedesktop.org/lumag/msm/-/commit/e8cd8224a307

Best regards,
-- 
Dmitry Baryshkov

Re:Re: [PATCH 0/6] Add support for RK3588 DisplayPort Controller

2025-03-06 Thread Andy Yan


Hi Piotr,
在 2025-03-06 22:28:08，"Piotr Oniszczuk"  写道：
>
>
>> Wiadomość napisana przez Piotr Oniszczuk  w dniu 
>> 6 mar 2025, o godz. 15:08:
>> 
>> 
>> 
>>> Wiadomość napisana przez Andy Yan  w dniu 6 mar 2025, o 
>>> godz. 13:15:
>>> 
>>> Hi Piotr,
>>> 
>>> 
>>> 
>>> Then when you DP cable plugin, you can run command as bellow to see if the 
>>> driver detects the HPD:
>>> 
>>> # cat /sys/class/drm/card0-DP-1/status 
>>> connected
>>> # 
>>> 
>> 
>> 
>> Andy,
>> Thx!
>> 
>> With above changes i’m getting „connected”.
>> Also it looks crtc gets reasonable mode: 
>> https://gist.github.com/warpme/d6220e3cc502086a4c95f05bd9f9cf0c
>> 
>> Still black screen however…
>>   
>
>some additional data point: /sys/kernel/debug/dri/1/vop2/summary 
>
>
>working hdmi:
>
>Video Port1: ACTIVE
>Connector: HDMI-A-1
>bus_format[0]: Unknown
>output_mode[f] color_space[0]
>Display mode: 1920x1080p60
>clk[148500] real_clk[148500] type[48] flag[5]
>H: 1920 2008 2052 2200
>V: 1080 1084 1089 1125
>Cluster0-win0: ACTIVE
>win_id: 0
>format: XR24 little-endian (0x34325258) glb_alpha[0xff]
>rotate: xmirror: 0 ymirror: 0 rotate_90: 0 rotate_270: 0
>zpos: 0
>src: pos[0, 0] rect[1920 x 1080]
>dst: pos[0, 0] rect[1920 x 1080]
>buf[0]: addr: 0x017e1000 pitch: 7680 offset: 0
>Video Port2: DISABLED
>
>
>
>
>non-working DP:
>
>Video Port1: DISABLED
>Video Port2: ACTIVE
>Connector: DP-1
>bus_format[100a]: RGB888_1X24
>output_mode[f] color_space[0]
>Display mode: 1920x1080p60
>clk[148500] real_clk[148500] type[48] flag[5]
>H: 1920 2008 2052 2200
>V: 1080 1084 1089 1125
>Cluster1-win0: ACTIVE
>win_id: 1
>format: XR24 little-endian (0x34325258) glb_alpha[0xff]
>rotate: xmirror: 0 ymirror: 0 rotate_90: 0 rotate_270: 0
>zpos: 0
>src: pos[0, 0] rect[1920 x 1080]
>dst: pos[0, 0] rect[1920 x 1080]
>buf[0]: addr: 0x007ed000 pitch: 7680 offset: 0

>

All dump information currently appears to be correct, so I'm temporarily unsure 
why
there is no display on the monitor.
Maybe try some plug and unplug for the DP cable, or try another cable or 
monitor?

It seems that this board uses a DP to HDMI converter? Does this transmitter 
need a driver?

I won't be at my computer over the next two or three days, so any further 
replies to your email
might have to wait until next week.

[PATCH 03/11] dt-bindings: power: qcom,kpss-acc-v2: Add MSM8916 compatible

2025-03-06 Thread Konrad Dybcio

From: Konrad Dybcio 

MSM8916 seems to reuse the same hardware as MSM8974 and friends (for
whom this binding document was created). Add a new compatible for it.

Signed-off-by: Konrad Dybcio 
---
 Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml 
b/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml
index 
202a5d51ee88c7190805efe8f1bf493bdb69ec45..27dae49163fa0790ceb6fda8a5c674f739d4a41a
 100644
--- a/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml
+++ b/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml
@@ -18,7 +18,9 @@ description:
 
 properties:
   compatible:
-const: qcom,kpss-acc-v2
+enum:
+  - qcom,msm8916-kpss-acc
+  - qcom,kpss-acc-v2
 
   reg:
 items:

-- 
2.48.1

Re: [PATCH v2 0/4] drm/msm/dpu: follow rules for drm_atomic_helper_check_modeset()

From: Dmitry Baryshkov 


On Thu, 23 Jan 2025 14:43:32 +0200, Dmitry Baryshkov wrote:
> As pointed out by Simona, the drm_atomic_helper_check_modeset() and
> drm_atomic_helper_check() require the former function is rerun if the
> driver's callbacks modify crtc_state->mode_changed. MSM is one of the
> drivers which failed to follow this requirement.
> 
> Rework the MSM / DPU driver to follow the requirements of the
> drm_atomic_helper_check_modeset() helper function.
> 
> [...]

Applied, thanks!

[1/4] drm/msm/dpu: don't use active in atomic_check()
  https://gitlab.freedesktop.org/lumag/msm/-/commit/25b4614843bc
[2/4] drm/msm/dpu: move needs_cdm setting to dpu_encoder_get_topology()
  https://gitlab.freedesktop.org/lumag/msm/-/commit/7d39f5bb82c0
[3/4] drm/msm/dpu: simplify dpu_encoder_get_topology() interface
  https://gitlab.freedesktop.org/lumag/msm/-/commit/41921f231abf
[4/4] drm/msm/dpu: don't set crtc_state->mode_changed from atomic_check()
  https://gitlab.freedesktop.org/lumag/msm/-/commit/2dde2aadaed1

Best regards,
-- 
Dmitry Baryshkov

Re: [PATCH v2] dt-bindings: display/msm: qcom, sa8775p-mdss: Add missing eDP phy

From: Dmitry Baryshkov 


On Fri, 21 Feb 2025 16:13:11 +0100, Krzysztof Kozlowski wrote:
> The Qualcomm SA8775p MDSS display block comes with eDP phy, already used
> in DTS and already documented in phy/qcom,edp-phy.yaml binding.  Add the
> missing device node in the binding and extend example to silence
> dtbs_check warnings like:
> 
>   sa8775p-ride.dtb: display-subsystem@ae0: Unevaluated properties are not 
> allowed ('phy@aec2a00', 'phy@aec5a00' were unexpected)
> 
> [...]

Applied, thanks!

[1/1] dt-bindings: display/msm: qcom,sa8775p-mdss: Add missing eDP phy
  https://gitlab.freedesktop.org/lumag/msm/-/commit/d3169ce5251b

Best regards,
-- 
Dmitry Baryshkov

Re: [PATCH next] drm/msm/dpu: fix error pointer dereference in msm_kms_init_aspace()

From: Dmitry Baryshkov 


On Tue, 25 Feb 2025 10:30:26 +0300, Dan Carpenter wrote:
> If msm_gem_address_space_create() fails, then return right away.
> Otherwise it leads to a Oops when we dereference "aspace" on the next
> line.
> 
> 

Applied, thanks!

[1/1] drm/msm/dpu: fix error pointer dereference in msm_kms_init_aspace()
  https://gitlab.freedesktop.org/lumag/msm/-/commit/f9d1b528219b

Best regards,
-- 
Dmitry Baryshkov

Re: [PATCH 04/11] arm64: dts: qcom: msm8916: Fix KPSS ACC compatible

On Thu, Mar 06, 2025 at 07:11:16PM +0100, Konrad Dybcio wrote:
> From: Konrad Dybcio 
> 
> The current compatible has been used with no corresponding
> documentation. Replace it with one that has been documented.

qcom,msm8916-kpss-acc is also not documented. Most likely you meant
qcom,kpss-acc-v2

> 
> This has no functional effect, as these nodes' resources are only
> consumed through a phandle reference, anyway.
> 
> Signed-off-by: Konrad Dybcio 
> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi 
> b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 
> 8f35c9af18782aa1da7089988692e6588c4b7c5d..33a28f8163dda0e53f4176d61738ce175efc096c
>  100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -2574,7 +2574,7 @@ frame@b028000 {
>   };
>  
>   cpu0_acc: power-manager@b088000 {
> - compatible = "qcom,msm8916-acc";
> + compatible = "qcom,msm8916-kpss-acc";
>   reg = <0x0b088000 0x1000>;
>   status = "reserved"; /* Controlled by PSCI firmware */
>   };
> @@ -2586,7 +2586,7 @@ cpu0_saw: power-manager@b089000 {
>   };
>  
>   cpu1_acc: power-manager@b098000 {
> - compatible = "qcom,msm8916-acc";
> + compatible = "qcom,msm8916-kpss-acc";
>   reg = <0x0b098000 0x1000>;
>   status = "reserved"; /* Controlled by PSCI firmware */
>   };
> @@ -2598,7 +2598,7 @@ cpu1_saw: power-manager@b099000 {
>   };
>  
>   cpu2_acc: power-manager@b0a8000 {
> - compatible = "qcom,msm8916-acc";
> + compatible = "qcom,msm8916-kpss-acc";
>   reg = <0x0b0a8000 0x1000>;
>   status = "reserved"; /* Controlled by PSCI firmware */
>   };
> @@ -2610,7 +2610,7 @@ cpu2_saw: power-manager@b0a9000 {
>   };
>  
>   cpu3_acc: power-manager@b0b8000 {
> - compatible = "qcom,msm8916-acc";
> + compatible = "qcom,msm8916-kpss-acc";
>   reg = <0x0b0b8000 0x1000>;
>   status = "reserved"; /* Controlled by PSCI firmware */
>   };
> 
> -- 
> 2.48.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 10/11] arm64: dts: qcom: x1e80100-romulus: Drop clock-names from PS8830

On Thu, Mar 06, 2025 at 07:11:22PM +0100, Konrad Dybcio wrote:
> From: Konrad Dybcio 
> 
> The preemptively-merged node contains a property absent from the final
> bindings. Remove it.
> 
> Signed-off-by: Konrad Dybcio 
> ---
>  arch/arm64/boot/dts/qcom/x1e80100-microsoft-romulus.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 

Fixes: b16ee3d0cda4 ("arm64: dts: qcom: x1e80100-romulus: Set up PS8830s")
Reviewed-by: Dmitry Baryshkov 


-- 
With best wishes
Dmitry

[PATCH v5 2/2] drm/msm/dp: reuse generic HDMI codec implementation

From: Dmitry Baryshkov 

The MSM DisplayPort driver implements several HDMI codec functions
in the driver, e.g. it manually manages HDMI codec device registration,
returning ELD and plugged_cb support. In order to reduce code
duplication reuse drm_hdmi_audio_* helpers and drm_bridge_connector
integration.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/Kconfig |   1 +
 drivers/gpu/drm/msm/dp/dp_audio.c   | 131 
 drivers/gpu/drm/msm/dp/dp_audio.h   |  27 ++--
 drivers/gpu/drm/msm/dp/dp_display.c |  28 ++--
 drivers/gpu/drm/msm/dp/dp_display.h |   6 --
 drivers/gpu/drm/msm/dp/dp_drm.c |   8 +++
 6 files changed, 31 insertions(+), 170 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 
974bc7c0ea761147d3326bdce9039d6f26f290d0..7f127e2ae44292f8f5c7ff6a9251c3d7ec8c9f58
 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -104,6 +104,7 @@ config DRM_MSM_DPU
 config DRM_MSM_DP
bool "Enable DisplayPort support in MSM DRM driver"
depends on DRM_MSM
+   select DRM_DISPLAY_HDMI_AUDIO_HELPER
select RATIONAL
default y
help
diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c 
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 
70fdc9fe228a7149546accd8479a9e4397f3d5dd..f8bfb908f9b4bf93ad5480f0785e3aed23dde160
 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -13,13 +13,13 @@
 
 #include "dp_catalog.h"
 #include "dp_audio.h"
+#include "dp_drm.h"
 #include "dp_panel.h"
 #include "dp_reg.h"
 #include "dp_display.h"
 #include "dp_utils.h"
 
 struct msm_dp_audio_private {
-   struct platform_device *audio_pdev;
struct platform_device *pdev;
struct drm_device *drm_dev;
struct msm_dp_catalog *catalog;
@@ -160,24 +160,11 @@ static void msm_dp_audio_enable(struct 
msm_dp_audio_private *audio, bool enable)
msm_dp_catalog_audio_enable(catalog, enable);
 }
 
-static struct msm_dp_audio_private *msm_dp_audio_get_data(struct 
platform_device *pdev)
+static struct msm_dp_audio_private *msm_dp_audio_get_data(struct msm_dp 
*msm_dp_display)
 {
struct msm_dp_audio *msm_dp_audio;
-   struct msm_dp *msm_dp_display;
-
-   if (!pdev) {
-   DRM_ERROR("invalid input\n");
-   return ERR_PTR(-ENODEV);
-   }
-
-   msm_dp_display = platform_get_drvdata(pdev);
-   if (!msm_dp_display) {
-   DRM_ERROR("invalid input\n");
-   return ERR_PTR(-ENODEV);
-   }
 
msm_dp_audio = msm_dp_display->msm_dp_audio;
-
if (!msm_dp_audio) {
DRM_ERROR("invalid msm_dp_audio data\n");
return ERR_PTR(-EINVAL);
@@ -186,68 +173,16 @@ static struct msm_dp_audio_private 
*msm_dp_audio_get_data(struct platform_device
return container_of(msm_dp_audio, struct msm_dp_audio_private, 
msm_dp_audio);
 }
 
-static int msm_dp_audio_hook_plugged_cb(struct device *dev, void *data,
-   hdmi_codec_plugged_cb fn,
-   struct device *codec_dev)
-{
-
-   struct platform_device *pdev;
-   struct msm_dp *msm_dp_display;
-
-   pdev = to_platform_device(dev);
-   if (!pdev) {
-   pr_err("invalid input\n");
-   return -ENODEV;
-   }
-
-   msm_dp_display = platform_get_drvdata(pdev);
-   if (!msm_dp_display) {
-   pr_err("invalid input\n");
-   return -ENODEV;
-   }
-
-   return msm_dp_display_set_plugged_cb(msm_dp_display, fn, codec_dev);
-}
-
-static int msm_dp_audio_get_eld(struct device *dev,
-   void *data, uint8_t *buf, size_t len)
-{
-   struct platform_device *pdev;
-   struct msm_dp *msm_dp_display;
-
-   pdev = to_platform_device(dev);
-
-   if (!pdev) {
-   DRM_ERROR("invalid input\n");
-   return -ENODEV;
-   }
-
-   msm_dp_display = platform_get_drvdata(pdev);
-   if (!msm_dp_display) {
-   DRM_ERROR("invalid input\n");
-   return -ENODEV;
-   }
-
-   mutex_lock(&msm_dp_display->connector->eld_mutex);
-   memcpy(buf, msm_dp_display->connector->eld,
-   min(sizeof(msm_dp_display->connector->eld), len));
-   mutex_unlock(&msm_dp_display->connector->eld_mutex);
-
-   return 0;
-}
-
-int msm_dp_audio_hw_params(struct device *dev,
-   void *data,
-   struct hdmi_codec_daifmt *daifmt,
-   struct hdmi_codec_params *params)
+int msm_dp_audio_prepare(struct drm_connector *connector,
+struct drm_bridge *bridge,
+struct hdmi_codec_daifmt *daifmt,
+struct hdmi_codec_params *params)
 {
int rc = 0;
struct msm_dp_audio_private *audio;
-   struct platform_device *pdev;
struct msm_dp *msm_dp_display;
 
-   pdev = to_platform_device(dev);
-   msm_dp_display = platform_get_drvdata(pdev);
+   msm_d

Re: [PATCH 11/11] arm64: dts: qcom: x1e001de-devkit: Drop clock-names from PS8830

On Thu, Mar 06, 2025 at 07:11:23PM +0100, Konrad Dybcio wrote:
> From: Konrad Dybcio 
> 
> The preemptively-merged node contains a property absent from the final
> bindings. Remove it.
> 
> Signed-off-by: Konrad Dybcio 
> ---
>  arch/arm64/boot/dts/qcom/x1e001de-devkit.dts | 3 ---
>  1 file changed, 3 deletions(-)
> 

Fixes: 019e1ee32fec ("arm64: dts: qcom: x1e001de-devkit: Enable external DP 
support")

Reviewed-by: Dmitry Baryshkov 

-- 
With best wishes
Dmitry

Re: [PATCH 03/11] dt-bindings: power: qcom,kpss-acc-v2: Add MSM8916 compatible

On Thu, Mar 06, 2025 at 07:11:15PM +0100, Konrad Dybcio wrote:
> From: Konrad Dybcio 
> 
> MSM8916 seems to reuse the same hardware as MSM8974 and friends (for
> whom this binding document was created). Add a new compatible for it.

Ok, I should have read dt-bindigns before sending a comment. But this
commit doesn't explain, why do you need an extra compat string.

> 
> Signed-off-by: Konrad Dybcio 
> ---
>  Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml 
> b/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml
> index 
> 202a5d51ee88c7190805efe8f1bf493bdb69ec45..27dae49163fa0790ceb6fda8a5c674f739d4a41a
>  100644
> --- a/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml
> +++ b/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml
> @@ -18,7 +18,9 @@ description:
>  
>  properties:
>compatible:
> -const: qcom,kpss-acc-v2
> +enum:
> +  - qcom,msm8916-kpss-acc
> +  - qcom,kpss-acc-v2
>  
>reg:
>  items:
> 
> -- 
> 2.48.1
> 

-- 
With best wishes
Dmitry

[PATCH v5 1/2] drm/bridge: split HDMI Audio from DRM_BRIDGE_OP_HDMI

From: Dmitry Baryshkov 

As pointed out by Laurent, OP bits are supposed to describe operations.
Split DRM_BRIDGE_OP_HDMI_AUDIO from DRM_BRIDGE_OP_HDMI instead of
overloading DRM_BRIDGE_OP_HDMI.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/bridge/lontium-lt9611.c|  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c   |  1 +
 drivers/gpu/drm/display/drm_bridge_connector.c | 59 +-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  1 +
 include/drm/drm_bridge.h   | 23 --
 5 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c 
b/drivers/gpu/drm/bridge/lontium-lt9611.c
index 
026803034231f78c17f619dc04119bdd9b2b6679..3b93c17e25c18ae0d13e9bb74553cf21dcc39f9d
 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -1130,7 +1130,7 @@ static int lt9611_probe(struct i2c_client *client)
lt9611->bridge.of_node = client->dev.of_node;
lt9611->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID |
 DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_MODES |
-DRM_BRIDGE_OP_HDMI;
+DRM_BRIDGE_OP_HDMI | DRM_BRIDGE_OP_HDMI_AUDIO;
lt9611->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
lt9611->bridge.vendor = "Lontium";
lt9611->bridge.product = "LT9611";
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index 
6166f197e37b552cb8a52b7b0d23ffc632f54557..5e5f8c2f95be1f5c4633f1093b17a00f9425bb37
 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -1077,6 +1077,7 @@ struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device 
*pdev,
hdmi->bridge.ops = DRM_BRIDGE_OP_DETECT |
   DRM_BRIDGE_OP_EDID |
   DRM_BRIDGE_OP_HDMI |
+  DRM_BRIDGE_OP_HDMI_AUDIO |
   DRM_BRIDGE_OP_HPD;
hdmi->bridge.of_node = pdev->dev.of_node;
hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c 
b/drivers/gpu/drm/display/drm_bridge_connector.c
index 
30c736fc0067e31a97db242e5b16ea8a5b4cf359..030f98d454608a63154827c65d4822d378df3b4c
 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -98,6 +98,13 @@ struct drm_bridge_connector {
 * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI).
 */
struct drm_bridge *bridge_hdmi;
+   /**
+* @bridge_hdmi_audio:
+*
+* The bridge in the chain that implements necessary support for the
+* HDMI Audio infrastructure, if any (see &DRM_BRIDGE_OP_HDMI_AUDIO).
+*/
+   struct drm_bridge *bridge_hdmi_audio;
 };
 
 #define to_drm_bridge_connector(x) \
@@ -433,7 +440,7 @@ static int drm_bridge_connector_audio_startup(struct 
drm_connector *connector)
to_drm_bridge_connector(connector);
struct drm_bridge *bridge;
 
-   bridge = bridge_connector->bridge_hdmi;
+   bridge = bridge_connector->bridge_hdmi_audio;
if (!bridge)
return -EINVAL;
 
@@ -451,7 +458,7 @@ static int drm_bridge_connector_audio_prepare(struct 
drm_connector *connector,
to_drm_bridge_connector(connector);
struct drm_bridge *bridge;
 
-   bridge = bridge_connector->bridge_hdmi;
+   bridge = bridge_connector->bridge_hdmi_audio;
if (!bridge)
return -EINVAL;
 
@@ -464,7 +471,7 @@ static void drm_bridge_connector_audio_shutdown(struct 
drm_connector *connector)
to_drm_bridge_connector(connector);
struct drm_bridge *bridge;
 
-   bridge = bridge_connector->bridge_hdmi;
+   bridge = bridge_connector->bridge_hdmi_audio;
if (!bridge)
return;
 
@@ -478,7 +485,7 @@ static int drm_bridge_connector_audio_mute_stream(struct 
drm_connector *connecto
to_drm_bridge_connector(connector);
struct drm_bridge *bridge;
 
-   bridge = bridge_connector->bridge_hdmi;
+   bridge = bridge_connector->bridge_hdmi_audio;
if (!bridge)
return -EINVAL;
 
@@ -576,6 +583,21 @@ struct drm_connector *drm_bridge_connector_init(struct 
drm_device *drm,
max_bpc = bridge->max_bpc;
}
 
+   if (bridge->ops & DRM_BRIDGE_OP_HDMI_AUDIO) {
+   if (bridge_connector->bridge_hdmi_audio)
+   return ERR_PTR(-EBUSY);
+
+   if (!bridge->hdmi_audio_max_i2s_playback_channels &&
+   !bridge->hdmi_audio_spdif_playback)
+   return ERR_PTR(-EINVAL);
+
+   if (!bridge->funcs->hdmi_audio_prepare ||
+   !

Re: [PATCH 0/6] Add support for RK3588 DisplayPort Controller

2025-03-06 Thread Piotr Oniszczuk




> Wiadomość napisana przez Andy Yan  w dniu 6 mar 2025, o 
> godz. 01:59:
> 
> 
> 
> 
> Both of the two config options should be enabled.
> andy@Pro480:~/WorkSpace/linux-next$ rg DW_DP .config
> 4044:CONFIG_ROCKCHIP_DW_DP=y

here i’m a bit lost….
greping on full kernel sources (with applied 
https://patchwork.kernel.org/project/linux-rockchip/list/?series=936784) gives 
me no single appearance of ROCKCHIP_DW_DP…
Do i miss something?

Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: add h/vsync-disable bindings

On Thu, Mar 06, 2025 at 10:11:32AM +0100, A. Zini wrote:
> From: Alessandro Zini 
> 
> Add hsync- and vsync-disable bindings, used to disable the generation of
> h/vsync signals.

Please describe, why this is necessary at all, instead of desribing the
contents of the patch.

> 
> Signed-off-by: Alessandro Zini 
> ---
>  .../bindings/display/bridge/ti,sn65dsi83.yaml| 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml 
> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> index 9b5f3f3eab198..ff80876d504ad 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -94,6 +94,18 @@ properties:
>- port@0
>- port@2
>  
> +  hsync-disable:
> +type: boolean
> +description: |
> +  Disable HSYNC generation on the LVDS output by setting the
> +  width in pixel clocks of the hsync pulse width to 0.
> +
> +  vsync-disable:
> +type: boolean
> +description: |
> +  Disable VSYNC generation on the LVDS output by setting the
> +  length in lines of the vsync pulse width to 0.
> +
>  required:
>- compatible
>- reg
> -- 
> 2.48.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH drm-next 1/2] vmalloc: Add atomic_vmap

2025-03-06 Thread Jocelyn Falempe

On 06/03/2025 16:52, Simona Vetter wrote:

On Thu, Mar 06, 2025 at 02:24:51PM +0100, Jocelyn Falempe wrote:

On 06/03/2025 05:52, Matthew Wilcox wrote:

On Thu, Mar 06, 2025 at 12:25:53AM +0900, Ryosuke Yasuoka wrote:

Some drivers can use vmap in drm_panic, however, vmap is sleepable and
takes locks. Since drm_panic will vmap in panic handler, atomic_vmap
requests pages with GFP_ATOMIC and maps KVA without locks and sleep.

In addition to the implicit GFP_KERNEL allocations Vlad mentioned, how
is this supposed to work?

+ vn = addr_to_node(va->va_start);
+
+ insert_vmap_area(va, &vn->busy.root, &vn->busy.head);

If someone else is holding the vn->busy.lock because they're modifying the
busy tree, you'll corrupt the tree. You can't just say "I can't take a
lock here, so I won't bother". You need to figure out how to do something
safe without taking the lock. For example, you could preallocate the
page tables and reserve a vmap area when the driver loads that would
then be usable for the panic situation. I don't know that we have APIs
to let you do that today, but it's something that could be added.

Regarding the lock, it should be possible to use the trylock() variant, and
fail if the lock is already taken. (In the panic handler, only 1 CPU remain
active, so it's unlikely the lock would be released anyway).

If we need to pre-allocate the page table and reserve the vmap area, maybe
it would be easier to just always vmap() the primary framebuffer, so it can
be used in the panic handler?

Yeah I really don't like the idea of creating some really brittle one-off
core mm code just so we don't have to vmap a buffer unconditionally. I
think even better would be if drm_panic can cope with non-linear buffers,
it's entirely fine if the drawing function absolutely crawls and sets each
individual byte ...

It already supports some non-linear buffer, like Nvidia block-linear:
https://elixir.bootlin.com/linux/v6.13.5/source/drivers/gpu/drm/nouveau/dispnv50/wndw.c#L606

And I've also sent some patches to support Intel's 4-tile and Y-tile format:
https://patchwork.freedesktop.org/patch/637200/?series=141936&rev=5
https://patchwork.freedesktop.org/patch/637202/?series=141936&rev=5

Hopefully Color Compression can be disabled on intel's GPU, otherwise
that would be a bit harder to implement than tiling.

The only thing you're allowed to do in panic is try_lock on a raw spinlock
(plus some really scare lockless tricks), imposing that on core mm sounds
like a non-starter to me.

Cheers, Sima

Re: [PATCH v2 0/2] Fix native cursors with vmwgfx

2025-03-06 Thread Maaz Mombasawala

On 3/5/25 20:11, Zack Rusin wrote:
> vmwgfx had a number of cursor issues that related both to our handling
> of dumb buffers and general detection when a cursor has actually changed.
> Fix those issues and bump the kernel module version to allow userspace
> to recognize fixed versions of the driver.
> 
> v2: Include the fix for leaked dirty trackers in kms fb surface
> cleanup code.
> 
> Zack Rusin (2):
>   drm/vmwgfx: Refactor cursor handling
>   drm/vmwgfx: Bump the minor version
> 
>  drivers/gpu/drm/vmwgfx/Makefile  |   2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c   |   6 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.h   |   2 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 844 ++
>  drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h |  81 ++
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   3 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |  12 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c  |   2 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 851 +--
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  49 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  |  10 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c   |  40 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  10 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  11 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c  |  25 +-
>  15 files changed, 1031 insertions(+), 917 deletions(-)
>  create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
>  create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h
> 


Should update the year in the copyright to 2025, looks good otherwise.

Reviewed-by: Maaz Mombasawala 

-- 
Maaz Mombasawala

Re:Re:Re: [PATCH v5 04/16] drm/atomic: Introduce helper to lookup connector by encoder

2025-03-06 Thread Andy Yan


Hi All,
At 2025-03-07 09:08:48, "Andy Yan"  wrote:
>Hi All,
>
>At 2025-03-06 23:41:24, "Simona Vetter"  wrote:
>>On Thu, Mar 06, 2025 at 08:10:16AM +0100, Maxime Ripard wrote:
>>> On Thu, Mar 06, 2025 at 09:16:24AM +0800, Andy Yan wrote:
>>> > 
>>> > Hi Maxime and Dmitry:
>>> > 
>>> > At 2025-03-06 04:13:53, "Dmitry Baryshkov"  
>>> > wrote:
>>> > >On Wed, Mar 05, 2025 at 02:19:36PM +0100, Maxime Ripard wrote:
>>> > >> Hi Andy,
>>> > >> 
>>> > >> On Wed, Mar 05, 2025 at 07:55:19PM +0800, Andy Yan wrote:
>>> > >> > At 2025-03-04 19:10:47, "Maxime Ripard"  wrote:
>>> > >> > >With the bridges switching over to drm_bridge_connector, the direct
>>> > >> > >association between a bridge driver and its connector was lost.
>>> > >> > >
>>> > >> > >This is mitigated for atomic bridge drivers by the fact you can 
>>> > >> > >access
>>> > >> > >the encoder, and then call 
>>> > >> > >drm_atomic_get_old_connector_for_encoder() or
>>> > >> > >drm_atomic_get_new_connector_for_encoder() with drm_atomic_state.
>>> > >> > >
>>> > >> > >This was also made easier by providing drm_atomic_state directly to 
>>> > >> > >all
>>> > >> > >atomic hooks bridges can implement.
>>> > >> > >
>>> > >> > >However, bridge drivers don't have a way to access drm_atomic_state
>>> > >> > >outside of the modeset path, like from the hotplug interrupt path 
>>> > >> > >or any
>>> > >> > >interrupt handler.
>>> > >> > >
>>> > >> > >Let's introduce a function to retrieve the connector currently 
>>> > >> > >assigned
>>> > >> > >to an encoder, without using drm_atomic_state, to make these 
>>> > >> > >drivers'
>>> > >> > >life easier.
>>> > >> > >
>>> > >> > >Reviewed-by: Dmitry Baryshkov 
>>> > >> > >Co-developed-by: Simona Vetter 
>>> > >> > >Signed-off-by: Maxime Ripard 
>>> > >> > >---
>>> > >> > > drivers/gpu/drm/drm_atomic.c | 45 
>>> > >> > > 
>>> > >> > > include/drm/drm_atomic.h |  3 +++
>>> > >> > > 2 files changed, 48 insertions(+)
>>> > >> > >
>>> > >> > >diff --git a/drivers/gpu/drm/drm_atomic.c 
>>> > >> > >b/drivers/gpu/drm/drm_atomic.c
>>> > >> > >index 
>>> > >> > >9ea2611770f43ce7ccba410406d5f2c528aab022..b926b132590e78f8d41d48eb4da4bccf170ee236
>>> > >> > > 100644
>>> > >> > >--- a/drivers/gpu/drm/drm_atomic.c
>>> > >> > >+++ b/drivers/gpu/drm/drm_atomic.c
>>> > >> > >@@ -985,10 +985,55 @@ 
>>> > >> > >drm_atomic_get_new_connector_for_encoder(const struct 
>>> > >> > >drm_atomic_state *state,
>>> > >> > > 
>>> > >> > >return NULL;
>>> > >> > > }
>>> > >> > > EXPORT_SYMBOL(drm_atomic_get_new_connector_for_encoder);
>>> > >> > > 
>>> > >> > >+/**
>>> > >> > >+ * drm_atomic_get_connector_for_encoder - Get connector currently 
>>> > >> > >assigned to an encoder
>>> > >> > >+ * @encoder: The encoder to find the connector of
>>> > >> > >+ * @ctx: Modeset locking context
>>> > >> > >+ *
>>> > >> > >+ * This function finds and returns the connector currently 
>>> > >> > >assigned to
>>> > >> > >+ * an @encoder.
>>> > >> > >+ *
>>> > >> > >+ * Returns:
>>> > >> > >+ * The connector connected to @encoder, or an error pointer 
>>> > >> > >otherwise.
>>> > >> > >+ * When the error is EDEADLK, a deadlock has been detected and the
>>> > >> > >+ * sequence must be restarted.
>>> > >> > >+ */
>>> > >> > >+struct drm_connector *
>>> > >> > >+drm_atomic_get_connector_for_encoder(const struct drm_encoder 
>>> > >> > >*encoder,
>>> > >> > >+struct drm_modeset_acquire_ctx 
>>> > >> > >*ctx)
>>> > >> > >+{
>>> > >> > >+   struct drm_connector_list_iter conn_iter;
>>> > >> > >+   struct drm_connector *out_connector = ERR_PTR(-EINVAL);
>>> > >> > >+   struct drm_connector *connector;
>>> > >> > >+   struct drm_device *dev = encoder->dev;
>>> > >> > >+   int ret;
>>> > >> > >+
>>> > >> > >+   ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
>>> > >> > >+   if (ret)
>>> > >> > >+   return ERR_PTR(ret);
>>> > >> > 
>>> > >> > It seems that this will cause a deadlock when called from a hotplug
>>> > >> > handling path, I have a WIP DP diver[0], which suggested by Dmitry to
>>> > >> > use this API from a &drm_bridge_funcs.detect callback to get the
>>> > >> > connector, as detect is called by drm_helper_probe_detect, which will
>>> > >> > hold connection_mutex first, so the deaklock happens:
>>> > >> >
>>> > >> > drm_helper_probe_detect(struct drm_connector *connector,
>>> > >> > struct drm_modeset_acquire_ctx *ctx,
>>> > >> > bool force)
>>> > >> > {
>>> > >> > const struct drm_connector_helper_funcs *funcs = 
>>> > >> > connector->helper_private;
>>> > >> > struct drm_device *dev = connector->dev;
>>> > >> > int ret;
>>> > >> > 
>>> > >> > if (!ctx)
>>> > >> > return drm_helper_probe_detect_ctx(connector, force);
>>> > >> > 
>>> > >> > ret = drm_modeset_lock(&dev->mode_config.connection_mutex, 
>>> > >> > ctx);
>>> > >> > if (ret)
>>> > >>

[PATCH v3 4/8] drm/msm/dpu: use single CTL if it is the only CTL returned by RM

From: Dmitry Baryshkov 

On DPU >= 5.0 CTL blocks were reworked in order to support using a
single CTL for all outputs. In preparation of reworking the RM code to
return single CTL make sure that dpu_encoder can cope with that.

Reviewed-by: Marijn Suijten 
Tested-by: Neil Armstrong  # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 
0eed93a4d056beda6b54c0d20f027a53c84f67db..b5e8ba592d8af298a52924d34a573d4f9e05c476
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1247,7 +1247,11 @@ static void dpu_encoder_virt_atomic_mode_set(struct 
drm_encoder *drm_enc,
return;
}
 
-   phys->hw_ctl = i < num_ctl ? to_dpu_hw_ctl(hw_ctl[i]) : NULL;
+   /* Use first (and only) CTL if active CTLs are supported */
+   if (num_ctl == 1)
+   phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[0]);
+   else
+   phys->hw_ctl = i < num_ctl ? to_dpu_hw_ctl(hw_ctl[i]) : 
NULL;
if (!phys->hw_ctl) {
DPU_ERROR_ENC(dpu_enc,
"no ctl block assigned at idx: %d\n", i);

-- 
2.39.5

Re: [PATCH] drm/ast: Support both SHMEM helper and VRAM helper

2025-03-06 Thread Huacai Chen

On Wed, Mar 5, 2025 at 5:04 PM Thomas Zimmermann  wrote:
>
> Hi
>
> Am 04.03.25 um 12:55 schrieb Huacai Chen:
> > On Tue, Mar 4, 2025 at 5:39 PM Thomas Zimmermann  
> > wrote:
> >> Hi
> >>
> >> Am 04.03.25 um 10:19 schrieb Huacai Chen:
> >>> On Tue, Mar 4, 2025 at 4:41 PM Thomas Zimmermann  
> >>> wrote:
>  Hi
> 
>  Am 04.03.25 um 07:33 schrieb Huacai Chen:
> > Commit f2fa5a99ca81ce105653 ("drm/ast: Convert ast to SHMEM") converts
> > ast from VRAM helper to SHMEM helper. The convertion makesast support
> > higher display resolutions, but the performance decreases significantly
> > on platforms which don't support writecombine (fallback to uncached).
> >
> > This patch implements both SHMEM helper and VRAM helper for ast driver
> > at the same time. A module parameter ast.shmem is added, 1 means SHMEM
> > hepler, 0 means VRAM helper, and the default (-1) means auto selection
> > according to drm_arch_can_wc_memory().
>  Sorry won't happen. There's one memory manager and that's it.
> 
>  The question is why there is a difference between the two. Because
>  conceptually, it's both software rendering that copies the final image
>  into video ram. Why is that much faster with VRAM helpers?
> >>> Correct me if I'm wrong.
> >>>
> >>> SHMEM helper means "copy damage data to a fixed VRAM buffer", VRAM
> >>> helper means "double buffers and swapping the two". So if WC is not
> >>> supported, SHMEM helper is slower from visible effect.
> >> First of all, which component is slow? The kernel console, the desktop,
> >> something else?
> > "Slow" here means the desktop ui is not smooth, not kernel console,
> > and not data copy rate.
>
> For a simple test you can set
>
>mode_config.prefer_shadow = false
>
> (It's somewhere in your patch). This will disable Xorg's shadow buffer
> and make it operate directly on I/O memory. This should be much slower then.
I have tested this case, but not slow from desktop visible effect, is
this option ignored by Xorg?

>
>
> >
> >> On your question. This is all software rendering. In the case of
> >> GEM-SHMEM, we mmap shmem pages to user-space compositors and let them
> >> draw the UI. That should be fast because these SHMEM pages should be
> >> mapped with full caching. The WC caching is the exception here. This
> >> could be the problem. Maybe these pages are uncached for some reason.
> >> But so far, we've not touched VRAM at all. After rendering to the
> >> provided pages, the compositor instructs the DRM driver to flush the
> >> changes to VRAM. That involves a vmap of the I/O range and a
> >> memcpy_toio(). Without WC caching, this will be slow. It is optimized by
> >> using damage information; only the updated part of the screen will be
> >> copied.
> >>
> >> With GEM-VRAM, the kernel mmap's the VRAM's I/O range to the compositor.
> >> The compositor renders into a malloc'ed memory buffer and then copies
> >> the result over to the mmap'ed range fom the driver. (And essentially
> >> into the VRAM). If your platform does not have WC caching, this would be
> >> uncached and fairly slow.  The compositor then instructs the driver to
> >> swap buffers, which is always fast because we've already done he copying
> >> in user space.
> >>
> >> Now the question is why the GEM-SHMEM code is slower than the GEM-VRAM
> >> path. In principle, they both do the same work.
> > Yes, they do the same work, so the data copy rate may be the same. But
> > GEM-SHMEM is an in-place update while GEM-VRAM is an off-screen
> > update, so the desktop effect is different.
>
> Oh, so you see tearing.
>
> Years ago, we had a bug report about ast's lack of performance on x86.
> Back then, we had a VRAM-based memory manager. It turned out that ast
> was misconfigured to use uncached framebuffer access, just like on your
> system. See [1][2]. Hence, returning to this memory manager would just
> paper over the fact that framebuffer access is too slow. And it would
> bring disadvantages to other users.
>
> Ast hardware provides a vretrace interrupt AFAIK. Getting this to work
> would allow for the copying to take place during the display's vblank
> interval. This might be a solution that benefits everyone.
Maybe, but I'm not familiar with ast hardware. :(


Huacai

>
> Best regards
> Thomas
>
> [1] https://bugzilla.opensuse.org/show_bug.cgi?id=1112963
> [2]
> https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5478ad10e7850ce3d8b7056db05ddfa3c9ddad9a
>
> >
> > Huacai
> >
> >> Best regard
> >> Thomas
> >>
> >>>
> >>> Huacai
> >>>
>  Best regards
>  Thomas
> 
> > Signed-off-by: Huacai Chen 
> > ---
> > drivers/gpu/drm/ast/Kconfig|   3 +
> > drivers/gpu/drm/ast/ast_drv.c  |  31 --
> > drivers/gpu/drm/ast/ast_drv.h  |   6 +-
> > drivers/gpu/drm/ast/ast_mm.c   |  11 +++-
> > drivers/gpu/drm/ast/ast_mode.c | 105 
> > +---

Re:Re: [PATCH v5 04/16] drm/atomic: Introduce helper to lookup connector by encoder

2025-03-06 Thread Andy Yan

Hi All,

At 2025-03-06 23:41:24, "Simona Vetter"  wrote:
>On Thu, Mar 06, 2025 at 08:10:16AM +0100, Maxime Ripard wrote:
>> On Thu, Mar 06, 2025 at 09:16:24AM +0800, Andy Yan wrote:
>> > 
>> > Hi Maxime and Dmitry:
>> > 
>> > At 2025-03-06 04:13:53, "Dmitry Baryshkov"  
>> > wrote:
>> > >On Wed, Mar 05, 2025 at 02:19:36PM +0100, Maxime Ripard wrote:
>> > >> Hi Andy,
>> > >> 
>> > >> On Wed, Mar 05, 2025 at 07:55:19PM +0800, Andy Yan wrote:
>> > >> > At 2025-03-04 19:10:47, "Maxime Ripard"  wrote:
>> > >> > >With the bridges switching over to drm_bridge_connector, the direct
>> > >> > >association between a bridge driver and its connector was lost.
>> > >> > >
>> > >> > >This is mitigated for atomic bridge drivers by the fact you can 
>> > >> > >access
>> > >> > >the encoder, and then call 
>> > >> > >drm_atomic_get_old_connector_for_encoder() or
>> > >> > >drm_atomic_get_new_connector_for_encoder() with drm_atomic_state.
>> > >> > >
>> > >> > >This was also made easier by providing drm_atomic_state directly to 
>> > >> > >all
>> > >> > >atomic hooks bridges can implement.
>> > >> > >
>> > >> > >However, bridge drivers don't have a way to access drm_atomic_state
>> > >> > >outside of the modeset path, like from the hotplug interrupt path or 
>> > >> > >any
>> > >> > >interrupt handler.
>> > >> > >
>> > >> > >Let's introduce a function to retrieve the connector currently 
>> > >> > >assigned
>> > >> > >to an encoder, without using drm_atomic_state, to make these drivers'
>> > >> > >life easier.
>> > >> > >
>> > >> > >Reviewed-by: Dmitry Baryshkov 
>> > >> > >Co-developed-by: Simona Vetter 
>> > >> > >Signed-off-by: Maxime Ripard 
>> > >> > >---
>> > >> > > drivers/gpu/drm/drm_atomic.c | 45 
>> > >> > > 
>> > >> > > include/drm/drm_atomic.h |  3 +++
>> > >> > > 2 files changed, 48 insertions(+)
>> > >> > >
>> > >> > >diff --git a/drivers/gpu/drm/drm_atomic.c 
>> > >> > >b/drivers/gpu/drm/drm_atomic.c
>> > >> > >index 
>> > >> > >9ea2611770f43ce7ccba410406d5f2c528aab022..b926b132590e78f8d41d48eb4da4bccf170ee236
>> > >> > > 100644
>> > >> > >--- a/drivers/gpu/drm/drm_atomic.c
>> > >> > >+++ b/drivers/gpu/drm/drm_atomic.c
>> > >> > >@@ -985,10 +985,55 @@ drm_atomic_get_new_connector_for_encoder(const 
>> > >> > >struct drm_atomic_state *state,
>> > >> > > 
>> > >> > > return NULL;
>> > >> > > }
>> > >> > > EXPORT_SYMBOL(drm_atomic_get_new_connector_for_encoder);
>> > >> > > 
>> > >> > >+/**
>> > >> > >+ * drm_atomic_get_connector_for_encoder - Get connector currently 
>> > >> > >assigned to an encoder
>> > >> > >+ * @encoder: The encoder to find the connector of
>> > >> > >+ * @ctx: Modeset locking context
>> > >> > >+ *
>> > >> > >+ * This function finds and returns the connector currently assigned 
>> > >> > >to
>> > >> > >+ * an @encoder.
>> > >> > >+ *
>> > >> > >+ * Returns:
>> > >> > >+ * The connector connected to @encoder, or an error pointer 
>> > >> > >otherwise.
>> > >> > >+ * When the error is EDEADLK, a deadlock has been detected and the
>> > >> > >+ * sequence must be restarted.
>> > >> > >+ */
>> > >> > >+struct drm_connector *
>> > >> > >+drm_atomic_get_connector_for_encoder(const struct drm_encoder 
>> > >> > >*encoder,
>> > >> > >+ struct drm_modeset_acquire_ctx 
>> > >> > >*ctx)
>> > >> > >+{
>> > >> > >+struct drm_connector_list_iter conn_iter;
>> > >> > >+struct drm_connector *out_connector = ERR_PTR(-EINVAL);
>> > >> > >+struct drm_connector *connector;
>> > >> > >+struct drm_device *dev = encoder->dev;
>> > >> > >+int ret;
>> > >> > >+
>> > >> > >+ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
>> > >> > >+if (ret)
>> > >> > >+return ERR_PTR(ret);
>> > >> > 
>> > >> > It seems that this will cause a deadlock when called from a hotplug
>> > >> > handling path, I have a WIP DP diver[0], which suggested by Dmitry to
>> > >> > use this API from a &drm_bridge_funcs.detect callback to get the
>> > >> > connector, as detect is called by drm_helper_probe_detect, which will
>> > >> > hold connection_mutex first, so the deaklock happens:
>> > >> >
>> > >> > drm_helper_probe_detect(struct drm_connector *connector,
>> > >> > struct drm_modeset_acquire_ctx *ctx,
>> > >> > bool force)
>> > >> > {
>> > >> > const struct drm_connector_helper_funcs *funcs = 
>> > >> > connector->helper_private;
>> > >> > struct drm_device *dev = connector->dev;
>> > >> > int ret;
>> > >> > 
>> > >> > if (!ctx)
>> > >> > return drm_helper_probe_detect_ctx(connector, force);
>> > >> > 
>> > >> > ret = drm_modeset_lock(&dev->mode_config.connection_mutex, 
>> > >> > ctx);
>> > >> > if (ret)
>> > >> > return ret;
>> > >> > 
>> > >> > if (funcs->detect_ctx)
>> > >> > ret = funcs->detect_ctx(connector, ctx, force);
>> > >> > else if (

[PATCH RFC v3 3/7] drm/display: dp: use new DCPD access helpers

From: Dmitry Baryshkov 

Switch drm_dp_helper.c to use new set of DPCD read / write helpers.

Reviewed-by: Lyude Paul 
Acked-by: Jani Nikula 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/display/drm_dp_helper.c | 296 +---
 1 file changed, 116 insertions(+), 180 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c 
b/drivers/gpu/drm/display/drm_dp_helper.c
index 
410be0be233ad94702af423262a7d98e21afbfeb..e2439c8a7fefe116b04aaa689b557e2387b05540
 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -327,7 +327,7 @@ static int __read_delay(struct drm_dp_aux *aux, const u8 
dpcd[DP_RECEIVER_CAP_SI
if (offset < DP_RECEIVER_CAP_SIZE) {
rd_interval = dpcd[offset];
} else {
-   if (drm_dp_dpcd_readb(aux, offset, &rd_interval) != 1) {
+   if (drm_dp_dpcd_read_byte(aux, offset, &rd_interval) < 0) {
drm_dbg_kms(aux->drm_dev, "%s: failed rd interval 
read\n",
aux->name);
/* arbitrary default delay */
@@ -358,7 +358,7 @@ int drm_dp_128b132b_read_aux_rd_interval(struct drm_dp_aux 
*aux)
int unit;
u8 val;
 
-   if (drm_dp_dpcd_readb(aux, DP_128B132B_TRAINING_AUX_RD_INTERVAL, &val) 
!= 1) {
+   if (drm_dp_dpcd_read_byte(aux, DP_128B132B_TRAINING_AUX_RD_INTERVAL, 
&val) < 0) {
drm_err(aux->drm_dev, "%s: failed rd interval read\n",
aux->name);
/* default to max */
@@ -807,30 +807,20 @@ int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux 
*aux,
 {
int ret;
 
-   if (dp_phy == DP_PHY_DPRX) {
-   ret = drm_dp_dpcd_read(aux,
-  DP_LANE0_1_STATUS,
-  link_status,
-  DP_LINK_STATUS_SIZE);
-
-   if (ret < 0)
-   return ret;
+   if (dp_phy == DP_PHY_DPRX)
+   return drm_dp_dpcd_read_data(aux,
+DP_LANE0_1_STATUS,
+link_status,
+DP_LINK_STATUS_SIZE);
 
-   WARN_ON(ret != DP_LINK_STATUS_SIZE);
-
-   return 0;
-   }
-
-   ret = drm_dp_dpcd_read(aux,
-  DP_LANE0_1_STATUS_PHY_REPEATER(dp_phy),
-  link_status,
-  DP_LINK_STATUS_SIZE - 1);
+   ret = drm_dp_dpcd_read_data(aux,
+   DP_LANE0_1_STATUS_PHY_REPEATER(dp_phy),
+   link_status,
+   DP_LINK_STATUS_SIZE - 1);
 
if (ret < 0)
return ret;
 
-   WARN_ON(ret != DP_LINK_STATUS_SIZE - 1);
-
/* Convert the LTTPR to the sink PHY link status layout */
memmove(&link_status[DP_SINK_STATUS - DP_LANE0_1_STATUS + 1],
&link_status[DP_SINK_STATUS - DP_LANE0_1_STATUS],
@@ -846,7 +836,7 @@ static int read_payload_update_status(struct drm_dp_aux 
*aux)
int ret;
u8 status;
 
-   ret = drm_dp_dpcd_readb(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS, &status);
+   ret = drm_dp_dpcd_read_byte(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS, 
&status);
if (ret < 0)
return ret;
 
@@ -873,21 +863,21 @@ int drm_dp_dpcd_write_payload(struct drm_dp_aux *aux,
int ret;
int retries = 0;
 
-   drm_dp_dpcd_writeb(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
-  DP_PAYLOAD_TABLE_UPDATED);
+   drm_dp_dpcd_write_byte(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
+  DP_PAYLOAD_TABLE_UPDATED);
 
payload_alloc[0] = vcpid;
payload_alloc[1] = start_time_slot;
payload_alloc[2] = time_slot_count;
 
-   ret = drm_dp_dpcd_write(aux, DP_PAYLOAD_ALLOCATE_SET, payload_alloc, 3);
-   if (ret != 3) {
+   ret = drm_dp_dpcd_write_data(aux, DP_PAYLOAD_ALLOCATE_SET, 
payload_alloc, 3);
+   if (ret < 0) {
drm_dbg_kms(aux->drm_dev, "failed to write payload allocation 
%d\n", ret);
goto fail;
}
 
 retry:
-   ret = drm_dp_dpcd_readb(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS, &status);
+   ret = drm_dp_dpcd_read_byte(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS, 
&status);
if (ret < 0) {
drm_dbg_kms(aux->drm_dev, "failed to read payload table status 
%d\n", ret);
goto fail;
@@ -1043,15 +1033,15 @@ bool drm_dp_send_real_edid_checksum(struct drm_dp_aux 
*aux,
 {
u8 link_edid_read = 0, auto_test_req = 0, test_resp = 0;
 
-   if (drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR,
-&auto_test_req, 1) < 1) {
+   if (drm_dp_dpcd_read_byte(aux, DP_DEVICE_SERVICE_IRQ_VECTOR,
+ &auto_test_re

[PATCH RFC v3 0/7] drm/display: dp: add new DPCD access functions

Existing DPCD access functions return an error code or the number of
bytes being read / write in case of partial access. However a lot of
drivers either (incorrectly) ignore partial access or mishandle error
codes. In other cases this results in a boilerplate code which compares
returned value with the size.

As suggested by Jani implement new set of DPCD access helpers, which
ignore partial access, always return 0 or an error code. Implement
new helpers using existing functions to ensure backwards compatibility
and to assess necessity to handle incomplete reads on a global scale.
Currently only one possible place has been identified, dp-aux-dev, which
needs to handle possible holes in DPCD.

This series targets only the DRM helpers code. If the approach is found
to be acceptable, each of the drivers should be converted on its own.

Signed-off-by: Dmitry Baryshkov 
---
Changes in v3:
- Fixed cover letter (Jani)
- Added intel-gfx and intel-xe to get the series CI-tested (Jani)
- Link to v2: 
https://lore.kernel.org/r/20250301-drm-rework-dpcd-access-v2-0-4d92602fc...@linaro.org

Changes in v2:
- Reimplemented new helpers using old ones (Lyude)
- Reworked the drm_dp_dpcd_read_link_status() patch (Lyude)
- Dropped the dp-aux-dev patch (Jani)
- Link to v1: 
https://lore.kernel.org/r/20250117-drm-rework-dpcd-access-v1-0-7fc020e04...@linaro.org

---
Dmitry Baryshkov (7):
  drm/display: dp: implement new access helpers
  drm/display: dp: change drm_dp_dpcd_read_link_status() return value
  drm/display: dp: use new DCPD access helpers
  drm/display: dp-aux-dev: use new DCPD access helpers
  drm/display: dp-cec: use new DCPD access helpers
  drm/display: dp-mst-topology: use new DCPD access helpers
  drm/display: dp-tunnel: use new DCPD access helpers

 drivers/gpu/drm/amd/amdgpu/atombios_dp.c   |   8 +-
 .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c|   2 +-
 drivers/gpu/drm/display/drm_dp_aux_dev.c   |  12 +-
 drivers/gpu/drm/display/drm_dp_cec.c   |  37 ++-
 drivers/gpu/drm/display/drm_dp_helper.c| 307 +
 drivers/gpu/drm/display/drm_dp_mst_topology.c  | 105 ---
 drivers/gpu/drm/display/drm_dp_tunnel.c|  20 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c   |   4 +-
 drivers/gpu/drm/msm/dp/dp_ctrl.c   |  24 +-
 drivers/gpu/drm/msm/dp/dp_link.c   |  18 +-
 drivers/gpu/drm/radeon/atombios_dp.c   |   8 +-
 include/drm/display/drm_dp_helper.h|  92 +-
 12 files changed, 322 insertions(+), 315 deletions(-)
---
base-commit: 565351ae7e0cee80e9b5ed84452a5b13644ffc4d
change-id: 20241231-drm-rework-dpcd-access-b0fc2e47d613

Best regards,
-- 
Dmitry Baryshkov

[PATCH RFC v3 1/7] drm/display: dp: implement new access helpers

From: Dmitry Baryshkov 

Existing DPCD access functions return an error code or the number of
bytes being read / write in case of partial access. However a lot of
drivers either (incorrectly) ignore partial access or mishandle error
codes. In other cases this results in a boilerplate code which compares
returned value with the size.

Implement new set of DPCD access helpers, which ignore partial access,
always return 0 or an error code.

Suggested-by: Jani Nikula 
Acked-by: Jani Nikula 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/display/drm_dp_helper.c |  4 ++
 include/drm/display/drm_dp_helper.h | 92 -
 2 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c 
b/drivers/gpu/drm/display/drm_dp_helper.c
index 
dbce1c3f49691fc687fee2404b723c73d533f23d..e43a8f4a252dae22eeaae1f4ca94da064303033d
 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -704,6 +704,8 @@ EXPORT_SYMBOL(drm_dp_dpcd_set_powered);
  * function returns -EPROTO. Errors from the underlying AUX channel transfer
  * function, with the exception of -EBUSY (which causes the transaction to
  * be retried), are propagated to the caller.
+ *
+ * In most of the cases you want to use drm_dp_dpcd_read_data() instead.
  */
 ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
 void *buffer, size_t size)
@@ -752,6 +754,8 @@ EXPORT_SYMBOL(drm_dp_dpcd_read);
  * function returns -EPROTO. Errors from the underlying AUX channel transfer
  * function, with the exception of -EBUSY (which causes the transaction to
  * be retried), are propagated to the caller.
+ *
+ * In most of the cases you want to use drm_dp_dpcd_write_data() instead.
  */
 ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,
  void *buffer, size_t size)
diff --git a/include/drm/display/drm_dp_helper.h 
b/include/drm/display/drm_dp_helper.h
index 
5ae4241959f24e2c1fb581d7c7d770485d603099..c5be44d72c9a04474f6c795e03bf02bf08f5eaef
 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -527,6 +527,64 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned 
int offset,
 ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,
  void *buffer, size_t size);
 
+/**
+ * drm_dp_dpcd_read_data() - read a series of bytes from the DPCD
+ * @aux: DisplayPort AUX channel (SST or MST)
+ * @offset: address of the (first) register to read
+ * @buffer: buffer to store the register values
+ * @size: number of bytes in @buffer
+ *
+ * Returns zero (0) on success, or a negative error
+ * code on failure. -EIO is returned if the request was NAKed by the sink or
+ * if the retry count was exceeded. If not all bytes were transferred, this
+ * function returns -EPROTO. Errors from the underlying AUX channel transfer
+ * function, with the exception of -EBUSY (which causes the transaction to
+ * be retried), are propagated to the caller.
+ */
+static inline int drm_dp_dpcd_read_data(struct drm_dp_aux *aux,
+   unsigned int offset,
+   void *buffer, size_t size)
+{
+   int ret;
+
+   ret = drm_dp_dpcd_read(aux, offset, buffer, size);
+   if (ret < 0)
+   return ret;
+   if (ret < size)
+   return -EPROTO;
+
+   return 0;
+}
+
+/**
+ * drm_dp_dpcd_write_data() - write a series of bytes to the DPCD
+ * @aux: DisplayPort AUX channel (SST or MST)
+ * @offset: address of the (first) register to write
+ * @buffer: buffer containing the values to write
+ * @size: number of bytes in @buffer
+ *
+ * Returns zero (0) on success, or a negative error
+ * code on failure. -EIO is returned if the request was NAKed by the sink or
+ * if the retry count was exceeded. If not all bytes were transferred, this
+ * function returns -EPROTO. Errors from the underlying AUX channel transfer
+ * function, with the exception of -EBUSY (which causes the transaction to
+ * be retried), are propagated to the caller.
+ */
+static inline int drm_dp_dpcd_write_data(struct drm_dp_aux *aux,
+unsigned int offset,
+void *buffer, size_t size)
+{
+   int ret;
+
+   ret = drm_dp_dpcd_write(aux, offset, buffer, size);
+   if (ret < 0)
+   return ret;
+   if (ret < size)
+   return -EPROTO;
+
+   return 0;
+}
+
 /**
  * drm_dp_dpcd_readb() - read a single byte from the DPCD
  * @aux: DisplayPort AUX channel
@@ -534,7 +592,8 @@ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned 
int offset,
  * @valuep: location where the value of the register will be stored
  *
  * Returns the number of bytes transferred (1) on success, or a negative
- * error code on failure.
+ * error code on failure. In most of the cases yo

[PATCH RFC v3 2/7] drm/display: dp: change drm_dp_dpcd_read_link_status() return value

From: Dmitry Baryshkov 

drm_dp_dpcd_read_link_status() follows the "return error code or number
of bytes read" protocol, with the code returning less bytes than
requested in case of some errors. However most of the drivers
interpreted that as "return error code in case of any error". Switch
drm_dp_dpcd_read_link_status() to drm_dp_dpcd_read_data() and make it
follow that protocol too.

Acked-by: Jani Nikula 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/amd/amdgpu/atombios_dp.c   |  8 
 .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c|  2 +-
 drivers/gpu/drm/display/drm_dp_helper.c|  7 +++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c   |  4 ++--
 drivers/gpu/drm/msm/dp/dp_ctrl.c   | 24 +-
 drivers/gpu/drm/msm/dp/dp_link.c   | 18 
 drivers/gpu/drm/radeon/atombios_dp.c   |  8 
 7 files changed, 28 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c 
b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
index 
521b9faab18059ed92ebb1dc9a9847e8426e7403..492813ab1b54197ba842075bc2909984c39bd5c1
 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
@@ -458,8 +458,8 @@ bool amdgpu_atombios_dp_needs_link_train(struct 
amdgpu_connector *amdgpu_connect
u8 link_status[DP_LINK_STATUS_SIZE];
struct amdgpu_connector_atom_dig *dig = amdgpu_connector->con_priv;
 
-   if (drm_dp_dpcd_read_link_status(&amdgpu_connector->ddc_bus->aux, 
link_status)
-   <= 0)
+   if (drm_dp_dpcd_read_link_status(&amdgpu_connector->ddc_bus->aux,
+link_status) < 0)
return false;
if (drm_dp_channel_eq_ok(link_status, dig->dp_lane_count))
return false;
@@ -616,7 +616,7 @@ amdgpu_atombios_dp_link_train_cr(struct 
amdgpu_atombios_dp_link_train_info *dp_i
drm_dp_link_train_clock_recovery_delay(dp_info->aux, 
dp_info->dpcd);
 
if (drm_dp_dpcd_read_link_status(dp_info->aux,
-dp_info->link_status) <= 0) {
+dp_info->link_status) < 0) {
DRM_ERROR("displayport link status failed\n");
break;
}
@@ -681,7 +681,7 @@ amdgpu_atombios_dp_link_train_ce(struct 
amdgpu_atombios_dp_link_train_info *dp_i
drm_dp_link_train_channel_eq_delay(dp_info->aux, dp_info->dpcd);
 
if (drm_dp_dpcd_read_link_status(dp_info->aux,
-dp_info->link_status) <= 0) {
+dp_info->link_status) < 0) {
DRM_ERROR("displayport link status failed\n");
break;
}
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c 
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 
81fad14c2cd598045d989c7d51f292bafb92c144..8d5420a5b691180c4d051a450d5d3d869a558d1a
 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2305,7 +2305,7 @@ static int cdns_mhdp_update_link_status(struct 
cdns_mhdp_device *mhdp)
 * If everything looks fine, just return, as we don't handle
 * DP IRQs.
 */
-   if (ret > 0 &&
+   if (!ret &&
drm_dp_channel_eq_ok(status, mhdp->link.num_lanes) &&
drm_dp_clock_recovery_ok(status, mhdp->link.num_lanes))
goto out;
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c 
b/drivers/gpu/drm/display/drm_dp_helper.c
index 
e43a8f4a252dae22eeaae1f4ca94da064303033d..410be0be233ad94702af423262a7d98e21afbfeb
 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -778,14 +778,13 @@ EXPORT_SYMBOL(drm_dp_dpcd_write);
  * @aux: DisplayPort AUX channel
  * @status: buffer to store the link status in (must be at least 6 bytes)
  *
- * Returns the number of bytes transferred on success or a negative error
- * code on failure.
+ * Returns a negative error code on failure or 0 on success.
  */
 int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 u8 status[DP_LINK_STATUS_SIZE])
 {
-   return drm_dp_dpcd_read(aux, DP_LANE0_1_STATUS, status,
-   DP_LINK_STATUS_SIZE);
+   return drm_dp_dpcd_read_data(aux, DP_LANE0_1_STATUS, status,
+DP_LINK_STATUS_SIZE);
 }
 EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c 
b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
index 
f6355c16cc0ab2e28408ab8a7246f4ca17710456..a3b78b0fd53ef854a54edf40fb333766da88f1c6
 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_li

[PATCH RFC v3 7/7] drm/display: dp-tunnel: use new DCPD access helpers

From: Dmitry Baryshkov 

Switch drm_dp_tunnel.c to use new set of DPCD read / write helpers.

Reviewed-by: Lyude Paul 
Acked-by: Jani Nikula 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/display/drm_dp_tunnel.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_tunnel.c 
b/drivers/gpu/drm/display/drm_dp_tunnel.c
index 
90fe07a89260e21e78f2db7f57a90602be921a11..076edf1610480275c62395334ab0536befa42f15
 100644
--- a/drivers/gpu/drm/display/drm_dp_tunnel.c
+++ b/drivers/gpu/drm/display/drm_dp_tunnel.c
@@ -222,7 +222,7 @@ static int read_tunnel_regs(struct drm_dp_aux *aux, struct 
drm_dp_tunnel_regs *r
while ((len = next_reg_area(&offset))) {
int address = DP_TUNNELING_BASE + offset;
 
-   if (drm_dp_dpcd_read(aux, address, tunnel_reg_ptr(regs, 
address), len) < 0)
+   if (drm_dp_dpcd_read_data(aux, address, tunnel_reg_ptr(regs, 
address), len) < 0)
return -EIO;
 
offset += len;
@@ -913,7 +913,7 @@ static int set_bw_alloc_mode(struct drm_dp_tunnel *tunnel, 
bool enable)
u8 mask = DP_DISPLAY_DRIVER_BW_ALLOCATION_MODE_ENABLE | 
DP_UNMASK_BW_ALLOCATION_IRQ;
u8 val;
 
-   if (drm_dp_dpcd_readb(tunnel->aux, DP_DPTX_BW_ALLOCATION_MODE_CONTROL, 
&val) < 0)
+   if (drm_dp_dpcd_read_byte(tunnel->aux, 
DP_DPTX_BW_ALLOCATION_MODE_CONTROL, &val) < 0)
goto out_err;
 
if (enable)
@@ -921,7 +921,7 @@ static int set_bw_alloc_mode(struct drm_dp_tunnel *tunnel, 
bool enable)
else
val &= ~mask;
 
-   if (drm_dp_dpcd_writeb(tunnel->aux, DP_DPTX_BW_ALLOCATION_MODE_CONTROL, 
val) < 0)
+   if (drm_dp_dpcd_write_byte(tunnel->aux, 
DP_DPTX_BW_ALLOCATION_MODE_CONTROL, val) < 0)
goto out_err;
 
tunnel->bw_alloc_enabled = enable;
@@ -1039,7 +1039,7 @@ static int clear_bw_req_state(struct drm_dp_aux *aux)
 {
u8 bw_req_mask = DP_BW_REQUEST_SUCCEEDED | DP_BW_REQUEST_FAILED;
 
-   if (drm_dp_dpcd_writeb(aux, DP_TUNNELING_STATUS, bw_req_mask) < 0)
+   if (drm_dp_dpcd_write_byte(aux, DP_TUNNELING_STATUS, bw_req_mask) < 0)
return -EIO;
 
return 0;
@@ -1052,7 +1052,7 @@ static int bw_req_complete(struct drm_dp_aux *aux, bool 
*status_changed)
u8 val;
int err;
 
-   if (drm_dp_dpcd_readb(aux, DP_TUNNELING_STATUS, &val) < 0)
+   if (drm_dp_dpcd_read_byte(aux, DP_TUNNELING_STATUS, &val) < 0)
return -EIO;
 
*status_changed = val & status_change_mask;
@@ -1095,7 +1095,7 @@ static int allocate_tunnel_bw(struct drm_dp_tunnel 
*tunnel, int bw)
if (err)
goto out;
 
-   if (drm_dp_dpcd_writeb(tunnel->aux, DP_REQUEST_BW, request_bw) < 0) {
+   if (drm_dp_dpcd_write_byte(tunnel->aux, DP_REQUEST_BW, request_bw) < 0) 
{
err = -EIO;
goto out;
}
@@ -1196,13 +1196,13 @@ static int check_and_clear_status_change(struct 
drm_dp_tunnel *tunnel)
u8 mask = DP_BW_ALLOCATION_CAPABILITY_CHANGED | DP_ESTIMATED_BW_CHANGED;
u8 val;
 
-   if (drm_dp_dpcd_readb(tunnel->aux, DP_TUNNELING_STATUS, &val) < 0)
+   if (drm_dp_dpcd_read_byte(tunnel->aux, DP_TUNNELING_STATUS, &val) < 0)
goto out_err;
 
val &= mask;
 
if (val) {
-   if (drm_dp_dpcd_writeb(tunnel->aux, DP_TUNNELING_STATUS, val) < 
0)
+   if (drm_dp_dpcd_write_byte(tunnel->aux, DP_TUNNELING_STATUS, 
val) < 0)
goto out_err;
 
return 1;
@@ -1215,7 +1215,7 @@ static int check_and_clear_status_change(struct 
drm_dp_tunnel *tunnel)
 * Check for estimated BW changes explicitly to account for lost
 * BW change notifications.
 */
-   if (drm_dp_dpcd_readb(tunnel->aux, DP_ESTIMATED_BW, &val) < 0)
+   if (drm_dp_dpcd_read_byte(tunnel->aux, DP_ESTIMATED_BW, &val) < 0)
goto out_err;
 
if (val * tunnel->bw_granularity != tunnel->estimated_bw)
@@ -1300,7 +1300,7 @@ int drm_dp_tunnel_handle_irq(struct drm_dp_tunnel_mgr 
*mgr, struct drm_dp_aux *a
 {
u8 val;
 
-   if (drm_dp_dpcd_readb(aux, DP_TUNNELING_STATUS, &val) < 0)
+   if (drm_dp_dpcd_read_byte(aux, DP_TUNNELING_STATUS, &val) < 0)
return -EIO;
 
if (val & (DP_BW_REQUEST_SUCCEEDED | DP_BW_REQUEST_FAILED))

-- 
2.39.5

[PATCH RFC v3 5/7] drm/display: dp-cec: use new DCPD access helpers

From: Dmitry Baryshkov 

Switch drm_dp_cec.c to use new set of DPCD read / write helpers.

Reviewed-by: Lyude Paul 
Acked-by: Jani Nikula 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/display/drm_dp_cec.c | 37 ++--
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_cec.c 
b/drivers/gpu/drm/display/drm_dp_cec.c
index 
56a4965e518cc237c992a2e31b9f6de05c14766a..ed31471bd0e28826254ecedac48c5c126729d470
 100644
--- a/drivers/gpu/drm/display/drm_dp_cec.c
+++ b/drivers/gpu/drm/display/drm_dp_cec.c
@@ -96,7 +96,7 @@ static int drm_dp_cec_adap_enable(struct cec_adapter *adap, 
bool enable)
u32 val = enable ? DP_CEC_TUNNELING_ENABLE : 0;
ssize_t err = 0;
 
-   err = drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_CONTROL, val);
+   err = drm_dp_dpcd_write_byte(aux, DP_CEC_TUNNELING_CONTROL, val);
return (enable && err < 0) ? err : 0;
 }
 
@@ -112,7 +112,7 @@ static int drm_dp_cec_adap_log_addr(struct cec_adapter 
*adap, u8 addr)
la_mask |= adap->log_addrs.log_addr_mask | (1 << addr);
mask[0] = la_mask & 0xff;
mask[1] = la_mask >> 8;
-   err = drm_dp_dpcd_write(aux, DP_CEC_LOGICAL_ADDRESS_MASK, mask, 2);
+   err = drm_dp_dpcd_write_data(aux, DP_CEC_LOGICAL_ADDRESS_MASK, mask, 2);
return (addr != CEC_LOG_ADDR_INVALID && err < 0) ? err : 0;
 }
 
@@ -123,15 +123,14 @@ static int drm_dp_cec_adap_transmit(struct cec_adapter 
*adap, u8 attempts,
unsigned int retries = min(5, attempts - 1);
ssize_t err;
 
-   err = drm_dp_dpcd_write(aux, DP_CEC_TX_MESSAGE_BUFFER,
-   msg->msg, msg->len);
+   err = drm_dp_dpcd_write_data(aux, DP_CEC_TX_MESSAGE_BUFFER,
+msg->msg, msg->len);
if (err < 0)
return err;
 
-   err = drm_dp_dpcd_writeb(aux, DP_CEC_TX_MESSAGE_INFO,
-(msg->len - 1) | (retries << 4) |
-DP_CEC_TX_MESSAGE_SEND);
-   return err < 0 ? err : 0;
+   return drm_dp_dpcd_write_byte(aux, DP_CEC_TX_MESSAGE_INFO,
+ (msg->len - 1) | (retries << 4) |
+ DP_CEC_TX_MESSAGE_SEND);
 }
 
 static int drm_dp_cec_adap_monitor_all_enable(struct cec_adapter *adap,
@@ -144,13 +143,13 @@ static int drm_dp_cec_adap_monitor_all_enable(struct 
cec_adapter *adap,
if (!(adap->capabilities & CEC_CAP_MONITOR_ALL))
return 0;
 
-   err = drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_CONTROL, &val);
-   if (err >= 0) {
+   err = drm_dp_dpcd_read_byte(aux, DP_CEC_TUNNELING_CONTROL, &val);
+   if (!err) {
if (enable)
val |= DP_CEC_SNOOPING_ENABLE;
else
val &= ~DP_CEC_SNOOPING_ENABLE;
-   err = drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_CONTROL, val);
+   err = drm_dp_dpcd_write_byte(aux, DP_CEC_TUNNELING_CONTROL, 
val);
}
return (enable && err < 0) ? err : 0;
 }
@@ -194,7 +193,7 @@ static int drm_dp_cec_received(struct drm_dp_aux *aux)
u8 rx_msg_info;
ssize_t err;
 
-   err = drm_dp_dpcd_readb(aux, DP_CEC_RX_MESSAGE_INFO, &rx_msg_info);
+   err = drm_dp_dpcd_read_byte(aux, DP_CEC_RX_MESSAGE_INFO, &rx_msg_info);
if (err < 0)
return err;
 
@@ -202,7 +201,7 @@ static int drm_dp_cec_received(struct drm_dp_aux *aux)
return 0;
 
msg.len = (rx_msg_info & DP_CEC_RX_MESSAGE_LEN_MASK) + 1;
-   err = drm_dp_dpcd_read(aux, DP_CEC_RX_MESSAGE_BUFFER, msg.msg, msg.len);
+   err = drm_dp_dpcd_read_data(aux, DP_CEC_RX_MESSAGE_BUFFER, msg.msg, 
msg.len);
if (err < 0)
return err;
 
@@ -215,7 +214,7 @@ static void drm_dp_cec_handle_irq(struct drm_dp_aux *aux)
struct cec_adapter *adap = aux->cec.adap;
u8 flags;
 
-   if (drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_IRQ_FLAGS, &flags) < 0)
+   if (drm_dp_dpcd_read_byte(aux, DP_CEC_TUNNELING_IRQ_FLAGS, &flags) < 0)
return;
 
if (flags & DP_CEC_RX_MESSAGE_INFO_VALID)
@@ -230,7 +229,7 @@ static void drm_dp_cec_handle_irq(struct drm_dp_aux *aux)
 (DP_CEC_TX_ADDRESS_NACK_ERROR | DP_CEC_TX_DATA_NACK_ERROR))
cec_transmit_attempt_done(adap, CEC_TX_STATUS_NACK |
CEC_TX_STATUS_MAX_RETRIES);
-   drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_IRQ_FLAGS, flags);
+   drm_dp_dpcd_write_byte(aux, DP_CEC_TUNNELING_IRQ_FLAGS, flags);
 }
 
 /**
@@ -253,13 +252,13 @@ void drm_dp_cec_irq(struct drm_dp_aux *aux)
if (!aux->cec.adap)
goto unlock;
 
-   ret = drm_dp_dpcd_readb(aux, DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
-   &cec_irq);
+   ret = drm_dp_dpcd_read_byte(aux, DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
+

[PATCH RFC v3 6/7] drm/display: dp-mst-topology: use new DCPD access helpers

From: Dmitry Baryshkov 

Switch drm_dp_mst_topology.c to use new set of DPCD read / write helpers.

Reviewed-by: Lyude Paul 
Acked-by: Jani Nikula 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 105 +-
 1 file changed, 51 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c 
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 
8b68bb3fbffb04dfcbd910fd0fd78b998440d6e8..e8716e73480bdf6abbef71897d1632f69a7b8a47
 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -2201,15 +2201,12 @@ static int drm_dp_check_mstb_guid(struct 
drm_dp_mst_branch *mstb, guid_t *guid)
 mstb->port_parent,
 DP_GUID, sizeof(buf), buf);
} else {
-   ret = drm_dp_dpcd_write(mstb->mgr->aux,
-   DP_GUID, buf, sizeof(buf));
+   ret = drm_dp_dpcd_write_data(mstb->mgr->aux,
+DP_GUID, buf, sizeof(buf));
}
}
 
-   if (ret < 16 && ret > 0)
-   return -EPROTO;
-
-   return ret == 16 ? 0 : ret;
+   return ret;
 }
 
 static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
@@ -2744,14 +2741,13 @@ static int drm_dp_send_sideband_msg(struct 
drm_dp_mst_topology_mgr *mgr,
do {
tosend = min3(mgr->max_dpcd_transaction_bytes, 16, total);
 
-   ret = drm_dp_dpcd_write(mgr->aux, regbase + offset,
-   &msg[offset],
-   tosend);
-   if (ret != tosend) {
-   if (ret == -EIO && retries < 5) {
-   retries++;
-   goto retry;
-   }
+   ret = drm_dp_dpcd_write_data(mgr->aux, regbase + offset,
+&msg[offset],
+tosend);
+   if (ret == -EIO && retries < 5) {
+   retries++;
+   goto retry;
+   } else if (ret < 0) {
drm_dbg_kms(mgr->dev, "failed to dpcd write %d %d\n", 
tosend, ret);
 
return -EIO;
@@ -3624,7 +3620,7 @@ enum drm_dp_mst_mode drm_dp_read_mst_cap(struct 
drm_dp_aux *aux,
if (dpcd[DP_DPCD_REV] < DP_DPCD_REV_12)
return DRM_DP_SST;
 
-   if (drm_dp_dpcd_readb(aux, DP_MSTM_CAP, &mstm_cap) != 1)
+   if (drm_dp_dpcd_read_byte(aux, DP_MSTM_CAP, &mstm_cap) < 0)
return DRM_DP_SST;
 
if (mstm_cap & DP_MST_CAP)
@@ -3679,10 +3675,10 @@ int drm_dp_mst_topology_mgr_set_mst(struct 
drm_dp_mst_topology_mgr *mgr, bool ms
mgr->mst_primary = mstb;
drm_dp_mst_topology_get_mstb(mgr->mst_primary);
 
-   ret = drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL,
-DP_MST_EN |
-DP_UP_REQ_EN |
-DP_UPSTREAM_IS_SRC);
+   ret = drm_dp_dpcd_write_byte(mgr->aux, DP_MSTM_CTRL,
+DP_MST_EN |
+DP_UP_REQ_EN |
+DP_UPSTREAM_IS_SRC);
if (ret < 0)
goto out_unlock;
 
@@ -3697,7 +3693,7 @@ int drm_dp_mst_topology_mgr_set_mst(struct 
drm_dp_mst_topology_mgr *mgr, bool ms
mstb = mgr->mst_primary;
mgr->mst_primary = NULL;
/* this can fail if the device is gone */
-   drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
+   drm_dp_dpcd_write_byte(mgr->aux, DP_MSTM_CTRL, 0);
ret = 0;
mgr->payload_id_table_cleared = false;
 
@@ -3763,8 +3759,8 @@ EXPORT_SYMBOL(drm_dp_mst_topology_queue_probe);
 void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr)
 {
mutex_lock(&mgr->lock);
-   drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL,
-  DP_MST_EN | DP_UPSTREAM_IS_SRC);
+   drm_dp_dpcd_write_byte(mgr->aux, DP_MSTM_CTRL,
+  DP_MST_EN | DP_UPSTREAM_IS_SRC);
mutex_unlock(&mgr->lock);
flush_work(&mgr->up_req_work);
flush_work(&mgr->work);
@@ -3813,18 +3809,18 @@ int drm_dp_mst_topology_mgr_resume(struct 
drm_dp_mst_topology_mgr *mgr,
goto out_fail;
}
 
-   ret = drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL,
-DP_MST_EN |
-DP_UP_REQ_EN |
-DP_UPSTREAM_IS_SRC);
+   ret = drm_dp_dpcd_write_byte(mgr->aux, DP_MSTM_CTRL,

[PATCH v3 0/8] drm/msm/dpu: improve CTL handling on DPU >= 5.0 platforms

Since version 5.0 the DPU got an improved way of handling multi-output
configurations. It is now possible to program all pending changes
through a single CTL and flush everything at the same time.

Implement corresponding changes in the DPU driver.

Signed-off-by: Dmitry Baryshkov 
---
Changes in v3:
- Rebased on top of msm-next
- Link to v2: 
https://lore.kernel.org/r/20250228-dpu-active-ctl-v2-0-9a9df2ee5...@linaro.org

Changes in v2:
- Made CTL_MERGE_3D_ACTIVE writes unconditional (Marijn)
- Added CTL_INTF_MASTER clearing in dpu_hw_ctl_reset_intf_cfg_v1
  (Marijn)
- Added a patch to drop extra rm->has_legacy_ctls condition (and an
  explanation why it can not be folded in an earlier patch).
- Link to v1: 
https://lore.kernel.org/r/20250220-dpu-active-ctl-v1-0-71ca67a56...@linaro.org

---
Dmitry Baryshkov (8):
  drm/msm/dpu: don't overwrite CTL_MERGE_3D_ACTIVE register
  drm/msm/dpu: program master INTF value
  drm/msm/dpu: pass master interface to CTL configuration
  drm/msm/dpu: use single CTL if it is the only CTL returned by RM
  drm/msm/dpu: don't select single flush for active CTL blocks
  drm/msm/dpu: allocate single CTL for DPU >= 5.0
  drm/msm/dpu: remove DPU_CTL_SPLIT_DISPLAY from CTL blocks on DPU >= 5.0
  drm/msm/dpu: drop now-unused condition for has_legacy_ctls

 .../drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h|  5 ++---
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h |  5 ++---
 .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h|  4 ++--
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_2_sm7150.h |  4 ++--
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h |  5 ++---
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h |  5 ++---
 .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h   |  5 ++---
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h |  5 ++---
 .../drm/msm/disp/dpu1/catalog/dpu_8_4_sa8775p.h|  5 ++---
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h |  5 ++---
 .../drm/msm/disp/dpu1/catalog/dpu_9_2_x1e80100.h   |  5 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c|  6 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |  2 ++
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  5 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 20 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 25 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h |  2 ++
 18 files changed, 67 insertions(+), 48 deletions(-)
---
base-commit: 565351ae7e0cee80e9b5ed84452a5b13644ffc4d
change-id: 20250209-dpu-active-ctl-08cca4d8b08a

Best regards,
-- 
Dmitry Baryshkov

[PATCH v3 1/8] drm/msm/dpu: don't overwrite CTL_MERGE_3D_ACTIVE register

From: Dmitry Baryshkov 

In case of complex pipelines (e.g. the forthcoming quad-pipe) the DPU
might use more that one MERGE_3D block for a single output.  Follow the
pattern and extend the CTL_MERGE_3D_ACTIVE active register instead of
simply writing new value there. Currently at most one MERGE_3D block is
being used, so this has no impact on existing targets.

Reviewed-by: Marijn Suijten 
Tested-by: Neil Armstrong  # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 
411a7cf088eb72f856940c09b0af9e108ccade4b..cef3bfaa4af82ebc55fb8cf76adef3075c7d73e3
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -563,6 +563,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
u32 wb_active = 0;
u32 cwb_active = 0;
u32 mode_sel = 0;
+   u32 merge_3d_active = 0;
 
/* CTL_TOP[31:28] carries group_id to collate CTL paths
 * per VM. Explicitly disable it until VM support is
@@ -578,6 +579,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
wb_active = DPU_REG_READ(c, CTL_WB_ACTIVE);
cwb_active = DPU_REG_READ(c, CTL_CWB_ACTIVE);
dsc_active = DPU_REG_READ(c, CTL_DSC_ACTIVE);
+   merge_3d_active = DPU_REG_READ(c, CTL_MERGE_3D_ACTIVE);
 
if (cfg->intf)
intf_active |= BIT(cfg->intf - INTF_0);
@@ -591,15 +593,15 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
if (cfg->dsc)
dsc_active |= cfg->dsc;
 
+   if (cfg->merge_3d)
+   merge_3d_active |= BIT(cfg->merge_3d - MERGE_3D_0);
+
DPU_REG_WRITE(c, CTL_TOP, mode_sel);
DPU_REG_WRITE(c, CTL_INTF_ACTIVE, intf_active);
DPU_REG_WRITE(c, CTL_WB_ACTIVE, wb_active);
DPU_REG_WRITE(c, CTL_CWB_ACTIVE, cwb_active);
DPU_REG_WRITE(c, CTL_DSC_ACTIVE, dsc_active);
-
-   if (cfg->merge_3d)
-   DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
- BIT(cfg->merge_3d - MERGE_3D_0));
+   DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE, merge_3d_active);
 
if (cfg->cdm)
DPU_REG_WRITE(c, CTL_CDM_ACTIVE, cfg->cdm);

-- 
2.39.5

[PATCH v3 2/8] drm/msm/dpu: program master INTF value

From: Dmitry Baryshkov 

If several interfaces are being handled through a single CTL, a main
('master') INTF needs to be programmed into a separate register. Write
corresponding value into that register.

Co-developed-by: Marijn Suijten 
Signed-off-by: Marijn Suijten 
Reviewed-by: Marijn Suijten 
Tested-by: Neil Armstrong  # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 12 
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 
cef3bfaa4af82ebc55fb8cf76adef3075c7d73e3..21f4d403e3c278d83d7eaa6a7dd53f471d9e296d
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -603,6 +603,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
DPU_REG_WRITE(c, CTL_DSC_ACTIVE, dsc_active);
DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE, merge_3d_active);
 
+   if (cfg->intf_master)
+   DPU_REG_WRITE(c, CTL_INTF_MASTER, BIT(cfg->intf_master - 
INTF_0));
+
if (cfg->cdm)
DPU_REG_WRITE(c, CTL_CDM_ACTIVE, cfg->cdm);
 }
@@ -645,6 +648,7 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl 
*ctx,
 {
struct dpu_hw_blk_reg_map *c = &ctx->hw;
u32 intf_active = 0;
+   u32 intf_master = 0;
u32 wb_active = 0;
u32 cwb_active = 0;
u32 merge3d_active = 0;
@@ -672,6 +676,14 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl 
*ctx,
intf_active = DPU_REG_READ(c, CTL_INTF_ACTIVE);
intf_active &= ~BIT(cfg->intf - INTF_0);
DPU_REG_WRITE(c, CTL_INTF_ACTIVE, intf_active);
+
+   intf_master = DPU_REG_READ(c, CTL_INTF_MASTER);
+
+   /* Unset this intf as master, if it is the current master */
+   if (intf_master == BIT(cfg->intf - INTF_0)) {
+   DPU_DEBUG_DRIVER("Unsetting INTF_%d master\n", 
cfg->intf - INTF_0);
+   DPU_REG_WRITE(c, CTL_INTF_MASTER, 0);
+   }
}
 
if (cfg->cwb) {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
index 
080a9550a0cc6530b4115165dd737857b6213d15..cea23436fc80a17a679363a47f9f287b72623a1c
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -36,6 +36,7 @@ struct dpu_hw_stage_cfg {
 /**
  * struct dpu_hw_intf_cfg :Describes how the DPU writes data to output 
interface
  * @intf : Interface id
+ * @intf_master:   Master interface id in the dual pipe topology
  * @mode_3d:   3d mux configuration
  * @merge_3d:  3d merge block used
  * @intf_mode_sel: Interface mode, cmd / vid
@@ -46,6 +47,7 @@ struct dpu_hw_stage_cfg {
  */
 struct dpu_hw_intf_cfg {
enum dpu_intf intf;
+   enum dpu_intf intf_master;
enum dpu_wb wb;
enum dpu_3d_blend_mode mode_3d;
enum dpu_merge_3d merge_3d;

-- 
2.39.5

[PATCH v3 6/8] drm/msm/dpu: allocate single CTL for DPU >= 5.0

From: Dmitry Baryshkov 

Unlike previous generation, since DPU 5.0 it is possible to use just one
CTL to handle all INTF and WB blocks for a single output. And one has to
use single CTL to support bonded DSI config. Allocate single CTL for
these DPU versions.

Reviewed-by: Marijn Suijten 
Tested-by: Neil Armstrong  # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 28 +++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h |  2 ++
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 
3efbba425ca6e037cb9646981ebb0f0354ffea8e..c72b968d58a65960605456e752278def2a21df7b
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -53,6 +53,8 @@ int dpu_rm_init(struct drm_device *dev,
/* Clear, setup lists */
memset(rm, 0, sizeof(*rm));
 
+   rm->has_legacy_ctls = (cat->mdss_ver->core_major_ver < 5);
+
/* Interrogate HW catalog and create tracking items for hw blocks */
for (i = 0; i < cat->mixer_count; i++) {
struct dpu_hw_mixer *hw;
@@ -434,20 +436,19 @@ static int _dpu_rm_reserve_ctls(
int i = 0, j, num_ctls;
bool needs_split_display;
 
-   /*
-* For non-CWB mode, each hw_intf needs its own hw_ctl to program its
-* control path.
-*
-* Hardcode num_ctls to 1 if CWB is enabled because in CWB, both the
-* writeback and real-time encoders must be driven by the same control
-* path
-*/
-   if (top->cwb_enabled)
-   num_ctls = 1;
-   else
+   if (rm->has_legacy_ctls) {
+   /*
+* TODO: check if there is a need for special handling if
+* DPU < 5.0 get CWB support.
+*/
num_ctls = top->num_intf;
 
-   needs_split_display = _dpu_rm_needs_split_display(top);
+   needs_split_display = _dpu_rm_needs_split_display(top);
+   } else {
+   /* use single CTL */
+   num_ctls = 1;
+   needs_split_display = false;
+   }
 
for (j = 0; j < ARRAY_SIZE(rm->ctl_blks); j++) {
const struct dpu_hw_ctl *ctl;
@@ -465,7 +466,8 @@ static int _dpu_rm_reserve_ctls(
 
DPU_DEBUG("ctl %d caps 0x%lX\n", j + CTL_0, features);
 
-   if (needs_split_display != has_split_display)
+   if (rm->has_legacy_ctls &&
+   needs_split_display != has_split_display)
continue;
 
ctl_idx[i] = j;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 
a19dbdb1b6f48ad708f0d512c2460d092856f52f..aa62966056d489d9c94c61f24051a2f3e7b7ed89
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -24,6 +24,7 @@ struct dpu_global_state;
  * @dspp_blks: array of dspp hardware resources
  * @hw_sspp: array of sspp hardware resources
  * @cdm_blk: cdm hardware resource
+ * @has_legacy_ctls: DPU uses pre-ACTIVE CTL blocks.
  */
 struct dpu_rm {
struct dpu_hw_blk *pingpong_blks[PINGPONG_MAX - PINGPONG_0];
@@ -37,6 +38,7 @@ struct dpu_rm {
struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
struct dpu_hw_sspp *hw_sspp[SSPP_MAX - SSPP_NONE];
struct dpu_hw_blk *cdm_blk;
+   bool has_legacy_ctls;
 };
 
 struct dpu_rm_sspp_requirements {

-- 
2.39.5

[PATCH v3 3/8] drm/msm/dpu: pass master interface to CTL configuration

From: Dmitry Baryshkov 

Active controls require setup of the master interface. Pass the selected
interface to CTL configuration.

Reviewed-by: Marijn Suijten 
Tested-by: Neil Armstrong  # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 
da9994a79ca293ec0265680c438835742102db2a..a0ba55ab3c894c200225fe48ec6214ae4135d059
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -60,6 +60,8 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
return;
 
intf_cfg.intf = phys_enc->hw_intf->idx;
+   if (phys_enc->split_role == ENC_ROLE_MASTER)
+   intf_cfg.intf_master = phys_enc->hw_intf->idx;
intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD;
intf_cfg.stream_sel = cmd_enc->stream_sel;
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 
abd6600046cb3a91bf88ca240fd9b9c306b0ea2e..232055473ba55998b79dd2e8c752c129bbffbff4
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -298,6 +298,8 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
if (phys_enc->hw_cdm)
intf_cfg.cdm = phys_enc->hw_cdm->idx;
intf_cfg.intf = phys_enc->hw_intf->idx;
+   if (phys_enc->split_role == ENC_ROLE_MASTER)
+   intf_cfg.intf_master = phys_enc->hw_intf->idx;
intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_VID;
intf_cfg.stream_sel = 0; /* Don't care value for video mode */
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);

-- 
2.39.5

[PATCH v3 8/8] drm/msm/dpu: drop now-unused condition for has_legacy_ctls

From: Dmitry Baryshkov 

Now as we have dropped the DPU_CTL_SPLIT_DISPLAY from DPU >= 5.0
configuration, drop the rm->has_legacy_ctl condition which short-cutted
the check for those platforms.

Suggested-by: Marijn Suijten 
Reviewed-by: Marijn Suijten 
Tested-by: Neil Armstrong  # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov 
---

Note, it is imposible to reoder commits in any other sensible way. The
DPU_CTL_SPLIT_DISPLAY can not be dropped before the patch that enables
single-CTL support.
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 
c72b968d58a65960605456e752278def2a21df7b..2e296f79cba1437470eeb30900a650f6f4e334b6
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -466,8 +466,7 @@ static int _dpu_rm_reserve_ctls(
 
DPU_DEBUG("ctl %d caps 0x%lX\n", j + CTL_0, features);
 
-   if (rm->has_legacy_ctls &&
-   needs_split_display != has_split_display)
+   if (needs_split_display != has_split_display)
continue;
 
ctl_idx[i] = j;

-- 
2.39.5

Re: [PATCH 2/2] drm/bridge: fsl-ldb: make warning message more informative

On 03/07/2025, Luca Ceresoli wrote:
> This warning notifies a clock was set to an inaccurate value. Modify the
> string to also show the clock name.
> 
> While doing that also rewrap the entire function call.
> 
> Signed-off-by: Luca Ceresoli 
> ---
>  drivers/gpu/drm/bridge/fsl-ldb.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c 
> b/drivers/gpu/drm/bridge/fsl-ldb.c
> index 
> 0fc8a14fd80062248a43b8b93272101a7ca6158a..c7c899a9644bb827845fb3fe96a9695d79a91474
>  100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -181,9 +181,9 @@ static void fsl_ldb_atomic_enable(struct drm_bridge 
> *bridge,
>  
>   configured_link_freq = clk_get_rate(fsl_ldb->clk);
>   if (configured_link_freq != requested_link_freq)
> - dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not 
> match requested LVDS clock: %lu Hz\n",
> -  configured_link_freq,
> -  requested_link_freq);
> + dev_warn(fsl_ldb->dev,
> +  "Configured %pC clock (%lu Hz) does not match 
> requested LVDS clock: %lu Hz\n",
> +  fsl_ldb->clk, configured_link_freq, 
> requested_link_freq);

Though this slightly changes the warning message userspace sees, I guess it is
acceptable.

Does it make sense to s/%pC/%pCn/ so that the clock name is printed in lower
case instead of upper case, since it seems that all i.MX specific clock names
are in lower case?

>  
>   clk_prepare_enable(fsl_ldb->clk);
>  
> 

-- 
Regards,
Liu Ying

Re: [PATCH 2/2] drm/bridge: fsl-ldb: make warning message more informative

On 03/07/2025, Frank Li wrote:
> On Thu, Mar 06, 2025 at 06:28:41PM +0100, Luca Ceresoli wrote:
>> This warning notifies a clock was set to an inaccurate value. Modify the
>> string to also show the clock name.
>>
>> While doing that also rewrap the entire function call.
>>
>> Signed-off-by: Luca Ceresoli 
>> ---
>>  drivers/gpu/drm/bridge/fsl-ldb.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c 
>> b/drivers/gpu/drm/bridge/fsl-ldb.c
>> index 
>> 0fc8a14fd80062248a43b8b93272101a7ca6158a..c7c899a9644bb827845fb3fe96a9695d79a91474
>>  100644
>> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
>> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
>> @@ -181,9 +181,9 @@ static void fsl_ldb_atomic_enable(struct drm_bridge 
>> *bridge,
>>
>>  configured_link_freq = clk_get_rate(fsl_ldb->clk);
>>  if (configured_link_freq != requested_link_freq)
>> -dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not 
>> match requested LVDS clock: %lu Hz\n",
>> - configured_link_freq,
>> - requested_link_freq);
>> +dev_warn(fsl_ldb->dev,
>> + "Configured %pC clock (%lu Hz) does not match 
>> requested LVDS clock: %lu Hz\n",
>> + fsl_ldb->clk, configured_link_freq, 
>> requested_link_freq);
> 
> commit message said show clock name, but %p is for pointer value. Are sure
> it show clock name?

%pC prints clock name.  Please see Documentation/core-api/printk-formats.rst.

> 
> Frank
> 
>>
>>  clk_prepare_enable(fsl_ldb->clk);
>>
>>
>> --
>> 2.48.1
>>

-- 
Regards,
Liu Ying

[PATCH v3 3/3] vfio/pci: Allow MMIO regions to be exported through dma-buf

>From Jason Gunthorpe:
"dma-buf has become a way to safely acquire a handle to non-struct page
memory that can still have lifetime controlled by the exporter. Notably
RDMA can now import dma-buf FDs and build them into MRs which allows for
PCI P2P operations. Extend this to allow vfio-pci to export MMIO memory
from PCI device BARs.

The patch design loosely follows the pattern in commit
db1a8dd916aa ("habanalabs: add support for dma-buf exporter") except this
does not support pinning.

Instead, this implements what, in the past, we've called a revocable
attachment using move. In normal situations the attachment is pinned, as a
BAR does not change physical address. However when the VFIO device is
closed, or a PCI reset is issued, access to the MMIO memory is revoked.

Revoked means that move occurs, but an attempt to immediately re-map the
memory will fail. In the reset case a future move will be triggered when
MMIO access returns. As both close and reset are under userspace control
it is expected that userspace will suspend use of the dma-buf before doing
these operations, the revoke is purely for kernel self-defense against a
hostile userspace."

Following enhancements are made to the original patch:
- Add support for creating dmabuf from multiple areas (or ranges)

Cc: Alex Williamson 
Cc: Simona Vetter 
Cc: Christian König 
Original-patch-by: Jason Gunthorpe 
Signed-off-by: Vivek Kasireddy 
---
 drivers/vfio/pci/Makefile  |   1 +
 drivers/vfio/pci/vfio_pci_config.c |  22 +-
 drivers/vfio/pci/vfio_pci_core.c   |  20 +-
 drivers/vfio/pci/vfio_pci_dmabuf.c | 359 +
 drivers/vfio/pci/vfio_pci_priv.h   |  23 ++
 include/linux/vfio_pci_core.h  |   1 +
 include/uapi/linux/vfio.h  |  25 ++
 7 files changed, 446 insertions(+), 5 deletions(-)
 create mode 100644 drivers/vfio/pci/vfio_pci_dmabuf.c

diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
index cf00c0a7e55c..c33ec0cbe930 100644
--- a/drivers/vfio/pci/Makefile
+++ b/drivers/vfio/pci/Makefile
@@ -2,6 +2,7 @@
 
 vfio-pci-core-y := vfio_pci_core.o vfio_pci_intrs.o vfio_pci_rdwr.o 
vfio_pci_config.o
 vfio-pci-core-$(CONFIG_VFIO_PCI_ZDEV_KVM) += vfio_pci_zdev.o
+vfio-pci-core-$(CONFIG_DMA_SHARED_BUFFER) += vfio_pci_dmabuf.o
 obj-$(CONFIG_VFIO_PCI_CORE) += vfio-pci-core.o
 
 vfio-pci-y := vfio_pci.o
diff --git a/drivers/vfio/pci/vfio_pci_config.c 
b/drivers/vfio/pci/vfio_pci_config.c
index 94142581c98c..ddbfea93a0b6 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -589,10 +589,12 @@ static int vfio_basic_config_write(struct 
vfio_pci_core_device *vdev, int pos,
virt_mem = !!(le16_to_cpu(*virt_cmd) & PCI_COMMAND_MEMORY);
new_mem = !!(new_cmd & PCI_COMMAND_MEMORY);
 
-   if (!new_mem)
+   if (!new_mem) {
vfio_pci_zap_and_down_write_memory_lock(vdev);
-   else
+   vfio_pci_dma_buf_move(vdev, true);
+   } else {
down_write(&vdev->memory_lock);
+   }
 
/*
 * If the user is writing mem/io enable (new_mem/io) and we
@@ -627,6 +629,8 @@ static int vfio_basic_config_write(struct 
vfio_pci_core_device *vdev, int pos,
*virt_cmd &= cpu_to_le16(~mask);
*virt_cmd |= cpu_to_le16(new_cmd & mask);
 
+   if (__vfio_pci_memory_enabled(vdev))
+   vfio_pci_dma_buf_move(vdev, false);
up_write(&vdev->memory_lock);
}
 
@@ -707,12 +711,16 @@ static int __init init_pci_cap_basic_perm(struct 
perm_bits *perm)
 static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
  pci_power_t state)
 {
-   if (state >= PCI_D3hot)
+   if (state >= PCI_D3hot) {
vfio_pci_zap_and_down_write_memory_lock(vdev);
-   else
+   vfio_pci_dma_buf_move(vdev, true);
+   } else {
down_write(&vdev->memory_lock);
+   }
 
vfio_pci_set_power_state(vdev, state);
+   if (__vfio_pci_memory_enabled(vdev))
+   vfio_pci_dma_buf_move(vdev, false);
up_write(&vdev->memory_lock);
 }
 
@@ -900,7 +908,10 @@ static int vfio_exp_config_write(struct 
vfio_pci_core_device *vdev, int pos,
 
if (!ret && (cap & PCI_EXP_DEVCAP_FLR)) {
vfio_pci_zap_and_down_write_memory_lock(vdev);
+   vfio_pci_dma_buf_move(vdev, true);
pci_try_reset_function(vdev->pdev);
+   if (__vfio_pci_memory_enabled(vdev))
+   vfio_pci_dma_buf_move(vdev, true);
up_write(&vdev->memory_lock);
}
}
@@ -982,7 +993,10 @@ static int vfio_af_config_write(struct 
vfio_pci_core_device *vdev, int pos,
 
if (!ret && (cap & PCI_AF_CAP_FLR) && (cap & PCI_AF

Re: [PATCH -next] drm/msm/dpu: Remove duplicate dpu_hw_cwb.h header

On Fri, Mar 07, 2025 at 09:50:30AM +0800, Jiapeng Chong wrote:
> ./drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c: dpu_hw_cwb.h is included more 
> than once.
> 
> Reported-by: Abaci Robot 
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=19239
> Signed-off-by: Jiapeng Chong 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 1 -
>  1 file changed, 1 deletion(-)
> 

Also

Fixes: dd331404ac7c ("drm/msm/dpu: Configure CWB in writeback encoder")


-- 
With best wishes
Dmitry

[PATCH v3 0/3] vfio/pci: Allow MMIO regions to be exported through dma-buf

This is an attempt to revive the patches posted by Jason Gunthorpe at:
https://patchwork.kernel.org/project/linux-media/cover/0-v2-472615b3877e+28f7-vfio_dma_buf_...@nvidia.com/

Here is the cover letter text from Jason's original series:
"dma-buf has become a way to safely acquire a handle to non-struct page
memory that can still have lifetime controlled by the exporter. Notably
RDMA can now import dma-buf FDs and build them into MRs which allows for
PCI P2P operations. Extend this to allow vfio-pci to export MMIO memory
from PCI device BARs.

This series supports a use case for SPDK where a NVMe device will be owned
by SPDK through VFIO but interacting with a RDMA device. The RDMA device
may directly access the NVMe CMB or directly manipulate the NVMe device's
doorbell using PCI P2P.

However, as a general mechanism, it can support many other scenarios with
VFIO. I imagine this dmabuf approach to be usable by iommufd as well for
generic and safe P2P mappings."

In addition to the SPDK use-case mentioned above, the capability added
in this patch series can also be useful when a buffer (located in device
memory such as VRAM) needs to be shared between any two dGPU devices or
instances (assuming one of them is bound to VFIO PCI) as long as they
are P2P DMA compatible.

The original series has been rebased and augmented to support creation
of the dmabuf from multiple ranges of a region.

Changelog:

v2 -> v3:
- Rebase onto 6.14-rc5
- Drop the mmap handler given that it is undesirable in some use-cases
- Rename the file dma_buf.c to vfio_pci_dmabuf.c to be consistent with
  other files in vfio/pci directory

v1 -> v2:
- Rebase on 6.10-rc4
- Update the copyright year in dma_buf.c (Zhu Yanjun)
- Check the revoked flag during mmap() and also revoke the mappings
  as part of move when access to the MMIO space is disabled (Alex)
- Include VM_ALLOW_ANY_UNCACHED and VM_IO flags for mmap (Alex)
- Fix memory leak of ranges when creation of priv fails (Alex)
- Check return value of vfio_device_try_get_registration() (Alex)
- Invoke dma_buf move for runtime PM and FLR cases as well (Alex)
- Add a separate patch to have all the feature functions take
  the core device pointer instead of the main device ptr (Alex)
- Use the regular DMA APIs (that were part of original series) instead
  of PCI P2P DMA APIs while mapping the dma_buf (Jason)
- Rename the region's ranges from p2p_areas to dma_ranges (Jason)
- Add comments in vfio_pci_dma_buf_move() to describe how the locking
  is expected to work (Jason)

This series is available at:
https://gitlab.freedesktop.org/Vivek/drm-tip/-/commits/vfio_dmabuf_v3

along with additional patches (needed for testing) for Qemu here:
https://gitlab.freedesktop.org/Vivek/qemu/-/commits/vfio_dmabuf_3

This series is tested using the following method:
- Run Qemu with the following relevant options:
  qemu-system-x86_64 -m 4096m 
  -device vfio-pci,host=:03:00.1
  -device virtio-vga,max_outputs=1,blob=true,xres=1920,yres=1080
  -display gtk,gl=on
  -object memory-backend-memfd,id=mem1,size=4096M
  -machine memory-backend=mem1 ...
- Run upstream Weston with following options in the Guest VM:
  ./weston --drm-device=card1 --additional-devices=card0
  where card1 is a dGPU (assigned to vfio-pci and using xe driver
  in Guest VM), card0 is virtio-gpu.
- Or run Mutter with Wayland backend in the Guest VM with the dGPU
  designated as the primary

Cc: Jason Gunthorpe 
Cc: Leon Romanovsky 
Cc: Christian König 
Cc: Simona Vetter 
Cc: Gerd Hoffmann 
Cc: Alex Williamson 
Cc: Yishai Hadas 
Cc: Shameer Kolothum 
Cc: Kevin Tian 
Cc: Wei Lin Guay 
Cc: Xu Yilun 

Vivek Kasireddy (3):
  vfio: Export vfio device get and put registration helpers
  vfio/pci: Share the core device pointer while invoking feature
functions
  vfio/pci: Allow MMIO regions to be exported through dma-buf

 drivers/vfio/pci/Makefile  |   1 +
 drivers/vfio/pci/vfio_pci_config.c |  22 +-
 drivers/vfio/pci/vfio_pci_core.c   |  50 ++--
 drivers/vfio/pci/vfio_pci_dmabuf.c | 359 +
 drivers/vfio/pci/vfio_pci_priv.h   |  23 ++
 drivers/vfio/vfio_main.c   |   2 +
 include/linux/vfio.h   |   2 +
 include/linux/vfio_pci_core.h  |   1 +
 include/uapi/linux/vfio.h  |  25 ++
 9 files changed, 463 insertions(+), 22 deletions(-)
 create mode 100644 drivers/vfio/pci/vfio_pci_dmabuf.c

-- 
2.48.1

[PATCH v3 1/3] vfio: Export vfio device get and put registration helpers

These helpers are useful for managing additional references taken
on the device from other associated VFIO modules.

Original-patch-by: Jason Gunthorpe 
Signed-off-by: Vivek Kasireddy 
---
 drivers/vfio/vfio_main.c | 2 ++
 include/linux/vfio.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 1fd261efc582..620a3ee5d04d 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -171,11 +171,13 @@ void vfio_device_put_registration(struct vfio_device 
*device)
if (refcount_dec_and_test(&device->refcount))
complete(&device->comp);
 }
+EXPORT_SYMBOL_GPL(vfio_device_put_registration);
 
 bool vfio_device_try_get_registration(struct vfio_device *device)
 {
return refcount_inc_not_zero(&device->refcount);
 }
+EXPORT_SYMBOL_GPL(vfio_device_try_get_registration);
 
 /*
  * VFIO driver API
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 000a6cab2d31..2258b0585330 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -279,6 +279,8 @@ static inline void vfio_put_device(struct vfio_device 
*device)
 int vfio_register_group_dev(struct vfio_device *device);
 int vfio_register_emulated_iommu_dev(struct vfio_device *device);
 void vfio_unregister_group_dev(struct vfio_device *device);
+bool vfio_device_try_get_registration(struct vfio_device *device);
+void vfio_device_put_registration(struct vfio_device *device);
 
 int vfio_assign_device_set(struct vfio_device *device, void *set_id);
 unsigned int vfio_device_set_open_count(struct vfio_device_set *dev_set);
-- 
2.48.1

[PATCH v3 2/3] vfio/pci: Share the core device pointer while invoking feature functions

There is no need to share the main device pointer (struct vfio_device *)
with all the feature functions as they only need the core device
pointer. Therefore, extract the core device pointer once in the
caller (vfio_pci_core_ioctl_feature) and share it instead.

Signed-off-by: Vivek Kasireddy 
---
 drivers/vfio/pci/vfio_pci_core.c | 30 +-
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 586e49efb81b..9070dc5cc529 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -300,11 +300,9 @@ static int vfio_pci_runtime_pm_entry(struct 
vfio_pci_core_device *vdev,
return 0;
 }
 
-static int vfio_pci_core_pm_entry(struct vfio_device *device, u32 flags,
+static int vfio_pci_core_pm_entry(struct vfio_pci_core_device *vdev, u32 flags,
  void __user *arg, size_t argsz)
 {
-   struct vfio_pci_core_device *vdev =
-   container_of(device, struct vfio_pci_core_device, vdev);
int ret;
 
ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET, 0);
@@ -321,12 +319,10 @@ static int vfio_pci_core_pm_entry(struct vfio_device 
*device, u32 flags,
 }
 
 static int vfio_pci_core_pm_entry_with_wakeup(
-   struct vfio_device *device, u32 flags,
+   struct vfio_pci_core_device *vdev, u32 flags,
struct vfio_device_low_power_entry_with_wakeup __user *arg,
size_t argsz)
 {
-   struct vfio_pci_core_device *vdev =
-   container_of(device, struct vfio_pci_core_device, vdev);
struct vfio_device_low_power_entry_with_wakeup entry;
struct eventfd_ctx *efdctx;
int ret;
@@ -377,11 +373,9 @@ static void vfio_pci_runtime_pm_exit(struct 
vfio_pci_core_device *vdev)
up_write(&vdev->memory_lock);
 }
 
-static int vfio_pci_core_pm_exit(struct vfio_device *device, u32 flags,
+static int vfio_pci_core_pm_exit(struct vfio_pci_core_device *vdev, u32 flags,
 void __user *arg, size_t argsz)
 {
-   struct vfio_pci_core_device *vdev =
-   container_of(device, struct vfio_pci_core_device, vdev);
int ret;
 
ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET, 0);
@@ -1482,11 +1476,10 @@ long vfio_pci_core_ioctl(struct vfio_device *core_vdev, 
unsigned int cmd,
 }
 EXPORT_SYMBOL_GPL(vfio_pci_core_ioctl);
 
-static int vfio_pci_core_feature_token(struct vfio_device *device, u32 flags,
-  uuid_t __user *arg, size_t argsz)
+static int vfio_pci_core_feature_token(struct vfio_pci_core_device *vdev,
+  u32 flags, uuid_t __user *arg,
+  size_t argsz)
 {
-   struct vfio_pci_core_device *vdev =
-   container_of(device, struct vfio_pci_core_device, vdev);
uuid_t uuid;
int ret;
 
@@ -1513,16 +1506,19 @@ static int vfio_pci_core_feature_token(struct 
vfio_device *device, u32 flags,
 int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags,
void __user *arg, size_t argsz)
 {
+   struct vfio_pci_core_device *vdev =
+   container_of(device, struct vfio_pci_core_device, vdev);
+
switch (flags & VFIO_DEVICE_FEATURE_MASK) {
case VFIO_DEVICE_FEATURE_LOW_POWER_ENTRY:
-   return vfio_pci_core_pm_entry(device, flags, arg, argsz);
+   return vfio_pci_core_pm_entry(vdev, flags, arg, argsz);
case VFIO_DEVICE_FEATURE_LOW_POWER_ENTRY_WITH_WAKEUP:
-   return vfio_pci_core_pm_entry_with_wakeup(device, flags,
+   return vfio_pci_core_pm_entry_with_wakeup(vdev, flags,
  arg, argsz);
case VFIO_DEVICE_FEATURE_LOW_POWER_EXIT:
-   return vfio_pci_core_pm_exit(device, flags, arg, argsz);
+   return vfio_pci_core_pm_exit(vdev, flags, arg, argsz);
case VFIO_DEVICE_FEATURE_PCI_VF_TOKEN:
-   return vfio_pci_core_feature_token(device, flags, arg, argsz);
+   return vfio_pci_core_feature_token(vdev, flags, arg, argsz);
default:
return -ENOTTY;
}
-- 
2.48.1

Re: [PATCH] drm/imx: legacy-bridge: fix inconsistent indenting warning

On 03/05/2025, Charles Han wrote:
> Fix below inconsistent indenting smatch warning.
> smatch warnings:
> drivers/gpu/drm/bridge/imx/imx-legacy-bridge.c:79 
> devm_imx_drm_legacy_bridge() warn: inconsistent indenting
> 
> Signed-off-by: Charles Han 
> ---
>  drivers/gpu/drm/bridge/imx/imx-legacy-bridge.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Applied to misc/kernel.git (drm-misc-next).
Thanks!

-- 
Regards,
Liu Ying

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

2025-03-06 Thread H. Peter Anvin

On March 6, 2025 8:25:25 AM PST, Kuan-Wei Chiu  wrote:
>Several parts of the kernel contain redundant implementations of parity
>calculations for 16/32/64-bit values. Introduces generic
>parity16/32/64() helpers in bitops.h, providing a standardized
>and optimized implementation. 
>
>Subsequent patches refactor various kernel components to replace
>open-coded parity calculations with the new helpers, reducing code
>duplication and improving maintainability.
>
>Co-developed-by: Yu-Chun Lin 
>Signed-off-by: Yu-Chun Lin 
>Signed-off-by: Kuan-Wei Chiu 
>---
>In v3, I use parityXX() instead of the parity() macro since the
>parity() macro may generate suboptimal code and requires special hacks
>to make GCC happy. If anyone still prefers a single parity() macro,
>please let me know.
>
>Additionally, I changed parityXX() << y users to !!parityXX() << y
>because, unlike C++, C does not guarantee that true casts to int as 1.
>
>Changes in v3:
>- Avoid using __builtin_parity.
>- Change return type to bool.
>- Drop parity() macro.
>- Change parityXX() << y to !!parityXX() << y.
>
>
>Changes in v2:
>- Provide fallback functions for __builtin_parity() when the compiler
>  decides not to inline it
>- Use __builtin_parity() when no architecture-specific implementation
>  is available
>- Optimize for constant folding when val is a compile-time constant
>- Add a generic parity() macro
>- Drop the x86 bootflag conversion patch since it has been merged into
>  the tip tree
>
>v1: https://lore.kernel.org/lkml/20250223164217.2139331-1-visitor...@gmail.com/
>v2: https://lore.kernel.org/lkml/20250301142409.2513835-1-visitor...@gmail.com/
>
>Kuan-Wei Chiu (16):
>  bitops: Change parity8() return type to bool
>  bitops: Add parity16(), parity32(), and parity64() helpers
>  media: media/test_drivers: Replace open-coded parity calculation with
>parity8()
>  media: pci: cx18-av-vbi: Replace open-coded parity calculation with
>parity8()
>  media: saa7115: Replace open-coded parity calculation with parity8()
>  serial: max3100: Replace open-coded parity calculation with parity8()
>  lib/bch: Replace open-coded parity calculation with parity32()
>  Input: joystick - Replace open-coded parity calculation with
>parity32()
>  net: ethernet: oa_tc6: Replace open-coded parity calculation with
>parity32()
>  wifi: brcm80211: Replace open-coded parity calculation with parity32()
>  drm/bridge: dw-hdmi: Replace open-coded parity calculation with
>parity32()
>  mtd: ssfdc: Replace open-coded parity calculation with parity32()
>  fsi: i2cr: Replace open-coded parity calculation with parity32()
>  fsi: i2cr: Replace open-coded parity calculation with parity64()
>  Input: joystick - Replace open-coded parity calculation with
>parity64()
>  nfp: bpf: Replace open-coded parity calculation with parity64()
>
> drivers/fsi/fsi-master-i2cr.c | 18 ++-
> .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  8 +--
> drivers/input/joystick/grip_mp.c  | 17 +-
> drivers/input/joystick/sidewinder.c   | 24 ++---
> drivers/media/i2c/saa7115.c   | 12 +
> drivers/media/pci/cx18/cx18-av-vbi.c  | 12 +
> .../media/test-drivers/vivid/vivid-vbi-gen.c  |  8 +--
> drivers/mtd/ssfdc.c   | 20 ++-
> drivers/net/ethernet/netronome/nfp/nfp_asm.c  |  7 +--
> drivers/net/ethernet/oa_tc6.c | 19 ++-
> .../broadcom/brcm80211/brcmsmac/dma.c | 16 +-
> drivers/tty/serial/max3100.c  |  3 +-
> include/linux/bitops.h| 52 +--
> lib/bch.c | 14 +
> 14 files changed, 77 insertions(+), 153 deletions(-)
>

!!x is used with a value that is not necessary booleanized already, and is 
exactly equivalent to (x ? true : false). It is totally redundant on a value 
known to be bool.

If (int)true wasn't inherently 1, then !! wouldn't work either. 

There was a time when some code would use as a temporary hack: 

typedef enum { false, true } bool;

... when compiling on pre-C99 compilers; in that case a (bool) case wouldn't 
necessarily work as expected, whereas !! would. Furthermore, unlike (bool), !! 
works in the preprocessor.

Re: [git pull] drm fixes for 6.14-rc6

2025-03-06 Thread pr-tracker-bot

The pull request you sent on Fri, 7 Mar 2025 10:13:09 +1000:

> https://gitlab.freedesktop.org/drm/kernel.git tags/drm-fixes-2025-03-07

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/28f587adb69957125241a8df359b68b134f3c4a1

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [PATCH v5 2/7] bits: introduce fixed-type genmasks

2025-03-06 Thread Vincent Mailhol

On 06/03/2025 à 22:08, Andy Shevchenko wrote:
> On Thu, Mar 06, 2025 at 08:29:53PM +0900, Vincent Mailhol via B4 Relay wrote:
>> From: Yury Norov 
>>
>> Add GENMASK_TYPE() which generalizes __GENMASK() to support different
>> types, and implement fixed-types versions of GENMASK() based on it.
>> The fixed-type version allows more strict checks to the min/max values
>> accepted, which is useful for defining registers like implemented by
>> i915 and xe drivers with their REG_GENMASK*() macros.
>>
>> The strict checks rely on shift-count-overflow compiler check to fail
>> the build if a number outside of the range allowed is passed.
>> Example:
>>
>>   #define FOO_MASK GENMASK_U32(33, 4)
>>
>> will generate a warning like:
>>
>>   include/linux/bits.h:51:27: error: right shift count >= width of type 
>> [-Werror=shift-count-overflow]
>>  51 |   type_max(t) >> (BITS_PER_TYPE(t) - 1 - (h)
>> |   ^~
> 
> Code LGTM

Does this mean I get your Reviewed-by tag? Or will you wait the v6 to
formally give it?

> but just to be sure: you prepared your series using histogram
> diff algo, right?

No, I never used the histogram diff. My git config is extremely boring.
Mostly vanilla.

I remember that Linus even commented on this:

https://lore.kernel.org/all/CAHk-=wiuxm-nz1si8dxwvttj9n3c+1srtc0v+lk7hoe4bdv...@mail.gmail.com/

But he made it clear this was *not* a requirement, so I just left the
diff algorithm to the default. Or did I miss any communication that
contributors should now use histogram diff?

Regardless, I do not mind activating it. I just did a:

  git config diff.algorithm histogram

The v6 will have histogram diffs.


Yours sincerely,
Vincent Mailhol

Re: [PATCH 3/5] dt-bindings: display: simple-bridge: Document DPI color encoder

On 03/07/2025, Rob Herring wrote:
> On Thu, Mar 06, 2025 at 12:35:49PM +0100, Maxime Ripard wrote:
>> On Thu, Mar 06, 2025 at 03:02:41PM +0800, Liu Ying wrote:
>>> On 03/06/2025, Rob Herring wrote:
 On Wed, Mar 05, 2025 at 10:35:26AM +0100, Alexander Stein wrote:
> Hi,
>
> Am Dienstag, 4. März 2025, 16:23:20 CET schrieb Rob Herring:
>> On Tue, Mar 04, 2025 at 06:15:28PM +0800, Liu Ying wrote:
>>> A DPI color encoder, as a simple display bridge, converts input DPI 
>>> color
>>> coding to output DPI color coding, like Adafruit Kippah DPI hat[1] which
>>> converts input 18-bit pixel data to 24-bit pixel data(with 2 low padding
>>> bits in every color component though). Document the DPI color encoder.
>>
>> Why do we need a node for this? Isn't this just wired how it is wired 
>> and there's nothing for s/w to see or do? I suppose if you are trying to 
>> resolve the mode with 24-bit on one end and 18-bit on the other end, you 
>> need to allow that and not require an exact match. You still might need 
>> to figure out which pins the 18-bit data comes out on, but you have that 
>> problem with an 18-bit panel too. IOW, how is this any different if you 
>> have an 18-bit panel versus 24-bit panel?
>
> Especially panel-simple.c has a fixed configuration for each display, 
> such as:
>> .bus_format = MEDIA_BUS_FMT_RGB666_1X18
>
> How would you allow or even know it should be addressed as
> MEDIA_BUS_FMT_RGB888_1X24 instead? I see different ways:
> 1. Create a new display setting/compatible
> 2. Add an overwrite property to the displays
> 3. Use a (transparent) bridge (this series)
>
> Number 1 is IMHO out of question. 

 Agreed.

> I personally don't like number 2 as this
> feels like adding quirks to displays, which they don't have.

 This is what I would do except apply it to the controller side. We know 
 the panel side already. This is a board variation, so a property makes 
 sense. I don't think you need any more than knowing what's on each end. 
>>>
>>> With option 2, no matter putting a property in source side or sink side,
>>> impacted display drivers and DT bindings need to be changed, once a board
>>> manipulates the DPI color coding.  This adds burdens and introduces new
>>> versions of those DT bindings.  Is this what we want?
>>
>> There's an option 4: make it a property of the OF graph endpoints. In
>> essence, it's similar to properties that are already there like
>> lane-mapping, and it wouldn't affect the panel drivers, or create an
>> intermediate bridge.
> 
> Yes, that's actually where I meant to put the property(ies).

Put optional dpi-color-coding or something else in endpoint-base?
Assuming it's optional, then it implies that it will overwrite OS's setting,
which sounds kinda awkward, because it is supposed to be required to describe
the actual color coding. If it's required, then we would have to create
something like dpi-endpoint-base, but existing display device DT nodes are
based on endpoint-base.

> 
> Rob

-- 
Regards,
Liu Ying

Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: add h/vsync-disable support

On Thu, Mar 06, 2025 at 10:11:33AM +0100, A. Zini wrote:
> From: Alessandro Zini 
> 
> The h/vsync-disable properties are used to control whether to use or
> not h/vsync signals, by configuring their pulse width to zero.
> 
> This is required on some panels which are driven in DE-only mode but do
> not ignore sync packets, and instead require them to be low-voltage level
> or ground.

If this is required by 'some panels', then it should be a property of
the panel, not by the bridge itself. Can the panel return the mode with
hsync_end = hsync_start and vsync_enc = vsync_start?

> 
> Signed-off-by: Alessandro Zini 
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c 
> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 95563aa1b450d..c94ea92159402 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -164,6 +164,8 @@ struct sn65dsi83 {
>   int irq;
>   struct delayed_work monitor_work;
>   struct work_struct  reset_work;
> + boolhsync_disable;
> + boolvsync_disable;
>  };
>  
>  static const struct regmap_range sn65dsi83_readable_ranges[] = {
> @@ -604,10 +606,12 @@ static void sn65dsi83_atomic_pre_enable(struct 
> drm_bridge *bridge,
>   /* 32 + 1 pixel clock to ensure proper operation */
>   le16val = cpu_to_le16(32 + 1);
>   regmap_bulk_write(ctx->regmap, REG_VID_CHA_SYNC_DELAY_LOW, &le16val, 2);
> - le16val = cpu_to_le16(mode->hsync_end - mode->hsync_start);
> + le16val = cpu_to_le16(ctx->hsync_disable ?
> + 0 : mode->hsync_end - mode->hsync_start);
>   regmap_bulk_write(ctx->regmap, REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
> &le16val, 2);
> - le16val = cpu_to_le16(mode->vsync_end - mode->vsync_start);
> + le16val = cpu_to_le16(ctx->vsync_disable ?
> + 0 : mode->vsync_end - mode->vsync_start);
>   regmap_bulk_write(ctx->regmap, REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
> &le16val, 2);
>   regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_BACK_PORCH,
> @@ -867,6 +871,14 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, 
> enum sn65dsi83_model model)
>   }
>   }
>  
> + ctx->hsync_disable = false;
> + if (of_property_present(dev->of_node, "hsync-disable"))
> + ctx->hsync_disable = true;
> +
> + ctx->vsync_disable = false;
> + if (of_property_present(dev->of_node, "vsync-disable"))
> + ctx->vsync_disable = true;
> +
>   panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 2, 0);
>   if (IS_ERR(panel_bridge))
>   return dev_err_probe(dev, PTR_ERR(panel_bridge), "Failed to get 
> panel bridge\n");
> -- 
> 2.48.1
> 

-- 
With best wishes
Dmitry

[PATCH 09/11] arm64: dts: qcom: x1e80100-dell-xps13-9345: Drop clock-names from PS8830

2025-03-06 Thread Konrad Dybcio

From: Konrad Dybcio 

The preemptively-merged node contains a property absent from the final
bindings. Remove it.

Signed-off-by: Konrad Dybcio 
---
 arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts 
b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
index 
124051334be072fce1351d12211eb61e154b3785..5d807fb34aee2dabf16fe32664ee05ea76532675
 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
@@ -612,7 +612,6 @@ typec-mux@8 {
reg = <0x08>;
 
clocks = <&rpmhcc RPMH_RF_CLK3>;
-   clock-names = "xo";
 
vdd-supply = <&vreg_rtmr0_1p15>;
vdd33-supply = <&vreg_rtmr0_3p3>;
@@ -676,7 +675,6 @@ typec-mux@8 {
reg = <0x8>;
 
clocks = <&rpmhcc RPMH_RF_CLK4>;
-   clock-names = "xo";
 
vdd-supply = <&vreg_rtmr1_1p15>;
vdd33-supply = <&vreg_rtmr1_3p3>;

-- 
2.48.1

Re: [PATCH drm-next 2/2] drm/virtio: Use atomic_vmap to work drm_panic in GUI

2025-03-06 Thread kernel test robot

Hi Ryosuke,

kernel test robot noticed the following build errors:

[auto build test ERROR on e21cba704714c301d04c5fd37a693734b623872a]

url:
https://github.com/intel-lab-lkp/linux/commits/Ryosuke-Yasuoka/vmalloc-Add-atomic_vmap/20250305-232918
base:   e21cba704714c301d04c5fd37a693734b623872a
patch link:
https://lore.kernel.org/r/20250305152555.318159-3-ryasuoka%40redhat.com
patch subject: [PATCH drm-next 2/2] drm/virtio: Use atomic_vmap to work 
drm_panic in GUI
config: i386-buildonly-randconfig-003-20250306 
(https://download.01.org/0day-ci/archive/20250307/202503071022.q1pg7suf-...@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project 
cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20250307/202503071022.q1pg7suf-...@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/202503071022.q1pg7suf-...@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in 
drivers/fpga/tests/fpga-mgr-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in 
drivers/fpga/tests/fpga-bridge-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in 
drivers/fpga/tests/fpga-region-test.o
ERROR: modpost: "drm_gem_atomic_get_pages" 
[drivers/gpu/drm/drm_shmem_helper.ko] undefined!
ERROR: modpost: "atomic_vmap" [drivers/gpu/drm/drm_shmem_helper.ko] undefined!
>> ERROR: modpost: "drm_gem_shmem_atomic_vmap" 
>> [drivers/gpu/drm/virtio/virtio-gpu.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] drm/panel: novatek-nt36523: transition to mipi_dsi wrapped functions

On Thu, Mar 06, 2025 at 11:08:18PM +0530, Tejas Vipin wrote:
> 
> 
> On 3/6/25 10:58 PM, Doug Anderson wrote:
> > Hi,
> > 
> > On Thu, Mar 6, 2025 at 6:05 AM  wrote:
> >>
> >> On 06/03/2025 14:43, Tejas Vipin wrote:
> >>> Changes the novatek-nt36523 panel to use multi style functions for
> >>> improved error handling.
> >>>
> >>> Signed-off-by: Tejas Vipin 
> >>> ---
> >>>   drivers/gpu/drm/panel/panel-novatek-nt36523.c | 1683 -
> >>>   1 file changed, 823 insertions(+), 860 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36523.c 
> >>> b/drivers/gpu/drm/panel/panel-novatek-nt36523.c
> >>> index 04f1d2676c78..922a225f6258 100644
> >>> --- a/drivers/gpu/drm/panel/panel-novatek-nt36523.c
> >>> +++ b/drivers/gpu/drm/panel/panel-novatek-nt36523.c
> >>> @@ -23,10 +23,12 @@
> >>>
> >>>   #define DSI_NUM_MIN 1
> >>>
> >>> -#define mipi_dsi_dual_dcs_write_seq(dsi0, dsi1, cmd, seq...)\
> >>> - do { \
> >>> - mipi_dsi_dcs_write_seq(dsi0, cmd, seq);  \
> >>> - mipi_dsi_dcs_write_seq(dsi1, cmd, seq);  \
> >>> +#define mipi_dsi_dual_dcs_write_seq_multi(dsi_ctx0, dsi_ctx1, cmd, 
> >>> seq...)  \
> >>> + do {
> >>> \
> >>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx0, cmd, seq);  
> >>> \
> >>> + dsi_ctx1.accum_err = dsi_ctx0.accum_err;
> >>> \
> >>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx1, cmd, seq);  
> >>> \
> >>> + dsi_ctx0.accum_err = dsi_ctx1.accum_err;
> >>> \
> >>
> >> Just thinking out loud, but can't we do :
> >>
> >> struct mipi_dsi_multi_context dsi_ctx = { .dsi = NULL };
> >>
> >> #define mipi_dsi_dual_dcs_write_seq_multi(dsi_ctx, dsi0, dsi1, cmd, 
> >> seq...)  \
> >> do {
> >> dsi_ctx.dsi = dsi0;
> >>  \
> >> mipi_dsi_dcs_write_seq_multi(&dsi_ctx, cmd, seq);  
> >>  \
> >> dsi_ctx.dsi = dsi1;
> >>  \
> >> mipi_dsi_dcs_write_seq_multi(&dsi_ctx, cmd, seq);  
> >>  \
> >>
> >> ?
> >>
> >> So we have a single accum_err.
> > 
> > Even though the code you used was what I suggested in IRC, I like
> > Neil's suggestion better here. What do you think?
> 
> I like Dmitry's suggestion [1]. If we went ahead with this we'd also
> only need to equate the accum_err for the few msleep calls.

I think we will have more and more double-DSI panels. So I'd suggest
to add msleep wrapper which gets two DSI contexts.

> Since it
> does change the behavior, I'd like to hear another opinion on it before
> I go ahead with it.
> 
> [1]: 
> https://lore.kernel.org/all/p2esqngynwfrshz5rqfnmx6qgwf4dclpkb3mphwg2vavx2jbcg@clqoy5tjh7bb/
> 
> > 
> > Other than that, it looks good to me.
> > 
> > -Doug
> 
> -- 
> Tejas Vipin

-- 
With best wishes
Dmitry

[PATCH] drm/gma500: Add NULL check for pci_gfx_root in mid_get_vbt_data()

2025-03-06 Thread Ivan Abramov

Since pci_get_domain_bus_and_slot() can return NULL, add NULL check for
pci_gfx_root in the mid_get_vbt_data().

This change is similar to the checks implemented in mid_get_fuse_settings()
and mid_get_pci_revID(), which were introduced by commit 0cecdd818cd7
("gma500: Final enables for Oaktrail") as "additional minor
bulletproofing".

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: ba99d8348864 ("drm/gma500: Deprecate pci_get_bus_and_slot()")

Signed-off-by: Ivan Abramov 
---
 drivers/gpu/drm/gma500/mid_bios.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/gma500/mid_bios.c 
b/drivers/gpu/drm/gma500/mid_bios.c
index 7e76790c6a81..cba97d7db131 100644
--- a/drivers/gpu/drm/gma500/mid_bios.c
+++ b/drivers/gpu/drm/gma500/mid_bios.c
@@ -279,6 +279,11 @@ static void mid_get_vbt_data(struct drm_psb_private 
*dev_priv)
0, PCI_DEVFN(2, 0));
int ret = -1;
 
+   if (pci_gfx_root == NULL) {
+   WARN_ON(1);
+   return;
+   }
+
/* Get the address of the platform config vbt */
pci_read_config_dword(pci_gfx_root, 0xFC, &addr);
pci_dev_put(pci_gfx_root);
-- 
2.48.1

Re: [PATCH v5 10/16] drm/bridge: ti-sn65dsi83: Switch to drm_bridge_helper_reset_crtc

2025-03-06 Thread Herve Codina

Hi Maxime,

On Tue, 04 Mar 2025 12:10:53 +0100
Maxime Ripard  wrote:

> Now that we have a helper for bridge drivers to call to reset the output
> pipeline, let's use it.
> 
> Reviewed-by: Dmitry Baryshkov 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 28 +++-
>  1 file changed, 11 insertions(+), 17 deletions(-)
> 

Reviewed-by: Herve Codina 

Also tested on my system with faults manually generated and no regressions
were observed in the driver recovery process. Works fine.

Tested-by: Herve Codina 

Best regards,
Hervé

Re: [PATCH] drm/gma500: Remove unused mrst_helper_funcs

On Sat, Feb 1, 2025 at 2:14 AM  wrote:
>
> From: "Dr. David Alan Gilbert" 
>
> The mrst_helper_funcs const was added in 2013 by
> commit ac6113ebb70d ("drm/gma500/mrst: Add SDVO clock calculation")
> and commented as 'Not used yet'.
>
> It's not been used since, so remove it.

You talk about mrst_helper_funcs but the patch removes
mrst_clock_funcs. I assume this is not intentional.

-Patrik

>
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  drivers/gpu/drm/gma500/oaktrail_crtc.c | 7 ---
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c 
> b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> index de8ccfe9890f..ea9b41af0867 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> @@ -658,10 +658,3 @@ const struct drm_crtc_helper_funcs oaktrail_helper_funcs 
> = {
> .prepare = gma_crtc_prepare,
> .commit = gma_crtc_commit,
>  };
> -
> -/* Not used yet */
> -const struct gma_clock_funcs mrst_clock_funcs = {
> -   .clock = mrst_lvds_clock,
> -   .limit = mrst_limit,
> -   .pll_is_valid = gma_pll_is_valid,
> -};
> --
> 2.48.1
>

Re: [PATCH v4 3/8] bits: introduce fixed-type genmasks

On Thu, Mar 06, 2025 at 06:22:33PM +0900, Vincent Mailhol wrote:
> On 06/03/2025 at 04:45, Andy Shevchenko wrote:
> >>> But GENMASK_U128() becomes a special case now.
> >>> The 128-bit GENMASK is unsued, but it's exported in uapi. Is there any
> >>> simple way to end up with a common implementation for all fixed-type
> >>> GENMASKs?
> >>
> >> What bothers me is that the 128 bit types are not something available on
> >> all architectures, c.f. the CONFIG_ARCH_SUPPORTS_INT128. So, I would
> >> need a U128() equivalent to the ULL() but which does not break on
> >> architectures which do not support 128 bits integers.
> >>
> >> This is where I am stuck. If someone can guide me on how to write a
> >> robust U128() macro, then I think the common implementation could be
> >> feasible.
> > 
> > I think we may leave that U128 stuff alone for now.
> 
> I found the solution! The trick is to use type_max() from overflow.h.
> 
> With this, GENMASK_TYPE() becomes:
> 
>   #define GENMASK_TYPE(t, h, l)   \
>   ((t)(GENMASK_INPUT_CHECK(h, l) +\
>(type_max(t) << (l) &  \
> type_max(t) >> (BITS_PER_TYPE(t) - 1 - (h)
> 
> and works with all the GENMASK variants, including the U128 one! The
> unit tests under lib/test_bits.c are all green.
> 
> Of course, this does *not* work in assembly. But as explained before,
> GENMASK_TYPE() is guarded by a #if !defined(__ASSEMBLY__), so all good!
> 
> The question raised by Yury on whether or not we should keep
> __GENMASK_U128() in the uapi still remains. And in full honesty, I will
> not touch that one. This is not in the scope of this series.

I vote for not touching it right now independently on its destiny.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] mailmap: remap all addresses to kernel.org alias

2025-03-06 Thread neil . armstrong


On 05/03/2025 23:44, Dmitry Baryshkov wrote:

Remap all historical and non-historical entries to my kernel.org email.

Signed-off-by: Dmitry Baryshkov 
---
  .mailmap | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/.mailmap b/.mailmap
index 
01145c078838bf9348e8d0e5e48b7b0954248dc5..a5c80ef0b7800519f3124e0d85294f34d6b275ca
 100644
--- a/.mailmap
+++ b/.mailmap
@@ -198,10 +198,11 @@ Dengcheng Zhu  

  Dengcheng Zhu  
   
  Dikshita Agarwal  
-Dmitry Baryshkov 
-Dmitry Baryshkov  <[dbarysh...@gmail.com]>
-Dmitry Baryshkov  
-Dmitry Baryshkov  
+Dmitry Baryshkov  
+Dmitry Baryshkov  <[dbarysh...@gmail.com]>
+Dmitry Baryshkov  
+Dmitry Baryshkov  
+Dmitry Baryshkov  
  Dmitry Safonov <0x7f454...@gmail.com> 
  Dmitry Safonov <0x7f454...@gmail.com> 
  Dmitry Safonov <0x7f454...@gmail.com> 



Reviewed-by: Neil Armstrong

Re: [PATCH v4 4/8] bits: introduce fixed-type BIT

2025-03-06 Thread Jani Nikula

On Wed, 05 Mar 2025, David Laight  wrote:
> So it is even questionable whether BIT8() and BIT16() should even
> exist at all.

If nothing else, they do provide compile time checks for the bit being
0..7 and 0..15, respectively.

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH v5 04/16] drm/atomic: Introduce helper to lookup connector by encoder

2025-03-06 Thread Herve Codina

Hi Maxime,
Hi Maxime,

On Tue, 04 Mar 2025 12:10:47 +0100
Maxime Ripard  wrote:

> With the bridges switching over to drm_bridge_connector, the direct
> association between a bridge driver and its connector was lost.
> 
> This is mitigated for atomic bridge drivers by the fact you can access
> the encoder, and then call drm_atomic_get_old_connector_for_encoder() or
> drm_atomic_get_new_connector_for_encoder() with drm_atomic_state.
> 
> This was also made easier by providing drm_atomic_state directly to all
> atomic hooks bridges can implement.
> 
> However, bridge drivers don't have a way to access drm_atomic_state
> outside of the modeset path, like from the hotplug interrupt path or any
> interrupt handler.
> 
> Let's introduce a function to retrieve the connector currently assigned
> to an encoder, without using drm_atomic_state, to make these drivers'
> life easier.
> 
> Reviewed-by: Dmitry Baryshkov 
> Co-developed-by: Simona Vetter 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_atomic.c | 45 
> 
>  include/drm/drm_atomic.h |  3 +++
>  2 files changed, 48 insertions(+)
> 

Tested the drm_atomic_get_connector_for_encoder() in the context of the
ti-sn65dsi83 driver recovery process (later modification in this series).

Tested-by: Herve Codina 

Best regards,
Hervé

[PATCH 1/2] drm/msm/dpu: correct dpu_crtc_check_mode_changed docs

Correct commit 20972609d12c ("drm/msm/dpu: Require modeset if clone mode
status changes") and describe old_crtc_state and new_crtc_state params
instead of the single previously used parameter crtc_state.

Fixes: 20972609d12c ("drm/msm/dpu: Require modeset if clone mode status 
changes")
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 
b0a062d6fa3bf942ffd687a91bccac3aa4f06f02..536d15818ba24f8b11f24e3bd9726d31c57ef531
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1395,7 +1395,8 @@ static int dpu_crtc_assign_resources(struct drm_crtc 
*crtc,
 
 /**
  * dpu_crtc_check_mode_changed: check if full modeset is required
- * @crtc_state:Corresponding CRTC state to be checked
+ * @old_crtc_state:Previous CRTC state
+ * @new_crtc_state:Corresponding CRTC state to be checked
  *
  * Check if the changes in the object properties demand full mode set.
  */

-- 
2.39.5

[PATCH 2/2] drm/msm/dpu: correct struct dpu_encoder_virt docs

Fix a typo in struct dpu_encoder_virt kerneldoc, which made it ignore
description of the cwb_mask field.

Fixes: dd331404ac7c ("drm/msm/dpu: Configure CWB in writeback encoder")
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 
0eed93a4d056beda6b54c0d20f027a53c84f67db..0637be07eb293041a350161b39a6276eb44bfb42
 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -140,7 +140,7 @@ enum dpu_enc_rc_states {
  * num_phys_encs.
  * @hw_dsc:Handle to the DSC blocks used for the display.
  * @dsc_mask:  Bitmask of used DSC blocks.
- * @cwb_mask   Bitmask of used CWB muxes
+ * @cwb_mask:  Bitmask of used CWB muxes
  * @intfs_swapped: Whether or not the phys_enc interfaces have been swapped
  * for partial update right-only cases, such as pingpong
  * split where virtual pingpong does not generate IRQs

-- 
2.39.5

[PATCH 0/2] drm/msm/dpu: two fixes for kerneldocs

Signed-off-by: Dmitry Baryshkov 
---
Dmitry Baryshkov (2):
  drm/msm/dpu: correct dpu_crtc_check_mode_changed docs
  drm/msm/dpu: correct struct dpu_encoder_virt docs

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 3 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)
---
base-commit: 6d3175a72cc07e90f81fb35841048a8a9b5134cb
change-id: 20250306-dpu-fix-docs-3700642b33ea

Best regards,
-- 
Dmitry Baryshkov

Re: RESEND Re: [PATCH v16 2/7] drm/ttm/pool, drm/ttm/tt: Provide a helper to shrink pages

2025-03-06 Thread Thomas Hellström

On Wed, 2025-03-05 at 13:01 +1000, Dave Airlie wrote:
> I've looked over the two patches mentioned here, I think they have
> seen enough time and we need to unblock,
> 
> Please add and merge them:
> Acked-by: Dave Airlie 
> 
> Dave.

Thanks for unblocking, Dave. Pushed to drm-misc-next yesterday.

/Thomas




> 
> On Tue, 25 Feb 2025 at 18:44, Thomas Hellström
>  wrote:
> > 
> > Hi, Christian,
> > 
> > Ping? I'd really want to get this in before -rc6
> > 
> > Thanks,
> > Thomas
> > 
> > 
> > 
> > On Tue, 2025-02-18 at 16:40 +0100, Thomas Hellström wrote:
> > > Hi, Christian,
> > > 
> > > On Wed, 2025-02-05 at 15:02 +0100, Christian König wrote:
> > > > Am 30.01.25 um 11:13 schrieb Thomas Hellström:
> > > > > Provide a helper to shrink ttm_tt page-vectors on a per-page
> > > > > basis. A ttm_backup backend could then in theory get away
> > > > > with
> > > > > allocating a single temporary page for each struct ttm_tt.
> > > > > 
> > > > > This is accomplished by splitting larger pages before trying
> > > > > to
> > > > > back them up.
> > > > > 
> > > > > In the future we could allow ttm_backup to handle backing up
> > > > > large pages as well, but currently there's no benefit in
> > > > > doing that, since the shmem backup backend would have to
> > > > > split those anyway to avoid allocating too much temporary
> > > > > memory, and if the backend instead inserts pages into the
> > > > > swap-cache, those are split on reclaim by the core.
> > > > > 
> > > > > Due to potential backup- and recover errors, allow partially
> > > > > swapped
> > > > > out struct ttm_tt's, although mark them as swapped out
> > > > > stopping
> > > > > them
> > > > > from being swapped out a second time. More details in the
> > > > > ttm_pool.c
> > > > > DOC section.
> > > > > 
> > > > > v2:
> > > > > - A couple of cleanups and error fixes in
> > > > > ttm_pool_back_up_tt.
> > > > > - s/back_up/backup/
> > > > > - Add a writeback parameter to the exported interface.
> > > > > v8:
> > > > > - Use a struct for flags for readability (Matt Brost)
> > > > > - Address misc other review comments (Matt Brost)
> > > > > v9:
> > > > > - Update the kerneldoc for the ttm_tt::backup field.
> > > > > v10:
> > > > > - Rebase.
> > > > > v13:
> > > > > - Rebase on ttm_backup interface change. Update kerneldoc.
> > > > > - Rebase and adjust ttm_tt_is_swapped().
> > > > > v15:
> > > > > - Rebase on ttm_backup return value change.
> > > > > - Rebase on previous restructuring of ttm_pool_alloc()
> > > > > - Rework the ttm_pool backup interface (Christian König)
> > > > > - Remove cond_resched() (Christian König)
> > > > > - Get rid of the need to allocate an intermediate page array
> > > > >    when restoring a multi-order page (Christian König)
> > > > > - Update documentation.
> > > > > 
> > > > > Cc: Christian König 
> > > > > Cc: Somalapuram Amaranath 
> > > > > Cc: Matthew Brost 
> > > > > Cc: 
> > > > > Signed-off-by: Thomas Hellström
> > > > > 
> > > > > Reviewed-by: Matthew Brost 
> > > > 
> > > > I've tried to wrap my head around all of this like twenty times
> > > > in
> > > > the
> > > > last three month, but was always interrupted at some point.
> > > > 
> > > > Feel free to add Acked-by: Christian Koenig
> > > > .
> > > > 
> > > > Sorry,
> > > > Christian.
> > > 
> > > Thanks a lot for all reviewing and comments so far. There are two
> > > TTM
> > > patches left in the series that don't have an ack by you:
> > > 
> > > https://patchwork.freedesktop.org/patch/634715/?series=131815&rev=17
> > > and
> > > 
> > > https://patchwork.freedesktop.org/patch/634716/?series=131815&rev=17
> > > 
> > > None of them particularly big considering the amount of doc text.
> > > 
> > > It'd be great if those could have an ack as well so we could
> > > finally
> > > merge this series.
> > > 
> > > Thanks,
> > > Thomas
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > > 
> > > > > ---
> > > > >   drivers/gpu/drm/ttm/ttm_pool.c | 554
> > > > > +
> > > > >   drivers/gpu/drm/ttm/ttm_tt.c   |  54 
> > > > >   include/drm/ttm/ttm_pool.h |   8 +
> > > > >   include/drm/ttm/ttm_tt.h   |  67 +++-
> > > > >   4 files changed, 629 insertions(+), 54 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
> > > > > b/drivers/gpu/drm/ttm/ttm_pool.c
> > > > > index c9eba76d5143..ffb7abf52bab 100644
> > > > > --- a/drivers/gpu/drm/ttm/ttm_pool.c
> > > > > +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> > > > > @@ -41,6 +41,7 @@
> > > > >   #include 
> > > > >   #endif
> > > > > 
> > > > > +#include 
> > > > >   #include 
> > > > >   #include 
> > > > >   #include 
> > > > > @@ -75,6 +76,35 @@ struct ttm_pool_alloc_state {
> > > > >   enum ttm_caching tt_caching;
> > > > >   };
> > > > > 
> > > > > +/**
> > > > > + * struct ttm_pool_tt_restore - State representing restore
> > > > > from
> > > > > backup
> > > > > + * @pool: The pool used for page allocation while restoring.
> > > > > + * @snapshot_alloc: A sna

Re: [PATCH 2/6] drm/bridge: synopsys: Add DW DPTX Controller support library

2025-03-06 Thread Yubing Zhang


Hi Dmitry,

On 2025/3/2 2:14, Dmitry Baryshkov wrote:

On Sun, Feb 23, 2025 at 07:30:25PM +0800, Andy Yan wrote:

From: Andy Yan 

The DW DP TX Controller is compliant with the DisplayPort Specification
Version 1.4 with the following features:

* DisplayPort 1.4a
* Main Link: 1/2/4 lanes
* Main Link Support 1.62Gbps, 2.7Gbps, 5.4Gbps and 8.1Gbps
* AUX channel 1Mbps
* Single Stream Transport(SST)
* Multistream Transport (MST)
*Type-C support (alternate mode)
* HDCP 2.2, HDCP 1.3
* Supports up to 8/10 bits per color component
* Supports RBG, YCbCr4:4:4, YCbCr4:2:2, YCbCr4:2:0
* Pixel clock up to 594MHz
* I2S, SPDIF audio interface

Add library with common helpers to make it can be shared with
other SoC.

Signed-off-by: Andy Yan 

drm/bridge: cleanup


Stray line?



---

  drivers/gpu/drm/bridge/synopsys/Kconfig  |7 +
  drivers/gpu/drm/bridge/synopsys/Makefile |1 +
  drivers/gpu/drm/bridge/synopsys/dw-dp.c  | 2155 ++
  include/drm/bridge/dw_dp.h   |   19 +
  4 files changed, 2182 insertions(+)
  create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-dp.c


..


+
+static u8 dw_dp_voltage_max(u8 preemph)
+{
+   switch (preemph & DP_TRAIN_PRE_EMPHASIS_MASK) {
+   case DP_TRAIN_PRE_EMPH_LEVEL_0:
+   return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
+   case DP_TRAIN_PRE_EMPH_LEVEL_1:
+   return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
+   case DP_TRAIN_PRE_EMPH_LEVEL_2:
+   return DP_TRAIN_VOLTAGE_SWING_LEVEL_1;
+   case DP_TRAIN_PRE_EMPH_LEVEL_3:
+   default:
+   return DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
+   }
+}
+
+static void dw_dp_link_get_adjustments(struct dw_dp_link *link,
+  u8 status[DP_LINK_STATUS_SIZE])
+{
+   struct dw_dp_link_train_set *adjust = &link->train.adjust;
+   u8 v = 0;
+   u8 p = 0;
+   unsigned int i;
+
+   for (i = 0; i < link->lanes; i++) {
+   v = drm_dp_get_adjust_request_voltage(status, i);
+   p = drm_dp_get_adjust_request_pre_emphasis(status, i);
+   if (p >=  DP_TRAIN_PRE_EMPH_LEVEL_3) {
+   adjust->pre_emphasis[i] = DP_TRAIN_PRE_EMPH_LEVEL_3 >>
+ DP_TRAIN_PRE_EMPHASIS_SHIFT;
+   adjust->pre_max_reached[i] = true;
+   } else {
+   adjust->pre_emphasis[i] = p >> 
DP_TRAIN_PRE_EMPHASIS_SHIFT;
+   adjust->pre_max_reached[i] = false;
+   }
+   v = min(v, dw_dp_voltage_max(p));
+   if (v >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) {
+   adjust->voltage_swing[i] = DP_TRAIN_VOLTAGE_SWING_LEVEL_3 
>>
+  DP_TRAIN_VOLTAGE_SWING_SHIFT;
+   adjust->voltage_max_reached[i] = true;
+   } else {
+   adjust->voltage_swing[i] = v >> 
DP_TRAIN_VOLTAGE_SWING_SHIFT;
+   adjust->voltage_max_reached[i] = false;
+   }
+   }
+}
+
+static void dw_dp_link_train_adjust(struct dw_dp_link_train *train)
+{
+   struct dw_dp_link_train_set *request = &train->request;
+   struct dw_dp_link_train_set *adjust = &train->adjust;
+   unsigned int i;
+
+   for (i = 0; i < 4; i++) {


Shouldn't it be a loop up to link->lanes?


+   if (request->voltage_swing[i] != adjust->voltage_swing[i])
+   request->voltage_swing[i] = adjust->voltage_swing[i];
+   if (request->voltage_max_reached[i] != 
adjust->voltage_max_reached[i])
+   request->voltage_max_reached[i] = 
adjust->voltage_max_reached[i];
+   }
+
+   for (i = 0; i < 4; i++) {
+   if (request->pre_emphasis[i] != adjust->pre_emphasis[i])
+   request->pre_emphasis[i] = adjust->pre_emphasis[i];
+   if (request->pre_max_reached[i] != adjust->pre_max_reached[i])
+   request->pre_max_reached[i] = 
adjust->pre_max_reached[i];


Why do you need separate request and adjustment structs?
During link training cr sequence, if dprx keep the LANEx_CR_DONE bit(s) 
cleared, the request and adjustment structs are used to check the

old and new valud of ADJUST_REQUEST_LANEx_y register(s) is changed or not

Re: [PATCH v4 4/8] bits: introduce fixed-type BIT

On Wed, Mar 05, 2025 at 09:50:27PM +, David Laight wrote:
> On Wed, 5 Mar 2025 21:56:22 +0200
> Andy Shevchenko  wrote:
> > On Thu, Mar 06, 2025 at 02:17:18AM +0900, Vincent Mailhol wrote:
> > > On 06/03/2025 at 00:48, Andy Shevchenko wrote:  
> > > > On Wed, Mar 05, 2025 at 11:48:10PM +0900, Vincent Mailhol wrote:  
> > > >> On 05/03/2025 at 23:33, Andy Shevchenko wrote:  
> > > >>> On Wed, Mar 05, 2025 at 10:00:16PM +0900, Vincent Mailhol via B4 
> > > >>> Relay wrote:  

...

> > >  +#define BIT_U8(b) (BIT_INPUT_CHECK(u8, b) + (unsigned int)BIT(b))
> > >  +#define BIT_U16(b) (BIT_INPUT_CHECK(u16, b) + (unsigned int)BIT(b)) 
> > >   
> > > >>>
> > > >>> Why not u8 and u16? This inconsistency needs to be well justified.  
> > > >>
> > > >> Because of the C integer promotion rules, if casted to u8 or u16, the
> > > >> expression will immediately become a signed integer as soon as it is 
> > > >> get
> > > >> used. For example, if casted to u8
> > > >>
> > > >>   BIT_U8(0) + BIT_U8(1)
> > > >>
> > > >> would be a signed integer. And that may surprise people.  
> > > > 
> > > > Yes, but wouldn't be better to put it more explicitly like
> > > > 
> > > > #define BIT_U8(b)   (BIT_INPUT_CHECK(u8, b) + (u8)BIT(b) + 0 + 
> > > > UL(0)) // + ULL(0) ?  
> > > 
> > > OK, the final result would be unsigned. But, I do not follow how this is
> > > more explicit.
> > > 
> > > Also, why doing:
> > > 
> > >   (u8)BIT(b) + 0 + UL(0)
> > > 
> > > and not just:
> > > 
> > >   (u8)BIT(b) + UL(0)
> > > 
> > > ?
> > > 
> > > What is that intermediary '+ 0' for?
> > > 
> > > I am sorry, but I am having a hard time understanding how casting to u8
> > > and then doing an addition with an unsigned long is more explicit than
> > > directly doing a cast to the desired type.  
> > 
> > Reading this again, I think we don't need it at all. u8, aka unsigned char,
> > will be promoted to int, but it will be int with a value < 256, can't be 
> > signed
> > as far as I understand this correctly.
> 
> The value can't be negative, but the type will be a signed one.

Yes, that's what I mentioned above: "int with the value < 256".

> Anything comparing types (and there are a few) will treat it as signed.
> It really is bad practise to even pretend you can have an expression
> (rather that a variable) that has a type smaller than 'int'.
> It wouldn't surprise me if even an 'a = b' assignment promotes 'b' to int.

We have tons of code with u8/u16, what you are proposing here is like
"let's get rid of those types and replace all of them by int/unsigned int".
We have ISAs that are byte-oriented despite being 32- or 64-bit platforms.

> So it is even questionable whether BIT8() and BIT16() should even exist at 
> all.

The point is to check the boundaries and not in the returned value per se.

> There can be reasons to return 'unsigned int' rather than 'unsigned long'.
> But with the type definitions that Linux uses (and can't really be changed)
> you can have BIT32() that is 'unsigned int' and BIT64() that is 'unsigned long
> long'. These are then the same on 32bit and 64bit.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] MAINTAINERS: use kernel.org alias

2025-03-06 Thread neil . armstrong


On 05/03/2025 23:44, Dmitry Baryshkov wrote:

My Linaro email will stop working soon. Use @kernel.org email instead.

Signed-off-by: Dmitry Baryshkov 
---
  MAINTAINERS | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 
29e1a423eee5bcf9df7938aaffe5bd3e2f6a2bbe..b3a67e278a839fa14d1329a249ecf4bbec00c26c
 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7459,7 +7459,7 @@ F:include/uapi/drm/msm_drm.h
  DRM DRIVER for Qualcomm display hardware
  M:Rob Clark 
  M:Abhinav Kumar 
-M: Dmitry Baryshkov 
+M: Dmitry Baryshkov 
  R:Sean Paul 
  R:Marijn Suijten 
  L:linux-arm-...@vger.kernel.org



Reviewed-by: Neil Armstrong

Re: [PATCH] drm/gma500: Replace deprecated strncpy() with strscpy()

On Tue, Feb 25, 2025 at 9:39 PM Thorsten Blum  wrote:
>
> strncpy() is deprecated for NUL-terminated destination buffers. Use
> strscpy() instead and remove the manual NUL-termination.
>
> Compile-tested only.
>
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-harden...@vger.kernel.org
> Signed-off-by: Thorsten Blum 

Thanks for the patch.
Applied to drm-misc-next

-Patrik

> ---
>  drivers/gpu/drm/gma500/cdv_intel_dp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c 
> b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> index cc2ed9b3fd2d..ca7f59ff1fda 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> @@ -855,8 +855,7 @@ cdv_intel_dp_i2c_init(struct gma_connector *connector,
>
> memset(&intel_dp->adapter, '\0', sizeof (intel_dp->adapter));
> intel_dp->adapter.owner = THIS_MODULE;
> -   strncpy (intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) 
> - 1);
> -   intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0';
> +   strscpy(intel_dp->adapter.name, name);
> intel_dp->adapter.algo_data = &intel_dp->algo;
> intel_dp->adapter.dev.parent = connector->base.kdev;
>
> --
> 2.48.1
>

Re: [PATCH v4 4/8] bits: introduce fixed-type BIT

2025-03-06 Thread Vincent Mailhol

On 06/03/2025 at 18:12, Andy Shevchenko wrote:
> On Wed, Mar 05, 2025 at 09:50:27PM +, David Laight wrote:
>> On Wed, 5 Mar 2025 21:56:22 +0200
>> Andy Shevchenko  wrote:
>>> On Thu, Mar 06, 2025 at 02:17:18AM +0900, Vincent Mailhol wrote:
 On 06/03/2025 at 00:48, Andy Shevchenko wrote:  
> On Wed, Mar 05, 2025 at 11:48:10PM +0900, Vincent Mailhol wrote:  
>> On 05/03/2025 at 23:33, Andy Shevchenko wrote:  
>>> On Wed, Mar 05, 2025 at 10:00:16PM +0900, Vincent Mailhol via B4 Relay 
>>> wrote:  
> 
> ...
> 
 +#define BIT_U8(b) (BIT_INPUT_CHECK(u8, b) + (unsigned int)BIT(b))
 +#define BIT_U16(b) (BIT_INPUT_CHECK(u16, b) + (unsigned int)BIT(b))  
>>>
>>> Why not u8 and u16? This inconsistency needs to be well justified.  
>>
>> Because of the C integer promotion rules, if casted to u8 or u16, the
>> expression will immediately become a signed integer as soon as it is get
>> used. For example, if casted to u8
>>
>>   BIT_U8(0) + BIT_U8(1)
>>
>> would be a signed integer. And that may surprise people.  
>
> Yes, but wouldn't be better to put it more explicitly like
>
> #define BIT_U8(b) (BIT_INPUT_CHECK(u8, b) + (u8)BIT(b) + 0 + UL(0)) // + 
> ULL(0) ?  

 OK, the final result would be unsigned. But, I do not follow how this is
 more explicit.

 Also, why doing:

   (u8)BIT(b) + 0 + UL(0)

 and not just:

   (u8)BIT(b) + UL(0)

 ?

 What is that intermediary '+ 0' for?

 I am sorry, but I am having a hard time understanding how casting to u8
 and then doing an addition with an unsigned long is more explicit than
 directly doing a cast to the desired type.  
>>>
>>> Reading this again, I think we don't need it at all. u8, aka unsigned char,
>>> will be promoted to int, but it will be int with a value < 256, can't be 
>>> signed
>>> as far as I understand this correctly.
>>
>> The value can't be negative, but the type will be a signed one.
> 
> Yes, that's what I mentioned above: "int with the value < 256".
> 
>> Anything comparing types (and there are a few) will treat it as signed.
>> It really is bad practise to even pretend you can have an expression
>> (rather that a variable) that has a type smaller than 'int'.
>> It wouldn't surprise me if even an 'a = b' assignment promotes 'b' to int.
> 
> We have tons of code with u8/u16, what you are proposing here is like
> "let's get rid of those types and replace all of them by int/unsigned int".
> We have ISAs that are byte-oriented despite being 32- or 64-bit platforms.
> 
>> So it is even questionable whether BIT8() and BIT16() should even exist at 
>> all.
> 
> The point is to check the boundaries and not in the returned value per se.

+1

I will also add that this adds to the readability of the code. In a
driver, if I see:

  #define REG_FOO1_MASK GENMASK(6, 2)
  #define REG_FOO2_MASK GENMASK(12, 7)

it does not tell me much about the register. Whereas if I see:

  #define REG_FOO1_MASK GENMASK_U16(6, 2)
  #define REG_FOO2_MASK GENMASK_U16(12, 7)

then I know that this is for a 16 bit register.

>> There can be reasons to return 'unsigned int' rather than 'unsigned long'.
>> But with the type definitions that Linux uses (and can't really be changed)
>> you can have BIT32() that is 'unsigned int' and BIT64() that is 'unsigned 
>> long
>> long'. These are then the same on 32bit and 64bit.

So, at the end, my goal when introducing that unsigned int cast was not
to confuse people. This had the opposite effect. Nearly all the
reviewers pointed at that cast.

I will revert this in the v5. The U8 and U16 variants of both GENMASK
and BIT will return an u8 and u16 respectively. And unless someone
manages to convince Yury otherwise, I will keep it as such.


Yours sincerely,
Vincent Mailhol

Re: [PATCH] drm/i915: implement vmap/vunmap GEM object functions

2025-03-06 Thread Andi Shyti

Hi Asbjorn,

On Sat, Jun 29, 2024 at 06:25:06PM +, Asbjørn Sloth Tønnesen wrote:
> Implement i915_gem_vmap_object() and i915_gem_vunmap_object(),
> based on i915_gem_dmabuf_vmap() and i915_gem_dmabuf_vunmap().
> 
> This enables a drm_client to use drm_client_buffer_vmap() and
> drm_client_buffer_vunmap() on hardware using the i915 driver.
> 
> Tested with a currently out of tree pixelflut drm_client[1] on:
> - Lenovo ThinkCentre M720q (CoffeeLake-S GT2 / Intel UHD Graphics 630)
> - Dell Wyse N06D - 3030 LT (ValleyView on Intel Celeron N2807 SOC)

do you mind sharing the tests?

> [1] XDP->DRM pixelflut: https://labitat.dk/wiki/Pixelflut-XDR
> 
> Signed-off-by: Asbjørn Sloth Tønnesen 

thanks, reviewed and merged to drm-intel-gt-next.

Andi

Re: [PATCH] drm/gma500: fix inconsistent indenting warning

On Wed, Mar 5, 2025 at 9:49 AM Charles Han  wrote:
>
> Fix below inconsistent indenting smatch warning.
> smatch warnings:
> drivers/gpu/drm/gma500/cdv_device.c:218 cdv_errata() warn: inconsistent 
> indenting
>
> Signed-off-by: Charles Han 

Thanks for the patch.
Applied to drm-misc-next

-Patrik

> ---
>  drivers/gpu/drm/gma500/cdv_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_device.c 
> b/drivers/gpu/drm/gma500/cdv_device.c
> index 3e83299113e3..718d45891fc7 100644
> --- a/drivers/gpu/drm/gma500/cdv_device.c
> +++ b/drivers/gpu/drm/gma500/cdv_device.c
> @@ -215,7 +215,7 @@ static void cdv_errata(struct drm_device *dev)
>  *  Bonus Launch to work around the issue, by degrading
>  *  performance.
>  */
> -CDV_MSG_WRITE32(pci_domain_nr(pdev->bus), 3, 0x30, 0x08027108);
> +   CDV_MSG_WRITE32(pci_domain_nr(pdev->bus), 3, 0x30, 0x08027108);
>  }
>
>  /**
> --
> 2.43.0
>

Re: [PATCH] drm/gma500: Add NULL check for pci_gfx_root in mid_get_vbt_data()

On Wed, Mar 5, 2025 at 12:20 PM Ivan Abramov
 wrote:
>
> Since pci_get_domain_bus_and_slot() can return NULL, add NULL check for
> pci_gfx_root in the mid_get_vbt_data().
>
> This change is similar to the checks implemented in mid_get_fuse_settings()
> and mid_get_pci_revID(), which were introduced by commit 0cecdd818cd7
> ("gma500: Final enables for Oaktrail") as "additional minor
> bulletproofing".
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: ba99d8348864 ("drm/gma500: Deprecate pci_get_bus_and_slot()")

Patch looks good but pci_get_bus_and_slot() also returned a struct
pci_dev so the issue was present before ba99d8348864.
The correct fixes tag should be:
Fixes: f910b411053f ("gma500: Add the glue to the various BIOS and
firmware interfaces")

-Patrik

>
> Signed-off-by: Ivan Abramov 
> ---
>  drivers/gpu/drm/gma500/mid_bios.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/gma500/mid_bios.c 
> b/drivers/gpu/drm/gma500/mid_bios.c
> index 7e76790c6a81..cba97d7db131 100644
> --- a/drivers/gpu/drm/gma500/mid_bios.c
> +++ b/drivers/gpu/drm/gma500/mid_bios.c
> @@ -279,6 +279,11 @@ static void mid_get_vbt_data(struct drm_psb_private 
> *dev_priv)
> 0, PCI_DEVFN(2, 0));
> int ret = -1;
>
> +   if (pci_gfx_root == NULL) {
> +   WARN_ON(1);
> +   return;
> +   }
> +
> /* Get the address of the platform config vbt */
> pci_read_config_dword(pci_gfx_root, 0xFC, &addr);
> pci_dev_put(pci_gfx_root);
> --
> 2.48.1
>

Re: [PATCH v3 1/4] drm/i915: Skip harmful unregister steps if not registered

2025-03-06 Thread Jani Nikula

On Wed, 05 Mar 2025, Janusz Krzysztofik  
wrote:
> Starting with commit ec3e00b4ee27 ("drm/i915: stop registering if
> drm_dev_register() fails"), we return from i915_driver_register()
> immediately if drm_dev_register() fails, skipping remaining registration
> steps.  However, the _unregister() counterpart called at device remove
> knows nothing about that skip and executes reverts of all those steps,
> some of those reverts possibly added or modified later.  As a consequence,
> a number of kernel warnings that taint the kernel are triggered:
>
> <3> [525.823143] i915 :00:02.0: [drm] *ERROR* Failed to register driver 
> for
> userspace access!
> ...
> <4> [525.831069] [ cut here ]
> <4> [525.831071] i915 :00:02.0: [drm] 
> drm_WARN_ON(power_domains->init_wakere
> f)
> <4> [525.831095] WARNING: CPU: 6 PID: 3440 at 
> drivers/gpu/drm/i915/display/intel
> _display_power.c:2074 intel_power_domains_disable+0xc2/0xd0 [i915]
> ...
> <4> [525.831328] CPU: 6 UID: 0 PID: 3440 Comm: i915_module_loa Tainted: G 
> U
> 6.14.0-rc1-CI_DRM_16076-g7a632b6798b6+ #1
> ...
> <4> [525.831334] RIP: 0010:intel_power_domains_disable+0xc2/0xd0 [i915]
> ...
> <4> [525.831483] Call Trace:
> <4> [525.831484]  
> ...
> <4> [525.831943]  i915_driver_remove+0x4b/0x140 [i915]
> <4> [525.832028]  i915_pci_remove+0x1e/0x40 [i915]
> <4> [525.832099]  pci_device_remove+0x3e/0xb0
> <4> [525.832103]  device_remove+0x40/0x80
> <4> [525.832107]  device_release_driver_internal+0x215/0x280
> ...
> <4> [525.947666] [ cut here ]
> <4> [525.947669] kobject: '(null)' (88814f62a218): is not initialized, 
> yet kobject_put() is being called.
> <4> [525.947707] WARNING: CPU: 6 PID: 3440 at lib/kobject.c:734 
> kobject_put+0xe4/0x200
> ...
> <4> [525.947875] RIP: 0010:kobject_put+0xe4/0x200
> ...
> <4> [525.947909] Call Trace:
> <4> [525.947911]  
> ...
> <4> [525.947963]  intel_gt_sysfs_unregister+0x25/0x40 [i915]
> <4> [525.948133]  intel_gt_driver_unregister+0x14/0x80 [i915]
> <4> [525.948291]  i915_driver_remove+0x6c/0x140 [i915]
> <4> [525.948411]  i915_pci_remove+0x1e/0x40 [i915]
> ...
> <4> [526.441186] [ cut here ]
> <4> [526.441191] kernfs: can not remove 'error', no directory
> <4> [526.441211] WARNING: CPU: 1 PID: 3440 at fs/kernfs/dir.c:1684 
> kernfs_remove_by_name_ns+0xbc/0xc0
> ...
> <4> [526.441536] RIP: 0010:kernfs_remove_by_name_ns+0xbc/0xc0
> ...
> <4> [526.441578] Call Trace:
> <4> [526.441581]  
> ...
> <4> [526.441686]  sysfs_remove_bin_file+0x17/0x30
> <4> [526.441691]  i915_gpu_error_sysfs_teardown+0x1d/0x30 [i915]
> <4> [526.442226]  i915_teardown_sysfs+0x1c/0x60 [i915]
> <4> [526.442369]  i915_driver_remove+0x9d/0x140 [i915]
> <4> [526.442473]  i915_pci_remove+0x1e/0x40 [i915]
> ...
> <4> [526.685700] [ cut here ]
> <4> [526.685706] i915 :00:02.0: [drm] i915 raw-wakerefs=1 wakelocks=1 on 
> cle
> anup
> <4> [526.685734] WARNING: CPU: 1 PID: 3440 at 
> drivers/gpu/drm/i915/intel_runtime
> _pm.c:443 intel_runtime_pm_driver_release+0x75/0x90 [i915]
> ...
> <4> [526.686090] RIP: 0010:intel_runtime_pm_driver_release+0x75/0x90 [i915]
> ...
> <4> [526.686294] Call Trace:
> <4> [526.686296]  
> ...
> <4> [526.687025]  i915_driver_release+0x7e/0xb0 [i915]
> <4> [526.687243]  drm_dev_put.part.0+0x47/0x90
> <4> [526.687250]  devm_drm_dev_init_release+0x13/0x30
> <4> [526.687255]  devm_action_release+0x12/0x30
> <4> [526.687261]  release_nodes+0x3a/0x120
> <4> [526.687268]  devres_release_all+0x97/0xe0
> <4> [526.687277]  device_unbind_cleanup+0x12/0x80
> <4> [526.687282]  device_release_driver_internal+0x23a/0x280
> ...
>
> A call to intel_power_domains_disable() that triggeres the drm_WARN_ON()
> and takes another wakeref even if the one taken during driver register was
> not released after device register error, was already there.  The WARN()
> triggered by kernfs_remove_by_name_ns() from i915_teardown_sysfs()->
> i915_gpu_error_sysfs_teardown(), formerly i915_teardown_error_capture(),
> was also there.  A call to intel_gt_sysfs_unregister() that triggers the
> WARN() from kobject_put() was added to intel_gt_driver_unregister() with
> commit 69d6bf5c3754ff ("drm/i915/gt: Fix memory leaks in per-gt sysfs").
>
> Introduce a flag that indicates device registration status and raise it on
> device registration success.  As a minimum (first step), when that flag is
> found not set while unregistering the driver, jump over those reverts of
> registration steps omitted after device registration failure that are not
> ready for being called unconditionally (and trigger kernel warnings).
>
> In case of i915_gt_driver_unregister() called for each GT, omitting it
> proved to introduce new issues.  Skip only its problematic
> intel_gt_sysfs_unregister() sub-step.
>
> Other unregister steps seem to be safe but may still occur redundant in
> that scenario -- that will be addressed in follow-up patches.
>
> v3: Based on Andi's c

Re: [PATCH v4 3/8] bits: introduce fixed-type genmasks

2025-03-06 Thread Vincent Mailhol

On 06/03/2025 at 04:45, Andy Shevchenko wrote:
>>> But GENMASK_U128() becomes a special case now.
>>> The 128-bit GENMASK is unsued, but it's exported in uapi. Is there any
>>> simple way to end up with a common implementation for all fixed-type
>>> GENMASKs?
>>
>> What bothers me is that the 128 bit types are not something available on
>> all architectures, c.f. the CONFIG_ARCH_SUPPORTS_INT128. So, I would
>> need a U128() equivalent to the ULL() but which does not break on
>> architectures which do not support 128 bits integers.
>>
>> This is where I am stuck. If someone can guide me on how to write a
>> robust U128() macro, then I think the common implementation could be
>> feasible.
> 
> I think we may leave that U128 stuff alone for now.

I found the solution! The trick is to use type_max() from overflow.h.

With this, GENMASK_TYPE() becomes:

  #define GENMASK_TYPE(t, h, l) \
((t)(GENMASK_INPUT_CHECK(h, l) +\
 (type_max(t) << (l) &  \
  type_max(t) >> (BITS_PER_TYPE(t) - 1 - (h)

and works with all the GENMASK variants, including the U128 one! The
unit tests under lib/test_bits.c are all green.

Of course, this does *not* work in assembly. But as explained before,
GENMASK_TYPE() is guarded by a #if !defined(__ASSEMBLY__), so all good!

The question raised by Yury on whether or not we should keep
__GENMASK_U128() in the uapi still remains. And in full honesty, I will
not touch that one. This is not in the scope of this series.

Yours sincerely,
Vincent Mailhol

Re: [PATCH v2 03/16] drm/vkms: Allow to configure multiple planes via configfs

2025-03-06 Thread José Expósito

Hi Louis,

On Tue, Mar 04, 2025 at 07:17:53PM +0100, Louis Chauvet wrote:
> 
> 
> Le 04/03/2025 à 17:23, José Expósito a écrit :
> > On Tue, Mar 04, 2025 at 04:35:50PM +0100, Louis Chauvet wrote:
> > > 
> > > 
> > > Le 04/03/2025 à 15:54, José Expósito a écrit :
> > > > Hi Louis,
> > > > 
> > > > On Mon, Mar 03, 2025 at 11:34:50AM +0100, Louis Chauvet wrote:
> > > > > 
> > > > > 
> > > > > Le 03/03/2025 à 09:50, José Expósito a écrit :
> > > > > > Hi Louis,
> > > > > > 
> > > > > > On Fri, Feb 28, 2025 at 03:43:25PM +0100, Louis Chauvet wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > Le 25/02/2025 à 18:59, José Expósito a écrit :
> > > > > > > > Create a default subgroup at /config/vkms/planes to allow to 
> > > > > > > > create as
> > > > > > > > many planes as required.
> > > > > > > > 
> > > > > > > > Reviewed-by: Louis Chauvet 
> > > > > > > > Co-developed-by: Louis Chauvet 
> > > > > > > > Signed-off-by: Louis Chauvet 
> > > > > > > > Signed-off-by: José Expósito 
> > > > > > > > [...]
> > > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c 
> > > > > > > > b/drivers/gpu/drm/vkms/vkms_configfs.c
> > > > > > > > index 92512d52ddae..4f9d3341e6c0 100644
> > > > > > > > --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> > > > > > > > +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> > > > > > > > [...]
> > > > > > > > +static void plane_release(struct config_item *item)
> > > > > > > > +{
> > > > > > > > +   struct vkms_configfs_plane *plane;
> > > > > > > > +   struct mutex *lock;
> > > > > > > > +
> > > > > > > > +   plane = plane_item_to_vkms_configfs_plane(item);
> > > > > > > > +   lock = &plane->dev->lock;
> > > > > > > > +
> > > > > > > > +   guard(mutex)(lock);
> > > > > > > > +   vkms_config_destroy_plane(plane->config);
> > > > > > > > +   kfree(plane);
> > > > > > > > +}
> > > > > > > 
> > > > > > > I just found a flaw in our work: there is currently no way to 
> > > > > > > forbid the
> > > > > > > deletion of item/symlinks...
> > > > > > > 
> > > > > > > If you do:
> > > > > > > 
> > > > > > > modprobe vkms
> > > > > > > cd /sys/kernel/config/vkms/
> > > > > > > mkdir DEV
> > > > > > > mkdir DEV/connectors/CON
> > > > > > > mkdir DEV/planes/PLA
> > > > > > > mkdir DEV/crtcs/CRT
> > > > > > > mkdir DEV/encoders/ENC
> > > > > > > ln -s DEV/crtcs/CRT DEV/planes/PLA/possible_crtcs/
> > > > > > > ln -s DEV/crtcs/CRT DEV/encoders/ENC/possible_crtcs
> > > > > > > ln -s DEV/encoders/ENC DEV/connectors/CON/possible_encoders
> > > > > > > echo 1 > DEV/planes/PLA/type
> > > > > > > tree
> > > > > > > echo 1 > DEV/enabled
> > > > > > > modetest -M vkms
> > > > > > > => everything fine
> > > > > > > 
> > > > > > > rm DEV/connectors/CON/possible_encoders/ENC
> > > > > > > rmdir DEV/connectors/CON
> > > > > > > modetest -M vkms
> > > > > > > => BUG: KASAN: slab-use-after-free
> > > > 
> > > > I'm trying to reproduce this issue, but those commands don't show any 
> > > > BUG
> > > > in dmesg. This is my Kasan .config:
> > > > 
> > > >   CONFIG_HAVE_ARCH_KASAN=y
> > > >   CONFIG_HAVE_ARCH_KASAN_VMALLOC=y
> > > >   CONFIG_CC_HAS_KASAN_GENERIC=y
> > > >   CONFIG_CC_HAS_KASAN_SW_TAGS=y
> > > >   CONFIG_CC_HAS_WORKING_NOSANITIZE_ADDRESS=y
> > > >   CONFIG_KASAN=y
> > > >   CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX=y
> > > >   CONFIG_KASAN_GENERIC=y
> > > >   # CONFIG_KASAN_OUTLINE is not set
> > > >   CONFIG_KASAN_INLINE=y
> > > >   CONFIG_KASAN_STACK=y
> > > >   CONFIG_KASAN_VMALLOC=y
> > > >   # CONFIG_KASAN_KUNIT_TEST is not set
> > > >   CONFIG_KASAN_EXTRA_INFO=y
> > > > 
> > > > I tryed to delete even more items:
> > > > 
> > > >   root@kernel-dev:/sys/kernel/config/vkms# tree
> > > >   .
> > > >   └── DEV
> > > >   ├── connectors
> > > >   ├── crtcs
> > > >   ├── enabled
> > > >   ├── encoders
> > > >   └── planes
> > > > 
> > > >   root@kernel-dev:/sys/kernel/config/vkms# cat DEV/enabled
> > > >   1
> > > > 
> > > > And I still don't see any errors. Is it possible that we are running 
> > > > different
> > > > branches? Asking because of the failing IGT tests you reported. There 
> > > > seems to
> > > > be a difference in our code or setup that is creating these differences.
> > > 
> > > I just re-applied your last vkms-config version and this series on top of
> > > drm-misc-next. See [1] for the exact commits.
> > > 
> > > Argg sorry, I just noticed something: you need to disable the default vkms
> > > device (I had this option in my kernel command line...), otherwise 
> > > modetest
> > > only use the first vkms gpu...
> > > 
> > > I will check again the igt tests, but I don't think this is the same issue
> > > (it should not use the default device to test)
> > > 
> > > So, with [1] and the defconfig below, I have this:
> > > 
> > > 
> > >  1  modprobe vkms create_default_dev=0
> > >  2  cd /sys/kernel/config/vkms/
> > >  3  mkdir DEV
> > >  4  m

Re: [RFC PATCH 0/3] gpu: nova-core: add basic timer subdevice implementation

2025-03-06 Thread Simona Vetter

On Wed, Mar 05, 2025 at 11:10:12AM -0400, Jason Gunthorpe wrote:
> On Wed, Mar 05, 2025 at 08:30:34AM +0100, Simona Vetter wrote:
> > - developers who want to quickly test new driver versions without full
> >   reboot. They're often preferring convenience over correctness, like with
> >   the removal of module refcounting that's strictly needed but means they
> >   first have to unbind drivers in sysfs before they can unload the driver.
> > 
> >   Another one is that this use-case prefers that the hw is cleanly shut
> >   down, so that you can actually load the new driver from a well-known
> >   state. And it's entirely ok if this all fails occasionally, it's just
> >   for development and testing.
> 
> I've never catered to this because if you do this one:
> 
> > - hotunplug as an actual use-case. Bugs are not ok. The hw can go away at
> >   any moment. And it might happen while you're holding console_lock. You
> >   generally do not remove the actual module here, which is why for the
> >   actual production use-case getting that part right isn't really
> >   required. But getting the lifetimes of all the various
> >   structs/objects/resources perfectly right is required.
> 
> Fully and properly then developers are happy too..
> 
> And we were always able to do this one..
> 
> > So the "stuck on console_lock" is the 2nd case, not the first. Module
> > unload doesn't even come into play on that one.
> 
> I don't see reliable hot unplug if the driver can get stuck on a
> lock :|
> 
> > > Assuming all your interrupt triggered sleeps have gained a shootdown
> > > mechanism.
> > 
> > Hence why I want revocable to only be rcu, not srcu.
> 
> Sorry, I was not clear. You also have to make the PCI interrupt(s)
> revocable. Just like the MMIO it cannot leak past the remove() as a
> matter of driver-model correctness.
> 
> So, you end up disabling the interrupt while the driver is still
> running and any sleeps in the driver that are waiting for an interrupt
> still need to be shot down.
> 
> Further, I just remembered, (Danilo please notice!) there is another
> related issue here that DMA mappings *may not* outlive remove()
> either. netdev had a bug related to this recently and it was all
> agreed that it is not allowed. The kernel can crash in a couple of
> different ways if you try to do this.
> 
> https://lore.kernel.org/lkml/8067f204-1380-4d37-8ffd-007fc6f26...@kernel.org/T/#m0c7dda0fb5981240879c5ca489176987d688844c

Hm for the physical dma I thought disabling pci bust master should put a
stop to all this stuff?

For the sw lifecycle stuff I honestly didn't know that was an issue, I
guess that needs to be adressed in the dma-api wrappers or rust can blow
up in funny ways. C drivers just walk all mappings and shoot them.

>  > a device with no driver bound should not be passed to the DMA API,
>  > much less a dead device that's already been removed from its parent
>  > bus.
> 
> So now we have a driver design that must have:
>  1) Revocable MMIO
>  2) Revocable Interrupts
>  3) Revocable DMA mappings
>  4) Revocable HW DMA - the HW MUST stop doing DMA before the DMA API
> is shut down. Failure is a correctness/UAF/security issue
> 
> Somehow the driver has to implement this, not get confused or lock up,
> all while Rust doesn't help you guarentee much of any of the important
> properties related to #2/#3/#4. And worse all this manual recvocable
> stuff is special and unique to hot-unplug. So it will all be untested
> and broken.

The trouble is that for real hotunplug, you need all this anyway. Because
when you physically hotunplug the interrupts will be dead, the mmio will
be gone any momem (not just at the beginnning of an rcu revocable
section), so real hotunplug is worse than what we're trying to do here.

Which means I think this actually helps you with testing, since it's much
easier to test stuff with pure software than physically yanking hardware.
You could perhaps fake that with mmiotrace-like infrastructure, but that's
not easy either.

So randomly interrupts not happening is something you need to cope with no
matter what.

> Looks really hard to me. *Especially* the wild DMA thing.
> 
> This has clearly been missed here as with the current suggestion to
> just revoke MMIO means the driver can't actually go out and shutdown
> it's HW DMA after-the-fact since the MMIO is gone. Thus you are pretty
> much guaranteed to fail #4, by design, which is a serious issue.
> 
> I'm sorry it has taken so many emails to reach this, I did know it,
> but didn't put the pieces coherently together till just now :\
> 
> Compare that to how RDMA works, where we do a DMA shutdown by
> destroying all the objects just the same as if the user closed a
> FD. The normal destruction paths fence the HW DMA and we end up in
> remove with cleanly shutdown HW and no DMA API open. The core code
> manages all of this. Simple, correct, no buggy hotplug only paths.

This is where it gets really annoying, because with a physical ho

Re: [PATCH v3 0/4] drm/i915: Fix harmfull driver register/unregister assymetry

2025-03-06 Thread Krzysztof Karas

Hi Janusz,

throughout the series you modify the code right after
introducing it. How about changing the order of things a bit:
 1) order the functions in a symmetrical way between
  register/unregister steps and group them as you see necessary,
  (At that point you would not be fixing the issue yet, but
  prepare the code for further changes)

 2) then introduce the new flag along with all the labels needed
  for clean unregistration.

I think that way you could reduce number of patches (and changes
in code needing review) while also fixing the original issue.

Overall, I believe this is a good effort and much needed change
in registration and unregistering process.

Best Regards,
Krzysztof

> Starting with commit ec3e00b4ee27 ("drm/i915: stop registering if
> drm_dev_register() fails"), we may return from i915_driver_register()
> immediately, skipping remaining registration steps.  However, the
> _unregister() counterpart called at device remove knows nothing about that
> skip and executes reverts of all those steps.  As a consequence, a number
> of kernel warnings that taint the kernel are triggered.
> 
> Introduce a flag that indicates device registration status and raise it on
> device registration success.  As a minimum (first patch), when that flag
> is found not set while unregistering the driver, jump over those reverts
> of registration steps omitted after device registration failure that are
> not ready for being called unconditionally (and trigger the kernel
> warnings).
> 
> With the second patch, also jump over reverts of other driver registration
> steps that were not called due to device registration failure.  Some
> unregister function calls, found implementing additional steps beyond the
> register reverts, are still executed.
> 
> To simplify i915_driver_unregister() code, the third patch makes sure
> reverts of driver registration steps executed before potentially
> unsuccessful device registration are symmetrically called after
> the device unplug.
> 
> Finally, the last patch further simplifies the i915_driver_unregister()
> code by moving two required steps down, right after device unplug.
> 
> The first patch may be squashed with one or more of its follow-ups if so
> decided.
> 
> Janusz Krzysztofik (4):
>   drm/i915: Skip harmful unregister steps if not registered
>   drm/i915: Omit unnecessary driver unregister steps
>   drm/i915: Fix asymmetry in PMU register/unregister step order
>   drm/i915: Group not skipped unregister steps
> 
>  drivers/gpu/drm/i915/gt/intel_gt.c |  6 ++
>  drivers/gpu/drm/i915/i915_driver.c | 18 --
>  drivers/gpu/drm/i915/i915_drv.h|  2 ++
>  3 files changed, 20 insertions(+), 6 deletions(-)
> 
> -- 
> 2.48.1
>

Re: [PATCH v4 3/8] mailbox: mtk-cmdq: Add driver data to support for MT8196

2025-03-06 Thread 林睿祥

Re: [PATCH] drm/i915/gt/uc: Fix typo in a comment

2025-03-06 Thread Andi Shyti

Hi Yuichiro,

On Mon, Feb 24, 2025 at 05:56:37PM +0900, Yuichiro Tsuji wrote:
> Fix typo in a comment.
> 
> explaination -> explanation
> 
> Signed-off-by: Yuichiro Tsuji 

reviewed and merged, thanks.

Andi

Re: [PATCH 0/6] Add support for RK3588 DisplayPort Controller

2025-03-06 Thread Piotr Oniszczuk




> Wiadomość napisana przez Piotr Oniszczuk  w dniu 6 
> mar 2025, o godz. 15:08:
> 
> 
> 
>> Wiadomość napisana przez Andy Yan  w dniu 6 mar 2025, o 
>> godz. 13:15:
>> 
>> Hi Piotr,
>> 
>> 
>> 
>> Then when you DP cable plugin, you can run command as bellow to see if the 
>> driver detects the HPD:
>> 
>> # cat /sys/class/drm/card0-DP-1/status 
>> connected
>> # 
>> 
> 
> 
> Andy,
> Thx!
> 
> With above changes i’m getting „connected”.
> Also it looks crtc gets reasonable mode: 
> https://gist.github.com/warpme/d6220e3cc502086a4c95f05bd9f9cf0c
> 
> Still black screen however…
>   

some additional data point: /sys/kernel/debug/dri/1/vop2/summary 


working hdmi:

Video Port1: ACTIVE
Connector: HDMI-A-1
bus_format[0]: Unknown
output_mode[f] color_space[0]
Display mode: 1920x1080p60
clk[148500] real_clk[148500] type[48] flag[5]
H: 1920 2008 2052 2200
V: 1080 1084 1089 1125
Cluster0-win0: ACTIVE
win_id: 0
format: XR24 little-endian (0x34325258) glb_alpha[0xff]
rotate: xmirror: 0 ymirror: 0 rotate_90: 0 rotate_270: 0
zpos: 0
src: pos[0, 0] rect[1920 x 1080]
dst: pos[0, 0] rect[1920 x 1080]
buf[0]: addr: 0x017e1000 pitch: 7680 offset: 0
Video Port2: DISABLED




non-working DP:

Video Port1: DISABLED
Video Port2: ACTIVE
Connector: DP-1
bus_format[100a]: RGB888_1X24
output_mode[f] color_space[0]
Display mode: 1920x1080p60
clk[148500] real_clk[148500] type[48] flag[5]
H: 1920 2008 2052 2200
V: 1080 1084 1089 1125
Cluster1-win0: ACTIVE
win_id: 1
format: XR24 little-endian (0x34325258) glb_alpha[0xff]
rotate: xmirror: 0 ymirror: 0 rotate_90: 0 rotate_270: 0
zpos: 0
src: pos[0, 0] rect[1920 x 1080]
dst: pos[0, 0] rect[1920 x 1080]
buf[0]: addr: 0x007ed000 pitch: 7680 offset: 0

Re: [PATCH v5 04/16] drm/atomic: Introduce helper to lookup connector by encoder

2025-03-06 Thread Simona Vetter

On Thu, Mar 06, 2025 at 08:10:16AM +0100, Maxime Ripard wrote:
> On Thu, Mar 06, 2025 at 09:16:24AM +0800, Andy Yan wrote:
> > 
> > Hi Maxime and Dmitry:
> > 
> > At 2025-03-06 04:13:53, "Dmitry Baryshkov"  
> > wrote:
> > >On Wed, Mar 05, 2025 at 02:19:36PM +0100, Maxime Ripard wrote:
> > >> Hi Andy,
> > >> 
> > >> On Wed, Mar 05, 2025 at 07:55:19PM +0800, Andy Yan wrote:
> > >> > At 2025-03-04 19:10:47, "Maxime Ripard"  wrote:
> > >> > >With the bridges switching over to drm_bridge_connector, the direct
> > >> > >association between a bridge driver and its connector was lost.
> > >> > >
> > >> > >This is mitigated for atomic bridge drivers by the fact you can access
> > >> > >the encoder, and then call drm_atomic_get_old_connector_for_encoder() 
> > >> > >or
> > >> > >drm_atomic_get_new_connector_for_encoder() with drm_atomic_state.
> > >> > >
> > >> > >This was also made easier by providing drm_atomic_state directly to 
> > >> > >all
> > >> > >atomic hooks bridges can implement.
> > >> > >
> > >> > >However, bridge drivers don't have a way to access drm_atomic_state
> > >> > >outside of the modeset path, like from the hotplug interrupt path or 
> > >> > >any
> > >> > >interrupt handler.
> > >> > >
> > >> > >Let's introduce a function to retrieve the connector currently 
> > >> > >assigned
> > >> > >to an encoder, without using drm_atomic_state, to make these drivers'
> > >> > >life easier.
> > >> > >
> > >> > >Reviewed-by: Dmitry Baryshkov 
> > >> > >Co-developed-by: Simona Vetter 
> > >> > >Signed-off-by: Maxime Ripard 
> > >> > >---
> > >> > > drivers/gpu/drm/drm_atomic.c | 45 
> > >> > > 
> > >> > > include/drm/drm_atomic.h |  3 +++
> > >> > > 2 files changed, 48 insertions(+)
> > >> > >
> > >> > >diff --git a/drivers/gpu/drm/drm_atomic.c 
> > >> > >b/drivers/gpu/drm/drm_atomic.c
> > >> > >index 
> > >> > >9ea2611770f43ce7ccba410406d5f2c528aab022..b926b132590e78f8d41d48eb4da4bccf170ee236
> > >> > > 100644
> > >> > >--- a/drivers/gpu/drm/drm_atomic.c
> > >> > >+++ b/drivers/gpu/drm/drm_atomic.c
> > >> > >@@ -985,10 +985,55 @@ drm_atomic_get_new_connector_for_encoder(const 
> > >> > >struct drm_atomic_state *state,
> > >> > > 
> > >> > >  return NULL;
> > >> > > }
> > >> > > EXPORT_SYMBOL(drm_atomic_get_new_connector_for_encoder);
> > >> > > 
> > >> > >+/**
> > >> > >+ * drm_atomic_get_connector_for_encoder - Get connector currently 
> > >> > >assigned to an encoder
> > >> > >+ * @encoder: The encoder to find the connector of
> > >> > >+ * @ctx: Modeset locking context
> > >> > >+ *
> > >> > >+ * This function finds and returns the connector currently assigned 
> > >> > >to
> > >> > >+ * an @encoder.
> > >> > >+ *
> > >> > >+ * Returns:
> > >> > >+ * The connector connected to @encoder, or an error pointer 
> > >> > >otherwise.
> > >> > >+ * When the error is EDEADLK, a deadlock has been detected and the
> > >> > >+ * sequence must be restarted.
> > >> > >+ */
> > >> > >+struct drm_connector *
> > >> > >+drm_atomic_get_connector_for_encoder(const struct drm_encoder 
> > >> > >*encoder,
> > >> > >+  struct drm_modeset_acquire_ctx 
> > >> > >*ctx)
> > >> > >+{
> > >> > >+ struct drm_connector_list_iter conn_iter;
> > >> > >+ struct drm_connector *out_connector = ERR_PTR(-EINVAL);
> > >> > >+ struct drm_connector *connector;
> > >> > >+ struct drm_device *dev = encoder->dev;
> > >> > >+ int ret;
> > >> > >+
> > >> > >+ ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
> > >> > >+ if (ret)
> > >> > >+ return ERR_PTR(ret);
> > >> > 
> > >> > It seems that this will cause a deadlock when called from a hotplug
> > >> > handling path, I have a WIP DP diver[0], which suggested by Dmitry to
> > >> > use this API from a &drm_bridge_funcs.detect callback to get the
> > >> > connector, as detect is called by drm_helper_probe_detect, which will
> > >> > hold connection_mutex first, so the deaklock happens:
> > >> >
> > >> > drm_helper_probe_detect(struct drm_connector *connector,
> > >> > struct drm_modeset_acquire_ctx *ctx,
> > >> > bool force)
> > >> > {
> > >> > const struct drm_connector_helper_funcs *funcs = 
> > >> > connector->helper_private;
> > >> > struct drm_device *dev = connector->dev;
> > >> > int ret;
> > >> > 
> > >> > if (!ctx)
> > >> > return drm_helper_probe_detect_ctx(connector, force);
> > >> > 
> > >> > ret = drm_modeset_lock(&dev->mode_config.connection_mutex, 
> > >> > ctx);
> > >> > if (ret)
> > >> > return ret;
> > >> > 
> > >> > if (funcs->detect_ctx)
> > >> > ret = funcs->detect_ctx(connector, ctx, force);
> > >> > else if (connector->funcs->detect)
> > >> > ret = connector->funcs->detect(connector, force);
> > >> > else
> > >> > ret = connector_status_con

Re: [PATCH v5 04/16] drm/atomic: Introduce helper to lookup connector by encoder

2025-03-06 Thread Simona Vetter

On Tue, Mar 04, 2025 at 12:10:47PM +0100, Maxime Ripard wrote:
> With the bridges switching over to drm_bridge_connector, the direct
> association between a bridge driver and its connector was lost.
> 
> This is mitigated for atomic bridge drivers by the fact you can access
> the encoder, and then call drm_atomic_get_old_connector_for_encoder() or
> drm_atomic_get_new_connector_for_encoder() with drm_atomic_state.
> 
> This was also made easier by providing drm_atomic_state directly to all
> atomic hooks bridges can implement.
> 
> However, bridge drivers don't have a way to access drm_atomic_state
> outside of the modeset path, like from the hotplug interrupt path or any
> interrupt handler.
> 
> Let's introduce a function to retrieve the connector currently assigned
> to an encoder, without using drm_atomic_state, to make these drivers'
> life easier.
> 
> Reviewed-by: Dmitry Baryshkov 
> Co-developed-by: Simona Vetter 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_atomic.c | 45 
> 
>  include/drm/drm_atomic.h |  3 +++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 
> 9ea2611770f43ce7ccba410406d5f2c528aab022..b926b132590e78f8d41d48eb4da4bccf170ee236
>  100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -985,10 +985,55 @@ drm_atomic_get_new_connector_for_encoder(const struct 
> drm_atomic_state *state,
>  
>   return NULL;
>  }
>  EXPORT_SYMBOL(drm_atomic_get_new_connector_for_encoder);
>  
> +/**
> + * drm_atomic_get_connector_for_encoder - Get connector currently assigned 
> to an encoder
> + * @encoder: The encoder to find the connector of
> + * @ctx: Modeset locking context
> + *
> + * This function finds and returns the connector currently assigned to
> + * an @encoder.

I think it'd be good to link to the other atomic connector functions like
drm_atomic_get_old/new_connector_for_encoder and explain when to use them.
So also add links to the kerneldoc of these other functions pointing to
here.

- This function is for detect, link repair or anything else that comes
  from the hardware. Or in general, anything that's not in atomic
  commit/check code paths.

- In atomic check/commit code you want to use the other functions.

Otherwise people will have an even harder time finding the right variant
in this maze of look-alikes :-)

With the kerneldoc suitably polished:

Reviewed-by: Simona Vetter 
Signed-off-by: Simona Vetter 

> + *
> + * Returns:
> + * The connector connected to @encoder, or an error pointer otherwise.
> + * When the error is EDEADLK, a deadlock has been detected and the
> + * sequence must be restarted.
> + */
> +struct drm_connector *
> +drm_atomic_get_connector_for_encoder(const struct drm_encoder *encoder,
> +  struct drm_modeset_acquire_ctx *ctx)
> +{
> + struct drm_connector_list_iter conn_iter;
> + struct drm_connector *out_connector = ERR_PTR(-EINVAL);
> + struct drm_connector *connector;
> + struct drm_device *dev = encoder->dev;
> + int ret;
> +
> + ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + drm_connector_list_iter_begin(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> + if (!connector->state)
> + continue;
> +
> + if (encoder == connector->state->best_encoder) {
> + out_connector = connector;
> + break;
> + }
> + }
> + drm_connector_list_iter_end(&conn_iter);
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +
> + return out_connector;
> +}
> +EXPORT_SYMBOL(drm_atomic_get_connector_for_encoder);
> +
> +
>  /**
>   * drm_atomic_get_old_crtc_for_encoder - Get old crtc for an encoder
>   * @state: Atomic state
>   * @encoder: The encoder to fetch the crtc state for
>   *
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 
> 4c673f0698fef6b60f77db980378d5e88e0e250e..38636a593c9d98cadda85ccd67326cb152f0dd27
>  100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -623,10 +623,13 @@ struct drm_connector *
>  drm_atomic_get_old_connector_for_encoder(const struct drm_atomic_state 
> *state,
>struct drm_encoder *encoder);
>  struct drm_connector *
>  drm_atomic_get_new_connector_for_encoder(const struct drm_atomic_state 
> *state,
>struct drm_encoder *encoder);
> +struct drm_connector *
> +drm_atomic_get_connector_for_encoder(const struct drm_encoder *encoder,
> +  struct drm_modeset_acquire_ctx *ctx);
>  
>  struct drm_crtc *
>  drm_atomic_get_old_crtc_for_encoder(struct drm_atomic_state *state,
>struct drm_encoder *en

[PATCH v5 3/7] bits: introduce fixed-type BIT_U*()

From: Lucas De Marchi 

Implement fixed-type BIT_U*() to help drivers add stricter checks,
like was done for GENMASK_U*().

Signed-off-by: Lucas De Marchi 
Acked-by: Jani Nikula 
Co-developed-by: Vincent Mailhol 
Signed-off-by: Vincent Mailhol 
---
Changelog:

  v4 -> v5:

- Rename GENMASK_t() to GENMASK_TYPE().

- Use tab indentations instead of single space to separate the
  macro name from its body.

- Add a global comment at the beginning of the file to explain why
  GENMASK_U*() and BIT_U*() are not available in asm.

- Add a new BIT_TYPE() helper function, similar to GENMASK_TYPE().

- Remove the unsigned int cast for the U8 and U16 variants. Move
  the cast to BIT_TYPE().

- Rename the argument from BIT_U*(b) to BIT_U=(nr) for consistency
  with vdso/bits.h.

  v3 -> v4:

- Use const_true() to simplify BIT_INPUT_CHECK().

- Make BIT_U8() and BIT_U16() return an unsigned int instead of a
  u8 and u16. Because of the integer promotion rules in C, an u8
  or an u16 would become a signed integer as soon as these are
  used in any expression. By casting these to unsigned ints, at
  least the signedness is kept.

- Put the cast next to the BIT() macro.

- In BIT_U64(): use BIT_ULL() instead of BIT().
---
 include/linux/bits.h | 26 ++
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 
74219521a56e2639ccff7fdc899d6805ee355a0c..f95e7815cb18636cc47ac17ef66d1bd6668e6819
 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -22,10 +22,10 @@
 /*
  * Missing asm support
  *
- * GENMASK_U*() depends on BITS_PER_TYPE() which relies on sizeof(),
- * something not available in asm. Nethertheless, fixed width integers
- * is a C concept. Assembly code can rely on the long and long long
- * versions instead.
+ * GENMASK_U*() and BIT_U*() depend on BITS_PER_TYPE() which relies on
+ * sizeof(), something not available in asm. Nethertheless, fixed
+ * width integers is a C concept. Assembly code can rely on the long
+ * and long long versions instead.
  */
 
 #include 
@@ -58,6 +58,24 @@
 #define GENMASK_U64(h, l)  GENMASK_TYPE(u64, h, l)
 #define GENMASK_U128(h, l) GENMASK_TYPE(u128, h, l)
 
+/*
+ * Fixed-type variants of BIT(), with additional checks like GENMASK_TYPE(). 
The
+ * following examples generate compiler warnings due to shift-count-overflow:
+ *
+ * - BIT_U8(8)
+ * - BIT_U32(-1)
+ * - BIT_U32(40)
+ */
+#define BIT_INPUT_CHECK(type, nr) \
+   BUILD_BUG_ON_ZERO(const_true((nr) >= BITS_PER_TYPE(type)))
+
+#define BIT_TYPE(type, nr) ((type)(BIT_INPUT_CHECK(type, nr) + BIT_ULL(nr)))
+
+#define BIT_U8(nr) BIT_TYPE(u8, nr)
+#define BIT_U16(nr)BIT_TYPE(u16, nr)
+#define BIT_U32(nr)BIT_TYPE(u32, nr)
+#define BIT_U64(nr)BIT_TYPE(u64, nr)
+
 #else /* defined(__ASSEMBLY__) */
 
 #define GENMASK(h, l)  __GENMASK(h, l)

-- 
2.45.3

[PATCH v5 5/7] test_bits: add tests for GENMASK() and GENMASK_ULL()

From: Vincent Mailhol 

The definitions of GENMASK() and GENMASK_ULL() do not depend any more
on __GENMASK() and __GENMASK_ULL(). Duplicate the existing unit tests
so that __GENMASK{,ULL}() is still covered.

Signed-off-by: Vincent Mailhol 
---
 lib/test_bits.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/lib/test_bits.c b/lib/test_bits.c
index 
c7b38d91e1f16d42b7ca92e62fbd6c19b37e76a0..dc93ded9fdb201e0d44b3c1cd71e233fd62258a5
 100644
--- a/lib/test_bits.c
+++ b/lib/test_bits.c
@@ -7,6 +7,22 @@
 #include 
 
 
+static void __genmask_test(struct kunit *test)
+{
+   KUNIT_EXPECT_EQ(test, 1ul, __GENMASK(0, 0));
+   KUNIT_EXPECT_EQ(test, 3ul, __GENMASK(1, 0));
+   KUNIT_EXPECT_EQ(test, 6ul, __GENMASK(2, 1));
+   KUNIT_EXPECT_EQ(test, 0xul, __GENMASK(31, 0));
+}
+
+static void __genmask_ull_test(struct kunit *test)
+{
+   KUNIT_EXPECT_EQ(test, 1ull, __GENMASK_ULL(0, 0));
+   KUNIT_EXPECT_EQ(test, 3ull, __GENMASK_ULL(1, 0));
+   KUNIT_EXPECT_EQ(test, 0x00e0ull, __GENMASK_ULL(39, 21));
+   KUNIT_EXPECT_EQ(test, 0xull, __GENMASK_ULL(63, 0));
+}
+
 static void genmask_test(struct kunit *test)
 {
KUNIT_EXPECT_EQ(test, 1ul, GENMASK(0, 0));
@@ -93,6 +109,8 @@ static void genmask_input_check_test(struct kunit *test)
 
 
 static struct kunit_case bits_test_cases[] = {
+   KUNIT_CASE(__genmask_test),
+   KUNIT_CASE(__genmask_ull_test),
KUNIT_CASE(genmask_test),
KUNIT_CASE(genmask_ull_test),
KUNIT_CASE(genmask_u128_test),

-- 
2.45.3

[PATCH v5 2/7] bits: introduce fixed-type genmasks

From: Yury Norov 

Add GENMASK_TYPE() which generalizes __GENMASK() to support different
types, and implement fixed-types versions of GENMASK() based on it.
The fixed-type version allows more strict checks to the min/max values
accepted, which is useful for defining registers like implemented by
i915 and xe drivers with their REG_GENMASK*() macros.

The strict checks rely on shift-count-overflow compiler check to fail
the build if a number outside of the range allowed is passed.
Example:

  #define FOO_MASK GENMASK_U32(33, 4)

will generate a warning like:

  include/linux/bits.h:51:27: error: right shift count >= width of type 
[-Werror=shift-count-overflow]
 51 |   type_max(t) >> (BITS_PER_TYPE(t) - 1 - (h)
|   ^~

Signed-off-by: Yury Norov 
Signed-off-by: Lucas De Marchi 
Acked-by: Jani Nikula 
Co-developed-by: Vincent Mailhol 
Signed-off-by: Vincent Mailhol 
---
Changelog:

  v4 -> v5:

- Rename GENMASK_t() to GENMASK_TYPE().

- Fix typo in patch description.

- Use tab indentations instead of single space to separate the
  macro name from its body.

- s/__GENMASK_U*()/GENMASK_U*()/g in the comment.

- Add a tag to credit myself as Co-developer. Keep Yury as the
  main author.

- Modify GENMASK_TYPE() to match the changes made to __GENMASK()
  in: https://github.com/norov/linux/commit/1e7933a575ed

- Replace (t)~_ULL(0) with type_max(t). This is OK because
  GENMASK_TYPE() is not available in asm.

- linux/const.h and asm/bitsperlong.h are not used anymore. Remove
  them.

- Apply GENMASK_TYPE() to GENMASK_U128().

- Remove the unsigned int cast for the U8 and U16 variants. Cast
  to the target type instead. Do that cast directly in
  GENMASK_TYPE().

  v3 -> v4:

- The v3 is one year old. Meanwhile people started using
  __GENMASK() directly. So instead of generalizing __GENMASK() to
  support different types, add a new GENMASK_t().

- replace ~0ULL by ~_ULL(0). Otherwise, GENMASK_t() would fail in
  asm code.

- Make GENMASK_U8() and GENMASK_U16() return an unsigned int. In
  v3, due to the integer promotion rules, these were returning a
  signed integer. By casting these to unsigned int, at least the
  signedness is kept.
---
 include/linux/bitops.h |  1 -
 include/linux/bits.h   | 47 +++
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 
c1cb53cf2f0f8662ed3e324578f74330e63f935d..9be2d50da09a417966b3d11c84092bb2f4cd0bef
 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -8,7 +8,6 @@
 
 #include 
 
-#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE)
 #define BITS_TO_LONGS(nr)  __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
 #define BITS_TO_U64(nr)__KERNEL_DIV_ROUND_UP(nr, 
BITS_PER_TYPE(u64))
 #define BITS_TO_U32(nr)__KERNEL_DIV_ROUND_UP(nr, 
BITS_PER_TYPE(u32))
diff --git a/include/linux/bits.h b/include/linux/bits.h
index 
4819cbe7bd48fbae796fc6005c9f92b1c93a034c..74219521a56e2639ccff7fdc899d6805ee355a0c
 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -2,16 +2,15 @@
 #ifndef __LINUX_BITS_H
 #define __LINUX_BITS_H
 
-#include 
 #include 
 #include 
-#include 
 
 #define BIT_MASK(nr)   (UL(1) << ((nr) % BITS_PER_LONG))
 #define BIT_WORD(nr)   ((nr) / BITS_PER_LONG)
 #define BIT_ULL_MASK(nr)   (ULL(1) << ((nr) % BITS_PER_LONG_LONG))
 #define BIT_ULL_WORD(nr)   ((nr) / BITS_PER_LONG_LONG)
 #define BITS_PER_BYTE  8
+#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE)
 
 /*
  * Create a contiguous bitmask starting at bit position @l and ending at
@@ -20,28 +19,44 @@
  */
 #if !defined(__ASSEMBLY__)
 
+/*
+ * Missing asm support
+ *
+ * GENMASK_U*() depends on BITS_PER_TYPE() which relies on sizeof(),
+ * something not available in asm. Nethertheless, fixed width integers
+ * is a C concept. Assembly code can rely on the long and long long
+ * versions instead.
+ */
+
 #include 
 #include 
+#include 
 
 #define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(const_true((l) > (h)))
 
-#define GENMASK(h, l) \
-   (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
-#define GENMASK_ULL(h, l) \
-   (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
-
 /*
- * Missing asm support
+ * Generate a mask for the specified type @t. Additional checks are made to
+ * guarantee the value returned fits in that type, relying on
+ * shift-count-overflow compiler check to detect incompatible arguments.
+ * For example, all these create build errors or warnings:
  *
- * __GENMASK_U128() depends on _BIT128() which would not work
- * in the asm code, as it shifts an 'unsigned __int128' data
- * type instead of direct representation of 128 bit constants
- * such as long and unsigned long. The fundamental problem is
- * that a 128 bit constant w

[PATCH v5 4/7] drm/i915: Convert REG_GENMASK() to fixed-width GENMASK_U()

From: Lucas De Marchi 

Now that include/linux/bits.h implements fixed-width GENMASK_U*(), use
them to implement the i915/xe specific macros. Converting each driver
to use the generic macros are left for later, when/if other
driver-specific macros are also generalized.

Signed-off-by: Lucas De Marchi 
Acked-by: Jani Nikula 
Signed-off-by: Vincent Mailhol 
---
Changelog:

  v4 -> v5:

- Add braket to macro names in patch description,
  e.g. 'REG_GENMASK*' -> 'REG_GENMASK*()'

  v3 -> v4:

- Remove the prefixes in macro parameters,
  e.g. 'REG_GENMASK(__high, __low)' -> 'REG_GENMASK(high, low)'
---
 drivers/gpu/drm/i915/i915_reg_defs.h | 108 ---
 1 file changed, 11 insertions(+), 97 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h 
b/drivers/gpu/drm/i915/i915_reg_defs.h
index 
e251bcc0c89f5710125bc70f07851b2cb978c89c..39e5ed9511174b8757b9201bff735fa362651b34
 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -9,76 +9,19 @@
 #include 
 #include 
 
-/**
- * REG_BIT() - Prepare a u32 bit value
- * @__n: 0-based bit number
- *
- * Local wrapper for BIT() to force u32, with compile time checks.
- *
- * @return: Value with bit @__n set.
- */
-#define REG_BIT(__n)   \
-   ((u32)(BIT(__n) +   \
-  BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
-((__n) < 0 || (__n) > 31
-
-/**
- * REG_BIT8() - Prepare a u8 bit value
- * @__n: 0-based bit number
- *
- * Local wrapper for BIT() to force u8, with compile time checks.
- *
- * @return: Value with bit @__n set.
- */
-#define REG_BIT8(__n)   \
-   ((u8)(BIT(__n) +\
-  BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
-((__n) < 0 || (__n) > 7
-
-/**
- * REG_GENMASK() - Prepare a continuous u32 bitmask
- * @__high: 0-based high bit
- * @__low: 0-based low bit
- *
- * Local wrapper for GENMASK() to force u32, with compile time checks.
- *
- * @return: Continuous bitmask from @__high to @__low, inclusive.
- */
-#define REG_GENMASK(__high, __low) \
-   ((u32)(GENMASK(__high, __low) + \
-  BUILD_BUG_ON_ZERO(__is_constexpr(__high) &&  \
-__is_constexpr(__low) &&   \
-((__low) < 0 || (__high) > 31 || (__low) > 
(__high)
-
-/**
- * REG_GENMASK64() - Prepare a continuous u64 bitmask
- * @__high: 0-based high bit
- * @__low: 0-based low bit
- *
- * Local wrapper for GENMASK_ULL() to force u64, with compile time checks.
- *
- * @return: Continuous bitmask from @__high to @__low, inclusive.
+/*
+ * Wrappers over the generic BIT_* and GENMASK_* implementations,
+ * for compatibility reasons with previous implementation
  */
-#define REG_GENMASK64(__high, __low)   \
-   ((u64)(GENMASK_ULL(__high, __low) + \
-  BUILD_BUG_ON_ZERO(__is_constexpr(__high) &&  \
-__is_constexpr(__low) &&   \
-((__low) < 0 || (__high) > 63 || (__low) > 
(__high)
+#define REG_GENMASK(high, low) GENMASK_U32(high, low)
+#define REG_GENMASK64(high, low)   GENMASK_U64(high, low)
+#define REG_GENMASK16(high, low)   GENMASK_U16(high, low)
+#define REG_GENMASK8(high, low)GENMASK_U8(high, low)
 
-/**
- * REG_GENMASK8() - Prepare a continuous u8 bitmask
- * @__high: 0-based high bit
- * @__low: 0-based low bit
- *
- * Local wrapper for GENMASK() to force u8, with compile time checks.
- *
- * @return: Continuous bitmask from @__high to @__low, inclusive.
- */
-#define REG_GENMASK8(__high, __low) \
-   ((u8)(GENMASK(__high, __low) +  \
-  BUILD_BUG_ON_ZERO(__is_constexpr(__high) &&  \
-__is_constexpr(__low) &&   \
-((__low) < 0 || (__high) > 7 || (__low) > 
(__high)
+#define REG_BIT(n) BIT_U32(n)
+#define REG_BIT64(n)   BIT_U64(n)
+#define REG_BIT16(n)   BIT_U16(n)
+#define REG_BIT8(n)BIT_U8(n)
 
 /*
  * Local integer constant expression version of is_power_of_2().
@@ -143,35 +86,6 @@
  */
 #define REG_FIELD_GET64(__mask, __val) ((u64)FIELD_GET(__mask, __val))
 
-/**
- * REG_BIT16() - Prepare a u16 bit value
- * @__n: 0-based bit number
- *
- * Local wrapper for BIT() to force u16, with compile time
- * checks.
- *
- * @return: Value with bit @__n set.
- */
-#define REG_BIT16(__n)

Re: [PATCH v5 1/7] bits: split the definition of the asm and non-asm GENMASK()

On Thu, Mar 06, 2025 at 08:29:52PM +0900, Vincent Mailhol via B4 Relay wrote:
> From: Vincent Mailhol 
> 
> In an upcoming change, GENMASK() and its friends will indirectly
> depend on sizeof() which is not available in asm.
> 
> Instead of adding further complexity to __GENMASK() to make it work
> for both asm and non asm, just split the definition of the two
> variants.

...

> -/*
> - * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
> - * disable the input check if that is the case.
> - */

I believe this comment is still valid...

> +#else /* defined(__ASSEMBLY__) */


...here.

Otherwise justify its removal in the commit message.

> +#define GENMASK(h, l)__GENMASK(h, l)
> +#define GENMASK_ULL(h, l)__GENMASK_ULL(h, l)
> +
> +#endif /* !defined(__ASSEMBLY__) */

-- 
With Best Regards,
Andy Shevchenko

[PULL] drm-misc-next

2025-03-06 Thread Thomas Zimmermann

Hi Dave, Sima,

here's the PR for drm-misc-next. It includes two new drivers for Apple
Touch Bar displays. There are new helpers for TTM's shrinker, which xe
now uses. There are a number of changes to the rockchip driver.

There are also cross-subsystem changes for drivers/base and fbdev. The
latter is actually a change required for memory management.

Best regards
Thomas

drm-misc-next-2025-03-06:
drm-misc-next for v6.15:

Cross-subsystem Changes:

base:
- component: Provide helper to query bound status

fbdev:
- fbtft: Remove access to page->index

Core Changes:

- Fix usage of logging macros in several places

gem:
- Add test function for imported dma-bufs and use it in core and helpers
- Avoid struct drm_gem_object.import_attach

tests:
- Fix lockdep warnings

ttm:
- Add helpers for TTM shrinker

Driver Changes:

adp:
- Add support for Apple Touch Bar displays on M1/M2

amdxdna:
- Fix interrupt handling

appletbdrm:
- Add support for Apple Touch Bar displays on x86

bridge:
- synopsys: Add HDMI audio support
- ti-sn65dsi83: Support negative DE polarity

ipu-v3:
- Remove unused code

nouveau:
- Avoid multiple -Wflex-array-member-not-at-end warnings

panthor:
- Fix CS_STATUS_ defines
- Improve locking

rockchip:
- analogix_dp: Add eDP support
- lvds: Improve logging
- vop2: Improve HDMI mode handling; Add support for RK3576
- Fix shutdown
- Support rk3562-mali

xe:
- Use TTM shrinker
The following changes since commit 7cb3274341bfa5977f3c90503b632986a82705fa:

  drm/panel: Add Visionox RM692E5 panel driver (2025-02-27 09:43:38 +0100)

are available in the Git repository at:

  https://gitlab.freedesktop.org/drm/misc/kernel.git 
tags/drm-misc-next-2025-03-06

for you to fetch changes up to 4423e607ff50157aaf088854b145936cbab4d560:

  drm/gma500: fix inconsistent indenting warning (2025-03-06 10:29:31 +0100)


drm-misc-next for v6.15:

Cross-subsystem Changes:

base:
- component: Provide helper to query bound status

fbdev:
- fbtft: Remove access to page->index

Core Changes:

- Fix usage of logging macros in several places

gem:
- Add test function for imported dma-bufs and use it in core and helpers
- Avoid struct drm_gem_object.import_attach

tests:
- Fix lockdep warnings

ttm:
- Add helpers for TTM shrinker

Driver Changes:

adp:
- Add support for Apple Touch Bar displays on M1/M2

amdxdna:
- Fix interrupt handling

appletbdrm:
- Add support for Apple Touch Bar displays on x86

bridge:
- synopsys: Add HDMI audio support
- ti-sn65dsi83: Support negative DE polarity

ipu-v3:
- Remove unused code

nouveau:
- Avoid multiple -Wflex-array-member-not-at-end warnings

panthor:
- Fix CS_STATUS_ defines
- Improve locking

rockchip:
- analogix_dp: Add eDP support
- lvds: Improve logging
- vop2: Improve HDMI mode handling; Add support for RK3576
- Fix shutdown
- Support rk3562-mali

xe:
- Use TTM shrinker


Adrián Larumbe (2):
  drm/panthor: Replace sleep locks with spinlocks in fdinfo path
  drm/panthor: Avoid sleep locking in the internal BO size path

Alexander Stein (1):
  drm/bridge: ti-sn65dsi83: Support negative DE polarity

Andy Yan (13):
  drm/rockchip: vop2: use devm_regmap_field_alloc for cluster-regs
  drm/rockchip: vop2: Remove AFBC from TRANSFORM_OFFSET register macro
  drm/rockchip: vop2: Add platform specific callback
  drm/rockchip: vop2: Merge vop2_cluster/esmart_init function
  drm/rockchip: vop2: Support for different layer select configuration 
between VPs
  drm/rockchip: vop2: Introduce vop hardware version
  drm/rockchip: vop2: Register the primary plane and overlay plane 
separately
  drm/rockchip: vop2: Set plane possible crtcs by possible vp mask
  drm/rockchip: vop2: Add uv swap for cluster window
  dt-bindings: display: vop2: describe constraint SoC by SoC
  dt-bindings: display: vop2: Add missing rockchip,grf property for rk3566/8
  dt-bindings: display: vop2: Add rk3576 support
  drm/rockchip: vop2: Add support for rk3576

Arnd Bergmann (1):
  drm/panel: fix Visionox RM692E5 dependencies

Ashley Smith (1):
  drm/panthor: Update CS_STATUS_ defines to correct values

Charles Han (1):
  drm/gma500: fix inconsistent indenting warning

Colin Ian King (1):
  drm/bridge: Fix spelling mistake "gettin" -> "getting"

Cristian Ciocaltea (2):
  drm/rockchip: vop2: Improve display modes handling on RK3588 HDMI1
  drm/rockchip: vop2: Consistently use dev_err_probe()

Damon Ding (2):
  drm/rockchip: analogix_dp: Use formalized struct definition for grf field
  drm/rockchip: analogix_dp: Expand device data to support multiple edp 
display

Dan Carpenter (1):
  drm/vc4: hdmi: Fix some NULL vs IS_ERR() bugs

Dr. David Alan Gilbert (9):
  drm/vboxvideo: Remove unused hgsmi_cursor_position
  gpu: host1x: Remove unused host1x_debug_dump_syncpts
  gpu: ipu-v3: ipu-ic: Re

Re: [PATCH v4 3/8] mailbox: mtk-cmdq: Add driver data to support for MT8196

2025-03-06 Thread AngeloGioacchino Del Regno


Il 06/03/25 12:00, Jason-JH Lin (林睿祥) ha scritto:

[snip]



CPR_GSIZE is the setting for allocating the CPR SRAM size to each
VM.


Would be awesome if you could then clarify the comment that you have
later in
the code here, from...

/* config cpr size for host vm */

to

/* Set the amount of CPR SRAM to allocate to each VM */

...that could be a way of more properly describing what the writel
there is doing.



OK, I'll change it.




The GCE stuff isn't even properly described in datasheets - I do
(probably!)
understand what those are for, but asking people to get years of
experience on
MediaTek to understand what's going on would be a bit rude,
wouldn't
it? :-D



I agree with you :-)
I'll put them in the VM patch and add some brief description for
them.



Thanks, much appreciated!


+
    #define CMDQ_THR_ACTIVE_SLOT_CYCLES 0x3200
    #define CMDQ_THR_ENABLED    0x1
    #define CMDQ_THR_DISABLED   0x0
@@ -87,11 +98,24 @@ struct cmdq {
    struct gce_plat {
    u32 thread_nr;
    u8 shift;
+ dma_addr_t mminfra_offset;


It looks like this is exactly the DRAM's iostart... at least, I
can
see that in the
downstream devicetree that's where it starts.

  memory: memory@8000 {
  device_type = "memory";
  reg = <0 0x8000 0 0x4000>;
  };

It doesn't really look like being a coincidence, but, for the
sake of
asking:
is this just a coincidence? :-)



As the confirmation with the hardware designer in previous reply
mail
for CK:
https://patchwork.kernel.org/project/linux-mediatek/patch/20250218054405.2017918-4-jason-jh@mediatek.com/*26258463



That explanation was simply wonderful.


Since the MMINFRA remap subtracting 2G is done in the hardware
circuit
and cannot be configured by software, the address +2G adjustment is
necessary to implement in the CMDQ driver.

So that might not be a coincidence.
But even if DRAM start address changes, this mminfra_offset is
still
subtracting 2G, so I think it is a better choice to define it as
the
driver data for MT8196.



so, this makes me think the following:

1. The DRAM start address cannot *ever* be less than 2G, because
otherwise the
     MMINFRA HW would have a hole in the usable address range;
     1a. If the start address changes to less than 2G, then also the
IOMMU would
     get limitations, not only the mminfra..!
     2b. This makes it very very very unlikely for the start address
to be changed
     to less than 0x8000

2. If the DRAM start address changes to be ABOVE 2G (so more than
0x8000),
     there would be no point for MMINFRA to start a "config path"
write (or read)
     in the SMMU DRAM block, would it? ;-)



GCE is using IOMMU in MT8196, so all the address put into the GCE
instruction or GCE register for GCE access should be IOVA.

The DRAM start address is 2G(PA=0x8000, IOVA=0x0) currently, so
when GCE want to access the IOVA=0x0, it will need to +2G into the
instruction, then the MMINFRA will see it as data path(IOVA > 2G) and
subtract 2G for that IOVA, so GCE can finally access the IOVA=0x0.

I'm not sure if I've misunderstood what you mean by ABOVE 2G. :-)
If DRAM start address is changed to 3G(PA=0xc000) the IOVA is still
0x0, so GCE still need to + 2G to make MMINFRA go to the data path.

But if you mean PA=0x8000 and IOVA start address is 3G(0xc000),
then MMINFRA will go to the data path without GCE +2G.
However, MMINFRA will -2G when going to the data path and that will
cause GCE access the wrong IOVA.
So GCE still need to +2G no matter IOVA start address is already can
make MMINFRA go to the data path(IOVA > 2G).

We have already complained to our hardware designer that MMINFRA -2G
con not be changed, which will make software operation very
troublesome.
So in the next few generations of SoC will change this MMINFRA -2G to
software configurable. Then we can just make IOVA start address to 2G
without adding the mminfra_offset to the IOVA for GCE.



Okay now I got it, the reality is way worse than I was thinking... eww... :-(


I get it - if the DRAM moves up, MMINFRA is still at 2G because
that's hard baked
into the hardware, but I foresee that it'll be unlikely to see a
platform changing
the DRAM start address arbitrarily, getting out-of-sync with MMINFRA.

I propose to just get the address from the memory node for now, and
to add a nice
comment in the code that explains that "In at least MT8196, the
MMINFRA hardware
subtracts xyz etc etc" (and that explanation from the previous email
is again
wonderful and shall not be lost: either use that in the comment, or
add it to
the commit description, because it's really that good).

Should a new SoC appear in the future requiring an offset from the
DRAM start
address, we will think about how to make that work in the best
possible way: in
that case we could either reference something else to get the right
address or
we can just change this driver to just use the 2G offset

Re: [PATCH v5 2/7] bits: introduce fixed-type genmasks

On Thu, Mar 06, 2025 at 08:29:53PM +0900, Vincent Mailhol via B4 Relay wrote:
> From: Yury Norov 
> 
> Add GENMASK_TYPE() which generalizes __GENMASK() to support different
> types, and implement fixed-types versions of GENMASK() based on it.
> The fixed-type version allows more strict checks to the min/max values
> accepted, which is useful for defining registers like implemented by
> i915 and xe drivers with their REG_GENMASK*() macros.
> 
> The strict checks rely on shift-count-overflow compiler check to fail
> the build if a number outside of the range allowed is passed.
> Example:
> 
>   #define FOO_MASK GENMASK_U32(33, 4)
> 
> will generate a warning like:
> 
>   include/linux/bits.h:51:27: error: right shift count >= width of type 
> [-Werror=shift-count-overflow]
>  51 |   type_max(t) >> (BITS_PER_TYPE(t) - 1 - (h)
> |   ^~

Code LGTM, but just to be sure: you prepared your series using histogram
diff algo, right?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 0/7] bits: Fixed-type GENMASK()/BIT()