[Intel-gfx] [PATCH 0/2] MTL Degamma implementation
MTL onwards Degamma LUT/PRE-CSC LUT precision has been increased from 16 bits to 24 bits. Currently, drm framework only supports LUTs up to 16 bit precision. Until a new uapi comes along to support higher bitdepth, upscale the values sent from userland to 24 bit before writing into the HW to continue supporting degamma on MTL. To avoid pipe config mismatch between 24 bit HW lut values and 16 bit userspace sent values, convert back the 24 bit lut values read from HW to 16 bit values. Chaitanya Kumar Borah (2): drm/i915/color: Upscale degamma values for MTL drm/i915/color: Downscale degamma lut values read from hardware drivers/gpu/drm/i915/display/intel_color.c | 27 +- 1 file changed, 26 insertions(+), 1 deletion(-) -- 2.25.1
[Intel-gfx] [PATCH v2 1/2] drm/i915/color: Upscale degamma values for MTL
MTL onwards Degamma LUT/PRE-CSC LUT precision has been increased from 16 bits to 24 bits. Currently, drm framework only supports LUTs up to 16 bit precision. Until a new uapi comes along to support higher bitdepth, upscale the values sent from userland to 24 bit before writing into the HW to continue supporting degamma on MTL. Add helper function to upscale or downscale lut values. Parameters 'to' and 'from' needs to be less than 32. This should be sufficient as currently there are no lut values exceeding 32 bit. v2: (Jani) - Reuse glk_load_degamma_lut() - Create a helper function for upscaling values Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_color.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 8966e6560516..15ada7fada96 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1453,6 +1453,16 @@ static int glk_degamma_lut_size(struct drm_i915_private *i915) return 35; } +/* change_lut_val_precision: helper function to upscale or downscale lut values. + * Parameters 'to' and 'from' needs to be less than 32. This should be sufficient + * as currently there are no lut values exceeding 32 bit. + **/ + +static u32 change_lut_val_precision(u32 lut_val, int to, int from) +{ + return mul_u32_u32(lut_val, (1 << to)) / (1 << from); +} + static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state, const struct drm_property_blob *blob) { @@ -1487,8 +1497,15 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state, * ToDo: Extend to max 7.0. Enable 32 bit input value * as compared to just 16 to achieve this. */ + u32 lut_val; + + if (DISPLAY_VER(i915) >= 14) + lut_val = change_lut_val_precision(lut[i].green, 24, 16); + else + lut_val = lut[i].green; + ilk_lut_write(crtc_state, PRE_CSC_GAMC_DATA(pipe), - lut[i].green); + lut_val); } /* Clamp values > 1.0. */ -- 2.25.1
[Intel-gfx] [PATCH v2 2/2] drm/i915/color: Downscale degamma lut values read from hardware
For MTL and beyond, convert back the 24 bit lut values read from HW to 16 bit values to maintain parity with userspace values. This way we avoid pipe config mismatch for pre-csc lut values. v2: Add helper function to downscale values (Jani) Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_color.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 15ada7fada96..41666e778c03 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -3456,6 +3456,14 @@ static struct drm_property_blob *glk_read_degamma_lut(struct intel_crtc *crtc) for (i = 0; i < lut_size; i++) { u32 val = intel_de_read_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe)); + /* +* For MTL and beyond, convert back the 24 bit lut values +* read from HW to 16 bit values to maintain parity with +* userspace values +*/ + if (DISPLAY_VER(dev_priv) >= 14) + val = change_lut_val_precision(val, 16, 24); + lut[i].red = val; lut[i].green = val; lut[i].blue = val; -- 2.25.1
[Intel-gfx] [PATCH 0/2] MTL Degamma implementation
MTL onwards Degamma LUT/PRE-CSC LUT precision has been increased from 16 bits to 24 bits. Currently, drm framework only supports LUTs up to 16 bit precision. Until a new uapi comes along to support higher bitdepth, upscale the values sent from userland to 24 bit before writing into the HW to continue supporting degamma on MTL. To avoid pipe config mismatch between 24 bit HW lut values and 16 bit userspace sent values, convert back the 24 bit lut values read from HW to 16 bit values. Chaitanya Kumar Borah (2): drm/i915/color: Upscale degamma values for MTL drm/i915/color: Downscale degamma lut values read from hardware drivers/gpu/drm/i915/display/intel_color.c | 28 +- 1 file changed, 27 insertions(+), 1 deletion(-) -- 2.25.1
[Intel-gfx] [PATCH v3 1/2] drm/i915/color: Upscale degamma values for MTL
MTL onwards Degamma LUT/PRE-CSC LUT precision has been increased from 16 bits to 24 bits. Currently, drm framework only supports LUTs up to 16 bit precision. Until a new uapi comes along to support higher bitdepth, upscale the values sent from userland to 24 bit before writing into the HW to continue supporting degamma on MTL. Add helper function to upscale or downscale lut values. Parameters 'to' and 'from' needs to be less than 32. This should be sufficient as currently there are no lut values exceeding 32 bit. v2: (Jani) - Reuse glk_load_degamma_lut() - Create a helper function for upscaling values v3: Fix multi line comment style (Uma) Signed-off-by: Chaitanya Kumar Borah Reviewed-by: Uma Shankar --- drivers/gpu/drm/i915/display/intel_color.c | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 8966e6560516..32182cdff928 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1453,6 +1453,17 @@ static int glk_degamma_lut_size(struct drm_i915_private *i915) return 35; } +/* + * change_lut_val_precision: helper function to upscale or downscale lut values. + * Parameters 'to' and 'from' needs to be less than 32. This should be sufficient + * as currently there are no lut values exceeding 32 bit. + */ + +static u32 change_lut_val_precision(u32 lut_val, int to, int from) +{ + return mul_u32_u32(lut_val, (1 << to)) / (1 << from); +} + static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state, const struct drm_property_blob *blob) { @@ -1487,8 +1498,15 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state, * ToDo: Extend to max 7.0. Enable 32 bit input value * as compared to just 16 to achieve this. */ + u32 lut_val; + + if (DISPLAY_VER(i915) >= 14) + lut_val = change_lut_val_precision(lut[i].green, 24, 16); + else + lut_val = lut[i].green; + ilk_lut_write(crtc_state, PRE_CSC_GAMC_DATA(pipe), - lut[i].green); + lut_val); } /* Clamp values > 1.0. */ -- 2.25.1
[Intel-gfx] [PATCH v2 2/2] drm/i915/color: Downscale degamma lut values read from hardware
For MTL and beyond, convert back the 24 bit lut values read from HW to 16 bit values to maintain parity with userspace values. This way we avoid pipe config mismatch for pre-csc lut values. v2: Add helper function to downscale values (Jani) Signed-off-by: Chaitanya Kumar Borah Reviewed-by: Uma Shankar --- drivers/gpu/drm/i915/display/intel_color.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 32182cdff928..d78e2715d419 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -3457,6 +3457,14 @@ static struct drm_property_blob *glk_read_degamma_lut(struct intel_crtc *crtc) for (i = 0; i < lut_size; i++) { u32 val = intel_de_read_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe)); + /* +* For MTL and beyond, convert back the 24 bit lut values +* read from HW to 16 bit values to maintain parity with +* userspace values +*/ + if (DISPLAY_VER(dev_priv) >= 14) + val = change_lut_val_precision(val, 16, 24); + lut[i].red = val; lut[i].green = val; lut[i].blue = val; -- 2.25.1
[Intel-gfx] [PATCH v4 1/2] drm/i915/color: Upscale degamma values for MTL
MTL onwards Degamma LUT/PRE-CSC LUT precision has been increased from 16 bits to 24 bits. Currently, drm framework only supports LUTs up to 16 bit precision. Until a new uapi comes along to support higher bitdepth, upscale the values sent from userland to 24 bit before writing into the HW to continue supporting degamma on MTL. Add helper function to upscale or downscale lut values. Parameters 'to' and 'from' needs to be less than 32. This should be sufficient as currently there are no lut values exceeding 32 bit. v2: (Jani) - Reuse glk_load_degamma_lut() - Create a helper function for upscaling values v3: Fix multi line comment style (Uma) v4: Remove extra line(Ankit) Signed-off-by: Chaitanya Kumar Borah Reviewed-by: Uma Shankar --- drivers/gpu/drm/i915/display/intel_color.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 8966e6560516..211b7fce1b27 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1453,6 +1453,16 @@ static int glk_degamma_lut_size(struct drm_i915_private *i915) return 35; } +/* + * change_lut_val_precision: helper function to upscale or downscale lut values. + * Parameters 'to' and 'from' needs to be less than 32. This should be sufficient + * as currently there are no lut values exceeding 32 bit. + */ +static u32 change_lut_val_precision(u32 lut_val, int to, int from) +{ + return mul_u32_u32(lut_val, (1 << to)) / (1 << from); +} + static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state, const struct drm_property_blob *blob) { @@ -1487,8 +1497,15 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state, * ToDo: Extend to max 7.0. Enable 32 bit input value * as compared to just 16 to achieve this. */ + u32 lut_val; + + if (DISPLAY_VER(i915) >= 14) + lut_val = change_lut_val_precision(lut[i].green, 24, 16); + else + lut_val = lut[i].green; + ilk_lut_write(crtc_state, PRE_CSC_GAMC_DATA(pipe), - lut[i].green); + lut_val); } /* Clamp values > 1.0. */ -- 2.25.1
[Intel-gfx] [PATCH] drm/i915/rpl: Add new RPL PCI-IDs
Add newly added PCI-IDs for RPL BSpec: 55376 Signed-off-by: Chaitanya Kumar Borah --- include/drm/i915_pciids.h | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 1661f9e552d2..1256770d3827 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -689,14 +689,18 @@ #define INTEL_RPLU_IDS(info) \ INTEL_VGA_DEVICE(0xA721, info), \ INTEL_VGA_DEVICE(0xA7A1, info), \ - INTEL_VGA_DEVICE(0xA7A9, info) + INTEL_VGA_DEVICE(0xA7A9, info), \ + INTEL_VGA_DEVICE(0xA7AC, info), \ + INTEL_VGA_DEVICE(0xA7AD, info) /* RPL-P */ #define INTEL_RPLP_IDS(info) \ INTEL_RPLU_IDS(info), \ INTEL_VGA_DEVICE(0xA720, info), \ INTEL_VGA_DEVICE(0xA7A0, info), \ - INTEL_VGA_DEVICE(0xA7A8, info) + INTEL_VGA_DEVICE(0xA7A8, info), \ + INTEL_VGA_DEVICE(0xA7AA, info), \ + INTEL_VGA_DEVICE(0xA7AB, info) /* DG2 */ #define INTEL_DG2_G10_IDS(info) \ -- 2.25.1
[Intel-gfx] [PATCH] drm/i915: Add wrapper for getting display step
Add a wrapper around intel_step_name that takes in driver data as an argument. This wrapper will help maintain compatibility with the proposed xe driver. Signed-off-by: Chaitanya Kumar Borah Signed-off-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_dmc.c | 2 +- drivers/gpu/drm/i915/intel_step.c| 5 + drivers/gpu/drm/i915/intel_step.h| 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c index 1623c0c5e8a1..63e080e07023 100644 --- a/drivers/gpu/drm/i915/display/intel_dmc.c +++ b/drivers/gpu/drm/i915/display/intel_dmc.c @@ -309,7 +309,7 @@ static const struct stepping_info * intel_get_stepping_info(struct drm_i915_private *i915, struct stepping_info *si) { - const char *step_name = intel_step_name(RUNTIME_INFO(i915)->step.display_step); + const char *step_name = intel_display_step_name(i915); si->stepping = step_name[0]; si->substepping = step_name[1]; diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c index ee4e5a2c0220..b4162f1be765 100644 --- a/drivers/gpu/drm/i915/intel_step.c +++ b/drivers/gpu/drm/i915/intel_step.c @@ -353,3 +353,8 @@ const char *intel_step_name(enum intel_step step) return "**"; } } + +const char *intel_display_step_name(struct drm_i915_private *i915) +{ + return intel_step_name(RUNTIME_INFO(i915)->step.display_step); +} diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h index 96dfca4cba73..b6f43b624774 100644 --- a/drivers/gpu/drm/i915/intel_step.h +++ b/drivers/gpu/drm/i915/intel_step.h @@ -78,5 +78,6 @@ enum intel_step { void intel_step_init(struct drm_i915_private *i915); const char *intel_step_name(enum intel_step step); +const char *intel_display_step_name(struct drm_i915_private *i915); #endif /* __INTEL_STEP_H__ */ -- 2.25.1
[Intel-gfx] [PATCH] drm/i915/mtl: Support HBR3 rate with C10 phy and eDP in MTL
eDP specification supports HBR3 link rate since v1.4a. Moreover, C10 phy can support HBR3 link rate for both DP and eDP. Therefore, do not clamp the supported rates for eDP at 6.75Gbps. Cc: BSpec: 70073 74224 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 1891c0cc187d..2c1034578984 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -430,7 +430,7 @@ static int mtl_max_source_rate(struct intel_dp *intel_dp) enum phy phy = intel_port_to_phy(i915, dig_port->base.port); if (intel_is_c10phy(i915, phy)) - return intel_dp_is_edp(intel_dp) ? 675000 : 81; + return 81; return 200; } -- 2.25.1
[Intel-gfx] [PATCH v2] drm/i915/display: Set correct voltage level for 480MHz CDCLK
According to Bspec, the voltage level for 480MHz is to be set as 1 instead of 2. BSpec: 49208 Fixes: 06f1b06dc5b7 ("drm/i915/display: Add 480 MHz CDCLK steps for RPL-U") v2: rebase Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 30 +++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 6bed75f1541a..1a5268e3d0a3 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1453,6 +1453,18 @@ static u8 tgl_calc_voltage_level(int cdclk) return 0; } +static u8 rplu_calc_voltage_level(int cdclk) +{ + if (cdclk > 556800) + return 3; + else if (cdclk > 48) + return 2; + else if (cdclk > 312000) + return 1; + else + return 0; +} + static void icl_readout_refclk(struct drm_i915_private *dev_priv, struct intel_cdclk_config *cdclk_config) { @@ -3397,6 +3409,13 @@ static const struct intel_cdclk_funcs mtl_cdclk_funcs = { .calc_voltage_level = tgl_calc_voltage_level, }; +static const struct intel_cdclk_funcs rplu_cdclk_funcs = { + .get_cdclk = bxt_get_cdclk, + .set_cdclk = bxt_set_cdclk, + .modeset_calc_cdclk = bxt_modeset_calc_cdclk, + .calc_voltage_level = rplu_calc_voltage_level, +}; + static const struct intel_cdclk_funcs tgl_cdclk_funcs = { .get_cdclk = bxt_get_cdclk, .set_cdclk = bxt_set_cdclk, @@ -3539,14 +3558,17 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; dev_priv->display.cdclk.table = dg2_cdclk_table; } else if (IS_ALDERLAKE_P(dev_priv)) { - dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; /* Wa_22011320316:adl-p[a0] */ - if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) + if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) { dev_priv->display.cdclk.table = adlp_a_step_cdclk_table; - else if (IS_ADLP_RPLU(dev_priv)) + dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; + } else if (IS_ADLP_RPLU(dev_priv)) { dev_priv->display.cdclk.table = rplu_cdclk_table; - else + dev_priv->display.funcs.cdclk = &rplu_cdclk_funcs; + } else { dev_priv->display.cdclk.table = adlp_cdclk_table; + dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; + } } else if (IS_ROCKETLAKE(dev_priv)) { dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; dev_priv->display.cdclk.table = rkl_cdclk_table; -- 2.25.1
[Intel-gfx] [RFC] fbcon: Reschedule cursor worker if try lock fails
With the removal of timer implementation for cursor work[1], the cursor delayed work does not get rescheduled if it fails to acquire the console lock. This change pushes the cursor work to the queue if we fail to acquire the lock. This has been found to cause issue during the resume sequence of the IGT test kms_fbcon_fbt@fbc-suspend[2]. The test expects cursor blinking to start after the system resumes from suspend state. fbcon_resumed() redraw_screen() set_cursor() fb_flashcursor() But the cursor work fails to acuire the lock and it never gets rescheduled unless some other function comes in and invokes fbcon_add_cursor_work() which adds the worker to the queue. >From empirical evidence this happens if we tweak the console log level. Re-scheduling the work heals the issue. I am not sure if this is the best solution we can have, particularly with the "Fix me" comment which indicates a bigger underlying problem. Hence, the RFC. [1] 3b0fb6ab25("fbcon: Use delayed work for cursor") [2] https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13243/shard-glk2/igt@kms_fbcon_...@fbc-suspend.html Signed-off-by: Chaitanya Kumar Borah --- drivers/video/fbdev/core/fbcon.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index c6c9d040bdec..b98ea62836ae 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -357,8 +357,11 @@ static void fb_flashcursor(struct work_struct *work) /* instead we just fail to flash the cursor if we can't get * the lock instead of blocking fbcon deinit */ ret = console_trylock(); - if (ret == 0) + if (ret == 0) { + queue_delayed_work(system_power_efficient_wq, &ops->cursor_work, + ops->cur_blink_jiffies); return; + } /* protected by console_lock */ info = ops->info; -- 2.25.1
[Intel-gfx] [PATCH 1/2] drm/i915/color: Add function to load degamma LUT in MTL
MTL onwards Degamma LUT/PRE-CSC LUT precision has been increased from 16 bits to 24 bits. Currently, drm framework only supports LUTs up to 16 bit precision. Until a new uapi comes along to support higher bitdepth, upscale the values sent from userland to 24 bit before writing into the HW to continue supporting degamma on MTL. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_color.c | 42 -- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 8966e6560516..25c73e2e6fa3 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1498,6 +1498,38 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state, ilk_lut_write(crtc_state, PRE_CSC_GAMC_INDEX(pipe), 0); } +static void mtl_load_degamma_lut(const struct intel_crtc_state *crtc_state, +const struct drm_property_blob *blob) +{ + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); + struct drm_i915_private *i915 = to_i915(crtc->base.dev); + struct drm_color_lut *degamma_lut = blob->data; + enum pipe pipe = crtc->pipe; + int i, lut_size = drm_color_lut_size(blob); + + /* +* When setting the auto-increment bit, the hardware seems to +* ignore the index bits, so we need to reset it to index 0 +* separately. +*/ + intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe), 0); + intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe), + PRE_CSC_GAMC_AUTO_INCREMENT); + + for (i = 0; i < lut_size; i++) { + u64 word = mul_u32_u32(degamma_lut[i].green, (1 << 24)) / (1 << 16); + u32 lut_val = (word & 0xff); + + intel_de_write_fw(i915, PRE_CSC_GAMC_DATA(pipe), + lut_val); + } + /* Clamp values > 1.0. */ + while (i++ < glk_degamma_lut_size(i915)) + intel_de_write_fw(i915, PRE_CSC_GAMC_DATA(pipe), 1 << 24); + + intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe), 0); +} + static void glk_load_luts(const struct intel_crtc_state *crtc_state) { const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut; @@ -1635,11 +1667,17 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state) static void icl_load_luts(const struct intel_crtc_state *crtc_state) { + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); + struct drm_i915_private *i915 = to_i915(crtc->base.dev); const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut; const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut; - if (pre_csc_lut) - glk_load_degamma_lut(crtc_state, pre_csc_lut); + if (pre_csc_lut) { + if (DISPLAY_VER(i915) >= 14) + mtl_load_degamma_lut(crtc_state, pre_csc_lut); + else + glk_load_degamma_lut(crtc_state, pre_csc_lut); + } switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) { case GAMMA_MODE_MODE_8BIT: -- 2.25.1
[Intel-gfx] [PATCH 2/2] drm/i915/color: For MTL convert 24 bit lut values to 16 bit
For MTL and beyond, convert back the 24 bit lut values read from HW to 16 bit values to maintain parity with userspace values. This way we avoid pipe config mismatch for pre-csc lut values. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_color.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 25c73e2e6fa3..856191640e71 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -3477,6 +3477,14 @@ static struct drm_property_blob *glk_read_degamma_lut(struct intel_crtc *crtc) for (i = 0; i < lut_size; i++) { u32 val = intel_de_read_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe)); + /* +* For MTL and beyond, convert back the 24 bit lut values +* read from HW to 16 bit values to maintain parity with +* userspace values +*/ + if (DISPLAY_VER(dev_priv) >= 14) + val = mul_u32_u32(val, (1 << 16)) / (1 << 24); + lut[i].red = val; lut[i].green = val; lut[i].blue = val; -- 2.25.1
[Intel-gfx] [PATCH 0/2] MTL Degamma implementation
MTL onwards Degamma LUT/PRE-CSC LUT precision has been increased from 16 bits to 24 bits. Currently, drm framework only supports LUTs up to 16 bit precision. Until a new uapi comes along to support higher bitdepth, upscale the values sent from userland to 24 bit before writing into the HW to continue supporting degamma on MTL. To avoid pipe config mismatch between 24 bit HW lut values and 16 bit userspace sent values, convert back the 24 bit lut values read from HW to 16 bit values. Chaitanya Kumar Borah (2): drm/i915/color: Add function to load degamma LUT in MTL drm/i915/color: For MTL convert 24 bit lut values to 16 bit drivers/gpu/drm/i915/display/intel_color.c | 50 +- 1 file changed, 48 insertions(+), 2 deletions(-) -- 2.25.1
[Intel-gfx] [PATCH] drm/i915/display: Add header file for struct seq_file
With change [1], visibility of struct seq_file is lost in intel_display_power.h leading to build errors. Add header file explicitly to restore visibility. [1] ef104443bffa ("procfs: consolidate arch_report_meminfo declaration") Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_display_power.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index be1a87bde0c9..888993079a7b 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -6,6 +6,8 @@ #ifndef __INTEL_DISPLAY_POWER_H__ #define __INTEL_DISPLAY_POWER_H__ +#include + #include "intel_wakeref.h" enum aux_ch; -- 2.25.1
[Intel-gfx] [PATCH] drm/i915/display: Add forward declaration for struct seq_file
With change [1], visibility of struct seq_file is lost in intel_display_power.h leading to build errors. Add forward declaration. [1] ef104443bffa ("procfs: consolidate arch_report_meminfo declaration") v2: Use forward declaration instead of headerfile inclusion [Jani] Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_display_power.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index be1a87bde0c9..0ba268e566b0 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -16,6 +16,7 @@ enum port; struct drm_i915_private; struct i915_power_well; struct intel_encoder; +struct seq_file; /* * Keep the pipe, transcoder, port (DDI_LANES,DDI_IO,AUX) domain instances -- 2.25.1
[Intel-gfx] [topic/core-for-CI] Revert "debugfs: annotate debugfs handlers vs. removal with lockdep"
From: Johannes Berg This reverts commit f4acfcd4deb1 ("debugfs: annotate debugfs handlers vs. removal with lockdep"), it appears to have false positives and really shouldn't have been in the -rc series with the fixes anyway. topic/core-for-CI note: cherry-picked from https://patchwork.kernel.org/project/linux-fsdevel/patch/20231202114936.fd55431ab160.I911aa53abeeca138126f690d383a89b13eb05667@changeid/ Signed-off-by: Johannes Berg Acked-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 10 -- fs/debugfs/inode.c| 7 --- fs/debugfs/internal.h | 6 -- 3 files changed, 23 deletions(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index a5ade8c16375..5063434be0fc 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -108,12 +108,6 @@ int debugfs_file_get(struct dentry *dentry) kfree(fsd); fsd = READ_ONCE(dentry->d_fsdata); } -#ifdef CONFIG_LOCKDEP - fsd->lock_name = kasprintf(GFP_KERNEL, "debugfs:%pd", dentry); - lockdep_register_key(&fsd->key); - lockdep_init_map(&fsd->lockdep_map, fsd->lock_name ?: "debugfs", -&fsd->key, 0); -#endif INIT_LIST_HEAD(&fsd->cancellations); mutex_init(&fsd->cancellations_mtx); } @@ -132,8 +126,6 @@ int debugfs_file_get(struct dentry *dentry) if (!refcount_inc_not_zero(&fsd->active_users)) return -EIO; - lock_map_acquire_read(&fsd->lockdep_map); - return 0; } EXPORT_SYMBOL_GPL(debugfs_file_get); @@ -151,8 +143,6 @@ void debugfs_file_put(struct dentry *dentry) { struct debugfs_fsdata *fsd = READ_ONCE(dentry->d_fsdata); - lock_map_release(&fsd->lockdep_map); - if (refcount_dec_and_test(&fsd->active_users)) complete(&fsd->active_users_drained); } diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index e4e7fe1bd9fb..034a617cb1a5 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -243,10 +243,6 @@ static void debugfs_release_dentry(struct dentry *dentry) /* check it wasn't a dir (no fsdata) or automount (no real_fops) */ if (fsd && fsd->real_fops) { -#ifdef CONFIG_LOCKDEP - lockdep_unregister_key(&fsd->key); - kfree(fsd->lock_name); -#endif WARN_ON(!list_empty(&fsd->cancellations)); mutex_destroy(&fsd->cancellations_mtx); } @@ -755,9 +751,6 @@ static void __debugfs_file_removed(struct dentry *dentry) if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) return; - lock_map_acquire(&fsd->lockdep_map); - lock_map_release(&fsd->lockdep_map); - /* if we hit zero, just wait for all to finish */ if (!refcount_dec_and_test(&fsd->active_users)) { wait_for_completion(&fsd->active_users_drained); diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h index 0c4c68cf161f..dae80c2a469e 100644 --- a/fs/debugfs/internal.h +++ b/fs/debugfs/internal.h @@ -7,7 +7,6 @@ #ifndef _DEBUGFS_INTERNAL_H_ #define _DEBUGFS_INTERNAL_H_ -#include #include struct file_operations; @@ -25,11 +24,6 @@ struct debugfs_fsdata { struct { refcount_t active_users; struct completion active_users_drained; -#ifdef CONFIG_LOCKDEP - struct lockdep_map lockdep_map; - struct lock_class_key key; - char *lock_name; -#endif /* protect cancellations */ struct mutex cancellations_mtx; -- 2.25.1
[Intel-gfx] [topic/core-for-CI] Revert "debugfs: annotate debugfs handlers vs. removal with lockdep"
From: Johannes Berg This reverts commit f4acfcd4deb1 ("debugfs: annotate debugfs handlers vs. removal with lockdep"), it appears to have false positives and really shouldn't have been in the -rc series with the fixes anyway. Link:https://patchwork.kernel.org/project/linux-fsdevel/patch/20231202114936.fd55431ab160.I911aa53abeeca138126f690d383a89b13eb05667@changeid/ Reference: https://gitlab.freedesktop.org/drm/intel/-/issues/9802 Signed-off-by: Johannes Berg Acked-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 10 -- fs/debugfs/inode.c| 7 --- fs/debugfs/internal.h | 6 -- 3 files changed, 23 deletions(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index a5ade8c16375..5063434be0fc 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -108,12 +108,6 @@ int debugfs_file_get(struct dentry *dentry) kfree(fsd); fsd = READ_ONCE(dentry->d_fsdata); } -#ifdef CONFIG_LOCKDEP - fsd->lock_name = kasprintf(GFP_KERNEL, "debugfs:%pd", dentry); - lockdep_register_key(&fsd->key); - lockdep_init_map(&fsd->lockdep_map, fsd->lock_name ?: "debugfs", -&fsd->key, 0); -#endif INIT_LIST_HEAD(&fsd->cancellations); mutex_init(&fsd->cancellations_mtx); } @@ -132,8 +126,6 @@ int debugfs_file_get(struct dentry *dentry) if (!refcount_inc_not_zero(&fsd->active_users)) return -EIO; - lock_map_acquire_read(&fsd->lockdep_map); - return 0; } EXPORT_SYMBOL_GPL(debugfs_file_get); @@ -151,8 +143,6 @@ void debugfs_file_put(struct dentry *dentry) { struct debugfs_fsdata *fsd = READ_ONCE(dentry->d_fsdata); - lock_map_release(&fsd->lockdep_map); - if (refcount_dec_and_test(&fsd->active_users)) complete(&fsd->active_users_drained); } diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index e4e7fe1bd9fb..034a617cb1a5 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -243,10 +243,6 @@ static void debugfs_release_dentry(struct dentry *dentry) /* check it wasn't a dir (no fsdata) or automount (no real_fops) */ if (fsd && fsd->real_fops) { -#ifdef CONFIG_LOCKDEP - lockdep_unregister_key(&fsd->key); - kfree(fsd->lock_name); -#endif WARN_ON(!list_empty(&fsd->cancellations)); mutex_destroy(&fsd->cancellations_mtx); } @@ -755,9 +751,6 @@ static void __debugfs_file_removed(struct dentry *dentry) if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) return; - lock_map_acquire(&fsd->lockdep_map); - lock_map_release(&fsd->lockdep_map); - /* if we hit zero, just wait for all to finish */ if (!refcount_dec_and_test(&fsd->active_users)) { wait_for_completion(&fsd->active_users_drained); diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h index 0c4c68cf161f..dae80c2a469e 100644 --- a/fs/debugfs/internal.h +++ b/fs/debugfs/internal.h @@ -7,7 +7,6 @@ #ifndef _DEBUGFS_INTERNAL_H_ #define _DEBUGFS_INTERNAL_H_ -#include #include struct file_operations; @@ -25,11 +24,6 @@ struct debugfs_fsdata { struct { refcount_t active_users; struct completion active_users_drained; -#ifdef CONFIG_LOCKDEP - struct lockdep_map lockdep_map; - struct lock_class_key key; - char *lock_name; -#endif /* protect cancellations */ struct mutex cancellations_mtx; -- 2.25.1
[PATCH 1/5] 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. This patch is slightly reworked by Maarten Cc: Maarten Lankhorst Signed-off-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/display/intel_cursor.c | 26 +-- drivers/gpu/drm/i915/display/intel_display.c | 3 +++ .../drm/i915/display/intel_display_types.h| 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index f8b33999d43f..9021c0c1683d 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -654,6 +654,17 @@ 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(&plane->base, &plane_state->uapi); +} + static int intel_legacy_cursor_update(struct drm_plane *_plane, struct drm_crtc *_crtc, @@ -797,14 +808,25 @@ intel_legacy_cursor_update(struct drm_plane *_plane, intel_psr_unlock(crtc_state); - intel_plane_unpin_fb(old_plane_state); + if (old_plane_state->ggtt_vma != new_plane_state->ggtt_vma) { + drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base, +intel_cursor_unpin_work); + + drm_vblank_work_schedule(&old_plane_state->unpin_work, + drm_crtc_accurate_vblank_count(&crtc->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(&crtc->base, &new_crtc_state->uapi); if (ret) intel_plane_destroy_state(&plane->base, &new_plane_state->uapi); - else + else if (old_plane_state) intel_plane_destroy_state(&plane->base, &old_plane_state->uapi); return ret; diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 7fb0f71652ac..bf684c4d1732 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -64,6 +64,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" @@ -6780,6 +6781,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) continue; intel_crtc_disable_planes(state, crtc); + + drm_vblank_work_flush_all(&crtc->base); } /* 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 ae2e8cff9d69..5fdb9eccab5a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -713,6 +713,9 @@ struct intel_plane_state { struct intel_fb_view view; + /* for legacy cursor fb unpin */ + struct drm_vblank_work unpin_work; + /* Plane pxp decryption state */ bool decrypt; -- 2.25.1
[RFC PATCH 0/5] Cursor Fault Fixes
This series is based on top of [1] floated by Maarten. To fix, regressions seen in CI following changes were made on top of the original series. 1. add a check in the plane state destroy hook to not move forward if the vblank worker is scheduled. 2. add checks before accessing frame buffer object (we are not sure yet how much this helps but we have found that this operation causes dump stacks) 3. do not defer the intel atomic cleanup into a work queue, instead execute it at the end of atomic commit tail. While this series is in noway a complete or proper solution it is meant to trigger a discussion to arrive at one. [1] https://patchwork.freedesktop.org/series/126934/ Chaitanya Kumar Borah (3): drm/i915: do not destroy plane state if cursor unpin worker is scheduled drm/i915: Add sanity checks before accessing fb buffer object drm/i915: do not defer cleanup work Maarten Lankhorst (1): drm/i915: Use the same vblank worker for atomic unpin Ville Syrjälä (1): drm/i915: Use vblank worker to unpin old legacy cursor fb safely .../gpu/drm/i915/display/intel_atomic_plane.c | 55 ++- .../gpu/drm/i915/display/intel_atomic_plane.h | 4 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 27 + drivers/gpu/drm/i915/display/intel_cursor.c | 26 - drivers/gpu/drm/i915/display/intel_cursor.h | 3 + drivers/gpu/drm/i915/display/intel_display.c | 11 ++-- .../drm/i915/display/intel_display_types.h| 3 + 7 files changed, 120 insertions(+), 9 deletions(-) -- 2.25.1
[PATCH 3/5] drm/i915: do not destroy plane state if cursor unpin worker is scheduled
The plane destroy hook can be called asynchronously even when vblank worker responsible for unpinning the cursor fb is scheduled. Since the vblank worker destroys the plane state, do not destroy the plane state if it is scheduled. Signed-off-by: Chaitanya Kumar Borah --- .../gpu/drm/i915/display/intel_atomic_plane.c | 19 +++ .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 06c5d8262443..a585e4aca309 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -136,6 +136,25 @@ intel_plane_destroy_state(struct drm_plane *plane, { struct intel_plane_state *plane_state = to_intel_plane_state(state); + /* Do not proceed if vblank unpin worker is yet to be executed */ + if (plane_state->unpin_work.vblank) + return; + + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); + drm_WARN_ON(plane->dev, plane_state->dpt_vma); + + __drm_atomic_helper_plane_destroy_state(&plane_state->uapi); + if (plane_state->hw.fb) + drm_framebuffer_put(plane_state->hw.fb); + kfree(plane_state); +} + +void +intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct intel_plane_state *plane_state = to_intel_plane_state(state); + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); drm_WARN_ON(plane->dev, plane_state->dpt_vma); diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 5a897cf6fa02..1e165b709a80 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -45,6 +45,8 @@ void intel_plane_free(struct intel_plane *plane); struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); void intel_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); +void intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state); void intel_crtc_planes_update_noarm(struct intel_atomic_state *state, struct intel_crtc *crtc); void intel_crtc_planes_update_arm(struct intel_atomic_state *state, diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index dbb26a212800..32f9bb753331 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -662,7 +662,7 @@ void intel_cursor_unpin_work(struct kthread_work *base) struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); intel_plane_unpin_fb(plane_state); - intel_plane_destroy_state(&plane->base, &plane_state->uapi); + intel_cursor_destroy_state(&plane->base, &plane_state->uapi); } static int -- 2.25.1
[PATCH 4/5] drm/i915: Add sanity checks before accessing fb buffer object
Now that cursor plane fb unpinning can be deferred to vblank work check if plane state and corresponding fb pointers are still valid before cleanup. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index a585e4aca309..a0217fef9920 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -1171,12 +1171,18 @@ static void intel_cleanup_plane_fb(struct drm_plane *plane, struct drm_plane_state *_old_plane_state) { + struct drm_i915_private *i915 = to_i915(_old_plane_state->plane->dev); struct intel_plane_state *old_plane_state = to_intel_plane_state(_old_plane_state); struct intel_atomic_state *state = to_intel_atomic_state(old_plane_state->uapi.state); struct drm_i915_private *dev_priv = to_i915(plane->dev); - struct drm_i915_gem_object *obj = intel_fb_obj(old_plane_state->hw.fb); + struct drm_i915_gem_object *obj = NULL; + + if (old_plane_state && old_plane_state->hw.fb) + obj = intel_fb_obj(old_plane_state->hw.fb); + else + drm_err_ratelimited(&i915->drm, "Invalid plane state or fb\n"); if (!obj) return; -- 2.25.1
[PATCH 5/5] drm/i915: do not defer cleanup work
After we move the cursor fb unpin to a vblank work, we encounter race conditions between the vblank work and the atomic clean up work leading to dump stacks[1]. Let's serialize the clean up to avoid theses races. [1] [ 278.748767] Workqueue: events_highpri intel_atomic_cleanup_work [i915] [ 278.749115] RIP: 0010:intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749425] Code: 92 cb 20 e1 e9 49 ff ff ff 5b 48 89 ef 5d 41 5c e9 11 23 44 e1 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 <38> 96 a5 05 00 00 74 2a 55 48 89 f5 0f b6 f2 53 48 8b bf 40 37 00 [ 278.749428] RSP: 0018:c929fdc8 EFLAGS: 00010246 [ 278.749433] RAX: 0060 RBX: RCX: [ 278.749435] RDX: RSI: RDI: 888124d7 [ 278.749438] RBP: 88810394c000 R08: R09: c929fc80 [ 278.749441] R10: 00f6d950 R11: 00f6da18 R12: 888124d7 [ 278.749443] R13: 88814c952000 R14: 8881000aac05 R15: 8881059baf10 [ 278.749446] FS: () GS:88817bd8() knlGS: [ 278.749449] CS: 0010 DS: ES: CR0: 80050033 [ 278.749452] CR2: 05a5 CR3: 000104078000 CR4: 00350ef0 [ 278.749454] Call Trace: [ 278.749458] [ 278.749461] ? __die_body+0x1a/0x60 [ 278.749469] ? page_fault_oops+0x156/0x450 [ 278.749474] ? do_user_addr_fault+0x65/0x9e0 [ 278.749479] ? exc_page_fault+0x68/0x1a0 [ 278.749486] ? asm_exc_page_fault+0x26/0x30 [ 278.749494] ? intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749802] intel_cleanup_plane_fb+0x6f/0xc0 [i915] [ 278.750114] drm_atomic_helper_cleanup_planes+0x42/0x60 [ 278.750122] intel_atomic_cleanup_work+0x70/0xc0 [i915] [ 278.750433] ? process_scheduled_works+0x264/0x530 [ 278.750438] process_scheduled_works+0x2db/0x530 [ 278.750444] ? __pfx_worker_thread+0x10/0x10 [ 278.750448] worker_thread+0x18c/0x350 [ 278.750452] ? __pfx_worker_thread+0x10/0x10 [ 278.750455] kthread+0xfe/0x130 [ 278.750460] ? __pfx_kthread+0x10/0x10 [ 278.750464] ret_from_fork+0x2c/0x50 [ 278.750468] ? __pfx_kthread+0x10/0x10 [ 278.750472] ret_from_fork_asm+0x1b/0x30 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_display.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index bf684c4d1732..b0e89036508e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7006,10 +7006,8 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat } } -static void intel_atomic_cleanup_work(struct work_struct *work) +static void intel_atomic_cleanup_work(struct intel_atomic_state *state) { - struct intel_atomic_state *state = - container_of(work, struct intel_atomic_state, base.commit_work); struct drm_i915_private *i915 = to_i915(state->base.dev); struct intel_crtc_state *old_crtc_state; struct intel_crtc *crtc; @@ -7283,8 +7281,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) * schedule point (cond_resched()) here anyway to keep latencies * down. */ - INIT_WORK(&state->base.commit_work, intel_atomic_cleanup_work); - queue_work(system_highpri_wq, &state->base.commit_work); + + intel_atomic_cleanup_work(state); } static void intel_atomic_commit_work(struct work_struct *work) -- 2.25.1
[PATCH 2/5] drm/i915: Use the same vblank worker for atomic unpin
From: Maarten Lankhorst In case of legacy cursor update, the cursor VMA needs to be unpinned only after vblank. This exceeds the lifetime of the whole atomic commit. Any trick I attempted to keep the atomic commit alive didn't work, as drm_atomic_helper_setup_commit() force throttles on any old commit that wasn't cleaned up. The only option remaining is to remove the plane from the atomic commit, and use the same path as the legacy cursor update to clean the state after vblank. Signed-off-by: Maarten Lankhorst --- .../gpu/drm/i915/display/intel_atomic_plane.c | 28 ++- .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 27 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- drivers/gpu/drm/i915/display/intel_cursor.h | 3 ++ 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 76d77d5a0409..06c5d8262443 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -42,6 +42,7 @@ #include "i915_reg.h" #include "intel_atomic_plane.h" #include "intel_cdclk.h" +#include "intel_cursor.h" #include "intel_display_rps.h" #include "intel_display_trace.h" #include "intel_display_types.h" @@ -1163,7 +1164,21 @@ intel_cleanup_plane_fb(struct drm_plane *plane, intel_display_rps_mark_interactive(dev_priv, state, false); - /* Should only be called after a successful intel_prepare_plane_fb()! */ + /* +* This branch can only ever be called after plane update is succesful, +* the error path will not cause unpin_work to be set. +*/ + if (old_plane_state->unpin_work.vblank) { + int i = drm_plane_index(old_plane_state->uapi.plane); + + /* +* Remove plane from atomic commit, +* free is done from vblank worker +*/ + memset(&state->base.planes[i], 0, sizeof(*state->base.planes)); + return; + } + intel_plane_unpin_fb(old_plane_state); } @@ -1176,3 +1191,14 @@ void intel_plane_helper_add(struct intel_plane *plane) { drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs); } + +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state) +{ + if (!old_plane_state->ggtt_vma || + old_plane_state->ggtt_vma == new_plane_state->ggtt_vma) + return; + + drm_vblank_work_init(&old_plane_state->unpin_work, old_plane_state->uapi.crtc, +intel_cursor_unpin_work); +} diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 191dad0efc8e..5a897cf6fa02 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -66,5 +66,7 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state); void intel_plane_set_invisible(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); void intel_plane_helper_add(struct intel_plane *plane); +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state); #endif /* __INTEL_ATOMIC_PLANE_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index 25593f6aae7d..a7fb7f5ace07 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -501,6 +501,18 @@ void intel_pipe_update_start(struct intel_atomic_state *state, intel_crtc_vblank_work_init(new_crtc_state); intel_vblank_evade_init(old_crtc_state, new_crtc_state, &evade); + if (state->base.legacy_cursor_update) { + struct intel_plane *plane; + struct intel_plane_state *old_plane_state, *new_plane_state; + int i; + + for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state, +new_plane_state, i) { + if (old_plane_state->uapi.crtc == &crtc->base) + intel_plane_init_cursor_vblank_work(old_plane_state, + new_plane_state); + } + } if (drm_WARN_ON(&dev_priv->drm, drm_crtc_vblank_get(&crtc->base))) goto irq_disable; @@ -616,6 +628,21 @@ void intel_pipe_update_end(struct intel_atomic_state *state, new_crtc_state->uapi.event = NULL; } + if (state->base.legacy_cursor_update) { + struc
[PATCH 3/6] drm/i915: Use the same vblank worker for atomic unpin
From: Maarten Lankhorst In case of legacy cursor update, the cursor VMA needs to be unpinned only after vblank. This exceeds the lifetime of the whole atomic commit. Any trick I attempted to keep the atomic commit alive didn't work, as drm_atomic_helper_setup_commit() force throttles on any old commit that wasn't cleaned up. The only option remaining is to remove the plane from the atomic commit, and use the same path as the legacy cursor update to clean the state after vblank. Signed-off-by: Maarten Lankhorst --- .../gpu/drm/i915/display/intel_atomic_plane.c | 28 ++- .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 27 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- drivers/gpu/drm/i915/display/intel_cursor.h | 3 ++ 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 76d77d5a0409..06c5d8262443 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -42,6 +42,7 @@ #include "i915_reg.h" #include "intel_atomic_plane.h" #include "intel_cdclk.h" +#include "intel_cursor.h" #include "intel_display_rps.h" #include "intel_display_trace.h" #include "intel_display_types.h" @@ -1163,7 +1164,21 @@ intel_cleanup_plane_fb(struct drm_plane *plane, intel_display_rps_mark_interactive(dev_priv, state, false); - /* Should only be called after a successful intel_prepare_plane_fb()! */ + /* +* This branch can only ever be called after plane update is succesful, +* the error path will not cause unpin_work to be set. +*/ + if (old_plane_state->unpin_work.vblank) { + int i = drm_plane_index(old_plane_state->uapi.plane); + + /* +* Remove plane from atomic commit, +* free is done from vblank worker +*/ + memset(&state->base.planes[i], 0, sizeof(*state->base.planes)); + return; + } + intel_plane_unpin_fb(old_plane_state); } @@ -1176,3 +1191,14 @@ void intel_plane_helper_add(struct intel_plane *plane) { drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs); } + +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state) +{ + if (!old_plane_state->ggtt_vma || + old_plane_state->ggtt_vma == new_plane_state->ggtt_vma) + return; + + drm_vblank_work_init(&old_plane_state->unpin_work, old_plane_state->uapi.crtc, +intel_cursor_unpin_work); +} diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 191dad0efc8e..5a897cf6fa02 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -66,5 +66,7 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state); void intel_plane_set_invisible(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); void intel_plane_helper_add(struct intel_plane *plane); +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state); #endif /* __INTEL_ATOMIC_PLANE_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index 25593f6aae7d..a7fb7f5ace07 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -501,6 +501,18 @@ void intel_pipe_update_start(struct intel_atomic_state *state, intel_crtc_vblank_work_init(new_crtc_state); intel_vblank_evade_init(old_crtc_state, new_crtc_state, &evade); + if (state->base.legacy_cursor_update) { + struct intel_plane *plane; + struct intel_plane_state *old_plane_state, *new_plane_state; + int i; + + for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state, +new_plane_state, i) { + if (old_plane_state->uapi.crtc == &crtc->base) + intel_plane_init_cursor_vblank_work(old_plane_state, + new_plane_state); + } + } if (drm_WARN_ON(&dev_priv->drm, drm_crtc_vblank_get(&crtc->base))) goto irq_disable; @@ -616,6 +628,21 @@ void intel_pipe_update_end(struct intel_atomic_state *state, new_crtc_state->uapi.event = NULL; } + if (state->base.legacy_cursor_update) { + struc
[PATCH 4/6] drm/i915: do not destroy plane state if cursor unpin worker is scheduled
The plane destroy hook can be called asynchronously even when vblank worker responsible for unpinning the cursor fb is scheduled. Since the vblank worker destroys the plane state, do not destroy the plane state if it is scheduled. Signed-off-by: Chaitanya Kumar Borah --- .../gpu/drm/i915/display/intel_atomic_plane.c | 19 +++ .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 06c5d8262443..a585e4aca309 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -136,6 +136,25 @@ intel_plane_destroy_state(struct drm_plane *plane, { struct intel_plane_state *plane_state = to_intel_plane_state(state); + /* Do not proceed if vblank unpin worker is yet to be executed */ + if (plane_state->unpin_work.vblank) + return; + + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); + drm_WARN_ON(plane->dev, plane_state->dpt_vma); + + __drm_atomic_helper_plane_destroy_state(&plane_state->uapi); + if (plane_state->hw.fb) + drm_framebuffer_put(plane_state->hw.fb); + kfree(plane_state); +} + +void +intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct intel_plane_state *plane_state = to_intel_plane_state(state); + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); drm_WARN_ON(plane->dev, plane_state->dpt_vma); diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 5a897cf6fa02..1e165b709a80 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -45,6 +45,8 @@ void intel_plane_free(struct intel_plane *plane); struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); void intel_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); +void intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state); void intel_crtc_planes_update_noarm(struct intel_atomic_state *state, struct intel_crtc *crtc); void intel_crtc_planes_update_arm(struct intel_atomic_state *state, diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index dbb26a212800..32f9bb753331 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -662,7 +662,7 @@ void intel_cursor_unpin_work(struct kthread_work *base) struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); intel_plane_unpin_fb(plane_state); - intel_plane_destroy_state(&plane->base, &plane_state->uapi); + intel_cursor_destroy_state(&plane->base, &plane_state->uapi); } static int -- 2.25.1
[PATCH 5/6] drm/i915: Add sanity checks before accessing fb buffer object
Now that cursor plane fb unpinning can be deferred to vblank work check if plane state and corresponding fb pointers are still valid before cleanup. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index a585e4aca309..a0217fef9920 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -1171,12 +1171,18 @@ static void intel_cleanup_plane_fb(struct drm_plane *plane, struct drm_plane_state *_old_plane_state) { + struct drm_i915_private *i915 = to_i915(_old_plane_state->plane->dev); struct intel_plane_state *old_plane_state = to_intel_plane_state(_old_plane_state); struct intel_atomic_state *state = to_intel_atomic_state(old_plane_state->uapi.state); struct drm_i915_private *dev_priv = to_i915(plane->dev); - struct drm_i915_gem_object *obj = intel_fb_obj(old_plane_state->hw.fb); + struct drm_i915_gem_object *obj = NULL; + + if (old_plane_state && old_plane_state->hw.fb) + obj = intel_fb_obj(old_plane_state->hw.fb); + else + drm_err_ratelimited(&i915->drm, "Invalid plane state or fb\n"); if (!obj) return; -- 2.25.1
[PATCH 1/6] drm: Add drm_vblank_work_flush_all().
From: Maarten Lankhorst In some cases we want to flush all vblank work, right before vblank_off for example. Add a simple function to make this possible. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/drm_vblank_work.c | 22 ++ include/drm/drm_vblank_work.h | 2 ++ 2 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c index 43cd5c0f4f6f..ff86f2b2e052 100644 --- a/drivers/gpu/drm/drm_vblank_work.c +++ b/drivers/gpu/drm/drm_vblank_work.c @@ -232,6 +232,28 @@ void drm_vblank_work_flush(struct drm_vblank_work *work) } EXPORT_SYMBOL(drm_vblank_work_flush); +/** + * drm_vblank_work_flush_all - flush all currently pending vblank work on crtc. + * @crtc: crtc for which vblank work to flush + * + * Wait until all currently queued vblank work on @crtc + * has finished executing once. + */ +void drm_vblank_work_flush_all(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_vblank_crtc *vblank = &dev->vblank[drm_crtc_index(crtc)]; + + spin_lock_irq(&dev->event_lock); + wait_event_lock_irq(vblank->work_wait_queue, + waitqueue_active(&vblank->work_wait_queue), + dev->event_lock); + spin_unlock_irq(&dev->event_lock); + + kthread_flush_worker(vblank->worker); +} +EXPORT_SYMBOL(drm_vblank_work_flush_all); + /** * drm_vblank_work_init - initialize a vblank work item * @work: vblank work item diff --git a/include/drm/drm_vblank_work.h b/include/drm/drm_vblank_work.h index eb41d0810c4f..e04d436b7297 100644 --- a/include/drm/drm_vblank_work.h +++ b/include/drm/drm_vblank_work.h @@ -17,6 +17,7 @@ struct drm_crtc; * drm_vblank_work_init() * drm_vblank_work_cancel_sync() * drm_vblank_work_flush() + * drm_vblank_work_flush_all() */ struct drm_vblank_work { /** @@ -67,5 +68,6 @@ void drm_vblank_work_init(struct drm_vblank_work *work, struct drm_crtc *crtc, void (*func)(struct kthread_work *work)); bool drm_vblank_work_cancel_sync(struct drm_vblank_work *work); void drm_vblank_work_flush(struct drm_vblank_work *work); +void drm_vblank_work_flush_all(struct drm_crtc *crtc); #endif /* !_DRM_VBLANK_WORK_H_ */ -- 2.25.1
[PATCH 2/6] 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. This patch is slightly reworked by Maarten Cc: Maarten Lankhorst Signed-off-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/display/intel_cursor.c | 26 +-- drivers/gpu/drm/i915/display/intel_display.c | 3 +++ .../drm/i915/display/intel_display_types.h| 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index f8b33999d43f..9021c0c1683d 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -654,6 +654,17 @@ 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(&plane->base, &plane_state->uapi); +} + static int intel_legacy_cursor_update(struct drm_plane *_plane, struct drm_crtc *_crtc, @@ -797,14 +808,25 @@ intel_legacy_cursor_update(struct drm_plane *_plane, intel_psr_unlock(crtc_state); - intel_plane_unpin_fb(old_plane_state); + if (old_plane_state->ggtt_vma != new_plane_state->ggtt_vma) { + drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base, +intel_cursor_unpin_work); + + drm_vblank_work_schedule(&old_plane_state->unpin_work, + drm_crtc_accurate_vblank_count(&crtc->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(&crtc->base, &new_crtc_state->uapi); if (ret) intel_plane_destroy_state(&plane->base, &new_plane_state->uapi); - else + else if (old_plane_state) intel_plane_destroy_state(&plane->base, &old_plane_state->uapi); return ret; diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 7fb0f71652ac..bf684c4d1732 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -64,6 +64,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" @@ -6780,6 +6781,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) continue; intel_crtc_disable_planes(state, crtc); + + drm_vblank_work_flush_all(&crtc->base); } /* 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 ae2e8cff9d69..5fdb9eccab5a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -713,6 +713,9 @@ struct intel_plane_state { struct intel_fb_view view; + /* for legacy cursor fb unpin */ + struct drm_vblank_work unpin_work; + /* Plane pxp decryption state */ bool decrypt; -- 2.25.1
[PATCH 6/6] drm/i915: do not defer cleanup work
After we move the cursor fb unpin to a vblank work, we encounter race conditions between the vblank work and the atomic clean up work leading to dump stacks[1]. Let's serialize the clean up to avoid theses races. [1] [ 278.748767] Workqueue: events_highpri intel_atomic_cleanup_work [i915] [ 278.749115] RIP: 0010:intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749425] Code: 92 cb 20 e1 e9 49 ff ff ff 5b 48 89 ef 5d 41 5c e9 11 23 44 e1 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 <38> 96 a5 05 00 00 74 2a 55 48 89 f5 0f b6 f2 53 48 8b bf 40 37 00 [ 278.749428] RSP: 0018:c929fdc8 EFLAGS: 00010246 [ 278.749433] RAX: 0060 RBX: RCX: [ 278.749435] RDX: RSI: RDI: 888124d7 [ 278.749438] RBP: 88810394c000 R08: R09: c929fc80 [ 278.749441] R10: 00f6d950 R11: 00f6da18 R12: 888124d7 [ 278.749443] R13: 88814c952000 R14: 8881000aac05 R15: 8881059baf10 [ 278.749446] FS: () GS:88817bd8() knlGS: [ 278.749449] CS: 0010 DS: ES: CR0: 80050033 [ 278.749452] CR2: 05a5 CR3: 000104078000 CR4: 00350ef0 [ 278.749454] Call Trace: [ 278.749458] [ 278.749461] ? __die_body+0x1a/0x60 [ 278.749469] ? page_fault_oops+0x156/0x450 [ 278.749474] ? do_user_addr_fault+0x65/0x9e0 [ 278.749479] ? exc_page_fault+0x68/0x1a0 [ 278.749486] ? asm_exc_page_fault+0x26/0x30 [ 278.749494] ? intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749802] intel_cleanup_plane_fb+0x6f/0xc0 [i915] [ 278.750114] drm_atomic_helper_cleanup_planes+0x42/0x60 [ 278.750122] intel_atomic_cleanup_work+0x70/0xc0 [i915] [ 278.750433] ? process_scheduled_works+0x264/0x530 [ 278.750438] process_scheduled_works+0x2db/0x530 [ 278.750444] ? __pfx_worker_thread+0x10/0x10 [ 278.750448] worker_thread+0x18c/0x350 [ 278.750452] ? __pfx_worker_thread+0x10/0x10 [ 278.750455] kthread+0xfe/0x130 [ 278.750460] ? __pfx_kthread+0x10/0x10 [ 278.750464] ret_from_fork+0x2c/0x50 [ 278.750468] ? __pfx_kthread+0x10/0x10 [ 278.750472] ret_from_fork_asm+0x1b/0x30 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_display.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index bf684c4d1732..b0e89036508e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7006,10 +7006,8 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat } } -static void intel_atomic_cleanup_work(struct work_struct *work) +static void intel_atomic_cleanup_work(struct intel_atomic_state *state) { - struct intel_atomic_state *state = - container_of(work, struct intel_atomic_state, base.commit_work); struct drm_i915_private *i915 = to_i915(state->base.dev); struct intel_crtc_state *old_crtc_state; struct intel_crtc *crtc; @@ -7283,8 +7281,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) * schedule point (cond_resched()) here anyway to keep latencies * down. */ - INIT_WORK(&state->base.commit_work, intel_atomic_cleanup_work); - queue_work(system_highpri_wq, &state->base.commit_work); + + intel_atomic_cleanup_work(state); } static void intel_atomic_commit_work(struct work_struct *work) -- 2.25.1
[PATCH 0/6] Cursor Fault Fixes
This series is based on top of [1] floated by Maarten. To fix regressions seen in CI, following changes were made on top of the original series. 1. add a check in the plane state destroy hook to not move forward if the vblank worker is scheduled. 2. add checks before accessing frame buffer object (we are not sure yet how much this helps but we have found that this operation causes dump stacks) 3. do not defer the intel atomic cleanup into a work queue, instead execute it at the end of atomic commit tail. While this series is in noway a complete or proper solution it is meant to trigger a discussion to arrive at one. [1] https://patchwork.freedesktop.org/series/126934/ v2: Add missing patch Chaitanya Kumar Borah (3): drm/i915: do not destroy plane state if cursor unpin worker is scheduled drm/i915: Add sanity checks before accessing fb buffer object drm/i915: do not defer cleanup work Maarten Lankhorst (2): drm: Add drm_vblank_work_flush_all(). drm/i915: Use the same vblank worker for atomic unpin Ville Syrjälä (1): drm/i915: Use vblank worker to unpin old legacy cursor fb safely drivers/gpu/drm/drm_vblank_work.c | 22 .../gpu/drm/i915/display/intel_atomic_plane.c | 55 ++- .../gpu/drm/i915/display/intel_atomic_plane.h | 4 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 27 + drivers/gpu/drm/i915/display/intel_cursor.c | 26 - drivers/gpu/drm/i915/display/intel_cursor.h | 3 + drivers/gpu/drm/i915/display/intel_display.c | 11 ++-- .../drm/i915/display/intel_display_types.h| 3 + include/drm/drm_vblank_work.h | 2 + 9 files changed, 144 insertions(+), 9 deletions(-) -- 2.25.1
[PATCH 0/6] Cursor Fault Fixes
This series is based on top of [1] floated by Maarten. To fix regressions seen in CI, following changes were made on top of the original series. 1. add a check in the plane state destroy hook to not move forward if the vblank worker is scheduled. 2. add checks before accessing frame buffer object (we are not sure yet how much this helps but we have found that this operation causes dump stacks) 3. do not defer the intel atomic cleanup into a work queue, instead execute it at the end of atomic commit tail. While this series is in noway a complete or proper solution it is meant to trigger a discussion to arrive at one. [1] https://patchwork.freedesktop.org/series/126934/ v2: Add missing patch v3: Remove misleading error log Change condition to access fb object Chaitanya Kumar Borah (3): drm/i915: do not destroy plane state if cursor unpin worker is scheduled drm/i915: Add sanity check before accessing fb buffer object drm/i915: do not defer cleanup work Maarten Lankhorst (2): drm: Add drm_vblank_work_flush_all(). drm/i915: Use the same vblank worker for atomic unpin Ville Syrjälä (1): drm/i915: Use vblank worker to unpin old legacy cursor fb safely drivers/gpu/drm/drm_vblank_work.c | 22 .../gpu/drm/i915/display/intel_atomic_plane.c | 53 ++- .../gpu/drm/i915/display/intel_atomic_plane.h | 4 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 27 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 26 - drivers/gpu/drm/i915/display/intel_cursor.h | 3 ++ drivers/gpu/drm/i915/display/intel_display.c | 11 ++-- .../drm/i915/display/intel_display_types.h| 3 ++ include/drm/drm_vblank_work.h | 2 + 9 files changed, 142 insertions(+), 9 deletions(-) -- 2.25.1
[PATCH 1/6] drm: Add drm_vblank_work_flush_all().
From: Maarten Lankhorst In some cases we want to flush all vblank work, right before vblank_off for example. Add a simple function to make this possible. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/drm_vblank_work.c | 22 ++ include/drm/drm_vblank_work.h | 2 ++ 2 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c index 43cd5c0f4f6f..ff86f2b2e052 100644 --- a/drivers/gpu/drm/drm_vblank_work.c +++ b/drivers/gpu/drm/drm_vblank_work.c @@ -232,6 +232,28 @@ void drm_vblank_work_flush(struct drm_vblank_work *work) } EXPORT_SYMBOL(drm_vblank_work_flush); +/** + * drm_vblank_work_flush_all - flush all currently pending vblank work on crtc. + * @crtc: crtc for which vblank work to flush + * + * Wait until all currently queued vblank work on @crtc + * has finished executing once. + */ +void drm_vblank_work_flush_all(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_vblank_crtc *vblank = &dev->vblank[drm_crtc_index(crtc)]; + + spin_lock_irq(&dev->event_lock); + wait_event_lock_irq(vblank->work_wait_queue, + waitqueue_active(&vblank->work_wait_queue), + dev->event_lock); + spin_unlock_irq(&dev->event_lock); + + kthread_flush_worker(vblank->worker); +} +EXPORT_SYMBOL(drm_vblank_work_flush_all); + /** * drm_vblank_work_init - initialize a vblank work item * @work: vblank work item diff --git a/include/drm/drm_vblank_work.h b/include/drm/drm_vblank_work.h index eb41d0810c4f..e04d436b7297 100644 --- a/include/drm/drm_vblank_work.h +++ b/include/drm/drm_vblank_work.h @@ -17,6 +17,7 @@ struct drm_crtc; * drm_vblank_work_init() * drm_vblank_work_cancel_sync() * drm_vblank_work_flush() + * drm_vblank_work_flush_all() */ struct drm_vblank_work { /** @@ -67,5 +68,6 @@ void drm_vblank_work_init(struct drm_vblank_work *work, struct drm_crtc *crtc, void (*func)(struct kthread_work *work)); bool drm_vblank_work_cancel_sync(struct drm_vblank_work *work); void drm_vblank_work_flush(struct drm_vblank_work *work); +void drm_vblank_work_flush_all(struct drm_crtc *crtc); #endif /* !_DRM_VBLANK_WORK_H_ */ -- 2.25.1
[PATCH 2/6] 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. This patch is slightly reworked by Maarten Cc: Maarten Lankhorst Signed-off-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/display/intel_cursor.c | 26 +-- drivers/gpu/drm/i915/display/intel_display.c | 3 +++ .../drm/i915/display/intel_display_types.h| 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index f8b33999d43f..9021c0c1683d 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -654,6 +654,17 @@ 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(&plane->base, &plane_state->uapi); +} + static int intel_legacy_cursor_update(struct drm_plane *_plane, struct drm_crtc *_crtc, @@ -797,14 +808,25 @@ intel_legacy_cursor_update(struct drm_plane *_plane, intel_psr_unlock(crtc_state); - intel_plane_unpin_fb(old_plane_state); + if (old_plane_state->ggtt_vma != new_plane_state->ggtt_vma) { + drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base, +intel_cursor_unpin_work); + + drm_vblank_work_schedule(&old_plane_state->unpin_work, + drm_crtc_accurate_vblank_count(&crtc->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(&crtc->base, &new_crtc_state->uapi); if (ret) intel_plane_destroy_state(&plane->base, &new_plane_state->uapi); - else + else if (old_plane_state) intel_plane_destroy_state(&plane->base, &old_plane_state->uapi); return ret; diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 7fb0f71652ac..bf684c4d1732 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -64,6 +64,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" @@ -6780,6 +6781,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) continue; intel_crtc_disable_planes(state, crtc); + + drm_vblank_work_flush_all(&crtc->base); } /* 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 ae2e8cff9d69..5fdb9eccab5a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -713,6 +713,9 @@ struct intel_plane_state { struct intel_fb_view view; + /* for legacy cursor fb unpin */ + struct drm_vblank_work unpin_work; + /* Plane pxp decryption state */ bool decrypt; -- 2.25.1
[PATCH 3/6] drm/i915: Use the same vblank worker for atomic unpin
From: Maarten Lankhorst In case of legacy cursor update, the cursor VMA needs to be unpinned only after vblank. This exceeds the lifetime of the whole atomic commit. Any trick I attempted to keep the atomic commit alive didn't work, as drm_atomic_helper_setup_commit() force throttles on any old commit that wasn't cleaned up. The only option remaining is to remove the plane from the atomic commit, and use the same path as the legacy cursor update to clean the state after vblank. Signed-off-by: Maarten Lankhorst --- .../gpu/drm/i915/display/intel_atomic_plane.c | 28 ++- .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 27 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- drivers/gpu/drm/i915/display/intel_cursor.h | 3 ++ 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 76d77d5a0409..06c5d8262443 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -42,6 +42,7 @@ #include "i915_reg.h" #include "intel_atomic_plane.h" #include "intel_cdclk.h" +#include "intel_cursor.h" #include "intel_display_rps.h" #include "intel_display_trace.h" #include "intel_display_types.h" @@ -1163,7 +1164,21 @@ intel_cleanup_plane_fb(struct drm_plane *plane, intel_display_rps_mark_interactive(dev_priv, state, false); - /* Should only be called after a successful intel_prepare_plane_fb()! */ + /* +* This branch can only ever be called after plane update is succesful, +* the error path will not cause unpin_work to be set. +*/ + if (old_plane_state->unpin_work.vblank) { + int i = drm_plane_index(old_plane_state->uapi.plane); + + /* +* Remove plane from atomic commit, +* free is done from vblank worker +*/ + memset(&state->base.planes[i], 0, sizeof(*state->base.planes)); + return; + } + intel_plane_unpin_fb(old_plane_state); } @@ -1176,3 +1191,14 @@ void intel_plane_helper_add(struct intel_plane *plane) { drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs); } + +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state) +{ + if (!old_plane_state->ggtt_vma || + old_plane_state->ggtt_vma == new_plane_state->ggtt_vma) + return; + + drm_vblank_work_init(&old_plane_state->unpin_work, old_plane_state->uapi.crtc, +intel_cursor_unpin_work); +} diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 191dad0efc8e..5a897cf6fa02 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -66,5 +66,7 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state); void intel_plane_set_invisible(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); void intel_plane_helper_add(struct intel_plane *plane); +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state); #endif /* __INTEL_ATOMIC_PLANE_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index 25593f6aae7d..a7fb7f5ace07 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -501,6 +501,18 @@ void intel_pipe_update_start(struct intel_atomic_state *state, intel_crtc_vblank_work_init(new_crtc_state); intel_vblank_evade_init(old_crtc_state, new_crtc_state, &evade); + if (state->base.legacy_cursor_update) { + struct intel_plane *plane; + struct intel_plane_state *old_plane_state, *new_plane_state; + int i; + + for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state, +new_plane_state, i) { + if (old_plane_state->uapi.crtc == &crtc->base) + intel_plane_init_cursor_vblank_work(old_plane_state, + new_plane_state); + } + } if (drm_WARN_ON(&dev_priv->drm, drm_crtc_vblank_get(&crtc->base))) goto irq_disable; @@ -616,6 +628,21 @@ void intel_pipe_update_end(struct intel_atomic_state *state, new_crtc_state->uapi.event = NULL; } + if (state->base.legacy_cursor_update) { + struc
[PATCH 4/6] drm/i915: do not destroy plane state if cursor unpin worker is scheduled
The plane destroy hook can be called asynchronously even when vblank worker responsible for unpinning the cursor fb is scheduled. Since the vblank worker destroys the plane state, do not destroy the plane state if it is scheduled. Signed-off-by: Chaitanya Kumar Borah --- .../gpu/drm/i915/display/intel_atomic_plane.c | 19 +++ .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 06c5d8262443..a585e4aca309 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -136,6 +136,25 @@ intel_plane_destroy_state(struct drm_plane *plane, { struct intel_plane_state *plane_state = to_intel_plane_state(state); + /* Do not proceed if vblank unpin worker is yet to be executed */ + if (plane_state->unpin_work.vblank) + return; + + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); + drm_WARN_ON(plane->dev, plane_state->dpt_vma); + + __drm_atomic_helper_plane_destroy_state(&plane_state->uapi); + if (plane_state->hw.fb) + drm_framebuffer_put(plane_state->hw.fb); + kfree(plane_state); +} + +void +intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct intel_plane_state *plane_state = to_intel_plane_state(state); + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); drm_WARN_ON(plane->dev, plane_state->dpt_vma); diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 5a897cf6fa02..1e165b709a80 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -45,6 +45,8 @@ void intel_plane_free(struct intel_plane *plane); struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); void intel_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); +void intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state); void intel_crtc_planes_update_noarm(struct intel_atomic_state *state, struct intel_crtc *crtc); void intel_crtc_planes_update_arm(struct intel_atomic_state *state, diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index dbb26a212800..32f9bb753331 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -662,7 +662,7 @@ void intel_cursor_unpin_work(struct kthread_work *base) struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); intel_plane_unpin_fb(plane_state); - intel_plane_destroy_state(&plane->base, &plane_state->uapi); + intel_cursor_destroy_state(&plane->base, &plane_state->uapi); } static int -- 2.25.1
[PATCH v2 5/6] drm/i915: Add sanity check before accessing fb buffer object
Now that cursor plane fb unpinning can be deferred to vblank work access the fb object only when vblank unpin worker is not scheduled. v2: - Remove misleading error log - Change condition for accessing fb object Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_atomic_plane.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index a585e4aca309..8395cba243dd 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -1171,12 +1171,16 @@ static void intel_cleanup_plane_fb(struct drm_plane *plane, struct drm_plane_state *_old_plane_state) { + struct drm_i915_private *i915 = to_i915(_old_plane_state->plane->dev); struct intel_plane_state *old_plane_state = to_intel_plane_state(_old_plane_state); struct intel_atomic_state *state = to_intel_atomic_state(old_plane_state->uapi.state); struct drm_i915_private *dev_priv = to_i915(plane->dev); - struct drm_i915_gem_object *obj = intel_fb_obj(old_plane_state->hw.fb); + struct drm_i915_gem_object *obj = NULL; + + if (!old_plane_state->unpin_work.vblank) + obj = intel_fb_obj(old_plane_state->hw.fb); if (!obj) return; -- 2.25.1
[PATCH 6/6] drm/i915: do not defer cleanup work
After we move the cursor fb unpin to a vblank work, we encounter race conditions between the vblank work and the atomic clean up work leading to dump stacks[1]. Let's serialize the clean up to avoid theses races. [1] [ 278.748767] Workqueue: events_highpri intel_atomic_cleanup_work [i915] [ 278.749115] RIP: 0010:intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749425] Code: 92 cb 20 e1 e9 49 ff ff ff 5b 48 89 ef 5d 41 5c e9 11 23 44 e1 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 <38> 96 a5 05 00 00 74 2a 55 48 89 f5 0f b6 f2 53 48 8b bf 40 37 00 [ 278.749428] RSP: 0018:c929fdc8 EFLAGS: 00010246 [ 278.749433] RAX: 0060 RBX: RCX: [ 278.749435] RDX: RSI: RDI: 888124d7 [ 278.749438] RBP: 88810394c000 R08: R09: c929fc80 [ 278.749441] R10: 00f6d950 R11: 00f6da18 R12: 888124d7 [ 278.749443] R13: 88814c952000 R14: 8881000aac05 R15: 8881059baf10 [ 278.749446] FS: () GS:88817bd8() knlGS: [ 278.749449] CS: 0010 DS: ES: CR0: 80050033 [ 278.749452] CR2: 05a5 CR3: 000104078000 CR4: 00350ef0 [ 278.749454] Call Trace: [ 278.749458] [ 278.749461] ? __die_body+0x1a/0x60 [ 278.749469] ? page_fault_oops+0x156/0x450 [ 278.749474] ? do_user_addr_fault+0x65/0x9e0 [ 278.749479] ? exc_page_fault+0x68/0x1a0 [ 278.749486] ? asm_exc_page_fault+0x26/0x30 [ 278.749494] ? intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749802] intel_cleanup_plane_fb+0x6f/0xc0 [i915] [ 278.750114] drm_atomic_helper_cleanup_planes+0x42/0x60 [ 278.750122] intel_atomic_cleanup_work+0x70/0xc0 [i915] [ 278.750433] ? process_scheduled_works+0x264/0x530 [ 278.750438] process_scheduled_works+0x2db/0x530 [ 278.750444] ? __pfx_worker_thread+0x10/0x10 [ 278.750448] worker_thread+0x18c/0x350 [ 278.750452] ? __pfx_worker_thread+0x10/0x10 [ 278.750455] kthread+0xfe/0x130 [ 278.750460] ? __pfx_kthread+0x10/0x10 [ 278.750464] ret_from_fork+0x2c/0x50 [ 278.750468] ? __pfx_kthread+0x10/0x10 [ 278.750472] ret_from_fork_asm+0x1b/0x30 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_display.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index bf684c4d1732..b0e89036508e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7006,10 +7006,8 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat } } -static void intel_atomic_cleanup_work(struct work_struct *work) +static void intel_atomic_cleanup_work(struct intel_atomic_state *state) { - struct intel_atomic_state *state = - container_of(work, struct intel_atomic_state, base.commit_work); struct drm_i915_private *i915 = to_i915(state->base.dev); struct intel_crtc_state *old_crtc_state; struct intel_crtc *crtc; @@ -7283,8 +7281,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) * schedule point (cond_resched()) here anyway to keep latencies * down. */ - INIT_WORK(&state->base.commit_work, intel_atomic_cleanup_work); - queue_work(system_highpri_wq, &state->base.commit_work); + + intel_atomic_cleanup_work(state); } static void intel_atomic_commit_work(struct work_struct *work) -- 2.25.1
[PATCH 1/6] drm: Add drm_vblank_work_flush_all().
From: Maarten Lankhorst In some cases we want to flush all vblank work, right before vblank_off for example. Add a simple function to make this possible. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/drm_vblank_work.c | 22 ++ include/drm/drm_vblank_work.h | 2 ++ 2 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c index 43cd5c0f4f6f..ff86f2b2e052 100644 --- a/drivers/gpu/drm/drm_vblank_work.c +++ b/drivers/gpu/drm/drm_vblank_work.c @@ -232,6 +232,28 @@ void drm_vblank_work_flush(struct drm_vblank_work *work) } EXPORT_SYMBOL(drm_vblank_work_flush); +/** + * drm_vblank_work_flush_all - flush all currently pending vblank work on crtc. + * @crtc: crtc for which vblank work to flush + * + * Wait until all currently queued vblank work on @crtc + * has finished executing once. + */ +void drm_vblank_work_flush_all(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_vblank_crtc *vblank = &dev->vblank[drm_crtc_index(crtc)]; + + spin_lock_irq(&dev->event_lock); + wait_event_lock_irq(vblank->work_wait_queue, + waitqueue_active(&vblank->work_wait_queue), + dev->event_lock); + spin_unlock_irq(&dev->event_lock); + + kthread_flush_worker(vblank->worker); +} +EXPORT_SYMBOL(drm_vblank_work_flush_all); + /** * drm_vblank_work_init - initialize a vblank work item * @work: vblank work item diff --git a/include/drm/drm_vblank_work.h b/include/drm/drm_vblank_work.h index eb41d0810c4f..e04d436b7297 100644 --- a/include/drm/drm_vblank_work.h +++ b/include/drm/drm_vblank_work.h @@ -17,6 +17,7 @@ struct drm_crtc; * drm_vblank_work_init() * drm_vblank_work_cancel_sync() * drm_vblank_work_flush() + * drm_vblank_work_flush_all() */ struct drm_vblank_work { /** @@ -67,5 +68,6 @@ void drm_vblank_work_init(struct drm_vblank_work *work, struct drm_crtc *crtc, void (*func)(struct kthread_work *work)); bool drm_vblank_work_cancel_sync(struct drm_vblank_work *work); void drm_vblank_work_flush(struct drm_vblank_work *work); +void drm_vblank_work_flush_all(struct drm_crtc *crtc); #endif /* !_DRM_VBLANK_WORK_H_ */ -- 2.25.1
[PATCH 0/6] Cursor Fault Fixes
This series is based on top of [1] floated by Maarten. To fix regressions seen in CI, following changes were made on top of the original series. 1. add a check in the plane state destroy hook to not move forward if the vblank worker is scheduled. 2. add checks before accessing frame buffer object (we are not sure yet how much this helps but we have found that this operation causes dump stacks) 3. do not defer the intel atomic cleanup into a work queue, instead execute it at the end of atomic commit tail. While this series is in noway a complete or proper solution it is meant to trigger a discussion to arrive at one. [1] https://patchwork.freedesktop.org/series/126934/ v2: Add missing patch v3: Remove misleading error log Change condition to access fb object v4: Remove unused variables Chaitanya Kumar Borah (3): drm/i915: do not destroy plane state if cursor unpin worker is scheduled drm/i915: Add sanity check before accessing fb buffer object drm/i915: do not defer cleanup work Maarten Lankhorst (2): drm: Add drm_vblank_work_flush_all(). drm/i915: Use the same vblank worker for atomic unpin Ville Syrjälä (1): drm/i915: Use vblank worker to unpin old legacy cursor fb safely drivers/gpu/drm/drm_vblank_work.c | 22 .../gpu/drm/i915/display/intel_atomic_plane.c | 52 ++- .../gpu/drm/i915/display/intel_atomic_plane.h | 4 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 27 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 26 +- drivers/gpu/drm/i915/display/intel_cursor.h | 3 ++ drivers/gpu/drm/i915/display/intel_display.c | 11 ++-- .../drm/i915/display/intel_display_types.h| 3 ++ include/drm/drm_vblank_work.h | 2 + 9 files changed, 141 insertions(+), 9 deletions(-) -- 2.25.1
[PATCH 2/6] 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. This patch is slightly reworked by Maarten Cc: Maarten Lankhorst Signed-off-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/display/intel_cursor.c | 26 +-- drivers/gpu/drm/i915/display/intel_display.c | 3 +++ .../drm/i915/display/intel_display_types.h| 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index f8b33999d43f..9021c0c1683d 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -654,6 +654,17 @@ 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(&plane->base, &plane_state->uapi); +} + static int intel_legacy_cursor_update(struct drm_plane *_plane, struct drm_crtc *_crtc, @@ -797,14 +808,25 @@ intel_legacy_cursor_update(struct drm_plane *_plane, intel_psr_unlock(crtc_state); - intel_plane_unpin_fb(old_plane_state); + if (old_plane_state->ggtt_vma != new_plane_state->ggtt_vma) { + drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base, +intel_cursor_unpin_work); + + drm_vblank_work_schedule(&old_plane_state->unpin_work, + drm_crtc_accurate_vblank_count(&crtc->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(&crtc->base, &new_crtc_state->uapi); if (ret) intel_plane_destroy_state(&plane->base, &new_plane_state->uapi); - else + else if (old_plane_state) intel_plane_destroy_state(&plane->base, &old_plane_state->uapi); return ret; diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 7fb0f71652ac..bf684c4d1732 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -64,6 +64,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" @@ -6780,6 +6781,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) continue; intel_crtc_disable_planes(state, crtc); + + drm_vblank_work_flush_all(&crtc->base); } /* 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 ae2e8cff9d69..5fdb9eccab5a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -713,6 +713,9 @@ struct intel_plane_state { struct intel_fb_view view; + /* for legacy cursor fb unpin */ + struct drm_vblank_work unpin_work; + /* Plane pxp decryption state */ bool decrypt; -- 2.25.1
[PATCH 3/6] drm/i915: Use the same vblank worker for atomic unpin
From: Maarten Lankhorst In case of legacy cursor update, the cursor VMA needs to be unpinned only after vblank. This exceeds the lifetime of the whole atomic commit. Any trick I attempted to keep the atomic commit alive didn't work, as drm_atomic_helper_setup_commit() force throttles on any old commit that wasn't cleaned up. The only option remaining is to remove the plane from the atomic commit, and use the same path as the legacy cursor update to clean the state after vblank. Signed-off-by: Maarten Lankhorst --- .../gpu/drm/i915/display/intel_atomic_plane.c | 28 ++- .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 27 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- drivers/gpu/drm/i915/display/intel_cursor.h | 3 ++ 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 76d77d5a0409..06c5d8262443 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -42,6 +42,7 @@ #include "i915_reg.h" #include "intel_atomic_plane.h" #include "intel_cdclk.h" +#include "intel_cursor.h" #include "intel_display_rps.h" #include "intel_display_trace.h" #include "intel_display_types.h" @@ -1163,7 +1164,21 @@ intel_cleanup_plane_fb(struct drm_plane *plane, intel_display_rps_mark_interactive(dev_priv, state, false); - /* Should only be called after a successful intel_prepare_plane_fb()! */ + /* +* This branch can only ever be called after plane update is succesful, +* the error path will not cause unpin_work to be set. +*/ + if (old_plane_state->unpin_work.vblank) { + int i = drm_plane_index(old_plane_state->uapi.plane); + + /* +* Remove plane from atomic commit, +* free is done from vblank worker +*/ + memset(&state->base.planes[i], 0, sizeof(*state->base.planes)); + return; + } + intel_plane_unpin_fb(old_plane_state); } @@ -1176,3 +1191,14 @@ void intel_plane_helper_add(struct intel_plane *plane) { drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs); } + +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state) +{ + if (!old_plane_state->ggtt_vma || + old_plane_state->ggtt_vma == new_plane_state->ggtt_vma) + return; + + drm_vblank_work_init(&old_plane_state->unpin_work, old_plane_state->uapi.crtc, +intel_cursor_unpin_work); +} diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 191dad0efc8e..5a897cf6fa02 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -66,5 +66,7 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state); void intel_plane_set_invisible(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); void intel_plane_helper_add(struct intel_plane *plane); +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state); #endif /* __INTEL_ATOMIC_PLANE_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index 25593f6aae7d..a7fb7f5ace07 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -501,6 +501,18 @@ void intel_pipe_update_start(struct intel_atomic_state *state, intel_crtc_vblank_work_init(new_crtc_state); intel_vblank_evade_init(old_crtc_state, new_crtc_state, &evade); + if (state->base.legacy_cursor_update) { + struct intel_plane *plane; + struct intel_plane_state *old_plane_state, *new_plane_state; + int i; + + for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state, +new_plane_state, i) { + if (old_plane_state->uapi.crtc == &crtc->base) + intel_plane_init_cursor_vblank_work(old_plane_state, + new_plane_state); + } + } if (drm_WARN_ON(&dev_priv->drm, drm_crtc_vblank_get(&crtc->base))) goto irq_disable; @@ -616,6 +628,21 @@ void intel_pipe_update_end(struct intel_atomic_state *state, new_crtc_state->uapi.event = NULL; } + if (state->base.legacy_cursor_update) { + struc
[PATCH 4/6] drm/i915: do not destroy plane state if cursor unpin worker is scheduled
The plane destroy hook can be called asynchronously even when vblank worker responsible for unpinning the cursor fb is scheduled. Since the vblank worker destroys the plane state, do not destroy the plane state if it is scheduled. Signed-off-by: Chaitanya Kumar Borah --- .../gpu/drm/i915/display/intel_atomic_plane.c | 19 +++ .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 06c5d8262443..a585e4aca309 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -136,6 +136,25 @@ intel_plane_destroy_state(struct drm_plane *plane, { struct intel_plane_state *plane_state = to_intel_plane_state(state); + /* Do not proceed if vblank unpin worker is yet to be executed */ + if (plane_state->unpin_work.vblank) + return; + + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); + drm_WARN_ON(plane->dev, plane_state->dpt_vma); + + __drm_atomic_helper_plane_destroy_state(&plane_state->uapi); + if (plane_state->hw.fb) + drm_framebuffer_put(plane_state->hw.fb); + kfree(plane_state); +} + +void +intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct intel_plane_state *plane_state = to_intel_plane_state(state); + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); drm_WARN_ON(plane->dev, plane_state->dpt_vma); diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 5a897cf6fa02..1e165b709a80 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -45,6 +45,8 @@ void intel_plane_free(struct intel_plane *plane); struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); void intel_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); +void intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state); void intel_crtc_planes_update_noarm(struct intel_atomic_state *state, struct intel_crtc *crtc); void intel_crtc_planes_update_arm(struct intel_atomic_state *state, diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index dbb26a212800..32f9bb753331 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -662,7 +662,7 @@ void intel_cursor_unpin_work(struct kthread_work *base) struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); intel_plane_unpin_fb(plane_state); - intel_plane_destroy_state(&plane->base, &plane_state->uapi); + intel_cursor_destroy_state(&plane->base, &plane_state->uapi); } static int -- 2.25.1
[PATCH 5/6] drm/i915: Add sanity check before accessing fb buffer object
Now that cursor plane fb unpinning can be deferred to vblank work access the fb object only when vblank unpin worker is not scheduled. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_atomic_plane.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index a585e4aca309..1c3ecfc91f00 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -1176,7 +1176,10 @@ intel_cleanup_plane_fb(struct drm_plane *plane, struct intel_atomic_state *state = to_intel_atomic_state(old_plane_state->uapi.state); struct drm_i915_private *dev_priv = to_i915(plane->dev); - struct drm_i915_gem_object *obj = intel_fb_obj(old_plane_state->hw.fb); + struct drm_i915_gem_object *obj = NULL; + + if (!old_plane_state->unpin_work.vblank) + obj = intel_fb_obj(old_plane_state->hw.fb); if (!obj) return; -- 2.25.1
[PATCH 6/6] drm/i915: do not defer cleanup work
After we move the cursor fb unpin to a vblank work, we encounter race conditions between the vblank work and the atomic clean up work leading to dump stacks[1]. Let's serialize the clean up to avoid theses races. [1] [ 278.748767] Workqueue: events_highpri intel_atomic_cleanup_work [i915] [ 278.749115] RIP: 0010:intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749425] Code: 92 cb 20 e1 e9 49 ff ff ff 5b 48 89 ef 5d 41 5c e9 11 23 44 e1 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 <38> 96 a5 05 00 00 74 2a 55 48 89 f5 0f b6 f2 53 48 8b bf 40 37 00 [ 278.749428] RSP: 0018:c929fdc8 EFLAGS: 00010246 [ 278.749433] RAX: 0060 RBX: RCX: [ 278.749435] RDX: RSI: RDI: 888124d7 [ 278.749438] RBP: 88810394c000 R08: R09: c929fc80 [ 278.749441] R10: 00f6d950 R11: 00f6da18 R12: 888124d7 [ 278.749443] R13: 88814c952000 R14: 8881000aac05 R15: 8881059baf10 [ 278.749446] FS: () GS:88817bd8() knlGS: [ 278.749449] CS: 0010 DS: ES: CR0: 80050033 [ 278.749452] CR2: 05a5 CR3: 000104078000 CR4: 00350ef0 [ 278.749454] Call Trace: [ 278.749458] [ 278.749461] ? __die_body+0x1a/0x60 [ 278.749469] ? page_fault_oops+0x156/0x450 [ 278.749474] ? do_user_addr_fault+0x65/0x9e0 [ 278.749479] ? exc_page_fault+0x68/0x1a0 [ 278.749486] ? asm_exc_page_fault+0x26/0x30 [ 278.749494] ? intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749802] intel_cleanup_plane_fb+0x6f/0xc0 [i915] [ 278.750114] drm_atomic_helper_cleanup_planes+0x42/0x60 [ 278.750122] intel_atomic_cleanup_work+0x70/0xc0 [i915] [ 278.750433] ? process_scheduled_works+0x264/0x530 [ 278.750438] process_scheduled_works+0x2db/0x530 [ 278.750444] ? __pfx_worker_thread+0x10/0x10 [ 278.750448] worker_thread+0x18c/0x350 [ 278.750452] ? __pfx_worker_thread+0x10/0x10 [ 278.750455] kthread+0xfe/0x130 [ 278.750460] ? __pfx_kthread+0x10/0x10 [ 278.750464] ret_from_fork+0x2c/0x50 [ 278.750468] ? __pfx_kthread+0x10/0x10 [ 278.750472] ret_from_fork_asm+0x1b/0x30 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_display.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index bf684c4d1732..b0e89036508e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7006,10 +7006,8 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat } } -static void intel_atomic_cleanup_work(struct work_struct *work) +static void intel_atomic_cleanup_work(struct intel_atomic_state *state) { - struct intel_atomic_state *state = - container_of(work, struct intel_atomic_state, base.commit_work); struct drm_i915_private *i915 = to_i915(state->base.dev); struct intel_crtc_state *old_crtc_state; struct intel_crtc *crtc; @@ -7283,8 +7281,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) * schedule point (cond_resched()) here anyway to keep latencies * down. */ - INIT_WORK(&state->base.commit_work, intel_atomic_cleanup_work); - queue_work(system_highpri_wq, &state->base.commit_work); + + intel_atomic_cleanup_work(state); } static void intel_atomic_commit_work(struct work_struct *work) -- 2.25.1
[core-for-CI PATCH] usb: typec: ucsi: Fix a potential deadlock in ucsi_send_command_common()
From: Heikki Krogerus The function returns with the ppm_lock held if there's an error or the PPM reports BUSY condition. This is a core-for-ci patch for [1] [1] https://lore.kernel.org/linux-usb/20240806112029.2984319-1-heikki.kroge...@linux.intel.com/ Reported-by: Luciano Coelho Fixes: 5e9c1662a89b ("usb: typec: ucsi: rework command execution functions") References: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11849 Signed-off-by: Heikki Krogerus --- drivers/usb/typec/ucsi/ucsi.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index dcd3765cc1f5..432a2d6266d7 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -238,13 +238,10 @@ static int ucsi_send_command_common(struct ucsi *ucsi, u64 cmd, mutex_lock(&ucsi->ppm_lock); ret = ucsi_run_command(ucsi, cmd, &cci, data, size, conn_ack); - if (cci & UCSI_CCI_BUSY) { - ret = ucsi_run_command(ucsi, UCSI_CANCEL, &cci, NULL, 0, false); - return ret ? ret : -EBUSY; - } - - if (cci & UCSI_CCI_ERROR) - return ucsi_read_error(ucsi, connector_num); + if (cci & UCSI_CCI_BUSY) + ret = ucsi_run_command(ucsi, UCSI_CANCEL, &cci, NULL, 0, false) ?: -EBUSY; + else if (cci & UCSI_CCI_ERROR) + ret = ucsi_read_error(ucsi, connector_num); mutex_unlock(&ucsi->ppm_lock); return ret; -- 2.25.1
[PATCH] drm/i915: Preserve value of fec_enable calculated before DSC compute config
Before DSC compute config, fec_enable value is set in other functions (e.g. intel_dp_mst_find_vcpi_slots_for_bpp). We are ignoring the value by OR'ing its value in DSC compute config. One unintended effect of this is setting fec_enable in UHBR use-cases which is not needed for Intel hardware. Therefore, change operator to AND. While at it, add a comment explaining why we don't enable FEC in eDP v1.5. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_dp.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 49a37b996530..3fbf9f33c3e4 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2298,7 +2298,12 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp, &pipe_config->hw.adjusted_mode; int ret; - pipe_config->fec_enable = pipe_config->fec_enable || + /* +* Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional. +* Since, FEC is a bandwidth overhead, continue to not enable it for +* eDP. Until, there is a good reason to do so. +*/ + pipe_config->fec_enable = pipe_config->fec_enable && (!intel_dp_is_edp(intel_dp) && intel_dp_supports_fec(intel_dp, connector, pipe_config)); -- 2.25.1
[PATCH] drm/i915: Do not explicilty enable FEC in DP_TP_CTL for UHBR rates
In case of UHBR rates, we do not need to explicitly enable FEC by writing to DP_TP_CTL register. For MST use-cases, intel_dp_mst_find_vcpi_slots_for_bpp() takes care of setting fec_enable to false. However, it gets overwritten in intel_dp_dsc_compute_config(). This change keeps fec_enable false across MST and SST use-cases for UHBR rates. While at it, add a comment explaining why we don't enable FEC in eDP v1.5. v2: Correct logic to cater to SST use-cases (Jani) Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_dp.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 49a37b996530..de30eaacd9b4 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2298,9 +2298,15 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp, &pipe_config->hw.adjusted_mode; int ret; + /* +* Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional. +* Since, FEC is a bandwidth overhead, continue to not enable it for +* eDP. Until, there is a good reason to do so. +*/ pipe_config->fec_enable = pipe_config->fec_enable || (!intel_dp_is_edp(intel_dp) && -intel_dp_supports_fec(intel_dp, connector, pipe_config)); +intel_dp_supports_fec(intel_dp, connector, pipe_config) && +!intel_dp_is_uhbr(pipe_config)); if (!intel_dp_supports_dsc(connector, pipe_config)) return -EINVAL; -- 2.25.1
[Intel-gfx] [PATCH] drm/i915/display: Set correct voltage level for 480MHz CDCLK
According to Bspec, the voltage level for 480MHz is to be set as 1 instead of 2. BSpec: 49208 Fixes: 06f1b06dc5b7 ("drm/i915/display: Add 480 MHz CDCLK steps for RPL-U") Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 30 +++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 084a483f9776..2aaaba090cc0 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1453,6 +1453,18 @@ static u8 tgl_calc_voltage_level(int cdclk) return 0; } +static u8 rplu_calc_voltage_level(int cdclk) +{ + if (cdclk > 556800) + return 3; + else if (cdclk > 48) + return 2; + else if (cdclk > 312000) + return 1; + else + return 0; +} + static void icl_readout_refclk(struct drm_i915_private *dev_priv, struct intel_cdclk_config *cdclk_config) { @@ -3242,6 +3254,13 @@ static const struct intel_cdclk_funcs mtl_cdclk_funcs = { .calc_voltage_level = tgl_calc_voltage_level, }; +static const struct intel_cdclk_funcs rplu_cdclk_funcs = { + .get_cdclk = bxt_get_cdclk, + .set_cdclk = bxt_set_cdclk, + .modeset_calc_cdclk = bxt_modeset_calc_cdclk, + .calc_voltage_level = rplu_calc_voltage_level, +}; + static const struct intel_cdclk_funcs tgl_cdclk_funcs = { .get_cdclk = bxt_get_cdclk, .set_cdclk = bxt_set_cdclk, @@ -3384,14 +3403,17 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; dev_priv->display.cdclk.table = dg2_cdclk_table; } else if (IS_ALDERLAKE_P(dev_priv)) { - dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; /* Wa_22011320316:adl-p[a0] */ - if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) + if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) { dev_priv->display.cdclk.table = adlp_a_step_cdclk_table; - else if (IS_ADLP_RPLU(dev_priv)) + dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; + } else if (IS_ADLP_RPLU(dev_priv)) { dev_priv->display.cdclk.table = rplu_cdclk_table; - else + dev_priv->display.funcs.cdclk = &rplu_cdclk_funcs; + } else { dev_priv->display.cdclk.table = adlp_cdclk_table; + dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; + } } else if (IS_ROCKETLAKE(dev_priv)) { dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; dev_priv->display.cdclk.table = rkl_cdclk_table; -- 2.25.1
[Intel-gfx] [PATCH] drm/i915/color: Fix typo for Plane CSC indexes
Replace _PLANE_INPUT_CSC_RY_GY_2_* with _PLANE_CSC_RY_GY_2_* for Plane CSC Fixes: 6eba56f64d5d ("drm/i915/pxp: black pixels on pxp disabled") Cc: Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/i915_reg.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 8e4aca888b7a..85885b01e6ac 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7496,8 +7496,8 @@ enum skl_power_gate { #define _PLANE_CSC_RY_GY_1(pipe) _PIPE(pipe, _PLANE_CSC_RY_GY_1_A, \ _PLANE_CSC_RY_GY_1_B) -#define _PLANE_CSC_RY_GY_2(pipe) _PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_2_A, \ - _PLANE_INPUT_CSC_RY_GY_2_B) +#define _PLANE_CSC_RY_GY_2(pipe) _PIPE(pipe, _PLANE_CSC_RY_GY_2_A, \ + _PLANE_CSC_RY_GY_2_B) #define PLANE_CSC_COEFF(pipe, plane, index)_MMIO_PLANE(plane, \ _PLANE_CSC_RY_GY_1(pipe) + (index) * 4, \ _PLANE_CSC_RY_GY_2(pipe) + (index) * 4) -- 2.25.1
[Intel-gfx] [PATCH] drm/i915/dg1: Remove require_force_probe protection
Remove force probe protection from DG1 platform as testing suggests that the platform is stable enough. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/i915_pci.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 19bf5ef6a20d..bbd3e8befb55 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -926,7 +926,6 @@ static const struct intel_device_info dg1_info = { .__runtime.graphics.ip.rel = 10, PLATFORM(INTEL_DG1), .__runtime.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C) | BIT(PIPE_D), - .require_force_probe = 1, .__runtime.platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2), -- 2.25.1
[Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U
A new step of 480MHz has been added on SKUs that have an RPL-U device id. This particular step is to support 120Hz panels more efficiently. This patchset adds a new table to include this new CDCLK step. Details can be found in BSpec entry 55409. Create a new sub-platform to identify RPL-U which will enable us to make the differentiation during CDCLK initialization. Furthermore, we need to make a distinction between ES (Engineering Sample) and QS (Quality Sample) parts as this change comes only to QS parts. This version of the patch does not include this change as we are yet to make a decision if this particular part needs to be upstreamed.(see comments on revision 2) Chaitanya Kumar Borah (2): drm/i915: Add sub platform for 480MHz CDCLK step drm/i915/display: Add 480 MHz CDCLK steps for RPL-U drivers/gpu/drm/i915/display/intel_cdclk.c | 26 ++ drivers/gpu/drm/i915/i915_drv.h| 2 ++ drivers/gpu/drm/i915/i915_pci.c| 1 + drivers/gpu/drm/i915/intel_device_info.c | 8 +++ drivers/gpu/drm/i915/intel_device_info.h | 2 ++ include/drm/i915_pciids.h | 11 + 6 files changed, 46 insertions(+), 4 deletions(-) -- 2.25.1
[Intel-gfx] [RFC v2 1/2] drm/i915: Add sub platform for 480MHz CDCLK step
Add a new sub platform INTEL_SUBPLATFORM_480CDCLK to identify device ids that need the 480MHz CDCLK step. Separate out RPLU device ids and add them to both RPL and 480CDCLK subplatforms. v2: (Matt) - Sort PCI-IDs numerically - Name the sub-platform to accurately depict what it is for - Make RPL-U part of RPL subplatform Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/intel_device_info.c | 8 drivers/gpu/drm/i915/intel_device_info.h | 2 ++ include/drm/i915_pciids.h| 11 +++ 5 files changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 48fd82722f12..fc5518314598 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -619,6 +619,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_N) #define IS_ADLP_RPLP(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPL) +#define IS_ADLP_WITH_480CDCLK(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_480CDCLK) #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \ (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00) #define IS_BDW_ULT(dev_priv) \ diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 6cc65079b18d..e9f3b99b3e00 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1234,6 +1234,7 @@ static const struct pci_device_id pciidlist[] = { INTEL_DG1_IDS(&dg1_info), INTEL_RPLS_IDS(&adl_s_info), INTEL_RPLP_IDS(&adl_p_info), + INTEL_RPLU_IDS(&adl_p_info), INTEL_DG2_IDS(&dg2_info), INTEL_ATS_M_IDS(&ats_m_info), INTEL_MTL_IDS(&mtl_info), diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 849baf6c3b3c..a1732ad519fb 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -199,6 +199,11 @@ static const u16 subplatform_n_ids[] = { static const u16 subplatform_rpl_ids[] = { INTEL_RPLS_IDS(0), INTEL_RPLP_IDS(0), + INTEL_RPLU_IDS(0) +}; + +static const u16 subplatform_480cdclk_ids[] = { + INTEL_RPLU_IDS(0), }; static const u16 subplatform_g10_ids[] = { @@ -268,6 +273,9 @@ static void intel_device_info_subplatform_init(struct drm_i915_private *i915) } else if (find_devid(devid, subplatform_rpl_ids, ARRAY_SIZE(subplatform_rpl_ids))) { mask = BIT(INTEL_SUBPLATFORM_RPL); + if (find_devid(devid, subplatform_480cdclk_ids, + ARRAY_SIZE(subplatform_480cdclk_ids))) + mask |= BIT(INTEL_SUBPLATFORM_480CDCLK); } else if (find_devid(devid, subplatform_g10_ids, ARRAY_SIZE(subplatform_g10_ids))) { mask = BIT(INTEL_SUBPLATFORM_G10); diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index d588e5fd2eea..4f488c900273 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -127,6 +127,8 @@ enum intel_platform { * bit set */ #define INTEL_SUBPLATFORM_N1 +/* Sub Platform with 480MHz CDCLK step */ +#define INTEL_SUBPLATFORM_480CDCLK 2 /* MTL */ #define INTEL_SUBPLATFORM_M0 diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 4a4c190f7698..758be5fb09a2 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -684,14 +684,17 @@ INTEL_VGA_DEVICE(0xA78A, info), \ INTEL_VGA_DEVICE(0xA78B, info) +/* RPL-U */ +#define INTEL_RPLU_IDS(info) \ + INTEL_VGA_DEVICE(0xA721, info), \ + INTEL_VGA_DEVICE(0xA7A1, info), \ + INTEL_VGA_DEVICE(0xA7A9, info) + /* RPL-P */ #define INTEL_RPLP_IDS(info) \ INTEL_VGA_DEVICE(0xA720, info), \ - INTEL_VGA_DEVICE(0xA721, info), \ INTEL_VGA_DEVICE(0xA7A0, info), \ - INTEL_VGA_DEVICE(0xA7A1, info), \ - INTEL_VGA_DEVICE(0xA7A8, info), \ - INTEL_VGA_DEVICE(0xA7A9, info) + INTEL_VGA_DEVICE(0xA7A8, info) /* DG2 */ #define INTEL_DG2_G10_IDS(info) \ -- 2.25.1
[Intel-gfx] [RFC 2/2] drm/i915/display: Add 480 MHz CDCLK steps for RPL-U
A new step of 480MHz has been added on SKUs that have a RPL-U device id to support 120Hz displays more efficiently. Use a new quirk to identify the machine for which this change needs to be applied. BSpec: 55409 v2: (Matt) - Add missing clock steps - Correct reference clock typo Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 0c107a38f9d0..ba365ef17abc 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1329,6 +1329,30 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = { {} }; +static const struct intel_cdclk_vals rplu_cdclk_table[] = { + { .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 }, + { .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 }, + { .refclk = 19200, .cdclk = 307200, .divider = 2, .ratio = 32 }, + { .refclk = 19200, .cdclk = 48, .divider = 2, .ratio = 50 }, + { .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 }, + { .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 }, + + { .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 }, + { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 }, + { .refclk = 24000, .cdclk = 312000, .divider = 2, .ratio = 26 }, + { .refclk = 24000, .cdclk = 48, .divider = 2, .ratio = 40 }, + { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 }, + { .refclk = 24000, .cdclk = 648000, .divider = 2, .ratio = 54 }, + + { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 }, + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 }, + { .refclk = 38400, .cdclk = 307200, .divider = 2, .ratio = 16 }, + { .refclk = 38400, .cdclk = 48, .divider = 2, .ratio = 25 }, + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 }, + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 }, + {} +}; + static const struct intel_cdclk_vals dg2_cdclk_table[] = { { .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, .waveform = 0x }, { .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, .waveform = 0x9248 }, @@ -3353,6 +3377,8 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) /* Wa_22011320316:adl-p[a0] */ if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) dev_priv->display.cdclk.table = adlp_a_step_cdclk_table; + else if (IS_ADLP_WITH_480CDCLK(dev_priv)) + dev_priv->display.cdclk.table = rplu_cdclk_table; else dev_priv->display.cdclk.table = adlp_cdclk_table; } else if (IS_ROCKETLAKE(dev_priv)) { -- 2.25.1
[Intel-gfx] [PATCH] drm/i915/display: Fix typo for reference clock
Fix typo for reference clock from 24400 to 24000 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 0c107a38f9d0..7e16b655c833 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1319,7 +1319,7 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = { { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 }, { .refclk = 24000, .cdclk = 312000, .divider = 2, .ratio = 26 }, { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 }, - { .refclk = 24400, .cdclk = 648000, .divider = 2, .ratio = 54 }, + { .refclk = 24000, .cdclk = 648000, .divider = 2, .ratio = 54 }, { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 }, { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 }, -- 2.25.1
[Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U
A new step of 480MHz has been added on SKUs that have an RPL-U device id. This particular step is to support 120Hz panels more efficiently. This patchset adds a new table to include this new CDCLK step. Details can be found in BSpec entry 55409. Create a new sub-platform to identify RPL-U which will enable us to make the differentiation during CDCLK initialization. Furthermore, we need to make a distinction between ES (Engineering Sample) and QS (Quality Sample) parts as this change comes only to QS parts. This version of the patch does not include this change as we are yet to make a decision if this particular part needs to be upstreamed.(see comments on revision 2) Chaitanya Kumar Borah (2): drm/i915: Add RPL-U sub platform drm/i915/display: Add 480 MHz CDCLK steps for RPL-U drivers/gpu/drm/i915/display/intel_cdclk.c | 27 ++ drivers/gpu/drm/i915/i915_drv.h| 2 ++ drivers/gpu/drm/i915/i915_pci.c| 1 + drivers/gpu/drm/i915/intel_device_info.c | 8 +++ drivers/gpu/drm/i915/intel_device_info.h | 2 ++ include/drm/i915_pciids.h | 11 + 6 files changed, 47 insertions(+), 4 deletions(-) -- 2.25.1
[Intel-gfx] [RFC 2/2] drm/i915/display: Add 480 MHz CDCLK steps for RPL-U
A new step of 480MHz has been added on SKUs that have a RPL-U device id to support 120Hz displays more efficiently. Use a new quirk to identify the machine for which this change needs to be applied. BSpec: 55409 v2: (Matt) - Add missing clock steps - Correct reference clock typo v3: - Revert to RPL-U subplatform Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 27 ++ 1 file changed, 27 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 0c107a38f9d0..a5e3497534f3 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1329,6 +1329,30 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = { {} }; +static const struct intel_cdclk_vals rplu_cdclk_table[] = { + { .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 }, + { .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 }, + { .refclk = 19200, .cdclk = 307200, .divider = 2, .ratio = 32 }, + { .refclk = 19200, .cdclk = 48, .divider = 2, .ratio = 50 }, + { .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 }, + { .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 }, + + { .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 }, + { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 }, + { .refclk = 24000, .cdclk = 312000, .divider = 2, .ratio = 26 }, + { .refclk = 24000, .cdclk = 48, .divider = 2, .ratio = 40 }, + { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 }, + { .refclk = 24000, .cdclk = 648000, .divider = 2, .ratio = 54 }, + + { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 }, + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 }, + { .refclk = 38400, .cdclk = 307200, .divider = 2, .ratio = 16 }, + { .refclk = 38400, .cdclk = 48, .divider = 2, .ratio = 25 }, + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 }, + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 }, + {} +}; + static const struct intel_cdclk_vals dg2_cdclk_table[] = { { .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, .waveform = 0x }, { .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, .waveform = 0x9248 }, @@ -3353,6 +3377,9 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) /* Wa_22011320316:adl-p[a0] */ if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) dev_priv->display.cdclk.table = adlp_a_step_cdclk_table; + /* Bspec: 55409 */ + else if (IS_ADLP_RPLU(dev_priv)) + dev_priv->display.cdclk.table = rplu_cdclk_table; else dev_priv->display.cdclk.table = adlp_cdclk_table; } else if (IS_ROCKETLAKE(dev_priv)) { -- 2.25.1
[Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U sub platform
Separate out RPLU device ids and add them to both RPL and newly created RPL-U subplatforms. v2: (Matt) - Sort PCI-IDs numerically - Name the sub-platform to accurately depict what it is for - Make RPL-U part of RPL subplatform v3: revert to RPL-U subplatform (Jani) Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/intel_device_info.c | 8 drivers/gpu/drm/i915/intel_device_info.h | 2 ++ include/drm/i915_pciids.h| 11 +++ 5 files changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 48fd82722f12..c88e514728a0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -619,6 +619,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_N) #define IS_ADLP_RPLP(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPL) +#define IS_ADLP_RPLU(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPLU) #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \ (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00) #define IS_BDW_ULT(dev_priv) \ diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 6cc65079b18d..e9f3b99b3e00 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1234,6 +1234,7 @@ static const struct pci_device_id pciidlist[] = { INTEL_DG1_IDS(&dg1_info), INTEL_RPLS_IDS(&adl_s_info), INTEL_RPLP_IDS(&adl_p_info), + INTEL_RPLU_IDS(&adl_p_info), INTEL_DG2_IDS(&dg2_info), INTEL_ATS_M_IDS(&ats_m_info), INTEL_MTL_IDS(&mtl_info), diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 849baf6c3b3c..fec8bd116436 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -199,6 +199,11 @@ static const u16 subplatform_n_ids[] = { static const u16 subplatform_rpl_ids[] = { INTEL_RPLS_IDS(0), INTEL_RPLP_IDS(0), + INTEL_RPLU_IDS(0) +}; + +static const u16 subplatform_rplu_ids[] = { + INTEL_RPLU_IDS(0), }; static const u16 subplatform_g10_ids[] = { @@ -268,6 +273,9 @@ static void intel_device_info_subplatform_init(struct drm_i915_private *i915) } else if (find_devid(devid, subplatform_rpl_ids, ARRAY_SIZE(subplatform_rpl_ids))) { mask = BIT(INTEL_SUBPLATFORM_RPL); + if (find_devid(devid, subplatform_rplu_ids, + ARRAY_SIZE(subplatform_rplu_ids))) + mask |= BIT(INTEL_SUBPLATFORM_RPLU); } else if (find_devid(devid, subplatform_g10_ids, ARRAY_SIZE(subplatform_g10_ids))) { mask = BIT(INTEL_SUBPLATFORM_G10); diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index d588e5fd2eea..4a5cd337e4b5 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -127,6 +127,8 @@ enum intel_platform { * bit set */ #define INTEL_SUBPLATFORM_N1 +/* Sub Platform for RPL-U */ +#define INTEL_SUBPLATFORM_RPLU 2 /* MTL */ #define INTEL_SUBPLATFORM_M0 diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 4a4c190f7698..758be5fb09a2 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -684,14 +684,17 @@ INTEL_VGA_DEVICE(0xA78A, info), \ INTEL_VGA_DEVICE(0xA78B, info) +/* RPL-U */ +#define INTEL_RPLU_IDS(info) \ + INTEL_VGA_DEVICE(0xA721, info), \ + INTEL_VGA_DEVICE(0xA7A1, info), \ + INTEL_VGA_DEVICE(0xA7A9, info) + /* RPL-P */ #define INTEL_RPLP_IDS(info) \ INTEL_VGA_DEVICE(0xA720, info), \ - INTEL_VGA_DEVICE(0xA721, info), \ INTEL_VGA_DEVICE(0xA7A0, info), \ - INTEL_VGA_DEVICE(0xA7A1, info), \ - INTEL_VGA_DEVICE(0xA7A8, info), \ - INTEL_VGA_DEVICE(0xA7A9, info) + INTEL_VGA_DEVICE(0xA7A8, info) /* DG2 */ #define INTEL_DG2_G10_IDS(info) \ -- 2.25.1
[Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U
A new step of 480MHz has been added on SKUs that have an RPL-U device id. This particular step is to support 120Hz panels more efficiently. This patchset adds a new table to include this new CDCLK step. Details can be found in BSpec entry 55409. Create a new sub-platform to identify RPL-U which will enable us to make the differentiation during CDCLK initialization. Furthermore, we need to make a distinction between ES (Engineering Sample) and QS (Quality Sample) parts as this change comes only to QS parts. This version of the patch does not include this change as we are yet to make a decision if this particular part needs to be upstreamed.(see comments on revision 2) Chaitanya Kumar Borah (2): drm/i915: Add RPL-U sub platform drm/i915/display: Add 480 MHz CDCLK steps for RPL-U drivers/gpu/drm/i915/display/intel_cdclk.c | 26 ++ drivers/gpu/drm/i915/i915_drv.h| 2 ++ drivers/gpu/drm/i915/intel_device_info.c | 7 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + include/drm/i915_pciids.h | 12 ++ 5 files changed, 44 insertions(+), 4 deletions(-) -- 2.25.1
[Intel-gfx] [RFC v4 1/2] drm/i915: Add RPL-U sub platform
Separate out RPLU device ids and add them to both RPL and newly created RPL-U subplatforms. v2: (Matt) - Sort PCI-IDs numerically - Name the sub-platform to accurately depict what it is for - Make RPL-U part of RPL subplatform v3: revert to RPL-U subplatform (Jani) v4: (Jani) - Add RPL-U ids to RPL-P platform - Remove redundant comment Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_device_info.c | 7 +++ drivers/gpu/drm/i915/intel_device_info.h | 1 + include/drm/i915_pciids.h| 12 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 48fd82722f12..c88e514728a0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -619,6 +619,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_N) #define IS_ADLP_RPLP(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPL) +#define IS_ADLP_RPLU(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPLU) #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \ (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00) #define IS_BDW_ULT(dev_priv) \ diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 849baf6c3b3c..322e1ef94c47 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -201,6 +201,10 @@ static const u16 subplatform_rpl_ids[] = { INTEL_RPLP_IDS(0), }; +static const u16 subplatform_rplu_ids[] = { + INTEL_RPLU_IDS(0), +}; + static const u16 subplatform_g10_ids[] = { INTEL_DG2_G10_IDS(0), INTEL_ATS_M150_IDS(0), @@ -268,6 +272,9 @@ static void intel_device_info_subplatform_init(struct drm_i915_private *i915) } else if (find_devid(devid, subplatform_rpl_ids, ARRAY_SIZE(subplatform_rpl_ids))) { mask = BIT(INTEL_SUBPLATFORM_RPL); + if (find_devid(devid, subplatform_rplu_ids, + ARRAY_SIZE(subplatform_rplu_ids))) + mask |= BIT(INTEL_SUBPLATFORM_RPLU); } else if (find_devid(devid, subplatform_g10_ids, ARRAY_SIZE(subplatform_g10_ids))) { mask = BIT(INTEL_SUBPLATFORM_G10); diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index d588e5fd2eea..3e3ca5eb073f 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -127,6 +127,7 @@ enum intel_platform { * bit set */ #define INTEL_SUBPLATFORM_N1 +#define INTEL_SUBPLATFORM_RPLU 2 /* MTL */ #define INTEL_SUBPLATFORM_M0 diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 4a4c190f7698..5824e1d7d162 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -684,14 +684,18 @@ INTEL_VGA_DEVICE(0xA78A, info), \ INTEL_VGA_DEVICE(0xA78B, info) +/* RPL-U */ +#define INTEL_RPLU_IDS(info) \ + INTEL_VGA_DEVICE(0xA721, info), \ + INTEL_VGA_DEVICE(0xA7A1, info), \ + INTEL_VGA_DEVICE(0xA7A9, info) + /* RPL-P */ #define INTEL_RPLP_IDS(info) \ + INTEL_RPLU_IDS(info), \ INTEL_VGA_DEVICE(0xA720, info), \ - INTEL_VGA_DEVICE(0xA721, info), \ INTEL_VGA_DEVICE(0xA7A0, info), \ - INTEL_VGA_DEVICE(0xA7A1, info), \ - INTEL_VGA_DEVICE(0xA7A8, info), \ - INTEL_VGA_DEVICE(0xA7A9, info) + INTEL_VGA_DEVICE(0xA7A8, info) /* DG2 */ #define INTEL_DG2_G10_IDS(info) \ -- 2.25.1
[Intel-gfx] [RFC v4 2/2] drm/i915/display: Add 480 MHz CDCLK steps for RPL-U
A new step of 480MHz has been added on SKUs that have a RPL-U device id to support 120Hz displays more efficiently. Use a new quirk to identify the machine for which this change needs to be applied. BSpec: 55409 v2: (Matt) - Add missing clock steps - Correct reference clock typo v3: - Revert to RPL-U subplatform (Jani) v4: - Remove Bspec reference from code (Jani) Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 0c107a38f9d0..2e26e6762f35 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1329,6 +1329,30 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = { {} }; +static const struct intel_cdclk_vals rplu_cdclk_table[] = { + { .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 }, + { .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 }, + { .refclk = 19200, .cdclk = 307200, .divider = 2, .ratio = 32 }, + { .refclk = 19200, .cdclk = 48, .divider = 2, .ratio = 50 }, + { .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 }, + { .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 }, + + { .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 }, + { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 }, + { .refclk = 24000, .cdclk = 312000, .divider = 2, .ratio = 26 }, + { .refclk = 24000, .cdclk = 48, .divider = 2, .ratio = 40 }, + { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 }, + { .refclk = 24000, .cdclk = 648000, .divider = 2, .ratio = 54 }, + + { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 }, + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 }, + { .refclk = 38400, .cdclk = 307200, .divider = 2, .ratio = 16 }, + { .refclk = 38400, .cdclk = 48, .divider = 2, .ratio = 25 }, + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 }, + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 }, + {} +}; + static const struct intel_cdclk_vals dg2_cdclk_table[] = { { .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, .waveform = 0x }, { .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, .waveform = 0x9248 }, @@ -3353,6 +3377,8 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) /* Wa_22011320316:adl-p[a0] */ if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) dev_priv->display.cdclk.table = adlp_a_step_cdclk_table; + else if (IS_ADLP_RPLU(dev_priv)) + dev_priv->display.cdclk.table = rplu_cdclk_table; else dev_priv->display.cdclk.table = adlp_cdclk_table; } else if (IS_ROCKETLAKE(dev_priv)) { -- 2.25.1
[Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U
A new step of 480MHz has been added on SKUs that have a RPL-U device id. This particular step is to better support 120Hz panels. This patchset adds a new table to include this new CDCLK step. Details can be found in BSpec entry 55409. In addition to identifying RPL-U device id, we need to make a distinction between ES and QS parts as this change comes only to QS parts. For this CPUID Brand string is used. 480Mhz step is only supported in SKUs which does not contain the string "Genuine Intel" in the Brand string. Even though ES parts will be deprecated in future we are adding this distinction since they are currently in use. However, here the question arises if we keep this change in upstream or not as this could just be dead code down the line. Feedbacks are appreciated on this. Chaitanya Kumar Borah (2): drm/i915: Add RPL-U CDCLK table drm/i915: Add additional check for 480Mhz step CDCLK drivers/gpu/drm/i915/display/intel_cdclk.c | 53 ++ 1 file changed, 53 insertions(+) -- 2.25.1
[Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz step CDCLK
There are still RPL-U boards which does not support the 480Mhz step of CDCLK. We can differentiate these board by checking the CPUID Brand String. 480Mhz step is only supported in SKUs which does not contain the string "Genuine Intel" in the Brand string. BSpec: 55409 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 9bfeb1abba47..1890e5135cfc 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -192,6 +192,19 @@ static bool is_rplu(struct drm_i915_private *dev_priv) } } +static bool is_480mhz_step_valid(void) +{ + struct cpuinfo_x86 *c; + unsigned int cpu = smp_processor_id(); + + c = &cpu_data(cpu); + + if (c->x86_model_id[0] && !strstr(c->x86_model_id, "Genuine Intel")) + return true; + + return false; +} + static void i915gm_get_cdclk(struct drm_i915_private *dev_priv, struct intel_cdclk_config *cdclk_config) { @@ -3389,8 +3402,9 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) /* * BSpec: 55409 * 480 MHz supported on SKUs that have a RPL-U Device ID +* and CPUID Brand String that does not contain "Genuine Intel". */ - else if (is_rplu(dev_priv)) + else if (is_rplu(dev_priv) && is_480mhz_step_valid()) dev_priv->cdclk.table = rplu_cdclk_table; else dev_priv->display.cdclk.table = adlp_cdclk_table; -- 2.25.1
[Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U CDCLK table
A new step of 480MHz has been added on SKUs that have a RPL-U device id. Add a new table which include this new CDCLK step. BSpec: 55409 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 39 ++ 1 file changed, 39 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 0c107a38f9d0..9bfeb1abba47 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -180,6 +180,18 @@ static void i85x_get_cdclk(struct drm_i915_private *dev_priv, } } +static bool is_rplu(struct drm_i915_private *dev_priv) +{ + switch (INTEL_DEVID(dev_priv)) { + case 0xA7A1: + case 0xA721: + case 0xA7A9: + return true; + default: + return false; + } +} + static void i915gm_get_cdclk(struct drm_i915_private *dev_priv, struct intel_cdclk_config *cdclk_config) { @@ -1329,6 +1341,27 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = { {} }; +static const struct intel_cdclk_vals rplu_cdclk_table[] = { + { .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 }, + { .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 }, + { .refclk = 19200, .cdclk = 48, .divider = 2, .ratio = 50 }, + { .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 }, + { .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 }, + + { .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 }, + { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 }, + { .refclk = 24000, .cdclk = 48, .divider = 2, .ratio = 40 }, + { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 }, + { .refclk = 24400, .cdclk = 648000, .divider = 2, .ratio = 54 }, + + { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 }, + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 }, + { .refclk = 38400, .cdclk = 48, .divider = 2, .ratio = 25 }, + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 }, + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 }, + {} +}; + static const struct intel_cdclk_vals dg2_cdclk_table[] = { { .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, .waveform = 0x }, { .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, .waveform = 0x9248 }, @@ -3353,6 +3386,12 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) /* Wa_22011320316:adl-p[a0] */ if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) dev_priv->display.cdclk.table = adlp_a_step_cdclk_table; + /* +* BSpec: 55409 +* 480 MHz supported on SKUs that have a RPL-U Device ID +*/ + else if (is_rplu(dev_priv)) + dev_priv->cdclk.table = rplu_cdclk_table; else dev_priv->display.cdclk.table = adlp_cdclk_table; } else if (IS_ROCKETLAKE(dev_priv)) { -- 2.25.1
[Intel-gfx] [PATCH] drm/i915/color: Add function to load degamma LUT in MTL
MTL onwards Degamma LUT/PRE-CSC LUT precision has been increased from 16 bit to 24 bits. Currently, drm framework only supports LUTs upto 16 bit precision. There is a new uapi in discussion which would support 32.32 bit precision. Until theni, upscale the values sent from userland to 24 bit before writing into the HW to continue supporting degamma on MTL. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_color.c | 42 -- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index d57631b0bb9a..04b941cbc083 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1104,6 +1104,38 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state, ilk_lut_write(crtc_state, PRE_CSC_GAMC_INDEX(pipe), 0); } +static void mtl_load_degamma_lut(const struct intel_crtc_state *crtc_state, +const struct drm_property_blob *blob) +{ + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); + struct drm_i915_private *i915 = to_i915(crtc->base.dev); + struct drm_color_lut *degamma_lut = blob->data; + enum pipe pipe = crtc->pipe; + int i, lut_size = drm_color_lut_size(blob); + + /* +* When setting the auto-increment bit, the hardware seems to +* ignore the index bits, so we need to reset it to index 0 +* separately. +*/ + intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe), 0); + intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe), + PRE_CSC_GAMC_AUTO_INCREMENT); + + for (i = 0; i < lut_size; i++) { + u64 word = mul_u32_u32(degamma_lut[i].green, (1 << 24)) / (1 << 16); + u32 lut_val = (word & 0xff); + + intel_de_write_fw(i915, PRE_CSC_GAMC_DATA(pipe), + lut_val); + } + /* Clamp values > 1.0. */ + while (i++ < glk_degamma_lut_size(i915)) + intel_de_write_fw(i915, PRE_CSC_GAMC_DATA(pipe), 1 << 24); + + intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe), 0); +} + static void glk_load_luts(const struct intel_crtc_state *crtc_state) { const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut; @@ -1241,11 +1273,17 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state) static void icl_load_luts(const struct intel_crtc_state *crtc_state) { + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); + struct drm_i915_private *i915 = to_i915(crtc->base.dev); const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut; const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut; - if (pre_csc_lut) - glk_load_degamma_lut(crtc_state, pre_csc_lut); + if (pre_csc_lut) { + if (DISPLAY_VER(i915) >= 14) + mtl_load_degamma_lut(crtc_state, pre_csc_lut); + else + glk_load_degamma_lut(crtc_state, pre_csc_lut); + } switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) { case GAMMA_MODE_MODE_8BIT: -- 2.25.1
[Intel-gfx] [RFC 0/4] Add new CDCLK step for RPL-U
A new step of 480MHz has been added on SKUs that have a RPL-U device id. This particular step is to support 120Hz panels more efficiently. This patchset adds a new table to include this new CDCLK step. Details can be found in BSpec entry 55409. RPL-U device ids are currently added within the RPL-P sub platform. It seems to be an overkill to add a separate sub platform just to support this change. Therefore, quirks are a good way to achieve this. In addition to identifying RPL-U device id, we need to make a distinction between ES and QS parts as this change comes only to QS parts. CPUID Brand string is the only way to make this distinction currently. ES parts have "Genuine Intel" in their brand string while QS parts have a more specific brand string, for ex. "13th Gen Intel(R) Core(TM) i5-1345U". Therefore, 480Mhz step is only supported in SKUs which does not contain the string "Genuine Intel" in the Brand string. The patch "drm/i915: Apply CDCLK quirk only on QS parts" adds this change. We have separated this patch because we request feedback from the community if this change needs to be upstreamed or not as ES parts will be deprecated in future. Feedbacks are welcome. Chaitanya Kumar Borah (4): drm/i915/quirks: Add quirk for 480MHz CDCLK step drm/i915/display: Add 480 MHz CDCLK steps for RPL-U drm/i915: Initialize intel quirks before CDCLK initialization drm/i915: Apply CDCLK quirk only on QS parts drivers/gpu/drm/i915/display/intel_cdclk.c | 25 drivers/gpu/drm/i915/display/intel_display.c | 2 - drivers/gpu/drm/i915/display/intel_quirks.c | 40 drivers/gpu/drm/i915/display/intel_quirks.h | 1 + drivers/gpu/drm/i915/i915_driver.c | 2 + 5 files changed, 68 insertions(+), 2 deletions(-) -- 2.25.1
[Intel-gfx] [RFC 1/4] drm/i915/quirks: Add quirk for 480MHz CDCLK step
A new CDCLK step of 480MHz has been added on SKUs that has a RPL-U device id. This is done to support 120Hz displays with more efficiency. RPL-U device ids are currently added within the RPL-P sub platform. It seems to be an overkill to add a separate sub platform just to support this change. Therefore, quirks are a good way to achieve the same. BSpec: 55409 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_quirks.c | 14 ++ drivers/gpu/drm/i915/display/intel_quirks.h | 1 + 2 files changed, 15 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c index 6e48d3bcdfec..0a30499835b3 100644 --- a/drivers/gpu/drm/i915/display/intel_quirks.c +++ b/drivers/gpu/drm/i915/display/intel_quirks.c @@ -65,6 +65,16 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915) drm_info(&i915->drm, "Applying no pps backlight power quirk\n"); } +/* + * A new step of 480MHz has been added on SKUs that have a RPL-U device id. + * This particular step is to better support 120Hz panels. + */ +static void quirk_480mhz_cdclk_step_hook(struct drm_i915_private *i915) +{ + intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP); + drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n"); +} + struct intel_quirk { int device; int subsystem_vendor; @@ -199,6 +209,10 @@ static struct intel_quirk intel_quirks[] = { /* ECS Liva Q2 */ { 0x3185, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time }, { 0x3184, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time }, + /* RPL-U */ + { 0xA7A1, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook }, + { 0xA721, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook }, + { 0xA7A9, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook }, }; void intel_init_quirks(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/display/intel_quirks.h b/drivers/gpu/drm/i915/display/intel_quirks.h index 10a4d163149f..71e05684f5f4 100644 --- a/drivers/gpu/drm/i915/display/intel_quirks.h +++ b/drivers/gpu/drm/i915/display/intel_quirks.h @@ -17,6 +17,7 @@ enum intel_quirk_id { QUIRK_INVERT_BRIGHTNESS, QUIRK_LVDS_SSC_DISABLE, QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK, + QUIRK_480MHZ_CDCLK_STEP, }; void intel_init_quirks(struct drm_i915_private *i915); -- 2.25.1
[Intel-gfx] [RFC 2/4] drm/i915/display: Add 480 MHz CDCLK steps for RPL-U
A new step of 480MHz has been added on SKUs that have a RPL-U device id to support 120Hz displays more efficiently. Use a new quirk to identify the machine for which this change needs to be applied. BSpec: 55409 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 25 ++ 1 file changed, 25 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 0c107a38f9d0..f5df0a806765 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -38,6 +38,7 @@ #include "intel_pcode.h" #include "intel_psr.h" #include "vlv_sideband.h" +#include "intel_quirks.h" /** * DOC: CDCLK / RAWCLK @@ -1329,6 +1330,27 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = { {} }; +static const struct intel_cdclk_vals rplu_cdclk_table[] = { + { .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 }, + { .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 }, + { .refclk = 19200, .cdclk = 48, .divider = 2, .ratio = 50 }, + { .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 }, + { .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 }, + + { .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 }, + { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 }, + { .refclk = 24000, .cdclk = 48, .divider = 2, .ratio = 40 }, + { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 }, + { .refclk = 24400, .cdclk = 648000, .divider = 2, .ratio = 54 }, + + { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 }, + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 }, + { .refclk = 38400, .cdclk = 48, .divider = 2, .ratio = 25 }, + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 }, + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 }, + {} +}; + static const struct intel_cdclk_vals dg2_cdclk_table[] = { { .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, .waveform = 0x }, { .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, .waveform = 0x9248 }, @@ -3353,6 +3375,9 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) /* Wa_22011320316:adl-p[a0] */ if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) dev_priv->display.cdclk.table = adlp_a_step_cdclk_table; + /* BSpec: 55409 */ + else if (intel_has_quirk(dev_priv, QUIRK_480MHZ_CDCLK_STEP)) + dev_priv->display.cdclk.table = rplu_cdclk_table; else dev_priv->display.cdclk.table = adlp_cdclk_table; } else if (IS_ROCKETLAKE(dev_priv)) { -- 2.25.1
[Intel-gfx] [RFC 3/4] drm/i915: Initialize intel quirks before CDCLK initialization
With addition of new quirk QUIRK_480MHZ_CDCLK_STEP, it is imperative that quirks should be initialized before CDCLK initialization. Refactor the code accordingly. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_display.c | 2 -- drivers/gpu/drm/i915/i915_driver.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index e75b9b2a0e01..5c71fd83c25b 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8666,8 +8666,6 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915) INIT_WORK(&i915->display.atomic_helper.free_work, intel_atomic_helper_free_state_worker); - intel_init_quirks(i915); - intel_fbc_init(i915); return 0; diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index c1e427ba57ae..4d1cb46f9863 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -104,6 +104,7 @@ #include "intel_pm.h" #include "intel_region_ttm.h" #include "vlv_suspend.h" +#include "display/intel_quirks.h" static const struct drm_driver i915_drm_driver; @@ -388,6 +389,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv) if (ret < 0) goto err_gem; intel_irq_init(dev_priv); + intel_init_quirks(dev_priv); intel_init_display_hooks(dev_priv); intel_init_clock_gating_hooks(dev_priv); -- 2.25.1
[Intel-gfx] [RFC 4/4] drm/i915: Apply CDCLK quirk only on QS parts
RPL-U boards with ES silicon does not support the 480Mhz step of CDCLK. To differentiate between QS and ES part CPU brand string is the only feasible way as of now. ES parts have "Genuine Intel" in their brand string while QS parts have a more specific brand string, for ex. "13th Gen Intel(R) Core(TM) i5-1345U" BSpec: 55409 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_quirks.c | 32 +++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c index 0a30499835b3..a6d7a2430626 100644 --- a/drivers/gpu/drm/i915/display/intel_quirks.c +++ b/drivers/gpu/drm/i915/display/intel_quirks.c @@ -14,6 +14,25 @@ static void intel_set_quirk(struct drm_i915_private *i915, enum intel_quirk_id q i915->display.quirks.mask |= BIT(quirk); } +/* + * To differentiate between QS and ES part CPU brand string is the only feasible way + * as of now. ES parts have "Genuine Intel" in their brand string while QS parts have a more + * specific brand string, for ex. "13th Gen Intel(R) Core(TM) i5-1345U" + */ +static bool is_QS_part(void) +{ + struct cpuinfo_x86 *c; + unsigned int cpu = get_cpu(); + + c = &cpu_data(cpu); + put_cpu(); + + if (c->x86_model_id[0] && !strstr(c->x86_model_id, "Genuine Intel")) + return true; + + return false; +} + /* * Some machines (Lenovo U160) do not work with SSC on LVDS for some reason */ @@ -67,12 +86,19 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915) /* * A new step of 480MHz has been added on SKUs that have a RPL-U device id. - * This particular step is to better support 120Hz panels. + * This particular step is to better support 120Hz panels. In addition to + * identifying RPL-U device id, we need to make a distinction between ES and + * QS parts as this change comes only to QS parts. For this CPUID Brand + * string is used. 480Mhz step is only supported in SKUs which does not + * contain the string "Genuine Intel" in the Brand string. */ + static void quirk_480mhz_cdclk_step_hook(struct drm_i915_private *i915) { - intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP); - drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n"); + if (is_QS_part()) { + intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP); + drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n"); + } } struct intel_quirk { -- 2.25.1
[Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U
A new step of 480MHz has been added on SKUs that have an RPL-U device id. This particular step is to support 120Hz panels more efficiently. This patchset adds a new table to include this new CDCLK step. Details can be found in BSpec entry 55409. Create a new sub-platform to identify RPL-U which will enable us to make the differentiation during CDCLK initialization. Furthermore, we need to make a distinction between ES (Engineering Sample) and QS (Quality Sample) parts as this change comes only to QS parts. This version of the patch does not include this change as we are yet to make a decision if this particular part needs to be upstreamed.(see comments on previous versions) Chaitanya Kumar Borah (2): drm/i915: Add rplu sub platform drm/i915/display: Add 480 MHz CDCLK steps for RPL-U arch/x86/kernel/early-quirks.c | 1 + drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++ drivers/gpu/drm/i915/i915_drv.h| 2 ++ drivers/gpu/drm/i915/i915_pci.c| 1 + drivers/gpu/drm/i915/intel_device_info.c | 7 +++ drivers/gpu/drm/i915/intel_device_info.h | 1 + drivers/gpu/drm/i915/intel_step.c | 3 +++ include/drm/i915_pciids.h | 7 +-- 8 files changed, 43 insertions(+), 2 deletions(-) -- 2.25.1
[Intel-gfx] [RFC 1/2] drm/i915: Add rplu sub platform
Adding RPL-U as a sub platform. In RPL-U a new CDCLK step has been added so we need to make a distinction between RPL-P and RPL-U while CDCLK initialization. Adding a sub-platform, enables us to make this differentiation in the code. Signed-off-by: Chaitanya Kumar Borah --- arch/x86/kernel/early-quirks.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/intel_device_info.c | 7 +++ drivers/gpu/drm/i915/intel_device_info.h | 1 + drivers/gpu/drm/i915/intel_step.c| 3 +++ include/drm/i915_pciids.h| 7 +-- 7 files changed, 20 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index a6c1867fc7aa..1ba9926c8974 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -559,6 +559,7 @@ static const struct pci_device_id intel_early_ids[] __initconst = { INTEL_ADLN_IDS(&gen11_early_ops), INTEL_RPLS_IDS(&gen11_early_ops), INTEL_RPLP_IDS(&gen11_early_ops), + INTEL_RPLU_IDS(&gen11_early_ops), }; struct resource intel_graphics_stolen_res __ro_after_init = DEFINE_RES_MEM(0, 0); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 48fd82722f12..c88e514728a0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -619,6 +619,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_N) #define IS_ADLP_RPLP(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPL) +#define IS_ADLP_RPLU(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPLU) #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \ (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00) #define IS_BDW_ULT(dev_priv) \ diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 6cc65079b18d..e9f3b99b3e00 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1234,6 +1234,7 @@ static const struct pci_device_id pciidlist[] = { INTEL_DG1_IDS(&dg1_info), INTEL_RPLS_IDS(&adl_s_info), INTEL_RPLP_IDS(&adl_p_info), + INTEL_RPLU_IDS(&adl_p_info), INTEL_DG2_IDS(&dg2_info), INTEL_ATS_M_IDS(&ats_m_info), INTEL_MTL_IDS(&mtl_info), diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 849baf6c3b3c..88f3da63948b 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -201,6 +201,10 @@ static const u16 subplatform_rpl_ids[] = { INTEL_RPLP_IDS(0), }; +static const u16 subplatform_rplu_ids[] = { + INTEL_RPLU_IDS(0), +}; + static const u16 subplatform_g10_ids[] = { INTEL_DG2_G10_IDS(0), INTEL_ATS_M150_IDS(0), @@ -268,6 +272,9 @@ static void intel_device_info_subplatform_init(struct drm_i915_private *i915) } else if (find_devid(devid, subplatform_rpl_ids, ARRAY_SIZE(subplatform_rpl_ids))) { mask = BIT(INTEL_SUBPLATFORM_RPL); + } else if (find_devid(devid, subplatform_rplu_ids, + ARRAY_SIZE(subplatform_rplu_ids))) { + mask = BIT(INTEL_SUBPLATFORM_RPLU); } else if (find_devid(devid, subplatform_g10_ids, ARRAY_SIZE(subplatform_g10_ids))) { mask = BIT(INTEL_SUBPLATFORM_G10); diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index d588e5fd2eea..3e3ca5eb073f 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -127,6 +127,7 @@ enum intel_platform { * bit set */ #define INTEL_SUBPLATFORM_N1 +#define INTEL_SUBPLATFORM_RPLU 2 /* MTL */ #define INTEL_SUBPLATFORM_M0 diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c index 84a6fe736a3b..df75057eaa65 100644 --- a/drivers/gpu/drm/i915/intel_step.c +++ b/drivers/gpu/drm/i915/intel_step.c @@ -194,6 +194,9 @@ void intel_step_init(struct drm_i915_private *i915) } else if (IS_ADLP_RPLP(i915)) { revids = adlp_rplp_revids; size = ARRAY_SIZE(adlp_rplp_revids); + } else if (IS_ADLP_RPLU(i915)) { + revids = adlp_rplp_revids; + size = ARRAY_SIZE(adlp_rplp_revids); } else if (IS_ALDERLAKE_P(i915)) { revids = adlp_revids; size = ARRAY_SIZE(adlp_revids); diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 4a4c190f7698..87bb7e26dfb6 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -687,10 +687,13 @@ /* RPL-P */ #define INTE
[Intel-gfx] [RFC 2/2] drm/i915/display: Add 480 MHz CDCLK steps for RPL-U
A new step of 480MHz has been added on SKUs that have a RPL-U device id to support 120Hz displays more efficiently. Use a new quirk to identify the machine for which this change needs to be applied. BSpec: 55409 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++ 1 file changed, 23 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 0c107a38f9d0..a437ac446871 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1329,6 +1329,27 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = { {} }; +static const struct intel_cdclk_vals rplu_cdclk_table[] = { + { .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 }, + { .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 }, + { .refclk = 19200, .cdclk = 48, .divider = 2, .ratio = 50 }, + { .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 }, + { .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 }, + + { .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 }, + { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 }, + { .refclk = 24000, .cdclk = 48, .divider = 2, .ratio = 40 }, + { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 }, + { .refclk = 24400, .cdclk = 648000, .divider = 2, .ratio = 54 }, + + { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 }, + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 }, + { .refclk = 38400, .cdclk = 48, .divider = 2, .ratio = 25 }, + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 }, + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 }, + {} +}; + static const struct intel_cdclk_vals dg2_cdclk_table[] = { { .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, .waveform = 0x }, { .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, .waveform = 0x9248 }, @@ -3353,6 +3374,8 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) /* Wa_22011320316:adl-p[a0] */ if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) dev_priv->display.cdclk.table = adlp_a_step_cdclk_table; + else if (IS_ADLP_RPLU(dev_priv)) + dev_priv->display.cdclk.table = rplu_cdclk_table; else dev_priv->display.cdclk.table = adlp_cdclk_table; } else if (IS_ROCKETLAKE(dev_priv)) { -- 2.25.1
[PATCH] drm/i915/audio: Fix audio time stamp programming for DP
Intel hardware is capable of programming the Maud/Naud SDPs on its own based on real-time clocks. While doing so, it takes care of any deviations from the theoretical values. Programming the registers explicitly with static values can interfere with this logic. Therefore, let the HW decide the Maud and Naud SDPs on it's own. Cc: sta...@vger.kernel.org # v5.17 Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8097 Co-developed-by: Kai Vehmanen Signed-off-by: Kai Vehmanen Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_audio.c | 111 ++--- 1 file changed, 6 insertions(+), 105 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index 07e0c73204f3..12e2ba462077 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -76,19 +76,6 @@ struct intel_audio_funcs { struct intel_crtc_state *crtc_state); }; -/* DP N/M table */ -#define LC_810M81 -#define LC_540M54 -#define LC_270M27 -#define LC_162M162000 - -struct dp_aud_n_m { - int sample_rate; - int clock; - u16 m; - u16 n; -}; - struct hdmi_aud_ncts { int sample_rate; int clock; @@ -96,60 +83,6 @@ struct hdmi_aud_ncts { int cts; }; -/* Values according to DP 1.4 Table 2-104 */ -static const struct dp_aud_n_m dp_aud_n_m[] = { - { 32000, LC_162M, 1024, 10125 }, - { 44100, LC_162M, 784, 5625 }, - { 48000, LC_162M, 512, 3375 }, - { 64000, LC_162M, 2048, 10125 }, - { 88200, LC_162M, 1568, 5625 }, - { 96000, LC_162M, 1024, 3375 }, - { 128000, LC_162M, 4096, 10125 }, - { 176400, LC_162M, 3136, 5625 }, - { 192000, LC_162M, 2048, 3375 }, - { 32000, LC_270M, 1024, 16875 }, - { 44100, LC_270M, 784, 9375 }, - { 48000, LC_270M, 512, 5625 }, - { 64000, LC_270M, 2048, 16875 }, - { 88200, LC_270M, 1568, 9375 }, - { 96000, LC_270M, 1024, 5625 }, - { 128000, LC_270M, 4096, 16875 }, - { 176400, LC_270M, 3136, 9375 }, - { 192000, LC_270M, 2048, 5625 }, - { 32000, LC_540M, 1024, 33750 }, - { 44100, LC_540M, 784, 18750 }, - { 48000, LC_540M, 512, 11250 }, - { 64000, LC_540M, 2048, 33750 }, - { 88200, LC_540M, 1568, 18750 }, - { 96000, LC_540M, 1024, 11250 }, - { 128000, LC_540M, 4096, 33750 }, - { 176400, LC_540M, 3136, 18750 }, - { 192000, LC_540M, 2048, 11250 }, - { 32000, LC_810M, 1024, 50625 }, - { 44100, LC_810M, 784, 28125 }, - { 48000, LC_810M, 512, 16875 }, - { 64000, LC_810M, 2048, 50625 }, - { 88200, LC_810M, 1568, 28125 }, - { 96000, LC_810M, 1024, 16875 }, - { 128000, LC_810M, 4096, 50625 }, - { 176400, LC_810M, 3136, 28125 }, - { 192000, LC_810M, 2048, 16875 }, -}; - -static const struct dp_aud_n_m * -audio_config_dp_get_n_m(const struct intel_crtc_state *crtc_state, int rate) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) { - if (rate == dp_aud_n_m[i].sample_rate && - crtc_state->port_clock == dp_aud_n_m[i].clock) - return &dp_aud_n_m[i]; - } - - return NULL; -} - static const struct { int clock; u32 config; @@ -387,47 +320,15 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state) { struct drm_i915_private *i915 = to_i915(encoder->base.dev); - struct i915_audio_component *acomp = i915->display.audio.component; enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; - enum port port = encoder->port; - const struct dp_aud_n_m *nm; - int rate; - u32 tmp; - rate = acomp ? acomp->aud_sample_rate[port] : 0; - nm = audio_config_dp_get_n_m(crtc_state, rate); - if (nm) - drm_dbg_kms(&i915->drm, "using Maud %u, Naud %u\n", nm->m, - nm->n); - else - drm_dbg_kms(&i915->drm, "using automatic Maud, Naud\n"); - - tmp = intel_de_read(i915, HSW_AUD_CFG(cpu_transcoder)); - tmp &= ~AUD_CONFIG_N_VALUE_INDEX; - tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; - tmp |= AUD_CONFIG_N_VALUE_INDEX; - - if (nm) { - tmp &= ~AUD_CONFIG_N_MASK; - tmp |= AUD_CONFIG_N(nm->n); - tmp |= AUD_CONFIG_N_PROG_ENABLE; - } - - intel_de_write(i915, HSW_AUD_CFG(cpu_transcoder), tmp); - - tmp = intel_de_read(i915, HSW_AUD_M_CTS_ENABLE(cpu_transcoder)); - tmp &= ~AUD_CONFIG_M_MASK; - tmp &= ~AUD_M_CTS_M_VALUE_INDEX; - tmp &= ~AUD_M_CTS_M_
[PATCH] drm/i915/audio: Fix audio time stamp programming for DP
Intel hardware is capable of programming the Maud/Naud SDPs on its own based on real-time clocks. While doing so, it takes care of any deviations from the theoretical values. Programming the registers explicitly with static values can interfere with this logic. Therefore, let the HW decide the Maud and Naud SDPs on it's own. Cc: sta...@vger.kernel.org # v5.17 Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8097 Co-developed-by: Kai Vehmanen Signed-off-by: Kai Vehmanen Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_audio.c | 113 ++--- 1 file changed, 8 insertions(+), 105 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index 07e0c73204f3..ed81e1466c4b 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -76,19 +76,6 @@ struct intel_audio_funcs { struct intel_crtc_state *crtc_state); }; -/* DP N/M table */ -#define LC_810M81 -#define LC_540M54 -#define LC_270M27 -#define LC_162M162000 - -struct dp_aud_n_m { - int sample_rate; - int clock; - u16 m; - u16 n; -}; - struct hdmi_aud_ncts { int sample_rate; int clock; @@ -96,60 +83,6 @@ struct hdmi_aud_ncts { int cts; }; -/* Values according to DP 1.4 Table 2-104 */ -static const struct dp_aud_n_m dp_aud_n_m[] = { - { 32000, LC_162M, 1024, 10125 }, - { 44100, LC_162M, 784, 5625 }, - { 48000, LC_162M, 512, 3375 }, - { 64000, LC_162M, 2048, 10125 }, - { 88200, LC_162M, 1568, 5625 }, - { 96000, LC_162M, 1024, 3375 }, - { 128000, LC_162M, 4096, 10125 }, - { 176400, LC_162M, 3136, 5625 }, - { 192000, LC_162M, 2048, 3375 }, - { 32000, LC_270M, 1024, 16875 }, - { 44100, LC_270M, 784, 9375 }, - { 48000, LC_270M, 512, 5625 }, - { 64000, LC_270M, 2048, 16875 }, - { 88200, LC_270M, 1568, 9375 }, - { 96000, LC_270M, 1024, 5625 }, - { 128000, LC_270M, 4096, 16875 }, - { 176400, LC_270M, 3136, 9375 }, - { 192000, LC_270M, 2048, 5625 }, - { 32000, LC_540M, 1024, 33750 }, - { 44100, LC_540M, 784, 18750 }, - { 48000, LC_540M, 512, 11250 }, - { 64000, LC_540M, 2048, 33750 }, - { 88200, LC_540M, 1568, 18750 }, - { 96000, LC_540M, 1024, 11250 }, - { 128000, LC_540M, 4096, 33750 }, - { 176400, LC_540M, 3136, 18750 }, - { 192000, LC_540M, 2048, 11250 }, - { 32000, LC_810M, 1024, 50625 }, - { 44100, LC_810M, 784, 28125 }, - { 48000, LC_810M, 512, 16875 }, - { 64000, LC_810M, 2048, 50625 }, - { 88200, LC_810M, 1568, 28125 }, - { 96000, LC_810M, 1024, 16875 }, - { 128000, LC_810M, 4096, 50625 }, - { 176400, LC_810M, 3136, 28125 }, - { 192000, LC_810M, 2048, 16875 }, -}; - -static const struct dp_aud_n_m * -audio_config_dp_get_n_m(const struct intel_crtc_state *crtc_state, int rate) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) { - if (rate == dp_aud_n_m[i].sample_rate && - crtc_state->port_clock == dp_aud_n_m[i].clock) - return &dp_aud_n_m[i]; - } - - return NULL; -} - static const struct { int clock; u32 config; @@ -387,47 +320,17 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state) { struct drm_i915_private *i915 = to_i915(encoder->base.dev); - struct i915_audio_component *acomp = i915->display.audio.component; enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; - enum port port = encoder->port; - const struct dp_aud_n_m *nm; - int rate; - u32 tmp; - - rate = acomp ? acomp->aud_sample_rate[port] : 0; - nm = audio_config_dp_get_n_m(crtc_state, rate); - if (nm) - drm_dbg_kms(&i915->drm, "using Maud %u, Naud %u\n", nm->m, - nm->n); - else - drm_dbg_kms(&i915->drm, "using automatic Maud, Naud\n"); - - tmp = intel_de_read(i915, HSW_AUD_CFG(cpu_transcoder)); - tmp &= ~AUD_CONFIG_N_VALUE_INDEX; - tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; - tmp |= AUD_CONFIG_N_VALUE_INDEX; - if (nm) { - tmp &= ~AUD_CONFIG_N_MASK; - tmp |= AUD_CONFIG_N(nm->n); - tmp |= AUD_CONFIG_N_PROG_ENABLE; - } - - intel_de_write(i915, HSW_AUD_CFG(cpu_transcoder), tmp); - - tmp = intel_de_read(i915, HSW_AUD_M_CTS_ENABLE(cpu_transcoder)); - tmp &= ~AUD_CONFIG_M_MASK; - tmp &= ~AUD_M_CTS_M_VALUE_INDEX; - tmp &= ~AUD_M_CTS_M_
[PATCH 0/4] Cursor Fault Fix
This series is based on the following series by Maarten https://patchwork.freedesktop.org/series/132900/ It is an attempt at solving CI issues seen in the original series https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_132900v1/shard-rkl-3/igt@kms_universal_plane@cursor-fb-l...@pipe-b-hdmi-a-2.html Chaitanya Kumar Borah (1): drm: Add helper to check if there are pending vblank work Maarten Lankhorst (2): drm: Add drm_vblank_work_flush_all(). drm/i915: Use the same vblank worker for atomic unpin Ville Syrjälä (1): drm/i915: Use vblank worker to unpin old legacy cursor fb safely drivers/gpu/drm/drm_vblank_work.c | 46 +++ .../gpu/drm/i915/display/intel_atomic_plane.c | 13 +- .../gpu/drm/i915/display/intel_atomic_plane.h | 2 + drivers/gpu/drm/i915/display/intel_crtc.c | 31 + drivers/gpu/drm/i915/display/intel_cursor.c | 26 ++- drivers/gpu/drm/i915/display/intel_cursor.h | 3 ++ drivers/gpu/drm/i915/display/intel_display.c | 6 +++ .../drm/i915/display/intel_display_types.h| 3 ++ include/drm/drm_vblank_work.h | 3 ++ 9 files changed, 130 insertions(+), 3 deletions(-) -- 2.25.1
[PATCH 1/4] drm: Add drm_vblank_work_flush_all().
From: Maarten Lankhorst In some cases we want to flush all vblank work, right before vblank_off for example. Add a simple function to make this possible. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/drm_vblank_work.c | 22 ++ include/drm/drm_vblank_work.h | 2 ++ 2 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c index 4fe9b1d3b00f..83a81a5e8280 100644 --- a/drivers/gpu/drm/drm_vblank_work.c +++ b/drivers/gpu/drm/drm_vblank_work.c @@ -232,6 +232,28 @@ void drm_vblank_work_flush(struct drm_vblank_work *work) } EXPORT_SYMBOL(drm_vblank_work_flush); +/** + * drm_vblank_work_flush_all - flush all currently pending vblank work on crtc. + * @crtc: crtc for which vblank work to flush + * + * Wait until all currently queued vblank work on @crtc + * has finished executing once. + */ +void drm_vblank_work_flush_all(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_vblank_crtc *vblank = &dev->vblank[drm_crtc_index(crtc)]; + + spin_lock_irq(&dev->event_lock); + wait_event_lock_irq(vblank->work_wait_queue, + !waitqueue_active(&vblank->work_wait_queue), + dev->event_lock); + spin_unlock_irq(&dev->event_lock); + + kthread_flush_worker(vblank->worker); +} +EXPORT_SYMBOL(drm_vblank_work_flush_all); + /** * drm_vblank_work_init - initialize a vblank work item * @work: vblank work item diff --git a/include/drm/drm_vblank_work.h b/include/drm/drm_vblank_work.h index eb41d0810c4f..e04d436b7297 100644 --- a/include/drm/drm_vblank_work.h +++ b/include/drm/drm_vblank_work.h @@ -17,6 +17,7 @@ struct drm_crtc; * drm_vblank_work_init() * drm_vblank_work_cancel_sync() * drm_vblank_work_flush() + * drm_vblank_work_flush_all() */ struct drm_vblank_work { /** @@ -67,5 +68,6 @@ void drm_vblank_work_init(struct drm_vblank_work *work, struct drm_crtc *crtc, void (*func)(struct kthread_work *work)); bool drm_vblank_work_cancel_sync(struct drm_vblank_work *work); void drm_vblank_work_flush(struct drm_vblank_work *work); +void drm_vblank_work_flush_all(struct drm_crtc *crtc); #endif /* !_DRM_VBLANK_WORK_H_ */ -- 2.25.1
[PATCH 2/4] drm: Add helper to check if there are pending vblank work
Add helper to check if there are pending vblank work for a crtc which are yet to be scheduled. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/drm_vblank_work.c | 24 include/drm/drm_vblank_work.h | 1 + 2 files changed, 25 insertions(+) diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c index 83a81a5e8280..1137a66d4153 100644 --- a/drivers/gpu/drm/drm_vblank_work.c +++ b/drivers/gpu/drm/drm_vblank_work.c @@ -254,6 +254,30 @@ void drm_vblank_work_flush_all(struct drm_crtc *crtc) } EXPORT_SYMBOL(drm_vblank_work_flush_all); +/** + * drm_vblank_has_pending_work - Check if there are any currently pending vblank work on crtc. + * @crtc: crtc for which vblank work to flush + * + * Check if there are any pending vblank work on crtc, which are yet to be scheduled. + * + * Returns: + * %True if there are pending vblank workers, %false + * otherwise. + */ +bool drm_vblank_has_pending_work(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_vblank_crtc *vblank = &dev->vblank[drm_crtc_index(crtc)]; + bool is_pending = false; + + spin_lock_irq(&dev->event_lock); + is_pending = !list_empty(&vblank->pending_work); + spin_unlock_irq(&dev->event_lock); + + return is_pending; +} +EXPORT_SYMBOL(drm_vblank_has_pending_work); + /** * drm_vblank_work_init - initialize a vblank work item * @work: vblank work item diff --git a/include/drm/drm_vblank_work.h b/include/drm/drm_vblank_work.h index e04d436b7297..41ed27c8f74f 100644 --- a/include/drm/drm_vblank_work.h +++ b/include/drm/drm_vblank_work.h @@ -69,5 +69,6 @@ void drm_vblank_work_init(struct drm_vblank_work *work, struct drm_crtc *crtc, bool drm_vblank_work_cancel_sync(struct drm_vblank_work *work); void drm_vblank_work_flush(struct drm_vblank_work *work); void drm_vblank_work_flush_all(struct drm_crtc *crtc); +bool drm_vblank_has_pending_work(struct drm_crtc *crtc); #endif /* !_DRM_VBLANK_WORK_H_ */ -- 2.25.1
[PATCH 3/4] 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. In case we are shutting down the crtc after vblank workers are scheduled and before vblank arrives, drm_handle_vblank_works() never gets called. This will lead to cancellation of the vblank worker during drm_crtc_vblank_off(), therefore, wait one vblank if there are pending vblank workers. This patch is slightly reworked by Maarten. Cc: Maarten Lankhorst Signed-off-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_cursor.c | 26 +-- drivers/gpu/drm/i915/display/intel_display.c | 6 + .../drm/i915/display/intel_display_types.h| 3 +++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index 2118b87ccb10..c9963f5d546e 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -674,6 +674,17 @@ 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(&plane->base, &plane_state->uapi); +} + static int intel_legacy_cursor_update(struct drm_plane *_plane, struct drm_crtc *_crtc, @@ -817,14 +828,25 @@ intel_legacy_cursor_update(struct drm_plane *_plane, intel_psr_unlock(crtc_state); - intel_plane_unpin_fb(old_plane_state); + if (old_plane_state->ggtt_vma != new_plane_state->ggtt_vma) { + drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base, +intel_cursor_unpin_work); + + drm_vblank_work_schedule(&old_plane_state->unpin_work, + drm_crtc_accurate_vblank_count(&crtc->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(&crtc->base, &new_crtc_state->uapi); if (ret) intel_plane_destroy_state(&plane->base, &new_plane_state->uapi); - else + else if (old_plane_state) intel_plane_destroy_state(&plane->base, &old_plane_state->uapi); return ret; diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index ef986b508431..bd428e04236f 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -65,6 +65,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" @@ -6923,6 +6924,11 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) continue; intel_crtc_disable_planes(state, crtc); + + if(drm_vblank_has_pending_work(&crtc->base)) { + intel_crtc_wait_for_next_vblank(crtc); + drm_vblank_work_flush_all(&crtc->base); + } } /* 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 00eba3fbcdc6..e2ccec8d36ff 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -730,6 +730,9 @@ struct intel_plane_state { struct intel_fb_view view; u32 phys_dma_addr; /* for cursor_needs_physical */ + /* for legacy cursor fb unpin */ + struct drm_vblank_work unpin_work; + /* Plane pxp decryption state */ bool decrypt; -- 2.25.1
[PATCH 4/4] drm/i915: Use the same vblank worker for atomic unpin
From: Maarten Lankhorst In case of legacy cursor update, the cursor VMA needs to be unpinned only after vblank. This exceeds the lifetime of the whole atomic commit. Any trick I attempted to keep the atomic commit alive didn't work, as drm_atomic_helper_setup_commit() force throttles on any old commit that wasn't cleaned up. The only option remaining is to remove the plane from the atomic commit, and use the same path as the legacy cursor update to clean the state after vblank. Changes since previous version: - Call the memset for plane state immediately when scheduling vblank, this prevents a use-after-free in cursor cleanup. Signed-off-by: Maarten Lankhorst --- .../gpu/drm/i915/display/intel_atomic_plane.c | 13 +++- .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 31 +++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- drivers/gpu/drm/i915/display/intel_cursor.h | 3 ++ 5 files changed, 49 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index b083b985d170..7f65b9eb3ff2 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -42,6 +42,7 @@ #include "i915_reg.h" #include "intel_atomic_plane.h" #include "intel_cdclk.h" +#include "intel_cursor.h" #include "intel_display_rps.h" #include "intel_display_trace.h" #include "intel_display_types.h" @@ -1180,7 +1181,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane, intel_display_rps_mark_interactive(dev_priv, state, false); - /* Should only be called after a successful intel_prepare_plane_fb()! */ intel_plane_unpin_fb(old_plane_state); } @@ -1193,3 +1193,14 @@ void intel_plane_helper_add(struct intel_plane *plane) { drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs); } + +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state) +{ + if (!old_plane_state->ggtt_vma || + old_plane_state->ggtt_vma == new_plane_state->ggtt_vma) + return; + + drm_vblank_work_init(&old_plane_state->unpin_work, old_plane_state->uapi.crtc, +intel_cursor_unpin_work); +} diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 191dad0efc8e..5a897cf6fa02 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -66,5 +66,7 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state); void intel_plane_set_invisible(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); void intel_plane_helper_add(struct intel_plane *plane); +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, +struct intel_plane_state *new_plane_state); #endif /* __INTEL_ATOMIC_PLANE_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index 25593f6aae7d..7f935c88726e 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -500,6 +500,19 @@ void intel_pipe_update_start(struct intel_atomic_state *state, if (intel_crtc_needs_vblank_work(new_crtc_state)) intel_crtc_vblank_work_init(new_crtc_state); + if (state->base.legacy_cursor_update) { + struct intel_plane *plane; + struct intel_plane_state *old_plane_state, *new_plane_state; + int i; + + for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state, +new_plane_state, i) { + if (old_plane_state->uapi.crtc == &crtc->base) + intel_plane_init_cursor_vblank_work(old_plane_state, + new_plane_state); + } + } + intel_vblank_evade_init(old_crtc_state, new_crtc_state, &evade); if (drm_WARN_ON(&dev_priv->drm, drm_crtc_vblank_get(&crtc->base))) @@ -616,6 +629,24 @@ void intel_pipe_update_end(struct intel_atomic_state *state, new_crtc_state->uapi.event = NULL; } + if (state->base.legacy_cursor_update) { + struct intel_plane *plane; + struct intel_plane_state *old_plane_state; + int i; + + for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { + if (old_plane_state->uapi.crtc == &crtc->base && + old_plane_state->unpin_work.vblank) { + drm_vblank_
[core-for-CI PATCH] Revert "e1000e: move force SMBUS near the end of enable_ulp function"
This reverts commit bfd546a552e140b0a4c8a21527c39d6d21addb28. The commit seems to cause problems in suspend-resume tests [212.204897] e1000e :00:1f.6: PM: pci_pm_suspend(): e1000e_pm_suspend [e1000e] returns -2 [212.204928] e1000e :00:1f.6: PM: dpm_run_callback(): pci_pm_suspend returns -2 [212.204943] e1000e :00:1f.6: PM: failed to suspend async: error -2 [212.205092] PM: suspend of devices aborted after 302.254 msecs References: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14904/shard-dg2-4/igt@gem_ccs@suspend-res...@linear-compressed-compfmt0-lmem0-lmem0.html References: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11305 Signed-off-by: Chaitanya Kumar Borah --- drivers/net/ethernet/intel/e1000e/ich8lan.c | 22 - drivers/net/ethernet/intel/e1000e/netdev.c | 18 + 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c index 2e98a2a0bead..f9e94be36e97 100644 --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c @@ -1225,28 +1225,6 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool to_sx) } release: - /* Switching PHY interface always returns MDI error -* so disable retry mechanism to avoid wasting time -*/ - e1000e_disable_phy_retry(hw); - - /* Force SMBus mode in PHY */ - ret_val = e1000_read_phy_reg_hv_locked(hw, CV_SMB_CTRL, &phy_reg); - if (ret_val) { - e1000e_enable_phy_retry(hw); - hw->phy.ops.release(hw); - goto out; - } - phy_reg |= CV_SMB_CTRL_FORCE_SMBUS; - e1000_write_phy_reg_hv_locked(hw, CV_SMB_CTRL, phy_reg); - - e1000e_enable_phy_retry(hw); - - /* Force SMBus mode in MAC */ - mac_reg = er32(CTRL_EXT); - mac_reg |= E1000_CTRL_EXT_FORCE_SMBUS; - ew32(CTRL_EXT, mac_reg); - hw->phy.ops.release(hw); out: if (ret_val) diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index da5c59daf8ba..220d62fca55d 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -6623,6 +6623,7 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool runtime) struct e1000_hw *hw = &adapter->hw; u32 ctrl, ctrl_ext, rctl, status, wufc; int retval = 0; + u16 smb_ctrl; /* Runtime suspend should only enable wakeup for link changes */ if (runtime) @@ -6696,6 +6697,23 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool runtime) if (retval) return retval; } + + /* Force SMBUS to allow WOL */ + /* Switching PHY interface always returns MDI error +* so disable retry mechanism to avoid wasting time +*/ + e1000e_disable_phy_retry(hw); + + e1e_rphy(hw, CV_SMB_CTRL, &smb_ctrl); + smb_ctrl |= CV_SMB_CTRL_FORCE_SMBUS; + e1e_wphy(hw, CV_SMB_CTRL, smb_ctrl); + + e1000e_enable_phy_retry(hw); + + /* Force SMBus mode in MAC */ + ctrl_ext = er32(CTRL_EXT); + ctrl_ext |= E1000_CTRL_EXT_FORCE_SMBUS; + ew32(CTRL_EXT, ctrl_ext); } /* Ensure that the appropriate bits are set in LPI_CTRL -- 2.25.1
[PATCH] drm/i915/audio: Fix audio time stamp programming for DP
Intel hardware is capable of programming the Maud/Naud SDPs on its own based on real-time clocks. While doing so, it takes care of any deviations from the theoretical values. Programming the registers explicitly with static values can interfere with this logic. Therefore, let the HW decide the Maud and Naud SDPs on it's own. Fixes: 6014ac122ed0 ("drm/i915/audio: set proper N/M in modeset") Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8097 Co-developed-by: Kai Vehmanen Signed-off-by: Kai Vehmanen Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_audio.c | 113 ++--- 1 file changed, 8 insertions(+), 105 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index 07e0c73204f3..ed81e1466c4b 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -76,19 +76,6 @@ struct intel_audio_funcs { struct intel_crtc_state *crtc_state); }; -/* DP N/M table */ -#define LC_810M81 -#define LC_540M54 -#define LC_270M27 -#define LC_162M162000 - -struct dp_aud_n_m { - int sample_rate; - int clock; - u16 m; - u16 n; -}; - struct hdmi_aud_ncts { int sample_rate; int clock; @@ -96,60 +83,6 @@ struct hdmi_aud_ncts { int cts; }; -/* Values according to DP 1.4 Table 2-104 */ -static const struct dp_aud_n_m dp_aud_n_m[] = { - { 32000, LC_162M, 1024, 10125 }, - { 44100, LC_162M, 784, 5625 }, - { 48000, LC_162M, 512, 3375 }, - { 64000, LC_162M, 2048, 10125 }, - { 88200, LC_162M, 1568, 5625 }, - { 96000, LC_162M, 1024, 3375 }, - { 128000, LC_162M, 4096, 10125 }, - { 176400, LC_162M, 3136, 5625 }, - { 192000, LC_162M, 2048, 3375 }, - { 32000, LC_270M, 1024, 16875 }, - { 44100, LC_270M, 784, 9375 }, - { 48000, LC_270M, 512, 5625 }, - { 64000, LC_270M, 2048, 16875 }, - { 88200, LC_270M, 1568, 9375 }, - { 96000, LC_270M, 1024, 5625 }, - { 128000, LC_270M, 4096, 16875 }, - { 176400, LC_270M, 3136, 9375 }, - { 192000, LC_270M, 2048, 5625 }, - { 32000, LC_540M, 1024, 33750 }, - { 44100, LC_540M, 784, 18750 }, - { 48000, LC_540M, 512, 11250 }, - { 64000, LC_540M, 2048, 33750 }, - { 88200, LC_540M, 1568, 18750 }, - { 96000, LC_540M, 1024, 11250 }, - { 128000, LC_540M, 4096, 33750 }, - { 176400, LC_540M, 3136, 18750 }, - { 192000, LC_540M, 2048, 11250 }, - { 32000, LC_810M, 1024, 50625 }, - { 44100, LC_810M, 784, 28125 }, - { 48000, LC_810M, 512, 16875 }, - { 64000, LC_810M, 2048, 50625 }, - { 88200, LC_810M, 1568, 28125 }, - { 96000, LC_810M, 1024, 16875 }, - { 128000, LC_810M, 4096, 50625 }, - { 176400, LC_810M, 3136, 28125 }, - { 192000, LC_810M, 2048, 16875 }, -}; - -static const struct dp_aud_n_m * -audio_config_dp_get_n_m(const struct intel_crtc_state *crtc_state, int rate) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) { - if (rate == dp_aud_n_m[i].sample_rate && - crtc_state->port_clock == dp_aud_n_m[i].clock) - return &dp_aud_n_m[i]; - } - - return NULL; -} - static const struct { int clock; u32 config; @@ -387,47 +320,17 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state) { struct drm_i915_private *i915 = to_i915(encoder->base.dev); - struct i915_audio_component *acomp = i915->display.audio.component; enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; - enum port port = encoder->port; - const struct dp_aud_n_m *nm; - int rate; - u32 tmp; - - rate = acomp ? acomp->aud_sample_rate[port] : 0; - nm = audio_config_dp_get_n_m(crtc_state, rate); - if (nm) - drm_dbg_kms(&i915->drm, "using Maud %u, Naud %u\n", nm->m, - nm->n); - else - drm_dbg_kms(&i915->drm, "using automatic Maud, Naud\n"); - - tmp = intel_de_read(i915, HSW_AUD_CFG(cpu_transcoder)); - tmp &= ~AUD_CONFIG_N_VALUE_INDEX; - tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; - tmp |= AUD_CONFIG_N_VALUE_INDEX; - if (nm) { - tmp &= ~AUD_CONFIG_N_MASK; - tmp |= AUD_CONFIG_N(nm->n); - tmp |= AUD_CONFIG_N_PROG_ENABLE; - } - - intel_de_write(i915, HSW_AUD_CFG(cpu_transcoder), tmp); - - tmp = intel_de_read(i915, HSW_AUD_M_CTS_ENABLE(cpu_transcoder)); - tmp &= ~AUD_CONFIG_M_MASK; - tmp &=