Re: [PATCH 3/5] drm/bridge: samsung-dsim: update PLL reference clock
2023년 8월 29일 (화) 오전 12:59, Michael Tretter 님이 작성:
>
> The PLL requires a clock between 2 MHz and 30 MHz after the pre-divider.
> The reference clock for the PLL may change due to changes to it's parent
> clock. Thus, the frequency may be out of range or unsuited for
> generating the high speed clock for MIPI DSI.
>
> Try to keep the pre-devider small, and set the reference clock close to
> 30 MHz before recalculating the PLL configuration. Use a divider with a
> power of two for the reference clock as this seems to work best in
> my tests.
Clock frequency is specific to SoC architecture so we have to handle
it carefully because samsung-dsim.c is a common module for I.MX and
Exynos series.
You mentioned "The PLL requires a clock between 2 MHz and 3MHz after
pre-divider", and the clock means that fin_pll - PFD input frequency -
which can be calculated with oscillator clock frequency / P value?
According to Exynos datasheet, the fin_pll should be 6 ~ 12Mhz.
For example,
In case of Exyhos, we use 24MHz as oscillator clock frequency, so
fin_pll frequency, 8MHz = 24MHz / P(3).
Can you tell me the source of the constraint that clocks must be
between 2MHz and 30MHz?
To other I.MX and Exynos engineers, please do not merge this patch
until two SoC platforms are tested correctly.
Thanks,
Inki Dae
>
> Signed-off-by: Michael Tretter
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 15 +--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index da90c2038042..4de6e4f116db 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -611,10 +611,21 @@ static unsigned long samsung_dsim_set_pll(struct
> samsung_dsim *dsi,
> u16 m;
> u32 reg;
>
> - if (dsi->pll_clk)
> + if (dsi->pll_clk) {
> + /*
> +* Ensure that the reference clock is generated with a power
> of
> +* two divider from its parent, but close to the PLLs upper
> +* limit of the valid range of 2 MHz to 30 MHz.
> +*/
> + fin = clk_get_rate(clk_get_parent(dsi->pll_clk));
> + while (fin > 30 * MHZ)
> + fin = fin / 2;
> + clk_set_rate(dsi->pll_clk, fin);
> +
> fin = clk_get_rate(dsi->pll_clk);
> - else
> + } else {
> fin = dsi->pll_clk_rate;
> + }
> dev_dbg(dsi->dev, "PLL ref clock freq %lu\n", fin);
>
> fout = samsung_dsim_pll_find_pms(dsi, fin, freq, , , );
>
> --
> 2.39.2
>
Re: [PATCH 2/5] drm/bridge: samsung-dsim: reread ref clock before configuring PLL
2023년 8월 29일 (화) 오전 12:59, Michael Tretter 님이 작성:
>
> The PLL reference clock may change at runtime. Thus, reading the clock
> rate during probe is not sufficient to correctly configure the PLL for
> the expected hs clock.
>
> Read the actual rate of the reference clock before calculating the PLL
> configuration parameters.
>
> Signed-off-by: Michael Tretter
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 16 +---
> include/drm/bridge/samsung-dsim.h | 1 +
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 6778f1751faa..da90c2038042 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -611,7 +611,12 @@ static unsigned long samsung_dsim_set_pll(struct
> samsung_dsim *dsi,
> u16 m;
> u32 reg;
>
> - fin = dsi->pll_clk_rate;
> + if (dsi->pll_clk)
> + fin = clk_get_rate(dsi->pll_clk);
> + else
> + fin = dsi->pll_clk_rate;
> + dev_dbg(dsi->dev, "PLL ref clock freq %lu\n", fin);
> +
Could you share us the actual use case that in runtime the pll
reference clock can be changed?
This patch is trying to change clock binding behavior which is
described in dt binding[1]
[1] Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
It says,
"DISM oscillator clock frequency. If absent, the clock frequency of
sclk_mipi will be used instead."
However, this patch makes the sclk_mipi to be used first.
Thanks,
Inki Dae
> fout = samsung_dsim_pll_find_pms(dsi, fin, freq, , , );
> if (!fout) {
> dev_err(dsi->dev,
> @@ -1821,18 +1826,15 @@ static int samsung_dsim_parse_dt(struct samsung_dsim
> *dsi)
> u32 lane_polarities[5] = { 0 };
> struct device_node *endpoint;
> int i, nr_lanes, ret;
> - struct clk *pll_clk;
>
> ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
>>pll_clk_rate, 1);
> /* If it doesn't exist, read it from the clock instead of failing */
> if (ret < 0) {
> dev_dbg(dev, "Using sclk_mipi for pll clock frequency\n");
> - pll_clk = devm_clk_get(dev, "sclk_mipi");
> - if (!IS_ERR(pll_clk))
> - dsi->pll_clk_rate = clk_get_rate(pll_clk);
> - else
> - return PTR_ERR(pll_clk);
> + dsi->pll_clk = devm_clk_get(dev, "sclk_mipi");
> + if (IS_ERR(dsi->pll_clk))
> + return PTR_ERR(dsi->pll_clk);
> }
>
> /* If it doesn't exist, use pixel clock instead of failing */
> diff --git a/include/drm/bridge/samsung-dsim.h
> b/include/drm/bridge/samsung-dsim.h
> index 05100e91ecb9..31ff88f152fb 100644
> --- a/include/drm/bridge/samsung-dsim.h
> +++ b/include/drm/bridge/samsung-dsim.h
> @@ -87,6 +87,7 @@ struct samsung_dsim {
> void __iomem *reg_base;
> struct phy *phy;
> struct clk **clks;
> + struct clk *pll_clk;
> struct regulator_bulk_data supplies[2];
> int irq;
> struct gpio_desc *te_gpio;
>
> --
> 2.39.2
>
[PATCH v2 2/2] drm/vblank: Warn when silently cancelling vblank works
From: Ville Syrjälä
Silently cancelling vblank works is a bit rude, especially
if said works do any resource management (eg. free memory).
WARN if we ever hit this.
TODO: Maybe drm_crtc_vblank_off() should wait for any
pending work to reach its target vblank before actually
doing anything drastic?
Cc: Lyude Paul
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_vblank_work.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank_work.c
b/drivers/gpu/drm/drm_vblank_work.c
index bd481fdd6b87..43cd5c0f4f6f 100644
--- a/drivers/gpu/drm/drm_vblank_work.c
+++ b/drivers/gpu/drm/drm_vblank_work.c
@@ -73,6 +73,9 @@ void drm_vblank_cancel_pending_works(struct drm_vblank_crtc
*vblank)
assert_spin_locked(>dev->event_lock);
+ drm_WARN_ONCE(vblank->dev, !list_empty(>pending_work),
+ "Cancelling pending vblank works!\n");
+
list_for_each_entry_safe(work, next, >pending_work, node) {
list_del_init(>node);
drm_vblank_put(vblank->dev, vblank->pipe);
--
2.41.0
[PATCH v2 1/2] drm/i915: Use vblank worker to unpin old legacy cursor fb safely
From: Ville Syrjälä
The cursor hardware only does sync updates, and thus the hardware
will be scanning out from the old fb until the next start of vblank.
So in order to make the legacy cursor fastpath actually safe we
should not unpin the old fb until we're sure the hardware has
ceased accessing it. The simplest approach is to just use a vblank
work here to do the delayed unpin.
Not 100% sure it's a good idea to put this onto the same high
priority vblank worker as eg. our timing critical gamma updates.
But let's keep it simple for now, and it we later discover that
this is causing problems we can think about adding a lower
priority worker for such things.
v2: wait for cursor unpins before turning off the vblank irq
Cc: Maarten Lankhorst
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_cursor.c | 36 +--
drivers/gpu/drm/i915/display/intel_cursor.h | 2 ++
drivers/gpu/drm/i915/display/intel_display.c | 3 ++
.../drm/i915/display/intel_display_types.h| 4 +++
4 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c
b/drivers/gpu/drm/i915/display/intel_cursor.c
index b342fad180ca..625540fd1dab 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -603,6 +603,26 @@ static bool intel_cursor_format_mod_supported(struct
drm_plane *_plane,
return format == DRM_FORMAT_ARGB;
}
+static void intel_cursor_unpin_work(struct kthread_work *base)
+{
+ struct drm_vblank_work *work = to_drm_vblank_work(base);
+ struct intel_plane_state *plane_state =
+ container_of(work, typeof(*plane_state), unpin_work);
+ struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
+
+ intel_plane_unpin_fb(plane_state);
+ intel_plane_destroy_state(>base, _state->uapi);
+
+ if (atomic_dec_and_test(>cursor.pending_unpins))
+ wake_up_var(>cursor.pending_unpins);
+}
+
+void intel_cursor_wait_unpin_works(struct intel_plane *plane)
+{
+ wait_var_event(>cursor.pending_unpins,
+ !atomic_read(>cursor.pending_unpins));
+}
+
static int
intel_legacy_cursor_update(struct drm_plane *_plane,
struct drm_crtc *_crtc,
@@ -730,14 +750,26 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
local_irq_enable();
- intel_plane_unpin_fb(old_plane_state);
+ if (old_plane_state->hw.fb != new_plane_state->hw.fb) {
+ drm_vblank_work_init(_plane_state->unpin_work, >base,
+intel_cursor_unpin_work);
+
+ atomic_inc(>cursor.pending_unpins);
+ drm_vblank_work_schedule(_plane_state->unpin_work,
+
drm_crtc_accurate_vblank_count(>base) + 1,
+false);
+
+ old_plane_state = NULL;
+ } else {
+ intel_plane_unpin_fb(old_plane_state);
+ }
out_free:
if (new_crtc_state)
intel_crtc_destroy_state(>base, _crtc_state->uapi);
if (ret)
intel_plane_destroy_state(>base, _plane_state->uapi);
- else
+ else if (old_plane_state)
intel_plane_destroy_state(>base, _plane_state->uapi);
return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.h
b/drivers/gpu/drm/i915/display/intel_cursor.h
index ce333bf4c2d5..e778aff77129 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.h
+++ b/drivers/gpu/drm/i915/display/intel_cursor.h
@@ -14,4 +14,6 @@ struct intel_plane *
intel_cursor_plane_create(struct drm_i915_private *dev_priv,
enum pipe pipe);
+void intel_cursor_wait_unpin_works(struct intel_plane *plane);
+
#endif
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index f6397462e4c2..90c1ed61ba0e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -63,6 +63,7 @@
#include "intel_crt.h"
#include "intel_crtc.h"
#include "intel_crtc_state_dump.h"
+#include "intel_cursor.h"
#include "intel_ddi.h"
#include "intel_de.h"
#include "intel_display_driver.h"
@@ -6618,6 +6619,8 @@ static void intel_commit_modeset_disables(struct
intel_atomic_state *state)
intel_pre_plane_update(state, crtc);
intel_crtc_disable_planes(state, crtc);
+
+
intel_cursor_wait_unpin_works(to_intel_plane(crtc->base.cursor));
}
/* Only disable port sync and MST slaves */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index c21064794f32..f6ca86f1d834 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -702,6 +702,9 @@ struct intel_plane_state {
struct
Re: [Intel-gfx] [PATCH v2 09/22] drm/dp_mst: Fix fractional bpp scaling in drm_dp_calc_pbn_mode()
On Thu, Aug 24, 2023 at 11:05:04AM +0300, Imre Deak wrote:
> For fractional bpp values passed to the function in a .4 fixed point
> format, the fractional part is currently ignored due to scaling bpp too
> early. Fix this by scaling the overhead factor instead and to avoid an
> overflow multiplying bpp with the overhead factor instead of the clock
> rate.
>
> While at it simplify the formula, and pass the expected fixed point bpp
> values in the kunit tests.
>
> Cc: Lyude Paul
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++-
> drivers/gpu/drm/tests/drm_dp_mst_helper_test.c | 8
> 2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index ed96cfcfa3040..bd0f35a0ea5fb 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -4712,12 +4712,9 @@ int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
>* factor in the numerator rather than the denominator to avoid
>* integer overflow
>*/
> + u32 bpp_m = (dsc ? 64 / 16 : 64) * 1006 * bpp;
>
> - if (dsc)
> - return DIV_ROUND_UP_ULL(mul_u32_u32(clock * (bpp / 16), 64 *
> 1006),
> - 8 * 54 * 1000 * 1000);
> -
> - return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006),
> + return DIV_ROUND_UP_ULL(mul_u32_u32(clock, bpp_m),
> 8 * 54 * 1000 * 1000);
I thought I sorted out this mess already...
https://patchwork.freedesktop.org/patch/535005/?series=117201=3
Apparently I forgot to push that.
> }
> EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
> diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> index 545beea33e8c7..ea2182815ebe8 100644
> --- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> @@ -40,15 +40,15 @@ static const struct drm_dp_mst_calc_pbn_mode_test
> drm_dp_mst_calc_pbn_mode_cases
> },
> {
> .clock = 332880,
> - .bpp = 24,
> + .bpp = 24 << 4,
> .dsc = true,
> - .expected = 50
> + .expected = 1191
> },
> {
> .clock = 324540,
> - .bpp = 24,
> + .bpp = 24 << 4,
> .dsc = true,
> - .expected = 49
> + .expected = 1161
> },
> };
>
> --
> 2.37.2
--
Ville Syrjälä
Intel
[PATCH v2] drm: gm12u320: Fix the timeout usage for usb_bulk_msg()
The timeout arg of usb_bulk_msg() is ms already, which has been converted
to jiffies by msecs_to_jiffies() in usb_start_wait_urb(). So fix the usage
by removing the redundant msecs_to_jiffies() in the macros.
And as Hans suggested, also remove msecs_to_jiffies() for the IDLE_TIMEOUT
macro to make it consistent here and so change IDLE_TIMEOUT to
msecs_to_jiffies(IDLE_TIMEOUT) where it is used.
Fixes: e4f86e437164 ("drm: Add Grain Media GM12U320 driver v2")
Signed-off-by: Jinjie Ruan
Suggested-by: Hans de Goede
---
v2:
- Remove the msecs_to_jiffies() also for IDLE_TIMEOUT.
- Update the fix tag.
- Update the commit message.
---
drivers/gpu/drm/tiny/gm12u320.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index c5bb683e440c..0187539ff5ea 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -70,10 +70,10 @@ MODULE_PARM_DESC(eco_mode, "Turn on Eco mode (less bright,
more silent)");
#define READ_STATUS_SIZE 13
#define MISC_VALUE_SIZE4
-#define CMD_TIMEOUTmsecs_to_jiffies(200)
-#define DATA_TIMEOUT msecs_to_jiffies(1000)
-#define IDLE_TIMEOUT msecs_to_jiffies(2000)
-#define FIRST_FRAME_TIMEOUTmsecs_to_jiffies(2000)
+#define CMD_TIMEOUT200
+#define DATA_TIMEOUT 1000
+#define IDLE_TIMEOUT 2000
+#define FIRST_FRAME_TIMEOUT2000
#define MISC_REQ_GET_SET_ECO_A 0xff
#define MISC_REQ_GET_SET_ECO_B 0x35
@@ -389,7 +389,7 @@ static void gm12u320_fb_update_work(struct work_struct
*work)
* switches back to showing its logo.
*/
queue_delayed_work(system_long_wq, >fb_update.work,
- IDLE_TIMEOUT);
+ msecs_to_jiffies(IDLE_TIMEOUT));
return;
err:
--
2.34.1
[PATCH v3 4/8] drm/msm/dpu: inline _setup_intf_ops()
Inline the _setup_intf_ops() function, it makes it easier to handle
different conditions involving INTF configuration.
Reviewed-by: Marijn Suijten
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 48 ++---
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 8ec6505d9e78..dd67686f5403 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -524,31 +524,6 @@ static void dpu_hw_intf_program_intf_cmd_cfg(struct
dpu_hw_intf *ctx,
DPU_REG_WRITE(>hw, INTF_CONFIG2, intf_cfg2);
}
-static void _setup_intf_ops(struct dpu_hw_intf_ops *ops,
- unsigned long cap, const struct dpu_mdss_version *mdss_rev)
-{
- ops->setup_timing_gen = dpu_hw_intf_setup_timing_engine;
- ops->setup_prg_fetch = dpu_hw_intf_setup_prg_fetch;
- ops->get_status = dpu_hw_intf_get_status;
- ops->enable_timing = dpu_hw_intf_enable_timing_engine;
- ops->get_line_count = dpu_hw_intf_get_line_count;
- if (cap & BIT(DPU_INTF_INPUT_CTRL))
- ops->bind_pingpong_blk = dpu_hw_intf_bind_pingpong_blk;
- ops->setup_misr = dpu_hw_intf_setup_misr;
- ops->collect_misr = dpu_hw_intf_collect_misr;
-
- if (cap & BIT(DPU_INTF_TE)) {
- ops->enable_tearcheck = dpu_hw_intf_enable_te;
- ops->disable_tearcheck = dpu_hw_intf_disable_te;
- ops->connect_external_te = dpu_hw_intf_connect_external_te;
- ops->vsync_sel = dpu_hw_intf_vsync_sel;
- ops->disable_autorefresh = dpu_hw_intf_disable_autorefresh;
- }
-
- if (mdss_rev->core_major_ver >= 7)
- ops->program_intf_cmd_cfg = dpu_hw_intf_program_intf_cmd_cfg;
-}
-
struct dpu_hw_intf *dpu_hw_intf_init(const struct dpu_intf_cfg *cfg,
void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
{
@@ -571,7 +546,28 @@ struct dpu_hw_intf *dpu_hw_intf_init(const struct
dpu_intf_cfg *cfg,
*/
c->idx = cfg->id;
c->cap = cfg;
- _setup_intf_ops(>ops, c->cap->features, mdss_rev);
+
+ c->ops.setup_timing_gen = dpu_hw_intf_setup_timing_engine;
+ c->ops.setup_prg_fetch = dpu_hw_intf_setup_prg_fetch;
+ c->ops.get_status = dpu_hw_intf_get_status;
+ c->ops.enable_timing = dpu_hw_intf_enable_timing_engine;
+ c->ops.get_line_count = dpu_hw_intf_get_line_count;
+ c->ops.setup_misr = dpu_hw_intf_setup_misr;
+ c->ops.collect_misr = dpu_hw_intf_collect_misr;
+
+ if (cfg->features & BIT(DPU_INTF_INPUT_CTRL))
+ c->ops.bind_pingpong_blk = dpu_hw_intf_bind_pingpong_blk;
+
+ if (cfg->features & BIT(DPU_INTF_TE)) {
+ c->ops.enable_tearcheck = dpu_hw_intf_enable_te;
+ c->ops.disable_tearcheck = dpu_hw_intf_disable_te;
+ c->ops.connect_external_te = dpu_hw_intf_connect_external_te;
+ c->ops.vsync_sel = dpu_hw_intf_vsync_sel;
+ c->ops.disable_autorefresh = dpu_hw_intf_disable_autorefresh;
+ }
+
+ if (mdss_rev->core_major_ver >= 7)
+ c->ops.program_intf_cmd_cfg = dpu_hw_intf_program_intf_cmd_cfg;
return c;
}
--
2.39.2
[PATCH v3 7/8] drm/msm/dpu: drop useless check from dpu_encoder_phys_cmd_te_rd_ptr_irq()
The dpu_encoder_phys_cmd_te_rd_ptr_irq() function uses neither hw_intf
nor hw_pp data, so we can drop the corresponding check.
Reviewed-by: Marijn Suijten
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index e03b2075639d..d18236bd98e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -108,14 +108,6 @@ static void dpu_encoder_phys_cmd_te_rd_ptr_irq(void *arg,
int irq_idx)
struct dpu_encoder_phys *phys_enc = arg;
struct dpu_encoder_phys_cmd *cmd_enc;
- if (phys_enc->has_intf_te) {
- if (!phys_enc->hw_intf)
- return;
- } else {
- if (!phys_enc->hw_pp)
- return;
- }
-
DPU_ATRACE_BEGIN("rd_ptr_irq");
cmd_enc = to_dpu_encoder_phys_cmd(phys_enc);
--
2.39.2
[PATCH v3 6/8] drm/msm/dpu: drop DPU_INTF_TE feature flag
Replace the only user of the DPU_INTF_TE feature flag with the direct
DPU version comparison.
Reviewed-by: Marijn Suijten
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 5 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 --
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index df88358e7037..e03b2075639d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -776,8 +776,9 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
phys_enc->intf_mode = INTF_MODE_CMD;
cmd_enc->stream_sel = 0;
- phys_enc->has_intf_te = test_bit(DPU_INTF_TE,
-_enc->hw_intf->cap->features);
+ /* DPU before 5.0 use PINGPONG for TE handling */
+ if (phys_enc->dpu_kms->catalog->mdss_ver->core_major_ver >= 5)
+ phys_enc->has_intf_te = true;
atomic_set(_enc->pending_vblank_cnt, 0);
init_waitqueue_head(_enc->pending_vblank_wq);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index d89bdd0dd27a..a1aada630780 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -100,7 +100,6 @@
#define INTF_SC7180_MASK \
(BIT(DPU_INTF_INPUT_CTRL) | \
-BIT(DPU_INTF_TE) | \
BIT(DPU_INTF_STATUS_SUPPORTED) | \
BIT(DPU_DATA_HCTL_EN))
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 9aac937285b1..e5add4384830 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -158,7 +158,6 @@ enum {
* INTF sub-blocks
* @DPU_INTF_INPUT_CTRL Supports the setting of pp block from which
* pixel data arrives to this INTF
- * @DPU_INTF_TE INTF block has TE configuration support
* @DPU_DATA_HCTL_ENAllows data to be transferred at different
rate
* than video timing
* @DPU_INTF_STATUS_SUPPORTED INTF block has INTF_STATUS register
@@ -166,7 +165,6 @@ enum {
*/
enum {
DPU_INTF_INPUT_CTRL = 0x1,
- DPU_INTF_TE,
DPU_DATA_HCTL_EN,
DPU_INTF_STATUS_SUPPORTED,
DPU_INTF_MAX
--
2.39.2
[PATCH v3 5/8] drm/msm/dpu: enable INTF TE operations only when supported by HW
The DPU_INTF_TE bit is set for all INTF blocks on DPU >= 5.0, however
only INTF_1 and INTF_2 actually support tearing control (both are
INTF_DSI). Rather than trying to limit the DPU_INTF_TE feature bit to
those two INTF instances, check for the major && INTF type.
Reviewed-by: Marijn Suijten
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index dd67686f5403..95ff2f5ebbaa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -558,7 +558,10 @@ struct dpu_hw_intf *dpu_hw_intf_init(const struct
dpu_intf_cfg *cfg,
if (cfg->features & BIT(DPU_INTF_INPUT_CTRL))
c->ops.bind_pingpong_blk = dpu_hw_intf_bind_pingpong_blk;
- if (cfg->features & BIT(DPU_INTF_TE)) {
+ /* INTF TE is only for DSI interfaces */
+ if (mdss_rev->core_major_ver >= 5 && cfg->type == INTF_DSI) {
+ WARN_ON(!cfg->intr_tear_rd_ptr);
+
c->ops.enable_tearcheck = dpu_hw_intf_enable_te;
c->ops.disable_tearcheck = dpu_hw_intf_disable_te;
c->ops.connect_external_te = dpu_hw_intf_connect_external_te;
--
2.39.2
[PATCH v3 1/8] drm/msm/dpu: inline _setup_pingpong_ops()
Inline the _setup_pingpong_ops() function, it makes it easier to handle
different conditions involving PINGPONG configuration.
Reviewed-by: Marijn Suijten
Signed-off-by: Dmitry Baryshkov
---
.../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 39 ---
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
index 437d9e62a841..9298c166b213 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -281,27 +281,6 @@ static int dpu_hw_pp_setup_dsc(struct dpu_hw_pingpong *pp)
return 0;
}
-static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
- unsigned long features)
-{
- if (test_bit(DPU_PINGPONG_TE, )) {
- c->ops.enable_tearcheck = dpu_hw_pp_enable_te;
- c->ops.disable_tearcheck = dpu_hw_pp_disable_te;
- c->ops.connect_external_te = dpu_hw_pp_connect_external_te;
- c->ops.get_line_count = dpu_hw_pp_get_line_count;
- c->ops.disable_autorefresh = dpu_hw_pp_disable_autorefresh;
- }
-
- if (test_bit(DPU_PINGPONG_DSC, )) {
- c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
- c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
- c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
- }
-
- if (test_bit(DPU_PINGPONG_DITHER, ))
- c->ops.setup_dither = dpu_hw_pp_setup_dither;
-};
-
struct dpu_hw_pingpong *dpu_hw_pingpong_init(const struct dpu_pingpong_cfg
*cfg,
void __iomem *addr)
{
@@ -316,7 +295,23 @@ struct dpu_hw_pingpong *dpu_hw_pingpong_init(const struct
dpu_pingpong_cfg *cfg,
c->idx = cfg->id;
c->caps = cfg;
- _setup_pingpong_ops(c, c->caps->features);
+
+ if (test_bit(DPU_PINGPONG_TE, >features)) {
+ c->ops.enable_tearcheck = dpu_hw_pp_enable_te;
+ c->ops.disable_tearcheck = dpu_hw_pp_disable_te;
+ c->ops.connect_external_te = dpu_hw_pp_connect_external_te;
+ c->ops.get_line_count = dpu_hw_pp_get_line_count;
+ c->ops.disable_autorefresh = dpu_hw_pp_disable_autorefresh;
+ }
+
+ if (test_bit(DPU_PINGPONG_DSC, >features)) {
+ c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
+ c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
+ c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
+ }
+
+ if (test_bit(DPU_PINGPONG_DITHER, >features))
+ c->ops.setup_dither = dpu_hw_pp_setup_dither;
return c;
}
--
2.39.2
[PATCH v3 8/8] drm/msm/dpu: move INTF tearing checks to dpu_encoder_phys_cmd_init
As the INTF is fixed at the encoder creation time, we can move the
check whether INTF supports tearchck to dpu_encoder_phys_cmd_init().
This function can return an error if INTF doesn't have required feature.
Performing this check in dpu_encoder_phys_cmd_tearcheck_config() is less
useful, as this function returns void.
Signed-off-by: Dmitry Baryshkov
---
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index d18236bd98e6..ca1296379c4d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -325,24 +325,21 @@ static void dpu_encoder_phys_cmd_tearcheck_config(
unsigned long vsync_hz;
struct dpu_kms *dpu_kms;
- if (phys_enc->has_intf_te) {
- if (!phys_enc->hw_intf ||
- !phys_enc->hw_intf->ops.enable_tearcheck) {
- DPU_DEBUG_CMDENC(cmd_enc, "tearcheck not supported\n");
- return;
- }
-
- DPU_DEBUG_CMDENC(cmd_enc, "");
- } else {
- if (!phys_enc->hw_pp ||
- !phys_enc->hw_pp->ops.enable_tearcheck) {
- DPU_DEBUG_CMDENC(cmd_enc, "tearcheck not supported\n");
- return;
- }
-
- DPU_DEBUG_CMDENC(cmd_enc, "pp %d\n", phys_enc->hw_pp->idx -
PINGPONG_0);
+ /*
+* TODO: if/when resource allocation is refactored, move this to a
+* place where the driver can actually return an error.
+*/
+ if (!phys_enc->has_intf_te &&
+ (!phys_enc->hw_pp ||
+!phys_enc->hw_pp->ops.enable_tearcheck)) {
+ DPU_DEBUG_CMDENC(cmd_enc, "tearcheck not supported\n");
+ return;
}
+ DPU_DEBUG_CMDENC(cmd_enc, "intf %d pp %d\n",
+phys_enc->hw_intf ? phys_enc->hw_intf->idx - INTF_0 :
-1,
+phys_enc->hw_pp ? phys_enc->hw_pp->idx - PINGPONG_0 :
-1);
+
mode = _enc->cached_mode;
dpu_kms = phys_enc->dpu_kms;
@@ -768,10 +765,20 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
phys_enc->intf_mode = INTF_MODE_CMD;
cmd_enc->stream_sel = 0;
+ if (!phys_enc->hw_intf) {
+ DPU_ERROR_CMDENC(cmd_enc, "no INTF provided\n");
+ return ERR_PTR(-EINVAL);
+ }
+
/* DPU before 5.0 use PINGPONG for TE handling */
if (phys_enc->dpu_kms->catalog->mdss_ver->core_major_ver >= 5)
phys_enc->has_intf_te = true;
+ if (phys_enc->has_intf_te && !phys_enc->hw_intf->ops.enable_tearcheck) {
+ DPU_ERROR_CMDENC(cmd_enc, "tearcheck not supported\n");
+ return ERR_PTR(-EINVAL);
+ }
+
atomic_set(_enc->pending_vblank_cnt, 0);
init_waitqueue_head(_enc->pending_vblank_wq);
--
2.39.2
[PATCH v3 2/8] drm/msm/dpu: enable PINGPONG TE operations only when supported by HW
The DPU_PINGPONG_TE bit is set for all PINGPONG blocks on DPU < 5.0.
Rather than checking for the flag, check for the presense of the
corresponding interrupt line.
Reviewed-by: Marijn Suijten
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 6 --
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 3 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 +-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
index 9298c166b213..057cac7f5d93 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -282,7 +282,7 @@ static int dpu_hw_pp_setup_dsc(struct dpu_hw_pingpong *pp)
}
struct dpu_hw_pingpong *dpu_hw_pingpong_init(const struct dpu_pingpong_cfg
*cfg,
- void __iomem *addr)
+ void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
{
struct dpu_hw_pingpong *c;
@@ -296,7 +296,9 @@ struct dpu_hw_pingpong *dpu_hw_pingpong_init(const struct
dpu_pingpong_cfg *cfg,
c->idx = cfg->id;
c->caps = cfg;
- if (test_bit(DPU_PINGPONG_TE, >features)) {
+ if (mdss_rev->core_major_ver < 5) {
+ WARN_ON(!cfg->intr_rdptr);
+
c->ops.enable_tearcheck = dpu_hw_pp_enable_te;
c->ops.disable_tearcheck = dpu_hw_pp_disable_te;
c->ops.connect_external_te = dpu_hw_pp_connect_external_te;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
index d3246a9a5808..0d541ca5b056 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
@@ -123,10 +123,11 @@ static inline struct dpu_hw_pingpong
*to_dpu_hw_pingpong(struct dpu_hw_blk *hw)
* pingpong catalog entry.
* @cfg: Pingpong catalog entry for which driver object is required
* @addr: Mapped register io address of MDP
+ * @mdss_rev: dpu core's major and minor versions
* Return: Error code or allocated dpu_hw_pingpong context
*/
struct dpu_hw_pingpong *dpu_hw_pingpong_init(const struct dpu_pingpong_cfg
*cfg,
- void __iomem *addr);
+ void __iomem *addr, const struct dpu_mdss_version *mdss_rev);
/**
* dpu_hw_pingpong_destroy - destroys pingpong driver context
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f9215643c71a..f3aff605554d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -146,7 +146,7 @@ int dpu_rm_init(struct dpu_rm *rm,
struct dpu_hw_pingpong *hw;
const struct dpu_pingpong_cfg *pp = >pingpong[i];
- hw = dpu_hw_pingpong_init(pp, mmio);
+ hw = dpu_hw_pingpong_init(pp, mmio, cat->mdss_ver);
if (IS_ERR(hw)) {
rc = PTR_ERR(hw);
DPU_ERROR("failed pingpong object creation: err %d\n",
--
2.39.2
[PATCH v3 0/8] drm/msm/dpu: move INTF tearing checks to dpu_encoder_phys_cmd_ini
rop two feature flags, DPU_INTF_TE and DPU_PINGPONG_TE, in favour of performing the MDSS revision checks instead. Changes since v2: - Added guarding checks for hw_intf and hw_pp in debug print (Marijn) - Removed extra empty lines (Marijn) Changes since v1: - Added missing patch - Reworked commit messages (following suggestions by Marijn) - Changed code to check for major & INTF type rather than checking for intr presence in catalog. Added WARN_ON()s instead. (Marijn) - Added severall comments & TODO item. Dmitry Baryshkov (8): drm/msm/dpu: inline _setup_pingpong_ops() drm/msm/dpu: enable PINGPONG TE operations only when supported by HW drm/msm/dpu: drop the DPU_PINGPONG_TE flag drm/msm/dpu: inline _setup_intf_ops() drm/msm/dpu: enable INTF TE operations only when supported by HW drm/msm/dpu: drop DPU_INTF_TE feature flag drm/msm/dpu: drop useless check from dpu_encoder_phys_cmd_te_rd_ptr_irq() drm/msm/dpu: move INTF tearing checks to dpu_encoder_phys_cmd_init .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 52 +-- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 3 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 6 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 51 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 41 +++ .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 2 +- 7 files changed, 75 insertions(+), 83 deletions(-) -- 2.39.2
[PATCH v3 3/8] drm/msm/dpu: drop the DPU_PINGPONG_TE flag
The DPU_PINGPONG_TE flag became unused, we can drop it now.
Reviewed-by: Marijn Suijten
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 713dfc079718..d89bdd0dd27a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -79,7 +79,7 @@
(BIT(DPU_DIM_LAYER) | BIT(DPU_MIXER_COMBINED_ALPHA))
#define PINGPONG_SDM845_MASK \
- (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_TE) |
BIT(DPU_PINGPONG_DSC))
+ (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_DSC))
#define PINGPONG_SDM845_TE2_MASK \
(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 6c9634209e9f..9aac937285b1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -119,7 +119,6 @@ enum {
/**
* PINGPONG sub-blocks
- * @DPU_PINGPONG_TE Tear check block
* @DPU_PINGPONG_TE2Additional tear check block for split pipes
* @DPU_PINGPONG_SPLIT PP block supports split fifo
* @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
@@ -128,8 +127,7 @@ enum {
* @DPU_PINGPONG_MAX
*/
enum {
- DPU_PINGPONG_TE = 0x1,
- DPU_PINGPONG_TE2,
+ DPU_PINGPONG_TE2 = 0x1,
DPU_PINGPONG_SPLIT,
DPU_PINGPONG_SLAVE,
DPU_PINGPONG_DITHER,
--
2.39.2
[PATCH 2/3] drm/bridge: migrate bridge_chains to per-encoder file
Instead of having a single file with all bridge chains, list bridges
under a corresponding per-encoder debugfs directory.
Example of the listing:
$ cat /sys/kernel/debug/dri/0/encoder-0/bridges
bridge[0]: dsi_mgr_bridge_funcs
type: [0] Unknown
ops: [0]
bridge[1]: lt9611uxc_bridge_funcs
type: [11] HDMI-A
OF: /soc@0/geniqup@9c/i2c@994000/hdmi-bridge@2b:lontium,lt9611uxc
ops: [7] detect edid hpd
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_bridge.c | 44 ---
drivers/gpu/drm/drm_debugfs.c | 39 ---
include/drm/drm_bridge.h | 2 --
3 files changed, 36 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 39e68e45bb12..cee3188adf3d 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1347,50 +1347,6 @@ struct drm_bridge *of_drm_find_bridge(struct device_node
*np)
EXPORT_SYMBOL(of_drm_find_bridge);
#endif
-#ifdef CONFIG_DEBUG_FS
-static int drm_bridge_chains_info(struct seq_file *m, void *data)
-{
- struct drm_debugfs_entry *entry = m->private;
- struct drm_device *dev = entry->dev;
- struct drm_printer p = drm_seq_file_printer(m);
- struct drm_mode_config *config = >mode_config;
- struct drm_encoder *encoder;
- unsigned int bridge_idx = 0;
-
- list_for_each_entry(encoder, >encoder_list, head) {
- struct drm_bridge *bridge;
-
- drm_printf(, "encoder[%u]\n", encoder->base.id);
-
- drm_for_each_bridge_in_chain(encoder, bridge) {
- drm_printf(, "\tbridge[%u] type: %u, ops: %#x",
- bridge_idx, bridge->type, bridge->ops);
-
-#ifdef CONFIG_OF
- if (bridge->of_node)
- drm_printf(, ", OF: %pOFfc", bridge->of_node);
-#endif
-
- drm_printf(, "\n");
-
- bridge_idx++;
- }
- }
-
- return 0;
-}
-
-static const struct drm_debugfs_info drm_bridge_debugfs_list[] = {
- { "bridge_chains", drm_bridge_chains_info, 0 },
-};
-
-void drm_bridge_debugfs_init(struct drm_minor *minor)
-{
- drm_debugfs_add_files(minor->dev, drm_bridge_debugfs_list,
- ARRAY_SIZE(drm_bridge_debugfs_list));
-}
-#endif
-
MODULE_AUTHOR("Ajay Kumar ");
MODULE_DESCRIPTION("DRM bridge infrastructure");
MODULE_LICENSE("GPL and additional rights");
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index cf7f33bdc963..70913067406d 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -273,10 +273,8 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
drm_debugfs_add_files(minor->dev, drm_debugfs_list,
DRM_DEBUGFS_ENTRIES);
- if (drm_drv_uses_atomic_modeset(dev)) {
+ if (drm_drv_uses_atomic_modeset(dev))
drm_atomic_debugfs_init(minor);
- drm_bridge_debugfs_init(minor);
- }
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
drm_framebuffer_debugfs_init(minor);
@@ -603,6 +601,37 @@ void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
crtc->debugfs_entry = NULL;
}
+static int bridges_show(struct seq_file *m, void *data)
+{
+ struct drm_encoder *encoder = m->private;
+ struct drm_bridge *bridge;
+ unsigned int idx = 0;
+
+ drm_for_each_bridge_in_chain(encoder, bridge) {
+ seq_printf(m, "bridge[%d]: %ps\n", idx++, bridge->funcs);
+ seq_printf(m, "\ttype: [%d] %s\n",
+ bridge->type,
+ drm_get_connector_type_name(bridge->type));
+#ifdef CONFIG_OF
+ if (bridge->of_node)
+ seq_printf(m, "\tOF: %pOFfc\n", bridge->of_node);
+#endif
+ seq_printf(m, "\tops: [0x%x]", bridge->ops);
+ if (bridge->ops & DRM_BRIDGE_OP_DETECT)
+ seq_puts(m, " detect");
+ if (bridge->ops & DRM_BRIDGE_OP_EDID)
+ seq_puts(m, " edid");
+ if (bridge->ops & DRM_BRIDGE_OP_HPD)
+ seq_puts(m, " hpd");
+ if (bridge->ops & DRM_BRIDGE_OP_MODES)
+ seq_puts(m, " modes");
+ seq_puts(m, "\n");
+ }
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(bridges);
+
void drm_debugfs_encoder_add(struct drm_encoder *encoder)
{
struct drm_minor *minor = encoder->dev->primary;
@@ -618,6 +647,10 @@ void drm_debugfs_encoder_add(struct drm_encoder *encoder)
encoder->debugfs_entry = root;
+ /* bridges list */
+ debugfs_create_file("bridges", 0444, root, encoder,
+ _fops);
+
if (encoder->funcs->debugfs_init)
encoder->funcs->debugfs_init(encoder, root);
}
diff --git
[PATCH 0/3] drm: introduce per-encoder debugfs directory
Each of connectors and CRTCs used by the DRM device provides debugfs directory, which is used by several standard debugfs files and can further be extended by the driver. Add such generic debugfs directories for encoder. As a showcase for this dir, migrate `bridge_chains' debugfs file (which contains per-encoder data) and MSM custom encoder status to this new debugfs directory. Dmitry Baryshkov (3): drm/encoder: register per-encoder debugfs dir drm/bridge: migrate bridge_chains to per-encoder file drm/msm/dpu: move encoder status to standard encoder debugfs dir drivers/gpu/drm/drm_bridge.c| 44 -- drivers/gpu/drm/drm_debugfs.c | 64 - drivers/gpu/drm/drm_encoder.c | 4 ++ drivers/gpu/drm/drm_internal.h | 9 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 45 ++- include/drm/drm_bridge.h| 2 - include/drm/drm_encoder.h | 16 +- 7 files changed, 95 insertions(+), 89 deletions(-) -- 2.39.2
[PATCH 3/3] drm/msm/dpu: move encoder status to standard encoder debugfs dir
Now as we have standard per-encoder debugfs directory, move DPU encoder
status file to that directory.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 45 +++--
1 file changed, 6 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d34e684a4178..b219382d1153 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -184,7 +184,6 @@ struct dpu_encoder_virt {
struct drm_crtc *crtc;
struct drm_connector *connector;
- struct dentry *debugfs_root;
struct mutex enc_lock;
DECLARE_BITMAP(frame_busy_mask, MAX_PHYS_ENCODERS_PER_VIRTUAL);
void (*crtc_frame_event_cb)(void *, u32 event);
@@ -2096,7 +2095,8 @@ void dpu_encoder_helper_phys_cleanup(struct
dpu_encoder_phys *phys_enc)
#ifdef CONFIG_DEBUG_FS
static int _dpu_encoder_status_show(struct seq_file *s, void *data)
{
- struct dpu_encoder_virt *dpu_enc = s->private;
+ struct drm_encoder *drm_enc = s->private;
+ struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
int i;
mutex_lock(_enc->enc_lock);
@@ -2118,48 +2118,16 @@ static int _dpu_encoder_status_show(struct seq_file *s,
void *data)
DEFINE_SHOW_ATTRIBUTE(_dpu_encoder_status);
-static int _dpu_encoder_init_debugfs(struct drm_encoder *drm_enc)
+static void dpu_encoder_debugfs_init(struct drm_encoder *drm_enc, struct
dentry *root)
{
- struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
-
- char name[12];
-
- if (!drm_enc->dev) {
- DPU_ERROR("invalid encoder or kms\n");
- return -EINVAL;
- }
-
- snprintf(name, sizeof(name), "encoder%u", drm_enc->base.id);
-
- /* create overall sub-directory for the encoder */
- dpu_enc->debugfs_root = debugfs_create_dir(name,
- drm_enc->dev->primary->debugfs_root);
-
/* don't error check these */
debugfs_create_file("status", 0600,
- dpu_enc->debugfs_root, dpu_enc, &_dpu_encoder_status_fops);
-
- return 0;
+ root, drm_enc, &_dpu_encoder_status_fops);
}
#else
-static int _dpu_encoder_init_debugfs(struct drm_encoder *drm_enc)
-{
- return 0;
-}
+#define dpu_encoder_debugfs_init NULL
#endif
-static int dpu_encoder_late_register(struct drm_encoder *encoder)
-{
- return _dpu_encoder_init_debugfs(encoder);
-}
-
-static void dpu_encoder_early_unregister(struct drm_encoder *encoder)
-{
- struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(encoder);
-
- debugfs_remove_recursive(dpu_enc->debugfs_root);
-}
-
static int dpu_encoder_virt_add_phys_encs(
struct msm_display_info *disp_info,
struct dpu_encoder_virt *dpu_enc,
@@ -2343,8 +2311,7 @@ static const struct drm_encoder_helper_funcs
dpu_encoder_helper_funcs = {
static const struct drm_encoder_funcs dpu_encoder_funcs = {
.destroy = dpu_encoder_destroy,
- .late_register = dpu_encoder_late_register,
- .early_unregister = dpu_encoder_early_unregister,
+ .debugfs_init = dpu_encoder_debugfs_init,
};
struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
--
2.39.2
[PATCH 1/3] drm/encoder: register per-encoder debugfs dir
Each of connectors and CRTCs used by the DRM device provides debugfs
directory, which is used by several standard debugfs files and can
further be extended by the driver. Add such generic debugfs directories
for encoder.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_debugfs.c | 25 +
drivers/gpu/drm/drm_encoder.c | 4
drivers/gpu/drm/drm_internal.h | 9 +
include/drm/drm_encoder.h | 16 +++-
4 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 2de43ff3ce0a..cf7f33bdc963 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -603,4 +603,29 @@ void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
crtc->debugfs_entry = NULL;
}
+void drm_debugfs_encoder_add(struct drm_encoder *encoder)
+{
+ struct drm_minor *minor = encoder->dev->primary;
+ struct dentry *root;
+ char *name;
+
+ name = kasprintf(GFP_KERNEL, "encoder-%d", encoder->index);
+ if (!name)
+ return;
+
+ root = debugfs_create_dir(name, minor->debugfs_root);
+ kfree(name);
+
+ encoder->debugfs_entry = root;
+
+ if (encoder->funcs->debugfs_init)
+ encoder->funcs->debugfs_init(encoder, root);
+}
+
+void drm_debugfs_encoder_remove(struct drm_encoder *encoder)
+{
+ debugfs_remove_recursive(encoder->debugfs_entry);
+ encoder->debugfs_entry = NULL;
+}
+
#endif /* CONFIG_DEBUG_FS */
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 1143bc7f3252..8f2bc6a28482 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -30,6 +30,7 @@
#include
#include "drm_crtc_internal.h"
+#include "drm_internal.h"
/**
* DOC: overview
@@ -74,6 +75,8 @@ int drm_encoder_register_all(struct drm_device *dev)
int ret = 0;
drm_for_each_encoder(encoder, dev) {
+ drm_debugfs_encoder_add(encoder);
+
if (encoder->funcs && encoder->funcs->late_register)
ret = encoder->funcs->late_register(encoder);
if (ret)
@@ -90,6 +93,7 @@ void drm_encoder_unregister_all(struct drm_device *dev)
drm_for_each_encoder(encoder, dev) {
if (encoder->funcs && encoder->funcs->early_unregister)
encoder->funcs->early_unregister(encoder);
+ drm_debugfs_encoder_remove(encoder);
}
}
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index ba12acd55139..173b4d872431 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -189,6 +189,8 @@ void drm_debugfs_connector_remove(struct drm_connector
*connector);
void drm_debugfs_crtc_add(struct drm_crtc *crtc);
void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
+void drm_debugfs_encoder_add(struct drm_encoder *encoder);
+void drm_debugfs_encoder_remove(struct drm_encoder *encoder);
#else
static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
struct dentry *root)
@@ -222,6 +224,13 @@ static inline void drm_debugfs_crtc_crc_add(struct
drm_crtc *crtc)
{
}
+static inline void drm_debugfs_encoder_add(struct drm_encoder *encoder)
+{
+}
+static inline void drm_debugfs_encoder_remove(struct drm_encoder *encoder)
+{
+}
+
#endif
drm_ioctl_t drm_version;
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 3a09682af685..977a9381c8ba 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -60,7 +60,7 @@ struct drm_encoder_funcs {
* @late_register:
*
* This optional hook can be used to register additional userspace
-* interfaces attached to the encoder like debugfs interfaces.
+* interfaces attached to the encoder.
* It is called late in the driver load sequence from
drm_dev_register().
* Everything added from this callback should be unregistered in
* the early_unregister callback.
@@ -81,6 +81,13 @@ struct drm_encoder_funcs {
* before data structures are torndown.
*/
void (*early_unregister)(struct drm_encoder *encoder);
+
+ /**
+* @debugfs_init:
+*
+* Allows encoders to create encoder-specific debugfs files.
+*/
+ void (*debugfs_init)(struct drm_encoder *encoder, struct dentry *root);
};
/**
@@ -184,6 +191,13 @@ struct drm_encoder {
const struct drm_encoder_funcs *funcs;
const struct drm_encoder_helper_funcs *helper_private;
+
+ /**
+* @debugfs_entry:
+*
+* Debugfs directory for this CRTC.
+*/
+ struct dentry *debugfs_entry;
};
#define obj_to_encoder(x) container_of(x, struct drm_encoder, base)
--
2.39.2
Re: [PATCH 1/5] drm/bridge: samsung-dsim: add more mipi-dsi device debug information
2023년 8월 29일 (화) 오전 7:38, Adam Ford 님이 작성: > > On Mon, Aug 28, 2023 at 10:59 AM Michael Tretter > wrote: > > > > From: Marco Felsch > > > > Since the MIPI configuration can be changed on demand it is very useful > > to print more MIPI settings during the MIPI device attach step. > > > > Signed-off-by: Marco Felsch > > Signed-off-by: Michael Tretter > > Reviewed-by: Adam Ford #imx8mm-beacon > Tested-by: Adam Ford #imx8mm-beacon Reviewed-by: Inki Dae Acked-by: Inki Dae > > > --- > > drivers/gpu/drm/bridge/samsung-dsim.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > > b/drivers/gpu/drm/bridge/samsung-dsim.c > > index 73ec60757dbc..6778f1751faa 100644 > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > @@ -1711,7 +1711,10 @@ static int samsung_dsim_host_attach(struct > > mipi_dsi_host *host, > > return ret; > > } > > > > - DRM_DEV_INFO(dev, "Attached %s device\n", device->name); > > + DRM_DEV_INFO(dev, "Attached %s device (lanes:%d bpp:%d > > mode-flags:0x%lx)\n", > > +device->name, device->lanes, > > +mipi_dsi_pixel_format_to_bpp(device->format), > > +device->mode_flags); > > > > drm_bridge_add(>bridge); > > > > > > -- > > 2.39.2 > >
Re: [PATCH AUTOSEL 6.1 08/10] drm/amdkfd: ignore crat by default
On Tue, Aug 22, 2023 at 03:41:18PM +, Deucher, Alexander wrote: [Public] -Original Message- From: Sasha Levin Sent: Tuesday, August 22, 2023 7:36 AM To: linux-ker...@vger.kernel.org; sta...@vger.kernel.org Cc: Deucher, Alexander ; Kuehling, Felix ; Koenig, Christian ; Mike Lothian ; Sasha Levin ; Pan, Xinhui ; airl...@gmail.com; dan...@ffwll.ch; amd- g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: [PATCH AUTOSEL 6.1 08/10] drm/amdkfd: ignore crat by default From: Alex Deucher [ Upstream commit a6dea2d64ff92851e68cd4e20a35f6534286e016 ] We are dropping the IOMMUv2 path, so no need to enable this. It's often buggy on consumer platforms anyway. This is not needed for stable. Dropped this and the other ones you've pointed out, thanks! -- Thanks, Sasha
Re: [PATCH] lockdep: Fix static memory detection even more
On 9/3/23 14:11, Helge Deller wrote:
* Guenter Roeck :
Hi,
On Sat, Aug 12, 2023 at 05:48:52PM +0200, Helge Deller wrote:
On the parisc architecture, lockdep reports for all static objects which
are in the __initdata section (e.g. "setup_done" in devtmpfs,
"kthreadd_done" in init/main.c) this warning:
INFO: trying to register non-static key.
The warning itself is wrong, because those objects are in the __initdata
section, but the section itself is on parisc outside of range from
_stext to _end, which is why the static_obj() functions returns a wrong
answer.
While fixing this issue, I noticed that the whole existing check can
be simplified a lot.
Instead of checking against the _stext and _end symbols (which include
code areas too) just check for the .data and .bss segments (since we check a
data object). This can be done with the existing is_kernel_core_data()
macro.
In addition objects in the __initdata section can be checked with
init_section_contains().
This partly reverts and simplifies commit bac59d18c701 ("x86/setup: Fix static
memory detection").
Tested on x86-64 and parisc.
Signed-off-by: Helge Deller
Fixes: bac59d18c701 ("x86/setup: Fix static memory detection")
On loongarch, this patch results in the following backtrace.
EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
EFI stub: Exiting boot services
[0.00] INFO: trying to register non-static key.
[0.00] The code is fine but needs lockdep annotation, or maybe
[0.00] you didn't initialize this object before use?
[0.00] turning off the locking correctness validator.
[0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 6.5.0+ #1
[0.00] Stack : 90223d6c
91df
[0.00] 91df39a0 91df39a8
[0.00] 91df39a8 0001
9154b910
[0.00] fffe 91df39a8
[0.00] 0001 0003 0010
0030
[0.00] 0063 0001
[0.00] 91c60650
91e12000
[0.00] 91560bc0
92ee6000
[0.00] 90223d84
[0.00] 00b0 0004
0800
[0.00] ...
[0.00] Call Trace:
[0.00] [<90223d84>] show_stack+0x5c/0x180
[0.00] [<9153e0b4>] dump_stack_lvl+0x88/0xd0
[0.00] [<902bc548>] register_lock_class+0x768/0x770
[0.00] [<902bc710>] __lock_acquire+0xb0/0x2a18
[0.00] [<902bba1c>] lock_acquire+0x11c/0x328
[0.00] [<90b34a60>] __debug_object_init+0x60/0x244
[0.00] [<90337f94>] init_cgroup_housekeeping+0xe8/0x144
[0.00] [<9033e364>] init_cgroup_root+0x38/0xa0
[0.00] [<917801ac>] cgroup_init_early+0x44/0x16c
[0.00] [<91770758>] start_kernel+0x50/0x624
[0.00] [<915410b4>] kernel_entry+0xb4/0xc4
Reverting it fixes the problem. Bisect log attached.
This is also seen in v6.5.y and v6.4.y since the patch has been applied
to those branches.
Does this happens with CONFIG_SMP=n ?
If so, I think the untested patch below might fix the issue.
No, this is loongarch:defconfig with various debug options enabled.
That has CONFIG_SMP=y.
Guenter
Helge
---
[PATCH] loogarch: Keep PERCPU section in init section even for !CONFIG_SMP
Signed-off-by: Helge Deller
diff --git a/arch/loongarch/kernel/vmlinux.lds.S
b/arch/loongarch/kernel/vmlinux.lds.S
index b1686afcf876..32d61e931cdc 100644
--- a/arch/loongarch/kernel/vmlinux.lds.S
+++ b/arch/loongarch/kernel/vmlinux.lds.S
@@ -99,9 +99,7 @@ SECTIONS
EXIT_DATA
}
-#ifdef CONFIG_SMP
PERCPU_SECTION(1 << CONFIG_L1_CACHE_SHIFT)
-#endif
.init.bss : {
*(.init.bss)
Re: [PATCH 0/7] drm/msm/dp: Simplify DPCD related code with helpers
On 29/08/2023 21:47, Stephen Boyd wrote:
This driver open-codes a few of the DPCD register reads when it can be
simplified by using the helpers instead. This series reworks the MSM DP
driver to use the DPCD helpers and removes some dead code along the way.
There's the potential for even more code reduction around the test
registers, but I haven't tried to do that yet.
For the whole series:
Tested-by: Dmitry Baryshkov
Using drm_dp_get_phy_test_pattern() / drm_dp_set_phy_test_pattern()
would be definitely a benefit, especially since the latter one has
support for DP >= 1.2, while msm DP code doesn't.
Stephen Boyd (7):
drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()
drm/msm/dp: Use drm_dp_read_sink_count() helper
drm/msm/dp: Remove dead code related to downstream cap info
drm/msm/dp: Remove aux_cfg_update_done and related code
drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()
drm/msm/dp: Inline dp_link_parse_sink_count()
drm/msm/dp: Remove dp_display_is_ds_bridge()
Cc: Vinod Polimera
Cc: Kuogee Hsieh
drivers/gpu/drm/msm/dp/dp_display.c | 9 +--
drivers/gpu/drm/msm/dp/dp_link.c| 38 +-
drivers/gpu/drm/msm/dp/dp_panel.c | 105 +---
drivers/gpu/drm/msm/dp/dp_panel.h | 10 +--
4 files changed, 22 insertions(+), 140 deletions(-)
base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
--
With best wishes
Dmitry
Re: [PATCH 7/7] drm/msm/dp: Remove dp_display_is_ds_bridge()
On 29/08/2023 21:47, Stephen Boyd wrote:
This function is simply drm_dp_is_branch() so use that instead of
open-coding it.
Cc: Vinod Polimera
Cc: Kuogee Hsieh
Signed-off-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_display.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
Reviewed-by: Dmitry Baryshkov
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 76f13954015b..96bbf6fec2f1 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -341,19 +341,12 @@ static const struct component_ops dp_display_comp_ops = {
.unbind = dp_display_unbind,
};
-static bool dp_display_is_ds_bridge(struct dp_panel *panel)
-{
- return (panel->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
- DP_DWN_STRM_PORT_PRESENT);
-}
-
static bool dp_display_is_sink_count_zero(struct dp_display_private *dp)
Nit: you might as well inline this function
{
drm_dbg_dp(dp->drm_dev, "present=%#x sink_count=%d\n",
dp->panel->dpcd[DP_DOWNSTREAMPORT_PRESENT],
dp->link->sink_count);
- return dp_display_is_ds_bridge(dp->panel) &&
- (dp->link->sink_count == 0);
+ return drm_dp_is_branch(dp->panel->dpcd) && dp->link->sink_count == 0;
}
static void dp_display_send_hpd_event(struct msm_dp *dp_display)
--
With best wishes
Dmitry
Re: [PATCH 6/7] drm/msm/dp: Inline dp_link_parse_sink_count()
On 29/08/2023 21:47, Stephen Boyd wrote: The function dp_link_parse_sink_count() is really just drm_dp_read_sink_count(). It debug prints out the bit for content protection (DP_SINK_CP_READY), but that is not useful beyond debug because 'link->dp_link.sink_count' is overwritten to only contain the sink_count in this same function. Just use drm_dp_read_sink_count() in the one place this function is called to simplify. Cc: Vinod Polimera Cc: Kuogee Hsieh Signed-off-by: Stephen Boyd --- drivers/gpu/drm/msm/dp/dp_link.c | 38 +++- 1 file changed, 3 insertions(+), 35 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 5/7] drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()
On 29/08/2023 21:47, Stephen Boyd wrote: These are open-coded versions of common functions. Replace them with the common code to improve readability. Cc: Vinod Polimera Cc: Kuogee Hsieh Signed-off-by: Stephen Boyd --- drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 4/7] drm/msm/dp: Remove aux_cfg_update_done and related code
On 29/08/2023 21:47, Stephen Boyd wrote: The member 'aux_cfg_update_done' is always false. This is dead code that never runs. Remove it. Cc: Vinod Polimera Cc: Kuogee Hsieh Signed-off-by: Stephen Boyd --- drivers/gpu/drm/msm/dp/dp_panel.c | 15 --- 1 file changed, 15 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 3/7] drm/msm/dp: Remove dead code related to downstream cap info
On 29/08/2023 21:47, Stephen Boyd wrote: We read the downstream port count and capability info but never use it anywhere. Remove 'ds_port_cnt' and 'ds_cap_info' and any associated code from this driver. Fold the check for 'dfp_present' into a call to drm_dp_is_branch() at the one place it is used to get rid of any member storage related to downstream ports. Cc: Vinod Polimera Cc: Kuogee Hsieh Signed-off-by: Stephen Boyd --- drivers/gpu/drm/msm/dp/dp_panel.c | 25 +++-- drivers/gpu/drm/msm/dp/dp_panel.h | 6 -- 2 files changed, 3 insertions(+), 28 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 2/7] drm/msm/dp: Use drm_dp_read_sink_count() helper
On 29/08/2023 21:47, Stephen Boyd wrote: Use the common function drm_dp_read_sink_count() instead of open-coding it. This shrinks the kernel text a tiny bit. Cc: Vinod Polimera Cc: Kuogee Hsieh Signed-off-by: Stephen Boyd --- drivers/gpu/drm/msm/dp/dp_panel.c | 19 +++ 1 file changed, 7 insertions(+), 12 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 1/7] drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()
On 29/08/2023 21:47, Stephen Boyd wrote: This function duplicates the common function drm_dp_read_dpcd_caps(). The array of DPCD registers filled in is one size larger than the function takes, but from what I can tell that extra byte was never used. Resize the array and use the common function to reduce the code here. Cc: Vinod Polimera Cc: Kuogee Hsieh Signed-off-by: Stephen Boyd --- drivers/gpu/drm/msm/dp/dp_panel.c | 42 --- drivers/gpu/drm/msm/dp/dp_panel.h | 4 +-- 2 files changed, 6 insertions(+), 40 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[PATCH] drm/msm/dp: support setting the DP subconnector type
Read the downstream port info and set the subconnector type accordingly.
Signed-off-by: Dmitry Baryshkov
---
Dependencies: https://patchwork.freedesktop.org/series/123221/
---
drivers/gpu/drm/msm/dp/dp_display.c | 9 -
drivers/gpu/drm/msm/dp/dp_panel.c | 5 +
drivers/gpu/drm/msm/dp/dp_panel.h | 1 +
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 96bbf6fec2f1..8abeae658200 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -372,8 +372,12 @@ static int dp_display_send_hpd_notification(struct
dp_display_private *dp,
}
/* reset video pattern flag on disconnect */
- if (!hpd)
+ if (!hpd) {
dp->panel->video_test = false;
+ drm_dp_set_subconnector_property(dp->dp_display.connector,
+connector_status_disconnected,
+dp->panel->dpcd,
dp->panel->downstream_ports);
+ }
dp->dp_display.is_connected = hpd;
@@ -401,6 +405,9 @@ static int dp_display_process_hpd_high(struct
dp_display_private *dp)
dp_link_process_request(dp->link);
+ drm_dp_set_subconnector_property(dp->dp_display.connector,
connector_status_connected,
+dp->panel->dpcd,
dp->panel->downstream_ports);
+
edid = dp->panel->edid;
dp->dp_display.psr_supported = dp->panel->psr_cap.version &&
psr_enabled;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 97ba41593820..1cb54f26f5aa 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -162,6 +162,11 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
}
}
+ rc = drm_dp_read_downstream_info(panel->aux, dp_panel->dpcd,
+dp_panel->downstream_ports);
+ if (rc)
+ return rc;
+
kfree(dp_panel->edid);
dp_panel->edid = NULL;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 3cb1f8dcfd3b..a0dfc579c5f9 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -36,6 +36,7 @@ struct dp_panel_psr {
struct dp_panel {
/* dpcd raw data */
u8 dpcd[DP_RECEIVER_CAP_SIZE];
+ u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
struct dp_link_info link_info;
struct drm_dp_desc desc;
--
2.39.2
Re: [PATCH v1] drm/msm/dp: do not reinitialize phy unless retry during link training
On 09/08/2023 01:19, Kuogee Hsieh wrote:
DP PHY re-initialization done using dp_ctrl_reinitialize_mainlink() will
cause PLL unlocked initially and then PLL gets locked at the end of
initialization. PLL_UNLOCKED interrupt will fire during this time if the
interrupt mask is enabled.
However currently DP driver link training implementation incorrectly
re-initializes PHY unconditionally during link training as the PHY was
already configured in dp_ctrl_enable_mainlink_clocks().
Fix this by re-initializing the PHY only if the previous link training
failed.
[drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x0100 when not busy
Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/30
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
Reviewed-by: Dmitry Baryshkov
Tested-by: Dmitry Baryshkov
--
With best wishes
Dmitry
[PATCH v2] drm/bridge-connector: handle subconnector types
If the created connector type supports subconnector type property,
create and attach corresponding it. The default subtype value is 0,
which maps to the DRM_MODE_SUBCONNECTOR_Unknown type.
Signed-off-by: Dmitry Baryshkov
---
This is a leftover of my previous attempt to implement USB-C DisplayPort
uABI. The idea was dropped, but I consider this part still to be useful,
as it allows one to register corresponding subconnector properties and
also to export the subconnector type.
Changes since v1:
- Dropped all DP and USB-related patches
- Dropped the patch adding default subtype to
drm_connector_attach_dp_subconnector_property()
- Replaced creation of TV subconnector property with the check that it
was created beforehand (Neil, Laurent)
---
drivers/gpu/drm/drm_bridge_connector.c | 29 +-
include/drm/drm_bridge.h | 4
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index ca255609fb08..74a3164825dd 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -332,6 +332,7 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
struct i2c_adapter *ddc = NULL;
struct drm_bridge *bridge, *panel_bridge = NULL;
const char *path = NULL;
+ enum drm_mode_subconnector subconnector;
int connector_type;
int ret;
@@ -369,8 +370,10 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
if (bridge->ops & DRM_BRIDGE_OP_MODES)
bridge_connector->bridge_modes = bridge;
- if (!drm_bridge_get_next_bridge(bridge))
+ if (!drm_bridge_get_next_bridge(bridge)) {
connector_type = bridge->type;
+ subconnector = bridge->subtype;
+ }
if (!drm_bridge_get_next_bridge(bridge) &&
bridge->of_node)
@@ -418,6 +421,30 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
if (panel_bridge)
drm_panel_bridge_set_orientation(connector, panel_bridge);
+ if (connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
+ drm_connector_attach_dp_subconnector_property(connector);
+ } else if (connector_type == DRM_MODE_CONNECTOR_DVII) {
+ ret = drm_mode_create_dvi_i_properties(drm);
+ if (ret)
+ return ERR_PTR(ret);
+
+ drm_object_attach_property(>base,
+
drm->mode_config.dvi_i_subconnector_property,
+ subconnector);
+ } else if (connector_type == DRM_MODE_CONNECTOR_TV &&
+ subconnector) {
+ /*
+* We do not know which modes are supported by the HW, so the
+* property should be created in advance.
+*/
+ if (!drm->mode_config.tv_subconnector_property)
+ return ERR_PTR(-EINVAL);
+
+ drm_object_attach_property(>base,
+
drm->mode_config.tv_subconnector_property,
+ subconnector);
+ }
+
return connector;
}
EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index afa1de791075..440a8aa91e65 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -738,6 +738,10 @@ struct drm_bridge {
* identifies the type of connected display.
*/
int type;
+ /**
+* @subtype: the subtype of the connector for the DP/TV/DVI-I cases.
+*/
+ enum drm_mode_subconnector subtype;
/**
* @interlace_allowed: Indicate that the bridge can handle interlaced
* modes.
--
2.39.2
[RFC PATCH v1 04/12] drm/bridge-connector: set the PATH property for the connector
In order to properly identify connectors (in particular, DisplayPort
connectors wrapped into USB-C) allow bridge drivers to specify the value
to be used for connector's PATH property.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_bridge_connector.c | 12
include/drm/drm_bridge.h | 7 +++
2 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index bf73960c2c2a..008d730e1c2f 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -331,6 +331,7 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
struct drm_connector *connector;
struct i2c_adapter *ddc = NULL;
struct drm_bridge *bridge, *panel_bridge = NULL;
+ const char *path = NULL;
int connector_type;
int ret;
@@ -377,6 +378,9 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
connector->fwnode =
fwnode_handle_get(of_fwnode_handle(bridge->of_node));
#endif
+ if (bridge->path)
+ path = bridge->path;
+
if (bridge->ddc)
ddc = bridge->ddc;
@@ -405,6 +409,14 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
connector->polled = DRM_CONNECTOR_POLL_CONNECT
| DRM_CONNECTOR_POLL_DISCONNECT;
+ if (path) {
+ drm_object_attach_property(>base,
+ drm->mode_config.path_property,
+ 0);
+
+ drm_connector_set_path_property(connector, path);
+ }
+
if (panel_bridge)
drm_panel_bridge_set_orientation(connector, panel_bridge);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index c339fc85fd07..98e9d76474f4 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -753,6 +753,13 @@ struct drm_bridge {
* before the peripheral.
*/
bool pre_enable_prev_first;
+ /**
+* @path: the 'path' of the bridge. For bridges at the end of this
+* chain this is used to set the 'PATH' property of the connector.
+* This string is not freed manually, so one either should use a static
+* string here or a devres-allocated one.
+*/
+ const char *path;
/**
* @ddc: Associated I2C adapter for DDC access, if any.
*/
--
2.39.2
[RFC PATCH v1 12/12] usb: typec: qcom: define the bridge's path
In order to notify the userspace about the DRM connector's USB-C port,
export the corresponding port's name as the bridge's path field.
Signed-off-by: Dmitry Baryshkov
---
drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 11 +++
drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c | 4 +++-
drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h | 6 --
3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
index b9d4856101c7..452dc6437861 100644
--- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
+++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
@@ -156,6 +156,7 @@ static int qcom_pmic_typec_probe(struct platform_device
*pdev)
struct device_node *np = dev->of_node;
const struct pmic_typec_resources *res;
struct regmap *regmap;
+ char *tcpm_name;
u32 base[2];
int ret;
@@ -211,10 +212,6 @@ static int qcom_pmic_typec_probe(struct platform_device
*pdev)
mutex_init(>lock);
platform_set_drvdata(pdev, tcpm);
- tcpm->pmic_typec_drm = qcom_pmic_typec_init_drm(dev);
- if (IS_ERR(tcpm->pmic_typec_drm))
- return PTR_ERR(tcpm->pmic_typec_drm);
-
tcpm->tcpc.fwnode = device_get_named_child_node(tcpm->dev, "connector");
if (!tcpm->tcpc.fwnode)
return -EINVAL;
@@ -225,6 +222,12 @@ static int qcom_pmic_typec_probe(struct platform_device
*pdev)
goto fwnode_remove;
}
+ tcpm_name = tcpm_port_get_name(tcpm->tcpm_port);
+ tcpm->pmic_typec_drm = qcom_pmic_typec_init_drm(dev, tcpm_name);
+ kfree(tcpm_name);
+ if (IS_ERR(tcpm->pmic_typec_drm))
+ return PTR_ERR(tcpm->pmic_typec_drm);
+
ret = qcom_pmic_typec_port_start(tcpm->pmic_typec_port,
tcpm->tcpm_port);
if (ret)
diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c
b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c
index e202eb7b52db..7bd7f4bf3539 100644
--- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c
+++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c
@@ -21,7 +21,8 @@ static const struct drm_bridge_funcs
qcom_pmic_typec_bridge_funcs = {
.attach = qcom_pmic_typec_attach,
};
-struct pmic_typec_drm *qcom_pmic_typec_init_drm(struct device *dev)
+struct pmic_typec_drm *qcom_pmic_typec_init_drm(struct device *dev,
+ const char *path)
{
struct pmic_typec_drm *tcpm_drm;
@@ -33,6 +34,7 @@ struct pmic_typec_drm *qcom_pmic_typec_init_drm(struct device
*dev)
tcpm_drm->bridge.of_node = of_get_child_by_name(dev->of_node,
"connector");
tcpm_drm->bridge.ops = DRM_BRIDGE_OP_HPD;
tcpm_drm->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
+ tcpm_drm->bridge.path = devm_kstrdup(dev, path, GFP_KERNEL);
return ERR_PTR(devm_drm_bridge_add(dev, _drm->bridge));
}
diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h
b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h
index 01f4bb71346b..259c047265f7 100644
--- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h
+++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h
@@ -9,9 +9,11 @@
struct pmic_typec_drm;
#if IS_ENABLED(CONFIG_DRM)
-struct pmic_typec_drm *qcom_pmic_typec_init_drm(struct device *dev);
+struct pmic_typec_drm *qcom_pmic_typec_init_drm(struct device *dev,
+ const char *path);
#else
-static inline pmic_typec_drm *qcom_pmic_typec_init_drm(struct device *dev)
+static inline pmic_typec_drm *qcom_pmic_typec_init_drm(struct device *dev,
+ const char *path)
{
return NULL;
}
--
2.39.2
[RFC PATCH v1 10/12] usb: typec: qcom: implement proper error path in probe()
Implement proper error path in the qcom_pmic_typec_probe(). This makes
sure that we properly disable hardware blocks and do not leak memory.
Fixes: a4422ff22142 ("usb: typec: qcom: Add Qualcomm PMIC Type-C driver")
Signed-off-by: Dmitry Baryshkov
---
drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
index 581199d37b49..ceda594a8d56 100644
--- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
+++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
@@ -258,15 +258,22 @@ static int qcom_pmic_typec_probe(struct platform_device
*pdev)
ret = qcom_pmic_typec_port_start(tcpm->pmic_typec_port,
tcpm->tcpm_port);
if (ret)
- goto fwnode_remove;
+ goto tcpm_unregister_port;
ret = qcom_pmic_typec_pdphy_start(tcpm->pmic_typec_pdphy,
tcpm->tcpm_port);
if (ret)
- goto fwnode_remove;
+ goto typec_stop;
return 0;
+
+typec_stop:
+ qcom_pmic_typec_port_stop(tcpm->pmic_typec_port);
+
+tcpm_unregister_port:
+ tcpm_unregister_port(tcpm->tcpm_port);
+
fwnode_remove:
fwnode_remove_software_node(tcpm->tcpc.fwnode);
--
2.39.2
[RFC PATCH v1 07/12] soc: qcom: pmic_glink_altmode: report that this is a Type-C connector
Set the bridge's path property to point out that this connector is wrapped into the Type-C port. We can not really identify the exact Type-C port because it is registered separately, by another driver, which is not mandatory and the corresponding device is not even present on some of platforms, like sc8180x or sm8350. Thus we use the shortened version of the PATH, which includes just the 'typec:' part. Signed-off-by: Dmitry Baryshkov --- drivers/soc/qcom/pmic_glink_altmode.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c index 974c14d1e0bf..a5b72046caaa 100644 --- a/drivers/soc/qcom/pmic_glink_altmode.c +++ b/drivers/soc/qcom/pmic_glink_altmode.c @@ -466,6 +466,7 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev, alt_port->bridge.of_node = to_of_node(fwnode); alt_port->bridge.ops = DRM_BRIDGE_OP_HPD; alt_port->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; + alt_port->bridge.path = "typec:"; ret = devm_drm_bridge_add(dev, _port->bridge); if (ret) -- 2.39.2
[RFC PATCH v1 01/12] Revert "drm/sysfs: Link DRM connectors to corresponding Type-C connectors"
The kdev->fwnode pointer is never set in drm_sysfs_connector_add(), so
dev_fwnode() checks never succeed, making the respective commit NOP.
And if drm_sysfs_connector_add() is modified to set kdev->fwnode, it
breaks drivers already using components (as it was pointed at [1]),
resulting in a deadlock. Lockdep trace is provided below.
Granted these two issues, it seems impractical to fix this commit in any
sane way. Revert it instead.
[1] https://lore.kernel.org/dri-devel/Y24bcYJKGy%2Fgd5fV@phenom.ffwll.local/
WARNING: possible recursive locking detected
6.5.0-rc6-next-20230816-10542-g090e2ca9feae-dirty #713 Tainted: GW
kworker/u16:0/11 is trying to acquire lock:
ce0f54bea490 (component_mutex){+.+.}-{3:3}, at: __component_add+0x64/0x170
but task is already holding lock:
ce0f54bea490 (component_mutex){+.+.}-{3:3}, at: __component_add+0x64/0x170
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
lock(component_mutex);
lock(component_mutex);
*** DEADLOCK ***
May be due to missing lock nesting notation
6 locks held by kworker/u16:0/11:
#0: 5b7680008d38 ((wq_completion)events_unbound){+.+.}-{0:0}, at:
process_one_work+0x14c/0x51c
#1: 8000800abde0 (deferred_probe_work){+.+.}-{0:0}, at:
process_one_work+0x14c/0x51c
#2: 5b76837a2908 (>mutex){}-{3:3}, at: __device_attach+0x38/0x188
#3: ce0f54bea490 (component_mutex){+.+.}-{3:3}, at:
__component_add+0x64/0x170
#4: ce0f54bdeb40 (drm_connector_list_iter){.+.+}-{0:0}, at:
drm_modeset_register_all+0x80/0x9c
#5: 5b76866ad0d0 (>mutex){+.+.}-{3:3}, at:
drm_connector_register.part.0+0x28/0x104
stack backtrace:
CPU: 6 PID: 11 Comm: kworker/u16:0 Tainted: GW
6.5.0-rc6-next-20230816-10542-g090e2ca9feae-dirty #713
Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
Workqueue: events_unbound deferred_probe_work_func
Call trace:
dump_backtrace+0x98/0xf0
show_stack+0x18/0x24
dump_stack_lvl+0x60/0xac
dump_stack+0x18/0x24
print_deadlock_bug+0x254/0x340
__lock_acquire+0x105c/0x1ebc
lock_acquire+0x1ec/0x314
__lock_acquire+0x105c/0x1ebc
lock_acquire+0x1ec/0x314
__mutex_lock+0xa0/0x77c
mutex_lock_nested+0x24/0x30
__component_add+0x64/0x170
component_add+0x14/0x20
drm_sysfs_connector_add+0x144/0x1a0
drm_connector_register.part.0+0x5c/0x104
drm_connector_register_all+0x84/0x160
drm_modeset_register_all+0x80/0x9c
drm_dev_register+0x120/0x238
msm_drm_bind+0x550/0x6e0
try_to_bring_up_aggregate_device+0x164/0x1d0
__component_add+0xa8/0x170
component_add+0x14/0x20
dsi_dev_attach+0x20/0x2c
dsi_host_attach+0x9c/0x144
devm_mipi_dsi_attach+0x34/0xb4
lt9611uxc_attach_dsi.isra.0+0x84/0xfc
lt9611uxc_probe+0x5ac/0x66c
i2c_device_probe+0x148/0x290
really_probe+0x148/0x2ac
__driver_probe_device+0x78/0x12c
driver_probe_device+0x3c/0x160
__device_attach_driver+0xb8/0x138
bus_for_each_drv+0x80/0xdc
__device_attach+0x9c/0x188
device_initial_probe+0x14/0x20
bus_probe_device+0xac/0xb0
deferred_probe_work_func+0x8c/0xc8
process_one_work+0x1ec/0x51c
worker_thread+0x1ec/0x3e4
kthread+0x120/0x124
ret_from_fork+0x10/0x20
Fixes: c5c51b242062 ("drm/sysfs: Link DRM connectors to corresponding Type-C
connectors")
Cc: Won Chung
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_sysfs.c | 40 -
1 file changed, 40 deletions(-)
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index b169b3e44a92..06662cc8d3f4 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -11,14 +11,12 @@
*/
#include
-#include
#include
#include
#include
#include
#include
#include
-#include
#include
#include
@@ -98,34 +96,6 @@ static char *drm_devnode(const struct device *dev, umode_t
*mode)
return kasprintf(GFP_KERNEL, "dri/%s", dev_name(dev));
}
-static int typec_connector_bind(struct device *dev,
- struct device *typec_connector, void *data)
-{
- int ret;
-
- ret = sysfs_create_link(>kobj, _connector->kobj,
"typec_connector");
- if (ret)
- return ret;
-
- ret = sysfs_create_link(_connector->kobj, >kobj,
"drm_connector");
- if (ret)
- sysfs_remove_link(>kobj, "typec_connector");
-
- return ret;
-}
-
-static void typec_connector_unbind(struct device *dev,
- struct device *typec_connector, void *data)
-{
- sysfs_remove_link(_connector->kobj, "drm_connector");
- sysfs_remove_link(>kobj, "typec_connector");
-}
-
-static const struct component_ops typec_connector_ops = {
- .bind = typec_connector_bind,
- .unbind = typec_connector_unbind,
-};
-
static CLASS_ATTR_STRING(version, S_IRUGO, "drm 1.1.0 20060810");
/**
@@ -394,16 +364,9 @@ int drm_sysfs_connector_add(struct drm_connector
*connector)
[RFC PATCH v1 02/12] drm/sysfs: link DRM connector device to the connector's fw nodes
Setup the of_node and fwnode fields for the DRM connector device, making respective links to appear in /sys. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_sysfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index 06662cc8d3f4..cb926d95ef0e 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -345,6 +345,8 @@ int drm_sysfs_connector_add(struct drm_connector *connector) kdev->class = drm_class; kdev->type = _sysfs_device_connector; kdev->parent = dev->primary->kdev; + kdev->of_node = to_of_node(connector->fwnode); + kdev->fwnode = connector->fwnode; kdev->groups = connector_dev_groups; kdev->release = drm_sysfs_release; dev_set_drvdata(kdev, connector); -- 2.39.2
[RFC PATCH v1 11/12] usb: typec: qcom: extract DRM bridge functionality to separate file
In order to simplify source code and to reduce the amount of ifdefs in
the C files, extract the DRM bridge functionality to the separate file.
Suggested-by: Heikki Krogerus
Suggested-by: Bryan O'Donoghue
Signed-off-by: Dmitry Baryshkov
---
drivers/usb/typec/tcpm/qcom/Makefile | 4 ++
drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 40 +++
.../usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c | 38 ++
.../usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h | 20 ++
4 files changed, 67 insertions(+), 35 deletions(-)
create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c
create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h
diff --git a/drivers/usb/typec/tcpm/qcom/Makefile
b/drivers/usb/typec/tcpm/qcom/Makefile
index dc1e8832e197..6d01ec97c9fd 100644
--- a/drivers/usb/typec/tcpm/qcom/Makefile
+++ b/drivers/usb/typec/tcpm/qcom/Makefile
@@ -4,3 +4,7 @@ obj-$(CONFIG_TYPEC_QCOM_PMIC) += qcom_pmic_tcpm.o
qcom_pmic_tcpm-y += qcom_pmic_typec.o \
qcom_pmic_typec_port.o \
qcom_pmic_typec_pdphy.o
+
+ifneq ($(CONFIG_DRM),)
+ qcom_pmic_tcpm-y+= qcom_pmic_typec_drm.o
+endif
diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
index ceda594a8d56..b9d4856101c7 100644
--- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
+++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
@@ -18,8 +18,7 @@
#include
#include
-#include
-
+#include "qcom_pmic_typec_drm.h"
#include "qcom_pmic_typec_pdphy.h"
#include "qcom_pmic_typec_port.h"
@@ -34,9 +33,9 @@ struct pmic_typec {
struct tcpc_dev tcpc;
struct pmic_typec_pdphy *pmic_typec_pdphy;
struct pmic_typec_port *pmic_typec_port;
+ struct pmic_typec_drm *pmic_typec_drm;
boolvbus_enabled;
struct mutexlock; /* VBUS state serialization */
- struct drm_bridge bridge;
};
#define tcpc_to_tcpm(_tcpc_) container_of(_tcpc_, struct pmic_typec, tcpc)
@@ -150,35 +149,6 @@ static int qcom_pmic_typec_init(struct tcpc_dev *tcpc)
return 0;
}
-#if IS_ENABLED(CONFIG_DRM)
-static int qcom_pmic_typec_attach(struct drm_bridge *bridge,
-enum drm_bridge_attach_flags flags)
-{
- return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
-}
-
-static const struct drm_bridge_funcs qcom_pmic_typec_bridge_funcs = {
- .attach = qcom_pmic_typec_attach,
-};
-
-static int qcom_pmic_typec_init_drm(struct pmic_typec *tcpm)
-{
- tcpm->bridge.funcs = _pmic_typec_bridge_funcs;
-#ifdef CONFIG_OF
- tcpm->bridge.of_node = of_get_child_by_name(tcpm->dev->of_node,
"connector");
-#endif
- tcpm->bridge.ops = DRM_BRIDGE_OP_HPD;
- tcpm->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
-
- return devm_drm_bridge_add(tcpm->dev, >bridge);
-}
-#else
-static int qcom_pmic_typec_init_drm(struct pmic_typec *tcpm)
-{
- return 0;
-}
-#endif
-
static int qcom_pmic_typec_probe(struct platform_device *pdev)
{
struct pmic_typec *tcpm;
@@ -241,9 +211,9 @@ static int qcom_pmic_typec_probe(struct platform_device
*pdev)
mutex_init(>lock);
platform_set_drvdata(pdev, tcpm);
- ret = qcom_pmic_typec_init_drm(tcpm);
- if (ret)
- return ret;
+ tcpm->pmic_typec_drm = qcom_pmic_typec_init_drm(dev);
+ if (IS_ERR(tcpm->pmic_typec_drm))
+ return PTR_ERR(tcpm->pmic_typec_drm);
tcpm->tcpc.fwnode = device_get_named_child_node(tcpm->dev, "connector");
if (!tcpm->tcpc.fwnode)
diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c
b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c
new file mode 100644
index ..e202eb7b52db
--- /dev/null
+++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023, Linaro Ltd. All rights reserved.
+ */
+
+#include
+
+#include
+
+struct pmic_typec_drm {
+ struct drm_bridge bridge;
+};
+
+static int qcom_pmic_typec_attach(struct drm_bridge *bridge,
+enum drm_bridge_attach_flags flags)
+{
+ return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
+}
+
+static const struct drm_bridge_funcs qcom_pmic_typec_bridge_funcs = {
+ .attach = qcom_pmic_typec_attach,
+};
+
+struct pmic_typec_drm *qcom_pmic_typec_init_drm(struct device *dev)
+{
+ struct pmic_typec_drm *tcpm_drm;
+
+ tcpm_drm = devm_kzalloc(dev, sizeof(*tcpm_drm), GFP_KERNEL);
+ if (!tcpm_drm)
+ return ERR_PTR(-ENOMEM);
+
+ tcpm_drm->bridge.funcs = _pmic_typec_bridge_funcs;
+ tcpm_drm->bridge.of_node = of_get_child_by_name(dev->of_node,
"connector");
+ tcpm_drm->bridge.ops = DRM_BRIDGE_OP_HPD;
+
[RFC PATCH v1 09/12] usb: typec: tcpm: support generating Type-C port names for userspace
We need a way to generate and return the Type-C port device names. For
example, it is going to be used by the DRM to provide PATH property for
DisplayPort connectors.
Signed-off-by: Dmitry Baryshkov
---
drivers/usb/typec/tcpm/tcpm.c | 14 ++
include/linux/usb/tcpm.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index d962f67c95ae..9709b56a3046 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -5539,6 +5539,20 @@ bool tcpm_port_is_toggling(struct tcpm_port *port)
}
EXPORT_SYMBOL_GPL(tcpm_port_is_toggling);
+/**
+ * tcpm_port_get_name - get the name of the corresponding USB Type-C port
+ * @port: TCPM port instance
+ *
+ * Returns a name of the USB Type-C port correponding to the passed TCPM port
+ * instance on success or NULL when the port has not been enumerated yet. The
+ * resulting string should be freed by the caller.
+ */
+char *tcpm_port_get_name(struct tcpm_port *port)
+{
+ return typec_port_get_name(port->typec_port);
+}
+EXPORT_SYMBOL_GPL(tcpm_port_get_name);
+
static void tcpm_enable_frs_work(struct kthread_work *work)
{
struct tcpm_port *port = container_of(work, struct tcpm_port,
enable_frs);
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index ab7ca872950b..623c34788680 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -161,6 +161,8 @@ struct tcpm_port;
struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev
*tcpc);
void tcpm_unregister_port(struct tcpm_port *port);
+char *tcpm_port_get_name(struct tcpm_port *port);
+
void tcpm_vbus_change(struct tcpm_port *port);
void tcpm_cc_change(struct tcpm_port *port);
void tcpm_sink_frs(struct tcpm_port *port);
--
2.39.2
[RFC PATCH v1 08/12] usb: typec: support generating Type-C port names for userspace
We need a way to generate and return the Type-C port device names. For
example, it is going to be used by the DRM to provide PATH property for
DisplayPort connectors.
Signed-off-by: Dmitry Baryshkov
---
drivers/usb/typec/class.c | 14 ++
include/linux/usb/typec.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9c1dbf3c00e0..7394a2ecef6f 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -2327,6 +2327,20 @@ void typec_unregister_port(struct typec_port *port)
}
EXPORT_SYMBOL_GPL(typec_unregister_port);
+/**
+ * typec_port_get_name - Get USB Type-C Port name
+ * @port: The port to describe
+ *
+ * Returns a name of the passed USB Type-C port on success or NULL when the
+ * port has not been enumerated yet. The resulting string should be freed by
+ * the caller.
+ */
+char *typec_port_get_name(struct typec_port *port)
+{
+ return kasprintf(GFP_KERNEL, "typec:%s", dev_name(>dev));
+}
+EXPORT_SYMBOL_GPL(typec_port_get_name);
+
static int __init typec_init(void)
{
int ret;
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 8fa781207970..4aa9c9378383 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -303,6 +303,8 @@ struct typec_plug *typec_register_plug(struct typec_cable
*cable,
struct typec_plug_desc *desc);
void typec_unregister_plug(struct typec_plug *plug);
+char *typec_port_get_name(struct typec_port *port);
+
void typec_set_data_role(struct typec_port *port, enum typec_data_role role);
void typec_set_pwr_role(struct typec_port *port, enum typec_role role);
void typec_set_vconn_role(struct typec_port *port, enum typec_role role);
--
2.39.2
[RFC PATCH v1 06/12] soc: qcom: pmic_glink_altmode: fix DRM connector type
During discussions regarding USB-C vs native DisplayPort it was pointed out that other implementations (i915, AMD) are using DRM_MODE_CONNECTOR_DisplayPort for both native and USB-C-wrapped DP output. Follow this example and make the pmic_glink_altmode driver also report DipslayPort connector rather than the USB one. Cc: Simon Ser --- drivers/soc/qcom/pmic_glink_altmode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c index d05e0d6edf49..974c14d1e0bf 100644 --- a/drivers/soc/qcom/pmic_glink_altmode.c +++ b/drivers/soc/qcom/pmic_glink_altmode.c @@ -465,7 +465,7 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev, alt_port->bridge.funcs = _glink_altmode_bridge_funcs; alt_port->bridge.of_node = to_of_node(fwnode); alt_port->bridge.ops = DRM_BRIDGE_OP_HPD; - alt_port->bridge.type = DRM_MODE_CONNECTOR_USB; + alt_port->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; ret = devm_drm_bridge_add(dev, _port->bridge); if (ret) -- 2.39.2
[RFC PATCH v1 05/12] drm/bridge: remove conditionals around devicetree pointers
Remove ifdef CONFIG_OF around the drm_bridge::of_node field. This follow
the correponding change to struct device we had since 2.6.39. Having
conditional around the of_node pointers turns out to make driver code
use ugly #ifdef blocks. Drop the conditionals and remove the #ifdef
blocks from the affected drivers.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/bridge/panel.c | 2 --
drivers/gpu/drm/drm_bridge_connector.c | 2 --
include/drm/drm_bridge.h | 2 --
3 files changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 9316384b4474..7f41525f7a6e 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -302,9 +302,7 @@ struct drm_bridge *drm_panel_bridge_add_typed(struct
drm_panel *panel,
panel_bridge->panel = panel;
panel_bridge->bridge.funcs = _bridge_bridge_funcs;
-#ifdef CONFIG_OF
panel_bridge->bridge.of_node = panel->dev->of_node;
-#endif
panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES;
panel_bridge->bridge.type = connector_type;
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index 008d730e1c2f..ca255609fb08 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -372,11 +372,9 @@ 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->path)
path = bridge->path;
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 98e9d76474f4..afa1de791075 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -716,10 +716,8 @@ struct drm_bridge {
struct drm_encoder *encoder;
/** @chain_node: used to form a bridge chain */
struct list_head chain_node;
-#ifdef CONFIG_OF
/** @of_node: device node pointer to the bridge */
struct device_node *of_node;
-#endif
/** @list: to keep track of all added bridges */
struct list_head list;
/**
--
2.39.2
[RFC PATCH v1 03/12] drm/connector: extend PATH property to covert Type-C case
Userspace needs to identify whether the DisplayPort connector is a native one or it is wrapped into the USB-C port. Moreover the userspace might need to point user to the exact location of this Type-C port on the laptop case. Existing USB-C altmode implementations (i915, amdgpu) have used the DRM_MODE_CONNECTOR_DisplayPort even for such ports. To keep backwards compatibility we can not change this to DRM_MODE_CONNECTOR_USB. Therefore the kernel needs some other way to represent this information. To facilitate this, reuse the 'PATH' property, which was used to describe the DP port in the DP MST configuration. Use either 'typec:portN' to point out to the Type-C port class device, or just 'typec:' if the corresponding port can not be identified. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/drm_connector.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 05fc29a54801..a326782e936e 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1185,10 +1185,14 @@ static const u32 dp_colorspaces = * never read back the value of "DPMS" because it can be incorrect. * PATH: * Connector path property to identify how this sink is physically - * connected. Used by DP MST. This should be set by calling - * drm_connector_set_path_property(), in the case of DP MST with the - * path property the MST manager created. Userspace cannot change this + * connected. This should be set by calling + * drm_connector_set_path_property(). Userspace cannot change this * property. + * In the case of DP MST this is equal to the path property the MST + * manager created. It is equal to 'mst:baseID-port-port...'. + * In the case of DP USB-C connector this is equal to the 'typec:portN', + * pointing to the corresponding Type-C port device or just 'typec' if the + * corresponding Type-C port can not be identified. * TILE: * Connector tile group property to indicate how a set of DRM connector * compose together into one logical screen. This is used by both high-res -- 2.39.2
[RFC PATCH v1 00/12] drm, usb/typec: uABI for USB-C DisplayPort connectors
During the discussion regarding DisplayPort wrapped in the USB-C connectors (via the USB-C altmode) it was pointed out that currently there is no good way to let userspace know if the DRM connector in question is the 'native' DP connector or if it is the USB-C connector. An attempt to use DRM_MODE_CONNECTOR_USB for such connectors was declined, as existing DP drivers (i915, AMD) use DRM_MODE_CONNECTOR_DisplayPort. New drivers should behave in the same way. An attempt to use subconnector property was also declined. It is defined to the type of the DP dongle connector rather than the host connector. This attempt targets reusing the connector's PATH property. Currently this property is only used for the DP MST connectors. This patchset reuses it to point out to the corresponding registered typec port device. Dmitry Baryshkov (12): Revert "drm/sysfs: Link DRM connectors to corresponding Type-C connectors" drm/sysfs: link DRM connector device to the connector's fw nodes drm/connector: extend PATH property to covert Type-C case drm/bridge-connector: set the PATH property for the connector drm/bridge: remove conditionals around devicetree pointers soc: qcom: pmic_glink_altmode: fix DRM connector type soc: qcom: pmic_glink_altmode: report that this is a Type-C connector usb: typec: support generating Type-C port names for userspace usb: typec: tcpm: support generating Type-C port names for userspace usb: typec: qcom: implement proper error path in probe() usb: typec: qcom: extract DRM bridge functionality to separate file usb: typec: qcom: define the bridge's path drivers/gpu/drm/bridge/panel.c| 2 - drivers/gpu/drm/drm_bridge_connector.c| 14 - drivers/gpu/drm/drm_connector.c | 10 +++- drivers/gpu/drm/drm_sysfs.c | 42 +- drivers/soc/qcom/pmic_glink_altmode.c | 3 +- drivers/usb/typec/class.c | 14 + drivers/usb/typec/tcpm/qcom/Makefile | 4 ++ drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 56 ++- .../usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c | 40 + .../usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h | 22 drivers/usb/typec/tcpm/tcpm.c | 14 + include/drm/drm_bridge.h | 9 ++- include/linux/usb/tcpm.h | 2 + include/linux/usb/typec.h | 2 + 14 files changed, 146 insertions(+), 88 deletions(-) create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h -- 2.39.2
[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 | 31 +++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index 10b52224db37..bf73960c2c2a 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 =
@@ -191,6 +207,8 @@ static void drm_bridge_connector_destroy(struct
drm_connector *connector)
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
+ fwnode_handle_put(connector->fwnode);
+
kfree(bridge_connector);
}
@@ -216,6 +234,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 +371,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 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 it through drm-intel. [1] https://patchwork.freedesktop.org/series/103449/ Changes since v6: - Fixed the fwnode refcount in drm/bridge-connector 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| 36 ++- 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, 62 insertions(+), 23 deletions(-) -- 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 c44d5bcf1284..05fc29a54801 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -3051,6 +3051,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
@@ -3060,7 +3061,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;
@@ -3069,7 +3071,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
Re: [PATCH] lockdep: Fix static memory detection even more
* Guenter Roeck :
> Hi,
>
> On Sat, Aug 12, 2023 at 05:48:52PM +0200, Helge Deller wrote:
> > On the parisc architecture, lockdep reports for all static objects which
> > are in the __initdata section (e.g. "setup_done" in devtmpfs,
> > "kthreadd_done" in init/main.c) this warning:
> >
> > INFO: trying to register non-static key.
> >
> > The warning itself is wrong, because those objects are in the __initdata
> > section, but the section itself is on parisc outside of range from
> > _stext to _end, which is why the static_obj() functions returns a wrong
> > answer.
> >
> > While fixing this issue, I noticed that the whole existing check can
> > be simplified a lot.
> > Instead of checking against the _stext and _end symbols (which include
> > code areas too) just check for the .data and .bss segments (since we check a
> > data object). This can be done with the existing is_kernel_core_data()
> > macro.
> >
> > In addition objects in the __initdata section can be checked with
> > init_section_contains().
> >
> > This partly reverts and simplifies commit bac59d18c701 ("x86/setup: Fix
> > static
> > memory detection").
> >
> > Tested on x86-64 and parisc.
> >
> > Signed-off-by: Helge Deller
> > Fixes: bac59d18c701 ("x86/setup: Fix static memory detection")
>
> On loongarch, this patch results in the following backtrace.
>
> EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
> EFI stub: Exiting boot services
> [0.00] INFO: trying to register non-static key.
> [0.00] The code is fine but needs lockdep annotation, or maybe
> [0.00] you didn't initialize this object before use?
> [0.00] turning off the locking correctness validator.
> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 6.5.0+ #1
> [0.00] Stack : 90223d6c
> 91df
> [0.00] 91df39a0 91df39a8
>
> [0.00] 91df39a8 0001
> 9154b910
> [0.00] fffe 91df39a8
>
> [0.00] 0001 0003 0010
> 0030
> [0.00] 0063 0001
>
> [0.00] 91c60650
> 91e12000
> [0.00] 91560bc0
> 92ee6000
> [0.00] 90223d84
>
> [0.00] 00b0 0004
> 0800
> [0.00] ...
> [0.00] Call Trace:
> [0.00] [<90223d84>] show_stack+0x5c/0x180
> [0.00] [<9153e0b4>] dump_stack_lvl+0x88/0xd0
> [0.00] [<902bc548>] register_lock_class+0x768/0x770
> [0.00] [<902bc710>] __lock_acquire+0xb0/0x2a18
> [0.00] [<902bba1c>] lock_acquire+0x11c/0x328
> [0.00] [<90b34a60>] __debug_object_init+0x60/0x244
> [0.00] [<90337f94>] init_cgroup_housekeeping+0xe8/0x144
> [0.00] [<9033e364>] init_cgroup_root+0x38/0xa0
> [0.00] [<917801ac>] cgroup_init_early+0x44/0x16c
> [0.00] [<91770758>] start_kernel+0x50/0x624
> [0.00] [<915410b4>] kernel_entry+0xb4/0xc4
>
> Reverting it fixes the problem. Bisect log attached.
>
> This is also seen in v6.5.y and v6.4.y since the patch has been applied
> to those branches.
Does this happens with CONFIG_SMP=n ?
If so, I think the untested patch below might fix the issue.
Helge
---
[PATCH] loogarch: Keep PERCPU section in init section even for !CONFIG_SMP
Signed-off-by: Helge Deller
diff --git a/arch/loongarch/kernel/vmlinux.lds.S
b/arch/loongarch/kernel/vmlinux.lds.S
index b1686afcf876..32d61e931cdc 100644
--- a/arch/loongarch/kernel/vmlinux.lds.S
+++ b/arch/loongarch/kernel/vmlinux.lds.S
@@ -99,9 +99,7 @@ SECTIONS
EXIT_DATA
}
-#ifdef CONFIG_SMP
PERCPU_SECTION(1 << CONFIG_L1_CACHE_SHIFT)
-#endif
.init.bss : {
*(.init.bss)
Re: [PATCH v4 0/3] drm: simplify support for transparent DRM bridges
On Tue, 22 Aug 2023 at 17:17, Laurent Pinchart wrote: > > Hi Dmitry, > > Thank you for the patches. > > On Thu, Aug 17, 2023 at 05:55:13PM +0300, Dmitry Baryshkov wrote: > > Supporting DP/USB-C can result in a chain of several transparent > > bridges (PHY, redrivers, mux, etc). This results in drivers having > > similar boilerplate code for such bridges. > > What do you mean by transparent bridge here ? Bridges are a DRM concept, > and as far as I can tell, a PHY isn't a bridge. Why does it need to be > handled as one, especially if it's completely transparent ? > > > Next, these drivers are susceptible to -EPROBE_DEFER loops: the next > > bridge can either be probed from the bridge->attach callback, when it is > > too late to return -EPROBE_DEFER, or from the probe() callback, when the > > next bridge might not yet be available, because it depends on the > > resources provided by the probing device. > > Can't device links help avoiding defer probing in those cases ? It looks like both Neil and I missed this question. Two items wrt devlinks. First, I view them as a helper. So if one disables the devlinks enforcement, he'd still get a deferral loop. Second, in this case we can not enforce devlinks (or return -EPROBE_DEFER from the probe() function) because the next bridge is not yet available when the main driver probes. Unfortunately bridges are allocated in the opposite order. So, using AUX devices helps us to break it. Because first typec mux/retimer/switch/etc devices probe (in the direction from the typec source to the typec port). Then DRM bridge devices are probed starting from the end of the chain (connector) to the DP source (root DP bridge/controller). > > > Last, but not least, this results in the the internal knowledge of DRM > > subsystem slowly diffusing into other subsystems, like PHY or USB/TYPEC. > > Why so ? The PHY subsystem should provide a PHY, without considering > what subsystem it will be used by. This patch series seems to me to > actually create this DRM dependency in other subsystems, which I don't > think is a very good idea. Resources should be registered in their own > subsystem with the appropriate API, not in a way that is tied to a > particular consumer. > > > To solve all these issues, define a separate DRM helper, which creates > > separate aux device just for the bridge. During probe such aux device > > doesn't result in the EPROBE_DEFER loops. Instead it allows the device > > drivers to probe properly, according to the actual resource > > dependencies. The bridge auxdevs are then probed when the next bridge > > becomes available, sparing drivers from drm_bridge_attach() returning > > -EPROBE_DEFER. > > I'm not thrilled :-( Let's discuss the questions above first. Laurent, please excuse me for the ping. Any further response from your side? I'd like to send the next iteration of this patchset. > > Proposed merge strategy: immutable branch with the drm commit, which is > > then merged into PHY and USB subsystems together with the corresponding > > patch. -- With best wishes Dmitry
Re: [PATCH v4 4/4] drm/msm/dsi: Enable widebus for DSI
On 2023-08-22 10:42:07, Jessica Zhang wrote:
> DSI 6G v2.5.x+ supports a data-bus widen mode that allows DSI to send
> 48 bits of compressed data instead of 24.
>
> Enable this mode whenever DSC is enabled for supported chipsets.
>
> Signed-off-by: Jessica Zhang
> ---
> drivers/gpu/drm/msm/dsi/dsi.c | 2 +-
> drivers/gpu/drm/msm/dsi/dsi.h | 1 +
> drivers/gpu/drm/msm/dsi/dsi_host.c | 31 +++
> 3 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 4cf424b3509f..7327bfc06a84 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -19,7 +19,7 @@ struct drm_dsc_config *msm_dsi_get_dsc_config(struct
> msm_dsi *msm_dsi)
>
> bool msm_dsi_wide_bus_enabled(struct msm_dsi *msm_dsi)
> {
> - return false;
> + return msm_dsi_host_is_widebus_enabled(msm_dsi->host);
> }
>
> static int dsi_get_phy(struct msm_dsi *msm_dsi)
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> index bd3763a5d723..a557d2c1aaff 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> @@ -134,6 +134,7 @@ int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host,
> bool is_bonded_dsi);
> void msm_dsi_host_snapshot(struct msm_disp_state *disp_state, struct
> mipi_dsi_host *host);
> void msm_dsi_host_test_pattern_en(struct mipi_dsi_host *host);
> struct drm_dsc_config *msm_dsi_host_get_dsc_config(struct mipi_dsi_host
> *host);
> +bool msm_dsi_host_is_widebus_enabled(struct mipi_dsi_host *host);
>
> /* dsi phy */
> struct msm_dsi_phy;
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 645927214871..267c7fda8854 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -710,6 +710,15 @@ static void dsi_ctrl_disable(struct msm_dsi_host
> *msm_host)
> dsi_write(msm_host, REG_DSI_CTRL, 0);
> }
>
> +bool msm_dsi_host_is_widebus_enabled(struct mipi_dsi_host *host)
I thought you settled on wide_bus?
> +{
> + struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> +
> + return msm_host->dsc &&
> + (msm_host->cfg_hnd->major == MSM_DSI_VER_MAJOR_6G &&
> + msm_host->cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_5_0);
> +}
> +
> static void dsi_ctrl_enable(struct msm_dsi_host *msm_host,
> struct msm_dsi_phy_shared_timings *phy_shared_timings,
> struct msm_dsi_phy *phy)
> {
> @@ -753,10 +762,16 @@ static void dsi_ctrl_enable(struct msm_dsi_host
> *msm_host,
> data |= DSI_CMD_CFG1_INSERT_DCS_COMMAND;
> dsi_write(msm_host, REG_DSI_CMD_CFG1, data);
>
> - if (msm_host->cfg_hnd->major == MSM_DSI_VER_MAJOR_6G &&
> - msm_host->cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V1_3) {
> + if (cfg_hnd->major == MSM_DSI_VER_MAJOR_6G) {
> data = dsi_read(msm_host, REG_DSI_CMD_MODE_MDP_CTRL2);
> - data |= DSI_CMD_MODE_MDP_CTRL2_BURST_MODE;
> +
> + if (cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V1_3)
> + data |= DSI_CMD_MODE_MDP_CTRL2_BURST_MODE;
> +
> + /* TODO: Allow for video-mode support once tested/fixed
> */
> + if (msm_dsi_host_is_widebus_enabled(_host->base))
> + data |= DSI_CMD_MODE_MDP_CTRL2_DATABUS_WIDEN;
> +
> dsi_write(msm_host, REG_DSI_CMD_MODE_MDP_CTRL2, data);
> }
> }
> @@ -894,6 +909,7 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
> u32 hdisplay = mode->hdisplay;
> u32 wc;
> int ret;
> + bool widebus_enabled = msm_dsi_host_is_widebus_enabled(_host->base);
>
> DBG("");
>
> @@ -914,6 +930,7 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
>
> if (msm_host->dsc) {
> struct drm_dsc_config *dsc = msm_host->dsc;
> + u32 bytes_per_pclk;
>
> /* update dsc params with timing params */
> if (!dsc || !mode->hdisplay || !mode->vdisplay) {
> @@ -937,7 +954,13 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
>* pulse width same
>*/
> h_total -= hdisplay;
> - hdisplay =
> DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
> + if (widebus_enabled && !(msm_host->mode_flags &
> MIPI_DSI_MODE_VIDEO))
> + bytes_per_pclk = 6;
> + else
> + bytes_per_pclk = 3;
> +
> + hdisplay =
> DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), bytes_per_pclk);
> +
> h_total += hdisplay;
> ha_end = ha_start + hdisplay;
> }
>
> --
> 2.42.0
>
[PATCH v2 1/6] drm_dbg: add trailing newlines to msgs
By at least strong convention, a print-buffer's trailing newline says "message complete, send it". The exception (no TNL, followed by a call to pr_cont) proves the general rule. Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG, 1288 drm_dbg. Clean up the remainders, in maintainer sized chunks. No functional changes. Signed-off-by: Jim Cromie --- drivers/gpu/drm/drm_connector.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index f28725736237..14020585bdc0 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2925,7 +2925,9 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, dev->mode_config.max_width, dev->mode_config.max_height); else - drm_dbg_kms(dev, "User-space requested a forced probe on [CONNECTOR:%d:%s] but is not the DRM master, demoting to read-only probe", + drm_dbg_kms(dev, + "User-space requested a forced probe on [CONNECTOR:%d:%s] " + "but is not the DRM master, demoting to read-only probe\n", connector->base.id, connector->name); } -- 2.41.0
[PATCH v2 6/6] drm: use correct ccflags-y syntax
Incorrect CFLAGS- usage failed to add -DDYNAMIC_DEBUG_MODULE,
which broke builds with:
CONFIG_DRM_USE_DYNAMIC_DEBUG=Y
CONFIG_DYNAMIC_DEBUG_CORE=Y, but CONFIG_DYNAMIC_DEBUG=N
Also add subdir-ccflags so that all drivers pick up the addition.
Fixes: 84ec67288c10 ("drm_print: wrap drm_*_dbg in dyndbg descriptor factory
macro")
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 7a09a89b493b..013cde886326 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -3,7 +3,8 @@
# Makefile for the drm device driver. This driver provides support for the
# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
-CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
+ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)+=
-DDYNAMIC_DEBUG_MODULE
+subdir-ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
drm-y := \
drm_aperture.o \
--
2.41.0
[PATCH v2 2/6] drm_dbg: add trailing newlines to msgs
By at least strong convention, a print-buffer's trailing newline says
"message complete, send it". The exception (no TNL, followed by a call
to pr_cont) proves the general rule.
Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
1288 drm_dbg. Clean up the remainders, in maintainer sized chunks.
No functional changes.
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/kmb/kmb_crtc.c | 10 +-
drivers/gpu/drm/kmb/kmb_plane.c | 6 +++---
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c b/drivers/gpu/drm/kmb/kmb_crtc.c
index 647872f65bff..a58baf25322d 100644
--- a/drivers/gpu/drm/kmb/kmb_crtc.c
+++ b/drivers/gpu/drm/kmb/kmb_crtc.c
@@ -94,7 +94,7 @@ static void kmb_crtc_set_mode(struct drm_crtc *crtc,
vm.hback_porch = 0;
vm.hsync_len = 28;
- drm_dbg(dev, "%s : %dactive height= %d vbp=%d vfp=%d vsync-w=%d
h-active=%d h-bp=%d h-fp=%d hsync-l=%d",
+ drm_dbg(dev, "%s : %dactive height= %d vbp=%d vfp=%d vsync-w=%d
h-active=%d h-bp=%d h-fp=%d hsync-l=%d\n",
__func__, __LINE__,
m->crtc_vdisplay, vm.vback_porch, vm.vfront_porch,
vm.vsync_len, m->crtc_hdisplay, vm.hback_porch,
@@ -194,24 +194,24 @@ static enum drm_mode_status
int vfp = mode->vsync_start - mode->vdisplay;
if (mode->vdisplay < KMB_CRTC_MAX_HEIGHT) {
- drm_dbg(dev, "height = %d less than %d",
+ drm_dbg(dev, "height = %d less than %d\n",
mode->vdisplay, KMB_CRTC_MAX_HEIGHT);
return MODE_BAD_VVALUE;
}
if (mode->hdisplay < KMB_CRTC_MAX_WIDTH) {
- drm_dbg(dev, "width = %d less than %d",
+ drm_dbg(dev, "width = %d less than %d\n",
mode->hdisplay, KMB_CRTC_MAX_WIDTH);
return MODE_BAD_HVALUE;
}
refresh = drm_mode_vrefresh(mode);
if (refresh < KMB_MIN_VREFRESH || refresh > KMB_MAX_VREFRESH) {
- drm_dbg(dev, "refresh = %d less than %d or greater than %d",
+ drm_dbg(dev, "refresh = %d less than %d or greater than %d\n",
refresh, KMB_MIN_VREFRESH, KMB_MAX_VREFRESH);
return MODE_BAD;
}
if (vfp < KMB_CRTC_MIN_VFP) {
- drm_dbg(dev, "vfp = %d less than %d", vfp, KMB_CRTC_MIN_VFP);
+ drm_dbg(dev, "vfp = %d less than %d\n", vfp, KMB_CRTC_MIN_VFP);
return MODE_BAD;
}
diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
index 9e0562aa2bcb..308bd1cb50c8 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.c
+++ b/drivers/gpu/drm/kmb/kmb_plane.c
@@ -78,7 +78,7 @@ static unsigned int check_pixel_format(struct drm_plane
*plane, u32 format)
* plane configuration is not supported.
*/
if (init_disp_cfg.format && init_disp_cfg.format != format) {
- drm_dbg(>drm, "Cannot change format after initial plane
configuration");
+ drm_dbg(>drm, "Cannot change format after initial plane
configuration\n");
return -EINVAL;
}
for (i = 0; i < plane->format_count; i++) {
@@ -124,7 +124,7 @@ static int kmb_plane_atomic_check(struct drm_plane *plane,
if ((init_disp_cfg.width && init_disp_cfg.height) &&
(init_disp_cfg.width != fb->width ||
init_disp_cfg.height != fb->height)) {
- drm_dbg(>drm, "Cannot change plane height or width after
initial configuration");
+ drm_dbg(>drm, "Cannot change plane height or width after
initial configuration\n");
return -EINVAL;
}
can_position = (plane->type == DRM_PLANE_TYPE_OVERLAY);
@@ -375,7 +375,7 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
spin_lock_irq(>irq_lock);
if (kmb->kmb_under_flow || kmb->kmb_flush_done) {
spin_unlock_irq(>irq_lock);
- drm_dbg(>drm, "plane_update:underflow returning");
+ drm_dbg(>drm, "plane_update:underflow returning\n");
return;
}
spin_unlock_irq(>irq_lock);
--
2.41.0
[PATCH v2 4/6] drm_dbg: add trailing newlines to msgs
By at least strong convention, a print-buffer's trailing newline says
"message complete, send it". The exception (no TNL, followed by a call
to pr_cont) proves the general rule.
Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
1288 drm_dbg. Clean up the remainders, in maintainer sized chunks.
No functional changes.
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/msm/msm_fb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c
index e3f61c39df69..80166f702a0d 100644
--- a/drivers/gpu/drm/msm/msm_fb.c
+++ b/drivers/gpu/drm/msm/msm_fb.c
@@ -89,7 +89,7 @@ int msm_framebuffer_prepare(struct drm_framebuffer *fb,
for (i = 0; i < n; i++) {
ret = msm_gem_get_and_pin_iova(fb->obj[i], aspace,
_fb->iova[i]);
- drm_dbg_state(fb->dev, "FB[%u]: iova[%d]: %08llx (%d)",
+ drm_dbg_state(fb->dev, "FB[%u]: iova[%d]: %08llx (%d)\n",
fb->base.id, i, msm_fb->iova[i], ret);
if (ret)
return ret;
@@ -176,7 +176,7 @@ static struct drm_framebuffer *msm_framebuffer_init(struct
drm_device *dev,
const struct msm_format *format;
int ret, i, n;
- drm_dbg_state(dev, "create framebuffer: mode_cmd=%p (%dx%d@%4.4s)",
+ drm_dbg_state(dev, "create framebuffer: mode_cmd=%p (%dx%d@%4.4s)\n",
mode_cmd, mode_cmd->width, mode_cmd->height,
(char *)_cmd->pixel_format);
@@ -232,7 +232,7 @@ static struct drm_framebuffer *msm_framebuffer_init(struct
drm_device *dev,
refcount_set(_fb->dirtyfb, 1);
- drm_dbg_state(dev, "create: FB ID: %d (%p)", fb->base.id, fb);
+ drm_dbg_state(dev, "create: FB ID: %d (%p)\n", fb->base.id, fb);
return fb;
--
2.41.0
[PATCH v2 5/6] drm_dbg: add trailing newlines to msgs
By at least strong convention, a print-buffer's trailing newline says "message complete, send it". The exception (no TNL, followed by a call to pr_cont) proves the general rule. Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG, 1288 drm_dbg. Clean up the remainders, in maintainer sized chunks. No functional changes. Signed-off-by: Jim Cromie --- drivers/gpu/drm/vc4/vc4_crtc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index bef9d45ef1df..959123759711 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -592,7 +592,7 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, old_state); struct drm_device *dev = crtc->dev; - drm_dbg(dev, "Disabling CRTC %s (%u) connected to Encoder %s (%u)", + drm_dbg(dev, "Disabling CRTC %s (%u) connected to Encoder %s (%u)\n", crtc->name, crtc->base.id, encoder->name, encoder->base.id); require_hvs_enabled(dev); @@ -620,7 +620,7 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder); int idx; - drm_dbg(dev, "Enabling CRTC %s (%u) connected to Encoder %s (%u)", + drm_dbg(dev, "Enabling CRTC %s (%u) connected to Encoder %s (%u)\n", crtc->name, crtc->base.id, encoder->name, encoder->base.id); if (!drm_dev_enter(dev, )) -- 2.41.0
[PATCH v2 3/6] drm_dbg: add trailing newlines to msgs
By at least strong convention, a print-buffer's trailing newline says
"message complete, send it". The exception (no TNL, followed by a call
to pr_cont) proves the general rule.
Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
1288 drm_dbg. Clean up the remainders, in maintainer sized chunks.
No functional changes.
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 090f242e610c..0a196348e2d1 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4171,7 +4171,7 @@ static int intel_ddi_compute_config_late(struct
intel_encoder *encoder,
struct drm_connector *connector = conn_state->connector;
u8 port_sync_transcoders = 0;
- drm_dbg_kms(>drm, "[ENCODER:%d:%s] [CRTC:%d:%s]",
+ drm_dbg_kms(>drm, "[ENCODER:%d:%s] [CRTC:%d:%s]\n",
encoder->base.base.id, encoder->base.name,
crtc_state->uapi.crtc->base.id,
crtc_state->uapi.crtc->name);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index cfd7929587d8..29c40e8a7183 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1436,7 +1436,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
if (unlikely(reloc->write_domain & (reloc->write_domain - 1))) {
drm_dbg(>drm, "reloc with multiple write domains: "
"target %d offset %d "
- "read %08x write %08x",
+ "read %08x write %08x\n",
reloc->target_handle,
(int) reloc->offset,
reloc->read_domains,
@@ -1447,7 +1447,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
& ~I915_GEM_GPU_DOMAINS)) {
drm_dbg(>drm, "reloc with read/write non-GPU domains: "
"target %d offset %d "
- "read %08x write %08x",
+ "read %08x write %08x\n",
reloc->target_handle,
(int) reloc->offset,
reloc->read_domains,
--
2.41.0
[PATCH v2 0/6] drm_dbg: add trailing newlines where missing
By at least strong convention, a print-buffer's trailing newline says "message complete, send it". The exception (no TNL, followed by a call to pr_cont) proves the general rule. Most DRM.debug calls already comport with this rule/convention: 207 DRM_DEV_DEBUG, 1288 drm_dbg. Clean up the remainders, in maintainer sized chunks. V1 got Fi.CI.IGT failure, on 2 Possible regressions: igt@api_intel_bb@render@render-y-1024: shard-snb: NOTRUN -> ABORT +1 similar issue igt@sysfs_timeslice_duration@timeout@ccs0: shard-dg2: PASS -> TIMEOUT Neither have any logs bearing anything connected with drm.debug output. V2 tries again. and fixes checkpatch warnings, by reusing 1st commit-msg. also fix a ccflags-y spelling error in drm Makefile. commits upon e2884fe84a83 (drm-misc/for-linux-next-fixes, drm-misc/drm-misc-fixes) drm/amd: \ Make fence wait in suballocator uninterruptible Jim Cromie (6): drm_dbg: add trailing newlines to msgs drm_dbg: add trailing newlines to msgs drm_dbg: add trailing newlines to msgs drm_dbg: add trailing newlines to msgs drm_dbg: add trailing newlines to msgs drm: use correct ccflags-y syntax drivers/gpu/drm/Makefile | 3 ++- drivers/gpu/drm/drm_connector.c| 4 +++- drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 4 ++-- drivers/gpu/drm/kmb/kmb_crtc.c | 10 +- drivers/gpu/drm/kmb/kmb_plane.c| 6 +++--- drivers/gpu/drm/msm/msm_fb.c | 6 +++--- drivers/gpu/drm/vc4/vc4_crtc.c | 4 ++-- 8 files changed, 21 insertions(+), 18 deletions(-) -- 2.41.0
Re: [PATCH] drm/panel/panel-sitronix-st7701: Move init sequence from prepare() to enable()
On 29.08.23 16:35, Jagan Teki wrote: On Sun, Aug 27, 2023 at 12:03 AM Mimoja wrote: I appreciate you taking the time to respond! On 26.08.23 17:18, Marek Vasut wrote: On 8/26/23 11:55, Mimoja wrote: "The .prepare() function is typically called before the display controller starts to transmit video data." and "After the display controller has started transmitting video data, it's safe to call the .enable() function." DSI commands are not DSI video, so this should be OK ? You are correct, my commit message is mixing things up here. I wanted to emphasize roughly the thought of "when enable() is called the dsi core is expected to have its clock initialized". Will take note to clarify this if I succeed to make a case for this patch below :) While generally fine this can lead to a fillup of the transmission queue before the transmission is set up on certain dsi bridges. This issue can also be seen on downstream imx8m* kernels. Can you reproduce this with current mainline Linux or linux-next tree ? I recall the display pipeline in the NXP downstream stuff is very different from mainline . You are very much correct. The NXP downstream kernel is completely different from the upstream one and is really a great example to show the issue (code cleaned up for readability): https://github.com/varigit/linux-imx/blob/5.15-2.0.x-imx_var01/drivers/gpu/drm/bridge/sec-dsim.c#L1368 ``` ret = drm_panel_prepare(dsim->panel); if (unlikely(ret)) [...] /* config esc clock, byte clock and etc */ sec_mipi_dsim_config_clkctrl(dsim); ret = drm_panel_enable(dsim->panel); if (unlikely(ret)) [...] ``` Which SoC does have this problem ? Sadly I don't have any SoCs available which would work perfectly with linux-next, let alone are confirmed affected :/ I were able to make my Kingway Panel work (Custom one and so far unsupported by the st7701 driver) with this patch on downstream 5.4 and 5.15 imx8mn as well as on a raspberry pi CM4 with 6.1. However raspberrypi/linux brings SPI support to the st7701 driver which should not affect this but I would just like to document it here. I could not find any success story with st7701 and the rpi on 6.1 online after a short search (and only one reference with 5.10 which seems to me a bit different in a short comparison) but again I can only offer circumstantial evidence. Sorry :/ If I understand correctly, 5.10 and 5.15 Would work as it is if the DSI host calls the panel's prepare and enable directly from encoder enable. Did you check that? No, I fear the downstream NXP driver is not following the correct / expected calling flow which upstream has implemented. After investigating this and the RPi issue I would like to follow Marek's recommendation and simply sunset this change in favor of fixing the downstream issues in the downstream tree(s). This seems to be mainly an issue for vendor-custom downstream host kernels, and while there might be a case of convention where panels don't sent commands in prepare() and only in enable() I don't think this patch should have a future solely to be cherry-picked into downstream kernels, when we can not make a case for current linux/next. After all: It is now on the internet for better or worse for others to pick up if needed. I appreciate you all taking the time to discuss and I will post the enablement code for the Kingway HW-021P0Z002-01 panel I tested this with in a bit. Cheers and thankful regards Mimoja
Re: [PATCH v4 4/4] drm/msm/dsi: Enable widebus for DSI
On 22/08/2023 20:42, Jessica Zhang wrote: DSI 6G v2.5.x+ supports a data-bus widen mode that allows DSI to send 48 bits of compressed data instead of 24. Enable this mode whenever DSC is enabled for supported chipsets. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/dsi/dsi.c | 2 +- drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 31 +++ 3 files changed, 29 insertions(+), 5 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v4 2/4] drm/msm/dpu: Enable widebus for DSI INTF
On 22/08/2023 20:42, Jessica Zhang wrote: DPU supports a data-bus widen mode for DSI INTF. Enable this mode for all supported chipsets if widebus is enabled for DSI. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 7 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c| 5 + drivers/gpu/drm/msm/msm_drv.h| 5 + 6 files changed, 25 insertions(+), 2 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[PATCH v16 16/20] drm/shmem-helper: Export drm_gem_shmem_get_pages_sgt_locked()
Export drm_gem_shmem_get_pages_sgt_locked() that will be used by virtio-gpu
shrinker during GEM swap-in operation done under the held reservation lock.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 3 ++-
include/drm/drm_gem_shmem_helper.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index a0a6c7e505c8..afd9b97ba593 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -874,7 +874,7 @@ struct sg_table *drm_gem_shmem_get_sg_table(struct
drm_gem_shmem_object *shmem)
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
-static struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct
drm_gem_shmem_object *shmem)
+struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct
drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = >base;
int ret;
@@ -919,6 +919,7 @@ static struct sg_table
*drm_gem_shmem_get_pages_sgt_locked(struct drm_gem_shmem_
drm_gem_shmem_put_pages_locked(shmem);
return ERR_PTR(ret);
}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_get_pages_sgt_locked);
/**
* drm_gem_shmem_get_pages_sgt - Pin pages, dma map them, and return a
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index 65c99da25048..2d7debc23ac1 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -161,6 +161,7 @@ void drm_gem_shmem_purge_locked(struct drm_gem_shmem_object
*shmem);
struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_shmem_object
*shmem);
struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_shmem_object
*shmem);
+struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct
drm_gem_shmem_object *shmem);
void drm_gem_shmem_print_info(const struct drm_gem_shmem_object *shmem,
struct drm_printer *p, unsigned int indent);
--
2.41.0
[PATCH v16 17/20] drm/virtio: Pin display framebuffer BO
Prepare to addition of memory shrinker support by pinning display
framebuffer BO pages in memory while they are in use by display on host.
Shrinker is free to relocate framebuffer BO pages if it doesn't know that
pages are in use, thus pin the pages to disallow shrinker to move them.
Acked-by: Gerd Hoffmann
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++
drivers/gpu/drm/virtio/virtgpu_gem.c | 19 +++
drivers/gpu/drm/virtio/virtgpu_plane.c | 17 +++--
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 4126c384286b..5a4b74b7b318 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -313,6 +313,8 @@ void virtio_gpu_array_put_free(struct
virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_work(struct work_struct *work);
+int virtio_gpu_gem_pin(struct virtio_gpu_object *bo);
+void virtio_gpu_gem_unpin(struct virtio_gpu_object *bo);
/* virtgpu_vq.c */
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c
b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 7db48d17ee3a..625c05d625bf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -294,3 +294,22 @@ void virtio_gpu_array_put_free_work(struct work_struct
*work)
}
spin_unlock(>obj_free_lock);
}
+
+int virtio_gpu_gem_pin(struct virtio_gpu_object *bo)
+{
+ int err;
+
+ if (virtio_gpu_is_shmem(bo)) {
+ err = drm_gem_shmem_pin(>base);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+void virtio_gpu_gem_unpin(struct virtio_gpu_object *bo)
+{
+ if (virtio_gpu_is_shmem(bo))
+ drm_gem_shmem_unpin(>base);
+}
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
b/drivers/gpu/drm/virtio/virtgpu_plane.c
index a2e045f3a000..def57b01a826 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -238,20 +238,28 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane
*plane,
struct virtio_gpu_device *vgdev = dev->dev_private;
struct virtio_gpu_framebuffer *vgfb;
struct virtio_gpu_object *bo;
+ int err;
if (!new_state->fb)
return 0;
vgfb = to_virtio_gpu_framebuffer(new_state->fb);
bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]);
- if (!bo || (plane->type == DRM_PLANE_TYPE_PRIMARY && !bo->guest_blob))
+
+ err = virtio_gpu_gem_pin(bo);
+ if (err)
+ return err;
+
+ if (plane->type == DRM_PLANE_TYPE_PRIMARY && !bo->guest_blob)
return 0;
if (bo->dumb && (plane->state->fb != new_state->fb)) {
vgfb->fence = virtio_gpu_fence_alloc(vgdev,
vgdev->fence_drv.context,
0);
- if (!vgfb->fence)
+ if (!vgfb->fence) {
+ virtio_gpu_gem_unpin(bo);
return -ENOMEM;
+ }
}
return 0;
@@ -261,15 +269,20 @@ static void virtio_gpu_plane_cleanup_fb(struct drm_plane
*plane,
struct drm_plane_state *state)
{
struct virtio_gpu_framebuffer *vgfb;
+ struct virtio_gpu_object *bo;
if (!state->fb)
return;
vgfb = to_virtio_gpu_framebuffer(state->fb);
+ bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]);
+
if (vgfb->fence) {
dma_fence_put(>fence->f);
vgfb->fence = NULL;
}
+
+ virtio_gpu_gem_unpin(bo);
}
static void virtio_gpu_cursor_plane_update(struct drm_plane *plane,
--
2.41.0
[PATCH v16 13/20] drm/shmem-helper: Switch drm_gem_shmem_vmap/vunmap to use pin/unpin
The vmapped pages shall be pinned in memory and previously get/put_pages()
were implicitly hard-pinning/unpinning the pages. This will no longer be
the case with addition of memory shrinker because pages_use_count > 0 won't
determine anymore whether pages are hard-pinned (they will be soft-pinned),
while the new pages_pin_count will do the hard-pinning. Switch the
vmap/vunmap() to use pin/unpin() functions in a preparation of addition
of the memory shrinker support to drm-shmem.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 19 ---
include/drm/drm_gem_shmem_helper.h | 2 +-
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index d93ebfef20c7..899f655a65bb 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -257,6 +257,14 @@ static int drm_gem_shmem_pin_locked(struct
drm_gem_shmem_object *shmem)
return ret;
}
+static void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem)
+{
+ dma_resv_assert_held(shmem->base.resv);
+
+ if (refcount_dec_and_test(>pages_pin_count))
+ drm_gem_shmem_put_pages_locked(shmem);
+}
+
/**
* drm_gem_shmem_pin - Pin backing pages for a shmem GEM object
* @shmem: shmem GEM object
@@ -304,10 +312,7 @@ void drm_gem_shmem_unpin(struct drm_gem_shmem_object
*shmem)
return;
dma_resv_lock(shmem->base.resv, NULL);
-
- if (refcount_dec_and_test(>pages_pin_count))
- drm_gem_shmem_put_pages_locked(shmem);
-
+ drm_gem_shmem_unpin_locked(shmem);
dma_resv_unlock(shmem->base.resv);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_unpin);
@@ -345,7 +350,7 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object
*shmem,
return 0;
}
- ret = drm_gem_shmem_get_pages_locked(shmem);
+ ret = drm_gem_shmem_pin_locked(shmem);
if (ret)
goto err_zero_use;
@@ -368,7 +373,7 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object
*shmem,
err_put_pages:
if (!obj->import_attach)
- drm_gem_shmem_put_pages_locked(shmem);
+ drm_gem_shmem_unpin_locked(shmem);
err_zero_use:
shmem->vmap_use_count = 0;
@@ -405,7 +410,7 @@ void drm_gem_shmem_vunmap_locked(struct
drm_gem_shmem_object *shmem,
return;
vunmap(shmem->vaddr);
- drm_gem_shmem_put_pages_locked(shmem);
+ drm_gem_shmem_unpin_locked(shmem);
}
shmem->vaddr = NULL;
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index bd545428a7ee..396958a98c34 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -137,7 +137,7 @@ int drm_gem_shmem_madvise_locked(struct
drm_gem_shmem_object *shmem, int madv);
static inline bool drm_gem_shmem_is_purgeable(struct drm_gem_shmem_object
*shmem)
{
return (shmem->madv > 0) &&
- !shmem->vmap_use_count && shmem->sgt &&
+ !refcount_read(>pages_pin_count) && shmem->sgt &&
!shmem->base.dma_buf && !shmem->base.import_attach;
}
--
2.41.0
[PATCH v16 19/20] drm/virtio: Support memory shrinking
Support generic drm-shmem memory shrinker and add new madvise IOCTL to
the VirtIO-GPU driver. BO cache manager of Mesa driver will mark BOs as
"don't need" using the new IOCTL to let shrinker purge the marked BOs on
OOM, the shrinker will also evict unpurgeable shmem BOs from memory if
guest supports SWAP file or partition.
Acked-by: Gerd Hoffmann
Signed-off-by: Daniel Almeida
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_drv.h| 13 +-
drivers/gpu/drm/virtio/virtgpu_gem.c| 35 ++
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 25 ++
drivers/gpu/drm/virtio/virtgpu_kms.c| 8
drivers/gpu/drm/virtio/virtgpu_object.c | 61 +
drivers/gpu/drm/virtio/virtgpu_vq.c | 40
include/uapi/drm/virtgpu_drm.h | 14 ++
7 files changed, 195 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 8c82530eae82..a34da2036221 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -278,7 +278,7 @@ struct virtio_gpu_fpriv {
};
/* virtgpu_ioctl.c */
-#define DRM_VIRTIO_NUM_IOCTLS 12
+#define DRM_VIRTIO_NUM_IOCTLS 13
extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
void virtio_gpu_create_context(struct drm_device *dev, struct drm_file *file);
@@ -316,6 +316,8 @@ void virtio_gpu_array_put_free_delayed(struct
virtio_gpu_device *vgdev,
void virtio_gpu_array_put_free_work(struct work_struct *work);
int virtio_gpu_array_prepare(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_array *objs);
+int virtio_gpu_gem_host_mem_release(struct virtio_gpu_object *bo);
+int virtio_gpu_gem_madvise(struct virtio_gpu_object *obj, int madv);
int virtio_gpu_gem_pin(struct virtio_gpu_object *bo);
void virtio_gpu_gem_unpin(struct virtio_gpu_object *bo);
@@ -329,6 +331,8 @@ void virtio_gpu_cmd_create_resource(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_fence *fence);
void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo);
+int virtio_gpu_cmd_release_resource(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *bo);
void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
uint64_t offset,
uint32_t width, uint32_t height,
@@ -349,6 +353,9 @@ void virtio_gpu_object_attach(struct virtio_gpu_device
*vgdev,
struct virtio_gpu_object *obj,
struct virtio_gpu_mem_entry *ents,
unsigned int nents);
+void virtio_gpu_object_detach(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *obj,
+ struct virtio_gpu_fence *fence);
int virtio_gpu_attach_status_page(struct virtio_gpu_device *vgdev);
int virtio_gpu_detach_status_page(struct virtio_gpu_device *vgdev);
void virtio_gpu_cursor_ping(struct virtio_gpu_device *vgdev,
@@ -499,4 +506,8 @@ void virtio_gpu_vram_unmap_dma_buf(struct device *dev,
int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
+/* virtgpu_gem_shrinker.c */
+int virtio_gpu_gem_shrinker_init(struct virtio_gpu_device *vgdev);
+void virtio_gpu_gem_shrinker_fini(struct virtio_gpu_device *vgdev);
+
#endif
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c
b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 97e67064c97e..748f7bbb0e6d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -147,10 +147,20 @@ void virtio_gpu_gem_object_close(struct drm_gem_object
*obj,
struct virtio_gpu_device *vgdev = obj->dev->dev_private;
struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
struct virtio_gpu_object_array *objs;
+ struct virtio_gpu_object *bo;
if (!vgdev->has_virgl_3d)
return;
+ bo = gem_to_virtio_gpu_obj(obj);
+
+ /*
+* Purged BO was already detached and released, the resource ID
+* is invalid by now.
+*/
+ if (!virtio_gpu_gem_madvise(bo, VIRTGPU_MADV_WILLNEED))
+ return;
+
objs = virtio_gpu_array_alloc(1);
if (!objs)
return;
@@ -315,6 +325,31 @@ int virtio_gpu_array_prepare(struct virtio_gpu_device
*vgdev,
return ret;
}
+int virtio_gpu_gem_madvise(struct virtio_gpu_object *bo, int madv)
+{
+ if (virtio_gpu_is_shmem(bo))
+ return drm_gem_shmem_object_madvise(>base.base, madv);
+
+ return 1;
+}
+
+int virtio_gpu_gem_host_mem_release(struct virtio_gpu_object *bo)
+{
+ struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
+ int err;
+
+
[PATCH v16 15/20] drm/shmem-helper: Add memory shrinker
Introduce common drm-shmem shrinker for DRM drivers.
To start using drm-shmem shrinker drivers should do the following:
1. Implement evict() callback of GEM object where driver should check
whether object is purgeable or evictable using drm-shmem helpers and
perform the shrinking action
2. Initialize drm-shmem internals using drmm_gem_shmem_init(drm_device),
which will register drm-shmem shrinker
3. Implement madvise IOCTL that will use drm_gem_shmem_madvise()
Signed-off-by: Daniel Almeida
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c| 442 --
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +-
include/drm/drm_device.h | 10 +-
include/drm/drm_gem_shmem_helper.h| 71 ++-
4 files changed, 494 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 4633a418faba..a0a6c7e505c8 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -88,8 +89,6 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size,
bool private)
if (ret)
goto err_release;
- INIT_LIST_HEAD(>madv_list);
-
if (!private) {
/*
* Our buffers are kept pinned, so allocating them
@@ -128,6 +127,62 @@ struct drm_gem_shmem_object *drm_gem_shmem_create(struct
drm_device *dev, size_t
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_create);
+static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object *shmem)
+{
+ dma_resv_assert_held(shmem->base.resv);
+
+ return (shmem->madv >= 0) && shmem->base.funcs->evict &&
+ refcount_read(>pages_use_count) &&
+ !refcount_read(>pages_pin_count) &&
+ !shmem->base.dma_buf && !shmem->base.import_attach &&
+ shmem->sgt && !shmem->evicted;
+}
+
+static void
+drm_gem_shmem_update_pages_state_locked(struct drm_gem_shmem_object *shmem)
+{
+ struct drm_gem_object *obj = >base;
+ struct drm_gem_shmem *shmem_mm = obj->dev->shmem_mm;
+ struct drm_gem_shmem_shrinker *shmem_shrinker = _mm->shrinker;
+
+ dma_resv_assert_held(shmem->base.resv);
+
+ if (!shmem_shrinker || obj->import_attach)
+ return;
+
+ if (shmem->madv < 0)
+ drm_gem_lru_remove(>base);
+ else if (drm_gem_shmem_is_evictable(shmem) ||
drm_gem_shmem_is_purgeable(shmem))
+ drm_gem_lru_move_tail(_shrinker->lru_evictable,
>base);
+ else if (shmem->evicted)
+ drm_gem_lru_move_tail(_shrinker->lru_evicted,
>base);
+ else if (!shmem->pages)
+ drm_gem_lru_remove(>base);
+ else
+ drm_gem_lru_move_tail(_shrinker->lru_pinned,
>base);
+}
+
+static void
+drm_gem_shmem_do_release_pages_locked(struct drm_gem_shmem_object *shmem)
+{
+ struct drm_gem_object *obj = >base;
+
+ if (!shmem->pages) {
+ drm_WARN_ON(obj->dev, !shmem->evicted && shmem->madv >= 0);
+ return;
+ }
+
+#ifdef CONFIG_X86
+ if (shmem->map_wc)
+ set_pages_array_wb(shmem->pages, obj->size >> PAGE_SHIFT);
+#endif
+
+ drm_gem_put_pages(obj, shmem->pages,
+ shmem->pages_mark_dirty_on_put,
+ shmem->pages_mark_accessed_on_put);
+ shmem->pages = NULL;
+}
+
/**
* drm_gem_shmem_free - Free resources associated with a shmem GEM object
* @shmem: shmem GEM object to free
@@ -142,8 +197,6 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
if (obj->import_attach) {
drm_prime_gem_destroy(obj, shmem->sgt);
} else if (!shmem->imported_sgt) {
- dma_resv_lock(shmem->base.resv, NULL);
-
drm_WARN_ON(obj->dev, refcount_read(>vmap_use_count));
if (shmem->sgt) {
@@ -152,14 +205,27 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
}
- if (refcount_read(>pages_use_count)) {
- drm_gem_shmem_put_pages_locked(shmem);
- drm_WARN_ON(obj->dev, !shmem->got_pages_sgt);
+
+ /*
+* Destroying the object is a special case..
drm_gem_shmem_free()
+* calls many things that WARN_ON if the obj lock is not held.
But
+* acquiring the obj lock in
drm_gem_shmem_release_pages_locked() can
+* cause a locking order inversion between
reservation_ww_class_mutex
+* and fs_reclaim.
+*
+* This deadlock is not actually possible, because no one should
+* be already holding the lock when drm_gem_shmem_free() is
called.
+
[PATCH v16 20/20] drm/panfrost: Switch to generic memory shrinker
Replace Panfrost's custom memory shrinker with a common drm-shmem
memory shrinker.
Tested-by: Steven Price # Firefly-RK3288
Reviewed-by: Steven Price
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/panfrost/Makefile | 1 -
drivers/gpu/drm/panfrost/panfrost_device.h| 4 -
drivers/gpu/drm/panfrost/panfrost_drv.c | 27 ++--
drivers/gpu/drm/panfrost/panfrost_gem.c | 30 ++--
drivers/gpu/drm/panfrost/panfrost_gem.h | 9 --
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 129 --
drivers/gpu/drm/panfrost/panfrost_job.c | 18 ++-
include/drm/drm_gem_shmem_helper.h| 7 -
8 files changed, 47 insertions(+), 178 deletions(-)
delete mode 100644 drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
diff --git a/drivers/gpu/drm/panfrost/Makefile
b/drivers/gpu/drm/panfrost/Makefile
index 7da2b3f02ed9..11622e22cf15 100644
--- a/drivers/gpu/drm/panfrost/Makefile
+++ b/drivers/gpu/drm/panfrost/Makefile
@@ -5,7 +5,6 @@ panfrost-y := \
panfrost_device.o \
panfrost_devfreq.o \
panfrost_gem.o \
- panfrost_gem_shrinker.o \
panfrost_gpu.o \
panfrost_job.o \
panfrost_mmu.o \
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index b0126b9fbadc..dcc2571c092b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -116,10 +116,6 @@ struct panfrost_device {
atomic_t pending;
} reset;
- struct mutex shrinker_lock;
- struct list_head shrinker_list;
- struct shrinker shrinker;
-
struct panfrost_devfreq pfdevfreq;
};
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 175443eacead..8cf338c2a03b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -170,7 +170,6 @@ panfrost_lookup_bos(struct drm_device *dev,
break;
}
- atomic_inc(>gpu_usecount);
job->mappings[i] = mapping;
}
@@ -395,7 +394,6 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
{
struct panfrost_file_priv *priv = file_priv->driver_priv;
struct drm_panfrost_madvise *args = data;
- struct panfrost_device *pfdev = dev->dev_private;
struct drm_gem_object *gem_obj;
struct panfrost_gem_object *bo;
int ret = 0;
@@ -408,11 +406,15 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
bo = to_panfrost_bo(gem_obj);
+ if (bo->is_heap) {
+ args->retained = 1;
+ goto out_put_object;
+ }
+
ret = dma_resv_lock_interruptible(bo->base.base.resv, NULL);
if (ret)
goto out_put_object;
- mutex_lock(>shrinker_lock);
mutex_lock(>mappings.lock);
if (args->madv == PANFROST_MADV_DONTNEED) {
struct panfrost_gem_mapping *first;
@@ -438,17 +440,8 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
args->retained = drm_gem_shmem_madvise_locked(>base, args->madv);
- if (args->retained) {
- if (args->madv == PANFROST_MADV_DONTNEED)
- list_move_tail(>base.madv_list,
- >shrinker_list);
- else if (args->madv == PANFROST_MADV_WILLNEED)
- list_del_init(>base.madv_list);
- }
-
out_unlock_mappings:
mutex_unlock(>mappings.lock);
- mutex_unlock(>shrinker_lock);
dma_resv_unlock(bo->base.base.resv);
out_put_object:
drm_gem_object_put(gem_obj);
@@ -577,9 +570,6 @@ static int panfrost_probe(struct platform_device *pdev)
ddev->dev_private = pfdev;
pfdev->ddev = ddev;
- mutex_init(>shrinker_lock);
- INIT_LIST_HEAD(>shrinker_list);
-
err = panfrost_device_init(pfdev);
if (err) {
if (err != -EPROBE_DEFER)
@@ -601,10 +591,14 @@ static int panfrost_probe(struct platform_device *pdev)
if (err < 0)
goto err_out1;
- panfrost_gem_shrinker_init(ddev);
+ err = drmm_gem_shmem_init(ddev);
+ if (err < 0)
+ goto err_out2;
return 0;
+err_out2:
+ drm_dev_unregister(ddev);
err_out1:
pm_runtime_disable(pfdev->dev);
panfrost_device_fini(pfdev);
@@ -620,7 +614,6 @@ static void panfrost_remove(struct platform_device *pdev)
struct drm_device *ddev = pfdev->ddev;
drm_dev_unregister(ddev);
- panfrost_gem_shrinker_cleanup(ddev);
pm_runtime_get_sync(pfdev->dev);
pm_runtime_disable(pfdev->dev);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 59c8c73c6a59..00165fca7f3d 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++
[PATCH v16 12/20] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation
lock if pages_use_count is non-zero, leveraging from atomicity of the
refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper.
Suggested-by: Boris Brezillon
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index a0faef3e762d..d93ebfef20c7 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -227,6 +227,20 @@ void drm_gem_shmem_put_pages_locked(struct
drm_gem_shmem_object *shmem)
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked);
+static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
+{
+ int ret;
+
+ if (refcount_inc_not_zero(>pages_use_count))
+ return 0;
+
+ dma_resv_lock(shmem->base.resv, NULL);
+ ret = drm_gem_shmem_get_pages_locked(shmem);
+ dma_resv_unlock(shmem->base.resv);
+
+ return ret;
+}
+
static int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem)
{
int ret;
@@ -610,10 +624,7 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem,
struct vm_area_struct
return ret;
}
- dma_resv_lock(shmem->base.resv, NULL);
- ret = drm_gem_shmem_get_pages_locked(shmem);
- dma_resv_unlock(shmem->base.resv);
-
+ ret = drm_gem_shmem_get_pages(shmem);
if (ret)
return ret;
--
2.41.0
[PATCH v16 18/20] drm/virtio: Attach shmem BOs dynamically
Prepare for addition of memory shrinker support by attaching shmem pages
to host dynamically on first use. The attachment vq command wasn't fenced
and there was no vq kick made in the BO creation code path, hence the
the attachment already was happening dynamically, but implicitly. Making
attachment explicitly dynamic will allow to simplify and reuse more code
when shrinker will be added. The virtio_gpu_object_shmem_init() now works
under held reservation lock, which will be important to have for shrinker.
Acked-by: Gerd Hoffmann
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_drv.h| 7 +++
drivers/gpu/drm/virtio/virtgpu_gem.c| 26
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 32 ++
drivers/gpu/drm/virtio/virtgpu_object.c | 80 -
drivers/gpu/drm/virtio/virtgpu_submit.c | 15 -
5 files changed, 132 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 5a4b74b7b318..8c82530eae82 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -89,6 +89,7 @@ struct virtio_gpu_object {
uint32_t hw_res_handle;
bool dumb;
bool created;
+ bool detached;
bool host3d_blob, guest_blob;
uint32_t blob_mem, blob_flags;
@@ -313,6 +314,8 @@ void virtio_gpu_array_put_free(struct
virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_work(struct work_struct *work);
+int virtio_gpu_array_prepare(struct virtio_gpu_device *vgdev,
+struct virtio_gpu_object_array *objs);
int virtio_gpu_gem_pin(struct virtio_gpu_object *bo);
void virtio_gpu_gem_unpin(struct virtio_gpu_object *bo);
@@ -458,6 +461,10 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo);
+int virtio_gpu_reattach_shmem_object_locked(struct virtio_gpu_object *bo);
+
+int virtio_gpu_reattach_shmem_object(struct virtio_gpu_object *bo);
+
int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
uint32_t *resid);
/* virtgpu_prime.c */
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c
b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 625c05d625bf..97e67064c97e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -295,6 +295,26 @@ void virtio_gpu_array_put_free_work(struct work_struct
*work)
spin_unlock(>obj_free_lock);
}
+int virtio_gpu_array_prepare(struct virtio_gpu_device *vgdev,
+struct virtio_gpu_object_array *objs)
+{
+ struct virtio_gpu_object *bo;
+ int ret = 0;
+ u32 i;
+
+ for (i = 0; i < objs->nents; i++) {
+ bo = gem_to_virtio_gpu_obj(objs->objs[i]);
+
+ if (virtio_gpu_is_shmem(bo) && bo->detached) {
+ ret = virtio_gpu_reattach_shmem_object_locked(bo);
+ if (ret)
+ break;
+ }
+ }
+
+ return ret;
+}
+
int virtio_gpu_gem_pin(struct virtio_gpu_object *bo)
{
int err;
@@ -303,6 +323,12 @@ int virtio_gpu_gem_pin(struct virtio_gpu_object *bo)
err = drm_gem_shmem_pin(>base);
if (err)
return err;
+
+ err = virtio_gpu_reattach_shmem_object(bo);
+ if (err) {
+ drm_gem_shmem_unpin(>base);
+ return err;
+ }
}
return 0;
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index b24b11f25197..070c29cea26a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -246,6 +246,10 @@ static int virtio_gpu_transfer_from_host_ioctl(struct
drm_device *dev,
if (ret != 0)
goto err_put_free;
+ ret = virtio_gpu_array_prepare(vgdev, objs);
+ if (ret)
+ goto err_unlock;
+
fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0);
if (!fence) {
ret = -ENOMEM;
@@ -288,11 +292,25 @@ static int virtio_gpu_transfer_to_host_ioctl(struct
drm_device *dev, void *data,
goto err_put_free;
}
+ ret = virtio_gpu_array_lock_resv(objs);
+ if (ret != 0)
+ goto err_put_free;
+
+ ret = virtio_gpu_array_prepare(vgdev, objs);
+ if (ret)
+ goto err_unlock;
+
+ fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0);
+ if (!fence) {
+ ret = -ENOMEM;
+ goto err_unlock;
+ }
+
if (!vgdev->has_virgl_3d) {
virtio_gpu_cmd_transfer_to_host_2d
[PATCH v16 09/20] drm/shmem-helper: Remove obsoleted is_iomem test
Everything that uses the mapped buffer should be agnostic to is_iomem.
The only reason for the is_iomem test is that we're setting shmem->vaddr
to the returned map->vaddr. Now that the shmem->vaddr code is gone, remove
the obsoleted is_iomem test to clean up the code.
Suggested-by: Thomas Zimmermann
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 2b50d1a7f718..25e99468ced2 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -317,12 +317,6 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object
*shmem,
if (obj->import_attach) {
ret = dma_buf_vmap(obj->import_attach->dmabuf, map);
- if (!ret) {
- if (drm_WARN_ON(obj->dev, map->is_iomem)) {
- dma_buf_vunmap(obj->import_attach->dmabuf, map);
- return -EIO;
- }
- }
} else {
pgprot_t prot = PAGE_KERNEL;
--
2.41.0
[PATCH v16 14/20] drm/shmem-helper: Use refcount_t for vmap_use_count
Use refcount_t helper for vmap_use_count to make refcounting consistent
with pages_use_count and pages_pin_count that use refcount_t. This will
allow to optimize unlocked vmappings by skipping reservation locking if
refcnt > 1 and also makes vmapping to benefit from the refcount_t's
overflow checks.
Suggested-by: Boris Brezillon
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 28 +++---
include/drm/drm_gem_shmem_helper.h | 2 +-
2 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 899f655a65bb..4633a418faba 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -144,7 +144,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
} else if (!shmem->imported_sgt) {
dma_resv_lock(shmem->base.resv, NULL);
- drm_WARN_ON(obj->dev, shmem->vmap_use_count);
+ drm_WARN_ON(obj->dev, refcount_read(>vmap_use_count));
if (shmem->sgt) {
dma_unmap_sgtable(obj->dev->dev, shmem->sgt,
@@ -345,23 +345,25 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object
*shmem,
dma_resv_assert_held(shmem->base.resv);
- if (shmem->vmap_use_count++ > 0) {
+ if (refcount_inc_not_zero(>vmap_use_count)) {
iosys_map_set_vaddr(map, shmem->vaddr);
return 0;
}
ret = drm_gem_shmem_pin_locked(shmem);
if (ret)
- goto err_zero_use;
+ return ret;
if (shmem->map_wc)
prot = pgprot_writecombine(prot);
shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
VM_MAP, prot);
- if (!shmem->vaddr)
+ if (!shmem->vaddr) {
ret = -ENOMEM;
- else
+ } else {
iosys_map_set_vaddr(map, shmem->vaddr);
+ refcount_set(>vmap_use_count, 1);
+ }
}
if (ret) {
@@ -374,8 +376,6 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object
*shmem,
err_put_pages:
if (!obj->import_attach)
drm_gem_shmem_unpin_locked(shmem);
-err_zero_use:
- shmem->vmap_use_count = 0;
return ret;
}
@@ -403,14 +403,10 @@ void drm_gem_shmem_vunmap_locked(struct
drm_gem_shmem_object *shmem,
} else {
dma_resv_assert_held(shmem->base.resv);
- if (drm_WARN_ON_ONCE(obj->dev, !shmem->vmap_use_count))
- return;
-
- if (--shmem->vmap_use_count > 0)
- return;
-
- vunmap(shmem->vaddr);
- drm_gem_shmem_unpin_locked(shmem);
+ if (refcount_dec_and_test(>vmap_use_count)) {
+ vunmap(shmem->vaddr);
+ drm_gem_shmem_unpin_locked(shmem);
+ }
}
shmem->vaddr = NULL;
@@ -656,7 +652,7 @@ void drm_gem_shmem_print_info(const struct
drm_gem_shmem_object *shmem,
drm_printf_indent(p, indent, "pages_pin_count=%u\n",
refcount_read(>pages_pin_count));
drm_printf_indent(p, indent, "pages_use_count=%u\n",
refcount_read(>pages_use_count));
- drm_printf_indent(p, indent, "vmap_use_count=%u\n",
shmem->vmap_use_count);
+ drm_printf_indent(p, indent, "vmap_use_count=%u\n",
refcount_read(>vmap_use_count));
drm_printf_indent(p, indent, "vaddr=%p\n", shmem->vaddr);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_print_info);
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index 396958a98c34..63e91e8f2d5c 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -81,7 +81,7 @@ struct drm_gem_shmem_object {
* Reference count on the virtual address.
* The address are un-mapped when the count reaches zero.
*/
- unsigned int vmap_use_count;
+ refcount_t vmap_use_count;
/**
* @got_pages_sgt:
--
2.41.0
[PATCH v16 00/20] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers
This series: 1. Adds common drm-shmem memory shrinker 2. Enables shrinker for VirtIO-GPU driver 3. Switches Panfrost driver to the common shrinker 4. Fixes bugs and improves drm-shmem code Mesa: https://gitlab.freedesktop.org/digetx/mesa/-/commits/virgl-madvise IGT: https://gitlab.freedesktop.org/digetx/igt-gpu-tools/-/commits/virtio-madvise https://gitlab.freedesktop.org/digetx/igt-gpu-tools/-/commits/panfrost-madvise Changelog: v16:- Added more comments to the code for the new drm-shmem flags - Added r-bs from Boris Brezillon - Fixed typos and made impovements pointed out by Boris Brezillon - Replaced kref with refcount_t as was suggested by Boris Brezillon - Corrected placement of got_sgt flag in the Lima driver, also renamed flag to got_pages_sgt - Removed drm_gem_shmem_resv_assert_held() and made drm_gem_shmem_free() to free pages without a new func that doesn't touch resv lock, as was suggested by Boris Brezillon - Added pages_pin_count to drm_gem_shmem_print_info() v15:- Moved drm-shmem reference counters to use kref that allows to optimize unlocked functions, like was suggested by Boris Brezillon. - Changed drm/gem/shmem function names to use _locked postfix and dropped the _unlocked, making the naming scheme consistent across DRM code, like was suggested by Boris Brezillon. - Added patch that fixes UAF in drm-shmem for drivers that import dma-buf and then release buffer in the import error code path. - Added patch that makes drm-shmem use new flag for SGT's get_pages() refcounting, preventing unbalanced refcounting when GEM is freed. - Fixed guest blob pinning in virtio-gpu driver that was missed previously in the shrinker patch. - Moved VC4 and virtio-gpu drivers to use drm_gem_put() in GEM-creation error code paths, which is now required by drm-shmem and was missed in a previous patch versions. - Virtio-GPU now attaches shmem pages to host on first use and not when BO is created. In older patch versions there was a potential race condition in the BO creation code path where both get_sgt()+object_attach() should've been made under same resv lock, otherwise pages could be evicted before attachment is invoked. - Virtio-GPU and drm-shmem shrinker patches are split into smaller ones. v14:- All the prerequisite reservation locking patches landed upstream, previously were a part of this series in v13 and older. https://lore.kernel.org/dri-devel/20230529223935.2672495-1-dmitry.osipe...@collabora.com/ - Added patches to improve locked/unlocked function names, like was suggested by Boris Brezillon for v13. - Made all exported drm-shmem symbols GPL, like was previously discussed with Thomas Zimmermann on this series. - Improved virtio-gpu shrinker patch. Now it won't detach purged BO when userspace closes GEM. Crosvm (and not qemu) checks res_id on CMD_CTX_DETACH_RESOURCE and prints noisy error message if ID is invalid, which wasn't noticed before. v13:- Updated virtio-gpu shrinker patch to use drm_gem_shmem_object_pin() directly instead of drm_gem_pin() and dropped patch that exported drm_gem_pin() functions, like was requested by Thomas Zimmermann in v12. v12:- Fixed the "no previous prototype for function" warning reported by kernel build bot for v11. - Fixed the missing reservation lock reported by Intel CI for VGEM driver. Other drivers using drm-shmem were affected similarly to VGEM. The problem was in the dma-buf attachment code path that led to drm-shmem pinning function which assumed the held reservation lock by drm_gem_pin(). In the past that code path was causing trouble for i915 driver and we've changed the locking scheme for the attachment code path in the dma-buf core to let exporters to handle the locking themselves. After a closer investigation, I realized that my assumption about testing of dma-buf export code path using Panfrost driver was incorrect. Now I created additional local test to exrecise the Panfrost export path. I also reproduced the issue reported by the Intel CI for v10. It's all fixed now by making the drm_gem_shmem_pin() to take the resv lock by itself. - Patches are based on top of drm-tip, CC'd intel-gfx CI for testing. v11:- Rebased on a recent linux-next. Added new patch as a result: drm/shmem-helper: Export drm_gem_shmem_get_pages_sgt_locked() It's needed by the virtio-gpu driver to swap-in/unevict shmem object, previously get_pages_sgt() didn't use locking. - Separated the "Add memory shrinker" patch into smaller parts to ease the reviewing, as was requested by Thomas Zimmermann: drm/shmem-helper: Factor out pages alloc/release from drm_gem_shmem_get/put_pages()
[PATCH v16 08/20] drm/shmem-helper: Refactor locked/unlocked functions
Add locked and remove unlocked postfixes from drm-shmem function names,
making names consistent with the drm/gem core code.
Reviewed-by: Boris Brezillon
Suggested-by: Boris Brezillon
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c| 64 +--
drivers/gpu/drm/lima/lima_gem.c | 8 +--
drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_gem.c | 6 +-
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_mmu.c | 2 +-
drivers/gpu/drm/v3d/v3d_bo.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_object.c | 4 +-
include/drm/drm_gem_shmem_helper.h| 36 +--
9 files changed, 64 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 5c777adf1bcb..2b50d1a7f718 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -43,8 +43,8 @@ static const struct drm_gem_object_funcs drm_gem_shmem_funcs
= {
.pin = drm_gem_shmem_object_pin,
.unpin = drm_gem_shmem_object_unpin,
.get_sg_table = drm_gem_shmem_object_get_sg_table,
- .vmap = drm_gem_shmem_object_vmap,
- .vunmap = drm_gem_shmem_object_vunmap,
+ .vmap = drm_gem_shmem_object_vmap_locked,
+ .vunmap = drm_gem_shmem_object_vunmap_locked,
.mmap = drm_gem_shmem_object_mmap,
.vm_ops = _gem_shmem_vm_ops,
};
@@ -153,7 +153,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
kfree(shmem->sgt);
}
if (shmem->pages) {
- drm_gem_shmem_put_pages(shmem);
+ drm_gem_shmem_put_pages_locked(shmem);
drm_WARN_ON(obj->dev, !shmem->got_pages_sgt);
}
@@ -167,7 +167,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
-static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
+static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = >base;
struct page **pages;
@@ -201,12 +201,12 @@ static int drm_gem_shmem_get_pages(struct
drm_gem_shmem_object *shmem)
}
/*
- * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a
shmem GEM object
+ * drm_gem_shmem_put_pages_locked - Decrease use count on the backing pages
for a shmem GEM object
* @shmem: shmem GEM object
*
* This function decreases the use count and puts the backing pages when use
drops to zero.
*/
-void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
+void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = >base;
@@ -228,7 +228,7 @@ void drm_gem_shmem_put_pages(struct drm_gem_shmem_object
*shmem)
shmem->pages_mark_accessed_on_put);
shmem->pages = NULL;
}
-EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages);
+EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked);
static int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem)
{
@@ -236,7 +236,7 @@ static int drm_gem_shmem_pin_locked(struct
drm_gem_shmem_object *shmem)
dma_resv_assert_held(shmem->base.resv);
- ret = drm_gem_shmem_get_pages(shmem);
+ ret = drm_gem_shmem_get_pages_locked(shmem);
return ret;
}
@@ -245,7 +245,7 @@ static void drm_gem_shmem_unpin_locked(struct
drm_gem_shmem_object *shmem)
{
dma_resv_assert_held(shmem->base.resv);
- drm_gem_shmem_put_pages(shmem);
+ drm_gem_shmem_put_pages_locked(shmem);
}
/**
@@ -295,7 +295,7 @@ void drm_gem_shmem_unpin(struct drm_gem_shmem_object *shmem)
EXPORT_SYMBOL_GPL(drm_gem_shmem_unpin);
/*
- * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object
+ * drm_gem_shmem_vmap_locked - Create a virtual mapping for a shmem GEM object
* @shmem: shmem GEM object
* @map: Returns the kernel virtual address of the SHMEM GEM object's backing
* store.
@@ -304,13 +304,13 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_unpin);
* exists for the buffer backing the shmem GEM object. It hides the differences
* between dma-buf imported and natively allocated objects.
*
- * Acquired mappings should be cleaned up by calling drm_gem_shmem_vunmap().
+ * Acquired mappings should be cleaned up by calling
drm_gem_shmem_vunmap_locked().
*
* Returns:
* 0 on success or a negative error code on failure.
*/
-int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
- struct iosys_map *map)
+int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
+ struct iosys_map *map)
{
struct drm_gem_object *obj = >base;
int ret = 0;
@@ -333,7 +333,7 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
[PATCH v16 05/20] drm/v3d: Replace open-coded drm_gem_shmem_free() with drm_gem_object_put()
The drm_gem_shmem_free() doesn't put GEM's kref to zero, which becomes
important with addition of the shrinker support to drm-shmem that will
use kref=0 in order to prevent taking lock during special GEM-freeing
time in order to avoid spurious lockdep warning about locking ordering
vs fs_reclaim code paths.
Replace open-coded drm_gem_shmem_free() with drm_gem_object_put() that
drops kref to zero before freeing GEM.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/v3d/v3d_bo.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 8b3229a37c6d..70c1095d6eec 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -33,16 +33,18 @@ void v3d_free_object(struct drm_gem_object *obj)
struct v3d_dev *v3d = to_v3d_dev(obj->dev);
struct v3d_bo *bo = to_v3d_bo(obj);
- v3d_mmu_remove_ptes(bo);
+ if (drm_mm_node_allocated(>node)) {
+ v3d_mmu_remove_ptes(bo);
- mutex_lock(>bo_lock);
- v3d->bo_stats.num_allocated--;
- v3d->bo_stats.pages_allocated -= obj->size >> PAGE_SHIFT;
- mutex_unlock(>bo_lock);
+ mutex_lock(>bo_lock);
+ v3d->bo_stats.num_allocated--;
+ v3d->bo_stats.pages_allocated -= obj->size >> PAGE_SHIFT;
+ mutex_unlock(>bo_lock);
- spin_lock(>mm_lock);
- drm_mm_remove_node(>node);
- spin_unlock(>mm_lock);
+ spin_lock(>mm_lock);
+ drm_mm_remove_node(>node);
+ spin_unlock(>mm_lock);
+ }
/* GPU execution may have dirtied any pages in the BO. */
bo->base.pages_mark_dirty_on_put = true;
@@ -142,7 +144,7 @@ struct v3d_bo *v3d_bo_create(struct drm_device *dev, struct
drm_file *file_priv,
return bo;
free_obj:
- drm_gem_shmem_free(shmem_obj);
+ drm_gem_object_put(_obj->base);
return ERR_PTR(ret);
}
@@ -160,7 +162,7 @@ v3d_prime_import_sg_table(struct drm_device *dev,
ret = v3d_bo_create_finish(obj);
if (ret) {
- drm_gem_shmem_free(_v3d_bo(obj)->base);
+ drm_gem_object_put(obj);
return ERR_PTR(ret);
}
--
2.41.0
[PATCH v16 10/20] drm/shmem-helper: Add and use pages_pin_count
Add separate pages_pin_count for tracking of whether drm-shmem pages are
moveable or not. With the addition of memory shrinker support to drm-shmem,
the pages_use_count will no longer determine whether pages are hard-pinned
in memory, but whether pages exist and are soft-pinned (and could be swapped
out). The pages_pin_count > 1 will hard-pin pages in memory.
Suggested-by: Boris Brezillon
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 24
include/drm/drm_gem_shmem_helper.h | 10 ++
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 25e99468ced2..7e1e674e2c9f 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -236,18 +236,16 @@ static int drm_gem_shmem_pin_locked(struct
drm_gem_shmem_object *shmem)
dma_resv_assert_held(shmem->base.resv);
+ if (refcount_inc_not_zero(>pages_pin_count))
+ return 0;
+
ret = drm_gem_shmem_get_pages_locked(shmem);
+ if (!ret)
+ refcount_set(>pages_pin_count, 1);
return ret;
}
-static void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem)
-{
- dma_resv_assert_held(shmem->base.resv);
-
- drm_gem_shmem_put_pages_locked(shmem);
-}
-
/**
* drm_gem_shmem_pin - Pin backing pages for a shmem GEM object
* @shmem: shmem GEM object
@@ -265,6 +263,9 @@ int drm_gem_shmem_pin(struct drm_gem_shmem_object *shmem)
drm_WARN_ON(obj->dev, obj->import_attach);
+ if (refcount_inc_not_zero(>pages_pin_count))
+ return 0;
+
ret = dma_resv_lock_interruptible(shmem->base.resv, NULL);
if (ret)
return ret;
@@ -288,8 +289,14 @@ void drm_gem_shmem_unpin(struct drm_gem_shmem_object
*shmem)
drm_WARN_ON(obj->dev, obj->import_attach);
+ if (refcount_dec_not_one(>pages_pin_count))
+ return;
+
dma_resv_lock(shmem->base.resv, NULL);
- drm_gem_shmem_unpin_locked(shmem);
+
+ if (refcount_dec_and_test(>pages_pin_count))
+ drm_gem_shmem_put_pages_locked(shmem);
+
dma_resv_unlock(shmem->base.resv);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_unpin);
@@ -634,6 +641,7 @@ void drm_gem_shmem_print_info(const struct
drm_gem_shmem_object *shmem,
if (shmem->base.import_attach)
return;
+ drm_printf_indent(p, indent, "pages_pin_count=%u\n",
refcount_read(>pages_pin_count));
drm_printf_indent(p, indent, "pages_use_count=%u\n",
shmem->pages_use_count);
drm_printf_indent(p, indent, "vmap_use_count=%u\n",
shmem->vmap_use_count);
drm_printf_indent(p, indent, "vaddr=%p\n", shmem->vaddr);
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index 808083279fd5..1cd74ae5761a 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -39,6 +39,16 @@ struct drm_gem_shmem_object {
*/
unsigned int pages_use_count;
+ /**
+* @pages_pin_count:
+*
+* Reference count on the pinned pages table.
+* The pages allowed to be evicted and purged by memory
+* shrinker only when the count is zero, otherwise pages
+* are hard-pinned in memory.
+*/
+ refcount_t pages_pin_count;
+
/**
* @madv: State for madvise
*
--
2.41.0
[PATCH v16 11/20] drm/shmem-helper: Use refcount_t for pages_use_count
Use atomic refcount_t helper for pages_use_count to optimize pin/unpin
functions by skipping reservation locking while GEM's pin refcount > 1.
Suggested-by: Boris Brezillon
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 35 +++--
drivers/gpu/drm/lima/lima_gem.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_mmu.c | 2 +-
include/drm/drm_gem_shmem_helper.h | 2 +-
4 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 7e1e674e2c9f..a0faef3e762d 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -152,12 +152,12 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
}
- if (shmem->pages) {
+ if (refcount_read(>pages_use_count)) {
drm_gem_shmem_put_pages_locked(shmem);
drm_WARN_ON(obj->dev, !shmem->got_pages_sgt);
}
- drm_WARN_ON(obj->dev, shmem->pages_use_count);
+ drm_WARN_ON(obj->dev, refcount_read(>pages_use_count));
dma_resv_unlock(shmem->base.resv);
}
@@ -174,14 +174,13 @@ static int drm_gem_shmem_get_pages_locked(struct
drm_gem_shmem_object *shmem)
dma_resv_assert_held(shmem->base.resv);
- if (shmem->pages_use_count++ > 0)
+ if (refcount_inc_not_zero(>pages_use_count))
return 0;
pages = drm_gem_get_pages(obj);
if (IS_ERR(pages)) {
drm_dbg_kms(obj->dev, "Failed to get pages (%ld)\n",
PTR_ERR(pages));
- shmem->pages_use_count = 0;
return PTR_ERR(pages);
}
@@ -197,6 +196,8 @@ static int drm_gem_shmem_get_pages_locked(struct
drm_gem_shmem_object *shmem)
shmem->pages = pages;
+ refcount_set(>pages_use_count, 1);
+
return 0;
}
@@ -212,21 +213,17 @@ void drm_gem_shmem_put_pages_locked(struct
drm_gem_shmem_object *shmem)
dma_resv_assert_held(shmem->base.resv);
- if (drm_WARN_ON_ONCE(obj->dev, !shmem->pages_use_count))
- return;
-
- if (--shmem->pages_use_count > 0)
- return;
-
+ if (refcount_dec_and_test(>pages_use_count)) {
#ifdef CONFIG_X86
- if (shmem->map_wc)
- set_pages_array_wb(shmem->pages, obj->size >> PAGE_SHIFT);
+ if (shmem->map_wc)
+ set_pages_array_wb(shmem->pages, obj->size >>
PAGE_SHIFT);
#endif
- drm_gem_put_pages(obj, shmem->pages,
- shmem->pages_mark_dirty_on_put,
- shmem->pages_mark_accessed_on_put);
- shmem->pages = NULL;
+ drm_gem_put_pages(obj, shmem->pages,
+ shmem->pages_mark_dirty_on_put,
+ shmem->pages_mark_accessed_on_put);
+ shmem->pages = NULL;
+ }
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked);
@@ -553,8 +550,8 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct
*vma)
* mmap'd, vm_open() just grabs an additional reference for the new
* mm the vma is getting copied into (ie. on fork()).
*/
- if (!drm_WARN_ON_ONCE(obj->dev, !shmem->pages_use_count))
- shmem->pages_use_count++;
+ drm_WARN_ON_ONCE(obj->dev,
+!refcount_inc_not_zero(>pages_use_count));
dma_resv_unlock(shmem->base.resv);
@@ -642,7 +639,7 @@ void drm_gem_shmem_print_info(const struct
drm_gem_shmem_object *shmem,
return;
drm_printf_indent(p, indent, "pages_pin_count=%u\n",
refcount_read(>pages_pin_count));
- drm_printf_indent(p, indent, "pages_use_count=%u\n",
shmem->pages_use_count);
+ drm_printf_indent(p, indent, "pages_use_count=%u\n",
refcount_read(>pages_use_count));
drm_printf_indent(p, indent, "vmap_use_count=%u\n",
shmem->vmap_use_count);
drm_printf_indent(p, indent, "vaddr=%p\n", shmem->vaddr);
}
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index ec8f718aa539..4be2fccbf6d9 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -47,8 +47,8 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
}
bo->base.pages = pages;
- bo->base.pages_use_count = 1;
bo->base.got_pages_sgt = true;
+ refcount_set(>base.pages_use_count, 1);
mapping_set_unevictable(mapping);
}
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index 7771769f0ce0..a91252053aa3 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++
[PATCH v16 07/20] drm/shmem-helper: Make all exported symbols GPL
Make all drm-shmem exported symbols GPL to make them consistent with
the rest of drm-shmem symbols.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 848435e08eb2..5c777adf1bcb 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -228,7 +228,7 @@ void drm_gem_shmem_put_pages(struct drm_gem_shmem_object
*shmem)
shmem->pages_mark_accessed_on_put);
shmem->pages = NULL;
}
-EXPORT_SYMBOL(drm_gem_shmem_put_pages);
+EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages);
static int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem)
{
@@ -273,7 +273,7 @@ int drm_gem_shmem_pin(struct drm_gem_shmem_object *shmem)
return ret;
}
-EXPORT_SYMBOL(drm_gem_shmem_pin);
+EXPORT_SYMBOL_GPL(drm_gem_shmem_pin);
/**
* drm_gem_shmem_unpin - Unpin backing pages for a shmem GEM object
@@ -292,7 +292,7 @@ void drm_gem_shmem_unpin(struct drm_gem_shmem_object *shmem)
drm_gem_shmem_unpin_locked(shmem);
dma_resv_unlock(shmem->base.resv);
}
-EXPORT_SYMBOL(drm_gem_shmem_unpin);
+EXPORT_SYMBOL_GPL(drm_gem_shmem_unpin);
/*
* drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object
@@ -362,7 +362,7 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
return ret;
}
-EXPORT_SYMBOL(drm_gem_shmem_vmap);
+EXPORT_SYMBOL_GPL(drm_gem_shmem_vmap);
/*
* drm_gem_shmem_vunmap - Unmap a virtual mapping for a shmem GEM object
@@ -398,7 +398,7 @@ void drm_gem_shmem_vunmap(struct drm_gem_shmem_object
*shmem,
shmem->vaddr = NULL;
}
-EXPORT_SYMBOL(drm_gem_shmem_vunmap);
+EXPORT_SYMBOL_GPL(drm_gem_shmem_vunmap);
static int
drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
@@ -437,7 +437,7 @@ int drm_gem_shmem_madvise(struct drm_gem_shmem_object
*shmem, int madv)
return (madv >= 0);
}
-EXPORT_SYMBOL(drm_gem_shmem_madvise);
+EXPORT_SYMBOL_GPL(drm_gem_shmem_madvise);
void drm_gem_shmem_purge(struct drm_gem_shmem_object *shmem)
{
@@ -469,7 +469,7 @@ void drm_gem_shmem_purge(struct drm_gem_shmem_object *shmem)
invalidate_mapping_pages(file_inode(obj->filp)->i_mapping, 0,
(loff_t)-1);
}
-EXPORT_SYMBOL(drm_gem_shmem_purge);
+EXPORT_SYMBOL_GPL(drm_gem_shmem_purge);
/**
* drm_gem_shmem_dumb_create - Create a dumb shmem buffer object
@@ -644,7 +644,7 @@ void drm_gem_shmem_print_info(const struct
drm_gem_shmem_object *shmem,
drm_printf_indent(p, indent, "vmap_use_count=%u\n",
shmem->vmap_use_count);
drm_printf_indent(p, indent, "vaddr=%p\n", shmem->vaddr);
}
-EXPORT_SYMBOL(drm_gem_shmem_print_info);
+EXPORT_SYMBOL_GPL(drm_gem_shmem_print_info);
/**
* drm_gem_shmem_get_sg_table - Provide a scatter/gather table of pinned
--
2.41.0
[PATCH v16 06/20] drm/virtio: Replace drm_gem_shmem_free() with drm_gem_object_put()
Prepare virtio_gpu_object_create() to addition of memory shrinker support by replacing open-coded drm_gem_shmem_free() with drm_gem_object_put() that decrements GEM refcount to 0, which becomes important for drm-shmem because it will start to use GEM's refcount during the shmem's BO freeing time in order to prevent spurious lockdep warning about resv lock ordering vs fs_reclaim code paths. Signed-off-by: Dmitry Osipenko --- drivers/gpu/drm/virtio/virtgpu_object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index c7e74cf13022..343b13428125 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -244,6 +244,6 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, err_put_id: virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle); err_free_gem: - drm_gem_shmem_free(shmem_obj); + drm_gem_object_put(>base.base); return ret; } -- 2.41.0
[PATCH v16 04/20] drm/gem: Add _locked postfix to functions that have unlocked counterpart
Add _locked postfix to drm_gem functions that have unlocked counterpart
functions to make GEM functions naming more consistent and intuitive in
regards to the locking requirements.
Reviewed-by: Boris Brezillon
Suggested-by: Boris Brezillon
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem.c | 6 +++---
include/drm/drm_gem.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index fae5832bb0bd..8c0268944199 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1488,10 +1488,10 @@ drm_gem_lru_scan(struct drm_gem_lru *lru,
EXPORT_SYMBOL(drm_gem_lru_scan);
/**
- * drm_gem_evict - helper to evict backing pages for a GEM object
+ * drm_gem_evict_locked - helper to evict backing pages for a GEM object
* @obj: obj in question
*/
-int drm_gem_evict(struct drm_gem_object *obj)
+int drm_gem_evict_locked(struct drm_gem_object *obj)
{
dma_resv_assert_held(obj->resv);
@@ -1503,4 +1503,4 @@ int drm_gem_evict(struct drm_gem_object *obj)
return 0;
}
-EXPORT_SYMBOL(drm_gem_evict);
+EXPORT_SYMBOL(drm_gem_evict_locked);
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 110a9c0ea42b..d8dedb1f0968 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -542,7 +542,7 @@ unsigned long drm_gem_lru_scan(struct drm_gem_lru *lru,
unsigned long *remaining,
bool (*shrink)(struct drm_gem_object *obj));
-int drm_gem_evict(struct drm_gem_object *obj);
+int drm_gem_evict_locked(struct drm_gem_object *obj);
#ifdef CONFIG_LOCKDEP
/**
--
2.41.0
[PATCH v16 02/20] drm/shmem-helper: Use flag for tracking page count bumped by get_pages_sgt()
Use separate flag for tracking page count bumped by shmem->sgt to avoid
imbalanced page counter during of drm_gem_shmem_free() time. It's fragile
to assume that populated shmem->pages at a freeing time means that the
count was bumped by drm_gem_shmem_get_pages_sgt(), using a flag removes
the ambiguity.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 11 ++-
drivers/gpu/drm/lima/lima_gem.c| 1 +
include/drm/drm_gem_shmem_helper.h | 7 +++
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 6693d4061ca1..848435e08eb2 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -152,8 +152,10 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
}
- if (shmem->pages)
+ if (shmem->pages) {
drm_gem_shmem_put_pages(shmem);
+ drm_WARN_ON(obj->dev, !shmem->got_pages_sgt);
+ }
drm_WARN_ON(obj->dev, shmem->pages_use_count);
@@ -693,6 +695,13 @@ static struct sg_table
*drm_gem_shmem_get_pages_sgt_locked(struct drm_gem_shmem_
if (ret)
goto err_free_sgt;
+ /*
+* This flag prevents imbalanced pages_use_count during
+* drm_gem_shmem_free(), where pages_use_count=1 only if
+* drm_gem_shmem_get_pages_sgt() was used by a driver.
+*/
+ shmem->got_pages_sgt = true;
+
shmem->sgt = sgt;
return sgt;
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 4f9736e5f929..67c39b95e30e 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -48,6 +48,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
bo->base.pages = pages;
bo->base.pages_use_count = 1;
+ bo->base.got_pages_sgt = true;
mapping_set_unevictable(mapping);
}
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index ec70a98a8fe1..a53c0874b3c4 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -73,6 +73,13 @@ struct drm_gem_shmem_object {
*/
unsigned int vmap_use_count;
+ /**
+* @got_pages_sgt:
+*
+* True if SG table was retrieved using drm_gem_shmem_get_pages_sgt()
+*/
+ bool got_pages_sgt : 1;
+
/**
* @imported_sgt:
*
--
2.41.0
[PATCH v16 01/20] drm/shmem-helper: Fix UAF in error path when freeing SGT of imported GEM
Freeing drm-shmem GEM right after creating it using
drm_gem_shmem_prime_import_sg_table() frees SGT of the imported dma-buf
and then dma-buf frees this SGT second time.
The v3d_prime_import_sg_table() is example of a error code path where
dma-buf's SGT is freed by drm-shmem and then it's freed second time by
dma_buf_unmap_attachment() in drm_gem_prime_import_dev().
Add drm-shmem GEM flag telling that this is imported SGT shall not be
treated as own SGT, fixing the use-after-free bug.
Cc: sta...@vger.kernel.org
Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects")
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 13 -
include/drm/drm_gem_shmem_helper.h | 7 +++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index e435f986cd13..6693d4061ca1 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -141,7 +141,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
if (obj->import_attach) {
drm_prime_gem_destroy(obj, shmem->sgt);
- } else {
+ } else if (!shmem->imported_sgt) {
dma_resv_lock(shmem->base.resv, NULL);
drm_WARN_ON(obj->dev, shmem->vmap_use_count);
@@ -765,6 +765,17 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
shmem->sgt = sgt;
+ /*
+* drm_gem_shmem_prime_import_sg_table() can be called from a
+* driver specific ->import_sg_table() implementations that
+* may fail, in that case drm_gem_shmem_free() will be invoked
+* without assigned drm_gem_object::import_attach.
+*
+* This flag lets drm_gem_shmem_free() differentiate whether
+* SGT belongs to dmabuf and shall not be freed by drm-shmem.
+*/
+ shmem->imported_sgt = true;
+
drm_dbg_prime(dev, "size = %zu\n", size);
return >base;
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index bf0c31aa8fbe..ec70a98a8fe1 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -73,6 +73,13 @@ struct drm_gem_shmem_object {
*/
unsigned int vmap_use_count;
+ /**
+* @imported_sgt:
+*
+* True if SG table belongs to imported dma-buf.
+*/
+ bool imported_sgt : 1;
+
/**
* @pages_mark_dirty_on_put:
*
--
2.41.0
[PATCH v16 03/20] drm/gem: Change locked/unlocked postfix of drm_gem_v/unmap() function names
Make drm/gem API function names consistent by having locked function
use the _locked postfix in the name, while the unlocked variants don't
use the _unlocked postfix. Rename drm_gem_v/unmap() function names to
make them consistent with the rest of the API functions.
Reviewed-by: Boris Brezillon
Suggested-by: Boris Brezillon
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_client.c | 6 +++---
drivers/gpu/drm/drm_gem.c| 20 ++--
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +++---
drivers/gpu/drm/drm_internal.h | 4 ++--
drivers/gpu/drm/drm_prime.c | 4 ++--
drivers/gpu/drm/lima/lima_sched.c| 4 ++--
drivers/gpu/drm/panfrost/panfrost_dump.c | 4 ++--
drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 6 +++---
include/drm/drm_gem.h| 4 ++--
9 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 037e36f2049c..29306657117a 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -265,7 +265,7 @@ void drm_client_dev_restore(struct drm_device *dev)
static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
{
if (buffer->gem) {
- drm_gem_vunmap_unlocked(buffer->gem, >map);
+ drm_gem_vunmap(buffer->gem, >map);
drm_gem_object_put(buffer->gem);
}
@@ -349,7 +349,7 @@ drm_client_buffer_vmap(struct drm_client_buffer *buffer,
* fd_install step out of the driver backend hooks, to make that
* final step optional for internal users.
*/
- ret = drm_gem_vmap_unlocked(buffer->gem, map);
+ ret = drm_gem_vmap(buffer->gem, map);
if (ret)
return ret;
@@ -371,7 +371,7 @@ void drm_client_buffer_vunmap(struct drm_client_buffer
*buffer)
{
struct iosys_map *map = >map;
- drm_gem_vunmap_unlocked(buffer->gem, map);
+ drm_gem_vunmap(buffer->gem, map);
}
EXPORT_SYMBOL(drm_client_buffer_vunmap);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 6129b89bb366..fae5832bb0bd 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1173,7 +1173,7 @@ void drm_gem_unpin(struct drm_gem_object *obj)
obj->funcs->unpin(obj);
}
-int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
+int drm_gem_vmap_locked(struct drm_gem_object *obj, struct iosys_map *map)
{
int ret;
@@ -1190,9 +1190,9 @@ int drm_gem_vmap(struct drm_gem_object *obj, struct
iosys_map *map)
return 0;
}
-EXPORT_SYMBOL(drm_gem_vmap);
+EXPORT_SYMBOL(drm_gem_vmap_locked);
-void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map)
+void drm_gem_vunmap_locked(struct drm_gem_object *obj, struct iosys_map *map)
{
dma_resv_assert_held(obj->resv);
@@ -1205,27 +1205,27 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct
iosys_map *map)
/* Always set the mapping to NULL. Callers may rely on this. */
iosys_map_clear(map);
}
-EXPORT_SYMBOL(drm_gem_vunmap);
+EXPORT_SYMBOL(drm_gem_vunmap_locked);
-int drm_gem_vmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map)
+int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
{
int ret;
dma_resv_lock(obj->resv, NULL);
- ret = drm_gem_vmap(obj, map);
+ ret = drm_gem_vmap_locked(obj, map);
dma_resv_unlock(obj->resv);
return ret;
}
-EXPORT_SYMBOL(drm_gem_vmap_unlocked);
+EXPORT_SYMBOL(drm_gem_vmap);
-void drm_gem_vunmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map)
+void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map)
{
dma_resv_lock(obj->resv, NULL);
- drm_gem_vunmap(obj, map);
+ drm_gem_vunmap_locked(obj, map);
dma_resv_unlock(obj->resv);
}
-EXPORT_SYMBOL(drm_gem_vunmap_unlocked);
+EXPORT_SYMBOL(drm_gem_vunmap);
/**
* drm_gem_lock_reservations - Sets up the ww context and acquires
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 3bdb6ba37ff4..3808f47310bf 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -362,7 +362,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct
iosys_map *map,
ret = -EINVAL;
goto err_drm_gem_vunmap;
}
- ret = drm_gem_vmap_unlocked(obj, [i]);
+ ret = drm_gem_vmap(obj, [i]);
if (ret)
goto err_drm_gem_vunmap;
}
@@ -384,7 +384,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct
iosys_map *map,
obj = drm_gem_fb_get_obj(fb, i);
if (!obj)
continue;
- drm_gem_vunmap_unlocked(obj, [i]);
+
Re: [RFT PATCH 01/15] drm/armada: Call drm_atomic_helper_shutdown() at shutdown time
On Fri, Sep 01, 2023 at 04:41:12PM -0700, Douglas Anderson wrote: > Based on grepping through the source code this driver appears to be > missing a call to drm_atomic_helper_shutdown() at system shutdown > time. Among other things, this means that if a panel is in use that it > won't be cleanly powered off at system shutdown time. > > The fact that we should call drm_atomic_helper_shutdown() in the case > of OS shutdown/restart comes straight out of the kernel doc "driver > instance overview" in drm_drv.c. > > This driver was fairly easy to update. The drm_device is stored in the > drvdata so we just have to make sure the drvdata is NULL whenever the > device is not bound. ... and there I think you have a misunderstanding of the driver model. Please have a look at device_unbind_cleanup() which will be called if probe fails, or when the device is removed (in other words, when it is not bound to a driver.) Also, devices which aren't bound to a driver won't have their shutdown method called (because there is no driver currently bound to that device.) So, ->probe must have completed successfully, and ->remove must not have been called for that device. So, I think that all these dev_set_drvdata(dev, NULL) that you're adding are just asking for a kernel janitor to come along later and remove them because they serve no purpose... so best not introduce them in the first place. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] drm/rcar-du: fix comment to rcar_du_group_get()
Hi Alexandra
Quoting Alexandra Diupina (2023-09-03 14:37:09)
> rcar_du_group_get() never returns a negative
> error code (always returns 0), so change
> the comment about returned value
If so, then perhaps this may as well become a void return and remove the
return 0.
That could then clean up some redundant error path handling in
drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c too ?
Still, this does correct the documentation to match the implementation
as it stands so... for that ...
Reviewed-by: Kieran Bingham
But removing an unused error path seems like a worthy clean up
opportunity too.
>
> Fixes: cb2025d2509f ("drm/rcar-du: Introduce CRTCs groups")
Hrm ... well the documented behaviour was the same even before this
commit in rcar_du_get(), so perhaps it was documenting the intent... But
it does seem that the return code has been redundant for quite some time
so perhaps it's just not required.
--
Kieran
> Signed-off-by: Alexandra Diupina
> ---
> drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> index 2ccd2581f544..499d4e56c32d 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> @@ -200,7 +200,7 @@ static void rcar_du_group_setup(struct rcar_du_group
> *rgrp)
> *
> * This function must be called with the DRM mode_config lock held.
> *
> - * Return 0 in case of success or a negative error code otherwise.
> + * Always return 0.
> */
> int rcar_du_group_get(struct rcar_du_group *rgrp)
> {
> --
> 2.30.2
>
Re: [PATCH] drm: gm12u320: Fix the timeout usage for usb_bulk_msg()
Hi Jinjie,
On 9/3/23 10:55, Jinjie Ruan wrote:
> The timeout arg of usb_bulk_msg() is ms already and it has convert it
> to jiffies by msecs_to_jiffies() in usb_start_wait_urb(). So fix the usage
> by remove the redundant msecs_to_jiffies() in the macros.
>
> Fixes: 77b8cabf3d52 ("drm/gm12u320: Move driver to drm/tiny")
> Signed-off-by: Jinjie Ruan
Good catch, thank you.
> ---
> drivers/gpu/drm/tiny/gm12u320.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index c5bb683e440c..31fa50c665d1 100644
> --- a/drivers/gpu/drm/tiny/gm12u320.c
> +++ b/drivers/gpu/drm/tiny/gm12u320.c
> @@ -70,10 +70,10 @@ MODULE_PARM_DESC(eco_mode, "Turn on Eco mode (less
> bright, more silent)");
> #define READ_STATUS_SIZE 13
> #define MISC_VALUE_SIZE 4
>
> -#define CMD_TIMEOUT msecs_to_jiffies(200)
> -#define DATA_TIMEOUT msecs_to_jiffies(1000)
> +#define CMD_TIMEOUT 200
> +#define DATA_TIMEOUT 1000
> #define IDLE_TIMEOUT msecs_to_jiffies(2000)
> -#define FIRST_FRAME_TIMEOUT msecs_to_jiffies(2000)
> +#define FIRST_FRAME_TIMEOUT 2000
Lets be consistent here and change all *_TIMEOUT defines to
be in milliseconds.
And then in the 1 place where IDLE_TIMEOUT is used, change
"IDLE_TIMEOUT" to "msecs_to_jiffies(IDLE_TIMEOUT)"
REgards,
Hans
>
> #define MISC_REQ_GET_SET_ECO_A 0xff
> #define MISC_REQ_GET_SET_ECO_B 0x35
[PATCH] drm: gm12u320: Fix the timeout usage for usb_bulk_msg()
The timeout arg of usb_bulk_msg() is ms already and it has convert it
to jiffies by msecs_to_jiffies() in usb_start_wait_urb(). So fix the usage
by remove the redundant msecs_to_jiffies() in the macros.
Fixes: 77b8cabf3d52 ("drm/gm12u320: Move driver to drm/tiny")
Signed-off-by: Jinjie Ruan
---
drivers/gpu/drm/tiny/gm12u320.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index c5bb683e440c..31fa50c665d1 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -70,10 +70,10 @@ MODULE_PARM_DESC(eco_mode, "Turn on Eco mode (less bright,
more silent)");
#define READ_STATUS_SIZE 13
#define MISC_VALUE_SIZE4
-#define CMD_TIMEOUTmsecs_to_jiffies(200)
-#define DATA_TIMEOUT msecs_to_jiffies(1000)
+#define CMD_TIMEOUT200
+#define DATA_TIMEOUT 1000
#define IDLE_TIMEOUT msecs_to_jiffies(2000)
-#define FIRST_FRAME_TIMEOUTmsecs_to_jiffies(2000)
+#define FIRST_FRAME_TIMEOUT2000
#define MISC_REQ_GET_SET_ECO_A 0xff
#define MISC_REQ_GET_SET_ECO_B 0x35
--
2.34.1
Re: mainline build failure due to 501126083855 ("fbdev/g364fb: Use fbdev I/O helpers")
[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]
On 31.08.23 20:48, Sudip Mukherjee (Codethink) wrote:
> Hi All,
>
> The latest mainline kernel branch fails to build mips jazz_defconfig with
> the error:
>
> drivers/video/fbdev/g364fb.c:115:9: error: 'FB_DEFAULT_IOMEM_HELPERS'
> undeclared here (not in a function); did you mean 'FB_DEFAULT_IOMEM_OPS'?
> 115 | FB_DEFAULT_IOMEM_HELPERS,
> | ^~~~
> | FB_DEFAULT_IOMEM_OPS
>
>
> git bisect pointed to 501126083855 ("fbdev/g364fb: Use fbdev I/O helpers").
>
> Reverting the commit has fixed the build failure.
>
> I will be happy to test any patch or provide any extra log if needed.
>
> #regzbot introduced: 5011260838551cefbf23d60b48c3243b6d5530a2
>
#regzbot fix: 8df0f84c3bb921f5aa1036223dd932bbc7df6d
#regzbot ignore-activity
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
