Re: [PATCH] drm/etnaviv: Create an accel device node if compute-only
On Thu, Apr 25, 2024 at 1:32 PM Christian Gmeiner
wrote:
>
> Hi Tomeu,
>
> >
> > If we expose a render node for NPUs without rendering capabilities, the
> > userspace stack will offer it to compositors and applications for
> > rendering, which of course won't work.
> >
> > Userspace is probably right in not questioning whether a render node
> > might not be capable of supporting rendering, so change it in the kernel
> > instead by exposing a /dev/accel node.
> >
> > Before we bring the device up we don't know whether it is capable of
> > rendering or not (depends on the features of its blocks), so first try
> > to probe a rendering node, and if we find out that there is no rendering
> > hardware, abort and retry with an accel node.
> >
>
> I really love this idea of moving away from a render node. What needs to be
> done
> on the userspace side?
Doesn't seem that bad, here is a proof of concept:
https://gitlab.freedesktop.org/tomeu/mesa/-/tree/teflon-accel
Thanks for taking a look.
Tomeu
> > Signed-off-by: Tomeu Vizoso
> > Cc: Oded Gabbay
>
> Reviewed-by: Christian Gmeiner
>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 46 ++-
> > 1 file changed, 38 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index 6500f3999c5f..8e7dd23115f4 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -11,6 +11,7 @@
> > #include
> > #include
> >
> > +#include
> > #include
> > #include
> > #include
> > @@ -488,10 +489,10 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] =
> > {
> > ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> > };
> >
> > -DEFINE_DRM_GEM_FOPS(fops);
> > +DEFINE_DRM_GEM_FOPS(render_fops);
> > +DEFINE_DRM_ACCEL_FOPS(accel_fops);
> >
> > -static const struct drm_driver etnaviv_drm_driver = {
> > - .driver_features= DRIVER_GEM | DRIVER_RENDER,
> > +static struct drm_driver etnaviv_drm_driver = {
> > .open = etnaviv_open,
> > .postclose = etnaviv_postclose,
> > .gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> > @@ -500,7 +501,6 @@ static const struct drm_driver etnaviv_drm_driver = {
> > #endif
> > .ioctls = etnaviv_ioctls,
> > .num_ioctls = DRM_ETNAVIV_NUM_IOCTLS,
> > - .fops = ,
> > .name = "etnaviv",
> > .desc = "etnaviv DRM",
> > .date = "20151214",
> > @@ -508,15 +508,20 @@ static const struct drm_driver etnaviv_drm_driver = {
> > .minor = 4,
> > };
> >
> > -/*
> > - * Platform driver:
> > - */
> > -static int etnaviv_bind(struct device *dev)
> > +static int etnaviv_bind_with_type(struct device *dev, u32 type)
> > {
> > struct etnaviv_drm_private *priv;
> > struct drm_device *drm;
> > + bool is_compute_only = true;
> > int ret;
> >
> > + etnaviv_drm_driver.driver_features = DRIVER_GEM | type;
> > +
> > + if (type == DRIVER_RENDER)
> > + etnaviv_drm_driver.fops = _fops;
> > + else
> > + etnaviv_drm_driver.fops = _fops;
> > +
> > drm = drm_dev_alloc(_drm_driver, dev);
> > if (IS_ERR(drm))
> > return PTR_ERR(drm);
> > @@ -553,6 +558,18 @@ static int etnaviv_bind(struct device *dev)
> >
> > load_gpu(drm);
> >
> > + for (unsigned int i = 0; i < ETNA_MAX_PIPES; i++) {
> > + struct etnaviv_gpu *g = priv->gpu[i];
> > +
> > + if (g && (g->identity.minor_features8 &
> > chipMinorFeatures8_COMPUTE_ONLY) == 0)
> > + is_compute_only = false;
> > + }
> > +
> > + if (type == DRIVER_RENDER && is_compute_only) {
> > + ret = -EINVAL;
> > + goto out_unbind;
> > + }
> > +
> > ret = drm_dev_register(drm, 0);
> > if (ret)
> > goto out_unbind;
> > @@ -571,6 +588,19 @@ static int etnaviv_bind(struct device *dev)
> > return ret;
> > }
> >
> > +/*
> > + * Platform driver:
> > + */
> > +static int etnaviv_bind(struct device *dev)
> > +{
> > + int ret = etnaviv_bind_with_type(dev, DRIVER_RENDER);
> > +
> > + if (ret == -EINVAL)
> > + return etnaviv_bind_with_type(dev, DRIVER_COMPUTE_ACCEL);
> > +
> > + return ret;
> > +}
> > +
> > static void etnaviv_unbind(struct device *dev)
> > {
> > struct drm_device *drm = dev_get_drvdata(dev);
> > --
> > 2.44.0
> >
>
>
> --
> greets
> --
> Christian Gmeiner, MSc
>
> https://christian-gmeiner.info/privacypolicy
Re: [PATCH V2 2/2] drm/bridge: samsung-dsim: Fix porch calcalcuation rounding
On 25.04.2024 22:30, Adam Ford wrote:
> On Thu, Apr 25, 2024 at 4:19 AM Marek Szyprowski
> wrote:
>> On 12.02.2024 00:09, Adam Ford wrote:
>>> When using video sync pulses, the HFP, HBP, and HSA are divided between
>>> the available lanes if there is more than one lane. For certain
>>> timings and lane configurations, the HFP may not be evenly divisible.
>>> If the HFP is rounded down, it ends up being too small which can cause
>>> some monitors to not sync properly. In these instances, adjust htotal
>>> and hsync to round the HFP up, and recalculate the htotal.
>>>
>>> Tested-by: Frieder Schrempf # Kontron BL
>>> i.MX8MM with HDMI monitor
>>> Signed-off-by: Adam Ford
>> Tested-by: Marek Szyprowski
> Thank you very much for testing!
>
>>> ---
>>> V2: No changes
>>>
>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> index 8476650c477c..52939211fe93 100644
>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> @@ -1606,6 +1606,27 @@ static int samsung_dsim_atomic_check(struct
>>> drm_bridge *bridge,
>>>adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC |
>>> DRM_MODE_FLAG_PVSYNC);
>>>}
>>>
>>> + /*
>>> + * When using video sync pulses, the HFP, HBP, and HSA are divided
>>> between
>>> + * the available lanes if there is more than one lane. For certain
>>> + * timings and lane configurations, the HFP may not be evenly
>>> divisible.
>>> + * If the HFP is rounded down, it ends up being too small which can
>>> cause
>>> + * some monitors to not sync properly. In these instances, adjust
>>> htotal
>>> + * and hsync to round the HFP up, and recalculate the htotal. Through
>>> trial
>>> + * and error, it appears that the HBP and HSA do not appearto need
>>> the same
>>> + * correction that HFP does.
>>> + */
>>> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE && dsi->lanes >
>>> 1) {
> Frieder & Marek S,
>
> Marek V is proposing we eliminate the check against the flags and do
> it unconditionally. If I send you both a different patch, would you
> be willing to try them on your platforms? I don't want to risk
> breaking a board.
I'm fine with testing it. I also have some additional spare boards to
replace the broken one, but so far none was bricked by my weird testing
activities.
> I used the check above from the NXP downstream kernel, so it felt
> safe, but I am not as familiar with the different DSI modes, so I am
> not sure what the impact would be if this read:
>
> if (dsi->lanes > 1) {
>
> Does anyone else have an opinion on this?
>>> + int hfp = adjusted_mode->hsync_start -
>>> adjusted_mode->hdisplay;
>>> + int remainder = hfp % dsi->lanes;
>>> +
>>> + if (remainder) {
>>> + adjusted_mode->hsync_start += remainder;
>>> + adjusted_mode->hsync_end += remainder;
>>> + adjusted_mode->htotal += remainder;
>>> + }
>>> + }
>>> +
>>>return 0;
>>>}
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
[git pull] drm fixes for 6.8-rc6
Hi Linus, Regular weekly merge request, mostly amdgpu and misc bits in xe/etnaviv/gma500 and some core changes. Nothing too outlandish, seems to be about normal for this time of release. Regards, Dave. drm-fixes-2024-04-26: drm fixes for 6.9-rc6 atomic-helpers: - Fix memory leak in drm_format_conv_state_copy() fbdev: - fbdefio: Fix address calculation amdgpu: - Suspend/resume fix - Don't expose gpu_od directory if it's empty - SDMA 4.4.2 fix - VPE fix - BO eviction fix - UMSCH fix - SMU 13.0.6 reset fixes - GPUVM flush accounting fix - SDMA 5.2 fix - Fix possible UAF in mes code amdkfd: - Eviction fence handling fix - Fix memory leak when GPU memory allocation fails - Fix dma-buf validation - Fix rescheduling of restore worker - SVM fix gma500: - Fix crash during boot etnaviv: - fix GC7000 TX clock gating - revert NPU UAPI changes xe: - Fix error paths on managed allocations - Fix PF/VF relay messages The following changes since commit ed30a4a51bb196781c8058073ea720133a65596f: Linux 6.9-rc5 (2024-04-21 12:35:54 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/drm/kernel.git tags/drm-fixes-2024-04-26 for you to fetch changes up to 3a8534035c0747610312f9552898a0ece10ef8a7: Merge tag 'drm-xe-fixes-2024-04-25' of https://gitlab.freedesktop.org/drm/xe/kernel into drm-fixes (2024-04-26 12:56:58 +1000) drm fixes for 6.9-rc6 atomic-helpers: - Fix memory leak in drm_format_conv_state_copy() fbdev: - fbdefio: Fix address calculation amdgpu: - Suspend/resume fix - Don't expose gpu_od directory if it's empty - SDMA 4.4.2 fix - VPE fix - BO eviction fix - UMSCH fix - SMU 13.0.6 reset fixes - GPUVM flush accounting fix - SDMA 5.2 fix - Fix possible UAF in mes code amdkfd: - Eviction fence handling fix - Fix memory leak when GPU memory allocation fails - Fix dma-buf validation - Fix rescheduling of restore worker - SVM fix gma500: - Fix crash during boot etnaviv: - fix GC7000 TX clock gating - revert NPU UAPI changes xe: - Fix error paths on managed allocations - Fix PF/VF relay messages Alex Deucher (1): drm/amdgpu/sdma5.2: use legacy HDP flush for SDMA2/3 Christian Gmeiner (1): Revert "drm/etnaviv: Expose a few more chipspecs to userspace" Dave Airlie (4): Merge tag 'amd-drm-fixes-6.9-2024-04-24' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Merge tag 'drm-misc-fixes-2024-04-25' of https://gitlab.freedesktop.org/drm/misc/kernel into drm-fixes Merge tag 'drm-etnaviv-fixes-2024-04-25' of https://git.pengutronix.de/git/lst/linux into drm-fixes Merge tag 'drm-xe-fixes-2024-04-25' of https://gitlab.freedesktop.org/drm/xe/kernel into drm-fixes Derek Foreman (1): drm/etnaviv: fix tx clock gating on some GC7000 variants Felix Kuehling (3): drm/amdkfd: Fix eviction fence handling drm/amdgpu: Update BO eviction priorities drm/amdkfd: Fix rescheduling of restore worker Himal Prasad Ghimiray (2): drm/xe: Remove sysfs only once on action add failure drm/xe: call free_gsc_pkt only once on action add failure Jack Xiao (1): drm/amdgpu/mes: fix use-after-free issue Joshua Ashton (1): drm/amd/display: Set color_mgmt_changed to true on unsuspend Lang Yu (2): drm/amdkfd: make sure VM is ready for updating operations drm/amdgpu/umsch: don't execute umsch test when GPU is in reset/suspend Lijo Lazar (2): drm/amdgpu: Assign correct bits for SDMA HDP flush drm/amd/pm: Restore config space after reset Lucas Stach (1): drm/atomic-helper: fix parameter order in drm_format_conv_state_copy() call Ma Jun (1): drm/amdgpu/pm: Remove gpu_od if it's an empty directory Michal Wajdeczko (1): drm/xe/guc: Fix arguments passed to relay G2H handlers Mukul Joshi (2): drm/amdgpu: Fix leak when GPU memory allocation fails drm/amdkfd: Add VRAM accounting for SVM migration Nam Cao (1): fbdev: fix incorrect address computation in deferred IO Patrik Jakobsson (1): drm/gma500: Remove lid code Peyton Lee (1): drm/amdgpu/vpe: fix vpe dpm setup failed Prike Liang (1): drm/amdgpu: Fix the ring buffer size for queue VM flush drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 35 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c | 3 + drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c| 2 +- drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 3 +- drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 3 +- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 - drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 3 +- drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 26 --- drivers/gpu/drm/amd/amdgpu/vpe_v6_1.c | 14 ++--
Re: [PATCH] MAINTAINERS: fix LG sw43408 panel driver drm-misc git URL
On Thu, Apr 25, 2024 at 02:03:52PM +0300, Jani Nikula wrote: > The drm-misc git repo has moved to Gitlab. Fix the URL. > > Cc: Sumit Semwal > Cc: Caleb Connolly > Signed-off-by: Jani Nikula Reviewed-by: Dmitry Baryshkov Applying to drm-misc > --- > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index d6327dc12cb1..23997d2ea91c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6766,7 +6766,7 @@ DRM DRIVER FOR LG SW43408 PANELS > M: Sumit Semwal > M: Caleb Connolly > S: Maintained > -T: git git://anongit.freedesktop.org/drm/drm-misc > +T: git https://gitlab.freedesktop.org/drm/misc/kernel.git > F: Documentation/devicetree/bindings/display/panel/lg,sw43408.yaml > F: drivers/gpu/drm/panel/panel-lg-sw43408.c > > -- > 2.39.2 > -- With best wishes Dmitry
Re: [PATCH] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq()
On Thu, Apr 25, 2024 at 10:04:49AM -0700, Doug Anderson wrote:
> Hi,
>
> On Thu, Apr 25, 2024 at 1:19 AM Jani Nikula
> wrote:
> >
> > > @@ -279,6 +281,8 @@ enum mipi_dsi_dcs_tear_mode {
> > >
> > > ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi,
> > > const void *data, size_t len);
> > > +ssize_t mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi,
> > > + const void *data, size_t len);
> > > ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
> > > const void *data, size_t len);
> > > ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void
> > > *data,
> > > @@ -317,14 +321,10 @@ int
> > > mipi_dsi_dcs_get_display_brightness_large(struct mipi_dsi_device *dsi,
> > > #define mipi_dsi_generic_write_seq(dsi, seq...)
> > > \
> > > do {
> > >\
> > > static const u8 d[] = { seq };
> > >\
> > > - struct device *dev = >dev;
> > >\
> > > int ret;
> > >\
> > > - ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));
> > >\
> > > - if (ret < 0) {
> > >\
> > > - dev_err_ratelimited(dev, "transmit data failed:
> > > %d\n", \
> > > - ret);
> > >\
> > > + ret = mipi_dsi_generic_write_chatty(dsi, d, ARRAY_SIZE(d));
> > >\
> > > + if (ret < 0)
> > >\
> > > return ret;
> > >\
> > > - }
> > >\
> > > } while (0)
Reading the thread makes me wonder whether we should be going into
slightly other direction:
Add __must_check() to mipi_dsi_ writing functions,
#define mipi_dsi_dcs_whatever_write(dsi, cmd, seq...) \
({ \
static const u8 d[] = { cmd, seq }; \
mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d));\
})
Then in panel drivers we actually have to explicitly handle the return
code (either by dropping to the error label or by just returning an
error).
> >
> > The one thing that I've always disliked about these macros (even if I've
> > never actually used them myself) is that they hide control flow from the
> > caller, i.e. return directly. You don't see that in the code, it's not
> > documented, and if you wanted to do better error handling yourself,
> > you're out of luck.
>
> Yeah, I agree that it's not the cleanest. That being said, it is
> existing code and making the existing code less bloated seems worth
> doing.
>
> I'd also say that it feels worth it to have _some_ solution so that
> the caller doesn't need to write error handling after every single cmd
> sent. If we get rid of / discourage these macros that's either going
> to end us up with ugly/verbose code or it's going to encourage people
> to totally skip error handling. IMO neither of those are wonderful
> solutions.
>
> While thinking about this there were a few ideas I came up with. None
> of them are amazing, but probably they are better than the hidden
> "return" like this. Perhaps we could mark the current function as
> "deprecated" and pick one of these depending on what others opinions
> are:
>
> 1. Use "goto" and force the caller to give a goto target for error handling.
>
> This is based on an idea that Dmitry came up with, but made a little
> more explicit. Example usage:
>
> int ret;
>
> ret = 0;
> mipi_dsi_dcs_write_seq_goto(dsi, , HX83102_SETSPCCMD, 0xcd,
> some_cmd_failed);
> mipi_dsi_dcs_write_seq_goto(dsi, , HX83102_SETMIPI, 0x84,
> some_cmd_failed);
> mipi_dsi_dcs_write_seq_goto(dsi, , HX83102_SETSPCCMD, 0x3f,
> some_cmd_failed);
> mipi_dsi_dcs_write_seq_goto(dsi, , HX83102_SETVDC, 0x1b, 0x04,
> some_cmd_failed);
>
> ...
>
> some_cmd_failed:
> pr_err("Commands failed to write: %d", ret);
> return ret;
> }
>
> One downside here is that you can't easily tell which command failed
> to put it in the error message. A variant of this idea (1a?) could be
> to hoist the print back into the write command. I'd want to pick one
> or the other. I guess my preference would be to hoist the print into
> the write command and if someone really doesn't want the print then
> they call mipi_dsi_dcs_write_buffer() directly.
Do we really care, which command has failed? I mean, usually either all
DSI transfers work, or we have
linux-next: build failures after merge of the drm-misc tree
Hi all,
After merging the drm-misc tree, today's linux-next builds (arm
multi_v7_defconfig and x86_64 allmodconfig) failed like this:
(from the arm build)
drivers/gpu/drm/omapdrm/omap_fb.c: In function 'omap_framebuffer_describe':
drivers/gpu/drm/omapdrm/omap_fb.c:325:9: error: implicit declaration of
function 'seq_printf'; did you mean 'drm_printf'?
[-Werror=implicit-function-declaration]
325 | seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
| ^~
| drm_printf
(from the x86_64 build)
drivers/gpu/drm/loongson/lsdc_crtc.c: In function 'lsdc_crtc_late_register':
drivers/gpu/drm/loongson/lsdc_crtc.c:692:9: error: implicit declaration of
function 'debugfs_create_file'; did you mean 'bus_create_file'?
[-Werror=implicit-function-declaration]
692 | debugfs_create_file("ops", 0644, crtc->debugfs_entry, lcrtc,
| ^~~
| bus_create_file
Caused by commits
9e2b84fb6cd7 ("drm/print: drop include seq_file.h")
33d5ae6cacf4 ("drm/print: drop include debugfs.h and include where needed")
I have used the drm-misc tree from next-20240423 for today.
--
Cheers,
Stephen Rothwell
pgpZEcxWZVIqt.pgp
Description: OpenPGP digital signature
[PATCH v2 0/2] Fix Kernel CI issues
Fix number of CI reported W=1 build issues. Patch 1/2: Fix function arguments description. Closes: https://lore.kernel.org/oe-kbuild-all/202404260616.kfgdpcdn-...@intel.com/ Patch 2/2: Fix clang compilation error. Closes: https://lore.kernel.org/oe-kbuild-all/202404260946.4ozxvhd2-...@intel.com/ Signed-off-by: Anatoliy Klymenko --- Changes in v2: - Compilation error fix added. - Link to v1: https://lore.kernel.org/r/20240425-dp-live-fmt-fix-v1-1-405f352d3...@amd.com --- Anatoliy Klymenko (2): drm: xlnx: zynqmp_dpsub: Fix few function comments drm: xlnx: zynqmp_dpsub: Fix compilation error drivers/gpu/drm/xlnx/zynqmp_disp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- base-commit: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f change-id: 20240425-dp-live-fmt-fix-a10bf7973596 Best regards, -- Anatoliy Klymenko
[PATCH v2 2/2] drm: xlnx: zynqmp_dpsub: Fix compilation error
Fix W=1 clang 19 compilation error in zynqmp_disp_layer_drm_formats().
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202404260946.4ozxvhd2-...@intel.com/
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 423f5f4943cc..c9fb432d4cbd 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -981,7 +981,7 @@ u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer
*layer,
unsigned int i;
u32 *formats;
- if (WARN_ON(!layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE)) {
+ if (WARN_ON(layer->mode != ZYNQMP_DPSUB_LAYER_NONLIVE)) {
*num_formats = 0;
return NULL;
}
--
2.25.1
[PATCH v2 1/2] drm: xlnx: zynqmp_dpsub: Fix few function comments
Fix arguments description for zynqmp_disp_layer_find_live_format() and zynqmp_disp_layer_set_live_format(). Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202404260616.kfgdpcdn-...@intel.com/ Signed-off-by: Anatoliy Klymenko --- drivers/gpu/drm/xlnx/zynqmp_disp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c index 13157da0089e..423f5f4943cc 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c @@ -940,7 +940,7 @@ zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer, * zynqmp_disp_layer_find_live_format - Find format information for given * media bus format * @layer: The layer - * @drm_fmt: Media bus format to search + * @media_bus_format: Media bus format to search * * Search display subsystem format information corresponding to the given media * bus format @media_bus_format for the @layer, and return a pointer to the @@ -1117,7 +1117,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer, /** * zynqmp_disp_layer_set_live_format - Set the live video layer format * @layer: The layer - * @info: The format info + * @media_bus_format: Media bus format to set * * NOTE: This function should not be used to set format for non-live video * layer. Use zynqmp_disp_layer_set_format() instead. -- 2.25.1
Re: [PATCH v2 2/9] drm/ttm: Use LRU hitches
On Tue, Apr 16, 2024 at 12:07:23PM +0200, Thomas Hellström wrote:
> Have iterators insert themselves into the list they are iterating
> over using hitch list nodes. Since only the iterator owner
> can remove these list nodes from the list, it's safe to unlock
> the list and when continuing, use them as a starting point. Due to
> the way LRU bumping works in TTM, newly added items will not be
> missed, and bumped items will be iterated over a second time before
> reaching the end of the list.
>
> The exception is list with bulk move sublists. When bumping a
> sublist, a hitch that is part of that sublist will also be moved
> and we might miss items if restarting from it. This will be
> addressed in a later patch.
>
> Changes in previous series:
> - Updated ttm_resource_cursor_fini() documentation.
> v2:
> - Don't reorder ttm_resource_manager_first() and _next().
> (Christian König).
> - Use list_add instead of list_move
> (Christian König)
>
> Cc: Christian König
> Cc: Somalapuram Amaranath
> Cc:
> Signed-off-by: Thomas Hellström
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 1 +
> drivers/gpu/drm/ttm/ttm_device.c | 9 ++-
> drivers/gpu/drm/ttm/ttm_resource.c | 94 --
> include/drm/ttm/ttm_resource.h | 16 +++--
> 4 files changed, 82 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 6396dece0db1..43eda720657f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -621,6 +621,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
> if (locked)
> dma_resv_unlock(res->bo->base.resv);
> }
> + ttm_resource_cursor_fini_locked();
>
> if (!bo) {
> if (busy_bo && !ttm_bo_get_unless_zero(busy_bo))
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c
> b/drivers/gpu/drm/ttm/ttm_device.c
> index f27406e851e5..e8a6a1dab669 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -169,12 +169,17 @@ int ttm_device_swapout(struct ttm_device *bdev, struct
> ttm_operation_ctx *ctx,
> num_pages = PFN_UP(bo->base.size);
> ret = ttm_bo_swapout(bo, ctx, gfp_flags);
> /* ttm_bo_swapout has dropped the lru_lock */
> - if (!ret)
> + if (!ret) {
> + ttm_resource_cursor_fini();
> return num_pages;
> - if (ret != -EBUSY)
> + }
> + if (ret != -EBUSY) {
> + ttm_resource_cursor_fini();
> return ret;
> + }
> }
> }
> + ttm_resource_cursor_fini_locked();
> spin_unlock(>lru_lock);
> return 0;
> }
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> b/drivers/gpu/drm/ttm/ttm_resource.c
> index 7aa5ca5c0e33..22f8ae4ff4c0 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -32,6 +32,37 @@
>
> #include
>
> +/**
> + * ttm_resource_cursor_fini_locked() - Finalize the LRU list cursor usage
> + * @cursor: The struct ttm_resource_cursor to finalize.
> + *
> + * The function pulls the LRU list cursor off any lists it was previusly
> + * attached to. Needs to be called with the LRU lock held. The function
> + * can be called multiple times after eachother.
> + */
> +void ttm_resource_cursor_fini_locked(struct ttm_resource_cursor *cursor)
> +{
> + lockdep_assert_held(>man->bdev->lru_lock);
> + list_del_init(>hitch.link);
> +}
> +
> +/**
> + * ttm_resource_cursor_fini() - Finalize the LRU list cursor usage
> + * @cursor: The struct ttm_resource_cursor to finalize.
> + *
> + * The function pulls the LRU list cursor off any lists it was previusly
> + * attached to. Needs to be called without the LRU list lock held. The
> + * function can be called multiple times after eachother.
> + */
> +void ttm_resource_cursor_fini(struct ttm_resource_cursor *cursor)
> +{
> + spinlock_t *lru_lock = >man->bdev->lru_lock;
> +
> + spin_lock(lru_lock);
> + ttm_resource_cursor_fini_locked(cursor);
> + spin_unlock(lru_lock);
> +}
> +
> /**
> * ttm_lru_bulk_move_init - initialize a bulk move structure
> * @bulk: the structure to init
> @@ -484,61 +515,62 @@ void ttm_resource_manager_debug(struct
> ttm_resource_manager *man,
> EXPORT_SYMBOL(ttm_resource_manager_debug);
>
> /**
> - * ttm_resource_manager_first
> - *
> + * ttm_resource_manager_first() - Start iterating over the resources
> + * of a resource manager
> * @man: resource manager to iterate over
> * @cursor: cursor to record the position
> *
> - * Returns the first resource from the resource manager.
> + * Initializes the cursor and starts iterating. When done iterating,
> + * the caller must explicitly call ttm_resource_cursor_fini().
> + *
> + *
Re: [PATCH v2 3/9] drm/ttm, drm/amdgpu, drm/xe: Consider hitch moves within bulk sublist moves
On Tue, Apr 16, 2024 at 12:07:24PM +0200, Thomas Hellström wrote:
> To address the problem with hitches moving when bulk move
> sublists are lru-bumped, register the list cursors with the
> ttm_lru_bulk_move structure when traversing its list, and
> when lru-bumping the list, move the cursor hitch to the tail.
> This also means it's mandatory for drivers to call
> ttm_lru_bulk_move_init() and ttm_lru_bulk_move_fini() when
> initializing and finalizing the bulk move structure, so add
> those calls to the amdgpu- and xe driver.
>
> Compared to v1 this is slightly more code but less fragile
> and hopefully easier to understand.
>
> Changes in previous series:
> - Completely rework the functionality
> - Avoid a NULL pointer dereference assigning manager->mem_type
> - Remove some leftover code causing build problems
> v2:
> - For hitch bulk tail moves, store the mem_type in the cursor
> instead of with the manager.
>
> Cc: Christian König
> Cc: Somalapuram Amaranath
> Cc:
> Signed-off-by: Thomas Hellström
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++
> drivers/gpu/drm/ttm/ttm_resource.c | 92 +-
> drivers/gpu/drm/xe/xe_vm.c | 4 ++
> include/drm/ttm/ttm_resource.h | 58 ++--
> 4 files changed, 137 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 4299ce386322..18bf174c8d47 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2368,6 +2368,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
> amdgpu_vm *vm,
> if (r)
> return r;
>
> + ttm_lru_bulk_move_init(>lru_bulk_move);
> +
> vm->is_compute_context = false;
>
> vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
> @@ -2431,6 +2433,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
> amdgpu_vm *vm,
> error_free_delayed:
> dma_fence_put(vm->last_tlb_flush);
> dma_fence_put(vm->last_unlocked);
> + ttm_lru_bulk_move_fini(>mman.bdev, >lru_bulk_move);
> amdgpu_vm_fini_entities(vm);
>
> return r;
> @@ -2587,6 +2590,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
> amdgpu_vm *vm)
> }
> }
>
> + ttm_lru_bulk_move_fini(>mman.bdev, >lru_bulk_move);
> }
>
> /**
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> b/drivers/gpu/drm/ttm/ttm_resource.c
> index 22f8ae4ff4c0..2b93727c78e5 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -32,6 +32,49 @@
>
> #include
>
> +/* Detach the cursor from the bulk move list*/
> +static void
> +ttm_resource_cursor_clear_bulk(struct ttm_resource_cursor *cursor)
> +{
> + cursor->bulk = NULL;
> + list_del_init(>bulk_link);
> +}
> +
> +/* Move the cursor to the end of the bulk move list it's in */
> +static void ttm_resource_cursor_move_bulk_tail(struct ttm_lru_bulk_move
> *bulk,
> +struct ttm_resource_cursor
> *cursor)
> +{
> + struct ttm_lru_bulk_move_pos *pos;
> +
> + if (WARN_ON_ONCE(bulk != cursor->bulk)) {
> + list_del_init(>bulk_link);
> + return;
> + }
> +
> + pos = >pos[cursor->mem_type][cursor->priority];
> + if (pos)
> + list_move(>hitch.link, >last->lru.link);
> + ttm_resource_cursor_clear_bulk(cursor);
> +}
> +
> +/* Move all cursors attached to a bulk move to its end */
> +static void ttm_bulk_move_adjust_cursors(struct ttm_lru_bulk_move *bulk)
> +{
> + struct ttm_resource_cursor *cursor, *next;
> +
> + list_for_each_entry_safe(cursor, next, >cursor_list, bulk_link)
> + ttm_resource_cursor_move_bulk_tail(bulk, cursor);
> +}
> +
> +/* Remove a cursor from an empty bulk move list */
> +static void ttm_bulk_move_drop_cursors(struct ttm_lru_bulk_move *bulk)
> +{
> + struct ttm_resource_cursor *cursor, *next;
> +
> + list_for_each_entry_safe(cursor, next, >cursor_list, bulk_link)
> + ttm_resource_cursor_clear_bulk(cursor);
>
+}
> +
> /**
> * ttm_resource_cursor_fini_locked() - Finalize the LRU list cursor usage
> * @cursor: The struct ttm_resource_cursor to finalize.
> @@ -44,6 +87,7 @@ void ttm_resource_cursor_fini_locked(struct
> ttm_resource_cursor *cursor)
> {
> lockdep_assert_held(>man->bdev->lru_lock);
> list_del_init(>hitch.link);
> + ttm_resource_cursor_clear_bulk(cursor);
> }
>
> /**
> @@ -72,9 +116,27 @@ void ttm_resource_cursor_fini(struct ttm_resource_cursor
> *cursor)
> void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk)
> {
> memset(bulk, 0, sizeof(*bulk));
> + INIT_LIST_HEAD(>cursor_list);
> }
> EXPORT_SYMBOL(ttm_lru_bulk_move_init);
>
> +/**
> + * ttm_lru_bulk_move_fini - finalize a bulk move structure
> + * @bdev: The struct ttm_device
> + * @bulk: the structure to finalize
> + *
> + * Sanity checks
[drm-misc:for-linux-next 4/6] drivers/gpu/drm/xlnx/zynqmp_disp.c:949:14: error: logical not is only applied to the left hand side of this comparison
tree: git://anongit.freedesktop.org/drm/drm-misc for-linux-next
head: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f
commit: b0f0469ab662159f182f5af292b71cc5d42468a8 [4/6] drm: xlnx: zynqmp_dpsub:
Anounce supported input formats
config: s390-allmodconfig
(https://download.01.org/0day-ci/archive/20240426/202404260946.4ozxvhd2-...@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project
5ef5eb66fb428aaf61fb51b709f065c069c11242)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240426/202404260946.4ozxvhd2-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202404260946.4ozxvhd2-...@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/gpu/drm/xlnx/zynqmp_disp.c:15:
In file included from include/drm/drm_plane.h:33:
In file included from include/drm/drm_util.h:36:
In file included from include/linux/kgdb.h:19:
In file included from include/linux/kprobes.h:28:
In file included from include/linux/ftrace.h:13:
In file included from include/linux/kallsyms.h:13:
In file included from include/linux/mm.h:2208:
include/linux/vmstat.h:508:43: error: arithmetic between different
enumeration types ('enum zone_stat_item' and 'enum numa_stat_item')
[-Werror,-Wenum-enum-conversion]
508 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
|~ ^
509 |item];
|
include/linux/vmstat.h:515:43: error: arithmetic between different
enumeration types ('enum zone_stat_item' and 'enum numa_stat_item')
[-Werror,-Wenum-enum-conversion]
515 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
|~ ^
516 |NR_VM_NUMA_EVENT_ITEMS +
|~~
include/linux/vmstat.h:522:36: error: arithmetic between different
enumeration types ('enum node_stat_item' and 'enum lru_list')
[-Werror,-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~ ^ ~~~
include/linux/vmstat.h:527:43: error: arithmetic between different
enumeration types ('enum zone_stat_item' and 'enum numa_stat_item')
[-Werror,-Wenum-enum-conversion]
527 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
|~ ^
528 |NR_VM_NUMA_EVENT_ITEMS +
|~~
include/linux/vmstat.h:536:43: error: arithmetic between different
enumeration types ('enum zone_stat_item' and 'enum numa_stat_item')
[-Werror,-Wenum-enum-conversion]
536 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
|~ ^
537 |NR_VM_NUMA_EVENT_ITEMS +
|~~
In file included from drivers/gpu/drm/xlnx/zynqmp_disp.c:19:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:78:
include/asm-generic/io.h:547:31: error: performing pointer arithmetic on a
null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~ ^
include/asm-generic/io.h:560:61: error: performing pointer arithmetic on a
null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE +
addr));
| ~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro
'__le16_to_cpu'
37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
| ^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
| ^
In file included from drivers/gpu/drm/xlnx/zynqmp_disp.c:19:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:78:
include/asm-generic/io.h:573:61: error: performing pointer arithmetic on a
null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE +
addr));
Re: [PATCH v3 4/4] drm/xe/FLR: Support PCIe FLR
On Thu, Apr 25, 2024 at 11:47:46AM +0530, Aravind Iddamsetty wrote:
>
> On 25/04/24 04:59, Michał Winiarski wrote:
> > On Wed, Apr 24, 2024 at 10:42:59AM +0530, Aravind Iddamsetty wrote:
> >> On 24/04/24 05:19, Michał Winiarski wrote:
> >>> On Mon, Apr 22, 2024 at 12:27:56PM +0530, Aravind Iddamsetty wrote:
> PCI subsystem provides callbacks to inform the driver about a request to
> do function level reset by user, initiated by writing to sysfs entry
> /sys/bus/pci/devices/.../reset. This will allow the driver to handle FLR
> without the need to do unbind and rebind as the driver needs to
> reinitialize the device afresh post FLR.
>
> v2:
> 1. separate out gt idle and pci save/restore to a separate patch (Lucas)
> 2. Fixed the warnings seen around xe_guc_submit_stop, xe_guc_puc_fini
>
> v3: declare xe_pci_err_handlers as static(Michal)
>
> Cc: Rodrigo Vivi
> Cc: Lucas De Marchi
> Cc: Michal Wajdeczko
>
> Reviewed-by: Rodrigo Vivi
> Signed-off-by: Aravind Iddamsetty
> ---
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device_types.h | 3 +
> drivers/gpu/drm/xe/xe_guc_pc.c | 4 ++
> drivers/gpu/drm/xe/xe_pci.c | 9 ++-
> drivers/gpu/drm/xe/xe_pci.h | 2 +
> drivers/gpu/drm/xe/xe_pci_err.c | 88
> drivers/gpu/drm/xe/xe_pci_err.h | 13
> 7 files changed, 119 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/xe/xe_pci_err.c
> create mode 100644 drivers/gpu/drm/xe/xe_pci_err.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 8bc62bfbc679..693971a1fac0 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -117,6 +117,7 @@ xe-y += xe_bb.o \
> xe_module.o \
> xe_pat.o \
> xe_pci.o \
> +xe_pci_err.o \
> xe_pcode.o \
> xe_pm.o \
> xe_preempt_fence.o \
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> b/drivers/gpu/drm/xe/xe_device_types.h
> index 0a66555229e9..8c749b378a92 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -465,6 +465,9 @@ struct xe_device {
> /** @pci_state: PCI state of device */
> struct pci_saved_state *pci_state;
>
> +/** @pci_device_is_reset: device went through PCIe FLR */
> +bool pci_device_is_reset;
> +
> /* private: */
>
> #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c
> b/drivers/gpu/drm/xe/xe_guc_pc.c
> index 509649d0e65e..efba0fbe2f5c 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -902,6 +902,10 @@ static void xe_guc_pc_fini(struct drm_device *drm,
> void *arg)
> return;
> }
>
> +/* We already have done this before going through a reset, so
> skip here */
> +if (xe->pci_device_is_reset)
> +return;
> +
> XE_WARN_ON(xe_force_wake_get(gt_to_fw(pc_to_gt(pc)),
> XE_FORCEWAKE_ALL));
> XE_WARN_ON(xe_guc_pc_gucrc_disable(pc));
> XE_WARN_ON(xe_guc_pc_stop(pc));
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index a62300990e19..b5a582afc9e7 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -23,6 +23,7 @@
> #include "xe_macros.h"
> #include "xe_mmio.h"
> #include "xe_module.h"
> +#include "xe_pci_err.h"
> #include "xe_pci_types.h"
> #include "xe_pm.h"
> #include "xe_sriov.h"
> @@ -738,7 +739,7 @@ static void xe_pci_remove(struct pci_dev *pdev)
> pci_set_drvdata(pdev, NULL);
> }
>
> -static int xe_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> +int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> {
> const struct xe_device_desc *desc = (const void
> *)ent->driver_data;
> const struct xe_subplatform_desc *subplatform_desc;
> @@ -986,6 +987,11 @@ static const struct dev_pm_ops xe_pm_ops = {
> };
> #endif
>
> +static const struct pci_error_handlers xe_pci_err_handlers = {
> +.reset_prepare = xe_pci_reset_prepare,
> +.reset_done = xe_pci_reset_done,
> +};
> +
> static struct pci_driver xe_pci_driver = {
> .name = DRIVER_NAME,
> .id_table = pciidlist,
> @@ -995,6 +1001,7 @@ static struct pci_driver xe_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
>
[PATCH] drm: xlnx: zynqmp_dpsub: Fix few function comments
Fix arguments description for zynqmp_disp_layer_find_live_format() and zynqmp_disp_layer_set_live_format(). Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202404260616.kfgdpcdn-...@intel.com/ Signed-off-by: Anatoliy Klymenko --- drivers/gpu/drm/xlnx/zynqmp_disp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c index 13157da0089e..423f5f4943cc 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c @@ -940,7 +940,7 @@ zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer, * zynqmp_disp_layer_find_live_format - Find format information for given * media bus format * @layer: The layer - * @drm_fmt: Media bus format to search + * @media_bus_format: Media bus format to search * * Search display subsystem format information corresponding to the given media * bus format @media_bus_format for the @layer, and return a pointer to the @@ -1117,7 +1117,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer, /** * zynqmp_disp_layer_set_live_format - Set the live video layer format * @layer: The layer - * @info: The format info + * @media_bus_format: Media bus format to set * * NOTE: This function should not be used to set format for non-live video * layer. Use zynqmp_disp_layer_set_format() instead. --- base-commit: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f change-id: 20240425-dp-live-fmt-fix-a10bf7973596 Best regards, -- Anatoliy Klymenko
[drm-misc:for-linux-next 4/6] drivers/gpu/drm/xlnx/zynqmp_disp.c:949:14: warning: logical not is only applied to the left hand side of this comparison
tree: git://anongit.freedesktop.org/drm/drm-misc for-linux-next
head: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f
commit: b0f0469ab662159f182f5af292b71cc5d42468a8 [4/6] drm: xlnx: zynqmp_dpsub:
Anounce supported input formats
config: i386-buildonly-randconfig-006-20240426
(https://download.01.org/0day-ci/archive/20240426/202404260800.7uecbp6c-...@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project
6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240426/202404260800.7uecbp6c-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202404260800.7uecbp6c-...@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/xlnx/zynqmp_disp.c:949:14: warning: logical not is only
>> applied to the left hand side of this comparison [-Wlogical-not-parentheses]
949 | if (WARN_ON(!layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE)) {
| ^~~
include/asm-generic/bug.h:123:25: note: expanded from macro 'WARN_ON'
123 | int __ret_warn_on = !!(condition);
\
|^
drivers/gpu/drm/xlnx/zynqmp_disp.c:949:14: note: add parentheses after the
'!' to evaluate the comparison first
drivers/gpu/drm/xlnx/zynqmp_disp.c:949:14: note: add parentheses around left
hand side expression to silence this warning
1 warning generated.
vim +949 drivers/gpu/drm/xlnx/zynqmp_disp.c
928
929 /**
930 * zynqmp_disp_layer_drm_formats - Return the DRM formats supported by
the layer
931 * @layer: The layer
932 * @num_formats: Pointer to the returned number of formats
933 *
934 * NOTE: This function doesn't make sense for live video layers and will
935 * always return an empty list in such cases.
zynqmp_disp_live_layer_formats()
936 * should be used to query a list of media bus formats supported by the
live
937 * video input layer.
938 *
939 * Return: A newly allocated u32 array that stores all the DRM formats
940 * supported by the layer. The number of formats in the array is
returned
941 * through the num_formats argument.
942 */
943 u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
944 unsigned int *num_formats)
945 {
946 unsigned int i;
947 u32 *formats;
948
> 949 if (WARN_ON(!layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE)) {
950 *num_formats = 0;
951 return NULL;
952 }
953
954 formats = kcalloc(layer->info->num_formats, sizeof(*formats),
955GFP_KERNEL);
956 if (!formats) {
957 *num_formats = 0;
958 return NULL;
959 }
960
961 for (i = 0; i < layer->info->num_formats; ++i)
962 formats[i] = layer->info->formats[i].drm_fmt;
963
964 *num_formats = layer->info->num_formats;
965 return formats;
966 }
967
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] drm/vmwgfx: Fix Legacy Display Unit
On Thu, Apr 25, 2024 at 4:07 PM Ian Forbes wrote:
>
> Legacy DU was broken by the referenced fixes commit because the placement
> and the busy_placement no longer pointed to the same object. This was later
> fixed indirectly by commit a78a8da51b36c7a0c0c16233f91d60aac03a5a49
> ("drm/ttm: replace busy placement with flags v6") in v6.9.
>
> Fixes: 39985eea5a6d ("drm/vmwgfx: Abstract placement selection")
> Signed-off-by: Ian Forbes
> Cc: # v6.4+
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 2bfac3aad7b7..98e73eb0ccf1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -204,6 +204,7 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private
> *dev_priv,
> VMW_BO_DOMAIN_VRAM,
> VMW_BO_DOMAIN_VRAM);
> buf->places[0].lpfn = PFN_UP(bo->resource->size);
> + buf->busy_places[0].lpfn = PFN_UP(bo->resource->size);
> ret = ttm_bo_validate(bo, >placement, );
>
> /* For some reason we didn't end up at the start of vram */
Looks great. I'll push it through drm-misc-fixes.
Reviewed-by: Zack Rusin
z
[PATCH] drm/i915/gt: Automate CCS Mode setting during engine resets
We missed setting the CCS mode during resume and engine resets.
Create a workaround to be added in the engine's workaround list.
This workaround sets the XEHP_CCS_MODE value at every reset.
The issue can be reproduced by running:
$ clpeak --kernel-latency
Without resetting the CCS mode, we encounter a fence timeout:
Fence expiration time out i915-:03:00.0:clpeak[2387]:2!
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10895
Fixes: 6db31251bb26 ("drm/i915/gt: Enable only one CCS for compute workload")
Reported-by: Gnattu OC
Signed-off-by: Andi Shyti
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Matt Roper
Cc: # v6.2+
---
Hi Gnattu,
thanks again for reporting this issue and for your prompt
replies on the issue. Would you give this patch a chance?
Andi
drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c | 6 +++---
drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h | 2 +-
drivers/gpu/drm/i915/gt/intel_workarounds.c | 4 +++-
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c
b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c
index 044219c5960a..99b71bb7da0a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c
@@ -8,14 +8,14 @@
#include "intel_gt_ccs_mode.h"
#include "intel_gt_regs.h"
-void intel_gt_apply_ccs_mode(struct intel_gt *gt)
+unsigned int intel_gt_apply_ccs_mode(struct intel_gt *gt)
{
int cslice;
u32 mode = 0;
int first_ccs = __ffs(CCS_MASK(gt));
if (!IS_DG2(gt->i915))
- return;
+ return 0;
/* Build the value for the fixed CCS load balancing */
for (cslice = 0; cslice < I915_MAX_CCS; cslice++) {
@@ -35,5 +35,5 @@ void intel_gt_apply_ccs_mode(struct intel_gt *gt)
XEHP_CCS_MODE_CSLICE_MASK);
}
- intel_uncore_write(gt->uncore, XEHP_CCS_MODE, mode);
+ return mode;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h
b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h
index 9e5549caeb26..55547f2ff426 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h
@@ -8,6 +8,6 @@
struct intel_gt;
-void intel_gt_apply_ccs_mode(struct intel_gt *gt);
+unsigned int intel_gt_apply_ccs_mode(struct intel_gt *gt);
#endif /* __INTEL_GT_CCS_MODE_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 68b6aa11bcf7..58693923bf6c 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2703,6 +2703,7 @@ add_render_compute_tuning_settings(struct intel_gt *gt,
static void ccs_engine_wa_mode(struct intel_engine_cs *engine, struct
i915_wa_list *wal)
{
struct intel_gt *gt = engine->gt;
+ u32 mode;
if (!IS_DG2(gt->i915))
return;
@@ -2719,7 +2720,8 @@ static void ccs_engine_wa_mode(struct intel_engine_cs
*engine, struct i915_wa_li
* After having disabled automatic load balancing we need to
* assign all slices to a single CCS. We will call it CCS mode 1
*/
- intel_gt_apply_ccs_mode(gt);
+ mode = intel_gt_apply_ccs_mode(gt);
+ wa_masked_en(wal, XEHP_CCS_MODE, mode);
}
/*
--
2.43.0
[drm-misc:for-linux-next 1/2] drivers/gpu/drm/i915/i915_debugfs.c:739:9: error: implicit declaration of function 'debugfs_create_file'; did you mean 'bus_create_file'?
tree: git://anongit.freedesktop.org/drm/drm-misc for-linux-next
head: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f
commit: 33d5ae6cacf46a043578d711ae7239bab55b4455 [1/2] drm/print: drop include
debugfs.h and include where needed
config: x86_64-defconfig
(https://download.01.org/0day-ci/archive/20240426/202404260726.nyguvl59-...@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240426/202404260726.nyguvl59-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202404260726.nyguvl59-...@intel.com/
All error/warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/i915_debugfs.c: In function 'i915_debugfs_register':
>> drivers/gpu/drm/i915/i915_debugfs.c:739:9: error: implicit declaration of
>> function 'debugfs_create_file'; did you mean 'bus_create_file'?
>> [-Werror=implicit-function-declaration]
739 | debugfs_create_file("i915_forcewake_user", S_IRUSR,
minor->debugfs_root,
| ^~~
| bus_create_file
cc1: some warnings being treated as errors
--
drivers/gpu/drm/i915/i915_debugfs_params.c: In function
'i915_debugfs_create_int':
>> drivers/gpu/drm/i915/i915_debugfs_params.c:213:16: error: implicit
>> declaration of function 'debugfs_create_file_unsafe'; did you mean
>> 'sysfs_create_file_ns'? [-Werror=implicit-function-declaration]
213 | return debugfs_create_file_unsafe(name, mode, parent, value,
|^~
|sysfs_create_file_ns
>> drivers/gpu/drm/i915/i915_debugfs_params.c:213:16: warning: returning 'int'
>> from a function with return type 'struct dentry *' makes pointer from
>> integer without a cast [-Wint-conversion]
213 | return debugfs_create_file_unsafe(name, mode, parent, value,
|^
214 | RO(mode) ?
_param_int_fops_ro :
|
215 | _param_int_fops);
| ~
drivers/gpu/drm/i915/i915_debugfs_params.c: In function
'i915_debugfs_create_uint':
drivers/gpu/drm/i915/i915_debugfs_params.c:222:16: warning: returning 'int'
from a function with return type 'struct dentry *' makes pointer from integer
without a cast [-Wint-conversion]
222 | return debugfs_create_file_unsafe(name, mode, parent, value,
|^
223 | RO(mode) ?
_param_uint_fops_ro :
|
~
224 | _param_uint_fops);
| ~~
drivers/gpu/drm/i915/i915_debugfs_params.c: In function
'i915_debugfs_create_charp':
>> drivers/gpu/drm/i915/i915_debugfs_params.c:231:16: error: implicit
>> declaration of function 'debugfs_create_file'; did you mean
>> 'bus_create_file'? [-Werror=implicit-function-declaration]
231 | return debugfs_create_file(name, mode, parent, value,
|^~~
|bus_create_file
drivers/gpu/drm/i915/i915_debugfs_params.c:231:16: warning: returning 'int'
from a function with return type 'struct dentry *' makes pointer from integer
without a cast [-Wint-conversion]
231 | return debugfs_create_file(name, mode, parent, value,
|^~
232 |RO(mode) ?
_param_charp_fops_ro :
|
~~
233 |_param_charp_fops);
|~~~
drivers/gpu/drm/i915/i915_debugfs_params.c: In function
'i915_debugfs_params':
>> drivers/gpu/drm/i915/i915_debugfs_params.c:254:15: error: implicit
>> declaration of function 'debugfs_create_dir'; did you mean
>> 'kernfs_create_dir'? [-Werror=implicit-function-declaration]
254 | dir = debugfs_create_dir("i915_params", minor->debugfs_root);
| ^~
| kernfs_create_dir
>> drivers/gpu/drm/i915/i915_debugfs_params.c:254:13: warning: assignment to
>> 'struct dentry *' from 'int' makes pointer from integer without a cast
>> [-Wint-conversion]
254 | dir =
[drm-misc:for-linux-next 6/6] drivers/gpu/drm/xlnx/zynqmp_disp.c:954: warning: Function parameter or struct member 'media_bus_format' not described in 'zynqmp_disp_layer_find_live_format'
tree: git://anongit.freedesktop.org/drm/drm-misc for-linux-next
head: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f
commit: 1b5151bd3a2e076653a935874b39dd2c3a00452a [6/6] drm: xlnx: zynqmp_dpsub:
Set input live format
config: m68k-allmodconfig
(https://download.01.org/0day-ci/archive/20240426/202404260616.kfgdpcdn-...@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240426/202404260616.kfgdpcdn-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202404260616.kfgdpcdn-...@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/xlnx/zynqmp_disp.c:164: warning: Function parameter or
struct member 'blend' not described in 'zynqmp_disp'
drivers/gpu/drm/xlnx/zynqmp_disp.c:164: warning: Function parameter or
struct member 'avbuf' not described in 'zynqmp_disp'
drivers/gpu/drm/xlnx/zynqmp_disp.c:164: warning: Function parameter or
struct member 'audio' not described in 'zynqmp_disp'
>> drivers/gpu/drm/xlnx/zynqmp_disp.c:954: warning: Function parameter or
>> struct member 'media_bus_format' not described in
>> 'zynqmp_disp_layer_find_live_format'
>> drivers/gpu/drm/xlnx/zynqmp_disp.c:954: warning: Excess function parameter
>> 'drm_fmt' description in 'zynqmp_disp_layer_find_live_format'
>> drivers/gpu/drm/xlnx/zynqmp_disp.c:1129: warning: Function parameter or
>> struct member 'media_bus_format' not described in
>> 'zynqmp_disp_layer_set_live_format'
drivers/gpu/drm/xlnx/zynqmp_disp.c:1129: warning: Excess function parameter
'info' description in 'zynqmp_disp_layer_set_live_format'
vim +954 drivers/gpu/drm/xlnx/zynqmp_disp.c
938
939 /**
940 * zynqmp_disp_layer_find_live_format - Find format information for
given
941 * media bus format
942 * @layer: The layer
943 * @drm_fmt: Media bus format to search
944 *
945 * Search display subsystem format information corresponding to the
given media
946 * bus format @media_bus_format for the @layer, and return a pointer to
the
947 * format descriptor.
948 *
949 * Return: A pointer to the format descriptor if found, NULL otherwise
950 */
951 static const struct zynqmp_disp_format *
952 zynqmp_disp_layer_find_live_format(struct zynqmp_disp_layer *layer,
953 u32 media_bus_format)
> 954 {
955 unsigned int i;
956
957 for (i = 0; i < layer->info->num_formats; i++)
958 if (layer->info->formats[i].bus_fmt == media_bus_format)
959 return >info->formats[i];
960
961 return NULL;
962 }
963
964 /**
965 * zynqmp_disp_layer_drm_formats - Return the DRM formats supported by
the layer
966 * @layer: The layer
967 * @num_formats: Pointer to the returned number of formats
968 *
969 * NOTE: This function doesn't make sense for live video layers and will
970 * always return an empty list in such cases.
zynqmp_disp_live_layer_formats()
971 * should be used to query a list of media bus formats supported by the
live
972 * video input layer.
973 *
974 * Return: A newly allocated u32 array that stores all the DRM formats
975 * supported by the layer. The number of formats in the array is
returned
976 * through the num_formats argument.
977 */
978 u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
979 unsigned int *num_formats)
980 {
981 unsigned int i;
982 u32 *formats;
983
984 if (WARN_ON(!layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE)) {
985 *num_formats = 0;
986 return NULL;
987 }
988
989 formats = kcalloc(layer->info->num_formats, sizeof(*formats),
990GFP_KERNEL);
991 if (!formats) {
992 *num_formats = 0;
993 return NULL;
994 }
995
996 for (i = 0; i < layer->info->num_formats; ++i)
997 formats[i] = layer->info->formats[i].drm_fmt;
998
999 *num_formats = layer->info->num_formats;
1000 return formats;
1001 }
1002
1003 /**
1004 * zynqmp_disp_live_layer_formats - Return the media bus formats
supported by
1005 * the live video layer
1006 * @layer: The layer
1007 * @num_formats: Pointer to the returned number of formats
1008 *
1009 * NOTE: This function should be used only for live video input layers.
1010 *
1011 * Return: A newly allocated u32 array of media bus formats supported
by the
1012 * layer. The number of formats in the array is
Re: [v1,1/3] drm/panel: ili9341: Correct use of device property APIs
On 2024/4/26 04:43, Sui Jingfeng wrote: But both of will be safe then. But both of us will be safe then. -- Best regards, Sui
Re: [PATCH] drm/vmwgfx: Fix invalid reads in fence signaled events
On 4/25/24 12:27, Zack Rusin wrote:
> Correctly set the length of the drm_event to the size of the structure
> that's actually used.
>
> The length of the drm_event was set to the parent structure instead of
> to the drm_vmw_event_fence which is supposed to be read. drm_read
> uses the length parameter to copy the event to the user space thus
> resuling in oob reads.
>
> Signed-off-by: Zack Rusin
> Fixes: 8b7de6aa8468 ("vmwgfx: Rework fence event action")
> Reported-by: zdi-disclosu...@trendmicro.com # ZDI-CAN-23566
> Cc: David Airlie
> CC: Daniel Vetter
> Cc: Zack Rusin
> Cc: Broadcom internal kernel review list
>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Cc: # v3.4+
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> index 2a0cda324703..5efc6a766f64 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> @@ -991,7 +991,7 @@ static int vmw_event_fence_action_create(struct drm_file
> *file_priv,
> }
>
> event->event.base.type = DRM_VMW_EVENT_FENCE_SIGNALED;
> - event->event.base.length = sizeof(*event);
> + event->event.base.length = sizeof(event->event);
> event->event.user_data = user_data;
>
> ret = drm_event_reserve_init(dev, file_priv, >base,
> >event.base);
LGTM!
Reviewed-by: Maaz Mombasawala
Thanks,
Maaz Mombasawala
Re: [v1,1/3] drm/panel: ili9341: Correct use of device property APIs
Hi,
On 2024/4/26 03:12, Andy Shevchenko wrote:
On Fri, Apr 26, 2024 at 02:53:22AM +0800, Sui Jingfeng wrote:
On 2024/4/26 02:08, Sui Jingfeng wrote:
Are you speaking to yourself? I'm totally lost.
Please, if you want to give a constructive feedback, try to understand
the topic from different aspects and then clearly express it.
OK,
The previous email analysis the non-DT cases exhaustively, this email intend to
demonstrate the more frequently use case.
That is, in the *DT('OF')* based systems,
device_get_match_data() is completely equivalent to
of_device_get_match_data().
So the net results of applying this patch are "no gains and no lost".
Things will become clear if we divide the whole problem into two cases(DT and
non-DT)
to discuss, that's it. That's all I can tell.
Sorry about my broken written and sorry for the noise. Thanks for your
education in
the past.
--
Best regards,
Sui
Re: [PATCH v2 21/25] nvdimm: virtio_pmem: drop owner assignment
Krzysztof Kozlowski wrote:
> virtio core already sets the .owner, so driver does not need to.
>
> Acked-by: Dave Jiang
Acked-by: Ira Weiny
> Signed-off-by: Krzysztof Kozlowski
> ---
>
> Depends on the first patch.
> ---
> drivers/nvdimm/virtio_pmem.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
> index 4ceced5cefcf..c9b97aeabf85 100644
> --- a/drivers/nvdimm/virtio_pmem.c
> +++ b/drivers/nvdimm/virtio_pmem.c
> @@ -151,7 +151,6 @@ static struct virtio_driver virtio_pmem_driver = {
> .feature_table = features,
> .feature_table_size = ARRAY_SIZE(features),
> .driver.name= KBUILD_MODNAME,
> - .driver.owner = THIS_MODULE,
> .id_table = id_table,
> .validate = virtio_pmem_validate,
> .probe = virtio_pmem_probe,
>
> --
> 2.34.1
>
[PULL] drm-xe-fixes
Hi Dave and Sima Please pull the drm-xe-fixes for this week targeting v6.9-rc6. Simple fixes not really visible to end users: 2 around error paths on drm managed allocations and 1 on PF/VF relay messages that are not enabled by default. thanks Lucas De Marchi drm-xe-fixes-2024-04-25: - Fix error paths on managed allocations - Fix PF/VF relay messages The following changes since commit ed30a4a51bb196781c8058073ea720133a65596f: Linux 6.9-rc5 (2024-04-21 12:35:54 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/drm/xe/kernel.git tags/drm-xe-fixes-2024-04-25 for you to fetch changes up to e3e989522ac9a6b7960c75b762e1e9568717b31e: drm/xe/guc: Fix arguments passed to relay G2H handlers (2024-04-24 10:20:00 -0500) - Fix error paths on managed allocations - Fix PF/VF relay messages Himal Prasad Ghimiray (2): drm/xe: Remove sysfs only once on action add failure drm/xe: call free_gsc_pkt only once on action add failure Michal Wajdeczko (1): drm/xe/guc: Fix arguments passed to relay G2H handlers drivers/gpu/drm/xe/xe_gt.c | 4 +++- drivers/gpu/drm/xe/xe_gt_ccs_mode.c | 19 +++ drivers/gpu/drm/xe/xe_gt_ccs_mode.h | 2 +- drivers/gpu/drm/xe/xe_guc_ct.c | 4 ++-- drivers/gpu/drm/xe/xe_huc.c | 9 + 5 files changed, 14 insertions(+), 24 deletions(-)
Re: [v1,1/3] drm/panel: ili9341: Correct use of device property APIs
Hi,
On 2024/4/26 03:10, Andy Shevchenko wrote:
On Fri, Apr 26, 2024 at 02:08:16AM +0800, Sui Jingfeng wrote:
On 2024/4/25 22:26, Andy Shevchenko wrote:
It seems driver missed the point of proper use of device property APIs.
Correct this by updating headers and calls respectively.
You are using the 'seems' here exactly saying that you are not 100% sure.
Please allow me to tell you the truth: This patch again has ZERO effect.
It fix nothing. And this patch is has the risks to be wrong.
Huh?! Really, stop commenting the stuff you do not understand.
I'm actually a professional display drivers developer at the downstream
in the past, despite my contribution to upstream is less. But I believe
that all panel driver developers know what I'm talking about. So please
have take a look at my replies.
Simple because the "ili9341_probe() ---> ili9341_dbi_prob()" code path
is DT dependent.
First of all, the devm_of_find_backlight() is called in ili9341_dbi_probe()
under *non-DT* environment, devm_of_find_backlight() is just a just a
no-op and will return NULL. NULL is not an error code, so ili9341_dbi_probe()
won't rage quit. But the several side effect is that the backlight will
NOT works at all.
Is it a problem?
Yes, it is.
The core problem is that the driver you are modifying has *implicit*
*dependency* on DT.
The implicit dependency is due to the calling of devm_of_find_backlight(). This
function
is a no-op under non-DT systems. Therefore, before the devm_of_find_backlight()
and
the device_get_match_data() function can truly DT independent.
Removing the "OF" dependency just let the tigers run out from the jail.
It is not really meant to targeting at you, but I thinks, all of drm_panel
drivers
that has the devm_of_find_backlight() invoked will suffer such concerns.
In short, the reason is that the *implicit* *dependency* populates and
the undefined behavior gets triggered.
I'm sure you know that device_get_match_data() is same with
of_device_get_match_data()
for DT based systems. For non DT based systems, device_get_match_data() is just
*undefined*
Note that ACPI is not in the scope of the discussion here, as all of the drm
bridges and
panels driver under drivers/gpu/drm/ hasn't the ACPI support yet. Therefore, at
present,
it safe to say that device_get_match_data() is *undefined* under no-DT
environment.
Removing the "OF" dependency hints to us that it allows the driver to be probed
as a
pure SPI device under non DT systems. When device_get_match_data() is called,
it returns
NULL to us now. As a result, the drm driver being modified will tears down.
See bellow code snippet extracted frompanel-ilitek-ili9341.c:
```
ili->conf = of_device_get_match_data(dev);
if (!ili->conf) {
dev_err(dev, "missing device configuration\n");
return -ENODEV;
}
```
It is actually considered as fatal bug for *panels* if the backlight of
it is not light up, at least the brightness of *won't* be able to adjust.
What's worse, if there is no sane platform setup code at the firmware
or boot loader stage to set a proper initial state. The screen is complete
dark. Even though the itself panel is refreshing framebuffers, it can not
be seen by human's eye. Simple because of no backlight.
Can you imagine that I may have different hardware that considered
this is non-fatal error?
Yes, I can imagine.
I believe you have the hardware which make you patch correct to run
in 99.9% of all cases. But as long as there one bug happened, you patch
are going to be blamed.
Because its your patch that open the door, both from the perceptive of
practice and from the perceptive of the concept (static analysis).
Second, the ili9341_dbi_probe() requires additional device properties to
be able to works very well on the rotation screen case. See the calling
of "device_property_read_u32(dev, "rotation", )" in
ili9341_dbi_probe() function.
Yes, exactly, and how does it object the purpose of this patch?
Because under *non-DT* environment, your commit message do not give a
valid description, how does the additional device property can be acquired
is not demonstrated.
And it is exactly your patch open the non-DT code path (way or possibility).
It isn't has such risks before your patch is applied. In other words,
previously, the driver has the 'OF' dependency as the guard, all of the
potential risk(or problem) are suppressed. It is a extremely safe policy,
and it is also a extremely perfect defend.
And suddenly, you patch release the dangerous tiger from the cage.
So I think you can imagine...
Combine with those two factors, it is actually can conclude that the
panel-ilitek-ili9394 driver has the *implicit* dependency on 'OF'.
Removing the 'OF' dependency from its Kconfig just trigger the
leakage of such risks.
What?!
Posting a patch is actually doing the defensive works, such a saying
may not sound fair for you, but this is just the hash cruel reality.
Sorry
Re: [PATCH V2 2/2] drm/bridge: samsung-dsim: Fix porch calcalcuation rounding
On Thu, Apr 25, 2024 at 4:19 AM Marek Szyprowski
wrote:
>
> On 12.02.2024 00:09, Adam Ford wrote:
> > When using video sync pulses, the HFP, HBP, and HSA are divided between
> > the available lanes if there is more than one lane. For certain
> > timings and lane configurations, the HFP may not be evenly divisible.
> > If the HFP is rounded down, it ends up being too small which can cause
> > some monitors to not sync properly. In these instances, adjust htotal
> > and hsync to round the HFP up, and recalculate the htotal.
> >
> > Tested-by: Frieder Schrempf # Kontron BL
> > i.MX8MM with HDMI monitor
> > Signed-off-by: Adam Ford
>
> Tested-by: Marek Szyprowski
Thank you very much for testing!
>
> > ---
> > V2: No changes
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index 8476650c477c..52939211fe93 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -1606,6 +1606,27 @@ static int samsung_dsim_atomic_check(struct
> > drm_bridge *bridge,
> > adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC |
> > DRM_MODE_FLAG_PVSYNC);
> > }
> >
> > + /*
> > + * When using video sync pulses, the HFP, HBP, and HSA are divided
> > between
> > + * the available lanes if there is more than one lane. For certain
> > + * timings and lane configurations, the HFP may not be evenly
> > divisible.
> > + * If the HFP is rounded down, it ends up being too small which can
> > cause
> > + * some monitors to not sync properly. In these instances, adjust
> > htotal
> > + * and hsync to round the HFP up, and recalculate the htotal. Through
> > trial
> > + * and error, it appears that the HBP and HSA do not appearto need
> > the same
> > + * correction that HFP does.
> > + */
> > + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE && dsi->lanes >
> > 1) {
Frieder & Marek S,
Marek V is proposing we eliminate the check against the flags and do
it unconditionally. If I send you both a different patch, would you
be willing to try them on your platforms? I don't want to risk
breaking a board.
I used the check above from the NXP downstream kernel, so it felt
safe, but I am not as familiar with the different DSI modes, so I am
not sure what the impact would be if this read:
if (dsi->lanes > 1) {
Does anyone else have an opinion on this?
> > + int hfp = adjusted_mode->hsync_start -
> > adjusted_mode->hdisplay;
> > + int remainder = hfp % dsi->lanes;
> > +
> > + if (remainder) {
> > + adjusted_mode->hsync_start += remainder;
> > + adjusted_mode->hsync_end += remainder;
> > + adjusted_mode->htotal += remainder;
> > + }
> > + }
> > +
> > return 0;
> > }
> >
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R Institute Poland
>
[PATCH] drm/vmwgfx: Fix Legacy Display Unit
Legacy DU was broken by the referenced fixes commit because the placement
and the busy_placement no longer pointed to the same object. This was later
fixed indirectly by commit a78a8da51b36c7a0c0c16233f91d60aac03a5a49
("drm/ttm: replace busy placement with flags v6") in v6.9.
Fixes: 39985eea5a6d ("drm/vmwgfx: Abstract placement selection")
Signed-off-by: Ian Forbes
Cc: # v6.4+
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 2bfac3aad7b7..98e73eb0ccf1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -204,6 +204,7 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private
*dev_priv,
VMW_BO_DOMAIN_VRAM,
VMW_BO_DOMAIN_VRAM);
buf->places[0].lpfn = PFN_UP(bo->resource->size);
+ buf->busy_places[0].lpfn = PFN_UP(bo->resource->size);
ret = ttm_bo_validate(bo, >placement, );
/* For some reason we didn't end up at the start of vram */
--
2.34.1
[PATCH 0/2] drm/rockchip: vop2: two fixes from working on DSI enablement
While working on enabling DSI support on rk3588 I stumbled across the issue of the display staying black while both the vop2 and dsi drivers were claiming to be running just fine. After a bit of checking I found that I got DSI output on VP3 when HDMI on VP0 was at least associated in the DTS, but not without. Andy's patch [0] helped and is definitly necessary, but did not fix the problem completely. The missing thing was that VP3 is rk3588-specific (rk3568 only has 3 video-ports, not 4 like rk3588) and the layers of VP3 never got configured. Patch2 fixes this. [0] https://lore.kernel.org/dri-devel/20240422101905.32703-2-andys...@163.com/ Heiko Stuebner (2): drm/rockchip: vop2: fix rk3588 dp+dsi maxclk verification drm/rockchip: vop2: configure layers for vp3 on rk3588 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 16 ++-- drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 1 + 2 files changed, 15 insertions(+), 2 deletions(-) -- 2.39.2
[PATCH 1/2] drm/rockchip: vop2: fix rk3588 dp+dsi maxclk verification
From: Heiko Stuebner
The clock is in Hz while the value checked against is in kHz, so
actual frequencies will never be able to be below to max value.
Fix this by specifying the max-value in Hz too.
Fixes: 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588")
Signed-off-by: Heiko Stuebner
---
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 9bee1fd88e6a2..523880a4e8e74 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1719,7 +1719,7 @@ static unsigned long rk3588_calc_cru_cfg(struct
vop2_video_port *vp, int id,
else
dclk_out_rate = v_pixclk >> 2;
- dclk_rate = rk3588_calc_dclk(dclk_out_rate, 60);
+ dclk_rate = rk3588_calc_dclk(dclk_out_rate, 6);
if (!dclk_rate) {
drm_err(vop2->drm, "DP dclk_out_rate out of range,
dclk_out_rate: %ld KHZ\n",
dclk_out_rate);
@@ -1736,7 +1736,7 @@ static unsigned long rk3588_calc_cru_cfg(struct
vop2_video_port *vp, int id,
* dclk_rate = N * dclk_core_rate N = (1,2,4 ),
* we get a little factor here
*/
- dclk_rate = rk3588_calc_dclk(dclk_out_rate, 60);
+ dclk_rate = rk3588_calc_dclk(dclk_out_rate, 6);
if (!dclk_rate) {
drm_err(vop2->drm, "MIPI dclk out of range,
dclk_out_rate: %ld KHZ\n",
dclk_out_rate);
--
2.39.2
[PATCH 2/2] drm/rockchip: vop2: configure layers for vp3 on rk3588
From: Heiko Stuebner
The rk3588 VOP2 has 4 video-ports, yet the driver currently only
configures the first 3, as used on the rk3568.
Add another block to configure the vp3 as well, if applicable.
Fixes: 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588")
Signed-off-by: Heiko Stuebner
---
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 12
drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 523880a4e8e74..1a327a9ed7ee4 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2303,6 +2303,7 @@ static void vop2_setup_alpha(struct vop2_video_port *vp)
static void vop2_setup_layer_mixer(struct vop2_video_port *vp)
{
struct vop2 *vop2 = vp->vop2;
+ const struct vop2_data *vop2_data = vop2->data;
struct drm_plane *plane;
u32 layer_sel = 0;
u32 port_sel;
@@ -2344,6 +2345,17 @@ static void vop2_setup_layer_mixer(struct
vop2_video_port *vp)
else
port_sel |= FIELD_PREP(RK3568_OVL_PORT_SET__PORT2_MUX, 8);
+ /* configure vp3 */
+ if (vop2_data->soc_id == 3588) {
+ struct vop2_video_port *vp3 = >vps[3];
+
+ if (vp3->nlayers)
+ port_sel |= FIELD_PREP(RK3588_OVL_PORT_SET__PORT3_MUX,
+ (vp3->nlayers + vp2->nlayers + vp1->nlayers +
vp0->nlayers - 1));
+ else
+ port_sel |= FIELD_PREP(RK3588_OVL_PORT_SET__PORT3_MUX,
8);
+ }
+
layer_sel = vop2_readl(vop2, RK3568_OVL_LAYER_SEL);
ofs = 0;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
index 615a16196aff6..f46fb795414e1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
@@ -489,6 +489,7 @@ enum dst_factor_mode {
#define RK3588_OVL_PORT_SEL__CLUSTER2 GENMASK(21, 20)
#define RK3568_OVL_PORT_SEL__CLUSTER1 GENMASK(19, 18)
#define RK3568_OVL_PORT_SEL__CLUSTER0 GENMASK(17, 16)
+#define RK3588_OVL_PORT_SET__PORT3_MUX GENMASK(15, 12)
#define RK3568_OVL_PORT_SET__PORT2_MUX GENMASK(11, 8)
#define RK3568_OVL_PORT_SET__PORT1_MUX GENMASK(7, 4)
#define RK3568_OVL_PORT_SET__PORT0_MUX GENMASK(3, 0)
--
2.39.2
[drm-misc:for-linux-next 2/2] drivers/gpu/drm/omapdrm/omap_fb.c:325:2: error: implicit declaration of function 'seq_printf' is invalid in C99
tree: git://anongit.freedesktop.org/drm/drm-misc for-linux-next
head: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f
commit: 9e2b84fb6cd7ee913aa61d461db65c1d6a08dcf2 [2/2] drm/print: drop include
seq_file.h
config: arm-defconfig
(https://download.01.org/0day-ci/archive/20240426/202404260318.h7uzpwsq-...@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git
f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240426/202404260318.h7uzpwsq-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202404260318.h7uzpwsq-...@intel.com/
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/omapdrm/omap_fb.c:325:2: error: implicit declaration of
>> function 'seq_printf' is invalid in C99
>> [-Werror,-Wimplicit-function-declaration]
seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
^
1 error generated.
vim +/seq_printf +325 drivers/gpu/drm/omapdrm/omap_fb.c
b33f34d3d10b9b drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 319
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 320
#ifdef CONFIG_DEBUG_FS
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 321
void omap_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m)
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 322
{
bcb0b461454c9c drivers/gpu/drm/omapdrm/omap_fb.c Ville Syrjälä 2016-12-14 323
int i, n = fb->format->num_planes;
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 324
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 @325
seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
438b74a5497c36 drivers/gpu/drm/omapdrm/omap_fb.c Ville Syrjälä 2016-12-14 326
(char *)>format->format);
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 327
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 328
for (i = 0; i < n; i++) {
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 329
seq_printf(m, " %d: offset=%d pitch=%d, obj: ",
2ecceeb53b1945 drivers/gpu/drm/omapdrm/omap_fb.c Daniel Stone 2018-03-30 330
i, fb->offsets[n], fb->pitches[i]);
3e44255260dc3c drivers/gpu/drm/omapdrm/omap_fb.c Daniel Stone 2018-03-30 331
omap_gem_describe(fb->obj[i], m);
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 332
}
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 333
}
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 334
#endif
f6b6036e56ca17 drivers/staging/omapdrm/omap_fb.c Rob Clark 2012-03-05 335
:: The code at line 325 was first introduced by commit
:: f6b6036e56ca17378dd0294b684db623abd6a901 staging: drm/omap: debugfs for
object and fb tracking
:: TO: Rob Clark
:: CC: Greg Kroah-Hartman
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[drm-misc:for-linux-next 2/2] drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c:366:9: error: implicit declaration of function 'seq_printf'; did you mean 'drm_printf'?
tree: git://anongit.freedesktop.org/drm/drm-misc for-linux-next
head: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f
commit: 9e2b84fb6cd7ee913aa61d461db65c1d6a08dcf2 [2/2] drm/print: drop include
seq_file.h
config: arm64-defconfig
(https://download.01.org/0day-ci/archive/20240426/202404260313.kfrxhgyu-...@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240426/202404260313.kfrxhgyu-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202404260313.kfrxhgyu-...@intel.com/
All errors (new ones prefixed by >>):
drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c: In function
'komeda_pipeline_dump_register':
>> drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c:366:9: error: implicit
>> declaration of function 'seq_printf'; did you mean 'drm_printf'?
>> [-Werror=implicit-function-declaration]
366 | seq_printf(sf, "\n Pipeline-%d ==\n",
pipe->id);
| ^~
| drm_printf
cc1: some warnings being treated as errors
--
drivers/gpu/drm/arm/display/komeda/d71/d71_component.c: In function
'dump_block_header':
>> drivers/gpu/drm/arm/display/komeda/d71/d71_component.c:94:9: error: implicit
>> declaration of function 'seq_printf'; did you mean 'drm_printf'?
>> [-Werror=implicit-function-declaration]
94 | seq_printf(sf, "BLOCK_INFO:\t\t0x%X\n", hdr.block_info);
| ^~
| drm_printf
drivers/gpu/drm/arm/display/komeda/d71/d71_component.c: In function
'd71_gcu_dump':
>> drivers/gpu/drm/arm/display/komeda/d71/d71_component.c:1360:9: error:
>> implicit declaration of function 'seq_puts'
>> [-Werror=implicit-function-declaration]
1360 | seq_puts(sf, "\n-- GCU --\n");
| ^~~~
cc1: some warnings being treated as errors
vim +366 drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
7d3cfb70a604d2 james qian wang (Arm Technology China 2019-01-22 358)
7d3cfb70a604d2 james qian wang (Arm Technology China 2019-01-22 359) void
komeda_pipeline_dump_register(struct komeda_pipeline *pipe,
7d3cfb70a604d2 james qian wang (Arm Technology China 2019-01-22 360)
struct seq_file *sf)
7d3cfb70a604d2 james qian wang (Arm Technology China 2019-01-22 361) {
7d3cfb70a604d2 james qian wang (Arm Technology China 2019-01-22 362) struct
komeda_component *c;
7d3cfb70a604d2 james qian wang (Arm Technology China 2019-01-22 363) u32 id;
a1c3be890440a1 Carsten Haitzler 2021-02-04 364
unsigned long avail_comps;
7d3cfb70a604d2 james qian wang (Arm Technology China 2019-01-22 365)
7d3cfb70a604d2 james qian wang (Arm Technology China 2019-01-22 @366)
seq_printf(sf, "\n Pipeline-%d ==\n", pipe->id);
:: The code at line 366 was first introduced by commit
:: 7d3cfb70a604d215e195ba9282df42dd6edff16a drm/komeda: Add debugfs node
"register" for register dump
:: TO: james qian wang (Arm Technology China)
:: CC: Liviu Dudau
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 2/2] drm/print: drop include seq_file.h
On 4/22/24 14:10, Jani Nikula wrote: > Never include where a forward declaration will suffice. > > Reviewed-by: Andrzej Hajda > Acked-by: Maxime Ripard > Link: > https://patchwork.freedesktop.org/patch/msgid/20240410141434.157908-2-jani.nik...@intel.com > Signed-off-by: Jani Nikula > --- > include/drm/drm_print.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > index 561c3b96b6fd..089950ad8681 100644 > --- a/include/drm/drm_print.h > +++ b/include/drm/drm_print.h > @@ -28,7 +28,6 @@ > > #include > #include > -#include > #include > #include > > @@ -36,6 +35,7 @@ > > struct debugfs_regset32; > struct drm_device; > +struct seq_file; > > /* Do *not* use outside of drm_print.[ch]! */ > extern unsigned long __drm_debug; Looks like this broke komeda and omapdrm on arm: /home/notro/develop/dim-linux/src/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c: In function ‘komeda_pipeline_dump_register’: /home/notro/develop/dim-linux/src/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c:366:9: error: implicit declaration of function ‘seq_printf’; did you mean ‘drm_printf’? [-Werror=implicit-function-declaration] 366 | seq_printf(sf, "\n Pipeline-%d ==\n", pipe->id); | ^~ | drm_printf /home/notro/develop/dim-linux/src/drivers/gpu/drm/omapdrm/omap_fb.c: In function ‘omap_framebuffer_describe’: /home/notro/develop/dim-linux/src/drivers/gpu/drm/omapdrm/omap_fb.c:325:9: error: implicit declaration of function ‘seq_printf’; did you mean ‘drm_printf’? [-Werror=implicit-function-declaration] 325 | seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height, | ^~ | drm_printf Noralf.
[PATCH] drm/vmwgfx: Fix invalid reads in fence signaled events
Correctly set the length of the drm_event to the size of the structure
that's actually used.
The length of the drm_event was set to the parent structure instead of
to the drm_vmw_event_fence which is supposed to be read. drm_read
uses the length parameter to copy the event to the user space thus
resuling in oob reads.
Signed-off-by: Zack Rusin
Fixes: 8b7de6aa8468 ("vmwgfx: Rework fence event action")
Reported-by: zdi-disclosu...@trendmicro.com # ZDI-CAN-23566
Cc: David Airlie
CC: Daniel Vetter
Cc: Zack Rusin
Cc: Broadcom internal kernel review list
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc: # v3.4+
---
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index 2a0cda324703..5efc6a766f64 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -991,7 +991,7 @@ static int vmw_event_fence_action_create(struct drm_file
*file_priv,
}
event->event.base.type = DRM_VMW_EVENT_FENCE_SIGNALED;
- event->event.base.length = sizeof(*event);
+ event->event.base.length = sizeof(event->event);
event->event.user_data = user_data;
ret = drm_event_reserve_init(dev, file_priv, >base,
>event.base);
--
2.40.1
Re: [PATCH v2] drivers/i915/intel_bios: Fix parsing backlight BDB data
A kind reminder to review this change.
On Wed, Feb 21, 2024 at 6:06 PM Karthikeyan Ramasubramanian
wrote:
>
> Starting BDB version 239, hdr_dpcd_refresh_timeout is introduced to
> backlight BDB data. Commit 700034566d68 ("drm/i915/bios: Define more BDB
> contents") updated the backlight BDB data accordingly. This broke the
> parsing of backlight BDB data in VBT for versions 236 - 238 (both
> inclusive) and hence the backlight controls are not responding on units
> with the concerned BDB version.
>
> backlight_control information has been present in backlight BDB data
> from at least BDB version 191 onwards, if not before. Hence this patch
> extracts the backlight_control information for BDB version 191 or newer.
> Tested on Chromebooks using Jasperlake SoC (reports bdb->version = 236).
> Tested on Chromebooks using Raptorlake SoC (reports bdb->version = 251).
>
> Fixes: 700034566d68 ("drm/i915/bios: Define more BDB contents")
> Cc: sta...@vger.kernel.org
> Cc: Jani Nikula
> Cc: Ville Syrjälä
> Signed-off-by: Karthikeyan Ramasubramanian
> ---
>
> Changes in v2:
> - removed checking the block size of the backlight BDB data
>
> drivers/gpu/drm/i915/display/intel_bios.c | 19 ---
> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 5 -
> 2 files changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index aa169b0055e97..8c1eb05fe77d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1042,22 +1042,11 @@ parse_lfp_backlight(struct drm_i915_private *i915,
> panel->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI;
> panel->vbt.backlight.controller = 0;
> if (i915->display.vbt.version >= 191) {
> - size_t exp_size;
> + const struct lfp_backlight_control_method *method;
>
> - if (i915->display.vbt.version >= 236)
> - exp_size = sizeof(struct bdb_lfp_backlight_data);
> - else if (i915->display.vbt.version >= 234)
> - exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_234;
> - else
> - exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_191;
> -
> - if (get_blocksize(backlight_data) >= exp_size) {
> - const struct lfp_backlight_control_method *method;
> -
> - method =
> _data->backlight_control[panel_type];
> - panel->vbt.backlight.type = method->type;
> - panel->vbt.backlight.controller = method->controller;
> - }
> + method = _data->backlight_control[panel_type];
> + panel->vbt.backlight.type = method->type;
> + panel->vbt.backlight.controller = method->controller;
> }
>
> panel->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index a9f44abfc9fc2..b50cd0dcabda9 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -897,11 +897,6 @@ struct lfp_brightness_level {
> u16 reserved;
> } __packed;
>
> -#define EXP_BDB_LFP_BL_DATA_SIZE_REV_191 \
> - offsetof(struct bdb_lfp_backlight_data, brightness_level)
> -#define EXP_BDB_LFP_BL_DATA_SIZE_REV_234 \
> - offsetof(struct bdb_lfp_backlight_data, brightness_precision_bits)
> -
> struct bdb_lfp_backlight_data {
> u8 entry_size;
> struct lfp_backlight_data_entry data[16];
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Re: [v1,1/3] drm/panel: ili9341: Correct use of device property APIs
On Fri, Apr 26, 2024 at 02:53:22AM +0800, Sui Jingfeng wrote: > On 2024/4/26 02:08, Sui Jingfeng wrote: Are you speaking to yourself? I'm totally lost. Please, if you want to give a constructive feedback, try to understand the topic from different aspects and then clearly express it. -- With Best Regards, Andy Shevchenko
Re: [v1,1/3] drm/panel: ili9341: Correct use of device property APIs
On Fri, Apr 26, 2024 at 02:08:16AM +0800, Sui Jingfeng wrote: > On 2024/4/25 22:26, Andy Shevchenko wrote: > > It seems driver missed the point of proper use of device property APIs. > > Correct this by updating headers and calls respectively. > > You are using the 'seems' here exactly saying that you are not 100% sure. > > Please allow me to tell you the truth: This patch again has ZERO effect. > It fix nothing. And this patch is has the risks to be wrong. Huh?! Really, stop commenting the stuff you do not understand. > Simple because the "ili9341_probe() ---> ili9341_dbi_prob()" code path > is DT dependent. > > First of all, the devm_of_find_backlight() is called in ili9341_dbi_probe() > under *non-DT* environment, devm_of_find_backlight() is just a just a > no-op and will return NULL. NULL is not an error code, so ili9341_dbi_probe() > won't rage quit. But the several side effect is that the backlight will > NOT works at all. Is it a problem? > It is actually considered as fatal bug for *panels* if the backlight of > it is not light up, at least the brightness of *won't* be able to adjust. > What's worse, if there is no sane platform setup code at the firmware > or boot loader stage to set a proper initial state. The screen is complete > dark. Even though the itself panel is refreshing framebuffers, it can not > be seen by human's eye. Simple because of no backlight. Can you imagine that I may have different hardware that considered this is non-fatal error? > Second, the ili9341_dbi_probe() requires additional device properties to > be able to works very well on the rotation screen case. See the calling > of "device_property_read_u32(dev, "rotation", )" in > ili9341_dbi_probe() function. Yes, exactly, and how does it object the purpose of this patch? > Combine with those two factors, it is actually can conclude that the > panel-ilitek-ili9394 driver has the *implicit* dependency on 'OF'. > Removing the 'OF' dependency from its Kconfig just trigger the > leakage of such risks. What?! > My software node related patches can help to reduce part of the potential > risks, but it still need some extra work. And it is not landed yet. Your patch has nothing to do with this series. -- With Best Regards, Andy Shevchenko
Re: [RFC PATCH 02/18] drm/ttm: Add per-BO eviction tracking
On Thu, Apr 25, 2024 at 08:18:38AM +0200, Christian König wrote:
> Am 24.04.24 um 18:56 schrieb Friedrich Vock:
> > Make each buffer object aware of whether it has been evicted or not.
>
> That reverts some changes we made a couple of years ago.
>
> In general the idea is that eviction isn't something we need to reverse in
> TTM.
>
> Rather the driver gives the desired placement.
>
> Regards,
> Christian.
>
We have added a concept similar to this in drm_gpuvm [1]. GPUVM
maintains a list of evicted BOs and when the GPUVM is locked for
submission it has validate vfunc which is called on each BO. If driver
is using TTM, this is where the driver would call TTM BO validate which
unevicts the BO. Well at least this what we do it Xe [2].
The uneviction is a per VM operation not a global one. With this, a
global eviction list does not seem correct (admittedly not through the
entire series).
Matt
[1]
https://elixir.bootlin.com/linux/v6.8.7/source/drivers/gpu/drm/drm_gpuvm.c#L86
[2]
https://elixir.bootlin.com/linux/v6.8.7/source/drivers/gpu/drm/xe/xe_vm.c#L464
> >
> > Signed-off-by: Friedrich Vock
> > ---
> > drivers/gpu/drm/ttm/ttm_bo.c | 1 +
> > include/drm/ttm/ttm_bo.h | 11 +++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> > index edf10618fe2b2..3968b17453569 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > @@ -980,6 +980,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
> > struct ttm_buffer_object *bo,
> > bo->pin_count = 0;
> > bo->sg = sg;
> > bo->bulk_move = NULL;
> > + bo->evicted_type = TTM_NUM_MEM_TYPES;
> > if (resv)
> > bo->base.resv = resv;
> > else
> > diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
> > index 0223a41a64b24..8a1a29c6fbc50 100644
> > --- a/include/drm/ttm/ttm_bo.h
> > +++ b/include/drm/ttm/ttm_bo.h
> > @@ -121,6 +121,17 @@ struct ttm_buffer_object {
> > unsigned priority;
> > unsigned pin_count;
> >
> > + /**
> > +* @evicted_type: Memory type this BO was evicted from, if any.
> > +* TTM_NUM_MEM_TYPES if this BO was not evicted.
> > +*/
> > + int evicted_type;
> > + /**
> > +* @evicted: Entry in the evicted list for the resource manager
> > +* this BO was evicted from.
> > +*/
> > + struct list_head evicted;
> > +
> > /**
> > * @delayed_delete: Work item used when we can't delete the BO
> > * immediately
> > --
> > 2.44.0
> >
>
Re: [PATCH] drm/etnaviv: Create an accel device node if compute-only
On 4/24/2024 12:37 AM, Tomeu Vizoso wrote: If we expose a render node for NPUs without rendering capabilities, the userspace stack will offer it to compositors and applications for rendering, which of course won't work. Userspace is probably right in not questioning whether a render node might not be capable of supporting rendering, so change it in the kernel instead by exposing a /dev/accel node. Before we bring the device up we don't know whether it is capable of rendering or not (depends on the features of its blocks), so first try to probe a rendering node, and if we find out that there is no rendering hardware, abort and retry with an accel node. Signed-off-by: Tomeu Vizoso Cc: Oded Gabbay I hope Oded chimes in as Accel maintainer. I think Airlie/Vetter had also previously mentioned they'd have opinions on what is Accel vs DRM. This gets a nack from me in its current state. This is not a strong nack, and I don't want to discourage you. I think there is a path forward. The Accel subsystem documentation says that accel drivers will reside in drivers/accel/ but this does not. Also, the commit text for "accel: add dedicated minor for accelerator devices" mentions - "for drivers that declare they handle compute accelerator, using a new driver feature flag called DRIVER_COMPUTE_ACCEL. It is important to note that this driver feature is mutually exclusive with DRIVER_RENDER. Devices that want to expose both graphics and compute device char files should be handled by two drivers that are connected using the auxiliary bus framework." I don't see any of that happening here (two drivers connected by aux bus, one in drivers/accel). I think this is the first case we've had of a combo DRM/Accel usecase, and so there isn't an existing example to refer you to on how to structure things. I think you are going to be the first example where we figure all of this out. On a more implementation note, ioctls for Accel devices should not be marked DRM_RENDER_ALLOW. Seems like your attempt to reuse as much of the code as possible trips over this. -Jeff
Re: [v1,1/3] drm/panel: ili9341: Correct use of device property APIs
Hi, On 2024/4/26 02:08, Sui Jingfeng wrote: Hi, On 2024/4/25 22:26, Andy Shevchenko wrote: It seems driver missed the point of proper use of device property APIs. Correct this by updating headers and calls respectively. You are using the 'seems' here exactly saying that you are not 100% sure. Using the word 'seems' here exactly saying that you are not 100% sure. And this patch is has the risks to be wrong. This patch has the risks of to be wrong. Simple because the "ili9341_probe() ---> ili9341_dbi_prob()" code path is DT dependent. Simply because part of the driver is DT dependent, plus its code(implementation) is deep(tight) coupling, as a result, it is became total DT dependent. First of all, the devm_of_find_backlight() is called in ili9341_dbi_probe() , under *non-DT* environment, Under *non-DT* environment, the use case probably should take into consideration. Since you remove it, then we can't stop imagining. But if we really care about the usage case on DT based systems, There is *NO* difference between the device_get_match_data() and the of_device_get_match_data(). This is the reason why I'm saying that you patch has the *ZERO* effects. And again, on non-DT systems, if there is no acpi_device_id stuff, calling the device_get_match_data() function will just get NULL. Which is nearly a undefined behavior. So the 'OF 'removal is don't really make much sense. But there is a way to save the awkward situation, that is, helps to get this patch[1] merged. Then we still tenable both at the practice side and at the concept side. [1] https://patchwork.freedesktop.org/patch/590653/?series=131296=2 devm_of_find_backlight() is just a just a no-op and will return NULL. NULL is not an error code, so ili9341_dbi_probe() won't rage quit. [...] But the several side effect is that the backlight will NOT works at all. s/several/severe It is actually considered as fatal bug for *panels* if the backlight of it is not light up, It's fatal error if backlight is not adjustable or not light-up at all. [...] Even though the itself panel is refreshing framebuffers, Even though the panel itself is consuming frame-buffers and displaying. But if the backlight not work, the screen is extremely dark, we can not see as well. Besides the ili9341_dbi_probe() requires additional device properties to able to work very well. Especially on the rotate screen use case.
Re: [PATCH v3] drm/i915/vma: Fix UAF on reopen vs destroy race
Hi Thomas,
On Tuesday, 16 April 2024 18:40:12 CEST Rodrigo Vivi wrote:
> On Tue, Apr 16, 2024 at 10:09:46AM +0200, Janusz Krzysztofik wrote:
> > Hi Rodrigo,
> >
> > On Tuesday, 16 April 2024 03:16:31 CEST Rodrigo Vivi wrote:
> > > On Mon, Apr 15, 2024 at 09:53:09PM +0200, Janusz Krzysztofik wrote:
> > > > We defer actually closing, unbinding and destroying a VMA until next
> > > > idle
> > > > point, or until the object is freed in the meantime. By postponing the
> > > > unbind, we allow for the VMA to be reopened by the client, avoiding the
> > > > work required to rebind the VMA.
> > > >
> > > > It was assumed that as long as a GT is held idle, no VMA would be
> > > > reopened
> > > > while we destroy them. That assumption is no longer true in multi-GT
> > > > configurations, where a VMA we reopen may be handled by a GT different
> > > > from the one that we already keep active via its engine while we set up
> > > > an execbuf request.
> > > >
> > > > <4> [260.290809] [ cut here ]
> > > > <4> [260.290988] list_del corruption. prev->next should be
> > > > 888118c5d990, but was 888118c5a510. (prev=888118c5a510)
> > > > <4> [260.291004] WARNING: CPU: 2 PID: 1143 at lib/list_debug.c:62
> > > > __list_del_entry_valid_or_report+0xb7/0xe0
> > > > ..
> > > > <4> [260.291055] CPU: 2 PID: 1143 Comm: kms_plane Not tainted
> > > > 6.9.0-rc2-CI_DRM_14524-ga25d180c6853+ #1
> > > > <4> [260.291058] Hardware name: Intel Corporation Meteor Lake Client
> > > > Platform/MTL-P LP5x T3 RVP, BIOS MTLPFWI1.R00.3471.D91.2401310918
> > > > 01/31/2024
> > > > <4> [260.291060] RIP: 0010:__list_del_entry_valid_or_report+0xb7/0xe0
> > > > ...
> > > > <4> [260.291087] Call Trace:
> > > > <4> [260.291089]
> > > > <4> [260.291124] i915_vma_reopen+0x43/0x80 [i915]
> > > > <4> [260.291298] eb_lookup_vmas+0x9cb/0xcc0 [i915]
> > > > <4> [260.291579] i915_gem_do_execbuffer+0xc9a/0x26d0 [i915]
> > > > <4> [260.291883] i915_gem_execbuffer2_ioctl+0x123/0x2a0 [i915]
> > > > ...
> > > > <4> [260.292301]
> > > > ...
> > > > <4> [260.292506] ---[ end trace ]---
> > > > <4> [260.292782] general protection fault, probably for non-canonical
> > > > address 0x6b6b6b6b6b6b6ca3: [#1] PREEMPT SMP NOPTI
> > > > <4> [260.303575] CPU: 2 PID: 1143 Comm: kms_plane Tainted: GW
> > > >6.9.0-rc2-CI_DRM_14524-ga25d180c6853+ #1
> > > > <4> [260.313851] Hardware name: Intel Corporation Meteor Lake Client
> > > > Platform/MTL-P LP5x T3 RVP, BIOS MTLPFWI1.R00.3471.D91.2401310918
> > > > 01/31/2024
> > > > <4> [260.326359] RIP: 0010:eb_validate_vmas+0x114/0xd80 [i915]
> > > > ...
> > > > <4> [260.428756] Call Trace:
> > > > <4> [260.431192]
> > > > <4> [639.283393] i915_gem_do_execbuffer+0xd05/0x26d0 [i915]
> > > > <4> [639.305245] i915_gem_execbuffer2_ioctl+0x123/0x2a0 [i915]
> > > > ...
> > > > <4> [639.411134]
> > > > ...
> > > > <4> [639.449979] ---[ end trace ]---
> > > >
> > > > As soon as we start unbinding and destroying a VMA, marked it as parked,
> > > > and also keep it marked as closed for the rest of its life. When a VMA
> > > > to be opened occurs closed, reopen it only if not yet parked.
> > > >
> > > > v3: Fix misplaced brackets.
> > > > v2: Since we no longer re-init the VMA closed list link on VMA park so
> > > > it
> > > > looks like still on a list, don't try to delete it from the list
> > > > again
> > > > after the VMA has been marked as parked.
> > > >
> > > > Fixes: b0647a5e79b1 ("drm/i915: Avoid live-lock with i915_vma_parked()")
> > >
> > > what about reverting that?
> >
> > I didn't think of that. Why you think that might be a better approach?
>
> well, I thought of that mainly because...
>
> >
> > Anyway, that's a 4 years old patch and a few things have changed since
> > then,
> > so simple revert won't work. Moreover, I've just checked that patch was
> > supposed to fix another patch, 77853186e547 ("drm/i915: Claim vma while
> > under
> > closed_lock in i915_vma_parked()"), which in turn was supposed to fix
> > aa5e4453dc05 ("drm/i915/gem: Try to flush pending unbind events"), and that
> > one also referenced still another, cb6c3d45f948 ("drm/i915/gem: Avoid
> > parking
> > the vma as we unbind") from December 2019, which finally wasn't a fix but
> > an
> > improvement.
>
> ... because of histories like that ^ and I was afraid of this patch here now
> just put us into a different corner case.
>
> I have a feeling that without locks there we might just hit another
> race soon with the the park and only using the atomic checks.
>
> > Then, we would have to consider new fixes alternative to at least
> > some of those three, I guess.
>
> Indeed.. I didn't think that deep on that...
>
> > I'd rather not dig that deep, unless we invest
> > in a completely new solution (e.g. backport VMA handling from xe if more
> > effective while compatible to some extent?). Even then, we need a
Re: [v1,1/3] drm/panel: ili9341: Correct use of device property APIs
Hi,
On 2024/4/25 22:26, Andy Shevchenko wrote:
It seems driver missed the point of proper use of device property APIs.
Correct this by updating headers and calls respectively.
You are using the 'seems' here exactly saying that you are not 100% sure.
Please allow me to tell you the truth: This patch again has ZERO effect.
It fix nothing. And this patch is has the risks to be wrong.
Simple because the "ili9341_probe() ---> ili9341_dbi_prob()" code path
is DT dependent.
First of all, the devm_of_find_backlight() is called in ili9341_dbi_probe()
under *non-DT* environment, devm_of_find_backlight() is just a just a
no-op and will return NULL. NULL is not an error code, so ili9341_dbi_probe()
won't rage quit. But the several side effect is that the backlight will
NOT works at all.
It is actually considered as fatal bug for *panels* if the backlight of
it is not light up, at least the brightness of *won't* be able to adjust.
What's worse, if there is no sane platform setup code at the firmware
or boot loader stage to set a proper initial state. The screen is complete
dark. Even though the itself panel is refreshing framebuffers, it can not
be seen by human's eye. Simple because of no backlight.
Second, the ili9341_dbi_probe() requires additional device properties to
be able to works very well on the rotation screen case. See the calling
of "device_property_read_u32(dev, "rotation", )" in
ili9341_dbi_probe() function.
Combine with those two factors, it is actually can conclude that the
panel-ilitek-ili9394 driver has the *implicit* dependency on 'OF'.
Removing the 'OF' dependency from its Kconfig just trigger the
leakage of such risks.
My software node related patches can help to reduce part of the potential
risks, but it still need some extra work. And it is not landed yet.
Fixes: 5a04227326b0 ("drm/panel: Add ilitek ili9341 panel driver")
Signed-off-by: Andy Shevchenko
Reviewed-by: Neil Armstrong
---
drivers/gpu/drm/panel/Kconfig| 2 +-
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index e54f6f5604ed..2d451820 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -177,7 +177,7 @@ config DRM_PANEL_ILITEK_IL9322
config DRM_PANEL_ILITEK_ILI9341
tristate "Ilitek ILI9341 240x320 QVGA panels"
- depends on OF && SPI
+ depends on SPI
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
depends on BACKLIGHT_CLASS_DEVICE
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 3574681891e8..7584ddb0e441 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -22,8 +22,9 @@
#include
#include
#include
+#include
#include
-#include
+#include
#include
#include
@@ -691,7 +692,7 @@ static int ili9341_dpi_probe(struct spi_device *spi, struct gpio_desc *dc,
* Every new incarnation of this display must have a unique
* data entry for the system in this driver.
*/
- ili->conf = of_device_get_match_data(dev);
+ ili->conf = device_get_match_data(dev);
if (!ili->conf) {
dev_err(dev, "missing device configuration\n");
return -ENODEV;
--
Best regards,
Sui
Re: [PATCH v9 1/8] x86/vmware: Correct macro names
On 4/25/24 8:21 AM, Borislav Petkov wrote: On Wed, Apr 24, 2024 at 04:14:06PM -0700, Alexey Makhalov wrote: VCPU_RESERVED and LEGACY_X2APIC are not VMware hypercall commands. These are bits in return value of VMWARE_CMD_GETVCPU_INFO command. Change VMWARE_CMD_ prefix to GETVCPU_INFO_ one. And move bit-shift operation to the macro body. I don't understand: $ git grep GETVCPU_INFO arch/x86/kernel/cpu/vmware.c:51:#define VMWARE_CMD_GETVCPU_INFO 68 arch/x86/kernel/cpu/vmware.c:478: VMWARE_CMD(GETVCPU_INFO, eax, ebx, ecx, edx); so that's a VMWARE_CMD 68, at least the prefix says so. And those two are *bits* in that eax which that hypercall returns. Or are those two bits generic but defined in a vmware-specific hypercall? Hm. These are VMware hypercall commands: #define VMWARE_CMD_GETVERSION10 #define VMWARE_CMD_GETHZ 45 #define VMWARE_CMD_GETVCPU_INFO 68 #define VMWARE_CMD_STEALCLOCK91 These are VMware-specific macros to analyze return values of corresponding commands. They are prefixed with command name. #define GETVCPU_INFO_LEGACY_X2APIC BIT(3) #define GETVCPU_INFO_VCPU_RESERVED BIT(31) #define STEALCLOCK_NOT_AVAILABLE (-1) #define STEALCLOCK_DISABLED0 #define STEALCLOCK_ENABLED 1 Name VMWARE_CMD_LEGACY_X2APIC was not correct as LEGACY_X2APIC is not a command but the meaning of 3rd bit of a return value of VMWARE_CMD_GETVCPU_INFO. So, change it to GETVCPU_INFO_LEGACY_X2APIC. The same change with GETVCPU_INFO_VCPU_RESERVED. Both these bits are not generic. --Alexey
Re: [PATCH v1 1/1] drm: fixed: Don't use "proxy" headers
On Mon, Apr 22, 2024 at 09:49:04PM +0300, Jani Nikula wrote: > On Mon, 22 Apr 2024, Andy Shevchenko > wrote: > > Update header inclusions to follow IWYU (Include What You Use) > > principle. > > > > Signed-off-by: Andy Shevchenko > > Assuming it builds, and nothing depends on other stuff from kernel.h via > drm_fixed.h, For the record, I have built-tested this via `make allyesconfig` on x86_64. > Reviewed-by: Jani Nikula Thank you! -- With Best Regards, Andy Shevchenko
Re: [PATCH] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq()
Hi,
On Thu, Apr 25, 2024 at 1:19 AM Jani Nikula wrote:
>
> > @@ -279,6 +281,8 @@ enum mipi_dsi_dcs_tear_mode {
> >
> > ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi,
> > const void *data, size_t len);
> > +ssize_t mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi,
> > + const void *data, size_t len);
> > ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
> > const void *data, size_t len);
> > ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data,
> > @@ -317,14 +321,10 @@ int mipi_dsi_dcs_get_display_brightness_large(struct
> > mipi_dsi_device *dsi,
> > #define mipi_dsi_generic_write_seq(dsi, seq...)
> > \
> > do {
> > \
> > static const u8 d[] = { seq };
> > \
> > - struct device *dev = >dev;
> > \
> > int ret;
> > \
> > - ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));
> > \
> > - if (ret < 0) {
> > \
> > - dev_err_ratelimited(dev, "transmit data failed:
> > %d\n", \
> > - ret);
> > \
> > + ret = mipi_dsi_generic_write_chatty(dsi, d, ARRAY_SIZE(d));
> > \
> > + if (ret < 0)
> > \
> > return ret;
> > \
> > - }
> > \
> > } while (0)
>
> The one thing that I've always disliked about these macros (even if I've
> never actually used them myself) is that they hide control flow from the
> caller, i.e. return directly. You don't see that in the code, it's not
> documented, and if you wanted to do better error handling yourself,
> you're out of luck.
Yeah, I agree that it's not the cleanest. That being said, it is
existing code and making the existing code less bloated seems worth
doing.
I'd also say that it feels worth it to have _some_ solution so that
the caller doesn't need to write error handling after every single cmd
sent. If we get rid of / discourage these macros that's either going
to end us up with ugly/verbose code or it's going to encourage people
to totally skip error handling. IMO neither of those are wonderful
solutions.
While thinking about this there were a few ideas I came up with. None
of them are amazing, but probably they are better than the hidden
"return" like this. Perhaps we could mark the current function as
"deprecated" and pick one of these depending on what others opinions
are:
1. Use "goto" and force the caller to give a goto target for error handling.
This is based on an idea that Dmitry came up with, but made a little
more explicit. Example usage:
int ret;
ret = 0;
mipi_dsi_dcs_write_seq_goto(dsi, , HX83102_SETSPCCMD, 0xcd,
some_cmd_failed);
mipi_dsi_dcs_write_seq_goto(dsi, , HX83102_SETMIPI, 0x84,
some_cmd_failed);
mipi_dsi_dcs_write_seq_goto(dsi, , HX83102_SETSPCCMD, 0x3f,
some_cmd_failed);
mipi_dsi_dcs_write_seq_goto(dsi, , HX83102_SETVDC, 0x1b, 0x04,
some_cmd_failed);
...
some_cmd_failed:
pr_err("Commands failed to write: %d", ret);
return ret;
}
One downside here is that you can't easily tell which command failed
to put it in the error message. A variant of this idea (1a?) could be
to hoist the print back into the write command. I'd want to pick one
or the other. I guess my preference would be to hoist the print into
the write command and if someone really doesn't want the print then
they call mipi_dsi_dcs_write_buffer() directly.
---
2. Accept that a slightly less efficient handling of the error case
and perhaps a less intuitive API, but avoid the goto.
Essentially you could pass in "ret" and have the function be a no-op
if an error is already present. Something like this:
void mipi_dsi_dcs_write_buffer_multi(struct mipi_dsi_device *dsi,
const void *data, size_t len, int *accum_ret)
{
if (*accum_ret)
return;
*accum_ret = mipi_dsi_dcs_write_buffer(dsi, data, len);
}
...and then the caller:
int ret;
ret = 0;
mipi_dsi_dcs_write_seq_multi(dsi, HX83102_SETSPCCMD, 0xcd, );
mipi_dsi_dcs_write_seq_multi(dsi, HX83102_SETMIPI, 0x84, );
mipi_dsi_dcs_write_seq_multi(dsi, HX83102_SETSPCCMD, 0x3f, );
mipi_dsi_dcs_write_seq_multi(dsi, HX83102_SETVDC, 0x1b, 0x04, );
if (ret)
goto some_cmd_failed;
This has similar properties to solution #1.
---
3. Accept that callers don't want to error handling but just need a print.
I'm not 100%
[GIT PULL] etnaviv-fixes for 6.9
Hi Dave, Sima, please pull the following fixes for the upcoming -rc. One small fix to properly disable TX clock gating on cores where it is known to be broken. The other patch is a bit more controversial, as it reverts a UAPI change that was introduced in the last merge window to better support NPUs. However, userspace decided to deduce the relevant properties from the chip ID itself [1], so there is no need for this UAPI anymore. So in hopes to keep this a blip in the 6.9-rc series, we are removing the exported properties again. Regards, Lucas [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28574 The following changes since commit 4cece764965020c22cff7665b18a012006359095: Linux 6.9-rc1 (2024-03-24 14:10:05 -0700) are available in the Git repository at: https://git.pengutronix.de/git/lst/linux tags/drm-etnaviv-fixes-2024-04-25 for you to fetch changes up to e877d705704d7c8fe17b6b5ebdfdb14b84c207a7: Revert "drm/etnaviv: Expose a few more chipspecs to userspace" (2024-04-25 16:56:20 +0200) - fix GC7000 TX clock gating - revert NPU UAPI changes Christian Gmeiner (1): Revert "drm/etnaviv: Expose a few more chipspecs to userspace" Derek Foreman (1): drm/etnaviv: fix tx clock gating on some GC7000 variants drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 24 ++-- drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 12 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 34 -- include/uapi/drm/etnaviv_drm.h | 5 - 4 files changed, 2 insertions(+), 73 deletions(-)
Re: [PATCH 08/10] dt-bindings: vendor-prefixes: Add AYN Technologies
On Wed, Apr 24, 2024 at 11:29:13PM +0800, Xilin Wu wrote: > Add an entry for AYN Technologies (https://www.ayntec.com/) > > Signed-off-by: Xilin Wu > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml > b/Documentation/devicetree/bindings/vendor-prefixes.yaml > index e4aeeb5fe4d1..c2365b0f4184 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > @@ -194,6 +194,8 @@ patternProperties: > description: Axentia Technologies AB >"^axis,.*": > description: Axis Communications AB > + "^ayn,.*": It is somewhat preferred to use the domain name (ayntec). > +description: AYN Technologies Co., Ltd. >"^azoteq,.*": > description: Azoteq (Pty) Ltd >"^azw,.*": > > -- > 2.44.0 >
Re: [PATCH 03/10] dt-bindings: display: panel: Add Synaptics TD4328
On Wed, Apr 24, 2024 at 11:29:08PM +0800, Xilin Wu wrote:
> Synaptics TD4328 is a display driver IC used to drive LCD DSI panels.
>
> Signed-off-by: Xilin Wu
> ---
> .../bindings/display/panel/synaptics,td4328.yaml | 69
> ++
> 1 file changed, 69 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/synaptics,td4328.yaml
> b/Documentation/devicetree/bindings/display/panel/synaptics,td4328.yaml
> new file mode 100644
> index ..216f2fb22b88
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/synaptics,td4328.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/synaptics,td4328.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synaptics TD4328-based DSI display panels
> +
> +maintainers:
> + - Xilin Wu
> +
> +description:
> + The Synaptics TD4328 is a generic DSI Panel IC used to control
> + LCD panels.
> +
> +allOf:
> + - $ref: panel-common.yaml#
> +
> +properties:
> + compatible:
> +contains:
> + const: syna,td4328
You need a compatible specific to a panel. This can be a fallback
though.
> +
> + vdd-supply:
> +description: Digital voltage rail
> +
> + vddio-supply:
> +description: Digital I/O voltage rail
> +
> + reg: true
> + port: true
> +
> +required:
> + - compatible
> + - reg
> + - reset-gpios
> + - vdd-supply
> + - vddio-supply
> + - port
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +#include
> +
> +dsi {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +panel@0 {
> +compatible = "syna,td4328";
> +reg = <0>;
> +
> +backlight = <>;
> +reset-gpios = < 133 GPIO_ACTIVE_LOW>;
> +
> +vdd-supply = <_lcm_2p8>;
> +vddio-supply = <_l12b_1p8>;
> +
> +port {
> +panel_in_0: endpoint {
> +remote-endpoint = <_out>;
> +};
> +};
> +};
> +};
> +
> +...
>
> --
> 2.44.0
>
Re: [PATCH v1 3/3] drm/panel: ili9341: Use predefined error codes
On 25/04/2024 16:26, Andy Shevchenko wrote:
In one case the -1 is returned which is quite confusing code for
the wrong device ID, in another the ret is returning instead of
plain 0 that also confusing as readed may ask the possible meaning
of positive codes, which are never the case there. Convert both
to use explicit predefined error codes to make it clear what's going
on there.
Fixes: 5a04227326b0 ("drm/panel: Add ilitek ili9341 panel driver")
Signed-off-by: Andy Shevchenko
---
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 24c74c56e564..b933380b7eb7 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -422,7 +422,7 @@ static int ili9341_dpi_prepare(struct drm_panel *panel)
ili9341_dpi_init(ili);
- return ret;
+ return 0;
}
static int ili9341_dpi_enable(struct drm_panel *panel)
@@ -726,7 +726,7 @@ static int ili9341_probe(struct spi_device *spi)
else if (!strcmp(id->name, "yx240qv29"))
return ili9341_dbi_probe(spi, dc, reset);
- return -1;
+ return -ENODEV;
}
static void ili9341_remove(struct spi_device *spi)
Reviewed-by: Neil Armstrong
Re: [PATCH v1 2/3] drm/panel: ili9341: Respect deferred probe
On 25/04/2024 16:26, Andy Shevchenko wrote:
GPIO controller might not be available when driver is being probed.
There are plenty of reasons why, one of which is deferred probe.
Since GPIOs are optional, return any error code we got to the upper
layer, including deferred probe. With that in mind, use dev_err_probe()
in order to avoid spamming the logs.
Fixes: 5a04227326b0 ("drm/panel: Add ilitek ili9341 panel driver")
Signed-off-by: Andy Shevchenko
---
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 7584ddb0e441..24c74c56e564 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -715,11 +715,11 @@ static int ili9341_probe(struct spi_device *spi)
reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(reset))
- dev_err(dev, "Failed to get gpio 'reset'\n");
+ return dev_err_probe(dev, PTR_ERR(reset), "Failed to get gpio
'reset'\n");
dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
if (IS_ERR(dc))
- dev_err(dev, "Failed to get gpio 'dc'\n");
+ return dev_err_probe(dev, PTR_ERR(dc), "Failed to get gpio
'dc'\n");
if (!strcmp(id->name, "sf-tc240t-9370-t"))
return ili9341_dpi_probe(spi, dc, reset);
Reviewed-by: Neil Armstrong
Re: [PATCH v1 1/3] drm/panel: ili9341: Correct use of device property APIs
On 25/04/2024 16:26, Andy Shevchenko wrote:
It seems driver missed the point of proper use of device property APIs.
Correct this by updating headers and calls respectively.
Fixes: 5a04227326b0 ("drm/panel: Add ilitek ili9341 panel driver")
Signed-off-by: Andy Shevchenko
---
drivers/gpu/drm/panel/Kconfig| 2 +-
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index e54f6f5604ed..2d451820 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -177,7 +177,7 @@ config DRM_PANEL_ILITEK_IL9322
config DRM_PANEL_ILITEK_ILI9341
tristate "Ilitek ILI9341 240x320 QVGA panels"
- depends on OF && SPI
+ depends on SPI
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
depends on BACKLIGHT_CLASS_DEVICE
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 3574681891e8..7584ddb0e441 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -22,8 +22,9 @@
#include
#include
#include
+#include
#include
-#include
+#include
#include
#include
@@ -691,7 +692,7 @@ static int ili9341_dpi_probe(struct spi_device *spi, struct gpio_desc *dc,
* Every new incarnation of this display must have a unique
* data entry for the system in this driver.
*/
- ili->conf = of_device_get_match_data(dev);
+ ili->conf = device_get_match_data(dev);
if (!ili->conf) {
dev_err(dev, "missing device configuration\n");
return -ENODEV;
Reviewed-by: Neil Armstrong
Re: [PATCH 1/4] WIP: rust: Add basic KMS bindings
On 22.04.24 03:47, Lyude Paul wrote: > On Wed, 2024-03-27 at 20:50 +, Benno Lossin wrote: >> Hi, >> >> I just took a quick look and commented on the things that stuck >> out to me. Some general things: >> - several `unsafe` blocks have missing SAFETY comments, >> - missing documentation and examples. > > This is really early on - so I had wanted to post a WIP before I > actually wrote up everything to make sure I'm going in the right > direction (I'm certainly not planning on leaving things undocumented > when this is actually ready for submission :). No worries, I just wanted to point out everything that I found. One thing that I missed was your "RFC WIP" in your cover letter. I think that it's a good idea to put "RFC" onto every patch, that way people without context immediately know that it is not yet ready. -- Cheers, Benno
Re: [PATCH 2/4] WIP: drm: Introduce rvkms
On 22.04.24 03:54, Lyude Paul wrote:
> On Wed, 2024-03-27 at 21:06 +, Benno Lossin wrote:
>> On 22.03.24 23:03, Lyude Paul wrote:
>>> +
>>> +pub(crate) type Connector = connector::Connector;
>>> +
>>> +impl connector::DriverConnector for DriverConnector {
>>> +type Initializer = impl PinInit;
>>> +
>>> +type State = ConnectorState;
>>> +
>>> +type Driver = RvkmsDriver;
>>> +
>>> +type Args = ();
>>> +
>>> +fn new(dev: , args: Self::Args) ->
>>> Self::Initializer {
>>
>> And then here just return `Self`.
>>
>> This works, since there is a blanket impl `PinInit for T`.
>>
>> Looking at how you use this API, I am not sure if you actually need
>> pin-init for the type that implements `DriverConnector`.
>> Do you need to store eg `Mutex` or something else that needs
>> pin-init in here in a more complex driver?
>
> Most likely yes - a lot of drivers have various private locks contained
> within their subclassed mode objects. I'm not sure we will in rvkms's
> connector since vkms doesn't really do much with connectors - but we at
> a minimum be using pinned types (spinlocks and hrtimers) in our
> DriverCrtc implementation once I've started implementing support for
> vblanks[1]
>
> [1]
> https://www.kernel.org/doc/html/v6.9-rc5/gpu/drm-kms.html?highlight=vblank#vertical-blanking
>
> In nova (the main reason I'm working on rvkms in the first place),
> we'll definitely have locks in our connectors and possibly other types.
I see, in that case it would be a good idea to either have an RFC of
the nova driver (or something else that needs pinned types) as
motivation for why it needs to be pin-initialized.
--
Cheers,
Benno
[GIT PULL] mediatek drm next for 6.10
Hi, Dave & Daniel:
This includes:
1. Use devm_platform_get_and_ioremap_resource() in mtk_hdmi_ddc_probe()
2. Add GAMMA 12-bit LUT support for MT8188
3. Add 0 size check to mtk_drm_gem_obj
4. Init `ddp_comp` with devm_kcalloc()
5. Rename mtk_drm_* to mtk_*
6. Drop driver owner initialization
7. Fix mtk_dp_aux_transfer return value
8. Correct calculation formula of PHY Timing
Regards,
Chun-Kuang.
The following changes since commit 4cece764965020c22cff7665b18a012006359095:
Linux 6.9-rc1 (2024-03-24 14:10:05 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git
tags/mediatek-drm-next-6.10
for you to fetch changes up to 417d8c47271d5cf1a705e997065873b2a9a36fd4:
drm/mediatek: dsi: Correct calculation formula of PHY Timing (2024-04-22
13:40:34 +)
Mediatek DRM Next for Linux 6.10
1. Use devm_platform_get_and_ioremap_resource() in mtk_hdmi_ddc_probe()
2. Add GAMMA 12-bit LUT support for MT8188
3. Add 0 size check to mtk_drm_gem_obj
4. Init `ddp_comp` with devm_kcalloc()
5. Rename mtk_drm_* to mtk_*
6. Drop driver owner initialization
7. Fix mtk_dp_aux_transfer return value
8. Correct calculation formula of PHY Timing
Douglas Anderson (1):
drm/mediatek: Init `ddp_comp` with devm_kcalloc()
Hsiao Chien Sung (14):
drm/mediatek: Rename "mtk_drm_crtc" to "mtk_crtc"
drm/mediatek: Rename "mtk_drm_ddp_comp" to "mtk_ddp_comp"
drm/mediatek: Rename "mtk_drm_plane" to "mtk_plane"
drm/mediatek: Rename "mtk_drm_gem" to "mtk_gem"
drm/mediatek: Rename "mtk_drm_hdmi" to "mtk_hdmi"
drm/mediatek: Rename files "mtk_drm_crtc.h" to "mtk_crtc.h"
drm/mediatek: Rename files "mtk_drm_crtc.c" to "mtk_crtc.c"
drm/mediatek: Rename files "mtk_drm_ddp_comp.h" to "mtk_ddp_comp.h"
drm/mediatek: Rename files "mtk_drm_ddp_comp.c" to "mtk_ddp_comp.c"
drm/mediatek: Rename files "mtk_drm_plane.h" to "mtk_plane.h"
drm/mediatek: Rename files "mtk_drm_plane.c" to "mtk_plane.c"
drm/mediatek: Rename files "mtk_drm_gem.h" to "mtk_gem.h"
drm/mediatek: Rename files "mtk_drm_gem.c" to "mtk_gem.c"
drm/mediatek: Rename mtk_ddp_comp functions
Jason-JH.Lin (3):
dt-bindings: display: mediatek: gamma: Change MT8195 to single enum group
dt-bindings: display: mediatek: gamma: Add support for MT8188
drm/mediatek: Add gamma support for MT8195
Justin Green (1):
drm/mediatek: Add 0 size check to mtk_drm_gem_obj
Krzysztof Kozlowski (11):
drm/mediatek: aal: drop driver owner initialization
drm/mediatek: ccorr: drop driver owner initialization
drm/mediatek: color: drop driver owner initialization
drm/mediatek: gamma: drop driver owner initialization
drm/mediatek: merge: drop driver owner initialization
drm/mediatek: ovl: drop driver owner initialization
drm/mediatek: ovl_adaptor: drop driver owner initialization
drm/mediatek: rdma: drop driver owner initialization
drm/mediatek: ethdr: drop driver owner initialization
drm/mediatek: mdp_rdma: drop driver owner initialization
drm/mediatek: padding: drop driver owner initialization
Markus Elfring (1):
drm/mediatek: Use devm_platform_get_and_ioremap_resource() in
mtk_hdmi_ddc_probe()
Shuijing Li (1):
drm/mediatek: dsi: Correct calculation formula of PHY Timing
Wojciech Macek (1):
drm/mediatek: dp: Fix mtk_dp_aux_transfer return value
.../bindings/display/mediatek/mediatek,gamma.yaml | 5 +
drivers/gpu/drm/mediatek/Makefile | 12 +-
.../drm/mediatek/{mtk_drm_crtc.c => mtk_crtc.c}| 218 ++---
drivers/gpu/drm/mediatek/mtk_crtc.h| 28 +++
.../{mtk_drm_ddp_comp.c => mtk_ddp_comp.c} | 51 ++---
.../{mtk_drm_ddp_comp.h => mtk_ddp_comp.h} | 9 +-
drivers/gpu/drm/mediatek/mtk_disp_aal.c| 5 +-
drivers/gpu/drm/mediatek/mtk_disp_ccorr.c | 5 +-
drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 +-
drivers/gpu/drm/mediatek/mtk_disp_drv.h| 2 +-
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 5 +-
drivers/gpu/drm/mediatek/mtk_disp_merge.c | 3 +-
drivers/gpu/drm/mediatek/mtk_disp_ovl.c| 5 +-
drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c| 5 +-
drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 +-
drivers/gpu/drm/mediatek/mtk_dp.c | 2 +-
drivers/gpu/drm/mediatek/mtk_dpi.c | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_crtc.h| 30 ---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 34 ++--
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 +-
drivers/gpu/drm/mediatek/mtk_dsi.c | 33 ++--
drivers/gpu/drm/mediatek/mtk_ethdr.c | 5 +-
.../gpu/drm/mediatek/{mtk_drm_gem.c
Re: [PATCH v9 1/8] x86/vmware: Correct macro names
On Wed, Apr 24, 2024 at 04:14:06PM -0700, Alexey Makhalov wrote: > VCPU_RESERVED and LEGACY_X2APIC are not VMware hypercall commands. > These are bits in return value of VMWARE_CMD_GETVCPU_INFO command. > Change VMWARE_CMD_ prefix to GETVCPU_INFO_ one. And move bit-shift > operation to the macro body. I don't understand: $ git grep GETVCPU_INFO arch/x86/kernel/cpu/vmware.c:51:#define VMWARE_CMD_GETVCPU_INFO 68 arch/x86/kernel/cpu/vmware.c:478: VMWARE_CMD(GETVCPU_INFO, eax, ebx, ecx, edx); so that's a VMWARE_CMD 68, at least the prefix says so. And those two are *bits* in that eax which that hypercall returns. Or are those two bits generic but defined in a vmware-specific hypercall? Hm. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 0/1] Fix the port mux of VP2
On Mon, 22 Apr 2024 18:19:04 +0800, Andy Yan wrote: > From: Andy Yan > > > The port mux bits of VP2 should be defined by RK3568_OVL_PORT_SET__PORT2_MUX, > this maybe a copy and paste error when this driver first introduced. > Hi Heiko: >Maybe thi is the problem you met when you porting the dsi2 driver. > I previously sent you this patch when you ask me about this issue on > email,but I'm not sure if you received it. > > [...] Applied, thanks! [1/1] drm/rockchip: vop2: Fix the port mux of VP2 commit: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f Best regards, -- Heiko Stuebner
Re: [PATCH v4 00/13] drm: zynqmp_dp: IRQ cleanups and debugfs support
On 4/24/24 14:54, Tomi Valkeinen wrote: > Hi Sean, > > On 23/04/2024 20:18, Sean Anderson wrote: >> This series cleans up the zyqnmp_dp IRQ and locking situation. Once >> that's done, it adds debugfs support. The intent is to enable compliance >> testing or to help debug signal-integrity issues. >> >> Last time I discussed converting the HPD work(s) to a threaded IRQ. I >> did not end up doing that for this series since the steps would be >> >> - Add locking >> - Move link retraining to a work function >> - Harden the IRQ >> - Merge the works into a threaded IRQ (omitted) >> >> Which with the exception of the final step is the same as leaving those >> works as-is. Conversion to a threaded IRQ can be done as a follow-up. > > If it's ok to you, I'd like to pick the first four patches to drm-misc > already. Fine by me. --Sean
Re: [PATCH v1 0/3] drm/panel: ili9341: Obvious fixes
On Thu, Apr 25, 2024 at 05:26:16PM +0300, Andy Shevchenko wrote: > A few obvious fixes to the driver. Note, despite the desire of removal Adafruit support from this driver, the older (read: stable) kernels will need this. -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/1] drm/ili9341: Remove the duplicative driver
On Thu, Apr 25, 2024 at 04:58:06PM +0200, Maxime Ripard wrote: > Hi, > > On Thu, Apr 25, 2024 at 03:42:07PM +0300, Andy Shevchenko wrote: > > First of all, the driver was introduced when it was already > > two drivers available for Ilitek 9341 panels. > > > > Second, the most recent (fourth!) driver has incorporated this one > > and hence, when enabled, it covers the provided functionality. > > > > Taking into account the above, remove duplicative driver and make > > maintenance and support eaiser for everybody. > > > > Also see discussion [1] for details about Ilitek 9341 duplication > > code. > > > > Link: https://lore.kernel.org/r/zxm9pg-53v4s8...@smile.fi.intel.com [1] > > Signed-off-by: Andy Shevchenko > > I think it should be the other way around and we should remove the > mipi-dbi handling from panel/panel-ilitek-ili9341.c Then please do it! I whining already for a few years about this. > It's basically two drivers glued together for no particular reason and > handling two very different use cases which just adds more complexity > than it needs to. > > And it's the only driver doing so afaik, so it's definitely not "least > surprise" compliant. -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/1] drm/ili9341: Remove the duplicative driver
Hi, On Thu, Apr 25, 2024 at 03:42:07PM +0300, Andy Shevchenko wrote: > First of all, the driver was introduced when it was already > two drivers available for Ilitek 9341 panels. > > Second, the most recent (fourth!) driver has incorporated this one > and hence, when enabled, it covers the provided functionality. > > Taking into account the above, remove duplicative driver and make > maintenance and support eaiser for everybody. > > Also see discussion [1] for details about Ilitek 9341 duplication > code. > > Link: https://lore.kernel.org/r/zxm9pg-53v4s8...@smile.fi.intel.com [1] > Signed-off-by: Andy Shevchenko I think it should be the other way around and we should remove the mipi-dbi handling from panel/panel-ilitek-ili9341.c It's basically two drivers glued together for no particular reason and handling two very different use cases which just adds more complexity than it needs to. And it's the only driver doing so afaik, so it's definitely not "least surprise" compliant. Maxime signature.asc Description: PGP signature
Re: [PATCH 1/1] drm/rockchip: vop2: Fix the port mux of VP2
Am Montag, 22. April 2024, 12:19:05 CEST schrieb Andy Yan:
> From: Andy Yan
>
> The port mux of VP2 should be RK3568_OVL_PORT_SET__PORT2_MUX.
>
> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> Signed-off-by: Andy Yan
on a rk3588 with VP3 connected to a DSI display
Tested-by: Heiko Stuebner
Re: [PATCH] drm/panthor: Add defer probe for firmware load
Steven Price writes: > Hi Javier, > > On 25/04/2024 10:22, Javier Martinez Canillas wrote: >> Steven Price writes: >> >> Hello Steven, >> >>> On 13/04/2024 12:49, Andy Yan wrote: From: Andy Yan The firmware in the rootfs will not be accessible until we are in the SYSTEM_RUNNING state, so return EPROBE_DEFER until that point. This let the driver can load firmware when it is builtin. >>> >>> The usual solution is that the firmware should be placed in the >>> initrd/initramfs if the module is included there (or built-in). The same >>> issue was brought up regarding the powervr driver: >>> >>> https://lore.kernel.org/dri-devel/20240109120604.603700-1-javi...@redhat.com/T/ >>> >>> I'm not sure if that ever actually reached a conclusion though. The >>> question was deferred to Greg KH but I didn't see Greg actually getting >>> involved in the thread. >>> >> >> Correct, there was not conclusion reached in that thread. > > So I think we need a conclusion before we start applying point fixes to > individual drivers. > Agreed. [...] >> >> In the thread you referenced I suggested to add that logic in >> request_firmware() >> (or add a new request_firmware_defer() helper function) that changes the >> request >> firmare behaviour to return -EPROBE_DEFER instead of -ENOENT. > > That would seem like a good feature if it's agreed that deferring on > request_firmware is a good idea. > Yeah. I didn't attempt to type that patch because didn't get an answer from Greg and didn't want to spent the time writing and testing a patch to just be nacked. >> Since as you mentioned, this isn't specific to panthor and an issue that I >> also >> faced with the powervr driver. > > I'm not in any way against the idea of deferring the probe until the > firmware is around - indeed it seems like a very sensible idea in many > respects. But I don't want panthor to be 'special' in this way. > > If the consensus is that the firmware should live with the module (i.e. > either both in the initramfs or both in the rootfs) then the code is > fine as it is. That seemed to be the view of Sima in that thread and > seems reasonable to me - why put the .ko in the initrd if you can't > actually use it until the rootfs comes along? > That's indeed a sensible position for me as well and is what I answered to the user who reported the powervr issue. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v3 1/4] drm: add devm release action
On 25/04/24 18:22, Maxime Ripard wrote: > On Wed, Apr 24, 2024 at 08:20:32AM -0400, Rodrigo Vivi wrote: >> On Wed, Apr 24, 2024 at 01:49:16PM +0200, Maxime Ripard wrote: >>> On Tue, Apr 23, 2024 at 01:42:22PM -0400, Rodrigo Vivi wrote: On Tue, Apr 23, 2024 at 02:25:06PM +0530, Aravind Iddamsetty wrote: > On 23/04/24 02:24, Rodrigo Vivi wrote: >> On Mon, Apr 22, 2024 at 12:27:53PM +0530, Aravind Iddamsetty wrote: >>> In scenarios where drm_dev_put is directly called by driver we want to >>> release devm_drm_dev_init_release action associated with struct >>> drm_device. >>> >>> v2: Directly expose the original function, instead of introducing a >>> helper (Rodrigo) >>> >>> v3: add kernel-doc (Maxime Ripard) >>> >>> Cc: Maxime Ripard >>> Cc: Thomas Hellstr_m >>> Cc: Rodrigo Vivi >>> >> please avoid these empty lines here cc, rv-b, sign-offs, links, >> etc are all in the same block. > ok. >>> Reviewed-by: Rodrigo Vivi >>> Signed-off-by: Aravind Iddamsetty >>> --- >>> drivers/gpu/drm/drm_drv.c | 13 + >>> include/drm/drm_drv.h | 2 ++ >>> 2 files changed, 15 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >>> index 243cacb3575c..9d0409165f1e 100644 >>> --- a/drivers/gpu/drm/drm_drv.c >>> +++ b/drivers/gpu/drm/drm_drv.c >>> @@ -714,6 +714,19 @@ static int devm_drm_dev_init(struct device *parent, >>> devm_drm_dev_init_release, dev); >>> } >>> >>> +/** >>> + * devm_drm_dev_release_action - Call the final release action of the >>> device >> Seeing the doc here gave me a second thought >> >> the original release should be renamed to _devm_drm_dev_release >> and this should be called devm_drm_dev_release without the 'action' word. > i believe, was suggested earlier to directly expose the main function, is > there any reason to have a __ version ? No no, just ignore me. Just remove the '_action' and don't change the other. I don't like exposing the a function with '__'. what would '__' that mean? This is what I meant on the first comment. Now, I believe that we don't need the '_action'. What does the 'action' mean? the devm_drm_dev_release should be enough. But then I got confused and I thought it would conflict with the original released function name. But I misread it. >>> I don't think devm_drm_dev_release is a good name either. Just like any >>> other devm_* function that cancels what a previous one has been doing >>> (devm_kfree, devm_backlight_device_unregister, devm_nvmem_device_put, >>> etc.) it should be called devm_drm_dev_put or something similar. >> I see what you mean, but I don't believe the 'put' is the best option, >> for 2 reasons: >> - in general, we have put paired with gets and this has not get equivalent > Yeah, that's true. _release is fine then I guess. > >> - this bypass the regular get/put mechanism and forces the releases that >> would be done only after all drm_dev_put() taking ref to zero. > I don't think it does? devm_release_action will only remove the devm > action and execute it directly, but this action here is a call to > drm_dev_put, so we might still have other references taken that would > defer the device being freed. yes i.e right, i assumed drm_dev_unplug would close all client handles but no. So i was thinking if it is ok to iterate over no of clients and call drm_dev_put in either drm_dev_unplug or as part of this devm_release. Thanks, Aravind. > > Maxime
[PULL] drm-misc-next
Hi Dave, Sima, One more pull request for v6.10! Cheers, ~Maarten drm-misc-next-2024-04-25: drm-misc-next for v6.10-rc1: UAPI Changes: Cross-subsystem Changes: - Devicetree updates for rockchip (#sound-dai-cells) - Add dt bindings for new panels. - Change bridge/tc358775 dt bindings. Core Changes: - Fix SIZE_HINTS cursor property doc. - Parse topology blocks for all DispID < 2.0. - Implement support for tracking cleared free memory, use it in amdgpu. - Drop seq_file.h from drm_print.h, and include debugfs.h explicitly where needed (drivers). Driver Changes: - Small fixes to rockchip, panthor, v3d, bridge chaining, xlx. - Add Khadas TS050 V2, EDO RM69380 OLED, CSOT MNB601LS1-1 panels, - Add SAM9X7 SoC's LVDS controller. - More driver conversions to struct drm_edid. - Support tc358765 in tc358775 bridge. The following changes since commit 0208ca55aa9c9b997da1f5bc45c4e98916323f08: Backmerge tag 'v6.9-rc5' into drm-next (2024-04-22 14:35:52 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/drm/misc/kernel.git tags/drm-misc-next-2024-04-25 for you to fetch changes up to 9e2b84fb6cd7ee913aa61d461db65c1d6a08dcf2: drm/print: drop include seq_file.h (2024-04-25 17:05:48 +0300) drm-misc-next for v6.10-rc1: UAPI Changes: Cross-subsystem Changes: - Devicetree updates for rockchip (#sound-dai-cells) - Add dt bindings for new panels. - Change bridge/tc358775 dt bindings. Core Changes: - Fix SIZE_HINTS cursor property doc. - Parse topology blocks for all DispID < 2.0. - Implement support for tracking cleared free memory, use it in amdgpu. - Drop seq_file.h from drm_print.h, and include debugfs.h explicitly where needed (drivers). Driver Changes: - Small fixes to rockchip, panthor, v3d, bridge chaining, xlx. - Add Khadas TS050 V2, EDO RM69380 OLED, CSOT MNB601LS1-1 panels, - Add SAM9X7 SoC's LVDS controller. - More driver conversions to struct drm_edid. - Support tc358765 in tc358775 bridge. Adam Ford (1): drm/bridge: imx: Fix unmet depenency for PHY_FSL_SAMSUNG_HDMI_PHY Anatoliy Klymenko (6): drm: xlnx: zynqmp_dpsub: Set layer mode during creation drm: xlnx: zynqmp_dpsub: Update live format defines drm: xlnx: zynqmp_dpsub: Add connected live layer helper drm: xlnx: zynqmp_dpsub: Anounce supported input formats drm: xlnx: zynqmp_dpsub: Minimize usage of global flag drm: xlnx: zynqmp_dpsub: Set input live format Andy Yan (1): drm/rockchip: lvds: Remove include of drm_dp_helper.h Arunpravin Paneer Selvam (3): drm/buddy: Implement tracking clear page feature drm/amdgpu: Enable clear page functionality drm/tests: Add a test case for drm buddy clear allocation Barnabás Czémán (1): drm/panel: jdi-fhd-r63452: make use of prepare_prev_first Dan Carpenter (1): drm/panthor: clean up some types in panthor_sched_suspend() David Wronek (2): dt-bindings: display: panel: Add Raydium RM69380 drm/panel: Add driver for EDO RM69380 OLED panel Detlev Casanova (1): drm/rockchip: vop2: Do not divide height twice for YUV Dharma Balasubiramani (3): dt-bindings: display: bridge: add sam9x75-lvds binding drm/bridge: add lvds controller support for sam9x7 MAINTAINERS: add SAM9X7 SoC's LVDS controller Dmitry Baryshkov (5): drm/panel: novatek-nt36672e: stop setting register load before disable drm/panel: novatek-nt36672e: stop calling regulator_set_load manually drm/panel: novatek-nt36672a: stop calling regulator_set_load manually drm/panel: visionox-rm69299: stop calling regulator_set_load manually drm/bridge: adv7511: make it honour next bridge in DT Jacobe Zang (2): dt-bindings: panel-simple-dsi: add Khadas TS050 V2 panel drm/panel: add Khadas TS050 V2 panel support Jani Nikula (11): drm/panel: simple: switch to struct drm_edid drm/panel-samsung-atna33xc20: switch to struct drm_edid drm/panel-edp: switch to struct drm_edid drm/sun4i: hdmi: switch to struct drm_edid drm/vc4: hdmi: switch to struct drm_edid drm/gud: switch to struct drm_edid drm/rockchip: cdn-dp: switch to struct drm_edid drm/rockchip: inno_hdmi: switch to struct drm_edid drm/rockchip: rk3066_hdmi: switch to struct drm_edid drm/print: drop include debugfs.h and include where needed drm/print: drop include seq_file.h Johan Jonker (3): dt-bindings: display: add #sound-dai-cells property to rockchip dw hdmi dt-bindings: display: add #sound-dai-cells property to rockchip rk3066 hdmi dt-bindings: display: add #sound-dai-cells property to rockchip inno hdmi Krzysztof Kozlowski (3): drm/rockchip: cdn-dp: drop driver owner assignment drm/bridge: chipone-icn6211: drop driver owner assignment drm/bridge: tc358764: drop driver owner
Re: [PATCH] drm/panthor: Add defer probe for firmware load
Hi Javier,
On 25/04/2024 10:22, Javier Martinez Canillas wrote:
> Steven Price writes:
>
> Hello Steven,
>
>> On 13/04/2024 12:49, Andy Yan wrote:
>>> From: Andy Yan
>>>
>>> The firmware in the rootfs will not be accessible until we
>>> are in the SYSTEM_RUNNING state, so return EPROBE_DEFER until
>>> that point.
>>> This let the driver can load firmware when it is builtin.
>>
>> The usual solution is that the firmware should be placed in the
>> initrd/initramfs if the module is included there (or built-in). The same
>> issue was brought up regarding the powervr driver:
>>
>> https://lore.kernel.org/dri-devel/20240109120604.603700-1-javi...@redhat.com/T/
>>
>> I'm not sure if that ever actually reached a conclusion though. The
>> question was deferred to Greg KH but I didn't see Greg actually getting
>> involved in the thread.
>>
>
> Correct, there was not conclusion reached in that thread.
So I think we need a conclusion before we start applying point fixes to
individual drivers.
>>> Signed-off-by: Andy Yan
>>> ---
>>>
>>> drivers/gpu/drm/panthor/panthor_fw.c | 11 ++-
>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c
>>> b/drivers/gpu/drm/panthor/panthor_fw.c
>>> index 33c87a59834e..25e375f8333c 100644
>>> --- a/drivers/gpu/drm/panthor/panthor_fw.c
>>> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
>>> @@ -1336,8 +1336,17 @@ int panthor_fw_init(struct panthor_device *ptdev)
>>> }
>>>
>>> ret = panthor_fw_load(ptdev);
>>> - if (ret)
>>> + if (ret) {
>>> + /*
>>> +* The firmware in the rootfs will not be accessible until we
>>> +* are in the SYSTEM_RUNNING state, so return EPROBE_DEFER until
>>> +* that point.
>>> +*/
>>> + if (system_state < SYSTEM_RUNNING)
>>
>> This should really only be in the case where ret == -ENOENT - any other
>> error and the firmware is apparently present but broken in some way, so
>> there's no point deferring.
>>
>> I also suspect we'd need some change in panthor_fw_load() to quieten
>> error messages if we're going to defer on this, in which case it might
>> make more sense to move this logic into that function.
>>
>
> In the thread you referenced I suggested to add that logic in
> request_firmware()
> (or add a new request_firmware_defer() helper function) that changes the
> request
> firmare behaviour to return -EPROBE_DEFER instead of -ENOENT.
That would seem like a good feature if it's agreed that deferring on
request_firmware is a good idea.
> Since as you mentioned, this isn't specific to panthor and an issue that I
> also
> faced with the powervr driver.
I'm not in any way against the idea of deferring the probe until the
firmware is around - indeed it seems like a very sensible idea in many
respects. But I don't want panthor to be 'special' in this way.
If the consensus is that the firmware should live with the module (i.e.
either both in the initramfs or both in the rootfs) then the code is
fine as it is. That seemed to be the view of Sima in that thread and
seems reasonable to me - why put the .ko in the initrd if you can't
actually use it until the rootfs comes along?
The other option is the user fallback mechanisms for firmware loading
which can wait arbitrarily for the firmware to become available. But
that isn't exactly popular these days.
Steve
[PATCH v1 2/3] drm/panel: ili9341: Respect deferred probe
GPIO controller might not be available when driver is being probed.
There are plenty of reasons why, one of which is deferred probe.
Since GPIOs are optional, return any error code we got to the upper
layer, including deferred probe. With that in mind, use dev_err_probe()
in order to avoid spamming the logs.
Fixes: 5a04227326b0 ("drm/panel: Add ilitek ili9341 panel driver")
Signed-off-by: Andy Shevchenko
---
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 7584ddb0e441..24c74c56e564 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -715,11 +715,11 @@ static int ili9341_probe(struct spi_device *spi)
reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(reset))
- dev_err(dev, "Failed to get gpio 'reset'\n");
+ return dev_err_probe(dev, PTR_ERR(reset), "Failed to get gpio
'reset'\n");
dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
if (IS_ERR(dc))
- dev_err(dev, "Failed to get gpio 'dc'\n");
+ return dev_err_probe(dev, PTR_ERR(dc), "Failed to get gpio
'dc'\n");
if (!strcmp(id->name, "sf-tc240t-9370-t"))
return ili9341_dpi_probe(spi, dc, reset);
--
2.43.0.rc1.1336.g36b5255a03ac
[PATCH v1 3/3] drm/panel: ili9341: Use predefined error codes
In one case the -1 is returned which is quite confusing code for
the wrong device ID, in another the ret is returning instead of
plain 0 that also confusing as readed may ask the possible meaning
of positive codes, which are never the case there. Convert both
to use explicit predefined error codes to make it clear what's going
on there.
Fixes: 5a04227326b0 ("drm/panel: Add ilitek ili9341 panel driver")
Signed-off-by: Andy Shevchenko
---
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 24c74c56e564..b933380b7eb7 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -422,7 +422,7 @@ static int ili9341_dpi_prepare(struct drm_panel *panel)
ili9341_dpi_init(ili);
- return ret;
+ return 0;
}
static int ili9341_dpi_enable(struct drm_panel *panel)
@@ -726,7 +726,7 @@ static int ili9341_probe(struct spi_device *spi)
else if (!strcmp(id->name, "yx240qv29"))
return ili9341_dbi_probe(spi, dc, reset);
- return -1;
+ return -ENODEV;
}
static void ili9341_remove(struct spi_device *spi)
--
2.43.0.rc1.1336.g36b5255a03ac
[PATCH v1 0/3] drm/panel: ili9341: Obvious fixes
A few obvious fixes to the driver. Andy Shevchenko (3): drm/panel: ili9341: Correct use of device property APIs drm/panel: ili9341: Respect deferred probe drm/panel: ili9341: Use predefined error codes drivers/gpu/drm/panel/Kconfig| 2 +- drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 13 +++-- 2 files changed, 8 insertions(+), 7 deletions(-) -- 2.43.0.rc1.1336.g36b5255a03ac
[PATCH v1 1/3] drm/panel: ili9341: Correct use of device property APIs
It seems driver missed the point of proper use of device property APIs.
Correct this by updating headers and calls respectively.
Fixes: 5a04227326b0 ("drm/panel: Add ilitek ili9341 panel driver")
Signed-off-by: Andy Shevchenko
---
drivers/gpu/drm/panel/Kconfig| 2 +-
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index e54f6f5604ed..2d451820 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -177,7 +177,7 @@ config DRM_PANEL_ILITEK_IL9322
config DRM_PANEL_ILITEK_ILI9341
tristate "Ilitek ILI9341 240x320 QVGA panels"
- depends on OF && SPI
+ depends on SPI
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
depends on BACKLIGHT_CLASS_DEVICE
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 3574681891e8..7584ddb0e441 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -22,8 +22,9 @@
#include
#include
#include
+#include
#include
-#include
+#include
#include
#include
@@ -691,7 +692,7 @@ static int ili9341_dpi_probe(struct spi_device *spi, struct
gpio_desc *dc,
* Every new incarnation of this display must have a unique
* data entry for the system in this driver.
*/
- ili->conf = of_device_get_match_data(dev);
+ ili->conf = device_get_match_data(dev);
if (!ili->conf) {
dev_err(dev, "missing device configuration\n");
return -ENODEV;
--
2.43.0.rc1.1336.g36b5255a03ac
Re: [PATCH] drm: ci: fix the xfails for apq8016
On 08/04/2024 14:04, Abhinav Kumar wrote: Hi Helen Gentle reminder on this. If you are okay, we can land it via msm-next tree... Thanks Abhinav On 4/1/2024 1:48 PM, Abhinav Kumar wrote: After IGT migrating to dynamic sub-tests, the pipe prefixes in the expected fails list are incorrect. Lets drop those to accurately match the expected fails. In addition, update the xfails list to match the current passing list. This should have ideally failed in the CI run because some tests were marked as fail even though they passed but due to the mismatch in test names, the matching didn't correctly work and was resulting in those failures not being seen. Here is the passing pipeline for apq8016 with this change: https://gitlab.freedesktop.org/drm/msm/-/jobs/57050562 Signed-off-by: Abhinav Kumar I'm sorry about my delay. Acked-by: Helen Koike I'm also merging it to msm-misc-next. Regards, Helen --- drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt index 44a5c62dedad..b14d4e884971 100644 --- a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt +++ b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt @@ -1,17 +1,6 @@ kms_3d,Fail kms_addfb_basic@addfb25-bad-modifier,Fail -kms_cursor_legacy@all-pipes-forked-bo,Fail -kms_cursor_legacy@all-pipes-forked-move,Fail -kms_cursor_legacy@all-pipes-single-bo,Fail -kms_cursor_legacy@all-pipes-single-move,Fail -kms_cursor_legacy@all-pipes-torture-bo,Fail -kms_cursor_legacy@all-pipes-torture-move,Fail -kms_cursor_legacy@pipe-A-forked-bo,Fail -kms_cursor_legacy@pipe-A-forked-move,Fail -kms_cursor_legacy@pipe-A-single-bo,Fail -kms_cursor_legacy@pipe-A-single-move,Fail -kms_cursor_legacy@pipe-A-torture-bo,Fail -kms_cursor_legacy@pipe-A-torture-move,Fail +kms_cursor_legacy@torture-bo,Fail kms_force_connector_basic@force-edid,Fail kms_hdmi_inject@inject-4k,Fail kms_selftest@drm_format,Timeout
Re: [PATCH v2] software node: Implement device_get_match_data fwnode callback
On Thu, Apr 25, 2024 at 09:42:53PM +0800, Sui Jingfeng wrote: > On 2024/4/25 00:44, Andy Shevchenko wrote: > > On Wed, Apr 24, 2024 at 07:34:54PM +0300, Dmitry Baryshkov wrote: > > > On Wed, Apr 24, 2024 at 05:52:03PM +0300, Andy Shevchenko wrote: > > > > On Wed, Apr 24, 2024 at 04:34:39PM +0300, Dmitry Baryshkov wrote: > > > > > On Wed, 24 Apr 2024 at 16:11, Andy Shevchenko > > > > > wrote: > > > > > > On Wed, Apr 24, 2024 at 12:37:16AM +0300, Dmitry Baryshkov wrote: > > > > > > > On Wed, Apr 24, 2024 at 12:49:18AM +0800, Sui Jingfeng wrote: > > > > > > > > On 2024/4/23 21:28, Andy Shevchenko wrote: > > > > > > > > > On Tue, Apr 23, 2024 at 12:46:58AM +0800, Sui Jingfeng wrote: ... > > > > > > > But let me throw an argument why this patch (or something > > > > > > > similar) looks > > > > > > > to be necessary. > > > > > > > > > > > > > > Both on DT and non-DT systems the kernel allows using the non-OF > > > > > > > based > > > > > > > matching. For the platform devices there is > > > > > > > platform_device_id-based > > > > > > > matching. > > > > > > > > > > > > > > Currently handling the data coming from such device_ids requires > > > > > > > using > > > > > > > special bits of code, e.g. > > > > > > > platform_get_device_id(pdev)->driver_data to > > > > > > > get the data from the platform_device_id. Having such codepaths > > > > > > > goes > > > > > > > against the goal of unifying DT and non-DT paths via generic > > > > > > > property / > > > > > > > fwnode code. > > > > > > > > > > > > > > As such, I support Sui's idea of being able to use > > > > > > > device_get_match_data > > > > > > > for non-DT, non-ACPI platform devices. > > > > > > I'm not sure I buy this. We have a special helpers based on the bus > > > > > > type to > > > > > > combine device_get_match_data() with the respective ID table > > > > > > crawling, see > > > > > > the SPI and I²C cases as the examples. > > > > > I was thinking that we might be able to deprecate these helpers and > > > > > always use device_get_match_data(). > > > > True, but that is orthogonal to swnode match_data support, right? > > > > There even was (still is?) a patch series to do something like a new > > > > member to struct device_driver (? don't remember) to achieve that. > > > Maybe the scenario was not properly described in the commit message, or > > > maybe I missed something. The usecase that I understood from the commit > > > message was to use instatiated i2c / spi devices, which means > > > i2c_device_id / spi_device_id. The commit message should describe why > > > the usecase requires using 'compatible' property and swnode. Ideally it > > > should describe how these devices are instantiated at the first place. > > Yep. I also do not clearly understand the use case and why we need to have > > a board file, because the swnodes all are about board files that we must not > > use for the new platforms. > > Would you like to tell us what's the 'board file'? > > I am asking because I can not understand those two words at all. > I'm really don't know what's the meanings of 'board file'. Hmm... This is very well established term meaning the hard coded platform description (you may consider that as "device tree" written in C inside the Linux kernel). There are plenty of legacy platforms still exist in the Linux kernel source tree, you may find examples, like (first comes to mind) arch/arm/mach-pxa/spitz.c. > Do you means that board file is something like the dts, or > somethings describe the stuff on the motherboard but outside > the CPU? > > Does the hardware IP core belong to the "board file"? > > Can we using more concrete vocabulary instead of the vague > vocabulary to communicate? Most of (I though 100% before this message) the Linux kernel developers _know_ this term, sorry that you maybe young enough :-) -- With Best Regards, Andy Shevchenko
Re: [PATCH v2] software node: Implement device_get_match_data fwnode callback
On 2024/4/25 00:44, Andy Shevchenko wrote: On Wed, Apr 24, 2024 at 07:34:54PM +0300, Dmitry Baryshkov wrote: On Wed, Apr 24, 2024 at 05:52:03PM +0300, Andy Shevchenko wrote: On Wed, Apr 24, 2024 at 04:34:39PM +0300, Dmitry Baryshkov wrote: On Wed, 24 Apr 2024 at 16:11, Andy Shevchenko wrote: On Wed, Apr 24, 2024 at 12:37:16AM +0300, Dmitry Baryshkov wrote: On Wed, Apr 24, 2024 at 12:49:18AM +0800, Sui Jingfeng wrote: On 2024/4/23 21:28, Andy Shevchenko wrote: On Tue, Apr 23, 2024 at 12:46:58AM +0800, Sui Jingfeng wrote: ... But let me throw an argument why this patch (or something similar) looks to be necessary. Both on DT and non-DT systems the kernel allows using the non-OF based matching. For the platform devices there is platform_device_id-based matching. Currently handling the data coming from such device_ids requires using special bits of code, e.g. platform_get_device_id(pdev)->driver_data to get the data from the platform_device_id. Having such codepaths goes against the goal of unifying DT and non-DT paths via generic property / fwnode code. As such, I support Sui's idea of being able to use device_get_match_data for non-DT, non-ACPI platform devices. I'm not sure I buy this. We have a special helpers based on the bus type to combine device_get_match_data() with the respective ID table crawling, see the SPI and I²C cases as the examples. I was thinking that we might be able to deprecate these helpers and always use device_get_match_data(). True, but that is orthogonal to swnode match_data support, right? There even was (still is?) a patch series to do something like a new member to struct device_driver (? don't remember) to achieve that. Maybe the scenario was not properly described in the commit message, or maybe I missed something. The usecase that I understood from the commit message was to use instatiated i2c / spi devices, which means i2c_device_id / spi_device_id. The commit message should describe why the usecase requires using 'compatible' property and swnode. Ideally it should describe how these devices are instantiated at the first place. Yep. I also do not clearly understand the use case and why we need to have a board file, because the swnodes all are about board files that we must not use for the new platforms. Would you like to tell us what's the 'board file'? I am asking because I can not understand those two words at all. I'm really don't know what's the meanings of 'board file'. Do you means that board file is something like the dts, or somethings describe the stuff on the motherboard but outside the CPU? Does the hardware IP core belong to the "board file"? Can we using more concrete vocabulary instead of the vague vocabulary to communicate? -- Best regards, Sui
Re: [RFC PATCH 00/18] TTM interface for managing VRAM oversubscription
Yeah, and this patch set here is removing that functionality. Which is major concern from my side as well. Instead of removing it my long term plan was to move this into TTM ( the recent flags rework is going into that direction), so that both amdgpu and radeon can use the same code again *and* we can also apply it on VM_ALWAYS_VALID BOs. Christian. Am 25.04.24 um 15:22 schrieb Marek Olšák: The most extreme ping-ponging is mitigated by throttling buffer moves in the kernel, but it only works without VM_ALWAYS_VALID and you can set BO priorities in the BO list. A better approach that works with VM_ALWAYS_VALID would be nice. Marek On Wed, Apr 24, 2024 at 1:12 PM Friedrich Vock wrote: Hi everyone, recently I've been looking into remedies for apps (in particular, newer games) that experience significant performance loss when they start to hit VRAM limits, especially on older or lower-end cards that struggle to fit both desktop apps and all the game data into VRAM at once. The root of the problem lies in the fact that from userspace's POV, buffer eviction is very opaque: Userspace applications/drivers cannot tell how oversubscribed VRAM is, nor do they have fine-grained control over which buffers get evicted. At the same time, with GPU APIs becoming increasingly lower-level and GPU-driven, only the application itself can know which buffers are used within a particular submission, and how important each buffer is. For this, GPU APIs include interfaces to query oversubscription and specify memory priorities: In Vulkan, oversubscription can be queried through the VK_EXT_memory_budget extension. Different buffers can also be assigned priorities via the VK_EXT_pageable_device_local_memory extension. Modern games, especially D3D12 games via vkd3d-proton, rely on oversubscription being reported and priorities being respected in order to perform their memory management. However, relaying this information to the kernel via the current KMD uAPIs is not possible. On AMDGPU for example, all work submissions include a "bo list" that contains any buffer object that is accessed during the course of the submission. If VRAM is oversubscribed and a buffer in the list was evicted to system memory, that buffer is moved back to VRAM (potentially evicting other unused buffers). Since the usermode driver doesn't know what buffers are used by the application, its only choice is to submit a bo list that contains every buffer the application has allocated. In case of VRAM oversubscription, it is highly likely that some of the application's buffers were evicted, which almost guarantees that some buffers will get moved around. Since the bo list is only known at submit time, this also means the buffers will get moved right before submitting application work, which is the worst possible time to move buffers from a latency perspective. Another consequence of the large bo list is that nearly all memory from other applications will be evicted, too. When different applications (e.g. game and compositor) submit work one after the other, this causes a ping-pong effect where each app's submission evicts the other app's memory, resulting in a large amount of unnecessary moves. This overly aggressive eviction behavior led to RADV adopting a change that effectively allows all VRAM applications to reside in system memory [1]. This worked around the ping-ponging/excessive buffer moving problem, but also meant that any memory evicted to system memory would forever stay there, regardless of how VRAM is used. My proposal aims at providing a middle ground between these extremes. The goals I want to meet are: - Userspace is accurately informed about VRAM oversubscription/how much VRAM has been evicted - Buffer eviction respects priorities set by userspace - Wasteful ping-ponging is avoided to the extent possible I have been testing out some prototypes, and came up with this rough sketch of an API: - For each ttm_resource_manager, the amount of evicted memory is tracked (similarly to how "usage" tracks the memory usage). When memory is evicted via ttm_bo_evict, the size of the evicted memory is added, when memory is un-evicted (see below), its size is subtracted. The amount of evicted memory for e.g. VRAM can be queried by userspace via an ioctl. - Each ttm_resource_manager maintains a list of evicted buffer objects. - ttm_mem_unevict walks the list of evicted bos for a given ttm_resource_manager and tries moving evicted resources back. When a buffer is freed, this function is called to immediately restore some evicted memory. - Each ttm_buffer_object independently tracks the mem_type it wants to reside in. - ttm_bo_try_unevict is added as a helper function which attempts to move the buffer to its preferred mem_type. If no space is available there, it fails with -ENOSPC/-ENOMEM. - Similar to how ttm_bo_evict works, each driver can implement uneviction_valuable/unevict_flags callbacks to
Re: [RFC PATCH 00/18] TTM interface for managing VRAM oversubscription
The most extreme ping-ponging is mitigated by throttling buffer moves in the kernel, but it only works without VM_ALWAYS_VALID and you can set BO priorities in the BO list. A better approach that works with VM_ALWAYS_VALID would be nice. Marek On Wed, Apr 24, 2024 at 1:12 PM Friedrich Vock wrote: > > Hi everyone, > > recently I've been looking into remedies for apps (in particular, newer > games) that experience significant performance loss when they start to > hit VRAM limits, especially on older or lower-end cards that struggle > to fit both desktop apps and all the game data into VRAM at once. > > The root of the problem lies in the fact that from userspace's POV, > buffer eviction is very opaque: Userspace applications/drivers cannot > tell how oversubscribed VRAM is, nor do they have fine-grained control > over which buffers get evicted. At the same time, with GPU APIs becoming > increasingly lower-level and GPU-driven, only the application itself > can know which buffers are used within a particular submission, and > how important each buffer is. For this, GPU APIs include interfaces > to query oversubscription and specify memory priorities: In Vulkan, > oversubscription can be queried through the VK_EXT_memory_budget > extension. Different buffers can also be assigned priorities via the > VK_EXT_pageable_device_local_memory extension. Modern games, especially > D3D12 games via vkd3d-proton, rely on oversubscription being reported and > priorities being respected in order to perform their memory management. > > However, relaying this information to the kernel via the current KMD uAPIs > is not possible. On AMDGPU for example, all work submissions include a > "bo list" that contains any buffer object that is accessed during the > course of the submission. If VRAM is oversubscribed and a buffer in the > list was evicted to system memory, that buffer is moved back to VRAM > (potentially evicting other unused buffers). > > Since the usermode driver doesn't know what buffers are used by the > application, its only choice is to submit a bo list that contains every > buffer the application has allocated. In case of VRAM oversubscription, > it is highly likely that some of the application's buffers were evicted, > which almost guarantees that some buffers will get moved around. Since > the bo list is only known at submit time, this also means the buffers > will get moved right before submitting application work, which is the > worst possible time to move buffers from a latency perspective. Another > consequence of the large bo list is that nearly all memory from other > applications will be evicted, too. When different applications (e.g. game > and compositor) submit work one after the other, this causes a ping-pong > effect where each app's submission evicts the other app's memory, > resulting in a large amount of unnecessary moves. > > This overly aggressive eviction behavior led to RADV adopting a change > that effectively allows all VRAM applications to reside in system memory > [1]. This worked around the ping-ponging/excessive buffer moving problem, > but also meant that any memory evicted to system memory would forever > stay there, regardless of how VRAM is used. > > My proposal aims at providing a middle ground between these extremes. > The goals I want to meet are: > - Userspace is accurately informed about VRAM oversubscription/how much > VRAM has been evicted > - Buffer eviction respects priorities set by userspace - Wasteful > ping-ponging is avoided to the extent possible > > I have been testing out some prototypes, and came up with this rough > sketch of an API: > > - For each ttm_resource_manager, the amount of evicted memory is tracked > (similarly to how "usage" tracks the memory usage). When memory is > evicted via ttm_bo_evict, the size of the evicted memory is added, when > memory is un-evicted (see below), its size is subtracted. The amount of > evicted memory for e.g. VRAM can be queried by userspace via an ioctl. > > - Each ttm_resource_manager maintains a list of evicted buffer objects. > > - ttm_mem_unevict walks the list of evicted bos for a given > ttm_resource_manager and tries moving evicted resources back. When a > buffer is freed, this function is called to immediately restore some > evicted memory. > > - Each ttm_buffer_object independently tracks the mem_type it wants > to reside in. > > - ttm_bo_try_unevict is added as a helper function which attempts to > move the buffer to its preferred mem_type. If no space is available > there, it fails with -ENOSPC/-ENOMEM. > > - Similar to how ttm_bo_evict works, each driver can implement > uneviction_valuable/unevict_flags callbacks to control buffer > un-eviction. > > This is what patches 1-10 accomplish (together with an amdgpu > implementation utilizing the new API). > > Userspace priorities could then be implemented as follows: > > - TTM already manages priorities for each buffer object. These
[PATCH v1 1/1] drm/mipi-dbi: Add missing MODULE_DESCRIPTION()
The modpost script is not happy
WARNING: modpost: missing MODULE_DESCRIPTION() in
drivers/gpu/drm/drm_mipi_dbi.o
because there is a missing module description.
Add it to the module.
Signed-off-by: Andy Shevchenko
---
drivers/gpu/drm/drm_mipi_dbi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index daac649aabdb..ee6fa8185b13 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -1475,4 +1475,5 @@ EXPORT_SYMBOL(mipi_dbi_debugfs_init);
#endif
+MODULE_DESCRIPTION("MIPI Display Bus Interface (DBI) LCD controller support");
MODULE_LICENSE("GPL");
--
2.43.0.rc1.1336.g36b5255a03ac
Re: [PATCH v3 1/4] drm: add devm release action
On Wed, Apr 24, 2024 at 08:20:32AM -0400, Rodrigo Vivi wrote: > On Wed, Apr 24, 2024 at 01:49:16PM +0200, Maxime Ripard wrote: > > On Tue, Apr 23, 2024 at 01:42:22PM -0400, Rodrigo Vivi wrote: > > > On Tue, Apr 23, 2024 at 02:25:06PM +0530, Aravind Iddamsetty wrote: > > > > > > > > On 23/04/24 02:24, Rodrigo Vivi wrote: > > > > > On Mon, Apr 22, 2024 at 12:27:53PM +0530, Aravind Iddamsetty wrote: > > > > >> In scenarios where drm_dev_put is directly called by driver we want > > > > >> to > > > > >> release devm_drm_dev_init_release action associated with struct > > > > >> drm_device. > > > > >> > > > > >> v2: Directly expose the original function, instead of introducing a > > > > >> helper (Rodrigo) > > > > >> > > > > >> v3: add kernel-doc (Maxime Ripard) > > > > >> > > > > >> Cc: Maxime Ripard > > > > >> Cc: Thomas Hellstr_m > > > > >> Cc: Rodrigo Vivi > > > > >> > > > > > please avoid these empty lines here cc, rv-b, sign-offs, links, > > > > > etc are all in the same block. > > > > ok. > > > > > > > > > >> Reviewed-by: Rodrigo Vivi > > > > >> Signed-off-by: Aravind Iddamsetty > > > > >> > > > > >> --- > > > > >> drivers/gpu/drm/drm_drv.c | 13 + > > > > >> include/drm/drm_drv.h | 2 ++ > > > > >> 2 files changed, 15 insertions(+) > > > > >> > > > > >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > > > >> index 243cacb3575c..9d0409165f1e 100644 > > > > >> --- a/drivers/gpu/drm/drm_drv.c > > > > >> +++ b/drivers/gpu/drm/drm_drv.c > > > > >> @@ -714,6 +714,19 @@ static int devm_drm_dev_init(struct device > > > > >> *parent, > > > > >> devm_drm_dev_init_release, dev); > > > > >> } > > > > >> > > > > >> +/** > > > > >> + * devm_drm_dev_release_action - Call the final release action of > > > > >> the device > > > > > Seeing the doc here gave me a second thought > > > > > > > > > > the original release should be renamed to _devm_drm_dev_release > > > > > and this should be called devm_drm_dev_release without the 'action' > > > > > word. > > > > i believe, was suggested earlier to directly expose the main function, > > > > is > > > > there any reason to have a __ version ? > > > > > > No no, just ignore me. Just remove the '_action' and don't change the > > > other. > > > > > > I don't like exposing the a function with '__'. what would '__' that mean? > > > This is what I meant on the first comment. > > > > > > Now, I believe that we don't need the '_action'. What does the 'action' > > > mean? > > > > > > the devm_drm_dev_release should be enough. But then I got confused and > > > I thought it would conflict with the original released function name. > > > But I misread it. > > > > I don't think devm_drm_dev_release is a good name either. Just like any > > other devm_* function that cancels what a previous one has been doing > > (devm_kfree, devm_backlight_device_unregister, devm_nvmem_device_put, > > etc.) it should be called devm_drm_dev_put or something similar. > > I see what you mean, but I don't believe the 'put' is the best option, > for 2 reasons: > - in general, we have put paired with gets and this has not get equivalent Yeah, that's true. _release is fine then I guess. > - this bypass the regular get/put mechanism and forces the releases that > would be done only after all drm_dev_put() taking ref to zero. I don't think it does? devm_release_action will only remove the devm action and execute it directly, but this action here is a call to drm_dev_put, so we might still have other references taken that would defer the device being freed. Maxime signature.asc Description: PGP signature
[PATCH v1 1/1] drm/ili9341: Remove the duplicative driver
First of all, the driver was introduced when it was already
two drivers available for Ilitek 9341 panels.
Second, the most recent (fourth!) driver has incorporated this one
and hence, when enabled, it covers the provided functionality.
Taking into account the above, remove duplicative driver and make
maintenance and support eaiser for everybody.
Also see discussion [1] for details about Ilitek 9341 duplication
code.
Link: https://lore.kernel.org/r/zxm9pg-53v4s8...@smile.fi.intel.com [1]
Signed-off-by: Andy Shevchenko
---
drivers/gpu/drm/tiny/Kconfig | 13 --
drivers/gpu/drm/tiny/Makefile | 1 -
drivers/gpu/drm/tiny/ili9341.c | 253 -
3 files changed, 267 deletions(-)
delete mode 100644 drivers/gpu/drm/tiny/ili9341.c
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index f6889f649bc1..2ab07bd0bb44 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -134,19 +134,6 @@ config TINYDRM_ILI9225
If M is selected the module will be called ili9225.
-config TINYDRM_ILI9341
- tristate "DRM support for ILI9341 display panels"
- depends on DRM && SPI
- select DRM_KMS_HELPER
- select DRM_GEM_DMA_HELPER
- select DRM_MIPI_DBI
- select BACKLIGHT_CLASS_DEVICE
- help
- DRM driver for the following Ilitek ILI9341 panels:
- * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
-
- If M is selected the module will be called ili9341.
-
config TINYDRM_ILI9486
tristate "DRM support for ILI9486 display panels"
depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 76dde89a044b..37cc9b27e79d 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -10,7 +10,6 @@ obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm.o
obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o
obj-$(CONFIG_TINYDRM_ILI9163) += ili9163.o
obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o
-obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o
obj-$(CONFIG_TINYDRM_ILI9486) += ili9486.o
obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o
obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o
diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
deleted file mode 100644
index 47b61c3bf145..
--- a/drivers/gpu/drm/tiny/ili9341.c
+++ /dev/null
@@ -1,253 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * DRM driver for Ilitek ILI9341 panels
- *
- * Copyright 2018 David Lechner
- *
- * Based on mi0283qt.c:
- * Copyright 2016 Noralf Trønnes
- */
-
-#include
-#include
-#include
-#include
-#include
-#include
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-#define ILI9341_FRMCTR10xb1
-#define ILI9341_DISCTRL0xb6
-#define ILI9341_ETMOD 0xb7
-
-#define ILI9341_PWCTRL10xc0
-#define ILI9341_PWCTRL20xc1
-#define ILI9341_VMCTRL10xc5
-#define ILI9341_VMCTRL20xc7
-#define ILI9341_PWCTRLA0xcb
-#define ILI9341_PWCTRLB0xcf
-
-#define ILI9341_PGAMCTRL 0xe0
-#define ILI9341_NGAMCTRL 0xe1
-#define ILI9341_DTCTRLA0xe8
-#define ILI9341_DTCTRLB0xea
-#define ILI9341_PWRSEQ 0xed
-
-#define ILI9341_EN3GAM 0xf2
-#define ILI9341_PUMPCTRL 0xf7
-
-#define ILI9341_MADCTL_BGR BIT(3)
-#define ILI9341_MADCTL_MV BIT(5)
-#define ILI9341_MADCTL_MX BIT(6)
-#define ILI9341_MADCTL_MY BIT(7)
-
-static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
-struct drm_crtc_state *crtc_state,
-struct drm_plane_state *plane_state)
-{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
- struct mipi_dbi *dbi = >dbi;
- u8 addr_mode;
- int ret, idx;
-
- if (!drm_dev_enter(pipe->crtc.dev, ))
- return;
-
- DRM_DEBUG_KMS("\n");
-
- ret = mipi_dbi_poweron_conditional_reset(dbidev);
- if (ret < 0)
- goto out_exit;
- if (ret == 1)
- goto out_enable;
-
- mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
-
- mipi_dbi_command(dbi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
- mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
- mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
- mipi_dbi_command(dbi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
- mipi_dbi_command(dbi, ILI9341_PUMPCTRL, 0x20);
- mipi_dbi_command(dbi, ILI9341_DTCTRLB, 0x00, 0x00);
-
- /* Power Control */
- mipi_dbi_command(dbi, ILI9341_PWCTRL1, 0x23);
- mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x10);
- /* VCOM */
- mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x3e, 0x28);
- mipi_dbi_command(dbi, ILI9341_VMCTRL2,
Re: [PATCH 2/3] drm/panthor: Make sure the tiler initial/max chunks are consistent
On Thu, 25 Apr 2024 11:43:39 +0100
Steven Price wrote:
> On 25/04/2024 10:28, Steven Price wrote:
> > On 25/04/2024 08:18, Boris Brezillon wrote:
> >> It doesn't make sense to have a maximum number of chunks smaller than
> >> the initial number of chunks attached to the context.
> >>
> >> Fix the uAPI header to reflect the new constraint, and mention the
> >> undocumented "initial_chunk_count > 0" constraint while at it.
> >>
> >> Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block")
> >> Signed-off-by: Boris Brezillon
> >
> > Reviewed-by: Steven Price
>
> Ok, I'll take that back... I've rebased (and fixed up all the out of
> tree patches) and this doesn't work when I actually test it!
>
> >
> >> ---
> >> drivers/gpu/drm/panthor/panthor_heap.c | 3 +++
> >> include/uapi/drm/panthor_drm.h | 8 ++--
> >> 2 files changed, 9 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c
> >> b/drivers/gpu/drm/panthor/panthor_heap.c
> >> index 143fa35f2e74..8728c9bb76e4 100644
> >> --- a/drivers/gpu/drm/panthor/panthor_heap.c
> >> +++ b/drivers/gpu/drm/panthor/panthor_heap.c
> >> @@ -281,6 +281,9 @@ int panthor_heap_create(struct panthor_heap_pool *pool,
> >>if (initial_chunk_count == 0)
> >>return -EINVAL;
> >>
> >> + if (initial_chunk_count < max_chunks)
>
> This should be initial_chunk_count > max_chunks. Otherwise you're
> requiring the initial chunk count to be equal *or greater* than the max
> chunks which makes no sense!
Damn it, here's what happens when you think your changes are too
trivial to be wrong...
But I swear I would have tested the whole thing before pushing to
drm-misc. :P
Re: [PATCH v8 6/6] drm/{i915,xe}: Implement fbdev emulation as in-kernel client
On Tue, 23 Apr 2024, Thomas Zimmermann wrote: > Hi > > Am 23.04.24 um 13:36 schrieb Hogander, Jouni: >> On Tue, 2024-04-23 at 13:13 +0200, Thomas Zimmermann wrote: >>> Hi >>> >>> Am 22.04.24 um 16:11 schrieb Hogander, Jouni: On Tue, 2024-04-09 at 10:04 +0200, Thomas Zimmermann wrote: > Replace all code that initializes or releases fbdev emulation > throughout the driver. Instead initialize the fbdev client by a > single call to intel_fbdev_setup() after i915 has registered its > DRM device. Just like similar code in other drivers, i915 fbdev > emulation now acts like a regular DRM client. Do the same for xe. > > The fbdev client setup consists of the initial preparation and > the > hot-plugging of the display. The latter creates the fbdev device > and sets up the fbdev framebuffer. The setup performs display > hot-plugging once. If no display can be detected, DRM probe > helpers > re-run the detection on each hotplug event. > > A call to drm_client_dev_unregister() releases all in-kernel > clients > automatically. No further action is required within i915. If the > fbdev > framebuffer has been fully set up, struct fb_ops.fb_destroy > implements > the release. For partially initialized emulation, the fbdev > client > reverts the initial setup. Do the same for xe and remove its call > to > intel_fbdev_fini(). > > v8: > - setup client in intel_display_driver_register (Jouni) > - mention xe in commit message > > v7: > - update xe driver > - reword commit message > > v6: > - use 'i915' for i915 device (Jouni) > - remove unnecessary code for non-atomic mode setting (Jouni, > Ville) > - fix function name in commit message (Jouni) > > v3: > - as before, silently ignore devices without displays > > v2: > - let drm_client_register() handle initial hotplug > - fix driver name in error message (Jani) > - fix non-fbdev build (kernel test robot) > > Signed-off-by: Thomas Zimmermann Reviewed-by: Jouni Högander >>> Thank you so much. All patches has R-bs. Can you add the series to >>> the >>> intel tree? >> Is it ok to merge patch 01/06 via intel tree as well? > > Sure, np. Pushed the series to drm-intel-next, thanks for the patches and review. And the patience in waiting for us to getting this merged! BR, Jani. > > Best regards > Thomas > >> >> Rodrigo, This set is containing Xe display changes as well. Is it ok to >> push this via drm-intel? >> >> BR, >> >> Jouni Högander >> >>> Best regards >>> Thomas >>> > --- > drivers/gpu/drm/i915/display/intel_display.c | 1 - > .../drm/i915/display/intel_display_driver.c | 20 +- > drivers/gpu/drm/i915/display/intel_fbdev.c | 177 --- > - > -- > drivers/gpu/drm/i915/display/intel_fbdev.h | 20 +- > drivers/gpu/drm/xe/display/xe_display.c | 2 - > 5 files changed, 80 insertions(+), 140 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 614e60420a29a..161a5aabf6746 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -85,7 +85,6 @@ > #include "intel_dvo.h" > #include "intel_fb.h" > #include "intel_fbc.h" > -#include "intel_fbdev.h" > #include "intel_fdi.h" > #include "intel_fifo_underrun.h" > #include "intel_frontbuffer.h" > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c > b/drivers/gpu/drm/i915/display/intel_display_driver.c > index e5f052d4ff1cc..ed8589fa35448 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c > @@ -514,10 +514,6 @@ int intel_display_driver_probe(struct > drm_i915_private *i915) > > intel_overlay_setup(i915); > > - ret = intel_fbdev_init(>drm); > - if (ret) > - return ret; > - > /* Only enable hotplug handling once the fbdev is fully > set > up. */ > intel_hpd_init(i915); > > @@ -544,16 +540,6 @@ void intel_display_driver_register(struct > drm_i915_private *i915) > > intel_display_debugfs_register(i915); > > - /* > - * Some ports require correctly set-up hpd registers for > - * detection to work properly (leading to ghost connected > - * connector status), e.g. VGA on gm45. Hence we can > only > set > - * up the initial fbdev config after hpd irqs are fully > - * enabled. We do it last so that the async config cannot > run > - * before the connectors are registered. > - */ > -
Re: [PATCH V2] drm/bridge: imx: Fix unmet depenency for PHY_FSL_SAMSUNG_HDMI_PHY
On Mon, 22 Apr 2024 05:33:52 -0500, Adam Ford wrote: > When enabling i.MX8MP DWC HDMI driver, it automatically selects > PHY_FSL_SAMSUNG_HDMI_PHY, since it wont' work without the phy. > This may cause some Kconfig warnings during various build tests. > Fix this by implying the phy instead of selecting the phy. > > To prevent this from happening with the DRM_IMX8MP_HDMI_PVI, also > imply it instead of selecting it. > > [...] Applied, thanks! [1/1] drm/bridge: imx: Fix unmet depenency for PHY_FSL_SAMSUNG_HDMI_PHY https://cgit.freedesktop.org/drm/drm-misc/commit/?id=cbdbd9ca718e Rob
Re: [PATCH 1/2] drm/print: drop include debugfs.h and include where needed
On Mon, Apr 22, 2024 at 2:10 PM Jani Nikula wrote: > > Surprisingly many places depend on debugfs.h to be included via > drm_print.h. Fix them. > > v3: Also fix armada, ite-it6505, imagination, msm, sti, vc4, and xe > > v2: Also fix ivpu and vmwgfx > > Reviewed-by: Andrzej Hajda > Acked-by: Maxime Ripard > Link: > https://patchwork.freedesktop.org/patch/msgid/20240410141434.157908-1-jani.nik...@intel.com > Signed-off-by: Jani Nikula > > --- > > Cc: Jacek Lawrynowicz > Cc: Stanislaw Gruszka > Cc: Oded Gabbay > Cc: Russell King > Cc: David Airlie > Cc: Daniel Vetter > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: Jani Nikula > Cc: Rodrigo Vivi > Cc: Joonas Lahtinen > Cc: Tvrtko Ursulin > Cc: Frank Binns > Cc: Matt Coster > Cc: Rob Clark > Cc: Abhinav Kumar > Cc: Dmitry Baryshkov > Cc: Sean Paul > Cc: Marijn Suijten > Cc: Karol Herbst > Cc: Lyude Paul > Cc: Danilo Krummrich > Cc: Alex Deucher > Cc: "Christian König" > Cc: "Pan, Xinhui" > Cc: Alain Volmat > Cc: Huang Rui > Cc: Zack Rusin > Cc: Broadcom internal kernel review list > > Cc: Lucas De Marchi > Cc: "Thomas Hellström" > Cc: dri-devel@lists.freedesktop.org > Cc: intel-...@lists.freedesktop.org > Cc: intel...@lists.freedesktop.org > Cc: linux-arm-...@vger.kernel.org > Cc: freedr...@lists.freedesktop.org > Cc: nouv...@lists.freedesktop.org > Cc: amd-...@lists.freedesktop.org > --- > drivers/accel/ivpu/ivpu_debugfs.c | 2 ++ > drivers/gpu/drm/armada/armada_debugfs.c | 1 + > drivers/gpu/drm/bridge/ite-it6505.c | 1 + > drivers/gpu/drm/bridge/panel.c | 2 ++ > drivers/gpu/drm/drm_print.c | 6 +++--- > drivers/gpu/drm/i915/display/intel_dmc.c| 1 + > drivers/gpu/drm/imagination/pvr_fw_trace.c | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 2 ++ > drivers/gpu/drm/nouveau/dispnv50/crc.c | 2 ++ > drivers/gpu/drm/radeon/r100.c | 1 + > drivers/gpu/drm/radeon/r300.c | 1 + > drivers/gpu/drm/radeon/r420.c | 1 + > drivers/gpu/drm/radeon/r600.c | 3 ++- > drivers/gpu/drm/radeon/radeon_fence.c | 1 + > drivers/gpu/drm/radeon/radeon_gem.c | 1 + > drivers/gpu/drm/radeon/radeon_ib.c | 2 ++ > drivers/gpu/drm/radeon/radeon_pm.c | 1 + > drivers/gpu/drm/radeon/radeon_ring.c| 2 ++ > drivers/gpu/drm/radeon/radeon_ttm.c | 1 + > drivers/gpu/drm/radeon/rs400.c | 1 + > drivers/gpu/drm/radeon/rv515.c | 1 + > drivers/gpu/drm/sti/sti_drv.c | 1 + > drivers/gpu/drm/ttm/ttm_device.c| 1 + > drivers/gpu/drm/ttm/ttm_resource.c | 3 ++- > drivers/gpu/drm/ttm/ttm_tt.c| 5 +++-- > drivers/gpu/drm/vc4/vc4_drv.h | 1 + > drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 2 ++ > drivers/gpu/drm/xe/xe_debugfs.c | 1 + > drivers/gpu/drm/xe/xe_gt_debugfs.c | 2 ++ > drivers/gpu/drm/xe/xe_uc_debugfs.c | 2 ++ > include/drm/drm_print.h | 2 +- > 31 files changed, 46 insertions(+), 8 deletions(-) > > diff --git a/drivers/accel/ivpu/ivpu_debugfs.c > b/drivers/accel/ivpu/ivpu_debugfs.c > index d09d29775b3f..e07e447d08d1 100644 > --- a/drivers/accel/ivpu/ivpu_debugfs.c > +++ b/drivers/accel/ivpu/ivpu_debugfs.c > @@ -3,6 +3,8 @@ > * Copyright (C) 2020-2023 Intel Corporation > */ > > +#include > + > #include > #include > #include > diff --git a/drivers/gpu/drm/armada/armada_debugfs.c > b/drivers/gpu/drm/armada/armada_debugfs.c > index 29f4b52e3c8d..a763349dd89f 100644 > --- a/drivers/gpu/drm/armada/armada_debugfs.c > +++ b/drivers/gpu/drm/armada/armada_debugfs.c > @@ -5,6 +5,7 @@ > */ > > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c > b/drivers/gpu/drm/bridge/ite-it6505.c > index 27334173e911..3f68c82888c2 100644 > --- a/drivers/gpu/drm/bridge/ite-it6505.c > +++ b/drivers/gpu/drm/bridge/ite-it6505.c > @@ -3,6 +3,7 @@ > * Copyright (c) 2020, The Linux Foundation. All rights reserved. > */ > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index 7f41525f7a6e..32506524d9a2 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -4,6 +4,8 @@ > * Copyright (C) 2017 Broadcom > */ > > +#include > + > #include > #include > #include > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c > index 699b7dbffd7b..cf2efb44722c 100644 > --- a/drivers/gpu/drm/drm_print.c > +++ b/drivers/gpu/drm/drm_print.c > @@ -23,13 +23,13 @@ > * Rob Clark > */ > > -#include > - > +#include > +#include > #include > #include > #include > #include > -#include > +#include
Re: [PATCH] drm/etnaviv: Create an accel device node if compute-only
Hi Tomeu,
>
> If we expose a render node for NPUs without rendering capabilities, the
> userspace stack will offer it to compositors and applications for
> rendering, which of course won't work.
>
> Userspace is probably right in not questioning whether a render node
> might not be capable of supporting rendering, so change it in the kernel
> instead by exposing a /dev/accel node.
>
> Before we bring the device up we don't know whether it is capable of
> rendering or not (depends on the features of its blocks), so first try
> to probe a rendering node, and if we find out that there is no rendering
> hardware, abort and retry with an accel node.
>
I really love this idea of moving away from a render node. What needs to be done
on the userspace side?
> Signed-off-by: Tomeu Vizoso
> Cc: Oded Gabbay
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 46 ++-
> 1 file changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6500f3999c5f..8e7dd23115f4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -11,6 +11,7 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> @@ -488,10 +489,10 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
> ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> };
>
> -DEFINE_DRM_GEM_FOPS(fops);
> +DEFINE_DRM_GEM_FOPS(render_fops);
> +DEFINE_DRM_ACCEL_FOPS(accel_fops);
>
> -static const struct drm_driver etnaviv_drm_driver = {
> - .driver_features= DRIVER_GEM | DRIVER_RENDER,
> +static struct drm_driver etnaviv_drm_driver = {
> .open = etnaviv_open,
> .postclose = etnaviv_postclose,
> .gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> @@ -500,7 +501,6 @@ static const struct drm_driver etnaviv_drm_driver = {
> #endif
> .ioctls = etnaviv_ioctls,
> .num_ioctls = DRM_ETNAVIV_NUM_IOCTLS,
> - .fops = ,
> .name = "etnaviv",
> .desc = "etnaviv DRM",
> .date = "20151214",
> @@ -508,15 +508,20 @@ static const struct drm_driver etnaviv_drm_driver = {
> .minor = 4,
> };
>
> -/*
> - * Platform driver:
> - */
> -static int etnaviv_bind(struct device *dev)
> +static int etnaviv_bind_with_type(struct device *dev, u32 type)
> {
> struct etnaviv_drm_private *priv;
> struct drm_device *drm;
> + bool is_compute_only = true;
> int ret;
>
> + etnaviv_drm_driver.driver_features = DRIVER_GEM | type;
> +
> + if (type == DRIVER_RENDER)
> + etnaviv_drm_driver.fops = _fops;
> + else
> + etnaviv_drm_driver.fops = _fops;
> +
> drm = drm_dev_alloc(_drm_driver, dev);
> if (IS_ERR(drm))
> return PTR_ERR(drm);
> @@ -553,6 +558,18 @@ static int etnaviv_bind(struct device *dev)
>
> load_gpu(drm);
>
> + for (unsigned int i = 0; i < ETNA_MAX_PIPES; i++) {
> + struct etnaviv_gpu *g = priv->gpu[i];
> +
> + if (g && (g->identity.minor_features8 &
> chipMinorFeatures8_COMPUTE_ONLY) == 0)
> + is_compute_only = false;
> + }
> +
> + if (type == DRIVER_RENDER && is_compute_only) {
> + ret = -EINVAL;
> + goto out_unbind;
> + }
> +
> ret = drm_dev_register(drm, 0);
> if (ret)
> goto out_unbind;
> @@ -571,6 +588,19 @@ static int etnaviv_bind(struct device *dev)
> return ret;
> }
>
> +/*
> + * Platform driver:
> + */
> +static int etnaviv_bind(struct device *dev)
> +{
> + int ret = etnaviv_bind_with_type(dev, DRIVER_RENDER);
> +
> + if (ret == -EINVAL)
> + return etnaviv_bind_with_type(dev, DRIVER_COMPUTE_ACCEL);
> +
> + return ret;
> +}
> +
> static void etnaviv_unbind(struct device *dev)
> {
> struct drm_device *drm = dev_get_drvdata(dev);
> --
> 2.44.0
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
[PATCH v2] drm/amd/display: re-indent dc_power_down_on_boot()
These lines are indented too far. Clean the whitespace.
Signed-off-by: Dan Carpenter
---
v2: Delete another blank line (checkpatch.pl --strict).
drivers/gpu/drm/amd/display/dc/core/dc.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 3e16041bf4f9..5a0835f884a8 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -5192,11 +5192,9 @@ void dc_interrupt_ack(struct dc *dc, enum dc_irq_source
src)
void dc_power_down_on_boot(struct dc *dc)
{
if (dc->ctx->dce_environment != DCE_ENV_VIRTUAL_HW &&
- dc->hwss.power_down_on_boot) {
-
- if (dc->caps.ips_support)
- dc_exit_ips_for_hw_access(dc);
-
+ dc->hwss.power_down_on_boot) {
+ if (dc->caps.ips_support)
+ dc_exit_ips_for_hw_access(dc);
dc->hwss.power_down_on_boot(dc);
}
}
--
2.43.0
Re: [PATCH] drm/panthor: Kill the faulty_slots variable in panthor_sched_suspend()
On 25/04/2024 11:39, Boris Brezillon wrote:
> We can use upd_ctx.timedout_mask directly, and the faulty_slots update
> in the flush_caches_failed situation is never used.
>
> Suggested-by: Suggested-by: Steven Price
I'm obviously too full of suggestions! ;)
And you're doing a much better job of my todo list than I am!
> Signed-off-by: Boris Brezillon
Reviewed-by: Steven Price
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
> b/drivers/gpu/drm/panthor/panthor_sched.c
> index fad4678ca4c8..fed28c16d5d1 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -2584,8 +2584,8 @@ void panthor_sched_suspend(struct panthor_device *ptdev)
> {
> struct panthor_scheduler *sched = ptdev->scheduler;
> struct panthor_csg_slots_upd_ctx upd_ctx;
> - u32 suspended_slots, faulty_slots;
> struct panthor_group *group;
> + u32 suspended_slots;
> u32 i;
>
> mutex_lock(>lock);
> @@ -2605,10 +2605,9 @@ void panthor_sched_suspend(struct panthor_device
> *ptdev)
>
> csgs_upd_ctx_apply_locked(ptdev, _ctx);
> suspended_slots &= ~upd_ctx.timedout_mask;
> - faulty_slots = upd_ctx.timedout_mask;
>
> - if (faulty_slots) {
> - u32 slot_mask = faulty_slots;
> + if (upd_ctx.timedout_mask) {
> + u32 slot_mask = upd_ctx.timedout_mask;
>
> drm_err(>base, "CSG suspend failed, escalating to
> termination");
> csgs_upd_ctx_init(_ctx);
> @@ -2659,9 +2658,6 @@ void panthor_sched_suspend(struct panthor_device *ptdev)
>
> slot_mask &= ~BIT(csg_id);
> }
> -
> - if (flush_caches_failed)
> - faulty_slots |= suspended_slots;
> }
>
> for (i = 0; i < sched->csg_slot_count; i++) {
Re: [PATCH V2] drm/bridge: imx: Fix unmet depenency for PHY_FSL_SAMSUNG_HDMI_PHY
Hi Adam,
Thank you for the patch.
On Mon, Apr 22, 2024 at 05:33:52AM -0500, Adam Ford wrote:
> When enabling i.MX8MP DWC HDMI driver, it automatically selects
> PHY_FSL_SAMSUNG_HDMI_PHY, since it wont' work without the phy.
> This may cause some Kconfig warnings during various build tests.
> Fix this by implying the phy instead of selecting the phy.
>
> To prevent this from happening with the DRM_IMX8MP_HDMI_PVI, also
> imply it instead of selecting it.
>
> Fixes: 1f36d634670d ("drm/bridge: imx: add bridge wrapper driver for i.MX8MP
> DWC HDMI")
> Reported-by: kernel test robot
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202404190103.llm8ltup-...@intel.com/
> Signed-off-by: Adam Ford
This looks sensible to me.
Reviewed-by: Laurent Pinchart
> ---
> V2: Change DRM_IMX8MP_HDMI_PVI at the same time since it was affected
> from the same commit.
>
> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
> b/drivers/gpu/drm/bridge/imx/Kconfig
> index 7687ed652df5..13142a6b8590 100644
> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> @@ -8,8 +8,8 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
> depends on COMMON_CLK
> depends on DRM_DW_HDMI
> depends on OF
> - select DRM_IMX8MP_HDMI_PVI
> - select PHY_FSL_SAMSUNG_HDMI_PHY
> + imply DRM_IMX8MP_HDMI_PVI
> + imply PHY_FSL_SAMSUNG_HDMI_PHY
> help
> Choose this to enable support for the internal HDMI encoder found
> on the i.MX8MP SoC.
--
Regards,
Laurent Pinchart
Re: [PATCH] drm/panthor: Kill the faulty_slots variable in panthor_sched_suspend()
On Thu, 2024-04-25 at 12:39 +0200, Boris Brezillon wrote:
> We can use upd_ctx.timedout_mask directly, and the faulty_slots
> update
> in the flush_caches_failed situation is never used.
>
> Suggested-by: Suggested-by: Steven Price
Whoops? :)
> Signed-off-by: Boris Brezillon
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
> b/drivers/gpu/drm/panthor/panthor_sched.c
> index fad4678ca4c8..fed28c16d5d1 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -2584,8 +2584,8 @@ void panthor_sched_suspend(struct
> panthor_device *ptdev)
> {
> struct panthor_scheduler *sched = ptdev->scheduler;
> struct panthor_csg_slots_upd_ctx upd_ctx;
> - u32 suspended_slots, faulty_slots;
> struct panthor_group *group;
> + u32 suspended_slots;
> u32 i;
>
> mutex_lock(>lock);
> @@ -2605,10 +2605,9 @@ void panthor_sched_suspend(struct
> panthor_device *ptdev)
>
> csgs_upd_ctx_apply_locked(ptdev, _ctx);
> suspended_slots &= ~upd_ctx.timedout_mask;
> - faulty_slots = upd_ctx.timedout_mask;
>
> - if (faulty_slots) {
> - u32 slot_mask = faulty_slots;
> + if (upd_ctx.timedout_mask) {
> + u32 slot_mask = upd_ctx.timedout_mask;
>
> drm_err(>base, "CSG suspend failed,
> escalating to termination");
> csgs_upd_ctx_init(_ctx);
> @@ -2659,9 +2658,6 @@ void panthor_sched_suspend(struct
> panthor_device *ptdev)
>
> slot_mask &= ~BIT(csg_id);
> }
> -
> - if (flush_caches_failed)
> - faulty_slots |= suspended_slots;
> }
>
> for (i = 0; i < sched->csg_slot_count; i++) {
[PATCH] MAINTAINERS: fix LG sw43408 panel driver drm-misc git URL
The drm-misc git repo has moved to Gitlab. Fix the URL. Cc: Sumit Semwal Cc: Caleb Connolly Signed-off-by: Jani Nikula --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index d6327dc12cb1..23997d2ea91c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6766,7 +6766,7 @@ DRM DRIVER FOR LG SW43408 PANELS M: Sumit Semwal M: Caleb Connolly S: Maintained -T: git git://anongit.freedesktop.org/drm/drm-misc +T: git https://gitlab.freedesktop.org/drm/misc/kernel.git F: Documentation/devicetree/bindings/display/panel/lg,sw43408.yaml F: drivers/gpu/drm/panel/panel-lg-sw43408.c -- 2.39.2
Re: [PATCH 2/3] drm/panthor: Make sure the tiler initial/max chunks are consistent
On 25/04/2024 10:28, Steven Price wrote:
> On 25/04/2024 08:18, Boris Brezillon wrote:
>> It doesn't make sense to have a maximum number of chunks smaller than
>> the initial number of chunks attached to the context.
>>
>> Fix the uAPI header to reflect the new constraint, and mention the
>> undocumented "initial_chunk_count > 0" constraint while at it.
>>
>> Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block")
>> Signed-off-by: Boris Brezillon
>
> Reviewed-by: Steven Price
Ok, I'll take that back... I've rebased (and fixed up all the out of
tree patches) and this doesn't work when I actually test it!
>
>> ---
>> drivers/gpu/drm/panthor/panthor_heap.c | 3 +++
>> include/uapi/drm/panthor_drm.h | 8 ++--
>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c
>> b/drivers/gpu/drm/panthor/panthor_heap.c
>> index 143fa35f2e74..8728c9bb76e4 100644
>> --- a/drivers/gpu/drm/panthor/panthor_heap.c
>> +++ b/drivers/gpu/drm/panthor/panthor_heap.c
>> @@ -281,6 +281,9 @@ int panthor_heap_create(struct panthor_heap_pool *pool,
>> if (initial_chunk_count == 0)
>> return -EINVAL;
>>
>> +if (initial_chunk_count < max_chunks)
This should be initial_chunk_count > max_chunks. Otherwise you're
requiring the initial chunk count to be equal *or greater* than the max
chunks which makes no sense!
Steve
>> +return -EINVAL;
>> +
>> if (hweight32(chunk_size) != 1 ||
>> chunk_size < SZ_256K || chunk_size > SZ_2M)
>> return -EINVAL;
>> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
>> index dadb05ab1235..5db80a0682d5 100644
>> --- a/include/uapi/drm/panthor_drm.h
>> +++ b/include/uapi/drm/panthor_drm.h
>> @@ -895,13 +895,17 @@ struct drm_panthor_tiler_heap_create {
>> /** @vm_id: VM ID the tiler heap should be mapped to */
>> __u32 vm_id;
>>
>> -/** @initial_chunk_count: Initial number of chunks to allocate. */
>> +/** @initial_chunk_count: Initial number of chunks to allocate. Must be
>> at least one. */
>> __u32 initial_chunk_count;
>>
>> /** @chunk_size: Chunk size. Must be a power of two at least 256KB
>> large. */
>> __u32 chunk_size;
>>
>> -/** @max_chunks: Maximum number of chunks that can be allocated. */
>> +/**
>> + * @max_chunks: Maximum number of chunks that can be allocated.
>> + *
>> + * Must be at least @initial_chunk_count.
>> + */
>> __u32 max_chunks;
>>
>> /**
>
[PATCH] drm/panthor: Kill the faulty_slots variable in panthor_sched_suspend()
We can use upd_ctx.timedout_mask directly, and the faulty_slots update
in the flush_caches_failed situation is never used.
Suggested-by: Suggested-by: Steven Price
Signed-off-by: Boris Brezillon
---
drivers/gpu/drm/panthor/panthor_sched.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
b/drivers/gpu/drm/panthor/panthor_sched.c
index fad4678ca4c8..fed28c16d5d1 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -2584,8 +2584,8 @@ void panthor_sched_suspend(struct panthor_device *ptdev)
{
struct panthor_scheduler *sched = ptdev->scheduler;
struct panthor_csg_slots_upd_ctx upd_ctx;
- u32 suspended_slots, faulty_slots;
struct panthor_group *group;
+ u32 suspended_slots;
u32 i;
mutex_lock(>lock);
@@ -2605,10 +2605,9 @@ void panthor_sched_suspend(struct panthor_device *ptdev)
csgs_upd_ctx_apply_locked(ptdev, _ctx);
suspended_slots &= ~upd_ctx.timedout_mask;
- faulty_slots = upd_ctx.timedout_mask;
- if (faulty_slots) {
- u32 slot_mask = faulty_slots;
+ if (upd_ctx.timedout_mask) {
+ u32 slot_mask = upd_ctx.timedout_mask;
drm_err(>base, "CSG suspend failed, escalating to
termination");
csgs_upd_ctx_init(_ctx);
@@ -2659,9 +2658,6 @@ void panthor_sched_suspend(struct panthor_device *ptdev)
slot_mask &= ~BIT(csg_id);
}
-
- if (flush_caches_failed)
- faulty_slots |= suspended_slots;
}
for (i = 0; i < sched->csg_slot_count; i++) {
--
2.44.0
[PULL] drm-misc-fixes
Hi Dave, Sima, here's the PR for drm-misc-fixes. Best regards Thomas drm-misc-fixes-2024-04-25: Short summary of fixes pull: atomic-helpers: - Fix memory leak in drm_format_conv_state_copy() fbdev: - fbdefio: Fix address calculation gma500: - Fix crash during boot The following changes since commit 941c0bdbc176df825adf77052263b2d63db6fef7: drm/panel: novatek-nt36682e: don't unregister DSI device (2024-04-16 23:17:59 +0300) are available in the Git repository at: https://gitlab.freedesktop.org/drm/misc/kernel.git tags/drm-misc-fixes-2024-04-25 for you to fetch changes up to 78d9161d2bcd442d93d917339297ffa057dbee8c: fbdev: fix incorrect address computation in deferred IO (2024-04-24 15:03:37 +0200) Short summary of fixes pull: atomic-helpers: - Fix memory leak in drm_format_conv_state_copy() fbdev: - fbdefio: Fix address calculation gma500: - Fix crash during boot Lucas Stach (1): drm/atomic-helper: fix parameter order in drm_format_conv_state_copy() call Nam Cao (1): fbdev: fix incorrect address computation in deferred IO Patrik Jakobsson (1): drm/gma500: Remove lid code drivers/gpu/drm/drm_gem_atomic_helper.c | 4 +- drivers/gpu/drm/gma500/Makefile | 1 - drivers/gpu/drm/gma500/psb_device.c | 5 +-- drivers/gpu/drm/gma500/psb_drv.h| 9 drivers/gpu/drm/gma500/psb_lid.c| 80 - drivers/video/fbdev/core/fb_defio.c | 2 +- 6 files changed, 4 insertions(+), 97 deletions(-) delete mode 100644 drivers/gpu/drm/gma500/psb_lid.c -- 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] video: fbdev: replacing of_node_put with __free(device_node)
On 4/23/24 03:20, Abdulrasaq Lawani wrote:
Replaced instance of of_node_put with __free(device_node)
to simplify code and protect against any memory leaks
due to future changes in the control flow.
Suggested-by: Julia Lawall
Signed-off-by: Abdulrasaq Lawani
applied.
Thanks!
Helge
---
drivers/video/fbdev/offb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index b421b46d88ef..ea38a260774b 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -357,7 +357,7 @@ static void offb_init_palette_hacks(struct fb_info *info,
struct device_node *dp
par->cmap_type = cmap_gxt2000;
} else if (of_node_name_prefix(dp, "vga,Display-")) {
/* Look for AVIVO initialized by SLOF */
- struct device_node *pciparent = of_get_parent(dp);
+ struct device_node *pciparent __free(device_node) =
of_get_parent(dp);
const u32 *vid, *did;
vid = of_get_property(pciparent, "vendor-id", NULL);
did = of_get_property(pciparent, "device-id", NULL);
@@ -369,7 +369,6 @@ static void offb_init_palette_hacks(struct fb_info *info,
struct device_node *dp
if (par->cmap_adr)
par->cmap_type = cmap_avivo;
}
- of_node_put(pciparent);
} else if (dp && of_device_is_compatible(dp, "qemu,std-vga")) {
#ifdef __BIG_ENDIAN
const __be32 io_of_addr[3] = { 0x0100, 0x0, 0x0 };
Re: [PATCH -next] fbdev: savage: Handle err return when savagefb_check_var failed
On 4/16/24 08:51, Cai Xinchen wrote:
The commit 04e5eac8f3ab("fbdev: savage: Error out if pixclock equals zero")
checks the value of pixclock to avoid divide-by-zero error. However
the function savagefb_probe doesn't handle the error return of
savagefb_check_var. When pixclock is 0, it will cause divide-by-zero error.
Fixes: 04e5eac8f3ab ("fbdev: savage: Error out if pixclock equals zero")
Signed-off-by: Cai Xinchen
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/savage/savagefb_driver.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
applied.
Thanks!
Helge
diff --git a/drivers/video/fbdev/savage/savagefb_driver.c
b/drivers/video/fbdev/savage/savagefb_driver.c
index ebc9aeffdde7..ac41f8f37589 100644
--- a/drivers/video/fbdev/savage/savagefb_driver.c
+++ b/drivers/video/fbdev/savage/savagefb_driver.c
@@ -2276,7 +2276,10 @@ static int savagefb_probe(struct pci_dev *dev, const
struct pci_device_id *id)
if (info->var.xres_virtual > 0x1000)
info->var.xres_virtual = 0x1000;
#endif
- savagefb_check_var(>var, info);
+ err = savagefb_check_var(>var, info);
+ if (err)
+ goto failed;
+
savagefb_set_fix(info);
/*
Re: [PATCH] video: fbdev: au1200fb: replace deprecated strncpy with strscpy
On 4/25/24 01:49, Kees Cook wrote:
On Wed, Mar 20, 2024 at 11:48:52PM +0100, Helge Deller wrote:
On 3/20/24 23:35, Justin Stitt wrote:
Hi,
On Wed, Mar 20, 2024 at 12:56 AM Helge Deller wrote:
On 3/19/24 00:46, Justin Stitt wrote:
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
Let's use the new 2-argument strscpy() which guarantees NUL-termination
on the destination buffer while also simplifying the syntax. Note that
strscpy() will not NUL-pad the destination buffer like strncpy() does.
However, the NUL-padding behavior of strncpy() is not required since
fbdev is already NUL-allocated from au1200fb_drv_probe() ->
frameuffer_alloc(), rendering any additional NUL-padding redundant.
| p = kzalloc(fb_info_size + size, GFP_KERNEL);
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
Found with: $ rg "strncpy\("
---
drivers/video/fbdev/au1200fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c
index 6f20efc663d7..e718fea63662 100644
--- a/drivers/video/fbdev/au1200fb.c
+++ b/drivers/video/fbdev/au1200fb.c
@@ -1557,7 +1557,7 @@ static int au1200fb_init_fbinfo(struct au1200fb_device
*fbdev)
return ret;
}
- strncpy(fbi->fix.id, "AU1200", sizeof(fbi->fix.id));
+ strscpy(fbi->fix.id, "AU1200");
I wonder if you really build-tested this, as this driver is for the mips
architecture...
And I don't see a strscpy() function which takes just 2 arguments.
But I might be wrong
I did build successfully :thumbs_up:
Commit e6584c3964f2f ("string: Allow 2-argument strscpy()") introduced
this new strscpy() form; it is present in string.h on Linus' tree.
Interesting patch.
Might give compile problems if patches like yours gets automatically
picked up to stable series as long as Kees patch hasn't been backported yet...
Anyway, thanks for the pointer!
I'll apply your patch in the next round for fbdev.
Hi! I haven't seen this show up in -next yet. Have you had a chance to
pick it up?
There are also these too:
https://lore.kernel.org/all/20240320-strncpy-drivers-video-fbdev-fsl-diu-fb-c-v1-1-3cd3c012f...@google.com/
https://patchwork.kernel.org/project/linux-hardening/patch/20240320-strncpy-drivers-video-fbdev-uvesafb-c-v1-1-fd6af3766...@google.com/
https://patchwork.kernel.org/project/linux-hardening/patch/20240320-strncpy-drivers-video-hdmi-c-v1-1-f9a08168c...@google.com/
All 4 patches picked up into fbdev for-next git tree now.
Thanks!
Helge
Re: [PATCH 3/3] drm/panthor: Relax the check on the tiler chunk size
On Thu, 25 Apr 2024 10:28:56 +0100
Steven Price wrote:
> On 25/04/2024 08:18, Boris Brezillon wrote:
> > The field used to store the chunk size if 12 bits wide, and the encoding
> NIT: ^^ is
>
> > is chunk_size = chunk_header.chunk_size << 12, which gives us a
> > theoretical [4k:8M] range. This range is further limited by
> > implementation constraints, but those shouldn't be enforced kernel side.
> >
> > Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block")
> > Signed-off-by: Boris Brezillon
>
> There's also a kerneldoc comment above panthor_heap_create that needs
> updating too:
>
> > /**
> > * panthor_heap_create() - Create a heap context
> > * @pool: Pool to instantiate the heap context from.
> > * @initial_chunk_count: Number of chunk allocated at initialization time.
> > * Must be at least 1.
> > * @chunk_size: The size of each chunk. Must be a power of two between 256k
> > * and 2M.
>
> I'm also a little unsure on whether this is the right change. The
> "implementation defined" min/max in the hardware docs say 128KiB to
> 8MiB. I'm not convinced it makes sense to increase the range beyond that.
>
> As far as I'm aware the "must be a power of 2" isn't actually a
> requirement (it's certainly not a requirement of the storage format) so
> the kernel is already being more restrictive than necessary.
Ah, I got that wrong because v9 has this must-be-a-power-of-two
constraint (which is also where the erroneous 256k:2M range came from
BTW).
Chris, I guess you'd prefer to have the power-of-two constraint relaxed
too, so we can fine-tune the chunk size.
>
> It seems like a good idea to keep the uAPI requirements stricter than
> necessary and relax them in the future if we have a good reason (e.g.
> new hardware supports a wider range). But matching the existing hardware
> range of 128KB-8MB would obviously make sense now.
Sure, I'll restrict the range to 128k:8M as you suggest.
[PATCH] drm/exynos: hdmi: report safe 640x480 mode as a fallback when no EDID found
When reading EDID fails and driver reports no modes available, the DRM
core adds an artificial 1024x786 mode to the connector. Unfortunately
some variants of the Exynos HDMI (like the one in Exynos4 SoCs) are not
able to drive such mode, so report a safe 640x480 mode instead of nothing
in case of the EDID reading failure.
This fixes the following issue observed on Trats2 board since commit
13d5b040363c ("drm/exynos: do not return negative values from .get_modes()"):
[drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations
exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops)
exynos-drm exynos-drm: bound 12c1.mixer (ops mixer_component_ops)
exynos-dsi 11c8.dsi: [drm:samsung_dsim_host_attach] Attached s6e8aa0 device
(lanes:4 bpp:24 mode-flags:0x10b)
exynos-drm exynos-drm: bound 11c8.dsi (ops exynos_dsi_component_ops)
exynos-drm exynos-drm: bound 12d0.hdmi (ops hdmi_component_ops)
[drm] Initialized exynos 1.1.0 20180330 for exynos-drm on minor 1
exynos-hdmi 12d0.hdmi: [drm:hdmiphy_enable.part.0] *ERROR* PLL could not
reach steady state
panel-samsung-s6e8aa0 11c8.dsi.0: ID: 0xa2, 0x20, 0x8c
exynos-mixer 12c1.mixer: timeout waiting for VSYNC
[ cut here ]
WARNING: CPU: 1 PID: 11 at drivers/gpu/drm/drm_atomic_helper.c:1682
drm_atomic_helper_wait_for_vblanks.part.0+0x2b0/0x2b8
[CRTC:70:crtc-1] vblank wait timed out
Modules linked in:
CPU: 1 PID: 11 Comm: kworker/u16:0 Not tainted 6.9.0-rc5-next-20240424 #14913
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: events_unbound deferred_probe_work_func
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x68/0x88
dump_stack_lvl from __warn+0x7c/0x1c4
__warn from warn_slowpath_fmt+0x11c/0x1a8
warn_slowpath_fmt from drm_atomic_helper_wait_for_vblanks.part.0+0x2b0/0x2b8
drm_atomic_helper_wait_for_vblanks.part.0 from
drm_atomic_helper_commit_tail_rpm+0x7c/0x8c
drm_atomic_helper_commit_tail_rpm from commit_tail+0x9c/0x184
commit_tail from drm_atomic_helper_commit+0x168/0x190
drm_atomic_helper_commit from drm_atomic_commit+0xb4/0xe0
drm_atomic_commit from drm_client_modeset_commit_atomic+0x23c/0x27c
drm_client_modeset_commit_atomic from
drm_client_modeset_commit_locked+0x60/0x1cc
drm_client_modeset_commit_locked from drm_client_modeset_commit+0x24/0x40
drm_client_modeset_commit from
__drm_fb_helper_restore_fbdev_mode_unlocked+0x9c/0xc4
__drm_fb_helper_restore_fbdev_mode_unlocked from
drm_fb_helper_set_par+0x2c/0x3c
drm_fb_helper_set_par from fbcon_init+0x3d8/0x550
fbcon_init from visual_init+0xc0/0x108
visual_init from do_bind_con_driver+0x1b8/0x3a4
do_bind_con_driver from do_take_over_console+0x140/0x1ec
do_take_over_console from do_fbcon_takeover+0x70/0xd0
do_fbcon_takeover from fbcon_fb_registered+0x19c/0x1ac
fbcon_fb_registered from register_framebuffer+0x190/0x21c
register_framebuffer from __drm_fb_helper_initial_config_and_unlock+0x350/0x574
__drm_fb_helper_initial_config_and_unlock from
exynos_drm_fbdev_client_hotplug+0x6c/0xb0
exynos_drm_fbdev_client_hotplug from drm_client_register+0x58/0x94
drm_client_register from exynos_drm_bind+0x160/0x190
exynos_drm_bind from try_to_bring_up_aggregate_device+0x200/0x2d8
try_to_bring_up_aggregate_device from __component_add+0xb0/0x170
__component_add from mixer_probe+0x74/0xcc
mixer_probe from platform_probe+0x5c/0xb8
platform_probe from really_probe+0xe0/0x3d8
really_probe from __driver_probe_device+0x9c/0x1e4
__driver_probe_device from driver_probe_device+0x30/0xc0
driver_probe_device from __device_attach_driver+0xa8/0x120
__device_attach_driver from bus_for_each_drv+0x80/0xcc
bus_for_each_drv from __device_attach+0xac/0x1fc
__device_attach from bus_probe_device+0x8c/0x90
bus_probe_device from deferred_probe_work_func+0x98/0xe0
deferred_probe_work_func from process_one_work+0x240/0x6d0
process_one_work from worker_thread+0x1a0/0x3f4
worker_thread from kthread+0x104/0x138
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf0895fb0 to 0xf0895ff8)
...
irq event stamp: 82357
hardirqs last enabled at (82363): [] vprintk_emit+0x308/0x33c
hardirqs last disabled at (82368): [] vprintk_emit+0x2bc/0x33c
softirqs last enabled at (81614): [] __do_softirq+0x320/0x500
softirqs last disabled at (81609): [] __irq_exit_rcu+0x130/0x184
---[ end trace ]---
exynos-drm exynos-drm: [drm] *ERROR* flip_done timed out
exynos-drm exynos-drm: [drm] *ERROR* [CRTC:70:crtc-1] commit wait timed out
exynos-drm exynos-drm: [drm] *ERROR* flip_done timed out
exynos-drm exynos-drm: [drm] *ERROR* [CONNECTOR:74:HDMI-A-1] commit wait timed
out
exynos-drm exynos-drm: [drm] *ERROR* flip_done timed out
exynos-drm exynos-drm: [drm] *ERROR* [PLANE:56:plane-5] commit wait timed out
exynos-mixer 12c1.mixer: timeout waiting for VSYNC
Cc: sta...@vger.kernel.org
Fixes: 13d5b040363c ("drm/exynos: do not return negative values from
.get_modes()")
Signed-off-by: Marek
Re: [PATCH 1/3] drm/panthor: Fix tiler OOM handling to allow incremental rendering
On Thu, 25 Apr 2024 10:28:49 +0100
Steven Price wrote:
> On 25/04/2024 08:18, Boris Brezillon wrote:
> > From: Antonino Maniscalco
> >
> > If the kernel couldn't allocate memory because we reached the maximum
> > number of chunks but no render passes are in flight
> > (panthor_heap_grow() returning -ENOMEM), we should defer the OOM
> > handling to the FW by returning a NULL chunk. The FW will then call
> > the tiler OOM exception handler, which is supposed to implement
> > incremental rendering (execute an intermediate fragment job to flush
> > the pending primitives, release the tiler memory that was used to
> > store those primitives, and start over from where it stopped).
> >
> > Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> > Signed-off-by: Antonino Maniscalco
> > Signed-off-by: Boris Brezillon
>
> Reviewed-by: Steven Price
>
> Although I think the real issue here is that we haven't clearly defined
> the return values from panthor_heap_grow - it's a bit weird to have two
> different error codes for the same "try again later after incremental
> rendering" result. But as a fix this seems most clear.
Yeah, I actually considered returning -EBUSY for the 'max_chunks
reached' situation, but then realized we would also want to trigger
incremental rendering for actual mem allocation failures (when
chunk_count < max_chunks) once the fail-able/non-blocking allocation
logic is implemented, and for this kind of failure it makes more sense
to return -ENOMEM, even though this implies checking against two values
instead of one.
I guess returning -ENOMEM instead of -EBUSY for the case where we have
render passes in-flight wouldn't be too awkward, as this can be seen as
the kernel refusing to allocate more memory.
>
> Steve
>
> > ---
> > drivers/gpu/drm/panthor/panthor_sched.c | 8 +++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
> > b/drivers/gpu/drm/panthor/panthor_sched.c
> > index b3a51a6de523..6de8c0c702cb 100644
> > --- a/drivers/gpu/drm/panthor/panthor_sched.c
> > +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> > @@ -1354,7 +1354,13 @@ static int group_process_tiler_oom(struct
> > panthor_group *group, u32 cs_id)
> > pending_frag_count, _chunk_va);
> > }
> >
> > - if (ret && ret != -EBUSY) {
> > + /* If the kernel couldn't allocate memory because we reached the maximum
> > +* number of chunks (EBUSY if we have render passes in flight, ENOMEM
> > +* otherwise), we want to let the FW try to reclaim memory by waiting
> > +* for fragment jobs to land or by executing the tiler OOM exception
> > +* handler, which is supposed to implement incremental rendering.
> > +*/
> > + if (ret && ret != -EBUSY && ret != -ENOMEM) {
> > drm_warn(>base, "Failed to extend the tiler heap\n");
> > group->fatal_queues |= BIT(cs_id);
> > sched_queue_delayed_work(sched, tick, 0);
>
Re: [PATCH] drm/panthor: Make sure we handled the unknown group state properly
On 25/04/2024 09:46, Boris Brezillon wrote:
> When we check for state values returned by the FW, we only cover part of
> the 0:7 range. Make sure we catch FW inconsistencies by adding a default
> to the switch statement, and flagging the group state as unknown in that
> case.
>
> When an unknown state is detected, we trigger a reset, and consider the
> group as unusable after that point, to prevent the potential corruption
> from creeping in other places if we continue executing stuff on this
> context.
>
> Reported-by: Dan Carpenter
> Suggested-by: Steven Price
> Signed-off-by: Boris Brezillon
Thanks for doing this up - I can tick it off my todo list ;)
Reviewed-by: Steven Price
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 37 +++--
> 1 file changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
> b/drivers/gpu/drm/panthor/panthor_sched.c
> index 6de8c0c702cb..fad4678ca4c8 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -490,6 +490,18 @@ enum panthor_group_state {
>* Can no longer be scheduled. The only allowed action is a destruction.
>*/
> PANTHOR_CS_GROUP_TERMINATED,
> +
> + /**
> + * @PANTHOR_CS_GROUP_UNKNOWN_STATE: Group is an unknown state.
> + *
> + * The FW returned an inconsistent state. The group is flagged unusable
> + * and can no longer be scheduled. The only allowed action is a
> + * destruction.
> + *
> + * When that happens, we also schedule a FW reset, to start from a fresh
> + * state.
> + */
> + PANTHOR_CS_GROUP_UNKNOWN_STATE,
> };
>
> /**
> @@ -1127,6 +1139,7 @@ csg_slot_sync_state_locked(struct panthor_device
> *ptdev, u32 csg_id)
> struct panthor_fw_csg_iface *csg_iface;
> struct panthor_group *group;
> enum panthor_group_state new_state, old_state;
> + u32 csg_state;
>
> lockdep_assert_held(>scheduler->lock);
>
> @@ -1137,7 +1150,8 @@ csg_slot_sync_state_locked(struct panthor_device
> *ptdev, u32 csg_id)
> return;
>
> old_state = group->state;
> - switch (csg_iface->output->ack & CSG_STATE_MASK) {
> + csg_state = csg_iface->output->ack & CSG_STATE_MASK;
> + switch (csg_state) {
> case CSG_STATE_START:
> case CSG_STATE_RESUME:
> new_state = PANTHOR_CS_GROUP_ACTIVE;
> @@ -1148,11 +1162,28 @@ csg_slot_sync_state_locked(struct panthor_device
> *ptdev, u32 csg_id)
> case CSG_STATE_SUSPEND:
> new_state = PANTHOR_CS_GROUP_SUSPENDED;
> break;
> + default:
> + /* The unknown state might be caused by a FW state corruption,
> + * which means the group metadata can't be trusted anymore, and
> + * the SUSPEND operation might propagate the corruption to the
> + * suspend buffers. Flag the group state as unknown to make
> + * sure it's unusable after that point.
> + */
> + drm_err(>base, "Invalid state on CSG %d (state=%d)",
> + csg_id, csg_state);
> + new_state = PANTHOR_CS_GROUP_UNKNOWN_STATE;
> + break;
> }
>
> if (old_state == new_state)
> return;
>
> + /* The unknown state might be caused by a FW issue, reset the FW to
> + * take a fresh start.
> + */
> + if (new_state == PANTHOR_CS_GROUP_UNKNOWN_STATE)
> + panthor_device_schedule_reset(ptdev);
> +
> if (new_state == PANTHOR_CS_GROUP_SUSPENDED)
> csg_slot_sync_queues_state_locked(ptdev, csg_id);
>
> @@ -1789,6 +1820,7 @@ static bool
> group_can_run(struct panthor_group *group)
> {
> return group->state != PANTHOR_CS_GROUP_TERMINATED &&
> +group->state != PANTHOR_CS_GROUP_UNKNOWN_STATE &&
> !group->destroyed && group->fatal_queues == 0 &&
> !group->timedout;
> }
> @@ -2563,7 +2595,8 @@ void panthor_sched_suspend(struct panthor_device *ptdev)
>
> if (csg_slot->group) {
> csgs_upd_ctx_queue_reqs(ptdev, _ctx, i,
> - CSG_STATE_SUSPEND,
> + group_can_run(csg_slot->group) ?
> + CSG_STATE_SUSPEND :
> CSG_STATE_TERMINATE,
> CSG_STATE_MASK);
> }
> }
Re: [PATCH 3/3] drm/panthor: Relax the check on the tiler chunk size
On 25/04/2024 08:18, Boris Brezillon wrote:
> The field used to store the chunk size if 12 bits wide, and the encoding
NIT: ^^ is
> is chunk_size = chunk_header.chunk_size << 12, which gives us a
> theoretical [4k:8M] range. This range is further limited by
> implementation constraints, but those shouldn't be enforced kernel side.
>
> Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block")
> Signed-off-by: Boris Brezillon
There's also a kerneldoc comment above panthor_heap_create that needs
updating too:
> /**
> * panthor_heap_create() - Create a heap context
> * @pool: Pool to instantiate the heap context from.
> * @initial_chunk_count: Number of chunk allocated at initialization time.
> * Must be at least 1.
> * @chunk_size: The size of each chunk. Must be a power of two between 256k
> * and 2M.
I'm also a little unsure on whether this is the right change. The
"implementation defined" min/max in the hardware docs say 128KiB to
8MiB. I'm not convinced it makes sense to increase the range beyond that.
As far as I'm aware the "must be a power of 2" isn't actually a
requirement (it's certainly not a requirement of the storage format) so
the kernel is already being more restrictive than necessary.
It seems like a good idea to keep the uAPI requirements stricter than
necessary and relax them in the future if we have a good reason (e.g.
new hardware supports a wider range). But matching the existing hardware
range of 128KB-8MB would obviously make sense now.
Steve
> ---
> drivers/gpu/drm/panthor/panthor_heap.c | 2 +-
> include/uapi/drm/panthor_drm.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c
> b/drivers/gpu/drm/panthor/panthor_heap.c
> index 8728c9bb76e4..146ea2f57764 100644
> --- a/drivers/gpu/drm/panthor/panthor_heap.c
> +++ b/drivers/gpu/drm/panthor/panthor_heap.c
> @@ -285,7 +285,7 @@ int panthor_heap_create(struct panthor_heap_pool *pool,
> return -EINVAL;
>
> if (hweight32(chunk_size) != 1 ||
> - chunk_size < SZ_256K || chunk_size > SZ_2M)
> + chunk_size < SZ_4K || chunk_size > SZ_8M)
> return -EINVAL;
>
> down_read(>lock);
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index 5db80a0682d5..80c0716361b9 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -898,7 +898,7 @@ struct drm_panthor_tiler_heap_create {
> /** @initial_chunk_count: Initial number of chunks to allocate. Must be
> at least one. */
> __u32 initial_chunk_count;
>
> - /** @chunk_size: Chunk size. Must be a power of two at least 256KB
> large. */
> + /** @chunk_size: Chunk size. Must be a power of two and lie in the
> [4k:8M] range. */
> __u32 chunk_size;
>
> /**
Re: [PATCH 1/3] drm/panthor: Fix tiler OOM handling to allow incremental rendering
On 25/04/2024 08:18, Boris Brezillon wrote:
> From: Antonino Maniscalco
>
> If the kernel couldn't allocate memory because we reached the maximum
> number of chunks but no render passes are in flight
> (panthor_heap_grow() returning -ENOMEM), we should defer the OOM
> handling to the FW by returning a NULL chunk. The FW will then call
> the tiler OOM exception handler, which is supposed to implement
> incremental rendering (execute an intermediate fragment job to flush
> the pending primitives, release the tiler memory that was used to
> store those primitives, and start over from where it stopped).
>
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Signed-off-by: Antonino Maniscalco
> Signed-off-by: Boris Brezillon
Reviewed-by: Steven Price
Although I think the real issue here is that we haven't clearly defined
the return values from panthor_heap_grow - it's a bit weird to have two
different error codes for the same "try again later after incremental
rendering" result. But as a fix this seems most clear.
Steve
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
> b/drivers/gpu/drm/panthor/panthor_sched.c
> index b3a51a6de523..6de8c0c702cb 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -1354,7 +1354,13 @@ static int group_process_tiler_oom(struct
> panthor_group *group, u32 cs_id)
> pending_frag_count, _chunk_va);
> }
>
> - if (ret && ret != -EBUSY) {
> + /* If the kernel couldn't allocate memory because we reached the maximum
> + * number of chunks (EBUSY if we have render passes in flight, ENOMEM
> + * otherwise), we want to let the FW try to reclaim memory by waiting
> + * for fragment jobs to land or by executing the tiler OOM exception
> + * handler, which is supposed to implement incremental rendering.
> + */
> + if (ret && ret != -EBUSY && ret != -ENOMEM) {
> drm_warn(>base, "Failed to extend the tiler heap\n");
> group->fatal_queues |= BIT(cs_id);
> sched_queue_delayed_work(sched, tick, 0);
Re: [PATCH 2/3] drm/panthor: Make sure the tiler initial/max chunks are consistent
On 25/04/2024 08:18, Boris Brezillon wrote:
> It doesn't make sense to have a maximum number of chunks smaller than
> the initial number of chunks attached to the context.
>
> Fix the uAPI header to reflect the new constraint, and mention the
> undocumented "initial_chunk_count > 0" constraint while at it.
>
> Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block")
> Signed-off-by: Boris Brezillon
Reviewed-by: Steven Price
> ---
> drivers/gpu/drm/panthor/panthor_heap.c | 3 +++
> include/uapi/drm/panthor_drm.h | 8 ++--
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c
> b/drivers/gpu/drm/panthor/panthor_heap.c
> index 143fa35f2e74..8728c9bb76e4 100644
> --- a/drivers/gpu/drm/panthor/panthor_heap.c
> +++ b/drivers/gpu/drm/panthor/panthor_heap.c
> @@ -281,6 +281,9 @@ int panthor_heap_create(struct panthor_heap_pool *pool,
> if (initial_chunk_count == 0)
> return -EINVAL;
>
> + if (initial_chunk_count < max_chunks)
> + return -EINVAL;
> +
> if (hweight32(chunk_size) != 1 ||
> chunk_size < SZ_256K || chunk_size > SZ_2M)
> return -EINVAL;
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index dadb05ab1235..5db80a0682d5 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -895,13 +895,17 @@ struct drm_panthor_tiler_heap_create {
> /** @vm_id: VM ID the tiler heap should be mapped to */
> __u32 vm_id;
>
> - /** @initial_chunk_count: Initial number of chunks to allocate. */
> + /** @initial_chunk_count: Initial number of chunks to allocate. Must be
> at least one. */
> __u32 initial_chunk_count;
>
> /** @chunk_size: Chunk size. Must be a power of two at least 256KB
> large. */
> __u32 chunk_size;
>
> - /** @max_chunks: Maximum number of chunks that can be allocated. */
> + /**
> + * @max_chunks: Maximum number of chunks that can be allocated.
> + *
> + * Must be at least @initial_chunk_count.
> + */
> __u32 max_chunks;
>
> /**
