Re: [PATCH v2 01/17] drm/tests: helpers: Move the helper header to include/drm
On 11/28/22 15:53, Maxime Ripard wrote:
> We'll need to use those helpers from drivers too, so let's move it to a
> more visible location.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/tests/drm_client_modeset_test.c| 3 +--
> drivers/gpu/drm/tests/drm_kunit_helpers.c | 3 +--
> drivers/gpu/drm/tests/drm_modes_test.c | 3 +--
> drivers/gpu/drm/tests/drm_probe_helper_test.c | 3 +--
> {drivers/gpu/drm/tests => include/drm}/drm_kunit_helpers.h | 0
> 5 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c
> b/drivers/gpu/drm/tests/drm_client_modeset_test.c
> index 52929536a158..ed2f62e92fea 100644
> --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
> +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
> @@ -8,12 +8,11 @@
> #include
> #include
> #include
> +#include
I wonder if now that this header was moved outside of the tests directory,
if we should add stub functions in the header file that are just defined
but do nothing if CONFIG_DRM_KUNIT_TEST isn't enabled. So that including
it in drivers will be a no-op.
Or do you plan to conditionally include this header file in drivers? So
that is only included when CONFIG_DRM_KUNIT_TEST is enabled?
Another thing that wondered is if we want a different namespace for this
header, i.e: , to make it clear that is
not part of the DRM API but just for testing helpers.
But these are open questions really, and they can be done as follow-up:
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [Intel-gfx] [PATCH 2/2] drm/i915/guc: Look for a guilty context when an engine reset fails
On 29/11/2022 21:12, john.c.harri...@intel.com wrote:
From: John Harrison
Engine resets are supposed to never happen. But in the case when one
Engine resets or engine reset failures? Hopefully the latter.
does (due to unknwon reasons that normally come down to a missing
w/a), it is useful to get as much information out of the system as
possible. Given that the GuC effectively dies on such a situation, it
is not possible to get a guilty context notification back. So do a
manual search instead. Given that GuC is dead, this is safe because
GuC won't be changing the engine state asynchronously.
Signed-off-by: John Harrison
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 0a42f1807f52c..c82730804a1c4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -4751,11 +4751,24 @@ static void reset_fail_worker_func(struct work_struct
*w)
guc->submission_state.reset_fail_mask = 0;
spin_unlock_irqrestore(&guc->submission_state.lock, flags);
- if (likely(reset_fail_mask))
+ if (likely(reset_fail_mask)) {
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+
+ /*
+* GuC is toast at this point - it dead loops after sending the
failed
+* reset notification. So need to manually determine the guilty
context.
+* Note that it should be safe/reliable to do this here because
the GuC
+* is toast and will not be scheduling behind the KMD's back.
+*/
+ for_each_engine_masked(engine, gt, reset_fail_mask, id)
+ intel_guc_find_hung_context(engine);
+
intel_gt_handle_error(gt, reset_fail_mask,
I915_ERROR_CAPTURE,
"GuC failed to reset engine mask=0x%x\n",
reset_fail_mask);
If GuC is defined by ABI contract to be dead, should the flow be
attempting to do a full GPU reset here, or maybe it happens somewhere
else as a consequence anyway? (In which case is the engine reset here
even needed?)
Regards,
Tvrtko
+ }
}
int intel_guc_engine_failure_process_msg(struct intel_guc *guc,
Re: [PATCH] drm/bridge: cdns-dsi: Fix issue with phy init
Hi, On 15/11/2022 10:39, Rahul T R wrote: Phy is not being initialized after suspend resume. Fix this by setting phy_initialized flag to false in suspend callback Signed-off-by: Rahul T R --- drivers/gpu/drm/bridge/cdns-dsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c index 20bece84ff8c..1a988f53424a 100644 --- a/drivers/gpu/drm/bridge/cdns-dsi.c +++ b/drivers/gpu/drm/bridge/cdns-dsi.c @@ -1187,6 +1187,7 @@ static int __maybe_unused cdns_dsi_suspend(struct device *dev) clk_disable_unprepare(dsi->dsi_p_clk); reset_control_assert(dsi->dsi_p_rst); dsi->link_initialized = false; + dsi->phy_initialized = false; return 0; } I'm not familiar with the IP, but the code related to enable/disable looks a bit odd. Why does cdns_dsi_bridge_enable() do: cdns_dsi_hs_init(dsi); cdns_dsi_init_link(dsi); but then in cdns_dsi_bridge_pre_enable(): cdns_dsi_init_link(dsi); cdns_dsi_hs_init(dsi); Doesn't the order matter? Why are the same functions called in both places? cdns_dsi_hs_init() seems to do enabling, like locking the PLL. But there's no counterpart, hs_uninit(). I see cdns_dsi_bridge_disable() doing some clearing of the registers, so perhaps that's where the disabling happens. But cdns_dsi_hs_init() is called from the pre-enable, and post-disable doesn't do anything else but pm_runtime_put(). More or less the same comments apply to cdns_dsi_init_link(), but it's a bit worse as it's also called in cdns_dsi_transfer(), and then there's no uninit counterpart that I can see. Well, maybe both functions are only doing configuration, not enabling this as such, and so it's fine to just turn off the IP without any uninit step. If that's the case, then probably this patch is fine. Tomi
Re: [PATCH v2 0/5] Fix probe failed when modprobe modules
On 11/29/22 9:06 AM, Li Zetao wrote: > This patchset fixes similar issue, the root cause of the > problem is that the virtqueues are not stopped on error > handling path. Not related to just this patchset, but guys, Huawei really *REALLY* need to get the email situation sorted. I'm digging whole/half patchsets out of spam every morning. This has been brought up in the past. And no, the cloud variant of the email also doesn't work properly. Talk to your IT department, get this sorted once and for all. You risk your patches being dumped on the floor because people don't see them, or only see small parts of a patchset. And it's really annoying to have to deal with as a recipient. -- Jens Axboe
Re: [PATCH v2 7/7] drm: rcar-du: dsi: Add r8a779g0 support
On 29/11/2022 03:49, Laurent Pinchart wrote:
@@ -198,70 +436,53 @@ static void rcar_mipi_dsi_parameters_calc(struct
rcar_mipi_dsi *dsi,
*/
fout_target = target * mipi_dsi_pixel_format_to_bpp(dsi->format)
/ (2 * dsi->lanes);
- if (fout_target < 4000 || fout_target > 125000)
+ if (fout_target < MHZ(40) || fout_target > MHZ(1250))
return;
/* Find vco_cntrl */
- for (vco_cntrl = vco_cntrl_table; vco_cntrl->min_freq != 0;
vco_cntrl++) {
- if (fout_target > vco_cntrl->min_freq &&
- fout_target <= vco_cntrl->max_freq) {
- setup_info->vco_cntrl = vco_cntrl->value;
- if (fout_target >= 115000)
- setup_info->prop_cntrl = 0x0c;
- else
- setup_info->prop_cntrl = 0x0b;
+ for (clkset = dsi->info->clk_cfg; clkset->min_freq != 0; clkset++) {
+ if (fout_target > clkset->min_freq &&
+ fout_target <= clkset->max_freq) {
+ setup_info->clkset = clkset;
break;
}
}
- /* Add divider */
- setup_info->div = (setup_info->vco_cntrl & 0x30) >> 4;
+ switch (dsi->info->model) {
+ case RCAR_DSI_R8A779A0:
+ setup_info->vclk_divider = 1 << ((clkset->vco_cntrl >> 4) &
0x3);
If you stored (clkset->vco_cntrl >> 4) & 0x3 in setup_info->vclk_divider
you wouldn't have to use __ffs() in rcar_mipi_dsi_startup(). You could
also drop the - 1 there, which would allow dropping one of the
switch(dsi->info->model). You can store the real divider value in
setup_info separately for rcar_mipi_dsi_pll_calc_r8a779a0(), or pass it
to the function.
That's true. The reason I chose this approach was to keep dsi_setup_info
"neutral", containing only the logical values, and the register specific
tinkering is done only where the register is written. Mixing the logical
and the register values in the old dsi_setup_info was confusing, and
implementing your suggestion would again go that direction. But as you
noticed, this is uglified a bit by the need to get the divider from the
vco_cntrl.
We could store the logical divider in the dsi_clk_config table, though,
which would remove the need for the above code.
Tomi
Re: [PATCH v2 7/7] drm: rcar-du: dsi: Add r8a779g0 support
On 29/11/2022 03:49, Laurent Pinchart wrote:
Hi Tomi,
Thank you for the patch.
On Wed, Nov 23, 2022 at 08:59:46AM +0200, Tomi Valkeinen wrote:
Add DSI support for r8a779g0. The main differences to r8a779a0 are in
the PLL and PHTW setups.
Signed-off-by: Tomi Valkeinen
---
drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 484 +++
drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h | 6 +-
2 files changed, 384 insertions(+), 106 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index a7f2b7f66a17..723c35726c38 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -28,6 +29,20 @@
#include "rcar_mipi_dsi.h"
#include "rcar_mipi_dsi_regs.h"
+#define MHZ(v) ((v) * 100u)
Isn't the U suffix usually spelled in uppercase ? Same below.
I couldn't find any coding style guidelines on that. I like the lower
case visually. The suffix stands out much clearer on 1000u than on
1000U. But I can change it if you feel otherwise.
+
+enum rcar_mipi_dsi_hw_model {
+ RCAR_DSI_R8A779A0,
+ RCAR_DSI_R8A779G0,
+};
+
+struct rcar_mipi_dsi_device_info {
+ enum rcar_mipi_dsi_hw_model model;
+ const struct dsi_clk_config *clk_cfg;
+ u8 clockset2_m_offset;
+ u8 clockset2_n_offset;
+};
+
struct rcar_mipi_dsi {
struct device *dev;
const struct rcar_mipi_dsi_device_info *info;
@@ -50,6 +65,17 @@ struct rcar_mipi_dsi {
unsigned int lanes;
};
+struct dsi_setup_info {
+ unsigned long hsfreq;
+ u16 hsfreqrange;
+
+ unsigned long fout;
+ u16 m;
+ u16 n;
+ u16 vclk_divider;
+ const struct dsi_clk_config *clkset;
+};
+
static inline struct rcar_mipi_dsi *
bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
{
@@ -62,22 +88,6 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
return container_of(host, struct rcar_mipi_dsi, host);
}
-static const u32 phtw[] = {
- 0x01020114, 0x01600115, /* General testing */
- 0x01030116, 0x0102011d, /* General testing */
- 0x011101a4, 0x018601a4, /* 1Gbps testing */
- 0x014201a0, 0x010001a3, /* 1Gbps testing */
- 0x0101011f, /* 1Gbps testing */
-};
-
-static const u32 phtw2[] = {
- 0x010c0130, 0x010c0140, /* General testing */
- 0x010c0150, 0x010c0180, /* General testing */
- 0x010c0190,
- 0x010a0160, 0x010a0170,
- 0x01800164, 0x01800174, /* 1Gbps testing */
-};
-
static const u32 hsfreqrange_table[][2] = {
{ 8000U, 0x00 }, { 9000U, 0x10 }, { 1U, 0x20 },
{ 11000U, 0x30 }, { 12000U, 0x01 }, { 13000U, 0x11 },
@@ -103,24 +113,53 @@ static const u32 hsfreqrange_table[][2] = {
{ /* sentinel */ },
};
-struct vco_cntrl_value {
+struct dsi_clk_config {
u32 min_freq;
u32 max_freq;
- u16 value;
+ u8 vco_cntrl;
+ u8 cpbias_cntrl;
+ u8 gmp_cntrl;
+ u8 int_cntrl;
+ u8 prop_cntrl;
};
-static const struct vco_cntrl_value vco_cntrl_table[] = {
- { .min_freq = 4000U, .max_freq = 5500U, .value = 0x3f },
- { .min_freq = 5250U, .max_freq = 8000U, .value = 0x39 },
- { .min_freq = 8000U, .max_freq = 11000U, .value = 0x2f },
- { .min_freq = 10500U, .max_freq = 16000U, .value = 0x29 },
- { .min_freq = 16000U, .max_freq = 22000U, .value = 0x1f },
- { .min_freq = 21000U, .max_freq = 32000U, .value = 0x19 },
- { .min_freq = 32000U, .max_freq = 44000U, .value = 0x0f },
- { .min_freq = 42000U, .max_freq = 66000U, .value = 0x09 },
- { .min_freq = 63000U, .max_freq = 114900U, .value = 0x03 },
- { .min_freq = 11U, .max_freq = 115200U, .value = 0x01 },
- { .min_freq = 115000U, .max_freq = 125000U, .value = 0x01 },
+static const struct dsi_clk_config dsi_clk_cfg_r8a779a0[] = {
+ { 4000u, 5500u, 0x3f, 0x10, 0x01, 0x00, 0x0b },
+ { 5250u, 8000u, 0x39, 0x10, 0x01, 0x00, 0x0b },
Would MHZ(52.5) do the right thing ? If so, I'd use the macro through
those tables.
That's a great idea. It had never occurred to me that we can use floats
in the kernel code, as long as we convert to ints when the preprocessor
is done.
+ { 8000u, 11000u, 0x2f, 0x10, 0x01, 0x00, 0x0b },
+ { 10500u, 16000u, 0x29, 0x10, 0x01, 0x00, 0x0b },
+ { 16000u, 22000u, 0x1f, 0x10, 0x01, 0x00, 0x0b },
+ { 21000u, 32000u, 0x19, 0x10, 0x01, 0x00, 0x0b },
+ { 32000u, 44000u, 0x0f, 0x10, 0x01, 0x00, 0x0b },
+ { 42000u, 66000u, 0x09, 0x10, 0x01, 0x00, 0x0b },
+ { 63000u, 114900u, 0x03, 0x10, 0x01,
[PATCH v2 3/5] virtio-input: Fix probe failed when modprobe virtio_input
When doing the following test steps, an error was found:
step 1: modprobe virtio_input succeeded
# modprobe virtio_input <-- OK
step 2: fault injection in input_allocate_device()
# modprobe -r virtio_input <-- OK
# ...
CPU: 0 PID: 4260 Comm: modprobe Tainted: GW
6.1.0-rc6-00285-g6a1e40c4b995-dirty #109
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
Call Trace:
should_fail.cold+0x5/0x1f
...
kmalloc_trace+0x27/0xa0
input_allocate_device+0x43/0x280
virtinput_probe+0x23b/0x1648 [virtio_input]
...
virtio_input: probe of virtio5 failed with error -12
step 3: modprobe virtio_input failed
# modprobe virtio_input <-- failed
virtio_input: probe of virtio1 failed with error -2
The root cause of the problem is that the virtqueues are not
stopped on the error handling path when input_allocate_device()
fails in virtinput_probe(), resulting in an error "-ENOENT"
returned in the next modprobe call in setup_vq().
virtio_pci_modern_device uses virtqueues to send or
receive message, and "queue_enable" records whether the
queues are available. In vp_modern_find_vqs(), all queues
will be selected and activated, but once queues are enabled
there is no way to go back except reset.
Fix it by reset virtio device on error handling path. After
virtinput_init_vqs() succeeded, all virtqueues should be
stopped on error handling path.
Fixes: 271c865161c5 ("Add virtio-input driver.")
Signed-off-by: Li Zetao
---
v1 -> v2: modify the description error of the test case in step 3 and
modify the fixes tag information.
drivers/virtio/virtio_input.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index 3aa46703872d..f638f1cd3531 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -330,6 +330,7 @@ static int virtinput_probe(struct virtio_device *vdev)
err_mt_init_slots:
input_free_device(vi->idev);
err_input_alloc:
+ virtio_reset_device(vdev);
vdev->config->del_vqs(vdev);
err_init_vq:
kfree(vi);
--
2.25.1
[PATCH] drm/tegra: Remove redundant null checks before kfree
From: Yushan Zhou Fix the following coccicheck warning: ./drivers/gpu/drm/tegra/submit.c:689:2-7: WARNING: NULL check before some freeing functions is not needed. Signed-off-by: Yushan Zhou --- drivers/gpu/drm/tegra/submit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tegra/submit.c b/drivers/gpu/drm/tegra/submit.c index b24738bdf3df..df34c5daa400 100644 --- a/drivers/gpu/drm/tegra/submit.c +++ b/drivers/gpu/drm/tegra/submit.c @@ -685,8 +685,7 @@ int tegra_drm_ioctl_channel_submit(struct drm_device *drm, void *data, kfree(job_data->used_mappings); } - if (job_data) - kfree(job_data); + kfree(job_data); put_bo: gather_bo_put(&bo->base); unlock: -- 2.27.0
[PATCH v2 2/5] virtio-mem: Fix probe failed when modprobe virtio_mem
When doing the following test steps, an error was found:
step 1: modprobe virtio_mem succeeded
# modprobe virtio_mem <-- OK
step 2: fault injection in virtio_mem_init()
# modprobe -r virtio_mem <-- OK
# ...
CPU: 0 PID: 1837 Comm: modprobe Not tainted
6.1.0-rc6-00285-g6a1e40c4b995-dirty
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
Call Trace:
should_fail.cold+0x5/0x1f
...
virtio_mem_init_hotplug+0x9ae/0xe57 [virtio_mem]
virtio_mem_init.cold+0x327/0x339 [virtio_mem]
virtio_mem_probe+0x48e/0x910 [virtio_mem]
virtio_dev_probe+0x608/0xae0
...
virtio_mem virtio4: could not reserve device region
virtio_mem: probe of virtio4 failed with error -16
step 3: modprobe virtio_mem failed
# modprobe virtio_mem <-- failed
virtio_mem: probe of virtio4 failed with error -16
The root cause of the problem is that the virtqueues are not
stopped on the error handling path when virtio_mem_init()
fails in virtio_mem_probe(), resulting in an error "-ENOENT"
returned in the next modprobe call in setup_vq().
virtio_pci_modern_device uses virtqueues to send or
receive message, and "queue_enable" records whether the
queues are available. In vp_modern_find_vqs(), all queues
will be selected and activated, but once queues are enabled
there is no way to go back except reset.
Fix it by reset virtio device on error handling path. After
virtio_mem_init_vq() succeeded, all virtqueues should be
stopped on error handling path.
Fixes: 5f1f79bbc9e2 ("virtio-mem: Paravirtualized memory hotplug")
Signed-off-by: Li Zetao
Reviewed-by: David Hildenbrand
---
v1 -> v2: modify the description error of the test case in step 3 and
modify the fixes tag information.
drivers/virtio/virtio_mem.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 0c2892ec6817..c7f09c2ce982 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -2793,6 +2793,7 @@ static int virtio_mem_probe(struct virtio_device *vdev)
return 0;
out_del_vq:
+ virtio_reset_device(vdev);
vdev->config->del_vqs(vdev);
out_free_vm:
kfree(vm);
--
2.25.1
[PATCH v3 7/7] drm: rcar-du: dsi: Add r8A779g0 support
Add DSI support for r8a779g0. The main differences to r8a779a0 are in
the PLL and PHTW setups.
Signed-off-by: Tomi Valkeinen
---
Changes to v2:
- Use MHZ() in the tables, with floating point values as inputs
- Use V3U/V4H names instead of R8A779A0/R8A779G0
- PLL diffs between V3U and V4H are now described in struct
rcar_mipi_dsi_device_info, and we have a single function to calculate
the pll settings.
- Refactor fin_rate to outside the pll calc functions.
- Fixed the PLL config debug print.
- Dropped clockset2_n_offset, which is always 1
drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 499 ++-
drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h | 6 +-
2 files changed, 377 insertions(+), 128 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index a7f2b7f66a17..3b812ec034fe 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -28,6 +29,31 @@
#include "rcar_mipi_dsi.h"
#include "rcar_mipi_dsi_regs.h"
+#define MHZ(v) ((u32)((v) * 100U))
+
+enum rcar_mipi_dsi_hw_model {
+ RCAR_DSI_V3U,
+ RCAR_DSI_V4H,
+};
+
+struct rcar_mipi_dsi_device_info {
+ enum rcar_mipi_dsi_hw_model model;
+
+ const struct dsi_clk_config *clk_cfg;
+
+ u8 clockset2_m_offset;
+
+ u8 n_min;
+ u8 n_max;
+ u8 n_mul;
+ unsigned long fpfd_min;
+ unsigned long fpfd_max;
+ u16 m_min;
+ u16 m_max;
+ unsigned long fout_min;
+ unsigned long fout_max;
+};
+
struct rcar_mipi_dsi {
struct device *dev;
const struct rcar_mipi_dsi_device_info *info;
@@ -50,6 +76,17 @@ struct rcar_mipi_dsi {
unsigned int lanes;
};
+struct dsi_setup_info {
+ unsigned long hsfreq;
+ u16 hsfreqrange;
+
+ unsigned long fout;
+ u16 m;
+ u16 n;
+ u16 vclk_divider;
+ const struct dsi_clk_config *clkset;
+};
+
static inline struct rcar_mipi_dsi *
bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
{
@@ -62,65 +99,78 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
return container_of(host, struct rcar_mipi_dsi, host);
}
-static const u32 phtw[] = {
- 0x01020114, 0x01600115, /* General testing */
- 0x01030116, 0x0102011d, /* General testing */
- 0x011101a4, 0x018601a4, /* 1Gbps testing */
- 0x014201a0, 0x010001a3, /* 1Gbps testing */
- 0x0101011f, /* 1Gbps testing */
-};
-
-static const u32 phtw2[] = {
- 0x010c0130, 0x010c0140, /* General testing */
- 0x010c0150, 0x010c0180, /* General testing */
- 0x010c0190,
- 0x010a0160, 0x010a0170,
- 0x01800164, 0x01800174, /* 1Gbps testing */
-};
-
static const u32 hsfreqrange_table[][2] = {
- { 8000U, 0x00 }, { 9000U, 0x10 }, { 1U, 0x20 },
- { 11000U, 0x30 }, { 12000U, 0x01 }, { 13000U, 0x11 },
- { 14000U, 0x21 }, { 15000U, 0x31 }, { 16000U, 0x02 },
- { 17000U, 0x12 }, { 18000U, 0x22 }, { 19000U, 0x32 },
- { 20500U, 0x03 }, { 22000U, 0x13 }, { 23500U, 0x23 },
- { 25000U, 0x33 }, { 27500U, 0x04 }, { 3U, 0x14 },
- { 32500U, 0x25 }, { 35000U, 0x35 }, { 4U, 0x05 },
- { 45000U, 0x16 }, { 5U, 0x26 }, { 55000U, 0x37 },
- { 6U, 0x07 }, { 65000U, 0x18 }, { 7U, 0x28 },
- { 75000U, 0x39 }, { 8U, 0x09 }, { 85000U, 0x19 },
- { 9U, 0x29 }, { 95000U, 0x3a }, { 10U, 0x0a },
- { 105000U, 0x1a }, { 11U, 0x2a }, { 115000U, 0x3b },
- { 12U, 0x0b }, { 125000U, 0x1b }, { 13U, 0x2b },
- { 135000U, 0x3c }, { 14U, 0x0c }, { 145000U, 0x1c },
- { 15U, 0x2c }, { 155000U, 0x3d }, { 16U, 0x0d },
- { 165000U, 0x1d }, { 17U, 0x2e }, { 175000U, 0x3e },
- { 18U, 0x0e }, { 185000U, 0x1e }, { 19U, 0x2f },
- { 195000U, 0x3f }, { 20U, 0x0f }, { 205000U, 0x40 },
- { 21U, 0x41 }, { 215000U, 0x42 }, { 22U, 0x43 },
- { 225000U, 0x44 }, { 23U, 0x45 }, { 235000U, 0x46 },
- { 24U, 0x47 }, { 245000U, 0x48 }, { 25U, 0x49 },
+ { MHZ(80), 0x00 }, { MHZ(90), 0x10 }, { MHZ(100), 0x20 },
+ { MHZ(110), 0x30 }, { MHZ(120), 0x01 }, { MHZ(130), 0x11 },
+ { MHZ(140), 0x21 }, { MHZ(150), 0x31 }, { MHZ(160), 0x02 },
+ { MHZ(170), 0x12 }, { MHZ(180), 0x22 }, { MHZ(190), 0x32 },
+ { MHZ(205), 0x03 }, { MHZ(220), 0x13 }, { MHZ(235), 0x23 },
+ { MHZ(250), 0x33 }, { MHZ(275), 0x04 }, { MHZ(300), 0x14 },
+ { MHZ(325), 0x25 }, { MHZ(350), 0x35 }, { MHZ(400), 0x05 },
+ { MHZ(4
[PATCH v2 1/5] 9p: Fix probe failed when modprobe 9pnet_virtio
When doing the following test steps, an error was found:
step 1: modprobe 9pnet_virtio succeeded
# modprobe 9pnet_virtio <-- OK
step 2: fault injection in sysfs_create_file()
# modprobe -r 9pnet_virtio <-- OK
# ...
FAULT_INJECTION: forcing a failure.
name failslab, interval 1, probability 0, space 0, times 0
CPU: 0 PID: 3790 Comm: modprobe Tainted: GW
6.1.0-rc6-00285-g6a1e40c4b995-dirty #108
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
Call Trace:
...
should_failslab+0xa/0x20
...
sysfs_create_file_ns+0x130/0x1d0
p9_virtio_probe+0x662/0xb30 [9pnet_virtio]
virtio_dev_probe+0x608/0xae0
...
9pnet_virtio: probe of virtio3 failed with error -12
step 3: modprobe 9pnet_virtio failed
# modprobe 9pnet_virtio <-- failed
9pnet_virtio: probe of virtio3 failed with error -2
The root cause of the problem is that the virtqueues are not
stopped on the error handling path when sysfs_create_file()
fails in p9_virtio_probe(), resulting in an error "-ENOENT"
returned in the next modprobe call in setup_vq().
virtio_pci_modern_device uses virtqueues to send or
receive message, and "queue_enable" records whether the
queues are available. In vp_modern_find_vqs(), all queues
will be selected and activated, but once queues are enabled
there is no way to go back except reset.
Fix it by reset virtio device on error handling path. After
virtio_find_single_vq() succeeded, all virtqueues should be
stopped on error handling path.
Fixes: ea52bf8eda98 ("9p/trans_virtio: reset virtio device on remove")
Signed-off-by: Li Zetao
Reviewed-by: Christian Schoenebeck
---
v1 -> v2: modify the description error of the test case in step 3 and
modify the fixes tag information.
net/9p/trans_virtio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index e757f0601304..39933187284b 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -668,6 +668,7 @@ static int p9_virtio_probe(struct virtio_device *vdev)
out_free_tag:
kfree(tag);
out_free_vq:
+ virtio_reset_device(vdev);
vdev->config->del_vqs(vdev);
out_free_chan:
kfree(chan);
--
2.25.1
Re: Screen corruption using radeon kernel driver
On Tue, Nov 29, 2022 at 11:05:28AM -0500, Alex Deucher wrote: > On Tue, Nov 29, 2022 at 10:59 AM Mikhail Krylov wrote: > > > > On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote: > > > On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov wrote: > > > > > > > > On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher wrote: > > > > > > > > >>> [excessive quoting removed] > > > > > > > > >> So, is there any progress on this issue? I do understand it's not a > > > > >> high > > > > >> priority one, and today I've checked it on 6.0 kernel, and > > > > >> unfortunately, it still persists... > > > > >> > > > > >> I'm considering writing a patch that will allow user to override > > > > >> need_dma32/dma_bits setting with a module parameter. I'll have some > > > > >> time > > > > >> after the New Year for that. > > > > >> > > > > >> Is it at all possible that such a patch will be merged into kernel? > > > > >> > > > > > On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov > > > > > wrote: > > > > > Unless someone familiar with HIMEM can figure out what is going wrong > > > > > we should just revert the patch. > > > > > > > > > > Alex > > > > > > > > > > > > Okay, I was suggesting that mostly because > > > > > > > > a) it works for me with dma_bits = 40 (I understand that's what it is > > > > without the original patch applied); > > > > > > > > b) there's a hint of uncertainity on this line > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359 > > > > saying that for AGP dma_bits = 32 is the safest option, so apparently > > > > there are > > > > setups, unlike mine, where dma_bits = 32 is better than 40. > > > > > > > > But I'm in no position to argue, just wanted to make myself clear. > > > > I'm okay with rebuilding the kernel for my machine until the original > > > > patch is reverted or any other fix is applied. > > > > > > What GPU do you have and is it AGP? If it is AGP, does setting > > > radeon.agpmode=-1 also fix it? > > > > > > Alex > > > > That is ATI Radeon X1950, and, unfortunately, radeon.agpmode=-1 doesn't > > help, it just makes 3D acceleration in games such as OpenArena stop > > working. > > Just to confirm, is the board AGP or PCIe? > > Alex It is AGP. That's an old machine. signature.asc Description: PGP signature
[PATCH v2 5/5] drm/virtio: Fix probe failed when modprobe virtio_gpu
When doing the following test steps, an error was found:
step 1: modprobe virtio_gpu succeeded
# modprobe virtio_gpu <-- OK
step 2: fault injection in virtio_gpu_alloc_vbufs()
# modprobe -r virtio_gpu <-- OK
# ...
CPU: 0 PID: 1714 Comm: modprobe Not tainted 6.1.0-rc7-dirty
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
Call Trace:
should_fail_ex.cold+0x1a/0x1f
...
kmem_cache_create+0x12/0x20
virtio_gpu_alloc_vbufs+0x2f/0x90 [virtio_gpu]
virtio_gpu_init.cold+0x659/0xcad [virtio_gpu]
virtio_gpu_probe+0x14f/0x260 [virtio_gpu]
virtio_dev_probe+0x608/0xae0
?...
kmem_cache_create_usercopy(virtio-gpu-vbufs) failed with error -12
step 3: modprobe virtio_gpu failed
# modprobe virtio_gpu <-- failed
failed to find virt queues
virtio_gpu: probe of virtio6 failed with error -2
The root cause of the problem is that the virtqueues are not
stopped on the error handling path when virtio_gpu_alloc_vbufs()
fails in virtio_gpu_init(), resulting in an error "-ENOENT"
returned in the next modprobe call in setup_vq().
virtio_pci_modern_device uses virtqueues to send or
receive message, and "queue_enable" records whether the
queues are available. In vp_modern_find_vqs(), all queues
will be selected and activated, but once queues are enabled
there is no way to go back except reset.
Fix it by reset virtio device on error handling path. After
virtio_find_vqs() succeeded, all virtqueues should be stopped
on error handling path.
Fixes: dc5698e80cf7 ("Add virtio gpu driver.")
Signed-off-by: Li Zetao
---
v1 -> v2: patch is new.
drivers/gpu/drm/virtio/virtgpu_kms.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 27b7f14dae89..1a03e8e13b5b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -255,6 +255,7 @@ int virtio_gpu_init(struct virtio_device *vdev, struct
drm_device *dev)
err_scanouts:
virtio_gpu_free_vbufs(vgdev);
err_vbufs:
+ virtio_reset_device(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
err_vqs:
dev->dev_private = NULL;
--
2.25.1
Re: [PATCH] drm/bridge: ti-sn65dsi86: Fix output polarity setting bug
Hi,
on Nov. 29, 2022, 11:45 a.m. Tomi wrote:
>On 29/11/2022 03:13, Doug Anderson wrote:
>> Hi,
>>
>> On Fri, Nov 25, 2022 at 2:54 AM Qiqi Zhang wrote:
>>>
>>> According to the description in ti-sn65dsi86's datasheet:
>>>
>>> CHA_HSYNC_POLARITY:
>>> 0 = Active High Pulse. Synchronization signal is high for the sync
>>> pulse width. (default)
>>> 1 = Active Low Pulse. Synchronization signal is low for the sync
>>> pulse width.
>>>
>>> CHA_VSYNC_POLARITY:
>>> 0 = Active High Pulse. Synchronization signal is high for the sync
>>> pulse width. (Default)
>>> 1 = Active Low Pulse. Synchronization signal is low for the sync
>>> pulse width.
>>>
>>> We should only set these bits when the polarity is negative.
>>> Signed-off-by: Qiqi Zhang
>>> ---
>>> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>> index 3c3561942eb6..eb24322df721 100644
>>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>> @@ -931,9 +931,9 @@ static void ti_sn_bridge_set_video_timings(struct
>>> ti_sn65dsi86 *pdata)
>>> &pdata->bridge.encoder->crtc->state->adjusted_mode;
>>> u8 hsync_polarity = 0, vsync_polarity = 0;
>>>
>>> - if (mode->flags & DRM_MODE_FLAG_PHSYNC)
>>> + if (mode->flags & DRM_MODE_FLAG_NHSYNC)
>>> hsync_polarity = CHA_HSYNC_POLARITY;
>>> - if (mode->flags & DRM_MODE_FLAG_PVSYNC)
>>> + if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>>> vsync_polarity = CHA_VSYNC_POLARITY;
>>
>> Looks right to me.
>>
>> Reviewed-by: Douglas Anderson
>>
>> I've never seen the polarity matter for any eDP panels I've worked
>> with, which presumably explains why this was wrong for so long. As far
>
>Afaik, DP doesn't have sync polarity as such (neither does DSI), and the
>sync polarity is just "metadata". So if you're in full-DP domain, I
>don't see why it would matter. I guess it becomes relevant when you
>convert from DP to some other bus format.
Just like Tomi said, the wrong polarity worked fine on my eDP panel(LP079QX1)
and standard DP monitor, I didn't notice the polarity configuration problem
here until my customer used the following solution and got a abnormal display:
GPU->mipi->eDP->DP->lvds->panel.
>> as I can tell, it's been wrong since the start. Probably you should
>> have:
>>
>> Fixes: a095f15c00e2 ("drm/bridge: add support for sn65dsi86 bridge driver")
Doug you mean I need to update my commit message? It's my first time using
kernel list and I'm a little confused about this.
>>
>> I put this on a sc7180-trogdor-lazor device and it didn't make
>> anything worse. Since the sync polarity never mattered to begin with,
>> I guess this isn't a surprise. ...so I guess that's a weak tested-by:
>>
>> Tested-by: Douglas Anderson
>>
>> I'm happy to land this patch, but sounds like we're hoping to get
>> extra testing so I'll hold off for now.
>
>Looks fine to me and works for me with my DP monitor.
>
>Reviewed-by: Tomi Valkeinen
-Eddy
Re: [PATCH v2 3/7] media: uapi: add MEDIA_BUS_FMT_BGR666_1X24_CPADHI
Hi Laurent, looks like linux/Documentation/userspace-api/media/v4l/subdev-formats.rst doesn't correlate at all to the arrangement and numbering in linux/include/uapi/linux/media-bus-format.h . Which sources do you want me to check? Looking at https://github.com/raspberrypi/linux/tree/rpi-6.1.y btw. Rgds Joerg Am Di., 29. Nov. 2022 um 13:38 Uhr schrieb Laurent Pinchart < laurent.pinch...@ideasonboard.com>: > Hi Maxime and Joerg, > > Thank you for the patch. > > On Thu, Oct 20, 2022 at 10:30:47AM +0200, Maxime Ripard wrote: > > From: Joerg Quinten > > > > Add the BGR666 format MEDIA_BUS_FMT_BGR666_1X24_CPADHI supported by the > > RaspberryPi. > > > > Signed-off-by: Joerg Quinten > > Signed-off-by: Maxime Ripard > > --- > > .../userspace-api/media/v4l/subdev-formats.rst | 37 > ++ > > include/uapi/linux/media-bus-format.h | 3 +- > > 2 files changed, 39 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst > b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > index 68f8d7d37984..604a30e2f890 100644 > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > @@ -1023,6 +1023,43 @@ The following tables list existing packed RGB > formats. > >- g\ :sub:`2` > >- g\ :sub:`1` > >- g\ :sub:`0` > > +* .. _MEDIA-BUS-FMT-BGR666-1X24_CPADHI: > > I would move this just below MEDIA_BUS_FMT_RGB565_1X24_CPADHI. Actually, > could you check 1/7 and 2/7 to make sure the formats are sorted in the > documentation in the same order as in the header ? > > > + > > + - MEDIA_BUS_FMT_BGR666_1X24_CPADHI > > + - 0x1024 > > + - > > + - > > + - > > + - > > + - > > + - > > + - > > + - > > + - > > + - 0 > > + - 0 > > + - b\ :sub:`5` > > + - b\ :sub:`4` > > + - b\ :sub:`3` > > + - b\ :sub:`2` > > + - b\ :sub:`1` > > + - b\ :sub:`0` > > + - 0 > > + - 0 > > + - g\ :sub:`5` > > + - g\ :sub:`4` > > + - g\ :sub:`3` > > + - g\ :sub:`2` > > + - g\ :sub:`1` > > + - g\ :sub:`0` > > + - 0 > > + - 0 > > + - r\ :sub:`5` > > + - r\ :sub:`4` > > + - r\ :sub:`3` > > + - r\ :sub:`2` > > + - r\ :sub:`1` > > + - r\ :sub:`0` > > * .. _MEDIA-BUS-FMT-RGB565-1X24_CPADHI: > > > >- MEDIA_BUS_FMT_RGB565_1X24_CPADHI > > diff --git a/include/uapi/linux/media-bus-format.h > b/include/uapi/linux/media-bus-format.h > > index 2ee0b38c0a71..d4228d038b54 100644 > > --- a/include/uapi/linux/media-bus-format.h > > +++ b/include/uapi/linux/media-bus-format.h > > @@ -34,7 +34,7 @@ > > > > #define MEDIA_BUS_FMT_FIXED 0x0001 > > > > -/* RGB - next is 0x1024 */ > > +/* RGB - next is 0x1025 */ > > #define MEDIA_BUS_FMT_RGB444_1X120x1016 > > #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE0x1001 > > #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE0x1002 > > @@ -49,6 +49,7 @@ > > #define MEDIA_BUS_FMT_BGR666_1X180x1023 > > #define MEDIA_BUS_FMT_RGB666_1X180x1009 > > #define MEDIA_BUS_FMT_RBG888_1X240x100e > > +#define MEDIA_BUS_FMT_BGR666_1X24_CPADHI 0x1024 > > #define MEDIA_BUS_FMT_RGB666_1X24_CPADHI 0x1015 > > #define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG 0x1010 > > #define MEDIA_BUS_FMT_BGR888_1X240x1013 > > > > -- > Regards, > > Laurent Pinchart >
[PATCH v4 7/7] drm: rcar-du: dsi: Add r8A779g0 support
Add DSI support for r8a779g0. The main differences to r8a779a0 are in
the PLL and PHTW setups.
Signed-off-by: Tomi Valkeinen
Reviewed-by: Laurent Pinchart
---
Changes to v3:
- Use v3h as the default case in "switch(model)" tests
- Add Laurent's rb
drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 497 ++-
drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h | 6 +-
2 files changed, 375 insertions(+), 128 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index a7f2b7f66a17..e10e4d4b89a2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -28,6 +29,31 @@
#include "rcar_mipi_dsi.h"
#include "rcar_mipi_dsi_regs.h"
+#define MHZ(v) ((u32)((v) * 100U))
+
+enum rcar_mipi_dsi_hw_model {
+ RCAR_DSI_V3U,
+ RCAR_DSI_V4H,
+};
+
+struct rcar_mipi_dsi_device_info {
+ enum rcar_mipi_dsi_hw_model model;
+
+ const struct dsi_clk_config *clk_cfg;
+
+ u8 clockset2_m_offset;
+
+ u8 n_min;
+ u8 n_max;
+ u8 n_mul;
+ unsigned long fpfd_min;
+ unsigned long fpfd_max;
+ u16 m_min;
+ u16 m_max;
+ unsigned long fout_min;
+ unsigned long fout_max;
+};
+
struct rcar_mipi_dsi {
struct device *dev;
const struct rcar_mipi_dsi_device_info *info;
@@ -50,6 +76,17 @@ struct rcar_mipi_dsi {
unsigned int lanes;
};
+struct dsi_setup_info {
+ unsigned long hsfreq;
+ u16 hsfreqrange;
+
+ unsigned long fout;
+ u16 m;
+ u16 n;
+ u16 vclk_divider;
+ const struct dsi_clk_config *clkset;
+};
+
static inline struct rcar_mipi_dsi *
bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
{
@@ -62,65 +99,78 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
return container_of(host, struct rcar_mipi_dsi, host);
}
-static const u32 phtw[] = {
- 0x01020114, 0x01600115, /* General testing */
- 0x01030116, 0x0102011d, /* General testing */
- 0x011101a4, 0x018601a4, /* 1Gbps testing */
- 0x014201a0, 0x010001a3, /* 1Gbps testing */
- 0x0101011f, /* 1Gbps testing */
-};
-
-static const u32 phtw2[] = {
- 0x010c0130, 0x010c0140, /* General testing */
- 0x010c0150, 0x010c0180, /* General testing */
- 0x010c0190,
- 0x010a0160, 0x010a0170,
- 0x01800164, 0x01800174, /* 1Gbps testing */
-};
-
static const u32 hsfreqrange_table[][2] = {
- { 8000U, 0x00 }, { 9000U, 0x10 }, { 1U, 0x20 },
- { 11000U, 0x30 }, { 12000U, 0x01 }, { 13000U, 0x11 },
- { 14000U, 0x21 }, { 15000U, 0x31 }, { 16000U, 0x02 },
- { 17000U, 0x12 }, { 18000U, 0x22 }, { 19000U, 0x32 },
- { 20500U, 0x03 }, { 22000U, 0x13 }, { 23500U, 0x23 },
- { 25000U, 0x33 }, { 27500U, 0x04 }, { 3U, 0x14 },
- { 32500U, 0x25 }, { 35000U, 0x35 }, { 4U, 0x05 },
- { 45000U, 0x16 }, { 5U, 0x26 }, { 55000U, 0x37 },
- { 6U, 0x07 }, { 65000U, 0x18 }, { 7U, 0x28 },
- { 75000U, 0x39 }, { 8U, 0x09 }, { 85000U, 0x19 },
- { 9U, 0x29 }, { 95000U, 0x3a }, { 10U, 0x0a },
- { 105000U, 0x1a }, { 11U, 0x2a }, { 115000U, 0x3b },
- { 12U, 0x0b }, { 125000U, 0x1b }, { 13U, 0x2b },
- { 135000U, 0x3c }, { 14U, 0x0c }, { 145000U, 0x1c },
- { 15U, 0x2c }, { 155000U, 0x3d }, { 16U, 0x0d },
- { 165000U, 0x1d }, { 17U, 0x2e }, { 175000U, 0x3e },
- { 18U, 0x0e }, { 185000U, 0x1e }, { 19U, 0x2f },
- { 195000U, 0x3f }, { 20U, 0x0f }, { 205000U, 0x40 },
- { 21U, 0x41 }, { 215000U, 0x42 }, { 22U, 0x43 },
- { 225000U, 0x44 }, { 23U, 0x45 }, { 235000U, 0x46 },
- { 24U, 0x47 }, { 245000U, 0x48 }, { 25U, 0x49 },
+ { MHZ(80), 0x00 }, { MHZ(90), 0x10 }, { MHZ(100), 0x20 },
+ { MHZ(110), 0x30 }, { MHZ(120), 0x01 }, { MHZ(130), 0x11 },
+ { MHZ(140), 0x21 }, { MHZ(150), 0x31 }, { MHZ(160), 0x02 },
+ { MHZ(170), 0x12 }, { MHZ(180), 0x22 }, { MHZ(190), 0x32 },
+ { MHZ(205), 0x03 }, { MHZ(220), 0x13 }, { MHZ(235), 0x23 },
+ { MHZ(250), 0x33 }, { MHZ(275), 0x04 }, { MHZ(300), 0x14 },
+ { MHZ(325), 0x25 }, { MHZ(350), 0x35 }, { MHZ(400), 0x05 },
+ { MHZ(450), 0x16 }, { MHZ(500), 0x26 }, { MHZ(550), 0x37 },
+ { MHZ(600), 0x07 }, { MHZ(650), 0x18 }, { MHZ(700), 0x28 },
+ { MHZ(750), 0x39 }, { MHZ(800), 0x09 }, { MHZ(850), 0x19 },
+ { MHZ(900), 0x29 }, { MHZ(950), 0x3a }, { MHZ(1000), 0x0a },
+ { MHZ(1050), 0x1a }, {
[PATCH v2 0/5] Fix probe failed when modprobe modules
This patchset fixes similar issue, the root cause of the problem is that the virtqueues are not stopped on error handling path. Changes since v1: - Modify the description error of the test case and fixes tag information. - Add patch to fix virtio_gpu module. v1 at: https://lore.kernel.org/all/20221128021005.232105-1-lizet...@huawei.com/ Li Zetao (5): 9p: Fix probe failed when modprobe 9pnet_virtio virtio-mem: Fix probe failed when modprobe virtio_mem virtio-input: Fix probe failed when modprobe virtio_input virtio-blk: Fix probe failed when modprobe virtio_blk drm/virtio: Fix probe failed when modprobe virtio_gpu drivers/block/virtio_blk.c | 1 + drivers/gpu/drm/virtio/virtgpu_kms.c | 1 + drivers/virtio/virtio_input.c| 1 + drivers/virtio/virtio_mem.c | 1 + net/9p/trans_virtio.c| 1 + 5 files changed, 5 insertions(+) -- 2.25.1
[PATCH v2 4/5] virtio-blk: Fix probe failed when modprobe virtio_blk
When doing the following test steps, an error was found: step 1: modprobe virtio_blk succeeded # modprobe virtio_blk <-- OK step 2: fault injection in __blk_mq_alloc_disk() # modprobe -r virtio_blk <-- OK # ... CPU: 0 PID: 4578 Comm: modprobe Tainted: GW 6.1.0-rc6-00308-g644e9524388a-dirty Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), Call Trace: should_failslab+0xa/0x20 ... blk_alloc_queue+0x3a4/0x780 __blk_mq_alloc_disk+0x91/0x1f0 virtblk_probe+0x6ff/0x1f20 [virtio_blk] ... virtio_blk: probe of virtio1 failed with error -12 step 3: modprobe virtio_blk failed # modprobe virtio_blk <-- failed virtio_blk: probe of virtio1 failed with error -2 The root cause of the problem is that the virtqueues are not stopped on the error handling path when __blk_mq_alloc_disk() fails in virtblk_probe(), resulting in an error "-ENOENT" returned in the next modprobe call in setup_vq(). virtio_pci_modern_device uses virtqueues to send or receive message, and "queue_enable" records whether the queues are available. In vp_modern_find_vqs(), all queues will be selected and activated, but once queues are enabled there is no way to go back except reset. Fix it by reset virtio device on error handling path. After init_vq() succeeded, all virtqueues should be stopped on error handling path. Signed-off-by: Li Zetao --- v1 -> v2: modify the description error. drivers/block/virtio_blk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 19da5defd734..f401546d4e85 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -1157,6 +1157,7 @@ static int virtblk_probe(struct virtio_device *vdev) put_disk(vblk->disk); out_free_tags: blk_mq_free_tag_set(&vblk->tag_set); + virtio_reset_device(vdev); out_free_vq: vdev->config->del_vqs(vdev); kfree(vblk->vqs); -- 2.25.1
Re: Screen corruption using radeon kernel driver
On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote: > On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov wrote: > > > > On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher wrote: > > > > >>> [excessive quoting removed] > > > > >> So, is there any progress on this issue? I do understand it's not a high > > >> priority one, and today I've checked it on 6.0 kernel, and > > >> unfortunately, it still persists... > > >> > > >> I'm considering writing a patch that will allow user to override > > >> need_dma32/dma_bits setting with a module parameter. I'll have some time > > >> after the New Year for that. > > >> > > >> Is it at all possible that such a patch will be merged into kernel? > > >> > > > On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov wrote: > > > Unless someone familiar with HIMEM can figure out what is going wrong > > > we should just revert the patch. > > > > > > Alex > > > > > > Okay, I was suggesting that mostly because > > > > a) it works for me with dma_bits = 40 (I understand that's what it is > > without the original patch applied); > > > > b) there's a hint of uncertainity on this line > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359 > > saying that for AGP dma_bits = 32 is the safest option, so apparently there > > are > > setups, unlike mine, where dma_bits = 32 is better than 40. > > > > But I'm in no position to argue, just wanted to make myself clear. > > I'm okay with rebuilding the kernel for my machine until the original > > patch is reverted or any other fix is applied. > > What GPU do you have and is it AGP? If it is AGP, does setting > radeon.agpmode=-1 also fix it? > > Alex That is ATI Radeon X1950, and, unfortunately, radeon.agpmode=-1 doesn't help, it just makes 3D acceleration in games such as OpenArena stop working. signature.asc Description: PGP signature
Re: [PATCH v2 08/17] drm/tests: helpers: Allow for a custom device struct to be allocated
On 11/28/22 15:53, Maxime Ripard wrote: > The current helper to allocate a DRM device doesn't allow for any > subclassing by drivers, which is going to be troublesome as we work on > getting some kunit testing on atomic modesetting code. > > Let's use a similar pattern to the other allocation helpers by providing > the structure size and offset as arguments. > > Signed-off-by: Maxime Ripard > --- [...] > -EXPORT_SYMBOL(drm_kunit_helper_alloc_drm_device); > +EXPORT_SYMBOL(__drm_kunit_helper_alloc_drm_device); > I'm not sure if I would add a __ prefix to exported symbols, but I see that this is a convention in the DRM subsystem so I'm OK with it. Another thing that came to mind is if we want to use EXPORT_SYMBOL_GPL() instead for the DRM KUnit helpers. But that's not related to this series. Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v3 1/6] dt-bindings: mediatek: modify VDOSYS0 display device tree Documentations for MT8188
Il 29/11/22 15:34, nathan.lu ha scritto: From: Nathan Lu modify VDOSYS0 display device tree Documentations for MT8188. Signed-off-by: Nathan Lu Reviewed-by: Krzysztof Kozlowski Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v3 4/6] soc: mediatek: add mtk-mmsys support for mt8188 vdosys0
Il 29/11/22 15:35, nathan.lu ha scritto: From: Nathan Lu 1. add mt8188 mmsys 2. add mt8188 vdosys0 routing table settings Signed-off-by: amy zhang Signed-off-by: Nathan Lu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v3 6/6] drm/mediatek: add mediatek-drm of vdosys0 support for mt8188
Il 29/11/22 15:35, nathan.lu ha scritto: From: Nathan Lu add driver data of mt8188 vdosys0 to mediatek-drm and the sub driver. Signed-off-by: amy zhang Signed-off-by: Nathan Lu Since series [1] was explicitly requested by maintainers... Reviewed-by: AngeloGioacchino Del Regno [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=699386
Re: [PATCH v5 1/3] arm64: dts: qcom: add data-lanes and link-freuencies into dp_out endpoint
On Wed, 30 Nov 2022 at 02:12, Kuogee Hsieh wrote:
>
> Move data-lanes property from mdss_dp node to dp_out endpoint. Also
> add link-frequencies property into dp_out endpoint as well. The last
> frequency specified at link-frequencies will be the max link rate
> supported by DP.
>
> Changes in v5:
> -- revert changes at sc7180.dtsi and sc7280.dtsi
> -- add &dp_out to sc7180-trogdor.dtsi and sc7280-herobrine.dtsi
Bindings update?
>
> Signed-off-by: Kuogee Hsieh
> ---
> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 6 +-
> arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 6 +-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> index 754d2d6..39f0844 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -812,7 +812,11 @@ hp_i2c: &i2c9 {
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <&dp_hot_plug_det>;
> - data-lanes = <0 1>;
> +};
> +
> +&dp_out {
> +data-lanes = <0 1>;
> +link-frequencies = /bits/ 64 <16000 27000 54000>;
> };
>
> &pm6150_adc {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> index 93e39fc..b7c343d 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -440,7 +440,11 @@ ap_i2c_tpm: &i2c14 {
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <&dp_hot_plug_det>;
> - data-lanes = <0 1>;
> +};
> +
> +&dp_out {
> + data-lanes = <0 1>;
> + link-frequencies = /bits/ 64 <16000 27000 54000
> 81000>;
> };
>
> &mdss_mdp {
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
With best wishes
Dmitry
[PATCH RESEND2 v4 0/2] Use devm helpers for regulator get and enable
Simplify couple of drivers by using the new devm_regulator_*get_enable*() These patches were previously part of the series: https://lore.kernel.org/lkml/cover.1660934107.git.mazziesacco...@gmail.com/ "Devm helpers for regulator get and enable". I did keep the patch series versioning even though I changed the series name (subject of this mail) to "Use devm helpers for regulator get and enable". Name was changed because the devm helpers are already in 6.1-rc1. Also, most of the patches in the series are already merged to subsystem trees so this series now contains only the patches that have not yet been merged. I hope they can be now directly taken sirectly into respective subsystem trees as the dependencies should be in v6.1-rc1. Please note that these changes are only compile-tested as I don't have the HW to do proper testing. Thus, reviewing / testing is highly appreciated. Revision history: v4: RESEND2: - resend code unchanged. Commit titles and and commit messages improved. v4: RESEND: - resend only the drm patches - others have been applied v3 => v4: - Drop applied patches - rewrite cover-letter - rebase on v6.1-rc1 - split meson and sii902x into own patches as requested. - slightly modify dev_err_probe() return in sii902x. --- Matti Vaittinen (2): drm/bridge: sii902x: Use devm_regulator_bulk_get_enable() drm/meson: dw-hdmi: Use devm_regulator_*get_enable*() drivers/gpu/drm/bridge/sii902x.c | 26 -- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++ 2 files changed, 7 insertions(+), 42 deletions(-) base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa -- 2.38.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =] signature.asc Description: PGP signature
[PATCH v5 0/2] Add pixel formats used in Synatpics SoC
Those pixel formats are used in Synaptics's VideoSmart series SoCs, likes VS640, VS680. I just disclose the pixel formats used in the video codecs and display pipeline this time. Actually any device connected to the MTR module could support those tiling and compressed pixel formats. https://synaptics.com/products/multimedia-solutions Changelog: v5: Moving back the document and rewriting the description. v4: Removed the patches for V4L2, V4L2 would use the drm_fourcc.h . Moving the documents to the mesa project. v3: There was a mistake in format macro. Correcting the description of 64L4 variant modifiers. v2: The DRM modifiers in the first draft is too simple, it can't tell the tiles in group attribute in memory layout. Removing the v4l2 fourcc. Adding a document for the future v4l2 extended fmt. v1: first draft of DRM modifiers Try to put basic tile formats into v4l2 fourcc Hsia-Jun(Randy) Li (1): drm/fourcc: Add Synaptics VideoSmart tiled modifiers Randy Li (1): Documentation/gpu: Add Synaptics tiling formats documentation Documentation/gpu/drivers.rst | 1 + Documentation/gpu/synaptics.rst | 104 include/uapi/drm/drm_fourcc.h | 76 +++ 3 files changed, 181 insertions(+) create mode 100644 Documentation/gpu/synaptics.rst -- 2.37.3
Re: [pull] drm/msm: drm-msm-display-for-6.2
On Wed, 30 Nov 2022 at 09:02, Dave Airlie wrote: > > On Sat, 26 Nov 2022 at 20:21, Dmitry Baryshkov > wrote: > > > > Hi Dave, > > > > As agreed with Rob Clark, a pull request for the non-GPU part of the > > drm/msm driver. Summary below. > > > > The following changes since commit 7f7a942c0a338c4a2a7b359bdb2b68e9896122ec: > > > > Merge tag 'drm-next-20221025' of git://linuxtv.org/pinchartl/media into > > drm-next (2022-10-27 14:44:15 +1000) > > > > are available in the Git repository at: > > > > https://gitlab.freedesktop.org/lumag/msm.git tags/drm-msm-display-for-6.2 > > > > for you to fetch changes up to 8d1d17d47eaebe4466459846d07e4ba8953fa585: > > > > Merge branches 'msm-next-lumag-core', 'msm-next-lumag-dpu', > > 'msm-next-lumag-dp', 'msm-next-lumag-dsi', 'msm-next-lumag-hdmi' and > > 'msm-next-lumag-mdp5' into msm-next-lumag (2022-11-26 12:06:29 +0200) > > > > > > drm/msm updates for 6.2 > > > > Core: > > - MSM_INFO_GET_FLAGS support > > - Cleaned up MSM IOMMU wrapper code > > > > DPU: > > - Added support for XR30 and P010 image formats > > - Reworked MDSS/DPU schema, added SM8250 MDSS bindings > > - Added Qualcomm SM6115 support > > > > DP: > > - Dropped unsane sanity checks > > > > DSI: > > - Fix calculation of DSC pps payload > > > > DSI PHY: > > - DSI PHY support for QCM2290 > > > > HDMI: > > - Reworked dev init path > > > > > > Adam Skladowski (2): > > dt-bindings: display/msm: add support for SM6115 > > drm/msm/disp/dpu1: add support for display on SM6115 > > > > Bryan O'Donoghue (1): > > dt-bindings: msm: dsi-controller-main: Drop redundant phy-names > > > > Dan Carpenter (1): > > drm/msm/hdmi: remove unnecessary NULL check > > > > Dmitry Baryshkov (25): > > Merge remote-tracking branch 'msm/msm-fixes' into HEAD > > This commit has no justification or signed off by line, I'll let it > slide this once, but no backmerges without justification and please > sign off merges. Roger. -- With best wishes Dmitry
[PATCH v5 2/2] Documentation/gpu: Add Synaptics tiling formats documentation
From: Randy Li
Signed-off-by: Randy Li
Signed-off-by: Hsia-Jun(Randy) Li
---
Documentation/gpu/drivers.rst | 1 +
Documentation/gpu/synaptics.rst | 104
2 files changed, 105 insertions(+)
create mode 100644 Documentation/gpu/synaptics.rst
diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
index 3a52f48215a3..7e820c93d994 100644
--- a/Documentation/gpu/drivers.rst
+++ b/Documentation/gpu/drivers.rst
@@ -18,6 +18,7 @@ GPU Driver Documentation
xen-front
afbc
komeda-kms
+ synaptics
.. only:: subproject and html
diff --git a/Documentation/gpu/synaptics.rst b/Documentation/gpu/synaptics.rst
new file mode 100644
index ..b0564d2fe3ce
--- /dev/null
+++ b/Documentation/gpu/synaptics.rst
@@ -0,0 +1,104 @@
+.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
+
+
+Synaptics Tiling
+
+
+The tiling pixel formats in Synpatics Video Smart platform have
+many variants. Tiles could form the group of tiles(GOT), pixels
+within a group rectangle are stored into tile.
+There are two parameters which consist a modifier described
+the (nearest) width and height pixels in a group.
+
+Meanwhile, the tile in a group may not follow dimension
+layout, tile could form a small group of tiles, then that (sub)group
+of tiles would form a bigger group. We won't describe the dimension
+layout inside the group of tiles here. The layout of the group
+of tiles is fixed with the group width and height parameters
+in the same generation of the platform.
+
+Compression
+===
+The proprietary lossless image compression protocol in Synaptics
+could minimizes the amount of data transferred (less memory bandwidth
+consumption) between devices. It would usually apply to the tiling
+pixel format.
+
+Each component would request an extra page aligned length buffer
+for storing the compression meta data. Also a 32 bytes parameters
+set would come with a compression meta data buffer.
+
+The component here corresponds to a signal type (i.e. Luma, Chroma).
+They could be encoded into one or multiple metadata planes, but
+their compression parameters would still be individual.
+
+Pixel format modifiers
+==
+Addition alignment requirement for stride and size of a memory plane
+could apply beyond what has been mentioned below. Remember always
+negotiating with all the devices in pipeline before allocation.
+
+.. raw:: latex
+
+\small
+
+.. tabularcolumns:: |p{5.8cm}|p{1.2cm}|p{10.3cm}|
+
+.. cssclass:: longtable
+
+.. flat-table:: Synpatics Image Format Modifiers
+:header-rows: 1
+:stub-columns: 0
+:widths: 3 1 8
+
+* - Identifier
+ - Fourcc
+ - Details
+* .. _DRM-FORMAT-MOD-SYNA-V4H1
+
+ - ``DRM_FORMAT_MOD_SYNA_V4H1``
+ - ``DRM_FORMAT_NV12``
+ - The plain uncompressed 8 bits tile format. It sounds similar to
+ Intel's Y-tile. but it won't take any pixel from the next X direction
+ in a tile group. The line stride and image height must be aligned to
+ a multiple of 16. The height of chrominance plane would plus 8.
+* .. _DRM-FORMAT-MOD-SYNA-V4H3P8
+
+ - ``DRM_FORMAT_MOD_SYNA_V4H3P8``
+ - ``DRM_FORMAT_NV15``
+ - The plain uncompressed 10 bits tile format. It stores pixel in 2D
+ 3x4 tiles with a 8bits padding to each of tile. Then a tile is in a
+ 128 bits cache line.
+* .. _DRM-FORMAT-MOD-SYNA-V4H1-64L4-COMPRESSED
+
+ - ``DRM_FORMAT_MOD_SYNA_V4H1_64L4_COMPRESSED``
+ - ``DRM_FORMAT_NV12``
+ - Group of tiles and compressed variant of ``DRM_FORMAT_MOD_SYNA_V4H1``.
+ A group of tiles would contain 64x4 pixels, where a tile has 1x4
+ pixel.
+* .. _DRM-FORMAT-MOD-SYNA-V4H3P8-64L4-COMPRESSED
+
+ - ``DRM_FORMAT_MOD_SYNA_V4H3P8_64L4_COMPRESSED``
+ - ``DRM_FORMAT_NV15``
+ - Group of tiles and compressed variant of
``DRM_FORMAT_MOD_SYNA_V4H3P8``.
+ A group of tiles would contains 48x4 pixels, where a tile has 3x4 pixels
+ and a 8 bits padding in the end of a tile. A group of tiles would
+ be 256 bytes.
+* .. _DRM-FORMAT-MOD-SYNA-V4H1-128L128-COMPRESSED
+
+ - ``DRM_FORMAT_MOD_SYNA_V4H1_128L128_COMPRESSED``
+ - ``DRM_FORMAT_NV12``
+ - Group of tiles and compressed variant of ``DRM_FORMAT_MOD_SYNA_V4H1``.
+ A group of tiles would contain 128x32 pixels, where a tile has 1x4
pixel.
+* .. _DRM-FORMAT-MOD-SYNA-V4H3P8-128L128-COMPRESSED
+
+ - ``DRM_FORMAT_MOD_SYNA_V4H3P8_128L128_COMPRESSED``
+ - ``DRM_FORMAT_NV15``
+ - Group of tiles and compressed variant of
``DRM_FORMAT_MOD_SYNA_V4H3P8``.
+ A group of tiles would contains 96x128 pixels, where a tile has 3x4
pixels
+ and a 8 bits padding in the end of a tile. A group of tiles would
+ be 16 KiB.
+
+.. raw:: latex
+
+\normalsize
--
2.37.3
[PATCH v5 1/2] drm/fourcc: Add Synaptics VideoSmart tiled modifiers
From: "Hsia-Jun(Randy) Li"
Those modifiers only record the parameters would effort pixel
layout or memory layout. Whether physical memory page mapping
is used is not a part of format.
Signed-off-by: Hsia-Jun(Randy) Li
---
include/uapi/drm/drm_fourcc.h | 76 +++
1 file changed, 76 insertions(+)
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index bc056f2d537d..e0905f573f43 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -407,6 +407,7 @@ extern "C" {
#define DRM_FORMAT_MOD_VENDOR_ARM 0x08
#define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
#define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
+#define DRM_FORMAT_MOD_VENDOR_SYNAPTICS 0x0b
/* add more to the end as needed */
@@ -1507,6 +1508,81 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
#define AMD_FMT_MOD_CLEAR(field) \
(~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
+/*
+ * Synaptics VideoSmart modifiers
+ *
+ * Tiles could be arranged in Groups of Tiles (GOTs), it is a small tile
+ * within a tile. GOT size and layout varies based on platform and
+ * performance concern.
+ *
+ * Besides, an 8 length 4 bytes arrary (32 bytes) would be need to store
+ * some compression parameters for a compression metadata plane.
+ *
+ * Further information can be found in
+ * Documentation/gpu/synaptics.rst
+ *
+ * Macro
+ * Bits Param Description
+ * -
-
+ *
+ * 7:0 f Scan direction description.
+ *
+ * 0 = Invalid
+ * 1 = V4, the scan would always start from vertical for 4 pixel
+ * then move back to the start pixel of the next horizontal
+ * direction.
+ * 2 = Reserved for future use.
+ *
+ * 15:8 m The times of pattern repeat in the right angle direction from
+ * the first scan direction.
+ *
+ * 19:16 p The padding bits after the whole scan, could be zero.
+ *
+ * 20:20 g GOT packing flag.
+ *
+ * 23:21 - Reserved for future use. Must be zero.
+ *
+ * 27:24 h log2(horizontal) of pixels, in GOTs.
+ *
+ * 31:28 v log2(vertical) of pixels, in GOTs.
+ *
+ * 35:32 - Reserved for future use. Must be zero.
+ *
+ * 36:36 c Compression flag.
+ *
+ * 55:37 - Reserved for future use. Must be zero.
+ *
+ */
+
+#define DRM_FORMAT_MOD_SYNA_V4_TILED fourcc_mod_code(SYNAPTICS, 1)
+
+#define DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(f, m, p, g, h, v, c) \
+ fourcc_mod_code(SYNAPTICS, ((__u64)((f) & 0xff) | \
+((__u64)((m) & 0xff) << 8) | \
+((__u64)((p) & 0xf) << 16) | \
+((__u64)((g) & 0x1) << 20) | \
+((__u64)((h) & 0xf) << 24) | \
+((__u64)((v) & 0xf) << 28) | \
+((__u64)((c) & 0x1) << 36)))
+
+#define DRM_FORMAT_MOD_SYNA_V4H1 \
+ DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 1, 0, 0, 0, 0, 0)
+
+#define DRM_FORMAT_MOD_SYNA_V4H3P8 \
+ DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 3, 8, 0, 0, 0, 0)
+
+#define DRM_FORMAT_MOD_SYNA_V4H1_64L4_COMPRESSED \
+ DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 1, 0, 1, 6, 2, 1)
+
+#define DRM_FORMAT_MOD_SYNA_V4H3P8_64L4_COMPRESSED \
+ DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 3, 8, 1, 6, 2, 1)
+
+#define DRM_FORMAT_MOD_SYNA_V4H1_128L128_COMPRESSED \
+ DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 1, 0, 1, 7, 7, 1)
+
+#define DRM_FORMAT_MOD_SYNA_V4H3P8_128L128_COMPRESSED \
+ DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 3, 8, 1, 7, 7, 1)
+
#if defined(__cplusplus)
}
#endif
--
2.37.3
[PATCH RESEND2 v4 1/2] drm/bridge: sii902x: Use devm_regulator_bulk_get_enable()
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen
Acked-by: Robert Foss
---
v4 resend 2:
Resending unchanged code with commit title prefix adjusted as suggested
by Robert
I am doing a clean-up for my local git and encountered this one.
Respinning as it seems this one fell through the cracks.
---
drivers/gpu/drm/bridge/sii902x.c | 26 --
1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 878fb7d3732b..f6e8b401069b 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -171,7 +171,6 @@ struct sii902x {
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
- struct regulator_bulk_data supplies[2];
bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
@@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client,
struct device *dev = &client->dev;
struct device_node *endpoint;
struct sii902x *sii902x;
+ static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
ret = i2c_check_functionality(client->adapter,
@@ -1122,27 +1122,11 @@ static int sii902x_probe(struct i2c_client *client,
mutex_init(&sii902x->mutex);
- sii902x->supplies[0].supply = "iovcc";
- sii902x->supplies[1].supply = "cvcc12";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies),
supplies);
if (ret < 0)
- return ret;
-
- ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- if (ret < 0) {
- dev_err_probe(dev, ret, "Failed to enable supplies");
- return ret;
- }
+ return dev_err_probe(dev, ret, "Failed to enable supplies");
- ret = sii902x_init(sii902x);
- if (ret < 0) {
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- }
-
- return ret;
+ return sii902x_init(sii902x);
}
static void sii902x_remove(struct i2c_client *client)
@@ -1152,8 +1136,6 @@ static void sii902x_remove(struct i2c_client *client)
i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(&sii902x->bridge);
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
}
static const struct of_device_id sii902x_dt_ids[] = {
--
2.38.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
signature.asc
Description: PGP signature
[PATCH RESEND2 v4 2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
Simplify using the devm_regulator_get_enable_optional(). Also drop the
now unused struct member 'hdmi_supply'.
Signed-off-by: Matti Vaittinen
Martin Blumenstingl
---
v4 resend 2:
Respinning unchanged code with the commit title changed as wa suggested
by Robert Foss and commit message changed as was suggested by Martin.
I am doing a clean-up for my local git and encountered this one.
Respinning as it seems this one fell through the cracks.
---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi
*meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct
device *master,
meson_dw_hdmi->data = match;
dw_plat_data = &meson_dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
"hdmitx_apb");
--
2.38.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
signature.asc
Description: PGP signature
Re: [PATCH v5 3/3] drm/msm/dp: add support of max dp link rate
On Wed, 30 Nov 2022 at 02:12, Kuogee Hsieh wrote: > > By default, HBR2 (5.4G) is the max link link be supported. This patch add > the capability to support max link rate at HBR3 (8.1G). > > Changes in v2: > -- add max link rate from dtsi > > Changes in v3: > -- parser max_data_lanes and max_dp_link_rate from dp_out endpoint > > Changes in v4: > -- delete unnecessary pr_err > > Changes in v5: > -- split parser function into different patch > > Signed-off-by: Kuogee Hsieh > --- > drivers/gpu/drm/msm/dp/dp_display.c | 4 > drivers/gpu/drm/msm/dp/dp_panel.c | 7 --- > drivers/gpu/drm/msm/dp/dp_panel.h | 1 + > 3 files changed, 9 insertions(+), 3 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v3 2/6] dt-bindings: mediatek: modify VDOSYS0 mmsys device tree Documentations for MT8188
Il 29/11/22 15:34, nathan.lu ha scritto:
From: Nathan Lu
modify VDOSYS0 mmsys device tree Documentations for MT8188.
Signed-off-by: Nathan Lu
---
.../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml | 4
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
index 0711f1834fbd..3e7fb33201c5 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
@@ -47,6 +47,10 @@ properties:
- const: mediatek,mt2701-mmsys
- const: syscon
+ - items:
+ - const: mediatek,mt8188-vdosys0
The devicetree node will be like:
something: something@somewhere {
compatible = "mediatek,mt8188-vdosys0", "syscon";
};
and will never get any additional compatible string, as when you'll add
vdosys1 support, we'll get a similar node like:
something_else: something_else@somewhere_else {
comaptible = "mediatek,mt8188-vdosys1", "syscon";
};
...so this should go upper in the enum that's listing all of the
mediatek,mt-mmsys compatibles, specifically after `mediatek,mt8186-mmsys`.
Please fix.
Regards,
Angelo
Re: [PATCH v2 09/17] drm/tests: helpers: Allow to pass a custom drm_driver
On 11/28/22 15:53, Maxime Ripard wrote: > Some tests will need to provide their own drm_driver instead of relying > on the dumb one in the helpers, so let's create a helper that allows to > do so. > > Signed-off-by: Maxime Ripard > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v5 2/3] drm/msm/dp: parser data-lanes and link-frequencies from endpoint node
On Wed, 30 Nov 2022 at 02:12, Kuogee Hsieh wrote:
>
> Both data-lanes and link-frequencies are property of endpoint. This
> patch parser endpoint to retrieve max data lanes and max link rate
> supported specified at dp_out endpoint. In the case where no endpoint
> specified, then 4 data lanes with HBR2 link rate (5.4G) will be the
> default link configuration.
So, you have two changes in a single patch.
1) Moving the data-lanes to the endpoint
2) Adding link-frequencies.
Please split the patch accordingly. Also keep in mind that you have to
provide backwards compatibility for the data-lanes property.
>
> Signed-off-by: Kuogee Hsieh
> ---
> drivers/gpu/drm/msm/dp/dp_parser.c | 34 ++
> drivers/gpu/drm/msm/dp/dp_parser.h | 2 ++
> 2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c
> b/drivers/gpu/drm/msm/dp/dp_parser.c
> index dd73221..9367f8c 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -94,16 +94,34 @@ static int dp_parser_ctrl_res(struct dp_parser *parser)
> static int dp_parser_misc(struct dp_parser *parser)
> {
> struct device_node *of_node = parser->pdev->dev.of_node;
> - int len;
> -
> - len = drm_of_get_data_lanes_count(of_node, 1, DP_MAX_NUM_DP_LANES);
> - if (len < 0) {
> - DRM_WARN("Invalid property \"data-lanes\", default max DP
> lanes = %d\n",
> -DP_MAX_NUM_DP_LANES);
> - len = DP_MAX_NUM_DP_LANES;
> + struct device_node *endpoint;
> + int cnt;
> + u64 frequence[4];
frequency
> +
> + endpoint = of_graph_get_endpoint_by_regs(of_node, 1, 0); /* port@1 */
> + if (endpoint) {
> + cnt = of_property_count_u32_elems(endpoint, "data-lanes");
> + if (cnt < 0)
> + parser->max_dp_lanes = DP_MAX_NUM_DP_LANES; /* 4
> lanes */
> + else
> + parser->max_dp_lanes = cnt;
> +
> + cnt = of_property_count_u64_elems(endpoint,
> "link-frequencies");
> + if (cnt < 0) {
> + parser->max_dp_link_rate = DP_LINK_FREQUENCY_HBR2; /*
> 54000 khz */
Wrong number of zeroes
> + } else {
> + if (cnt > 4)/* 4 frequency at most */
> + cnt = 4;
'4 frequencies'. Not to mention that magic '4' should be defined
somewhere. Or removed completely. See below.
> + of_property_read_u64_array(endpoint,
> "link-frequencies", frequence, cnt);
Can you please use of_property_read_u64_index() instead? It also has a
nice feature of modifying the out_value only if the proper data was
found. So you can set the default and then override it with the
of_property_read function. And then divide it by 1000 to get the value
in KHz.
> + parser->max_dp_link_rate = (u32)frequence[cnt -1];
> + parser->max_dp_link_rate /= 1000; /* khz */
The HDR3 rate is 8100 Mb/s. 8 100 000 000. This doesn't fit into u32
(U32_MAX = 4 294 967 295).
> + }
> + } else {
> + /* default */
> + parser->max_dp_lanes = DP_MAX_NUM_DP_LANES; /* 4 lanes */
> + parser->max_dp_link_rate = DP_LINK_FREQUENCY_HBR2; /* 54000
> khz */
Wrong number of zeroes. Better use Mb/s or Gb/s directly. Also it is a
rate, not a frequency, so the define should also use 'RATE' in its
name.
> }
>
> - parser->max_dp_lanes = len;
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h
> b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 866c1a8..76ddb751 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -15,6 +15,7 @@
> #define DP_LABEL "MDSS DP DISPLAY"
> #define DP_MAX_PIXEL_CLK_KHZ 675000
> #define DP_MAX_NUM_DP_LANES4
> +#define DP_LINK_FREQUENCY_HBR2 54
>
> enum dp_pm_type {
> DP_CORE_PM,
> @@ -119,6 +120,7 @@ struct dp_parser {
> struct dp_io io;
> struct dp_display_data disp_data;
> u32 max_dp_lanes;
> + u32 max_dp_link_rate;
> struct drm_bridge *next_bridge;
>
> int (*parse)(struct dp_parser *parser);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
With best wishes
Dmitry
Re: [PATCH v2 15/17] drm/vc4: tests: Introduce a mocking infrastructure
On 11/28/22 15:53, Maxime Ripard wrote:
> In order to test the current atomic_check hooks we need to have a DRM
> device that has roughly the same capabilities and layout that the actual
> hardware. We'll also need a bunch of functions to create arbitrary
> atomic states.
>
> Let's create some helpers to create a device that behaves like the real
> one, and some helpers to maintain the atomic state we want to check.
>
> Signed-off-by: Maxime Ripard
> ---
[...]
> +
> +config DRM_VC4_KUNIT_TEST
> + bool "KUnit tests for VC4" if !KUNIT_ALL_TESTS
> + depends on DRM_VC4 && KUNIT
shouldn't this depend on DRM_KUNIT_TEST instead ?
[...]
> +static struct vc4_dev *__mock_device(struct kunit *test, bool is_vc5)
> +{
> + struct drm_device *drm;
> + const struct drm_driver *drv = is_vc5 ? &vc5_drm_driver :
> &vc4_drm_driver;
> + const struct vc4_mock_desc *desc = is_vc5 ? &vc5_mock : &vc4_mock;
> + struct vc4_dev *vc4;
Since it could be vc4 or vc5, maybe can be renamed to just struct vc_dev *vc ?
> +struct vc4_dummy_plane *vc4_dummy_plane(struct kunit *test,
> + struct drm_device *drm,
> + enum drm_plane_type type)
> +{
> + struct vc4_dummy_plane *dummy_plane;
> + struct drm_plane *plane;
> +
> + dummy_plane = drmm_universal_plane_alloc(drm,
> + struct vc4_dummy_plane,
> plane.base,
> + 0,
> + &vc4_dummy_plane_funcs,
> + vc4_dummy_plane_formats,
> +
> ARRAY_SIZE(vc4_dummy_plane_formats),
> + NULL,
> + DRM_PLANE_TYPE_PRIMARY,
> + NULL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_plane);
> +
> + plane = &dummy_plane->plane.base;
> + drm_plane_helper_add(plane, &vc4_dummy_plane_helper_funcs);
> +
> + return dummy_plane;
> +}
I guess many of these helpers could grow to be generic, like this one since
most drivers support the DRM_FORMAT_XRGB format for their primary plane.
[...]
>
> +extern const struct vc4_pv_data bcm2835_pv0_data;
> +extern const struct vc4_pv_data bcm2835_pv1_data;
> +extern const struct vc4_pv_data bcm2835_pv2_data;
> +extern const struct vc4_pv_data bcm2711_pv0_data;
> +extern const struct vc4_pv_data bcm2711_pv1_data;
> +extern const struct vc4_pv_data bcm2711_pv2_data;
> +extern const struct vc4_pv_data bcm2711_pv3_data;
> +extern const struct vc4_pv_data bcm2711_pv4_data;
> +
Maybe the driver could expose a helper function to get the pixelvalve data
and avoid having to expose all of these variables? For example you could
define an enum vc4_pixelvalve type and have something like the following:
const struct vc4_pv_data *vc4_crtc_get_pixelvalve_data(enum vc4_pixelvalve pv);
All these are small nits though, the patch looks great to me and I think is
awesome to have this level of testing with KUnit. Hope other drivers follow
your lead.
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH v2 16/17] drm/vc4: tests: Fail the current test if we access a register
On 11/28/22 15:53, Maxime Ripard wrote:
> Accessing a register when running under kunit is a bad idea since our
> device is completely mocked.
>
> Fail the current test if we ever access any of our hardware registers.
>
> Signed-off-by: Maxime Ripard
> ---
[...]
> -#define CRTC_WRITE(offset, val) writel(val, vc4_crtc->regs + (offset))
> -#define CRTC_READ(offset) readl(vc4_crtc->regs + (offset))
> +#define CRTC_WRITE(offset, val)
> \
> + do {
> \
> + kunit_fail_current_test("Accessing a register in a unit
> test!\n"); \
> + writel(val, vc4_crtc->regs + (offset));
> \
> + } while (0)
> +
> +#define CRTC_READ(offset)
> \
> + ({
> \
> + kunit_fail_current_test("Accessing a register in a unit
> test!\n"); \
> + readl(vc4_crtc->regs + (offset));
> \
> + })
>
Should this be made conditional on whether DRM_VC4_KUNIT_TEST is enabled ?
That is, just define the simpler macros when is disabled? The
kunit_fail_current_test()
is just a no-op if CONFIG_KUNIT isn't enabled, but I think my question still
stands.
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH v2 17/17] drm/vc4: tests: Add unit test suite for the PV muxing
On 11/28/22 15:53, Maxime Ripard wrote: > The HVS to PixelValve muxing code is fairly error prone and has a bunch > of arbitrary constraints due to the hardware setup. > > Let's create a test suite that makes sure that the possible combinations > work and the invalid ones don't. > > Signed-off-by: Maxime Ripard > --- Thanks for this patch. It shows how powerful KUnit can be for testing drivers. Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: Try to address the DMA-buf coherency problem
On Fri, Nov 25, 2022 at 11:40:04AM -0500, Nicolas Dufresne wrote: > Le mercredi 23 novembre 2022 à 17:33 +0100, Daniel Vetter a écrit : > > On Wed, Nov 23, 2022 at 10:33:38AM +0200, Pekka Paalanen wrote: > > > On Tue, 22 Nov 2022 18:33:59 +0100 > > > Christian König wrote: > > > > > > > We should have come up with dma-heaps earlier and make it clear that > > > > exporting a DMA-buf from a device gives you something device specific > > > > which might or might not work with others. > > > > > > > > Apart from that I agree, DMA-buf should be capable of handling this. > > > > Question left is what documentation is missing to make it clear how > > > > things are supposed to work? > > > > > > Perhaps somewhat related from Daniel Stone that seems to have been > > > forgotten: > > > https://lore.kernel.org/dri-devel/20210905122742.86029-1-dani...@collabora.com/ > > > > > > It aimed mostly at userspace, but sounds to me like the coherency stuff > > > could use a section of its own there? > > > > Hm yeah it would be great to land that and then eventually extend. Daniel? > > There is a lot of things documented in this document that have been said to be > completely wrong user-space behaviour in this thread. But it seems to pre-date > the DMA Heaps. The document also assume that DMA Heaps completely solves the > CMA > vs system memory issue. But it also underline a very important aspect, that > userland is not aware which one to use. What this document suggest though > seems > more realist then what has been said here. > > Its overall a great document, it unfortunate that it only makes it into the > DRM > mailing list. The doc is more about document the current status quo/best practices, which is very much not using dma-heaps. The issue there is that currently userspace has no idea which dma-heap to use for shared buffers, plus not all allocators are exposed through heaps to begin with. We had this noted as a todo item (add some device->heap sysfs links was the idea), until that's done all you can do is hardcode the right heaps for the right usage in userspace, which is what android does. Plus android doesn't have dgpu, so doesn't need the missing ttm heap. But yeah the long-term aspiration also hasn't changed, because the dma-heap todo list is also very, very old by now :-/ -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] drm/mediatek: Add checks for devm_kcalloc
On Fri, Nov 25, 2022 at 01:41:23AM +, Yuan Can wrote:
> As the devm_kcalloc may return NULL, the return value needs to be checked
> to avoid NULL poineter dereference.
>
> Fixes: 66b2cf9623fa ("drm/mediatek: use layer_nr function to get layer number
> to init plane")
> Signed-off-by: Yuan Can
> ---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 112615817dcb..5071f1263216 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -945,6 +945,8 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
>
> mtk_crtc->planes = devm_kcalloc(dev, num_comp_planes,
> sizeof(struct drm_plane), GFP_KERNEL);
> + if (!mtk_crtc->planes)
> + return -ENOMEM;
These (= drm object allocations like drm_planes, drm_crtc) should all be
moved over to drmm_, since devm_ has the wrong lifetimes.
Just in case you want to spend a pile of time in here, the error handling
needs to be fixed either way.
-Daniel
>
> for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> ret = mtk_drm_crtc_init_comp_planes(drm_dev, mtk_crtc, i,
> --
> 2.17.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v3 0/2] drm: Add GPU reset sysfs
On Fri, Nov 25, 2022 at 02:52:01PM -0300, André Almeida wrote: > This patchset adds a udev event for DRM device's resets. > > Userspace apps can trigger GPU resets by misuse of graphical APIs or driver > bugs. Either way, the GPU reset might lead the system to a broken state[1], > that > might be recovered if user has access to a tty or a remote shell. Arguably, > this > recovery could happen automatically by the system itself, thus this is the > goal > of this patchset. > > For debugging and report purposes, device coredump support was already added > for amdgpu[2], but it's not suitable for programmatic usage like this one > given > the uAPI not being stable and the need for parsing. > > GL/VK is out of scope for this use, giving that we are dealing with device > resets regardless of API. > > A basic userspace daemon is provided at [3] showing how the interface is used > to recovery from resets. > > [1] A search for "reset" in DRM/AMD issue tracker shows reports of resets > making the system unusable: > https://gitlab.freedesktop.org/drm/amd/-/issues/?search=reset > > [2] > https://lore.kernel.org/amd-gfx/20220602081538.1652842-2-amaranath.somalapu...@amd.com/ > > [3] https://gitlab.freedesktop.org/andrealmeid/gpu-resetd > > v2: > https://lore.kernel.org/dri-devel/20220308180403.75566-1-contactshashanksha...@gmail.com/ > > André Almeida (1): > drm/amdgpu: Add work function for GPU reset event > > Shashank Sharma (1): > drm: Add GPU reset sysfs event This seems a bit much amd specific, and a bit much like an ad-hoc stopgap. On the amd specific piece: - amd's gpus suck the most for gpu hangs, because aside from the shader unblock, there's only device reset, which thrashes vram and display and absolutely everything. Which is terrible. Everyone else has engine only reset since years (i.e. doesn't thrash display or vram), and very often even just context reset (i.e. unless the driver is busted somehow or hw bug, malicious userspace will _only_ ever impact itself). - robustness extensions for gl/vk already have very clear specifications of all cases of reset, and this work here just ignores that. Yes on amd you only have device reset, but this is drm infra, so you need to be able to cope with ctx reset or reset which only affected a limited set of context. If this is for compute and compute apis lack robustness extensions, then those apis need to be fixed to fill that gap. - the entire deamon thing feels a bit like overkill and I'm not sure why it exists. I think for a start it would be much simpler if we just have a (per-device maybe) sysfs knob to enable automatic killing of process that die and which don't have arb robustness enabled (for gl case, for vk case the assumption is that _every_ app supports VK_DEVICE_LOST and can recover). Now onto the ad-hoc part: - Everyone hand-rolls ad-hoc gpu context structures and how to associate them with a pid. I think we need to stop doing that, because it's just endless pain and prevents us from building useful management stuff like cgroups for drivers that work across drivers (and driver/vendor specific cgroup wont be accepted by upstream cgroup maintainers). Or gpu reset events and dumps like here. This is going to be some work unforutnately. - I think the best starting point is the context structure drm/scheduler already has, but it needs some work: * untangling it from the scheduler part, so it can be used also for compute context that are directly scheduled by hw * (amd specific) moving amdkfd over to that context structure, at least internally * tracking the pid in there - I think the error dump facility should also be integrated into this. Userspace needs to know which dump is associated with which reset event, so that remote crash reporting works correctly. - ideally this framework can keep track of impacted context so that drivers don't have to reinvent the "which context are impacted" robustness ioctl book-keeping all on their own. For amd gpus it's kinda easy, since the impact is "everything", but for other gpus the impact can be all the way from "only one context" to "only contexts actively running on $set_of_engines" to "all the context actively running" to "we thrashed vram, everything is gone" - i915 has a bunch of this already, but I have honestly no idea whether it's any use because i915-gem is terminally not switching over to drm/scheduler (it needs a full rewrite, which is happening somewhere). So might only be useful to look at to make sure we're not building something which only works for full device reset gpus and nothing else. Over the various generations i915 has pretty much every possible gpu reset options you can think of, with resulting different reporting requirements to make sure robustness extensions work correctly. - pid isn't enough once you have engine/context reset, you need pid (well drm_file really, but I guess we
Re: [PATCH 04/12] arm64: dts: qcom: sm6115: Add TSENS node
On 29.11.2022 21:46, Adam Skladowski wrote:
> Add nodes required for TSENS block using the common qcom,tsens-v2 binding.
>
> Signed-off-by: Adam Skladowski
> ---
Reviewed-by: Konrad Dybcio
Konrad
> arch/arm64/boot/dts/qcom/sm6115.dtsi | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index 2003a2519a54..decbf7ca8a03 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -515,6 +515,17 @@ spmi_bus: spmi@1c4 {
> #interrupt-cells = <4>;
> };
>
> + tsens0: thermal-sensor@441 {
> + compatible = "qcom,sm6115-tsens", "qcom,tsens-v2";
> + reg = <0x04411000 0x1ff>, /* TM */
> + <0x0441 0x8>; /* SROT */
> + #qcom,sensors = <16>;
> + interrupts = ,
> + ;
> + interrupt-names = "uplow", "critical";
> + #thermal-sensor-cells = <1>;
> + };
> +
> rpm_msg_ram: sram@45f {
> compatible = "qcom,rpm-msg-ram";
> reg = <0x045f 0x7000>;
Re: [PATCH 05/12] arm64: dts: qcom: sm6115: Add PRNG node
On 29.11.2022 21:46, Adam Skladowski wrote:
> Add a node for the PRNG to enable hw-accelerated pseudo-random number
> generation.
>
> Signed-off-by: Adam Skladowski
> ---
> ar
Reviewed-by: Konrad Dybcio
Konrad
ch/arm64/boot/dts/qcom/sm6115.dtsi | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index decbf7ca8a03..04620c272227 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -497,6 +497,13 @@ qusb2_hstx_trim: hstx-trim@25b {
> };
> };
>
> + rng: rng@1b53000 {
> + compatible = "qcom,prng-ee";
> + reg = <0x01b53000 0x1000>;
> + clocks = <&gcc GCC_PRNG_AHB_CLK>;
> + clock-names = "core";
> + };
> +
> spmi_bus: spmi@1c4 {
> compatible = "qcom,spmi-pmic-arb";
> reg = <0x01c4 0x1100>,
Re: [PATCH 06/12] arm64: dts: qcom: sm6115: Add rpm-stats node
On 29.11.2022 21:46, Adam Skladowski wrote:
> Add rpm stats node.
>
> Signed-off-by: Adam Skladowski
> ---
Reviewed-by: Konrad Dybcio
Konrad
> arch/arm64/boot/dts/qcom/sm6115.dtsi | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index 04620c272227..6d14bbcda9d3 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -538,6 +538,11 @@ rpm_msg_ram: sram@45f {
> reg = <0x045f 0x7000>;
> };
>
> + sram@469 {
> + compatible = "qcom,rpm-stats";
> + reg = <0x0469 0x1>;
> + };
> +
> sdhc_1: mmc@4744000 {
> compatible = "qcom,sm6115-sdhci", "qcom,sdhci-msm-v5";
> reg = <0x04744000 0x1000>, <0x04745000 0x1000>,
> <0x04748000 0x8000>;
Re: [PATCH 07/12] arm64: dts: qcom: sm6115: Add dispcc node
On 29.11.2022 21:46, Adam Skladowski wrote:
> Add display clock controller to allow controlling display related clocks.
>
> Signed-off-by: Adam Skladowski
> ---
Reviewed-by: Konrad Dybcio
Konrad
> arch/arm64/boot/dts/qcom/sm6115.dtsi | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index 6d14bbcda9d3..ea0e0b3c5d84 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -4,6 +4,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -717,6 +718,19 @@ usb_1_dwc3: usb@4e0 {
> };
> };
>
> + dispcc: clock-controller@5f0 {
> + compatible = "qcom,sm6115-dispcc";
> + reg = <0x05f0 0x2>;
> + clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> + <&sleep_clk>,
> + <&dsi0_phy 0>,
> + <&dsi0_phy 1>,
> + <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + #power-domain-cells = <1>;
> + };
> +
> apps_smmu: iommu@c60 {
> compatible = "qcom,sm6115-smmu-500", "arm,mmu-500";
> reg = <0x0c60 0x8>;
Re: [PATCH 08/12] arm64: dts: qcom: sm6115: Add mdss/dpu node
On 29.11.2022 21:46, Adam Skladowski wrote:
> Add mdss and dpu node to enable display support on SM6115.
>
> Signed-off-by: Adam Skladowski
> ---
Reviewed-by: Konrad Dybcio
Konrad
> arch/arm64/boot/dts/qcom/sm6115.dtsi | 183 +++
> 1 file changed, 183 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index ea0e0b3c5d84..b459f1746a7f 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -718,6 +718,189 @@ usb_1_dwc3: usb@4e0 {
> };
> };
>
> + mdss: display-subsystem@5e0 {
> + compatible = "qcom,sm6115-mdss";
> + reg = <0x05e0 0x1000>;
> + reg-names = "mdss";
> +
> + power-domains = <&dispcc MDSS_GDSC>;
> +
> + clocks = <&gcc GCC_DISP_AHB_CLK>,
> + <&gcc GCC_DISP_HF_AXI_CLK>,
> + <&dispcc DISP_CC_MDSS_MDP_CLK>;
> +
> + interrupts = ;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + iommus = <&apps_smmu 0x420 0x2>,
> + <&apps_smmu 0x421 0x0>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + status = "disabled";
> +
> + mdp: display-controller@5e01000 {
> + compatible = "qcom,sm6115-dpu";
> + reg = <0x05e01000 0x8f000>,
> + <0x05eb 0x2008>;
> + reg-names = "mdp", "vbif";
> +
> + clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> + <&dispcc DISP_CC_MDSS_AHB_CLK>,
> + <&dispcc DISP_CC_MDSS_MDP_CLK>,
> + <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> + <&dispcc DISP_CC_MDSS_ROT_CLK>,
> + <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> + clock-names = "bus",
> + "iface",
> + "core",
> + "lut",
> + "rot",
> + "vsync";
> +
> + operating-points-v2 = <&mdp_opp_table>;
> + power-domains = <&rpmpd SM6115_VDDCX>;
> +
> + interrupt-parent = <&mdss>;
> + interrupts = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + dpu_intf1_out: endpoint {
> + remote-endpoint =
> <&dsi0_in>;
> + };
> + };
> + };
> +
> + mdp_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-1920 {
> + opp-hz = /bits/ 64 <1920>;
> + required-opps =
> <&rpmpd_opp_min_svs>;
> + };
> +
> + opp-19200 {
> + opp-hz = /bits/ 64 <19200>;
> + required-opps =
> <&rpmpd_opp_low_svs>;
> + };
> +
> + opp-25600 {
> + opp-hz = /bits/ 64 <25600>;
> + required-opps =
> <&rpmpd_opp_svs>;
> + };
> +
> + opp-30720 {
> + opp-hz = /bits/ 64 <30720>;
> + required-opps =
> <&rpmpd_opp_svs_plus>;
> + };
> +
> + opp-38400 {
> + opp-hz = /bits/ 64 <38400>;
> + required-opps =
> <&rpmpd_opp_nom>;
> + };
> + };
> + };
> +
> + dsi0: d
Re: [PATCH 09/12] arm64: dts: qcom: sm6115: Add GPI DMA
On 29.11.2022 21:46, Adam Skladowski wrote:
> Add GPI DMA node which will be wired to i2c/spi/uart.
>
> Signed-off-by: Adam Skladowski
> ---
Reviewed-by: Konrad Dybcio
Konrad
> arch/arm64/boot/dts/qcom/sm6115.dtsi | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index b459f1746a7f..e9de7aa1efdd 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -673,6 +673,26 @@ ufs_mem_phy_lanes: phy@4807400 {
> };
> };
>
> + gpi_dma0: dma-controller@4a0 {
> + compatible = "qcom,sm6115-gpi-dma",
> "qcom,sm6350-gpi-dma";
> + reg = <0x04a0 0x6>;
> + interrupts = ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ;
> + dma-channels = <10>;
> + dma-channel-mask = <0xf>;
> + iommus = <&apps_smmu 0xf6 0x0>;
> + #dma-cells = <3>;
> + status = "disabled";
> + };
> +
> usb_1: usb@4ef8800 {
> compatible = "qcom,sm6115-dwc3", "qcom,dwc3";
> reg = <0x04ef8800 0x400>;
Re: [PATCH 1/2] drm/shmem-helper: Remove errant put in error path
On Tue, Nov 29, 2022 at 12:02:41PM -0800, Rob Clark wrote:
> From: Rob Clark
>
> drm_gem_shmem_mmap() doesn't own this reference!
>
> Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Rob Clark
With Guenter's nits addressed:
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 35138f8a375c..110a9eac2af8 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -623,7 +623,6 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object
> *shmem, struct vm_area_struct
>
> ret = drm_gem_shmem_get_pages(shmem);
> if (ret) {
> - drm_gem_vm_close(vma);
> return ret;
> }
>
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 12/12] arm64: dts: qcom: sm6115: Fallback smmu to qcom generic compatible
On 29.11.2022 21:46, Adam Skladowski wrote:
> Change fallback to qcom generic compatible
> in order to prevent reboot during SMMU initialization.
>
> Signed-off-by: Adam Skladowski
> ---
Reviewed-by: Konrad Dybcio
Konrad
> arch/arm64/boot/dts/qcom/sm6115.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index 36d1cff23d10..b00d74055eb1 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -1222,7 +1222,7 @@ dispcc: clock-controller@5f0 {
> };
>
> apps_smmu: iommu@c60 {
> - compatible = "qcom,sm6115-smmu-500", "arm,mmu-500";
> + compatible = "qcom,sm6115-smmu-500", "qcom,smmu-500",
> "arm,mmu-500";
> reg = <0x0c60 0x8>;
> #iommu-cells = <2>;
> #global-interrupts = <1>;
Re: [2/2] drm/shmem-helper: Avoid vm_open error paths
On Tue, Nov 29, 2022 at 12:47:42PM -0800, Rob Clark wrote:
> On Tue, Nov 29, 2022 at 12:32 PM Guenter Roeck wrote:
> >
> > On Tue, Nov 29, 2022 at 12:02:42PM -0800, Rob Clark wrote:
> > > From: Rob Clark
> > >
> > > vm_open() is not allowed to fail. Fortunately we are guaranteed that
> > > the pages are already pinned, and only need to increment the refcnt. So
> > > just increment it directly.
> >
> > I don't know anything about drm or gem, but I am wondering _how_
> > this would be guaranteed. Would it be through the pin function ?
> > Just wondering, because that function does not seem to be mandatory.
>
> We've pinned the pages already in mmap.. vm->open() is perhaps not the
> best name for the callback function, but it is called for copying an
> existing vma into a new process (and for some other cases which do not
> apply here because VM_DONTEXPAND).
>
> (Other drivers pin pages in the fault handler, where there is actually
> potential to return an error, but that change was a bit more like
> re-writing shmem helper ;-))
Yhea vm_ops->open should really be called vm_ops->dupe or ->copy or
something like that ...
-Daniel
>
> BR,
> -R
>
> > >
> > > Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects")
> > > Cc: sta...@vger.kernel.org
> > > Signed-off-by: Rob Clark
> > > ---
> > > drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++---
> > > 1 file changed, 11 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > > b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > > index 110a9eac2af8..9885ba64127f 100644
> > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > > @@ -571,12 +571,20 @@ static void drm_gem_shmem_vm_open(struct
> > > vm_area_struct *vma)
> > > {
> > > struct drm_gem_object *obj = vma->vm_private_data;
> > > struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> > > - int ret;
> > >
> > > WARN_ON(shmem->base.import_attach);
> > >
> > > - ret = drm_gem_shmem_get_pages(shmem);
> > > - WARN_ON_ONCE(ret != 0);
> > > + mutex_lock(&shmem->pages_lock);
> > > +
> > > + /*
> > > + * We should have already pinned the pages, vm_open() just grabs
> >
> > should or guaranteed ? This sounds a bit weaker than the commit
> > description.
> >
> > > + * an additional reference for the new mm the vma is getting
> > > + * copied into.
> > > + */
> > > + WARN_ON_ONCE(!shmem->pages_use_count);
> > > +
> > > + shmem->pages_use_count++;
> > > + mutex_unlock(&shmem->pages_lock);
> >
> > The previous code, in that situation, would not increment pages_use_count,
> > and it would not set not set shmem->pages. Hopefully, it would not try to
> > do anything with the pages it was unable to get. The new code assumes that
> > shmem->pages is valid even if pages_use_count is 0, while at the same time
> > taking into account that this can possibly happen (or the WARN_ON_ONCE
> > would not be needed).
> >
> > Again, I don't know anything about gem and drm, but it seems to me that
> > there might now be a severe problem later on if the WARN_ON_ONCE()
> > ever triggers.
> >
> > Thanks,
> > Guenter
> >
> > >
> > > drm_gem_vm_open(vma);
> > > }
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v5 1/2] drm/fourcc: Add Synaptics VideoSmart tiled modifiers
On Wed, Nov 30, 2022 at 05:21:48PM +0800, Hsia-Jun Li wrote:
> From: "Hsia-Jun(Randy) Li"
>
> Those modifiers only record the parameters would effort pixel
> layout or memory layout. Whether physical memory page mapping
> is used is not a part of format.
>
> Signed-off-by: Hsia-Jun(Randy) Li
> ---
> include/uapi/drm/drm_fourcc.h | 76 +++
> 1 file changed, 76 insertions(+)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index bc056f2d537d..e0905f573f43 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -407,6 +407,7 @@ extern "C" {
> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
> +#define DRM_FORMAT_MOD_VENDOR_SYNAPTICS 0x0b
>
> /* add more to the end as needed */
>
> @@ -1507,6 +1508,81 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64
> modifier)
> #define AMD_FMT_MOD_CLEAR(field) \
> (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
>
> +/*
> + * Synaptics VideoSmart modifiers
> + *
> + * Tiles could be arranged in Groups of Tiles (GOTs), it is a small tile
> + * within a tile. GOT size and layout varies based on platform and
> + * performance concern.
> + *
> + * Besides, an 8 length 4 bytes arrary (32 bytes) would be need to store
> + * some compression parameters for a compression metadata plane.
> + *
> + * Further information can be found in
> + * Documentation/gpu/synaptics.rst
> + *
> + * Macro
> + * Bits Param Description
> + * -
> -
> + *
> + * 7:0 f Scan direction description.
> + *
> + * 0 = Invalid
> + * 1 = V4, the scan would always start from vertical for 4
> pixel
> + * then move back to the start pixel of the next horizontal
> + * direction.
> + * 2 = Reserved for future use.
> + *
> + * 15:8 m The times of pattern repeat in the right angle direction from
> + * the first scan direction.
> + *
> + * 19:16 p The padding bits after the whole scan, could be zero.
> + *
> + * 20:20 g GOT packing flag.
> + *
> + * 23:21 - Reserved for future use. Must be zero.
Can you pls fold all the future use reservations into the top end? Also I
think it'd be good to at least reserve maybe the top 8 bits or so for a
synaptics specific format indicator, so that it's easier to extend this in
the future ...
-Daniel
> + *
> + * 27:24 h log2(horizontal) of pixels, in GOTs.
> + *
> + * 31:28 v log2(vertical) of pixels, in GOTs.
> + *
> + * 35:32 - Reserved for future use. Must be zero.
> + *
> + * 36:36 c Compression flag.
> + *
> + * 55:37 - Reserved for future use. Must be zero.
> + *
> + */
> +
> +#define DRM_FORMAT_MOD_SYNA_V4_TILED fourcc_mod_code(SYNAPTICS, 1)
> +
> +#define DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(f, m, p, g, h, v, c) \
> + fourcc_mod_code(SYNAPTICS, ((__u64)((f) & 0xff) | \
> + ((__u64)((m) & 0xff) << 8) | \
> + ((__u64)((p) & 0xf) << 16) | \
> + ((__u64)((g) & 0x1) << 20) | \
> + ((__u64)((h) & 0xf) << 24) | \
> + ((__u64)((v) & 0xf) << 28) | \
> + ((__u64)((c) & 0x1) << 36)))
> +
> +#define DRM_FORMAT_MOD_SYNA_V4H1 \
> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 1, 0, 0, 0, 0, 0)
> +
> +#define DRM_FORMAT_MOD_SYNA_V4H3P8 \
> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 3, 8, 0, 0, 0, 0)
> +
> +#define DRM_FORMAT_MOD_SYNA_V4H1_64L4_COMPRESSED \
> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 1, 0, 1, 6, 2, 1)
> +
> +#define DRM_FORMAT_MOD_SYNA_V4H3P8_64L4_COMPRESSED \
> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 3, 8, 1, 6, 2, 1)
> +
> +#define DRM_FORMAT_MOD_SYNA_V4H1_128L128_COMPRESSED \
> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 1, 0, 1, 7, 7, 1)
> +
> +#define DRM_FORMAT_MOD_SYNA_V4H3P8_128L128_COMPRESSED \
> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 3, 8, 1, 7, 7, 1)
> +
> #if defined(__cplusplus)
> }
> #endif
> --
> 2.37.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: Screen corruption using radeon kernel driver
On 2022-11-29 17:11, Mikhail Krylov wrote: On Tue, Nov 29, 2022 at 11:05:28AM -0500, Alex Deucher wrote: On Tue, Nov 29, 2022 at 10:59 AM Mikhail Krylov wrote: On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote: On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov wrote: On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher wrote: [excessive quoting removed] So, is there any progress on this issue? I do understand it's not a high priority one, and today I've checked it on 6.0 kernel, and unfortunately, it still persists... I'm considering writing a patch that will allow user to override need_dma32/dma_bits setting with a module parameter. I'll have some time after the New Year for that. Is it at all possible that such a patch will be merged into kernel? On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov wrote: Unless someone familiar with HIMEM can figure out what is going wrong we should just revert the patch. Alex Okay, I was suggesting that mostly because a) it works for me with dma_bits = 40 (I understand that's what it is without the original patch applied); b) there's a hint of uncertainity on this line https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359 saying that for AGP dma_bits = 32 is the safest option, so apparently there are setups, unlike mine, where dma_bits = 32 is better than 40. But I'm in no position to argue, just wanted to make myself clear. I'm okay with rebuilding the kernel for my machine until the original patch is reverted or any other fix is applied. What GPU do you have and is it AGP? If it is AGP, does setting radeon.agpmode=-1 also fix it? Alex That is ATI Radeon X1950, and, unfortunately, radeon.agpmode=-1 doesn't help, it just makes 3D acceleration in games such as OpenArena stop working. Just to confirm, is the board AGP or PCIe? Alex It is AGP. That's an old machine. Can you check whether dma_addressing_limited() is actually returning the expected result at the point of radeon_ttm_init()? Disabling highmem is presumably just hiding whatever problem exists, by throwing away all >32-bit RAM such that use_dma32 doesn't matter. Robin.
Re: [Intel-gfx] [PATCH 7/9] drm/i915: stop using ttm_bo_wait
On 29/11/2022 18:05, Matthew Auld wrote:
On Fri, 25 Nov 2022 at 11:14, Tvrtko Ursulin
wrote:
+ Matt
On 25/11/2022 10:21, Christian König wrote:
TTM is just wrapping core DMA functionality here, remove the mid-layer.
No functional change.
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 5247d88b3c13..d409a77449a3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -599,13 +599,16 @@ i915_ttm_resource_get_st(struct drm_i915_gem_object *obj,
static int i915_ttm_truncate(struct drm_i915_gem_object *obj)
{
struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
- int err;
+ long err;
WARN_ON_ONCE(obj->mm.madv == I915_MADV_WILLNEED);
- err = ttm_bo_wait(bo, true, false);
- if (err)
+ err = dma_resv_wait_timeout(bo->base.resv, DMA_RESV_USAGE_BOOKKEEP,
+ true, 15 * HZ);
This 15 second stuck out a bit for me and then on a slightly deeper look
it seems this timeout will "leak" into a few of i915 code paths. If we
look at the difference between the legacy shmem and ttm backend I am not
sure if the legacy one is blocking or not - but if it can block I don't
think it would have an arbitrary timeout like this. Matt your thoughts?
Not sure what is meant by leak here, but the legacy shmem must also
wait/block when unbinding each VMA, before calling truncate. It's the
By "leak" I meant if 15s timeout propagates into some code paths visible
from userspace which with a legacy backend instead have an indefinite
wait. If we have that it's probably not very good to have this
inconsistency, or to apply an arbitrary timeout to those path to start with.
same story for the ttm backend, except slightly more complicated in
that there might be no currently bound VMA, and yet the GPU could
still be accessing the pages due to async unbinds, kernel moves etc,
which the wait here (and in i915_ttm_shrink) is meant to protect
against. If the wait times out it should just fail gracefully. I guess
we could just use MAX_SCHEDULE_TIMEOUT here? Not sure if it really
matters though.
Right, depends if it can leak or not to userspace and diverge between
backends.
Regards,
Tvrtko
[PULL] drm-misc-fixes
Hey Daniel and Dave, A single fix to vmwgfx mks-guest-stats ioctl. I lost my internet connection when pushing the tag, so I put together this mail manually. I hope you remember where drm-misc is hosted. :) Enjoy! Maarten Lankhorst drm-misc-fixes-2022-11-30: drm-misc-fixes for v6.1-rc8/final: - Single fix for mks-guest-stats ioctl userpages pinning. Dawei Li (1): drm/vmwgfx: Fix race issue calling pin_user_pages drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Re: [PATCH v2] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On 29/11/2022 19:04, Uwe Kleine-König wrote: > Compared to the txt description this adds clocks and clock-names to > match reality. (...) > + > +maintainers: > + - Sascha Hauer > + - Pengutronix Kernel Team > + > +properties: > + compatible: > +oneOf: > + - enum: > + - fsl,imx1-fb > + - fsl,imx21-fb > + - items: > + - enum: > + - fsl,imx25-fb > + - fsl,imx27-fb > + - const: fsl,imx21-fb > + > + clocks: > +maxItems: 3 > + > + clock-names: > +items: > + - const: ipg > + - const: ahb > + - const: per > + > + display: > +$ref: /schemas/types.yaml#/definitions/phandle You could mention here in "description" expected properties of display, just like original binding. Anyway, looks good: Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
[RFC 2/3] drm: Track clients by tgid and not tid
From: Tvrtko Ursulin Thread group id (aka pid from userspace point of view) is a more interesting thing to show as an owner of a DRM fd, so track and show that instead of the thread id. In the next patch we will make the owner updated post file descriptor handover, which will also be tgid based to avoid ping-pong when multiple threads access the fd. Signed-off-by: Tvrtko Ursulin Cc: Zack Rusin Cc: linux-graphics-maintai...@vmware.com Cc: Alex Deucher Cc: "Christian König" Reviewed-by: Zack Rusin --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +- drivers/gpu/drm/drm_debugfs.c | 4 ++-- drivers/gpu/drm/drm_file.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index a0780a4e3e61..30e24da1f398 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -968,7 +968,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused) * Therefore, we need to protect this ->comm access using RCU. */ rcu_read_lock(); - task = pid_task(file->pid, PIDTYPE_PID); + task = pid_task(file->pid, PIDTYPE_TGID); seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), task ? task->comm : ""); rcu_read_unlock(); diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index ee445f4605ba..42f657772025 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -80,7 +80,7 @@ static int drm_clients_info(struct seq_file *m, void *data) seq_printf(m, "%20s %5s %3s master a %5s %10s\n", "command", - "pid", + "tgid", "dev", "uid", "magic"); @@ -94,7 +94,7 @@ static int drm_clients_info(struct seq_file *m, void *data) bool is_current_master = drm_is_current_master(priv); rcu_read_lock(); /* locks pid_task()->comm */ - task = pid_task(priv->pid, PIDTYPE_PID); + task = pid_task(priv->pid, PIDTYPE_TGID); uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID; seq_printf(m, "%20s %5d %3d %c%c %5d %10u\n", task ? task->comm : "", diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index b0f24cea1e1e..20a9aef2b398 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -156,7 +156,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) if (!file) return ERR_PTR(-ENOMEM); - file->pid = get_pid(task_pid(current)); + file->pid = get_pid(task_tgid(current)); file->minor = minor; /* for compatibility root is always authenticated */ diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c index ce609e7d758f..f2985337aa53 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c @@ -260,7 +260,7 @@ static int vmw_debugfs_gem_info_show(struct seq_file *m, void *unused) * Therefore, we need to protect this ->comm access using RCU. */ rcu_read_lock(); - task = pid_task(file->pid, PIDTYPE_PID); + task = pid_task(file->pid, PIDTYPE_TGID); seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), task ? task->comm : ""); rcu_read_unlock(); -- 2.34.1
[RFC 0/3] File owner follows use
From: Tvrtko Ursulin Not so long ago when I sent out my DRM cgroup controller RFC I had some pieces in it which were tracking the real client using a specific drm_file. Christian then suggested that should probably be extracted and improved in the DRM core from the start, which was on his wishlist for a long period. So this mini-series is an attempt at that. First patch is just a logging cleanup, 2nd probably makes sense on it's own since it replaces tracking thread names with progresses which are more meaningful. Third one is where action is. The benefit on it's own is rather small, especially relative to the complication to track it, where it essentially changes the debugfs clients output from: command pid dev master a uid magic Xorg 1744 0 yy 0 0 Xorg 1744 0 ny 0 1 Xorg 1744 0 ny 0 2 Xorg 1744 0 ny 0 3 To something like: command tgid dev master a uid magic Xorg 830 0 yy 0 0 xfce4-session 880 0 ny 0 1 xfwm4 943 0 ny 0 2 neverball 1095 0 ny 0 3 One ugly part is one synchronise_rcu() on the first (hopefully) only fd handover. The latency of that could be improved by further wrapping and kfree_rcu() if desired. Another part I am unsure of is whether master nodes are ever handed over via sockets. I assumed no and exluded them from ownership updates. If they need to be then drm_master_check_perm() would break, I think. So looking for some feedback in this area please. Tvrtko Ursulin (3): drm: Replace DRM_DEBUG with drm_dbg_core in file and ioctl handling drm: Track clients by tgid and not tid drm: Update file owner during use drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++- drivers/gpu/drm/drm_auth.c | 3 +- drivers/gpu/drm/drm_debugfs.c | 12 +++--- drivers/gpu/drm/drm_file.c | 53 - drivers/gpu/drm/drm_ioc32.c | 13 +++--- drivers/gpu/drm/drm_ioctl.c | 28 +++-- drivers/gpu/drm/nouveau/nouveau_drm.c | 5 ++- drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 6 ++- include/drm/drm_file.h | 13 +- 9 files changed, 97 insertions(+), 42 deletions(-) -- 2.34.1
[RFC 1/3] drm: Replace DRM_DEBUG with drm_dbg_core in file and ioctl handling
From: Tvrtko Ursulin
Replace the deprecated macro with the per-device one.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/drm_file.c | 21 +++--
drivers/gpu/drm/drm_ioc32.c | 13 +++--
drivers/gpu/drm/drm_ioctl.c | 25 +
3 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 64b4a3a87fbb..b0f24cea1e1e 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -245,10 +245,10 @@ void drm_file_free(struct drm_file *file)
dev = file->minor->dev;
- DRM_DEBUG("comm=\"%s\", pid=%d, dev=0x%lx, open_count=%d\n",
- current->comm, task_pid_nr(current),
- (long)old_encode_dev(file->minor->kdev->devt),
- atomic_read(&dev->open_count));
+ drm_dbg_core(dev, "comm=\"%s\", pid=%d, dev=0x%lx, open_count=%d\n",
+current->comm, task_pid_nr(current),
+(long)old_encode_dev(file->minor->kdev->devt),
+atomic_read(&dev->open_count));
#ifdef CONFIG_DRM_LEGACY
if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
@@ -340,8 +340,8 @@ int drm_open_helper(struct file *filp, struct drm_minor
*minor)
dev->switch_power_state != DRM_SWITCH_POWER_DYNAMIC_OFF)
return -EINVAL;
- DRM_DEBUG("comm=\"%s\", pid=%d, minor=%d\n", current->comm,
- task_pid_nr(current), minor->index);
+ drm_dbg_core(dev, "comm=\"%s\", pid=%d, minor=%d\n",
+current->comm, task_pid_nr(current), minor->index);
priv = drm_file_alloc(minor);
if (IS_ERR(priv))
@@ -450,11 +450,12 @@ EXPORT_SYMBOL(drm_open);
void drm_lastclose(struct drm_device * dev)
{
- DRM_DEBUG("\n");
+ drm_dbg_core(dev, "\n");
- if (dev->driver->lastclose)
+ if (dev->driver->lastclose) {
dev->driver->lastclose(dev);
- DRM_DEBUG("driver lastclose completed\n");
+ drm_dbg_core(dev, "driver lastclose completed\n");
+ }
if (drm_core_check_feature(dev, DRIVER_LEGACY))
drm_legacy_dev_reinit(dev);
@@ -485,7 +486,7 @@ int drm_release(struct inode *inode, struct file *filp)
if (drm_dev_needs_global_mutex(dev))
mutex_lock(&drm_global_mutex);
- DRM_DEBUG("open_count = %d\n", atomic_read(&dev->open_count));
+ drm_dbg_core(dev, "open_count = %d\n", atomic_read(&dev->open_count));
drm_close_helper(filp);
diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
index 5d82891c3222..49a743f62b4a 100644
--- a/drivers/gpu/drm/drm_ioc32.c
+++ b/drivers/gpu/drm/drm_ioc32.c
@@ -972,6 +972,7 @@ long drm_compat_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
unsigned int nr = DRM_IOCTL_NR(cmd);
struct drm_file *file_priv = filp->private_data;
+ struct drm_device *dev = file_priv->minor->dev;
drm_ioctl_compat_t *fn;
int ret;
@@ -986,14 +987,14 @@ long drm_compat_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
if (!fn)
return drm_ioctl(filp, cmd, arg);
- DRM_DEBUG("comm=\"%s\", pid=%d, dev=0x%lx, auth=%d, %s\n",
- current->comm, task_pid_nr(current),
- (long)old_encode_dev(file_priv->minor->kdev->devt),
- file_priv->authenticated,
- drm_compat_ioctls[nr].name);
+ drm_dbg_core(dev, "comm=\"%s\", pid=%d, dev=0x%lx, auth=%d, %s\n",
+current->comm, task_pid_nr(current),
+(long)old_encode_dev(file_priv->minor->kdev->devt),
+file_priv->authenticated,
+drm_compat_ioctls[nr].name);
ret = (*fn)(filp, cmd, arg);
if (ret)
- DRM_DEBUG("ret = %d\n", ret);
+ drm_dbg_core(dev, "ret = %d\n", ret);
return ret;
}
EXPORT_SYMBOL(drm_compat_ioctl);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index ca2a6e6101dc..7c9d66ee917d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -440,7 +440,7 @@ static int drm_setversion(struct drm_device *dev, void
*data, struct drm_file *f
int drm_noop(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
- DRM_DEBUG("\n");
+ drm_dbg_core(dev, "\n");
return 0;
}
EXPORT_SYMBOL(drm_noop);
@@ -856,16 +856,16 @@ long drm_ioctl(struct file *filp,
out_size = 0;
ksize = max(max(in_size, out_size), drv_size);
- DRM_DEBUG("comm=\"%s\" pid=%d, dev=0x%lx, auth=%d, %s\n",
- current->comm, task_pid_nr(current),
- (long)old_encode_dev(file_priv->minor->kdev->devt),
- file_priv->authenticated, ioctl->name);
+ drm_dbg_core(dev, "comm=\"%s\" pid=%d, dev=0x%lx, auth=%d, %s\n",
+current->comm, t
[RFC 3/3] drm: Update file owner during use
From: Tvrtko Ursulin
With the typical model where the display server opends the file descriptor
and then hands it over to the client we were showing stale data in
debugfs.
Fix it by updating the drm_file->pid on ioctl access from a different
process.
The field is also made RCU protected to allow for lockless readers. Update
side is protected with dev->filelist_mutex.
Signed-off-by: Tvrtko Ursulin
Cc: "Christian König"
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++--
drivers/gpu/drm/drm_auth.c | 3 ++-
drivers/gpu/drm/drm_debugfs.c | 10
drivers/gpu/drm/drm_file.c | 32 -
drivers/gpu/drm/drm_ioctl.c | 3 +++
drivers/gpu/drm/nouveau/nouveau_drm.c | 5 +++-
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 6 +++--
include/drm/drm_file.h | 13 --
8 files changed, 65 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 30e24da1f398..385deb044058 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -959,6 +959,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m,
void *unused)
list_for_each_entry(file, &dev->filelist, lhead) {
struct task_struct *task;
struct drm_gem_object *gobj;
+ struct pid *pid;
int id;
/*
@@ -968,8 +969,9 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m,
void *unused)
* Therefore, we need to protect this ->comm access using RCU.
*/
rcu_read_lock();
- task = pid_task(file->pid, PIDTYPE_TGID);
- seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
+ pid = rcu_dereference(file->pid);
+ task = pid_task(pid, PIDTYPE_TGID);
+ seq_printf(m, "pid %8d command %s:\n", pid_nr(pid),
task ? task->comm : "");
rcu_read_unlock();
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index cf92a9ae8034..2ed2585ded37 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -235,7 +235,8 @@ static int drm_new_set_master(struct drm_device *dev,
struct drm_file *fpriv)
static int
drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv)
{
- if (file_priv->pid == task_pid(current) && file_priv->was_master)
+ if (file_priv->was_master &&
+ rcu_access_pointer(file_priv->pid) == task_pid(current))
return 0;
if (!capable(CAP_SYS_ADMIN))
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 42f657772025..9d4e3146a2b8 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -90,15 +90,17 @@ static int drm_clients_info(struct seq_file *m, void *data)
*/
mutex_lock(&dev->filelist_mutex);
list_for_each_entry_reverse(priv, &dev->filelist, lhead) {
- struct task_struct *task;
bool is_current_master = drm_is_current_master(priv);
+ struct task_struct *task;
+ struct pid *pid;
- rcu_read_lock(); /* locks pid_task()->comm */
- task = pid_task(priv->pid, PIDTYPE_TGID);
+ rcu_read_lock(); /* Locks priv->pid and pid_task()->comm! */
+ pid = rcu_dereference(priv->pid);
+ task = pid_task(pid, PIDTYPE_TGID);
uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
seq_printf(m, "%20s %5d %3d %c%c %5d %10u\n",
task ? task->comm : "",
- pid_vnr(priv->pid),
+ pid_vnr(pid),
priv->minor->index,
is_current_master ? 'y' : 'n',
priv->authenticated ? 'y' : 'n',
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 20a9aef2b398..3433f9610dba 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -156,7 +156,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
if (!file)
return ERR_PTR(-ENOMEM);
- file->pid = get_pid(task_tgid(current));
+ rcu_assign_pointer(file->pid, get_pid(task_tgid(current)));
file->minor = minor;
/* for compatibility root is always authenticated */
@@ -502,6 +502,36 @@ int drm_release(struct inode *inode, struct file *filp)
}
EXPORT_SYMBOL(drm_release);
+void drm_file_update_pid(struct drm_file *filp)
+{
+ struct drm_device *dev;
+ struct pid *pid, *old;
+
+ /* Master nodes are not expected to be passed between processes. */
+ if (filp->was_master)
+ return;
+
+ pid = task_tgid(current);
+
+ /*
+* Quick unlocked check since the model is
Re: [PULL] drm-misc-fixes
Hi Maarten On Wed, Nov 30, 2022 at 02:16:05PM +0100, Maarten Lankhorst wrote: > A single fix to vmwgfx mks-guest-stats ioctl. > I lost my internet connection when pushing the tag, so I put together this > mail > manually. I hope you remember where drm-misc is hosted. :) For reference, you can generate the mail content after the fact by using something like: git request-pull drm/fixes drm-misc drm-misc-fixes-2022-11-30 Maxime signature.asc Description: PGP signature
[PATCH v5 4/4] xhci: Convert to use list_count_nodes()
The list API provides the list_count_nodes() to help with counting
existing nodes in the list. Utilise it.
Acked-by: Mathias Nyman
Signed-off-by: Andy Shevchenko
---
v5: used renamed API (LKP)
v4: added tag (Mathias)
v3: no change
v2: no change
drivers/usb/host/xhci-ring.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ddc30037f9ce..aa4d34efecd2 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2528,7 +2528,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
union xhci_trb *ep_trb;
int status = -EINPROGRESS;
struct xhci_ep_ctx *ep_ctx;
- struct list_head *tmp;
u32 trb_comp_code;
int td_num = 0;
bool handling_skipped_tds = false;
@@ -2582,10 +2581,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
}
/* Count current td numbers if ep->skip is set */
- if (ep->skip) {
- list_for_each(tmp, &ep_ring->td_list)
- td_num++;
- }
+ if (ep->skip)
+ td_num += list_count_nodes(&ep_ring->td_list);
/* Look for common error cases */
switch (trb_comp_code) {
--
2.35.1
[PATCH v5 1/4] i915: Move list_count() to list.h as list_count_nodes() for broader use
Some of the existing users, and definitely will be new ones, want to
count existing nodes in the list. Provide a generic API for that by
moving code from i915 to list.h.
Reviewed-by: Lucas De Marchi
Acked-by: Jani Nikula
Signed-off-by: Andy Shevchenko
---
v5: added tag (Lucas), renamed API to list_count_nodes() (LKP)
v4: fixed prototype when converting to static inline
v3: added tag (Jani), changed to be static inline (Mike)
v2: dropped the duplicate code in i915 (LKP)
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 15 ++-
include/linux/list.h | 15 +++
2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 1f7188129cd1..370164363b0d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -2004,17 +2004,6 @@ static void print_request_ring(struct drm_printer *m,
struct i915_request *rq)
}
}
-static unsigned long list_count(struct list_head *list)
-{
- struct list_head *pos;
- unsigned long count = 0;
-
- list_for_each(pos, list)
- count++;
-
- return count;
-}
-
static unsigned long read_ul(void *p, size_t x)
{
return *(unsigned long *)(p + x);
@@ -2189,8 +2178,8 @@ void intel_engine_dump(struct intel_engine_cs *engine,
spin_lock_irqsave(&engine->sched_engine->lock, flags);
engine_dump_active_requests(engine, m);
- drm_printf(m, "\tOn hold?: %lu\n",
- list_count(&engine->sched_engine->hold));
+ drm_printf(m, "\tOn hold?: %zu\n",
+ list_count_nodes(&engine->sched_engine->hold));
spin_unlock_irqrestore(&engine->sched_engine->lock, flags);
drm_printf(m, "\tMMIO base: 0x%08x\n", engine->mmio_base);
diff --git a/include/linux/list.h b/include/linux/list.h
index 61762054b4be..f10344dbad4d 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -655,6 +655,21 @@ static inline void list_splice_tail_init(struct list_head
*list,
!list_is_head(pos, (head)); \
pos = n, n = pos->prev)
+/**
+ * list_count_nodes - count nodes in the list
+ * @head: the head for your list.
+ */
+static inline size_t list_count_nodes(struct list_head *head)
+{
+ struct list_head *pos;
+ size_t count = 0;
+
+ list_for_each(pos, head)
+ count++;
+
+ return count;
+}
+
/**
* list_entry_is_head - test if the entry points to the head of the list
* @pos: the type * to cursor
--
2.35.1
[PATCH v5 2/4] usb: gadget: hid: Convert to use list_count_nodes()
The list API provides the list_count_nodes() to help with counting
existing nodes in the list. Utilise it.
Signed-off-by: Andy Shevchenko
---
v5: used renamed API (LKP)
v4: no change
v3: fixed typo in the commit message (Fabio)
v2: no change
drivers/usb/gadget/legacy/hid.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/legacy/hid.c b/drivers/usb/gadget/legacy/hid.c
index 1187ee4f316a..133daf88162e 100644
--- a/drivers/usb/gadget/legacy/hid.c
+++ b/drivers/usb/gadget/legacy/hid.c
@@ -133,14 +133,11 @@ static struct usb_configuration config_driver = {
static int hid_bind(struct usb_composite_dev *cdev)
{
struct usb_gadget *gadget = cdev->gadget;
- struct list_head *tmp;
struct hidg_func_node *n = NULL, *m, *iter_n;
struct f_hid_opts *hid_opts;
- int status, funcs = 0;
-
- list_for_each(tmp, &hidg_func_list)
- funcs++;
+ int status, funcs;
+ funcs = list_count_nodes(&hidg_func_list);
if (!funcs)
return -ENODEV;
--
2.35.1
[PATCH v5 3/4] usb: gadget: udc: bcm63xx: Convert to use list_count_nodes()
The list API provides the list_count_nodes() to help with counting
existing nodes in the list. Utilise it.
Signed-off-by: Andy Shevchenko
---
v5: used renamed API (LKP)
v4: no change
v3: no change
v2: no change
drivers/usb/gadget/udc/bcm63xx_udc.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c
b/drivers/usb/gadget/udc/bcm63xx_udc.c
index 2cdb07905bde..771ba1ffce95 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -2172,7 +2172,6 @@ static int bcm63xx_iudma_dbg_show(struct seq_file *s,
void *p)
for (ch_idx = 0; ch_idx < BCM63XX_NUM_IUDMA; ch_idx++) {
struct iudma_ch *iudma = &udc->iudma[ch_idx];
- struct list_head *pos;
seq_printf(s, "IUDMA channel %d -- ", ch_idx);
switch (iudma_defaults[ch_idx].ep_type) {
@@ -2205,14 +2204,10 @@ static int bcm63xx_iudma_dbg_show(struct seq_file *s,
void *p)
seq_printf(s, " desc: %d/%d used", iudma->n_bds_used,
iudma->n_bds);
- if (iudma->bep) {
- i = 0;
- list_for_each(pos, &iudma->bep->queue)
- i++;
- seq_printf(s, "; %d queued\n", i);
- } else {
+ if (iudma->bep)
+ seq_printf(s, "; %zu queued\n",
list_count_nodes(&iudma->bep->queue));
+ else
seq_printf(s, "\n");
- }
for (i = 0; i < iudma->n_bds; i++) {
struct bcm_enet_desc *d = &iudma->bd_ring[i];
--
2.35.1
[PATCH] dt-bindings: msm/dsi: Don't require vcca-supply on 14nm PHY
On some SoCs (hello SM6115) vcca-supply is not wired to any smd-rpm or rpmh regulator, but instead powered by the VDD_MX line, which is voted for in the DSI ctrl node. Signed-off-by: Konrad Dybcio --- Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml index 819de5ce0bc9..a43e11d3b00d 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml @@ -39,7 +39,6 @@ required: - compatible - reg - reg-names - - vcca-supply unevaluatedProperties: false -- 2.38.1
[PATCH 0/5] drm/rockchip: vop2: add support for the rgb output block
Hi all,
This series adds support for the RGB output block that can be found in the
Rockchip Video Output Processor (VOP) 2.
Patches 1-2 prepare the RGB part, which has been used only with the VOP(1)
so far.
Patch 3 is a cleanup patch and aims to make the creation and destruction of
the CRTCs and planes more readable.
Patch 4 activates the support for the RGB output block in the VOP2 driver.
Patch 5 adds pinctrls for the 16-bit and 18-bit RGB data lines.
Tested on a custom board featuring the RK3568 SoC with a 18-bit RGB
display.
Looking forward to your comments!
Best regards,
Michael
Michael Riesch (5):
drm/rockchip: rgb: embed drm_encoder into rockchip_encoder
drm/rockchip: rgb: add video_port parameter to init function
drm/rockchip: vop2: use symmetric function pair
vop2_{create,destroy}_crtcs
drm/rockchip: vop2: add support for the rgb output block
arm64: dts: rockchip: add pinctrls for 16-bit/18-bit rgb interface to
rk356x
.../boot/dts/rockchip/rk3568-pinctrl.dtsi | 94 +++
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 50 +++---
drivers/gpu/drm/rockchip/rockchip_rgb.c | 21 +++--
drivers/gpu/drm/rockchip/rockchip_rgb.h | 6 +-
5 files changed, 147 insertions(+), 26 deletions(-)
base-commit: b7b275e60bcd5f89771e865a8239325f86d9927d
--
2.30.2
[PATCH 2/5] drm/rockchip: rgb: add video_port parameter to init function
The VOP2 driver has more than one video port, hence the hard-coded
port id will not work anymore. Add an extra parameter for the video
port id to the rockchip_rgb_init function.
Signed-off-by: Michael Riesch
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_rgb.c | 9 +
drivers/gpu/drm/rockchip/rockchip_rgb.h | 6 --
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index c356de5dd220..f7335f9cac73 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -2221,7 +2221,7 @@ static int vop_bind(struct device *dev, struct device
*master, void *data)
goto err_disable_pm_runtime;
if (vop->data->feature & VOP_FEATURE_INTERNAL_RGB) {
- vop->rgb = rockchip_rgb_init(dev, &vop->crtc, vop->drm_dev);
+ vop->rgb = rockchip_rgb_init(dev, &vop->crtc, vop->drm_dev, 0);
if (IS_ERR(vop->rgb)) {
ret = PTR_ERR(vop->rgb);
goto err_disable_pm_runtime;
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c
b/drivers/gpu/drm/rockchip/rockchip_rgb.c
index 16201a5cf1e8..ed6ccd1db465 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -74,7 +74,8 @@ struct drm_encoder_helper_funcs
rockchip_rgb_encoder_helper_funcs = {
struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
struct drm_crtc *crtc,
- struct drm_device *drm_dev)
+ struct drm_device *drm_dev,
+ int video_port)
{
struct rockchip_rgb *rgb;
struct drm_encoder *encoder;
@@ -92,7 +93,7 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
rgb->dev = dev;
rgb->drm_dev = drm_dev;
- port = of_graph_get_port_by_id(dev->of_node, 0);
+ port = of_graph_get_port_by_id(dev->of_node, video_port);
if (!port)
return ERR_PTR(-EINVAL);
@@ -105,8 +106,8 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
continue;
child_count++;
- ret = drm_of_find_panel_or_bridge(dev->of_node, 0, endpoint_id,
- &panel, &bridge);
+ ret = drm_of_find_panel_or_bridge(dev->of_node, video_port,
+ endpoint_id, &panel, &bridge);
if (!ret) {
of_node_put(endpoint);
break;
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.h
b/drivers/gpu/drm/rockchip/rockchip_rgb.h
index 27b9635124bc..1bd4e20e91eb 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.h
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.h
@@ -8,12 +8,14 @@
#ifdef CONFIG_ROCKCHIP_RGB
struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
struct drm_crtc *crtc,
- struct drm_device *drm_dev);
+ struct drm_device *drm_dev,
+ int video_port);
void rockchip_rgb_fini(struct rockchip_rgb *rgb);
#else
static inline struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
struct drm_crtc *crtc,
-struct drm_device *drm_dev)
+struct drm_device *drm_dev,
+int video_port)
{
return NULL;
}
--
2.30.2
[PATCH 3/5] drm/rockchip: vop2: use symmetric function pair vop2_{create, destroy}_crtcs
Let the function name vop2_create_crtcs reflect that the function creates
multiple CRTCS. Also, use a symmetric function pair to create and destroy
the CRTCs and the corresponding planes.
Signed-off-by: Michael Riesch
---
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 31 ++--
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 105a548d0abe..94fddbf70ff6 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2246,7 +2246,7 @@ static struct vop2_video_port
*find_vp_without_primary(struct vop2 *vop2)
#define NR_LAYERS 6
-static int vop2_create_crtc(struct vop2 *vop2)
+static int vop2_create_crtcs(struct vop2 *vop2)
{
const struct vop2_data *vop2_data = vop2->data;
struct drm_device *drm = vop2->drm;
@@ -2371,15 +2371,25 @@ static int vop2_create_crtc(struct vop2 *vop2)
return 0;
}
-static void vop2_destroy_crtc(struct drm_crtc *crtc)
+static void vop2_destroy_crtcs(struct vop2 *vop2)
{
- of_node_put(crtc->port);
+ struct drm_device *drm = vop2->drm;
+ struct list_head *crtc_list = &drm->mode_config.crtc_list;
+ struct list_head *plane_list = &drm->mode_config.plane_list;
+ struct drm_crtc *crtc, *tmpc;
+ struct drm_plane *plane, *tmpp;
+
+ list_for_each_entry_safe(plane, tmpp, plane_list, head)
+ drm_plane_cleanup(plane);
/*
* Destroy CRTC after vop2_plane_destroy() since vop2_disable_plane()
* references the CRTC.
*/
- drm_crtc_cleanup(crtc);
+ list_for_each_entry_safe(crtc, tmpc, crtc_list, head) {
+ of_node_put(crtc->port);
+ drm_crtc_cleanup(crtc);
+ }
}
static struct reg_field vop2_cluster_regs[VOP2_WIN_MAX_REG] = {
@@ -2683,7 +2693,7 @@ static int vop2_bind(struct device *dev, struct device
*master, void *data)
if (ret)
return ret;
- ret = vop2_create_crtc(vop2);
+ ret = vop2_create_crtcs(vop2);
if (ret)
return ret;
@@ -2697,19 +2707,10 @@ static int vop2_bind(struct device *dev, struct device
*master, void *data)
static void vop2_unbind(struct device *dev, struct device *master, void *data)
{
struct vop2 *vop2 = dev_get_drvdata(dev);
- struct drm_device *drm = vop2->drm;
- struct list_head *plane_list = &drm->mode_config.plane_list;
- struct list_head *crtc_list = &drm->mode_config.crtc_list;
- struct drm_crtc *crtc, *tmpc;
- struct drm_plane *plane, *tmpp;
pm_runtime_disable(dev);
- list_for_each_entry_safe(plane, tmpp, plane_list, head)
- drm_plane_cleanup(plane);
-
- list_for_each_entry_safe(crtc, tmpc, crtc_list, head)
- vop2_destroy_crtc(crtc);
+ vop2_destroy_crtcs(vop2);
}
const struct component_ops vop2_component_ops = {
--
2.30.2
Re: [PATCH 2/2] drm/shmem-helper: Avoid vm_open error paths
On Tue, Nov 29, 2022 at 12:02:42PM -0800, Rob Clark wrote:
> From: Rob Clark
>
> vm_open() is not allowed to fail. Fortunately we are guaranteed that
> the pages are already pinned, and only need to increment the refcnt. So
> just increment it directly.
Please mention hare that the only issue is the mutex_lock_interruptible,
and the only way we've found to hit this is if you send a signal to the
original process in a fork() at _just_ the right time.
With that: Reviewed-by: Daniel Vetter
>
> Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 110a9eac2af8..9885ba64127f 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -571,12 +571,20 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct
> *vma)
> {
> struct drm_gem_object *obj = vma->vm_private_data;
> struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> - int ret;
>
> WARN_ON(shmem->base.import_attach);
>
> - ret = drm_gem_shmem_get_pages(shmem);
> - WARN_ON_ONCE(ret != 0);
> + mutex_lock(&shmem->pages_lock);
> +
> + /*
> + * We should have already pinned the pages, vm_open() just grabs
> + * an additional reference for the new mm the vma is getting
> + * copied into.
> + */
> + WARN_ON_ONCE(!shmem->pages_use_count);
> +
> + shmem->pages_use_count++;
> + mutex_unlock(&shmem->pages_lock);
>
> drm_gem_vm_open(vma);
> }
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 4/5] drm/rockchip: vop2: add support for the rgb output block
The Rockchip VOP2 features an internal RGB output block, which can be
attached to the video port 2 of the VOP2. Add support for this output
block.
Signed-off-by: Michael Riesch
---
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 21
1 file changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 94fddbf70ff6..16041c79d228 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -39,6 +39,7 @@
#include "rockchip_drm_gem.h"
#include "rockchip_drm_fb.h"
#include "rockchip_drm_vop2.h"
+#include "rockchip_rgb.h"
/*
* VOP2 architecture
@@ -212,6 +213,9 @@ struct vop2 {
struct clk *hclk;
struct clk *aclk;
+ /* optional internal rgb encoder */
+ struct rockchip_rgb *rgb;
+
/* must be put at the end of the struct */
struct vop2_win win[];
};
@@ -2697,11 +2701,25 @@ static int vop2_bind(struct device *dev, struct device
*master, void *data)
if (ret)
return ret;
+ vop2->rgb = rockchip_rgb_init(dev, &vop2->vps[2].crtc, vop2->drm, 2);
+ if (IS_ERR(vop2->rgb)) {
+ if (PTR_ERR(vop2->rgb) == -EPROBE_DEFER) {
+ ret = PTR_ERR(vop2->rgb);
+ goto err_crtcs;
+ }
+ vop2->rgb = NULL;
+ }
+
rockchip_drm_dma_init_device(vop2->drm, vop2->dev);
pm_runtime_enable(&pdev->dev);
return 0;
+
+err_crtcs:
+ vop2_destroy_crtcs(vop2);
+
+ return ret;
}
static void vop2_unbind(struct device *dev, struct device *master, void *data)
@@ -2710,6 +2728,9 @@ static void vop2_unbind(struct device *dev, struct device
*master, void *data)
pm_runtime_disable(dev);
+ if (vop2->rgb)
+ rockchip_rgb_fini(vop2->rgb);
+
vop2_destroy_crtcs(vop2);
}
--
2.30.2
[PATCH 5/5] arm64: dts: rockchip: add pinctrls for 16-bit/18-bit rgb interface to rk356x
The rk3568-pinctrl.dtsi only defines the 24-bit RGB interface. Add separate
nodes for the 16-bit and 18-bit version, respectively. While at it, split
off the clock/sync signals from the data signals.
The exact mapping of the data pins was discussed here:
https://lore.kernel.org/linux-rockchip/f33a0488-528c-99de-3279-3c0346a03...@wolfvision.net/T/
Signed-off-by: Michael Riesch
---
.../boot/dts/rockchip/rk3568-pinctrl.dtsi | 94 +++
1 file changed, 94 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi
b/arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi
index 8f90c66dd9e9..0a979bfb63d9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi
@@ -3117,4 +3117,98 @@ tsadc_pin: tsadc-pin {
<0 RK_PA1 0 &pcfg_pull_none>;
};
};
+
+ lcdc {
+ /omit-if-no-ref/
+ lcdc_clock: lcdc-clock {
+ rockchip,pins =
+ /* lcdc_clk */
+ <3 RK_PA0 1 &pcfg_pull_none>,
+ /* lcdc_den */
+ <3 RK_PC3 1 &pcfg_pull_none>,
+ /* lcdc_hsync */
+ <3 RK_PC1 1 &pcfg_pull_none>,
+ /* lcdc_vsync */
+ <3 RK_PC2 1 &pcfg_pull_none>;
+ };
+
+ /omit-if-no-ref/
+ lcdc_data16: lcdc-data16 {
+ rockchip,pins =
+ /* lcdc_d3 */
+ <2 RK_PD3 1 &pcfg_pull_none>,
+ /* lcdc_d4 */
+ <2 RK_PD4 1 &pcfg_pull_none>,
+ /* lcdc_d5 */
+ <2 RK_PD5 1 &pcfg_pull_none>,
+ /* lcdc_d6 */
+ <2 RK_PD6 1 &pcfg_pull_none>,
+ /* lcdc_d7 */
+ <2 RK_PD7 1 &pcfg_pull_none>,
+ /* lcdc_d10 */
+ <3 RK_PA3 1 &pcfg_pull_none>,
+ /* lcdc_d11 */
+ <3 RK_PA4 1 &pcfg_pull_none>,
+ /* lcdc_d12 */
+ <3 RK_PA5 1 &pcfg_pull_none>,
+ /* lcdc_d13 */
+ <3 RK_PA6 1 &pcfg_pull_none>,
+ /* lcdc_d14 */
+ <3 RK_PA7 1 &pcfg_pull_none>,
+ /* lcdc_d15 */
+ <3 RK_PB0 1 &pcfg_pull_none>,
+ /* lcdc_d19 */
+ <3 RK_PB4 1 &pcfg_pull_none>,
+ /* lcdc_d20 */
+ <3 RK_PB5 1 &pcfg_pull_none>,
+ /* lcdc_d21 */
+ <3 RK_PB6 1 &pcfg_pull_none>,
+ /* lcdc_d22 */
+ <3 RK_PB7 1 &pcfg_pull_none>,
+ /* lcdc_d23 */
+ <3 RK_PC0 1 &pcfg_pull_none>;
+ };
+
+ /omit-if-no-ref/
+ lcdc_data18: lcdc-data18 {
+ rockchip,pins =
+ /* lcdc_d2 */
+ <2 RK_PD2 1 &pcfg_pull_none>,
+ /* lcdc_d3 */
+ <2 RK_PD3 1 &pcfg_pull_none>,
+ /* lcdc_d4 */
+ <2 RK_PD4 1 &pcfg_pull_none>,
+ /* lcdc_d5 */
+ <2 RK_PD5 1 &pcfg_pull_none>,
+ /* lcdc_d6 */
+ <2 RK_PD6 1 &pcfg_pull_none>,
+ /* lcdc_d7 */
+ <2 RK_PD7 1 &pcfg_pull_none>,
+ /* lcdc_d10 */
+ <3 RK_PA3 1 &pcfg_pull_none>,
+ /* lcdc_d11 */
+ <3 RK_PA4 1 &pcfg_pull_none>,
+ /* lcdc_d12 */
+ <3 RK_PA5 1 &pcfg_pull_none>,
+ /* lcdc_d13 */
+ <3 RK_PA6 1 &pcfg_pull_none>,
+ /* lcdc_d14 */
+ <3 RK_PA7 1 &pcfg_pull_none>,
+ /* lcdc_d15 */
+ <3 RK_PB0 1 &pcfg_pull_none>,
+ /* lcdc_d18 */
+ <3 RK_PB3 1 &pcfg_pull_none>,
+ /* lcdc_d19 */
+ <3 RK_PB4 1 &pcfg_pull_none>,
[PATCH 1/5] drm/rockchip: rgb: embed drm_encoder into rockchip_encoder
Commit 540b8f271e53 ("drm/rockchip: Embed drm_encoder into
rockchip_decoder") provides the means to pass the endpoint ID to the
VOP2 driver, which sets the interface MUX accordingly. However, this
step has not yet been carried out for the RGB output block. Embed the
drm_encoder structure into the rockchip_encoder structure and set the
endpoint ID correctly.
Signed-off-by: Michael Riesch
---
drivers/gpu/drm/rockchip/rockchip_rgb.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c
b/drivers/gpu/drm/rockchip/rockchip_rgb.c
index 75eb7cca3d82..16201a5cf1e8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -18,17 +18,17 @@
#include
#include
+#include
+
#include "rockchip_drm_drv.h"
#include "rockchip_drm_vop.h"
#include "rockchip_rgb.h"
-#define encoder_to_rgb(c) container_of(c, struct rockchip_rgb, encoder)
-
struct rockchip_rgb {
struct device *dev;
struct drm_device *drm_dev;
struct drm_bridge *bridge;
- struct drm_encoder encoder;
+ struct rockchip_encoder encoder;
struct drm_connector connector;
int output_mode;
};
@@ -125,7 +125,7 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
return ERR_PTR(ret);
}
- encoder = &rgb->encoder;
+ encoder = &rgb->encoder.encoder;
encoder->possible_crtcs = drm_crtc_mask(crtc);
ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_NONE);
@@ -161,6 +161,8 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
goto err_free_encoder;
}
+ rgb->encoder.crtc_endpoint_id = ROCKCHIP_VOP2_EP_RGB0;
+
ret = drm_connector_attach_encoder(connector, encoder);
if (ret < 0) {
DRM_DEV_ERROR(drm_dev->dev,
@@ -182,6 +184,6 @@ void rockchip_rgb_fini(struct rockchip_rgb *rgb)
{
drm_panel_bridge_remove(rgb->bridge);
drm_connector_cleanup(&rgb->connector);
- drm_encoder_cleanup(&rgb->encoder);
+ drm_encoder_cleanup(&rgb->encoder.encoder);
}
EXPORT_SYMBOL_GPL(rockchip_rgb_fini);
--
2.30.2
Re: [Intel-gfx] [PATCH 7/9] drm/i915: stop using ttm_bo_wait
On Wed, 30 Nov 2022 at 14:03, Tvrtko Ursulin
wrote:
> On 29/11/2022 18:05, Matthew Auld wrote:
> > On Fri, 25 Nov 2022 at 11:14, Tvrtko Ursulin
> > wrote:
> >>
> >>
> >> + Matt
> >>
> >> On 25/11/2022 10:21, Christian König wrote:
> >>> TTM is just wrapping core DMA functionality here, remove the mid-layer.
> >>> No functional change.
> >>>
> >>> Signed-off-by: Christian König
> >>> ---
> >>>drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 ++---
> >>>1 file changed, 6 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> >>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> >>> index 5247d88b3c13..d409a77449a3 100644
> >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> >>> @@ -599,13 +599,16 @@ i915_ttm_resource_get_st(struct drm_i915_gem_object
> >>> *obj,
> >>>static int i915_ttm_truncate(struct drm_i915_gem_object *obj)
> >>>{
> >>>struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
> >>> - int err;
> >>> + long err;
> >>>
> >>>WARN_ON_ONCE(obj->mm.madv == I915_MADV_WILLNEED);
> >>>
> >>> - err = ttm_bo_wait(bo, true, false);
> >>> - if (err)
> >>> + err = dma_resv_wait_timeout(bo->base.resv, DMA_RESV_USAGE_BOOKKEEP,
> >>> + true, 15 * HZ);
> >>
> >> This 15 second stuck out a bit for me and then on a slightly deeper look
> >> it seems this timeout will "leak" into a few of i915 code paths. If we
> >> look at the difference between the legacy shmem and ttm backend I am not
> >> sure if the legacy one is blocking or not - but if it can block I don't
> >> think it would have an arbitrary timeout like this. Matt your thoughts?
> >
> > Not sure what is meant by leak here, but the legacy shmem must also
> > wait/block when unbinding each VMA, before calling truncate. It's the
>
> By "leak" I meant if 15s timeout propagates into some code paths visible
> from userspace which with a legacy backend instead have an indefinite
> wait. If we have that it's probably not very good to have this
> inconsistency, or to apply an arbitrary timeout to those path to start with.
>
> > same story for the ttm backend, except slightly more complicated in
> > that there might be no currently bound VMA, and yet the GPU could
> > still be accessing the pages due to async unbinds, kernel moves etc,
> > which the wait here (and in i915_ttm_shrink) is meant to protect
> > against. If the wait times out it should just fail gracefully. I guess
> > we could just use MAX_SCHEDULE_TIMEOUT here? Not sure if it really
> > matters though.
>
> Right, depends if it can leak or not to userspace and diverge between
> backends.
Generally lock_timeout() is a design bug. It's either
lock_interruptible (or maybe lock_killable) or try_lock, but
lock_timeout is just duct-tape. I haven't dug in to figure out what
should be here, but it smells fishy.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PULL] drm-misc-fixes
On Wed, 30 Nov 2022 at 14:43, Maxime Ripard wrote: > > Hi Maarten > > On Wed, Nov 30, 2022 at 02:16:05PM +0100, Maarten Lankhorst wrote: > > A single fix to vmwgfx mks-guest-stats ioctl. > > I lost my internet connection when pushing the tag, so I put together this > > mail > > manually. I hope you remember where drm-misc is hosted. :) > > For reference, you can generate the mail content after the fact by using > something like: > > git request-pull drm/fixes drm-misc drm-misc-fixes-2022-11-30 Maarten, can you pls do that? Otherwise we can't feed the thing into dim for processing, and have to do that also manually :-) -Daniel > > Maxime -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [RFC 3/3] drm: Update file owner during use
On Wed, Nov 30, 2022 at 01:34:07PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> With the typical model where the display server opends the file descriptor
> and then hands it over to the client we were showing stale data in
> debugfs.
>
> Fix it by updating the drm_file->pid on ioctl access from a different
> process.
>
> The field is also made RCU protected to allow for lockless readers. Update
> side is protected with dev->filelist_mutex.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: "Christian König"
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++--
> drivers/gpu/drm/drm_auth.c | 3 ++-
> drivers/gpu/drm/drm_debugfs.c | 10
> drivers/gpu/drm/drm_file.c | 32 -
> drivers/gpu/drm/drm_ioctl.c | 3 +++
> drivers/gpu/drm/nouveau/nouveau_drm.c | 5 +++-
> drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 6 +++--
> include/drm/drm_file.h | 13 --
> 8 files changed, 65 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 30e24da1f398..385deb044058 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -959,6 +959,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file
> *m, void *unused)
> list_for_each_entry(file, &dev->filelist, lhead) {
> struct task_struct *task;
> struct drm_gem_object *gobj;
> + struct pid *pid;
> int id;
>
> /*
> @@ -968,8 +969,9 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file
> *m, void *unused)
>* Therefore, we need to protect this ->comm access using RCU.
>*/
> rcu_read_lock();
> - task = pid_task(file->pid, PIDTYPE_TGID);
> - seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
> + pid = rcu_dereference(file->pid);
> + task = pid_task(pid, PIDTYPE_TGID);
> + seq_printf(m, "pid %8d command %s:\n", pid_nr(pid),
> task ? task->comm : "");
> rcu_read_unlock();
>
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index cf92a9ae8034..2ed2585ded37 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -235,7 +235,8 @@ static int drm_new_set_master(struct drm_device *dev,
> struct drm_file *fpriv)
> static int
> drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv)
> {
> - if (file_priv->pid == task_pid(current) && file_priv->was_master)
> + if (file_priv->was_master &&
> + rcu_access_pointer(file_priv->pid) == task_pid(current))
This scares me, and also makes me wonder whether we really want to
conflate the original owner with the rendering owner. And also, whether we
really want to keep updating that, because for some of the "bind an fd to
a pid" use-cases like svm we really do not want to ever again allow a
change.
So sligthly different idea:
- we have a separate render pid/drm_file owner frome the open() owner that
we track in drm_auth.c
- that one is set the first time a driver specific ioctl is called (which
for the "pass me the fd" dri3 mode should never be the compositor)
- we start out with nothing and only set it once, which further simplifies
the model (still need the mutex for concurrent first ioctl ofc)
Eventually we could then use that to enforce static binding to a pid,
which is what we want for svm style models, i.e. if the pid changes, the
fd does an -EACCESS or similar.
Thoughts?
-Daniel
> return 0;
>
> if (!capable(CAP_SYS_ADMIN))
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 42f657772025..9d4e3146a2b8 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -90,15 +90,17 @@ static int drm_clients_info(struct seq_file *m, void
> *data)
>*/
> mutex_lock(&dev->filelist_mutex);
> list_for_each_entry_reverse(priv, &dev->filelist, lhead) {
> - struct task_struct *task;
> bool is_current_master = drm_is_current_master(priv);
> + struct task_struct *task;
> + struct pid *pid;
>
> - rcu_read_lock(); /* locks pid_task()->comm */
> - task = pid_task(priv->pid, PIDTYPE_TGID);
> + rcu_read_lock(); /* Locks priv->pid and pid_task()->comm! */
> + pid = rcu_dereference(priv->pid);
> + task = pid_task(pid, PIDTYPE_TGID);
> uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
> seq_printf(m, "%20s %5d %3d %c%c %5d %10u\n",
> task ? task->comm : "",
> -pid_vnr(priv->pid),
> +pid_vnr(pid),
> priv->minor->index,
>
Re: Screen corruption using radeon kernel driver
On Wed, Nov 30, 2022 at 7:54 AM Robin Murphy wrote: > > On 2022-11-29 17:11, Mikhail Krylov wrote: > > On Tue, Nov 29, 2022 at 11:05:28AM -0500, Alex Deucher wrote: > >> On Tue, Nov 29, 2022 at 10:59 AM Mikhail Krylov wrote: > >>> > >>> On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote: > On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov wrote: > > > > On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher wrote: > > > [excessive quoting removed] > > > >>> So, is there any progress on this issue? I do understand it's not a > >>> high > >>> priority one, and today I've checked it on 6.0 kernel, and > >>> unfortunately, it still persists... > >>> > >>> I'm considering writing a patch that will allow user to override > >>> need_dma32/dma_bits setting with a module parameter. I'll have some > >>> time > >>> after the New Year for that. > >>> > >>> Is it at all possible that such a patch will be merged into kernel? > >>> > >> On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov > >> wrote: > >> Unless someone familiar with HIMEM can figure out what is going wrong > >> we should just revert the patch. > >> > >> Alex > > > > > > Okay, I was suggesting that mostly because > > > > a) it works for me with dma_bits = 40 (I understand that's what it is > > without the original patch applied); > > > > b) there's a hint of uncertainity on this line > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359 > > saying that for AGP dma_bits = 32 is the safest option, so apparently > > there are > > setups, unlike mine, where dma_bits = 32 is better than 40. > > > > But I'm in no position to argue, just wanted to make myself clear. > > I'm okay with rebuilding the kernel for my machine until the original > > patch is reverted or any other fix is applied. > > What GPU do you have and is it AGP? If it is AGP, does setting > radeon.agpmode=-1 also fix it? > > Alex > >>> > >>> That is ATI Radeon X1950, and, unfortunately, radeon.agpmode=-1 doesn't > >>> help, it just makes 3D acceleration in games such as OpenArena stop > >>> working. > >> > >> Just to confirm, is the board AGP or PCIe? > >> > >> Alex > > > > It is AGP. That's an old machine. > > Can you check whether dma_addressing_limited() is actually returning the > expected result at the point of radeon_ttm_init()? Disabling highmem is > presumably just hiding whatever problem exists, by throwing away all > >32-bit RAM such that use_dma32 doesn't matter. The device in question only supports a 32 bit DMA mask so dma_addressing_limited() should return true. Bounce buffers are not really usable on GPUs because they map so much memory. If dma_addressing_limited() returns false, that would explain it. Alex
Re: [PATCH v2] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On Tue, 29 Nov 2022 19:04:14 +0100, Uwe Kleine-König wrote: > Compared to the txt description this adds clocks and clock-names to > match reality. > > Note that fsl,imx-lcdc was picked as the new name as this is the actual > hardware's name. There will be a new binding implementing the saner drm > concept that is supposed to supersede this legacy fb binding > > Signed-off-by: Uwe Kleine-König > --- > Changes since v1, sent with Message-Id: > - mention clock stuff being added (Philipp) > - dropped some quotes (Rob) > - fix specification of compatible >(I kept claiming though that imx21 isn't compatible to imx1. While >that might be true, I don't have an i.MX1 to check the details and >currently the imx*.dtsi don't claim that compatibility.) > > I tried to implement the suggestion by Rob to formalize the display > binding. But I learned that this doesn't change how the display property > is formalized in the fsl,imx-lcdc.yaml (which is just a phandle without > means to specify that it should point to a node which fulfills a certain > binding.) > > Best regards > Uwe > > .../bindings/display/imx/fsl,imx-fb.txt | 57 -- > .../bindings/display/imx/fsl,imx-lcdc.yaml| 102 ++ > 2 files changed, 102 insertions(+), 57 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt > create mode 100644 > Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml > Applied, thanks!
Re: [PATCH] net: tun: Remove redundant null checks before kfree
+ Thierry Reding, linux-tegra, dri-devel
On Tue, Nov 29, 2022 at 04:43:29PM +0800, zys.zlj...@gmail.com wrote:
> From: Yushan Zhou
>
> Fix the following coccicheck warning:
> ./drivers/gpu/host1x/fence.c:97:2-7: WARNING:
> NULL check before some freeing functions is not needed.
>
> Signed-off-by: Yushan Zhou
Hi,
the change in the patch looks good to me.
However, it does not appear to be a networking patch,
so I think you have sent it to the wrong place.
With reference to:
$ ./scripts/get_maintainer.pl drivers/gpu/host1x/fence.c
Thierry Reding (supporter:DRM DRIVERS FOR NVIDIA
TEGRA)
David Airlie (maintainer:DRM DRIVERS)
Daniel Vetter (maintainer:DRM DRIVERS)
Sumit Semwal (maintainer:DMA BUFFER SHARING FRAMEWORK)
"Christian König" (maintainer:DMA BUFFER SHARING
FRAMEWORK)
dri-devel@lists.freedesktop.org (open list:DRM DRIVERS FOR NVIDIA TEGRA)
linux-te...@vger.kernel.org (open list:DRM DRIVERS FOR NVIDIA TEGRA)
linux-ker...@vger.kernel.org (open list)
linux-me...@vger.kernel.org (open list:DMA BUFFER SHARING FRAMEWORK)
linaro-mm-...@lists.linaro.org (moderated list:DMA BUFFER SHARING FRAMEWORK)
And
https://lore.kernel.org/dri-devel/39c44dce203112a8dfe279e8e2c4ad164e3cf5e5.1666275461.git.robin.mur...@arm.com/
I would suggest that the patch subject should be:
[PATCH] gpu: host1x: Remove redundant null check before kfree
And you should send it:
To: Thierry Reding
Cc: linux-te...@vger.kernel.org, dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/host1x/fence.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/host1x/fence.c b/drivers/gpu/host1x/fence.c
> index ecab72882192..05b36bfc8b74 100644
> --- a/drivers/gpu/host1x/fence.c
> +++ b/drivers/gpu/host1x/fence.c
> @@ -93,8 +93,7 @@ static void host1x_syncpt_fence_release(struct dma_fence *f)
> {
> struct host1x_syncpt_fence *sf = to_host1x_fence(f);
>
> - if (sf->waiter)
> - kfree(sf->waiter);
> + kfree(sf->waiter);
>
> dma_fence_free(f);
> }
> --
> 2.27.0
>
Re: [PATCH] drm/bridge: ti-sn65dsi86: Fix output polarity setting bug
Hi,
On Tue, Nov 29, 2022 at 9:46 PM Qiqi Zhang wrote:
>
> Hi,
>
> on Nov. 29, 2022, 11:45 a.m. Tomi wrote:
> >On 29/11/2022 03:13, Doug Anderson wrote:
> >> Hi,
> >>
> >> On Fri, Nov 25, 2022 at 2:54 AM Qiqi Zhang
> >> wrote:
> >>>
> >>> According to the description in ti-sn65dsi86's datasheet:
> >>>
> >>> CHA_HSYNC_POLARITY:
> >>> 0 = Active High Pulse. Synchronization signal is high for the sync
> >>> pulse width. (default)
> >>> 1 = Active Low Pulse. Synchronization signal is low for the sync
> >>> pulse width.
> >>>
> >>> CHA_VSYNC_POLARITY:
> >>> 0 = Active High Pulse. Synchronization signal is high for the sync
> >>> pulse width. (Default)
> >>> 1 = Active Low Pulse. Synchronization signal is low for the sync
> >>> pulse width.
> >>>
> >>> We should only set these bits when the polarity is negative.
> >>> Signed-off-by: Qiqi Zhang
> >>> ---
> >>> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >>> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >>> index 3c3561942eb6..eb24322df721 100644
> >>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >>> @@ -931,9 +931,9 @@ static void ti_sn_bridge_set_video_timings(struct
> >>> ti_sn65dsi86 *pdata)
> >>> &pdata->bridge.encoder->crtc->state->adjusted_mode;
> >>> u8 hsync_polarity = 0, vsync_polarity = 0;
> >>>
> >>> - if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> >>> + if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> >>> hsync_polarity = CHA_HSYNC_POLARITY;
> >>> - if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> >>> + if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> >>> vsync_polarity = CHA_VSYNC_POLARITY;
> >>
> >> Looks right to me.
> >>
> >> Reviewed-by: Douglas Anderson
> >>
> >> I've never seen the polarity matter for any eDP panels I've worked
> >> with, which presumably explains why this was wrong for so long. As far
> >
> >Afaik, DP doesn't have sync polarity as such (neither does DSI), and the
> >sync polarity is just "metadata". So if you're in full-DP domain, I
> >don't see why it would matter. I guess it becomes relevant when you
> >convert from DP to some other bus format.
>
> Just like Tomi said, the wrong polarity worked fine on my eDP panel(LP079QX1)
> and standard DP monitor, I didn't notice the polarity configuration problem
> here until my customer used the following solution and got a abnormal display:
> GPU->mipi->eDP->DP->lvds->panel.
Wow, that's convoluted, but makes sense. I think this fully explains
why this is a problem for you but wasn't in the past.
> >> as I can tell, it's been wrong since the start. Probably you should
> >> have:
> >>
> >> Fixes: a095f15c00e2 ("drm/bridge: add support for sn65dsi86 bridge driver")
>
> Doug you mean I need to update my commit message? It's my first time using
> kernel list and I'm a little confused about this.
Nah, I'll add it in and land it. OK, pushed to drm-misc-fixes:
8c115864501f drm/bridge: ti-sn65dsi86: Fix output polarity setting bug
Re: [PATCH] dt-bindings: msm/dsi: Don't require vcca-supply on 14nm PHY
On 30/11/2022 14:58, Konrad Dybcio wrote: > On some SoCs (hello SM6115) vcca-supply is not wired to any smd-rpm > or rpmh regulator, but instead powered by the VDD_MX line, which is > voted for in the DSI ctrl node. > > Signed-off-by: Konrad Dybcio Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 01/12] dt-bindings: display: msm: Replace mdss with display-subsystem
On 29/11/2022 21:46, Adam Skladowski wrote: > Follow other YAMLs and replace mdss name. That's really not explaining what you are doing here. Your commit msg and subject suggest you rename mdss. But you don't. You touch only examples. > > Signed-off-by: Adam Skladowski > --- > .../devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml | 2 +- > .../devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > Best regards, Krzysztof
Re: [PATCH 02/12] dt-bindings: thermal: tsens: Add SM6115 compatible
On 29/11/2022 21:46, Adam Skladowski wrote: > Document compatible for tsens on Qualcomm SM6115 platform > according to downstream dts it ship v2.4 of IP > > Signed-off-by: Adam Skladowski Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 10/12] arm64: dts: qcom: sm6115: Add i2c/spi nodes
On 29/11/2022 21:46, Adam Skladowski wrote:
> Add I2C/SPI nodes for SM6115.
>
> Signed-off-by: Adam Skladowski
> ---
> arch/arm64/boot/dts/qcom/sm6115.dtsi | 287 +++
> 1 file changed, 287 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index e9de7aa1efdd..d14a4595be8a 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -6,6 +6,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -357,6 +358,90 @@ tlmm: pinctrl@50 {
> interrupt-controller;
> #interrupt-cells = <2>;
>
> + qup_i2c0_default: qup-i2c0-default {
Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Missing -state suffix. Same in other places.
> + pins = "gpio0", "gpio1";
> + function = "qup0";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
Best regards,
Krzysztof
[PATCH v2 03/11] drm/bridge: ti-sn65dsi86: Propagate errors in .get_state() to the caller
.get_state() can return an error indication. Make use of it to propagate failing hardware accesses. Signed-off-by: Uwe Kleine-König --- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 6826d2423ae9..9671071490d8 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -1512,19 +1512,19 @@ static int ti_sn_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, ret = regmap_read(pdata->regmap, SN_PWM_EN_INV_REG, &pwm_en_inv); if (ret) - return 0; + return ret; ret = ti_sn65dsi86_read_u16(pdata, SN_BACKLIGHT_SCALE_REG, &scale); if (ret) - return 0; + return ret; ret = ti_sn65dsi86_read_u16(pdata, SN_BACKLIGHT_REG, &backlight); if (ret) - return 0; + return ret; ret = regmap_read(pdata->regmap, SN_PWM_PRE_DIV_REG, &pre_div); if (ret) - return 0; + return ret; state->enabled = FIELD_GET(SN_PWM_EN_MASK, pwm_en_inv); if (FIELD_GET(SN_PWM_INV_MASK, pwm_en_inv)) -- 2.38.1
[PATCH v2 00/11] pwm: Allow .get_state to fail
Hello, I forgot about this series and was remembered when I talked to Conor Dooley about how .get_state() should behave in an error case. Compared to (implicit) v1, sent with Message-Id: 20220916151506.298488-1-u.kleine-koe...@pengutronix.de I changed: - Patch #1 which does the prototype change now just adds "return 0" to all implementations and so gets simpler and doesn't change behaviour. The adaptions to the different .get_state() implementations are split out into individual patches to ease review. - One minor inconsistency fixed in "pwm: Handle .get_state() failures" that I noticed while looking into this patch. - I skipped changing sun4i.c as I don't know how to handle the error there. Someone might want to have a look. (That's not ideal, but it's not worse than the same issue before this series.) In v1 Thierry had the concern: | That raises the question about what to do in these cases. If we return | an error, that could potentially throw off consumers. So perhaps the | closest would be to return a disabled PWM? Or perhaps it'd be up to the | consumer to provide some fallback configuration for invalidly configured | or unconfigured PWMs. .get_state() is only called in pwm_device_request on a pwm_state that a consumer might see. Before my series a consumer might have seen a partial modified pwm_state (because .get_state() might have modified .period, then stumbled and returned silently). The last patch ensures that this partial modification isn't given out to the consumer. Instead they now see the same as if .get_state wasn't implemented at all. Best regards Uwe Uwe Kleine-König (11): pwm: Make .get_state() callback return an error code pwm/tracing: Also record trace events for failed API calls drm/bridge: ti-sn65dsi86: Propagate errors in .get_state() to the caller leds: qcom-lpg: Propagate errors in .get_state() to the caller pwm: crc: Propagate errors in .get_state() to the caller pwm: cros-ec: Propagate errors in .get_state() to the caller pwm: imx27: Propagate errors in .get_state() to the caller pwm: mtk-disp: Propagate errors in .get_state() to the caller pwm: rockchip: Propagate errors in .get_state() to the caller pwm: sprd: Propagate errors in .get_state() to the caller pwm: Handle .get_state() failures drivers/gpio/gpio-mvebu.c | 9 ++--- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 14 -- drivers/leds/rgb/leds-qcom-lpg.c | 14 -- drivers/pwm/core.c| 28 +-- drivers/pwm/pwm-atmel.c | 6 -- drivers/pwm/pwm-bcm-iproc.c | 8 +--- drivers/pwm/pwm-crc.c | 10 ++ drivers/pwm/pwm-cros-ec.c | 8 +--- drivers/pwm/pwm-dwc.c | 6 -- drivers/pwm/pwm-hibvt.c | 6 -- drivers/pwm/pwm-imx-tpm.c | 8 +--- drivers/pwm/pwm-imx27.c | 8 +--- drivers/pwm/pwm-intel-lgm.c | 6 -- drivers/pwm/pwm-iqs620a.c | 6 -- drivers/pwm/pwm-keembay.c | 6 -- drivers/pwm/pwm-lpss.c| 6 -- drivers/pwm/pwm-meson.c | 8 +--- drivers/pwm/pwm-mtk-disp.c| 12 +++- drivers/pwm/pwm-pca9685.c | 8 +--- drivers/pwm/pwm-raspberrypi-poe.c | 8 +--- drivers/pwm/pwm-rockchip.c| 12 +++- drivers/pwm/pwm-sifive.c | 6 -- drivers/pwm/pwm-sl28cpld.c| 8 +--- drivers/pwm/pwm-sprd.c| 8 +--- drivers/pwm/pwm-stm32-lp.c| 8 +--- drivers/pwm/pwm-sun4i.c | 12 +++- drivers/pwm/pwm-sunplus.c | 6 -- drivers/pwm/pwm-visconti.c| 6 -- drivers/pwm/pwm-xilinx.c | 8 +--- include/linux/pwm.h | 4 ++-- include/trace/events/pwm.h| 20 +-- 31 files changed, 174 insertions(+), 109 deletions(-) base-commit: 50315945d178eebec4e8e2c50c265767ddb926eb -- 2.38.1
[PATCH v2 01/11] pwm: Make .get_state() callback return an error code
.get_state() might fail in some cases. To make it possible that a driver
signals such a failure change the prototype of .get_state() to return an
error code.
This patch was created using coccinelle and the following semantic patch:
@p1@
identifier getstatefunc;
identifier driver;
@@
struct pwm_ops driver = {
...,
.get_state = getstatefunc
,...
};
@p2@
identifier p1.getstatefunc;
identifier chip, pwm, state;
@@
-void
+int
getstatefunc(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state
*state)
{
...
- return;
+ return 0;
...
}
plus the actual change of the prototype in include/linux/pwm.h (plus some
manual fixing of indentions and empty lines).
So for now all drivers return success unconditionally. They are adapted
in the following patches to make the changes easier reviewable.
Signed-off-by: Uwe Kleine-König
---
drivers/gpio/gpio-mvebu.c | 9 ++---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 14 --
drivers/leds/rgb/leds-qcom-lpg.c | 14 --
drivers/pwm/pwm-atmel.c | 6 --
drivers/pwm/pwm-bcm-iproc.c | 8 +---
drivers/pwm/pwm-crc.c | 10 ++
drivers/pwm/pwm-cros-ec.c | 8 +---
drivers/pwm/pwm-dwc.c | 6 --
drivers/pwm/pwm-hibvt.c | 6 --
drivers/pwm/pwm-imx-tpm.c | 8 +---
drivers/pwm/pwm-imx27.c | 8 +---
drivers/pwm/pwm-intel-lgm.c | 6 --
drivers/pwm/pwm-iqs620a.c | 6 --
drivers/pwm/pwm-keembay.c | 6 --
drivers/pwm/pwm-lpss.c| 6 --
drivers/pwm/pwm-meson.c | 8 +---
drivers/pwm/pwm-mtk-disp.c| 12 +++-
drivers/pwm/pwm-pca9685.c | 8 +---
drivers/pwm/pwm-raspberrypi-poe.c | 8 +---
drivers/pwm/pwm-rockchip.c| 12 +++-
drivers/pwm/pwm-sifive.c | 6 --
drivers/pwm/pwm-sl28cpld.c| 8 +---
drivers/pwm/pwm-sprd.c| 8 +---
drivers/pwm/pwm-stm32-lp.c| 8 +---
drivers/pwm/pwm-sun4i.c | 12 +++-
drivers/pwm/pwm-sunplus.c | 6 --
drivers/pwm/pwm-visconti.c| 6 --
drivers/pwm/pwm-xilinx.c | 8 +---
include/linux/pwm.h | 4 ++--
29 files changed, 146 insertions(+), 89 deletions(-)
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 1bb317b8dcce..91a4232ee58c 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -657,9 +657,10 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct
pwm_device *pwm)
spin_unlock_irqrestore(&mvpwm->lock, flags);
}
-static void mvebu_pwm_get_state(struct pwm_chip *chip,
- struct pwm_device *pwm,
- struct pwm_state *state) {
+static int mvebu_pwm_get_state(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ struct pwm_state *state)
+{
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
@@ -693,6 +694,8 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
state->enabled = false;
spin_unlock_irqrestore(&mvpwm->lock, flags);
+
+ return 0;
}
static int mvebu_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 3c3561942eb6..6826d2423ae9 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -1500,8 +1500,8 @@ static int ti_sn_pwm_apply(struct pwm_chip *chip, struct
pwm_device *pwm,
return ret;
}
-static void ti_sn_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+static int ti_sn_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
{
struct ti_sn65dsi86 *pdata = pwm_chip_to_ti_sn_bridge(chip);
unsigned int pwm_en_inv;
@@ -1512,19 +1512,19 @@ static void ti_sn_pwm_get_state(struct pwm_chip *chip,
struct pwm_device *pwm,
ret = regmap_read(pdata->regmap, SN_PWM_EN_INV_REG, &pwm_en_inv);
if (ret)
- return;
+ return 0;
ret = ti_sn65dsi86_read_u16(pdata, SN_BACKLIGHT_SCALE_REG, &scale);
if (ret)
- return;
+ return 0;
ret = ti_sn65dsi86_read_u16(pdata, SN_BACKLIGHT_REG, &backlight);
if (ret)
- return;
+ return 0;
ret = regmap_read(pdata->regmap, SN_PWM_PRE_DIV_REG, &pre_div);
if (ret)
- return;
+ return 0;
state->enabled = FIELD_GET(SN_PWM_EN_MASK, pwm_en_inv);
if (FIE
Re: [PATCH v3 0/2] drm: Add GPU reset sysfs
On 11/28/22 06:30, Simon Ser wrote: The PID is racy, the user-space daemon could end up killing an unrelated process… Is there any way we could use a pidfd instead? Is the PID race condition something that really happens or rather something theoretical? Anyway, I can't see how pidfd and uevent would work together. Since uevent it's kind of a broadcast and pidfd is an anon file, it wouldn't be possible to say to userspace which is the fd to be used giving that file descriptors are per process resources. On the other hand, this interface could be converted to be an ioctl that userspace would block waiting for a reset notification, then the kernel could create a pidfd and give to the blocked process the right fd. We would probably need a queue to make sure no event is lost. Thanks André
Re: [PATCH v3 0/2] drm: Add GPU reset sysfs
On Wednesday, November 30th, 2022 at 16:23, André Almeida wrote: > On 11/28/22 06:30, Simon Ser wrote: > > > The PID is racy, the user-space daemon could end up killing an > > unrelated process… Is there any way we could use a pidfd instead? > > Is the PID race condition something that really happens or rather > something theoretical? A PID race can happen in practice if many PIDs get spawned. On Linux PIDs wrap around pretty quickly. Note, even a sandboxed program inside its own PID namespace can trigger the wrap-around. > Anyway, I can't see how pidfd and uevent would work together. Since > uevent it's kind of a broadcast and pidfd is an anon file, it wouldn't > be possible to say to userspace which is the fd to be used giving that > file descriptors are per process resources. Yeah, I can see how this can be difficult to integrate with uevent. > On the other hand, this interface could be converted to be an ioctl that > userspace would block waiting for a reset notification, then the kernel > could create a pidfd and give to the blocked process the right fd. We > would probably need a queue to make sure no event is lost. A blocking IOCTL wouldn't be very nice, you can't integrate that into an event loop for instance…
Re: Screen corruption using radeon kernel driver
On 2022-11-30 14:28, Alex Deucher wrote: On Wed, Nov 30, 2022 at 7:54 AM Robin Murphy wrote: On 2022-11-29 17:11, Mikhail Krylov wrote: On Tue, Nov 29, 2022 at 11:05:28AM -0500, Alex Deucher wrote: On Tue, Nov 29, 2022 at 10:59 AM Mikhail Krylov wrote: On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote: On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov wrote: On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher wrote: [excessive quoting removed] So, is there any progress on this issue? I do understand it's not a high priority one, and today I've checked it on 6.0 kernel, and unfortunately, it still persists... I'm considering writing a patch that will allow user to override need_dma32/dma_bits setting with a module parameter. I'll have some time after the New Year for that. Is it at all possible that such a patch will be merged into kernel? On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov wrote: Unless someone familiar with HIMEM can figure out what is going wrong we should just revert the patch. Alex Okay, I was suggesting that mostly because a) it works for me with dma_bits = 40 (I understand that's what it is without the original patch applied); b) there's a hint of uncertainity on this line https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359 saying that for AGP dma_bits = 32 is the safest option, so apparently there are setups, unlike mine, where dma_bits = 32 is better than 40. But I'm in no position to argue, just wanted to make myself clear. I'm okay with rebuilding the kernel for my machine until the original patch is reverted or any other fix is applied. What GPU do you have and is it AGP? If it is AGP, does setting radeon.agpmode=-1 also fix it? Alex That is ATI Radeon X1950, and, unfortunately, radeon.agpmode=-1 doesn't help, it just makes 3D acceleration in games such as OpenArena stop working. Just to confirm, is the board AGP or PCIe? Alex It is AGP. That's an old machine. Can you check whether dma_addressing_limited() is actually returning the expected result at the point of radeon_ttm_init()? Disabling highmem is presumably just hiding whatever problem exists, by throwing away all >32-bit RAM such that use_dma32 doesn't matter. The device in question only supports a 32 bit DMA mask so dma_addressing_limited() should return true. Bounce buffers are not really usable on GPUs because they map so much memory. If dma_addressing_limited() returns false, that would explain it. Right, it appears to be the only part of the offending commit that *could* reasonably make any difference, so I'm primarily wondering if dma_get_required_mask() somehow gets confused. Thanks, Robin.
Re: [PATCH v2 1/5] drm/i915/gt: Correct kerneldoc for intel_gt_mcr_wait_for_reg()
On 28.11.2022 15:30, Matt Roper wrote:
> The kerneldoc function name was not updated when this function was
> converted to a non-fw form.
>
> Fixes: 192bb40f030a ("drm/i915/gt: Manage uncore->lock while waiting on MCR
> register")
> Reported-by: kernel test robot
> Signed-off-by: Matt Roper
Reviewed-by: Balasubramani Vivekanandan
Regards,
Bala
> ---
> drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> index d9a8ff9e5e57..ea86c1ab5dc5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> @@ -702,7 +702,7 @@ void intel_gt_mcr_get_ss_steering(struct intel_gt *gt,
> unsigned int dss,
> }
>
> /**
> - * intel_gt_mcr_wait_for_reg_fw - wait until MCR register matches expected
> state
> + * intel_gt_mcr_wait_for_reg - wait until MCR register matches expected state
> * @gt: GT structure
> * @reg: the register to read
> * @mask: mask to apply to register value
> --
> 2.38.1
>
Re: [PATCH v2 2/5] drm/i915/gt: Pass gt rather than uncore to lowest-level reads/writes
On 28.11.2022 15:30, Matt Roper wrote:
> Passing the GT rather than uncore to the lowest level MCR read and write
> functions will make it easier to introduce dedicated MCR locking in a
> following patch.
>
> Signed-off-by: Matt Roper
Reviewed-by: Balasubramani Vivekanandan
Regards,
Bala
> ---
> drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 18 ++
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> index ea86c1ab5dc5..f4484bb18ec9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> @@ -221,7 +221,7 @@ static i915_reg_t mcr_reg_cast(const i915_mcr_reg_t mcr)
>
> /*
> * rw_with_mcr_steering_fw - Access a register with specific MCR steering
> - * @uncore: pointer to struct intel_uncore
> + * @gt: GT to read register from
> * @reg: register being accessed
> * @rw_flag: FW_REG_READ for read access or FW_REG_WRITE for write access
> * @group: group number (documented as "sliceid" on older platforms)
> @@ -232,10 +232,11 @@ static i915_reg_t mcr_reg_cast(const i915_mcr_reg_t mcr)
> *
> * Caller needs to make sure the relevant forcewake wells are up.
> */
> -static u32 rw_with_mcr_steering_fw(struct intel_uncore *uncore,
> +static u32 rw_with_mcr_steering_fw(struct intel_gt *gt,
> i915_mcr_reg_t reg, u8 rw_flag,
> int group, int instance, u32 value)
> {
> + struct intel_uncore *uncore = gt->uncore;
> u32 mcr_mask, mcr_ss, mcr, old_mcr, val = 0;
>
> lockdep_assert_held(&uncore->lock);
> @@ -308,11 +309,12 @@ static u32 rw_with_mcr_steering_fw(struct intel_uncore
> *uncore,
> return val;
> }
>
> -static u32 rw_with_mcr_steering(struct intel_uncore *uncore,
> +static u32 rw_with_mcr_steering(struct intel_gt *gt,
> i915_mcr_reg_t reg, u8 rw_flag,
> int group, int instance,
> u32 value)
> {
> + struct intel_uncore *uncore = gt->uncore;
> enum forcewake_domains fw_domains;
> u32 val;
>
> @@ -325,7 +327,7 @@ static u32 rw_with_mcr_steering(struct intel_uncore
> *uncore,
> spin_lock_irq(&uncore->lock);
> intel_uncore_forcewake_get__locked(uncore, fw_domains);
>
> - val = rw_with_mcr_steering_fw(uncore, reg, rw_flag, group, instance,
> value);
> + val = rw_with_mcr_steering_fw(gt, reg, rw_flag, group, instance, value);
>
> intel_uncore_forcewake_put__locked(uncore, fw_domains);
> spin_unlock_irq(&uncore->lock);
> @@ -347,7 +349,7 @@ u32 intel_gt_mcr_read(struct intel_gt *gt,
> i915_mcr_reg_t reg,
> int group, int instance)
> {
> - return rw_with_mcr_steering(gt->uncore, reg, FW_REG_READ, group,
> instance, 0);
> + return rw_with_mcr_steering(gt, reg, FW_REG_READ, group, instance, 0);
> }
>
> /**
> @@ -364,7 +366,7 @@ u32 intel_gt_mcr_read(struct intel_gt *gt,
> void intel_gt_mcr_unicast_write(struct intel_gt *gt, i915_mcr_reg_t reg, u32
> value,
> int group, int instance)
> {
> - rw_with_mcr_steering(gt->uncore, reg, FW_REG_WRITE, group, instance,
> value);
> + rw_with_mcr_steering(gt, reg, FW_REG_WRITE, group, instance, value);
> }
>
> /**
> @@ -588,7 +590,7 @@ u32 intel_gt_mcr_read_any_fw(struct intel_gt *gt,
> i915_mcr_reg_t reg)
> for (type = 0; type < NUM_STEERING_TYPES; type++) {
> if (reg_needs_read_steering(gt, reg, type)) {
> get_nonterminated_steering(gt, type, &group, &instance);
> - return rw_with_mcr_steering_fw(gt->uncore, reg,
> + return rw_with_mcr_steering_fw(gt, reg,
> FW_REG_READ,
> group, instance, 0);
> }
> @@ -615,7 +617,7 @@ u32 intel_gt_mcr_read_any(struct intel_gt *gt,
> i915_mcr_reg_t reg)
> for (type = 0; type < NUM_STEERING_TYPES; type++) {
> if (reg_needs_read_steering(gt, reg, type)) {
> get_nonterminated_steering(gt, type, &group, &instance);
> - return rw_with_mcr_steering(gt->uncore, reg,
> + return rw_with_mcr_steering(gt, reg,
> FW_REG_READ,
> group, instance, 0);
> }
> --
> 2.38.1
>
Re: [PATCH v2 3/5] drm/i915/gt: Add dedicated MCR lock
On 28.11.2022 15:30, Matt Roper wrote:
> We've been overloading uncore->lock to protect access to the MCR
> steering register. That's not really what uncore->lock is intended for,
> and it would be better if we didn't need to hold such a high-traffic
> spinlock for the whole sequence of (apply steering, access MCR register,
> restore steering). Let's create a dedicated MCR lock to protect the
> steering control register over this critical section and stop relying on
> the high-traffic uncore->lock.
>
> For now the new lock is a software lock. However some platforms (MTL
> and beyond) have a hardware-provided locking mechanism that can be used
> to serialize not only software accesses, but also hardware/firmware
> accesses as well; support for that hardware level lock will be added in
> a future patch.
>
> v2:
> - Use irqsave/irqrestore spinlock calls; platforms using execlist
>submission rather than GuC submission can perform MCR accesses in
>interrupt context because reset -> errordump happens in a tasklet.
>
> Cc: Chris Wilson
> Cc: Mika Kuoppala
> Cc: Balasubramani Vivekanandan
> Signed-off-by: Matt Roper
Reviewed-by: Balasubramani Vivekanandan
Regards,
Bala
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 7 +-
> drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 79 +++--
> drivers/gpu/drm/i915/gt/intel_gt_mcr.h | 2 +
> drivers/gpu/drm/i915/gt/intel_gt_types.h| 8 +++
> drivers/gpu/drm/i915/gt/intel_mocs.c| 3 +
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 12 ++--
> 6 files changed, 101 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 7ef0edb2e37c..6847f3bd2b03 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -1079,6 +1079,7 @@ static void mmio_invalidate_full(struct intel_gt *gt)
> enum intel_engine_id id;
> const i915_reg_t *regs;
> unsigned int num = 0;
> + unsigned long flags;
>
> if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> regs = NULL;
> @@ -1099,7 +1100,8 @@ static void mmio_invalidate_full(struct intel_gt *gt)
>
> intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
>
> - spin_lock_irq(&uncore->lock); /* serialise invalidate with GT reset */
> + intel_gt_mcr_lock(gt, &flags);
> + spin_lock(&uncore->lock); /* serialise invalidate with GT reset */
>
> awake = 0;
> for_each_engine(engine, gt, id) {
> @@ -1133,7 +1135,8 @@ static void mmio_invalidate_full(struct intel_gt *gt)
>IS_ALDERLAKE_P(i915)))
> intel_uncore_write_fw(uncore, GEN12_OA_TLB_INV_CR, 1);
>
> - spin_unlock_irq(&uncore->lock);
> + spin_unlock(&uncore->lock);
> + intel_gt_mcr_unlock(gt, flags);
>
> for_each_engine_masked(engine, gt, awake, tmp) {
> struct reg_and_bit rb;
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> index f4484bb18ec9..aa070ae57f11 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> @@ -143,6 +143,8 @@ void intel_gt_mcr_init(struct intel_gt *gt)
> unsigned long fuse;
> int i;
>
> + spin_lock_init(>->mcr_lock);
> +
> /*
>* An mslice is unavailable only if both the meml3 for the slice is
>* disabled *and* all of the DSS in the slice (quadrant) are disabled.
> @@ -228,6 +230,7 @@ static i915_reg_t mcr_reg_cast(const i915_mcr_reg_t mcr)
> * @instance: instance number (documented as "subsliceid" on older platforms)
> * @value: register value to be written (ignored for read)
> *
> + * Context: The caller must hold the MCR lock
> * Return: 0 for write access. register value for read access.
> *
> * Caller needs to make sure the relevant forcewake wells are up.
> @@ -239,7 +242,7 @@ static u32 rw_with_mcr_steering_fw(struct intel_gt *gt,
> struct intel_uncore *uncore = gt->uncore;
> u32 mcr_mask, mcr_ss, mcr, old_mcr, val = 0;
>
> - lockdep_assert_held(&uncore->lock);
> + lockdep_assert_held(>->mcr_lock);
>
> if (GRAPHICS_VER_FULL(uncore->i915) >= IP_VER(12, 70)) {
> /*
> @@ -316,6 +319,7 @@ static u32 rw_with_mcr_steering(struct intel_gt *gt,
> {
> struct intel_uncore *uncore = gt->uncore;
> enum forcewake_domains fw_domains;
> + unsigned long flags;
> u32 val;
>
> fw_domains = intel_uncore_forcewake_for_reg(uncore, mcr_reg_cast(reg),
> @@ -324,17 +328,59 @@ static u32 rw_with_mcr_steering(struct intel_gt *gt,
>GEN8_MCR_SELECTOR,
>FW_REG_READ |
> FW_REG_WRITE);
>
> - spin_lock_irq(&uncore->lock);
> + intel_gt_mcr_lock(gt, &flags);
> + spin_lock(&uncore->lock);
> intel_uncore_forcewake_get__locked(uncore, fw_domains);
>
> val =
Re: [PATCH v2 5/5] drm/i915/mtl: Hold forcewake and MCR lock over PPAT setup
On 28.11.2022 15:30, Matt Roper wrote:
> PPAT setup involves a series of multicast writes. This can be optimized
> slightly be acquiring forcewake and the steering lock just once for the
> entire sequence.
>
> Suggested-by: Balasubramani Vivekanandan
>
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/i915/gt/intel_gtt.c | 27 +++
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
> b/drivers/gpu/drm/i915/gt/intel_gtt.c
> index 2ba3983984b9..288d9f118ee9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> @@ -482,14 +482,25 @@ static void tgl_setup_private_ppat(struct intel_uncore
> *uncore)
>
> static void xehp_setup_private_ppat(struct intel_gt *gt)
> {
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(0), GEN8_PPAT_WB);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(1), GEN8_PPAT_WC);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(2), GEN8_PPAT_WT);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(3), GEN8_PPAT_UC);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(4), GEN8_PPAT_WB);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(5), GEN8_PPAT_WB);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(6), GEN8_PPAT_WB);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(7), GEN8_PPAT_WB);
> + enum forcewake_domains fw;
> + unsigned long flags;
> +
> + fw = intel_uncore_forcewake_for_reg(gt->uncore,
> _MMIO(XEHP_PAT_INDEX(0).reg),
> + FW_REG_READ);
I am not completely aware of forcewake implementation. I am wondering if
the last parameter should be FW_REG_WRITE since it is register write
which is happening later.
Regards,
Bala
> + intel_uncore_forcewake_get(gt->uncore, fw);
> +
> + intel_gt_mcr_lock(gt, &flags);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(0), GEN8_PPAT_WB);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(1), GEN8_PPAT_WC);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(2), GEN8_PPAT_WT);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(3), GEN8_PPAT_UC);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(4), GEN8_PPAT_WB);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(5), GEN8_PPAT_WB);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(6), GEN8_PPAT_WB);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(7), GEN8_PPAT_WB);
> + intel_gt_mcr_unlock(gt, flags);
> +
> + intel_uncore_forcewake_put(gt->uncore, fw);
> }
>
> static void icl_setup_private_ppat(struct intel_uncore *uncore)
> --
> 2.38.1
>
[PATCH v3 5/5] drm/i915/mtl: Hold forcewake and MCR lock over PPAT setup
PPAT setup involves a series of multicast writes. This can be optimized
slightly be acquiring forcewake and the steering lock just once for the
entire sequence.
v2:
- We should use FW_REG_WRITE instead of FW_REG_READ. (Bala)
Suggested-by: Balasubramani Vivekanandan
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/intel_gtt.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 2ba3983984b9..e37164a60d37 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -482,14 +482,25 @@ static void tgl_setup_private_ppat(struct intel_uncore
*uncore)
static void xehp_setup_private_ppat(struct intel_gt *gt)
{
- intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(0), GEN8_PPAT_WB);
- intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(1), GEN8_PPAT_WC);
- intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(2), GEN8_PPAT_WT);
- intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(3), GEN8_PPAT_UC);
- intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(4), GEN8_PPAT_WB);
- intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(5), GEN8_PPAT_WB);
- intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(6), GEN8_PPAT_WB);
- intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(7), GEN8_PPAT_WB);
+ enum forcewake_domains fw;
+ unsigned long flags;
+
+ fw = intel_uncore_forcewake_for_reg(gt->uncore,
_MMIO(XEHP_PAT_INDEX(0).reg),
+ FW_REG_WRITE);
+ intel_uncore_forcewake_get(gt->uncore, fw);
+
+ intel_gt_mcr_lock(gt, &flags);
+ intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(0), GEN8_PPAT_WB);
+ intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(1), GEN8_PPAT_WC);
+ intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(2), GEN8_PPAT_WT);
+ intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(3), GEN8_PPAT_UC);
+ intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(4), GEN8_PPAT_WB);
+ intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(5), GEN8_PPAT_WB);
+ intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(6), GEN8_PPAT_WB);
+ intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(7), GEN8_PPAT_WB);
+ intel_gt_mcr_unlock(gt, flags);
+
+ intel_uncore_forcewake_put(gt->uncore, fw);
}
static void icl_setup_private_ppat(struct intel_uncore *uncore)
--
2.38.1
Re: [PATCH v2 5/5] drm/i915/mtl: Hold forcewake and MCR lock over PPAT setup
On Wed, Nov 30, 2022 at 09:21:07PM +0530, Balasubramani Vivekanandan wrote:
> On 28.11.2022 15:30, Matt Roper wrote:
> > PPAT setup involves a series of multicast writes. This can be optimized
> > slightly be acquiring forcewake and the steering lock just once for the
> > entire sequence.
> >
> > Suggested-by: Balasubramani Vivekanandan
> >
> > Signed-off-by: Matt Roper
> > ---
> > drivers/gpu/drm/i915/gt/intel_gtt.c | 27 +++
> > 1 file changed, 19 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
> > b/drivers/gpu/drm/i915/gt/intel_gtt.c
> > index 2ba3983984b9..288d9f118ee9 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> > @@ -482,14 +482,25 @@ static void tgl_setup_private_ppat(struct
> > intel_uncore *uncore)
> >
> > static void xehp_setup_private_ppat(struct intel_gt *gt)
> > {
> > - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(0), GEN8_PPAT_WB);
> > - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(1), GEN8_PPAT_WC);
> > - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(2), GEN8_PPAT_WT);
> > - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(3), GEN8_PPAT_UC);
> > - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(4), GEN8_PPAT_WB);
> > - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(5), GEN8_PPAT_WB);
> > - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(6), GEN8_PPAT_WB);
> > - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(7), GEN8_PPAT_WB);
> > + enum forcewake_domains fw;
> > + unsigned long flags;
> > +
> > + fw = intel_uncore_forcewake_for_reg(gt->uncore,
> > _MMIO(XEHP_PAT_INDEX(0).reg),
> > + FW_REG_READ);
>
> I am not completely aware of forcewake implementation. I am wondering if
> the last parameter should be FW_REG_WRITE since it is register write
> which is happening later.
Yep, good catch. Using FW_REG_WRITE allows the driver to potentially
skip obtaining forcewake and waking the hardware if the registers being
written are "shadowed" so it's always good to use FW_REG_WRITE in places
where we're only writing and not reading.
In this case the specific registers in question don't appear to be part
of the shadow table for any affected platforms (e.g.,
dg2_shadowed_regs[] and such in intel_uncore.c), so FW_REG_WRITE will
still behave the same as FW_REG_READ here. But it's always possible
that future platforms could decide to shadow these registers, so it's
good to fix anyway; I just sent an updated copy of this patch making
that change.
Matt
>
> Regards,
> Bala
>
> > + intel_uncore_forcewake_get(gt->uncore, fw);
> > +
> > + intel_gt_mcr_lock(gt, &flags);
> > + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(0), GEN8_PPAT_WB);
> > + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(1), GEN8_PPAT_WC);
> > + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(2), GEN8_PPAT_WT);
> > + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(3), GEN8_PPAT_UC);
> > + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(4), GEN8_PPAT_WB);
> > + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(5), GEN8_PPAT_WB);
> > + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(6), GEN8_PPAT_WB);
> > + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(7), GEN8_PPAT_WB);
> > + intel_gt_mcr_unlock(gt, flags);
> > +
> > + intel_uncore_forcewake_put(gt->uncore, fw);
> > }
> >
> > static void icl_setup_private_ppat(struct intel_uncore *uncore)
> > --
> > 2.38.1
> >
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
Re: Screen corruption using radeon kernel driver
On Wed, Nov 30, 2022 at 10:42 AM Robin Murphy wrote: > > On 2022-11-30 14:28, Alex Deucher wrote: > > On Wed, Nov 30, 2022 at 7:54 AM Robin Murphy wrote: > >> > >> On 2022-11-29 17:11, Mikhail Krylov wrote: > >>> On Tue, Nov 29, 2022 at 11:05:28AM -0500, Alex Deucher wrote: > On Tue, Nov 29, 2022 at 10:59 AM Mikhail Krylov > wrote: > > > > On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote: > >> On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov > >> wrote: > >>> > >>> On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher wrote: > >>> > >> [excessive quoting removed] > >>> > > So, is there any progress on this issue? I do understand it's not a > > high > > priority one, and today I've checked it on 6.0 kernel, and > > unfortunately, it still persists... > > > > I'm considering writing a patch that will allow user to override > > need_dma32/dma_bits setting with a module parameter. I'll have some > > time > > after the New Year for that. > > > > Is it at all possible that such a patch will be merged into kernel? > > > On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov > wrote: > Unless someone familiar with HIMEM can figure out what is going wrong > we should just revert the patch. > > Alex > >>> > >>> > >>> Okay, I was suggesting that mostly because > >>> > >>> a) it works for me with dma_bits = 40 (I understand that's what it is > >>> without the original patch applied); > >>> > >>> b) there's a hint of uncertainity on this line > >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359 > >>> saying that for AGP dma_bits = 32 is the safest option, so apparently > >>> there are > >>> setups, unlike mine, where dma_bits = 32 is better than 40. > >>> > >>> But I'm in no position to argue, just wanted to make myself clear. > >>> I'm okay with rebuilding the kernel for my machine until the original > >>> patch is reverted or any other fix is applied. > >> > >> What GPU do you have and is it AGP? If it is AGP, does setting > >> radeon.agpmode=-1 also fix it? > >> > >> Alex > > > > That is ATI Radeon X1950, and, unfortunately, radeon.agpmode=-1 doesn't > > help, it just makes 3D acceleration in games such as OpenArena stop > > working. > > Just to confirm, is the board AGP or PCIe? > > Alex > >>> > >>> It is AGP. That's an old machine. > >> > >> Can you check whether dma_addressing_limited() is actually returning the > >> expected result at the point of radeon_ttm_init()? Disabling highmem is > >> presumably just hiding whatever problem exists, by throwing away all > >> >32-bit RAM such that use_dma32 doesn't matter. > > > > The device in question only supports a 32 bit DMA mask so > > dma_addressing_limited() should return true. Bounce buffers are not > > really usable on GPUs because they map so much memory. If > > dma_addressing_limited() returns false, that would explain it. > > Right, it appears to be the only part of the offending commit that > *could* reasonably make any difference, so I'm primarily wondering if > dma_get_required_mask() somehow gets confused. Mikhail, Can you see that dma_addressing_limited() and dma_get_required_mask() return in this case? Alex > > Thanks, > Robin.
[linux-next:master] BUILD REGRESSION 700e0cd3a5ce6a2cb90d9a2aab729b52f092a7d6
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 700e0cd3a5ce6a2cb90d9a2aab729b52f092a7d6 Add linux-next specific files for 20221130 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202211090634.ryfkk0ws-...@intel.com https://lore.kernel.org/oe-kbuild-all/20220149.0etifpy6-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211242021.fdzrfna8-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211242120.mzzvguln-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211282102.qur7hhrw-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211301622.rxgmfgtv-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211301634.cejlltjp-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211301840.y7rrob13-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211302059.viaoimsf-...@intel.com Error/Warning: (recently discovered and may have been fixed) arch/arm/mach-s3c/devs.c:32:10: fatal error: linux/platform_data/dma-s3c24xx.h: No such file or directory arch/powerpc/kernel/kvm_emul.o: warning: objtool: kvm_template_end(): can't find starting instruction arch/powerpc/kernel/optprobes_head.o: warning: objtool: optprobe_template_end(): can't find starting instruction drivers/gpu/drm/amd/amdgpu/../display/dc/irq/dcn201/irq_service_dcn201.c:40:20: warning: no previous prototype for 'to_dal_irq_source_dcn201' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gf100.c:451:1: warning: no previous prototype for 'gf100_fifo_nonstall_block' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gf100.c:451:1: warning: no previous prototype for function 'gf100_fifo_nonstall_block' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c:34:1: warning: no previous prototype for 'nvkm_engn_cgrp_get' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c:34:1: warning: no previous prototype for function 'nvkm_engn_cgrp_get' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c:210:1: warning: no previous prototype for 'tu102_gr_load' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c:210:1: warning: no previous prototype for function 'tu102_gr_load' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:49:1: warning: no previous prototype for 'wpr_generic_header_dump' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:49:1: warning: no previous prototype for function 'wpr_generic_header_dump' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c:221:21: warning: variable 'loc' set but not used [-Wunused-but-set-variable] drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:1474:38: warning: unused variable 'mt8173_jpeg_drvdata' [-Wunused-const-variable] drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:1489:38: warning: unused variable 'mtk_jpeg_drvdata' [-Wunused-const-variable] drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:1890:38: warning: unused variable 'mtk8195_jpegdec_drvdata' [-Wunused-const-variable] drivers/regulator/tps65219-regulator.c:310:60: warning: parameter 'dev' set but not used [-Wunused-but-set-parameter] drivers/regulator/tps65219-regulator.c:370:26: warning: ordered comparison of pointer with integer zero [-Wextra] include/linux/hugetlb.h:1240:33: error: 'VM_MAYSHARE' undeclared (first use in this function) include/linux/hugetlb.h:1240:47: error: 'VM_SHARED' undeclared (first use in this function); did you mean 'MNT_SHARED'? include/linux/hugetlb.h:1262:56: error: invalid use of undefined type 'struct hugetlb_vma_lock' net/netfilter/nf_conntrack_netlink.c:2674:6: warning: unused variable 'mark' [-Wunused-variable] vmlinux.o: warning: objtool: __btrfs_map_block+0x1d77: unreachable instruction Unverified Error/Warning (likely false positive, please contact us if interested): drivers/media/i2c/ov9282.c:470:3: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores] Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-irq-dcn201-irq_service_dcn201.c:warning:no-previous-prototype-for-to_dal_irq_source_dcn201 | |-- drivers-gpu-drm-nouveau-nvkm-engine-fifo-gf100.c:warning:no-previous-prototype-for-gf100_fifo_nonstall_block | |-- drivers-gpu-drm-nouveau-nvkm-engine-fifo-runl.c:warning:no-previous-prototype-for-nvkm_engn_cgrp_get | |-- drivers-gpu-drm-nouveau-nvkm-engine-gr-tu102.c:warning:no-previous-prototype-for-tu102_gr_load | |-- drivers-gpu-drm-nouveau-nvkm-nvfw-acr.c:warning:no-previous-prototype-for-wpr_generic_header_dump | `-- drivers-gpu-drm-nouveau-nvkm-subdev-acr-lsfw.c:warning:variable-loc-set-but-not-used |-- arc-allyesconfig
[linux-gfx] [PATCH] drm/i915/pvc: Implement recommended caching policy
As per the performance tuning guide, set the HOSTCACHEEN bit to
implement the recommended caching policy on PVC.
Signed-off-by: Wayne Boyer
---
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 +
drivers/gpu/drm/i915/gt/intel_workarounds.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 784152548472..f96570995cfc 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -973,6 +973,7 @@
#define GEN7_L3AGDIS (1 << 19)
#define XEHPC_LNCFMISCCFGREG0 _MMIO(0xb01c)
+#define XEHPC_HOSTCACHEENREG_BIT(1)
#define XEHPC_OVRLSCCC REG_BIT(0)
#define GEN7_L3CNTLREG2_MMIO(0xb020)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 1b0e40e68a9d..35e3f43e8b06 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2903,6 +2903,7 @@ add_render_compute_tuning_settings(struct
drm_i915_private *i915,
if (IS_PONTEVECCHIO(i915)) {
wa_write(wal, XEHPC_L3SCRUB,
SCRUB_CL_DWNGRADE_SHARED | SCRUB_RATE_4B_PER_CLK);
+ wa_masked_en(wal, XEHPC_LNCFMISCCFGREG0, XEHPC_HOSTCACHEEN);
}
if (IS_DG2(i915)) {
--
2.37.3
[PATCH] drm: Add orientation quirk for DynaBook K50
Like the ASUS T100HAN for which there is already a quirk,
the DynaBook K50 has a 800x1280 portrait screen mounted
in the tablet part of a landscape oriented 2-in-1.
Update the quirk to be more generic and apply to this device.
Signed-off-by: Allen Ballway
---
.../gpu/drm/drm_panel_orientation_quirks.c| 20 ---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index 52d8800a8ab86..14f870fb2db04 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -30,12 +30,6 @@ struct drm_dmi_panel_orientation_data {
int orientation;
};
-static const struct drm_dmi_panel_orientation_data asus_t100ha = {
- .width = 800,
- .height = 1280,
- .orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP,
-};
-
static const struct drm_dmi_panel_orientation_data gpd_micropc = {
.width = 720,
.height = 1280,
@@ -121,6 +115,12 @@ static const struct drm_dmi_panel_orientation_data
lcd1280x1920_rightside_up = {
.orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
};
+static const struct drm_dmi_panel_orientation_data lcd800x1280_leftside_up = {
+ .width = 800,
+ .height = 1280,
+ .orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP,
+};
+
static const struct drm_dmi_panel_orientation_data lcd1600x2560_leftside_up = {
.width = 1600,
.height = 2560,
@@ -151,7 +151,7 @@ static const struct dmi_system_id orientation_data[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100HAN"),
},
- .driver_data = (void *)&asus_t100ha,
+ .driver_data = (void *)&lcd800x1280_leftside_up,
}, {/* Asus T101HA */
.matches = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
@@ -196,6 +196,12 @@ static const struct dmi_system_id orientation_data[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Hi10 pro tablet"),
},
.driver_data = (void *)&lcd1200x1920_rightside_up,
+ }, {/* Dynabook K50 */
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dynabook Inc."),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "dynabook K50/FR"),
+ },
+ .driver_data = (void *)&lcd800x1280_leftside_up,
}, {/* GPD MicroPC (generic strings, also match on bios date) */
.matches = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Default string"),
--
2.38.1.584.g0f3c55d4c2-goog
Re: [PATCH 1/6] drm/i915/uc: Introduce GSC FW
On 11/29/2022 3:48 PM, Teres Alexis, Alan Previn wrote: Besides the nit below, just would like to echo the same thing Nikula said about not including the type definition in the main uc header (which i know can be a bit more work especially if we go with allocation of the structure at init time and using a ptr in the uc structure). I had a stab at this and it is quite complicated due to the inter-dependencies between the headers. Even if I split out the type to its own header, I'd have to include it in the current one for the status checker functions to work, so there would be no gain. The whole infrastructure needs to be reworked so that the headers can be fully decoupled. I have a few ideas on how to go at that and I'll try to send out a few patches on the side to get that effort going. That said, Reviewed-by: Alan Previn On Mon, 2022-11-21 at 15:16 -0800, Ceraolo Spurio, Daniele wrote: On MTL the GSC FW needs to be loaded on the media GT by the graphics @@ -246,6 +253,10 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) int i; bool found; + /* FW is not defined until all the support is in place */ Nit: perhaps a little bit more explanation would be better here for folks reading thru the codes Not sure what extra explanation I can provide here. The reason we're not defining the blob is because the support code is not fully there, there is no need to go into details of what's actually missing as that will evolve with time. I can however rephrase this if you think the current sentence is not clear, would something like this work: "GSC FW support is still not fully in place, so we're not defining the FW blob yet because we don't want the driver to attempt to load it until we're ready for it". Daniele + if (uc_fw->type == INTEL_UC_FW_TYPE_GSC) + return; +
