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
[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 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));
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
[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 ++--
[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
[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
[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
[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 =
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
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
[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
[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
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
>
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 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
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 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
[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
Re: [PATCH 02/10] pwm: Add SI-EN SN3112 PWM support
Hello, On Wed, Apr 24, 2024 at 09:37:25PM +0200, Konrad Dybcio wrote: > On 4/24/24 17:29, Xilin Wu via B4 Relay wrote: > > + > > + /* use random value to apply changes */ > > + ret = sn3112_write_reg(priv, SN3112_REG_APPLY, 0x66); > > "a random value"? sounds suspicious.. I smiled about that one, too, remembering https://xkcd.com/221/ > [...] > > +#if IS_ENABLED(CONFIG_GPIOLIB) > > + /* enable hardware shutdown pin */ > > + if (priv->sdb) > > + gpiod_set_value(priv->sdb, 1); > > +#endif > > + > > + /* power-off sn5112 power vdd */ > > + regulator_disable(priv->vdd); > > + > > + pwmchip_remove(chip); > > devm_pwmchip_add? Note using devm_xyz only works if all requests before are also using devm. (There are a few exceptions, but these need proper thinking and extensive commenting.) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH v3 4/4] drm/xe/FLR: Support PCIe FLR
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
.driver.pm = _pm_ops,
#endif
+ .err_handler = _pci_err_handlers,
};
int xe_register_pci_driver(void)
diff --git a/drivers/gpu/drm/xe/xe_pci.h b/drivers/gpu/drm/xe/xe_pci.h
index 73b90a430d1f..9faf5380a09e 100644
--- a/drivers/gpu/drm/xe/xe_pci.h
+++ b/drivers/gpu/drm/xe/xe_pci.h
@@ -7,8 +7,10 @@
#define _XE_PCI_H_
struct pci_dev;
+struct pci_device_id;
int xe_register_pci_driver(void);
Re: [RFC PATCH 09/18] drm/amdgpu: Don't mark VRAM as a busy placement for VRAM|GTT resources
Am 24.04.24 um 18:56 schrieb Friedrich Vock:
We will never try evicting things from VRAM for these resources anyway.
This affects TTM buffer uneviction logic, which would otherwise try to
move these buffers into VRAM (clashing with VRAM-only allocations).
You are working on outdated code. That change was already done by me as
well.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 5834a95d680d9..85c10d8086188 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -127,6 +127,7 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo,
u32 domain)
struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
struct ttm_placement *placement = >placement;
struct ttm_place *places = abo->placements;
+ bool skip_vram_busy = false;
u64 flags = abo->flags;
u32 c = 0;
@@ -156,6 +157,13 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo
*abo, u32 domain)
if (flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)
places[c].flags |= TTM_PL_FLAG_CONTIGUOUS;
c++;
+
+ /*
+* If GTT is preferred by the buffer as well, don't try VRAM
when it's
+* busy.
+*/
+ if ((domain & abo->preferred_domains) & AMDGPU_GEM_DOMAIN_GTT)
+ skip_vram_busy = true;
}
if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
@@ -223,6 +231,11 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo
*abo, u32 domain)
placement->num_busy_placement = c;
placement->busy_placement = places;
+
+ if (skip_vram_busy) {
+ --placement->num_busy_placement;
+ ++placement->busy_placement;
+ }
}
/**
--
2.44.0
Re: [RFC PATCH 16/18] drm/amdgpu: Implement SET_PRIORITY GEM op
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
Used by userspace to adjust buffer priorities in response to changes in
application demand and memory pressure.
Yeah, that was discussed over and over again. One big design criteria is
that we can't have global priorities from userspace!
The background here is that this can trivially be abused.
What we can do is to have per process priorities, but that needs to be
in the VM subsystem.
That's also the reason why I personally think that the handling
shouldn't be inside TTM at all.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 20
include/uapi/drm/amdgpu_drm.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 5ca13e2e50f50..6107810a9c205 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -836,8 +836,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
{
struct amdgpu_device *adev = drm_to_adev(dev);
struct drm_amdgpu_gem_op *args = data;
+ struct ttm_resource_manager *man;
struct drm_gem_object *gobj;
struct amdgpu_vm_bo_base *base;
+ struct ttm_operation_ctx ctx;
struct amdgpu_bo *robj;
int r;
@@ -851,6 +853,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
if (unlikely(r))
goto out;
+ memset(, 0, sizeof(ctx));
+ ctx.interruptible = true;
+
switch (args->op) {
case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
struct drm_amdgpu_gem_create_in info;
@@ -898,6 +903,21 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
amdgpu_bo_unreserve(robj);
break;
+ case AMDGPU_GEM_OP_SET_PRIORITY:
+ if (args->value > AMDGPU_BO_PRIORITY_MAX_USER)
+ args->value = AMDGPU_BO_PRIORITY_MAX_USER;
+ ttm_bo_update_priority(>tbo, args->value);
+ if (robj->tbo.evicted_type != TTM_NUM_MEM_TYPES) {
+ ttm_bo_try_unevict(>tbo, );
+ amdgpu_bo_unreserve(robj);
+ } else {
+ amdgpu_bo_unreserve(robj);
+ man = ttm_manager_type(robj->tbo.bdev,
+ robj->tbo.resource->mem_type);
+ ttm_mem_unevict_evicted(robj->tbo.bdev, man,
+ true);
+ }
+ break;
default:
amdgpu_bo_unreserve(robj);
r = -EINVAL;
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index bdbe6b262a78d..53552dd489b9b 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -531,6 +531,7 @@ union drm_amdgpu_wait_fences {
#define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
#define AMDGPU_GEM_OP_SET_PLACEMENT 1
+#define AMDGPU_GEM_OP_SET_PRIORITY 2
/* Sets or returns a value associated with a buffer. */
struct drm_amdgpu_gem_op {
--
2.44.0
Re: [RFC PATCH 08/18] drm/amdgpu: Don't try moving BOs to preferred domain before submit
Am 24.04.24 um 18:56 schrieb Friedrich Vock:
TTM now takes care of moving buffers to the best possible domain.
Yeah, I've been planning to do this for a while as well. The problem is
really that we need to keep the functionality.
For example TTM currently doesn't have a concept of an userspace client.
So it can't track the bytes already evicted for each client.
This needs to be added as infrastructure first and then we can start to
change this over into moving more functionality into TTM.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h| 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 191 +
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h | 4 -
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 7 -
4 files changed, 3 insertions(+), 201 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index cac0ca64367b3..3004adc6fa679 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1404,8 +1404,6 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev);
bool amdgpu_device_seamless_boot_supported(struct amdgpu_device *adev);
bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev);
-void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes,
- u64 num_vis_bytes);
int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev);
void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
const u32 *registers,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index e9168677ef0a6..92a0cffc1adc3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -638,196 +638,19 @@ static int amdgpu_cs_pass2(struct amdgpu_cs_parser *p)
return 0;
}
-/* Convert microseconds to bytes. */
-static u64 us_to_bytes(struct amdgpu_device *adev, s64 us)
-{
- if (us <= 0 || !adev->mm_stats.log2_max_MBps)
- return 0;
-
- /* Since accum_us is incremented by a million per second, just
-* multiply it by the number of MB/s to get the number of bytes.
-*/
- return us << adev->mm_stats.log2_max_MBps;
-}
-
-static s64 bytes_to_us(struct amdgpu_device *adev, u64 bytes)
-{
- if (!adev->mm_stats.log2_max_MBps)
- return 0;
-
- return bytes >> adev->mm_stats.log2_max_MBps;
-}
-
-/* Returns how many bytes TTM can move right now. If no bytes can be moved,
- * it returns 0. If it returns non-zero, it's OK to move at least one buffer,
- * which means it can go over the threshold once. If that happens, the driver
- * will be in debt and no other buffer migrations can be done until that debt
- * is repaid.
- *
- * This approach allows moving a buffer of any size (it's important to allow
- * that).
- *
- * The currency is simply time in microseconds and it increases as the clock
- * ticks. The accumulated microseconds (us) are converted to bytes and
- * returned.
- */
-static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev,
- u64 *max_bytes,
- u64 *max_vis_bytes)
-{
- s64 time_us, increment_us;
- u64 free_vram, total_vram, used_vram;
- /* Allow a maximum of 200 accumulated ms. This is basically per-IB
-* throttling.
-*
-* It means that in order to get full max MBps, at least 5 IBs per
-* second must be submitted and not more than 200ms apart from each
-* other.
-*/
- const s64 us_upper_bound = 20;
-
- if (!adev->mm_stats.log2_max_MBps) {
- *max_bytes = 0;
- *max_vis_bytes = 0;
- return;
- }
-
- total_vram = adev->gmc.real_vram_size -
atomic64_read(>vram_pin_size);
- used_vram = ttm_resource_manager_usage(>mman.vram_mgr.manager);
- free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram;
-
- spin_lock(>mm_stats.lock);
-
- /* Increase the amount of accumulated us. */
- time_us = ktime_to_us(ktime_get());
- increment_us = time_us - adev->mm_stats.last_update_us;
- adev->mm_stats.last_update_us = time_us;
- adev->mm_stats.accum_us = min(adev->mm_stats.accum_us + increment_us,
- us_upper_bound);
-
- /* This prevents the short period of low performance when the VRAM
-* usage is low and the driver is in debt or doesn't have enough
-* accumulated us to fill VRAM quickly.
-*
-* The situation can occur in these cases:
-* - a lot of VRAM is freed by userspace
-* - the presence of a big buffer causes a lot of evictions
-* (solution: split buffers into smaller ones)
-*
-* If 128 MB or 1/8th of VRAM
Re: [RFC PATCH 16/18] drm/amdgpu: Implement SET_PRIORITY GEM op
On 25.04.24 08:32, Christian König wrote:
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
Used by userspace to adjust buffer priorities in response to changes in
application demand and memory pressure.
Yeah, that was discussed over and over again. One big design criteria
is that we can't have global priorities from userspace!
The background here is that this can trivially be abused.
I see your point when apps are allowed to prioritize themselves above
other apps, and I agree that should probably be disallowed at least for
unprivileged apps.
Disallowing this is a pretty trivial change though, and I don't really
see the abuse potential in being able to downgrade your own priority?
Regards,
Friedrich
What we can do is to have per process priorities, but that needs to be
in the VM subsystem.
That's also the reason why I personally think that the handling
shouldn't be inside TTM at all.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 20
include/uapi/drm/amdgpu_drm.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 5ca13e2e50f50..6107810a9c205 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -836,8 +836,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
{
struct amdgpu_device *adev = drm_to_adev(dev);
struct drm_amdgpu_gem_op *args = data;
+ struct ttm_resource_manager *man;
struct drm_gem_object *gobj;
struct amdgpu_vm_bo_base *base;
+ struct ttm_operation_ctx ctx;
struct amdgpu_bo *robj;
int r;
@@ -851,6 +853,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
if (unlikely(r))
goto out;
+ memset(, 0, sizeof(ctx));
+ ctx.interruptible = true;
+
switch (args->op) {
case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
struct drm_amdgpu_gem_create_in info;
@@ -898,6 +903,21 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
amdgpu_bo_unreserve(robj);
break;
+ case AMDGPU_GEM_OP_SET_PRIORITY:
+ if (args->value > AMDGPU_BO_PRIORITY_MAX_USER)
+ args->value = AMDGPU_BO_PRIORITY_MAX_USER;
+ ttm_bo_update_priority(>tbo, args->value);
+ if (robj->tbo.evicted_type != TTM_NUM_MEM_TYPES) {
+ ttm_bo_try_unevict(>tbo, );
+ amdgpu_bo_unreserve(robj);
+ } else {
+ amdgpu_bo_unreserve(robj);
+ man = ttm_manager_type(robj->tbo.bdev,
+ robj->tbo.resource->mem_type);
+ ttm_mem_unevict_evicted(robj->tbo.bdev, man,
+ true);
+ }
+ break;
default:
amdgpu_bo_unreserve(robj);
r = -EINVAL;
diff --git a/include/uapi/drm/amdgpu_drm.h
b/include/uapi/drm/amdgpu_drm.h
index bdbe6b262a78d..53552dd489b9b 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -531,6 +531,7 @@ union drm_amdgpu_wait_fences {
#define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
#define AMDGPU_GEM_OP_SET_PLACEMENT 1
+#define AMDGPU_GEM_OP_SET_PRIORITY 2
/* Sets or returns a value associated with a buffer. */
struct drm_amdgpu_gem_op {
--
2.44.0
[PATCH 0/3] drm/panthor: Collection of tiler heap related fixes
Hello, This is a collection of tiler heap fixes for bugs/oddities found while looking at incremental rendering. We need to make sure those land before 6.10 is released (we still have plenty of time), so we don't need to increment the driver version to reflect the ABI changes. Regards, Boris Antonino Maniscalco (1): drm/panthor: Fix tiler OOM handling to allow incremental rendering Boris Brezillon (2): drm/panthor: Make sure the tiler initial/max chunks are consistent drm/panthor: Relax the check on the tiler chunk size drivers/gpu/drm/panthor/panthor_heap.c | 5 - drivers/gpu/drm/panthor/panthor_sched.c | 8 +++- 2 files changed, 11 insertions(+), 2 deletions(-) -- 2.44.0
[PATCH 1/3] drm/panthor: Fix tiler OOM handling to allow incremental rendering
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
---
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);
--
2.44.0
[PATCH 2/3] drm/panthor: Make sure the tiler initial/max chunks are consistent
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
---
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;
/**
--
2.44.0
[PATCH 3/3] drm/panthor: Relax the check on the tiler chunk size
The field used to store the chunk size if 12 bits wide, and the encoding
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
---
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;
/**
--
2.44.0
Re: [PATCH] gpu: drm: exynos: hdmi: eliminate uses of of_node_put()
Good cleanup. Applied. :)
Thanks,
Inki Dae
2024년 4월 15일 (월) 오전 9:40, Shivani Gupta 님이 작성:
>
> Utilize the __free() cleanup handler within the hdmi_get_phy_io function
> to automatically release the device node when it is out of scope.
> This eliminates the manual invocation of of_node_put(), reducing the
> potential for memory leaks.
>
> The modification requires initializing the device node at the beginning
> of the function, ensuring that the automatic cleanup is safely executed.
>
> Consequently, this removes the need for error cleanup paths that utilize
> goto statements and the jump to out is no longer necessary.
>
> Suggested-by: Julia Lawall
> Signed-off-by: Shivani Gupta
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c | 15 +--
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index b1d02dec3774..a741fd949482 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -1919,10 +1919,9 @@ static int hdmi_get_ddc_adapter(struct hdmi_context
> *hdata)
> static int hdmi_get_phy_io(struct hdmi_context *hdata)
> {
> const char *compatible_str = "samsung,exynos4212-hdmiphy";
> - struct device_node *np;
> - int ret = 0;
> + struct device_node *np __free(device_node) =
> + of_find_compatible_node(NULL, NULL, compatible_str);
>
> - np = of_find_compatible_node(NULL, NULL, compatible_str);
> if (!np) {
> np = of_parse_phandle(hdata->dev->of_node, "phy", 0);
> if (!np) {
> @@ -1937,21 +1936,17 @@ static int hdmi_get_phy_io(struct hdmi_context *hdata)
> if (!hdata->regs_hdmiphy) {
> DRM_DEV_ERROR(hdata->dev,
> "failed to ioremap hdmi phy\n");
> - ret = -ENOMEM;
> - goto out;
> + return -ENOMEM;
> }
> } else {
> hdata->hdmiphy_port = of_find_i2c_device_by_node(np);
> if (!hdata->hdmiphy_port) {
> DRM_INFO("Failed to get hdmi phy i2c client\n");
> - ret = -EPROBE_DEFER;
> - goto out;
> + return -EPROBE_DEFER;
> }
> }
>
> -out:
> - of_node_put(np);
> - return ret;
> + return 0;
> }
>
> static int hdmi_probe(struct platform_device *pdev)
> --
> 2.34.1
>
>
Re: [RFC PATCH 05/18] drm/ttm: Add option to evict no BOs in operation
Am 24.04.24 um 18:56 schrieb Friedrich Vock:
When undoing evictions because of decreased memory pressure, it makes no
sense to try evicting other buffers.
That duplicates some functionality.
If a driver doesn't want eviction to happen it just needs to mark the
desired placements as non-evictable with the TTM_PL_FLAG_DESIRED flag.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/ttm/ttm_bo.c | 2 ++
include/drm/ttm/ttm_bo.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 9a0efbf79316c..3b89fabc2f00a 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -764,6 +764,8 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object
*bo,
break;
if (unlikely(ret != -ENOSPC))
return ret;
+ if (ctx->no_evict)
+ return -ENOSPC;
ret = ttm_mem_evict_first(bdev, man, place, ctx,
ticket);
if (unlikely(ret != 0))
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 8a1a29c6fbc50..a8f21092403d6 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -192,6 +192,7 @@ struct ttm_operation_ctx {
bool gfp_retry_mayfail;
bool allow_res_evict;
bool force_alloc;
+ bool no_evict;
struct dma_resv *resv;
uint64_t bytes_moved;
};
@@ -358,6 +359,7 @@ static inline void *ttm_kmap_obj_virtual(struct
ttm_bo_kmap_obj *map,
return map->virtual;
}
+
int ttm_bo_wait_ctx(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx);
int ttm_bo_validate(struct ttm_buffer_object *bo,
--
2.44.0
Re: [PATCH 10/10] arm64: dts: qcom: Add AYN Odin 2
On 24/04/2024 17:29, Xilin Wu via B4 Relay wrote:
> From: Xilin Wu
>
> AYN Odin 2 is a gaming handheld based on QCS8550, which is derived
> from SM8550 but without modem RF system.
>
> +
> +/ {
> + model = "AYN Odin 2";
> + compatible = "ayn,odin2", "qcom,qcs8550", "qcom,sm8550";
> + chassis-type = "handset";
> +
> + qcom,msm-id = ;
> + qcom,board-id = <0x1001f 0>;
No, these are not allowed. You did not test your dts.
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
> +
> + aliases {
> + serial0 =
> + serial1 =
> + serial2 =
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + pwms = <_pwm 0 86>;
> + brightness-levels = <1023 0>;
> + num-interpolated-steps = <1023>;
> + default-brightness-level = <600>;
> + power-supply = <_pwr>;
> + enable-gpios = <_gpios 5 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_backlight_default>;
> + status = "okay";
Drop, why do you need it? Do you see it anywhere else in the backlight
nodes in DTS?
> + };
> +
> + fan_pwr: fan-pwr-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "fan_pwr";
> +
> + regulator-min-microvolt = <500>;
> + regulator-max-microvolt = <500>;
> +
> + gpio = < 109 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_pwr_en>;
> +
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> +
> + pinctrl-0 = <_up_n>, <_m2_keys_default>;
> + pinctrl-names = "default";
> +
> + key-volume-up {
> + label = "Volume Up";
> + linux,code = ;
> + gpios = <_gpios 6 GPIO_ACTIVE_LOW>;
> + debounce-interval = <15>;
> + linux,can-disable;
> + wakeup-source;
> + };
> +
> + m1-button {
> + label = "M1";
> + linux,code = ;
> + gpios = < 57 GPIO_ACTIVE_LOW>;
> + };
> +
> + m2-button {
> + label = "M2";
> + linux,code = ;
> + gpios = < 58 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> + hdmi-out {
> + compatible = "hdmi-connector";
> + type = "d";
> + hpd-gpios = < 9 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + hdmi_con: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +
> + hdmi_pwr: hdmi-pwr-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "hdmi_pwr";
> +
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> +
> + gpio = < 10 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + vdd_lcm_2p8: vdd-lcm-2p8-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vdd_lcm_2p8";
> +
> + regulator-min-microvolt = <280>;
> + regulator-max-microvolt = <280>;
> +
> + gpio = < 142 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + led_left_side: led-controller-1 {
> + compatible = "pwm-leds-multicolor";
> +
> + multi-led {
> + label = "left-side";
> + color = ;
> + max-brightness = <255>;
> +
> + led-red {
> + color = ;
> + pwms = <_rgb_left 0>;
> + };
> +
> + led-green {
> + color = ;
> + pwms = <_rgb_left 1>;
> + };
> +
> + led-blue {
> + color = ;
> + pwms = <_rgb_left 2>;
> + };
> + };
> + };
> +
> + led_left_joystick: led-controller-2 {
> + compatible = "pwm-leds-multicolor";
> +
> + multi-led {
> + label = "left-joystick";
> + color = ;
> + max-brightness = <255>;
> +
> + led-red {
> + color
Re: [RFC PATCH 13/18] drm/ttm: Implement ttm_bo_update_priority
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
Used to dynamically adjust priorities of buffers at runtime, to react to
changes in memory pressure/usage patterns.
And another big NAK. TTM priorities are meant to be static based on in
kernel decisions which are not exposed to userspace.
In other words we can group BOs based on kernel, user, SVM etc... but
never on something userspace can influence.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/ttm/ttm_bo.c | 17 +
include/drm/ttm/ttm_bo.h | 2 ++
2 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index eae54cd4a7ce9..6ac939c58a6b8 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -112,6 +112,23 @@ void ttm_bo_set_bulk_move(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_bo_set_bulk_move);
+void ttm_bo_update_priority(struct ttm_buffer_object *bo, unsigned int
new_prio)
+{
+ struct ttm_resource_manager *man;
+
+ if (!bo->resource)
+ return;
+
+ man = ttm_manager_type(bo->bdev, bo->resource->mem_type);
+
+ spin_lock(>bdev->lru_lock);
+ ttm_resource_del_bulk_move(bo->resource, bo);
+ bo->priority = new_prio;
+ ttm_resource_add_bulk_move(bo->resource, bo);
+ spin_unlock(>bdev->lru_lock);
+}
+EXPORT_SYMBOL(ttm_bo_update_priority);
+
static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
struct ttm_resource *mem, bool evict,
struct ttm_operation_ctx *ctx,
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 91299a3b6fcfa..51040bc443ea0 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -359,6 +359,8 @@ static inline void *ttm_kmap_obj_virtual(struct
ttm_bo_kmap_obj *map,
return map->virtual;
}
+void ttm_bo_update_priority(struct ttm_buffer_object *bo,
+ unsigned int new_prio);
int ttm_bo_wait_ctx(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx);
--
2.44.0
Re: [PATCH] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq()
On 25/04/2024 02:20, Douglas Anderson wrote:
The consensus of many DRM folks is that we want to move away from DSI
drivers defining tables of init commands. Instead, we want to move to
init functions that can use common DRM functions. The issue thus far
has been that using the macros mipi_dsi_generic_write_seq() and
mipi_dsi_dcs_write_seq() bloats the driver using them.
Through a cooperative effort between Hsin-Yi Wang and Dmitry
Baryshkov, we have realized that the majority of the bloat is the fact
that we have the dev_err_ratelimited() directly in the macros. Let's
hoist this call into drm_mipi_dsi.c by adding a "chatty" version of
the functions that includes the print.
Without any changes to clients this gives a dramatic savings. Building
with my build system shows one example:
$ scripts/bloat-o-meter \
.../before/panel-novatek-nt36672e.ko \
.../after/panel-novatek-nt36672e.ko
add/remove: 0/1 grow/shrink: 1/1 up/down: 6/-19652 (-19646)
Function old new delta
__UNIQUE_ID_vermagic520 64 70 +6
nt36672e_1080x2408_60hz_init 165927260 -9332
nt36672e_1080x2408_60hz_init._rs 10320 - -10320
Total: Before=31503, After=11857, chg -62.36%
Note that given the change in location of the print it's harder to
include the "cmd" in the printout for mipi_dsi_dcs_write_seq() since,
theoretically, someone could call the new chatty function with a
zero-size array and it would be illegal to dereference data[0].
There's a printk format to print the whole buffer and this is probably
more useful for debugging anyway. Given that we're doing this for
mipi_dsi_dcs_write_seq(), let's also print the buffer for
mipi_dsi_generic_write_seq() in the error case.
Signed-off-by: Douglas Anderson
---
The MIPI device I have in front of me (wormdingler) hasn't been
converted to use these functions yet, so this is just compile
tested. It's about my end of day so I'm sending this out since it's
pretty straightforward. Hopefully others can confirm it works well for
them and also saves space under their compilers.
drivers/gpu/drm/drm_mipi_dsi.c | 54 ++
include/drm/drm_mipi_dsi.h | 31 ---
2 files changed, 67 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 795001bb7ff1..5ded6aef38ed 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -764,6 +764,33 @@ ssize_t mipi_dsi_generic_write(struct mipi_dsi_device
*dsi, const void *payload,
}
EXPORT_SYMBOL(mipi_dsi_generic_write);
+/**
+ * mipi_dsi_generic_write_chatty() - mipi_dsi_generic_write() w/ an error log
+ * @dsi: DSI peripheral device
+ * @payload: buffer containing the payload
+ * @size: size of payload buffer
+ *
+ * Just like mipi_dsi_generic_write() but includes a dev_err_ratelimited()
+ * call for you.
+ *
+ * Return: The number of bytes transmitted on success or a negative error code
+ * on failure.
+ */
+ssize_t mipi_dsi_generic_write_chatty(struct mipi_dsi_device *dsi,
+ const void *payload, size_t size)
+{
+ struct device *dev = >dev;
+ int ret;
+
+ ret = mipi_dsi_generic_write(dsi, payload, size);
+ if (ret < 0)
+ dev_err_ratelimited(dev, "sending generic data %*ph failed:
%d\n",
+ (int)size, payload, ret);
+
+ return ret;
+}
+EXPORT_SYMBOL(mipi_dsi_generic_write_chatty);
+
/**
* mipi_dsi_generic_read() - receive data using a generic read packet
* @dsi: DSI peripheral device
@@ -852,6 +879,33 @@ ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device
*dsi,
}
EXPORT_SYMBOL(mipi_dsi_dcs_write_buffer);
+/**
+ * mipi_dsi_dcs_write_buffer_chatty - mipi_dsi_dcs_write_buffer() w/ an error
log
+ * @dsi: DSI peripheral device
+ * @data: buffer containing data to be transmitted
+ * @len: size of transmission buffer
+ *
+ * Just like mipi_dsi_dcs_write_buffer() but includes a dev_err_ratelimited()
+ * call for you.
+ *
+ * Return: The number of bytes successfully transmitted or a negative error
+ * code on failure.
+ */
+ssize_t mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi,
+const void *data, size_t len)
+{
+ struct device *dev = >dev;
+ int ret;
+
+ ret = mipi_dsi_dcs_write_buffer(dsi, data, len);
+ if (ret < 0)
+ dev_err_ratelimited(dev, "sending dcs data %*ph failed: %d\n",
+ (int)len, data, ret);
+
+ return ret;
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_write_buffer_chatty);
+
/**
* mipi_dsi_dcs_write() - send DCS write command
* @dsi: DSI peripheral device
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 82b1cc434ea3..784e425dc4c8 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -256,6 +256,8 @@
Re: [RFC PATCH 10/18] drm/amdgpu: Don't add GTT to initial domains after failing to allocate VRAM
Am 25.04.24 um 09:39 schrieb Friedrich Vock:
On 25.04.24 08:25, Christian König wrote:
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
This adds GTT to the "preferred domains" of this buffer object, which
will also prevent any attempts at moving the buffer back to VRAM if
there is space. If VRAM is full, GTT will already be chosen as a
fallback.
Big NAK to that one, this is mandatory for correct operation.
Hm, how is correctness affected here? We still fall back to GTT if
allocating in VRAM doesn't work, I don't see a difference except that
now we'll actually try moving it back into VRAM again.
Well this is the fallback. Only during CS we try to allocate from GTT if
allocating in VRAM doesn't work.
When you remove this here then any failed allocation from VRAM would be
fatal.
What could be is that the handling is buggy and when we update the
initial domain we also add GTT to the preferred domain, but that should
then be fixed.
Regards,
Christian.
Regards,
Friedrich
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 6bbab141eaaeb..aea3770d3ea2e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -378,10 +378,6 @@ int amdgpu_gem_create_ioctl(struct drm_device
*dev, void *data,
goto retry;
}
- if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
- initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
- goto retry;
- }
DRM_DEBUG("Failed to allocate GEM object (%llu, %d, %llu,
%d)\n",
size, initial_domain, args->in.alignment, r);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 85c10d8086188..9978b85ed6f40 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -619,7 +619,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
AMDGPU_GEM_DOMAIN_GDS))
amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
else
- amdgpu_bo_placement_from_domain(bo, bp->domain);
+ amdgpu_bo_placement_from_domain(bo, bo->allowed_domains);
if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 2;
else if (!(bp->flags & AMDGPU_GEM_CREATE_DISCARDABLE))
--
2.44.0
Re: [PATCH 1/2] drm/bridge: chipone-icn6211: drop driver owner assignment
On Thu, 25 Apr 2024 at 10:47, Neil Armstrong wrote: > > Hi Dmitry, > > On 24/04/2024 11:12, Dmitry Baryshkov wrote: > > On Sat, Mar 30, 2024 at 09:27:40PM +0100, Krzysztof Kozlowski wrote: > >> Core in mipi_dsi_driver_register() already sets the .owner, so driver > >> does not need to. > >> > >> Signed-off-by: Krzysztof Kozlowski > >> --- > >> drivers/gpu/drm/bridge/chipone-icn6211.c | 1 - > >> 1 file changed, 1 deletion(-) > >> > > > > Reviewed-by: Dmitry Baryshkov > > > > > > I tried to apply them but you already applied them... could you make sure to > notify the list when applying ? Ack, excuse me. -- With best wishes Dmitry
Re: [PATCH v3 1/7] dt-bindings: display: panel: Add himax hx83102 panel bindings
Hi,
Thanks for review.
Conor Dooley 于2024年4月25日周四 00:55写道：
>
> On Wed, Apr 24, 2024 at 10:30:04AM +0800, Cong Yang wrote:
> > In V1, discussed with Doug and Linus [1], we need break out as separate
> > driver for the himax83102-j02 controller. Beacuse "starry,himax83102-j02"
> > and in this series "BOE nv110wum-l60" "IVO t109nw41" panels use same
> > controller, they have some common CMDS. So add new documentation for
> > this panels.
>
> It'd be good to note in the commit message that the 3v3 supply is not
> present on these panels, given it was present in the other binding and
> not here.
Got it, fix in V4,thanks.
>
> > [1]:
> > https://lore.kernel.org/all/CACRpkdbzYZAS0=zbqjuc4cb2wj4s1h6n6asazqvdmv95r3z...@mail.gmail.com
> >
> > Signed-off-by: Cong Yang
> > ---
> > Chage since V3:
> >
> > - Update commit message.
> >
> > V2:
> > https://lore.kernel.org/all/20240422090310.3311429-2-yangco...@huaqin.corp-partner.google.com
> >
> > ---
> > .../display/panel/boe,tv101wum-nl6.yaml | 2 -
> > .../bindings/display/panel/himax,hx83102.yaml | 73 +++
> > 2 files changed, 73 insertions(+), 2 deletions(-)
> > create mode 100644
> > Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > index 906ef62709b8..53fb35f5c9de 100644
> > --- a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > @@ -32,8 +32,6 @@ properties:
> >- innolux,hj110iz-01a
> > # STARRY 2081101QFH032011-53G 10.1" WUXGA TFT LCD panel
> >- starry,2081101qfh032011-53g
> > -# STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> > - - starry,himax83102-j02
> > # STARRY ili9882t 10.51" WUXGA TFT LCD panel
> >- starry,ili9882t
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> > b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> > new file mode 100644
> > index ..2e0cd6998ba8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
>
> Filename matching a compatible please. What you've done here makes it
> seem like there's a fallback compatible missing, given this looks like
> the LCD panel controller and the starry compatible below is an LCD panel.
So change the filename to starry,himax83102-j02.yaml?
>
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/himax,hx83102.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Himax HX83102 MIPI-DSI LCD panel controller
> > +
> > +maintainers:
> > + - Cong Yang
> > +
> > +allOf:
> > + - $ref: panel-common.yaml#
> > +
> > +properties:
> > + compatible:
> > +enum:
> > +# STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> > + - starry,himax83102-j02
> > +
> > + reg:
> > +description: the virtual channel number of a DSI peripheral
> > +
> > + enable-gpios:
> > +description: a GPIO spec for the enable pin
> > +
> > + pp1800-supply:
> > +description: core voltage supply
> > +
> > + avdd-supply:
> > +description: phandle of the regulator that provides positive voltage
> > +
> > + avee-supply:
> > +description: phandle of the regulator that provides negative voltage
> > +
> > + backlight:
> > +description: phandle of the backlight device attached to the panel
>
> I'm not sure why this was given a description when port or rotation
> was not.
So change it to backlight: true ?
Thanks.
>
> Otherwise, this looks fine to me.
>
> Cheers,
> Conor.
>
> > +
> > + port: true
> > + rotation: true
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - enable-gpios
> > + - pp1800-supply
> > + - avdd-supply
> > + - avee-supply
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +dsi {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +panel@0 {
> > +compatible = "starry,himax83102-j02";
> > +reg = <0>;
> > +enable-gpios = < 45 0>;
> > +avdd-supply = <_lcd>;
> > +avee-supply = <_lcd>;
> > +pp1800-supply = <_lcd>;
> > +backlight = <_lcd0>;
> > +port {
> > +panel_in: endpoint {
> > +remote-endpoint = <_out>;
> > +};
> > +};
> > +};
> > +};
> > +
> > +...
> > --
> > 2.25.1
> >
Re: [RFC PATCH 10/18] drm/amdgpu: Don't add GTT to initial domains after failing to allocate VRAM
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
This adds GTT to the "preferred domains" of this buffer object, which
will also prevent any attempts at moving the buffer back to VRAM if
there is space. If VRAM is full, GTT will already be chosen as a
fallback.
Big NAK to that one, this is mandatory for correct operation.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 4
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 6bbab141eaaeb..aea3770d3ea2e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -378,10 +378,6 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void
*data,
goto retry;
}
- if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
- initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
- goto retry;
- }
DRM_DEBUG("Failed to allocate GEM object (%llu, %d, %llu,
%d)\n",
size, initial_domain, args->in.alignment, r);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 85c10d8086188..9978b85ed6f40 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -619,7 +619,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
AMDGPU_GEM_DOMAIN_GDS))
amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
else
- amdgpu_bo_placement_from_domain(bo, bp->domain);
+ amdgpu_bo_placement_from_domain(bo, bo->allowed_domains);
if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 2;
else if (!(bp->flags & AMDGPU_GEM_CREATE_DISCARDABLE))
--
2.44.0
Re: [PATCH v3 03/18] ASoC: dt-bindings: mt6357: Add audio codec document
On 23/04/2024 19:07, Alexandre Mergnat wrote:
>
>
> On 09/04/2024 17:55, Krzysztof Kozlowski wrote:
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> +codec {
>>> +mediatek,micbias0-microvolt = <190>;
>>> +mediatek,micbias1-microvolt = <170>;
>>> +mediatek,vaud28-supply = <_vaud28_reg>;
>> Sorry, this does not work. Change voltage to 111 and check the results.
>
> Actually it's worst ! I've removed the required property (vaud28-supply) but
> the dt check pass.
> Same behavior for some other docs like mt6359.yaml
Yeah, the schema is not applied. There is nothing selecting it, so this
is no-op schema. I don't know what exactly you want to describe, but
usually either you miss compatible or this should be just part of parent
node.
>
> The at24.yaml doc works as expected, then I tried compare an find the issue,
> without success...
>
> I've replaced "codec" by "audio-codec", according to [1].
> I've tried multiple manner to implement the example code, without success.
> I'm wondering if what I
> try to do is the correct way or parse-able by the dt_check.
>
> If I drop this file and implement all these new properties into the MFD PMIC
> documentation directly,
> I've the expected dt_check result (function to good or wrong parameters)
Yes.
Best regards,
Krzysztof
Re: [RFC PATCH 00/18] TTM interface for managing VRAM oversubscription
In general: Yes please :) But are exercising a lot of ideas we have already thrown over board over the years. The general idea Marek and I have been working on for a while now is rather to make TTM aware of userspace "clients". In other words we should start with having a TTM structure in the fpriv of the drivers and then track there how much VRAM was evicted for each client. This should then be balanced so that each client gets it's equal share of VRAM and we pretty much end up with a static situation which only changes when applications become inactive/active (based on their GPU activity). I will mail you some of the stuff we already came up with later on. Regards, Christian. Am 24.04.24 um 18:56 schrieb Friedrich Vock: 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
Re: [PATCH v6 05/14] drm/mediatek: Set DRM mode configs accordingly
Re: [PATCH 02/10] pwm: Add SI-EN SN3112 PWM support
On 2024/4/25 14:02, Uwe Kleine-König wrote:
> Hello,
>
> On Wed, Apr 24, 2024 at 11:29:07PM +0800, Xilin Wu via B4 Relay wrote:
>> From: Junhao Xie
>>
>> Add a new driver for the SI-EN SN3112 12-channel 8-bit PWM LED controller.
>>
[...]
>> +MODULE_LICENSE("GPL");
>
> Best regards
> Uwe
>
Thank you for your reply! I will fix them and resend this commit.
This is the link of datasheet for SI-EN SN3112, but it is written in Chinese:
https://datasheetspdf.com/pdf-down/S/N/3/SN3112-12-SI-EN.pdf
[PATCH] drm/panthor: Make sure we handled the unknown group state properly
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
---
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);
}
}
--
2.44.0
Re: [RFC PATCH 02/18] drm/ttm: Add per-BO eviction tracking
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.
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: [RFC PATCH 16/18] drm/amdgpu: Implement SET_PRIORITY GEM op
Am 25.04.24 um 08:46 schrieb Friedrich Vock:
On 25.04.24 08:32, Christian König wrote:
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
Used by userspace to adjust buffer priorities in response to changes in
application demand and memory pressure.
Yeah, that was discussed over and over again. One big design criteria
is that we can't have global priorities from userspace!
The background here is that this can trivially be abused.
I see your point when apps are allowed to prioritize themselves above
other apps, and I agree that should probably be disallowed at least for
unprivileged apps.
Disallowing this is a pretty trivial change though, and I don't really
see the abuse potential in being able to downgrade your own priority?
Yeah, I know what you mean and I'm also leaning towards that
argumentation. But another good point is also that it doesn't actually help.
For example when you have desktop apps fighting with a game, you
probably don't want to use static priorities, but rather evict the apps
which are inactive and keep the apps which are active in the background.
In other words the priority just tells you which stuff from each app to
evict first, but not which app to globally throw out.
Regards,
Christian.
Regards,
Friedrich
What we can do is to have per process priorities, but that needs to be
in the VM subsystem.
That's also the reason why I personally think that the handling
shouldn't be inside TTM at all.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 20
include/uapi/drm/amdgpu_drm.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 5ca13e2e50f50..6107810a9c205 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -836,8 +836,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
{
struct amdgpu_device *adev = drm_to_adev(dev);
struct drm_amdgpu_gem_op *args = data;
+ struct ttm_resource_manager *man;
struct drm_gem_object *gobj;
struct amdgpu_vm_bo_base *base;
+ struct ttm_operation_ctx ctx;
struct amdgpu_bo *robj;
int r;
@@ -851,6 +853,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
if (unlikely(r))
goto out;
+ memset(, 0, sizeof(ctx));
+ ctx.interruptible = true;
+
switch (args->op) {
case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
struct drm_amdgpu_gem_create_in info;
@@ -898,6 +903,21 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
amdgpu_bo_unreserve(robj);
break;
+ case AMDGPU_GEM_OP_SET_PRIORITY:
+ if (args->value > AMDGPU_BO_PRIORITY_MAX_USER)
+ args->value = AMDGPU_BO_PRIORITY_MAX_USER;
+ ttm_bo_update_priority(>tbo, args->value);
+ if (robj->tbo.evicted_type != TTM_NUM_MEM_TYPES) {
+ ttm_bo_try_unevict(>tbo, );
+ amdgpu_bo_unreserve(robj);
+ } else {
+ amdgpu_bo_unreserve(robj);
+ man = ttm_manager_type(robj->tbo.bdev,
+ robj->tbo.resource->mem_type);
+ ttm_mem_unevict_evicted(robj->tbo.bdev, man,
+ true);
+ }
+ break;
default:
amdgpu_bo_unreserve(robj);
r = -EINVAL;
diff --git a/include/uapi/drm/amdgpu_drm.h
b/include/uapi/drm/amdgpu_drm.h
index bdbe6b262a78d..53552dd489b9b 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -531,6 +531,7 @@ union drm_amdgpu_wait_fences {
#define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
#define AMDGPU_GEM_OP_SET_PLACEMENT 1
+#define AMDGPU_GEM_OP_SET_PRIORITY 2
/* Sets or returns a value associated with a buffer. */
struct drm_amdgpu_gem_op {
--
2.44.0
Re: [PATCH 1/2] drm/bridge: chipone-icn6211: drop driver owner assignment
Hi Dmitry, On 24/04/2024 11:12, Dmitry Baryshkov wrote: On Sat, Mar 30, 2024 at 09:27:40PM +0100, Krzysztof Kozlowski wrote: Core in mipi_dsi_driver_register() already sets the .owner, so driver does not need to. Signed-off-by: Krzysztof Kozlowski --- drivers/gpu/drm/bridge/chipone-icn6211.c | 1 - 1 file changed, 1 deletion(-) Reviewed-by: Dmitry Baryshkov I tried to apply them but you already applied them... could you make sure to notify the list when applying ? Thanks, Neil
Re: [PATCH 02/10] pwm: Add SI-EN SN3112 PWM support
On 25/04/2024 02:57, Junhao Xie wrote: On 2024/4/25 03:37, Konrad Dybcio wrote: On 4/24/24 17:29, Xilin Wu via B4 Relay wrote: From: Junhao Xie Add a new driver for the SI-EN SN3112 12-channel 8-bit PWM LED controller. Signed-off-by: Junhao Xie --- [...] + return sn3112_write_reg(priv, SN3112_REG_PWM_EN + reg, + priv->pwm_en_reg[reg]); This looks like a weird reimplementation of regmap_update_bits We cannot use regmap_update_bits because this chip does not support read command. It will discard all read command. You could use regmap cache with all registers marked as cacheable, but not sure it's worth doing this. Neil +} + [...] devm_pwmchip_add? Konrad Thank you for your reply, I will fix them.
Re: [PATCH 01/10] dt-bindings: pwm: Add SI-EN SN3112 PWM support
Hello, On Wed, Apr 24, 2024 at 11:29:06PM +0800, Xilin Wu via B4 Relay wrote: > From: Junhao Xie > > Add a new driver for the SI-EN SN3112 12-channel 8-bit PWM LED controller. > > Signed-off-by: Junhao Xie Missing S-o-b for patch submitter. > + "#pwm-cells": > +const: 1 please use 3 here (which is also what the driver implements) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH 02/10] pwm: Add SI-EN SN3112 PWM support
Hello,
On Wed, Apr 24, 2024 at 04:55:26PM +0100, Bryan O'Donoghue wrote:
> On 24/04/2024 16:29, Xilin Wu via B4 Relay wrote:
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> Includes should be alphabetised
Also check if you need them all. (E.g. I wonder about delay.h)
> > + dev_dbg(priv->pdev, "request regmap_write 0x%x 0x%x\n", reg, val);
> > + err = regmap_write(priv->regmap, reg, val);
> > + if (err)
> > + dev_warn_ratelimited(
> > + priv->pdev,
> > + "regmap_write to register 0x%x failed: %pe\n", reg,
> > + ERR_PTR(err));
>
> Multi-line should be encapsulated in {}
>
> if (err) {
> stuff
> goes here
> }
In my eyes a single state doesn't need {} even when spanning multiple
lines.
> > + return err;
> > +}
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [RFC PATCH 12/18] drm/ttm: Do not evict BOs with higher priority
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
This makes buffer eviction significantly more stable by avoiding
ping-ponging caused by low-priority buffers evicting high-priority
buffers and vice versa.
And creates a deny of service for the whole system by fork() bombing.
This is another very big NAK.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/ttm/ttm_bo.c | 9 +++--
drivers/gpu/drm/ttm/ttm_resource.c | 5 +++--
include/drm/ttm/ttm_bo.h | 1 +
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3047c763eb4eb..eae54cd4a7ce9 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -776,6 +776,7 @@ static int ttm_mem_evict_wait_busy(struct ttm_buffer_object
*busy_bo,
int ttm_mem_evict_first(struct ttm_device *bdev,
struct ttm_resource_manager *man,
const struct ttm_place *place,
+ unsigned int max_priority,
struct ttm_operation_ctx *ctx,
struct ww_acquire_ctx *ticket)
{
@@ -788,6 +789,8 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
spin_lock(>lru_lock);
ttm_resource_manager_for_each_res(man, , res) {
bool busy;
+ if (res->bo->priority > max_priority)
+ break;
if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place,
, )) {
@@ -930,8 +933,10 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object
*bo,
return ret;
if (ctx->no_evict)
return -ENOSPC;
- ret = ttm_mem_evict_first(bdev, man, place, ctx,
- ticket);
+ if (!bo->priority)
+ return -ENOSPC;
+ ret = ttm_mem_evict_first(bdev, man, place, bo->priority - 1,
+ ctx, ticket);
if (unlikely(ret != 0))
return ret;
} while (1);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
b/drivers/gpu/drm/ttm/ttm_resource.c
index 1d6755a1153b1..63d4371adb519 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -431,8 +431,9 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev,
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
while (!list_empty(>lru[i])) {
spin_unlock(>lru_lock);
- ret = ttm_mem_evict_first(bdev, man, NULL, ,
- NULL);
+ ret = ttm_mem_evict_first(bdev, man, NULL,
+ TTM_MAX_BO_PRIORITY,
+ , NULL);
if (ret)
return ret;
spin_lock(>lru_lock);
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 8f4e6366c0417..91299a3b6fcfa 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -396,6 +396,7 @@ void ttm_bo_unpin(struct ttm_buffer_object *bo);
int ttm_mem_evict_first(struct ttm_device *bdev,
struct ttm_resource_manager *man,
const struct ttm_place *place,
+ unsigned int max_priority,
struct ttm_operation_ctx *ctx,
struct ww_acquire_ctx *ticket);
void ttm_mem_unevict_evicted(struct ttm_device *bdev,
--
2.44.0
Re: [RFC PATCH 16/18] drm/amdgpu: Implement SET_PRIORITY GEM op
On 25.04.24 08:58, Christian König wrote:
Am 25.04.24 um 08:46 schrieb Friedrich Vock:
On 25.04.24 08:32, Christian König wrote:
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
Used by userspace to adjust buffer priorities in response to
changes in
application demand and memory pressure.
Yeah, that was discussed over and over again. One big design criteria
is that we can't have global priorities from userspace!
The background here is that this can trivially be abused.
I see your point when apps are allowed to prioritize themselves above
other apps, and I agree that should probably be disallowed at least for
unprivileged apps.
Disallowing this is a pretty trivial change though, and I don't really
see the abuse potential in being able to downgrade your own priority?
Yeah, I know what you mean and I'm also leaning towards that
argumentation. But another good point is also that it doesn't actually
help.
For example when you have desktop apps fighting with a game, you
probably don't want to use static priorities, but rather evict the
apps which are inactive and keep the apps which are active in the
background.
Sadly things are not as simple as "evict everything from app 1, keep
everything from app 2 active". The simplest failure case of this is
games that already oversubscribe VRAM on their own. Keeping the whole
app inside VRAM is literally impossible there, and it helps a lot to
know which buffers the app is most happy with evicting.
In other words the priority just tells you which stuff from each app
to evict first, but not which app to globally throw out.
Yeah, but per-buffer priority system could do both of these.
Regards,
Friedrich
Regards,
Christian.
Regards,
Friedrich
What we can do is to have per process priorities, but that needs to be
in the VM subsystem.
That's also the reason why I personally think that the handling
shouldn't be inside TTM at all.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 20
include/uapi/drm/amdgpu_drm.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 5ca13e2e50f50..6107810a9c205 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -836,8 +836,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
{
struct amdgpu_device *adev = drm_to_adev(dev);
struct drm_amdgpu_gem_op *args = data;
+ struct ttm_resource_manager *man;
struct drm_gem_object *gobj;
struct amdgpu_vm_bo_base *base;
+ struct ttm_operation_ctx ctx;
struct amdgpu_bo *robj;
int r;
@@ -851,6 +853,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
if (unlikely(r))
goto out;
+ memset(, 0, sizeof(ctx));
+ ctx.interruptible = true;
+
switch (args->op) {
case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
struct drm_amdgpu_gem_create_in info;
@@ -898,6 +903,21 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
amdgpu_bo_unreserve(robj);
break;
+ case AMDGPU_GEM_OP_SET_PRIORITY:
+ if (args->value > AMDGPU_BO_PRIORITY_MAX_USER)
+ args->value = AMDGPU_BO_PRIORITY_MAX_USER;
+ ttm_bo_update_priority(>tbo, args->value);
+ if (robj->tbo.evicted_type != TTM_NUM_MEM_TYPES) {
+ ttm_bo_try_unevict(>tbo, );
+ amdgpu_bo_unreserve(robj);
+ } else {
+ amdgpu_bo_unreserve(robj);
+ man = ttm_manager_type(robj->tbo.bdev,
+ robj->tbo.resource->mem_type);
+ ttm_mem_unevict_evicted(robj->tbo.bdev, man,
+ true);
+ }
+ break;
default:
amdgpu_bo_unreserve(robj);
r = -EINVAL;
diff --git a/include/uapi/drm/amdgpu_drm.h
b/include/uapi/drm/amdgpu_drm.h
index bdbe6b262a78d..53552dd489b9b 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -531,6 +531,7 @@ union drm_amdgpu_wait_fences {
#define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
#define AMDGPU_GEM_OP_SET_PLACEMENT 1
+#define AMDGPU_GEM_OP_SET_PRIORITY 2
/* Sets or returns a value associated with a buffer. */
struct drm_amdgpu_gem_op {
--
2.44.0
Re: [RFC PATCH 10/18] drm/amdgpu: Don't add GTT to initial domains after failing to allocate VRAM
On 25.04.24 08:25, Christian König wrote:
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
This adds GTT to the "preferred domains" of this buffer object, which
will also prevent any attempts at moving the buffer back to VRAM if
there is space. If VRAM is full, GTT will already be chosen as a
fallback.
Big NAK to that one, this is mandatory for correct operation.
Hm, how is correctness affected here? We still fall back to GTT if
allocating in VRAM doesn't work, I don't see a difference except that
now we'll actually try moving it back into VRAM again.
Regards,
Friedrich
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 6bbab141eaaeb..aea3770d3ea2e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -378,10 +378,6 @@ int amdgpu_gem_create_ioctl(struct drm_device
*dev, void *data,
goto retry;
}
- if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
- initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
- goto retry;
- }
DRM_DEBUG("Failed to allocate GEM object (%llu, %d, %llu,
%d)\n",
size, initial_domain, args->in.alignment, r);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 85c10d8086188..9978b85ed6f40 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -619,7 +619,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
AMDGPU_GEM_DOMAIN_GDS))
amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
else
- amdgpu_bo_placement_from_domain(bo, bp->domain);
+ amdgpu_bo_placement_from_domain(bo, bo->allowed_domains);
if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 2;
else if (!(bp->flags & AMDGPU_GEM_CREATE_DISCARDABLE))
--
2.44.0
Re: [RFC PATCH 16/18] drm/amdgpu: Implement SET_PRIORITY GEM op
On 25.04.24 09:15, Christian König wrote:
Am 25.04.24 um 09:06 schrieb Friedrich Vock:
On 25.04.24 08:58, Christian König wrote:
Am 25.04.24 um 08:46 schrieb Friedrich Vock:
On 25.04.24 08:32, Christian König wrote:
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
Used by userspace to adjust buffer priorities in response to
changes in
application demand and memory pressure.
Yeah, that was discussed over and over again. One big design criteria
is that we can't have global priorities from userspace!
The background here is that this can trivially be abused.
I see your point when apps are allowed to prioritize themselves above
other apps, and I agree that should probably be disallowed at least
for
unprivileged apps.
Disallowing this is a pretty trivial change though, and I don't really
see the abuse potential in being able to downgrade your own priority?
Yeah, I know what you mean and I'm also leaning towards that
argumentation. But another good point is also that it doesn't actually
help.
For example when you have desktop apps fighting with a game, you
probably don't want to use static priorities, but rather evict the
apps which are inactive and keep the apps which are active in the
background.
Sadly things are not as simple as "evict everything from app 1, keep
everything from app 2 active". The simplest failure case of this is
games that already oversubscribe VRAM on their own. Keeping the whole
app inside VRAM is literally impossible there, and it helps a lot to
know which buffers the app is most happy with evicting.
In other words the priority just tells you which stuff from each app
to evict first, but not which app to globally throw out.
Yeah, but per-buffer priority system could do both of these.
Yeah, but we already have that. See amdgpu_bo_list_entry_cmp() and
amdgpu_bo_list_create().
This is the per application priority which can be used by userspace to
give priority to each BO in a submission (or application wide).
The problem is rather that amdgpu/TTM never really made good use of
that information.
I think it's nigh impossible to make good use of priority information if
you wrap it in the BO list which you only know on submit. For example,
you don't know when priorities change unless you duplicate all the
tracking work (that the application has to do too!) in the kernel. You
also have no way of knowing the priority changed until right when the
app wants to submit work using that BO, and starting to move BOs around
at that point is bad for submission latency. That's why I didn't go
forward with tracking priorities on a BO-list basis.
Also, the priorities being local to a submission is actually not that
great when talking about lowering priorities. Consider a case where an
app's working set fits into VRAM completely, but combined with the
working set of other apps running in parallel, VRAM is oversubscribed.
The app recognizes this and asks the kernel to evict one of its
rarely-used buffers by setting the priority to the lowest possible, to
make space for the other applications.
Without global priorities, the kernel can't honor that request, even
though it would solve the oversubscription with minimal performance
impact. Even with per-app priorities, the kernel isn't likely to evict
buffers from the requesting application unless all the other
applications have a higher priority.
Regards,
Friedrich
Regards,
Christian.
Regards,
Friedrich
Regards,
Christian.
Regards,
Friedrich
What we can do is to have per process priorities, but that needs
to be
in the VM subsystem.
That's also the reason why I personally think that the handling
shouldn't be inside TTM at all.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 20
include/uapi/drm/amdgpu_drm.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 5ca13e2e50f50..6107810a9c205 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -836,8 +836,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
{
struct amdgpu_device *adev = drm_to_adev(dev);
struct drm_amdgpu_gem_op *args = data;
+ struct ttm_resource_manager *man;
struct drm_gem_object *gobj;
struct amdgpu_vm_bo_base *base;
+ struct ttm_operation_ctx ctx;
struct amdgpu_bo *robj;
int r;
@@ -851,6 +853,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
if (unlikely(r))
goto out;
+ memset(, 0, sizeof(ctx));
+ ctx.interruptible = true;
+
switch (args->op) {
case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
struct drm_amdgpu_gem_create_in info;
@@ -898,6 +903,21 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
amdgpu_bo_unreserve(robj);
break;
+ case AMDGPU_GEM_OP_SET_PRIORITY:
+ if (args->value >
Re: [PATCH] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq()
On Wed, 24 Apr 2024, Douglas Anderson wrote:
> The consensus of many DRM folks is that we want to move away from DSI
> drivers defining tables of init commands. Instead, we want to move to
> init functions that can use common DRM functions. The issue thus far
> has been that using the macros mipi_dsi_generic_write_seq() and
> mipi_dsi_dcs_write_seq() bloats the driver using them.
>
> Through a cooperative effort between Hsin-Yi Wang and Dmitry
> Baryshkov, we have realized that the majority of the bloat is the fact
> that we have the dev_err_ratelimited() directly in the macros. Let's
> hoist this call into drm_mipi_dsi.c by adding a "chatty" version of
> the functions that includes the print.
>
> Without any changes to clients this gives a dramatic savings. Building
> with my build system shows one example:
>
> $ scripts/bloat-o-meter \
> .../before/panel-novatek-nt36672e.ko \
> .../after/panel-novatek-nt36672e.ko
>
> add/remove: 0/1 grow/shrink: 1/1 up/down: 6/-19652 (-19646)
> Function old new delta
> __UNIQUE_ID_vermagic520 64 70 +6
> nt36672e_1080x2408_60hz_init 165927260 -9332
> nt36672e_1080x2408_60hz_init._rs 10320 - -10320
> Total: Before=31503, After=11857, chg -62.36%
>
> Note that given the change in location of the print it's harder to
> include the "cmd" in the printout for mipi_dsi_dcs_write_seq() since,
> theoretically, someone could call the new chatty function with a
> zero-size array and it would be illegal to dereference data[0].
> There's a printk format to print the whole buffer and this is probably
> more useful for debugging anyway. Given that we're doing this for
> mipi_dsi_dcs_write_seq(), let's also print the buffer for
> mipi_dsi_generic_write_seq() in the error case.
>
> Signed-off-by: Douglas Anderson
> ---
> The MIPI device I have in front of me (wormdingler) hasn't been
> converted to use these functions yet, so this is just compile
> tested. It's about my end of day so I'm sending this out since it's
> pretty straightforward. Hopefully others can confirm it works well for
> them and also saves space under their compilers.
>
> drivers/gpu/drm/drm_mipi_dsi.c | 54 ++
> include/drm/drm_mipi_dsi.h | 31 ---
> 2 files changed, 67 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 795001bb7ff1..5ded6aef38ed 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -764,6 +764,33 @@ ssize_t mipi_dsi_generic_write(struct mipi_dsi_device
> *dsi, const void *payload,
> }
> EXPORT_SYMBOL(mipi_dsi_generic_write);
>
> +/**
> + * mipi_dsi_generic_write_chatty() - mipi_dsi_generic_write() w/ an error log
> + * @dsi: DSI peripheral device
> + * @payload: buffer containing the payload
> + * @size: size of payload buffer
> + *
> + * Just like mipi_dsi_generic_write() but includes a dev_err_ratelimited()
> + * call for you.
> + *
> + * Return: The number of bytes transmitted on success or a negative error
> code
> + * on failure.
> + */
> +ssize_t mipi_dsi_generic_write_chatty(struct mipi_dsi_device *dsi,
> + const void *payload, size_t size)
> +{
> + struct device *dev = >dev;
> + int ret;
> +
> + ret = mipi_dsi_generic_write(dsi, payload, size);
> + if (ret < 0)
> + dev_err_ratelimited(dev, "sending generic data %*ph failed:
> %d\n",
> + (int)size, payload, ret);
Why does this even need to be ratelimited? See below.
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(mipi_dsi_generic_write_chatty);
> +
> /**
> * mipi_dsi_generic_read() - receive data using a generic read packet
> * @dsi: DSI peripheral device
> @@ -852,6 +879,33 @@ ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device
> *dsi,
> }
> EXPORT_SYMBOL(mipi_dsi_dcs_write_buffer);
>
> +/**
> + * mipi_dsi_dcs_write_buffer_chatty - mipi_dsi_dcs_write_buffer() w/ an
> error log
> + * @dsi: DSI peripheral device
> + * @data: buffer containing data to be transmitted
> + * @len: size of transmission buffer
> + *
> + * Just like mipi_dsi_dcs_write_buffer() but includes a dev_err_ratelimited()
> + * call for you.
> + *
> + * Return: The number of bytes successfully transmitted or a negative error
> + * code on failure.
> + */
> +ssize_t mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi,
> + const void *data, size_t len)
> +{
> + struct device *dev = >dev;
> + int ret;
> +
> + ret = mipi_dsi_dcs_write_buffer(dsi, data, len);
> + if (ret < 0)
> + dev_err_ratelimited(dev, "sending dcs data %*ph failed: %d\n",
> + (int)len, data, ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_write_buffer_chatty);
> +
> /**
> * mipi_dsi_dcs_write() - send
Re: [PATCH 02/10] pwm: Add SI-EN SN3112 PWM support
Hello,
On Wed, Apr 24, 2024 at 11:29:07PM +0800, Xilin Wu via B4 Relay wrote:
> From: Junhao Xie
>
> Add a new driver for the SI-EN SN3112 12-channel 8-bit PWM LED controller.
>
> Signed-off-by: Junhao Xie
Missing S-o-b for patch sender.
> ---
> drivers/pwm/Kconfig | 10 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-sn3112.c | 336
> +++
> 3 files changed, 347 insertions(+)
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 1dd7921194f5..e21c37c7991e 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -553,6 +553,16 @@ config PWM_SL28CPLD
> To compile this driver as a module, choose M here: the module
> will be called pwm-sl28cpld.
>
> +config PWM_SN3112
> + tristate "SI-EN SN3112 PWM driver"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + Generic PWM framework driver for SI-EN SN3112 LED controller.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called pwm-sn3112.
> +
> config PWM_SPEAR
> tristate "STMicroelectronics SPEAr PWM support"
> depends on PLAT_SPEAR || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 90913519f11a..6aab2d113159 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_PWM_RZ_MTU3) += pwm-rz-mtu3.o
> obj-$(CONFIG_PWM_SAMSUNG)+= pwm-samsung.o
> obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o
> obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o
> +obj-$(CONFIG_PWM_SN3112) += pwm-sn3112.o
> obj-$(CONFIG_PWM_SPEAR) += pwm-spear.o
> obj-$(CONFIG_PWM_SPRD) += pwm-sprd.o
> obj-$(CONFIG_PWM_STI)+= pwm-sti.o
> diff --git a/drivers/pwm/pwm-sn3112.c b/drivers/pwm/pwm-sn3112.c
> new file mode 100644
> index ..38ef948602a3
> --- /dev/null
> +++ b/drivers/pwm/pwm-sn3112.c
> @@ -0,0 +1,336 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for SN3112 12-channel 8-bit PWM LED controller
> + *
> + * Copyright (c) 2024 Junhao Xie
> + *
Please document here some hardware features in the same format as e.g.
pwm-sl28cpld.c such that
sed -rn '/Limitations:/,/\*\/?$/p' drivers/pwm/*.c
can easily extract it. Interesting facts that I want to have documented
are:
- How does the HW behave on reconfiguration, i.e. does it complete the
active period or is it aborted and can it happen that the signal
gliches (e.g. because it emits for a moment a signal using the old
period but the new duty cycle).
- How does the HW behave on disable? Does it complete the active
period? Does it emit low? Or the inactive level? Or does it freeze?
- "Doesn't support read-back of configured output" belongs here.
- Only supports a single fixed period and normal polarity.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define SN3112_CHANNELS 12
> +#define SN3112_REG_ENABLE 0x00
> +#define SN3112_REG_PWM_VAL 0x04
> +#define SN3112_REG_PWM_EN 0x13
> +#define SN3112_REG_APPLY 0x16
> +#define SN3112_REG_RESET 0x17
> +
> +struct sn3112 {
> + struct device *pdev;
pdev is a usual name for pointers to struct platform_device or struct
pci_device. For struct device please use "dev".
> + struct regmap *regmap;
> + struct mutex lock;
> + struct regulator *vdd;
> + uint8_t pwm_val[SN3112_CHANNELS];
> + uint8_t pwm_en_reg[3];
> + bool pwm_en[SN3112_CHANNELS];
> +#if IS_ENABLED(CONFIG_GPIOLIB)
> + struct gpio_desc *sdb;
> +#endif
> +};
> +
> +static int sn3112_write_reg(struct sn3112 *priv, unsigned int reg,
> + unsigned int val)
> +{
> + int err;
> +
> + dev_dbg(priv->pdev, "request regmap_write 0x%x 0x%x\n", reg, val);
> + err = regmap_write(priv->regmap, reg, val);
> + if (err)
> + dev_warn_ratelimited(
> + priv->pdev,
> + "regmap_write to register 0x%x failed: %pe\n", reg,
> + ERR_PTR(err));
> +
> + return err;
> +}
> +
> +static int sn3112_set_en_reg(struct sn3112 *priv, unsigned int channel,
> + bool enabled, bool write)
> +{
> + unsigned int reg, bit;
> +
> + if (channel >= SN3112_CHANNELS)
> + return -EINVAL;
> +
> + /* LED_EN1: BIT5:BIT3 = OUT3:OUT1 */
> + if (channel >= 0 && channel <= 2)
> + reg = 0, bit = channel + 3;
> + /* LED_EN2: BIT5:BIT0 = OUT9:OUT4 */
> + else if (channel >= 3 && channel <= 8)
> + reg = 1, bit = channel - 3;
> + /* LED_EN3: BIT2:BIT0 = OUT12:OUT10 */
> + else if (channel >= 9 && channel <= 11)
> + reg = 2, bit = channel - 9;
Please use ; instead of , and proper { }.
And huh, this is inconsitent. Is it possible to renumber somehow such
that this simplifies to
reg = channel / 3;
Re: [RFC PATCH 16/18] drm/amdgpu: Implement SET_PRIORITY GEM op
Am 25.04.24 um 09:06 schrieb Friedrich Vock:
On 25.04.24 08:58, Christian König wrote:
Am 25.04.24 um 08:46 schrieb Friedrich Vock:
On 25.04.24 08:32, Christian König wrote:
Am 24.04.24 um 18:57 schrieb Friedrich Vock:
Used by userspace to adjust buffer priorities in response to
changes in
application demand and memory pressure.
Yeah, that was discussed over and over again. One big design criteria
is that we can't have global priorities from userspace!
The background here is that this can trivially be abused.
I see your point when apps are allowed to prioritize themselves above
other apps, and I agree that should probably be disallowed at least for
unprivileged apps.
Disallowing this is a pretty trivial change though, and I don't really
see the abuse potential in being able to downgrade your own priority?
Yeah, I know what you mean and I'm also leaning towards that
argumentation. But another good point is also that it doesn't actually
help.
For example when you have desktop apps fighting with a game, you
probably don't want to use static priorities, but rather evict the
apps which are inactive and keep the apps which are active in the
background.
Sadly things are not as simple as "evict everything from app 1, keep
everything from app 2 active". The simplest failure case of this is
games that already oversubscribe VRAM on their own. Keeping the whole
app inside VRAM is literally impossible there, and it helps a lot to
know which buffers the app is most happy with evicting.
In other words the priority just tells you which stuff from each app
to evict first, but not which app to globally throw out.
Yeah, but per-buffer priority system could do both of these.
Yeah, but we already have that. See amdgpu_bo_list_entry_cmp() and
amdgpu_bo_list_create().
This is the per application priority which can be used by userspace to
give priority to each BO in a submission (or application wide).
The problem is rather that amdgpu/TTM never really made good use of that
information.
Regards,
Christian.
Regards,
Friedrich
Regards,
Christian.
Regards,
Friedrich
What we can do is to have per process priorities, but that needs to be
in the VM subsystem.
That's also the reason why I personally think that the handling
shouldn't be inside TTM at all.
Regards,
Christian.
Signed-off-by: Friedrich Vock
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 20
include/uapi/drm/amdgpu_drm.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 5ca13e2e50f50..6107810a9c205 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -836,8 +836,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
{
struct amdgpu_device *adev = drm_to_adev(dev);
struct drm_amdgpu_gem_op *args = data;
+ struct ttm_resource_manager *man;
struct drm_gem_object *gobj;
struct amdgpu_vm_bo_base *base;
+ struct ttm_operation_ctx ctx;
struct amdgpu_bo *robj;
int r;
@@ -851,6 +853,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
if (unlikely(r))
goto out;
+ memset(, 0, sizeof(ctx));
+ ctx.interruptible = true;
+
switch (args->op) {
case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
struct drm_amdgpu_gem_create_in info;
@@ -898,6 +903,21 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
void *data,
amdgpu_bo_unreserve(robj);
break;
+ case AMDGPU_GEM_OP_SET_PRIORITY:
+ if (args->value > AMDGPU_BO_PRIORITY_MAX_USER)
+ args->value = AMDGPU_BO_PRIORITY_MAX_USER;
+ ttm_bo_update_priority(>tbo, args->value);
+ if (robj->tbo.evicted_type != TTM_NUM_MEM_TYPES) {
+ ttm_bo_try_unevict(>tbo, );
+ amdgpu_bo_unreserve(robj);
+ } else {
+ amdgpu_bo_unreserve(robj);
+ man = ttm_manager_type(robj->tbo.bdev,
+ robj->tbo.resource->mem_type);
+ ttm_mem_unevict_evicted(robj->tbo.bdev, man,
+ true);
+ }
+ break;
default:
amdgpu_bo_unreserve(robj);
r = -EINVAL;
diff --git a/include/uapi/drm/amdgpu_drm.h
b/include/uapi/drm/amdgpu_drm.h
index bdbe6b262a78d..53552dd489b9b 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -531,6 +531,7 @@ union drm_amdgpu_wait_fences {
#define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
#define AMDGPU_GEM_OP_SET_PLACEMENT 1
+#define AMDGPU_GEM_OP_SET_PRIORITY 2
/* Sets or returns a value associated with a buffer. */
struct drm_amdgpu_gem_op {
--
2.44.0
[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
[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
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;
>>
>> /**
>
Re: [PATCH V2 2/2] drm/bridge: samsung-dsim: Fix porch calcalcuation rounding
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
> ---
> 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)
> {
> + 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
Re: [PATCH V2 1/2] drm/bridge: samsung-dsim: Set P divider based on min/max of fin pll
On 12.02.2024 00:09, Adam Ford wrote:
> The P divider should be set based on the min and max values of
> the fin pll which may vary between different platforms.
> These ranges are defined per platform, but hard-coded values
> were used instead which resulted in a smaller range available
> on the i.MX8M[MNP] than what was possible.
>
> As noted by Frieder, there are descripencies between the reference
> manuals of the Mini, Nano and Plus, so I reached out to my NXP
> rep and got the following response regarding the varing notes
> in the documentation.
>
> "Yes it is definitely wrong, the one that is part of the NOTE in
> MIPI_DPHY_M_PLLPMS register table against PMS_P, PMS_M and PMS_S is
> not correct. I will report this to Doc team, the one customer should
> be take into account is the Table 13-40 DPHY PLL Parameters and the
> Note above."
>
> With this patch, the clock rates now match the values used in NXP's
> downstream kernel.
>
> Fixes: 846307185f0f ("drm/bridge: samsung-dsim: update PLL reference clock")
> Signed-off-by: Adam Ford
> Reviewed-by: Frieder Schrempf
> Tested-by: Frieder Schrempf
Feel free to add:
Tested-by: Marek Szyprowski
although all Exynos based boards have panels with fixed timings afair.
> ---
> V2: Only update the commit message to reflect why these values
> were chosen. No code change present
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 95fedc68b0ae..8476650c477c 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -574,8 +574,8 @@ static unsigned long samsung_dsim_pll_find_pms(struct
> samsung_dsim *dsi,
> u16 _m, best_m;
> u8 _s, best_s;
>
> - p_min = DIV_ROUND_UP(fin, (12 * MHZ));
> - p_max = fin / (6 * MHZ);
> + p_min = DIV_ROUND_UP(fin, (driver_data->pll_fin_max * MHZ));
> + p_max = fin / (driver_data->pll_fin_min * MHZ);
>
> for (_p = p_min; _p <= p_max; ++_p) {
> for (_s = 0; _s <= 5; ++_s) {
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
Re: [PATCH] drm/panthor: Add defer probe for firmware load
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.
>> 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.
Since as you mentioned, this isn't specific to panthor and an issue that I also
faced with the powervr driver.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
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] 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
[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 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 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;
>
> /**
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 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 -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);
/*
[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] 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++) {
[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 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] 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 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.
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++) {
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. > - */ > -
[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] 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 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/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
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: [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
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 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
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: [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
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
[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 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: [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
[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 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
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] 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
[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 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 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
