[PATCH 1/3] clk: keep clock rate when parent rate changes
Allow clocks to keep their rate when parent (or grandparent) rate
changes.
Signed-off-by: Frank Oltmanns
---
drivers/clk/clk.c| 48 +++-
include/linux/clk-provider.h | 2 ++
2 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c249f9791ae8..a382876c18da 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2245,6 +2245,9 @@ static struct clk_core *clk_calc_new_rates(struct
clk_core *core,
best_parent_rate != parent->rate)
top = clk_calc_new_rates(parent, best_parent_rate);
+ if ((core->flags & CLK_KEEP_RATE))
+ core->req_rate = new_rate;
+
out:
clk_calc_subtree(core, new_rate, parent, p_index);
@@ -2343,9 +2346,51 @@ static void clk_change_rate(struct clk_core *core)
clk_core_prepare_enable(parent);
trace_clk_set_rate(core, core->new_rate);
+ if (!skip_set_rate && core->ops->set_rate) {
+ if (core->flags & CLK_KEEP_RATE && clk_core_can_round(core)) {
+ struct clk_rate_request req;
+ unsigned long flags;
+ int ret;
+
+ clk_core_init_rate_req(core, , core->req_rate);
- if (!skip_set_rate && core->ops->set_rate)
+ /*
+* Re-determine the new rate for the clock based on the
requested rate.
+*
+* In this stage, the clock must not set a new parent
rate or try a
+* different parent, so temporarily prevent that from
happening.
+*/
+ flags = core->flags;
+ core->flags &= ~(CLK_SET_RATE_PARENT);
+ core->flags |= CLK_SET_RATE_NO_REPARENT;
+ ret = clk_core_determine_round_nolock(core, );
+ core->flags = flags;
+
+ /*
+* If necessary, store the new rate and propagate to
the subtree.
+*
+* The previously calculated rates (new_rate) of this
core's subtree are no
+* longer correct, because this clock will be set to a
rate that differs
+* from the rate that was used to calculate the subtree.
+*
+* FIXME: This means that the rate also differs from
the new_rate that was
+*announced in the PRE_RATE_CHANGE
notification. Be careful when
+*applying this flag, that the subtree does not
depend on the
+*correct new rate being propagated.
+*/
+ if (ret >= 0 && req.rate != core->new_rate) {
+ core->new_rate = req.rate;
+ pr_debug("%s: clk %s: keeping rate %lu as
%lu\n",
+ __func__, core->name, core->req_rate,
core->new_rate);
+
+ hlist_for_each_entry(child, >children,
child_node) {
+ child->new_rate = clk_recalc(child,
core->new_rate);
+ clk_calc_subtree(child,
child->new_rate, NULL, 0);
+ }
+ }
+ }
core->ops->set_rate(core->hw, core->new_rate, best_parent_rate);
+ }
trace_clk_set_rate_complete(core, core->new_rate);
@@ -3388,6 +3433,7 @@ static const struct {
ENTRY(CLK_IS_CRITICAL),
ENTRY(CLK_OPS_PARENT_ENABLE),
ENTRY(CLK_DUTY_CYCLE_PARENT),
+ ENTRY(CLK_KEEP_RATE),
#undef ENTRY
};
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index ec32ec58c59f..fba78a99ac56 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -32,6 +32,8 @@
#define CLK_OPS_PARENT_ENABLE BIT(12)
/* duty cycle call may be forwarded to the parent clock */
#define CLK_DUTY_CYCLE_PARENT BIT(13)
+/* try to keep rate, if parent rate changes */
+#define CLK_KEEP_RATE BIT(14)
struct clk;
struct clk_hw;
--
2.41.0
[PATCH 3/3] drm/sun4i: tcon: parent keeps TCON0 clock stable on A64
From: Icenowy Zheng
As the clk framework keeps A64's TCON0 clock stable when HDMI changes
its parent's clock, do not protect TCON0 clock on A64 in the TCON driver
to allow PLL-Video0 to get changed by HDMI.
Signed-off-by: Icenowy Zheng
Signed-off-by: Frank Oltmanns
---
drivers/gpu/drm/sun4i/sun4i_tcon.c | 15 +--
drivers/gpu/drm/sun4i/sun4i_tcon.h | 1 +
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 6a52fb12cbfb..4439e62b7a34 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -108,9 +108,11 @@ static void sun4i_tcon_channel_set_status(struct
sun4i_tcon *tcon, int channel,
if (enabled) {
clk_prepare_enable(clk);
- clk_rate_exclusive_get(clk);
+ if (!tcon->quirks->rate_kept_by_parent)
+ clk_rate_exclusive_get(clk);
} else {
- clk_rate_exclusive_put(clk);
+ if (!tcon->quirks->rate_kept_by_parent)
+ clk_rate_exclusive_put(clk);
clk_disable_unprepare(clk);
}
}
@@ -1505,6 +1507,14 @@ static const struct sun4i_tcon_quirks sun8i_a33_quirks =
{
.supports_lvds = true,
};
+static const struct sun4i_tcon_quirks sun50i_a64_lcd_quirks = {
+ .supports_lvds = true,
+ .has_channel_0 = true,
+ .rate_kept_by_parent= true,
+ .dclk_min_div = 1,
+ .setup_lvds_phy = sun6i_tcon_setup_lvds_phy,
+};
+
static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = {
.supports_lvds = true,
.has_channel_0 = true,
@@ -1563,6 +1573,7 @@ const struct of_device_id sun4i_tcon_of_table[] = {
{ .compatible = "allwinner,sun9i-a80-tcon-tv", .data =
_a80_tcon_tv_quirks },
{ .compatible = "allwinner,sun20i-d1-tcon-lcd", .data =
_d1_lcd_quirks },
{ .compatible = "allwinner,sun20i-d1-tcon-tv", .data =
_r40_tv_quirks },
+ { .compatible = "allwinner,sun50i-a64-tcon-lcd", .data =
_a64_lcd_quirks },
{ }
};
MODULE_DEVICE_TABLE(of, sun4i_tcon_of_table);
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h
b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index fa23aa23fe4a..c4ce7c29192e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -243,6 +243,7 @@ struct sun4i_tcon_quirks {
boolneeds_edp_reset; /* a80 edp reset needed for tcon0 access */
boolsupports_lvds; /* Does the TCON support an LVDS output? */
boolpolarity_in_ch0; /* some tcon1 channels have polarity bits in
tcon0 pol register */
+ boolrate_kept_by_parent; /* Does parent keep TCON0 clock stable? */
u8 dclk_min_div; /* minimum divider for TCON0 DCLK */
/* callback to handle tcon muxing options */
--
2.41.0
[PATCH 2/3] clk: sunxi-ng: a64: keep rate of pll-mipi stable across parent rate changes
Keep the clock rate of Allwinner A64's pll-mipi even when the parent
(pll-video0) changes its rate. This is required, to drive both an LCD
and HDMI display, because both have pll-video0 as an ancestor.
Signed-off-by: Frank Oltmanns
---
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 8951ffc14ff5..d22094ce1d66 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -180,7 +180,8 @@ static struct ccu_nkm pll_mipi_clk = {
.reg= 0x040,
.hw.init= CLK_HW_INIT("pll-mipi", "pll-video0",
_nkm_ops,
- CLK_SET_RATE_UNGATE |
CLK_SET_RATE_PARENT),
+ CLK_SET_RATE_UNGATE |
CLK_SET_RATE_PARENT |
+ CLK_KEEP_RATE),
.features = CCU_FEATURE_CLOSEST_RATE,
},
};
--
2.41.0
[PATCH 0/3] Make Allwinner A64's pll-mipi keep its rate when parent rate changes
I would like to make the Allwinner A64's pll-mipi to keep its rate when its parent's (pll-video0) rate changes. Keeping pll-mipi's rate is required, to let the A64 drive both an LCD and HDMI display at the same time, because both have pll-video0 as an ancestor. PATCH 1 adds this functionality as a feature into the clk framework (new flag: CLK_KEEP_RATE). Cores that use this flag, store a rate as req_rate when it or one of its descendants requests a new rate. That rate is then restored in the clk_change_rate recursion, which walks through the tree. It will reach the flagged core (e.g. pll-mipi) after the parent's rate (e.g. pll-video0) has already been set to the new rate. It will then call determine_rate (which requests the parent's current, i.e. new, rate) to determine a rate that is close to the flagged core's previous rate. Afterward it will re-calculate the rates for the flagged core's subtree. PATCH 2 & 3 demonstrate how the new flag can be used for A64's pll-mipi. By setting this flag, it is no longer required to get an exclusive lock when setting tcon0's rate, because the rate will be restored when its parent's (pll-mipi) rate is restored. This work is inspired by an out-of-tree patchset [1] [2] [3]. Unfortunately, the patchset uses clk_set_rate() in a notifier callback, which the following comment on clk_notifier_register() forbids: "The callbacks associated with the notifier must not re-enter into the clk framework by calling any top-level clk APIs." [4] Furthermore, that out-of-tree patchset no longer works with the current linux-next, because setting pll-mipi is now also resetting pll-video0 [5]. Thank you for considering this contribution, Frank [1] https://github.com/megous/linux/commit/4124e115de82797f604808aaa5caad4512a9a1ed [2] https://github.com/megous/linux/commit/edc93fd70ee759fd989664fcb85996cb48a006e6 [3] https://github.com/megous/linux/commit/40f5fc5b08b21142931662147d039ec217c9ba2f [4] https://elixir.bootlin.com/linux/v6.4.11/source/drivers/clk/clk.c#L4578 [5] https://lore.kernel.org/linux-kernel/20230807-pll-mipi_set_rate_parent-v6-0-f173239a4...@oltmanns.dev/ Signed-off-by: Frank Oltmanns --- Frank Oltmanns (2): clk: keep clock rate when parent rate changes clk: sunxi-ng: a64: keep rate of pll-mipi stable across parent rate changes Icenowy Zheng (1): drm/sun4i: tcon: parent keeps TCON0 clock stable on A64 drivers/clk/clk.c | 48 ++- drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 3 ++- drivers/gpu/drm/sun4i/sun4i_tcon.c| 15 +-- drivers/gpu/drm/sun4i/sun4i_tcon.h| 1 + include/linux/clk-provider.h | 2 ++ 5 files changed, 65 insertions(+), 4 deletions(-) --- base-commit: c539c5c0a7ccafe7169c02564cceeb50317b540b change-id: 20230824-pll-mipi_keep_rate-0a3a0d3574cf Best regards, -- Frank Oltmanns
Re: [PATCH v3] drm/bridge/analogix/anx78xx: Drop ID table
On Thu, Aug 24, 2023 at 9:26 PM Laurent Pinchart wrote: > On Thu, Aug 24, 2023 at 07:15:46PM +0100, Biju Das wrote: ... > I wonder, as the device can only be instantiated from OF, should we add > > depends on OF Generally speaking this is a bad idea. It prevents a component from being instantiated on ACPI based systems (even if there is no ACPI ID table, due to a gateway called PRP0001). > to Kconfig, and drop the > > #if IS_ENABLED(CONFIG_OF) > > from the driver ? Contrary this is an idea I fully support! -- With Best Regards, Andy Shevchenko
[git pull] drm fixes for 6.5 final
Hi Linus, A bit bigger than I'd care for, but it's mostly a single vmwgfx fix and a fix for an i915 hotplug probing. Otherwise misc i915, bridge, panfrost and dma-buf fixes. Dave. drm-fixes-2023-08-25: drm fixes for 6.5-rc8 core: - add a HPD poll helper i915: - fix regression in i915 polling - fix docs build warning - fix DG2 idle power consumption bridge: - samsung-dsim: init fix panfrost: - fix speed binning issue dma-buf: - fix recursive lock in fence signal vmwgfx: - fix shader stage validation - fix NULL ptr derefs in gem put The following changes since commit 706a741595047797872e669b3101429ab8d378ef: Linux 6.5-rc7 (2023-08-20 15:02:52 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2023-08-25 for you to fetch changes up to 59fe2029b9e05cd490eaf972053dd86f96f77869: Merge tag 'drm-intel-fixes-2023-08-24' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2023-08-25 09:12:02 +1000) drm fixes for 6.5-rc8 core: - add a HPD poll helper i915: - fix regression in i915 polling - fix docs build warning - fix DG2 idle power consumption bridge: - samsung-dsim: init fix panfrost: - fix speed binning issue dma-buf: - fix recursive lock in fence signal vmwgfx: - fix shader stage validation - fix NULL ptr derefs in gem put Ankit Nautiyal (1): drm/display/dp: Fix the DP DSC Receiver cap size Anshuman Gupta (1): drm/i915/dgfx: Enable d3cold at s2idle Dave Airlie (2): Merge tag 'drm-misc-fixes-2023-08-24' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'drm-intel-fixes-2023-08-24' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes David Michael (1): drm/panfrost: Skip speed binning on EOPNOTSUPP Frieder Schrempf (1): drm: bridge: samsung-dsim: Fix init during host transfer Imre Deak (2): drm: Add an HPD poll helper to reschedule the poll work drm/i915: Fix HPD polling, reenabling the output poll work as needed Jani Nikula (1): drm/i915: fix Sphinx indentation warning Rob Clark (1): dma-buf/sw_sync: Avoid recursive lock during fence signal Zack Rusin (2): drm/vmwgfx: Fix shader stage validation drm/vmwgfx: Fix possible invalid drm gem put calls drivers/dma-buf/sw_sync.c| 18 drivers/gpu/drm/bridge/samsung-dsim.c| 27 +++ drivers/gpu/drm/drm_probe_helper.c | 68 +++- drivers/gpu/drm/i915/display/intel_hotplug.c | 4 +- drivers/gpu/drm/i915/gt/uc/intel_huc.c | 2 + drivers/gpu/drm/i915/i915_driver.c | 33 -- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 6 +-- drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 8 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 12 + drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 35 ++ drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 +-- drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c | 3 +- drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 3 +- include/drm/display/drm_dp.h | 2 +- include/drm/drm_probe_helper.h | 1 + 16 files changed, 136 insertions(+), 94 deletions(-)
Re: [PATCH v2 4/9] drm/sched: Split free_job into own work item
On Fri, Aug 25, 2023 at 01:04:10AM +0200, Danilo Krummrich wrote:
> On Thu, Aug 10, 2023 at 07:31:32PM -0700, Matthew Brost wrote:
> > Rather than call free_job and run_job in same work item have a dedicated
> > work item for each. This aligns with the design and intended use of work
> > queues.
> >
> > Signed-off-by: Matthew Brost
> > ---
> > drivers/gpu/drm/scheduler/sched_main.c | 137 ++---
> > include/drm/gpu_scheduler.h| 8 +-
> > 2 files changed, 106 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> > b/drivers/gpu/drm/scheduler/sched_main.c
> > index cede47afc800..b67469eac179 100644
> > --- a/drivers/gpu/drm/scheduler/sched_main.c
> > +++ b/drivers/gpu/drm/scheduler/sched_main.c
> > @@ -213,11 +213,12 @@ void drm_sched_rq_remove_entity(struct drm_sched_rq
> > *rq,
> > * drm_sched_rq_select_entity_rr - Select an entity which could provide a
> > job to run
> > *
> > * @rq: scheduler run queue to check.
> > + * @dequeue: dequeue selected entity
> > *
> > * Try to find a ready entity, returns NULL if none found.
> > */
> > static struct drm_sched_entity *
> > -drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> > +drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq, bool dequeue)
> > {
> > struct drm_sched_entity *entity;
> >
> > @@ -227,8 +228,10 @@ drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> > if (entity) {
> > list_for_each_entry_continue(entity, >entities, list) {
> > if (drm_sched_entity_is_ready(entity)) {
> > - rq->current_entity = entity;
> > - reinit_completion(>entity_idle);
> > + if (dequeue) {
> > + rq->current_entity = entity;
> > + reinit_completion(>entity_idle);
> > + }
> > spin_unlock(>lock);
> > return entity;
> > }
> > @@ -238,8 +241,10 @@ drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> > list_for_each_entry(entity, >entities, list) {
> >
> > if (drm_sched_entity_is_ready(entity)) {
> > - rq->current_entity = entity;
> > - reinit_completion(>entity_idle);
> > + if (dequeue) {
> > + rq->current_entity = entity;
> > + reinit_completion(>entity_idle);
> > + }
> > spin_unlock(>lock);
> > return entity;
> > }
> > @@ -257,11 +262,12 @@ drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> > * drm_sched_rq_select_entity_fifo - Select an entity which provides a job
> > to run
> > *
> > * @rq: scheduler run queue to check.
> > + * @dequeue: dequeue selected entity
> > *
> > * Find oldest waiting ready entity, returns NULL if none found.
> > */
> > static struct drm_sched_entity *
> > -drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq)
> > +drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq, bool dequeue)
> > {
> > struct rb_node *rb;
> >
> > @@ -271,8 +277,10 @@ drm_sched_rq_select_entity_fifo(struct drm_sched_rq
> > *rq)
> >
> > entity = rb_entry(rb, struct drm_sched_entity, rb_tree_node);
> > if (drm_sched_entity_is_ready(entity)) {
> > - rq->current_entity = entity;
> > - reinit_completion(>entity_idle);
> > + if (dequeue) {
> > + rq->current_entity = entity;
> > + reinit_completion(>entity_idle);
> > + }
> > break;
> > }
> > }
> > @@ -282,13 +290,54 @@ drm_sched_rq_select_entity_fifo(struct drm_sched_rq
> > *rq)
> > }
> >
> > /**
> > - * drm_sched_submit_queue - scheduler queue submission
> > + * drm_sched_run_job_queue - queue job submission
> > * @sched: scheduler instance
> > */
> > -static void drm_sched_submit_queue(struct drm_gpu_scheduler *sched)
> > +static void drm_sched_run_job_queue(struct drm_gpu_scheduler *sched)
> > {
> > if (!READ_ONCE(sched->pause_submit))
> > - queue_work(sched->submit_wq, >work_submit);
> > + queue_work(sched->submit_wq, >work_run_job);
> > +}
> > +
> > +static struct drm_sched_entity *
> > +drm_sched_select_entity(struct drm_gpu_scheduler *sched, bool dequeue);
> > +
> > +/**
> > + * drm_sched_run_job_queue_if_ready - queue job submission if ready
> > + * @sched: scheduler instance
> > + */
> > +static void drm_sched_run_job_queue_if_ready(struct drm_gpu_scheduler
> > *sched)
> > +{
> > + if (drm_sched_select_entity(sched, false))
> > + drm_sched_run_job_queue(sched);
> > +}
> > +
> > +/**
> > + * drm_sched_free_job_queue - queue free job
> > + *
> > + * @sched: scheduler instance to
[PATCH v7 2/3] drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb()
In some cases the bridge drivers would like to receive hotplug events even in the case new status is equal to the old status. In the DP case this is used to deliver "attention" messages to the DP host. Stop filtering the events in the drm_bridge_connector_hpd_cb() and let drivers decide whether they would like to receive the event or not. Reviewed-By: Janne Grunau Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_bridge_connector.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 1da93d5a1f61..10b52224db37 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -113,16 +113,11 @@ static void drm_bridge_connector_hpd_cb(void *cb_data, struct drm_bridge_connector *drm_bridge_connector = cb_data; struct drm_connector *connector = _bridge_connector->base; struct drm_device *dev = connector->dev; - enum drm_connector_status old_status; mutex_lock(>mode_config.mutex); - old_status = connector->status; connector->status = status; mutex_unlock(>mode_config.mutex); - if (old_status == status) - return; - drm_bridge_connector_hpd_notify(connector, status); drm_kms_helper_connector_hotplug_event(connector); -- 2.39.2
[PATCH v7 3/3] drm/bridge_connector: implement oob_hotplug_event
Implement the oob_hotplug_event() callback. Translate it to the HPD
notification sent to the HPD bridge in the chain.
Reviewed-by: Janne Grunau
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_bridge_connector.c | 29 +++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index 10b52224db37..3aa129b3f8e9 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -5,6 +5,8 @@
#include
#include
+#include
+#include
#include
#include
@@ -107,10 +109,9 @@ static void drm_bridge_connector_hpd_notify(struct
drm_connector *connector,
}
}
-static void drm_bridge_connector_hpd_cb(void *cb_data,
- enum drm_connector_status status)
+static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector
*drm_bridge_connector,
+ enum drm_connector_status status)
{
- struct drm_bridge_connector *drm_bridge_connector = cb_data;
struct drm_connector *connector = _bridge_connector->base;
struct drm_device *dev = connector->dev;
@@ -123,6 +124,21 @@ static void drm_bridge_connector_hpd_cb(void *cb_data,
drm_kms_helper_connector_hotplug_event(connector);
}
+static void drm_bridge_connector_hpd_cb(void *cb_data,
+ enum drm_connector_status status)
+{
+ drm_bridge_connector_handle_hpd(cb_data, status);
+}
+
+static void drm_bridge_connector_oob_hotplug_event(struct drm_connector
*connector,
+ enum drm_connector_status
status)
+{
+ struct drm_bridge_connector *bridge_connector =
+ to_drm_bridge_connector(connector);
+
+ drm_bridge_connector_handle_hpd(bridge_connector, status);
+}
+
static void drm_bridge_connector_enable_hpd(struct drm_connector *connector)
{
struct drm_bridge_connector *bridge_connector =
@@ -216,6 +232,7 @@ static const struct drm_connector_funcs
drm_bridge_connector_funcs = {
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
.debugfs_init = drm_bridge_connector_debugfs_init,
+ .oob_hotplug_event = drm_bridge_connector_oob_hotplug_event,
};
/*
-
@@ -352,6 +369,12 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
if (!drm_bridge_get_next_bridge(bridge))
connector_type = bridge->type;
+#ifdef CONFIG_OF
+ if (!drm_bridge_get_next_bridge(bridge) &&
+ bridge->of_node)
+ connector->fwnode =
fwnode_handle_get(of_fwnode_handle(bridge->of_node));
+#endif
+
if (bridge->ddc)
ddc = bridge->ddc;
--
2.39.2
[PATCH v7 1/3] drm: Add HPD state to drm_connector_oob_hotplug_event()
From: Bjorn Andersson
In some implementations, such as the Qualcomm platforms, the display
driver has no way to query the current HPD state and as such it's
impossible to distinguish between disconnect and attention events.
Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD
state.
Also push the test for unchanged state in the displayport altmode driver
into the i915 driver, to allow other drivers to act upon each update.
Signed-off-by: Bjorn Andersson
Reviewed-by: Hans de Goede
Acked-by: Heikki Krogerus
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_connector.c | 6 --
.../gpu/drm/i915/display/intel_display_core.h | 3 +++
drivers/gpu/drm/i915/display/intel_dp.c | 17 ++---
drivers/usb/typec/altmodes/displayport.c| 17 +
include/drm/drm_connector.h | 6 --
5 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index bf8371dc2a61..a3d3e7dc08b2 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -3049,6 +3049,7 @@ struct drm_connector *drm_connector_find_by_fwnode(struct
fwnode_handle *fwnode)
/**
* drm_connector_oob_hotplug_event - Report out-of-band hotplug event to
connector
* @connector_fwnode: fwnode_handle to report the event on
+ * @status: hot plug detect logical state
*
* On some hardware a hotplug event notification may come from outside the
display
* driver / device. An example of this is some USB Type-C setups where the
hardware
@@ -3058,7 +3059,8 @@ struct drm_connector *drm_connector_find_by_fwnode(struct
fwnode_handle *fwnode)
* This function can be used to report these out-of-band events after obtaining
* a drm_connector reference through calling drm_connector_find_by_fwnode().
*/
-void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
+void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
+enum drm_connector_status status)
{
struct drm_connector *connector;
@@ -3067,7 +3069,7 @@ void drm_connector_oob_hotplug_event(struct fwnode_handle
*connector_fwnode)
return;
if (connector->funcs->oob_hotplug_event)
- connector->funcs->oob_hotplug_event(connector);
+ connector->funcs->oob_hotplug_event(connector, status);
drm_connector_put(connector);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h
b/drivers/gpu/drm/i915/display/intel_display_core.h
index 53e5c33e08c3..ccfe27630fb6 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -175,6 +175,9 @@ struct intel_hotplug {
/* Whether or not to count short HPD IRQs in HPD storms */
u8 hpd_short_storm_enabled;
+ /* Last state reported by oob_hotplug_event for each encoder */
+ unsigned long oob_hotplug_last_state;
+
/*
* if we get a HPD irq from DP and a HPD irq from non-DP
* the non-DP HPD could block the workqueue on a mode config
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 12bd2f322e62..2af815b5c22b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5253,15 +5253,26 @@ static int intel_dp_connector_atomic_check(struct
drm_connector *conn,
return intel_modeset_synced_crtcs(state, conn);
}
-static void intel_dp_oob_hotplug_event(struct drm_connector *connector)
+static void intel_dp_oob_hotplug_event(struct drm_connector *connector,
+ enum drm_connector_status hpd_state)
{
struct intel_encoder *encoder =
intel_attached_encoder(to_intel_connector(connector));
struct drm_i915_private *i915 = to_i915(connector->dev);
+ bool hpd_high = hpd_state == connector_status_connected;
+ unsigned int hpd_pin = encoder->hpd_pin;
+ bool need_work = false;
spin_lock_irq(>irq_lock);
- i915->display.hotplug.event_bits |= BIT(encoder->hpd_pin);
+ if (hpd_high != test_bit(hpd_pin,
>display.hotplug.oob_hotplug_last_state)) {
+ i915->display.hotplug.event_bits |= BIT(hpd_pin);
+
+ __assign_bit(hpd_pin,
>display.hotplug.oob_hotplug_last_state, hpd_high);
+ need_work = true;
+ }
spin_unlock_irq(>irq_lock);
- queue_delayed_work(i915->unordered_wq,
>display.hotplug.hotplug_work, 0);
+
+ if (need_work)
+ queue_delayed_work(i915->unordered_wq,
>display.hotplug.hotplug_work, 0);
}
static const struct drm_connector_funcs intel_dp_connector_funcs = {
diff --git a/drivers/usb/typec/altmodes/displayport.c
b/drivers/usb/typec/altmodes/displayport.c
index 426c88a516e5..29f40e658772 100644
--- a/drivers/usb/typec/altmodes/displayport.c
[PATCH v7 0/3] drm/bridge_connector: implement OOB HPD handling
Note, numbering for this series starts from v5, since there were several revisions for this patchset under a different series title ([1]). USB altmodes code would send OOB notifications to the drm_connector specified in the device tree. However as the MSM DP driver uses drm_bridge_connector, there is no way to receive these event directly. Implement a bridge between oob_hotplug_event and drm_bridge's hpd_notify. Merge strategy: since this series touches i915 code, it might make sense to merge all three patches through drm-intel. [1] https://patchwork.freedesktop.org/series/103449/ Changes since v6: - Rebased on top of linux-next. Fixed the freshly added new drm_connector_oob_hotplug_event() call. Changes since v5: - Fixed checkpatch warning in the first patch (noted by intel-gfx CI). Changes since v4: - Picked up the patchset - Dropped msm-specific patches - Changed drm_bridge_connector_oob_hotplug_event to call connector's HPD callback directly, rather than going through the last bridge's hpd_notify - Added proper fwnode for the drm_bridge_connector Bjorn Andersson (1): drm: Add HPD state to drm_connector_oob_hotplug_event() Dmitry Baryshkov (2): drm/bridge_connector: stop filtering events in drm_bridge_connector_hpd_cb() drm/bridge_connector: implement oob_hotplug_event drivers/gpu/drm/drm_bridge_connector.c| 34 ++- drivers/gpu/drm/drm_connector.c | 6 ++-- .../gpu/drm/i915/display/intel_display_core.h | 3 ++ drivers/gpu/drm/i915/display/intel_dp.c | 17 -- drivers/usb/typec/altmodes/displayport.c | 17 +- include/drm/drm_connector.h | 6 ++-- 6 files changed, 60 insertions(+), 23 deletions(-) -- 2.39.2
Re: [PATCH v2 4/9] drm/sched: Split free_job into own work item
On Thu, Aug 10, 2023 at 07:31:32PM -0700, Matthew Brost wrote:
> Rather than call free_job and run_job in same work item have a dedicated
> work item for each. This aligns with the design and intended use of work
> queues.
>
> Signed-off-by: Matthew Brost
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 137 ++---
> include/drm/gpu_scheduler.h| 8 +-
> 2 files changed, 106 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index cede47afc800..b67469eac179 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -213,11 +213,12 @@ void drm_sched_rq_remove_entity(struct drm_sched_rq *rq,
> * drm_sched_rq_select_entity_rr - Select an entity which could provide a
> job to run
> *
> * @rq: scheduler run queue to check.
> + * @dequeue: dequeue selected entity
> *
> * Try to find a ready entity, returns NULL if none found.
> */
> static struct drm_sched_entity *
> -drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> +drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq, bool dequeue)
> {
> struct drm_sched_entity *entity;
>
> @@ -227,8 +228,10 @@ drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> if (entity) {
> list_for_each_entry_continue(entity, >entities, list) {
> if (drm_sched_entity_is_ready(entity)) {
> - rq->current_entity = entity;
> - reinit_completion(>entity_idle);
> + if (dequeue) {
> + rq->current_entity = entity;
> + reinit_completion(>entity_idle);
> + }
> spin_unlock(>lock);
> return entity;
> }
> @@ -238,8 +241,10 @@ drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> list_for_each_entry(entity, >entities, list) {
>
> if (drm_sched_entity_is_ready(entity)) {
> - rq->current_entity = entity;
> - reinit_completion(>entity_idle);
> + if (dequeue) {
> + rq->current_entity = entity;
> + reinit_completion(>entity_idle);
> + }
> spin_unlock(>lock);
> return entity;
> }
> @@ -257,11 +262,12 @@ drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> * drm_sched_rq_select_entity_fifo - Select an entity which provides a job
> to run
> *
> * @rq: scheduler run queue to check.
> + * @dequeue: dequeue selected entity
> *
> * Find oldest waiting ready entity, returns NULL if none found.
> */
> static struct drm_sched_entity *
> -drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq)
> +drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq, bool dequeue)
> {
> struct rb_node *rb;
>
> @@ -271,8 +277,10 @@ drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq)
>
> entity = rb_entry(rb, struct drm_sched_entity, rb_tree_node);
> if (drm_sched_entity_is_ready(entity)) {
> - rq->current_entity = entity;
> - reinit_completion(>entity_idle);
> + if (dequeue) {
> + rq->current_entity = entity;
> + reinit_completion(>entity_idle);
> + }
> break;
> }
> }
> @@ -282,13 +290,54 @@ drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq)
> }
>
> /**
> - * drm_sched_submit_queue - scheduler queue submission
> + * drm_sched_run_job_queue - queue job submission
> * @sched: scheduler instance
> */
> -static void drm_sched_submit_queue(struct drm_gpu_scheduler *sched)
> +static void drm_sched_run_job_queue(struct drm_gpu_scheduler *sched)
> {
> if (!READ_ONCE(sched->pause_submit))
> - queue_work(sched->submit_wq, >work_submit);
> + queue_work(sched->submit_wq, >work_run_job);
> +}
> +
> +static struct drm_sched_entity *
> +drm_sched_select_entity(struct drm_gpu_scheduler *sched, bool dequeue);
> +
> +/**
> + * drm_sched_run_job_queue_if_ready - queue job submission if ready
> + * @sched: scheduler instance
> + */
> +static void drm_sched_run_job_queue_if_ready(struct drm_gpu_scheduler *sched)
> +{
> + if (drm_sched_select_entity(sched, false))
> + drm_sched_run_job_queue(sched);
> +}
> +
> +/**
> + * drm_sched_free_job_queue - queue free job
> + *
> + * @sched: scheduler instance to queue free job
> + */
> +static void drm_sched_free_job_queue(struct drm_gpu_scheduler *sched)
> +{
> + if (!READ_ONCE(sched->pause_submit))
> + queue_work(sched->submit_wq, >work_free_job);
> +}
> +
> +/**
> + *
Re: [PATCH v3] drm/bridge/analogix/anx78xx: Drop ID table
On Thu, Aug 24, 2023 at 01:51:59PM -0700, Doug Anderson wrote:
> On Thu, Aug 24, 2023 at 11:26 AM Laurent Pinchart wrote:
> > On Thu, Aug 24, 2023 at 07:15:46PM +0100, Biju Das wrote:
> > > The driver has an ID table, but it uses the wrong API for retrieving match
> > > data and that will lead to a crash, if it is instantiated by user space or
> > > using ID. From this, there is no user for the ID table and let's drop it
> > > from the driver as it saves some memory.
> > >
> > > Signed-off-by: Biju Das
> >
> > Reviewed-by: Laurent Pinchart
> >
> > I wonder, as the device can only be instantiated from OF, should we add
> >
> > depends on OF
> >
> > to Kconfig, and drop the
> >
> > #if IS_ENABLED(CONFIG_OF)
> >
> > from the driver ?
>
> In my opinion we shouldn't add the "depends on OF" since that will
> decrease the amount of compile testing. It's somewhat the opposite of
> adding "if COMPILE_TEST" to your driver. ;-)
We could add a || COMPILE_TEST :-)
> I think we could get rid of one of the "#if" statements in the driver
> anyway as of commit c9e358dfc4a8 ("driver-core: remove conditionals
> around devicetree pointers") from ~12 years ago. If we did something
> similar in "struct drm_bridge" we could drop both #ifs.
I'd be fine with that too.
--
Regards,
Laurent Pinchart
[PATCH] accel/ivpu: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! Also remove extraneous if-statement as it can never be entered. The return value from `strncpy` is it's first argument. In this case, `...dyndbg_cmd` is an array: | char dyndbg_cmd[VPU_DYNDBG_CMD_MAX_LEN]; ^^ This can never be NULL which means `strncpy`'s return value cannot be NULL here. Just use `strscpy` which is more robust and results in simpler and less ambiguous code. Moreover, remove needless `... - 1` as `strscpy`'s implementation ensures NUL-termination and we do not need to carefully dance around ending boundaries with a "- 1" anymore. Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/accel/ivpu/ivpu_jsm_msg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/accel/ivpu/ivpu_jsm_msg.c b/drivers/accel/ivpu/ivpu_jsm_msg.c index 831bfd2b2d39..bdddef2c59ee 100644 --- a/drivers/accel/ivpu/ivpu_jsm_msg.c +++ b/drivers/accel/ivpu/ivpu_jsm_msg.c @@ -118,8 +118,7 @@ int ivpu_jsm_dyndbg_control(struct ivpu_device *vdev, char *command, size_t size struct vpu_jsm_msg resp; int ret; - if (!strncpy(req.payload.dyndbg_control.dyndbg_cmd, command, VPU_DYNDBG_CMD_MAX_LEN - 1)) - return -ENOMEM; + strscpy(req.payload.dyndbg_control.dyndbg_cmd, command, VPU_DYNDBG_CMD_MAX_LEN); ret = ivpu_ipc_send_receive(vdev, , VPU_JSM_MSG_DYNDBG_CONTROL_RSP, , VPU_IPC_CHAN_ASYNC_CMD, vdev->timeout.jsm); --- base-commit: f9604036a3fb6149badf346994b46b03f9292db7 change-id: 20230824-strncpy-drivers-accel-ivpu-ivpu_jsm_msg-c-2422f7f00fc8 Best regards, -- Justin Stitt
[PATCH v4 5/6] drm/i915/dp_link_training: Set all downstream MST ports to BAD before retrying
Before sending a uevent to userspace in order to trigger a corrective
modeset, we change the failing connector's link-status to BAD. However,
the downstream MST branch ports are left in their original GOOD state.
This patch utilizes the drm helper function
drm_dp_set_mst_topology_link_status() to rectify this and set all
downstream MST connectors' link-status to BAD before emitting the uevent
to userspace.
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/i915/display/intel_dp.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 42353b1ac487..e8b10f59e141 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5995,16 +5995,20 @@ static void intel_dp_modeset_retry_work_fn(struct
work_struct *work)
struct intel_dp *intel_dp =
container_of(work, typeof(*intel_dp), modeset_retry_work);
struct drm_connector *connector = _dp->attached_connector->base;
- drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s]\n", connector->base.id,
- connector->name);
- /* Grab the locks before changing connector property*/
- mutex_lock(>dev->mode_config.mutex);
- /* Set connector link status to BAD and send a Uevent to notify
-* userspace to do a modeset.
+ /* Set the connector's (and possibly all its downstream MST ports') link
+* status to BAD.
*/
+ mutex_lock(>dev->mode_config.mutex);
+ drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] link status %d -> %d\n",
+ connector->base.id, connector->name,
+ connector->state->link_status, DRM_MODE_LINK_STATUS_BAD);
drm_connector_set_link_status_property(connector,
DRM_MODE_LINK_STATUS_BAD);
+ if (intel_dp->is_mst) {
+ drm_dp_set_mst_topology_link_status(_dp->mst_mgr,
+ DRM_MODE_LINK_STATUS_BAD);
+ }
mutex_unlock(>dev->mode_config.mutex);
/* Send Hotplug uevent so userspace can reprobe */
drm_kms_helper_connector_hotplug_event(connector);
--
Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
[PATCH v4 6/6] drm/i915/dp_link_training: Emit a link-status=Bad uevent with trigger property
When a link-training attempt fails, emit a uevent to user space that includes the trigger property, which in this case will be link-statue=Bad. This will allow userspace to parse the uevent property and better understand the reason for the previous modeset failure. Signed-off-by: Gil Dekel V2: - init link_status_property inline. --- drivers/gpu/drm/i915/display/intel_dp.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index e8b10f59e141..328e9f030033 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -42,6 +42,7 @@ #include #include #include +#include #include "g4x_dp.h" #include "i915_drv.h" @@ -5995,6 +5996,8 @@ static void intel_dp_modeset_retry_work_fn(struct work_struct *work) struct intel_dp *intel_dp = container_of(work, typeof(*intel_dp), modeset_retry_work); struct drm_connector *connector = _dp->attached_connector->base; + struct drm_property *link_status_property = + connector->dev->mode_config.link_status_property; /* Set the connector's (and possibly all its downstream MST ports') link * status to BAD. @@ -6011,7 +6014,7 @@ static void intel_dp_modeset_retry_work_fn(struct work_struct *work) } mutex_unlock(>dev->mode_config.mutex); /* Send Hotplug uevent so userspace can reprobe */ - drm_kms_helper_connector_hotplug_event(connector); + drm_sysfs_connector_property_event(connector, link_status_property); } bool -- Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
[PATCH v4 4/6] drm/i915: Move DP modeset_retry_work into intel_dp
Currently, link-training fallback is only implemented for SST, so having
modeset_retry_work in intel_connector makes sense. However, we hope to
implement link training fallback for MST in a follow-up patchset, so
moving modeset_retry_work to indel_dp will make handling both SST and
MST connectors simpler. This patch does exactly that, and updates all
modeset_retry_work dependencies to use an intel_dp instead.
Credit: this patch is a rebase of Lyude Pual's original patch:
https://patchwork.freedesktop.org/patch/216627/?series=41576=3
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/i915/display/intel_display.c | 14 +++---
drivers/gpu/drm/i915/display/intel_display_types.h | 6 +++---
drivers/gpu/drm/i915/display/intel_dp.c| 11 ---
.../gpu/drm/i915/display/intel_dp_link_training.c | 3 +--
4 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index db3c26e013e3..2ec75aa0b4ee 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7962,20 +7962,28 @@ void i830_disable_pipe(struct drm_i915_private
*dev_priv, enum pipe pipe)
void intel_hpd_poll_fini(struct drm_i915_private *i915)
{
- struct intel_connector *connector;
struct drm_connector_list_iter conn_iter;
+ struct intel_connector *connector;
+ struct intel_dp *intel_dp;
+ struct intel_encoder *encoder;
/* Kill all the work that may have been queued by hpd. */
drm_connector_list_iter_begin(>drm, _iter);
for_each_intel_connector_iter(connector, _iter) {
- if (connector->modeset_retry_work.func)
- cancel_work_sync(>modeset_retry_work);
if (connector->hdcp.shim) {
cancel_delayed_work_sync(>hdcp.check_work);
cancel_work_sync(>hdcp.prop_work);
}
}
drm_connector_list_iter_end(_iter);
+
+ for_each_intel_dp(>drm, encoder) {
+ if (encoder->type == DRM_MODE_CONNECTOR_eDP ||
+ encoder->type == DRM_MODE_CONNECTOR_DisplayPort) {
+ intel_dp = enc_to_intel_dp(encoder);
+ cancel_work_sync(_dp->modeset_retry_work);
+ }
+ }
}
bool intel_scanout_needs_vtd_wa(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 731f2ec04d5c..b92bb69a3fe4 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -620,9 +620,6 @@ struct intel_connector {
struct intel_dp *mst_port;
- /* Work struct to schedule a uevent on link train failure */
- struct work_struct modeset_retry_work;
-
struct intel_hdcp hdcp;
};
@@ -1779,6 +1776,9 @@ struct intel_dp {
/* Displayport compliance testing */
struct intel_dp_compliance compliance;
+ /* Work struct to schedule a uevent on link train failure */
+ struct work_struct modeset_retry_work;
+
/* Downstream facing port caps */
struct {
int min_tmds_clock, max_tmds_clock;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 01b180c8d9bd..42353b1ac487 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5992,12 +5992,9 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
{
- struct intel_connector *intel_connector;
- struct drm_connector *connector;
-
- intel_connector = container_of(work, typeof(*intel_connector),
- modeset_retry_work);
- connector = _connector->base;
+ struct intel_dp *intel_dp =
+ container_of(work, typeof(*intel_dp), modeset_retry_work);
+ struct drm_connector *connector = _dp->attached_connector->base;
drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s]\n", connector->base.id,
connector->name);
@@ -6027,7 +6024,7 @@ intel_dp_init_connector(struct intel_digital_port
*dig_port,
int type;
/* Initialize the work for modeset in case of link train failure */
- INIT_WORK(_connector->modeset_retry_work,
+ INIT_WORK(_dp->modeset_retry_work,
intel_dp_modeset_retry_work_fn);
if (drm_WARN(dev, dig_port->max_lanes < 1,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 31d0d7854003..87d13cd03ef5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1063,7 +1063,6 @@ intel_dp_link_train_phy(struct intel_dp
[PATCH v4 3/6] drm/dp_mst: Add drm_dp_set_mst_topology_link_status()
Unlike SST, MST can support multiple displays connected to a single
connector. However, this also means that if the DisplayPort link to the
top-level MST branch device becomes unstable, then every single branch
device has an unstable link.
Since there are multiple downstream ports per connector, setting the
link status of the parent mstb's port to BAD is not enough. All of the
downstream mstb ports must also have their link status set to BAD.
This aligns to how the DP link status logic in DRM works. We notify
userspace that all of the mstb ports need retraining and apply new lower
bandwidth constraints to all future atomic commits on the topology that
follow.
Since any driver supporting MST needs to figure out which connectors
live downstream on an MST topology and update their link status in order
to retrain MST links properly, we add the
drm_dp_set_mst_topology_link_status() helper. This helper simply marks
the link status of all connectors living in that topology as bad. We
will make use of this helper in i915 later in this series.
Credit: this patch is a refactor of Lyude Pual's original patch:
https://patchwork.kernel.org/project/dri-devel/patch/20180308232421.14049-5-ly...@redhat.com/
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 +++
include/drm/display/drm_dp_mst_helper.h | 3 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index ed96cfcfa304..17cbadfb6ccb 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3566,6 +3566,45 @@ int drm_dp_get_vc_payload_bw(const struct
drm_dp_mst_topology_mgr *mgr,
}
EXPORT_SYMBOL(drm_dp_get_vc_payload_bw);
+/**
+ * drm_dp_set_mst_topology_link_status() - set all downstream MST ports' link
status
+ * @mgr: MST topology manager to set state for
+ * @status: The new status to set the MST topology to
+ *
+ * Set all downstream ports' link-status within the topology to the given
status.
+ */
+void drm_dp_set_mst_topology_link_status(struct drm_dp_mst_topology_mgr *mgr,
+enum drm_link_status status)
+{
+ struct drm_dp_mst_port *port;
+ struct drm_dp_mst_branch *rmstb;
+ struct drm_dp_mst_branch *mstb =
+ drm_dp_mst_topology_get_mstb_validated(mgr, mgr->mst_primary);
+
+ list_for_each_entry_reverse(port, >ports, next) {
+ struct drm_connector *connector = port->connector;
+
+ if (connector) {
+ mutex_lock(>dev->mode_config.mutex);
+ drm_dbg_kms(
+ connector->dev,
+ "[MST-CONNECTOR:%d:%s] link status %d -> %d\n",
+ connector->base.id, connector->name,
+ connector->state->link_status, status);
+ connector->state->link_status = status;
+ mutex_unlock(>dev->mode_config.mutex);
+ }
+
+ rmstb = drm_dp_mst_topology_get_mstb_validated(mstb->mgr,
+ port->mstb);
+ if (rmstb) {
+ drm_dp_set_mst_topology_link_status(rmstb->mgr, status);
+ drm_dp_mst_topology_put_mstb(rmstb);
+ }
+ }
+}
+EXPORT_SYMBOL(drm_dp_set_mst_topology_link_status);
+
/**
* drm_dp_read_mst_cap() - check whether or not a sink supports MST
* @aux: The DP AUX channel to use
diff --git a/include/drm/display/drm_dp_mst_helper.h
b/include/drm/display/drm_dp_mst_helper.h
index ed5c9660563c..855d488bf364 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -832,6 +832,9 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector
*connector,
int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr,
int link_rate, int link_lane_count);
+void drm_dp_set_mst_topology_link_status(struct drm_dp_mst_topology_mgr *mgr,
+enum drm_link_status status);
+
int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
void drm_dp_mst_update_slots(struct drm_dp_mst_topology_state *mst_state,
uint8_t link_encoding_cap);
--
Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
[PATCH v4 2/6] drm/i915/dp_link_training: Add a final failing state to link training fallback for MST
Currently, MST link training has no fallback. This means that if an MST
base connector fails to link-train once, the training completely fails,
which makes this case significantly more common than a complete SST link
training failure.
Similar to the final failure state of SST, this patch zeros out both
max_link_rate and max_link_lane_count. In addition, it stops resetting
MST params so the zeroing of the HBR fields stick. This ensures that
the MST base connector's modes will be completely pruned, since it is
effectively left with 0Gbps bandwidth.
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/i915/display/intel_dp.c | 27 ++-
drivers/gpu/drm/i915/display/intel_dp.h | 2 +-
.../drm/i915/display/intel_dp_link_training.c | 8 +++---
3 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 2152ddbab557..01b180c8d9bd 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -630,7 +630,7 @@ static bool intel_dp_can_link_train_fallback_for_edp(struct
intel_dp *intel_dp,
return true;
}
-int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
+void intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
int link_rate, u8 lane_count)
{
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
@@ -638,18 +638,23 @@ int intel_dp_get_link_train_fallback_values(struct
intel_dp *intel_dp,
/*
* TODO: Enable fallback on MST links once MST link compute can handle
-* the fallback params.
+* the fallback params. For now, similar to the SST case, ensure all of
+* the base connector's modes are pruned in the next connector probe by
+* effectively reducing its bandwidth to 0 so userspace can ignore it
+* within the next modeset attempt.
*/
if (intel_dp->is_mst) {
drm_err(>drm, "Link Training Unsuccessful\n");
- return -1;
+ intel_dp->max_link_rate = 0;
+ intel_dp->max_link_lane_count = 0;
+ return;
}
if (intel_dp_is_edp(intel_dp) && !intel_dp->use_max_params) {
drm_dbg_kms(>drm,
"Retrying Link training for eDP with max
parameters\n");
intel_dp->use_max_params = true;
- return 0;
+ return;
}
index = intel_dp_rate_index(intel_dp->common_rates,
@@ -662,7 +667,7 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp
*intel_dp,
lane_count)) {
drm_dbg_kms(>drm,
"Retrying Link training for eDP with same
parameters\n");
- return 0;
+ return;
}
intel_dp->max_link_rate = intel_dp_common_rate(intel_dp, index
- 1);
intel_dp->max_link_lane_count = lane_count;
@@ -673,7 +678,7 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp
*intel_dp,
lane_count >> 1))
{
drm_dbg_kms(>drm,
"Retrying Link training for eDP with same
parameters\n");
- return 0;
+ return;
}
intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
intel_dp->max_link_lane_count = lane_count >> 1;
@@ -686,10 +691,7 @@ int intel_dp_get_link_train_fallback_values(struct
intel_dp *intel_dp,
*/
intel_dp->max_link_rate = 0;
intel_dp->max_link_lane_count = 0;
- return 0;
}
-
- return 0;
}
u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
@@ -5310,10 +5312,11 @@ intel_dp_detect(struct drm_connector *connector,
intel_dp_configure_mst(intel_dp);
/*
-* TODO: Reset link params when switching to MST mode, until MST
-* supports link training fallback params.
+* Note: Even though MST link training fallback is not yet implemented,
+* do not reset. This is because the base connector needs to have all
+* its modes pruned when link training for the MST port fails.
*/
- if (intel_dp->reset_link_params || intel_dp->is_mst) {
+ if (intel_dp->reset_link_params) {
intel_dp_reset_max_link_params(intel_dp);
intel_dp->reset_link_params = false;
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
b/drivers/gpu/drm/i915/display/intel_dp.h
index 788a577ebe16..7388510e0cb2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -40,7 +40,7 @@ bool intel_dp_init_connector(struct intel_digital_port
[PATCH v4 1/6] drm/i915/dp_link_training: Add a final failing state to link training fallback
Instead of silently giving up when all link-training fallback values are
exhausted, this patch modifies the fallback's failure branch to reduces
both max_link_lane_count and max_link_rate to zero (0) and continues to
emit uevents until userspace stops attempting to modeset.
By doing so, we ensure the failing connector, which is in
link-status=Bad, has all its modes pruned (due to effectively having a
bandwidth of 0Gbps).
It is then the userspace's responsibility to ignore connectors with no
modes, even if they are marked as connected.
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/i915/display/intel_dp.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 7067ee3a4bd3..2152ddbab557 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -276,8 +276,12 @@ static int intel_dp_common_len_rate_limit(const struct
intel_dp *intel_dp,
static int intel_dp_common_rate(struct intel_dp *intel_dp, int index)
{
+ /* This occurs when max link rate drops to 0 via link training
fallback*/
+ if (index < 0)
+ return 0;
+
if (drm_WARN_ON(_to_i915(intel_dp)->drm,
- index < 0 || index >= intel_dp->num_common_rates))
+ index >= intel_dp->num_common_rates))
return 162000;
return intel_dp->common_rates[index];
@@ -318,6 +322,9 @@ static int intel_dp_max_common_lane_count(struct intel_dp
*intel_dp)
int intel_dp_max_lane_count(struct intel_dp *intel_dp)
{
switch (intel_dp->max_link_lane_count) {
+ /* This occurs when max link lane count drops to 0 via link training
fallback*/
+ case 0:
+ return 0;
case 1:
case 2:
case 4:
@@ -672,7 +679,14 @@ int intel_dp_get_link_train_fallback_values(struct
intel_dp *intel_dp,
intel_dp->max_link_lane_count = lane_count >> 1;
} else {
drm_err(>drm, "Link Training Unsuccessful\n");
- return -1;
+ /*
+* Ensure all of the connector's modes are pruned in the next
+* probe by effectively reducing its bandwidth to 0 so userspace
+* can ignore it within the next modeset attempt.
+*/
+ intel_dp->max_link_rate = 0;
+ intel_dp->max_link_lane_count = 0;
+ return 0;
}
return 0;
--
Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
[PATCH v4 0/6] drm/i915/dp_link_training: Define a final failure state when link training fails
Next version of https://patchwork.freedesktop.org/series/122850/ v4: Another blunder. I uploaded the patches from my ChromeiumOS kernel dev repo instead of drm-tip/drm-tip. Apologies for the noise :( v3: Still learning the ropes of upstream workflow. Apologies for mucking up v2. This is just a re-upload. v2: Reorganize into: 1) Add for final failure state for SST and MST link training fallback. 2) Add a DRM helper for setting downstream MST ports' link-status state. 3) Make handling SST and MST connectors simpler via intel_dp. 4) Update link-status for downstream MST ports. 5) Emit a uevent with the "link-status" trigger property. v1: Currently, when link training fails after all fallback values have been exhausted, the i915 driver seizes to send uevents to userspace. This leave userspace thinking that the last passing atomic commit was successful, and that all connectors (displays) are connected and operational, when in fact, the last link failed to train and the displays remain dark. This manifests as "zombie" displays in userspace, in which users observe the displays appear in their display settings page, but they are dark and unresponsive. Since, at the time of writing, MST link training fallback is not implemented, failing MST link training is a significantly more common case then a complete SST link training failure. And with users using MST hubs more than ever to connect multiple displays via their USB-C ports we observe this case often. This patchset series suggest a solution, in which a final failure state is defined. In this final state, the connector's bit rate capabilities, namely max_link_rate and max_link_lane_count, are set to 0. This effectively set the connector's bandwidth to 0Gbps, thus causing all its modes to be pruned in the following connector probing. Next, with this state defined, we emit a link-status=Bad uevent. The next time userspace probes the connector, it should recognize that the connector has no modes and ignore it since it is in a bad state. I am aware that always sending a uevent and never stopping may result in some userspaces having their expectations broken and enter an infinite loop of modesets and link-training attempts. However, per DRM link-status spec: ``` * link-status: * Connector link-status property to indicate the status of link. The * default value of link-status is "GOOD". If something fails during or * after modeset, the kernel driver may set this to "BAD" and issue a * hotplug uevent. Drivers should update this value using * drm_connector_set_link_status_property(). * * When user-space receives the hotplug uevent and detects a "BAD" * link-status, the sink doesn't receive pixels anymore (e.g. the screen * becomes completely black). The list of available modes may have * changed. User-space is expected to pick a new mode if the current one * has disappeared and perform a new modeset with link-status set to * "GOOD" to re-enable the connector. ``` (form drivers/gpu/drm/drm_connector.c - DOC: standard connector properties) it seems reasonable to assume that the suggested state is an extension of the spec's guidelines, in which the next new mode userspace picks for a connector with no modes is - none, thus breaking the cycle of failed link-training attempts. I suspect that, maybe, zeroing out the bit rate capabilities is not the right way to go, and perhaps marking the connector as disconnected instead may be a better solution. However, if marking a connector disconnected is the way to go, We will have to iterate over all MST ports in the MST case and mark the spawned connectors as disconnected as well. As a final note I should add that this approach was tested with ChromeOS as userspace, and we observed that the zombie displays stop showing up once the connectors are pruned of all their modes and are ignored by userspace. For your consideration and guidance. Thanks, Gil Dekel (6): drm/i915/dp_link_training: Add a final failing state to link training fallback drm/i915/dp_link_training: Add a final failing state to link training fallback for MST drm/dp_mst: Add drm_dp_set_mst_topology_link_status() drm/i915: Move DP modeset_retry_work into intel_dp drm/i915/dp_link_training: Set all downstream MST ports to BAD before retrying drm/i915/dp_link_training: Emit a link-status=Bad uevent with trigger property drivers/gpu/drm/display/drm_dp_mst_topology.c | 39 ++ drivers/gpu/drm/i915/display/intel_display.c | 14 +++- .../drm/i915/display/intel_display_types.h| 6 +- drivers/gpu/drm/i915/display/intel_dp.c | 75 --- drivers/gpu/drm/i915/display/intel_dp.h | 2 +- .../drm/i915/display/intel_dp_link_training.c | 11 ++- include/drm/display/drm_dp_mst_helper.h | 3 + 7 files changed, 110 insertions(+), 40 deletions(-) -- Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
Re: [PATCH v3] drm/bridge/analogix/anx78xx: Drop ID table
Hi,
On Thu, Aug 24, 2023 at 11:26 AM Laurent Pinchart
wrote:
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Thu, Aug 24, 2023 at 07:15:46PM +0100, Biju Das wrote:
> > The driver has an ID table, but it uses the wrong API for retrieving match
> > data and that will lead to a crash, if it is instantiated by user space or
> > using ID. From this, there is no user for the ID table and let's drop it
> > from the driver as it saves some memory.
> >
> > Signed-off-by: Biju Das
>
> Reviewed-by: Laurent Pinchart
>
> I wonder, as the device can only be instantiated from OF, should we add
>
> depends on OF
>
> to Kconfig, and drop the
>
> #if IS_ENABLED(CONFIG_OF)
>
> from the driver ?
In my opinion we shouldn't add the "depends on OF" since that will
decrease the amount of compile testing. It's somewhat the opposite of
adding "if COMPILE_TEST" to your driver. ;-)
I think we could get rid of one of the "#if" statements in the driver
anyway as of commit c9e358dfc4a8 ("driver-core: remove conditionals
around devicetree pointers") from ~12 years ago. If we did something
similar in "struct drm_bridge" we could drop both #ifs.
-Doug
RE: [PATCH v3] drm/bridge/analogix/anx78xx: Drop ID table
Hi Laurent Pinchart,
Thanks for the feedback.
> Subject: Re: [PATCH v3] drm/bridge/analogix/anx78xx: Drop ID table
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Thu, Aug 24, 2023 at 07:15:46PM +0100, Biju Das wrote:
> > The driver has an ID table, but it uses the wrong API for retrieving
> > match data and that will lead to a crash, if it is instantiated by
> > user space or using ID. From this, there is no user for the ID table
> > and let's drop it from the driver as it saves some memory.
> >
> > Signed-off-by: Biju Das
>
> Reviewed-by: Laurent Pinchart
>
> I wonder, as the device can only be instantiated from OF, should we add
>
> depends on OF
>
> to Kconfig, and drop the
>
> #if IS_ENABLED(CONFIG_OF)
>
> from the driver ?
OK, will send a separate patch for this, if there
is no objection.
Cheers,
Biju
>
> > ---
> > v2->v3:
> > * Updated commit header.
> > v1->v2:
> > * Dropped ID table support.
> > ---
> > drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 7 ---
> > 1 file changed, 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> > b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> > index 800555aef97f..6169db73d2fe 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> > @@ -1367,12 +1367,6 @@ static void anx78xx_i2c_remove(struct i2c_client
> *client)
> > kfree(anx78xx->edid);
> > }
> >
> > -static const struct i2c_device_id anx78xx_id[] = {
> > - { "anx7814", 0 },
> > - { /* sentinel */ }
> > -};
> > -MODULE_DEVICE_TABLE(i2c, anx78xx_id);
> > -
> > static const struct of_device_id anx78xx_match_table[] = {
> > { .compatible = "analogix,anx7808", .data = anx7808_i2c_addresses },
> > { .compatible = "analogix,anx7812", .data = anx781x_i2c_addresses },
> > @@ -1389,7 +1383,6 @@ static struct i2c_driver anx78xx_driver = {
> > },
> > .probe = anx78xx_i2c_probe,
> > .remove = anx78xx_i2c_remove,
> > - .id_table = anx78xx_id,
> > };
> > module_i2c_driver(anx78xx_driver);
> >
>
> --
> Regards,
>
> Laurent Pinchart
[PATCH] habanalabs/goya: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/accel/habanalabs/goya/goya.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/accel/habanalabs/goya/goya.c b/drivers/accel/habanalabs/goya/goya.c index 7c685e6075f6..d0ac7065f3d7 100644 --- a/drivers/accel/habanalabs/goya/goya.c +++ b/drivers/accel/habanalabs/goya/goya.c @@ -466,7 +466,7 @@ int goya_set_fixed_properties(struct hl_device *hdev) prop->pcie_dbi_base_address = mmPCIE_DBI_BASE; prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; - strncpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, + strscpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); prop->max_pending_cs = GOYA_MAX_PENDING_CS; @@ -5122,7 +5122,7 @@ int goya_cpucp_info_get(struct hl_device *hdev) } if (!strlen(prop->cpucp_info.card_name)) - strncpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, + strscpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); return 0; --- base-commit: f9604036a3fb6149badf346994b46b03f9292db7 change-id: 20230824-strncpy-drivers-accel-habanalabs-goya-goya-c-2a05a2202c78 Best regards, -- Justin Stitt
[PATCH] habanalabs/gaudi2: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/accel/habanalabs/gaudi2/gaudi2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/accel/habanalabs/gaudi2/gaudi2.c b/drivers/accel/habanalabs/gaudi2/gaudi2.c index 20c4583f12b0..755b2d92357d 100644 --- a/drivers/accel/habanalabs/gaudi2/gaudi2.c +++ b/drivers/accel/habanalabs/gaudi2/gaudi2.c @@ -2431,7 +2431,7 @@ static int gaudi2_set_fixed_properties(struct hl_device *hdev) prop->pcie_dbi_base_address = CFG_BASE + mmPCIE_DBI_BASE; prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; - strncpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); + strscpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); prop->mme_master_slave_mode = 1; @@ -2884,7 +2884,7 @@ static int gaudi2_cpucp_info_get(struct hl_device *hdev) } if (!strlen(prop->cpucp_info.card_name)) - strncpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); + strscpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); /* Overwrite binning masks with the actual binning values from F/W */ hdev->dram_binning = prop->cpucp_info.dram_binning_mask; --- base-commit: f9604036a3fb6149badf346994b46b03f9292db7 change-id: 20230824-strncpy-drivers-accel-habanalabs-gaudi2-gaudi2-c-0b3f717bee12 Best regards, -- Justin Stitt
[PATCH] habanalabs/gaudi: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only --- drivers/accel/habanalabs/gaudi/gaudi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/accel/habanalabs/gaudi/gaudi.c b/drivers/accel/habanalabs/gaudi/gaudi.c index 056e2ef44afb..f175456cdef0 100644 --- a/drivers/accel/habanalabs/gaudi/gaudi.c +++ b/drivers/accel/habanalabs/gaudi/gaudi.c @@ -660,7 +660,7 @@ static int gaudi_set_fixed_properties(struct hl_device *hdev) prop->pcie_dbi_base_address = mmPCIE_DBI_BASE; prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; - strncpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, + strscpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); prop->max_pending_cs = GAUDI_MAX_PENDING_CS; @@ -8000,7 +8000,7 @@ static int gaudi_cpucp_info_get(struct hl_device *hdev) return rc; if (!strlen(prop->cpucp_info.card_name)) - strncpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, + strscpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); hdev->card_type = le32_to_cpu(hdev->asic_prop.cpucp_info.card_type); --- base-commit: f9604036a3fb6149badf346994b46b03f9292db7 change-id: 20230824-strncpy-drivers-accel-habanalabs-gaudi-gaudi-c-f0b5814ced38 Best regards, -- Justin Stitt
Re: [PATCH v1 0/3] udmabuf: Add support for page migration out of movable zone or CMA
On 24.08.23 20:30, Jason Gunthorpe wrote: On Thu, Aug 24, 2023 at 08:30:17PM +0200, David Hildenbrand wrote: On 24.08.23 08:31, Kasireddy, Vivek wrote: Hi David, - Add a new API to the backing store/allocator to longterm-pin the page. For example, something along the lines of shmem_pin_mapping_page_longterm() for shmem as suggested by Daniel. A similar one needs to be added for hugetlbfs as well. This may also be reasonable. Sounds reasonable to keep the old API (that we unfortunately have) working. I agree; I'd like to avoid adding new APIs unless absolutely necessary. Given this, and considering the options I have mentioned earlier, what would be your recommendation for how page migration needs to be done in udmabuf driver? I guess using proper APIs for shmem and hugetlb. So, turning roughly what you have in patch#1 for now into common code, and only calling into that from udmabug. This is a lot of work for an obscure uapi :\ Well, what can we otherwise to to *not* break existing users? I'm not happy about this either. Of course, we can come up with a new uapi, but we have to handle the old uapi somehow. Sure, we can simply always fail when we detect ZONE_MOVABLE or MIGRATE_CMA. Maybe that keeps at least some use cases working. -- Cheers, David / dhildenb
Re: [PATCH v1 0/3] udmabuf: Add support for page migration out of movable zone or CMA
On Thu, Aug 24, 2023 at 08:30:17PM +0200, David Hildenbrand wrote: > On 24.08.23 08:31, Kasireddy, Vivek wrote: > > Hi David, > > > > > > > > > > - Add a new API to the backing store/allocator to longterm-pin the > > > > > page. > > > > > For example, something along the lines of > > > shmem_pin_mapping_page_longterm() > > > > > for shmem as suggested by Daniel. A similar one needs to be added > > > > > for > > > > > hugetlbfs as well. > > > > > > > > This may also be reasonable. > > > > > > Sounds reasonable to keep the old API (that we unfortunately have) > > > working. > > I agree; I'd like to avoid adding new APIs unless absolutely necessary. > > Given this, > > and considering the options I have mentioned earlier, what would be your > > recommendation for how page migration needs to be done in udmabuf driver? > > I guess using proper APIs for shmem and hugetlb. So, turning roughly what > you have in patch#1 for now into common code, and only calling into that > from udmabug. This is a lot of work for an obscure uapi :\ Jason
Re: [PATCH v1 0/3] udmabuf: Add support for page migration out of movable zone or CMA
On 24.08.23 08:31, Kasireddy, Vivek wrote: Hi David, - Add a new API to the backing store/allocator to longterm-pin the page. For example, something along the lines of shmem_pin_mapping_page_longterm() for shmem as suggested by Daniel. A similar one needs to be added for hugetlbfs as well. This may also be reasonable. Sounds reasonable to keep the old API (that we unfortunately have) working. I agree; I'd like to avoid adding new APIs unless absolutely necessary. Given this, and considering the options I have mentioned earlier, what would be your recommendation for how page migration needs to be done in udmabuf driver? I guess using proper APIs for shmem and hugetlb. So, turning roughly what you have in patch#1 for now into common code, and only calling into that from udmabug. -- Cheers, David / dhildenb
[Bug 217664] Laptop doesnt wake up from suspend mode.
https://bugzilla.kernel.org/show_bug.cgi?id=217664 --- Comment #28 from Mario Limonciello (AMD) (mario.limoncie...@amd.com) --- > https://mega.nz/file/OoBmED6L#6Tw4c1kwsUirYXK_XQ7pw6vtLyghrho8MMyB5rzmYYw - > ppa install > https://mega.nz/file/3gRwgDyD#trmYYtnvYSP8z03U4Ggr3BvKFG-KFMOjhJvGnowBjFU - > kern log > https://mega.nz/file/X8gjCZJB#go4CsVkVdbtQNAlWcgEhshci8SGSe4bjYqeDyBQESLg - > journalctrl (be aware huge spam batch) Please attach your logs directly to kernel Bugzilla. I can't access this resource. > https://www.youtube.com/watch?v=d8z1gjoIoUk symptom is exactly same as on > kernel 6.2.xxx, maybe few more complains in console. This looks "like" what I would expect happens when you don't have nouveau/nvidia blacklisted to me, but I don't know for sure since I can't access your logs. Please also look in /var/lib/systemd/pstore to see if anything was captured. As this is s2idle not s3, can you reproduce it under the context of this script? https://gitlab.freedesktop.org/drm/amd/-/blob/master/scripts/amd_s2idle.py -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v3] drm/bridge/analogix/anx78xx: Drop ID table
Hi Biju,
Thank you for the patch.
On Thu, Aug 24, 2023 at 07:15:46PM +0100, Biju Das wrote:
> The driver has an ID table, but it uses the wrong API for retrieving match
> data and that will lead to a crash, if it is instantiated by user space or
> using ID. From this, there is no user for the ID table and let's drop it
> from the driver as it saves some memory.
>
> Signed-off-by: Biju Das
Reviewed-by: Laurent Pinchart
I wonder, as the device can only be instantiated from OF, should we add
depends on OF
to Kconfig, and drop the
#if IS_ENABLED(CONFIG_OF)
from the driver ?
> ---
> v2->v3:
> * Updated commit header.
> v1->v2:
> * Dropped ID table support.
> ---
> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 7 ---
> 1 file changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> index 800555aef97f..6169db73d2fe 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> @@ -1367,12 +1367,6 @@ static void anx78xx_i2c_remove(struct i2c_client
> *client)
> kfree(anx78xx->edid);
> }
>
> -static const struct i2c_device_id anx78xx_id[] = {
> - { "anx7814", 0 },
> - { /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(i2c, anx78xx_id);
> -
> static const struct of_device_id anx78xx_match_table[] = {
> { .compatible = "analogix,anx7808", .data = anx7808_i2c_addresses },
> { .compatible = "analogix,anx7812", .data = anx781x_i2c_addresses },
> @@ -1389,7 +1383,6 @@ static struct i2c_driver anx78xx_driver = {
> },
> .probe = anx78xx_i2c_probe,
> .remove = anx78xx_i2c_remove,
> - .id_table = anx78xx_id,
> };
> module_i2c_driver(anx78xx_driver);
>
--
Regards,
Laurent Pinchart
Re: [PATCH v3] drm/bridge/analogix/anx78xx: Drop ID table
Hi, On Thu, Aug 24, 2023 at 11:15 AM Biju Das wrote: > > The driver has an ID table, but it uses the wrong API for retrieving match > data and that will lead to a crash, if it is instantiated by user space or > using ID. From this, there is no user for the ID table and let's drop it > from the driver as it saves some memory. > > Signed-off-by: Biju Das > --- > v2->v3: > * Updated commit header. > v1->v2: > * Dropped ID table support. > --- > drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 7 --- > 1 file changed, 7 deletions(-) Reviewed-by: Douglas Anderson Unless there are objections, I'm happy to apply this late next week to drm-misc-next. -Doug
[PATCH v3] drm/bridge/analogix/anx78xx: Drop ID table
The driver has an ID table, but it uses the wrong API for retrieving match
data and that will lead to a crash, if it is instantiated by user space or
using ID. From this, there is no user for the ID table and let's drop it
from the driver as it saves some memory.
Signed-off-by: Biju Das
---
v2->v3:
* Updated commit header.
v1->v2:
* Dropped ID table support.
---
drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
index 800555aef97f..6169db73d2fe 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
@@ -1367,12 +1367,6 @@ static void anx78xx_i2c_remove(struct i2c_client *client)
kfree(anx78xx->edid);
}
-static const struct i2c_device_id anx78xx_id[] = {
- { "anx7814", 0 },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(i2c, anx78xx_id);
-
static const struct of_device_id anx78xx_match_table[] = {
{ .compatible = "analogix,anx7808", .data = anx7808_i2c_addresses },
{ .compatible = "analogix,anx7812", .data = anx781x_i2c_addresses },
@@ -1389,7 +1383,6 @@ static struct i2c_driver anx78xx_driver = {
},
.probe = anx78xx_i2c_probe,
.remove = anx78xx_i2c_remove,
- .id_table = anx78xx_id,
};
module_i2c_driver(anx78xx_driver);
--
2.25.1
[PULL] drm-misc-next-fixes
Hi Dave and Daniel, here is this week's PR for drm-misc-next. One of the patches is a change to nouveau's UAPI. Best regards Thomas drm-misc-next-fixes-2023-08-24: Short summary of fixes pull: * gpuva: Cleanups * kunit: Documentation fixes * nouveau: * UAPI: Avoid implicit NO_PREFETCH flag * Scheduler fixes * Fix remap * ttm: Fix type conversion in tests The following changes since commit ff065eaf5502384c0d0a3bd3a9459eb5eb0811e1: drm/ttm/tests: Require MMU when testing (2023-08-17 15:05:51 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2023-08-24 for you to fetch changes up to cdf4100eaa1f4107fcf7c95b5eccca96cca6c777: drm/gpuva_mgr: remove unused prev pointer in __drm_gpuva_sm_map() (2023-08-24 14:27:14 +0200) Short summary of fixes pull: * gpuva: Cleanups * kunit: Documentation fixes * nouveau: * UAPI: Avoid implicit NO_PREFETCH flag * Scheduler fixes * Fix remap * ttm: Fix type conversion in tests Danilo Krummrich (4): drm/nouveau: sched: avoid job races between entities drm/nouveau: uvmm: fix unset region pointer on remap drm/nouveau: uapi: don't pass NO_PREFETCH flag implicitly drm/gpuva_mgr: remove unused prev pointer in __drm_gpuva_sm_map() Karolina Stolarek (1): drm/ttm/tests: Fix type conversion in ttm_pool_test Lee Jones (1): drm/tests/drm_kunit_helpers: Place correct function name in the comment header drivers/gpu/drm/drm_gpuva_mgr.c | 10 -- drivers/gpu/drm/nouveau/nouveau_dma.c | 7 +-- drivers/gpu/drm/nouveau/nouveau_dma.h | 8 ++-- drivers/gpu/drm/nouveau/nouveau_exec.c| 19 --- drivers/gpu/drm/nouveau/nouveau_gem.c | 6 -- drivers/gpu/drm/nouveau/nouveau_sched.c | 22 ++ drivers/gpu/drm/nouveau/nouveau_uvmm.c| 1 + drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +- drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 4 ++-- include/uapi/drm/nouveau_drm.h| 8 +++- 10 files changed, 68 insertions(+), 19 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Re: [PATCH v2] drm/bridge/analogix/anx78xx: Extend match data support for ID table
Hi,
On Thu, Aug 24, 2023 at 11:10 AM Biju Das wrote:
>
> The driver has an ID table, but it uses the wrong API for retrieving match
> data and that will lead to a crash, if it is instantiated by user space or
> using ID. From this, there is no user for the ID table and let's drop it
> from the driver as it saves some memory.
>
> Signed-off-by: Biju Das
> ---
> v1->v2:
> * Dropped ID table support.
> ---
> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 7 ---
> 1 file changed, 7 deletions(-)
Patch ${SUBJECT} needs to be updated to match the change from v1 to v2.
[PATCH v2] drm/bridge/analogix/anx78xx: Extend match data support for ID table
The driver has an ID table, but it uses the wrong API for retrieving match
data and that will lead to a crash, if it is instantiated by user space or
using ID. From this, there is no user for the ID table and let's drop it
from the driver as it saves some memory.
Signed-off-by: Biju Das
---
v1->v2:
* Dropped ID table support.
---
drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
index 800555aef97f..6169db73d2fe 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
@@ -1367,12 +1367,6 @@ static void anx78xx_i2c_remove(struct i2c_client *client)
kfree(anx78xx->edid);
}
-static const struct i2c_device_id anx78xx_id[] = {
- { "anx7814", 0 },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(i2c, anx78xx_id);
-
static const struct of_device_id anx78xx_match_table[] = {
{ .compatible = "analogix,anx7808", .data = anx7808_i2c_addresses },
{ .compatible = "analogix,anx7812", .data = anx781x_i2c_addresses },
@@ -1389,7 +1383,6 @@ static struct i2c_driver anx78xx_driver = {
},
.probe = anx78xx_i2c_probe,
.remove = anx78xx_i2c_remove,
- .id_table = anx78xx_id,
};
module_i2c_driver(anx78xx_driver);
--
2.25.1
Re: [RFC PATCH 3/3] drm/virtio: drm_gem_plane_helper_prepare_fb for obj synchronization
On 8/23/2023 8:52 PM, Dmitry Osipenko wrote: On 8/20/23 23:58, Kim, Dongwon wrote: On 8/17/2023 7:33 PM, Dmitry Osipenko wrote: On 7/13/23 01:44, Dongwon Kim wrote: This helper is needed for framebuffer synchronization. Old framebuffer data is often displayed on the guest display without this helper. Cc: Gerd Hoffmann Cc: Vivek Kasireddy Signed-off-by: Dongwon Kim --- drivers/gpu/drm/virtio/virtgpu_plane.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index a063f06ab6c5..e197299489ce 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "virtgpu_drv.h" @@ -271,6 +272,9 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane, vgfb = to_virtio_gpu_framebuffer(new_state->fb); vgplane_st = to_virtio_gpu_plane_state(new_state); bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); + + drm_gem_plane_helper_prepare_fb(plane, new_state); The implicit display BO sync should happen on a host side, unless you're rendering with Venus and then displaying with virgl. Doing it on guest side should be a major performance hit. Please provide a complete description of your setup: what VMM you use, config options, what tests you're running. We use virtio-gpu as a kms device while using i915 as the render device in our setup. And we use QEMU as VMM. Virtio-gpu driver flushes the scanout to QEMU as a blob resource (reference to the buffer). QEMU then creates a dmabuf using udmabuf for the blob then renders it as a texture using OGL. The test I ran is simple. Just starting terminal app and typing things to see if there is any frame regression. I believe this helper is required since the BO on the guest is basically dmabuf that is being shared between i915 and virtio-gpu driver. I didn't think about the performance impact. If the impact is too much and that is not acceptable, is there any other suggestions or some tests I can try? You can do fence-wait in the guest userspace/Mesa after blitting/drawing to the udmabuf. There is already synchronization between QEMU and virtio-gpu driver on the guest. Upon resource flush, virtio-gpu waits for the response for the message from the QEMU and QEMU sends out the response once rendering is done. The problem we are seeing is not that the rendering part is reusing the buffer before it's displayed by the QEMU. Problem we are facing is more like some frame is often not finished when "resource-flush" is issued. So unless there is a way for QEMU to wait for this fence (guest drm), I think we should have some synchronization point in the guest side. I saw other DRM drivers, omap, tegra, vc4 and so on are doing the similar so I guess this is a generic solution for such cases. But I do understand your concern as the primary use case of virtio-gpu driver is for virgl. So this extra wait would cost some performance drop. But I have a couple of points here as well. 1. Wouldn't this extra wait caused by drm_gem_plane_helper_prepare_fb be minimal as the actual rendering is done in the host? 2. Can we just make this helper called only if virgl is not used as 3D driver? You may run popular vk/gl gfx benchmarks using gl/sdl outputs to see the fps impact. ok Virglrender today supports native contexts. The method you're using for GPU priming was proven to be slow in comparison to multi-gpu native contexts. There is ongoing work for supporting fence passing from guest to host [1] that allows to do fence-syncing on host. You'll find links to the WIP virtio-intel native context in [1] as well. You won't find GPU priming support using native context in [1], patches hasn't been published yet. [1] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1138 Note that in general it's not acceptable to upstream patches that serve downstream only. Yours display sync issue is irrelevant to the upstream stack unless you're going to upstream all the VMM and guest userspace patches, and in such case you should always publish all the patches and provide links. So, you need to check the performance impact and publish all the patches to the relevant upstream projects. QEMU has all patches regarding this (blob scanout support) but guest Mesa patch for KMSRO is still outstanding. https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9592 I understand this specific patch would need more discussion/justification but what about other two, are you generally ok with those in the same series? Thanks!!
Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver
On 20.08.23 12:06, Christophe JAILLET wrote:
[...]
+static void hid_bl_remove(struct hid_device *hdev)
+{
+ struct backlight_device *bl;
+ struct hid_bl_data *data;
+
+ hid_dbg(hdev, "remove\n");
+ bl = hid_get_drvdata(hdev);
+ data = bl_get_data(bl);
+
+ devm_backlight_device_unregister(>dev, bl);
Hi,
If this need to be called here, before the hid_() calls below, there
seems to be no real point in using devm_backlight_device_register() /
devm_backlight_device_unregister().
The non-devm_ version should be enough.
The non-devm_ version is marked deprecated. As for the order, I am not
really sure. I am
concerned about someone updating the brightness while its being removed.
So the HID device
would already have been stopped and then I would issue a request and
wait for completion.
If hid_hw_request and hid_hw_wait can handle this situation we are fine.
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+ hid_set_drvdata(hdev, NULL);
+ devm_kfree(>dev, data);
'data' is devm_kzalloc()'ed.
It is likely that this explicit devm_kfree() could be saved. It will
be freed by the framework.
+}
[...]
+ if (input_field->logical_maximum !=
feature_field->logical_maximum) {
+ hid_warn(hdev, "maximums do not match: %d / %d\n",
+ input_field->logical_maximum,
+ feature_field->logical_maximum);
+ ret = -ENODEV;
+ goto exit_stop;
+ }
+
+ hid_dbg(hdev, "Monitor VESA VCP with brightness control\n");
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "hw open failed: %d\n", ret);
+ goto exit_stop;
+ }
+
+ data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
+ if (data == NULL) {
+ ret = -ENOMEM;
+ goto exit_stop;
exit_free?
in order to undo the hid_hw_open() just above.
Yes, my mistake. This does not call hid_hw_close(). I will fix it in v4.
+ }
+ data->hdev = hdev;
+ data->min_brightness = input_field->logical_minimum;
+ data->max_brightness = input_field->logical_maximum;
+ data->input_field = input_field;
+ data->feature_field = feature_field;
+
+ memset(, 0, sizeof(props));
+ props.type = BACKLIGHT_RAW;
+ props.max_brightness = data->max_brightness - data->min_brightness;
+
+ bl = devm_backlight_device_register(>dev, "vesa_vcp",
+ >dev, data,
+ _bl_ops,
+ );
+ if (IS_ERR(bl)) {
+ ret = PTR_ERR(bl);
+ hid_err(hdev, "failed to register backlight: %d\n", ret);
+ goto exit_free;
+ }
+
+ hid_set_drvdata(hdev, bl);
+
+ return 0;
+
+exit_free:
+ hid_hw_close(hdev);
+ devm_kfree(>dev, data);
'data' is devm_kzalloc()'ed.
It is likely that this explicit devm_kfree() could be saved. It will
be freed by the framework.
+
+exit_stop:
+ hid_hw_stop(hdev);
+ return ret;
+}
[...]
I will remove all of the explicit calls to devm_kfree (and others) in v4
(and test it thoroughly).
Thank you for the helpful review.
Julius
Re: [BUG] KCSAN: data-race in drm_sched_entity_is_ready [gpu_sched] / drm_sched_entity_push_job [gpu_sched]
Thank you, Christian. Glad to hear about that. However, I guess this assumes that this piece of code between -<>- preempt_disable(); tail = (struct spsc_node **)atomic_long_xchg(>tail, (long)>next); WRITE_ONCE(*tail, node); atomic_inc(>job_count); /* * In case of first element verify new node will be visible to the consumer * thread when we ping the kernel thread that there is new work to do. */ smp_wmb(); preempt_enable(); -<>- ... executes only on one CPU/core/thread? I understood that preempt_disable() disables only interrupts on one core/CPU: https://kernelnewbies.kernelnewbies.narkive.com/6LTlgsAe/preempt-disable-disables-preemption-on-all-processors So, we might have a race in theory between WRITE_ONCE() and atomic_inc(). Kind regards, Mirsad On 8/21/2023 8:22 PM, Christian König wrote: I'm not sure about that. On the one hand it might generate some noise. I know tons of cases where logic is: Ok if we see the updated value immediately it will optimize things, but if not it's unproblematic because there is another check after the next memory barrier. On the other hand we probably have cases where this is not correctly implemented. So double checking those would most like be good idea. Regards, Christian. Am 21.08.23 um 16:28 schrieb Mirsad Todorovac: Hi Christian, Thank you for the update. Should I continue reporting what KCSAN gives? I will try to filter these to save your time for evaluation ... Kind regards, Mirsad On 8/21/23 15:20, Christian König wrote: Hi Mirsad, well this is a false positive. That drm_sched_entity_is_ready() doesn't see the data written by drm_sched_entity_push_job() is part of the logic here. Regards, Christian. Am 18.08.23 um 15:44 schrieb Mirsad Todorovac: On 8/17/23 21:54, Mirsad Todorovac wrote: Hi, This is your friendly bug reporter. The environment is vanilla torvalds tree kernel on Ubuntu 22.04 LTS and a Ryzen 7950X box. Please find attached the complete dmesg output from the ring buffer and lshw output. NOTE: The kernel reports tainted kernel, but to my knowledge there are no proprietary (G) modules, but this taint is turned on by the previous bugs. dmesg excerpt: [ 8791.864576] == [ 8791.864648] BUG: KCSAN: data-race in drm_sched_entity_is_ready [gpu_sched] / drm_sched_entity_push_job [gpu_sched] [ 8791.864776] write (marked) to 0x9b74491b7c40 of 8 bytes by task 3807 on cpu 18: [ 8791.864788] drm_sched_entity_push_job+0xf4/0x2a0 [gpu_sched] [ 8791.864852] amdgpu_cs_ioctl+0x3888/0x3de0 [amdgpu] [ 8791.868731] drm_ioctl_kernel+0x127/0x210 [drm] [ 8791.869222] drm_ioctl+0x38f/0x6f0 [drm] [ 8791.869711] amdgpu_drm_ioctl+0x7e/0xe0 [amdgpu] [ 8791.873660] __x64_sys_ioctl+0xd2/0x120 [ 8791.873676] do_syscall_64+0x58/0x90 [ 8791.873688] entry_SYSCALL_64_after_hwframe+0x73/0xdd [ 8791.873710] read to 0x9b74491b7c40 of 8 bytes by task 1119 on cpu 27: [ 8791.873722] drm_sched_entity_is_ready+0x16/0x50 [gpu_sched] [ 8791.873786] drm_sched_select_entity+0x1c7/0x220 [gpu_sched] [ 8791.873849] drm_sched_main+0xd2/0x500 [gpu_sched] [ 8791.873912] kthread+0x18b/0x1d0 [ 8791.873924] ret_from_fork+0x43/0x70 [ 8791.873939] ret_from_fork_asm+0x1b/0x30 [ 8791.873955] value changed: 0x -> 0x9b750ebcfc00 [ 8791.873971] Reported by Kernel Concurrency Sanitizer on: [ 8791.873980] CPU: 27 PID: 1119 Comm: gfx_0.0.0 Tainted: G L 6.5.0-rc6-net-cfg-kcsan-00038-g16931859a650 #35 [ 8791.873994] Hardware name: ASRock X670E PG Lightning/X670E PG Lightning, BIOS 1.21 04/26/2023 [ 8791.874002] == P.S. According to Mr. Heo's instructions, I am adding the unwound trace here: [ 1879.706518] == [ 1879.706616] BUG: KCSAN: data-race in drm_sched_entity_is_ready [gpu_sched] / drm_sched_entity_push_job [gpu_sched] [ 1879.706737] write (marked) to 0x8f3672748c40 of 8 bytes by task 4087 on cpu 10: [ 1879.706748] drm_sched_entity_push_job (./include/drm/spsc_queue.h:74 drivers/gpu/drm/scheduler/sched_entity.c:574) gpu_sched [ 1879.706808] amdgpu_cs_ioctl (drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:1375 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:1469) amdgpu [ 1879.710589] drm_ioctl_kernel (drivers/gpu/drm/drm_ioctl.c:788) drm [ 1879.711068] drm_ioctl (drivers/gpu/drm/drm_ioctl.c:892) drm [ 1879.711551] amdgpu_drm_ioctl (drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:2748) amdgpu [ 1879.715319] __x64_sys_ioctl (fs/ioctl.c:51 fs/ioctl.c:870 fs/ioctl.c:856 fs/ioctl.c:856) [ 1879.715334] do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80) [ 1879.715345] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120) [ 1879.715365] read to 0x8f3672748c40 of 8 bytes by task 1098 on cpu 11: [ 1879.715376]
Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver
On 21.08.23 18:36, Daniel Thompson wrote:
@@ -472,6 +472,14 @@ config BACKLIGHT_LED
If you have a LCD backlight adjustable by LED class driver, say Y
to enable this driver.
+config BACKLIGHT_HID
+ tristate "VESA VCP HID Backlight Driver"
+ depends on HID
+ help
+ If you have an external display with VESA compliant HID brightness
+ controls then say Y to enable this backlight driver. Currently the
+ only supported device is the Apple Studio Display.
This contradicts the description which says you write the driver to the
standard but only tested on Apple Studio Display. There is no need to
spell what has been tested in the Kconfig text. Remove the final
sentence!
Will remove it in v4.
diff --git a/drivers/video/backlight/hid_bl.c b/drivers/video/backlight/hid_bl.c
new file mode 100644
index ..b40f8f412ee2
--- /dev/null
+++ b/drivers/video/backlight/hid_bl.c
+static void hid_bl_remove(struct hid_device *hdev)
+{
+ struct backlight_device *bl;
+ struct hid_bl_data *data;
+
+ hid_dbg(hdev, "remove\n");
This message probably should be removed (if you want to know if a function was
executed use ftrace).
+ bl = hid_get_drvdata(hdev);
+ data = bl_get_data(bl);
+
+ devm_backlight_device_unregister(>dev, bl);
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+ hid_set_drvdata(hdev, NULL);
+ devm_kfree(>dev, data);
+}
+
+static int hid_bl_get_brightness_raw(struct hid_bl_data *data)
+{
+ struct hid_field *field;
+ int result;
+
+ field = data->input_field;
+ hid_hw_request(data->hdev, field->report, HID_REQ_GET_REPORT);
+ hid_hw_wait(data->hdev);
+ result = *field->new_value;
+ hid_dbg(data->hdev, "get brightness: %d\n", result);
To be honest I'm a little dubious about *all* the hid_dbg() calls. They
add very little value (e.g. they are useful to get the driver working
but not that important to keeping it working). As such I don't think
they are worth the clutter in a CONFIG_DYNAMIC_DEBUG kernel.
Note this is strictly for the hid_dbg() stuff... the hid_err() stuff in
the probe error paths are much more useful!
You are right, I will remove all hid_dbg calls in v4.
Thank you very much for the review.
Julius
Re: [PATCH v3 3/3] dt-bindings: display: novatek,nt36523: define ports
On Wed, 23 Aug 2023 10:15:00 +0200, Krzysztof Kozlowski wrote: > The panel-common schema does not define what "ports" property is, so > bring the definition by referencing the panel-common-dual.yaml. Panels > can be single- or dual-link, depending on the compatible, thus add > if:then:else: block narrowing ports per variant. > > Signed-off-by: Krzysztof Kozlowski > > --- > > Changes since v2: > 1. Use panel-common-dual. > 2. Add if:then:else: > > Changes since v1: > 1. Rework to add ports to device schema, not to panel-common. > --- > .../display/panel/novatek,nt36523.yaml| 25 +++ > 1 file changed, 20 insertions(+), 5 deletions(-) > Reviewed-by: Rob Herring
Re: [PATCH v3 2/3] dt-bindings: display: novatek,nt35950: define ports
On Wed, Aug 23, 2023 at 10:14:59AM +0200, Krzysztof Kozlowski wrote: > The panel-common schema does not define what "ports" property is, so > bring the definition by referencing the panel-common-dual.yaml. Panels > can be single- or dual-link, thus require only one port@0. > > Signed-off-by: Krzysztof Kozlowski > > --- > > Changes since v2: > 1. Use panel-common-dual > > Changes since v1: > 1. Rework to add ports to device schema, not to panel-common. > --- > .../devicetree/bindings/display/panel/novatek,nt35950.yaml | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Rob Herring
[PATCH v2] drm: ci: docs: fix build warning - add missing escape
Fix the following warning:
Documentation/gpu/automated_testing.rst:55: WARNING: Inline emphasis
start-string without end-string.
Reported-by: Stephen Rothwell
Signed-off-by: Helen Koike
---
Patch for topic/drm-ci
V2:
- Fix typo s/scape/escape
---
Documentation/gpu/automated_testing.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/gpu/automated_testing.rst
b/Documentation/gpu/automated_testing.rst
index 1b87b802ac7f..469b6fb65c30 100644
--- a/Documentation/gpu/automated_testing.rst
+++ b/Documentation/gpu/automated_testing.rst
@@ -52,7 +52,7 @@ IGT_VERSION
drivers/gpu/drm/ci/testlist.txt
---
-IGT tests to be run on all drivers (unless mentioned in a driver's *-skips.txt
+IGT tests to be run on all drivers (unless mentioned in a driver's \*-skips.txt
file, see below).
drivers/gpu/drm/ci/${DRIVER_NAME}-${HW_REVISION}-fails.txt
--
2.34.1
[PATCH] drm: ci: docs: fix build warning - add missing scape
Fix the following warning:
Documentation/gpu/automated_testing.rst:55: WARNING: Inline emphasis
start-string without end-string.
Reported-by: Stephen Rothwell
Signed-off-by: Helen Koike
---
Patch for topic/drm-ci
---
Documentation/gpu/automated_testing.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/gpu/automated_testing.rst
b/Documentation/gpu/automated_testing.rst
index 1b87b802ac7f..469b6fb65c30 100644
--- a/Documentation/gpu/automated_testing.rst
+++ b/Documentation/gpu/automated_testing.rst
@@ -52,7 +52,7 @@ IGT_VERSION
drivers/gpu/drm/ci/testlist.txt
---
-IGT tests to be run on all drivers (unless mentioned in a driver's *-skips.txt
+IGT tests to be run on all drivers (unless mentioned in a driver's \*-skips.txt
file, see below).
drivers/gpu/drm/ci/${DRIVER_NAME}-${HW_REVISION}-fails.txt
--
2.34.1
[PATCH 1/2] drm/amdgpu: Merge debug module parameters
Merge all developer debug options available as separated module
parameters in one, making it obvious that are for developers.
Signed-off-by: André Almeida
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 24
drivers/gpu/drm/amd/include/amd_shared.h | 9 +
2 files changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index f5856b82605e..d53e4097acc0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -194,6 +194,7 @@ int amdgpu_use_xgmi_p2p = 1;
int amdgpu_vcnfw_log;
int amdgpu_sg_display = -1; /* auto */
int amdgpu_user_partt_mode = AMDGPU_AUTO_COMPUTE_PARTITION_MODE;
+uint amdgpu_debug_mask;
static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
@@ -938,6 +939,9 @@ module_param_named(user_partt_mode, amdgpu_user_partt_mode,
uint, 0444);
module_param(enforce_isolation, bool, 0444);
MODULE_PARM_DESC(enforce_isolation, "enforce process isolation between
graphics and compute . enforce_isolation = on");
+MODULE_PARM_DESC(debug_mask, "debug options for amdgpu, disabled by default");
+module_param_named(debug_mask, amdgpu_debug_mask, uint, 0444);
+
/* These devices are not supported by amdgpu.
* They are supported by the mach64, r128, radeon drivers
*/
@@ -2871,6 +2875,24 @@ static struct pci_driver amdgpu_kms_pci_driver = {
.dev_groups = amdgpu_sysfs_groups,
};
+static void amdgpu_init_debug_options(void)
+{
+ if (amdgpu_debug_mask & DEBUG_VERBOSE_EVICTIONS) {
+ pr_info("debug: eviction debug messages enabled\n");
+ debug_evictions = true;
+ }
+
+ if (amdgpu_debug_mask & DEBUG_VM) {
+ pr_info("debug: VM handling debug enabled\n");
+ amdgpu_vm_debug = true;
+ }
+
+ if (amdgpu_debug_mask & DEBUG_LARGEBAR) {
+ pr_info("debug: enabled simulating large-bar capability on
non-large bar system\n");
+ debug_largebar = true;
+ }
+}
+
static int __init amdgpu_init(void)
{
int r;
@@ -2893,6 +2915,8 @@ static int __init amdgpu_init(void)
/* Ignore KFD init failures. Normal when CONFIG_HSA_AMD is not set. */
amdgpu_amdkfd_init();
+ amdgpu_init_debug_options();
+
/* let modprobe override vga console setting */
return pci_register_driver(_kms_pci_driver);
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
b/drivers/gpu/drm/amd/include/amd_shared.h
index 67d7b7ee8a2a..6fa644c249a5 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -257,6 +257,15 @@ enum DC_DEBUG_MASK {
enum amd_dpm_forced_level;
+/*
+ * amdgpu.debug module options. Are all disabled by default
+ */
+enum AMDGPU_DEBUG_MASK {
+ DEBUG_VERBOSE_EVICTIONS = (1 << 0), // 0x1
+ DEBUG_VM = (1 << 1),// 0x2
+ DEBUG_LARGEBAR = (1 << 2), // 0x4
+};
+
/**
* struct amd_ip_funcs - general hooks for managing amdgpu IP Blocks
* @name: Name of IP block
--
2.41.0
[PATCH 2/2] drm/amdgpu: Create an option to disable soft recovery
Create a module option to disable soft recoveries on amdgpu, making
every recovery go through the device reset path. This option makes
easier to force device resets for testing and debugging purposes.
Signed-off-by: André Almeida
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 6 +-
drivers/gpu/drm/amd/include/amd_shared.h | 1 +
4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4de074243c4d..8f4a93890345 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -189,6 +189,7 @@ extern uint amdgpu_force_long_training;
extern int amdgpu_lbpw;
extern int amdgpu_compute_multipipe;
extern int amdgpu_gpu_recovery;
+extern bool amdgpu_soft_recovery;
extern int amdgpu_emu_mode;
extern uint amdgpu_smu_memory_pool_size;
extern int amdgpu_smu_pptable_id;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index d53e4097acc0..7d6c39b547cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -163,6 +163,7 @@ uint amdgpu_force_long_training;
int amdgpu_lbpw = -1;
int amdgpu_compute_multipipe = -1;
int amdgpu_gpu_recovery = -1; /* auto */
+bool amdgpu_soft_recovery = true;
int amdgpu_emu_mode;
uint amdgpu_smu_memory_pool_size;
int amdgpu_smu_pptable_id = -1;
@@ -2891,6 +2892,11 @@ static void amdgpu_init_debug_options(void)
pr_info("debug: enabled simulating large-bar capability on
non-large bar system\n");
debug_largebar = true;
}
+
+ if (amdgpu_debug_mask & DEBUG_DISABLE_GPU_SOFT_RECOVERY) {
+ pr_info("debug: soft reset for GPU recovery disabled\n");
+ amdgpu_soft_recovery = false;
+ }
}
static int __init amdgpu_init(void)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 80d6e132e409..40678d9fb17e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -434,8 +434,12 @@ bool amdgpu_ring_soft_recovery(struct amdgpu_ring *ring,
unsigned int vmid,
struct dma_fence *fence)
{
unsigned long flags;
+ ktime_t deadline;
- ktime_t deadline = ktime_add_us(ktime_get(), 1);
+ if (!amdgpu_soft_recovery)
+ return false;
+
+ deadline = ktime_add_us(ktime_get(), 1);
if (amdgpu_sriov_vf(ring->adev) || !ring->funcs->soft_recovery ||
!fence)
return false;
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
b/drivers/gpu/drm/amd/include/amd_shared.h
index 6fa644c249a5..afcbacce0a13 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -264,6 +264,7 @@ enum AMDGPU_DEBUG_MASK {
DEBUG_VERBOSE_EVICTIONS = (1 << 0), // 0x1
DEBUG_VM = (1 << 1),// 0x2
DEBUG_LARGEBAR = (1 << 2), // 0x4
+ DEBUG_DISABLE_GPU_SOFT_RECOVERY = (1 << 3), // 0x8
};
/**
--
2.41.0
[PATCH 0/2] drm/amdgpu: Merge all debug module parameters
As suggested by Christian at [0], this patchset merges all debug modules parameters and creates a new one for disabling soft recovery: > Maybe we can overload the amdgpu_gpu_recovery module option with this. > Or even better merge all the developer module parameter into a > amdgpu_debug option. This way it should be pretty obvious that this > isn't meant to be used by someone who doesn't know how to use it. [0] https://lore.kernel.org/dri-devel/55f69184-1aa2-55d6-4a10-1560d75c7...@amd.com/ André Almeida (2): drm/amdgpu: Merge debug module parameters drm/amdgpu: Create an option to disable soft recovery drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 30 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 6 - drivers/gpu/drm/amd/include/amd_shared.h | 10 4 files changed, 46 insertions(+), 1 deletion(-) -- 2.41.0
[Bug 201957] amdgpu: ring gfx timeout
https://bugzilla.kernel.org/show_bug.cgi?id=201957 Priit O. (pr...@ww.ee) changed: What|Removed |Added CC||pr...@ww.ee --- Comment #89 from Priit O. (pr...@ww.ee) --- AMD Vega 64 (vega10 chip) kernel: 6.4.9 linux-firmware: 20230724 # graphical session died and had to log in again, computer didn't boot though... aug 20 02:11:06 Zen kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx_low timeout, signaled seq=368426139, emitted seq=368426141 aug 20 02:11:06 Zen kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process firefox pid 414636 thread firefox:cs0 pid 414712 linux-firmware: 20230810 (upgraded it... although there was no "vega10" changes inbetween) # just freeze for like 30s and then it got unstuck again. aug 23 23:09:24 Zen kernel: [drm:amdgpu_dm_atomic_check [amdgpu]] *ERROR* [CRTC:60:crtc-0] hw_done or flip_done timed out aug 23 23:09:34 Zen kernel: [drm:amdgpu_dm_atomic_check [amdgpu]] *ERROR* [CRTC:63:crtc-1] hw_done or flip_done timed out aug 23 23:09:44 Zen kernel: [drm:amdgpu_dm_atomic_check [amdgpu]] *ERROR* [CRTC:66:crtc-2] hw_done or flip_done timed out -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v3 6/6] drm/i915/dp_link_training: Emit a link-status=Bad uevent with trigger property
When a link-training attempt fails, emit a uevent to user space that includes the trigger property, which in this case will be link-statue=Bad. This will allow userspace to parse the uevent property and better understand the reason for the previous modeset failure. Change-Id: I6170e2755121adf04621ae4fff06985bf4b26d3a Signed-off-by: Gil Dekel --- drivers/gpu/drm/i915/display/intel_dp.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 71f54e56c4434..f45c3bab743cc 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -42,6 +42,7 @@ #include #include #include +#include #include "g4x_dp.h" #include "i915_debugfs.h" @@ -5326,6 +5327,8 @@ static void intel_dp_modeset_retry_work_fn(struct work_struct *work) struct intel_dp *intel_dp = container_of(work, typeof(*intel_dp), modeset_retry_work); struct drm_connector *connector = _dp->attached_connector->base; + struct drm_property *link_status_property = + connector->dev->mode_config.link_status_property; /* Set the connector's (and possibly all its downstream MST ports') link * status to BAD. @@ -5342,7 +5345,7 @@ static void intel_dp_modeset_retry_work_fn(struct work_struct *work) } mutex_unlock(>dev->mode_config.mutex); /* Send Hotplug uevent so userspace can reprobe */ - drm_kms_helper_connector_hotplug_event(connector); + drm_sysfs_connector_status_event(connector, link_status_property); } bool -- Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
[PATCH v3 4/6] drm/i915: Move DP modeset_retry_work into intel_dp
Currently, link-training fallback is only implemented for SST, so having
modeset_retry_work in intel_connector makes sense. However, we hope to
implement link training fallback for MST in a follow-up patchset, so
moving modeset_retry_work to indel_dp will make handling both SST and
MST connectors simpler. This patch does exactly that, and updates all
modeset_retry_work dependencies to use an intel_dp instead.
Credit: this patch is a rebase of Lyude Pual's original patch:
https://patchwork.freedesktop.org/patch/216627/?series=41576=3
Change-Id: I3d80d58b05efe7b07450fb800c438c6e2a34f30c
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/i915/display/intel_display.c | 14 +++---
drivers/gpu/drm/i915/display/intel_display_types.h | 6 +++---
drivers/gpu/drm/i915/display/intel_dp.c| 11 ---
.../gpu/drm/i915/display/intel_dp_link_training.c | 3 +--
4 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index f387215f713af..8e3c7abda3193 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8937,20 +8937,28 @@ void intel_display_resume(struct drm_device *dev)
static void intel_hpd_poll_fini(struct drm_i915_private *i915)
{
- struct intel_connector *connector;
struct drm_connector_list_iter conn_iter;
+ struct intel_connector *connector;
+ struct intel_dp *intel_dp;
+ struct intel_encoder *encoder;
/* Kill all the work that may have been queued by hpd. */
drm_connector_list_iter_begin(>drm, _iter);
for_each_intel_connector_iter(connector, _iter) {
- if (connector->modeset_retry_work.func)
- cancel_work_sync(>modeset_retry_work);
if (connector->hdcp.shim) {
cancel_delayed_work_sync(>hdcp.check_work);
cancel_work_sync(>hdcp.prop_work);
}
}
drm_connector_list_iter_end(_iter);
+
+ for_each_intel_dp(>drm, encoder) {
+ if (encoder->type == DRM_MODE_CONNECTOR_eDP ||
+ encoder->type == DRM_MODE_CONNECTOR_DisplayPort) {
+ intel_dp = enc_to_intel_dp(encoder);
+ cancel_work_sync(_dp->modeset_retry_work);
+ }
+ }
}
/* part #1: call before irq uninstall */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 4b88f4d60a81f..9181a2fef81e0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -608,9 +608,6 @@ struct intel_connector {
struct intel_dp *mst_port;
- /* Work struct to schedule a uevent on link train failure */
- struct work_struct modeset_retry_work;
-
struct intel_hdcp hdcp;
};
@@ -1724,6 +1721,9 @@ struct intel_dp {
/* Displayport compliance testing */
struct intel_dp_compliance compliance;
+ /* Work struct to schedule a uevent on link train failure */
+ struct work_struct modeset_retry_work;
+
/* Downstream facing port caps */
struct {
int min_tmds_clock, max_tmds_clock;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 9a5bcd630068e..2e562e09c704e 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5323,12 +5323,9 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
{
- struct intel_connector *intel_connector;
- struct drm_connector *connector;
-
- intel_connector = container_of(work, typeof(*intel_connector),
- modeset_retry_work);
- connector = _connector->base;
+ struct intel_dp *intel_dp =
+ container_of(work, typeof(*intel_dp), modeset_retry_work);
+ struct drm_connector *connector = _dp->attached_connector->base;
drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s]\n", connector->base.id,
connector->name);
@@ -5358,7 +5355,7 @@ intel_dp_init_connector(struct intel_digital_port
*dig_port,
int type;
/* Initialize the work for modeset in case of link train failure */
- INIT_WORK(_connector->modeset_retry_work,
+ INIT_WORK(_dp->modeset_retry_work,
intel_dp_modeset_retry_work_fn);
if (drm_WARN(dev, dig_port->max_lanes < 1,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 720af16a654c9..6f16609a0820b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1107,7 +1107,6 @@
[PATCH v3 5/6] drm/i915/dp_link_training: Set all downstream MST ports to BAD before retrying
Before sending a uevent to userspace in order to trigger a corrective
modeset, we change the failing connector's link-status to BAD. However,
the downstream MST branch ports are left in their original GOOD state.
This patch utilizes the drm helper function
drm_dp_set_mst_topology_link_status() to rectify this and set all
downstream MST connectors' link-status to BAD before emitting the uevent
to userspace.
Change-Id: Iaae8f0b12b8bce4b16ecad63063c04d3c8ec93a8
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/i915/display/intel_dp.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 2e562e09c704e..71f54e56c4434 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5326,16 +5326,20 @@ static void intel_dp_modeset_retry_work_fn(struct
work_struct *work)
struct intel_dp *intel_dp =
container_of(work, typeof(*intel_dp), modeset_retry_work);
struct drm_connector *connector = _dp->attached_connector->base;
- drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s]\n", connector->base.id,
- connector->name);
- /* Grab the locks before changing connector property*/
- mutex_lock(>dev->mode_config.mutex);
- /* Set connector link status to BAD and send a Uevent to notify
-* userspace to do a modeset.
+ /* Set the connector's (and possibly all its downstream MST ports') link
+* status to BAD.
*/
+ mutex_lock(>dev->mode_config.mutex);
+ drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] link status %d -> %d\n",
+ connector->base.id, connector->name,
+ connector->state->link_status, DRM_MODE_LINK_STATUS_BAD);
drm_connector_set_link_status_property(connector,
DRM_MODE_LINK_STATUS_BAD);
+ if (intel_dp->is_mst) {
+ drm_dp_set_mst_topology_link_status(_dp->mst_mgr,
+ DRM_MODE_LINK_STATUS_BAD);
+ }
mutex_unlock(>dev->mode_config.mutex);
/* Send Hotplug uevent so userspace can reprobe */
drm_kms_helper_connector_hotplug_event(connector);
--
Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
[PATCH v3 3/6] drm/dp_mst: Add drm_dp_set_mst_topology_link_status()
Unlike SST, MST can support multiple displays connected to a single
connector. However, this also means that if the DisplayPort link to the
top-level MST branch device becomes unstable, then every single branch
device has an unstable link.
Since there are multiple downstream ports per connector, setting the
link status of the parent mstb's port to BAD is not enough. All of the
downstream mstb ports must also have their link status set to BAD.
This aligns to how the DP link status logic in DRM works. We notify
userspace that all of the mstb ports need retraining and apply new lower
bandwidth constraints to all future atomic commits on the topology that
follow.
Since any driver supporting MST needs to figure out which connectors
live downstream on an MST topology and update their link status in order
to retrain MST links properly, we add the
drm_dp_set_mst_topology_link_status() helper. This helper simply marks
the link status of all connectors living in that topology as bad. We
will make use of this helper in i915 later in this series.
Credit: this patch is a refactor of Lyude Pual's original patch:
https://patchwork.kernel.org/project/dri-devel/patch/20180308232421.14049-5-ly...@redhat.com/
Change-Id: I42ca477f61e57d23b67e168b0f306c7c1f29
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 38 +++
include/drm/display/drm_dp_mst_helper.h | 3 ++
2 files changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 9ec189fb78a84..d8d92f4a84df1 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3562,6 +3562,44 @@ int drm_dp_get_vc_payload_bw(const struct
drm_dp_mst_topology_mgr *mgr,
}
EXPORT_SYMBOL(drm_dp_get_vc_payload_bw);
+/**
+ * drm_dp_set_mst_topology_link_status() - set all downstream MST ports' link
status
+ * @mgr: MST topology manager to set state for
+ * @status: The new status to set the MST topology to
+ *
+ * Set all downstream ports' link-status within the topology to the given
status.
+ */
+void drm_dp_set_mst_topology_link_status(struct drm_dp_mst_topology_mgr *mgr,
+enum drm_link_status status)
+{
+ struct drm_dp_mst_port *port;
+ struct drm_dp_mst_branch *rmstb;
+ struct drm_dp_mst_branch *mstb =
+ drm_dp_mst_topology_get_mstb_validated(mgr, mgr->mst_primary);
+
+ list_for_each_entry_reverse (port, >ports, next) {
+ struct drm_connector *connector = port->connector;
+ if (connector) {
+ mutex_lock(>dev->mode_config.mutex);
+ drm_dbg_kms(
+ connector->dev,
+ "[MST-CONNECTOR:%d:%s] link status %d -> %d\n",
+ connector->base.id, connector->name,
+ connector->state->link_status, status);
+ connector->state->link_status = status;
+ mutex_unlock(>dev->mode_config.mutex);
+ }
+
+ rmstb = drm_dp_mst_topology_get_mstb_validated(mstb->mgr,
+ port->mstb);
+ if (rmstb) {
+ drm_dp_set_mst_topology_link_status(rmstb->mgr, status);
+ drm_dp_mst_topology_put_mstb(rmstb);
+ }
+ }
+}
+EXPORT_SYMBOL(drm_dp_set_mst_topology_link_status);
+
/**
* drm_dp_read_mst_cap() - check whether or not a sink supports MST
* @aux: The DP AUX channel to use
diff --git a/include/drm/display/drm_dp_mst_helper.h
b/include/drm/display/drm_dp_mst_helper.h
index 41fd8352ab656..ff5d3d86bc2e9 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -829,6 +829,9 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector
*connector, struct drm_dp_
int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr,
int link_rate, int link_lane_count);
+void drm_dp_set_mst_topology_link_status(struct drm_dp_mst_topology_mgr *mgr,
+enum drm_link_status status);
+
int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
void drm_dp_mst_update_slots(struct drm_dp_mst_topology_state *mst_state,
uint8_t link_encoding_cap);
--
Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
[PATCH v3 2/6] drm/i915/dp_link_training: Add a final failing state to link training fallback for MST
Currently, MST link training has no fallback. This means that if an MST
base connector fails to link-train once, the training completely fails,
which makes this case significantly more common than a complete SST link
training failure.
Similar to the final failure state of SST, this patch zeros out both
max_link_rate and max_link_lane_count. In addition, it stops reseting
MST params so the zeroing of the HBR fields stick. This ensures that
the MST base connector's modes will be completely pruned, since it is
effectively left with 0Gbps bandwidth.
Change-Id: Id5de137d0ce4e1ad34e137733a73a1ebbc5b94e5
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/i915/display/intel_dp.c | 27 ++-
drivers/gpu/drm/i915/display/intel_dp.h | 4 +--
.../drm/i915/display/intel_dp_link_training.c | 8 +++---
3 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 2b8d2ee08a2b2..9a5bcd630068e 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -608,7 +608,7 @@ static bool intel_dp_can_link_train_fallback_for_edp(struct
intel_dp *intel_dp,
return true;
}
-int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
+void intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
int link_rate, u8 lane_count)
{
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
@@ -616,18 +616,23 @@ int intel_dp_get_link_train_fallback_values(struct
intel_dp *intel_dp,
/*
* TODO: Enable fallback on MST links once MST link compute can handle
-* the fallback params.
+* the fallback params. For now, similar to the SST case, ensure all of
+* the base connector's modes are pruned in the next connector probe by
+* effectively reducing its bandwidth to 0 so userspace can ignore it
+* within the next modeset attempt.
*/
if (intel_dp->is_mst) {
drm_err(>drm, "Link Training Unsuccessful\n");
- return -1;
+ intel_dp->max_link_rate = 0;
+ intel_dp->max_link_lane_count = 0;
+ return;
}
if (intel_dp_is_edp(intel_dp) && !intel_dp->use_max_params) {
drm_dbg_kms(>drm,
"Retrying Link training for eDP with max
parameters\n");
intel_dp->use_max_params = true;
- return 0;
+ return;
}
index = intel_dp_rate_index(intel_dp->common_rates,
@@ -640,7 +645,7 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp
*intel_dp,
lane_count)) {
drm_dbg_kms(>drm,
"Retrying Link training for eDP with same
parameters\n");
- return 0;
+ return;
}
intel_dp->max_link_rate = intel_dp_common_rate(intel_dp, index
- 1);
intel_dp->max_link_lane_count = lane_count;
@@ -651,7 +656,7 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp
*intel_dp,
lane_count >> 1))
{
drm_dbg_kms(>drm,
"Retrying Link training for eDP with same
parameters\n");
- return 0;
+ return;
}
intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
intel_dp->max_link_lane_count = lane_count >> 1;
@@ -664,10 +669,7 @@ int intel_dp_get_link_train_fallback_values(struct
intel_dp *intel_dp,
*/
intel_dp->max_link_rate = 0;
intel_dp->max_link_lane_count = 0;
- return 0;
}
-
- return 0;
}
u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
@@ -4669,10 +4671,11 @@ intel_dp_detect(struct drm_connector *connector,
intel_dp_configure_mst(intel_dp);
/*
-* TODO: Reset link params when switching to MST mode, until MST
-* supports link training fallback params.
+* Note: Even though MST link training fallback is not yet implemented,
+* do not reset. This is because the base connector needs to have all
+* its modes pruned when link training for the MST port fails.
*/
- if (intel_dp->reset_link_params || intel_dp->is_mst) {
+ if (intel_dp->reset_link_params) {
intel_dp_reset_max_link_params(intel_dp);
intel_dp->reset_link_params = false;
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
b/drivers/gpu/drm/i915/display/intel_dp.h
index a54902c713a34..7069ac5afbb81 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -40,8 +40,8
[PATCH v3 1/6] drm/i915/dp_link_training: Add a final failing state to link training fallback
Instead of silently giving up when all link-training fallback values are
exhausted, this patch modifies the fallback's failure branch to reduces
both max_link_lane_count and max_link_rate to zero (0) and continues to
emit uevents until userspace stops attempting to modeset.
By doing so, we ensure the failing connector, which is in
link-status=Bad, has all its modes pruned (due to effectively having a
bandwidth of 0Gbps).
It is then the userspace's responsibility to ignore connectors with no
modes, even if they are marked as connected.
Change-Id: Ifc0f6a1ee15cc02da6d65a3eeb9e2cf4e8adb8ec
Signed-off-by: Gil Dekel
---
drivers/gpu/drm/i915/display/intel_dp.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 49a34298b1834..2b8d2ee08a2b2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -274,8 +274,12 @@ static int intel_dp_common_len_rate_limit(const struct
intel_dp *intel_dp,
static int intel_dp_common_rate(struct intel_dp *intel_dp, int index)
{
+ /* This occurs when max link rate drops to 0 via link training
fallback*/
+ if (index < 0)
+ return 0;
+
if (drm_WARN_ON(_to_i915(intel_dp)->drm,
- index < 0 || index >= intel_dp->num_common_rates))
+ index >= intel_dp->num_common_rates))
return 162000;
return intel_dp->common_rates[index];
@@ -316,6 +320,9 @@ static int intel_dp_max_common_lane_count(struct intel_dp
*intel_dp)
int intel_dp_max_lane_count(struct intel_dp *intel_dp)
{
switch (intel_dp->max_link_lane_count) {
+ /* This occurs when max link lane count drops to 0 via link training
fallback*/
+ case 0:
+ return 0;
case 1:
case 2:
case 4:
@@ -650,7 +657,14 @@ int intel_dp_get_link_train_fallback_values(struct
intel_dp *intel_dp,
intel_dp->max_link_lane_count = lane_count >> 1;
} else {
drm_err(>drm, "Link Training Unsuccessful\n");
- return -1;
+ /*
+ * Ensure all of the connector's modes are pruned in the next
+ * probe by effectively reducing its bandwidth to 0 so
userspace
+ * can ignore it within the next modeset attempt.
+ */
+ intel_dp->max_link_rate = 0;
+ intel_dp->max_link_lane_count = 0;
+ return 0;
}
return 0;
--
Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
[PATCH v3 0/6] drm/i915/dp_link_training: Define a final failure state when link training fails
Next version of https://patchwork.freedesktop.org/series/122643/ v3: Still learning the ropes of upstream workflow. Apologies for mucking up v2. This is just a re-upload. v2: Reorganize into: 1) Add for final failure state for SST and MST link training fallback. 2) Add a DRM helper for setting downstream MST ports' link-status state. 3) Make handling SST and MST connectors simpler via intel_dp. 4) Update link-status for downstream MST ports. 5) Emit a uevent with the "link-status" trigger property. v1: Currently, when link training fails after all fallback values have been exhausted, the i915 driver seizes to send uevents to userspace. This leave userspace thinking that the last passing atomic commit was successful, and that all connectors (displays) are connected and operational, when in fact, the last link failed to train and the displays remain dark. This manifests as "zombie" displays in userspace, in which users observe the displays appear in their display settings page, but they are dark and unresponsive. Since, at the time of writing, MST link training fallback is not implemented, failing MST link training is a significantly more common case then a complete SST link training failure. And with users using MST hubs than ever to connect multiple displays via their USB-C ports we observe this case often. This patchset series suggest a solution, in which a final failure state is defined. In this final state, the connector's bit rate capabilities, namely max_link_rate and max_link_lane_count, are set to 0. This effectively set the connector's bandwidth to 0Gbps, thus causing all its modes to be pruned in the following connector probing. Next, with this state defined, we emit a link-status=Bad uevent. The next time userspace probes the connector, it should recognize that the connector has no modes and ignore it since it is in a bad state. I am aware that always sending a uevent and never stopping may result in some userspaces having their expectations broken and enter an infinite loop of modesets and link-training attempts. However, per DRM link-status spec: ``` * link-status: * Connector link-status property to indicate the status of link. The * default value of link-status is "GOOD". If something fails during or * after modeset, the kernel driver may set this to "BAD" and issue a * hotplug uevent. Drivers should update this value using * drm_connector_set_link_status_property(). * * When user-space receives the hotplug uevent and detects a "BAD" * link-status, the sink doesn't receive pixels anymore (e.g. the screen * becomes completely black). The list of available modes may have * changed. User-space is expected to pick a new mode if the current one * has disappeared and perform a new modeset with link-status set to * "GOOD" to re-enable the connector. ``` (form drivers/gpu/drm/drm_connector.c - DOC: standard connector properties) it seems reasonable to assume that the suggested state is an extension of the spec's guidelines, in which the next new mode userspace picks for a connector with no modes is - none, thus breaking the cycle of failed link-training attempts. I suspect that, maybe, zeroing out the bit rate capabilities is not the right way to go, and perhaps marking the connector as disconnected instead may be a better solution. However, if marking a connector disconnected is the way to go, We will have to iterate over all MST ports in the MST case and mark the spawned connectors as disconnected as well. As a final note I should add that this approach was tested with ChromeOS as userspace, and we observed that the zombie displays stop showing up once the connectors are pruned of all their modes and are ignored by userspace. For your consideration and guidance. Thanks, Gil Dekel (6): drm/i915/dp_link_training: Add a final failing state to link training fallback drm/i915/dp_link_training: Add a final failing state to link training fallback for MST drm/dp_mst: Add drm_dp_set_mst_topology_link_status() drm/i915: Move DP modeset_retry_work into intel_dp drm/i915/dp_link_training: Set all downstream MST ports to BAD before retrying drm/i915/dp_link_training: Emit a link-status=Bad uevent with trigger property drivers/gpu/drm/display/drm_dp_mst_topology.c | 38 ++ drivers/gpu/drm/i915/display/intel_display.c | 14 +++- .../drm/i915/display/intel_display_types.h| 6 +- drivers/gpu/drm/i915/display/intel_dp.c | 75 --- drivers/gpu/drm/i915/display/intel_dp.h | 4 +- .../drm/i915/display/intel_dp_link_training.c | 11 ++- include/drm/display/drm_dp_mst_helper.h | 3 + 7 files changed, 110 insertions(+), 41 deletions(-) -- Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics
Re: [PATCH v2 5/8] drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()
Hi Daniel,
On Thu, Aug 24, 2023 at 5:12 PM Daniel Stone wrote:
> On Thu, 24 Aug 2023 at 16:09, Geert Uytterhoeven wrote:
> > struct drm_client_dev *client = buffer->client;
> > - struct drm_mode_fb_cmd fb_req = { };
> > - const struct drm_format_info *info;
> > + struct drm_mode_fb_cmd2 fb_req = { };
> > int ret;
> >
> > - info = drm_format_info(format);
> > - fb_req.bpp = drm_format_info_bpp(info, 0);
> > - fb_req.depth = info->depth;
> > fb_req.width = width;
> > fb_req.height = height;
> > - fb_req.handle = handle;
> > - fb_req.pitch = buffer->pitch;
> > + fb_req.pixel_format = format;
> > + fb_req.handles[0] = handle;
> > + fb_req.pitches[0] = buffer->pitch;
> >
> > - ret = drm_mode_addfb(client->dev, _req, client->file);
> > + ret = drm_mode_addfb2(client->dev, _req, client->file);
> > if (ret)
> > return ret;
>
> This should explicitly set the LINEAR modifier (and the modifier flag)
> if the driver supports modifiers.
Thanks for your comment!
I have no idea how to do that, and I do not know what the impact
would be. All I know is that the current implementation of
drm_client_buffer_addfb() does not do that, so changing that in this
patch would mean that this would no longer be a trivial conversion.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 5/8] drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()
Hi Geert,
On Thu, 24 Aug 2023 at 16:09, Geert Uytterhoeven wrote:
> struct drm_client_dev *client = buffer->client;
> - struct drm_mode_fb_cmd fb_req = { };
> - const struct drm_format_info *info;
> + struct drm_mode_fb_cmd2 fb_req = { };
> int ret;
>
> - info = drm_format_info(format);
> - fb_req.bpp = drm_format_info_bpp(info, 0);
> - fb_req.depth = info->depth;
> fb_req.width = width;
> fb_req.height = height;
> - fb_req.handle = handle;
> - fb_req.pitch = buffer->pitch;
> + fb_req.pixel_format = format;
> + fb_req.handles[0] = handle;
> + fb_req.pitches[0] = buffer->pitch;
>
> - ret = drm_mode_addfb(client->dev, _req, client->file);
> + ret = drm_mode_addfb2(client->dev, _req, client->file);
> if (ret)
> return ret;
This should explicitly set the LINEAR modifier (and the modifier flag)
if the driver supports modifiers.
Cheers,
Daniel
[PATCH v2 0/8] drm: fb-helper/ssd130x: Add support for DRM_FORMAT_R1
Hi all,
The native display format of ssd1306 OLED displays is monochrome
light-on-dark (R1). This patch series adds support for the R1 buffer
format to both the ssd130x DRM driver and the FB helpers, so monochrome
applications (including fbdev emulation and the text console) not only
look better, but also avoid the overhead of back-and-forth conversions
between R1 and XR24.
This series consists of 4 parts:
- Patches 1-2 contain fixes,
- Patch 3 contains a small improvement,
- Patch 4 adds R1 support to the ssd130x DRM driver,
- Patches 5-6 update the DRM client and FB helper code to avoid
calling drm_mode_legacy_fb_format() where the exact buffer format is
already known, to prepare for R1 support,
- Patch 7 adds support for R1 to fbdev emulation and the text console,
- Patch 8 switches ssd130x to R1 for fbdev emulation and the text
console.
Changes compared to v1[1]:
- Drop "[PATCH 1/8] drm/ssd130x: Fix pitch calculation in
ssd130x_fb_blit_rect()" (already applied),
- Drop "[PATCH/RFC 3/8] drm/ssd130x: Bail out early if data_array is
not yet available" (no longer needed),
- New patch "[PATCH v2 2/8] drm/ssd130x: Fix screen clearing",
- New patch "[PATCH v2 3/8] drm/ssd130x: Use bool for
ssd130x_deviceinfo flags",
- Add Reviewed-by, Tested-by,
- Rework on top op commit 8c3926367ac9df6c ("drm/ssd130x: Use
shadow-buffer helpers when managing plane's state") in drm/drm-next,
- Do not allocate intermediate buffer when not needed,
- s/drm_mode_create_dumb/drm_client_buffer_addfb/ in one-line summary,
- Fix accidental debug level increase.
This has been tested on an Adafruit FeatherWing 128x32 OLED, connected
to an OrangeCrab ECP5 FPGA board running a 64 MHz VexRiscv RISC-V
softcore, using the fbdev text console.
Thanks for your comments!
P.S. Note that the biggest hurdle was the copious use of the
drm_mode_legacy_fb_format() helper in various places. This helper
cannot decide between C1 and R1 without knowledge of the
capabilities of the full display pipeline. Instead of
special-casing its return value in three callers, I did so in only
one place, and got rid of two of these calls in the call chain.
I think Thomas' grand plan is to replace preferred_{bpp,depth} by a
preferred fourcc format? That would simplify things a lot...
[1] "[PATCH 0/8] drm: fb-helper/ssd130x: Add support for DRM_FORMAT_R1"
https://lore.kernel.org/r/cover.1689252746.git.ge...@linux-m68k.org
Geert Uytterhoeven (8):
drm/dumb-buffers: Fix drm_mode_create_dumb() for bpp < 8
drm/ssd130x: Fix screen clearing
drm/ssd130x: Use bool for ssd130x_deviceinfo flags
drm/ssd130x: Add support for DRM_FORMAT_R1
drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()
drm/fb-helper: Pass buffer format via drm_fb_helper_surface_size
drm/fb-helper: Add support for DRM_FORMAT_R1
drm/ssd130x: Switch preferred_bpp/depth to 1
drivers/gpu/drm/drm_client.c| 13 ++-
drivers/gpu/drm/drm_dumb_buffers.c | 3 +-
drivers/gpu/drm/drm_fb_helper.c | 42 ++---
drivers/gpu/drm/drm_fbdev_generic.c | 9 +-
drivers/gpu/drm/solomon/ssd130x.c | 127
drivers/gpu/drm/solomon/ssd130x.h | 4 +-
include/drm/drm_fb_helper.h | 2 +
7 files changed, 138 insertions(+), 62 deletions(-)
--
2.34.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH v2 1/8] drm/dumb-buffers: Fix drm_mode_create_dumb() for bpp < 8
drm_mode_create_dumb() calculates the number of characters per pixel from the number of bits per pixel by rounding up, which is not correct as the actual value of cpp may be non-integer. While we do not need to care here about complex formats like YUV, bpp < 8 is a valid use case. - The overflow check for the buffer width is not correct if bpp < 8. However, it doesn't hurt, as widths larger than U32_MAX / 8 should not happen for real anyway. Add a comment to clarify. - Calculating the stride from the number of characters per pixel is not correct. Fix this by calculating it from the number of bits per pixel instead. Signed-off-by: Geert Uytterhoeven Reviewed-by: Javier Martinez Canillas Tested-by: Javier Martinez Canillas --- v2: - Add Reviewed-by, Tested-by. --- drivers/gpu/drm/drm_dumb_buffers.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c index 70032bba1c97e787..21a04c32a5e3d785 100644 --- a/drivers/gpu/drm/drm_dumb_buffers.c +++ b/drivers/gpu/drm/drm_dumb_buffers.c @@ -71,10 +71,11 @@ int drm_mode_create_dumb(struct drm_device *dev, /* overflow checks for 32bit size calculations */ if (args->bpp > U32_MAX - 8) return -EINVAL; + /* Incorrect (especially if bpp < 8), but doesn't hurt much */ cpp = DIV_ROUND_UP(args->bpp, 8); if (cpp > U32_MAX / args->width) return -EINVAL; - stride = cpp * args->width; + stride = DIV_ROUND_UP(args->bpp * args->width, 8); if (args->height > U32_MAX / stride) return -EINVAL; -- 2.34.1
[PATCH v2 3/8] drm/ssd130x: Use bool for ssd130x_deviceinfo flags
The .need_pwm and .need_chargepump fields in struct ssd130x_deviceinfo
are flags that can have only two possible values: 0 and 1.
Reduce kernel size by changing their types from int to bool.
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
drivers/gpu/drm/solomon/ssd130x.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x.h
b/drivers/gpu/drm/solomon/ssd130x.h
index 87968b3e7fb8279d..aa39b13615ebedf2 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd130x.h
@@ -40,8 +40,8 @@ struct ssd130x_deviceinfo {
u32 default_width;
u32 default_height;
u32 page_height;
- int need_pwm;
- int need_chargepump;
+ bool need_pwm;
+ bool need_chargepump;
bool page_mode_only;
};
--
2.34.1
[PATCH v2 8/8] drm/ssd130x: Switch preferred_bpp/depth to 1
The native display format is R1. Hence change the preferred_depth and preferred_bpp to 1, to avoid the overhead of using XR24 and the associated conversions when using fbdev emulation and its text console. Signed-off-by: Geert Uytterhoeven Reviewed-by: Javier Martinez Canillas Tested-by: Javier Martinez Canillas --- v2: - Add Reviewed-by, Tested-by. --- drivers/gpu/drm/solomon/ssd130x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c index 18007cb4f3de5aa1..0d2b36ba40113fa3 100644 --- a/drivers/gpu/drm/solomon/ssd130x.c +++ b/drivers/gpu/drm/solomon/ssd130x.c @@ -1049,7 +1049,7 @@ static int ssd130x_init_modeset(struct ssd130x_device *ssd130x) drm->mode_config.max_width = max_width; drm->mode_config.min_height = mode->vdisplay; drm->mode_config.max_height = max_height; - drm->mode_config.preferred_depth = 24; + drm->mode_config.preferred_depth = 1; drm->mode_config.funcs = _mode_config_funcs; /* Primary plane */ @@ -1179,7 +1179,7 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap) if (ret) return ERR_PTR(dev_err_probe(dev, ret, "DRM device register failed\n")); - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_generic_setup(drm, 1); return ssd130x; } -- 2.34.1
[PATCH v2 5/8] drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()
Currently drm_client_buffer_addfb() uses the legacy drm_mode_addfb(),
which uses bpp and depth to guess the wanted buffer format.
However, drm_client_buffer_addfb() already knows the exact buffer
format, so there is no need to convert back and forth between buffer
format and bpp/depth, and the function can just call drm_mode_addfb2()
directly instead.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Javier Martinez Canillas
Tested-by: Javier Martinez Canillas
---
v2:
- Add Reviewed-by, Tested-by,
- s/drm_mode_create_dumb/drm_client_buffer_addfb/ in one-line summary.
---
drivers/gpu/drm/drm_client.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 037e36f2049c1793..0ced189befebae12 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -395,19 +395,16 @@ static int drm_client_buffer_addfb(struct
drm_client_buffer *buffer,
u32 handle)
{
struct drm_client_dev *client = buffer->client;
- struct drm_mode_fb_cmd fb_req = { };
- const struct drm_format_info *info;
+ struct drm_mode_fb_cmd2 fb_req = { };
int ret;
- info = drm_format_info(format);
- fb_req.bpp = drm_format_info_bpp(info, 0);
- fb_req.depth = info->depth;
fb_req.width = width;
fb_req.height = height;
- fb_req.handle = handle;
- fb_req.pitch = buffer->pitch;
+ fb_req.pixel_format = format;
+ fb_req.handles[0] = handle;
+ fb_req.pitches[0] = buffer->pitch;
- ret = drm_mode_addfb(client->dev, _req, client->file);
+ ret = drm_mode_addfb2(client->dev, _req, client->file);
if (ret)
return ret;
--
2.34.1
[PATCH v2 2/8] drm/ssd130x: Fix screen clearing
Due to the reuse of buffers, ssd130x_clear_screen() no longers clears
the screen, but merely redraws the last image that is residing in the
intermediate buffer.
As there is no point in clearing the intermediate buffer and transposing
an all-black image, fix this by just clearing the HW format buffer, and
writing it to the panel.
Fixes: 49d7d581ceaf4cf8 ("drm/ssd130x: Don't allocate buffers on each plane
update")
Signed-off-by: Geert Uytterhoeven
---
ssd130x_clear_screen() is only called from
ssd130x_primary_plane_helper_atomic_disable(), but this never happens on
my system.
Tested by adding some extra calls to ssd130x_clear_screen() at regular
intervals.
v2:
- New.
---
drivers/gpu/drm/solomon/ssd130x.c | 47 +--
1 file changed, 39 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x.c
b/drivers/gpu/drm/solomon/ssd130x.c
index 5a80b228d18cae33..78272b1f9d5b167f 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -553,14 +553,45 @@ static int ssd130x_update_rect(struct ssd130x_device
*ssd130x,
static void ssd130x_clear_screen(struct ssd130x_device *ssd130x,
struct ssd130x_plane_state *ssd130x_state)
{
- struct drm_rect fullscreen = {
- .x1 = 0,
- .x2 = ssd130x->width,
- .y1 = 0,
- .y2 = ssd130x->height,
- };
-
- ssd130x_update_rect(ssd130x, ssd130x_state, );
+ unsigned int page_height = ssd130x->device_info->page_height;
+ unsigned int pages = DIV_ROUND_UP(ssd130x->height, page_height);
+ u8 *data_array = ssd130x_state->data_array;
+ unsigned int width = ssd130x->width;
+ int ret, i;
+
+ if (!ssd130x->page_address_mode) {
+ memset(data_array, 0, width * pages);
+
+ /* Set address range for horizontal addressing mode */
+ ret = ssd130x_set_col_range(ssd130x, ssd130x->col_offset,
width);
+ if (ret < 0)
+ return;
+
+ ret = ssd130x_set_page_range(ssd130x, ssd130x->page_offset,
pages);
+ if (ret < 0)
+ return;
+
+ /* Write out update in one go if we aren't using page
addressing mode */
+ ssd130x_write_data(ssd130x, data_array, width * pages);
+ } else {
+ /*
+* In page addressing mode, the start address needs to be reset,
+* and each page then needs to be written out separately.
+*/
+ memset(data_array, 0, width);
+
+ for (i = 0; i < pages; i++) {
+ ret = ssd130x_set_page_pos(ssd130x,
+ ssd130x->page_offset + i,
+ ssd130x->col_offset);
+ if (ret < 0)
+ return;
+
+ ret = ssd130x_write_data(ssd130x, data_array, width);
+ if (ret < 0)
+ return;
+ }
+ }
}
static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
--
2.34.1
[PATCH v2 4/8] drm/ssd130x: Add support for DRM_FORMAT_R1
The native display format is monochrome light-on-dark (R1).
Hence add support for R1, so monochrome applications not only look
better, but also avoid the overhead of back-and-forth conversions
between R1 and XR24.
Do not allocate the intermediate conversion buffer when it is not
needed, and reorder the two buffer allocations to streamline operation.
Signed-off-by: Geert Uytterhoeven
---
v2:
- Rework on top op commit 8c3926367ac9df6c ("drm/ssd130x: Use
shadow-buffer helpers when managing plane's state") in drm/drm-next.
Hence I did not add Javier's tags given on v1.
- Do not allocate intermediate buffer when not needed.
---
drivers/gpu/drm/solomon/ssd130x.c | 76 +--
1 file changed, 51 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x.c
b/drivers/gpu/drm/solomon/ssd130x.c
index 78272b1f9d5b167f..18007cb4f3de5aa1 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -449,15 +449,14 @@ static int ssd130x_init(struct ssd130x_device *ssd130x)
static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
struct ssd130x_plane_state *ssd130x_state,
+ u8 *buf, unsigned int pitch,
struct drm_rect *rect)
{
unsigned int x = rect->x1;
unsigned int y = rect->y1;
- u8 *buf = ssd130x_state->buffer;
u8 *data_array = ssd130x_state->data_array;
unsigned int width = drm_rect_width(rect);
unsigned int height = drm_rect_height(rect);
- unsigned int line_length = DIV_ROUND_UP(width, 8);
unsigned int page_height = ssd130x->device_info->page_height;
unsigned int pages = DIV_ROUND_UP(height, page_height);
struct drm_device *drm = >drm;
@@ -516,7 +515,7 @@ static int ssd130x_update_rect(struct ssd130x_device
*ssd130x,
u8 data = 0;
for (k = 0; k < m; k++) {
- u8 byte = buf[(8 * i + k) * line_length + j /
8];
+ u8 byte = buf[(8 * i + k) * pitch + j / 8];
u8 bit = (byte >> (j % 8)) & 1;
data |= bit << k;
@@ -602,27 +601,51 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state
*state,
struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
unsigned int page_height = ssd130x->device_info->page_height;
struct ssd130x_plane_state *ssd130x_state =
to_ssd130x_plane_state(state);
- u8 *buf = ssd130x_state->buffer;
struct iosys_map dst;
unsigned int dst_pitch;
int ret = 0;
+ u8 *buf;
/* Align y to display page boundaries */
rect->y1 = round_down(rect->y1, page_height);
rect->y2 = min_t(unsigned int, round_up(rect->y2, page_height),
ssd130x->height);
- dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8);
+ switch (fb->format->format) {
+ case DRM_FORMAT_R1:
+ /* Align x to byte boundaries */
+ rect->x1 = round_down(rect->x1, 8);
+ rect->x2 = round_up(rect->x2, 8);
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret)
- return ret;
+ ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+ if (ret)
+ return ret;
+
+ dst_pitch = fb->pitches[0];
+ buf = vmap[0].vaddr + rect->y1 * dst_pitch + rect->x1 / 8;
+
+ ssd130x_update_rect(ssd130x, ssd130x_state, buf, dst_pitch,
+ rect);
+
+ drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+ break;
+
+ case DRM_FORMAT_XRGB:
+ dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8);
+ buf = ssd130x_state->buffer;
+
+ ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+ if (ret)
+ return ret;
- iosys_map_set_vaddr(, buf);
- drm_fb_xrgb_to_mono(, _pitch, vmap, fb, rect);
+ iosys_map_set_vaddr(, buf);
+ drm_fb_xrgb_to_mono(, _pitch, vmap, fb, rect);
- drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+ drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
- ssd130x_update_rect(ssd130x, ssd130x_state, rect);
+ ssd130x_update_rect(ssd130x, ssd130x_state, buf, dst_pitch,
+ rect);
+ break;
+ }
return ret;
}
@@ -644,22 +667,24 @@ static int
ssd130x_primary_plane_helper_atomic_check(struct drm_plane *plane,
if (ret)
return ret;
- fi = drm_format_info(DRM_FORMAT_R1);
- if (!fi)
- return -EINVAL;
+ ssd130x_state->data_array = kcalloc(ssd130x->width, pages, GFP_KERNEL);
+ if (!ssd130x_state->data_array)
+ return
[PATCH v2 6/8] drm/fb-helper: Pass buffer format via drm_fb_helper_surface_size
drm_fb_helper_single_fb_probe() first calls drm_fb_helper_find_sizes(),
followed by drm_fbdev_generic_helper_fb_probe():
- The former tries to find a suitable buffer format, taking into
account limitations of the whole display pipeline,
- The latter just calls drm_mode_legacy_fb_format() again.
Simplify this by passing the buffer format between these functions
via a new buffer format member in the drm_fb_helper_surface_size
structure.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Javier Martinez Canillas
Tested-by: Javier Martinez Canillas
---
v2:
- Fix accidental debug level increase,
- Add Reviewed-by, Tested-by.
---
drivers/gpu/drm/drm_fb_helper.c | 1 +
drivers/gpu/drm/drm_fbdev_generic.c | 9 -
include/drm/drm_fb_helper.h | 2 ++
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d612133e2cf7ec99..4dc28fdcc1e0a6a4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1564,6 +1564,7 @@ static int __drm_fb_helper_find_sizes(struct
drm_fb_helper *fb_helper,
info = drm_format_info(surface_format);
sizes->surface_bpp = drm_format_info_bpp(info, 0);
sizes->surface_depth = info->depth;
+ sizes->surface_format = surface_format;
/* first up get a count of crtcs now in use and new min/maxes
width/heights */
crtc_count = 0;
diff --git a/drivers/gpu/drm/drm_fbdev_generic.c
b/drivers/gpu/drm/drm_fbdev_generic.c
index d647d89764cb9894..3830d25bcc3ad035 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -77,16 +77,15 @@ static int drm_fbdev_generic_helper_fb_probe(struct
drm_fb_helper *fb_helper,
struct fb_info *info;
size_t screen_size;
void *screen_buffer;
- u32 format;
int ret;
- drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
+ drm_dbg_kms(dev, "surface width(%d), height(%d), bpp(%d) and
format(%p4cc)\n",
sizes->surface_width, sizes->surface_height,
- sizes->surface_bpp);
+ sizes->surface_bpp, >surface_format);
- format = drm_mode_legacy_fb_format(sizes->surface_bpp,
sizes->surface_depth);
buffer = drm_client_framebuffer_create(client, sizes->surface_width,
- sizes->surface_height, format);
+ sizes->surface_height,
+ sizes->surface_format);
if (IS_ERR(buffer))
return PTR_ERR(buffer);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 375737fd6c36ed19..aa3d62a531d12f37 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -45,6 +45,7 @@ struct drm_fb_helper;
* @surface_height: scanout buffer height
* @surface_bpp: scanout buffer bpp
* @surface_depth: scanout buffer depth
+ * @surface_format: scanout buffer format (optional)
*
* Note that the scanout surface width/height may be larger than the fbdev
* width/height. In case of multiple displays, the scanout surface is sized
@@ -61,6 +62,7 @@ struct drm_fb_helper_surface_size {
u32 surface_height;
u32 surface_bpp;
u32 surface_depth;
+ u32 surface_format;
};
/**
--
2.34.1
[PATCH v2 7/8] drm/fb-helper: Add support for DRM_FORMAT_R1
Add support for the monochrome light-on-dark buffer format (R1) to the
fb helper, so this format can be used for fbdev emulation and for the
text console. This avoids the overhead of using XR24 and the associated
conversions on display hardware that supports only a simple monochrome
format.
R1 is very similar to C1 (monochrome indexed color), and shares the same
depth and bpp. As drm_mode_legacy_fb_format() returns a format based on
only depth and bpp, it cannot distinguish between R1 and C1. Hence
drm_fb_helper_find_format() is modified to try to fall back to R1 if C1
is not supported.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Javier Martinez Canillas
Tested-by: Javier Martinez Canillas
---
v2:
- Add Reviewed-by, Tested-by.
---
drivers/gpu/drm/drm_fb_helper.c | 41 -
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4dc28fdcc1e0a6a4..71baf8597516e9fd 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1130,7 +1130,7 @@ static void drm_fb_helper_fill_pixel_fmt(struct
fb_var_screeninfo *var,
{
u8 depth = format->depth;
- if (format->is_color_indexed) {
+ if (format->format == DRM_FORMAT_R1 || format->is_color_indexed) {
var->red.offset = 0;
var->green.offset = 0;
var->blue.offset = 0;
@@ -1236,6 +1236,7 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
case DRM_FORMAT_C1:
case DRM_FORMAT_C2:
case DRM_FORMAT_C4:
+ case DRM_FORMAT_R1:
/* supported format with sub-byte pixels */
break;
@@ -1439,12 +1440,24 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo
*var,
}
EXPORT_SYMBOL(drm_fb_helper_pan_display);
+static bool is_supported_format(uint32_t format, const uint32_t *formats,
+ size_t format_count)
+{
+ size_t i;
+
+ for (i = 0; i < format_count; ++i) {
+ if (formats[i] == format)
+ return true;
+ }
+
+ return false;
+}
+
static uint32_t drm_fb_helper_find_format(struct drm_fb_helper *fb_helper,
const uint32_t *formats,
size_t format_count, uint32_t bpp,
uint32_t depth)
{
struct drm_device *dev = fb_helper->dev;
uint32_t format;
- size_t i;
/*
* Do not consider YUV or other complicated formats
@@ -1457,10 +1470,12 @@ static uint32_t drm_fb_helper_find_format(struct
drm_fb_helper *fb_helper, const
if (!format)
goto err;
- for (i = 0; i < format_count; ++i) {
- if (formats[i] == format)
- return format;
- }
+ if (is_supported_format(format, formats, format_count))
+ return format;
+
+ if (format == DRM_FORMAT_C1 &&
+ is_supported_format(DRM_FORMAT_R1, formats, format_count))
+ return DRM_FORMAT_R1;
err:
/* We found nothing. */
@@ -1680,11 +1695,15 @@ static int drm_fb_helper_single_fb_probe(struct
drm_fb_helper *fb_helper)
}
static void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
- bool is_color_indexed)
+ const struct drm_format_info *format)
{
info->fix.type = FB_TYPE_PACKED_PIXELS;
- info->fix.visual = is_color_indexed ? FB_VISUAL_PSEUDOCOLOR
- : FB_VISUAL_TRUECOLOR;
+ if (format->format == DRM_FORMAT_R1)
+ info->fix.visual = FB_VISUAL_MONO10;
+ else if (format->is_color_indexed)
+ info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
+ else
+ info->fix.visual = FB_VISUAL_TRUECOLOR;
info->fix.mmio_start = 0;
info->fix.mmio_len = 0;
info->fix.type_aux = 0;
@@ -1707,6 +1726,7 @@ static void drm_fb_helper_fill_var(struct fb_info *info,
case DRM_FORMAT_C1:
case DRM_FORMAT_C2:
case DRM_FORMAT_C4:
+ case DRM_FORMAT_R1:
/* supported format with sub-byte pixels */
break;
@@ -1747,8 +1767,7 @@ void drm_fb_helper_fill_info(struct fb_info *info,
{
struct drm_framebuffer *fb = fb_helper->fb;
- drm_fb_helper_fill_fix(info, fb->pitches[0],
- fb->format->is_color_indexed);
+ drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format);
drm_fb_helper_fill_var(info, fb_helper,
sizes->fb_width, sizes->fb_height);
--
2.34.1
[PATCH v5 0/2] PCI/VGA: Make the vga_is_firmware_default() less arch-dependent
Currently, the vga_is_firmware_default() function only works on x86 and ia64, it is a no-op on the rest of the architectures. This patch completes the implementation for it, the added code tries to capture the PCI (e) VGA device that owns the firmware framebuffer address range before the PCI resource relocation. Since only one GPU could owns the firmware fb in normal case, things are almost done once we have determined the boot VGA successfully. Note that this patch requires the target platform has a way to set up the kernel's screen_info. On muiltiple GPU co-exist machines, the firmware framebuffer should be put into the VRAM BAR of the primary GPU. While changing PCI class code of the GPU to be non-primary is not required for the arbitration purpose. The provided method is effective at least on x86, arm64 and loongarch, see below for more testing information. 1) LS3A5000+LS7A1000 platform with three video cards: $ lspci | grep VGA 00:06.1 VGA compatible controller: Loongson Technology LLC DC (Display Controller) (rev 01) 03:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Caicos XT [Radeon HD 7470/8470 / R5 235/310 OEM] 07:00.0 VGA compatible controller: S3 Graphics Ltd. Device 9070 (rev 01) 08:00.0 VGA compatible controller: S3 Graphics Ltd. Device 9070 (rev 01) Before apply this series: pci :00:06.1: vgaarb: setting as boot VGA device pci :00:06.1: vgaarb: VGA device added: decodes=io+mem,owns=io+mem,locks=none pci :03:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none pci :07:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none pci :08:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none vgaarb: loaded After apply this series: pci :03:00.0: vgaarb: BAR 0: [mem 0xe005000-0xe005fff 64bit pref] contains firmware FB [0xe005000-0xe00500ea5ff] pci :00:06.1: vgaarb: setting as boot VGA device pci :00:06.1: vgaarb: bridge control possible pci :03:00.0: vgaarb: setting as boot VGA device (overriding previous) pci :03:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none pci :07:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none pci :08:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none vgaarb: loaded $ dmesg | grep :03:00.0 pci :03:00.0: [1002:6778] type 00 class 0x03 pci :03:00.0: reg 0x10: [mem 0xe005000-0xe005fff 64bit pref] pci :03:00.0: reg 0x18: [mem 0xe006530-0xe006531 64bit] pci :03:00.0: reg 0x20: [io 0x2-0x200ff] pci :03:00.0: reg 0x30: [mem 0xfffe-0x pref] pci :03:00.0: vgaarb: BAR 0: [mem 0xe005000-0xe005fff 64bit pref] contains firmware FB [0xe005000-0xe00500ea5ff] pci :03:00.0: BAR 0: assigned [mem 0xe003000-0xe003fff 64bit pref] pci :03:00.0: BAR 2: assigned [mem 0xe006520-0xe006521 64bit] pci :03:00.0: BAR 6: assigned [mem 0xe006522-0xe006523 pref] pci :03:00.0: BAR 4: assigned [io 0x5000-0x50ff] pci :03:00.0: vgaarb: setting as boot VGA device (overriding previous) pci :03:00.0: vgaarb: bridge control possible pci :03:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none Loongson UEFI firmware does not support specify which GPU to be the primary, the firmware set the ATI GPU(03:00.0) as the primary GPU with this hardware configuration by hardcode. However, the firmware do support passing the screen_into the to kernel. The problem is that VGAARB can not override the platform integrated one(00:06.1) before apply this series. Please note that BAR 0 of the ATI GPU moved by PCI core from [0xe005000-0xe005fff] to [0xe003000-0xe003fff], the vga_is_firmware_default() function will return wrong results by simply remove #ifdefs while without take relocation into account. 2) ARM64 (Kunpeng 920) with three video card: Before apply this series: pci :02:00.0: vgaarb: setting as boot VGA device pci :02:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none pci :05:00.0: vgaarb: setting as boot VGA device (overriding previous) <--- (Because it has IO or MEM enabled) pci :05:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none pci :06:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none vgaarb: loaded After apply this series: pci :05:00.0: vgaarb: BAR 0: [mem 0x8001000-0x8001fff 64bit pref] contains firmware FB [0x8001000-0x800101e77ff] pci :02:00.0: vgaarb: setting as boot VGA device pci :02:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none pci :05:00.0: vgaarb: Boot VGA selected by firmware pci :05:00.0: vgaarb: setting as boot VGA device (overriding previous) <--- (Because it owns firmware framebuffer) pci :05:00.0: vgaarb: VGA device added: decodes=io+mem,owns=none,locks=none pci :06:00.0:
[PATCH v5 1/2] PCI/VGA: Make the vga_is_firmware_default() less arch-dependent
Currently, the vga_is_firmware_default() function only works on x86 and
ia64, it is a no-op on the rest of the architectures. This patch completes
the implementation for it, the added code tries to capture the PCI (e) VGA
device that owns the firmware framebuffer, since only one GPU could own
the firmware fb, things are almost done once we have determined the boot
VGA device. As the PCI resource relocation do have a influence on the
results of identification, we make it available on architectures where PCI
resource relocation does happen at first. Because this patch is more
important for those architectures(such as arm, arm64, loongarch, mips and
risc-v etc).
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 76 +++-
1 file changed, 75 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 5a696078b382..bc5fcc855513 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -60,7 +60,8 @@ static int vga_count, vga_decode_count;
static bool vga_arbiter_used;
static DEFINE_SPINLOCK(vga_lock);
static DECLARE_WAIT_QUEUE_HEAD(vga_wait_queue);
-
+/* The PCI(e) device who owns the firmware framebuffer */
+static struct pci_dev *pdev_boot_vga;
static const char *vga_iostate_to_str(unsigned int iostate)
{
@@ -571,6 +572,9 @@ static bool vga_is_firmware_default(struct pci_dev *pdev)
return true;
}
+#else
+ if (pdev_boot_vga && pdev_boot_vga == pdev)
+ return true;
#endif
return false;
}
@@ -1555,3 +1559,73 @@ static int __init vga_arb_device_init(void)
return rc;
}
subsys_initcall_sync(vga_arb_device_init);
+
+/*
+ * Get the physical address range that the firmware framebuffer occupies.
+ *
+ * Note that the global screen_info is arch-specific, thus CONFIG_SYSFB is
+ * chosen as compile-time conditional to suppress linkage problems on non-x86
+ * architectures.
+ *
+ * Returns true on success, otherwise return false.
+ */
+static bool vga_arb_get_firmware_fb_range(u64 *start, u64 *end)
+{
+ u64 fb_start = 0;
+ u64 fb_size = 0;
+ u64 fb_end;
+
+#if defined(CONFIG_X86) || defined(CONFIG_IA64) || defined(CONFIG_SYSFB)
+ fb_start = screen_info.lfb_base;
+ if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+ fb_start |= (u64)screen_info.ext_lfb_base << 32;
+
+ fb_size = screen_info.lfb_size;
+#endif
+
+ /* No firmware framebuffer support */
+ if (!fb_start || !fb_size)
+ return false;
+
+ fb_end = fb_start + fb_size - 1;
+
+ *start = fb_start;
+ *end = fb_end;
+
+ return true;
+}
+
+/*
+ * Identify the PCI VGA device that contains the firmware framebuffer
+ */
+static void pci_boot_vga_capturer(struct pci_dev *pdev)
+{
+ u64 fb_start, fb_end;
+ struct resource *res;
+ unsigned int i;
+
+ if (pdev_boot_vga)
+ return;
+
+ if (!vga_arb_get_firmware_fb_range(_start, _end))
+ return;
+
+ pci_dev_for_each_resource(pdev, res, i) {
+ if (resource_type(res) != IORESOURCE_MEM)
+ continue;
+
+ if (!res->start || !res->end)
+ continue;
+
+ if (res->start <= fb_start && fb_end <= res->end) {
+ pdev_boot_vga = pdev;
+
+ vgaarb_info(>dev,
+ "BAR %u: %pR contains firmware FB
[0x%llx-0x%llx]\n",
+ i, res, fb_start, fb_end);
+ break;
+ }
+ }
+}
+DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA,
+ 8, pci_boot_vga_capturer);
--
2.34.1
[PATCH v5 2/2] PCI/VGA: Remove vga_is_firmware_default() function
The new implemented pci_boot_vga_capturer() function is also effective on
X86 and IA64, it can determine the default boot VGA device before VRAM BAR
relocations done by the PCI core. Since the fixup handler has already
identified the firmware framebuffer, there no need to look again later. So,
switch to using the pci_boot_vga_capturer() on X86 and IA64 also, remove
vga_is_firmware_default() and its relevant codes.
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 46 +++-
1 file changed, 3 insertions(+), 43 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index bc5fcc855513..1dff74f778cd 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -51,7 +51,6 @@ struct vga_device {
unsigned int io_norm_cnt; /* normal IO count */
unsigned int mem_norm_cnt; /* normal MEM count */
bool bridge_has_one_vga;
- bool is_firmware_default; /* device selected by firmware */
unsigned int (*set_decode)(struct pci_dev *pdev, bool decode);
};
@@ -544,41 +543,6 @@ void vga_put(struct pci_dev *pdev, unsigned int rsrc)
}
EXPORT_SYMBOL(vga_put);
-static bool vga_is_firmware_default(struct pci_dev *pdev)
-{
-#if defined(CONFIG_X86) || defined(CONFIG_IA64)
- u64 base = screen_info.lfb_base;
- u64 size = screen_info.lfb_size;
- struct resource *r;
- u64 limit;
-
- /* Select the device owning the boot framebuffer if there is one */
-
- if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
- base |= (u64)screen_info.ext_lfb_base << 32;
-
- limit = base + size;
-
- /* Does firmware framebuffer belong to us? */
- pci_dev_for_each_resource(pdev, r) {
- if (resource_type(r) != IORESOURCE_MEM)
- continue;
-
- if (!r->start || !r->end)
- continue;
-
- if (base < r->start || limit >= r->end)
- continue;
-
- return true;
- }
-#else
- if (pdev_boot_vga && pdev_boot_vga == pdev)
- return true;
-#endif
- return false;
-}
-
static bool vga_arb_integrated_gpu(struct device *dev)
{
#if defined(CONFIG_ACPI)
@@ -610,14 +574,10 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
*/
/*
-* We always prefer a firmware default device, so if we've already
-* found one, there's no need to consider vgadev.
+* We always prefer a firmware default device.
*/
- if (boot_vga && boot_vga->is_firmware_default)
- return false;
-
- if (vga_is_firmware_default(pdev)) {
- vgadev->is_firmware_default = true;
+ if (pdev == pdev_boot_vga) {
+ vgaarb_dbg(>dev, "Boot VGA selected by firmware\n");
return true;
}
--
2.34.1
Re: [PATCH 11/20] drm/amd/amdgpu/amdgpu_doorbell_mgr: Correct misdocumented param 'doorbell_index'
Applied. Thanks! On Thu, Aug 24, 2023 at 4:35 AM Sharma, Shashank wrote: > > [AMD Official Use Only - General] > > Reviewed-by: : Shashank Sharma > > Regards > Shashank > -Original Message- > From: Lee Jones > Sent: Thursday, August 24, 2023 9:37 AM > To: l...@kernel.org > Cc: linux-ker...@vger.kernel.org; Deucher, Alexander > ; Koenig, Christian ; > Pan, Xinhui ; David Airlie ; Daniel > Vetter ; Sharma, Shashank ; > amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Subject: [PATCH 11/20] drm/amd/amdgpu/amdgpu_doorbell_mgr: Correct > misdocumented param 'doorbell_index' > > Fixes the following W=1 kernel build warning(s): > > drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c:123: warning: Function > parameter or member 'doorbell_index' not described in > 'amdgpu_doorbell_index_on_bar' > drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c:123: warning: Excess > function parameter 'db_index' description in 'amdgpu_doorbell_index_on_bar' > > Signed-off-by: Lee Jones > --- > Cc: Alex Deucher > Cc: "Christian König" > Cc: "Pan, Xinhui" > Cc: David Airlie > Cc: Daniel Vetter > Cc: Shashank Sharma > Cc: amd-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c > index da4be0bbb4466..d0249ada91d30 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c > @@ -113,7 +113,7 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, > u32 index, u64 v) > * > * @adev: amdgpu_device pointer > * @db_bo: doorbell object's bo > - * @db_index: doorbell relative index in this doorbell object > + * @doorbell_index: doorbell relative index in this doorbell object > * > * returns doorbell's absolute index in BAR > */ > -- > 2.42.0.rc1.204.g551eb34607-goog >
Re: [PATCH 20/20] drm/amd/amdgpu/imu_v11_0: Increase buffer size to ensure all possible values can be stored
Applied. Thanks!
On Thu, Aug 24, 2023 at 3:38 AM Lee Jones wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/amd/amdgpu/imu_v11_0.c: In function
> ‘imu_v11_0_init_microcode’:
> drivers/gpu/drm/amd/amdgpu/imu_v11_0.c:52:54: warning: ‘_imu.bin’ directive
> output may be truncated writing 8 bytes into a region of size between 4 and
> 33 [-Wformat-truncation=]
> drivers/gpu/drm/amd/amdgpu/imu_v11_0.c:52:9: note: ‘snprintf’ output between
> 16 and 45 bytes into a destination of size 40
>
> Signed-off-by: Lee Jones
> ---
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: "Pan, Xinhui"
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Mario Limonciello
> Cc: amd-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/amd/amdgpu/imu_v11_0.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/imu_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/imu_v11_0.c
> index 4ab90c7852c3e..ca123ff553477 100644
> --- a/drivers/gpu/drm/amd/amdgpu/imu_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/imu_v11_0.c
> @@ -39,7 +39,7 @@ MODULE_FIRMWARE("amdgpu/gc_11_0_4_imu.bin");
>
> static int imu_v11_0_init_microcode(struct amdgpu_device *adev)
> {
> - char fw_name[40];
> + char fw_name[45];
> char ucode_prefix[30];
> int err;
> const struct imu_firmware_header_v1_0 *imu_hdr;
> --
> 2.42.0.rc1.204.g551eb34607-goog
>
Re: [PATCH 19/20] drm/amd/amdgpu/amdgpu_sdma: Increase buffer size to account for all possible values
Applied. Thanks! On Thu, Aug 24, 2023 at 3:38 AM Lee Jones wrote: > > Fixes the following W=1 kernel build warning(s): > > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c: In function > ‘amdgpu_sdma_init_microcode’: > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c:217:64: warning: ‘.bin’ directive > output may be truncated writing 4 bytes into a region of size between 0 and > 32 [-Wformat-truncation=] > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c:217:17: note: ‘snprintf’ output > between 13 and 52 bytes into a destination of size 40 > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c:215:66: warning: ‘snprintf’ output > may be truncated before the last format character [-Wformat-truncation=] > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c:215:17: note: ‘snprintf’ output > between 12 and 41 bytes into a destination of size 40 > > Signed-off-by: Lee Jones > --- > Cc: Alex Deucher > Cc: "Christian König" > Cc: "Pan, Xinhui" > Cc: David Airlie > Cc: Daniel Vetter > Cc: amd-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c > index e2b9392d7f0de..572f861e3f706 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c > @@ -208,7 +208,7 @@ int amdgpu_sdma_init_microcode(struct amdgpu_device *adev, > const struct sdma_firmware_header_v2_0 *sdma_hdr; > uint16_t version_major; > char ucode_prefix[30]; > - char fw_name[40]; > + char fw_name[52]; > > amdgpu_ucode_ip_version_decode(adev, SDMA0_HWIP, ucode_prefix, > sizeof(ucode_prefix)); > if (instance == 0) > -- > 2.42.0.rc1.204.g551eb34607-goog >
Re: [PATCH 17/20] drm/amd/amdgpu/amdgpu_ras: Increase buffer size to account for all possible values
Applied. Thanks!
On Thu, Aug 24, 2023 at 3:38 AM Lee Jones wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c: In function
> ‘amdgpu_ras_sysfs_create’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1406:20: warning: ‘_err_count’
> directive output may be truncated writing 10 bytes into a region of size
> between 1 and 32 [-Wformat-truncation=]
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1405:9: note: ‘snprintf’ output
> between 11 and 42 bytes into a destination of size 32
>
> Signed-off-by: Lee Jones
> ---
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: "Pan, Xinhui"
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Hawking Zhang
> Cc: amd-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> index ffb49b2d533ad..7999d202c9bc5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> @@ -436,7 +436,7 @@ struct amdgpu_ras {
> };
>
> struct ras_fs_data {
> - char sysfs_name[32];
> + char sysfs_name[48];
> char debugfs_name[32];
> };
>
> --
> 2.42.0.rc1.204.g551eb34607-goog
>
Re: [PATCH 12/20] drm/amd/amdgpu/amdgpu_device: Provide suitable description for param 'xcc_id'
Applied. Thanks! On Thu, Aug 24, 2023 at 3:38 AM Lee Jones wrote: > > Fixes the following W=1 kernel build warning(s): > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:516: warning: Function parameter > or member 'xcc_id' not described in 'amdgpu_mm_wreg_mmio_rlc' > > Signed-off-by: Lee Jones > --- > Cc: Alex Deucher > Cc: "Christian König" > Cc: "Pan, Xinhui" > Cc: David Airlie > Cc: Daniel Vetter > Cc: Sumit Semwal > Cc: amd-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Cc: linux-me...@vger.kernel.org > Cc: linaro-mm-...@lists.linaro.org > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index e77f048c99d85..d4f0e4327dd3f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -507,6 +507,7 @@ void amdgpu_device_wreg(struct amdgpu_device *adev, > * @adev: amdgpu_device pointer > * @reg: mmio/rlc register > * @v: value to write > + * @xcc_id: xcc accelerated compute core id > * > * this function is invoked only for the debugfs register access > */ > -- > 2.42.0.rc1.204.g551eb34607-goog >
Re: [PATCH 07/20] drm/radeon/radeon_ttm: Remove unused variable 'rbo' from radeon_bo_move()
Applied. Thanks!
On Thu, Aug 24, 2023 at 3:37 AM Lee Jones wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/radeon/radeon_ttm.c: In function ‘radeon_bo_move’:
> drivers/gpu/drm/radeon/radeon_ttm.c:201:27: warning: variable ‘rbo’ set but
> not used [-Wunused-but-set-variable]
>
> Signed-off-by: Lee Jones
> ---
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: "Pan, Xinhui"
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Jerome Glisse
> Cc: amd-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/radeon/radeon_ttm.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
> b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 4eb83ccc4906a..de4e6d78f1e12 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -197,7 +197,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo,
> bool evict,
> {
> struct ttm_resource *old_mem = bo->resource;
> struct radeon_device *rdev;
> - struct radeon_bo *rbo;
> int r;
>
> if (new_mem->mem_type == TTM_PL_TT) {
> @@ -210,7 +209,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo,
> bool evict,
> if (r)
> return r;
>
> - rbo = container_of(bo, struct radeon_bo, tbo);
> rdev = radeon_get_rdev(bo->bdev);
> if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM &&
> bo->ttm == NULL)) {
> --
> 2.42.0.rc1.204.g551eb34607-goog
>
Re: [PATCH v2 0/9] DRM scheduler changes for Xe
On 8/24/23 05:23, Matthew Brost wrote: On Thu, Aug 24, 2023 at 02:08:59AM +0200, Danilo Krummrich wrote: Hi Matt, On 8/11/23 04:31, Matthew Brost wrote: As a prerequisite to merging the new Intel Xe DRM driver [1] [2], we have been asked to merge our common DRM scheduler patches first. This a continuation of a RFC [3] with all comments addressed, ready for a full review, and hopefully in state which can merged in the near future. More details of this series can found in the cover letter of the RFC [3]. These changes have been tested with the Xe driver. Do you keep a branch with these patches somewhere? Pushed a branch for you: https://gitlab.freedesktop.org/mbrost/nouveau-drm-scheduler/-/tree/xe-sched-changes?ref_type=heads Great - gonna pick this up to work on making use of DRM_SCHED_POLICY_SINGLE_ENTITY in Nouveau. - Danilo Matt - Danilo v2: - Break run job, free job, and process message in own work items - This might break other drivers as run job and free job now can run in parallel, can fix up if needed Matt Matthew Brost (9): drm/sched: Convert drm scheduler to use a work queue rather than kthread drm/sched: Move schedule policy to scheduler / entity drm/sched: Add DRM_SCHED_POLICY_SINGLE_ENTITY scheduling policy drm/sched: Split free_job into own work item drm/sched: Add generic scheduler message interface drm/sched: Add drm_sched_start_timeout_unlocked helper drm/sched: Start run wq before TDR in drm_sched_start drm/sched: Submit job before starting TDR drm/sched: Add helper to set TDR timeout drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +- drivers/gpu/drm/etnaviv/etnaviv_sched.c| 5 +- drivers/gpu/drm/lima/lima_sched.c | 5 +- drivers/gpu/drm/msm/msm_ringbuffer.c | 5 +- drivers/gpu/drm/nouveau/nouveau_sched.c| 5 +- drivers/gpu/drm/panfrost/panfrost_job.c| 5 +- drivers/gpu/drm/scheduler/sched_entity.c | 85 - drivers/gpu/drm/scheduler/sched_fence.c| 2 +- drivers/gpu/drm/scheduler/sched_main.c | 408 - drivers/gpu/drm/v3d/v3d_sched.c| 25 +- include/drm/gpu_scheduler.h| 75 +++- 11 files changed, 487 insertions(+), 136 deletions(-)
Re: [PATCH][next] drm/amd: Fix spelling mistake "throtting" -> "throttling"
Applied. Thanks!
Alex
On Wed, Aug 23, 2023 at 5:50 AM Wang, Yang(Kevin)
wrote:
>
> [AMD Official Use Only - General]
>
> Reviewed-by: Yang Wang
>
> Best Regards,
> Kevin
>
> -Original Message-
> From: dri-devel On Behalf Of Colin
> Ian King
> Sent: Wednesday, August 23, 2023 5:03 PM
> To: Deucher, Alexander ; Koenig, Christian
> ; Pan, Xinhui ; David Airlie
> ; Daniel Vetter ; Lazar, Lijo
> ; amd-...@lists.freedesktop.org;
> dri-devel@lists.freedesktop.org
> Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [PATCH][next] drm/amd: Fix spelling mistake "throtting" ->
> "throttling"
>
> There is a spelling mistake in variable throtting_events, rename it to
> throttling_events.
>
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c| 6 +++---
> drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 6 +++---
> drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 6 +++---
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> index 704a2b577a0e..fbcff154b1d0 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> @@ -2313,7 +2313,7 @@ static const struct throttling_logging_label { static
> void arcturus_log_thermal_throttling_event(struct smu_context *smu) {
> int ret;
> - int throttler_idx, throtting_events = 0, buf_idx = 0;
> + int throttler_idx, throttling_events = 0, buf_idx = 0;
> struct amdgpu_device *adev = smu->adev;
> uint32_t throttler_status;
> char log_buf[256];
> @@ -2328,11 +2328,11 @@ static void
> arcturus_log_thermal_throttling_event(struct smu_context *smu)
> for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
> throttler_idx++) {
> if (throttler_status &
> logging_label[throttler_idx].feature_mask) {
> - throtting_events++;
> + throttling_events++;
> buf_idx += snprintf(log_buf + buf_idx,
> sizeof(log_buf) - buf_idx,
> "%s%s",
> - throtting_events > 1 ? " and " :
> "",
> + throttling_events > 1 ? " and " :
> "",
>
> logging_label[throttler_idx].label);
> if (buf_idx >= sizeof(log_buf)) {
> dev_err(adev->dev, "buffer overflow!\n");
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index cc3169400c9b..bed5a9df1c06 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -1674,7 +1674,7 @@ static const struct throttling_logging_label { static
> void aldebaran_log_thermal_throttling_event(struct smu_context *smu) {
> int ret;
> - int throttler_idx, throtting_events = 0, buf_idx = 0;
> + int throttler_idx, throttling_events = 0, buf_idx = 0;
> struct amdgpu_device *adev = smu->adev;
> uint32_t throttler_status;
> char log_buf[256];
> @@ -1689,11 +1689,11 @@ static void
> aldebaran_log_thermal_throttling_event(struct smu_context *smu)
> for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
> throttler_idx++) {
> if (throttler_status &
> logging_label[throttler_idx].feature_mask) {
> - throtting_events++;
> + throttling_events++;
> buf_idx += snprintf(log_buf + buf_idx,
> sizeof(log_buf) - buf_idx,
> "%s%s",
> - throtting_events > 1 ? " and " :
> "",
> + throttling_events > 1 ? " and " :
> "",
>
> logging_label[throttler_idx].label);
> if (buf_idx >= sizeof(log_buf)) {
> dev_err(adev->dev, "buffer overflow!\n");
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> index d3b578e6bc2a..fa4ad08099ef 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> @@ -1888,7 +1888,7 @@ static const char *const throttling_logging_label[] = {
>
> static void smu_v13_0_6_log_thermal_throttling_event(struct smu_context
> *smu) {
> - int throttler_idx, throtting_events = 0, buf_idx = 0;
> + int throttler_idx, throttling_events = 0, buf_idx = 0;
> struct
Re: [Intel-xe] [PATCH v2 4/9] drm/sched: Split free_job into own work item
On Thu, Aug 24, 2023 at 01:44:41PM +0200, Christian König wrote: > Am 24.08.23 um 01:12 schrieb Matthew Brost: > > On Wed, Aug 23, 2023 at 01:26:09PM -0400, Rodrigo Vivi wrote: > > > On Wed, Aug 23, 2023 at 11:41:19AM -0400, Alex Deucher wrote: > > > > On Wed, Aug 23, 2023 at 11:26 AM Matthew Brost > > > > wrote: > > > > > On Wed, Aug 23, 2023 at 09:10:51AM +0200, Christian König wrote: > > > > > > Am 23.08.23 um 05:27 schrieb Matthew Brost: > > > > > > > [SNIP] > > > > > > > > That is exactly what I want to avoid, tying the TDR to the job > > > > > > > > is what some > > > > > > > > AMD engineers pushed for because it looked like a simple > > > > > > > > solution and made > > > > > > > > the whole thing similar to what Windows does. > > > > > > > > > > > > > > > > This turned the previous relatively clean scheduler and TDR > > > > > > > > design into a > > > > > > > > complete nightmare. The job contains quite a bunch of things > > > > > > > > which are not > > > > > > > > necessarily available after the application which submitted the > > > > > > > > job is torn > > > > > > > > down. > > > > > > > > > > > > > > > Agree the TDR shouldn't be accessing anything application specific > > > > > > > rather just internal job state required to tear the job down on > > > > > > > the > > > > > > > hardware. > > > > > > > > So what happens is that you either have stale pointers in the > > > > > > > > TDR which can > > > > > > > > go boom extremely easily or we somehow find a way to keep the > > > > > > > > necessary > > > > > > > I have not experenced the TDR going boom in Xe. > > > > > > > > > > > > > > > structures (which include struct thread_info and struct file > > > > > > > > for this driver > > > > > > > > connection) alive until all submissions are completed. > > > > > > > > > > > > > > > In Xe we keep everything alive until all submissions are > > > > > > > completed. By > > > > > > > everything I mean the drm job, entity, scheduler, and VM via a > > > > > > > reference > > > > > > > counting scheme. All of these structures are just kernel state > > > > > > > which can > > > > > > > safely be accessed even if the application has been killed. > > > > > > Yeah, but that might just not be such a good idea from memory > > > > > > management > > > > > > point of view. > > > > > > > > > > > > When you (for example) kill a process all resource from that > > > > > > progress should > > > > > > at least be queued to be freed more or less immediately. > > > > > > > > > > > We do this, the TDR kicks jobs off the hardware as fast as the hw > > > > > interface allows and signals all pending hw fences immediately after. > > > > > Free jobs then is immediately called and the reference count goes to > > > > > zero. I think max time for all of this to occur is a handful of ms. > > > > > > > > > > > What Linux is doing for other I/O operations is to keep the > > > > > > relevant pages > > > > > > alive until the I/O operation is completed, but for GPUs that > > > > > > usually means > > > > > > keeping most of the memory of the process alive and that in turn is > > > > > > really > > > > > > not something you can do. > > > > > > > > > > > > You can of course do this if your driver has a reliable way of > > > > > > killing your > > > > > > submissions and freeing resources in a reasonable amount of time. > > > > > > This > > > > > > should then be done in the flush callback. > > > > > > > > > > > 'flush callback' - Do you mean drm_sched_entity_flush? I looked at > > > > > that > > > > > and think that function doesn't even work for what I tell. It flushes > > > > > the spsc queue but what about jobs on the hardware, how do those get > > > > > killed? > > > > > > > > > > As stated we do via the TDR which is rather clean design and fits with > > > > > our reference couting scheme. > > > > > > > > > > > > If we need to teardown on demand we just set the TDR to a minimum > > > > > > > value and > > > > > > > it kicks the jobs off the hardware, gracefully cleans everything > > > > > > > up and > > > > > > > drops all references. This is a benefit of the 1 to 1 > > > > > > > relationship, not > > > > > > > sure if this works with how AMDGPU uses the scheduler. > > > > > > > > > > > > > > > Delaying application tear down is also not an option because > > > > > > > > then you run > > > > > > > > into massive trouble with the OOM killer (or more generally OOM > > > > > > > > handling). > > > > > > > > See what we do in drm_sched_entity_flush() as well. > > > > > > > > > > > > > > > Not an issue for Xe, we never call drm_sched_entity_flush as our > > > > > > > referencing counting scheme is all jobs are finished before we > > > > > > > attempt > > > > > > > to tear down entity / scheduler. > > > > > > I don't think you can do that upstream. Calling > > > > > > drm_sched_entity_flush() is > > > > > > a must have from your flush callback for the file descriptor. > > > > > > > > > > > Again
Re: [PATCH 0/4] drm/amdgpu: Explicitly add a flexible array at the end of 'struct amdgpu_bo_list' and simplify amdgpu_bo_list_create()
On Tue, Aug 22, 2023 at 9:35 AM Somalapuram, Amaranath wrote: > > > On 8/21/2023 6:30 PM, Shashank Sharma wrote: > > + Amar should be able to help. > > > > Amar, > > > > Can you please check this patch (series if required) with a few IGTs > > and probably with Xonotic as well ? > > > Tested patch series with IGT and Xonoti, > 1st time I observed while launching xonoti, some kernel WARNING. But > unable to reproduced it. > Attaching dmesg. This looks unrelated. I think I saw a display patch that went out this week to fix this. I've applied the patches. We'll see how CI likes them as well. Thanks! Alex > > [ 739.564460] [ cut here ] > [ 739.564466] amdgpu :03:00.0: > drm_WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev)) > [ 739.564518] WARNING: CPU: 15 PID: 1072 at > drivers/gpu/drm/drm_vblank.c:728 > drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x375/0x3d0 [drm] > [ 739.564593] Modules linked in: amdgpu snd_hda_codec_realtek > snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi intel_rapl_msr > intel_rapl_common snd_hda_intel edac_mce_amd binfmt_misc > snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec iommu_v2 snd_hda_core > drm_buddy kvm_amd snd_hwdep gpu_sched nls_iso8859_1 snd_pcm > drm_suballoc_helper drm_ttm_helper kvm ttm snd_seq_midi > drm_display_helper snd_seq_midi_event snd_rawmidi cec rc_core snd_seq > crct10dif_pclmul ghash_clmulni_intel drm_kms_helper sha512_ssse3 > aesni_intel snd_seq_device i2c_algo_bit crypto_simd snd_timer > snd_rn_pci_acp3x syscopyarea cryptd sysfillrect snd_acp_config > snd_soc_acpi joydev input_leds rapl wmi_bmof sysimgblt snd snd_pci_acp3x > ccp k10temp soundcore mac_hid amd_pmc sch_fq_codel msr parport_pc ppdev > lp drm parport ramoops reed_solomon pstore_blk pstore_zone efi_pstore > ip_tables x_tables autofs4 uas usb_storage hid_generic nvme usbhid hid > crc32_pclmul i2c_piix4 r8169 nvme_core ahci xhci_pci libahci realtek > xhci_pci_renesas video wmi gpio_amdpt > [ 739.564703] gpio_generic > [ 739.564707] CPU: 15 PID: 1072 Comm: gnome-shell Not tainted 6.3.0-rc4+ #5 > [ 739.564713] Hardware name: AMD Artic/Artic-RN, BIOS WAF2309N 03/07/2022 > [ 739.564716] RIP: > 0010:drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x375/0x3d0 [drm] > [ 739.564779] Code: 4c 8b 67 50 4d 85 e4 75 03 4c 8b 27 e8 44 23 d9 d3 > 48 c7 c1 58 7a 62 c0 4c 89 e2 48 c7 c7 74 a4 62 c0 48 89 c6 e8 cb 12 4c > d3 <0f> 0b eb 85 4c 8b 65 98 44 8b 5d 94 41 89 c5 4d 85 e4 74 05 4d 8b > [ 739.564782] RSP: 0018:bde6c172faf0 EFLAGS: 00010282 > [ 739.564787] RAX: RBX: c0e89370 RCX: > > [ 739.564790] RDX: 0002 RSI: 950ca531 RDI: > > [ 739.564793] RBP: bde6c172fb60 R08: R09: > dfff > [ 739.564795] R10: bde6c172f958 R11: 95556ec8 R12: > 9d4881cd0820 > [ 739.564798] R13: R14: R15: > 9d4886b9b8a8 > [ 739.564800] FS: 7fac3d72c5c0() GS:9d4da5fc() > knlGS: > [ 739.564804] CS: 0010 DS: ES: CR0: 80050033 > [ 739.564807] CR2: 55e4351b4a38 CR3: 000122ad8000 CR4: > 00350ee0 > [ 739.564810] Call Trace: > [ 739.564813] > [ 739.564817] ? dma_resv_get_singleton+0xb7/0x130 > [ 739.564828] drm_crtc_vblank_helper_get_vblank_timestamp+0x20/0x30 [drm] > [ 739.564889] drm_crtc_get_last_vbltimestamp+0x59/0x90 [drm] > [ 739.564950] drm_crtc_next_vblank_start+0x44/0x80 [drm] > [ 739.565010] drm_atomic_helper_wait_for_fences+0x87/0x1e0 > [drm_kms_helper] > [ 739.565045] drm_atomic_helper_commit+0xa1/0x160 [drm_kms_helper] > [ 739.565076] drm_atomic_commit+0x9d/0xd0 [drm] > [ 739.565172] ? __pfx___drm_printfn_info+0x10/0x10 [drm] > [ 739.565240] drm_mode_atomic_ioctl+0xa0b/0xba0 [drm] > [ 739.565306] ? __pfx_drm_mode_atomic_ioctl+0x10/0x10 [drm] > [ 739.565381] drm_ioctl_kernel+0xbf/0x150 [drm] > [ 739.565474] drm_ioctl+0x29e/0x500 [drm] > [ 739.565539] ? __pfx_drm_mode_atomic_ioctl+0x10/0x10 [drm] > [ 739.565606] ? _raw_spin_unlock_irqrestore+0x2b/0x50 > [ 739.565614] amdgpu_drm_ioctl+0x52/0x90 [amdgpu] > [ 739.566064] __x64_sys_ioctl+0x99/0xd0 > [ 739.566073] do_syscall_64+0x3f/0x90 > [ 739.566079] entry_SYSCALL_64_after_hwframe+0x72/0xdc > [ 739.566086] RIP: 0033:0x7fac40b1aaff > [ 739.566091] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 > 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f > 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00 > [ 739.566094] RSP: 002b:7ffc71f5c250 EFLAGS: 0246 ORIG_RAX: > 0010 > [ 739.566099] RAX: ffda RBX: 7ffc71f5c2f0 RCX: > 7fac40b1aaff > [ 739.566102] RDX: 7ffc71f5c2f0 RSI: c03864bc RDI: > 0009 > [ 739.566104] RBP: c03864bc R08: 0010 R09: > 0010 > [ 739.566107] R10: 0007 R11:
Re: [PATCH AUTOSEL 6.4 10/11] drm/amdkfd: disable IOMMUv2 support for KV/CZ
On Tue, Aug 22, 2023 at 7:36 AM Sasha Levin wrote:
>
> From: Alex Deucher
>
> [ Upstream commit 616f92d188ee7142a95a52068efdbea82645f859 ]
>
> Use the dGPU path instead. There were a lot of platform
> issues with IOMMU in general on these chips due to windows
> not enabling IOMMU at the time. The dGPU path has been
> used for a long time with newer APUs and works fine. This
> also paves the way to simplify the driver significantly.
>
> v2: use the dGPU queue manager functions
>
> Reviewed-by: Felix Kuehling
> Acked-by: Christian König
> Tested-by: Mike Lothian
> Signed-off-by: Alex Deucher
> Signed-off-by: Sasha Levin
This is not necessary for stable.
Alex
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 6 --
> drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 +---
> 2 files changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 00f528eb98126..9c8197573dee7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -224,10 +224,6 @@ static void kfd_device_info_init(struct kfd_dev *kfd,
> asic_type != CHIP_TONGA)
> kfd->device_info.supports_cwsr = true;
>
> - if (asic_type == CHIP_KAVERI ||
> - asic_type == CHIP_CARRIZO)
> - kfd->device_info.needs_iommu_device = true;
> -
> if (asic_type != CHIP_HAWAII && !vf)
> kfd->device_info.needs_pci_atomics = true;
> }
> @@ -240,7 +236,6 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev,
> bool vf)
> uint32_t gfx_target_version = 0;
>
> switch (adev->asic_type) {
> -#ifdef KFD_SUPPORT_IOMMU_V2
> #ifdef CONFIG_DRM_AMDGPU_CIK
> case CHIP_KAVERI:
> gfx_target_version = 7;
> @@ -253,7 +248,6 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev,
> bool vf)
> if (!vf)
> f2g = _v8_kfd2kgd;
> break;
> -#endif
> #ifdef CONFIG_DRM_AMDGPU_CIK
> case CHIP_HAWAII:
> gfx_target_version = 70001;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 7a95698d83f73..c73417e79745e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -2335,18 +2335,12 @@ struct device_queue_manager
> *device_queue_manager_init(struct kfd_dev *dev)
> }
>
> switch (dev->adev->asic_type) {
> - case CHIP_CARRIZO:
> - device_queue_manager_init_vi(>asic_ops);
> - break;
> -
> case CHIP_KAVERI:
> - device_queue_manager_init_cik(>asic_ops);
> - break;
> -
> case CHIP_HAWAII:
> device_queue_manager_init_cik_hawaii(>asic_ops);
> break;
>
> + case CHIP_CARRIZO:
> case CHIP_TONGA:
> case CHIP_FIJI:
> case CHIP_POLARIS10:
> --
> 2.40.1
>
[PATCH 6/6] media: cec: core: add note about *_from_edid() function usage in drm
In the drm subsystem, the source physical address is, in most cases,
available without having to parse the EDID again. Add notes about
preferring to use the pre-parsed address instead.
Cc: Hans Verkuil
Cc: linux-me...@vger.kernel.org
Signed-off-by: Jani Nikula
---
drivers/media/cec/core/cec-adap.c | 4
drivers/media/cec/core/cec-notifier.c | 4
2 files changed, 8 insertions(+)
diff --git a/drivers/media/cec/core/cec-adap.c
b/drivers/media/cec/core/cec-adap.c
index 241b1621b197..2c627ed611ed 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1688,6 +1688,10 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16
phys_addr, bool block)
}
EXPORT_SYMBOL_GPL(cec_s_phys_addr);
+/*
+ * Note: In the drm subsystem, prefer calling cec_s_phys_addr() with
+ * connector->display_info.source_physical_address if possible.
+ */
void cec_s_phys_addr_from_edid(struct cec_adapter *adap,
const struct edid *edid)
{
diff --git a/drivers/media/cec/core/cec-notifier.c
b/drivers/media/cec/core/cec-notifier.c
index 389dc664b211..13f043b3025b 100644
--- a/drivers/media/cec/core/cec-notifier.c
+++ b/drivers/media/cec/core/cec-notifier.c
@@ -195,6 +195,10 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n,
u16 pa)
}
EXPORT_SYMBOL_GPL(cec_notifier_set_phys_addr);
+/*
+ * Note: In the drm subsystem, prefer calling cec_notifier_set_phys_addr() with
+ * connector->display_info.source_physical_address if possible.
+ */
void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n,
const struct edid *edid)
{
--
2.39.2
[PATCH 5/6] drm/i915/cec: switch to setting physical address directly
Avoid parsing the EDID again for source physical address. Also gets rids of a few remaining raw EDID usages. Cc: Hans Verkuil Cc: linux-me...@vger.kernel.org Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 7 ++- drivers/gpu/drm/i915/display/intel_hdmi.c | 5 ++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 7067ee3a4bd3..c4b8e0e74c15 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5198,7 +5198,6 @@ intel_dp_set_edid(struct intel_dp *intel_dp) struct drm_i915_private *i915 = dp_to_i915(intel_dp); struct intel_connector *connector = intel_dp->attached_connector; const struct drm_edid *drm_edid; - const struct edid *edid; bool vrr_capable; intel_dp_unset_edid(intel_dp); @@ -5216,10 +5215,8 @@ intel_dp_set_edid(struct intel_dp *intel_dp) intel_dp_update_dfp(intel_dp, drm_edid); intel_dp_update_420(intel_dp); - /* FIXME: Get rid of drm_edid_raw() */ - edid = drm_edid_raw(drm_edid); - - drm_dp_cec_set_edid(_dp->aux, edid); + drm_dp_cec_attach(_dp->aux, + connector->base.display_info.source_physical_address); } static void diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index aa9915098dda..5d6255ee8b54 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2482,9 +2482,8 @@ intel_hdmi_set_edid(struct drm_connector *connector) intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref); - /* FIXME: Get rid of drm_edid_raw() */ - cec_notifier_set_phys_addr_from_edid(intel_hdmi->cec_notifier, -drm_edid_raw(drm_edid)); + cec_notifier_set_phys_addr(intel_hdmi->cec_notifier, + connector->display_info.source_physical_address); return connected; } -- 2.39.2
[PATCH 3/6] drm/edid: parse source physical address
CEC needs the source physical address. Parsing it is trivial with the
existing EDID CEA DB infrastructure.
Default to CEC_PHYS_ADDR_INVALID (0x) instead of 0 to cater for
easier CEC usage.
Cc: Hans Verkuil
Cc: linux-me...@vger.kernel.org
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 5 +
include/drm/drm_connector.h | 8
2 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1dbb15439468..39dd3f694544 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -29,6 +29,7 @@
*/
#include
+#include
#include
#include
#include
@@ -6192,6 +6193,8 @@ drm_parse_hdmi_vsdb_video(struct drm_connector
*connector, const u8 *db)
info->is_hdmi = true;
+ info->source_physical_address = (db[4] << 8) | db[5];
+
if (len >= 6)
info->dvi_dual = db[6] & 1;
if (len >= 7)
@@ -6470,6 +6473,8 @@ static void drm_reset_display_info(struct drm_connector
*connector)
info->vics_len = 0;
info->quirks = 0;
+
+ info->source_physical_address = CEC_PHYS_ADDR_INVALID;
}
static void update_displayid_info(struct drm_connector *connector,
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index d300fde6c1a4..40a5e7acf2fa 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -816,6 +816,14 @@ struct drm_display_info {
* @quirks: EDID based quirks. Internal to EDID parsing.
*/
u32 quirks;
+
+ /**
+* @source_physical_address: Source Physical Address from HDMI
+* Vendor-Specific Data Block, for CEC usage.
+*
+* Defaults to CEC_PHYS_ADDR_INVALID (0x).
+*/
+ u16 source_physical_address;
};
int drm_display_info_set_bus_formats(struct drm_display_info *info,
--
2.39.2
[PATCH 4/6] drm/cec: add drm_dp_cec_attach() as the non-edid version of set edid
Connectors have source physical address available in display
info. There's no need to parse the EDID again for this. Add
drm_dp_cec_attach() to do this.
Seems like the set_edid/unset_edid naming is a bit specific now that
there's no need to pass the EDID at all, so aim for attach/detach going
forward.
Cc: Hans Verkuil
Cc: linux-me...@vger.kernel.org
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/display/drm_dp_cec.c | 22 +++---
include/drm/display/drm_dp_helper.h | 6 ++
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_cec.c
b/drivers/gpu/drm/display/drm_dp_cec.c
index ae39dc794190..da7a7d357446 100644
--- a/drivers/gpu/drm/display/drm_dp_cec.c
+++ b/drivers/gpu/drm/display/drm_dp_cec.c
@@ -297,7 +297,7 @@ static void drm_dp_cec_unregister_work(struct work_struct
*work)
* were unchanged and just update the CEC physical address. Otherwise
* unregister the old CEC adapter and create a new one.
*/
-void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
+void drm_dp_cec_attach(struct drm_dp_aux *aux, u16 source_physical_address)
{
struct drm_connector *connector = aux->cec.connector;
u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD |
@@ -339,7 +339,7 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const
struct edid *edid)
if (aux->cec.adap->capabilities == cec_caps &&
aux->cec.adap->available_log_addrs == num_las) {
/* Unchanged, so just set the phys addr */
- cec_s_phys_addr_from_edid(aux->cec.adap, edid);
+ cec_s_phys_addr(adap, source_physical_address, false);
goto unlock;
}
/*
@@ -370,11 +370,27 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const
struct edid *edid)
* from drm_dp_cec_register_connector() edid == NULL, so in
* that case the phys addr is just invalidated.
*/
- cec_s_phys_addr_from_edid(aux->cec.adap, edid);
+ cec_s_phys_addr(adap, source_physical_address, false);
}
unlock:
mutex_unlock(>cec.lock);
}
+EXPORT_SYMBOL(drm_dp_cec_attach);
+
+/*
+ * Note: Prefer calling drm_dp_cec_attach() with
+ * connector->display_info.source_physical_address if possible.
+ */
+void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
+{
+ u16 source_physical_address = CEC_PHYS_ADDR_INVALID;
+
+ if (edid && edid->extensions)
+ pa = cec_get_edid_phys_addr((const u8 *)edid,
+ EDID_LENGTH * (edid->extensions +
1), NULL);
+
+ drm_dp_cec_attach(aux, source_physical_address);
+}
EXPORT_SYMBOL(drm_dp_cec_set_edid);
/*
diff --git a/include/drm/display/drm_dp_helper.h
b/include/drm/display/drm_dp_helper.h
index 86f24a759268..3369104e2d25 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -699,6 +699,7 @@ void drm_dp_cec_irq(struct drm_dp_aux *aux);
void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
struct drm_connector *connector);
void drm_dp_cec_unregister_connector(struct drm_dp_aux *aux);
+void drm_dp_cec_attach(struct drm_dp_aux *aux, u16 source_physical_address);
void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid);
void drm_dp_cec_unset_edid(struct drm_dp_aux *aux);
#else
@@ -716,6 +717,11 @@ static inline void drm_dp_cec_unregister_connector(struct
drm_dp_aux *aux)
{
}
+static inline void drm_dp_cec_attach(struct drm_dp_aux *aux,
+u16 source_physical_address)
+{
+}
+
static inline void drm_dp_cec_set_edid(struct drm_dp_aux *aux,
const struct edid *edid)
{
--
2.39.2
[PATCH 1/6] drm/edid: add drm_edid_is_digital()
Checking edid->input & DRM_EDID_INPUT_DIGITAL is common enough to
deserve a helper that also lets us abstract the raw EDID a bit better.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 17 +++--
include/drm/drm_edid.h | 1 +
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 340da8257b51..1dbb15439468 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3110,7 +3110,7 @@ drm_monitor_supports_rb(const struct drm_edid *drm_edid)
return ret;
}
- return ((drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
+ return drm_edid_is_digital(drm_edid);
}
static void
@@ -6519,7 +6519,7 @@ static void update_display_info(struct drm_connector
*connector,
if (edid->revision < 3)
goto out;
- if (!(edid->input & DRM_EDID_INPUT_DIGITAL))
+ if (!drm_edid_is_digital(drm_edid))
goto out;
info->color_formats |= DRM_COLOR_FORMAT_RGB444;
@@ -7335,3 +7335,16 @@ static void _drm_update_tile_info(struct drm_connector
*connector,
connector->tile_group = NULL;
}
}
+
+/**
+ * drm_edid_is_digital - is digital?
+ * @drm_edid: The EDID
+ *
+ * Return true if input is digital.
+ */
+bool drm_edid_is_digital(const struct drm_edid *drm_edid)
+{
+ return drm_edid && drm_edid->edid &&
+ drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL;
+}
+EXPORT_SYMBOL(drm_edid_is_digital);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 48e93f909ef6..882d2638708e 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -612,6 +612,7 @@ const struct drm_edid *drm_edid_read_switcheroo(struct
drm_connector *connector,
int drm_edid_connector_update(struct drm_connector *connector,
const struct drm_edid *edid);
int drm_edid_connector_add_modes(struct drm_connector *connector);
+bool drm_edid_is_digital(const struct drm_edid *drm_edid);
const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
int ext_id, int *ext_index);
--
2.39.2
[PATCH 2/6] drm/i915/display: use drm_edid_is_digital()
Reduce the use of struct edid and drm_edid_raw().
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/display/intel_crt.c | 11 ---
drivers/gpu/drm/i915/display/intel_hdmi.c | 9 -
drivers/gpu/drm/i915/display/intel_sdvo.c | 7 ++-
3 files changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_crt.c
b/drivers/gpu/drm/i915/display/intel_crt.c
index f66340b4caf0..310670bb6c25 100644
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -657,21 +657,18 @@ static bool intel_crt_detect_ddc(struct drm_connector
*connector)
drm_edid = intel_crt_get_edid(connector, i2c);
if (drm_edid) {
- const struct edid *edid = drm_edid_raw(drm_edid);
- bool is_digital = edid->input & DRM_EDID_INPUT_DIGITAL;
-
/*
* This may be a DVI-I connector with a shared DDC
* link between analog and digital outputs, so we
* have to check the EDID input spec of the attached device.
*/
- if (!is_digital) {
+ if (drm_edid_is_digital(drm_edid)) {
drm_dbg_kms(_priv->drm,
- "CRT detected via DDC:0x50 [EDID]\n");
- ret = true;
+ "CRT not detected via DDC:0x50 [EDID
reports a digital panel]\n");
} else {
drm_dbg_kms(_priv->drm,
- "CRT not detected via DDC:0x50 [EDID
reports a digital panel]\n");
+ "CRT detected via DDC:0x50 [EDID]\n");
+ ret = true;
}
} else {
drm_dbg_kms(_priv->drm,
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9442bf43550e..aa9915098dda 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2452,7 +2452,6 @@ intel_hdmi_set_edid(struct drm_connector *connector)
struct intel_hdmi *intel_hdmi =
intel_attached_hdmi(to_intel_connector(connector));
intel_wakeref_t wakeref;
const struct drm_edid *drm_edid;
- const struct edid *edid;
bool connected = false;
struct i2c_adapter *i2c;
@@ -2475,9 +2474,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
to_intel_connector(connector)->detect_edid = drm_edid;
- /* FIXME: Get rid of drm_edid_raw() */
- edid = drm_edid_raw(drm_edid);
- if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
+ if (drm_edid_is_digital(drm_edid)) {
intel_hdmi_dp_dual_mode_detect(connector);
connected = true;
@@ -2485,7 +2482,9 @@ intel_hdmi_set_edid(struct drm_connector *connector)
intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
- cec_notifier_set_phys_addr_from_edid(intel_hdmi->cec_notifier, edid);
+ /* FIXME: Get rid of drm_edid_raw() */
+ cec_notifier_set_phys_addr_from_edid(intel_hdmi->cec_notifier,
+drm_edid_raw(drm_edid));
return connected;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 7d25a64698e2..917771e19e38 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2094,10 +2094,8 @@ intel_sdvo_tmds_sink_detect(struct drm_connector
*connector)
status = connector_status_unknown;
if (drm_edid) {
- const struct edid *edid = drm_edid_raw(drm_edid);
-
/* DDC bus is shared, match EDID to connector type */
- if (edid && edid->input & DRM_EDID_INPUT_DIGITAL)
+ if (drm_edid_is_digital(drm_edid))
status = connector_status_connected;
else
status = connector_status_disconnected;
@@ -2111,8 +2109,7 @@ static bool
intel_sdvo_connector_matches_edid(struct intel_sdvo_connector *sdvo,
const struct drm_edid *drm_edid)
{
- const struct edid *edid = drm_edid_raw(drm_edid);
- bool monitor_is_digital = !!(edid->input & DRM_EDID_INPUT_DIGITAL);
+ bool monitor_is_digital = drm_edid_is_digital(drm_edid);
bool connector_is_digital = !!IS_DIGITAL(sdvo);
DRM_DEBUG_KMS("connector_is_digital? %d, monitor_is_digital? %d\n",
--
2.39.2
[PATCH 0/6] drm, cec and edid updates
Avoid accessing the raw edid directly. Pre-parse the source physical address during normal EDID parsing and use that for CEC. Jani Nikula (6): drm/edid: add drm_edid_is_digital() drm/i915/display: use drm_edid_is_digital() drm/edid: parse source physical address drm/cec: add drm_dp_cec_attach() as the non-edid version of set edid drm/i915/cec: switch to setting physical address directly media: cec: core: add note about *_from_edid() function usage in drm drivers/gpu/drm/display/drm_dp_cec.c | 22 +++--- drivers/gpu/drm/drm_edid.c| 22 -- drivers/gpu/drm/i915/display/intel_crt.c | 11 --- drivers/gpu/drm/i915/display/intel_dp.c | 7 ++- drivers/gpu/drm/i915/display/intel_hdmi.c | 8 +++- drivers/gpu/drm/i915/display/intel_sdvo.c | 7 ++- drivers/media/cec/core/cec-adap.c | 4 drivers/media/cec/core/cec-notifier.c | 4 include/drm/display/drm_dp_helper.h | 6 ++ include/drm/drm_connector.h | 8 include/drm/drm_edid.h| 1 + 11 files changed, 73 insertions(+), 27 deletions(-) -- 2.39.2
Re: [PATCH 15/20] drm/tegra/hub: Increase buffer size to ensure all possible values can be stored
On Thu, Aug 24, 2023 at 01:01:24PM +0100, Lee Jones wrote: > On Thu, 24 Aug 2023, Jani Nikula wrote: > > > On Thu, 24 Aug 2023, Thierry Reding wrote: > > > On Thu, Aug 24, 2023 at 08:37:00AM +0100, Lee Jones wrote: > > >> When converting from int to string, we must allow for up to 10-chars > > >> (2147483647). > > >> > > >> Fixes the following W=1 kernel build warning(s): > > >> > > >> drivers/gpu/drm/tegra/hub.c: In function ‘tegra_display_hub_probe’: > > >> drivers/gpu/drm/tegra/hub.c:1106:47: warning: ‘%u’ directive output may > > >> be truncated writing between 1 and 10 bytes into a region of size 4 > > >> [-Wformat-truncation=] > > >> drivers/gpu/drm/tegra/hub.c:1106:42: note: directive argument in the > > >> range [0, 4294967294] > > >> drivers/gpu/drm/tegra/hub.c:1106:17: note: ‘snprintf’ output between 6 > > >> and 15 bytes into a destination of size 8 > > > > > > I wish there was (perhaps there is?) a better way to annotate that i > > > will always be within a given range. In practice this is always going to > > > be smaller than 10 and even in future hardware I wouldn't expect this to > > > ever exceed anything like 32 or 64, so 8 characters is already generous. > > > > I assume you could do > > > > snprintf(id, sizeof(id), "wgrp%u", (unsigned char)i); > > > > but it's perhaps less obvious than just increasing the size of the > > buffer. > > I had the very same thought process. It's not a big deal, this happens all on the stack, so adding a couple of bytes isn't going to hurt very much. Still with all the tooling that we have it'd be nice if something could be added. Perhaps an alternative would be to reject values for num_wgrp that are bigger than 32. With that the compiler might have enough information not to warn about this. Anyway, no need to spend any more time on this, I'm fine with the patch as-is. Thierry signature.asc Description: PGP signature
[PATCH] drm/amd/display: enable cursor degamma for DCN3+ DRM legacy gamma
For DRM legacy gamma, AMD display manager applies implicit sRGB degamma
using a pre-defined sRGB transfer function. It works fine for DCN2
family where degamma ROM and custom curves go to the same color block.
But, on DCN3+, degamma is split into two blocks: degamma ROM for
pre-defined TFs and `gamma correction` for user/custom curves and
degamma ROM settings doesn't apply to cursor plane. To get DRM legacy
gamma working as expected, enable cursor degamma ROM for implict sRGB
degamma on HW with this configuration.
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2803
Fixes: 96b020e2163f ("drm/amd/display: check attr flag before set cursor
degamma on DCN3+")
Signed-off-by: Melissa Wen
---
Hi,
It seems that the previous color fix for atomic API brought out a
difference in behavior of degamma color blocks between DCN2 and DCN3, as
reported in the link.
AFAIU, settings of the `degamma ROM` block for pre-defined TF doesn't
apply to cursor plane. So, whenever we wants degamma ROM for cursor
plane, we have to explicitly enable it using the attribute flag. This is
the case when we do an implicit sRGB degamma to match DRM legacy gamma
requirements.
Another option would be changing the legacy gamma approach to use the
`gamma correction` block where the pre-defined sRGB curve is calculated
by AMD color module. I think that keeping degamma ROM usage on legacy
gamma is better for performance, this is why I opted for this patch. But
let me know if changing legacy gamma implementation in amdgpu_dm_color
is better for consistence or any other thing to take into account.
Thanks,
Melissa
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 8eeca160d434..2aa7efd798e2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1269,6 +1269,13 @@ void amdgpu_dm_plane_handle_cursor_update(struct
drm_plane *plane,
attributes.rotation_angle= 0;
attributes.attribute_flags.value = 0;
+ /* Enable cursor degamma ROM on DCN3+ for implicit sRGB degamma in DRM
+* legacy gamma setup.
+*/
+ if (crtc_state->cm_is_degamma_srgb &&
+ adev->dm.dc->caps.color.dpp.gamma_corr)
+ attributes.attribute_flags.bits.ENABLE_CURSOR_DEGAMMA = 1;
+
attributes.pitch = afb->base.pitches[0] / afb->base.format->cpp[0];
if (crtc_state->stream) {
--
2.40.1
[PATCH 2/2] drivers/drm/i915: Honor limits->max_bpp while computing DSC max input bpp
Edid specific BPC constraints are stored in limits->max_bpp. Honor these
limits while computing the input bpp for DSC.
v2: Use int instead of u8 for computations. (Jani)
Add closes tag. (Ankit)
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9161
Signed-off-by: Ankit Nautiyal
Reviewed-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/display/intel_dp.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 7067ee3a4bd3..8f3dc79089ea 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2061,9 +2061,10 @@ static int intel_edp_dsc_compute_pipe_bpp(struct
intel_dp *intel_dp,
if (forced_bpp) {
pipe_bpp = forced_bpp;
} else {
+ int max_bpc = min(limits->max_bpp / 3,
(int)conn_state->max_requested_bpc);
+
/* For eDP use max bpp that can be supported with DSC. */
- pipe_bpp = intel_dp_dsc_compute_max_bpp(intel_dp,
-
conn_state->max_requested_bpc);
+ pipe_bpp = intel_dp_dsc_compute_max_bpp(intel_dp, max_bpc);
if (!is_dsc_pipe_bpp_sufficient(i915, conn_state, limits,
pipe_bpp)) {
drm_dbg_kms(>drm,
"Computed BPC is not in DSC BPC limits\n");
--
2.40.1
[PATCH 1/2] drm/display/dp: Assume 8 bpc support when DSC is supported
As per DP v1.4, a DP DSC Sink device shall support 8bpc in DPCD 6Ah. Apparently some panels that do support DSC, are not setting the bit for 8bpc. So always assume 8bpc support by DSC decoder, when DSC is claimed to be supported. v2: Use helper to get check dsc support. (Ankit) v3: Fix styling and other typos. (Jani) Signed-off-by: Ankit Nautiyal --- drivers/gpu/drm/display/drm_dp_helper.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index e6a78fd32380..8a1b64c57dfd 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -2449,12 +2449,16 @@ int drm_dp_dsc_sink_supported_input_bpcs(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_S int num_bpc = 0; u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT]; + if (!drm_dp_sink_supports_dsc(dsc_dpcd)) + return 0; + if (color_depth & DP_DSC_12_BPC) dsc_bpc[num_bpc++] = 12; if (color_depth & DP_DSC_10_BPC) dsc_bpc[num_bpc++] = 10; - if (color_depth & DP_DSC_8_BPC) - dsc_bpc[num_bpc++] = 8; + + /* A DP DSC Sink device shall support 8 bpc. */ + dsc_bpc[num_bpc++] = 8; return num_bpc; } -- 2.40.1
[PATCH 0/2] eDP DSC fixes
Assume 8bpc is supported if Sink claims DSC support. Also consider bpc constraint coming from EDID while computing input BPC for DSC. Rev2: Fix check for dsc support. Rev3: Minor styling and typos fix. Ankit Nautiyal (2): drm/display/dp: Assume 8 bpc support when DSC is supported drivers/drm/i915: Honor limits->max_bpp while computing DSC max input bpp drivers/gpu/drm/display/drm_dp_helper.c | 8 ++-- drivers/gpu/drm/i915/display/intel_dp.c | 5 +++-- 2 files changed, 9 insertions(+), 4 deletions(-) -- 2.40.1
Re: [PATCH 18/20] drm/drm_gpuva_mgr: Remove set but unused variable 'prev'
Hi Lee,
On 8/24/23 09:37, Lee Jones wrote:
Fixes the following W=1 kernel build warning(s):
drivers/gpu/drm/drm_gpuva_mgr.c: In function ‘__drm_gpuva_sm_map’:
drivers/gpu/drm/drm_gpuva_mgr.c:1079:39: warning: variable ‘prev’ set but not
used [-Wunused-but-set-variable]
Thanks for fixing this up.
However, I already had a patch in the queue addressing the warning,
which I already applied to drm-misc/drm-misc-next-fixes.
- Danilo
Signed-off-by: Lee Jones
---
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
Cc: Daniel Vetter
Cc: Sumit Semwal
Cc: "Christian König"
Cc: Danilo Krummrich
Cc: dri-devel@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/gpu/drm/drm_gpuva_mgr.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpuva_mgr.c b/drivers/gpu/drm/drm_gpuva_mgr.c
index f86bfad74ff8a..ad99c9cfedac7 100644
--- a/drivers/gpu/drm/drm_gpuva_mgr.c
+++ b/drivers/gpu/drm/drm_gpuva_mgr.c
@@ -1076,7 +1076,7 @@ __drm_gpuva_sm_map(struct drm_gpuva_manager *mgr,
u64 req_addr, u64 req_range,
struct drm_gem_object *req_obj, u64 req_offset)
{
- struct drm_gpuva *va, *next, *prev = NULL;
+ struct drm_gpuva *va, *next;
u64 req_end = req_addr + req_range;
int ret;
@@ -1106,7 +1106,7 @@ __drm_gpuva_sm_map(struct drm_gpuva_manager *mgr,
ret = op_unmap_cb(ops, priv, va, merge);
if (ret)
return ret;
- goto next;
+ continue;
}
if (end > req_end) {
@@ -1151,7 +1151,7 @@ __drm_gpuva_sm_map(struct drm_gpuva_manager *mgr,
ret = op_remap_cb(ops, priv, , NULL, );
if (ret)
return ret;
- goto next;
+ continue;
}
if (end > req_end) {
@@ -1184,7 +1184,7 @@ __drm_gpuva_sm_map(struct drm_gpuva_manager *mgr,
ret = op_unmap_cb(ops, priv, va, merge);
if (ret)
return ret;
- goto next;
+ continue;
}
if (end > req_end) {
@@ -1205,8 +1205,6 @@ __drm_gpuva_sm_map(struct drm_gpuva_manager *mgr,
break;
}
}
-next:
- prev = va;
}
return op_map_cb(ops, priv,
[PULL] drm-intel-fixes
Hi Dave and Daniel, And this is our fixes targeting 6.5 (rc8?). I'm again covering for Tvrtko at this round. Please also notice that here we also have the drm patches fixing the HPD polling that I had mentioned in our next-fixes. One is the fix itself and the other is a dependency to add the helper to reschedule the poll work. Both patches also targeting stable 6.4+. drm-intel-fixes-2023-08-24: - Fix power consumption at s2idle on DG2 (Anshuman) - Fix documentation build warning (Jani) - Fix Display HPD (Imre) Thanks, Rodrigo. The following changes since commit 706a741595047797872e669b3101429ab8d378ef: Linux 6.5-rc7 (2023-08-20 15:02:52 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2023-08-24 for you to fetch changes up to 1dcc437427bbcebc8381226352f7ade08a271191: drm/i915: Fix HPD polling, reenabling the output poll work as needed (2023-08-23 17:10:57 -0400) - Fix power consumption at s2idle on DG2 (Anshuman) - Fix documentation build warning (Jani) - Fix Display HPD (Imre) Anshuman Gupta (1): drm/i915/dgfx: Enable d3cold at s2idle Imre Deak (2): drm: Add an HPD poll helper to reschedule the poll work drm/i915: Fix HPD polling, reenabling the output poll work as needed Jani Nikula (1): drm/i915: fix Sphinx indentation warning drivers/gpu/drm/drm_probe_helper.c | 68 +++- drivers/gpu/drm/i915/display/intel_hotplug.c | 4 +- drivers/gpu/drm/i915/gt/uc/intel_huc.c | 2 + drivers/gpu/drm/i915/i915_driver.c | 33 -- include/drm/drm_probe_helper.h | 1 + 5 files changed, 69 insertions(+), 39 deletions(-)
[PULL] drm-intel-next-fixes
Hi Dave and Daniel, Here goes our next-fixes targeting 6.6-rc1. Please notice that we have 2 drm level patches there, one to fix the display HPD polling and one dependency introducing a helper to reschedule the poll work. drm-intel-next-fixes-2023-08-24: - Fix TLB invalidation (Alan) - Fix Display HPD polling (Imre) Thanks, Rodrigo. The following changes since commit cacaeb27ade4b793c456179bb6eda4592d206cd8: Merge tag 'amd-drm-next-6.6-2023-08-18' of https://gitlab.freedesktop.org/agd5f/linux into drm-next (2023-08-21 12:32:16 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-fixes-2023-08-24 for you to fetch changes up to cfd48ad8c4a9137b0fde7f0ecf463d44b01875dc: drm/i915: Fix HPD polling, reenabling the output poll work as needed (2023-08-23 17:15:41 -0400) - Fix TLB invalidation (Alan) - Fix Display HPD polling (Imre) Alan Previn (1): drm/i915: Fix TLB-Invalidation seqno store Imre Deak (2): drm: Add an HPD poll helper to reschedule the poll work drm/i915: Fix HPD polling, reenabling the output poll work as needed drivers/gpu/drm/drm_probe_helper.c | 68 +++- drivers/gpu/drm/i915/display/intel_hotplug.c | 4 +- drivers/gpu/drm/i915/i915_vma.c | 2 +- include/drm/drm_probe_helper.h | 1 + 4 files changed, 50 insertions(+), 25 deletions(-)
Re: [PATCH (set 1) 00/20] Rid W=1 warnings from GPU
On Thu, 24 Aug 2023, Hamza Mahfooz wrote: > > On 8/24/23 08:07, Lee Jones wrote: > > On Thu, 24 Aug 2023, Jani Nikula wrote: > > > > > On Thu, 24 Aug 2023, Lee Jones wrote: > > > > This set is part of a larger effort attempting to clean-up W=1 > > > > kernel builds, which are currently overwhelmingly riddled with > > > > niggly little warnings. > > > > > > The next question is, how do we keep it W=1 clean going forward? > > > > My plan was to fix them all, then move each warning to W=0. > > > > Arnd recently submitted a set doing just that for a bunch of them. > > > > https://lore.kernel.org/all/20230811140327.3754597-1-a...@kernel.org/ > > > > I like to think a bunch of this is built on top of my previous efforts. > > > > GPU is a particularly tricky though - the warnings seem to come in faster > > than I can squash them. Maybe the maintainers can find a way to test > > new patches on merge? > > I guess on that note, do you know if there is a way to run > `scripts/kernel-doc` on patches instead of whole files? That would make > much easier to block new kernel-doc issues from appearing. Not off hand. When I run builds on patches I author, I run them twice concurrently. Once on the commit I'm basing on and once on the HEAD of my patchset. I then diff the two. So as long as the number of errors and warnings stay the same or reduce, we're golden. Perhaps the same method could be used with `kernel-doc`? -- Lee Jones [李琼斯]
Re: [PATCH (set 1) 00/20] Rid W=1 warnings from GPU
On 8/24/23 08:07, Lee Jones wrote: On Thu, 24 Aug 2023, Jani Nikula wrote: On Thu, 24 Aug 2023, Lee Jones wrote: This set is part of a larger effort attempting to clean-up W=1 kernel builds, which are currently overwhelmingly riddled with niggly little warnings. The next question is, how do we keep it W=1 clean going forward? My plan was to fix them all, then move each warning to W=0. Arnd recently submitted a set doing just that for a bunch of them. https://lore.kernel.org/all/20230811140327.3754597-1-a...@kernel.org/ I like to think a bunch of this is built on top of my previous efforts. GPU is a particularly tricky though - the warnings seem to come in faster than I can squash them. Maybe the maintainers can find a way to test new patches on merge? I guess on that note, do you know if there is a way to run `scripts/kernel-doc` on patches instead of whole files? That would make much easier to block new kernel-doc issues from appearing. Most people don't use W=1 because it's too noisy, so it's a bit of a catch-22. In i915, we enable a lot of W=1 warnings using subdir-ccflags-y in our Makefile. For CI/developer use we also enable kernel-doc warnings by default. Should we start enabling some of those warning flags in drm/Makefile to to keep the entire subsystem warning free? That would we awesome! We'd just need buy-in. -- Hamza
Re: [PATCH 13/20] drm/tests/drm_kunit_helpers: Correct possible double-entry typo in 'ddrm_kunit_helper_acquire_ctx_alloc'
On Thu, 24 Aug 2023, Maxime Ripard wrote: > Hi, > > On Thu, Aug 24, 2023 at 08:36:58AM +0100, Lee Jones wrote: > > Fixes the following W=1 kernel build warning(s): > > > > drivers/gpu/drm/tests/drm_kunit_helpers.c:172: warning: expecting > > prototype for ddrm_kunit_helper_acquire_ctx_alloc(). Prototype was for > > drm_kunit_helper_acquire_ctx_alloc() instead > > > > Signed-off-by: Lee Jones > > --- > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Maxime Ripard > > Cc: "Maíra Canal" > > Cc: dri-devel@lists.freedesktop.org > > --- > > drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c > > b/drivers/gpu/drm/tests/drm_kunit_helpers.c > > index f102c23eee1dd..c1dfbfcaa0001 100644 > > --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c > > +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c > > @@ -156,7 +156,7 @@ static void action_drm_release_context(void *ptr) > > } > > > > /** > > - * ddrm_kunit_helper_acquire_ctx_alloc - Allocates an acquire context > > + * drm_kunit_helper_acquire_ctx_alloc - Allocates an acquire context > > * @test: The test context object > > * > > * Allocates and initializes a modeset acquire context. > > The typo was added by your patch 9. Whoops! Good catch. > > I've applied and squashed them both Perfect, thank you. -- Lee Jones [李琼斯]
Re: [PATCH (set 1) 00/20] Rid W=1 warnings from GPU
On Thu, 24 Aug 2023, Maxime Ripard wrote: > Hi, > > On Thu, Aug 24, 2023 at 10:59:54AM +0200, Maxime Ripard wrote: > > On Thu, 24 Aug 2023 08:36:45 +0100, Lee Jones wrote: > > > This set is part of a larger effort attempting to clean-up W=1 > > > kernel builds, which are currently overwhelmingly riddled with > > > niggly little warnings. > > > > > > Cc: Alex Deucher > > > Cc: amd-...@lists.freedesktop.org > > > Cc: Ben Skeggs > > > Cc: "Christian König" > > > Cc: Daniel Vetter > > > Cc: Danilo Krummrich > > > Cc: David Airlie > > > Cc: dri-devel@lists.freedesktop.org > > > Cc: Fabio Estevam > > > Cc: Gourav Samaiya > > > Cc: Hawking Zhang > > > Cc: Hyun Kwon > > > Cc: Jerome Glisse > > > Cc: Jonathan Hunter > > > Cc: Karol Herbst > > > Cc: Laurent Pinchart > > > Cc: linaro-mm-...@lists.linaro.org > > > Cc: linux-arm-ker...@lists.infradead.org > > > Cc: linux-me...@vger.kernel.org > > > Cc: linux-te...@vger.kernel.org > > > Cc: Luben Tuikov > > > Cc: Lyude Paul > > > Cc: Maarten Lankhorst > > > Cc: "Maíra Canal" > > > Cc: Mario Limonciello > > > Cc: Maxime Ripard > > > Cc: Michal Simek > > > Cc: Mikko Perttunen > > > Cc: nouv...@lists.freedesktop.org > > > Cc: NXP Linux Team > > > Cc: "Pan, Xinhui" > > > Cc: Pengutronix Kernel Team > > > Cc: Philipp Zabel > > > Cc: Sascha Hauer > > > Cc: Shashank Sharma > > > Cc: Shawn Guo > > > Cc: Stanley Yang > > > Cc: Sumit Semwal > > > Cc: Thierry Reding > > > Cc: Thomas Zimmermann > > > > > > [...] > > > > Applied to drm/drm-misc (drm-misc-fixes). > > I got confused with b4 usage, but that wasn't actually applied. Only the > three patches I explicitly mentioned were, sorry for the confusion. :) Thanks Maxime. -- Lee Jones [李琼斯]
Re: [PATCH (set 1) 00/20] Rid W=1 warnings from GPU
On Thu, 24 Aug 2023, Lee Jones wrote: > On Thu, 24 Aug 2023, Jani Nikula wrote: > > > On Thu, 24 Aug 2023, Lee Jones wrote: > > > This set is part of a larger effort attempting to clean-up W=1 > > > kernel builds, which are currently overwhelmingly riddled with > > > niggly little warnings. > > > > The next question is, how do we keep it W=1 clean going forward? > > My plan was to fix them all, then move each warning to W=0. Some history: - Starting with v5.8-rc1: 18867 - 2020-07-01: 18089 - 2020-07-07: 17288 - 2020-07-17: 15762 - 2020-07-20: 15724 - 2020-07-23: 15116 - 2020-08-12: 15184 - 2020-10-19: 10909 - 2020-11-04:9385 - 2021-01-04:5478 - 2021-01-12 4749 - 2021-01-29 4911 - 2021-04-07 3594 - 2021-05-20 2938 - 2021-07-01 2587 - 2023-02-10 2587 - 2023-08-22 1650 > Arnd recently submitted a set doing just that for a bunch of them. > > https://lore.kernel.org/all/20230811140327.3754597-1-a...@kernel.org/ > > I like to think a bunch of this is built on top of my previous efforts. > > GPU is a particularly tricky though - the warnings seem to come in faster > than I can squash them. Maybe the maintainers can find a way to test > new patches on merge? > > > Most people don't use W=1 because it's too noisy, so it's a bit of a > > catch-22. > > > > In i915, we enable a lot of W=1 warnings using subdir-ccflags-y in our > > Makefile. For CI/developer use we also enable kernel-doc warnings by > > default. > > > > Should we start enabling some of those warning flags in drm/Makefile to > > to keep the entire subsystem warning free? > > That would we awesome! We'd just need buy-in. -- Lee Jones [李琼斯]
