Re: [PATCH v2] kselftests: dmabuf-heaps: Ensure the driver name is null-terminated
On Mon, Jul 29, 2024 at 10:46:04AM +0800, Zenghui Yu wrote: > Even if a vgem device is configured in, we will skip the import_vgem_fd() > test almost every time. > > TAP version 13 > 1..11 > # Testing heap: system > # === > # Testing allocation and importing: > ok 1 # SKIP Could not open vgem -1 > > The problem is that we use the DRM_IOCTL_VERSION ioctl to query the driver > version information but leave the name field a non-null-terminated string. > Terminate it properly to actually test against the vgem device. > > While at it, let's check the length of the driver name is exactly 4 bytes > and return early otherwise (in case there is a name like "vgemfoo" that > gets converted to "vgem\0" unexpectedly). > > Signed-off-by: Zenghui Yu > --- > * From v1 [1]: > - Check version.name_len is exactly 4 bytes and return early otherwise > > [1] https://lore.kernel.org/r/20240708134654.1725-1-yuzeng...@huawei.com Thanks for your patch, I'll push it to drm-misc-next-fixes. > P.S., Maybe worth including the kselftests file into "DMA-BUF HEAPS > FRAMEWORK" MAINTAINERS entry? Good idea, want to do the patch for that too? Cheers, Sima > > tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c > b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c > index 5f541522364f..5d0a809dc2df 100644 > --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c > +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c > @@ -29,9 +29,11 @@ static int check_vgem(int fd) > version.name = name; > > ret = ioctl(fd, DRM_IOCTL_VERSION, &version); > - if (ret) > + if (ret || version.name_len != 4) > return 0; > > + name[4] = '\0'; > + > return !strcmp(name, "vgem"); > } > > -- > 2.33.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v2 2/2] drm/panel: simple: add Innolux G070ACE-LH3 LVDS display support
The G070ACE-LH3 is a 7" TFT Color LCD module with WLED backlight.
https://www.data-modul.com/sites/default/files/products/G070ACE-LH3-specification-12058417.pdf
Signed-off-by: Steffen Trumtrar
---
drivers/gpu/drm/panel/panel-simple.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index dcb6d0b6ced06..d3200dd035662 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2509,6 +2509,38 @@ static const struct panel_desc innolux_g070y2_l01 = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
};
+static const struct display_timing innolux_g070ace_lh3_timing = {
+ .pixelclock = { 2520, 2540, 3570 },
+ .hactive = { 800, 800, 800 },
+ .hfront_porch = { 30, 32, 87 },
+ .hback_porch = { 29, 31, 86 },
+ .hsync_len = { 1, 1, 1 },
+ .vactive = { 480, 480, 480 },
+ .vfront_porch = { 4, 5, 65 },
+ .vback_porch = { 3, 4, 65 },
+ .vsync_len = { 1, 1, 1 },
+ .flags = DISPLAY_FLAGS_DE_HIGH,
+};
+
+static const struct panel_desc innolux_g070ace_lh3 = {
+ .timings = &innolux_g070ace_lh3_timing,
+ .num_timings = 1,
+ .bpc = 8,
+ .size = {
+ .width = 152,
+ .height = 91,
+ },
+ .delay = {
+ .prepare = 10,
+ .enable = 450,
+ .disable = 200,
+ .unprepare = 510,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
+ .bus_flags = DRM_BUS_FLAG_DE_HIGH,
+ .connector_type = DRM_MODE_CONNECTOR_LVDS,
+};
+
static const struct drm_display_mode innolux_g070y2_t02_mode = {
.clock = 3,
.hdisplay = 800,
@@ -4599,6 +4631,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "innolux,g070ace-l01",
.data = &innolux_g070ace_l01,
+ }, {
+ .compatible = "innolux,g070ace-lh3",
+ .data = &innolux_g070ace_lh3,
}, {
.compatible = "innolux,g070y2-l01",
.data = &innolux_g070y2_l01,
--
2.45.1
[PATCH v2 1/2] dt-bindings: display: simple: Document support for Innolux G070ACE-LH3
Add Innolux G070ACE-LH3 7" WVGA (800x480) TFT LCD panel compatible string. Signed-off-by: Steffen Trumtrar Acked-by: Conor Dooley --- Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 5067f5c0a2723..e9941a077a20d 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -180,6 +180,8 @@ properties: - innolux,at070tn92 # Innolux G070ACE-L01 7" WVGA (800x480) TFT LCD panel - innolux,g070ace-l01 +# Innolux G070ACE-LH3 7" WVGA (800x480) TFT LCD panel with WLED backlight + - innolux,g070ace-lh3 # Innolux G070Y2-L01 7" WVGA (800x480) TFT LCD panel - innolux,g070y2-l01 # Innolux G070Y2-T02 7" WVGA (800x480) TFT LCD TTL panel -- 2.45.1
[PATCH v2 0/2] drm/panel: simple: add Innolux G070ACE-LH3
This series adds support for the Innolux G070ACE-LH3 to the panel-simple driver and adds the according compatible to the devicetree bindings. Signed-off-by: Steffen Trumtrar --- Changes in v2: - add acked-by - update [vh]blank min/max values - Link to v1: https://lore.kernel.org/r/20240712-b4-v6-10-topic-innolux-v1-0-bb0acf273...@pengutronix.de --- Steffen Trumtrar (2): dt-bindings: display: simple: Document support for Innolux G070ACE-LH3 drm/panel: simple: add Innolux G070ACE-LH3 LVDS display support .../bindings/display/panel/panel-simple.yaml | 2 ++ drivers/gpu/drm/panel/panel-simple.c | 35 ++ 2 files changed, 37 insertions(+) --- base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 change-id: 20240712-b4-v6-10-topic-innolux-3f0ef0fed4b1 Best regards, -- Steffen Trumtrar
[PATCH] drm: Add the missing symbol '.'
From: Shixiong Ou Signed-off-by: Shixiong Ou --- drivers/gpu/drm/drm_probe_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index bb49d552e671..285290067056 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -888,7 +888,7 @@ EXPORT_SYMBOL(drm_kms_helper_is_poll_worker); * disabled. Polling is re-enabled by calling drm_kms_helper_poll_enable(). * * If however, the polling was never initialized, this call will trigger a - * warning and return + * warning and return. * * Note that calls to enable and disable polling must be strictly ordered, which * is automatically the case when they're only call from suspend/resume -- 2.25.1
Re: [PATCH v5 2/2] drm/loongson: Add dummy gpu driver as a subcomponent
… > the driver is loaded, drm/loongson driver still need to wait all of needs to wait on …? … > design. Therefore, add a dummy driver for the GPU, … Is there a need to reconsider the categorisation and usability descriptions another bit for such a software component? Regards, Markus
[PATCH] staging: fbtft: Fix mutex and spinlock without comment warning
Adhere to Linux kernel coding style
Reported by checkpatch:
CHECK: spinlock_t definition without comment
CHECK: mutex definition without comment
Signed-off-by: Riyan Dhiman
---
drivers/staging/fbtft/fbtft.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index f86ed9d470b8..3e00a26a29d5 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -202,6 +202,7 @@ struct fbtft_par {
u8 *buf;
u8 startbyte;
struct fbtft_ops fbtftops;
+ /* Spinlock to ensure thread-safe access to dirty_lines_start and
dirty_lines_end */
spinlock_t dirty_lock;
unsigned int dirty_lines_start;
unsigned int dirty_lines_end;
@@ -218,6 +219,7 @@ struct fbtft_par {
} gpio;
const s16 *init_sequence;
struct {
+ /* Mutex to synchronize access to gamma curve locking */
struct mutex lock;
u32 *curves;
int num_values;
--
2.39.2
Re: [REGRESSION] No image on 4k display port displays connected through usb-c dock in kernel 6.10
> [adding a few people and lists to the recipients]
>
> Hi! Thx for your rpeort.
>
> On 27.07.24 18:07, ke...@holm.dev wrote:
>
> >
> > Connecting two 4k displays with display port through a lenovo usb-c
> >
> > dock (type 40AS) to a Lenovo P14s Gen 2 (type 21A0) results in no
> >
> > image on the connected displays.
> >
> >
> >
> > The CPU in the Lenovo P14s is a 'AMD Ryzen 7 PRO 5850U with Radeon
> >
> > Graphics' and it has no discrete GPU.
> >
> >
> >
> > I first noticed the issue with kernel version '6.10.0-arch1-2'
> >
> > provided by arch linux. With the previous kernel version
> >
> > '6.9.10.arch1-1' both connected displays worked normally. I reported
> >
> > the issue in the arch forums at
> >
> > https://bbs.archlinux.org/viewtopic.php?id=297999 and was guided to
> >
> > do a bisection to find the commit that caused the problem. Through
> >
> > testing I identified that the issue is not present in the latest
> >
> > kernel directly compiled from the trovalds/linux git repository.
> >
> >
> >
> > With git bisect I identified 4df96ba66760345471a85ef7bb29e1cd4e956057
> >
>
> That's 4df96ba6676034 ("drm/amd/display: Add timing pixel encoding for
>
> mst mode validation") [v6.10-rc1] from Hersen Wu.
>
> Did you try if reverting that commit is possible and might fix the problem?
Reverting is not easily possible:
$ git checkout v6.10
[...]
HEAD is now at 0c3836482481 Linux 6.10
$ git revert 4df96ba66760345471a85ef7bb29e1cd4e956057
Auto-merging drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
CONFLICT (content): Merge conflict in
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
error: could not revert 4df96ba66760... drm/amd/display: Add timing pixel
encoding for mst mode validation
I do not know enough to try and solve the conflict myself without breaking more
things.
>
> >
> > as the first bad commit and fa57924c76d995e87ca3533ec60d1d5e55769a27
> >
>
> That's fa57924c76d995 ("drm/amd/display: Refactor function
>
> dm_dp_mst_is_port_support_mode()") [v6.10-post] from Wayne Lin.
>
> >
> > as the first commit that fixed the problem again.
> >
>
> Hmm, the latter commit does not have a fixes tag and might or might not
>
> be to invasive to backport to 6.10. Let's see what the AMD developers say.
>
> >
> > The initial commit only still shows an image on one of the connected
> >
> > 4k screens. I have not investigated further to find out at what point
> >
> > both displays stopped showing an image.
> >
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>
> --
>
> Everything you wanna know about Linux kernel regression tracking:
>
> https://linux-regtracking.leemhuis.info/about/#tldr
>
> If I did something stupid, please tell me, as explained on that page.
>
Re: [PATCH v3] rockchip/drm: vop2: add support for gamma LUT
On Friday, July 26th, 2024 at 1:31 PM, Daniel Stone
wrote:
> Hi Piotr,
Hi Daniel, sorry for delayed response.
>
> > static void vop2_dither_setup(struct drm_crtc *crtc, u32 *dsp_ctrl)
> > @@ -2152,6 +2127,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc
> > *crtc,
> >
> > vop2_post_config(crtc);
> >
> > + if (crtc->state->gamma_lut)
> > + vop2_crtc_gamma_set(vop2, crtc, old_state, &dsp_ctrl);
>
>
> I think this call should be unconditional, so that we correctly
> program LUT_DIS if there is no LUT set up during enable.
>
Noted
> > @@ -2599,8 +2575,17 @@ static void vop2_crtc_atomic_begin(struct drm_crtc
> > *crtc,
> > vop2_setup_alpha(vp);
> > vop2_setup_dly_for_windows(vop2);
> >
> > - if (crtc_state->color_mgmt_changed && !crtc_state->active_changed)
> > - vop2_crtc_gamma_set(vop2, crtc, old_crtc_state);
> > + if (crtc_state->color_mgmt_changed && !crtc_state->active_changed) {
> > + u32 dsp_ctrl = vop2_vp_read(vp, RK3568_VP_DSP_CTRL);;
> > +
> > + vop2_lock(vop2);
> > +
> > + vop2_crtc_gamma_set(vop2, crtc, old_crtc_state, &dsp_ctrl);
> > +
> > + vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);
> > + vop2_cfg_done(vp);
> > + vop2_unlock(vop2);
> > + }
>
>
> Calling lock/set/write/done/unlock here seems like an anti-pattern;
> the cfg_done is already written in atomic_flush, so at least that part
> is not necessary.
Right sorry for sending such code. I wanted to demonstrate the idea.
> On platforms like RK3588, it looks like the new LUT can just be
> written directly from atomic_begin without needing to program
> DSP_CTRL, take locks, or synchronise against anything, so that should
> be an easy straight-line path.
>
> On platforms like RK3568, it would probably be better to set
> mode_changed when the colour management configuration changes. That
> will give you a good point to synchronise the cross-VOP dependencies
> (i.e. claim or release the shared LUT when it is being
> enabled/disabled), and also a hint to userspace that it is not going
> to be a seamless transition as the LUT is disabled, programmed, then
> re-enabled.
>
> I think this would end up in a net reduction of LoC as well as a net
> reduction of code weirdness.
Okay so if I understood you correctly you suggest setting mode_changed in
order to trigger full modeset (check->begin->flush->enable) to cleanly
handle the RK356x case and for RK3588 just write the LUT in begin and
don't do anything more.
I tried to do this but couldn't get the thing to work. It seems like
setting the mode_changed manually in atomic_check, messes something up
with the CRTC states. Namely, the retrieved new state in subsequent
atomic_begin call isn't the same state (as a result, flags
color_mgmt_changed and mode_changed are both false when they are checked
in atomic_begin which stops further gamma LUT reconfiguration). Below is
how I reworked this (included only parts which changed).
As already mentioned, in atomic check the mode_changed flag is set, then in
atomic begin gamma LUT is disabled and DSP_LUT_EN bit unset (or gamma LUT
is written directly based on if it's RK356x or not). Then in atomic_flush
video port is selected and gamma LUT is written. Gamma LUT is enabled in
atomic_enable.
Perhaps I'm missing something important, if so please hint what exactly.
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 9873172e3fd3..9c5ee2d85a58 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2051,6 +2092,13 @@ static void vop2_crtc_atomic_enable(struct drm_crtc
*crtc,
clk_set_rate(vp->dclk, clock);
+ /**
+* Enable gamma LUT in atomic enable
+*/
+ if (crtc_state->gamma_lut && (vop2->data->soc_id == 3566 ||
+ vop2->data->soc_id == 3568))
+ dsp_ctrl |= RK3568_VP_DSP_CTRL__DSP_LUT_EN;
+
vop2_post_config(crtc);
vop2_cfg_done(vp);
@@ -2062,6 +2110,39 @@ static void vop2_crtc_atomic_enable(struct drm_crtc
*crtc,
vop2_unlock(vop2);
}
+static int vop2_crtc_atomic_check_gamma(struct vop2_video_port *vp,
+ struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state)
+{
+ struct vop2 *vop2 = vp->vop2;
+ unsigned int len;
+
+ if (!vp->vop2->lut_regs || !crtc_state->color_mgmt_changed ||
+ !crtc_state->gamma_lut)
+ return 0;
+
+ len = drm_color_lut_size(crtc_state->gamma_lut);
+ if (len != crtc->gamma_size) {
+ DRM_DEBUG_KMS("Invalid LUT size; got %d, expected %d\n",
+ len, crtc->gamma_size);
+ return -EINVAL;
+ }
+
+ if (!crtc_state->mode_changed && (vop2->data->soc_id == 3566 ||
+ vop2->data->soc_id == 3568)) {
+ int ret;
+
+ crtc_state-
panel-simple-dp-aux: "DP AUX done_probing() can't defer" on MT8186 w/ Collabora kernel
Hi, I have a MT8186 "Magneton" Chromebook that I'm trying to boot a kernel based on Collabora's for-kernelci branch [1], using a config from postmarketOS [2] (intended for that), on a Debian sid installation. This sometimes fails to enable the display with: > panel-simple-dp-aux aux-0-0058: DP AUX done_probing() can't defer I know this is a weird out-of-tree combination of things but I've been asked to report this on the mailing list by wens on the #linux-mediatek IRC channel. [1] https://gitlab.collabora.com/google/chromeos-kernel/-/commits/for-kernelci/ [2] https://gitlab.com/postmarketOS/pmaports/-/blob/master/device/testing/linux-postmarketos-mediatek-mt81xx/config-postmarketos-mediatek-mt81xx.aarch64 Serial output from the system starting with dmesg and some more info copied below. 8< [0.00] Booting Linux on physical CPU 0x00 [0x411fd050] [0.00] Linux version 6.10.0-rc4-next-20240620+ (alpernebbi@ALPER-C340) (aarch64-linux-gnu-gcc (Debian 13.2.0-12) 13.2.0, GNU ld (GNU Binutils for Debian) 2.42.90.2024 [0.00] KASLR enabled [0.00] random: crng init done [0.00] Machine model: Google Magneton board [0.00] earlycon: mtk8250 at MMIO32 0x11002000 (options '115200n8') [0.00] printk: legacy bootconsole [mtk8250] enabled [0.00] printk: debug: skip boot console de-registration. [0.00] Reserved memory: created DMA memory pool at 0x5000, size 16 MiB [0.00] OF: reserved mem: initialized node memory@5000, compatible id shared-dma-pool [0.00] OF: reserved mem: 0x5000..0x5109 (17024 KiB) nomap non-reusable memory@5000 [0.00] Reserved memory: created DMA memory pool at 0x6000, size 10 MiB [0.00] OF: reserved mem: initialized node memory@6000, compatible id shared-dma-pool [0.00] OF: reserved mem: 0x6000..0x609f (10240 KiB) nomap non-reusable memory@6000 [0.00] Reserved memory: created DMA memory pool at 0x6100, size 1 MiB [0.00] OF: reserved mem: initialized node memory@6100, compatible id shared-dma-pool [0.00] OF: reserved mem: 0x6100..0x610f (1024 KiB) nomap non-reusable memory@6100 [0.00] OF: reserved mem: 0xffec5000..0xfffc4fff (1024 KiB) map non-reusable ramoops [0.00] NUMA: No NUMA configuration found [0.00] NUMA: Faking a node at [mem 0x4000-0x00013fff] [0.00] NUMA: NODE_DATA [mem 0x13f7ac340-0x13f7aefff] [0.00] Zone ranges: [0.00] DMA [mem 0x4000-0x] [0.00] DMA32empty [0.00] Normal [mem 0x0001-0x00013fff] [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x4000-0x4fff] [0.00] node 0: [mem 0x5000-0x5109] [0.00] node 0: [mem 0x510a-0x545f] [0.00] node 0: [mem 0x5470-0x5fff] [0.00] node 0: [mem 0x6000-0x609f] [0.00] node 0: [mem 0x60a0-0x60ff] [0.00] node 0: [mem 0x6100-0x610f] [0.00] node 0: [mem 0x6110-0x69ff] [0.00] node 0: [mem 0x6a10-0xffdf] [0.00] node 0: [mem 0x0001-0x00013fff] [0.00] Initmem setup node 0 [mem 0x4000-0x00013fff] [0.00] On node 0, zone DMA: 256 pages in unavailable ranges [0.00] On node 0, zone DMA: 256 pages in unavailable ranges [0.00] On node 0, zone Normal: 512 pages in unavailable ranges [0.00] cma: Reserved 32 MiB at 0xfde0 on node -1 [0.00] psci: probing for conduit method from DT. [0.00] psci: PSCIv1.1 detected in firmware. [0.00] psci: Using standard PSCI v0.2 function IDs [0.00] psci: MIGRATE_INFO_TYPE not supported. [0.00] psci: SMC Calling Convention v1.2 [0.00] percpu: Embedded 34 pages/cpu s98648 r8192 d32424 u139264 [0.00] Detected VIPT I-cache on CPU0 [0.00] CPU features: detected: GIC system register CPU interface [0.00] CPU features: detected: Virtualization Host Extensions [0.00] CPU features: kernel page table isolation forced ON by KASLR [0.00] CPU features: detected: Kernel page table isolation (KPTI) [0.00] CPU features: detected: Qualcomm erratum 1009, or ARM erratum 1286807, 2441009 [0.00] CPU features: detected: ARM errata 1165522, 1319367, or 1530923 [0.00] alternatives: applying boot alternatives [0.00] Kernel command line: cros_secure kern_guid=62abee0c-620d-4f78-b09a-7ea2441e553d earlycon keep
Re: [PATCH v3] rockchip/drm: vop2: add support for gamma LUT
On Friday, July 26th, 2024 at 10:55 AM, Andy Yan
wrote:
> Hi Piotr
Hi Andy!
> Thanks for your work.
>
> On 7/25/24 06:05, Piotr Zalewski wrote:
>
> > Add support for gamma LUT in VOP2 driver. The implementation is based on
> > the one found in VOP driver and modified to be compatible with VOP2. Blue
> > and red channels in gamma LUT register write were swapped with respect to
> > how gamma LUT values are written in VOP. Write of the current video port id
> > to VOP2_SYS_LUT_PORT_SEL register was added before the write of DSP_LUT_EN
> > bit. Gamma size is set and drm color management is enabled for each video
> > port's CRTC except ones which have no associated device. Tested on RK3566
> > (Pinetab2).
> >
> > Helped-by: Dragan Simic dsi...@manjaro.org
> > Signed-off-by: Piotr Zalewski pz010001011...@proton.me
> > ---
> >
> > Notes:
> > Changes in v3:
> > - v3 is patch v2 "resend", by mistake the incremental patch were
> > sent in v2
> >
> > Changes in v2:
> > - Apply code styling corrections [1]
> > - Move gamma LUT write inside the vop2 lock
> >
> > Link to v2:
> > https://lore.kernel.org/linux-rockchip/Hk03HDb6wSSHWtEFZHUye06HR0-9YzP5nCHx9A8_kHzWSZawDrU1o1pjEGkCOJFoRg0nTB4BWEv6V0XBOjF4-0Mj44lp2TrjaQfnytzp-Pk=@proton.me/T/#u
> > Link to v1:
> > https://lore.kernel.org/linux-rockchip/9736eadf6a9d8e97eef919c6b3d88...@manjaro.org/T/#t
> >
> > [1]
> > https://lore.kernel.org/linux-rockchip/d019761504b540600d9fc7a585d6f...@manjaro.org/
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > index 9873172e3fd3..37fcf544a5fd 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > @@ -278,6 +278,15 @@ static u32 vop2_readl(struct vop2 *vop2, u32 offset)
> > return val;
> > }
> >
> > +static u32 vop2_vp_read(struct vop2_video_port *vp, u32 offset)
> > +{
> > + u32 val;
> > +
> > + regmap_read(vp->vop2->map, vp->data->offset + offset, &val);
> > +
> > + return val;
> > +}
> > +
> > static void vop2_win_write(const struct vop2_win *win, unsigned int reg,
> > u32 v)
> > {
> > regmap_field_write(win->reg[reg], v);
> > @@ -1482,6 +1491,95 @@ static bool vop2_crtc_mode_fixup(struct drm_crtc
> > *crtc,
> > return true;
> > }
> >
> > +static bool vop2_vp_dsp_lut_is_enabled(struct vop2_video_port *vp)
> > +{
> > + return (u32) (vop2_vp_read(vp, RK3568_VP_DSP_CTRL) &
> > RK3568_VP_DSP_CTRL__DSP_LUT_EN) >
> > + 0;
> > +}
> > +
> > +static void vop2_vp_dsp_lut_enable(struct vop2_video_port *vp)
> > +{
> > + u32 dsp_ctrl = vop2_vp_read(vp, RK3568_VP_DSP_CTRL);
> > +
> > + dsp_ctrl |= RK3568_VP_DSP_CTRL__DSP_LUT_EN;
> > + vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);
> > +}
> > +
> > +static void vop2_vp_dsp_lut_disable(struct vop2_video_port *vp)
> > +{
> > + u32 dsp_ctrl = vop2_vp_read(vp, RK3568_VP_DSP_CTRL);
> > +
> > + dsp_ctrl &= ~RK3568_VP_DSP_CTRL__DSP_LUT_EN;
> > + vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);
> > +}
> > +
> > +static void vop2_crtc_write_gamma_lut(struct vop2 *vop2, struct drm_crtc
> > *crtc)
> > +{
> > + const struct vop2_video_port *vp = to_vop2_video_port(crtc);
> > + const struct vop2_video_port_data *vp_data = &vop2->data->vp[vp->id];
> > +
> > + struct drm_color_lut *lut = crtc->state->gamma_lut->data;
> > + unsigned int i, bpc = ilog2(vp_data->gamma_lut_len);
> > + u32 word;
> > +
> > + for (i = 0; i < crtc->gamma_size; i++) {
> > + word = (drm_color_lut_extract(lut[i].blue, bpc) << (2 * bpc)) |
> > + (drm_color_lut_extract(lut[i].green, bpc) << bpc) |
> > + drm_color_lut_extract(lut[i].red, bpc);
> > +
> > + writel(word, vop2->lut_regs + i * 4);
> > + }
> > +}
> > +
> > +static void vop2_crtc_gamma_set(struct vop2 *vop2, struct drm_crtc *crtc,
> > + struct drm_crtc_state *old_state)
> > +{
> > + struct drm_crtc_state *state = crtc->state;
> > + struct vop2_video_port *vp = to_vop2_video_port(crtc);
> > + u32 dsp_ctrl;
> > + int ret;
> > +
> > + if (!vop2->lut_regs)
> > + return;
> > +
> > + if (!state->gamma_lut) {
>
>
> What's the purpose of checking !state->gamma_lut here,
>
> and you check it again at the end for return.
> This makes me very confused.
I understood it this way - since the vop2 lock is unlocked after disabling
gamma LUT, the CRTC state can change while waiting for DSP_LUT_EN bit to
be unset. With the change I sent in response to Daniel's reply, gamma LUT
state modification should now be fully atomic so there shouldn't be a need
for the second state check there anymore (if my logic is incorrect please
explain).
> > + /*
> > + * To disable gamma (gamma_lut is null) or to write
> > + * an update to the LUT, clear dsp_lut_en.
> > + /
> > + vop2_lock(vop2);
> > +
> > + vop2_vp_dsp_lut_disable(vp);
> > +
> > + vop2_cfg_done(vp);
> > + vop2_unlock(vop2);
> > + /
> > + * In order to write the LUT to the internal memory,
> > + * we need to first make sure the dsp_lut_en bit is cleared.
> > + */
> > + ret = readx_poll_t
Re: [PATCH v5 1/2] drm/loongson: Introduce component framework support
…
> +++ b/drivers/gpu/drm/loongson/loongson_drv.c
> @@ -0,0 +1,298 @@
…
> +static int loongson_drm_driver_probe(struct platform_device *pdev)
> +{
…
> + dev_info(&pdev->dev, "probed\n");
…
> +}
…
Do you find such information really relevant?
Regards,
Markus
Re: [PATCH v2] kselftests: dmabuf-heaps: Ensure the driver name is null-terminated
On 2024/7/29 15:01, Daniel Vetter wrote: On Mon, Jul 29, 2024 at 10:46:04AM +0800, Zenghui Yu wrote: > Even if a vgem device is configured in, we will skip the import_vgem_fd() > test almost every time. > > TAP version 13 > 1..11 > # Testing heap: system > # === > # Testing allocation and importing: > ok 1 # SKIP Could not open vgem -1 > > The problem is that we use the DRM_IOCTL_VERSION ioctl to query the driver > version information but leave the name field a non-null-terminated string. > Terminate it properly to actually test against the vgem device. > > While at it, let's check the length of the driver name is exactly 4 bytes > and return early otherwise (in case there is a name like "vgemfoo" that > gets converted to "vgem\0" unexpectedly). > > Signed-off-by: Zenghui Yu > --- > * From v1 [1]: > - Check version.name_len is exactly 4 bytes and return early otherwise > > [1] https://lore.kernel.org/r/20240708134654.1725-1-yuzeng...@huawei.com Thanks for your patch, I'll push it to drm-misc-next-fixes. > P.S., Maybe worth including the kselftests file into "DMA-BUF HEAPS > FRAMEWORK" MAINTAINERS entry? Good idea, want to do the patch for that too? Sure, I can send a patch for that today. Thanks, Zenghui
Re: [PATCH v4 2/6] drm/gma500: Make I2C terminology more inclusive
I merged this patch into drm-misc-next.
Am 11.07.24 um 07:27 schrieb Easwar Hariharan:
I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced "master/slave"
with more appropriate terms. Inspired by Wolfram's series to fix drivers/i2c/,
fix the terminology for users of I2C_ALGOBIT bitbanging interface, now that
the approved verbiage exists in the specification.
Acked-by: Thomas Zimmermann
Signed-off-by: Easwar Hariharan
---
drivers/gpu/drm/gma500/cdv_intel_lvds.c | 2 +-
drivers/gpu/drm/gma500/intel_bios.c | 22 ++---
drivers/gpu/drm/gma500/intel_bios.h | 4 ++--
drivers/gpu/drm/gma500/intel_gmbus.c| 2 +-
drivers/gpu/drm/gma500/psb_drv.h| 2 +-
drivers/gpu/drm/gma500/psb_intel_drv.h | 2 +-
drivers/gpu/drm/gma500/psb_intel_lvds.c | 4 ++--
drivers/gpu/drm/gma500/psb_intel_sdvo.c | 26 -
8 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index f08a6803dc184..c7652a02b42ec 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -565,7 +565,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
dev->dev, "I2C bus registration failed.\n");
goto err_encoder_cleanup;
}
- gma_encoder->i2c_bus->slave_addr = 0x2C;
+ gma_encoder->i2c_bus->target_addr = 0x2C;
dev_priv->lvds_i2c_bus = gma_encoder->i2c_bus;
/*
diff --git a/drivers/gpu/drm/gma500/intel_bios.c
b/drivers/gpu/drm/gma500/intel_bios.c
index 8245b5603d2c0..d5924ca3ed050 100644
--- a/drivers/gpu/drm/gma500/intel_bios.c
+++ b/drivers/gpu/drm/gma500/intel_bios.c
@@ -14,8 +14,8 @@
#include "psb_intel_drv.h"
#include "psb_intel_reg.h"
-#define SLAVE_ADDR1 0x70
-#defineSLAVE_ADDR2 0x72
+#defineTARGET_ADDR10x70
+#defineTARGET_ADDR20x72
static void *find_section(struct bdb_header *bdb, int section_id)
{
@@ -357,10 +357,10 @@ parse_sdvo_device_mapping(struct drm_psb_private
*dev_priv,
/* skip the device block if device type is invalid */
continue;
}
- if (p_child->slave_addr != SLAVE_ADDR1 &&
- p_child->slave_addr != SLAVE_ADDR2) {
+ if (p_child->target_addr != TARGET_ADDR1 &&
+ p_child->target_addr != TARGET_ADDR2) {
/*
-* If the slave address is neither 0x70 nor 0x72,
+* If the target address is neither 0x70 nor 0x72,
* it is not a SDVO device. Skip it.
*/
continue;
@@ -371,22 +371,22 @@ parse_sdvo_device_mapping(struct drm_psb_private
*dev_priv,
DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
continue;
}
- DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
+ DRM_DEBUG_KMS("the SDVO device with target addr %2x is found on"
" %s port\n",
- p_child->slave_addr,
+ p_child->target_addr,
(p_child->dvo_port == DEVICE_PORT_DVOB) ?
"SDVOB" : "SDVOC");
p_mapping = &(dev_priv->sdvo_mappings[p_child->dvo_port - 1]);
if (!p_mapping->initialized) {
p_mapping->dvo_port = p_child->dvo_port;
- p_mapping->slave_addr = p_child->slave_addr;
+ p_mapping->target_addr = p_child->target_addr;
p_mapping->dvo_wiring = p_child->dvo_wiring;
p_mapping->ddc_pin = p_child->ddc_pin;
p_mapping->i2c_pin = p_child->i2c_pin;
p_mapping->initialized = 1;
DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d,
ddc_pin=%d, i2c_pin=%d\n",
p_mapping->dvo_port,
- p_mapping->slave_addr,
+ p_mapping->target_addr,
p_mapping->dvo_wiring,
p_mapping->ddc_pin,
p_mapping->i2c_pin);
@@ -394,10 +394,10 @@ parse_sdvo_device_mapping(struct drm_psb_private
*dev_priv,
DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
"two SDVO device.\n");
}
- if (p_child->slave2_addr) {
+ if (p_child->target2_addr) {
/* Maybe this is a SDVO device with multiple inputs */
/* And the mapping info is not added */
- DRM_DEBUG_KMS("
Re: [PATCH v5 2/2] drm/loongson: Add dummy gpu driver as a subcomponent
… > +++ b/drivers/gpu/drm/loongson/Makefile > @@ -17,6 +17,9 @@ loongson-y := \ > lsdc_probe.o \ > lsdc_ttm.o > > +loongson-y += \ > + loonggpu_pci_drv.o > + > loongson-y += loongson_device.o \ … Why do you propose to adjust the macro contents multiple times here? (Can it be sufficient to specify the desired file dependencies directly?) Regards, Markus
Patch "drm/xe: Use write-back caching mode for system memory on DGFX" has been added to the 6.10-stable tree
This is a note to let you know that I've just added the patch titled
drm/xe: Use write-back caching mode for system memory on DGFX
to the 6.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-xe-use-write-back-caching-mode-for-system-memory-on-dgfx.patch
and it can be found in the queue-6.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 5207c393d3e7dda9aff813d6b3e2264370d241be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20Hellstr=C3=B6m?=
Date: Fri, 5 Jul 2024 15:28:28 +0200
Subject: drm/xe: Use write-back caching mode for system memory on DGFX
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Thomas Hellström
commit 5207c393d3e7dda9aff813d6b3e2264370d241be upstream.
The caching mode for buffer objects with VRAM as a possible
placement was forced to write-combined, regardless of placement.
However, write-combined system memory is expensive to allocate and
even though it is pooled, the pool is expensive to shrink, since
it involves global CPU TLB flushes.
Moreover write-combined system memory from TTM is only reliably
available on x86 and DGFX doesn't have an x86 restriction.
So regardless of the cpu caching mode selected for a bo,
internally use write-back caching mode for system memory on DGFX.
Coherency is maintained, but user-space clients may perceive a
difference in cpu access speeds.
v2:
- Update RB- and Ack tags.
- Rephrase wording in xe_drm.h (Matt Roper)
v3:
- Really rephrase wording.
Signed-off-by: Thomas Hellström
Fixes: 622f709ca629 ("drm/xe/uapi: Add support for CPU caching mode")
Cc: Pallavi Mishra
Cc: Matthew Auld
Cc: dri-devel@lists.freedesktop.org
Cc: Joonas Lahtinen
Cc: Effie Yu
Cc: Matthew Brost
Cc: Maarten Lankhorst
Cc: Jose Souza
Cc: Michal Mrozek
Cc: # v6.8+
Acked-by: Matthew Auld
Acked-by: José Roberto de Souza
Reviewed-by: Rodrigo Vivi
Fixes: 622f709ca629 ("drm/xe/uapi: Add support for CPU caching mode")
Acked-by: Michal Mrozek
Acked-by: Effie Yu #On chat
Link:
https://patchwork.freedesktop.org/patch/msgid/20240705132828.27714-1-thomas.hellst...@linux.intel.com
(cherry picked from commit 01e0cfc994be484ddcb9e121e353e51d8bb837c0)
Signed-off-by: Lucas De Marchi
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/xe/xe_bo.c | 47 +++
drivers/gpu/drm/xe/xe_bo_types.h |3 +-
include/uapi/drm/xe_drm.h|8 +-
3 files changed, 37 insertions(+), 21 deletions(-)
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -317,7 +317,7 @@ static struct ttm_tt *xe_ttm_tt_create(s
struct xe_device *xe = xe_bo_device(bo);
struct xe_ttm_tt *tt;
unsigned long extra_pages;
- enum ttm_caching caching;
+ enum ttm_caching caching = ttm_cached;
int err;
tt = kzalloc(sizeof(*tt), GFP_KERNEL);
@@ -331,26 +331,35 @@ static struct ttm_tt *xe_ttm_tt_create(s
extra_pages = DIV_ROUND_UP(xe_device_ccs_bytes(xe, bo->size),
PAGE_SIZE);
- switch (bo->cpu_caching) {
- case DRM_XE_GEM_CPU_CACHING_WC:
- caching = ttm_write_combined;
- break;
- default:
- caching = ttm_cached;
- break;
- }
-
- WARN_ON((bo->flags & XE_BO_FLAG_USER) && !bo->cpu_caching);
-
/*
-* Display scanout is always non-coherent with the CPU cache.
-*
-* For Xe_LPG and beyond, PPGTT PTE lookups are also non-coherent and
-* require a CPU:WC mapping.
+* DGFX system memory is always WB / ttm_cached, since
+* other caching modes are only supported on x86. DGFX
+* GPU system memory accesses are always coherent with the
+* CPU.
*/
- if ((!bo->cpu_caching && bo->flags & XE_BO_FLAG_SCANOUT) ||
- (xe->info.graphics_verx100 >= 1270 && bo->flags &
XE_BO_FLAG_PAGETABLE))
- caching = ttm_write_combined;
+ if (!IS_DGFX(xe)) {
+ switch (bo->cpu_caching) {
+ case DRM_XE_GEM_CPU_CACHING_WC:
+ caching = ttm_write_combined;
+ break;
+ default:
+ caching = ttm_cached;
+ break;
+ }
+
+ WARN_ON((bo->flags & XE_BO_FLAG_USER) && !bo->cpu_caching);
+
+ /*
+* Display scanout is always non-coherent with the CPU cache.
+*
+* For Xe_LPG and beyond, PPGTT PTE lookups are also
+* non-coherent and require a CPU:WC mapping.
+*/
+ if ((!bo->cpu_caching && bo->flags & XE_BO_FLAG_SCANOUT) ||
+ (xe->info.graphics_verx100 >= 1270 &&
+
[PATCH] MAINTAINERS: Add selftests to DMA-BUF HEAPS FRAMEWORK entry
Include dmabuf-heaps selftests in the correct entry so that updates to it can be sent to the right place. Signed-off-by: Zenghui Yu --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 42decde38320..b7f24c9fb0e2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6660,6 +6660,7 @@ F:drivers/dma-buf/dma-heap.c F: drivers/dma-buf/heaps/* F: include/linux/dma-heap.h F: include/uapi/linux/dma-heap.h +F: tools/testing/selftests/dmabuf-heaps/ DMC FREQUENCY DRIVER FOR SAMSUNG EXYNOS5422 M: Lukasz Luba -- 2.33.0
Re: [PATCH] drm: bridge: adv7511: Accept audio sample widths of 32 bits via I2S
Hi, I submitted the patch below a while ago (two months) but as far as I can make out it has not been included. There was an initial concern from Dmitry Baryshkov which was subsequently addressed but no other objections. On Tue, 28 May 2024, Ricard Wanderlof wrote: > > Even though data is truncated to 24 bits, the I2S interface does > accept 32 bit data (the slot widths according to the data sheet > can be 16 or 32 bits) so let the hw_params callback reflect this, > even if the lowest 8 bits are not used when 32 bits are specified. > > This is normally how 24 bit audio data is handled (i.e. as 32 bit > data, with the LSB:s unused) and this is also reflected in other > bridge drivers which handle audio, for instance sii902x.c and > synopsis/dw-hdmi-i2s-audio.c . > > Signed-off-by: Ricard Wanderlof > --- > drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > index 61f4a38e7d2b..4563f5d8136f 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > @@ -101,11 +101,14 @@ static int adv7511_hdmi_hw_params(struct device *dev, > void *data, > case 20: > len = ADV7511_I2S_SAMPLE_LEN_20; > break; > - case 32: > - if (fmt->bit_fmt != SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE) > - return -EINVAL; > - fallthrough; > case 24: > + case 32: > + /* > + * 32 bits are handled like 24 bits, except that the lowest > + * 8 bits are discarded. In fact, the accepted I2S slot widths > + * are 16 and 32 bits, so the chip is fully compatible with > + * 32 bit data. > + */ > len = ADV7511_I2S_SAMPLE_LEN_24; > break; > default: > -- > 2.30.2 > > I recently discovered that the maintainer for the ADV7511 driver (in the I2C) framework is not included by the get_maintainers script, so perhaps this is the reason? Otherwise, please enlighten me on what I need to do to get this patch accepted! /Ricard -- Ricard Wolf Wanderlof ricardw(at)axis.com Axis Communications AB, Lund, Swedenwww.axis.com Phone +46 46 272 2016 Fax +46 46 13 61 30
RE: [REGRESSION] No image on 4k display port displays connected through usb-c dock in kernel 6.10
[Public]
Hi,
Thanks for the report.
Patch fa57924c76d995 ("drm/amd/display: Refactor function
dm_dp_mst_is_port_support_mode()")
is kind of correcting problems causing by commit:
4df96ba6676034 ("drm/amd/display: Add timing pixel encoding for mst mode
validation")
Sorry if it misses fixes tag and would suggest to backport to fix it. Thanks!
Regards,
Wayne Lin
> -Original Message-
> From: ke...@holm.dev
> Sent: Sunday, July 28, 2024 12:43 AM
> To: Linux regressions mailing list ; Deucher,
> Alexander ; Wu, Hersen
> ; Lin, Wayne
> Cc: regressi...@lists.linux.dev; sta...@vger.kernel.org; LKML ker...@vger.kernel.org>; ML dri-devel ;
> amd-...@lists.freedesktop.org
> Subject: Re: [REGRESSION] No image on 4k display port displays connected
> through usb-c dock in kernel 6.10
>
> > [adding a few people and lists to the recipients]
> >
> > Hi! Thx for your rpeort.
> >
> > On 27.07.24 18:07, ke...@holm.dev wrote:
> >
> > >
> > > Connecting two 4k displays with display port through a lenovo usb-c
> > >
> > > dock (type 40AS) to a Lenovo P14s Gen 2 (type 21A0) results in no
> > >
> > > image on the connected displays.
> > >
> > >
> > >
> > > The CPU in the Lenovo P14s is a 'AMD Ryzen 7 PRO 5850U with Radeon
> > >
> > > Graphics' and it has no discrete GPU.
> > >
> > >
> > >
> > > I first noticed the issue with kernel version '6.10.0-arch1-2'
> > >
> > > provided by arch linux. With the previous kernel version
> > >
> > > '6.9.10.arch1-1' both connected displays worked normally. I reported
> > >
> > > the issue in the arch forums at
> > >
> > > https://bbs.archlinux.org/viewtopic.php?id=297999 and was guided to
> > >
> > > do a bisection to find the commit that caused the problem. Through
> > >
> > > testing I identified that the issue is not present in the latest
> > >
> > > kernel directly compiled from the trovalds/linux git repository.
> > >
> > >
> > >
> > > With git bisect I identified
> 4df96ba66760345471a85ef7bb29e1cd4e956057
> > >
> >
> > That's 4df96ba6676034 ("drm/amd/display: Add timing pixel encoding for
> >
> > mst mode validation") [v6.10-rc1] from Hersen Wu.
> >
> > Did you try if reverting that commit is possible and might fix the problem?
>
> Reverting is not easily possible:
>
> $ git checkout v6.10
> [...]
> HEAD is now at 0c3836482481 Linux 6.10
>
> $ git revert 4df96ba66760345471a85ef7bb29e1cd4e956057
> Auto-merging
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> CONFLICT (content): Merge conflict in
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> error: could not revert 4df96ba66760... drm/amd/display: Add timing pixel
> encoding for mst mode validation
>
> I do not know enough to try and solve the conflict myself without breaking
> more things.
>
> >
> > >
> > > as the first bad commit and
> fa57924c76d995e87ca3533ec60d1d5e55769a27
> > >
> >
> > That's fa57924c76d995 ("drm/amd/display: Refactor function
> >
> > dm_dp_mst_is_port_support_mode()") [v6.10-post] from Wayne Lin.
> >
> > >
> > > as the first commit that fixed the problem again.
> > >
> >
> > Hmm, the latter commit does not have a fixes tag and might or might not
> >
> > be to invasive to backport to 6.10. Let's see what the AMD developers say.
> >
> > >
> > > The initial commit only still shows an image on one of the connected
> > >
> > > 4k screens. I have not investigated further to find out at what point
> > >
> > > both displays stopped showing an image.
> > >
> >
> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> >
> > --
> >
> > Everything you wanna know about Linux kernel regression tracking:
> >
> > https://linux-regtracking.leemhuis.info/about/#tldr
> >
> > If I did something stupid, please tell me, as explained on that page.
> >
Re: [PATCH] fbdev/hpfb: Fix an error handling path in hpfb_dio_probe()
On 7/28/24 20:29, Christophe JAILLET wrote:
If an error occurs after request_mem_region(), a corresponding
release_mem_region() should be called, as already done in the remove
function.
True.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I think we can drop this "Fixes" tag, as it gives no real info.
Signed-off-by: Christophe JAILLET
---
*Not* even compile tested only.
Ok.
I don't know on what architecture it relies on.
HP300 are old HP machines with an m68k CPU.
Not sure if someone still has such a machine :-)
So it is provided as-is
---
drivers/video/fbdev/hpfb.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/hpfb.c b/drivers/video/fbdev/hpfb.c
index 66fac8e5393e..87b8dcdc1cf3 100644
--- a/drivers/video/fbdev/hpfb.c
+++ b/drivers/video/fbdev/hpfb.c
@@ -342,12 +342,17 @@ static int hpfb_dio_probe(struct dio_dev *d, const struct
dio_device_id *ent)
}
printk(KERN_INFO "Topcat found at DIO select code %d "
"(secondary id %02x)\n", d->scode, (d->id >> 8) & 0xff);
- if (hpfb_init_one(paddr, vaddr)) {
- if (d->scode >= DIOII_SCBASE)
- iounmap((void *)vaddr);
This driver hasn't changed in years, and I don't expect we will
have many other changes, so in this case I think simply adding the one line:
+ release_mem_region(d->resource.start, resource_size(&d->resource));
here is sufficient without adding additional jump targets.
I can fix it up here, or please send a new patch.
Helge
- return -ENOMEM;
- }
+ if (hpfb_init_one(paddr, vaddr))
+ goto err_unmap;
+
return 0;
+
+err_unmap:
+ if (d->scode >= DIOII_SCBASE)
+ iounmap((void *)vaddr);
+ release_mem_region(d->resource.start, resource_size(&d->resource));
+
+ return -ENOMEM;
}
static void hpfb_remove_one(struct dio_dev *d)
Re: [REGRESSION] No image on 4k display port displays connected through usb-c dock in kernel 6.10
[+Greg +stable]
On 29.07.24 10:16, Lin, Wayne wrote:
>
> Thanks for the report.
>
> Patch fa57924c76d995 ("drm/amd/display: Refactor function
> dm_dp_mst_is_port_support_mode()")
> is kind of correcting problems causing by commit:
> 4df96ba6676034 ("drm/amd/display: Add timing pixel encoding for mst mode
> validation")
>
> Sorry if it misses fixes tag and would suggest to backport to fix it. Thanks!
Greg, seem it would be wise to pick up fa57924c76d995 for 6.10.y as
well, despite a lack of Fixes or stable tags.
Ciao, Thorsten
>> -Original Message-
>> From: ke...@holm.dev
>> Sent: Sunday, July 28, 2024 12:43 AM
>> To: Linux regressions mailing list ; Deucher,
>> Alexander ; Wu, Hersen
>> ; Lin, Wayne
>> Cc: regressi...@lists.linux.dev; sta...@vger.kernel.org; LKML > ker...@vger.kernel.org>; ML dri-devel ;
>> amd-...@lists.freedesktop.org
>> Subject: Re: [REGRESSION] No image on 4k display port displays connected
>> through usb-c dock in kernel 6.10
>>
>>> [adding a few people and lists to the recipients]
>>>
>>> Hi! Thx for your rpeort.
>>>
>>> On 27.07.24 18:07, ke...@holm.dev wrote:
>>>
Connecting two 4k displays with display port through a lenovo usb-c
dock (type 40AS) to a Lenovo P14s Gen 2 (type 21A0) results in no
image on the connected displays.
The CPU in the Lenovo P14s is a 'AMD Ryzen 7 PRO 5850U with Radeon
Graphics' and it has no discrete GPU.
I first noticed the issue with kernel version '6.10.0-arch1-2'
provided by arch linux. With the previous kernel version
'6.9.10.arch1-1' both connected displays worked normally. I reported
the issue in the arch forums at
https://bbs.archlinux.org/viewtopic.php?id=297999 and was guided to
do a bisection to find the commit that caused the problem. Through
testing I identified that the issue is not present in the latest
kernel directly compiled from the trovalds/linux git repository.
With git bisect I identified
>> 4df96ba66760345471a85ef7bb29e1cd4e956057
>>>
>>> That's 4df96ba6676034 ("drm/amd/display: Add timing pixel encoding for
>>>
>>> mst mode validation") [v6.10-rc1] from Hersen Wu.
>>>
>>> Did you try if reverting that commit is possible and might fix the problem?
>>
>> Reverting is not easily possible:
>>
>> $ git checkout v6.10
>> [...]
>> HEAD is now at 0c3836482481 Linux 6.10
>>
>> $ git revert 4df96ba66760345471a85ef7bb29e1cd4e956057
>> Auto-merging
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> CONFLICT (content): Merge conflict in
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> error: could not revert 4df96ba66760... drm/amd/display: Add timing pixel
>> encoding for mst mode validation
>>
>> I do not know enough to try and solve the conflict myself without breaking
>> more things.
>>
>>>
as the first bad commit and
>> fa57924c76d995e87ca3533ec60d1d5e55769a27
>>>
>>> That's fa57924c76d995 ("drm/amd/display: Refactor function
>>>
>>> dm_dp_mst_is_port_support_mode()") [v6.10-post] from Wayne Lin.
>>>
as the first commit that fixed the problem again.
>>>
>>> Hmm, the latter commit does not have a fixes tag and might or might not
>>>
>>> be to invasive to backport to 6.10. Let's see what the AMD developers say.
>>>
The initial commit only still shows an image on one of the connected
4k screens. I have not investigated further to find out at what point
both displays stopped showing an image.
>>>
>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>>
>>> --
>>>
>>> Everything you wanna know about Linux kernel regression tracking:
>>>
>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>>
>>> If I did something stupid, please tell me, as explained on that page.
>>>
[PATCH v5 00/12] spi: add driver for Intel discrete graphics
Add driver for access to Intel discrete graphics card internal SPI device. Expose device on auxiliary bus by i915 and Xe drivers and provide spi driver to register this device with MTD framework. This is a rewrite of "drm/i915/spi: spi access for discrete graphics" series with connection to the Xe driver and splitting the spi driver part to separate module in spi subsystem. This series intended to be pushed through drm-xe-next. V5: depend solely on AUXILIARY_BUS, not on COMPILE_TEST disable spi driver on virtual function in Xe, no spi device there V4: fix white-spaces add check for discrete graphics missed in i915 intel_spi_fini V3: rebase over drm-xe-next to enable CI run V2: fix review comments fix signatures order depend spi presence in Xe on special flag, as not all new discrete cards have such spi Alexander Usyskin (6): spi: add driver for intel graphics on-die spi device spi: intel-dg: align 64bit read and write spi: intel-dg: wake card on operations drm/i915/spi: add support for access mode drm/xe/spi: add on-die spi device drm/xe/spi: add support for access mode Tomas Winkler (6): spi: intel-dg: implement region enumeration spi: intel-dg: implement spi access functions spi: intel-dg: spi register with mtd spi: intel-dg: implement mtd access handlers drm/i915/spi: add spi device for discrete graphics drm/i915/spi: add intel_spi_region map MAINTAINERS | 7 + drivers/gpu/drm/i915/Makefile | 4 + drivers/gpu/drm/i915/i915_driver.c| 6 + drivers/gpu/drm/i915/i915_drv.h | 4 + drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/spi/intel_spi.c | 101 +++ drivers/gpu/drm/i915/spi/intel_spi.h | 15 + drivers/gpu/drm/xe/Makefile | 1 + drivers/gpu/drm/xe/regs/xe_gsc_regs.h | 4 + drivers/gpu/drm/xe/xe_device.c| 3 + drivers/gpu/drm/xe/xe_device_types.h | 8 + drivers/gpu/drm/xe/xe_heci_gsc.c | 5 +- drivers/gpu/drm/xe/xe_pci.c | 5 + drivers/gpu/drm/xe/xe_spi.c | 113 drivers/gpu/drm/xe/xe_spi.h | 15 + drivers/spi/Kconfig | 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-intel-dg.c| 863 ++ include/linux/intel_dg_spi_aux.h | 27 + 19 files changed, 1190 insertions(+), 4 deletions(-) create mode 100644 drivers/gpu/drm/i915/spi/intel_spi.c create mode 100644 drivers/gpu/drm/i915/spi/intel_spi.h create mode 100644 drivers/gpu/drm/xe/xe_spi.c create mode 100644 drivers/gpu/drm/xe/xe_spi.h create mode 100644 drivers/spi/spi-intel-dg.c create mode 100644 include/linux/intel_dg_spi_aux.h -- 2.34.1
[PATCH v5 02/12] spi: intel-dg: implement region enumeration
From: Tomas Winkler
In intel-dg spi, there is no access to the spi controller,
the information is extracted from the descriptor region.
CC: Rodrigo Vivi
CC: Lucas De Marchi
Signed-off-by: Tomas Winkler
Signed-off-by: Alexander Usyskin
---
drivers/spi/spi-intel-dg.c | 190 +
1 file changed, 190 insertions(+)
diff --git a/drivers/spi/spi-intel-dg.c b/drivers/spi/spi-intel-dg.c
index 4e302957f077..661e5189fa58 100644
--- a/drivers/spi/spi-intel-dg.c
+++ b/drivers/spi/spi-intel-dg.c
@@ -17,14 +17,197 @@ struct intel_dg_spi {
void __iomem *base;
size_t size;
unsigned int nregions;
+ u32 access_map;
struct {
const char *name;
u8 id;
u64 offset;
u64 size;
+ unsigned int is_readable:1;
+ unsigned int is_writable:1;
} regions[];
};
+#define SPI_TRIGGER_REG 0x
+#define SPI_VALSIG_REG0x0010
+#define SPI_ADDRESS_REG 0x0040
+#define SPI_REGION_ID_REG 0x0044
+/*
+ * [15:0]-Erase size = 0x0010 4K 0x0080 32K 0x0100 64K
+ * [23:16]-Reserved
+ * [31:24]-Erase SPI RegionID
+ */
+#define SPI_ERASE_REG 0x0048
+#define SPI_ACCESS_ERROR_REG 0x0070
+#define SPI_ADDRESS_ERROR_REG 0x0074
+
+/* Flash Valid Signature */
+#define SPI_FLVALSIG 0x0FF0A55A
+
+#define SPI_MAP_ADDR_MASK 0x00FF
+#define SPI_MAP_ADDR_SHIFT0x0004
+
+#define REGION_ID_DESCRIPTOR 0
+/* Flash Region Base Address */
+#define FRBA 0x40
+/* Flash Region __n - Flash Descriptor Record */
+#define FLREG(__n) (FRBA + ((__n) * 4))
+/* Flash Map 1 Register */
+#define FLMAP1_REG 0x18
+#define FLMSTR4_OFFSET 0x00C
+
+#define SPI_ACCESS_ERROR_PCIE_MASK 0x7
+
+static inline void spi_set_region_id(struct intel_dg_spi *spi, u8 region)
+{
+ iowrite32((u32)region, spi->base + SPI_REGION_ID_REG);
+}
+
+static inline u32 spi_error(struct intel_dg_spi *spi)
+{
+ u32 reg = ioread32(spi->base + SPI_ACCESS_ERROR_REG) &
+ SPI_ACCESS_ERROR_PCIE_MASK;
+
+ /* reset error bits */
+ if (reg)
+ iowrite32(reg, spi->base + SPI_ACCESS_ERROR_REG);
+
+ return reg;
+}
+
+static inline u32 spi_read32(struct intel_dg_spi *spi, u32 address)
+{
+ void __iomem *base = spi->base;
+
+ iowrite32(address, base + SPI_ADDRESS_REG);
+
+ return ioread32(base + SPI_TRIGGER_REG);
+}
+
+static int spi_get_access_map(struct intel_dg_spi *spi)
+{
+ u32 flmap1;
+ u32 fmba;
+ u32 fmstr4;
+ u32 fmstr4_addr;
+
+ spi_set_region_id(spi, REGION_ID_DESCRIPTOR);
+
+ flmap1 = spi_read32(spi, FLMAP1_REG);
+ if (spi_error(spi))
+ return -EIO;
+ /* Get Flash Master Baser Address (FMBA) */
+ fmba = ((flmap1 & SPI_MAP_ADDR_MASK) << SPI_MAP_ADDR_SHIFT);
+ fmstr4_addr = fmba + FLMSTR4_OFFSET;
+
+ fmstr4 = spi_read32(spi, fmstr4_addr);
+ if (spi_error(spi))
+ return -EIO;
+
+ spi->access_map = fmstr4;
+ return 0;
+}
+
+static bool spi_region_readable(struct intel_dg_spi *spi, u8 region)
+{
+ if (region < 12)
+ return spi->access_map & (1 << (region + 8)); /* [19:8] */
+ else
+ return spi->access_map & (1 << (region - 12)); /* [3:0] */
+}
+
+static bool spi_region_writeable(struct intel_dg_spi *spi, u8 region)
+{
+ if (region < 12)
+ return spi->access_map & (1 << (region + 20)); /* [31:20] */
+ else
+ return spi->access_map & (1 << (region - 8)); /* [7:4] */
+}
+
+static int intel_dg_spi_is_valid(struct intel_dg_spi *spi)
+{
+ u32 is_valid;
+
+ spi_set_region_id(spi, REGION_ID_DESCRIPTOR);
+
+ is_valid = spi_read32(spi, SPI_VALSIG_REG);
+ if (spi_error(spi))
+ return -EIO;
+
+ if (is_valid != SPI_FLVALSIG)
+ return -ENODEV;
+
+ return 0;
+}
+
+static int intel_dg_spi_init(struct intel_dg_spi *spi, struct device *device)
+{
+ int ret;
+ unsigned int i, n;
+
+ /* clean error register, previous errors are ignored */
+ spi_error(spi);
+
+ ret = intel_dg_spi_is_valid(spi);
+ if (ret) {
+ dev_err(device, "The SPI is not valid %d\n", ret);
+ return ret;
+ }
+
+ if (spi_get_access_map(spi))
+ return -EIO;
+
+ for (i = 0, n = 0; i < spi->nregions; i++) {
+ u32 address, base, limit, region;
+ u8 id = spi->regions[i].id;
+
+ address = FLREG(id);
+ region = spi_read32(spi, address);
+
+ base = (region & 0x) << 12;
+ limit = (((region & 0x) >> 16) << 12) | 0xFFF;
+
+ dev_dbg(device, "[%d] %s: region: 0x%08X base: 0x%08x limit:
0x%08x\n",
+ id, spi->regions[i].name, region, base, limit);
+
+ if (
[PATCH v5 01/12] spi: add driver for intel graphics on-die spi device
Add auxiliary driver for intel discrete graphics
on-die spi device.
CC: Rodrigo Vivi
CC: Lucas De Marchi
Signed-off-by: Tomas Winkler
Signed-off-by: Alexander Usyskin
---
MAINTAINERS | 7 ++
drivers/spi/Kconfig | 11 +++
drivers/spi/Makefile | 1 +
drivers/spi/spi-intel-dg.c | 142 +++
include/linux/intel_dg_spi_aux.h | 27 ++
5 files changed, 188 insertions(+)
create mode 100644 drivers/spi/spi-intel-dg.c
create mode 100644 include/linux/intel_dg_spi_aux.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 082483b40fac..90e06701f988 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11003,6 +11003,13 @@ L: linux-ker...@vger.kernel.org
S: Supported
F: arch/x86/include/asm/intel-family.h
+INTEL DISCRETE GRAPHIC SPI FLASH DRIVER
+M: Alexander Usyskin
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/spi/spi-intel-dg.c
+F: include/linux/intel_dg_spi_aux.h
+
INTEL DRM DISPLAY FOR XE AND I915 DRIVERS
M: Jani Nikula
M: Rodrigo Vivi
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index a2c99ff33e0a..3a4ca44d94d2 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -518,6 +518,17 @@ config SPI_INTEL_PLATFORM
To compile this driver as a module, choose M here: the module
will be called spi-intel-platform.
+config SPI_INTEL_DG
+ tristate "Intel Discrete Graphic SPI flash driver"
+ depends on AUXILIARY_BUS
+ depends on MTD
+ help
+ This enables support for Intel Discrete Graphic SPI
+ auxiliary device.
+
+ To compile this driver as a module, choose M here: the module
+ will be called spi-intel-dg.
+
config SPI_JCORE
tristate "J-Core SPI Master"
depends on OF && (SUPERH || COMPILE_TEST)
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index e694254dec04..3c48a086c0e0 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_SPI_INGENIC) += spi-ingenic.o
obj-$(CONFIG_SPI_INTEL)+= spi-intel.o
obj-$(CONFIG_SPI_INTEL_PCI)+= spi-intel-pci.o
obj-$(CONFIG_SPI_INTEL_PLATFORM) += spi-intel-platform.o
+obj-$(CONFIG_SPI_INTEL_DG) += spi-intel-dg.o
obj-$(CONFIG_SPI_LANTIQ_SSC) += spi-lantiq-ssc.o
obj-$(CONFIG_SPI_JCORE)+= spi-jcore.o
obj-$(CONFIG_SPI_LJCA) += spi-ljca.o
diff --git a/drivers/spi/spi-intel-dg.c b/drivers/spi/spi-intel-dg.c
new file mode 100644
index ..4e302957f077
--- /dev/null
+++ b/drivers/spi/spi-intel-dg.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(c) 2019-2024, Intel Corporation. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct intel_dg_spi {
+ struct kref refcnt;
+ void __iomem *base;
+ size_t size;
+ unsigned int nregions;
+ struct {
+ const char *name;
+ u8 id;
+ u64 offset;
+ u64 size;
+ } regions[];
+};
+
+static void intel_dg_spi_release(struct kref *kref)
+{
+ struct intel_dg_spi *spi = container_of(kref, struct intel_dg_spi,
refcnt);
+ int i;
+
+ pr_debug("freeing spi memory\n");
+ for (i = 0; i < spi->nregions; i++)
+ kfree(spi->regions[i].name);
+ kfree(spi);
+}
+
+static int intel_dg_spi_probe(struct auxiliary_device *aux_dev,
+ const struct auxiliary_device_id *aux_dev_id)
+{
+ struct intel_dg_spi_dev *ispi =
auxiliary_dev_to_intel_dg_spi_dev(aux_dev);
+ struct device *device;
+ struct intel_dg_spi *spi;
+ unsigned int nregions;
+ unsigned int i, n;
+ size_t size;
+ char *name;
+ size_t name_size;
+ int ret;
+
+ device = &aux_dev->dev;
+
+ /* count available regions */
+ for (nregions = 0, i = 0; i < INTEL_DG_SPI_REGIONS; i++) {
+ if (ispi->regions[i].name)
+ nregions++;
+ }
+
+ if (!nregions) {
+ dev_err(device, "no regions defined\n");
+ return -ENODEV;
+ }
+
+ size = sizeof(*spi) + sizeof(spi->regions[0]) * nregions;
+ spi = kzalloc(size, GFP_KERNEL);
+ if (!spi)
+ return -ENOMEM;
+
+ kref_init(&spi->refcnt);
+
+ spi->nregions = nregions;
+ for (n = 0, i = 0; i < INTEL_DG_SPI_REGIONS; i++) {
+ if (ispi->regions[i].name) {
+ name_size = strlen(dev_name(&aux_dev->dev)) +
+ strlen(ispi->regions[i].name) + 2; /* for
point */
+ name = kzalloc(name_size, GFP_KERNEL);
+ if (!name)
+ continue;
+ snprintf(name, name_size, "%s.%s",
+
[PATCH v5 03/12] spi: intel-dg: implement spi access functions
From: Tomas Winkler
Implement spi_read(), spi_erase() and spi_write() functions.
CC: Lucas De Marchi
CC: Rodrigo Vivi
Signed-off-by: Tomas Winkler
Signed-off-by: Vitaly Lubart
Signed-off-by: Alexander Usyskin
---
drivers/spi/spi-intel-dg.c | 199 +
1 file changed, 199 insertions(+)
diff --git a/drivers/spi/spi-intel-dg.c b/drivers/spi/spi-intel-dg.c
index 661e5189fa58..863898c8739c 100644
--- a/drivers/spi/spi-intel-dg.c
+++ b/drivers/spi/spi-intel-dg.c
@@ -3,13 +3,16 @@
* Copyright(c) 2019-2024, Intel Corporation. All rights reserved.
*/
+#include
#include
#include
#include
+#include
#include
#include
#include
#include
+#include
#include
struct intel_dg_spi {
@@ -84,6 +87,33 @@ static inline u32 spi_read32(struct intel_dg_spi *spi, u32
address)
return ioread32(base + SPI_TRIGGER_REG);
}
+static inline u64 spi_read64(struct intel_dg_spi *spi, u32 address)
+{
+ void __iomem *base = spi->base;
+
+ iowrite32(address, base + SPI_ADDRESS_REG);
+
+ return readq(base + SPI_TRIGGER_REG);
+}
+
+static void spi_write32(struct intel_dg_spi *spi, u32 address, u32 data)
+{
+ void __iomem *base = spi->base;
+
+ iowrite32(address, base + SPI_ADDRESS_REG);
+
+ iowrite32(data, base + SPI_TRIGGER_REG);
+}
+
+static void spi_write64(struct intel_dg_spi *spi, u32 address, u64 data)
+{
+ void __iomem *base = spi->base;
+
+ iowrite32(address, base + SPI_ADDRESS_REG);
+
+ writeq(data, base + SPI_TRIGGER_REG);
+}
+
static int spi_get_access_map(struct intel_dg_spi *spi)
{
u32 flmap1;
@@ -140,6 +170,175 @@ static int intel_dg_spi_is_valid(struct intel_dg_spi *spi)
return 0;
}
+__maybe_unused
+static unsigned int spi_get_region(const struct intel_dg_spi *spi, loff_t from)
+{
+ unsigned int i;
+
+ for (i = 0; i < spi->nregions; i++) {
+ if ((spi->regions[i].offset + spi->regions[i].size - 1) > from
&&
+ spi->regions[i].offset <= from &&
+ spi->regions[i].size != 0)
+ break;
+ }
+
+ return i;
+}
+
+static ssize_t spi_rewrite_partial(struct intel_dg_spi *spi, loff_t to,
+ loff_t offset, size_t len, const u32
*newdata)
+{
+ u32 data = spi_read32(spi, to);
+
+ if (spi_error(spi))
+ return -EIO;
+
+ memcpy((u8 *)&data + offset, newdata, len);
+
+ spi_write32(spi, to, data);
+ if (spi_error(spi))
+ return -EIO;
+
+ return len;
+}
+
+__maybe_unused
+static ssize_t spi_write(struct intel_dg_spi *spi, u8 region,
+loff_t to, size_t len, const unsigned char *buf)
+{
+ size_t i;
+ size_t len8;
+ size_t len4;
+ size_t to4;
+ size_t to_shift;
+ size_t len_s = len;
+ ssize_t ret;
+
+ spi_set_region_id(spi, region);
+
+ to4 = ALIGN_DOWN(to, sizeof(u32));
+ to_shift = min(sizeof(u32) - ((size_t)to - to4), len);
+ if (to - to4) {
+ ret = spi_rewrite_partial(spi, to4, to - to4, to_shift,
+ (uint32_t *)&buf[0]);
+ if (ret < 0)
+ return ret;
+
+ buf += to_shift;
+ to += to_shift;
+ len_s -= to_shift;
+ }
+
+ len8 = ALIGN_DOWN(len_s, sizeof(u64));
+ for (i = 0; i < len8; i += sizeof(u64)) {
+ u64 data;
+
+ memcpy(&data, &buf[i], sizeof(u64));
+ spi_write64(spi, to + i, data);
+ if (spi_error(spi))
+ return -EIO;
+ }
+
+ len4 = len_s - len8;
+ if (len4 >= sizeof(u32)) {
+ u32 data;
+
+ memcpy(&data, &buf[i], sizeof(u32));
+ spi_write32(spi, to + i, data);
+ if (spi_error(spi))
+ return -EIO;
+ i += sizeof(u32);
+ len4 -= sizeof(u32);
+ }
+
+ if (len4 > 0) {
+ ret = spi_rewrite_partial(spi, to + i, 0, len4,
+ (uint32_t *)&buf[i]);
+ if (ret < 0)
+ return ret;
+ }
+
+ return len;
+}
+
+__maybe_unused
+static ssize_t spi_read(struct intel_dg_spi *spi, u8 region,
+ loff_t from, size_t len, unsigned char *buf)
+{
+ size_t i;
+ size_t len8;
+ size_t len4;
+ size_t from4;
+ size_t from_shift;
+ size_t len_s = len;
+
+ spi_set_region_id(spi, region);
+
+ from4 = ALIGN_DOWN(from, sizeof(u32));
+ from_shift = min(sizeof(u32) - ((size_t)from - from4), len);
+
+ if (from - from4) {
+ u32 data = spi_read32(spi, from4);
+
+ if (spi_error(spi))
+ return -EIO;
+ memcpy(&buf[0], (u8 *)&data + (from - from4), from_shift);
+
[PATCH v5 04/12] spi: intel-dg: spi register with mtd
From: Tomas Winkler
Register the on-die spi device with the mtd subsystem.
Refcount spi object on _get and _put mtd callbacks.
CC: Rodrigo Vivi
CC: Lucas De Marchi
Signed-off-by: Tomas Winkler
Signed-off-by: Alexander Usyskin
---
drivers/spi/spi-intel-dg.c | 111 +
1 file changed, 111 insertions(+)
diff --git a/drivers/spi/spi-intel-dg.c b/drivers/spi/spi-intel-dg.c
index 863898c8739c..a936014f1a76 100644
--- a/drivers/spi/spi-intel-dg.c
+++ b/drivers/spi/spi-intel-dg.c
@@ -10,6 +10,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -17,6 +19,8 @@
struct intel_dg_spi {
struct kref refcnt;
+ struct mtd_info mtd;
+ struct mutex lock; /* region access lock */
void __iomem *base;
size_t size;
unsigned int nregions;
@@ -407,6 +411,23 @@ static int intel_dg_spi_init(struct intel_dg_spi *spi,
struct device *device)
return n;
}
+static int intel_dg_spi_erase(struct mtd_info *mtd, struct erase_info *info)
+{
+ return 0;
+}
+
+static int intel_dg_spi_read(struct mtd_info *mtd, loff_t from, size_t len,
+size_t *retlen, u_char *buf)
+{
+ return 0;
+}
+
+static int intel_dg_spi_write(struct mtd_info *mtd, loff_t to, size_t len,
+ size_t *retlen, const u_char *buf)
+{
+ return 0;
+}
+
static void intel_dg_spi_release(struct kref *kref)
{
struct intel_dg_spi *spi = container_of(kref, struct intel_dg_spi,
refcnt);
@@ -415,9 +436,90 @@ static void intel_dg_spi_release(struct kref *kref)
pr_debug("freeing spi memory\n");
for (i = 0; i < spi->nregions; i++)
kfree(spi->regions[i].name);
+ mutex_destroy(&spi->lock);
kfree(spi);
}
+static int intel_dg_spi_get_device(struct mtd_info *mtd)
+{
+ struct mtd_info *master;
+ struct intel_dg_spi *spi;
+
+ if (!mtd)
+ return -ENODEV;
+
+ master = mtd_get_master(mtd);
+ spi = master->priv;
+ if (WARN_ON(!spi))
+ return -EINVAL;
+ pr_debug("get spi %s %d\n", mtd->name, kref_read(&spi->refcnt));
+ kref_get(&spi->refcnt);
+
+ return 0;
+}
+
+static void intel_dg_spi_put_device(struct mtd_info *mtd)
+{
+ struct mtd_info *master;
+ struct intel_dg_spi *spi;
+
+ if (!mtd)
+ return;
+
+ master = mtd_get_master(mtd);
+ spi = master->priv;
+ if (WARN_ON(!spi))
+ return;
+ pr_debug("put spi %s %d\n", mtd->name, kref_read(&spi->refcnt));
+ kref_put(&spi->refcnt, intel_dg_spi_release);
+}
+
+static int intel_dg_spi_init_mtd(struct intel_dg_spi *spi, struct device
*device,
+unsigned int nparts, bool writeable_override)
+{
+ unsigned int i;
+ unsigned int n;
+ struct mtd_partition *parts = NULL;
+ int ret;
+
+ dev_dbg(device, "registering with mtd\n");
+
+ spi->mtd.owner = THIS_MODULE;
+ spi->mtd.dev.parent = device;
+ spi->mtd.flags = MTD_CAP_NORFLASH | MTD_WRITEABLE;
+ spi->mtd.type = MTD_DATAFLASH;
+ spi->mtd.priv = spi;
+ spi->mtd._write = intel_dg_spi_write;
+ spi->mtd._read = intel_dg_spi_read;
+ spi->mtd._erase = intel_dg_spi_erase;
+ spi->mtd._get_device = intel_dg_spi_get_device;
+ spi->mtd._put_device = intel_dg_spi_put_device;
+ spi->mtd.writesize = SZ_1; /* 1 byte granularity */
+ spi->mtd.erasesize = SZ_4K; /* 4K bytes granularity */
+ spi->mtd.size = spi->size;
+
+ parts = kcalloc(spi->nregions, sizeof(*parts), GFP_KERNEL);
+ if (!parts)
+ return -ENOMEM;
+
+ for (i = 0, n = 0; i < spi->nregions && n < nparts; i++) {
+ if (!spi->regions[i].is_readable)
+ continue;
+ parts[n].name = spi->regions[i].name;
+ parts[n].offset = spi->regions[i].offset;
+ parts[n].size = spi->regions[i].size;
+ if (!spi->regions[i].is_writable && !writeable_override)
+ parts[n].mask_flags = MTD_WRITEABLE;
+ n++;
+ }
+
+ ret = mtd_device_register(&spi->mtd, parts, n);
+
+ kfree(parts);
+
+ return ret;
+}
+
static int intel_dg_spi_probe(struct auxiliary_device *aux_dev,
const struct auxiliary_device_id *aux_dev_id)
{
@@ -449,6 +551,7 @@ static int intel_dg_spi_probe(struct auxiliary_device
*aux_dev,
if (!spi)
return -ENOMEM;
+ mutex_init(&spi->lock);
kref_init(&spi->refcnt);
spi->nregions = nregions;
@@ -481,6 +584,12 @@ static int intel_dg_spi_probe(struct auxiliary_device
*aux_dev,
goto err;
}
+ ret = intel_dg_spi_init_mtd(spi, device, ret, ispi->writeable_override);
+ if (ret) {
+ dev_err(device, "failed init mtd %d\n", ret
[PATCH v5 05/12] spi: intel-dg: implement mtd access handlers
From: Tomas Winkler
Implement mtd read, erase, and write handlers.
For erase operation address and size should be 4K aligned.
For write operation address and size has to be 4bytes aligned.
CC: Rodrigo Vivi
CC: Lucas De Marchi
Signed-off-by: Tomas Winkler
Signed-off-by: Vitaly Lubart
Signed-off-by: Alexander Usyskin
---
drivers/spi/spi-intel-dg.c | 152 +++--
1 file changed, 147 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi-intel-dg.c b/drivers/spi/spi-intel-dg.c
index a936014f1a76..dfb457c43a5d 100644
--- a/drivers/spi/spi-intel-dg.c
+++ b/drivers/spi/spi-intel-dg.c
@@ -174,7 +174,6 @@ static int intel_dg_spi_is_valid(struct intel_dg_spi *spi)
return 0;
}
-__maybe_unused
static unsigned int spi_get_region(const struct intel_dg_spi *spi, loff_t from)
{
unsigned int i;
@@ -206,7 +205,6 @@ static ssize_t spi_rewrite_partial(struct intel_dg_spi
*spi, loff_t to,
return len;
}
-__maybe_unused
static ssize_t spi_write(struct intel_dg_spi *spi, u8 region,
loff_t to, size_t len, const unsigned char *buf)
{
@@ -265,7 +263,6 @@ static ssize_t spi_write(struct intel_dg_spi *spi, u8
region,
return len;
}
-__maybe_unused
static ssize_t spi_read(struct intel_dg_spi *spi, u8 region,
loff_t from, size_t len, unsigned char *buf)
{
@@ -324,7 +321,6 @@ static ssize_t spi_read(struct intel_dg_spi *spi, u8 region,
return len;
}
-__maybe_unused
static ssize_t
spi_erase(struct intel_dg_spi *spi, u8 region, loff_t from, u64 len, u64
*fail_addr)
{
@@ -413,18 +409,164 @@ static int intel_dg_spi_init(struct intel_dg_spi *spi,
struct device *device)
static int intel_dg_spi_erase(struct mtd_info *mtd, struct erase_info *info)
{
- return 0;
+ struct intel_dg_spi *spi;
+ unsigned int idx;
+ u8 region;
+ u64 addr;
+ ssize_t bytes;
+ loff_t from;
+ size_t len;
+ size_t total_len;
+ int ret = 0;
+
+ if (!mtd || !info)
+ return -EINVAL;
+
+ spi = mtd->priv;
+ if (WARN_ON(!spi))
+ return -EINVAL;
+
+ if (!IS_ALIGNED(info->addr, SZ_4K) || !IS_ALIGNED(info->len, SZ_4K)) {
+ dev_err(&mtd->dev, "unaligned erase %llx %llx\n",
+ info->addr, info->len);
+ info->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
+ return -EINVAL;
+ }
+
+ total_len = info->len;
+ addr = info->addr;
+
+ mutex_lock(&spi->lock);
+
+ while (total_len > 0) {
+ if (!IS_ALIGNED(addr, SZ_4K) || !IS_ALIGNED(total_len, SZ_4K)) {
+ dev_err(&mtd->dev, "unaligned erase %llx %zx\n", addr,
total_len);
+ info->fail_addr = addr;
+ ret = -ERANGE;
+ goto out;
+ }
+
+ idx = spi_get_region(spi, addr);
+ if (idx >= spi->nregions) {
+ dev_err(&mtd->dev, "out of range");
+ info->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
+ ret = -ERANGE;
+ goto out;
+ }
+
+ from = addr - spi->regions[idx].offset;
+ region = spi->regions[idx].id;
+ len = total_len;
+ if (len > spi->regions[idx].size - from)
+ len = spi->regions[idx].size - from;
+
+ dev_dbg(&mtd->dev, "erasing region[%d] %s from %llx len %zx\n",
+ region, spi->regions[idx].name, from, len);
+
+ bytes = spi_erase(spi, region, from, len, &info->fail_addr);
+ if (bytes < 0) {
+ dev_dbg(&mtd->dev, "erase failed with %zd\n", bytes);
+ info->fail_addr += spi->regions[idx].offset;
+ ret = bytes;
+ goto out;
+ }
+
+ addr += len;
+ total_len -= len;
+ }
+
+out:
+ mutex_unlock(&spi->lock);
+ return ret;
}
static int intel_dg_spi_read(struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, u_char *buf)
{
+ struct intel_dg_spi *spi;
+ ssize_t ret;
+ unsigned int idx;
+ u8 region;
+
+ if (!mtd)
+ return -EINVAL;
+
+ spi = mtd->priv;
+ if (WARN_ON(!spi))
+ return -EINVAL;
+
+ idx = spi_get_region(spi, from);
+
+ dev_dbg(&mtd->dev, "reading region[%d] %s from %lld len %zd\n",
+ spi->regions[idx].id, spi->regions[idx].name, from, len);
+
+ if (idx >= spi->nregions) {
+ dev_err(&mtd->dev, "out of ragnge");
+ return -ERANGE;
+ }
+
+ from -= spi->regions[idx].offset;
+ region = spi->regions[idx].id;
+ if (len > spi->regions[idx].size - from)
+ len = spi->regions[idx].size - from;
+
[PATCH v5 06/12] spi: intel-dg: align 64bit read and write
GSC SPI HW errors on quad access overlapping 1K border.
Align 64bit read and write to avoid readq/writeq over 1K border.
Signed-off-by: Alexander Usyskin
---
drivers/spi/spi-intel-dg.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/drivers/spi/spi-intel-dg.c b/drivers/spi/spi-intel-dg.c
index dfb457c43a5d..c76b0a70f8d8 100644
--- a/drivers/spi/spi-intel-dg.c
+++ b/drivers/spi/spi-intel-dg.c
@@ -231,6 +231,24 @@ static ssize_t spi_write(struct intel_dg_spi *spi, u8
region,
len_s -= to_shift;
}
+ if (!IS_ALIGNED(to, sizeof(u64)) &&
+ ((to ^ (to + len_s)) & GENMASK(31, 10))) {
+ /*
+* Workaround reads/writes across 1k-aligned addresses
+* (start u32 before 1k, end u32 after)
+* as this fails on hardware.
+*/
+ u32 data;
+
+ memcpy(&data, &buf[0], sizeof(u32));
+ spi_write32(spi, to, data);
+ if (spi_error(spi))
+ return -EIO;
+ buf += sizeof(u32);
+ to += sizeof(u32);
+ len_s -= sizeof(u32);
+ }
+
len8 = ALIGN_DOWN(len_s, sizeof(u64));
for (i = 0; i < len8; i += sizeof(u64)) {
u64 data;
@@ -289,6 +307,23 @@ static ssize_t spi_read(struct intel_dg_spi *spi, u8
region,
from += from_shift;
}
+ if (!IS_ALIGNED(from, sizeof(u64)) &&
+ ((from ^ (from + len_s)) & GENMASK(31, 10))) {
+ /*
+* Workaround reads/writes across 1k-aligned addresses
+* (start u32 before 1k, end u32 after)
+* as this fails on hardware.
+*/
+ u32 data = spi_read32(spi, from);
+
+ if (spi_error(spi))
+ return -EIO;
+ memcpy(&buf[0], &data, sizeof(data));
+ len_s -= sizeof(u32);
+ buf += sizeof(u32);
+ from += sizeof(u32);
+ }
+
len8 = ALIGN_DOWN(len_s, sizeof(u64));
for (i = 0; i < len8; i += sizeof(u64)) {
u64 data = spi_read64(spi, from + i);
--
2.34.1
[PATCH v5 07/12] spi: intel-dg: wake card on operations
Enable runtime PM in spi driver to notify graphics driver that
whole card should be kept awake while spi operations are
performed through this driver.
CC: Lucas De Marchi
Signed-off-by: Alexander Usyskin
---
drivers/spi/spi-intel-dg.c | 44 ++
1 file changed, 44 insertions(+)
diff --git a/drivers/spi/spi-intel-dg.c b/drivers/spi/spi-intel-dg.c
index c76b0a70f8d8..a14fc3190520 100644
--- a/drivers/spi/spi-intel-dg.c
+++ b/drivers/spi/spi-intel-dg.c
@@ -12,11 +12,14 @@
#include
#include
#include
+#include
#include
#include
#include
#include
+#define INTEL_DG_SPI_RPM_TIMEOUT 500
+
struct intel_dg_spi {
struct kref refcnt;
struct mtd_info mtd;
@@ -471,6 +474,12 @@ static int intel_dg_spi_erase(struct mtd_info *mtd, struct
erase_info *info)
total_len = info->len;
addr = info->addr;
+ ret = pm_runtime_resume_and_get(mtd->dev.parent);
+ if (ret < 0) {
+ dev_err(&mtd->dev, "rpm: get failed %d\n", ret);
+ return ret;
+ }
+
mutex_lock(&spi->lock);
while (total_len > 0) {
@@ -512,6 +521,8 @@ static int intel_dg_spi_erase(struct mtd_info *mtd, struct
erase_info *info)
out:
mutex_unlock(&spi->lock);
+ pm_runtime_mark_last_busy(mtd->dev.parent);
+ pm_runtime_put_autosuspend(mtd->dev.parent);
return ret;
}
@@ -545,6 +556,12 @@ static int intel_dg_spi_read(struct mtd_info *mtd, loff_t
from, size_t len,
if (len > spi->regions[idx].size - from)
len = spi->regions[idx].size - from;
+ ret = pm_runtime_resume_and_get(mtd->dev.parent);
+ if (ret < 0) {
+ dev_err(&mtd->dev, "rpm: get failed %zd\n", ret);
+ return ret;
+ }
+
mutex_lock(&spi->lock);
ret = spi_read(spi, region, from, len, buf);
@@ -557,6 +574,8 @@ static int intel_dg_spi_read(struct mtd_info *mtd, loff_t
from, size_t len,
*retlen = ret;
mutex_unlock(&spi->lock);
+ pm_runtime_mark_last_busy(mtd->dev.parent);
+ pm_runtime_put_autosuspend(mtd->dev.parent);
return 0;
}
@@ -590,6 +609,12 @@ static int intel_dg_spi_write(struct mtd_info *mtd, loff_t
to, size_t len,
if (len > spi->regions[idx].size - to)
len = spi->regions[idx].size - to;
+ ret = pm_runtime_resume_and_get(mtd->dev.parent);
+ if (ret < 0) {
+ dev_err(&mtd->dev, "rpm: get failed %zd\n", ret);
+ return ret;
+ }
+
mutex_lock(&spi->lock);
ret = spi_write(spi, region, to, len, buf);
@@ -602,6 +627,8 @@ static int intel_dg_spi_write(struct mtd_info *mtd, loff_t
to, size_t len,
*retlen = ret;
mutex_unlock(&spi->lock);
+ pm_runtime_mark_last_busy(mtd->dev.parent);
+ pm_runtime_put_autosuspend(mtd->dev.parent);
return 0;
}
@@ -747,6 +774,17 @@ static int intel_dg_spi_probe(struct auxiliary_device
*aux_dev,
}
}
+ pm_runtime_enable(device);
+
+ pm_runtime_set_autosuspend_delay(device, INTEL_DG_SPI_RPM_TIMEOUT);
+ pm_runtime_use_autosuspend(device);
+
+ ret = pm_runtime_resume_and_get(device);
+ if (ret < 0) {
+ dev_err(device, "rpm: get failed %d\n", ret);
+ goto err_norpm;
+ }
+
spi->base = devm_ioremap_resource(device, &ispi->bar);
if (IS_ERR(spi->base)) {
dev_err(device, "mmio not mapped\n");
@@ -769,9 +807,13 @@ static int intel_dg_spi_probe(struct auxiliary_device
*aux_dev,
dev_set_drvdata(&aux_dev->dev, spi);
+ pm_runtime_put(device);
return 0;
err:
+ pm_runtime_put(device);
+err_norpm:
+ pm_runtime_disable(device);
kref_put(&spi->refcnt, intel_dg_spi_release);
return ret;
}
@@ -783,6 +825,8 @@ static void intel_dg_spi_remove(struct auxiliary_device
*aux_dev)
if (!spi)
return;
+ pm_runtime_disable(&aux_dev->dev);
+
mtd_device_unregister(&spi->mtd);
dev_set_drvdata(&aux_dev->dev, NULL);
--
2.34.1
[PATCH v5 08/12] drm/i915/spi: add spi device for discrete graphics
From: Tomas Winkler
Enable access to internal spi on DGFX devices via a child device.
The spi child device is exposed via auxiliary bus.
CC: Rodrigo Vivi
CC: Lucas De Marchi
Signed-off-by: Tomas Winkler
Signed-off-by: Alexander Usyskin
---
drivers/gpu/drm/i915/Makefile| 4 ++
drivers/gpu/drm/i915/i915_driver.c | 6 +++
drivers/gpu/drm/i915/i915_drv.h | 4 ++
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/spi/intel_spi.c | 68
drivers/gpu/drm/i915/spi/intel_spi.h | 15 ++
6 files changed, 98 insertions(+)
create mode 100644 drivers/gpu/drm/i915/spi/intel_spi.c
create mode 100644 drivers/gpu/drm/i915/spi/intel_spi.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index c63fa2133ccb..02a9faf049a7 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -211,6 +211,10 @@ i915-y += \
i915-y += \
gt/intel_gsc.o
+# graphics spi device (DGFX) support
+i915-y += \
+ spi/intel_spi.o
+
# graphics hardware monitoring (HWMON) support
i915-$(CONFIG_HWMON) += \
i915_hwmon.o
diff --git a/drivers/gpu/drm/i915/i915_driver.c
b/drivers/gpu/drm/i915/i915_driver.c
index 161b21eff694..49916a586dac 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -81,6 +81,8 @@
#include "soc/intel_dram.h"
#include "soc/intel_gmch.h"
+#include "spi/intel_spi.h"
+
#include "i915_debugfs.h"
#include "i915_driver.h"
#include "i915_drm_client.h"
@@ -619,6 +621,8 @@ static void i915_driver_register(struct drm_i915_private
*dev_priv)
/* Depends on sysfs having been initialized */
i915_perf_register(dev_priv);
+ intel_spi_init(dev_priv);
+
for_each_gt(gt, dev_priv, i)
intel_gt_driver_register(gt);
@@ -662,6 +666,8 @@ static void i915_driver_unregister(struct drm_i915_private
*dev_priv)
i915_hwmon_unregister(dev_priv);
+ intel_spi_fini(dev_priv);
+
i915_perf_unregister(dev_priv);
i915_pmu_unregister(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d1d21d433766..dd48fae4efe0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -34,6 +34,8 @@
#include
+#include
+
#include
#include "display/intel_display_limits.h"
@@ -315,6 +317,8 @@ struct drm_i915_private {
struct i915_perf perf;
+ struct intel_dg_spi_dev spi;
+
struct i915_hwmon *hwmon;
struct intel_gt *gt[I915_MAX_GT];
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8e4478194d11..50dc19c58666 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -323,6 +323,7 @@
#define DG2_GSC_HECI2_BASE 0x00374000
#define MTL_GSC_HECI1_BASE 0x00116000
#define MTL_GSC_HECI2_BASE 0x00117000
+#define GEN12_GUNIT_SPI_BASE 0x00102040
#define HECI_H_CSR(base) _MMIO((base) + 0x4)
#define HECI_H_CSR_IEREG_BIT(0)
diff --git a/drivers/gpu/drm/i915/spi/intel_spi.c
b/drivers/gpu/drm/i915/spi/intel_spi.c
new file mode 100644
index ..4b90e42b0f86
--- /dev/null
+++ b/drivers/gpu/drm/i915/spi/intel_spi.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright(c) 2019-2024, Intel Corporation. All rights reserved.
+ */
+
+#include
+#include
+#include "i915_reg.h"
+#include "i915_drv.h"
+#include "spi/intel_spi.h"
+
+#define GEN12_GUNIT_SPI_SIZE 0x80
+
+static void i915_spi_release_dev(struct device *dev)
+{
+}
+
+void intel_spi_init(struct drm_i915_private *dev_priv)
+{
+ struct intel_dg_spi_dev *spi = &dev_priv->spi;
+ struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
+ struct auxiliary_device *aux_dev = &spi->aux_dev;
+ int ret;
+
+ /* Only the DGFX devices have internal SPI */
+ if (!IS_DGFX(dev_priv))
+ return;
+
+ spi->bar.parent = &pdev->resource[0];
+ spi->bar.start = GEN12_GUNIT_SPI_BASE + pdev->resource[0].start;
+ spi->bar.end = spi->bar.start + GEN12_GUNIT_SPI_SIZE - 1;
+ spi->bar.flags = IORESOURCE_MEM;
+ spi->bar.desc = IORES_DESC_NONE;
+
+ aux_dev->name = "spi";
+ aux_dev->id = (pci_domain_nr(pdev->bus) << 16) |
+ PCI_DEVID(pdev->bus->number, pdev->devfn);
+ aux_dev->dev.parent = &pdev->dev;
+ aux_dev->dev.release = i915_spi_release_dev;
+
+ ret = auxiliary_device_init(aux_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "i915-spi aux init failed %d\n", ret);
+ return;
+ }
+
+ ret = auxiliary_device_add(aux_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "i915-spi aux add failed %d\n", ret);
+ auxiliary_device_uninit(aux_dev);
+ return;
+ }
+}
+
+void intel_spi_fini(struct drm_i915_private *dev_priv)
+{
+ struct intel_dg_spi_dev *spi = &dev_pri
[PATCH v5 09/12] drm/i915/spi: add intel_spi_region map
From: Tomas Winkler
Add the dGFX spi region map and convey it via auxiliary device
to the spi child device.
CC: Rodrigo Vivi
CC: Lucas De Marchi
Signed-off-by: Tomas Winkler
Signed-off-by: Alexander Usyskin
---
drivers/gpu/drm/i915/spi/intel_spi.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/spi/intel_spi.c
b/drivers/gpu/drm/i915/spi/intel_spi.c
index 4b90e42b0f86..200139531d26 100644
--- a/drivers/gpu/drm/i915/spi/intel_spi.c
+++ b/drivers/gpu/drm/i915/spi/intel_spi.c
@@ -11,6 +11,13 @@
#define GEN12_GUNIT_SPI_SIZE 0x80
+static const struct intel_dg_spi_region regions[INTEL_DG_SPI_REGIONS] = {
+ [0] = { .name = "DESCRIPTOR", },
+ [2] = { .name = "GSC", },
+ [11] = { .name = "OptionROM", },
+ [12] = { .name = "DAM", },
+};
+
static void i915_spi_release_dev(struct device *dev)
{
}
@@ -31,6 +38,7 @@ void intel_spi_init(struct drm_i915_private *dev_priv)
spi->bar.end = spi->bar.start + GEN12_GUNIT_SPI_SIZE - 1;
spi->bar.flags = IORESOURCE_MEM;
spi->bar.desc = IORES_DESC_NONE;
+ spi->regions = regions;
aux_dev->name = "spi";
aux_dev->id = (pci_domain_nr(pdev->bus) << 16) |
--
2.34.1
[PATCH v5 10/12] drm/i915/spi: add support for access mode
Check SPI access mode from GSC FW status registers
and overwrite access status read from SPI descriptor, if needed.
Signed-off-by: Alexander Usyskin
---
drivers/gpu/drm/i915/spi/intel_spi.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/spi/intel_spi.c
b/drivers/gpu/drm/i915/spi/intel_spi.c
index 200139531d26..e2b76e5cbc0c 100644
--- a/drivers/gpu/drm/i915/spi/intel_spi.c
+++ b/drivers/gpu/drm/i915/spi/intel_spi.c
@@ -10,6 +10,7 @@
#include "spi/intel_spi.h"
#define GEN12_GUNIT_SPI_SIZE 0x80
+#define HECI_FW_STATUS_2_SPI_ACCESS_MODE BIT(3)
static const struct intel_dg_spi_region regions[INTEL_DG_SPI_REGIONS] = {
[0] = { .name = "DESCRIPTOR", },
@@ -22,6 +23,29 @@ static void i915_spi_release_dev(struct device *dev)
{
}
+static bool i915_spi_writeable_override(struct drm_i915_private *dev_priv)
+{
+ struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
+ resource_size_t base;
+ bool writeable_override;
+
+ if (IS_DG1(dev_priv)) {
+ base = DG1_GSC_HECI2_BASE;
+ } else if (IS_DG2(dev_priv)) {
+ base = DG2_GSC_HECI2_BASE;
+ } else {
+ dev_err(&pdev->dev, "Unknown platform\n");
+ return true;
+ }
+
+ writeable_override =
+ !(intel_uncore_read(&dev_priv->uncore, HECI_FWSTS(base, 2)) &
+ HECI_FW_STATUS_2_SPI_ACCESS_MODE);
+ if (writeable_override)
+ dev_info(&pdev->dev, "SPI access overridden by jumper\n");
+ return writeable_override;
+}
+
void intel_spi_init(struct drm_i915_private *dev_priv)
{
struct intel_dg_spi_dev *spi = &dev_priv->spi;
@@ -33,6 +57,7 @@ void intel_spi_init(struct drm_i915_private *dev_priv)
if (!IS_DGFX(dev_priv))
return;
+ spi->writeable_override = i915_spi_writeable_override(dev_priv);
spi->bar.parent = &pdev->resource[0];
spi->bar.start = GEN12_GUNIT_SPI_BASE + pdev->resource[0].start;
spi->bar.end = spi->bar.start + GEN12_GUNIT_SPI_SIZE - 1;
--
2.34.1
[PATCH v5 11/12] drm/xe/spi: add on-die spi device
Enable access to internal spi on DGFX with GSC/CSC devices
via a child device.
The spi child device is exposed via auxiliary bus.
Signed-off-by: Alexander Usyskin
---
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_device.c | 3 +
drivers/gpu/drm/xe/xe_device_types.h | 8 +++
drivers/gpu/drm/xe/xe_pci.c | 5 ++
drivers/gpu/drm/xe/xe_spi.c | 82
drivers/gpu/drm/xe/xe_spi.h | 15 +
6 files changed, 114 insertions(+)
create mode 100644 drivers/gpu/drm/xe/xe_spi.c
create mode 100644 drivers/gpu/drm/xe/xe_spi.h
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 1ff9602a52f6..f98e26b81035 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -95,6 +95,7 @@ xe-y += xe_bb.o \
xe_ring_ops.o \
xe_sa.o \
xe_sched_job.o \
+ xe_spi.o \
xe_step.o \
xe_sync.o \
xe_tile.o \
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 1aba6f9eaa19..7b7aee91497e 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -47,6 +47,7 @@
#include "xe_pcode.h"
#include "xe_pm.h"
#include "xe_query.h"
+#include "xe_spi.h"
#include "xe_sriov.h"
#include "xe_tile.h"
#include "xe_ttm_stolen_mgr.h"
@@ -720,6 +721,7 @@ int xe_device_probe(struct xe_device *xe)
goto err_fini_gt;
}
+ xe_spi_init(xe);
xe_heci_gsc_init(xe);
err = xe_oa_init(xe);
@@ -788,6 +790,7 @@ void xe_device_remove(struct xe_device *xe)
xe_oa_fini(xe);
xe_heci_gsc_fini(xe);
+ xe_spi_fini(xe);
for_each_gt(gt, xe, id)
xe_gt_remove(gt);
diff --git a/drivers/gpu/drm/xe/xe_device_types.h
b/drivers/gpu/drm/xe/xe_device_types.h
index 5b7292a9a66d..c41f6004eb5b 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -12,6 +12,8 @@
#include
#include
+#include
+
#include "xe_devcoredump_types.h"
#include "xe_heci_gsc.h"
#include "xe_gt_types.h"
@@ -45,6 +47,7 @@ struct xe_pat_ops;
#define IS_DGFX(xe) ((xe)->info.is_dgfx)
#define HAS_HECI_GSCFI(xe) ((xe)->info.has_heci_gscfi)
#define HAS_HECI_CSCFI(xe) ((xe)->info.has_heci_cscfi)
+#define HAS_GSC_SPI(xe) ((xe)->info.has_gsc_spi)
#define XE_VRAM_FLAGS_NEED64K BIT(0)
@@ -292,6 +295,8 @@ struct xe_device {
u8 has_heci_gscfi:1;
/** @info.has_heci_cscfi: device has heci cscfi */
u8 has_heci_cscfi:1;
+ /** @info.has_gsc_spi: device has gsc spi */
+ u8 has_gsc_spi:1;
/** @info.skip_guc_pc: Skip GuC based PM feature init */
u8 skip_guc_pc:1;
/** @info.has_atomic_enable_pte_bit: Device has atomic enable
PTE bit */
@@ -470,6 +475,9 @@ struct xe_device {
/** @heci_gsc: graphics security controller */
struct xe_heci_gsc heci_gsc;
+ /** @spi: discrete graphics spi */
+ struct intel_dg_spi_dev spi;
+
/** @oa: oa observation subsystem */
struct xe_oa oa;
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 3c4a3c91377a..c74c36ee7fa6 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -60,6 +60,7 @@ struct xe_device_desc {
u8 has_display:1;
u8 has_heci_gscfi:1;
u8 has_heci_cscfi:1;
+ u8 has_gsc_spi:1;
u8 has_llc:1;
u8 has_mmio_ext:1;
u8 has_sriov:1;
@@ -283,6 +284,7 @@ static const struct xe_device_desc dg1_desc = {
PLATFORM(DG1),
.has_display = true,
.has_heci_gscfi = 1,
+ .has_gsc_spi = 1,
.require_force_probe = true,
};
@@ -294,6 +296,7 @@ static const u16 dg2_g12_ids[] = { XE_DG2_G12_IDS(NOP), 0 };
DGFX_FEATURES, \
PLATFORM(DG2), \
.has_heci_gscfi = 1, \
+ .has_gsc_spi = 1, \
.subplatforms = (const struct xe_subplatform_desc[]) { \
{ XE_SUBPLATFORM_DG2_G10, "G10", dg2_g10_ids }, \
{ XE_SUBPLATFORM_DG2_G11, "G11", dg2_g11_ids }, \
@@ -325,6 +328,7 @@ static const __maybe_unused struct xe_device_desc pvc_desc
= {
PLATFORM(PVC),
.has_display = false,
.has_heci_gscfi = 1,
+ .has_gsc_spi = 1,
.require_force_probe = true,
};
@@ -609,6 +613,7 @@ static int xe_info_init_early(struct xe_device *xe,
xe->info.is_dgfx = desc->is_dgfx;
xe->info.has_heci_gscfi = desc->has_heci_gscfi;
xe->info.has_heci_cscfi = desc->has_heci_cscfi;
+ xe->info.has_gsc_spi = desc->has_gsc_spi;
xe->info.has_llc = desc->has_llc;
xe->info.has_mmio_ext = desc->has_mmio_ext;
xe->info.has_sriov = desc->has_sriov;
diff --git a/drivers/gpu/drm/xe/xe_spi.c b/drivers/gpu/drm/xe/xe_spi.c
new file mode 100644
index ..37080b82e9ae
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_spi.c
@
[PATCH v5 12/12] drm/xe/spi: add support for access mode
Check SPI access mode from GSC FW status registers
and overwrite access status read from SPI descriptor, if needed.
Signed-off-by: Alexander Usyskin
---
drivers/gpu/drm/xe/regs/xe_gsc_regs.h | 4
drivers/gpu/drm/xe/xe_heci_gsc.c | 5 +---
drivers/gpu/drm/xe/xe_spi.c | 33 ++-
3 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/xe/regs/xe_gsc_regs.h
b/drivers/gpu/drm/xe/regs/xe_gsc_regs.h
index e2a925be137c..e22082875811 100644
--- a/drivers/gpu/drm/xe/regs/xe_gsc_regs.h
+++ b/drivers/gpu/drm/xe/regs/xe_gsc_regs.h
@@ -16,6 +16,10 @@
#define MTL_GSC_HECI1_BASE 0x00116000
#define MTL_GSC_HECI2_BASE 0x00117000
+#define DG1_GSC_HECI2_BASE 0x00259000
+#define PVC_GSC_HECI2_BASE 0x00285000
+#define DG2_GSC_HECI2_BASE 0x00374000
+
#define HECI_H_CSR(base) XE_REG((base) + 0x4)
#define HECI_H_CSR_IEREG_BIT(0)
#define HECI_H_CSR_ISREG_BIT(1)
diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.c b/drivers/gpu/drm/xe/xe_heci_gsc.c
index 65b2e147c4b9..27734085164e 100644
--- a/drivers/gpu/drm/xe/xe_heci_gsc.c
+++ b/drivers/gpu/drm/xe/xe_heci_gsc.c
@@ -11,14 +11,11 @@
#include "xe_device_types.h"
#include "xe_drv.h"
#include "xe_heci_gsc.h"
+#include "regs/xe_gsc_regs.h"
#include "xe_platform_types.h"
#define GSC_BAR_LENGTH 0x0FFC
-#define DG1_GSC_HECI2_BASE 0x259000
-#define PVC_GSC_HECI2_BASE 0x285000
-#define DG2_GSC_HECI2_BASE 0x374000
-
static void heci_gsc_irq_mask(struct irq_data *d)
{
/* generic irq handling */
diff --git a/drivers/gpu/drm/xe/xe_spi.c b/drivers/gpu/drm/xe/xe_spi.c
index 37080b82e9ae..47341418f772 100644
--- a/drivers/gpu/drm/xe/xe_spi.c
+++ b/drivers/gpu/drm/xe/xe_spi.c
@@ -5,7 +5,10 @@
#include
#include
+#include "xe_device.h"
#include "xe_device_types.h"
+#include "xe_mmio.h"
+#include "regs/xe_gsc_regs.h"
#include "xe_spi.h"
#include "xe_sriov.h"
@@ -24,6 +27,34 @@ static void xe_spi_release_dev(struct device *dev)
{
}
+static bool xe_spi_writeable_override(struct xe_device *xe)
+{
+ struct xe_gt *gt = xe_root_mmio_gt(xe);
+ struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
+ resource_size_t base;
+ bool writeable_override;
+
+ if (xe->info.platform == XE_BATTLEMAGE) {
+ base = DG2_GSC_HECI2_BASE;
+ } else if (xe->info.platform == XE_PVC) {
+ base = PVC_GSC_HECI2_BASE;
+ } else if (xe->info.platform == XE_DG2) {
+ base = DG2_GSC_HECI2_BASE;
+ } else if (xe->info.platform == XE_DG1) {
+ base = DG1_GSC_HECI2_BASE;
+ } else {
+ dev_err(&pdev->dev, "Unknown platform\n");
+ return true;
+ }
+
+ writeable_override =
+ !(xe_mmio_read32(gt, HECI_H_GS1(base)) &
+ HECI_FW_STATUS_2_SPI_ACCESS_MODE);
+ if (writeable_override)
+ dev_info(&pdev->dev, "SPI access overridden by jumper\n");
+ return writeable_override;
+}
+
void xe_spi_init(struct xe_device *xe)
{
struct intel_dg_spi_dev *spi = &xe->spi;
@@ -38,7 +69,7 @@ void xe_spi_init(struct xe_device *xe)
if (IS_SRIOV_VF(xe))
return;
- spi->writeable_override = false;
+ spi->writeable_override = xe_spi_writeable_override(xe);
spi->bar.parent = &pdev->resource[0];
spi->bar.start = GEN12_GUNIT_SPI_BASE + pdev->resource[0].start;
spi->bar.end = spi->bar.start + GEN12_GUNIT_SPI_SIZE - 1;
--
2.34.1
RE: [PATCH v2 3/9] dt-bindings: display: renesas,rzg2l-du: Document RZ/G2UL DU bindings
Hi Laurent Pinchart,
Thanks for the feedback.
> -Original Message-
> From: Laurent Pinchart
> Sent: Saturday, July 27, 2024 1:50 AM
> Subject: Re: [PATCH v2 3/9] dt-bindings: display: renesas,rzg2l-du: Document
> RZ/G2UL DU bindings
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Tue, Jul 09, 2024 at 02:51:41PM +0100, Biju Das wrote:
> > Document DU found in RZ/G2UL SoC. The DU block is identical to RZ/G2L
> > SoC, but has only DPI interface.
> >
> > While at it, add missing required property port@1 for RZ/G2L and
> > RZ/V2L SoCs. Currently there is no user for the DPI interface and
> > hence there won't be any ABI breakage for adding port@1 as required
> > property for RZ/G2L and RZ/V2L SoCs.
> >
> > Signed-off-by: Biju Das
> > Acked-by: Conor Dooley
> > ---
> > v1->v2:
> > * Updated commit description related to non ABI breakage.
> > * Added Ack from Conor.
> > ---
> > .../bindings/display/renesas,rzg2l-du.yaml| 32 +--
> > 1 file changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > index 08e5b9478051..c0fec282fa45 100644
> > --- a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > +++ b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > @@ -18,6 +18,7 @@ properties:
> >compatible:
> > oneOf:
> >- enum:
> > + - renesas,r9a07g043u-du # RZ/G2UL
> >- renesas,r9a07g044-du # RZ/G2{L,LC}
> >- items:
> >- enum:
> > @@ -60,9 +61,6 @@ properties:
> > $ref: /schemas/graph.yaml#/properties/port
> > unevaluatedProperties: false
> >
> > -required:
> > - - port@0
> > -
> > unevaluatedProperties: false
> >
> >renesas,vsps:
> > @@ -88,6 +86,34 @@ required:
> >
> > additionalProperties: false
> >
> > +allOf:
> > + - if:
> > + properties:
> > +compatible:
> > + contains:
> > +const: renesas,r9a07g043u-du
> > +then:
> > + properties:
> > +ports:
> > + properties:
> > +port@0: false
> > +port@1:
> > + description: DPI
> > +
> > + required:
> > +- port@1
>
> Why do you use port@1 for the DPI output here, and not port@0 ?
Currently the output is based on port number and port = 1 corresponds to DPI.
See [1].
For consistency, I documented bindings for RZ/G2L family DU's similar to
RZ/G2{H,M,N,E} DU [2].
So please let me know, are you ok with this?
[1]
https://elixir.bootlin.com/linux/v6.10.2/source/drivers/gpu/drm/renesas/rz-du/rzg2l_du_kms.c#L187
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/display/renesas,du.yaml?h=next-20240729#n546
>
> > +else:
> > + properties:
> > +ports:
> > + properties:
> > +port@0:
> > + description: DSI
> > +port@1:
> > + description: DPI
> > +
> > + required:
> > +- port@0
> > +- port@1
>
> You're missing a blank line here.
OK, will fix this'
Cheers,
Biju
Re: [REGRESSION] No image on 4k display port displays connected through usb-c dock in kernel 6.10
On 29.07.24 10:47, Christian Heusel wrote:
> On 24/07/29 10:35AM, Linux regression tracking (Thorsten Leemhuis) wrote:
>> [+Greg +stable]
>>
>> On 29.07.24 10:16, Lin, Wayne wrote:
>>>
>>> Thanks for the report.
>>>
>>> Patch fa57924c76d995 ("drm/amd/display: Refactor function
>>> dm_dp_mst_is_port_support_mode()")
>>> is kind of correcting problems causing by commit:
>>> 4df96ba6676034 ("drm/amd/display: Add timing pixel encoding for mst mode
>>> validation")
>>>
>>> Sorry if it misses fixes tag and would suggest to backport to fix it.
>>> Thanks!
>>
>> Greg, seem it would be wise to pick up fa57924c76d995 for 6.10.y as
>> well, despite a lack of Fixes or stable tags.
>>
>> Ciao, Thorsten
>
> The issue is that the fixing commit does not apply to the 6.10 series
> without conflict and the offending commit does not revert cleanly
> aswell.
Hah, many thx, I should have checked that.
Lin, Wayne: could you maybe help out here and provide something for 6.10.y?
Ciao, Thorsten
RE: [PATCH v2 5/9] drm: renesas: rz-du: Add RZ/G2UL DU Support
Hi Laurent,
Thanks for the feedback.
> -Original Message-
> From: Laurent Pinchart
> Sent: Saturday, July 27, 2024 2:00 AM
> Subject: Re: [PATCH v2 5/9] drm: renesas: rz-du: Add RZ/G2UL DU Support
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Tue, Jul 09, 2024 at 02:51:43PM +0100, Biju Das wrote:
> > The LCD controller is composed of Frame Compression Processor (FCPVD),
> > Video Signal Processor (VSPD), and Display Unit (DU).
> >
> > It has DPI interface and supports a maximum resolution of WXGA along
> > with 2 RPFs to support the blending of two picture layers and raster
> > operations (ROPs).
> >
> > The DU module is connected to VSPD. Add RZ/G2UL DU support.
> >
> > Signed-off-by: Biju Das
> > ---
> > v1->v2:
> > * No change.
> > ---
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c | 9 -
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c | 11 +++
> > 2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > index 6e7aac6219be..b1812f947252 100644
> > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > @@ -28,6 +28,7 @@
> > #include "rzg2l_du_vsp.h"
> >
> > #define DU_MCR00x00
> > +#define DU_MCR0_DPI_OE BIT(0)
> > #define DU_MCR0_DI_EN BIT(8)
> >
> > #define DU_DITR0 0x10
> > @@ -216,9 +217,15 @@ static void rzg2l_du_crtc_put(struct
> > rzg2l_du_crtc *rcrtc)
> >
> > static void rzg2l_du_start_stop(struct rzg2l_du_crtc *rcrtc, bool
> > start) {
> > + struct rzg2l_du_crtc_state *rstate =
> > + to_rzg2l_crtc_state(rcrtc->crtc.state);
>
> I think you can avoid the line break here.
OK, I will make it inlined.
>
> > struct rzg2l_du_device *rcdu = rcrtc->dev;
> > + u32 val = DU_MCR0_DI_EN;
> >
> > - writel(start ? DU_MCR0_DI_EN : 0, rcdu->mmio + DU_MCR0);
> > + if (rstate->outputs == BIT(RZG2L_DU_OUTPUT_DPAD0))
> > + val |= DU_MCR0_DPI_OE;
> > +
> > + writel(start ? val : 0, rcdu->mmio + DU_MCR0);
> > }
> >
> > static void rzg2l_du_crtc_start(struct rzg2l_du_crtc *rcrtc) diff
> > --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c
> > b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c
> > index e5eca8691a33..34534441b7ec 100644
> > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c
> > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c
> > @@ -25,6 +25,16 @@
> > * Device Information
> > */
> >
> > +static const struct rzg2l_du_device_info rzg2l_du_r9a07g043u_info = {
> > + .channels_mask = BIT(0),
> > + .routes = {
> > + [RZG2L_DU_OUTPUT_DPAD0] = {
> > + .possible_outputs = BIT(0),
> > + .port = 1,
>
> This may need to be port 0 depending on the outcome of the discussion on the
> DT bindings.
Agreed.
Cheers,
Biju
RE: [PATCH v2 6/9] arm64: dts: renesas: r9a07g043u: Add vspd node
Hi Laurent,
Thanks for the feedback.
> -Original Message-
> From: Laurent Pinchart
> Sent: Saturday, July 27, 2024 2:08 AM
> Subject: Re: [PATCH v2 6/9] arm64: dts: renesas: r9a07g043u: Add vspd node
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Tue, Jul 09, 2024 at 02:51:44PM +0100, Biju Das wrote:
> > Add vspd node to RZ/G2UL SoC DTSI.
> >
> > Signed-off-by: Biju Das
> > ---
> > v1->v2:
> > * No change.
> > ---
> > arch/arm64/boot/dts/renesas/r9a07g043u.dtsi | 13 +
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > index 18ef297db933..15e84a5428ef 100644
> > --- a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > @@ -129,6 +129,19 @@ csi2cru: endpoint@0 {
> > };
> > };
> >
> > + vspd: vsp@1087 {
> > + compatible = "renesas,r9a07g043u-vsp2",
> > "renesas,r9a07g044-vsp2";
> > + reg = <0 0x1087 0 0x1>;
> > + interrupts = ;
> > + clocks = <&cpg CPG_MOD R9A07G043_LCDC_CLK_A>,
> > +<&cpg CPG_MOD R9A07G043_LCDC_CLK_P>,
> > +<&cpg CPG_MOD R9A07G043_LCDC_CLK_D>;
> > + clock-names = "aclk", "pclk", "vclk";
> > + power-domains = <&cpg>;
> > + resets = <&cpg R9A07G043_LCDC_RESET_N>;
> > + renesas,fcp = <&fcpvd>;
>
> This patch looks fine, but I would move it after 7/9, as here you reference a
> node that doesn't exist
> yet.
Good catch. Will fix this in next version.
Cheers,
Biju
>
> Reviewed-by: Laurent Pinchart
>
> > + };
> > +
> > irqc: interrupt-controller@110a {
> > compatible = "renesas,r9a07g043u-irqc",
> > "renesas,rzg2l-irqc";
>
> --
> Regards,
>
> Laurent Pinchart
RE: [PATCH v2 8/9] arm64: dts: renesas: r9a07g043u: Add DU node
Hi Laurent,
Thanks for the feedback.
> -Original Message-
> From: Laurent Pinchart
> Sent: Saturday, July 27, 2024 2:12 AM
> Subject: Re: [PATCH v2 8/9] arm64: dts: renesas: r9a07g043u: Add DU node
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Tue, Jul 09, 2024 at 02:51:46PM +0100, Biju Das wrote:
> > Add DU node to RZ/G2UL SoC DTSI.
> >
> > Signed-off-by: Biju Das
> > ---
> > v1->v2:
> > * No change.
> > ---
> > arch/arm64/boot/dts/renesas/r9a07g043u.dtsi | 25
> > +
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > index d88bf23b0782..0a4f24d83791 100644
> > --- a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > @@ -153,6 +153,31 @@ fcpvd: fcp@1088 {
> > resets = <&cpg R9A07G043_LCDC_RESET_N>;
> > };
> >
> > + du: display@1089 {
> > + compatible = "renesas,r9a07g043u-du";
> > + reg = <0 0x1089 0 0x1>;
> > + interrupts = ;
> > + clocks = <&cpg CPG_MOD R9A07G043_LCDC_CLK_A>,
> > +<&cpg CPG_MOD R9A07G043_LCDC_CLK_P>,
> > +<&cpg CPG_MOD R9A07G043_LCDC_CLK_D>;
> > + clock-names = "aclk", "pclk", "vclk";
> > + power-domains = <&cpg>;
> > + resets = <&cpg R9A07G043_LCDC_RESET_N>;
> > + renesas,vsps = <&vspd 0>;
> > + status = "disabled";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@1 {
> > + reg = <1>;
>
> This may need to change depending on the outcome of the DT bindings
> discussion. Other than that,
Agreed.
Cheers,
Biju
>
> Reviewed-by: Laurent Pinchart
>
> > + du_out_rgb: endpoint {
> > + };
> > + };
> > + };
> > + };
> > +
> > irqc: interrupt-controller@110a {
> > compatible = "renesas,r9a07g043u-irqc",
> > "renesas,rzg2l-irqc";
>
> --
> Regards,
>
> Laurent Pinchart
Re: [PATCH] drm/radeon/evergreen_cs: fix int overflow errors in cs track offsets
Am 26.07.24 um 14:52 schrieb Alex Deucher:
On Fri, Jul 26, 2024 at 3:05 AM Christian König
wrote:
Am 25.07.24 um 20:09 schrieb Nikita Zhandarovich:
Several cs track offsets (such as 'track->db_s_read_offset')
either are initialized with or plainly take big enough values that,
once shifted 8 bits left, may be hit with integer overflow if the
resulting values end up going over u32 limit.
Some debug prints take this into account (see according dev_warn() in
evergreen_cs_track_validate_stencil()), even if the actual
calculated value assigned to local 'offset' variable is missing
similar proper expansion.
Mitigate the problem by casting the type of right operands to the
wider type of corresponding left ones in all such cases.
Found by Linux Verification Center (linuxtesting.org) with static
analysis tool SVACE.
Fixes: 285484e2d55e ("drm/radeon: add support for evergreen/ni tiling informations
v11")
Cc: sta...@vger.kernel.org
Well first of all the long cast doesn't makes the value 64bit, it
depends on the architecture.
Then IIRC the underlying hw can only handle a 32bit address space so
having the offset as long is incorrect to begin with.
Evergreen chips support a 36 bit internal address space and NI and
newer support a 40 bit one, so this is applicable.
In that case I strongly suggest that we replace the unsigned long with
u64 or otherwise we get different behavior on 32 and 64bit machines.
Regards,
Christian.
Alex
And finally that is absolutely not material for stable.
Regards,
Christian.
Signed-off-by: Nikita Zhandarovich
---
P.S. While I am not certain that track->cb_color_bo_offset[id]
actually ends up taking values high enough to cause an overflow,
nonetheless I thought it prudent to cast it to ulong as well.
drivers/gpu/drm/radeon/evergreen_cs.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
b/drivers/gpu/drm/radeon/evergreen_cs.c
index 1fe6e0d883c7..d734d221e2da 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -433,7 +433,7 @@ static int evergreen_cs_track_validate_cb(struct
radeon_cs_parser *p, unsigned i
return r;
}
- offset = track->cb_color_bo_offset[id] << 8;
+ offset = (unsigned long)track->cb_color_bo_offset[id] << 8;
if (offset & (surf.base_align - 1)) {
dev_warn(p->dev, "%s:%d cb[%d] bo base %ld not aligned with
%ld\n",
__func__, __LINE__, id, offset, surf.base_align);
@@ -455,7 +455,7 @@ static int evergreen_cs_track_validate_cb(struct
radeon_cs_parser *p, unsigned i
min = surf.nby - 8;
}
bsize = radeon_bo_size(track->cb_color_bo[id]);
- tmp = track->cb_color_bo_offset[id] << 8;
+ tmp = (unsigned long)track->cb_color_bo_offset[id] << 8;
for (nby = surf.nby; nby > min; nby--) {
size = nby * surf.nbx * surf.bpe * surf.nsamples;
if ((tmp + size * mslice) <= bsize) {
@@ -476,10 +476,10 @@ static int evergreen_cs_track_validate_cb(struct
radeon_cs_parser *p, unsigned i
}
}
dev_warn(p->dev, "%s:%d cb[%d] bo too small (layer size %d, "
- "offset %d, max layer %d, bo size %ld, slice %d)\n",
+ "offset %ld, max layer %d, bo size %ld, slice %d)\n",
__func__, __LINE__, id, surf.layer_size,
- track->cb_color_bo_offset[id] << 8, mslice,
- radeon_bo_size(track->cb_color_bo[id]), slice);
+ (unsigned long)track->cb_color_bo_offset[id] << 8,
+ mslice, radeon_bo_size(track->cb_color_bo[id]), slice);
dev_warn(p->dev, "%s:%d problematic surf: (%d %d) (%d %d %d %d %d %d
%d)\n",
__func__, __LINE__, surf.nbx, surf.nby,
surf.mode, surf.bpe, surf.nsamples,
@@ -608,7 +608,7 @@ static int evergreen_cs_track_validate_stencil(struct
radeon_cs_parser *p)
return r;
}
- offset = track->db_s_read_offset << 8;
+ offset = (unsigned long)track->db_s_read_offset << 8;
if (offset & (surf.base_align - 1)) {
dev_warn(p->dev, "%s:%d stencil read bo base %ld not aligned with
%ld\n",
__func__, __LINE__, offset, surf.base_align);
@@ -627,7 +627,7 @@ static int evergreen_cs_track_validate_stencil(struct
radeon_cs_parser *p)
return -EINVAL;
}
- offset = track->db_s_write_offset << 8;
+ offset = (unsigned long)track->db_s_write_offset << 8;
if (offset & (surf.base_align - 1)) {
dev_warn(p->dev, "%s:%d stencil write bo base %ld not aligned with
%ld\n",
__func__, __LINE__, offset, surf.base_alig
[PATCH] drm/xe/oa/uapi: Make bit masks unsigned
When building with gcc-5:
In function ‘decode_oa_format.isra.26’,
inlined from ‘xe_oa_set_prop_oa_format’ at
drivers/gpu/drm/xe/xe_oa.c:1664:6:
././include/linux/compiler_types.h:510:38: error: call to
‘__compiletime_assert_1336’ declared with attribute error: FIELD_GET: mask is
not constant
[...]
./include/linux/bitfield.h:155:3: note: in expansion of macro
‘__BF_FIELD_CHECK’
__BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
^
drivers/gpu/drm/xe/xe_oa.c:1573:18: note: in expansion of macro ‘FIELD_GET’
u32 bc_report = FIELD_GET(DRM_XE_OA_FORMAT_MASK_BC_REPORT, fmt);
^
Fixes: b6fd51c6211910b1 ("drm/xe/oa/uapi: Define and parse OA stream
properties")
Signed-off-by: Geert Uytterhoeven
---
Compile-tested only.
---
include/uapi/drm/xe_drm.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
index 19619d4952a863f7..db232a25189eba9f 100644
--- a/include/uapi/drm/xe_drm.h
+++ b/include/uapi/drm/xe_drm.h
@@ -1590,10 +1590,10 @@ enum drm_xe_oa_property_id {
* b. Counter select c. Counter size and d. BC report. Also refer to the
* oa_formats array in drivers/gpu/drm/xe/xe_oa.c.
*/
-#define DRM_XE_OA_FORMAT_MASK_FMT_TYPE (0xff << 0)
-#define DRM_XE_OA_FORMAT_MASK_COUNTER_SEL (0xff << 8)
-#define DRM_XE_OA_FORMAT_MASK_COUNTER_SIZE (0xff << 16)
-#define DRM_XE_OA_FORMAT_MASK_BC_REPORT(0xff << 24)
+#define DRM_XE_OA_FORMAT_MASK_FMT_TYPE (0xffu << 0)
+#define DRM_XE_OA_FORMAT_MASK_COUNTER_SEL (0xffu << 8)
+#define DRM_XE_OA_FORMAT_MASK_COUNTER_SIZE (0xffu << 16)
+#define DRM_XE_OA_FORMAT_MASK_BC_REPORT(0xffu << 24)
/**
* @DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT: Requests periodic OA unit
--
2.34.1
Re: [PATCH resend v4 00/11] Improve the copy of task comm
On Mon, 29 Jul 2024, Yafang Shao wrote:
> Hello Andrew,
>
> Is it appropriate for you to apply this to the mm tree?
>
> Using {memcpy,strncpy,strcpy,kstrdup} to copy the task comm relies on the
> length of task comm. Changes in the task comm could result in a destination
> string that is overflow. Therefore, we should explicitly ensure the
> destination
> string is always NUL-terminated, regardless of the task comm. This approach
> will facilitate future extensions to the task comm.
Why are we normalizing calling double-underscore prefixed functions all
over the place? i.e. __get_task_comm().
get_task_comm() is widely used. At a glance, looks like it could be used
in many of the patches here too.
BR,
Jani.
>
> As suggested by Linus [0], we can identify all relevant code with the
> following git grep command:
>
> git grep 'memcpy.*->comm\>'
> git grep 'kstrdup.*->comm\>'
> git grep 'strncpy.*->comm\>'
> git grep 'strcpy.*->comm\>'
>
> PATCH #2~#4: memcpy
> PATCH #5~#6: kstrdup
> PATCH #7~#9: strncpy
> PATCH #10~#11: strcpy
>
> Suggested-by: Linus Torvalds
> Link:
> https://lore.kernel.org/all/CAHk-=wjAmmHUg6vho1KjzQi2=psr30+cogfd4axrthr2gsi...@mail.gmail.com/
> [0]
>
> Changes:
> v3->v4:
> - Rename __kstrndup() to __kmemdup_nul() and define it inside mm/util.c
> (Matthew)
> - Remove unused local varaible (Simon)
>
> v2->v3:
> https://lore.kernel.org/all/20240621022959.9124-1-laoar.s...@gmail.com/
> - Deduplicate code around kstrdup (Andrew)
> - Add commit log for dropping task_lock (Catalin)
>
> v1->v2:
> https://lore.kernel.org/bpf/20240613023044.45873-1-laoar.s...@gmail.com/
> - Add comment for dropping task_lock() in __get_task_comm() (Alexei)
> - Drop changes in trace event (Steven)
> - Fix comment on task comm (Matus)
>
> v1: https://lore.kernel.org/all/20240602023754.25443-1-laoar.s...@gmail.com/
>
> Yafang Shao (11):
> fs/exec: Drop task_lock() inside __get_task_comm()
> auditsc: Replace memcpy() with __get_task_comm()
> security: Replace memcpy() with __get_task_comm()
> bpftool: Ensure task comm is always NUL-terminated
> mm/util: Fix possible race condition in kstrdup()
> mm/util: Deduplicate code in {kstrdup,kstrndup,kmemdup_nul}
> mm/kmemleak: Replace strncpy() with __get_task_comm()
> tsacct: Replace strncpy() with __get_task_comm()
> tracing: Replace strncpy() with __get_task_comm()
> net: Replace strcpy() with __get_task_comm()
> drm: Replace strcpy() with __get_task_comm()
>
> drivers/gpu/drm/drm_framebuffer.c | 2 +-
> drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
> fs/exec.c | 10 -
> include/linux/sched.h | 4 +-
> kernel/auditsc.c | 6 +--
> kernel/trace/trace.c | 2 +-
> kernel/trace/trace_events_hist.c | 2 +-
> kernel/tsacct.c | 2 +-
> mm/kmemleak.c | 8 +---
> mm/util.c | 61 ---
> net/ipv6/ndisc.c | 2 +-
> security/lsm_audit.c | 4 +-
> security/selinux/selinuxfs.c | 2 +-
> tools/bpf/bpftool/pids.c | 2 +
> 14 files changed, 51 insertions(+), 58 deletions(-)
--
Jani Nikula, Intel
[PATCH 0/2] Use pcim_request_region() in vboxvideo
Hi everyone, Now that we've got the simplified PCI devres API available we can slowly start using it in drivers and step by step phase the more problematic API out. vboxvideo currently does not have a region request, so it is a suitable first user. P. Philipp Stanner (2): PCI: Make pcim_request_region() a public function drm/vboxvideo: Add PCI region request drivers/gpu/drm/vboxvideo/vbox_main.c | 4 drivers/pci/devres.c | 1 + drivers/pci/pci.h | 2 -- include/linux/pci.h | 1 + 4 files changed, 6 insertions(+), 2 deletions(-) -- 2.45.2
[PATCH 1/2] PCI: Make pcim_request_region() a public function
pcim_request_region() is the managed counterpart of
pci_request_region(). It is currently only used internally for PCI.
It can be useful for a number of drivers and exporting it is a step
towards deprecating more complicated functions.
Make pcim_request_region a public function.
Signed-off-by: Philipp Stanner
---
drivers/pci/devres.c | 1 +
drivers/pci/pci.h| 2 --
include/linux/pci.h | 1 +
3 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 3780a9f9ec00..0127ca58c6e5 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -863,6 +863,7 @@ int pcim_request_region(struct pci_dev *pdev, int bar,
const char *name)
{
return _pcim_request_region(pdev, bar, name, 0);
}
+EXPORT_SYMBOL(pcim_request_region);
/**
* pcim_request_region_exclusive - Request a PCI BAR exclusively
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 79c8398f3938..2fe6055a334d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -887,8 +887,6 @@ static inline pci_power_t mid_pci_get_power_state(struct
pci_dev *pdev)
#endif
int pcim_intx(struct pci_dev *dev, int enable);
-
-int pcim_request_region(struct pci_dev *pdev, int bar, const char *name);
int pcim_request_region_exclusive(struct pci_dev *pdev, int bar,
const char *name);
void pcim_release_region(struct pci_dev *pdev, int bar);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9e36b6c1810e..e5d8406874e2 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2294,6 +2294,7 @@ static inline void pci_fixup_device(enum pci_fixup_pass
pass,
void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen);
void pcim_iounmap(struct pci_dev *pdev, void __iomem *addr);
void __iomem * const *pcim_iomap_table(struct pci_dev *pdev);
+int pcim_request_region(struct pci_dev *pdev, int bar, const char *name);
int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name);
int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask,
const char *name);
--
2.45.2
[PATCH 2/2] drm/vboxvideo: Add PCI region request
vboxvideo currently does not reserve its PCI BAR through a region
request.
Implement the request through the managed function
pcim_request_region().
Signed-off-by: Philipp Stanner
---
drivers/gpu/drm/vboxvideo/vbox_main.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_main.c
b/drivers/gpu/drm/vboxvideo/vbox_main.c
index d4ade9325401..7f686a0190e6 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_main.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_main.c
@@ -114,6 +114,10 @@ int vbox_hw_init(struct vbox_private *vbox)
DRM_INFO("VRAM %08x\n", vbox->full_vram_size);
+ ret = pcim_request_region(pdev, 0, "vboxvideo");
+ if (ret)
+ return ret;
+
/* Map guest-heap at end of vram */
vbox->guest_heap = pcim_iomap_range(pdev, 0,
GUEST_HEAP_OFFSET(vbox), GUEST_HEAP_SIZE);
--
2.45.2
Re: [PATCH v2 1/2] drm/amdgpu: convert bios_hardcoded_edid to drm_edid
On Fri, 26 Jul 2024, Thomas Weißschuh wrote:
> Instead of manually passing around 'struct edid *' and its size,
> use 'struct drm_edid', which encapsulates a validated combination of
> both.
>
> As the drm_edid_ can handle NULL gracefully, the explicit checks can be
> dropped.
>
> Also save a few characters by transforming '&array[0]' to the equivalent
> 'array' and using 'max_t(int, ...)' instead of manual casts.
>
> Signed-off-by: Thomas Weißschuh
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 4 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 17 ++---
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
> 8 files changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index bd0fbdc5f55d..344e0a9ee08a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -249,11 +249,7 @@ amdgpu_connector_find_encoder(struct drm_connector
> *connector,
> static struct edid *
> amdgpu_connector_get_hardcoded_edid(struct amdgpu_device *adev)
> {
> - if (adev->mode_info.bios_hardcoded_edid) {
> - return kmemdup((unsigned char
> *)adev->mode_info.bios_hardcoded_edid,
> -adev->mode_info.bios_hardcoded_edid_size,
> GFP_KERNEL);
> - }
> - return NULL;
> + return
> drm_edid_duplicate(drm_edid_raw(adev->mode_info.bios_hardcoded_edid));
> }
>
> static void amdgpu_connector_get_edid(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index d002b845d8ac..5e3faefc5510 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -51,6 +51,7 @@ struct amdgpu_encoder;
> struct amdgpu_router;
> struct amdgpu_hpd;
> struct edid;
> +struct drm_edid;
>
> #define to_amdgpu_crtc(x) container_of(x, struct amdgpu_crtc, base)
> #define to_amdgpu_connector(x) container_of(x, struct amdgpu_connector, base)
> @@ -326,8 +327,7 @@ struct amdgpu_mode_info {
> /* FMT dithering */
> struct drm_property *dither_property;
> /* hardcoded DFP edid from BIOS */
> - struct edid *bios_hardcoded_edid;
> - int bios_hardcoded_edid_size;
> + const struct drm_edid *bios_hardcoded_edid;
>
> /* firmware flags */
> u32 firmware_flags;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> index 6415d0d039e1..e5f508d34ed8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> @@ -549,7 +549,7 @@ static int amdgpu_vkms_sw_fini(void *handle)
>
> adev->mode_info.mode_config_initialized = false;
>
> - kfree(adev->mode_info.bios_hardcoded_edid);
> + drm_edid_free(adev->mode_info.bios_hardcoded_edid);
> kfree(adev->amdgpu_vkms_output);
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
> b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
> index ebf83fee43bb..8defca3705d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
> @@ -2064,23 +2064,18 @@ amdgpu_atombios_encoder_get_lcd_info(struct
> amdgpu_encoder *encoder)
> case LCD_FAKE_EDID_PATCH_RECORD_TYPE:
> fake_edid_record =
> (ATOM_FAKE_EDID_PATCH_RECORD *)record;
> if (fake_edid_record->ucFakeEDIDLength)
> {
> - struct edid *edid;
> + const struct drm_edid *edid;
Bikeshedding follows, up to you and the AMD maintainers to decide
whether it matters.
I know it's a bit verbose, but personally I've named the struct drm_edid
variables drm_edid everywhere when making conversions, just to make a
clear distinction from struct edid. And I like the fact that it forces
you to account for every place the variable is used, in particular
passing it to functions that don't have type safety e.g. kfree().
> int edid_size;
>
> if
> (fake_edid_record->ucFakeEDIDLength == 128)
> edid_size =
> fake_edid_record->ucFakeEDIDLength;
> else
> edid_size =
> fake_edid_record->ucFakeEDIDLength * 128;
> -
Re: [PATCH v2 2/2] drm/radeon: convert bios_hardcoded_edid to drm_edid
On Fri, 26 Jul 2024, Alex Deucher wrote: > Applied the series. Thanks! Ah, replied to patch 1 before noticing this. Never mind about the bikeshedding. :) BR, Jani. -- Jani Nikula, Intel
Re: [REGRESSION] No image on 4k display port displays connected through usb-c dock in kernel 6.10
On 24/07/29 10:35AM, Linux regression tracking (Thorsten Leemhuis) wrote:
> [+Greg +stable]
>
> On 29.07.24 10:16, Lin, Wayne wrote:
> >
> > Thanks for the report.
> >
> > Patch fa57924c76d995 ("drm/amd/display: Refactor function
> > dm_dp_mst_is_port_support_mode()")
> > is kind of correcting problems causing by commit:
> > 4df96ba6676034 ("drm/amd/display: Add timing pixel encoding for mst mode
> > validation")
> >
> > Sorry if it misses fixes tag and would suggest to backport to fix it.
> > Thanks!
>
> Greg, seem it would be wise to pick up fa57924c76d995 for 6.10.y as
> well, despite a lack of Fixes or stable tags.
>
> Ciao, Thorsten
The issue is that the fixing commit does not apply to the 6.10 series
without conflict and the offending commit does not revert cleanly
aswell.
Cheers,
Chris
signature.asc
Description: PGP signature
Re: [PATCH v3 2/2] drm/tiny: Add driver for Sharp Memory LCD
…
> +++ b/drivers/gpu/drm/tiny/sharp-memory.c
> @@ -0,0 +1,684 @@
…
> static int sharp_memory_update_display(struct sharp_memory_device *smd,
> +struct drm_framebuffer *fb,
> +struct drm_rect clip,
> +struct drm_format_conv_state
> *fmtcnv_state)
> +{
…
> + mutex_lock(&smd->tx_mutex);
> +
> + /* Populate the transmit buffer with frame data */
…
> + mutex_unlock(&smd->tx_mutex);
> +
> + return ret;
> +}
…
Under which circumstances would you become interested to apply a statement
like “guard(mutex)(&smd->tx_mutex);”?
https://elixir.bootlin.com/linux/v6.10.2/source/include/linux/mutex.h#L196
Regards,
Markus
Re: [PATCH v6 1/2] iommu/io-pgtable-arm: Add way to debug pgtable walk
Hi Rob,
On Wed, Jul 17, 2024 at 09:36:21AM -0700, Rob Clark wrote:
> From: Rob Clark
>
> Add an io-pgtable method to walk the pgtable returning the raw PTEs that
> would be traversed for a given iova access.
>
> Signed-off-by: Rob Clark
> Acked-by: Joerg Roedel
>
> ---
> drivers/iommu/io-pgtable-arm.c | 36 +-
> include/linux/io-pgtable.h | 17
> 2 files changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 3d23b924cec1..e70803940b46 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -690,9 +690,11 @@ static size_t arm_lpae_unmap_pages(struct io_pgtable_ops
> *ops, unsigned long iov
> data->start_level, ptep);
> }
>
> -static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
> - unsigned long iova)
> +static int arm_lpae_pgtable_walk(struct io_pgtable_ops *ops,
> + unsigned long iova,
> + void *_wd)
> {
> + struct arm_lpae_io_pgtable_walk_data *wd = _wd;
> struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
> arm_lpae_iopte pte, *ptep = data->pgd;
> int lvl = data->start_level;
> @@ -700,7 +702,7 @@ static phys_addr_t arm_lpae_iova_to_phys(struct
> io_pgtable_ops *ops,
> do {
> /* Valid IOPTE pointer? */
> if (!ptep)
> - return 0;
> + return -ENOENT;
>
> /* Grab the IOPTE we're interested in */
> ptep += ARM_LPAE_LVL_IDX(iova, lvl, data);
> @@ -708,22 +710,37 @@ static phys_addr_t arm_lpae_iova_to_phys(struct
> io_pgtable_ops *ops,
>
> /* Valid entry? */
> if (!pte)
> - return 0;
> + return -ENOENT;
>
> - /* Leaf entry? */
> + wd->ptes[wd->level++] = pte;
> +
> + /* Leaf entry? If so, we've found the translation */
> if (iopte_leaf(pte, lvl, data->iop.fmt))
> - goto found_translation;
> + return 0;
>
> /* Take it to the next level */
> ptep = iopte_deref(pte, data);
> } while (++lvl < ARM_LPAE_MAX_LEVELS);
>
> /* Ran out of page tables to walk */
> - return 0;
> + return -ENOENT;
> +}
> +
> +static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
> + unsigned long iova)
> +{
> + struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
> + struct arm_lpae_io_pgtable_walk_data wd = {};
> + int ret, lvl;
> +
> + ret = arm_lpae_pgtable_walk(ops, iova, &wd);
> + if (ret)
> + return 0;
> +
> + lvl = wd.level + data->start_level;
>
> -found_translation:
> iova &= (ARM_LPAE_BLOCK_SIZE(lvl, data) - 1);
> - return iopte_to_paddr(pte, data) | iova;
> + return iopte_to_paddr(wd.ptes[wd.level - 1], data) | iova;
> }
>
> static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg)
> @@ -804,6 +821,7 @@ arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg)
> .map_pages = arm_lpae_map_pages,
> .unmap_pages= arm_lpae_unmap_pages,
> .iova_to_phys = arm_lpae_iova_to_phys,
> + .pgtable_walk = arm_lpae_pgtable_walk,
> };
>
> return data;
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index 86cf1f7ae389..df6f6e58310c 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -171,12 +171,28 @@ struct io_pgtable_cfg {
> };
> };
>
> +/**
> + * struct arm_lpae_io_pgtable_walk_data - information from a pgtable walk
> + *
> + * @ptes: The recorded PTE values from the walk
> + * @level:The level of the last PTE
> + *
> + * @level also specifies the last valid index in @ptes
> + */
> +struct arm_lpae_io_pgtable_walk_data {
> + u64 ptes[4];
I don’t see a reason to save the whole walk for iova_to_phys,
I see that the DRM driver uses those next, but I am worried that
won’t scale, a callback mechanism sounds better.
Also, there is a page table walker recently added to io-pagtable-arm,
for dirty bit tracking:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/iommu/io-pgtable-arm.c?id=4fe88fd8b4aecb7f9680bf898811db76b94095a9
I’d suggest consolidating those into one walker where each caller
has its own logic in a callback.
Thanks,
Mostafa
> + int level;
> +};
> +
> /**
> * struct io_pgtable_ops - Page table manipulation API for IOMMU drivers.
> *
> * @map_pages:Map a physically contiguous range of pages of the same
> size.
> * @unmap_pages: Unmap a range of virtually contiguous pages of the same
> size.
> * @iova_to_phys: Translate iova to physical address.
> +
Re: [PATCH resend v4 00/11] Improve the copy of task comm
On Mon, Jul 29, 2024 at 5:29 PM Jani Nikula wrote:
>
> On Mon, 29 Jul 2024, Yafang Shao wrote:
> > Hello Andrew,
> >
> > Is it appropriate for you to apply this to the mm tree?
> >
> > Using {memcpy,strncpy,strcpy,kstrdup} to copy the task comm relies on the
> > length of task comm. Changes in the task comm could result in a destination
> > string that is overflow. Therefore, we should explicitly ensure the
> > destination
> > string is always NUL-terminated, regardless of the task comm. This approach
> > will facilitate future extensions to the task comm.
>
> Why are we normalizing calling double-underscore prefixed functions all
> over the place? i.e. __get_task_comm().
>
> get_task_comm() is widely used. At a glance, looks like it could be used
> in many of the patches here too.
There is a BUILD_BUG_ON() inside get_task_comm(), so when you use
get_task_comm(), it implies that the BUILD_BUG_ON() is necessary.
However, we don't want to impose this restriction on code where the
length can be dynamically changed.
One use case of get_task_comm() is in code that has already exposed
the length to userspace. In such cases, we specifically add the
BUILD_BUG_ON() to prevent developers from changing it. For more
information, see commit 95af469c4f60 ("fs/binfmt_elf: replace
open-coded string copy with get_task_comm").
--
Regards
Yafang
Re: [PATCH v3 2/2] drm/tiny: Add driver for Sharp Memory LCD
On Mon, Jul 29, 2024 at 01:00:47PM GMT, Markus Elfring wrote:
> …
> > +++ b/drivers/gpu/drm/tiny/sharp-memory.c
> > @@ -0,0 +1,684 @@
> …
> > static int sharp_memory_update_display(struct sharp_memory_device *smd,
> > + struct drm_framebuffer *fb,
> > + struct drm_rect clip,
> > + struct drm_format_conv_state
> > *fmtcnv_state)
> > +{
> …
> > + mutex_lock(&smd->tx_mutex);
> > +
> > + /* Populate the transmit buffer with frame data */
> …
> > + mutex_unlock(&smd->tx_mutex);
> > +
> > + return ret;
> > +}
> …
>
> Under which circumstances would you become interested to apply a statement
> like “guard(mutex)(&smd->tx_mutex);”?
> https://elixir.bootlin.com/linux/v6.10.2/source/include/linux/mutex.h#L196
>
Ah, I didn't realize guarded mutexes were implemented. That's really
good to know.
I'd usually use them when it's possible to return before mutex_unlock is
called to avoid goto cleanup logic. But it's probably best to just use them
by default.
Thanks for the review. Will clean up in v4.
Best regards,
Alex
Re: [PATCH v5 1/5] drm/msm/adreno: Implement SMEM-based speed bin
On 16.07.2024 1:56 PM, Konrad Dybcio wrote: > On 15.07.2024 10:04 PM, Akhil P Oommen wrote: >> On Tue, Jul 09, 2024 at 12:45:29PM +0200, Konrad Dybcio wrote: >>> On recent (SM8550+) Snapdragon platforms, the GPU speed bin data is >>> abstracted through SMEM, instead of being directly available in a fuse. >>> >>> Add support for SMEM-based speed binning, which includes getting >>> "feature code" and "product code" from said source and parsing them >>> to form something that lets us match OPPs against. >>> >>> Due to the product code being ignored in the context of Adreno on >>> production parts (as of SM8650), hardcode it to SOCINFO_PC_UNKNOWN. >>> >>> Signed-off-by: Konrad Dybcio >>> --- > > [...] > >>> >>> - if (adreno_read_speedbin(dev, &speedbin) || !speedbin) >>> + if (adreno_read_speedbin(adreno_gpu, dev, &speedbin) || !speedbin) >>> speedbin = 0x; >>> - adreno_gpu->speedbin = (uint16_t) (0x & speedbin); >>> + adreno_gpu->speedbin = speedbin; >> >> There are some chipsets which uses both Speedbin and Socinfo data for >> SKU detection [1]. > > 0_0 > > >> We don't need to worry about that logic for now. But >> I am worried about mixing Speedbin and SKU_ID in the UABI with this patch. >> It will be difficult when we have to expose both to userspace. >> >> I think we can use a separate bitfield to expose FCODE/PCODE. Currently, >> the lower 32 bit is reserved for chipid and 33-48 is reserved for speedbin, >> so I think we can use the rest of the 16 bits for SKU_ID. And within that >> 16bits, 12 bits should be sufficient for FCODE and the rest 8 bits >> reserved for future PCODE. > > Right, sounds reasonable. Hopefully nothing overflows.. +CC Elliot Would you know whether these sizes ^ are going to be sufficient for the foreseeable future? Konrad
[PATCH 1/1] MAINTAINERS: Change habanalabs maintainer
I will be leaving Intel soon, Yaron Avizrat will take the role of habanalabs driver maintainer. Signed-off-by: Ofir Bitton --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index ed2d2dbcec81..a4b36590061e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9599,7 +9599,7 @@ S:Maintained F: block/partitions/efi.* HABANALABS PCI DRIVER -M: Ofir Bitton +M: Yaron Avizrat L: dri-devel@lists.freedesktop.org S: Supported C: irc://irc.oftc.net/dri-devel -- 2.34.1
[PATCH 0/1] Update on habanalabs maintainer status
Hi Dave, Sima. As I am about to leave Intel during the next weeks, I'm stepping down from the maintainer role of the habanalabs driver. Yaron Avizrat from Intel will replace me as the new maintainer. Ofir Bitton (1): MAINTAINERS: Change habanalabs maintainer MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.34.1
Re: [PATCH v5 00/12] spi: add driver for Intel discrete graphics
On Mon, Jul 29, 2024 at 11:43:14AM +0300, Alexander Usyskin wrote: > V5: depend solely on AUXILIARY_BUS, not on COMPILE_TEST > disable spi driver on virtual function in Xe, no spi device there > V4: fix white-spaces > add check for discrete graphics missed in i915 intel_spi_fini > V3: rebase over drm-xe-next to enable CI run > V2: fix review comments > fix signatures order > depend spi presence in Xe on special flag, > as not all new discrete cards have such spi You're sending approximately one version of this large series a day. Please stop resending until it's in a reviewable state. signature.asc Description: PGP signature
Re: [PATCH v3 2/2] drm/tiny: Add driver for Sharp Memory LCD
On 29/07/2024 14:05, Alex Lanzano wrote: >> Under which circumstances would you become interested to apply a statement >> like “guard(mutex)(&smd->tx_mutex);”? >> https://elixir.bootlin.com/linux/v6.10.2/source/include/linux/mutex.h#L196 >> > > Ah, I didn't realize guarded mutexes were implemented. That's really > good to know. > > I'd usually use them when it's possible to return before mutex_unlock is > called to avoid goto cleanup logic. But it's probably best to just use them > by default. > > Thanks for the review. Will clean up in v4. Feel free to ignore all comments from Markus, regardless whether the suggestion is reasonable or not. This person is banned from LKML and several maintainers ignore Markus' feedback, because it is just a waste of time. Best regards, Krzysztof
Re: [PATCH] drm/ast: add multiple connectors support
Hi
Am 11.07.24 um 11:01 schrieb oushixiong1...@163.com:
From: Shixiong Ou
[WHY]
The AST2600 tx_chip_types will be detected as AST_TX_DP, but some BMC
boards that use AST2600 use the VGA interface instead of the DP interface.
In this case, it will use Virtual connector as the DP is disconnected.
[HOW]
Allows multiple physical connectors to exist at the same time.
I have another question about this patch. Is there a physical connector
for each type (VGA, DP) on your board? Or just one of them?
Best regards
Thomas
Signed-off-by: Shixiong Ou
---
drivers/gpu/drm/ast/ast_drv.h | 6 -
drivers/gpu/drm/ast/ast_main.c | 8 +++
drivers/gpu/drm/ast/ast_mode.c | 40 --
3 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index ba3d86973995..e326124b3fec 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -150,9 +150,13 @@ static inline struct ast_plane *to_ast_plane(struct
drm_plane *plane)
* BMC
*/
+#define MAX_CONNECTORS 2
+
struct ast_bmc_connector {
struct drm_connector base;
- struct drm_connector *physical_connector;
+
+ struct drm_connector *physical_connectors[MAX_CONNECTORS];
+ int count;
};
static inline struct ast_bmc_connector *
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 0637abb70361..428529749ae6 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -85,7 +85,7 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool
need_post)
if (!need_post) {
jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xa3, 0xff);
if (jreg & 0x80)
- ast->tx_chip_types = AST_TX_SIL164_BIT;
+ ast->tx_chip_types |= AST_TX_SIL164_BIT;
}
if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast)) {
@@ -97,7 +97,7 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool
need_post)
jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, 0xff);
switch (jreg) {
case 0x04:
- ast->tx_chip_types = AST_TX_SIL164_BIT;
+ ast->tx_chip_types |= AST_TX_SIL164_BIT;
break;
case 0x08:
ast->dp501_fw_addr = drmm_kzalloc(dev, 32*1024,
GFP_KERNEL);
@@ -110,12 +110,12 @@ static void ast_detect_tx_chip(struct ast_device *ast,
bool need_post)
}
fallthrough;
case 0x0c:
- ast->tx_chip_types = AST_TX_DP501_BIT;
+ ast->tx_chip_types |= AST_TX_DP501_BIT;
}
} else if (IS_AST_GEN7(ast)) {
if (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1,
TX_TYPE_MASK) ==
ASTDP_DPMCU_TX) {
- ast->tx_chip_types = AST_TX_ASTDP_BIT;
+ ast->tx_chip_types |= AST_TX_ASTDP_BIT;
ast_dp_launch(&ast->base);
}
}
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 6695af70768f..31a49d32e506 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1717,7 +1717,8 @@ static int ast_bmc_connector_helper_detect_ctx(struct
drm_connector *connector,
bool force)
{
struct ast_bmc_connector *bmc_connector =
to_ast_bmc_connector(connector);
- struct drm_connector *physical_connector =
bmc_connector->physical_connector;
+ struct drm_connector *physical_connector;
+ int i, count = bmc_connector->count;
/*
* Most user-space compositors cannot handle more than one connected
@@ -1730,10 +1731,13 @@ static int ast_bmc_connector_helper_detect_ctx(struct
drm_connector *connector,
*than one connector per CRTC. The BMC should always be
connected.
*/
- if (physical_connector && physical_connector->status == connector_status_disconnected)
- return connector_status_connected;
+ for (i = 0; i < count; i++) {
+ physical_connector = bmc_connector->physical_connectors[i];
+ if (physical_connector && physical_connector->status ==
connector_status_connected)
+ return connector_status_disconnected;
+ }
- return connector_status_disconnected;
+ return connector_status_connected;
}
static int ast_bmc_connector_helper_get_modes(struct drm_connector *connector)
@@ -1756,10 +1760,11 @@ static const struct drm_connector_funcs
ast_bmc_connector_funcs = {
static int ast_bmc_connector_init(struct drm_device *dev,
struct ast_bmc_connector *bmc_connector,
- struct drm_connector *phys
Re: [PATCH 0/5] drm/ast: Fix DP hotplugging and clean up
Ping for review Am 17.07.24 um 16:24 schrieb Thomas Zimmermann: Here are a number of updates for ast's ASTDP transmitter code. So far the ast driver required the DisplayPort to be connected at boot. Later detection was not supported. Re-connecting the cable was also not supported. Once atomic_disable powered off the physical ASTDP connector, there was no way of detecting a conencted display. Patch 1 makes Hot Plug Detection work. If ncesessary, the connector's detect helper powers up the physical connector to read the HPD status. That's a good oportunity to clean up ast's whole detection code for ASTDP transmitters. So patch 2 to 4 remove duplicated status tests throughout the ASTDP code. Patch 5 simplified the code for reading the display's EDID data from the firmware. Tested on AST2600 hardware with an ASTDP transmitter. Thomas Zimmermann (5): drm/ast: astdp: Wake up during connector status detection drm/ast: astdp: Test firmware status once during probing drm/ast: astdp: Only test HDP state in ast_astdp_is_connected() drm/ast: astdp: Perform link training during atomic_enable drm/ast: astdp: Clean up EDID reading drivers/gpu/drm/ast/ast_dp.c | 186 +++-- drivers/gpu/drm/ast/ast_drv.h | 4 +- drivers/gpu/drm/ast/ast_main.c | 6 +- drivers/gpu/drm/ast/ast_mode.c | 31 +- drivers/gpu/drm/ast/ast_post.c | 2 +- drivers/gpu/drm/ast/ast_reg.h | 22 ++-- 6 files changed, 126 insertions(+), 125 deletions(-) -- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Re: [PATCH] drm/ast: add multiple connectors support
Hi
Am 11.07.24 um 11:01 schrieb oushixiong1...@163.com:
From: Shixiong Ou
[WHY]
The AST2600 tx_chip_types will be detected as AST_TX_DP, but some BMC
boards that use AST2600 use the VGA interface instead of the DP interface.
In this case, it will use Virtual connector as the DP is disconnected.
[HOW]
Allows multiple physical connectors to exist at the same time.
And another question: does the patch series at
https://patchwork.freedesktop.org/series/136198/
fix the problem?
Best regards
Thomas
Signed-off-by: Shixiong Ou
---
drivers/gpu/drm/ast/ast_drv.h | 6 -
drivers/gpu/drm/ast/ast_main.c | 8 +++
drivers/gpu/drm/ast/ast_mode.c | 40 --
3 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index ba3d86973995..e326124b3fec 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -150,9 +150,13 @@ static inline struct ast_plane *to_ast_plane(struct
drm_plane *plane)
* BMC
*/
+#define MAX_CONNECTORS 2
+
struct ast_bmc_connector {
struct drm_connector base;
- struct drm_connector *physical_connector;
+
+ struct drm_connector *physical_connectors[MAX_CONNECTORS];
+ int count;
};
static inline struct ast_bmc_connector *
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 0637abb70361..428529749ae6 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -85,7 +85,7 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool
need_post)
if (!need_post) {
jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xa3, 0xff);
if (jreg & 0x80)
- ast->tx_chip_types = AST_TX_SIL164_BIT;
+ ast->tx_chip_types |= AST_TX_SIL164_BIT;
}
if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast)) {
@@ -97,7 +97,7 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool
need_post)
jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, 0xff);
switch (jreg) {
case 0x04:
- ast->tx_chip_types = AST_TX_SIL164_BIT;
+ ast->tx_chip_types |= AST_TX_SIL164_BIT;
break;
case 0x08:
ast->dp501_fw_addr = drmm_kzalloc(dev, 32*1024,
GFP_KERNEL);
@@ -110,12 +110,12 @@ static void ast_detect_tx_chip(struct ast_device *ast,
bool need_post)
}
fallthrough;
case 0x0c:
- ast->tx_chip_types = AST_TX_DP501_BIT;
+ ast->tx_chip_types |= AST_TX_DP501_BIT;
}
} else if (IS_AST_GEN7(ast)) {
if (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1,
TX_TYPE_MASK) ==
ASTDP_DPMCU_TX) {
- ast->tx_chip_types = AST_TX_ASTDP_BIT;
+ ast->tx_chip_types |= AST_TX_ASTDP_BIT;
ast_dp_launch(&ast->base);
}
}
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 6695af70768f..31a49d32e506 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1717,7 +1717,8 @@ static int ast_bmc_connector_helper_detect_ctx(struct
drm_connector *connector,
bool force)
{
struct ast_bmc_connector *bmc_connector =
to_ast_bmc_connector(connector);
- struct drm_connector *physical_connector =
bmc_connector->physical_connector;
+ struct drm_connector *physical_connector;
+ int i, count = bmc_connector->count;
/*
* Most user-space compositors cannot handle more than one connected
@@ -1730,10 +1731,13 @@ static int ast_bmc_connector_helper_detect_ctx(struct
drm_connector *connector,
*than one connector per CRTC. The BMC should always be
connected.
*/
- if (physical_connector && physical_connector->status == connector_status_disconnected)
- return connector_status_connected;
+ for (i = 0; i < count; i++) {
+ physical_connector = bmc_connector->physical_connectors[i];
+ if (physical_connector && physical_connector->status ==
connector_status_connected)
+ return connector_status_disconnected;
+ }
- return connector_status_disconnected;
+ return connector_status_connected;
}
static int ast_bmc_connector_helper_get_modes(struct drm_connector *connector)
@@ -1756,10 +1760,11 @@ static const struct drm_connector_funcs
ast_bmc_connector_funcs = {
static int ast_bmc_connector_init(struct drm_device *dev,
struct ast_bmc_connector *bmc_connector,
- struct drm_connector *physical_connector)
+
[PATCH] drm/nouveau: remove unused variable ret
Fix build with CONFIG_NOUVEAU_PLATFORM_DRIVER enabled:
../drivers/gpu/drm/nouveau/nouveau_platform.c: In function
‘nouveau_platform_probe’:
../drivers/gpu/drm/nouveau/nouveau_platform.c:29:13: error: unused variable
‘ret’ [-Werror=unused-variable]
29 | int ret;
| ^~~
Fixes: 961ae5f9807b ("drm/nouveau: handle pci/tegra drm_dev_{alloc, register}
from common code")
Cc: Ben Skeggs
Cc: Danilo Krummrich
Signed-off-by: Jani Nikula
---
Cc: Karol Herbst
Cc: Lyude Paul
Cc: Danilo Krummrich
Cc: dri-devel@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
---
drivers/gpu/drm/nouveau/nouveau_platform.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c
b/drivers/gpu/drm/nouveau/nouveau_platform.c
index 3194b110eff8..829fdc6e4031 100644
--- a/drivers/gpu/drm/nouveau/nouveau_platform.c
+++ b/drivers/gpu/drm/nouveau/nouveau_platform.c
@@ -26,7 +26,6 @@ static int nouveau_platform_probe(struct platform_device
*pdev)
const struct nvkm_device_tegra_func *func;
struct nvkm_device *device = NULL;
struct drm_device *drm;
- int ret;
func = of_device_get_match_data(&pdev->dev);
--
2.39.2
Re: [PATCH v5 1/5] drm/msm/adreno: Implement SMEM-based speed bin
On 29.07.2024 2:13 PM, Konrad Dybcio wrote: > On 16.07.2024 1:56 PM, Konrad Dybcio wrote: >> On 15.07.2024 10:04 PM, Akhil P Oommen wrote: >>> On Tue, Jul 09, 2024 at 12:45:29PM +0200, Konrad Dybcio wrote: On recent (SM8550+) Snapdragon platforms, the GPU speed bin data is abstracted through SMEM, instead of being directly available in a fuse. Add support for SMEM-based speed binning, which includes getting "feature code" and "product code" from said source and parsing them to form something that lets us match OPPs against. Due to the product code being ignored in the context of Adreno on production parts (as of SM8650), hardcode it to SOCINFO_PC_UNKNOWN. Signed-off-by: Konrad Dybcio --- >> [...] >> - if (adreno_read_speedbin(dev, &speedbin) || !speedbin) + if (adreno_read_speedbin(adreno_gpu, dev, &speedbin) || !speedbin) speedbin = 0x; - adreno_gpu->speedbin = (uint16_t) (0x & speedbin); + adreno_gpu->speedbin = speedbin; >>> There are some chipsets which uses both Speedbin and Socinfo data for >>> SKU detection [1]. >> 0_0 >> >> >>> We don't need to worry about that logic for now. But >>> I am worried about mixing Speedbin and SKU_ID in the UABI with this patch. >>> It will be difficult when we have to expose both to userspace. >>> >>> I think we can use a separate bitfield to expose FCODE/PCODE. Currently, >>> the lower 32 bit is reserved for chipid and 33-48 is reserved for speedbin, >>> so I think we can use the rest of the 16 bits for SKU_ID. And within that >>> 16bits, 12 bits should be sufficient for FCODE and the rest 8 bits >>> reserved for future PCODE. >> Right, sounds reasonable. Hopefully nothing overflows.. > +CC Elliot > > Would you know whether these sizes ^ are going to be sufficient for > the foreseeable future? Also Akhil, 12 + 8 > 16.. did you mean 8 bits for both P and FCODE? Or 12 for FCODE and 4 for PCODE? Konrad
Re: [PATCH v3 0/2] Basic support for TI TDP158
On 27/06/2024 13:13, Marc Gonzalez wrote: > Changes in v3: > - Add 'select DRM_PANEL_BRIDGE' in driver Kconfig > - Fix checkpatch errors > - log errors using dev_err (so save dev pointer) > - expand a few error messages > - expand commit messages with info from the datasheet > - mark regulators as required in the DT binding > - Link to v2: > https://lore.kernel.org/r/20240625-tdp158-v2-0-a3b344707...@freebox.fr Series has been rebased on top of v6.11-rc1 Current changelog is: Changes in v4: - Add blurb about I2C vs pin strap mode in cover letter - Add blurb about I2C vs pin strap mode in binding commit message - Add blurb about I2C mode in driver commit message - Add comment in binding explaining when reg is required - Add comment in binding describing Operation Enable / Reset Pin - Link to v3: https://lore.kernel.org/r/20240627-tdp158-v3-0-fb2fbc808...@freebox.fr For reference, binding commit message blurb: Like the TFP410, the TDP158 can be set up in 2 different ways: 1) hard-coding its configuration settings using pin-strapping resistors 2) placing it on an I2C bus, and defer set-up until run-time The mode is selected via pin 8 = I2C_EN I2C_EN high = I2C Control Mode I2C_EN low = Pin Strap Mode On our board, I2C_EN is pulled high. driver commit message blurb: On our board, I2C_EN is pulled high. Thus, this code defines a module_i2c_driver. The default settings work fine for our use-case. So this basic driver doesn't need to tweak any I2C registers. binding comments: +# The reg property is required if and only if the device is connected +# to an I2C bus. In pin strap mode, reg must not be specified. + reg: +description: I2C address of the device + +# Pin 36 = Operation Enable / Reset Pin +# OE = L: Power Down Mode +# OE = H: Normal Operation +# Internal weak pullup: device resets on H to L transitions + enable-gpios: +description: GPIO controlling bridge enable I plan to send a formal v4 in a few hours. Regards
[PATCH] MAINTAINERS: update email for Konrad Dybcio
The old email address bounced. I found the newer one in MAINTAINERS, so update entries accordingly. Cc: Konrad Dybcio Signed-off-by: Wolfram Sang --- Against v6.11-rc1. Still needs ack from Konrad Dybcio MAINTAINERS | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 42decde38320..7b599269a821 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2745,7 +2745,7 @@ F:include/linux/soc/qcom/ ARM/QUALCOMM SUPPORT M: Bjorn Andersson -M: Konrad Dybcio +M: Konrad Dybcio L: linux-arm-...@vger.kernel.org S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git @@ -7106,7 +7106,7 @@ F:drivers/gpu/drm/tiny/panel-mipi-dbi.c DRM DRIVER for Qualcomm Adreno GPUs M: Rob Clark R: Sean Paul -R: Konrad Dybcio +R: Konrad Dybcio L: linux-arm-...@vger.kernel.org L: dri-devel@lists.freedesktop.org L: freedr...@lists.freedesktop.org @@ -18771,7 +18771,7 @@ F: include/uapi/drm/qaic_accel.h QUALCOMM CORE POWER REDUCTION (CPR) AVS DRIVER M: Bjorn Andersson -M: Konrad Dybcio +M: Konrad Dybcio L: linux...@vger.kernel.org L: linux-arm-...@vger.kernel.org S: Maintained -- 2.43.0
Re: [PATCH] MAINTAINERS: update email for Konrad Dybcio
Hi Konrad, > Already sent a series of fixups, but thanks for keeping track Welcome. Cool that you are at it! Happy hacking, Wolfram signature.asc Description: PGP signature
Re: [PATCH] drm/nouveau: remove unused variable ret
Hi Jani,
On Mon, Jul 29, 2024 at 03:36:24PM +0300, Jani Nikula wrote:
> Fix build with CONFIG_NOUVEAU_PLATFORM_DRIVER enabled:
>
> ../drivers/gpu/drm/nouveau/nouveau_platform.c: In function
> ‘nouveau_platform_probe’:
> ../drivers/gpu/drm/nouveau/nouveau_platform.c:29:13: error: unused variable
> ‘ret’ [-Werror=unused-variable]
>29 | int ret;
> | ^~~
>
> Fixes: 961ae5f9807b ("drm/nouveau: handle pci/tegra drm_dev_{alloc, register}
> from common code")
> Cc: Ben Skeggs
> Cc: Danilo Krummrich
> Signed-off-by: Jani Nikula
Thanks for fixing this, applied to drm-misc-next.
>
> ---
>
> Cc: Karol Herbst
> Cc: Lyude Paul
> Cc: Danilo Krummrich
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> ---
> drivers/gpu/drm/nouveau/nouveau_platform.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c
> b/drivers/gpu/drm/nouveau/nouveau_platform.c
> index 3194b110eff8..829fdc6e4031 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_platform.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_platform.c
> @@ -26,7 +26,6 @@ static int nouveau_platform_probe(struct platform_device
> *pdev)
> const struct nvkm_device_tegra_func *func;
> struct nvkm_device *device = NULL;
> struct drm_device *drm;
> - int ret;
>
> func = of_device_get_match_data(&pdev->dev);
>
> --
> 2.39.2
>
Re: [PATCH] drm/xe/oa/uapi: Make bit masks unsigned
On Mon, Jul 29, 2024 at 11:26:34AM GMT, Geert Uytterhoeven wrote:
When building with gcc-5:
In function ‘decode_oa_format.isra.26’,
inlined from ‘xe_oa_set_prop_oa_format’ at
drivers/gpu/drm/xe/xe_oa.c:1664:6:
././include/linux/compiler_types.h:510:38: error: call to
‘__compiletime_assert_1336’ declared with attribute error: FIELD_GET: mask is
not constant
[...]
./include/linux/bitfield.h:155:3: note: in expansion of macro
‘__BF_FIELD_CHECK’
__BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
^
drivers/gpu/drm/xe/xe_oa.c:1573:18: note: in expansion of macro ‘FIELD_GET’
u32 bc_report = FIELD_GET(DRM_XE_OA_FORMAT_MASK_BC_REPORT, fmt);
^
Fixes: b6fd51c6211910b1 ("drm/xe/oa/uapi: Define and parse OA stream
properties")
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Lucas De Marchi
That fixes the build, but question to Ashutosh: it's odd to tie the
format to a bspec. What happens on next platform if the HW changes?
Hopefully it doesn't change in an incompatible way, but looking at this
code it seems we could still keep the uapi by untying the HW from the
uapi in the documentation.
Lucas De Marchi
---
Compile-tested only.
---
include/uapi/drm/xe_drm.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
index 19619d4952a863f7..db232a25189eba9f 100644
--- a/include/uapi/drm/xe_drm.h
+++ b/include/uapi/drm/xe_drm.h
@@ -1590,10 +1590,10 @@ enum drm_xe_oa_property_id {
* b. Counter select c. Counter size and d. BC report. Also refer to the
* oa_formats array in drivers/gpu/drm/xe/xe_oa.c.
*/
-#define DRM_XE_OA_FORMAT_MASK_FMT_TYPE (0xff << 0)
-#define DRM_XE_OA_FORMAT_MASK_COUNTER_SEL (0xff << 8)
-#define DRM_XE_OA_FORMAT_MASK_COUNTER_SIZE (0xff << 16)
-#define DRM_XE_OA_FORMAT_MASK_BC_REPORT(0xff << 24)
+#define DRM_XE_OA_FORMAT_MASK_FMT_TYPE (0xffu << 0)
+#define DRM_XE_OA_FORMAT_MASK_COUNTER_SEL (0xffu << 8)
+#define DRM_XE_OA_FORMAT_MASK_COUNTER_SIZE (0xffu << 16)
+#define DRM_XE_OA_FORMAT_MASK_BC_REPORT(0xffu << 24)
/**
* @DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT: Requests periodic OA unit
--
2.34.1
Re: [PATCH v4 01/11] drm/amd/display: clean unused variables for hdmi freesync parser
On 2024-07-28 19:32, Melissa Wen wrote:
On 07/25, Alex Hung wrote:
Hi Melissa,
There are no commit messages in this patch.
Also, do you think this can be merged with Patch 5 "drm/amd/display: remove
redundant freesync parser for DP"?
Hi Alex,
Thanks for your feedback.
I'll add a brief description in the next version.
Regarding merging it into patch 5, I'd prefer to keep it detached
because here we have a non-functional change. I can send it as a
separate, single patch from this series to reduce noise and make
validation faster. WDYT?
Make thanks. Thanks for clarification.
Melissa
On 2024-07-05 21:35, Melissa Wen wrote:
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 98cf523a629e..1dfa7ec9af35 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -12108,9 +12108,6 @@ void amdgpu_dm_update_freesync_caps(struct
drm_connector *connector,
} else if (edid && sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A) {
i = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
if (i >= 0 && vsdb_info.freesync_supported) {
- timing = &edid->detailed_timings[i];
- data= &timing->data.other_data;
-
amdgpu_dm_connector->min_vfreq =
vsdb_info.min_refresh_rate_hz;
amdgpu_dm_connector->max_vfreq =
vsdb_info.max_refresh_rate_hz;
if (amdgpu_dm_connector->max_vfreq -
amdgpu_dm_connector->min_vfreq > 10)
Re: [PATCH v4 09/11] drm/amd/display: get SAD from drm_eld when parsing EDID caps
On 2024-07-28 20:02, Melissa Wen wrote:
On 07/25, Alex Hung wrote:
On 2024-07-05 21:35, Melissa Wen wrote:
instead of parsing struct edid.
A more informative commit message will be helpful.
sure. I'll improve it in the next version.
A soft reminder - a few other patches need improved commit messages too.
Signed-off-by: Melissa Wen
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 7657b1051c54..45c04de08c65 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -97,7 +97,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
const struct drm_edid *drm_edid = aconnector->drm_edid;
struct drm_edid_product_id product_id;
struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
- struct cea_sad *sads;
int sad_count, sadb_count;
int i = 0;
uint8_t *sadb = NULL;
@@ -127,18 +126,21 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
apply_edid_quirks(&product_id, edid_caps);
- sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
+ sad_count = drm_eld_sad_count(connector->eld);
if (sad_count <= 0)
return result;
edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
for (i = 0; i < edid_caps->audio_mode_count; ++i) {
- struct cea_sad *sad = &sads[i];
+ struct cea_sad sad;
- edid_caps->audio_modes[i].format_code = sad->format;
- edid_caps->audio_modes[i].channel_count = sad->channels + 1;
- edid_caps->audio_modes[i].sample_rate = sad->freq;
- edid_caps->audio_modes[i].sample_size = sad->byte2;
+ if (drm_eld_sad_get(connector->eld, i, &sad) < 0)
+ continue;
+
+ edid_caps->audio_modes[i].format_code = sad.format;
+ edid_caps->audio_modes[i].channel_count = sad.channels + 1;
+ edid_caps->audio_modes[i].sample_rate = sad.freq;
+ edid_caps->audio_modes[i].sample_size = sad.byte2;
}
sadb_count = drm_edid_to_speaker_allocation((struct edid *) edid->raw_edid,
&sadb);
@@ -153,7 +155,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
else
edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
- kfree(sads);
kfree(sadb);
return result;
Re: [PATCH v2] drm/amd/amdgpu: Properly tune the size of struct
On Mon, Jul 29, 2024 at 10:00:00PM +0800, WangYuli wrote:
> The struct assertion is failed because sparse cannot parse
> `#pragma pack(push, 1)` and `#pragma pack(pop)` correctly.
> GCC's output is still 1-byte-aligned. No harm to memory layout.
>
> The error can be filtered out by sparse-diff, but sometimes
> multiple lines queezed into one, making the sparse-diff thinks
> its a new error. I'm trying to aviod this by fixing errors.
>
> Link: https://lore.kernel.org/all/20230620045919.492128-1-su...@nfschina.com/
> Fixes: 1721bc1b2afa ("drm/amdgpu: Update VF2PF interface")
> Signed-off-by: Su Hui
> Signed-off-by: Dan Carpenter
> Signed-off-by: wenlunpeng
> Signed-off-by: WangYuli
> ---
Thanks for doing this, but these Signed-off-by tags aren't correct.
Signed-off-by is like signing a legal document. It came out of the SCO
lawsuits. SCO was a scam where SCO claimed that Linux stole their
source code and tried to get Linux users to pay licensing fees. (No one
stole anything and SCO didn't even own the copyrights to Unix, those
belonged to IBM).
You could maybe use the Reported-by: or Suggested-by: tags for Su Hui.
But the rest of us could just be Cc:
Cc: Dan Carpenter
Cc: wenlunpeng
Reported-by: Su Hui
Signed-off-by: WangYuli
regards,
dan carpenter
Re: [PATCH v5 1/5] drm/msm/adreno: Implement SMEM-based speed bin
On Mon, Jul 29, 2024 at 02:40:30PM +0200, Konrad Dybcio wrote: > > > On 29.07.2024 2:13 PM, Konrad Dybcio wrote: > > On 16.07.2024 1:56 PM, Konrad Dybcio wrote: > >> On 15.07.2024 10:04 PM, Akhil P Oommen wrote: > >>> On Tue, Jul 09, 2024 at 12:45:29PM +0200, Konrad Dybcio wrote: > On recent (SM8550+) Snapdragon platforms, the GPU speed bin data is > abstracted through SMEM, instead of being directly available in a fuse. > > Add support for SMEM-based speed binning, which includes getting > "feature code" and "product code" from said source and parsing them > to form something that lets us match OPPs against. > > Due to the product code being ignored in the context of Adreno on > production parts (as of SM8650), hardcode it to SOCINFO_PC_UNKNOWN. > > Signed-off-by: Konrad Dybcio > --- > >> [...] > >> > > -if (adreno_read_speedbin(dev, &speedbin) || !speedbin) > +if (adreno_read_speedbin(adreno_gpu, dev, &speedbin) || > !speedbin) > speedbin = 0x; > -adreno_gpu->speedbin = (uint16_t) (0x & speedbin); > +adreno_gpu->speedbin = speedbin; > >>> There are some chipsets which uses both Speedbin and Socinfo data for > >>> SKU detection [1]. > >> 0_0 > >> > >> > >>> We don't need to worry about that logic for now. But > >>> I am worried about mixing Speedbin and SKU_ID in the UABI with this patch. > >>> It will be difficult when we have to expose both to userspace. > >>> > >>> I think we can use a separate bitfield to expose FCODE/PCODE. Currently, > >>> the lower 32 bit is reserved for chipid and 33-48 is reserved for > >>> speedbin, > >>> so I think we can use the rest of the 16 bits for SKU_ID. And within that > >>> 16bits, 12 bits should be sufficient for FCODE and the rest 8 bits > >>> reserved for future PCODE. > >> Right, sounds reasonable. Hopefully nothing overflows.. > > +CC Elliot > > > > Would you know whether these sizes ^ are going to be sufficient for > > the foreseeable future? > > Also Akhil, 12 + 8 > 16.. did you mean 8 bits for both P and FCODE? Or > 12 for FCODE and 4 for PCODE? Sorry, "8 bits" was a typo. You are right, 12 bits for Fcode and 4 bits for PCODE. -Akhil > > Konrad
Re: [PATCH 3/3] dt-bindings: Batch-update Konrad Dybcio's email
On Fri, 26 Jul 2024 13:18:25 +0200, Konrad Dybcio wrote: > Use my @kernel.org address everywhere. > > Signed-off-by: Konrad Dybcio > --- > Documentation/devicetree/bindings/clock/qcom,dispcc-sm6350.yaml | 2 > +- > Documentation/devicetree/bindings/clock/qcom,gcc-msm8994.yaml | 2 > +- > Documentation/devicetree/bindings/clock/qcom,gcc-sm6125.yaml| 2 > +- > Documentation/devicetree/bindings/clock/qcom,gcc-sm6350.yaml| 2 > +- > Documentation/devicetree/bindings/clock/qcom,sm6115-gpucc.yaml | 2 > +- > Documentation/devicetree/bindings/clock/qcom,sm6125-gpucc.yaml | 2 > +- > Documentation/devicetree/bindings/clock/qcom,sm6350-camcc.yaml | 2 > +- > Documentation/devicetree/bindings/clock/qcom,sm6375-dispcc.yaml | 2 > +- > Documentation/devicetree/bindings/clock/qcom,sm6375-gcc.yaml| 2 > +- > Documentation/devicetree/bindings/clock/qcom,sm6375-gpucc.yaml | 2 > +- > Documentation/devicetree/bindings/clock/qcom,sm8350-videocc.yaml| 2 > +- > Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml | 2 > +- > Documentation/devicetree/bindings/display/msm/qcom,sm6375-mdss.yaml | 2 > +- > .../devicetree/bindings/display/panel/asus,z00t-tm5p5-nt35596.yaml | 2 > +- > Documentation/devicetree/bindings/display/panel/sony,td4353-jdi.yaml| 2 > +- > Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml| 2 > +- > Documentation/devicetree/bindings/interconnect/qcom,sc8280xp-rpmh.yaml | 2 > +- > Documentation/devicetree/bindings/interconnect/qcom,sm8450-rpmh.yaml| 2 > +- > Documentation/devicetree/bindings/iommu/qcom,iommu.yaml | 2 > +- > Documentation/devicetree/bindings/pinctrl/qcom,mdm9607-tlmm.yaml| 2 > +- > Documentation/devicetree/bindings/pinctrl/qcom,sm6350-tlmm.yaml | 2 > +- > Documentation/devicetree/bindings/pinctrl/qcom,sm6375-tlmm.yaml | 2 > +- > Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml | 2 > +- > Documentation/devicetree/bindings/soc/qcom/qcom,rpm-master-stats.yaml | 2 > +- > 24 files changed, 24 insertions(+), 24 deletions(-) > Applied, thanks!
Re: [PATCH] fbdev/hpfb: Fix an error handling path in hpfb_dio_probe()
On Mon, Jul 29, 2024 at 10:13:17AM +0200, Helge Deller wrote:
> On 7/28/24 20:29, Christophe JAILLET wrote:
> > If an error occurs after request_mem_region(), a corresponding
> > release_mem_region() should be called, as already done in the remove
> > function.
>
> True.
>
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>
> I think we can drop this "Fixes" tag, as it gives no real info.
>
If we're backporting patches then these tags really are useful. As
I've been doing more and more backporting, I've come to believe this
more firmly.
I don't necessarily think this patch is worth backporting, but leaving
the Fixes tag off doesn't mean it won't happen. People quite often
leave the Fixes tags off of real fixes by mistake so AUTOSEL could still
pick it up. You'd have to add:
Cc: # reason goes here, and must be present
> > Signed-off-by: Christophe JAILLET
> > ---
> > *Not* even compile tested only.
>
> Ok.
>
> > I don't know on what architecture it relies on.
>
> HP300 are old HP machines with an m68k CPU.
> Not sure if someone still has such a machine :-)
>
It surprised me how many patches we backport for ancient stuff. But I
guess the risk/reward equation still works because if the code isn't
used there the risk is very small.
regards,
dan carpenter
[PATCH v2] drm/amd/amdgpu: Properly tune the size of struct
The struct assertion is failed because sparse cannot parse
`#pragma pack(push, 1)` and `#pragma pack(pop)` correctly.
GCC's output is still 1-byte-aligned. No harm to memory layout.
The error can be filtered out by sparse-diff, but sometimes
multiple lines queezed into one, making the sparse-diff thinks
its a new error. I'm trying to aviod this by fixing errors.
Link: https://lore.kernel.org/all/20230620045919.492128-1-su...@nfschina.com/
Fixes: 1721bc1b2afa ("drm/amdgpu: Update VF2PF interface")
Signed-off-by: Su Hui
Signed-off-by: Dan Carpenter
Signed-off-by: wenlunpeng
Signed-off-by: WangYuli
---
drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
index fb2b394bb9c5..6e9eeaeb3de1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
@@ -213,7 +213,7 @@ struct amd_sriov_msg_pf2vf_info {
uint32_t gpu_capacity;
/* reserved */
uint32_t reserved[256 - AMD_SRIOV_MSG_PF2VF_INFO_FILLED_SIZE];
-};
+} __packed;
struct amd_sriov_msg_vf2pf_info_header {
/* the total structure size in byte */
@@ -273,7 +273,7 @@ struct amd_sriov_msg_vf2pf_info {
uint32_t mes_info_size;
/* reserved */
uint32_t reserved[256 - AMD_SRIOV_MSG_VF2PF_INFO_FILLED_SIZE];
-};
+} __packed;
/* mailbox message send from guest to host */
enum amd_sriov_mailbox_request_message {
--
2.43.4
Re: [Linux-stm32] [PATCH RESEND v3 0/3] Update STM DSI PHY driver
On Fri, Jul 26, 2024 at 09:55:35AM +0200, Philippe CORNU wrote: > > > On 7/22/24 10:38, Yanjun Yang wrote: > > > > This patch (commit id:185f99b614427360) seems to break the dsi of > > stm32f469 chip. > > I'm not familiar with the drm and the clock framework, maybe it's > > because there is no > > "ck_dsi_phy" defined for stm32f469. > > PS: Sorry for receiving multiple copies of this email, I forgot to > > use plain text mode last time. > > > > Hi, > Thank you for letting us know that there was this error. We should have > detected this before merging, really sorry for the problems caused by this > patch. We will investigate the issue and get back to you as soon as > possible. In the meantime, I think you can revert this patch in your git > tree. > > Philippe :-) > Hi, After some testing, the reason behind my problem is the parent's name of 'clk_dsi_phy' for stm32f4 is 'clk-hse' other than 'ck_hse'. I don't know which is the better why to fix it: 1. Change "ck_hse" to "clk-hse" in where "clk_dsi_phy" is defined. 2. Use "pll_in_khz = clk_get_rate(dsi->pllref_clk) / 1000" instead of "pll_in_khz = (unsigned int)(parent_rate / 1000)" when get the clock rate. Both method can fix my problem. The first one might break other platforms. Maybe I should change the clock name of 'clk-hse'. However, I can't find the defination of this clock name for stm32f4.
[PATCH] drm/amd/amdgpu: Properly tune the size of struct
The struct assertion is failed because sparse cannot parse
`#pragma pack(push, 1)` and `#pragma pack(pop)` correctly.
GCC's output is still 1-byte-aligned. No harm to memory layout.
The error can be filtered out by sparse-diff, but sometimes
multiple lines queezed into one, making the sparse-diff thinks
its a new error. I'm trying to aviod this by fixing errors.
Link: https://lore.kernel.org/all/20230620045919.492128-1-su...@nfschina.com/
Fixes: 1721bc1b2afa ("drm/amdgpu: Update VF2PF interface")
Signed-off-by: Su Hui
Signed-off-by: Dan Carpenter
Signed-off-by: wenlunpeng
Signed-off-by: WangYuli
---
drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
index fb2b394bb9c5..6e9eeaeb3de1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
@@ -213,7 +213,7 @@ struct amd_sriov_msg_pf2vf_info {
uint32_t gpu_capacity;
/* reserved */
uint32_t reserved[256 - AMD_SRIOV_MSG_PF2VF_INFO_FILLED_SIZE];
-};
+} __packed;
struct amd_sriov_msg_vf2pf_info_header {
/* the total structure size in byte */
@@ -273,7 +273,7 @@ struct amd_sriov_msg_vf2pf_info {
uint32_t mes_info_size;
/* reserved */
uint32_t reserved[256 - AMD_SRIOV_MSG_VF2PF_INFO_FILLED_SIZE];
-};
+} __packed;
/* mailbox message send from guest to host */
enum amd_sriov_mailbox_request_message {
--
2.43.4
Re: [PATCH] MAINTAINERS: update email for Konrad Dybcio
On 29.07.2024 2:51 PM, Wolfram Sang wrote: > The old email address bounced. I found the newer one in MAINTAINERS, > so update entries accordingly. > > Cc: Konrad Dybcio > Signed-off-by: Wolfram Sang > --- Already sent a series of fixups, but thanks for keeping track https://lore.kernel.org/linux-arm-msm/39a2303c-c89c-4fa3-a2e3-87589d242...@kernel.org/T/#me914f204e70ab34dd8bc3e6cbb51747490a81817
Re: [REGRESSION] No image on 4k display port displays connected through usb-c dock in kernel 6.10
July 29, 2024 at 11:15 AM, "Linux regression tracking (Thorsten Leemhuis)"
wrote:
>
> On 29.07.24 10:47, Christian Heusel wrote:
>
> >
> > On 24/07/29 10:35AM, Linux regression tracking (Thorsten Leemhuis) wrote:
> >
> > >
> > > [+Greg +stable]
> > >
> > > On 29.07.24 10:16, Lin, Wayne wrote:
> > >
> >
> > Thanks for the report.
> >
> > Patch fa57924c76d995 ("drm/amd/display: Refactor function
> > dm_dp_mst_is_port_support_mode()")
> >
> > is kind of correcting problems causing by commit:
> >
> > 4df96ba6676034 ("drm/amd/display: Add timing pixel encoding for mst mode
> > validation")
> >
> > Sorry if it misses fixes tag and would suggest to backport to fix it.
> > Thanks!
> >
> > >
> > > Greg, seem it would be wise to pick up fa57924c76d995 for 6.10.y as
> > >
> > > well, despite a lack of Fixes or stable tags.
> > >
> > > Ciao, Thorsten
> > >
> >
> >
> >
> > The issue is that the fixing commit does not apply to the 6.10 series
> >
> > without conflict and the offending commit does not revert cleanly
> >
> > aswell.
> >
>
> Hah, many thx, I should have checked that.
>
> Lin, Wayne: could you maybe help out here and provide something for 6.10.y?
>
> Ciao, Thorsten
>
I reverted 4df96ba6676034 from v6.10.2 from the stable/linux git, resolving the
conflict by removing everything that git marked as from the current branch and
kept everything marked as from before the branch to merge. That resulted in a
patch that is fixing the problem on my machine. Since I don't understand what
the code is actually doing it might break things on other machines.
>From cd1674a469cede83f6b0907f320b6af08c3c8950 Mon Sep 17 00:00:00 2001
From: Kevin Holm
Date: Mon, 29 Jul 2024 13:24:38 +0200
Subject: [PATCH] Test patch
---
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 33 +++
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index a5e1a93ddaea..5c555a37e367 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -1599,7 +1599,7 @@ enum dc_status dm_dp_mst_is_port_support_mode(
struct amdgpu_dm_connector *aconnector,
struct dc_stream_state *stream)
{
-int pbn, branch_max_throughput_mps = 0;
+int bpp, pbn, branch_max_throughput_mps = 0;
struct dc_link_settings cur_link_settings;
unsigned int end_to_end_bw_in_kbps = 0;
unsigned int upper_link_bw_in_kbps = 0, down_link_bw_in_kbps = 0;
@@ -1649,34 +1649,11 @@ enum dc_status dm_dp_mst_is_port_support_mode(
}
}
} else {
-/* Check if mode could be supported within max slot
- * number of current mst link and full_pbn of mst links.
- */
-int pbn_div, slot_num, max_slot_num;
-enum dc_link_encoding_format link_encoding;
-uint32_t stream_kbps =
-dc_bandwidth_in_kbps_from_timing(&stream->timing,
-dc_link_get_highest_encoding_format(stream->link));
-
-pbn = kbps_to_peak_pbn(stream_kbps);
-pbn_div = dm_mst_get_pbn_divider(stream->link);
-slot_num = DIV_ROUND_UP(pbn, pbn_div);
-
-link_encoding = dc_link_get_highest_encoding_format(stream->link);
-if (link_encoding == DC_LINK_ENCODING_DP_8b_10b)
-max_slot_num = 63;
-else if (link_encoding == DC_LINK_ENCODING_DP_128b_132b)
-max_slot_num = 64;
-else {
-DRM_DEBUG_DRIVER("Invalid link encoding format\n");
+/* check if mode could be supported within full_pbn */
+bpp =
convert_dc_color_depth_into_bpc(stream->timing.display_color_depth) * 3;
+pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp <<
4);
+if (pbn > aconnector->mst_output_port->full_pbn)
return DC_FAIL_BANDWIDTH_VALIDATE;
-}
-
-if (slot_num > max_slot_num ||
-pbn > aconnector->mst_output_port->full_pbn) {
-DRM_DEBUG_DRIVER("Mode can not be supported within mst links!");
-return DC_FAIL_BANDWIDTH_VALIDATE;
-}
}
/* check is mst dsc output bandwidth branch_overall_throughput_0_mps */
--
2.45.2
Regards,
Kevin
Re: [PATCH] drm/radeon/evergreen_cs: fix int overflow errors in cs track offsets
Hi,
On 7/29/24 02:23, Christian König wrote:
> Am 26.07.24 um 14:52 schrieb Alex Deucher:
>> On Fri, Jul 26, 2024 at 3:05 AM Christian König
>> wrote:
>>> Am 25.07.24 um 20:09 schrieb Nikita Zhandarovich:
Several cs track offsets (such as 'track->db_s_read_offset')
either are initialized with or plainly take big enough values that,
once shifted 8 bits left, may be hit with integer overflow if the
resulting values end up going over u32 limit.
Some debug prints take this into account (see according dev_warn() in
evergreen_cs_track_validate_stencil()), even if the actual
calculated value assigned to local 'offset' variable is missing
similar proper expansion.
Mitigate the problem by casting the type of right operands to the
wider type of corresponding left ones in all such cases.
Found by Linux Verification Center (linuxtesting.org) with static
analysis tool SVACE.
Fixes: 285484e2d55e ("drm/radeon: add support for evergreen/ni
tiling informations v11")
Cc: sta...@vger.kernel.org
>>> Well first of all the long cast doesn't makes the value 64bit, it
>>> depends on the architecture.
>>>
>>> Then IIRC the underlying hw can only handle a 32bit address space so
>>> having the offset as long is incorrect to begin with.
>> Evergreen chips support a 36 bit internal address space and NI and
>> newer support a 40 bit one, so this is applicable.
>
> In that case I strongly suggest that we replace the unsigned long with
> u64 or otherwise we get different behavior on 32 and 64bit machines.
>
> Regards,
> Christian.
>
To be clear, I'll prepare v2 patch that changes 'offset' to u64 as well
as the cast of 'track->db_z_read_offset' (and the likes) to u64 too.
On the other note, should I also include casting to wider type of the
expression surf.layer_size * mslice (example down below) in
evergreen_cs_track_validate_cb() and other similar functions? I can't
properly gauge if the result will definitively fit into u32, maybe it
makes sense to expand it as well?
441 }
442
443 offset += surf.layer_size * mslice;
444 if (offset > radeon_bo_size(track->cb_color_bo[id])) {
445 /* old ddx are broken they allocate bo with w*h*bpp
Regards,
Nikita
>>
>> Alex
>>
>>> And finally that is absolutely not material for stable.
>>>
>>> Regards,
>>> Christian.
>>>
Signed-off-by: Nikita Zhandarovich
---
P.S. While I am not certain that track->cb_color_bo_offset[id]
actually ends up taking values high enough to cause an overflow,
nonetheless I thought it prudent to cast it to ulong as well.
drivers/gpu/drm/radeon/evergreen_cs.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
b/drivers/gpu/drm/radeon/evergreen_cs.c
index 1fe6e0d883c7..d734d221e2da 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -433,7 +433,7 @@ static int evergreen_cs_track_validate_cb(struct
radeon_cs_parser *p, unsigned i
return r;
}
- offset = track->cb_color_bo_offset[id] << 8;
+ offset = (unsigned long)track->cb_color_bo_offset[id] << 8;
if (offset & (surf.base_align - 1)) {
dev_warn(p->dev, "%s:%d cb[%d] bo base %ld not
aligned with %ld\n",
__func__, __LINE__, id, offset,
surf.base_align);
@@ -455,7 +455,7 @@ static int evergreen_cs_track_validate_cb(struct
radeon_cs_parser *p, unsigned i
min = surf.nby - 8;
}
bsize = radeon_bo_size(track->cb_color_bo[id]);
- tmp = track->cb_color_bo_offset[id] << 8;
+ tmp = (unsigned
long)track->cb_color_bo_offset[id] << 8;
for (nby = surf.nby; nby > min; nby--) {
size = nby * surf.nbx * surf.bpe *
surf.nsamples;
if ((tmp + size * mslice) <= bsize) {
@@ -476,10 +476,10 @@ static int
evergreen_cs_track_validate_cb(struct radeon_cs_parser *p, unsigned i
}
}
dev_warn(p->dev, "%s:%d cb[%d] bo too small (layer
size %d, "
- "offset %d, max layer %d, bo size %ld, slice
%d)\n",
+ "offset %ld, max layer %d, bo size %ld, slice
%d)\n",
__func__, __LINE__, id, surf.layer_size,
- track->cb_color_bo_offset[id] << 8, mslice,
- radeon_bo_size(track->cb_color_bo[id]), slice);
+ (unsigned long)track->cb_color_bo_offset[id]
<< 8,
+ mslice,
radeon_bo_size
Re: [PATCH 3/4] drm/msm/a5xx: fix races in preemption evaluation stage
On Thu, Jul 11, 2024 at 11:10 AM Vladimir Lypak
wrote:
>
> On A5XX GPUs when preemption is used it's invietable to enter a soft
> lock-up state in which GPU is stuck at empty ring-buffer doing nothing.
> This appears as full UI lockup and not detected as GPU hang (because
> it's not). This happens due to not triggering preemption when it was
> needed. Sometimes this state can be recovered by some new submit but
> generally it won't happen because applications are waiting for old
> submits to retire.
>
> One of the reasons why this happens is a race between a5xx_submit and
> a5xx_preempt_trigger called from IRQ during submit retire. Former thread
> updates ring->cur of previously empty and not current ring right after
> latter checks it for emptiness. Then both threads can just exit because
> for first one preempt_state wasn't NONE yet and for second one all rings
> appeared to be empty.
>
> To prevent such situations from happening we need to establish guarantee
> for preempt_trigger to be called after each submit. To implement it this
> patch adds trigger call at the end of a5xx_preempt_irq to re-check if we
> should switch to non-empty or higher priority ring. Also we find next
> ring in new preemption state "EVALUATE". If the thread that updated some
> ring with new submit sees this state it should wait until it passes.
>
> Fixes: b1fc2839d2f9 ("drm/msm: Implement preemption for A5XX targets")
> Signed-off-by: Vladimir Lypak
> ---
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 6 +++---
> drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 11 +++
> drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 24 +++
> 3 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 6c80d3003966..266744ee1d5f 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -110,7 +110,7 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct
> msm_gem_submit *submit
> }
>
> a5xx_flush(gpu, ring, true);
> - a5xx_preempt_trigger(gpu);
> + a5xx_preempt_trigger(gpu, true);
>
> /* we might not necessarily have a cmd from userspace to
> * trigger an event to know that submit has completed, so
> @@ -240,7 +240,7 @@ static void a5xx_submit(struct msm_gpu *gpu, struct
> msm_gem_submit *submit)
> a5xx_flush(gpu, ring, false);
>
> /* Check to see if we need to start preemption */
> - a5xx_preempt_trigger(gpu);
> + a5xx_preempt_trigger(gpu, true);
> }
>
> static const struct adreno_five_hwcg_regs {
> @@ -1296,7 +1296,7 @@ static irqreturn_t a5xx_irq(struct msm_gpu *gpu)
> a5xx_gpmu_err_irq(gpu);
>
> if (status & A5XX_RBBM_INT_0_MASK_CP_CACHE_FLUSH_TS) {
> - a5xx_preempt_trigger(gpu);
> + a5xx_preempt_trigger(gpu, false);
> msm_gpu_retire(gpu);
> }
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
> b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
> index c7187bcc5e90..1120824853d4 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
> @@ -57,10 +57,12 @@ void a5xx_debugfs_init(struct msm_gpu *gpu, struct
> drm_minor *minor);
> * through the process.
> *
> * PREEMPT_NONE - no preemption in progress. Next state START.
> - * PREEMPT_START - The trigger is evaulating if preemption is possible. Next
> - * states: TRIGGERED, NONE
> + * PREEMPT_EVALUATE - The trigger is evaulating if preemption is possible.
> Next
> + * states: START, ABORT
> * PREEMPT_ABORT - An intermediate state before moving back to NONE. Next
> * state: NONE.
> + * PREEMPT_START - The trigger is preparing for preemption. Next state:
> + * TRIGGERED
> * PREEMPT_TRIGGERED: A preemption has been executed on the hardware. Next
> * states: FAULTED, PENDING
> * PREEMPT_FAULTED: A preemption timed out (never completed). This will
> trigger
> @@ -71,8 +73,9 @@ void a5xx_debugfs_init(struct msm_gpu *gpu, struct
> drm_minor *minor);
>
> enum preempt_state {
> PREEMPT_NONE = 0,
> - PREEMPT_START,
> + PREEMPT_EVALUATE,
> PREEMPT_ABORT,
> + PREEMPT_START,
> PREEMPT_TRIGGERED,
> PREEMPT_FAULTED,
> PREEMPT_PENDING,
> @@ -156,7 +159,7 @@ void a5xx_set_hwcg(struct msm_gpu *gpu, bool state);
>
> void a5xx_preempt_init(struct msm_gpu *gpu);
> void a5xx_preempt_hw_init(struct msm_gpu *gpu);
> -void a5xx_preempt_trigger(struct msm_gpu *gpu);
> +void a5xx_preempt_trigger(struct msm_gpu *gpu, bool new_submit);
> void a5xx_preempt_irq(struct msm_gpu *gpu);
> void a5xx_preempt_fini(struct msm_gpu *gpu);
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> index 67a8ef4adf6b..f8d09a83c5ae 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> @@ -87,21 +8
[PATCH v2] drm/i915: Fix possible int overflow in skl_ddi_calculate_wrpll()
On the off chance that clock value ends up being too high (by means
of skl_ddi_calculate_wrpll() having benn called with big enough
value of crtc_state->port_clock * 1000), one possible consequence
may be that the result will not be able to fit into signed int.
Fix this issue by moving conversion of clock parameter from kHz to Hz
into the body of skl_ddi_calculate_wrpll(), as well as casting the
same parameter to u64 type while calculating the value for AFE clock.
This both mitigates the overflow problem and avoids possible erroneous
integer promotion mishaps.
Found by Linux Verification Center (linuxtesting.org) with static
analysis tool SVACE.
Fixes: fe70b262e781 ("drm/i915: Move a bunch of stuff into rodata from the
stack")
Cc: sta...@vger.kernel.org
Signed-off-by: Nikita Zhandarovich
---
v2: instead of double casting of 'clock' with (u64)(u32), convert
'clock' to Hz inside skl_ddi_calculate_wrpll() and cast it only
to u64 to mitigate the issue. Per Jani's
helpful suggestion made here:
https://lore.kernel.org/all/87ed7gzhin@intel.com/
Also, change commit description accordingly.
v1:
https://lore.kernel.org/all/20240724184911.12250-1-n.zhandarov...@fintech.ru/
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 90998b037349..292d163036b1 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -1658,7 +1658,7 @@ static void skl_wrpll_params_populate(struct
skl_wrpll_params *params,
}
static int
-skl_ddi_calculate_wrpll(int clock /* in Hz */,
+skl_ddi_calculate_wrpll(int clock,
int ref_clock,
struct skl_wrpll_params *wrpll_params)
{
@@ -1683,7 +1683,7 @@ skl_ddi_calculate_wrpll(int clock /* in Hz */,
};
unsigned int dco, d, i;
unsigned int p0, p1, p2;
- u64 afe_clock = clock * 5; /* AFE Clock is 5x Pixel clock */
+ u64 afe_clock = (u64)clock * 1000 * 5; /* AFE Clock is 5x Pixel clock,
in Hz */
for (d = 0; d < ARRAY_SIZE(dividers); d++) {
for (dco = 0; dco < ARRAY_SIZE(dco_central_freq); dco++) {
@@ -1808,7 +1808,7 @@ static int skl_ddi_hdmi_pll_dividers(struct
intel_crtc_state *crtc_state)
struct skl_wrpll_params wrpll_params = {};
int ret;
- ret = skl_ddi_calculate_wrpll(crtc_state->port_clock * 1000,
+ ret = skl_ddi_calculate_wrpll(crtc_state->port_clock,
i915->display.dpll.ref_clks.nssc,
&wrpll_params);
if (ret)
return ret;
Re: [PATCH] drm/radeon/evergreen_cs: fix int overflow errors in cs track offsets
Am 29.07.24 um 19:26 schrieb Nikita Zhandarovich:
Hi,
On 7/29/24 02:23, Christian König wrote:
Am 26.07.24 um 14:52 schrieb Alex Deucher:
On Fri, Jul 26, 2024 at 3:05 AM Christian König
wrote:
Am 25.07.24 um 20:09 schrieb Nikita Zhandarovich:
Several cs track offsets (such as 'track->db_s_read_offset')
either are initialized with or plainly take big enough values that,
once shifted 8 bits left, may be hit with integer overflow if the
resulting values end up going over u32 limit.
Some debug prints take this into account (see according dev_warn() in
evergreen_cs_track_validate_stencil()), even if the actual
calculated value assigned to local 'offset' variable is missing
similar proper expansion.
Mitigate the problem by casting the type of right operands to the
wider type of corresponding left ones in all such cases.
Found by Linux Verification Center (linuxtesting.org) with static
analysis tool SVACE.
Fixes: 285484e2d55e ("drm/radeon: add support for evergreen/ni
tiling informations v11")
Cc: sta...@vger.kernel.org
Well first of all the long cast doesn't makes the value 64bit, it
depends on the architecture.
Then IIRC the underlying hw can only handle a 32bit address space so
having the offset as long is incorrect to begin with.
Evergreen chips support a 36 bit internal address space and NI and
newer support a 40 bit one, so this is applicable.
In that case I strongly suggest that we replace the unsigned long with
u64 or otherwise we get different behavior on 32 and 64bit machines.
Regards,
Christian.
To be clear, I'll prepare v2 patch that changes 'offset' to u64 as well
as the cast of 'track->db_z_read_offset' (and the likes) to u64 too.
On the other note, should I also include casting to wider type of the
expression surf.layer_size * mslice (example down below) in
evergreen_cs_track_validate_cb() and other similar functions? I can't
properly gauge if the result will definitively fit into u32, maybe it
makes sense to expand it as well?
The integer overflows caused by shifts are irrelevant and doesn't need
any fixing in the first place.
The point is rather that we need to avoid multiplication overflows and
the security problems which come with those.
441 }
442
443 offset += surf.layer_size * mslice;
In other words that here needs to be validated correctly.
Regards,
Christian.
444 if (offset > radeon_bo_size(track->cb_color_bo[id])) {
445 /* old ddx are broken they allocate bo with w*h*bpp
Regards,
Nikita
Alex
And finally that is absolutely not material for stable.
Regards,
Christian.
Signed-off-by: Nikita Zhandarovich
---
P.S. While I am not certain that track->cb_color_bo_offset[id]
actually ends up taking values high enough to cause an overflow,
nonetheless I thought it prudent to cast it to ulong as well.
drivers/gpu/drm/radeon/evergreen_cs.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
b/drivers/gpu/drm/radeon/evergreen_cs.c
index 1fe6e0d883c7..d734d221e2da 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -433,7 +433,7 @@ static int evergreen_cs_track_validate_cb(struct
radeon_cs_parser *p, unsigned i
return r;
}
- offset = track->cb_color_bo_offset[id] << 8;
+ offset = (unsigned long)track->cb_color_bo_offset[id] << 8;
if (offset & (surf.base_align - 1)) {
dev_warn(p->dev, "%s:%d cb[%d] bo base %ld not
aligned with %ld\n",
__func__, __LINE__, id, offset,
surf.base_align);
@@ -455,7 +455,7 @@ static int evergreen_cs_track_validate_cb(struct
radeon_cs_parser *p, unsigned i
min = surf.nby - 8;
}
bsize = radeon_bo_size(track->cb_color_bo[id]);
- tmp = track->cb_color_bo_offset[id] << 8;
+ tmp = (unsigned
long)track->cb_color_bo_offset[id] << 8;
for (nby = surf.nby; nby > min; nby--) {
size = nby * surf.nbx * surf.bpe *
surf.nsamples;
if ((tmp + size * mslice) <= bsize) {
@@ -476,10 +476,10 @@ static int
evergreen_cs_track_validate_cb(struct radeon_cs_parser *p, unsigned i
}
}
dev_warn(p->dev, "%s:%d cb[%d] bo too small (layer
size %d, "
- "offset %d, max layer %d, bo size %ld, slice
%d)\n",
+ "offset %ld, max layer %d, bo size %ld, slice
%d)\n",
__func__, __LINE__, id, surf.layer_size,
- track->cb_color_bo_offset[id] << 8, mslice,
- radeon_bo_size(track->cb_color_bo[id]), slice);
+ (unsigned long)track->cb_color_bo_offset[id]
<< 8,
+ mslice,
radeon_bo_size(track->cb_color_bo[id]), slice);
de
Re: [PATCH] drm/radeon/evergreen_cs: fix int overflow errors in cs track offsets
Am 29.07.24 um 20:04 schrieb Christian König:
Am 29.07.24 um 19:26 schrieb Nikita Zhandarovich:
Hi,
On 7/29/24 02:23, Christian König wrote:
Am 26.07.24 um 14:52 schrieb Alex Deucher:
On Fri, Jul 26, 2024 at 3:05 AM Christian König
wrote:
Am 25.07.24 um 20:09 schrieb Nikita Zhandarovich:
Several cs track offsets (such as 'track->db_s_read_offset')
either are initialized with or plainly take big enough values that,
once shifted 8 bits left, may be hit with integer overflow if the
resulting values end up going over u32 limit.
Some debug prints take this into account (see according
dev_warn() in
evergreen_cs_track_validate_stencil()), even if the actual
calculated value assigned to local 'offset' variable is missing
similar proper expansion.
Mitigate the problem by casting the type of right operands to the
wider type of corresponding left ones in all such cases.
Found by Linux Verification Center (linuxtesting.org) with static
analysis tool SVACE.
Fixes: 285484e2d55e ("drm/radeon: add support for evergreen/ni
tiling informations v11")
Cc: sta...@vger.kernel.org
Well first of all the long cast doesn't makes the value 64bit, it
depends on the architecture.
Then IIRC the underlying hw can only handle a 32bit address space so
having the offset as long is incorrect to begin with.
Evergreen chips support a 36 bit internal address space and NI and
newer support a 40 bit one, so this is applicable.
In that case I strongly suggest that we replace the unsigned long with
u64 or otherwise we get different behavior on 32 and 64bit machines.
Regards,
Christian.
To be clear, I'll prepare v2 patch that changes 'offset' to u64 as well
as the cast of 'track->db_z_read_offset' (and the likes) to u64 too.
On the other note, should I also include casting to wider type of the
expression surf.layer_size * mslice (example down below) in
evergreen_cs_track_validate_cb() and other similar functions? I can't
properly gauge if the result will definitively fit into u32, maybe it
makes sense to expand it as well?
The integer overflows caused by shifts are irrelevant and doesn't need
any fixing in the first place.
Wait a second.
Thinking more about it the integer overflows are actually necessary
because that is exactly what happens in the hardware as well.
If you don't overflow those shifts you actually create a security
problem because the HW the might access at a different offset then you
calculated here.
We need to use something like a mask or use lower_32_bits() here.
Regards,
Christian.
The point is rather that we need to avoid multiplication overflows and
the security problems which come with those.
441 }
442
443 offset += surf.layer_size * mslice;
In other words that here needs to be validated correctly.
Regards,
Christian.
444 if (offset > radeon_bo_size(track->cb_color_bo[id])) {
445 /* old ddx are broken they allocate bo with w*h*bpp
Regards,
Nikita
Alex
And finally that is absolutely not material for stable.
Regards,
Christian.
Signed-off-by: Nikita Zhandarovich
---
P.S. While I am not certain that track->cb_color_bo_offset[id]
actually ends up taking values high enough to cause an overflow,
nonetheless I thought it prudent to cast it to ulong as well.
drivers/gpu/drm/radeon/evergreen_cs.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
b/drivers/gpu/drm/radeon/evergreen_cs.c
index 1fe6e0d883c7..d734d221e2da 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -433,7 +433,7 @@ static int evergreen_cs_track_validate_cb(struct
radeon_cs_parser *p, unsigned i
return r;
}
- offset = track->cb_color_bo_offset[id] << 8;
+ offset = (unsigned long)track->cb_color_bo_offset[id] << 8;
if (offset & (surf.base_align - 1)) {
dev_warn(p->dev, "%s:%d cb[%d] bo base %ld not
aligned with %ld\n",
__func__, __LINE__, id, offset,
surf.base_align);
@@ -455,7 +455,7 @@ static int evergreen_cs_track_validate_cb(struct
radeon_cs_parser *p, unsigned i
min = surf.nby - 8;
}
bsize =
radeon_bo_size(track->cb_color_bo[id]);
- tmp = track->cb_color_bo_offset[id] << 8;
+ tmp = (unsigned
long)track->cb_color_bo_offset[id] << 8;
for (nby = surf.nby; nby > min; nby--) {
size = nby * surf.nbx * surf.bpe *
surf.nsamples;
if ((tmp + size * mslice) <=
bsize) {
@@ -476,10 +476,10 @@ static int
evergreen_cs_track_validate_cb(struct radeon_cs_parser *p,
unsigned i
}
}
dev_warn(p->dev, "%s:%d cb[%d] bo too small (layer
size %d, "
- "offset %d, max layer %d, bo size %ld, slice
%d)\
Re: [PATCH] drm/msm/dp: fix the max supported bpp logic
Hi Stephen
On 7/26/2024 5:24 PM, Stephen Boyd wrote:
Quoting Abhinav Kumar (2024-07-25 15:03:19)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index a916b5f3b317..56ce5e4008f8 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -423,8 +424,10 @@ int dp_panel_init_panel_info(struct dp_panel *dp_panel)
drm_mode->clock);
drm_dbg_dp(panel->drm_dev, "bpp = %d\n", dp_panel->dp_mode.bpp);
- dp_panel->dp_mode.bpp = max_t(u32, 18,
- min_t(u32, dp_panel->dp_mode.bpp, 30));
+ max_supported_bpp = dp_panel_get_mode_bpp(dp_panel,
dp_panel->dp_mode.bpp,
+
dp_panel->dp_mode.drm_mode.clock);
+ dp_panel->dp_mode.bpp = max_t(u32, 18, max_supported_bpp);
Is the max_t() usage still required once 'max_supported_bpp' is also a
u32? Also, what is 18? Shouldn't that be some sort of define so we know
what it represents?
Or maybe none of that is required? From what I can tell,
dp_panel_get_mode_bpp() calls dp_panel_get_supported_bpp() which will
essentially clamp the bpp range between 18 and 30, unless
dp_panel->dp_mode.bpp is between 30 and 18 but not divisible by 6, e.g.
29. Perhaps this patch can be included and the max_t above dropped.
---8<--
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 07db8f37cd06..5cd7c138afd3 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -90,22 +90,22 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
static u32 dp_panel_get_supported_bpp(struct dp_panel *dp_panel,
u32 mode_edid_bpp, u32 mode_pclk_khz)
{
- struct dp_link_info *link_info;
+ const struct dp_link_info *link_info;
const u32 max_supported_bpp = 30, min_supported_bpp = 18;
- u32 bpp = 0, data_rate_khz = 0;
+ u32 bpp, data_rate_khz;
bpp = min_t(u32, mode_edid_bpp, max_supported_bpp);
link_info = &dp_panel->link_info;
data_rate_khz = link_info->num_lanes * link_info->rate * 8;
- while (bpp > min_supported_bpp) {
+ do {
if (mode_pclk_khz * bpp <= data_rate_khz)
- break;
+ return bpp;
bpp -= 6;
- }
+ } while (bpp > min_supported_bpp);
- return bpp;
+ return min_supported_bpp;
}
Thanks for the feedback.
Your change looks valid. We can use this and drop the max_t usage.
Let me push this with your Suggested-by credits.
static int dp_panel_update_modes(struct drm_connector *connector,
Re: [RFC PATCH] drm: panthor: add dev_coredumpv support
On Fri, 2024-07-26 at 15:40 +0200, Daniel Vetter wrote:
> On Thu, Jul 25, 2024 at 03:35:18PM -0400, Lyude Paul wrote:
> > On Tue, 2024-07-16 at 11:25 +0200, Daniel Vetter wrote:
> > > On Mon, Jul 15, 2024 at 02:05:49PM -0300, Daniel Almeida wrote:
> > > > Hi Sima!
> > > >
> > > >
> > > > >
> > > > > Yeah I'm not sure a partially converted driver where the main driver
> > > > > is
> > > > > still C really works, that pretty much has to throw out all the type
> > > > > safety in the interfaces.
> > > > >
> > > > > What I think might work is if such partial drivers register as full
> > > > > rust
> > > > > drivers, and then largely delegate the implementation to their
> > > > > existing C
> > > > > code with a big "safety: trust me, the C side is bug free" comment
> > > > > since
> > > > > it's all going to be unsafe :-)
> > > > >
> > > > > It would still be a big change, since all the driver's callbacks need
> > > > > to
> > > > > switch from container_of to upcast to their driver structure to some
> > > > > small
> > > > > rust shim (most likely, I didn't try this out) to get at the driver
> > > > > parts
> > > > > on the C side. And I think you also need a small function to downcast
> > > > > to
> > > > > the drm base class. But that should be all largely mechanical.
> > > > >
> > > > > More freely allowing to mix&match is imo going to be endless pains. We
> > > > > kinda tried that with the atomic conversion helpers for legacy kms
> > > > > drivers, and the impendance mismatch was just endless amounts of very
> > > > > subtle pain. Rust will exacerbate this, because it encodes semantics
> > > > > into
> > > > > the types and interfaces. And that was with just one set of helpers,
> > > > > for
> > > > > rust we'll likely need a custom one for each driver that's partially
> > > > > written in rust.
> > > > > -Sima
> > > > >
> > > >
> > > > I humbly disagree here.
> > > >
> > > > I know this is a bit tangential, but earlier this year I converted a
> > > > bunch of codec libraries to Rust in v4l2. That worked just fine with the
> > > > C codec drivers. There were no regressions as per our test tools.
> > > >
> > > > The main idea is that you isolate all unsafety to a single point: so
> > > > long as the C code upholds the safety guarantees when calling into Rust,
> > > > the Rust layer will be safe. This is just the same logic used in unsafe
> > > > blocks in Rust itself, nothing new really.
> > > >
> > > > This is not unlike what is going on here, for example:
> > > >
> > > >
> > > > ```
> > > > +unsafe extern "C" fn open_callback, U:
> > > > BaseObject>(
> > > > + raw_obj: *mut bindings::drm_gem_object,
> > > > + raw_file: *mut bindings::drm_file,
> > > > +) -> core::ffi::c_int {
> > > > + // SAFETY: The pointer we got has to be valid.
> > > > + let file = unsafe {
> > > > + file::File::<<::Driver as
> > > > drv::Driver>::File>::from_raw(raw_file)
> > > > + };
> > > > + let obj =
> > > > + <<::Driver as drv::Driver>::Object as
> > > > IntoGEMObject>::from_gem_obj(
> > > > + raw_obj,
> > > > + );
> > > > +
> > > > + // SAFETY: from_gem_obj() returns a valid pointer as long as the type
> > > > is
> > > > + // correct and the raw_obj we got is valid.
> > > > + match T::open(unsafe { &*obj }, &file) {
> > > > + Err(e) => e.to_errno(),
> > > > + Ok(()) => 0,
> > > > + }
> > > > +}
> > > > ```
> > > >
> > > > We have to trust that the kernel is passing in a valid pointer. By the
> > > > same token, we can choose to trust drivers if we so desire.
> > > >
> > > > > that pretty much has to throw out all the type
> > > > > safety in the interfaces.
> > > >
> > > > Can you expand on that?
> > >
> > > Essentially what you've run into, in a pure rust driver we assume that
> > > everything is living in the rust world. In a partial conversion you might
> > > want to freely convert GEMObject back&forth, but everything else
> > > (drm_file, drm_device, ...) is still living in the pure C world. I think
> > > there's roughly three solutions to this:
> > >
> > > - we allow this on the rust side, but that means the associated
> > > types/generics go away. We drop a lot of enforced type safety for pure
> > > rust drivers.
> > >
> > > - we don't allow this. Your mixed driver is screwed.
> > >
> > > - we allow this for specific functions, with a pinky finger promise that
> > > those rust functions will not look at any of the associated types. From
> > > my experience these kind of in-between worlds functions are really
> > > brittle and a pain, e.g. rust-native driver people might accidentally
> > > change the code to again assume a drv::Driver exists, or people don't
> > > want to touch the code because it's too risky, or we're forced to
> > > implement stuff in C instead of rust more than necessary.
> > >
> > > > In particular, I believe that we should ideally be able to convert from
> > > > a C "struct Foo * " to a Rust “FooRef" for types whose lifetimes are
> > > > managed e
Re: [PATCH] drm/sched: add optional errno to drm_sched_start()
Am 26.07.24 um 14:30 schrieb Matthew Brost:
On Fri, Jul 26, 2024 at 09:55:50AM +0200, Christian König wrote:
The current implementation of drm_sched_start uses a hardcoded
-ECANCELED to dispose of a job when the parent/hw fence is NULL.
This results in drm_sched_job_done being called with -ECANCELED for
each job with a NULL parent in the pending list, making it difficult
to distinguish between recovery methods, whether a queue reset or a
full GPU reset was used.
To improve this, we first try a soft recovery for timeout jobs and
use the error code -ENODATA. If soft recovery fails, we proceed with
a queue reset, where the error code remains -ENODATA for the job.
Finally, for a full GPU reset, we use error codes -ECANCELED or
-ETIME. This patch adds an error code parameter to drm_sched_start,
allowing us to differentiate between queue reset and GPU reset
failures. This enables user mode and test applications to validate
the expected correctness of the requested operation. After a
successful queue reset, the only way to continue normal operation is
to call drm_sched_job_done with the specific error code -ENODATA.
v1: Initial implementation by Jesse utilized amdgpu_device_lock_reset_domain
and amdgpu_device_unlock_reset_domain to allow user mode to track
the queue reset status and distinguish between queue reset and
GPU reset.
v2: Christian suggested using the error codes -ENODATA for queue reset
and -ECANCELED or -ETIME for GPU reset, returned to
amdgpu_cs_wait_ioctl.
v3: To meet the requirements, we introduce a new function
drm_sched_start_ex with an additional parameter to set
dma_fence_set_error, allowing us to handle the specific error
codes appropriately and dispose of bad jobs with the selected
error code depending on whether it was a queue reset or GPU reset.
v4: Alex suggested using a new name, drm_sched_start_with_recovery_error,
which more accurately describes the function's purpose.
Additionally, it was recommended to add documentation details
about the new method.
v5: Fixed declaration of new function drm_sched_start_with_recovery_error.(Alex)
v6 (chk): rebase on upstream changes, cleanup the commit message,
drop the new function again and update all callers,
apply the errno also to scheduler fences with hw fences
Seems responablie to me, but all the caller pass in an errno of zero to
drm_sched_start. Going to change in a follow up?
Yes, exactly that's the idea. Just wanted to double check if the
approach makes sense.
Regards,
Christian.
Anyways, LGTM but will leave RB for a user a user of this interface.
Acked-by: Matthew Brost
Signed-off-by: Jesse Zhang
Signed-off-by: Vitaly Prosyak
Signed-off-by: Christian König
Cc: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++--
drivers/gpu/drm/etnaviv/etnaviv_sched.c | 4 ++--
drivers/gpu/drm/imagination/pvr_queue.c | 4 ++--
drivers/gpu/drm/lima/lima_sched.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_job.c | 2 +-
drivers/gpu/drm/panthor/panthor_mmu.c | 2 +-
drivers/gpu/drm/scheduler/sched_main.c | 7 ---
drivers/gpu/drm/v3d/v3d_sched.c | 2 +-
include/drm/gpu_scheduler.h | 2 +-
11 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
index 2320df51c914..18135d8235f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
@@ -300,7 +300,7 @@ static int suspend_resume_compute_scheduler(struct
amdgpu_device *adev, bool sus
if (r)
goto out;
} else {
- drm_sched_start(&ring->sched);
+ drm_sched_start(&ring->sched, 0);
}
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c186fdb198ad..861827deb03f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5862,7 +5862,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
if (!amdgpu_ring_sched_ready(ring))
continue;
- drm_sched_start(&ring->sched);
+ drm_sched_start(&ring->sched, 0);
}
if (!drm_drv_uses_atomic_modeset(adev_to_drm(tmp_adev)) && !job_signaled)
@@ -6360,7 +6360,7 @@ void amdgpu_pci_resume(struct pci_dev *pdev)
if (!amdgpu_ring_sched_ready(ring))
continue;
- drm_sched_start(&ring->sched);
+ drm_sched_start(&rin
Re: [PATCH] drm/msm/dp: fix the max supported bpp logic
On 7/27/2024 5:51 AM, Dmitry Baryshkov wrote:
On Fri, 26 Jul 2024 at 01:04, Abhinav Kumar wrote:
Currently the DP driver hard-codes the max supported bpp to 30.
This is incorrect because the number of lanes and max data rate
supported by the lanes need to be taken into account.
Replace the hardcoded limit with the appropriate math which accounts
for the accurate number of lanes and max data rate.
Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/dp/dp_panel.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index a916b5f3b317..56ce5e4008f8 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -397,6 +397,7 @@ int dp_panel_init_panel_info(struct dp_panel *dp_panel)
{
struct drm_display_mode *drm_mode;
struct dp_panel_private *panel;
+ u32 max_supported_bpp;
drm_mode = &dp_panel->dp_mode.drm_mode;
@@ -423,8 +424,10 @@ int dp_panel_init_panel_info(struct dp_panel *dp_panel)
drm_mode->clock);
drm_dbg_dp(panel->drm_dev, "bpp = %d\n", dp_panel->dp_mode.bpp);
- dp_panel->dp_mode.bpp = max_t(u32, 18,
- min_t(u32, dp_panel->dp_mode.bpp, 30));
+ max_supported_bpp = dp_panel_get_mode_bpp(dp_panel,
dp_panel->dp_mode.bpp,
+
dp_panel->dp_mode.drm_mode.clock);
+ dp_panel->dp_mode.bpp = max_t(u32, 18, max_supported_bpp);
I think that in mode_valid() the driver should filter out modes that
result in BPP being less than 18. Then the max_t can be dropped
completely.
With Stephen's suggested change, dp_panel_get_supported_bpp() will not
return anything below min_supported_bpp which is 18 so we can absorb
that part and drop the max_t part here.
Nevertheless this indeed fixes an issue with the screen corruption,
this is great!
Tested-by: Dmitry Baryshkov # SM8350-HDK
Thanks for reporting and testing this. I need to give you Reported-by
credits as well.
+
drm_dbg_dp(panel->drm_dev, "updated bpp = %d\n",
dp_panel->dp_mode.bpp);
--
2.44.0
Re: [PATCH] drm/sched: add optional errno to drm_sched_start()
Am 26.07.24 um 16:21 schrieb Daniel Vetter:
On Fri, Jul 26, 2024 at 09:55:50AM +0200, Christian König wrote:
The current implementation of drm_sched_start uses a hardcoded
-ECANCELED to dispose of a job when the parent/hw fence is NULL.
This results in drm_sched_job_done being called with -ECANCELED for
each job with a NULL parent in the pending list, making it difficult
to distinguish between recovery methods, whether a queue reset or a
full GPU reset was used.
To improve this, we first try a soft recovery for timeout jobs and
use the error code -ENODATA. If soft recovery fails, we proceed with
a queue reset, where the error code remains -ENODATA for the job.
Finally, for a full GPU reset, we use error codes -ECANCELED or
-ETIME. This patch adds an error code parameter to drm_sched_start,
allowing us to differentiate between queue reset and GPU reset
failures. This enables user mode and test applications to validate
the expected correctness of the requested operation. After a
successful queue reset, the only way to continue normal operation is
to call drm_sched_job_done with the specific error code -ENODATA.
v1: Initial implementation by Jesse utilized amdgpu_device_lock_reset_domain
and amdgpu_device_unlock_reset_domain to allow user mode to track
the queue reset status and distinguish between queue reset and
GPU reset.
v2: Christian suggested using the error codes -ENODATA for queue reset
and -ECANCELED or -ETIME for GPU reset, returned to
amdgpu_cs_wait_ioctl.
v3: To meet the requirements, we introduce a new function
drm_sched_start_ex with an additional parameter to set
dma_fence_set_error, allowing us to handle the specific error
codes appropriately and dispose of bad jobs with the selected
error code depending on whether it was a queue reset or GPU reset.
v4: Alex suggested using a new name, drm_sched_start_with_recovery_error,
which more accurately describes the function's purpose.
Additionally, it was recommended to add documentation details
about the new method.
v5: Fixed declaration of new function drm_sched_start_with_recovery_error.(Alex)
v6 (chk): rebase on upstream changes, cleanup the commit message,
drop the new function again and update all callers,
apply the errno also to scheduler fences with hw fences
Signed-off-by: Jesse Zhang
Signed-off-by: Vitaly Prosyak
Signed-off-by: Christian König
Cc: Alex Deucher
Maybe I'm extremely missing the point, but it's kind hard to be sure
without the testcase/mesa side code too, but for gl robustness I don't
think this is enough, because you also need to know whether it was your
context or someone else that caused the gpu reset. Probably biased, but I
think the per-ctx guilty/reset counters is more then right code here. Or
something along those lines.
Exactly that ctx based approach blew up pretty nicely because it doesn't
match the lifetime of the ctx.
On the one hand you don't want the ctx to outlive the file descriptor
which it was created with since it points back to the fd, on the other
hand when you need it for error handling you need to keep it around
until all submissions are completed.
In the end you have a really nice circle dependency.
If we really want to stuff this into per-job fences then I think we should
at least try to document this mess in the sync_file uapi, for a bit of
consistency.
Good point. Going to add some documentation.
Regards,
Christian.
But yeah without the full picture no idea really what we want here.
-Sima
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++--
drivers/gpu/drm/etnaviv/etnaviv_sched.c | 4 ++--
drivers/gpu/drm/imagination/pvr_queue.c | 4 ++--
drivers/gpu/drm/lima/lima_sched.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_job.c | 2 +-
drivers/gpu/drm/panthor/panthor_mmu.c | 2 +-
drivers/gpu/drm/scheduler/sched_main.c | 7 ---
drivers/gpu/drm/v3d/v3d_sched.c | 2 +-
include/drm/gpu_scheduler.h | 2 +-
11 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
index 2320df51c914..18135d8235f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
@@ -300,7 +300,7 @@ static int suspend_resume_compute_scheduler(struct
amdgpu_device *adev, bool sus
if (r)
goto out;
} else {
- drm_sched_start(&ring->sched);
+ drm_sched_start(&ring->sched, 0);
}
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu
Re: [PATCH] drm/plane: Fix IS_ERR() vs NULL bug
On Sat, Jul 27, 2024 at 1:32 AM Dan Carpenter wrote:
>
> The drm_property_create_signed_range() function returns NULL on error,
> it doesn't return error pointers. Change the IS_ERR() tests to check
> for NULL.
>
> Fixes: 8f7179a1027d ("drm/atomic: Add support for mouse hotspots")
> Signed-off-by: Dan Carpenter
> ---
> drivers/gpu/drm/drm_plane.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index a28b22fdd7a4..4fcb5d486de6 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -328,14 +328,14 @@ static int drm_plane_create_hotspot_properties(struct
> drm_plane *plane)
>
> prop_x = drm_property_create_signed_range(plane->dev, 0, "HOTSPOT_X",
> INT_MIN, INT_MAX);
> - if (IS_ERR(prop_x))
> - return PTR_ERR(prop_x);
> + if (!prop_x)
> + return -ENOMEM;
>
> prop_y = drm_property_create_signed_range(plane->dev, 0, "HOTSPOT_Y",
> INT_MIN, INT_MAX);
> - if (IS_ERR(prop_y)) {
> + if (!prop_y) {
> drm_property_destroy(plane->dev, prop_x);
> - return PTR_ERR(prop_y);
> + return -ENOMEM;
> }
>
> drm_object_attach_property(&plane->base, prop_x, 0);
Thanks, that looks good to me.
Reviewed-by: Zack Rusin
z
Re: [PATCH] drm/msm/dp: fix the max supported bpp logic
Quoting Abhinav Kumar (2024-07-29 11:28:35) > > Thanks for the feedback. > > Your change looks valid. We can use this and drop the max_t usage. > > Let me push this with your Suggested-by credits. You can take my Signed-off-by: Stephen Boyd and either squash it in or make a follow-up.
Re: [PATCH v1 1/2] drm/panel: jd9365da: Move the sending location of the 11/29 command
On Mon, 29 Jul 2024 at 06:10, zhaoxiong lv wrote: > > On Sun, Jul 28, 2024 at 12:59 AM Dmitry Baryshkov > wrote: > > > > On Thu, Jul 25, 2024 at 04:32:44PM GMT, Zhaoxiong Lv wrote: > > > Move the 11/29 command from enable() to init() function > > > > > > As mentioned in the patch: > > > https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxi...@huaqin.corp-partner.google.com/ > > > > > > Our DSI host has different modes in prepare() and enable() > > > functions. prepare() is in LP mode and enable() is in HS mode. > > > Since the 11/29 command must also be sent in LP mode, > > > so we also move 11/29 command to the init() function. > > > > > > After moving the 11/29 command to the init() function, > > > we no longer need additional delay judgment, so we delete > > > variables "exit_sleep_to_display_on_delay_ms" and > > > "display_on_delay_ms". > > > > Won't this result in a garbage being displayed on the panel during > > startup? > > Hi Dmitry > > We just moved "Exit sleep mode" and "set display on" from the enable() > function to the init() function and did not make any other changes. > It seems that many drivers also put the "init code" and "Exit sleep > mode" in one function. You have moved the functions that actually enable the display out. And by the definition it's expected that there is no video stream during pre_enable(), it gets turned on afterwards. That's why I asked if there is any kind of garbage or not. > In addition, we briefly tested the kingdisplay_kd101ne3 panel and > melfas_lmfbx101117480 panel, and it seems that there is no garbage on > the panel. Ack. > > BR > > > > > > > > Signed-off-by: Zhaoxiong Lv > > > --- > > > .../gpu/drm/panel/panel-jadard-jd9365da-h3.c | 59 ++- > > > 1 file changed, 32 insertions(+), 27 deletions(-) > > -- With best wishes Dmitry
Re: [PATCH] fbdev/hpfb: Fix an error handling path in hpfb_dio_probe()
On 7/29/24 17:59, Dan Carpenter wrote:
On Mon, Jul 29, 2024 at 10:13:17AM +0200, Helge Deller wrote:
On 7/28/24 20:29, Christophe JAILLET wrote:
If an error occurs after request_mem_region(), a corresponding
release_mem_region() should be called, as already done in the remove
function.
True.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I think we can drop this "Fixes" tag, as it gives no real info.
If we're backporting patches then these tags really are useful. As
I've been doing more and more backporting, I've come to believe this
more firmly.
Sure, "Fixes" tags are useful, but only if they really refer
to another patch which introduced the specific issue.
But the tag 1da177e4c3f4 ("Linux-2.6.12-rc2") isn't useful, as it's
just the initial git commit. It has no relation to why release_mem_region()
might have been initially missed. See:
commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 (tag: v2.6.12-rc2)
Author: Linus Torvalds
Date: Sat Apr 16 15:20:36 2005 -0700
Linux-2.6.12-rc2
Initial git repository build. I'm not bothering with the full history,
even though we have it. We can create a separate "historical" git
archive of that later if we want to, and in the meantime it's about
3.2GB when imported into git - space that would just make the early
git days unnecessarily complicated, when we don't have a lot of good
infrastructure for it.
Helge
Re: (subset) [PATCH 00/13] rockchip: Enable 4K@60Hz mode on RK3228, RK3328, RK3399 and RK356x
On Sat, 15 Jun 2024 17:03:51 +, Jonas Karlman wrote: > This prepares and enable use of HDMI2.0 modes, e.g. 4K@60Hz, on RK3228, > RK3328, RK3399 and RK356x. > > Patch 1-3 fixes some issues to help support use of high-resolution modes. > > Patch 4 fixes reading of EDID on RK3328 when using a forced mode. > > [...] Applied, thanks! [02/13] clk: rockchip: Set parent rate for DCLK_VOP clock on RK3228 commit: 1d34b9757523c1ad547bd6d040381f62d74a3189 Best regards, -- Heiko Stuebner
Re: [PATCH] fbdev/hpfb: Fix an error handling path in hpfb_dio_probe()
Le 29/07/2024 à 22:09, Helge Deller a écrit :
On 7/29/24 17:59, Dan Carpenter wrote:
On Mon, Jul 29, 2024 at 10:13:17AM +0200, Helge Deller wrote:
On 7/28/24 20:29, Christophe JAILLET wrote:
If an error occurs after request_mem_region(), a corresponding
release_mem_region() should be called, as already done in the remove
function.
True.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I think we can drop this "Fixes" tag, as it gives no real info.
If we're backporting patches then these tags really are useful. As
I've been doing more and more backporting, I've come to believe this
more firmly.
Sure, "Fixes" tags are useful, but only if they really refer
to another patch which introduced the specific issue.
But the tag 1da177e4c3f4 ("Linux-2.6.12-rc2") isn't useful, as it's
just the initial git commit. It has no relation to why release_mem_region()
might have been initially missed. See:
I agree that the description of this specific tag is not useful by itself.
But at least it means: should it be backported, it can be done up to
there. (and sometimes LWN gives some statistics on how long it took to
fix an "issue", should it be considered as such)
Without it, it is not easy to guess in which branch the patch is meaningful.
I'll sent a v2 with your suggested minimal change, but I'll keep the
Fixes tag.
Up to you to remove it or not, and to add a or a
or none of them.
Any solution is fine with me.
commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 (tag: v2.6.12-rc2)
Author: Linus Torvalds
Date: Sat Apr 16 15:20:36 2005 -0700
Linux-2.6.12-rc2
Initial git repository build. I'm not bothering with the full history,
even though we have it. We can create a separate "historical" git
archive of that later if we want to, and in the meantime it's about
3.2GB when imported into git - space that would just make the early
git days unnecessarily complicated, when we don't have a lot of good
infrastructure for it.
Helge
On:
> HP300 are old HP machines with an m68k CPU.
> Not sure if someone still has such a machine 🙂
so it really was the one I found on wikipedia, LoL!
So, another way to "fix" the issue is maybe to deprecate the driver or
everything related to this old architecture?
No strong opinion about it.
CJ
[PATCH v3 1/2] dt-bindings: display: panel: samsung, atna45dc02: Document ATNA45DC02
From: Rob Clark The Samsung ATNA45DC02 panel is an AMOLED eDP panel, similar to the existing ATNA45AF01 and ATNA33XC20 panel but with a higher resolution. Signed-off-by: Rob Clark Acked-by: Conor Dooley --- .../bindings/display/panel/samsung,atna33xc20.yaml | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml index 5192c93fbd67..87c601bcf20a 100644 --- a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml +++ b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml @@ -17,10 +17,13 @@ properties: oneOf: # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel - const: samsung,atna33xc20 - # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel - items: - - const: samsung,atna45af01 - - const: samsung,atna33xc20 +- enum: + # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel + - samsung,atna45af01 + # Samsung 14.5" 3K (2944x1840 pixels) eDP AMOLED panel + - samsung,atna45dc02 +- const: samsung,atna33xc20 enable-gpios: true port: true -- 2.45.2
Re: [PATCH 1/2] drivers: drm/msm/a6xx_catalog: Add A642L speedbin (0x81)
On 7/29/24 06:09, Bjorn Andersson wrote: On Mon, Jul 22, 2024 at 09:43:13PM GMT, Danila Tikhonov wrote: From: Eugene Lepshy Please make sure the subject prefix matches other changes in the same driver/files. Regards, Bjorn Thanks for the advice "drm/msm/a6xx: --//--" will be better? Best wishes, Danila
Re: [PATCH] fbdev/hpfb: Fix an error handling path in hpfb_dio_probe()
On Mon, Jul 29, 2024 at 10:09:39PM +0200, Helge Deller wrote:
> On 7/29/24 17:59, Dan Carpenter wrote:
> > On Mon, Jul 29, 2024 at 10:13:17AM +0200, Helge Deller wrote:
> > > On 7/28/24 20:29, Christophe JAILLET wrote:
> > > > If an error occurs after request_mem_region(), a corresponding
> > > > release_mem_region() should be called, as already done in the remove
> > > > function.
> > >
> > > True.
> > >
> > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > >
> > > I think we can drop this "Fixes" tag, as it gives no real info.
> >
> > If we're backporting patches then these tags really are useful. As
> > I've been doing more and more backporting, I've come to believe this
> > more firmly.
>
> Sure, "Fixes" tags are useful, but only if they really refer
> to another patch which introduced the specific issue.
>
> But the tag 1da177e4c3f4 ("Linux-2.6.12-rc2") isn't useful, as it's
> just the initial git commit. It has no relation to why release_mem_region()
> might have been initially missed. See:
In the last couple stable kernels we've backported some pretty serious
networking commits that have Linux-2.6.12-rc2 for the Fixes tag. So if
it's security related that's really important information.
For minor stuff like this, the commit will be backported as far back as
possible and until it ends up in a list of failed commits.
When I'm reviewing the list of failed patches and there is no Fixes tag
I think maybe it was backported to all the affected kernels? In that
case, I could have skipped the manual review if the patch was just
tagged correctly. Then I wonder, why wasn't it tagged? I just assume
it was sloppiness honestly. I'm probably not going to spend that much
time on it, but it's annoying.
When a commit lists Linux-2.6.12-rc2 as the Fixes then it still ends up
in the failed list. But it can't affect too many users if we're only
getting around to fixing it now. It's easier to ignore.
regards,
dan carpenter
Re: [PATCH] drm/xe/oa/uapi: Make bit masks unsigned
On Mon, 29 Jul 2024 06:21:20 -0700, Lucas De Marchi wrote: > Hi Lucas, > Reviewed-by: Lucas De Marchi > > That fixes the build, but question to Ashutosh: it's odd to tie the > format to a bspec. What happens on next platform if the HW changes? > Hopefully it doesn't change in an incompatible way, but looking at this > code it seems we could still keep the uapi by untying the HW from the > uapi in the documentation. IMO, in this case, it is not possible to decouple the formats from Bspec because that is where they are specified (in Bspec 52198/60942). In i915 the OA formats were specified by an enum (enum drm_i915_oa_format), but I would argue that enum is meaningful only when we refer back to Bspec. Also the i915 enum had to constantly updated when HW added new formats. For Xe, we changed the way the formats are specified in a way which we believe will make the uapi more robust and uapi header update much less frequent (hopefully we will never have to update the header and if at all we have to, we should be able to do it in a backwards compatible way since we have sufficient number of free bits). HW has followed this scheme for specifying the formats for years and only recently for Xe2 has added a couple of bits and introduced new PEC formats which I think it is not going to change now for some time. But as I said the formats have to refer back to Bspec since that is where there are specified and there are too many of them. Any description or enum is ambiguous unless it refers back to Bspec. So I don't see how not to refer to Bspec in the documentation. If anyone has any ideas about not referring to Bspec I am willing to consider it but I think the best way forward is to leave the documentation as is: /* * OA_FORMAT's are specified the same way as in PRM/Bspec 52198/60942, * in terms of the following quantities: a. enum @drm_xe_oa_format_type * b. Counter select c. Counter size and d. BC report. Also refer to the * oa_formats array in drivers/gpu/drm/xe/xe_oa.c. */ #define DRM_XE_OA_FORMAT_MASK_FMT_TYPE (0xff << 0) #define DRM_XE_OA_FORMAT_MASK_COUNTER_SEL (0xff << 8) #define DRM_XE_OA_FORMAT_MASK_COUNTER_SIZE (0xff << 16) #define DRM_XE_OA_FORMAT_MASK_BC_REPORT (0xff << 24) Thanks. -- Ashutosh
Re: [PATCH] RFC: drm/drm_plane: Expose the plane capability and interoperability
On Mon, Jul 29, 2024 at 04:59:14AM GMT, Murthy, Arun R wrote:
> Gentle Reminder!
> Any comments?
First of all, the format is underdocumented. Second, there is a usual
requirement for new uAPI: please provide a pointer to IGT patch and to
the userspace utilising the property.
>
> Thanks and Regards,
> Arun R Murthy
>
>
> > -Original Message-
> > From: Murthy, Arun R
> > Sent: Tuesday, July 9, 2024 1:17 PM
> > To: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org
> > Cc: Murthy, Arun R
> > Subject: [PATCH] RFC: drm/drm_plane: Expose the plane capability and
> > interoperability
> >
> > Each plane has its own capability or interoperability based on the harware
> > restrictions. If this is exposed to the user, then user can read it once on
> > boot
> > and store this. Later can be used as a lookup table to check a corresponding
> > capability is supported by plane then only go ahead with the framebuffer
> > creation/calling atomic_ioctl.
> >
> > For Ex: There are few restiction as to async flip doesnt support all the
> > formats/modifiers. Similar restrictions on scaling. With the availabililty
> > of this
> > info to user, failures in atomic_check can be avoided as these are more the
> > hardware capabilities.
> >
> > There are two options on how this can be acheived.
> > Option 1:
> >
> > Add a new element to struct drm_mode_get_plane that holds the addr to the
> > array of a new struct. This new struct consists of the formats supported
> > and the
> > corresponding plane capabilities.
> >
> > Option 2:
> >
> > These can be exposed to user as a plane property so that the user can get to
> > know the limitation ahead and avoid failures in atomic_check.
> >
> > Usually atomic_get_property is controlled over the state struct for the
> > parameters/elements that can change. But here these capabilities or the
> > interoperabilities are rather hardware restrictions and wont change over
> > flips.
> > Hence having as a plane_property may not make much sense.
> > On the other hand, Option 1 include changes in the uapi struct
> > drm_mode_get_plane. Shouldnt have impact on backward compatibility, but if
> > userspace has some implementation so as to check the size of the struct,
> > then it
> > might a challenge.
> >
> > Signed-off-by: Arun R Murthy
> > ---
> > drivers/gpu/drm/drm_atomic_uapi.c | 3 +++
> > include/drm/drm_plane.h | 8
> > include/uapi/drm/drm_mode.h | 20
> > 3 files changed, 31 insertions(+)
> >
> > =Option 2
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 22bbb2d83e30..b46177d5fc8c 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -631,6 +631,9 @@ drm_atomic_plane_get_property(struct drm_plane
> > *plane,
> > *val = state->hotspot_x;
> > } else if (property == plane->hotspot_y_property) {
> > *val = state->hotspot_y;
> > + } else if (property == config->prop_plane_caps) {
> > + *val = (state->plane_caps) ?
> > + state->plane_caps->base.id : 0;
> > } else {
> > drm_dbg_atomic(dev,
> >"[PLANE:%d:%s] unknown property
> > [PROP:%d:%s]\n", diff --git a/include/drm/drm_plane.h
> > b/include/drm/drm_plane.h index dd718c62ac31..dfe931677d0a 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -260,6 +260,14 @@ struct drm_plane_state {
> > * flow.
> > */
> > bool color_mgmt_changed : 1;
> > +
> > + /**
> > +* @plane_caps:
> > +*
> > +* Blob representing plane capcabilites and interoperability.
> > +* This element is a pointer to the array of struct drm_format_blob.
> > +*/
> > + struct drm_property_blob *plane_caps;
> > };
> >
> > static inline struct drm_rect
> >
> > =Option 1
> >
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index d390011b89b4..0b5c1b65ef63 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -312,6 +312,20 @@ struct drm_mode_set_plane {
> > __u32 src_w;
> > };
> >
> > +#define DRM_FORMAT_PLANE_CAP_LINEAR_TILE BIT(0)
> > +#define DRM_FORMAT_PLANE_CAP_X_TILEBIT(1)
> > +#define DRM_FORMAT_PLANE_CAP_Y_TILEBIT(2)
> > +#define DRM_FORMAT_PLANE_CAP_Yf_TILE BIT(3)
> > +#define DRM_FORMAT_PLANE_CAP_ASYNC_FLIPBIT(4)
> > +#define DRM_FORMAT_PLANE_CAP_FBC BIT(5)
> > +#define DRM_FORMAT_PLANE_CAP_RCBIT(6)
> > +
> > +struct drm_format_blob {
> > + __u64 modifier;
> > + __u32 plane_caps;
> > +
> > +};
> > +
> > /**
> > * struct drm_mode_get_plane - Get plane metadata.
> > *
> > @@ -355,6 +369,12 @@ struct drm_mode_get_plane {
> > * supported by the plane. These fo
Re: [PATCH v3] rockchip/drm: vop2: add support for gamma LUT
Hi Andy
On Monday, July 29th, 2024 at 4:35 AM, Andy Yan wrote:
> > > > +
> > > > +static void vop2_crtc_gamma_set(struct vop2 *vop2, struct drm_crtc
> > > > *crtc,
> > > > + struct drm_crtc_state *old_state)
> > > > +{
> > > > + struct drm_crtc_state *state = crtc->state;
> > > > + struct vop2_video_port *vp = to_vop2_video_port(crtc);
> > > > + u32 dsp_ctrl;
> > > > + int ret;
> > > > +
> > > > + if (!vop2->lut_regs)
> > > > + return;
> > > > +
> > > > + if (!state->gamma_lut) {
> > >
> > > What's the purpose of checking !state->gamma_lut here,
> > >
> > > and you check it again at the end for return.
> > > This makes me very confused.
> >
> > I understood it this way - since the vop2 lock is unlocked after disabling
> > gamma LUT, the CRTC state can change while waiting for DSP_LUT_EN bit to
> > be unset. With the change I sent in response to Daniel's reply, gamma LUT
> > state modification should now be fully atomic so there shouldn't be a need
> > for the second state check there anymore (if my logic is incorrect please
> > explain).
>
>
> After reading the commit message for adding gamma control for rk3399[0] i
> understand
> what is going on here:
>
> we should run into the if block in two cases:
>
> (1) if the state->gamma_lut is null, we just need to disable dsp_lut and
> return,
>
> this is why vop1 code check !state->gamma_lut again at the end.
>
> (2) for platform unlinke rk3399(rk3566/8), we also need to disable dsp_lut
> befor we
> write gamma_lut data, for platform like rk3399(rk3588), we don't need do the
> disable,
> this is why vop1 code also has a !VOP_HAS_REG(vop, common, update_gamma_lut)
> check.
>
> so we also need a similary check here:
> (1) if the state->gamma_lut is null, disable dsp lut and return directly.
>
> (1) if the state has a gamma_lut, we shoud dsiable dsp_lut than write gamma
> lut data on rk3566/8,
> buf for rk3588, we should not disable dsp_lut before write gamma
>
>
> [0]https://lists.infradead.org/pipermail/linux-rockchip/2021-October/028132.html
>
Ok I see it. So In my patch it doesn't make sense at all to check it again
(forgot about that extra if statement condition there, which I cut out
when porting to VOP2). I reworked my patch further for it to handle RK3588
case and to better utilize DRM atomic updates. It's contained in the
response to Daniel's review [1]. I experienced some problems so I'm waiting
for his response/comment on that.
Regarding RK3588, I checked RK3588 TRM v1.0 part2. In its VOP2 section I
found:
- SYS_CTRL_LUT_PORT_SEL: gamma_ahb_write_sel (seems to represent the
same concept as LUT_PORT_SEL in case of RK356x)
- VOP2_POST0_DSP_CTRL: gamma_update_en (seems to represent the same
concept as in VOP1 in case of RK3399)
- I also found dsp_lut_en but I presume it is a bug in documentation.
Should RK3588 be handled as RK3399? (gamma LUT can be written directly but
gamma_update_en bit has to be set before). What about gamma_ahb_write_sel?
[1] https://lkml.org/lkml/2024/7/27/293
Best Regards, Piotr Zalewski
