Re: [Intel-gfx] [PATCH v5] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry
Zhenyu Wang 于2022年12月21日周三 11:01写道：
>
> On 2022.12.20 17:40:14 +0800, Zheng Wang wrote:
> > If intel_gvt_dma_map_guest_page failed, it will call
> > ppgtt_invalidate_spt, which will finally free the spt. But the
> > caller function ppgtt_populate_spt_by_guest_entry does not notice
> > that, it will free spt again in its error path.
>
> indent
Yeap :)
> > + if (ret)
> > + goto err;
> > sub_se.val64 = se->val64;
> >
> > /* Copy the PAT field from PDE. */
> > @@ -1231,6 +1229,18 @@ static int split_2MB_gtt_entry(struct intel_vgpu
> > *vgpu,
> > ops->set_pfn(se, sub_spt->shadow_page.mfn);
> > ppgtt_set_shadow_entry(spt, se, index);
> > return 0;
> > +err:
> > + /* Undone the existing mappings of DMA addr. */
>
> We need a verb here for Undo.
Get it.
>
> > + for_each_present_shadow_entry(sub_spt, _se, sub_index) {
> > + gvt_vdbg_mm("invalidate 4K entry\n");
> > + ppgtt_invalidate_pte(sub_spt, _se);
> > + }
> > + /* Release the new allocated spt. */
> > + trace_spt_change(sub_spt->vgpu->id, "release", sub_spt,
> > + sub_spt->guest_page.gfn, sub_spt->shadow_page.type);
> > + ppgtt_free_spt(sub_spt);
> > + sub_spt = NULL;
>
> Not need to reset local variable that has no use then.
>
> I'll handle these trivial fixes during the merge.
>
Very thanks for that.
Best regards,
Zheng Wang
[Intel-gfx] [PATCHv7] drm/i915/dp: change aux_ctl reg read to polling read
The busy timeout logic checks for the AUX BUSY, then waits for the timeout period and then after timeout reads the register for BUSY or Success. Instead replace interrupt with polling so as to read the AUX CTL register often before the timeout period. Looks like there might be some issue with interrupt-on-read. Hence changing the logic to polling read. v2: replace interrupt with polling read v3: use usleep_rang instead of msleep, updated commit msg v4: use intel_wait_for_regiter internal function v5: use __intel_de_wait_for_register with 500us slow and 10ms fast timeout v6: check return value of __intel_de_wait_for_register v7: using default 2us for intel_de_wait_for_register Signed-off-by: Arun R Murthy --- drivers/gpu/drm/i915/display/intel_dp_aux.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c index 91c93c93e5fc..5a176bfb10a2 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c @@ -41,20 +41,16 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp) i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp); const unsigned int timeout_ms = 10; u32 status; - bool done; - -#define C (((status = intel_de_read_notrace(i915, ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0) - done = wait_event_timeout(i915->display.gmbus.wait_queue, C, - msecs_to_jiffies_timeout(timeout_ms)); + int ret; - /* just trace the final value */ - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); + ret = __intel_de_wait_for_register(i915, ch_ctl, + DP_AUX_CH_CTL_SEND_BUSY, 0, + 2, timeout_ms, ); - if (!done) + if (ret == -ETIMEDOUT) drm_err(>drm, "%s: did not complete or timeout within %ums (status 0x%08x)\n", intel_dp->aux.name, timeout_ms, status); -#undef C return status; } -- 2.25.1
Re: [Intel-gfx] [PATCH v5] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry
On 2022.12.20 17:40:14 +0800, Zheng Wang wrote:
> If intel_gvt_dma_map_guest_page failed, it will call
> ppgtt_invalidate_spt, which will finally free the spt. But the
> caller function ppgtt_populate_spt_by_guest_entry does not notice
> that, it will free spt again in its error path.
indent
>
> Fix this by undoing the mapping of DMA address and freeing sub_spt.
> Besides, leave the handle of spt destroy to caller function instead of
> callee function when error occurs.
>
> Fixes: b901b252b6cf ("drm/i915/gvt: Add 2M huge gtt support")
> Signed-off-by: Zheng Wang
> ---
> v5:
> - remove unnecessary switch-case code for there is only one particular case,
> correct the unmap target from parent_spt to sub_spt.add more details in
> commit message. All suggested by Zhenyu
>
> v4:
> - fix by undo the mapping of DMA address and free sub_spt suggested by Zhi
>
> v3:
> - correct spelling mistake and remove unused variable suggested by Greg
>
> v2: https://lore.kernel.org/all/20221006165845.1735393-1-zyytlz...@163.com/
>
> v1: https://lore.kernel.org/all/20220928033340.1063949-1-zyytlz...@163.com/
> ---
> drivers/gpu/drm/i915/gvt/gtt.c | 18 ++
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 51e5e8fb505b..4d478a59eb7d 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1209,10 +1209,8 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
> for_each_shadow_entry(sub_spt, _se, sub_index) {
> ret = intel_gvt_dma_map_guest_page(vgpu, start_gfn + sub_index,
> PAGE_SIZE, _addr);
> - if (ret) {
> - ppgtt_invalidate_spt(spt);
> - return ret;
> - }
> + if (ret)
> + goto err;
> sub_se.val64 = se->val64;
>
> /* Copy the PAT field from PDE. */
> @@ -1231,6 +1229,18 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
> ops->set_pfn(se, sub_spt->shadow_page.mfn);
> ppgtt_set_shadow_entry(spt, se, index);
> return 0;
> +err:
> + /* Undone the existing mappings of DMA addr. */
We need a verb here for Undo.
> + for_each_present_shadow_entry(sub_spt, _se, sub_index) {
> + gvt_vdbg_mm("invalidate 4K entry\n");
> + ppgtt_invalidate_pte(sub_spt, _se);
> + }
> + /* Release the new allocated spt. */
> + trace_spt_change(sub_spt->vgpu->id, "release", sub_spt,
> + sub_spt->guest_page.gfn, sub_spt->shadow_page.type);
> + ppgtt_free_spt(sub_spt);
> + sub_spt = NULL;
Not need to reset local variable that has no use then.
I'll handle these trivial fixes during the merge.
Reviewed-by: Zhenyu Wang
thanks
> + return ret;
> }
>
> static int split_64KB_gtt_entry(struct intel_vgpu *vgpu,
> --
> 2.25.1
>
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH 1/1] drm/i915/pxp: Use drm_dbg if arb session failed due to fw version
Ignore this, use rev2 instead, apologies for the mistake. On Tue, 2022-12-20 at 14:03 -0800, Teres Alexis, Alan Previn wrote: > If PXP arb-session is being attempted on older hardware SKUs or > on hardware with older, unsupported, firmware versions, then don't > report the failure with a drm_error. Instead, look specifically for > the API-version error reply and drm_dbg that reply. In this case, the > user-space will eventually get a -ENODEV for the protected context > creation which is the correct behavior and we don't create unnecessary > drm_error's in our dmesg (for what is unsupported platforms). > > Signed-off-by: Alan Previn
[Intel-gfx] [PATCH v2 1/1] drm/i915/pxp: Use drm_dbg if arb session failed due to fw version
If PXP arb-session is being attempted on older hardware SKUs or
on hardware with older, unsupported, firmware versions, then don't
report the failure with a drm_error. Instead, look specifically for
the API-version error reply and drm_dbg that reply. In this case, the
user-space will eventually get a -ENODEV for the protected context
creation which is the correct behavior and we don't create unnecessary
drm_error's in our dmesg (for what is unsupported platforms).
Changes from prio revs:
v1 : - print incorrect version from input packet, not output.
Signed-off-by: Alan Previn
---
drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h | 1 +
drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 5 +
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
index c2f23394f9b8..aaa8187a0afb 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
@@ -17,6 +17,7 @@
*/
enum pxp_status {
PXP_STATUS_SUCCESS = 0x0,
+ PXP_STATUS_ERROR_API_VERSION = 0x1002,
PXP_STATUS_OP_NOT_PERMITTED = 0x4013
};
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
index d50354bfb993..9d084ed9cc50 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
@@ -298,6 +298,11 @@ int intel_pxp_tee_cmd_create_arb_session(struct intel_pxp
*pxp,
if (ret)
drm_err(>drm, "Failed to send tee msg ret=[%d]\n", ret);
+
+ else if (msg_out.header.status == PXP_STATUS_ERROR_API_VERSION)
+ drm_dbg(>drm, "PXP firmware version unsupported,
requested: "
+ "CMD-ID-[0x%08x] on API-Ver-[0x%08x]\n",
+ msg_in.header.command_id, msg_in.header.api_version);
else if (msg_out.header.status != 0x0)
drm_warn(>drm, "PXP firmware failed arb session init
request ret=[0x%08x]\n",
msg_out.header.status);
base-commit: cc44a1e87ea6b788868878295119398966f98a81
--
2.34.1
[Intel-gfx] [PATCH 1/1] drm/i915/pxp: Use drm_dbg if arb session failed due to fw version
If PXP arb-session is being attempted on older hardware SKUs or
on hardware with older, unsupported, firmware versions, then don't
report the failure with a drm_error. Instead, look specifically for
the API-version error reply and drm_dbg that reply. In this case, the
user-space will eventually get a -ENODEV for the protected context
creation which is the correct behavior and we don't create unnecessary
drm_error's in our dmesg (for what is unsupported platforms).
Signed-off-by: Alan Previn
---
drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h | 1 +
drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 5 +
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
index c2f23394f9b8..aaa8187a0afb 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h
@@ -17,6 +17,7 @@
*/
enum pxp_status {
PXP_STATUS_SUCCESS = 0x0,
+ PXP_STATUS_ERROR_API_VERSION = 0x1002,
PXP_STATUS_OP_NOT_PERMITTED = 0x4013
};
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
index d50354bfb993..1e2fef39a675 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
@@ -298,6 +298,11 @@ int intel_pxp_tee_cmd_create_arb_session(struct intel_pxp
*pxp,
if (ret)
drm_err(>drm, "Failed to send tee msg ret=[%d]\n", ret);
+
+ else if (msg_out.header.status == PXP_STATUS_ERROR_API_VERSION)
+ drm_dbg(>drm, "PXP firmware version unsupported,
requested: "
+ "CMD-ID-[0x%08x] on API-Ver-[0x%08x]\n",
+ msg_in.header.command_id, msg_out.header.api_version);
else if (msg_out.header.status != 0x0)
drm_warn(>drm, "PXP firmware failed arb session init
request ret=[0x%08x]\n",
msg_out.header.status);
base-commit: cc44a1e87ea6b788868878295119398966f98a81
--
2.34.1
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support of Tile4 to MTL
On 20.12.2022 18.29, Stanislav Lisovskiy wrote:
We have some Tile4 tests now skipping, which were
supposed to be working. So lets make them work, by
adding display_ver 14 as supported.
v2: - Remove "14" for Tile 4 CCS formats, as they
seem to be not supported by DG2(Juha-Pekka Heikkila)
DG2 ccs modifiers not supported on MTL. DG2 ccs modifiers are of course
supported on DG2.
- For generic Tile 4, the opposite - lets use -1
in order to make sure all the next gens support it by
default(Juha-Pekka Heikkila)
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/display/intel_fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
b/drivers/gpu/drm/i915/display/intel_fb.c
index 63137ae5ab217..93d0e46e54813 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -174,7 +174,7 @@ static const struct intel_modifier_desc intel_modifiers[] =
{
.plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_RC,
}, {
.modifier = I915_FORMAT_MOD_4_TILED,
- .display_ver = { 13, 13 },
+ .display_ver = { 13, -1 },
.plane_caps = INTEL_PLANE_CAP_TILING_4,
}, {
.modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS,
Reviewed-by: Juha-Pekka Heikkila
Re: [Intel-gfx] [PATCH v6 2/2] drm/i915/mtl: Limit scaler input to 4k in plane scaling
On Tue, Dec 20, 2022 at 02:07:24PM +0200, Luca Coelho wrote:
> From: Animesh Manna
>
> As part of die area reduction max input source modified to 4096
> for MTL so modified range check logic of scaler.
>
> Signed-off-by: Jos? Roberto de Souza
> Signed-off-by: Animesh Manna
> Signed-off-by: Luca Coelho
> ---
> drivers/gpu/drm/i915/display/skl_scaler.c | 31 +--
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
> b/drivers/gpu/drm/i915/display/skl_scaler.c
> index d7390067b7d4..6baa07142b03 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -103,6 +103,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
> bool force_detach,
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> const struct drm_display_mode *adjusted_mode =
> _state->hw.adjusted_mode;
> + int min_src_w, min_src_h, min_dst_w, min_dst_h;
> + int max_src_w, max_src_h, max_dst_w, max_dst_h;
>
> /*
>* Src coordinates are already rotated by 270 degrees for
> @@ -157,15 +159,28 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
> bool force_detach,
> return -EINVAL;
> }
>
> + min_src_w = SKL_MIN_SRC_W;
> + min_src_h = SKL_MIN_SRC_H;
> + min_dst_w = SKL_MIN_DST_W;
> + min_dst_h = SKL_MIN_DST_H;
> +
> + if (DISPLAY_VER(dev_priv) >= 11 && DISPLAY_VER(dev_priv) < 14) {
> + max_src_w = ICL_MAX_SRC_W;
> + max_src_h = ICL_MAX_SRC_H;
> + max_dst_w = ICL_MAX_DST_W;
> + max_dst_h = ICL_MAX_DST_H;
> + } else {
> + max_src_w = SKL_MAX_SRC_W;
> + max_src_h = SKL_MAX_SRC_H;
> + max_dst_w = SKL_MAX_DST_W;
> + max_dst_h = SKL_MAX_DST_H;
> + }
Bspec says max_src_w=4096, max_src_h=8192, max_dst_w=8192,
max_dst_h=8192.
> +
> /* range checks */
> - if (src_w < SKL_MIN_SRC_W || src_h < SKL_MIN_SRC_H ||
> - dst_w < SKL_MIN_DST_W || dst_h < SKL_MIN_DST_H ||
> - (DISPLAY_VER(dev_priv) >= 11 &&
> - (src_w > ICL_MAX_SRC_W || src_h > ICL_MAX_SRC_H ||
> - dst_w > ICL_MAX_DST_W || dst_h > ICL_MAX_DST_H)) ||
> - (DISPLAY_VER(dev_priv) < 11 &&
> - (src_w > SKL_MAX_SRC_W || src_h > SKL_MAX_SRC_H ||
> - dst_w > SKL_MAX_DST_W || dst_h > SKL_MAX_DST_H))) {
> + if (src_w < min_src_w || src_h < min_src_h ||
> + dst_w < min_dst_w || dst_h < min_dst_h ||
> + src_w > max_src_w || src_h > max_src_h ||
> + dst_w > max_dst_w || dst_h > max_dst_h) {
> drm_dbg_kms(_priv->drm,
> "scaler_user index %u.%u: src %ux%u dst %ux%u "
> "size is out of scaler range\n",
> --
> 2.38.1
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH v6 1/2] drm/i915/mtl: limit second scaler vertical scaling in ver >= 14
On Tue, Dec 20, 2022 at 02:07:23PM +0200, Luca Coelho wrote:
> In newer hardware versions (i.e. display version >= 14), the second
> scaler doesn't support vertical scaling.
>
> The current implementation of the scaling limits is simplified and
> only occurs when the planes are created, so we don't know which scaler
> is being used.
>
> In order to handle separate scaling limits for horizontal and vertical
> scaling, and different limits per scaler, split the checks in two
> phases. We first do a simple check during plane creation and use the
> best-case scenario (because we don't know the scaler that may be used
> at a later point) and then do a more specific check when the scalers
> are actually being set up.
>
> Signed-off-by: Luca Coelho
> ---
>
> In v2:
>* fix DRM_PLANE_NO_SCALING renamed macros;
>
> In v3:
>* No changes.
>
> In v4:
>* Got rid of the changes in the general planes max scale code;
>* Added a couple of FIXMEs;
>* Made intel_atomic_setup_scaler() return an int with errors;
>
> In v5:
>* Just resent with a cover letter.
>
> In v6:
>* Now the correct version again (same as v4).
>
>
> drivers/gpu/drm/i915/display/intel_atomic.c | 83 ++---
> 1 file changed, 73 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index 6621aa245caf..8373be283d8b 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -41,6 +41,7 @@
> #include "intel_global_state.h"
> #include "intel_hdcp.h"
> #include "intel_psr.h"
> +#include "intel_fb.h"
> #include "skl_universal_plane.h"
>
> /**
> @@ -310,11 +311,11 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
> kfree(crtc_state);
> }
>
> -static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state
> *scaler_state,
> - int num_scalers_need, struct intel_crtc
> *intel_crtc,
> - const char *name, int idx,
> - struct intel_plane_state *plane_state,
> - int *scaler_id)
> +static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state
> *scaler_state,
> + int num_scalers_need, struct intel_crtc
> *intel_crtc,
> + const char *name, int idx,
> + struct intel_plane_state *plane_state,
> + int *scaler_id)
> {
> struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> int j;
> @@ -334,7 +335,7 @@ static void intel_atomic_setup_scaler(struct
> intel_crtc_scaler_state *scaler_sta
>
> if (drm_WARN(_priv->drm, *scaler_id < 0,
>"Cannot find scaler for %s:%d\n", name, idx))
> - return;
> + return -EBUSY;
>
> /* set scaler mode */
> if (plane_state && plane_state->hw.fb &&
> @@ -375,9 +376,69 @@ static void intel_atomic_setup_scaler(struct
> intel_crtc_scaler_state *scaler_sta
> mode = SKL_PS_SCALER_MODE_DYN;
> }
>
> + /*
> + * FIXME: we should also check the scaler factors for pfit, so
> + * this shouldn't be tied directly to planes.
> + */
> + if (plane_state && plane_state->hw.fb) {
> + const struct drm_framebuffer *fb = plane_state->hw.fb;
> + struct drm_rect *src = _state->uapi.src;
> + struct drm_rect *dst = _state->uapi.dst;
Those can be const.
> + int hscale, vscale, max_vscale, max_hscale;
> +
> + /*
> + * FIXME: When two scalers are needed, but only one of
> + * them needs to downscale, we should make sure that
> + * the one that needs downscaling support is assigned
> + * as the first scaler, so we don't reject downscaling
> + * unnecessarily.
> + */
> +
> + if (DISPLAY_VER(dev_priv) >= 14) {
> + /*
> + * On versions 14 and up, only the first
> + * scaler supports a vertical scaling factor
> + * of more than 1.0, while a horizontal
> + * scaling factor of 3.0 is supported.
> + */
> + max_hscale = 0x3 - 1;
> + if (*scaler_id == 0)
> + max_vscale = 0x3 - 1;
> + else
> + max_vscale = 0x1;
> +
> + } else if (DISPLAY_VER(dev_priv) >= 10 ||
> +!intel_format_info_is_yuv_semiplanar(fb->format,
> fb->modifier)) {
> + max_hscale = 0x3 - 1;
> + max_vscale = 0x3 - 1;
> + } else {
> + max_hscale = 0x2 - 1;
> +
Re: [Intel-gfx] [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling read
On Tue, 20 Dec 2022, "Murthy, Arun R" wrote: >> -Original Message- >> From: Nikula, Jani >> Sent: Tuesday, December 20, 2022 9:33 PM >> To: Murthy, Arun R ; intel- >> g...@lists.freedesktop.org; ville.syrj...@linux.intel.com; Deak, Imre >> >> Subject: RE: [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling read >> >> On Mon, 19 Dec 2022, "Murthy, Arun R" wrote: >> > Any comments? >> >> From #intel-gfx: >> >> bashing the hw for 500 usec seems a bit harsh >> >> Which is true. The default for intel_de_wait_for_register() is 2 us. Should >> probably stick to that. > > Recommendation as per windows code base is 50us interval for polling > the register, so will change it to 50us. The parameter is *not* the interval. It's the timeout for fast wait in a loop with just cpu_relax() in between. So please keep it 2 us. If the condition doesn't happen in the fast timeout, it'll fall back to the slow wait (in this case 10 ms timeout), which starts off with 10 us interval, doubling on every poll until it's above 1000 us. BR, Jani. > > Thanks and Regards, > Arun R Murthy > >> >> BR, >> Jani. >> >> >> > >> > Thanks and Regards, >> > Arun R Murthy >> > >> > >> >> -Original Message- >> >> From: Murthy, Arun R >> >> Sent: Thursday, December 15, 2022 4:44 PM >> >> To: intel-gfx@lists.freedesktop.org; ville.syrj...@linux.intel.com; >> >> Nikula, Jani ; Deak, Imre >> >> >> >> Cc: Murthy, Arun R >> >> Subject: [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling >> >> read >> >> >> >> The busy timeout logic checks for the AUX BUSY, then waits for the >> >> timeout period and then after timeout reads the register for BUSY or >> Success. >> >> Instead replace interrupt with polling so as to read the AUX CTL >> >> register often before the timeout period. Looks like there might be >> >> some issue with interrupt-on-read. Hence changing the logic to polling >> read. >> >> >> >> v2: replace interrupt with polling read >> >> v3: use usleep_rang instead of msleep, updated commit msg >> >> v4: use intel_wait_for_regiter internal function >> >> v5: use __intel_de_wait_for_register with 500us slow and 10ms fast >> >> timeout >> >> v6: check return value of __intel_de_wait_for_register >> >> >> >> Signed-off-by: Arun R Murthy >> >> --- >> >> drivers/gpu/drm/i915/display/intel_dp_aux.c | 14 +- >> >> 1 file changed, 5 insertions(+), 9 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c >> >> b/drivers/gpu/drm/i915/display/intel_dp_aux.c >> >> index 91c93c93e5fc..973dadecf712 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c >> >> @@ -41,20 +41,16 @@ intel_dp_aux_wait_done(struct intel_dp >> *intel_dp) >> >> i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp); >> >> const unsigned int timeout_ms = 10; >> >> u32 status; >> >> - bool done; >> >> - >> >> -#define C (((status = intel_de_read_notrace(i915, ch_ctl)) & >> >> DP_AUX_CH_CTL_SEND_BUSY) == 0) >> >> - done = wait_event_timeout(i915->display.gmbus.wait_queue, C, >> >> - msecs_to_jiffies_timeout(timeout_ms)); >> >> + int ret; >> >> >> >> - /* just trace the final value */ >> >> - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); >> >> + ret = __intel_de_wait_for_register(i915, ch_ctl, >> >> +DP_AUX_CH_CTL_SEND_BUSY, 0, >> >> +500, timeout_ms, ); >> >> >> >> - if (!done) >> >> + if (ret == -ETIMEDOUT) >> >> drm_err(>drm, >> >> "%s: did not complete or timeout within %ums >> >> (status 0x%08x)\n", >> >> intel_dp->aux.name, timeout_ms, status); >> >> -#undef C >> >> >> >> return status; >> >> } >> >> -- >> >> 2.25.1 >> > >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths
As we do not require specific versions anymore, change the convention
for blob filenames to stop using version numbers. This simplifies code
maintenance, since we do not need to keep updating blob paths for new
DMC releases, and also makes DMC loading compatible with systems that do
not have the latest firmware release.
References: https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
Signed-off-by: Gustavo Sousa
---
drivers/gpu/drm/i915/display/intel_dmc.c | 98
1 file changed, 82 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
b/drivers/gpu/drm/i915/display/intel_dmc.c
index 4124b3d37110..b11f0f451dd7 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -42,51 +42,70 @@
#define DMC_VERSION_MAJOR(version) ((version) >> 16)
#define DMC_VERSION_MINOR(version) ((version) & 0x)
-#define DMC_PATH(platform, major, minor) \
- "i915/" \
- __stringify(platform) "_dmc_ver" \
- __stringify(major) "_" \
+#define DMC_PATH(platform) \
+ "i915/" __stringify(platform) "_dmc.bin"
+
+/*
+ * New DMC additions should not use this. This is used solely to remain
+ * compatible with systems that have not yet updated DMC blobs to use
+ * unversioned file names.
+ */
+#define DMC_LEGACY_PATH(platform, major, minor) \
+ "i915/" \
+ __stringify(platform) "_dmc_ver"\
+ __stringify(major) "_" \
__stringify(minor) ".bin"
#define DISPLAY_VER13_DMC_MAX_FW_SIZE 0x2
#define DISPLAY_VER12_DMC_MAX_FW_SIZE ICL_DMC_MAX_FW_SIZE
-#define DG2_DMC_PATH DMC_PATH(dg2, 2, 08)
+#define DG2_DMC_PATH DMC_PATH(dg2)
+#define DG2_DMC_LEGACY_PATHDMC_LEGACY_PATH(dg2, 2, 08)
MODULE_FIRMWARE(DG2_DMC_PATH);
-#define ADLP_DMC_PATH DMC_PATH(adlp, 2, 16)
+#define ADLP_DMC_PATH DMC_PATH(adlp)
+#define ADLP_DMC_LEGACY_PATH DMC_LEGACY_PATH(adlp, 2, 16)
MODULE_FIRMWARE(ADLP_DMC_PATH);
-#define ADLS_DMC_PATH DMC_PATH(adls, 2, 01)
+#define ADLS_DMC_PATH DMC_PATH(adls)
+#define ADLS_DMC_LEGACY_PATH DMC_LEGACY_PATH(adls, 2, 01)
MODULE_FIRMWARE(ADLS_DMC_PATH);
-#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02)
+#define DG1_DMC_PATH DMC_PATH(dg1)
+#define DG1_DMC_LEGACY_PATHDMC_LEGACY_PATH(dg1, 2, 02)
MODULE_FIRMWARE(DG1_DMC_PATH);
-#define RKL_DMC_PATH DMC_PATH(rkl, 2, 03)
+#define RKL_DMC_PATH DMC_PATH(rkl)
+#define RKL_DMC_LEGACY_PATHDMC_LEGACY_PATH(rkl, 2, 03)
MODULE_FIRMWARE(RKL_DMC_PATH);
-#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12)
+#define TGL_DMC_PATH DMC_PATH(tgl)
+#define TGL_DMC_LEGACY_PATHDMC_LEGACY_PATH(tgl, 2, 12)
MODULE_FIRMWARE(TGL_DMC_PATH);
-#define ICL_DMC_PATH DMC_PATH(icl, 1, 09)
+#define ICL_DMC_PATH DMC_PATH(icl)
+#define ICL_DMC_LEGACY_PATHDMC_LEGACY_PATH(icl, 1, 09)
#define ICL_DMC_MAX_FW_SIZE0x6000
MODULE_FIRMWARE(ICL_DMC_PATH);
-#define GLK_DMC_PATH DMC_PATH(glk, 1, 04)
+#define GLK_DMC_PATH DMC_PATH(glk)
+#define GLK_DMC_LEGACY_PATHDMC_LEGACY_PATH(glk, 1, 04)
#define GLK_DMC_MAX_FW_SIZE0x4000
MODULE_FIRMWARE(GLK_DMC_PATH);
-#define KBL_DMC_PATH DMC_PATH(kbl, 1, 04)
+#define KBL_DMC_PATH DMC_PATH(kbl)
+#define KBL_DMC_LEGACY_PATHDMC_LEGACY_PATH(kbl, 1, 04)
#define KBL_DMC_MAX_FW_SIZEBXT_DMC_MAX_FW_SIZE
MODULE_FIRMWARE(KBL_DMC_PATH);
-#define SKL_DMC_PATH DMC_PATH(skl, 1, 27)
+#define SKL_DMC_PATH DMC_PATH(skl)
+#define SKL_DMC_LEGACY_PATHDMC_LEGACY_PATH(skl, 1, 27)
#define SKL_DMC_MAX_FW_SIZEBXT_DMC_MAX_FW_SIZE
MODULE_FIRMWARE(SKL_DMC_PATH);
-#define BXT_DMC_PATH DMC_PATH(bxt, 1, 07)
+#define BXT_DMC_PATH DMC_PATH(bxt)
+#define BXT_DMC_LEGACY_PATHDMC_LEGACY_PATH(bxt, 1, 07)
#define BXT_DMC_MAX_FW_SIZE0x3000
MODULE_FIRMWARE(BXT_DMC_PATH);
@@ -821,16 +840,63 @@ static void intel_dmc_runtime_pm_put(struct
drm_i915_private *dev_priv)
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT, wakeref);
}
+static const char *dmc_legacy_path(struct drm_i915_private *i915)
+{
+ if (IS_DG2(i915)) {
+ return DG2_DMC_LEGACY_PATH;
+ } else if (IS_ALDERLAKE_P(i915)) {
+ return ADLP_DMC_LEGACY_PATH;
+ } else if (IS_ALDERLAKE_S(i915)) {
+ return ADLS_DMC_LEGACY_PATH;
+ } else if (IS_DG1(i915)) {
+ return DG1_DMC_LEGACY_PATH;
+ } else if (IS_ROCKETLAKE(i915)) {
+ return
[Intel-gfx] [PATCH 1/2] drm/i915/dmc: Do not require specific versions
Currently there is no DMC version requirement with respect to how i915
interacts with it and new versions of the firmware serve as drop-in
replacements of older ones. As such, do not require specific versions.
References: https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
Signed-off-by: Gustavo Sousa
---
drivers/gpu/drm/i915/display/intel_dmc.c | 35
drivers/gpu/drm/i915/display/intel_dmc.h | 1 -
2 files changed, 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
b/drivers/gpu/drm/i915/display/intel_dmc.c
index 905b5dcdca14..4124b3d37110 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -53,51 +53,40 @@
#define DISPLAY_VER12_DMC_MAX_FW_SIZE ICL_DMC_MAX_FW_SIZE
#define DG2_DMC_PATH DMC_PATH(dg2, 2, 08)
-#define DG2_DMC_VERSION_REQUIRED DMC_VERSION(2, 8)
MODULE_FIRMWARE(DG2_DMC_PATH);
#define ADLP_DMC_PATH DMC_PATH(adlp, 2, 16)
-#define ADLP_DMC_VERSION_REQUIRED DMC_VERSION(2, 16)
MODULE_FIRMWARE(ADLP_DMC_PATH);
#define ADLS_DMC_PATH DMC_PATH(adls, 2, 01)
-#define ADLS_DMC_VERSION_REQUIRED DMC_VERSION(2, 1)
MODULE_FIRMWARE(ADLS_DMC_PATH);
#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02)
-#define DG1_DMC_VERSION_REQUIRED DMC_VERSION(2, 2)
MODULE_FIRMWARE(DG1_DMC_PATH);
#define RKL_DMC_PATH DMC_PATH(rkl, 2, 03)
-#define RKL_DMC_VERSION_REQUIRED DMC_VERSION(2, 3)
MODULE_FIRMWARE(RKL_DMC_PATH);
#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12)
-#define TGL_DMC_VERSION_REQUIRED DMC_VERSION(2, 12)
MODULE_FIRMWARE(TGL_DMC_PATH);
#define ICL_DMC_PATH DMC_PATH(icl, 1, 09)
-#define ICL_DMC_VERSION_REQUIRED DMC_VERSION(1, 9)
#define ICL_DMC_MAX_FW_SIZE0x6000
MODULE_FIRMWARE(ICL_DMC_PATH);
#define GLK_DMC_PATH DMC_PATH(glk, 1, 04)
-#define GLK_DMC_VERSION_REQUIRED DMC_VERSION(1, 4)
#define GLK_DMC_MAX_FW_SIZE0x4000
MODULE_FIRMWARE(GLK_DMC_PATH);
#define KBL_DMC_PATH DMC_PATH(kbl, 1, 04)
-#define KBL_DMC_VERSION_REQUIRED DMC_VERSION(1, 4)
#define KBL_DMC_MAX_FW_SIZEBXT_DMC_MAX_FW_SIZE
MODULE_FIRMWARE(KBL_DMC_PATH);
#define SKL_DMC_PATH DMC_PATH(skl, 1, 27)
-#define SKL_DMC_VERSION_REQUIRED DMC_VERSION(1, 27)
#define SKL_DMC_MAX_FW_SIZEBXT_DMC_MAX_FW_SIZE
MODULE_FIRMWARE(SKL_DMC_PATH);
#define BXT_DMC_PATH DMC_PATH(bxt, 1, 07)
-#define BXT_DMC_VERSION_REQUIRED DMC_VERSION(1, 7)
#define BXT_DMC_MAX_FW_SIZE0x3000
MODULE_FIRMWARE(BXT_DMC_PATH);
@@ -765,17 +754,6 @@ static u32 parse_dmc_fw_css(struct intel_dmc *dmc,
return 0;
}
- if (dmc->required_version &&
- css_header->version != dmc->required_version) {
- drm_info(>drm, "Refusing to load DMC firmware v%u.%u,"
-" please use v%u.%u\n",
-DMC_VERSION_MAJOR(css_header->version),
-DMC_VERSION_MINOR(css_header->version),
-DMC_VERSION_MAJOR(dmc->required_version),
-DMC_VERSION_MINOR(dmc->required_version));
- return 0;
- }
-
dmc->version = css_header->version;
return sizeof(struct intel_css_header);
@@ -903,49 +881,38 @@ void intel_dmc_ucode_init(struct drm_i915_private
*dev_priv)
if (IS_DG2(dev_priv)) {
dmc->fw_path = DG2_DMC_PATH;
- dmc->required_version = DG2_DMC_VERSION_REQUIRED;
dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
} else if (IS_ALDERLAKE_P(dev_priv)) {
dmc->fw_path = ADLP_DMC_PATH;
- dmc->required_version = ADLP_DMC_VERSION_REQUIRED;
dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
} else if (IS_ALDERLAKE_S(dev_priv)) {
dmc->fw_path = ADLS_DMC_PATH;
- dmc->required_version = ADLS_DMC_VERSION_REQUIRED;
dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
} else if (IS_DG1(dev_priv)) {
dmc->fw_path = DG1_DMC_PATH;
- dmc->required_version = DG1_DMC_VERSION_REQUIRED;
dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
} else if (IS_ROCKETLAKE(dev_priv)) {
dmc->fw_path = RKL_DMC_PATH;
- dmc->required_version = RKL_DMC_VERSION_REQUIRED;
dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
} else if (IS_TIGERLAKE(dev_priv)) {
dmc->fw_path = TGL_DMC_PATH;
- dmc->required_version = TGL_DMC_VERSION_REQUIRED;
dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
} else if (DISPLAY_VER(dev_priv) == 11) {
dmc->fw_path = ICL_DMC_PATH;
- dmc->required_version =
[Intel-gfx] [PATCH 0/2] drm/i915/dmc: Make firmware loading backwards-compatible
This patch series changes DMC loading to be backwards-compatible by removing version checking and loading blobs from unversioned filenames. Should this be accepted, the next step would be to update linux-firmware to start using the unversioned filenames. That said, this change still allows to use the previously versioned paths as fallback, allowing DMC loading to still work with the current state of linux-firmware. Signed-off-by: Gustavo Sousa Gustavo Sousa (2): drm/i915/dmc: Do not require specific versions drm/i915/dmc: Use unversioned firmware paths drivers/gpu/drm/i915/display/intel_dmc.c | 133 ++- drivers/gpu/drm/i915/display/intel_dmc.h | 1 - 2 files changed, 82 insertions(+), 52 deletions(-) -- 2.38.1
Re: [Intel-gfx] [PATCH] treewide: Convert del_timer*() to timer_shutdown*()
On Tue 2022-12-20 13:45:19, Steven Rostedt wrote: > [ > Linus, > > I ran the script against your latest master branch: > commit b6bb9676f2165d518b35ba3bea5f1fcfc0d969bf > > As the timer_shutdown*() code is now in your tree, I figured > we can start doing the conversions. At least add the trivial ones > now as Thomas suggested that this gets applied at the end of the > merge window, to avoid conflicts with linux-next during the > development cycle. I can wait to Friday to run it again, and > resubmit. > > What is the best way to handle this? > ] > > From: "Steven Rostedt (Google)" > > Due to several bugs caused by timers being re-armed after they are > shutdown and just before they are freed, a new state of timers was added > called "shutdown". After a timer is set to this state, then it can no > longer be re-armed. > > The following script was run to find all the trivial locations where > del_timer() or del_timer_sync() is called in the same function that the > object holding the timer is freed. It also ignores any locations where the > timer->function is modified between the del_timer*() and the free(), as > that is not considered a "trivial" case. > > This was created by using a coccinelle script and the following commands: LED parts looks good to me. Getting it in just before -rc1 would be best solution for me. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH] drm/i915/display: Fix a use-after-free when intel_edp_init_connector fails
On Tue, Dec 20, 2022 at 02:40:47PM +0200, Jani Nikula wrote:
> On Tue, 20 Dec 2022, Maarten Lankhorst
> wrote:
> > We enable the DP aux channel during probe, but may free the connector
> > soon afterwards. Ensure the DP aux display power put is completed before
> > everything is freed, to prevent a use-after-free in icl_aux_pw_to_phy(),
> > called from icl_combo_phy_aux_power_well_disable.
>
> Feels like the placement of the intel_display_power_flush_work_sync()
> call in intel_dp_aux_fini() is a bit arbitrary.
>
> If we add it in intel_dp_aux_fini(), the async and sync waits will both
> be called on the regular encoder destroy path.
Yes, calling intel_display_power_flush_work() from the error handler at
the end of intel_dp_init_connector() would be better.
> Maybe both intel_ddi_encoder_destroy() and intel_dp_encoder_destroy()
> should call intel_display_power_flush_work_sync(), instead of async,
intel_display_power_flush_work() ensures that power wells without a
reference held are disabled when it returns, so no need to call the
_sync() version for encoders (the _sync() version ensures in addition
during driver unloading that the work function is not running).
> and maybe the error paths should call those functions instead of just
> drm_encoder_cleanup()?
Yes, the cleanup in those functions could be shared with the error
handling in g4x_dp_init() and intel_ddi_init(), except kfree(dig_port)
which also happens if drm_encoder_init() fails.
For this intel_pps_vdd_off_sync() / intel_dp_aux_fini() would also
happen later at the end of g4x_dp_init()/intel_ddi_init(), I guess
that's ok.
I wonder if not handling drm_encoder_init() error in intel_ddi_init()
was on purpose, can't see a reason for it.
> Imre?
>
>
> BR,
> Jani.
>
>
> >
> > Signed-off-by: Maarten Lankhorst
> > ---
> > drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
> > drivers/gpu/drm/i915/display/intel_display_power.h | 1 +
> > drivers/gpu/drm/i915/display/intel_dp_aux.c| 2 ++
> > 3 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 04915f85a0df..0edb5532461f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -776,7 +776,7 @@ void intel_display_power_flush_work(struct
> > drm_i915_private *i915)
> > * Like intel_display_power_flush_work(), but also ensure that the work
> > * handler function is not running any more when this function returns.
> > */
> > -static void
> > +void
> > intel_display_power_flush_work_sync(struct drm_i915_private *i915)
> > {
> > struct i915_power_domains *power_domains = >display.power.domains;
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h
> > b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index 7136ea3f233e..dc10ee0519e6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -188,6 +188,7 @@ void __intel_display_power_put_async(struct
> > drm_i915_private *i915,
> > enum intel_display_power_domain domain,
> > intel_wakeref_t wakeref);
> > void intel_display_power_flush_work(struct drm_i915_private *i915);
> > +void intel_display_power_flush_work_sync(struct drm_i915_private *i915);
> > #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> > void intel_display_power_put(struct drm_i915_private *dev_priv,
> > enum intel_display_power_domain domain,
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > index f1835c74bff0..1006dddad2d5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > @@ -680,6 +680,8 @@ void intel_dp_aux_fini(struct intel_dp *intel_dp)
> > if (cpu_latency_qos_request_active(_dp->pm_qos))
> > cpu_latency_qos_remove_request(_dp->pm_qos);
> >
> > + /* Ensure async work from intel_dp_aux_xfer() is flushed before we
> > clean up */
> > + intel_display_power_flush_work_sync(dp_to_i915(intel_dp));
> > kfree(intel_dp->aux.name);
> > }
>
> --
> Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PATCH] treewide: Convert del_timer*() to timer_shutdown*()
[ Linus, I ran the script against your latest master branch: commit b6bb9676f2165d518b35ba3bea5f1fcfc0d969bf As the timer_shutdown*() code is now in your tree, I figured we can start doing the conversions. At least add the trivial ones now as Thomas suggested that this gets applied at the end of the merge window, to avoid conflicts with linux-next during the development cycle. I can wait to Friday to run it again, and resubmit. What is the best way to handle this? ] From: "Steven Rostedt (Google)" Due to several bugs caused by timers being re-armed after they are shutdown and just before they are freed, a new state of timers was added called "shutdown". After a timer is set to this state, then it can no longer be re-armed. The following script was run to find all the trivial locations where del_timer() or del_timer_sync() is called in the same function that the object holding the timer is freed. It also ignores any locations where the timer->function is modified between the del_timer*() and the free(), as that is not considered a "trivial" case. This was created by using a coccinelle script and the following commands: $ cat timer.cocci @@ expression ptr, slab; identifier timer, rfield; @@ ( - del_timer(>timer); + timer_shutdown(>timer); | - del_timer_sync(>timer); + timer_shutdown_sync(>timer); ) ... when strict when != ptr->timer ( kfree_rcu(ptr, rfield); | kmem_cache_free(slab, ptr); | kfree(ptr); ) $ spatch timer.cocci . > /tmp/t.patch $ patch -p1 < /tmp/t.patch Link: https://lore.kernel.org/lkml/20221123201306.823305...@linutronix.de/ Signed-off-by: Steven Rostedt (Google) --- arch/sh/drivers/push-switch.c| 2 +- block/blk-iocost.c | 2 +- block/blk-iolatency.c| 2 +- block/kyber-iosched.c| 2 +- drivers/acpi/apei/ghes.c | 2 +- drivers/atm/idt77252.c | 6 +++--- drivers/block/drbd/drbd_main.c | 2 +- drivers/block/loop.c | 2 +- drivers/bluetooth/hci_bcsp.c | 2 +- drivers/gpu/drm/i915/i915_sw_fence.c | 2 +- drivers/hid/hid-wiimote-core.c | 2 +- drivers/input/keyboard/locomokbd.c | 2 +- drivers/input/keyboard/omap-keypad.c | 2 +- drivers/input/mouse/alps.c | 2 +- drivers/isdn/mISDN/l1oip_core.c | 4 ++-- drivers/isdn/mISDN/timerdev.c| 4 ++-- drivers/leds/trigger/ledtrig-activity.c | 2 +- drivers/leds/trigger/ledtrig-heartbeat.c | 2 +- drivers/leds/trigger/ledtrig-pattern.c | 2 +- drivers/leds/trigger/ledtrig-transient.c | 2 +- drivers/media/pci/ivtv/ivtv-driver.c | 2 +- drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 16 drivers/media/usb/s2255/s2255drv.c | 4 ++-- drivers/net/ethernet/intel/i40e/i40e_main.c | 6 +++--- drivers/net/ethernet/marvell/sky2.c | 2 +- drivers/net/ethernet/sun/sunvnet.c | 2 +- drivers/net/usb/sierra_net.c | 2 +- .../broadcom/brcm80211/brcmfmac/btcoex.c | 2 +- drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 2 +- drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 2 +- drivers/net/wireless/intersil/hostap/hostap_ap.c | 2 +- drivers/net/wireless/marvell/mwifiex/main.c | 2 +- drivers/net/wireless/microchip/wilc1000/hif.c| 6 +++--- drivers/nfc/pn533/pn533.c| 2 +- drivers/nfc/pn533/uart.c | 2 +- drivers/pcmcia/bcm63xx_pcmcia.c | 2 +- drivers/pcmcia/electra_cf.c | 2 +- drivers/pcmcia/omap_cf.c | 2 +- drivers/pcmcia/pd6729.c | 4 ++-- drivers/pcmcia/yenta_socket.c| 4 ++-- drivers/scsi/qla2xxx/qla_edif.c | 4 ++-- .../staging/media/atomisp/i2c/atomisp-lm3554.c | 2 +- drivers/staging/wlan-ng/prism2usb.c | 6 +++--- drivers/tty/n_gsm.c | 2 +- drivers/tty/sysrq.c | 2 +- drivers/usb/gadget/udc/m66592-udc.c | 2 +- drivers/usb/serial/garmin_gps.c | 2 +- drivers/usb/serial/mos7840.c | 4 ++-- fs/ext4/super.c | 2 +- fs/nilfs2/segment.c | 2 +- net/802/garp.c | 2 +- net/802/mrp.c| 4 ++-- net/bridge/br_multicast.c| 8 net/bridge/br_multicast_eht.c| 4 ++-- net/core/gen_estimator.c | 2 +- net/ipv4/ipmr.c
Re: [Intel-gfx] [PATCH] drm/i915/fbdev: Implement fb_dirty for intel custom fb helper
On Tue, Dec 20, 2022 at 07:53:06PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 29, 2022 at 02:43:02PM +0200, Jouni Högander wrote:
> > After splitting generic drm_fb_helper into it's own file it's left to
> > helper implementation to have fb_dirty function. Currently intel
> > fb doesn't have it. This is causing problems when PSR is enabled.
> >
> > Implement simple fb_dirty callback to deliver notifications to psr
> > about updates in fb console.
>
> Just found this regression myself after being baffled why the
> vt console was inoperable right after the driver gets loaded.
>
> It's also not just psr, but also fbc that is having issues.
>
> Needs a fixes + cc:stable tags.
Actually looks like it didn't make it into 6.1 so I guess
no cc:stable needed.
>
> >
> > Cc: Thomas Zimmermann
> > Cc: Jani Nikula
> > Signed-off-by: Jouni Högander
> > ---
> > drivers/gpu/drm/i915/display/intel_fbdev.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > index 5575d7abdc09..7c7fba3fe69e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > @@ -328,8 +328,17 @@ static int intelfb_create(struct drm_fb_helper *helper,
> > return ret;
> > }
> >
> > +static int intelfb_dirty(struct drm_fb_helper *helper, struct
> > drm_clip_rect *clip)
> > +{
>
> The original thing had some kind of "is this rect actually visible?"
> check here. Does anyone know why it was there, and if so maybe it should
> go back to the higher level function so everyone doens't need to add it
> back in?
>
> > + if (helper->fb->funcs->dirty)
> > + return helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, clip,
> > 1);
> > +
> > + return 0;
> > +}
> > +
> > static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
> > .fb_probe = intelfb_create,
> > + .fb_dirty = intelfb_dirty,
> > };
> >
> > static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
> > --
> > 2.34.1
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH] drm/i915/fbdev: Implement fb_dirty for intel custom fb helper
On Tue, Nov 29, 2022 at 02:43:02PM +0200, Jouni Högander wrote:
> After splitting generic drm_fb_helper into it's own file it's left to
> helper implementation to have fb_dirty function. Currently intel
> fb doesn't have it. This is causing problems when PSR is enabled.
>
> Implement simple fb_dirty callback to deliver notifications to psr
> about updates in fb console.
Just found this regression myself after being baffled why the
vt console was inoperable right after the driver gets loaded.
It's also not just psr, but also fbc that is having issues.
Needs a fixes + cc:stable tags.
>
> Cc: Thomas Zimmermann
> Cc: Jani Nikula
> Signed-off-by: Jouni Högander
> ---
> drivers/gpu/drm/i915/display/intel_fbdev.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 5575d7abdc09..7c7fba3fe69e 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -328,8 +328,17 @@ static int intelfb_create(struct drm_fb_helper *helper,
> return ret;
> }
>
> +static int intelfb_dirty(struct drm_fb_helper *helper, struct drm_clip_rect
> *clip)
> +{
The original thing had some kind of "is this rect actually visible?"
check here. Does anyone know why it was there, and if so maybe it should
go back to the higher level function so everyone doens't need to add it
back in?
> + if (helper->fb->funcs->dirty)
> + return helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, clip,
> 1);
> +
> + return 0;
> +}
> +
> static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
> .fb_probe = intelfb_create,
> + .fb_dirty = intelfb_dirty,
> };
>
> static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
> --
> 2.34.1
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling read
> -Original Message- > From: Nikula, Jani > Sent: Tuesday, December 20, 2022 9:33 PM > To: Murthy, Arun R ; intel- > g...@lists.freedesktop.org; ville.syrj...@linux.intel.com; Deak, Imre > > Subject: RE: [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling read > > On Mon, 19 Dec 2022, "Murthy, Arun R" wrote: > > Any comments? > > From #intel-gfx: > > bashing the hw for 500 usec seems a bit harsh > > Which is true. The default for intel_de_wait_for_register() is 2 us. Should > probably stick to that. Recommendation as per windows code base is 50us interval for polling the register, so will change it to 50us. Thanks and Regards, Arun R Murthy > > BR, > Jani. > > > > > > Thanks and Regards, > > Arun R Murthy > > > > > >> -Original Message- > >> From: Murthy, Arun R > >> Sent: Thursday, December 15, 2022 4:44 PM > >> To: intel-gfx@lists.freedesktop.org; ville.syrj...@linux.intel.com; > >> Nikula, Jani ; Deak, Imre > >> > >> Cc: Murthy, Arun R > >> Subject: [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling > >> read > >> > >> The busy timeout logic checks for the AUX BUSY, then waits for the > >> timeout period and then after timeout reads the register for BUSY or > Success. > >> Instead replace interrupt with polling so as to read the AUX CTL > >> register often before the timeout period. Looks like there might be > >> some issue with interrupt-on-read. Hence changing the logic to polling > read. > >> > >> v2: replace interrupt with polling read > >> v3: use usleep_rang instead of msleep, updated commit msg > >> v4: use intel_wait_for_regiter internal function > >> v5: use __intel_de_wait_for_register with 500us slow and 10ms fast > >> timeout > >> v6: check return value of __intel_de_wait_for_register > >> > >> Signed-off-by: Arun R Murthy > >> --- > >> drivers/gpu/drm/i915/display/intel_dp_aux.c | 14 +- > >> 1 file changed, 5 insertions(+), 9 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c > >> b/drivers/gpu/drm/i915/display/intel_dp_aux.c > >> index 91c93c93e5fc..973dadecf712 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > >> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > >> @@ -41,20 +41,16 @@ intel_dp_aux_wait_done(struct intel_dp > *intel_dp) > >> i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp); > >> const unsigned int timeout_ms = 10; > >> u32 status; > >> - bool done; > >> - > >> -#define C (((status = intel_de_read_notrace(i915, ch_ctl)) & > >> DP_AUX_CH_CTL_SEND_BUSY) == 0) > >> - done = wait_event_timeout(i915->display.gmbus.wait_queue, C, > >> - msecs_to_jiffies_timeout(timeout_ms)); > >> + int ret; > >> > >> - /* just trace the final value */ > >> - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > >> + ret = __intel_de_wait_for_register(i915, ch_ctl, > >> +DP_AUX_CH_CTL_SEND_BUSY, 0, > >> +500, timeout_ms, ); > >> > >> - if (!done) > >> + if (ret == -ETIMEDOUT) > >> drm_err(>drm, > >> "%s: did not complete or timeout within %ums > >> (status 0x%08x)\n", > >> intel_dp->aux.name, timeout_ms, status); > >> -#undef C > >> > >> return status; > >> } > >> -- > >> 2.25.1 > > > > -- > Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PATCH] drm/i915/mtl: Add support of Tile4 to MTL
We have some Tile4 tests now skipping, which were
supposed to be working. So lets make them work, by
adding display_ver 14 as supported.
v2: - Remove "14" for Tile 4 CCS formats, as they
seem to be not supported by DG2(Juha-Pekka Heikkila)
- For generic Tile 4, the opposite - lets use -1
in order to make sure all the next gens support it by
default(Juha-Pekka Heikkila)
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/display/intel_fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
b/drivers/gpu/drm/i915/display/intel_fb.c
index 63137ae5ab217..93d0e46e54813 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -174,7 +174,7 @@ static const struct intel_modifier_desc intel_modifiers[] =
{
.plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_RC,
}, {
.modifier = I915_FORMAT_MOD_4_TILED,
- .display_ver = { 13, 13 },
+ .display_ver = { 13, -1 },
.plane_caps = INTEL_PLANE_CAP_TILING_4,
}, {
.modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS,
--
2.37.3
Re: [Intel-gfx] [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling read
On Mon, 19 Dec 2022, "Murthy, Arun R" wrote: > Any comments? >From #intel-gfx: bashing the hw for 500 usec seems a bit harsh Which is true. The default for intel_de_wait_for_register() is 2 us. Should probably stick to that. BR, Jani. > > Thanks and Regards, > Arun R Murthy > > >> -Original Message- >> From: Murthy, Arun R >> Sent: Thursday, December 15, 2022 4:44 PM >> To: intel-gfx@lists.freedesktop.org; ville.syrj...@linux.intel.com; Nikula, >> Jani >> ; Deak, Imre >> Cc: Murthy, Arun R >> Subject: [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling read >> >> The busy timeout logic checks for the AUX BUSY, then waits for the timeout >> period and then after timeout reads the register for BUSY or Success. >> Instead replace interrupt with polling so as to read the AUX CTL register >> often >> before the timeout period. Looks like there might be some issue with >> interrupt-on-read. Hence changing the logic to polling read. >> >> v2: replace interrupt with polling read >> v3: use usleep_rang instead of msleep, updated commit msg >> v4: use intel_wait_for_regiter internal function >> v5: use __intel_de_wait_for_register with 500us slow and 10ms fast timeout >> v6: check return value of __intel_de_wait_for_register >> >> Signed-off-by: Arun R Murthy >> --- >> drivers/gpu/drm/i915/display/intel_dp_aux.c | 14 +- >> 1 file changed, 5 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c >> b/drivers/gpu/drm/i915/display/intel_dp_aux.c >> index 91c93c93e5fc..973dadecf712 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c >> @@ -41,20 +41,16 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp) >> i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp); >> const unsigned int timeout_ms = 10; >> u32 status; >> - bool done; >> - >> -#define C (((status = intel_de_read_notrace(i915, ch_ctl)) & >> DP_AUX_CH_CTL_SEND_BUSY) == 0) >> - done = wait_event_timeout(i915->display.gmbus.wait_queue, C, >> - msecs_to_jiffies_timeout(timeout_ms)); >> + int ret; >> >> - /* just trace the final value */ >> - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); >> + ret = __intel_de_wait_for_register(i915, ch_ctl, >> +DP_AUX_CH_CTL_SEND_BUSY, 0, >> +500, timeout_ms, ); >> >> - if (!done) >> + if (ret == -ETIMEDOUT) >> drm_err(>drm, >> "%s: did not complete or timeout within %ums >> (status 0x%08x)\n", >> intel_dp->aux.name, timeout_ms, status); -#undef C >> >> return status; >> } >> -- >> 2.25.1 > -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] Possible regression in drm/i915 driver: memleak
Hi,
On 20/12/2022 15:22, srinivas pandruvada wrote:
+Added DRM mailing list and maintainers
On Tue, 2022-12-20 at 15:33 +0100, Mirsad Todorovac wrote:
Hi all,
I have been unsuccessful to find any particular Intel i915 maintainer
emails, so my best bet is to post here, as you will must assuredly
already know them.
For future reference you can use ${kernel_dir}/scripts/get_maintainer.pl -f ...
The problem is a kernel memory leak that is repeatedly occurring
triggered during the execution of Chrome browser under the latest
6.1.0+
kernel of this morning and Almalinux 8.6 on a Lenovo desktop box
with Intel(R) Core(TM) i5-8400 CPU @ 2.80GHz CPU.
The build is with KMEMLEAK, KASAN and MGLRU turned on during the
build,
on a vanilla mainline kernel from Mr. Torvalds' tree.
The leaks look like this one:
unreferenced object 0x888131754880 (size 64):
comm "chrome", pid 13058, jiffies 4298568878 (age 3708.084s)
hex dump (first 32 bytes):
01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 80 1e 3e 83 88 ff ff
...>
backtrace:
[] slab_post_alloc_hook+0xb2/0x340
[] __kmem_cache_alloc_node+0x1bf/0x2c0
[] kmalloc_trace+0x2a/0xb0
[] drm_vma_node_allow+0x45/0x150 [drm]
[] __assign_mmap_offset_handle+0x615/0x820
[i915]
[] i915_gem_mmap_offset_ioctl+0x77/0x110
[i915]
[] drm_ioctl_kernel+0x181/0x280 [drm]
[] drm_ioctl+0x2dd/0x6a0 [drm]
[] __x64_sys_ioctl+0xc4/0x100
[] do_syscall_64+0x58/0x80
[] entry_SYSCALL_64_after_hwframe+0x72/0xdc
The complete list of leaks in attachment, but they seem similar or
the same.
Please find attached lshw and kernel build config file.
I will probably check the same parms on my laptop at home, which is
also
Lenovo, but a different hw config and Ubuntu 22.10.
Could you try the below patch?
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index c3ea243d414d..0b07534c203a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -679,9 +679,10 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
insert:
mmo = insert_mmo(obj, mmo);
GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo);
-out:
+
if (file)
drm_vma_node_allow(>vma_node, file);
+out:
return mmo;
err:
Maybe it is not the best fix but curious to know if it will make the leak go
away.
Regards,
Tvrtko
Re: [Intel-gfx] [PATCH 1/1] drm/i915: re-disable RC6p on Sandy Bridge
On Tue, Dec 20, 2022 at 11:32 AM Ville Syrjälä
wrote:
>
> On Mon, Dec 19, 2022 at 06:29:27PM +0100, Sasa Dragic wrote:
> > RC6p on Sandy Bridge got re-enabled over time, causing visual glitches
> > and GPU hangs.
> >
> > Disabled originally in commit 1c8ecf80fdee ("drm/i915: do not enable
> > RC6p on Sandy Bridge").
>
> re cover letter:
> > It was originally disabled in commit 1c8ecf80fdee ("drm/i915: do not
> > enable RC6p on Sandy Bridge"), and subsequently re-enabled by
> > 13c5a577b342 ("drm/i915/gt: Select the deepest available parking mode
> > for rc6"), which seems to focus only on Ivy Bridge.
>
> That only kicks in while parked (ie. fully idle from
> software POV). I think the real bad commit is
> fb6db0f5bf1d ("drm/i915: Remove unsafe i915.enable_rc6")
> which seems to affects which rc6 level is selected while
> unparked.
You are correct. Although I'd like to add that most of the glitching
happens when system is switching to / from fully idle (e.g. running
glxgears in background reduces symptoms tenfold).
> We should mention both of those commits here:
> Fixes: fb6db0f5bf1d ("drm/i915: Remove unsafe i915.enable_rc6")
> Fixes: 13c5a577b342 ("drm/i915/gt: Select the deepest available parking mode
> for rc6")
>
> Also we want
> Cc: sta...@vger.kernel.org
>
> We can add those while pushing, so no need to resend for that.
Ok, thanks.
> >
> > Signed-off-by: Sasa Dragic
> > ---
> > drivers/gpu/drm/i915/i915_pci.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index 668e9da52584..69377564028a 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -423,7 +423,8 @@ static const struct intel_device_info ilk_m_info = {
> > .has_coherent_ggtt = true, \
> > .has_llc = 1, \
> > .has_rc6 = 1, \
> > - .has_rc6p = 1, \
> > + /* snb does support rc6p, but enabling it causes various issues */ \
> > + .has_rc6p = 0, \
>
> The one downside of doing it this way is that we also lose
> the debugfs/sysfs files so we can't monitor rc6+/++
> residency anymore to make sure they are not entered.
>
> I think ideally we'd split this into two parts: which rc6
> states the hw actually has vs. which rc6 states we actually
> want to use. But at least for the time being I think this
> simple should be sufficient, and should be easy to backport
> to stable releases.
Agreed.
> > .has_rps = true, \
> > .dma_mask_size = 40, \
> > .__runtime.ppgtt_type = INTEL_PPGTT_ALIASING, \
> > --
> > 2.37.2
>
> --
> Ville Syrjälä
> Intel
Regards,
Sasa
Re: [Intel-gfx] Possible regression in drm/i915 driver: memleak
+Added DRM mailing list and maintainers On Tue, 2022-12-20 at 15:33 +0100, Mirsad Todorovac wrote: > Hi all, > > I have been unsuccessful to find any particular Intel i915 maintainer > emails, so my best bet is to post here, as you will must assuredly > already know them. > > The problem is a kernel memory leak that is repeatedly occurring > triggered during the execution of Chrome browser under the latest > 6.1.0+ > kernel of this morning and Almalinux 8.6 on a Lenovo desktop box > with Intel(R) Core(TM) i5-8400 CPU @ 2.80GHz CPU. > > The build is with KMEMLEAK, KASAN and MGLRU turned on during the > build, > on a vanilla mainline kernel from Mr. Torvalds' tree. > > The leaks look like this one: > > unreferenced object 0x888131754880 (size 64): > comm "chrome", pid 13058, jiffies 4298568878 (age 3708.084s) > hex dump (first 32 bytes): > 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > 00 00 00 00 00 00 00 00 00 80 1e 3e 83 88 ff ff > ...> > backtrace: > [] slab_post_alloc_hook+0xb2/0x340 > [] __kmem_cache_alloc_node+0x1bf/0x2c0 > [] kmalloc_trace+0x2a/0xb0 > [] drm_vma_node_allow+0x45/0x150 [drm] > [] __assign_mmap_offset_handle+0x615/0x820 > [i915] > [] i915_gem_mmap_offset_ioctl+0x77/0x110 > [i915] > [] drm_ioctl_kernel+0x181/0x280 [drm] > [] drm_ioctl+0x2dd/0x6a0 [drm] > [] __x64_sys_ioctl+0xc4/0x100 > [] do_syscall_64+0x58/0x80 > [] entry_SYSCALL_64_after_hwframe+0x72/0xdc > > The complete list of leaks in attachment, but they seem similar or > the same. > > Please find attached lshw and kernel build config file. > > I will probably check the same parms on my laptop at home, which is > also > Lenovo, but a different hw config and Ubuntu 22.10. > > Thanks, > Mirsad > > -- > Mirsad Goran Todorovac > Sistem inženjer > Grafički fakultet | Akademija likovnih umjetnosti > Sveučilište u Zagrebu
Re: [Intel-gfx] [PATCH] drm/i915/dsi: fix MIPI_BKLT_EN_1 native GPIO index
On Tue, Dec 20, 2022 at 04:01:05PM +0200, Jani Nikula wrote:
> Due to copy-paste fail, MIPI_BKLT_EN_1 would always use PPS index 1,
> never 0. Fix the sloppiest commit in recent memory.
>
> Fixes: f087cfe6fcff ("drm/i915/dsi: add support for ICL+ native MIPI GPIO
> sequence")
> Reported-by: Ville Syrjälä
> Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index 41f025f089d9..2cbc1292ab38 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -430,7 +430,7 @@ static void icl_native_gpio_set_value(struct
> drm_i915_private *dev_priv,
> break;
> case MIPI_BKLT_EN_1:
> case MIPI_BKLT_EN_2:
> - index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
> + index = gpio == MIPI_BKLT_EN_1 ? 0 : 1;
>
> intel_de_rmw(dev_priv, PP_CONTROL(index), EDP_BLC_ENABLE,
>value ? EDP_BLC_ENABLE : 0);
> --
> 2.34.1
--
Ville Syrjälä
Intel
[Intel-gfx] [PATCH] drm/i915/dsi: fix MIPI_BKLT_EN_1 native GPIO index
Due to copy-paste fail, MIPI_BKLT_EN_1 would always use PPS index 1,
never 0. Fix the sloppiest commit in recent memory.
Fixes: f087cfe6fcff ("drm/i915/dsi: add support for ICL+ native MIPI GPIO
sequence")
Reported-by: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
index 41f025f089d9..2cbc1292ab38 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
@@ -430,7 +430,7 @@ static void icl_native_gpio_set_value(struct
drm_i915_private *dev_priv,
break;
case MIPI_BKLT_EN_1:
case MIPI_BKLT_EN_2:
- index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
+ index = gpio == MIPI_BKLT_EN_1 ? 0 : 1;
intel_de_rmw(dev_priv, PP_CONTROL(index), EDP_BLC_ENABLE,
value ? EDP_BLC_ENABLE : 0);
--
2.34.1
Re: [Intel-gfx] [PATCH v6 06/12] drm/edid: refactor _drm_edid_connector_update() and rename
On Tue, 20 Dec 2022, Ville Syrjälä wrote:
> On Tue, Dec 20, 2022 at 02:52:01PM +0200, Jani Nikula wrote:
>> On Tue, 20 Dec 2022, Ville Syrjälä wrote:
>> > On Fri, Dec 16, 2022 at 06:00:20PM +0200, Jani Nikula wrote:
>> >> By moving update_display_info() out of _drm_edid_connector_update() we
>> >> make the function purely about adding modes.
>> >
>> > I don't think that's quite true. The 4:2:0 stuff still updates
>> > various display_info things from the mode parsing functions.
>>
>> Right. I meant the top level. Will amend the commit message.
>
> So what's going to happen with the 4:2:0 stuff? Are we just clobbering
> it if/when someone calls the update_display_info() stuff w/o calling
> add_modes()?
Don't do that then?
*sigh*
I don't know.
It's pretty much the same thing as only calling update_display_info(),
before adding modes, and looking at the relevant info fields *before*
the add modes call. That's probably already happening.
I really wanted to put this all together into one call so nobody could
do that silly stuff. But then for various reasons drivers want to not
only read the EDID but also parse it in ->detect, and ->detect gets
called a lot.
We can't keep adding modes to probed modes in ->detect all the time
without pruning them. I thought about trying to avoid adding duplicated
modes in probed modes list, but it looks like another rabbit hole with
no end in sight. Don't really want to go that route.
If we want to make this fool proof, one way to fix this would be to do
all the info changes in step 1, even the ones that are currently added
via add modes. Iterate through everything, but only do the info
changes. And in step 2 only add the modes to probed modes list.
In any case, the current state of affairs is bonkers already. There's
supposed to be 1) reading the EDID, 2) updating the info, and 3) adding
the modes. But both drm_get_edid() *and* drm_add_edid_modes() do the
display info and property update part too. We've just been adding calls
here and there to patch things up. Nobody cares. Just add more calls
updating stuff, and hope it'll be fine. That's gotta stop.
BR,
Jani.
>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> >> Rename accordingly.
>> >>
>> >> Cc: Imre Deak
>> >> Cc: Ville Syrjälä
>> >> Signed-off-by: Jani Nikula
>> >> ---
>> >> drivers/gpu/drm/drm_edid.c | 25 -
>> >> 1 file changed, 12 insertions(+), 13 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> >> index 15f69c362fc3..4ebfd7212bce 100644
>> >> --- a/drivers/gpu/drm/drm_edid.c
>> >> +++ b/drivers/gpu/drm/drm_edid.c
>> >> @@ -6575,19 +6575,12 @@ static int add_displayid_detailed_modes(struct
>> >> drm_connector *connector,
>> >> return num_modes;
>> >> }
>> >>
>> >> -static int _drm_edid_connector_update(struct drm_connector *connector,
>> >> - const struct drm_edid *drm_edid)
>> >> +static int _drm_edid_connector_add_modes(struct drm_connector *connector,
>> >> + const struct drm_edid *drm_edid)
>> >> {
>> >> const struct drm_display_info *info = >display_info;
>> >> int num_modes = 0;
>> >>
>> >> - /*
>> >> - * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks.
>> >> - * To avoid multiple parsing of same block, lets parse that map
>> >> - * from sink info, before parsing CEA modes.
>> >> - */
>> >> - update_display_info(connector, drm_edid);
>> >> -
>> >> if (!drm_edid)
>> >> return 0;
>> >>
>> >> @@ -6692,7 +6685,9 @@ int drm_edid_connector_update(struct drm_connector
>> >> *connector,
>> >> {
>> >> int count;
>> >>
>> >> - count = _drm_edid_connector_update(connector, drm_edid);
>> >> + update_display_info(connector, drm_edid);
>> >> +
>> >> + count = _drm_edid_connector_add_modes(connector, drm_edid);
>> >>
>> >> _drm_update_tile_info(connector, drm_edid);
>> >>
>> >> @@ -6762,7 +6757,8 @@ EXPORT_SYMBOL(drm_connector_update_edid_property);
>> >> */
>> >> int drm_add_edid_modes(struct drm_connector *connector, struct edid
>> >> *edid)
>> >> {
>> >> - struct drm_edid drm_edid;
>> >> + struct drm_edid _drm_edid;
>> >> + const struct drm_edid *drm_edid;
>> >>
>> >> if (edid && !drm_edid_is_valid(edid)) {
>> >> drm_warn(connector->dev, "[CONNECTOR:%d:%s] EDID invalid.\n",
>> >> @@ -6770,8 +6766,11 @@ int drm_add_edid_modes(struct drm_connector
>> >> *connector, struct edid *edid)
>> >> edid = NULL;
>> >> }
>> >>
>> >> - return _drm_edid_connector_update(connector,
>> >> - drm_edid_legacy_init(_edid,
>> >> edid));
>> >> + drm_edid = drm_edid_legacy_init(&_drm_edid, edid);
>> >> +
>> >> + update_display_info(connector, drm_edid);
>> >> +
>> >> + return _drm_edid_connector_add_modes(connector, drm_edid);
>> >> }
>> >> EXPORT_SYMBOL(drm_add_edid_modes);
>> >>
>> >> --
>> >> 2.34.1
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
--
Jani
Re: [Intel-gfx] [PATCH v6 06/12] drm/edid: refactor _drm_edid_connector_update() and rename
On Tue, Dec 20, 2022 at 02:52:01PM +0200, Jani Nikula wrote:
> On Tue, 20 Dec 2022, Ville Syrjälä wrote:
> > On Fri, Dec 16, 2022 at 06:00:20PM +0200, Jani Nikula wrote:
> >> By moving update_display_info() out of _drm_edid_connector_update() we
> >> make the function purely about adding modes.
> >
> > I don't think that's quite true. The 4:2:0 stuff still updates
> > various display_info things from the mode parsing functions.
>
> Right. I meant the top level. Will amend the commit message.
So what's going to happen with the 4:2:0 stuff? Are we just clobbering
it if/when someone calls the update_display_info() stuff w/o calling
add_modes()?
>
> BR,
> Jani.
>
>
> >
> >> Rename accordingly.
> >>
> >> Cc: Imre Deak
> >> Cc: Ville Syrjälä
> >> Signed-off-by: Jani Nikula
> >> ---
> >> drivers/gpu/drm/drm_edid.c | 25 -
> >> 1 file changed, 12 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> index 15f69c362fc3..4ebfd7212bce 100644
> >> --- a/drivers/gpu/drm/drm_edid.c
> >> +++ b/drivers/gpu/drm/drm_edid.c
> >> @@ -6575,19 +6575,12 @@ static int add_displayid_detailed_modes(struct
> >> drm_connector *connector,
> >>return num_modes;
> >> }
> >>
> >> -static int _drm_edid_connector_update(struct drm_connector *connector,
> >> -const struct drm_edid *drm_edid)
> >> +static int _drm_edid_connector_add_modes(struct drm_connector *connector,
> >> + const struct drm_edid *drm_edid)
> >> {
> >>const struct drm_display_info *info = >display_info;
> >>int num_modes = 0;
> >>
> >> - /*
> >> - * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks.
> >> - * To avoid multiple parsing of same block, lets parse that map
> >> - * from sink info, before parsing CEA modes.
> >> - */
> >> - update_display_info(connector, drm_edid);
> >> -
> >>if (!drm_edid)
> >>return 0;
> >>
> >> @@ -6692,7 +6685,9 @@ int drm_edid_connector_update(struct drm_connector
> >> *connector,
> >> {
> >>int count;
> >>
> >> - count = _drm_edid_connector_update(connector, drm_edid);
> >> + update_display_info(connector, drm_edid);
> >> +
> >> + count = _drm_edid_connector_add_modes(connector, drm_edid);
> >>
> >>_drm_update_tile_info(connector, drm_edid);
> >>
> >> @@ -6762,7 +6757,8 @@ EXPORT_SYMBOL(drm_connector_update_edid_property);
> >> */
> >> int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> >> {
> >> - struct drm_edid drm_edid;
> >> + struct drm_edid _drm_edid;
> >> + const struct drm_edid *drm_edid;
> >>
> >>if (edid && !drm_edid_is_valid(edid)) {
> >>drm_warn(connector->dev, "[CONNECTOR:%d:%s] EDID invalid.\n",
> >> @@ -6770,8 +6766,11 @@ int drm_add_edid_modes(struct drm_connector
> >> *connector, struct edid *edid)
> >>edid = NULL;
> >>}
> >>
> >> - return _drm_edid_connector_update(connector,
> >> -drm_edid_legacy_init(_edid,
> >> edid));
> >> + drm_edid = drm_edid_legacy_init(&_drm_edid, edid);
> >> +
> >> + update_display_info(connector, drm_edid);
> >> +
> >> + return _drm_edid_connector_add_modes(connector, drm_edid);
> >> }
> >> EXPORT_SYMBOL(drm_add_edid_modes);
> >>
> >> --
> >> 2.34.1
>
> --
> Jani Nikula, Intel Open Source Graphics Center
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH v6 06/12] drm/edid: refactor _drm_edid_connector_update() and rename
On Tue, 20 Dec 2022, Ville Syrjälä wrote:
> On Fri, Dec 16, 2022 at 06:00:20PM +0200, Jani Nikula wrote:
>> By moving update_display_info() out of _drm_edid_connector_update() we
>> make the function purely about adding modes.
>
> I don't think that's quite true. The 4:2:0 stuff still updates
> various display_info things from the mode parsing functions.
Right. I meant the top level. Will amend the commit message.
BR,
Jani.
>
>> Rename accordingly.
>>
>> Cc: Imre Deak
>> Cc: Ville Syrjälä
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/drm_edid.c | 25 -
>> 1 file changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 15f69c362fc3..4ebfd7212bce 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -6575,19 +6575,12 @@ static int add_displayid_detailed_modes(struct
>> drm_connector *connector,
>> return num_modes;
>> }
>>
>> -static int _drm_edid_connector_update(struct drm_connector *connector,
>> - const struct drm_edid *drm_edid)
>> +static int _drm_edid_connector_add_modes(struct drm_connector *connector,
>> + const struct drm_edid *drm_edid)
>> {
>> const struct drm_display_info *info = >display_info;
>> int num_modes = 0;
>>
>> -/*
>> - * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks.
>> - * To avoid multiple parsing of same block, lets parse that map
>> - * from sink info, before parsing CEA modes.
>> - */
>> -update_display_info(connector, drm_edid);
>> -
>> if (!drm_edid)
>> return 0;
>>
>> @@ -6692,7 +6685,9 @@ int drm_edid_connector_update(struct drm_connector
>> *connector,
>> {
>> int count;
>>
>> -count = _drm_edid_connector_update(connector, drm_edid);
>> +update_display_info(connector, drm_edid);
>> +
>> +count = _drm_edid_connector_add_modes(connector, drm_edid);
>>
>> _drm_update_tile_info(connector, drm_edid);
>>
>> @@ -6762,7 +6757,8 @@ EXPORT_SYMBOL(drm_connector_update_edid_property);
>> */
>> int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>> {
>> -struct drm_edid drm_edid;
>> +struct drm_edid _drm_edid;
>> +const struct drm_edid *drm_edid;
>>
>> if (edid && !drm_edid_is_valid(edid)) {
>> drm_warn(connector->dev, "[CONNECTOR:%d:%s] EDID invalid.\n",
>> @@ -6770,8 +6766,11 @@ int drm_add_edid_modes(struct drm_connector
>> *connector, struct edid *edid)
>> edid = NULL;
>> }
>>
>> -return _drm_edid_connector_update(connector,
>> - drm_edid_legacy_init(_edid,
>> edid));
>> +drm_edid = drm_edid_legacy_init(&_drm_edid, edid);
>> +
>> +update_display_info(connector, drm_edid);
>> +
>> +return _drm_edid_connector_add_modes(connector, drm_edid);
>> }
>> EXPORT_SYMBOL(drm_add_edid_modes);
>>
>> --
>> 2.34.1
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH] drm/i915/display: Fix a use-after-free when intel_edp_init_connector fails
On Tue, 20 Dec 2022, Maarten Lankhorst
wrote:
> We enable the DP aux channel during probe, but may free the connector
> soon afterwards. Ensure the DP aux display power put is completed before
> everything is freed, to prevent a use-after-free in icl_aux_pw_to_phy(),
> called from icl_combo_phy_aux_power_well_disable.
Feels like the placement of the intel_display_power_flush_work_sync()
call in intel_dp_aux_fini() is a bit arbitrary.
If we add it in intel_dp_aux_fini(), the async and sync waits will both
be called on the regular encoder destroy path.
Maybe both intel_ddi_encoder_destroy() and intel_dp_encoder_destroy()
should call intel_display_power_flush_work_sync(), instead of async, and
maybe the error paths should call those functions instead of just
drm_encoder_cleanup()?
Imre?
BR,
Jani.
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
> drivers/gpu/drm/i915/display/intel_display_power.h | 1 +
> drivers/gpu/drm/i915/display/intel_dp_aux.c| 2 ++
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 04915f85a0df..0edb5532461f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -776,7 +776,7 @@ void intel_display_power_flush_work(struct
> drm_i915_private *i915)
> * Like intel_display_power_flush_work(), but also ensure that the work
> * handler function is not running any more when this function returns.
> */
> -static void
> +void
> intel_display_power_flush_work_sync(struct drm_i915_private *i915)
> {
> struct i915_power_domains *power_domains = >display.power.domains;
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h
> b/drivers/gpu/drm/i915/display/intel_display_power.h
> index 7136ea3f233e..dc10ee0519e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -188,6 +188,7 @@ void __intel_display_power_put_async(struct
> drm_i915_private *i915,
>enum intel_display_power_domain domain,
>intel_wakeref_t wakeref);
> void intel_display_power_flush_work(struct drm_i915_private *i915);
> +void intel_display_power_flush_work_sync(struct drm_i915_private *i915);
> #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> void intel_display_power_put(struct drm_i915_private *dev_priv,
>enum intel_display_power_domain domain,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index f1835c74bff0..1006dddad2d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -680,6 +680,8 @@ void intel_dp_aux_fini(struct intel_dp *intel_dp)
> if (cpu_latency_qos_request_active(_dp->pm_qos))
> cpu_latency_qos_remove_request(_dp->pm_qos);
>
> + /* Ensure async work from intel_dp_aux_xfer() is flushed before we
> clean up */
> + intel_display_power_flush_work_sync(dp_to_i915(intel_dp));
> kfree(intel_dp->aux.name);
> }
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v6 06/12] drm/edid: refactor _drm_edid_connector_update() and rename
On Fri, Dec 16, 2022 at 06:00:20PM +0200, Jani Nikula wrote:
> By moving update_display_info() out of _drm_edid_connector_update() we
> make the function purely about adding modes.
I don't think that's quite true. The 4:2:0 stuff still updates
various display_info things from the mode parsing functions.
> Rename accordingly.
>
> Cc: Imre Deak
> Cc: Ville Syrjälä
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/drm_edid.c | 25 -
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 15f69c362fc3..4ebfd7212bce 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6575,19 +6575,12 @@ static int add_displayid_detailed_modes(struct
> drm_connector *connector,
> return num_modes;
> }
>
> -static int _drm_edid_connector_update(struct drm_connector *connector,
> - const struct drm_edid *drm_edid)
> +static int _drm_edid_connector_add_modes(struct drm_connector *connector,
> + const struct drm_edid *drm_edid)
> {
> const struct drm_display_info *info = >display_info;
> int num_modes = 0;
>
> - /*
> - * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks.
> - * To avoid multiple parsing of same block, lets parse that map
> - * from sink info, before parsing CEA modes.
> - */
> - update_display_info(connector, drm_edid);
> -
> if (!drm_edid)
> return 0;
>
> @@ -6692,7 +6685,9 @@ int drm_edid_connector_update(struct drm_connector
> *connector,
> {
> int count;
>
> - count = _drm_edid_connector_update(connector, drm_edid);
> + update_display_info(connector, drm_edid);
> +
> + count = _drm_edid_connector_add_modes(connector, drm_edid);
>
> _drm_update_tile_info(connector, drm_edid);
>
> @@ -6762,7 +6757,8 @@ EXPORT_SYMBOL(drm_connector_update_edid_property);
> */
> int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> {
> - struct drm_edid drm_edid;
> + struct drm_edid _drm_edid;
> + const struct drm_edid *drm_edid;
>
> if (edid && !drm_edid_is_valid(edid)) {
> drm_warn(connector->dev, "[CONNECTOR:%d:%s] EDID invalid.\n",
> @@ -6770,8 +6766,11 @@ int drm_add_edid_modes(struct drm_connector
> *connector, struct edid *edid)
> edid = NULL;
> }
>
> - return _drm_edid_connector_update(connector,
> - drm_edid_legacy_init(_edid,
> edid));
> + drm_edid = drm_edid_legacy_init(&_drm_edid, edid);
> +
> + update_display_info(connector, drm_edid);
> +
> + return _drm_edid_connector_add_modes(connector, drm_edid);
> }
> EXPORT_SYMBOL(drm_add_edid_modes);
>
> --
> 2.34.1
--
Ville Syrjälä
Intel
[Intel-gfx] [PATCH v6 2/2] drm/i915/mtl: Limit scaler input to 4k in plane scaling
From: Animesh Manna
As part of die area reduction max input source modified to 4096
for MTL so modified range check logic of scaler.
Signed-off-by: Jos� Roberto de Souza
Signed-off-by: Animesh Manna
Signed-off-by: Luca Coelho
---
drivers/gpu/drm/i915/display/skl_scaler.c | 31 +--
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
b/drivers/gpu/drm/i915/display/skl_scaler.c
index d7390067b7d4..6baa07142b03 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -103,6 +103,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool
force_detach,
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
const struct drm_display_mode *adjusted_mode =
_state->hw.adjusted_mode;
+ int min_src_w, min_src_h, min_dst_w, min_dst_h;
+ int max_src_w, max_src_h, max_dst_w, max_dst_h;
/*
* Src coordinates are already rotated by 270 degrees for
@@ -157,15 +159,28 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
bool force_detach,
return -EINVAL;
}
+ min_src_w = SKL_MIN_SRC_W;
+ min_src_h = SKL_MIN_SRC_H;
+ min_dst_w = SKL_MIN_DST_W;
+ min_dst_h = SKL_MIN_DST_H;
+
+ if (DISPLAY_VER(dev_priv) >= 11 && DISPLAY_VER(dev_priv) < 14) {
+ max_src_w = ICL_MAX_SRC_W;
+ max_src_h = ICL_MAX_SRC_H;
+ max_dst_w = ICL_MAX_DST_W;
+ max_dst_h = ICL_MAX_DST_H;
+ } else {
+ max_src_w = SKL_MAX_SRC_W;
+ max_src_h = SKL_MAX_SRC_H;
+ max_dst_w = SKL_MAX_DST_W;
+ max_dst_h = SKL_MAX_DST_H;
+ }
+
/* range checks */
- if (src_w < SKL_MIN_SRC_W || src_h < SKL_MIN_SRC_H ||
- dst_w < SKL_MIN_DST_W || dst_h < SKL_MIN_DST_H ||
- (DISPLAY_VER(dev_priv) >= 11 &&
-(src_w > ICL_MAX_SRC_W || src_h > ICL_MAX_SRC_H ||
- dst_w > ICL_MAX_DST_W || dst_h > ICL_MAX_DST_H)) ||
- (DISPLAY_VER(dev_priv) < 11 &&
-(src_w > SKL_MAX_SRC_W || src_h > SKL_MAX_SRC_H ||
- dst_w > SKL_MAX_DST_W || dst_h > SKL_MAX_DST_H))) {
+ if (src_w < min_src_w || src_h < min_src_h ||
+ dst_w < min_dst_w || dst_h < min_dst_h ||
+ src_w > max_src_w || src_h > max_src_h ||
+ dst_w > max_dst_w || dst_h > max_dst_h) {
drm_dbg_kms(_priv->drm,
"scaler_user index %u.%u: src %ux%u dst %ux%u "
"size is out of scaler range\n",
--
2.38.1
[Intel-gfx] [PATCH v6 1/2] drm/i915/mtl: limit second scaler vertical scaling in ver >= 14
In newer hardware versions (i.e. display version >= 14), the second
scaler doesn't support vertical scaling.
The current implementation of the scaling limits is simplified and
only occurs when the planes are created, so we don't know which scaler
is being used.
In order to handle separate scaling limits for horizontal and vertical
scaling, and different limits per scaler, split the checks in two
phases. We first do a simple check during plane creation and use the
best-case scenario (because we don't know the scaler that may be used
at a later point) and then do a more specific check when the scalers
are actually being set up.
Signed-off-by: Luca Coelho
---
In v2:
* fix DRM_PLANE_NO_SCALING renamed macros;
In v3:
* No changes.
In v4:
* Got rid of the changes in the general planes max scale code;
* Added a couple of FIXMEs;
* Made intel_atomic_setup_scaler() return an int with errors;
In v5:
* Just resent with a cover letter.
In v6:
* Now the correct version again (same as v4).
drivers/gpu/drm/i915/display/intel_atomic.c | 83 ++---
1 file changed, 73 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index 6621aa245caf..8373be283d8b 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -41,6 +41,7 @@
#include "intel_global_state.h"
#include "intel_hdcp.h"
#include "intel_psr.h"
+#include "intel_fb.h"
#include "skl_universal_plane.h"
/**
@@ -310,11 +311,11 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
kfree(crtc_state);
}
-static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state
*scaler_state,
- int num_scalers_need, struct intel_crtc
*intel_crtc,
- const char *name, int idx,
- struct intel_plane_state *plane_state,
- int *scaler_id)
+static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state
*scaler_state,
+int num_scalers_need, struct intel_crtc
*intel_crtc,
+const char *name, int idx,
+struct intel_plane_state *plane_state,
+int *scaler_id)
{
struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
int j;
@@ -334,7 +335,7 @@ static void intel_atomic_setup_scaler(struct
intel_crtc_scaler_state *scaler_sta
if (drm_WARN(_priv->drm, *scaler_id < 0,
"Cannot find scaler for %s:%d\n", name, idx))
- return;
+ return -EBUSY;
/* set scaler mode */
if (plane_state && plane_state->hw.fb &&
@@ -375,9 +376,69 @@ static void intel_atomic_setup_scaler(struct
intel_crtc_scaler_state *scaler_sta
mode = SKL_PS_SCALER_MODE_DYN;
}
+ /*
+* FIXME: we should also check the scaler factors for pfit, so
+* this shouldn't be tied directly to planes.
+*/
+ if (plane_state && plane_state->hw.fb) {
+ const struct drm_framebuffer *fb = plane_state->hw.fb;
+ struct drm_rect *src = _state->uapi.src;
+ struct drm_rect *dst = _state->uapi.dst;
+ int hscale, vscale, max_vscale, max_hscale;
+
+ /*
+* FIXME: When two scalers are needed, but only one of
+* them needs to downscale, we should make sure that
+* the one that needs downscaling support is assigned
+* as the first scaler, so we don't reject downscaling
+* unnecessarily.
+*/
+
+ if (DISPLAY_VER(dev_priv) >= 14) {
+ /*
+* On versions 14 and up, only the first
+* scaler supports a vertical scaling factor
+* of more than 1.0, while a horizontal
+* scaling factor of 3.0 is supported.
+*/
+ max_hscale = 0x3 - 1;
+ if (*scaler_id == 0)
+ max_vscale = 0x3 - 1;
+ else
+ max_vscale = 0x1;
+
+ } else if (DISPLAY_VER(dev_priv) >= 10 ||
+ !intel_format_info_is_yuv_semiplanar(fb->format,
fb->modifier)) {
+ max_hscale = 0x3 - 1;
+ max_vscale = 0x3 - 1;
+ } else {
+ max_hscale = 0x2 - 1;
+ max_vscale = 0x2 - 1;
+ }
+
+ /* Check if required scaling is within limits */
+ hscale = drm_rect_calc_hscale(src, dst, 1, max_hscale);
+ vscale =
[Intel-gfx] [PATCH v6 0/2] drm/i915/mtl: handle some MTL scaler limitations
Hi, I stupidly sent an old version of the patches in my v5... Resending the correct ones (which were sent as v4). The versioning history is in the patches themselves. Please review. Cheers, Luca. Animesh Manna (1): drm/i915/mtl: Limit scaler input to 4k in plane scaling Luca Coelho (1): drm/i915/mtl: limit second scaler vertical scaling in ver >= 14 drivers/gpu/drm/i915/display/intel_atomic.c | 83 ++--- drivers/gpu/drm/i915/display/skl_scaler.c | 31 ++-- 2 files changed, 96 insertions(+), 18 deletions(-) -- 2.38.1
[Intel-gfx] [PATCH 2/2] drm/i915/display/misc: use intel_de_rmw if possible
The helper makes the code more compact and readable.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/i915/display/g4x_dp.c | 12 ++--
drivers/gpu/drm/i915/display/g4x_hdmi.c | 8 +--
.../gpu/drm/i915/display/intel_backlight.c| 59 +++
.../gpu/drm/i915/display/intel_combo_phy.c| 43 --
drivers/gpu/drm/i915/display/intel_ddi.c | 49 +--
drivers/gpu/drm/i915/display/intel_dpio_phy.c | 51 +---
drivers/gpu/drm/i915/display/intel_drrs.c | 12 +---
drivers/gpu/drm/i915/display/intel_dvo.c | 7 +--
drivers/gpu/drm/i915/display/intel_fdi.c | 3 +-
drivers/gpu/drm/i915/display/intel_gmbus.c| 30 ++
drivers/gpu/drm/i915/display/intel_hdcp.c | 15 ++---
drivers/gpu/drm/i915/display/intel_hdmi.c | 40 -
drivers/gpu/drm/i915/display/intel_lvds.c | 12 ++--
.../gpu/drm/i915/display/intel_pch_display.c | 41 -
.../gpu/drm/i915/display/intel_pch_refclk.c | 10 +---
drivers/gpu/drm/i915/display/intel_pps.c | 14 ++---
drivers/gpu/drm/i915/display/intel_psr.c | 40 -
drivers/gpu/drm/i915/display/intel_tv.c | 18 ++
18 files changed, 141 insertions(+), 323 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c
b/drivers/gpu/drm/i915/display/g4x_dp.c
index 24ef36ec2d3d3c..9629b174ec5d2c 100644
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -136,16 +136,12 @@ static void intel_dp_prepare(struct intel_encoder
*encoder,
intel_dp->DP |= DP_PIPE_SEL_IVB(crtc->pipe);
} else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
- u32 trans_dp;
-
intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT;
- trans_dp = intel_de_read(dev_priv, TRANS_DP_CTL(crtc->pipe));
- if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
- trans_dp |= TRANS_DP_ENH_FRAMING;
- else
- trans_dp &= ~TRANS_DP_ENH_FRAMING;
- intel_de_write(dev_priv, TRANS_DP_CTL(crtc->pipe), trans_dp);
+ intel_de_rmw(dev_priv, TRANS_DP_CTL(crtc->pipe),
+TRANS_DP_ENH_FRAMING,
+drm_dp_enhanced_frame_cap(intel_dp->dpcd) ?
+TRANS_DP_ENH_FRAMING : 0);
} else {
if (IS_G4X(dev_priv) && pipe_config->limited_color_range)
intel_dp->DP |= DP_COLOR_RANGE_16_235;
diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c
b/drivers/gpu/drm/i915/display/g4x_hdmi.c
index c3580d96765c6c..f58849b416ea89 100644
--- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
+++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
@@ -271,8 +271,8 @@ static void cpt_enable_hdmi(struct intel_atomic_state
*state,
*/
if (pipe_config->pipe_bpp > 24) {
- intel_de_write(dev_priv, TRANS_CHICKEN1(pipe),
- intel_de_read(dev_priv, TRANS_CHICKEN1(pipe)) |
TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE);
+ intel_de_rmw(dev_priv, TRANS_CHICKEN1(pipe),
+0, TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE);
temp &= ~SDVO_COLOR_FORMAT_MASK;
temp |= SDVO_COLOR_FORMAT_8bpc;
@@ -288,8 +288,8 @@ static void cpt_enable_hdmi(struct intel_atomic_state
*state,
intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
- intel_de_write(dev_priv, TRANS_CHICKEN1(pipe),
- intel_de_read(dev_priv, TRANS_CHICKEN1(pipe)) &
~TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE);
+ intel_de_rmw(dev_priv, TRANS_CHICKEN1(pipe),
+TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE, 0);
}
drm_WARN_ON(_priv->drm, pipe_config->has_audio &&
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c
b/drivers/gpu/drm/i915/display/intel_backlight.c
index 5b7da72c95b8c5..b088921c543eaa 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -349,8 +349,7 @@ static void lpt_disable_backlight(const struct
drm_connector_state *old_conn_sta
intel_de_write(i915, BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
}
- tmp = intel_de_read(i915, BLC_PWM_PCH_CTL1);
- intel_de_write(i915, BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
+ tmp = intel_de_rmw(i915, BLC_PWM_PCH_CTL1, BLM_PCH_PWM_ENABLE, 0);
}
static void pch_disable_backlight(const struct drm_connector_state
*old_conn_state, u32 val)
@@ -361,11 +360,9 @@ static void pch_disable_backlight(const struct
drm_connector_state *old_conn_sta
intel_backlight_set_pwm_level(old_conn_state, val);
- tmp = intel_de_read(i915, BLC_PWM_CPU_CTL2);
- intel_de_write(i915, BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
+ intel_de_rmw(i915,
[Intel-gfx] [PATCH 1/2] drm/i915/display/core: use intel_de_rmw if possible
The helper makes the code more compact and readable.
Signed-off-by: Andrzej Hajda
---
Hi all,
This is the last patchset converting different flavours
of register updates to intel_de_rmw. There are probably
still some remnants, which would require functional changes
to allow conversion. They would need probably more individual
approach.
Jani, thank you for reviews.
And the final diffstat for all patches:
26 files changed, 425 insertions(+), 952 deletions(-)
Regards
Andrzej
---
drivers/gpu/drm/i915/display/intel_display.c | 22 +--
.../drm/i915/display/intel_display_power.c| 49 ++
.../i915/display/intel_display_power_well.c | 82 +++--
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 165 ++
.../drm/i915/display/intel_modeset_setup.c| 17 +-
5 files changed, 108 insertions(+), 227 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index e75b9b2a0e015a..c6c566a6572b8b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -293,11 +293,11 @@ static void
skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, bool enable)
{
if (enable)
- intel_de_write(dev_priv, CLKGATE_DIS_PSL(pipe),
- intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) |
DUPS1_GATING_DIS | DUPS2_GATING_DIS);
+ intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(pipe),
+0, DUPS1_GATING_DIS | DUPS2_GATING_DIS);
else
- intel_de_write(dev_priv, CLKGATE_DIS_PSL(pipe),
- intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) &
~(DUPS1_GATING_DIS | DUPS2_GATING_DIS));
+ intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(pipe),
+DUPS1_GATING_DIS | DUPS2_GATING_DIS, 0);
}
/* Wa_2006604312:icl,ehl */
@@ -306,11 +306,9 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv,
enum pipe pipe,
bool enable)
{
if (enable)
- intel_de_write(dev_priv, CLKGATE_DIS_PSL(pipe),
- intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) |
DPFR_GATING_DIS);
+ intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(pipe), 0,
DPFR_GATING_DIS);
else
- intel_de_write(dev_priv, CLKGATE_DIS_PSL(pipe),
- intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) &
~DPFR_GATING_DIS);
+ intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(pipe), DPFR_GATING_DIS,
0);
}
/* Wa_1604331009:icl,jsl,ehl */
@@ -1852,12 +1850,10 @@ static void hsw_set_frame_start_delay(const struct
intel_crtc_state *crtc_state)
enum transcoder transcoder = crtc_state->cpu_transcoder;
i915_reg_t reg = DISPLAY_VER(dev_priv) >= 14 ?
MTL_CHICKEN_TRANS(transcoder) :
CHICKEN_TRANS(transcoder);
- u32 val;
- val = intel_de_read(dev_priv, reg);
- val &= ~HSW_FRAME_START_DELAY_MASK;
- val |= HSW_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
- intel_de_write(dev_priv, reg, val);
+ intel_de_rmw(dev_priv, reg,
+HSW_FRAME_START_DELAY_MASK,
+HSW_FRAME_START_DELAY(crtc_state->framestart_delay - 1));
}
static void icl_ddi_bigjoiner_pre_enable(struct intel_atomic_state *state,
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 1a23ecd4623a53..90d7a623d6e3cc 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1260,9 +1260,7 @@ static void hsw_disable_lcpll(struct drm_i915_private
*dev_priv,
drm_err(_priv->drm, "D_COMP RCOMP still in progress\n");
if (allow_power_down) {
- val = intel_de_read(dev_priv, LCPLL_CTL);
- val |= LCPLL_POWER_DOWN_ALLOW;
- intel_de_write(dev_priv, LCPLL_CTL, val);
+ intel_de_rmw(dev_priv, LCPLL_CTL, 0, LCPLL_POWER_DOWN_ALLOW);
intel_de_posting_read(dev_priv, LCPLL_CTL);
}
}
@@ -1306,9 +1304,7 @@ static void hsw_restore_lcpll(struct drm_i915_private
*dev_priv)
drm_err(_priv->drm, "LCPLL not locked yet\n");
if (val & LCPLL_CD_SOURCE_FCLK) {
- val = intel_de_read(dev_priv, LCPLL_CTL);
- val &= ~LCPLL_CD_SOURCE_FCLK;
- intel_de_write(dev_priv, LCPLL_CTL, val);
+ intel_de_rmw(dev_priv, LCPLL_CTL, LCPLL_CD_SOURCE_FCLK, 0);
if (wait_for_us((intel_de_read(dev_priv, LCPLL_CTL) &
LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
@@ -1347,15 +1343,11 @@ static void hsw_restore_lcpll(struct drm_i915_private
*dev_priv)
*/
static void hsw_enable_pc8(struct drm_i915_private *dev_priv)
{
- u32 val;
-
drm_dbg_kms(_priv->drm, "Enabling package C8+\n");
-
Re: [Intel-gfx] [PATCH v5 1/2] drm/i915/mtl: limit second scaler vertical scaling in ver >= 14
On Tue, 2022-12-20 at 13:09 +0200, Ville Syrjälä wrote:
> On Tue, Dec 20, 2022 at 10:11:16AM +0200, Luca Coelho wrote:
> > In newer hardware versions (i.e. display version >= 14), the second
> > scaler doesn't support vertical scaling.
> >
> > The current implementation of the scaling limits is simplified and
> > only occurs when the planes are created, so we don't know which scaler
> > is being used.
> >
> > In order to handle separate scaling limits for horizontal and vertical
> > scaling, and different limits per scaler, split the checks in two
> > phases. We first do a simple check during plane creation and use the
> > best-case scenario (because we don't know the scaler that may be used
> > at a later point) and then do a more specific check when the scalers
> > are actually being set up.
> >
> > Signed-off-by: Luca Coelho
> > ---
> >
> > In v2:
> >* fix DRM_PLANE_NO_SCALING renamed macros;
> >
> > In v3:
> >* No changes.
> >
> > In v4:
> >* Got rid of the changes in the general planes max scale code;
> >* Added a couple of FIXMEs;
> >* Made intel_atomic_setup_scaler() return an int with errors;
> >
> > In v5:
> >* Just resent with a cover letter.
> >
> > drivers/gpu/drm/i915/display/i9xx_plane.c | 4 +-
> > drivers/gpu/drm/i915/display/intel_atomic.c | 83 ---
> > .../gpu/drm/i915/display/intel_atomic_plane.c | 30 ++-
> > .../gpu/drm/i915/display/intel_atomic_plane.h | 2 +-
> > drivers/gpu/drm/i915/display/intel_cursor.c | 4 +-
> > drivers/gpu/drm/i915/display/intel_sprite.c | 20 +
> > .../drm/i915/display/skl_universal_plane.c| 45 +-
> > 7 files changed, 128 insertions(+), 60 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
> > b/drivers/gpu/drm/i915/display/i9xx_plane.c
> > index ecaeb7dc196b..390e96f0692b 100644
> > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> > @@ -326,9 +326,7 @@ i9xx_plane_check(struct intel_crtc_state *crtc_state,
> > if (ret)
> > return ret;
> >
> > - ret = intel_atomic_plane_check_clipping(plane_state, crtc_state,
> > - DRM_PLANE_NO_SCALING,
> > - DRM_PLANE_NO_SCALING,
> > + ret = intel_atomic_plane_check_clipping(plane_state, crtc_state, false,
> >
> > i9xx_plane_has_windowing(plane));
> > if (ret)
> > return ret;
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index 6621aa245caf..bf4761a40675 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -38,6 +38,7 @@
> > #include "intel_atomic.h"
> > #include "intel_cdclk.h"
> > #include "intel_display_types.h"
> > +#include "intel_fb.h"
> > #include "intel_global_state.h"
> > #include "intel_hdcp.h"
> > #include "intel_psr.h"
> > @@ -310,11 +311,11 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
> > kfree(crtc_state);
> > }
> >
> > -static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state
> > *scaler_state,
> > - int num_scalers_need, struct intel_crtc
> > *intel_crtc,
> > - const char *name, int idx,
> > - struct intel_plane_state *plane_state,
> > - int *scaler_id)
> > +static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state
> > *scaler_state,
> > +int num_scalers_need, struct intel_crtc
> > *intel_crtc,
> > +const char *name, int idx,
> > +struct intel_plane_state *plane_state,
> > +int *scaler_id)
> > {
> > struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> > int j;
> > @@ -334,7 +335,7 @@ static void intel_atomic_setup_scaler(struct
> > intel_crtc_scaler_state *scaler_sta
> >
> > if (drm_WARN(_priv->drm, *scaler_id < 0,
> > "Cannot find scaler for %s:%d\n", name, idx))
> > - return;
> > + return -EBUSY;
> >
> > /* set scaler mode */
> > if (plane_state && plane_state->hw.fb &&
> > @@ -375,9 +376,69 @@ static void intel_atomic_setup_scaler(struct
> > intel_crtc_scaler_state *scaler_sta
> > mode = SKL_PS_SCALER_MODE_DYN;
> > }
> >
> > + /*
> > +* FIXME: we should also check the scaler factors for pfit, so
> > +* this shouldn't be tied directly to planes.
> > +*/
> > + if (plane_state && plane_state->hw.fb) {
> > + const struct drm_framebuffer *fb = plane_state->hw.fb;
> > + struct drm_rect *src = _state->uapi.src;
> > + struct drm_rect *dst = _state->uapi.dst;
> > + int hscale, vscale, max_vscale, max_hscale;
[Intel-gfx] [PATCH] drm/i915/ttm: remove the virtualized start hack
This was mostly needed to differentiate between mappable and non-mappable lmem, such that ttm would understand non-mappable -> mappable moves (or vice versa), and not just turn them into noops. We have since gained proper .intersects() and .compatible() hooks for the resource manager, which takes care of this for us. Signed-off-by: Matthew Auld Cc: Nirmoy Das Cc: Andrzej Hajda --- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index a72698a2dbc8..a1bc804cfa15 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -139,13 +139,6 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, mutex_unlock(>lock); - if (place->lpfn - place->fpfn == n_pages) - bman_res->base.start = place->fpfn; - else if (lpfn <= bman->visible_size) - bman_res->base.start = 0; - else - bman_res->base.start = bman->visible_size; - *res = _res->base; return 0; -- 2.38.1
Re: [Intel-gfx] [PATCH] Copy highest enabled wm level to disabled wm levels for gen >= 12
On Mon, Dec 19, 2022 at 09:29:19AM +0200, Stanislav Lisovskiy wrote:
> There was a specific SW workaround requested, which should prevent
> some watermark issues happening, which requires copying highest
> enabled wm level to those disabled wm levels(bit 31 is of course
> still needs to be cleared).
>
> Signed-off-by: Stanislav Lisovskiy
> ---
> drivers/gpu/drm/i915/display/skl_watermark.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index ae4e9e680c2e3..b12e11cd6e579 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -1591,6 +1591,13 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state
> *state,
> wm->wm[level].lines = wm->wm[0].lines;
> wm->wm[level].ignore_lines =
> wm->wm[0].ignore_lines;
> }
> +
> + /* Wa_14017887344 */
> + if (DISPLAY_VER(i915) >= 12 && level > 0) {
> + wm->wm[level].blocks = wm->wm[level - 1].blocks;
> + wm->wm[level].lines = wm->wm[level - 1].lines;
> + wm->wm[level].ignore_lines = wm->wm[level -
> 1].ignore_lines;
> + }
Hmm. Reading the parent hsd this smells at least partially as
some kind of race in the Windows driver between async flip,
wm1+ disable, and the PSR wm level override behaviour.
We never do async flip + wm1+ disable from the same commit
which itself might be sufficient to avoid the issue. I
didn't think that even worked, but maybe it sort of does
if Windows attempts it. However since we don't do that we
might never hit this. Not sure.
The PSR wm level override stuff we don't handle at all
currently. I'm thinking that is something we should
remedy first.
Also while thinking about how to unify this and the
already existing wm1 w/a I realized that we don't
check if the wm level is actually enabled or not.
So it's interfering with commit a301cb0fca2d ("drm/i915:
Keep plane watermarks enabled more aggressively").
My gut reaction is that we want a wm[level].enable
check there, but I've not fully thought through the
implications...
> }
> }
>
> --
> 2.37.3
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH 3/3] drm/i915/uc: Fix two issues with over-size firmware files
On 12/20/2022 3:41 AM, john.c.harri...@intel.com wrote:
From: John Harrison
In the case where a firmware file is too large (e.g. someone
downloaded a web page ASCII dump from github...), the firmware object
is released but the pointer is not zerod. If no other firmware file
was found then release would be called again leading to a double kfree.
Also, the size check was only being applied to the initial firmware
load not any of the subsequent attempts. So move the check into a
wrapper that is used for all loads.
Fixes: 016241168dc5 ("drm/i915/uc: use different ggtt pin offsets for uc loads")
Signed-off-by: John Harrison
Cc: Daniele Ceraolo Spurio
Cc: Alan Previn
Cc: Rodrigo Vivi
Cc: Matt Roper
Cc: Jani Nikula
Cc: Matthew Auld
Cc: "Thomas Hellström"
There was another patch that was sent to fix the double free
(https://patchwork.freedesktop.org/series/111545/), but this one is
better because it also fixes the size check.
Reviewed-by: Daniele Ceraolo Spurio
Daniele
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 42
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index d6ff6c584c1e1..06b5f92ba3a55 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -675,6 +675,32 @@ static int check_fw_header(struct intel_gt *gt,
return 0;
}
+int try_firmware_load(struct intel_uc_fw *uc_fw, const struct firmware **fw)
+{
+ struct intel_gt *gt = __uc_fw_to_gt(uc_fw);
+ struct device *dev = gt->i915->drm.dev;
+ int err;
+
+ err = firmware_request_nowarn(fw, uc_fw->file_selected.path, dev);
+
+ if (err)
+ return err;
+
+ if ((*fw)->size > INTEL_UC_RSVD_GGTT_PER_FW) {
+ drm_err(>i915->drm,
+ "%s firmware %s: size (%zuKB) exceeds max supported size
(%uKB)\n",
+ intel_uc_fw_type_repr(uc_fw->type),
uc_fw->file_selected.path,
+ (*fw)->size / SZ_1K, INTEL_UC_RSVD_GGTT_PER_FW / SZ_1K);
+
+ /* try to find another blob to load */
+ release_firmware(*fw);
+ *fw = NULL;
+ return -ENOENT;
+ }
+
+ return 0;
+}
+
/**
* intel_uc_fw_fetch - fetch uC firmware
* @uc_fw: uC firmware
@@ -688,7 +714,6 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
struct intel_gt *gt = __uc_fw_to_gt(uc_fw);
struct drm_i915_private *i915 = gt->i915;
struct intel_uc_fw_file file_ideal;
- struct device *dev = i915->drm.dev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
bool old_ver = false;
@@ -704,20 +729,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
__force_fw_fetch_failures(uc_fw, -EINVAL);
__force_fw_fetch_failures(uc_fw, -ESTALE);
- err = firmware_request_nowarn(, uc_fw->file_selected.path, dev);
+ err = try_firmware_load(uc_fw, );
memcpy(_ideal, _fw->file_wanted, sizeof(file_ideal));
- if (!err && fw->size > INTEL_UC_RSVD_GGTT_PER_FW) {
- drm_err(>drm,
- "%s firmware %s: size (%zuKB) exceeds max supported size
(%uKB)\n",
- intel_uc_fw_type_repr(uc_fw->type),
uc_fw->file_selected.path,
- fw->size / SZ_1K, INTEL_UC_RSVD_GGTT_PER_FW / SZ_1K);
-
- /* try to find another blob to load */
- release_firmware(fw);
- err = -ENOENT;
- }
-
/* Any error is terminal if overriding. Don't bother searching for
older versions */
if (err && intel_uc_fw_is_overridden(uc_fw))
goto fail;
@@ -738,7 +752,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
break;
}
- err = firmware_request_nowarn(, uc_fw->file_selected.path, dev);
+ err = try_firmware_load(uc_fw, );
}
if (err)
Re: [Intel-gfx] [PATCH v2] drm/i915/hdmi: Go for scrambling only if platform supports TMDS clock > 340MHz
On 12/20/2022 2:31 PM, Jani Nikula wrote:
On Tue, 20 Dec 2022, Ankit Nautiyal wrote:
There are cases, where devices have an HDMI1.4 retimer, and TMDS clock rate
is capped to 340MHz via VBT. In such cases scrambling might be supported
by the platform and an HDMI2.0 sink for lower TMDS rates, but not
supported by the retimer, causing blankouts.
So avoid enabling scrambling, if the TMDS clock is capped to <= 340MHz.
v2: Added comment, documenting the rationale to check for TMDS clock,
before going for scrambling. (Arun)
Signed-off-by: Ankit Nautiyal
Reviewed-by: Arun R Murthy
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index efa2da080f62..7603426af9a0 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2244,6 +2244,25 @@ static bool intel_hdmi_is_cloned(const struct
intel_crtc_state *crtc_state)
!is_power_of_2(crtc_state->uapi.encoder_mask);
}
+static bool source_can_support_scrambling(struct intel_encoder *encoder)
"source can support scrambling" reads like "if this function returns
true, source might support scrambing, but also might not, depends on
something else".
So does this mean "source supports scrambling" or "source can support
scrambling"? If the latter, as it is now named, what *else* is required
for source to support scrambling?
Hmm.. the intention is to have a function to check if source supports
scrambling.
Sink side support is checked by 'scdc->scrambling.supported' so we want
to have a function that tells us, whether source supports scrambling.
I will modify the function name to : source_supports_scrambling() to
avoid the confusion.
Thanks & Regards,
Ankit
BR,
Jani.
+{
+ /*
+* Gen 10+ support HDMI 2.0 : the max tmds clock is 594MHz, and
+* scrambling is supported.
+* But there seem to be cases where certain platforms that support
+* HDMI 2.0, have an HDMI1.4 retimer chip, and the max tmds clock is
+* capped by VBT to less than 340MHz.
+*
+* In such cases when an HDMI2.0 sink is connected, it creates a
+* problem : the platform and the sink both support scrambling but the
+* HDMI 1.4 retimer chip doesn't.
+*
+* So go for scrambling, based on the max tmds clock taking into
account,
+* restrictions coming from VBT.
+*/
+ return intel_hdmi_source_max_tmds_clock(encoder) > 34;
+}
+
int intel_hdmi_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config,
struct drm_connector_state *conn_state)
@@ -2301,7 +2320,7 @@ int intel_hdmi_compute_config(struct intel_encoder
*encoder,
pipe_config->lane_count = 4;
- if (scdc->scrambling.supported && DISPLAY_VER(dev_priv) >= 10) {
+ if (scdc->scrambling.supported &&
source_can_support_scrambling(encoder)) {
if (scdc->scrambling.low_rates)
pipe_config->hdmi_scrambling = true;
Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp_mst: don't pull unregistered connectors into state
On Tue, Dec 20, 2022 at 12:39:17PM +0200, Ville Syrjälä wrote:
> On Thu, Dec 15, 2022 at 03:51:50PM +, Simon Ser wrote:
> > In intel_dp_mst_atomic_master_trans_check(), we pull connectors
> > sharing the same DP-MST stream into the atomic state. However,
> > if the connector is unregistered, this later fails with:
> >
> > [ 559.425658] i915 :00:02.0: [drm:drm_atomic_helper_check_modeset]
> > [CONNECTOR:378:DP-7] is not registered
> >
> > Skip these unregistered connectors to allow user-space to turn them
> > off.
> >
> > Fixes part of this wlroots issue:
> > https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3407
> >
> > Signed-off-by: Simon Ser
> > Cc: Ville Syrjälä
> > Cc: Jani Nikula
> > Cc: Lyude Paul
> > ---
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index f773e117ebc4..70859a927a9d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -280,7 +280,8 @@ intel_dp_mst_atomic_master_trans_check(struct
> > intel_connector *connector,
> > struct intel_crtc *crtc;
> >
> > if (connector_iter->mst_port != connector->mst_port ||
> > - connector_iter == connector)
> > + connector_iter == connector ||
> > + connector_iter->base.registration_state ==
> > DRM_CONNECTOR_UNREGISTERED)
> > continue;
>
> We can't really do that. It would risk leaving slave transcoders
> enabled while the master is undergoing a full modeset.
>
> I think a couple of ways we could go about this:
> - kill the registration check entirely/partially
> I think Imre already has some plans for the partial killing
> due to some type-c vs. pm firmware issues that also need force
> a full modeset
Looks like in this case the problem is that the core's check for routing
changes should be applied only to connectors passed in via the commit state,
however it's also done for some of the connectors added by drivers as a
dependency. Making that consistent would need the following change, probably
fixing the above issue as well:
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index d579fd8f7cb83..9c4c67f8059b8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -673,9 +673,15 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
if (ret)
return ret;
+ for_each_new_connector_in_state(state, connector, new_connector_state,
i)
+ connectors_mask |= BIT(i);
+
for_each_oldnew_connector_in_state(state, connector,
old_connector_state, new_connector_state, i) {
const struct drm_connector_helper_funcs *funcs =
connector->helper_private;
+ if (!(BIT(i) & connectors_mask))
+ continue;
+
WARN_ON(!drm_modeset_is_locked(>mode_config.connection_mutex));
/*
@@ -708,8 +714,6 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
connector->base.id, connector->name);
return ret;
}
-
- connectors_mask |= BIT(i);
}
/*
> - relocate this stuff to happen after drm_atomic_helper_check_modeset()
> like we already do for eg. bigjoiner. IIRC this was discussed as an
> option when we added intel_dp_mst_atomic_master_trans_check() but
> I don't recall anymore why we specifically chose to do this from
> connector.atomic_check().
>
> >
> > conn_iter_state =
> > intel_atomic_get_digital_connector_state(state,
> > --
> > 2.39.0
> >
>
> --
> Ville Syrjälä
> Intel
Re: [Intel-gfx] [PATCH v5 1/2] drm/i915/mtl: limit second scaler vertical scaling in ver >= 14
On Tue, Dec 20, 2022 at 10:11:16AM +0200, Luca Coelho wrote:
> In newer hardware versions (i.e. display version >= 14), the second
> scaler doesn't support vertical scaling.
>
> The current implementation of the scaling limits is simplified and
> only occurs when the planes are created, so we don't know which scaler
> is being used.
>
> In order to handle separate scaling limits for horizontal and vertical
> scaling, and different limits per scaler, split the checks in two
> phases. We first do a simple check during plane creation and use the
> best-case scenario (because we don't know the scaler that may be used
> at a later point) and then do a more specific check when the scalers
> are actually being set up.
>
> Signed-off-by: Luca Coelho
> ---
>
> In v2:
>* fix DRM_PLANE_NO_SCALING renamed macros;
>
> In v3:
>* No changes.
>
> In v4:
>* Got rid of the changes in the general planes max scale code;
>* Added a couple of FIXMEs;
>* Made intel_atomic_setup_scaler() return an int with errors;
>
> In v5:
>* Just resent with a cover letter.
>
> drivers/gpu/drm/i915/display/i9xx_plane.c | 4 +-
> drivers/gpu/drm/i915/display/intel_atomic.c | 83 ---
> .../gpu/drm/i915/display/intel_atomic_plane.c | 30 ++-
> .../gpu/drm/i915/display/intel_atomic_plane.h | 2 +-
> drivers/gpu/drm/i915/display/intel_cursor.c | 4 +-
> drivers/gpu/drm/i915/display/intel_sprite.c | 20 +
> .../drm/i915/display/skl_universal_plane.c| 45 +-
> 7 files changed, 128 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
> b/drivers/gpu/drm/i915/display/i9xx_plane.c
> index ecaeb7dc196b..390e96f0692b 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> @@ -326,9 +326,7 @@ i9xx_plane_check(struct intel_crtc_state *crtc_state,
> if (ret)
> return ret;
>
> - ret = intel_atomic_plane_check_clipping(plane_state, crtc_state,
> - DRM_PLANE_NO_SCALING,
> - DRM_PLANE_NO_SCALING,
> + ret = intel_atomic_plane_check_clipping(plane_state, crtc_state, false,
>
> i9xx_plane_has_windowing(plane));
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index 6621aa245caf..bf4761a40675 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -38,6 +38,7 @@
> #include "intel_atomic.h"
> #include "intel_cdclk.h"
> #include "intel_display_types.h"
> +#include "intel_fb.h"
> #include "intel_global_state.h"
> #include "intel_hdcp.h"
> #include "intel_psr.h"
> @@ -310,11 +311,11 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
> kfree(crtc_state);
> }
>
> -static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state
> *scaler_state,
> - int num_scalers_need, struct intel_crtc
> *intel_crtc,
> - const char *name, int idx,
> - struct intel_plane_state *plane_state,
> - int *scaler_id)
> +static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state
> *scaler_state,
> + int num_scalers_need, struct intel_crtc
> *intel_crtc,
> + const char *name, int idx,
> + struct intel_plane_state *plane_state,
> + int *scaler_id)
> {
> struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> int j;
> @@ -334,7 +335,7 @@ static void intel_atomic_setup_scaler(struct
> intel_crtc_scaler_state *scaler_sta
>
> if (drm_WARN(_priv->drm, *scaler_id < 0,
>"Cannot find scaler for %s:%d\n", name, idx))
> - return;
> + return -EBUSY;
>
> /* set scaler mode */
> if (plane_state && plane_state->hw.fb &&
> @@ -375,9 +376,69 @@ static void intel_atomic_setup_scaler(struct
> intel_crtc_scaler_state *scaler_sta
> mode = SKL_PS_SCALER_MODE_DYN;
> }
>
> + /*
> + * FIXME: we should also check the scaler factors for pfit, so
> + * this shouldn't be tied directly to planes.
> + */
> + if (plane_state && plane_state->hw.fb) {
> + const struct drm_framebuffer *fb = plane_state->hw.fb;
> + struct drm_rect *src = _state->uapi.src;
> + struct drm_rect *dst = _state->uapi.dst;
> + int hscale, vscale, max_vscale, max_hscale;
> +
> + /*
> + * FIXME: When two scalers are needed, but only one of
> + * them needs to downscale, we should make sure that
> + * the one that needs downscaling
[Intel-gfx] [PATCH i-g-t v3] tests/i915/gem_ppgtt: verify GTT eviction with contended locks
We should still be able to GTT evict objects during execbuf (old
bindings can linger around), even if there is object lock contention. In
the worst case the execbuf should just wait on the contented locks.
Returning -ENOSPC smells like a regression from past behaviour, and
seems to break userspace.
v2:
- Add coverage for explicit softpin
- Add timeout for the spinner
v3:
- Improve the test description
References: https://gitlab.freedesktop.org/drm/intel/-/issues/7570
Signed-off-by: Matthew Auld
Cc: Andrzej Hajda
Cc: Nirmoy Das
Cc: Mani Milani
---
tests/i915/gem_ppgtt.c | 133 +
1 file changed, 133 insertions(+)
diff --git a/tests/i915/gem_ppgtt.c b/tests/i915/gem_ppgtt.c
index 9673ce22..024e8d47 100644
--- a/tests/i915/gem_ppgtt.c
+++ b/tests/i915/gem_ppgtt.c
@@ -255,6 +255,131 @@ static void flink_and_close(void)
close(fd2);
}
+#define PAGE_SIZE 4096
+
+static uint32_t batch_create_size(int fd, uint64_t size)
+{
+ const uint32_t bbe = MI_BATCH_BUFFER_END;
+ uint32_t handle;
+
+ handle = gem_create(fd, size);
+ gem_write(fd, handle, 0, , sizeof(bbe));
+
+ return handle;
+}
+
+#define IGT_USE_ANY0x1
+#define IGT_USE_PINNED 0x2
+static void upload(int fd, uint32_t handle, uint32_t in_fence, uint32_t ctx_id,
+ unsigned int flags)
+{
+ struct drm_i915_gem_exec_object2 exec[2] = {};
+ struct drm_i915_gem_execbuffer2 execbuf = {
+ .buffers_ptr = to_user_pointer(),
+ .buffer_count = 1,
+ .rsvd1 = ctx_id,
+ };
+
+ if (in_fence) {
+ execbuf.rsvd2 = in_fence;
+ execbuf.flags = I915_EXEC_FENCE_IN;
+ }
+
+ exec[0].handle = handle;
+ exec[0].flags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+
+ if (flags & IGT_USE_PINNED)
+ exec[0].flags |= EXEC_OBJECT_PINNED; /* offset = 0 */
+
+ if (flags & IGT_USE_ANY) {
+ exec[0].flags |= EXEC_OBJECT_PAD_TO_SIZE;
+ exec[0].pad_to_size = gem_aperture_size(fd);
+ }
+
+ gem_execbuf(fd, );
+}
+
+static void shrink_vs_evict(unsigned int flags)
+{
+ const unsigned int nproc = sysconf(_SC_NPROCESSORS_ONLN) + 1;
+ const uint64_t timeout_5s = 50LL;
+ int fd = drm_open_driver(DRIVER_INTEL);
+ uint64_t ahnd = get_reloc_ahnd(fd, 0);
+ const intel_ctx_t *ctx_arr[nproc];
+ igt_spin_t *spinner;
+ uint32_t handle1;
+ int i;
+
+ /*
+* Try to simulate some nasty object lock contention during GTT
+* eviction. Create a BO and bind across several different VMs. Invoke
+* the shrinker on that shared BO, followed by triggering GTT eviction
+* across all VMs. Both require the object lock to make forward
+* progress when trying to unbind the BO, but the shrinker will be
+* blocked by the spinner (until killed). Once the spinner is killed
+* the shrinker should be able to unbind the object and drop the object
+* lock, and GTT eviction should eventually succeed. At no point should
+* we see -ENOSPC from the execbuf, even if we can't currently grab the
+* object lock.
+*/
+
+ igt_require(gem_uses_full_ppgtt(fd));
+
+ igt_drop_caches_set(fd, DROP_ALL);
+
+ handle1 = gem_create(fd, PAGE_SIZE);
+
+ spinner = igt_spin_new(fd,
+ .ahnd = ahnd,
+ .flags = IGT_SPIN_FENCE_OUT);
+ igt_spin_set_timeout(spinner, timeout_5s);
+
+ /*
+* Create several VMs to ensure we don't block on the same vm lock. The
+* goal of the test is to ensure that object lock contention doesn't
+* somehow result in -ENOSPC from execbuf, if we need to trigger GTT
+* eviction.
+*/
+ for (i = 0; i < nproc; i++) {
+ ctx_arr[i] = intel_ctx_create(fd, NULL);
+
+ upload(fd, handle1, spinner->execbuf.rsvd2 >> 32,
+ ctx_arr[i]->id, flags);
+ }
+
+ igt_fork(child, 1)
+ igt_drop_caches_set(fd, DROP_ALL);
+
+ sleep(2); /* Give the shrinker time to find handle1 */
+
+ igt_fork(child, nproc) {
+ uint32_t handle2 = gem_create(fd, PAGE_SIZE);
+
+ /*
+* One of these forks will be stuck on the vm mutex, since the
+* shrinker is holding it (along with the object lock) while
+* trying to unbind the chosen vma, but is blocked by the
+* spinner. The rest should only block waiting to grab the
+* object lock for handle1, before then trying to GTT evict it
+* from their respective vm. In either case the contention of
+* the vm->mutex or object lock should never result in -ENOSPC
+* or some other error.
+*/
+ handle2 =
Re: [Intel-gfx] [PATCH v5] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence
On Mon, Dec 19, 2022 at 12:59:55PM +0200, Jani Nikula wrote:
> Starting from ICL, the default for MIPI GPIO sequences seems to be using
> native GPIOs i.e. GPIOs available in the GPU. These native GPIOs reuse
> many pins that quite frankly seem scary to poke based on the VBT
> sequences. We pretty much have to trust that the board is configured
> such that the relevant HPD, PP_CONTROL and GPIO bits aren't used for
> anything else.
>
> MIPI sequence v4 also adds a flag to fall back to non-native sequences.
>
> v5:
> - Wrap SHOTPLUG_CTL_DDI modification in spin_lock() in icp_irq_handler()
> too (Ville)
> - References instead of Closes issue 6131 because this does not fix everything
>
> v4:
> - Wrap SHOTPLUG_CTL_DDI modification in spin_lock_irq() (Ville)
>
> v3:
> - Fix -Wbitwise-conditional-parentheses (kernel test robot )
>
> v2:
> - Fix HPD pin output set (impacts GPIOs 0 and 5)
> - Fix GPIO data output direction set (impacts GPIOs 4 and 9)
> - Reduce register accesses to single intel_de_rwm()
>
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/6131
> Cc: Ville Syrjälä
> Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 94 +++-
> drivers/gpu/drm/i915/i915_irq.c | 3 +
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 3 files changed, 95 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index fce69fa446d5..41f025f089d9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -41,9 +41,11 @@
>
> #include "i915_drv.h"
> #include "i915_reg.h"
> +#include "intel_de.h"
> #include "intel_display_types.h"
> #include "intel_dsi.h"
> #include "intel_dsi_vbt.h"
> +#include "intel_gmbus_regs.h"
> #include "vlv_dsi.h"
> #include "vlv_dsi_regs.h"
> #include "vlv_sideband.h"
> @@ -377,6 +379,85 @@ static void icl_exec_gpio(struct intel_connector
> *connector,
> drm_dbg_kms(_priv->drm, "Skipping ICL GPIO element execution\n");
> }
>
> +enum {
> + MIPI_RESET_1 = 0,
> + MIPI_AVDD_EN_1,
> + MIPI_BKLT_EN_1,
> + MIPI_AVEE_EN_1,
> + MIPI_VIO_EN_1,
> + MIPI_RESET_2,
> + MIPI_AVDD_EN_2,
> + MIPI_BKLT_EN_2,
> + MIPI_AVEE_EN_2,
> + MIPI_VIO_EN_2,
> +};
> +
> +static void icl_native_gpio_set_value(struct drm_i915_private *dev_priv,
> + int gpio, bool value)
> +{
> + int index;
> +
> + if (drm_WARN_ON(_priv->drm, DISPLAY_VER(dev_priv) == 11 && gpio >=
> MIPI_RESET_2))
> + return;
> +
> + switch (gpio) {
> + case MIPI_RESET_1:
> + case MIPI_RESET_2:
> + index = gpio == MIPI_RESET_1 ? HPD_PORT_A : HPD_PORT_B;
> +
> + /*
> + * Disable HPD to set the pin to output, and set output
> + * value. The HPD pin should not be enabled for DSI anyway,
> + * assuming the board design and VBT are sane, and the pin isn't
> + * used by a non-DSI encoder.
> + *
> + * The locking protects against concurrent SHOTPLUG_CTL_DDI
> + * modifications in irq setup and handling.
> + */
> + spin_lock_irq(_priv->irq_lock);
> + intel_de_rmw(dev_priv, SHOTPLUG_CTL_DDI,
> + SHOTPLUG_CTL_DDI_HPD_ENABLE(index) |
> + SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(index),
> + value ? SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(index) :
> 0);
> + spin_unlock_irq(_priv->irq_lock);
> + break;
> + case MIPI_AVDD_EN_1:
> + case MIPI_AVDD_EN_2:
> + index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
> +
> + intel_de_rmw(dev_priv, PP_CONTROL(index), PANEL_POWER_ON,
> + value ? PANEL_POWER_ON : 0);
> + break;
> + case MIPI_BKLT_EN_1:
> + case MIPI_BKLT_EN_2:
> + index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
> +
> + intel_de_rmw(dev_priv, PP_CONTROL(index), EDP_BLC_ENABLE,
> + value ? EDP_BLC_ENABLE : 0);
> + break;
> + case MIPI_AVEE_EN_1:
> + case MIPI_AVEE_EN_2:
> + index = gpio == MIPI_AVEE_EN_1 ? 1 : 2;
> +
> + intel_de_rmw(dev_priv, GPIO(dev_priv, index),
> + GPIO_CLOCK_VAL_OUT,
> + GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_DIR_OUT |
> + GPIO_CLOCK_VAL_MASK | (value ? GPIO_CLOCK_VAL_OUT
> : 0));
> + break;
> + case MIPI_VIO_EN_1:
> + case MIPI_VIO_EN_2:
> + index = gpio == MIPI_VIO_EN_1 ? 1 : 2;
> +
> + intel_de_rmw(dev_priv, GPIO(dev_priv, index),
> + GPIO_DATA_VAL_OUT,
> + GPIO_DATA_DIR_MASK | GPIO_DATA_DIR_OUT |
> +
Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp_mst: don't pull unregistered connectors into state
On Thu, Dec 15, 2022 at 03:51:50PM +, Simon Ser wrote: > In intel_dp_mst_atomic_master_trans_check(), we pull connectors > sharing the same DP-MST stream into the atomic state. However, > if the connector is unregistered, this later fails with: > > [ 559.425658] i915 :00:02.0: [drm:drm_atomic_helper_check_modeset] > [CONNECTOR:378:DP-7] is not registered > > Skip these unregistered connectors to allow user-space to turn them > off. > > Fixes part of this wlroots issue: > https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3407 > > Signed-off-by: Simon Ser > Cc: Ville Syrjälä > Cc: Jani Nikula > Cc: Lyude Paul > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index f773e117ebc4..70859a927a9d 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -280,7 +280,8 @@ intel_dp_mst_atomic_master_trans_check(struct > intel_connector *connector, > struct intel_crtc *crtc; > > if (connector_iter->mst_port != connector->mst_port || > - connector_iter == connector) > + connector_iter == connector || > + connector_iter->base.registration_state == > DRM_CONNECTOR_UNREGISTERED) > continue; We can't really do that. It would risk leaving slave transcoders enabled while the master is undergoing a full modeset. I think a couple of ways we could go about this: - kill the registration check entirely/partially I think Imre already has some plans for the partial killing due to some type-c vs. pm firmware issues that also need force a full modeset - relocate this stuff to happen after drm_atomic_helper_check_modeset() like we already do for eg. bigjoiner. IIRC this was discussed as an option when we added intel_dp_mst_atomic_master_trans_check() but I don't recall anymore why we specifically chose to do this from connector.atomic_check(). > > conn_iter_state = > intel_atomic_get_digital_connector_state(state, > -- > 2.39.0 > -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH v2] drm/i915: Use helper func to find out map type
On 12/19/2022 8:04 PM, Andrzej Hajda wrote:
On 19.12.2022 12:29, Nirmoy Das wrote:
Use i915_coherent_map_type() function to find out
map_type of the shmem obj.
v2: handle non-llc platform(Matt)
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/gt/shmem_utils.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.c
b/drivers/gpu/drm/i915/gt/shmem_utils.c
index 402f085f3a02..449c9ed44382 100644
--- a/drivers/gpu/drm/i915/gt/shmem_utils.c
+++ b/drivers/gpu/drm/i915/gt/shmem_utils.c
@@ -8,6 +8,7 @@
#include
#include
+#include "i915_drv.h"
#include "gem/i915_gem_object.h"
#include "gem/i915_gem_lmem.h"
#include "shmem_utils.h"
@@ -32,6 +33,8 @@ struct file *shmem_create_from_data(const char
*name, void *data, size_t len)
struct file *shmem_create_from_object(struct drm_i915_gem_object
*obj)
{
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ enum i915_map_type map_type;
struct file *file;
void *ptr;
@@ -41,8 +44,8 @@ struct file *shmem_create_from_object(struct
drm_i915_gem_object *obj)
return file;
}
- ptr = i915_gem_object_pin_map_unlocked(obj,
i915_gem_object_is_lmem(obj) ?
- I915_MAP_WC : I915_MAP_WB);
+ map_type = i915_coherent_map_type(i915, obj, true);
+ ptr = i915_gem_object_pin_map_unlocked(obj, map_type);
More lines, but less branches, some gain :)
Yes and I am more behind the idea of unifying finding map_type with this
helper.
Reviewed-by: Andrzej Hajda
Thanks!
Nirmoy
Regards
Andrzej
if (IS_ERR(ptr))
return ERR_CAST(ptr);
Re: [Intel-gfx] [PATCH 1/1] drm/i915: re-disable RC6p on Sandy Bridge
On Mon, Dec 19, 2022 at 06:29:27PM +0100, Sasa Dragic wrote:
> RC6p on Sandy Bridge got re-enabled over time, causing visual glitches
> and GPU hangs.
>
> Disabled originally in commit 1c8ecf80fdee ("drm/i915: do not enable
> RC6p on Sandy Bridge").
re cover letter:
> It was originally disabled in commit 1c8ecf80fdee ("drm/i915: do not
> enable RC6p on Sandy Bridge"), and subsequently re-enabled by
> 13c5a577b342 ("drm/i915/gt: Select the deepest available parking mode
> for rc6"), which seems to focus only on Ivy Bridge.
That only kicks in while parked (ie. fully idle from
software POV). I think the real bad commit is
fb6db0f5bf1d ("drm/i915: Remove unsafe i915.enable_rc6")
which seems to affects which rc6 level is selected while
unparked.
We should mention both of those commits here:
Fixes: fb6db0f5bf1d ("drm/i915: Remove unsafe i915.enable_rc6")
Fixes: 13c5a577b342 ("drm/i915/gt: Select the deepest available parking mode
for rc6")
Also we want
Cc: sta...@vger.kernel.org
We can add those while pushing, so no need to resend for that.
>
> Signed-off-by: Sasa Dragic
> ---
> drivers/gpu/drm/i915/i915_pci.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 668e9da52584..69377564028a 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -423,7 +423,8 @@ static const struct intel_device_info ilk_m_info = {
> .has_coherent_ggtt = true, \
> .has_llc = 1, \
> .has_rc6 = 1, \
> - .has_rc6p = 1, \
> + /* snb does support rc6p, but enabling it causes various issues */ \
> + .has_rc6p = 0, \
The one downside of doing it this way is that we also lose
the debugfs/sysfs files so we can't monitor rc6+/++
residency anymore to make sure they are not entered.
I think ideally we'd split this into two parts: which rc6
states the hw actually has vs. which rc6 states we actually
want to use. But at least for the time being I think this
simple should be sufficient, and should be easy to backport
to stable releases.
> .has_rps = true, \
> .dma_mask_size = 40, \
> .__runtime.ppgtt_type = INTEL_PPGTT_ALIASING, \
> --
> 2.37.2
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support of Tile4 to MTL
On Mon, Dec 19, 2022 at 08:27:30PM +0200, Juha-Pekka Heikkila wrote:
> Hi Stan,
>
> On 19.12.2022 15.50, Stanislav Lisovskiy wrote:
> > We have some Tile4 tests now skipping, which were
> > supposed to be working. So lets make them work, by
> > adding display_ver 14 as supported.
> >
> > Signed-off-by: Stanislav Lisovskiy
> > ---
> > drivers/gpu/drm/i915/display/intel_fb.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
> > b/drivers/gpu/drm/i915/display/intel_fb.c
> > index 63137ae5ab217..75a17f38def53 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fb.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> > @@ -158,11 +158,11 @@ struct intel_modifier_desc {
> > static const struct intel_modifier_desc intel_modifiers[] = {
> > {
> > .modifier = I915_FORMAT_MOD_4_TILED_DG2_MC_CCS,
> > - .display_ver = { 13, 13 },
> > + .display_ver = { 13, 14 },
>
> I don't think you'd want to do this. These DG2 ccs modifiers rely on usage
> of flat ccs which is not present in Meteorlake.
>
> > .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_MC,
> > }, {
> > .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC,
> > - .display_ver = { 13, 13 },
> > + .display_ver = { 13, 14 },
>
> Let's drop this change too.
>
> > .plane_caps = INTEL_PLANE_CAP_TILING_4 |
> > INTEL_PLANE_CAP_CCS_RC_CC,
> > .ccs.cc_planes = BIT(1),
> > @@ -170,11 +170,11 @@ static const struct intel_modifier_desc
> > intel_modifiers[] = {
> > FORMAT_OVERRIDE(gen12_flat_ccs_cc_formats),
> > }, {
> > .modifier = I915_FORMAT_MOD_4_TILED_DG2_RC_CCS,
> > - .display_ver = { 13, 13 },
> > + .display_ver = { 13, 14 },
>
> And this.
Ah, I have been surfing yesterday the spec, trying to find if ccs is supported
or not, looks like
I got bit confused.
>
> > .plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_RC,
> > }, {
> > .modifier = I915_FORMAT_MOD_4_TILED,
> > - .display_ver = { 13, 13 },
> > + .display_ver = { 13, 14 },
>
> Here you could do something like ".display_ver = { 13, -1 }," to enable
> tile4 from version 13 onward and we'll fix it if it ever change in the
> future.
Yeah, recently had similar approach in other feature.
Stan
>
> /Juha-Pekka
>
> > .plane_caps = INTEL_PLANE_CAP_TILING_4,
> > }, {
> > .modifier = I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS,
>
[Intel-gfx] [PATCH] drm/i915/display: Fix a use-after-free when intel_edp_init_connector fails
We enable the DP aux channel during probe, but may free the connector
soon afterwards. Ensure the DP aux display power put is completed before
everything is freed, to prevent a use-after-free in icl_aux_pw_to_phy(),
called from icl_combo_phy_aux_power_well_disable.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
drivers/gpu/drm/i915/display/intel_display_power.h | 1 +
drivers/gpu/drm/i915/display/intel_dp_aux.c| 2 ++
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 04915f85a0df..0edb5532461f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -776,7 +776,7 @@ void intel_display_power_flush_work(struct drm_i915_private
*i915)
* Like intel_display_power_flush_work(), but also ensure that the work
* handler function is not running any more when this function returns.
*/
-static void
+void
intel_display_power_flush_work_sync(struct drm_i915_private *i915)
{
struct i915_power_domains *power_domains = >display.power.domains;
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h
b/drivers/gpu/drm/i915/display/intel_display_power.h
index 7136ea3f233e..dc10ee0519e6 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -188,6 +188,7 @@ void __intel_display_power_put_async(struct
drm_i915_private *i915,
enum intel_display_power_domain domain,
intel_wakeref_t wakeref);
void intel_display_power_flush_work(struct drm_i915_private *i915);
+void intel_display_power_flush_work_sync(struct drm_i915_private *i915);
#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
void intel_display_power_put(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
b/drivers/gpu/drm/i915/display/intel_dp_aux.c
index f1835c74bff0..1006dddad2d5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
@@ -680,6 +680,8 @@ void intel_dp_aux_fini(struct intel_dp *intel_dp)
if (cpu_latency_qos_request_active(_dp->pm_qos))
cpu_latency_qos_remove_request(_dp->pm_qos);
+ /* Ensure async work from intel_dp_aux_xfer() is flushed before we
clean up */
+ intel_display_power_flush_work_sync(dp_to_i915(intel_dp));
kfree(intel_dp->aux.name);
}
--
2.37.2
Re: [Intel-gfx] [PATCH 18/18] drm/fbdev: Remove aperture handling and FBINFO_MISC_FIRMWARE
On 12/19/22 17:05, Thomas Zimmermann wrote: > There are no users left of struct fb_info.apertures and the flag > FBINFO_MISC_FIRMWARE. Remove both and the aperture-ownership code > in the fbdev core. All code for aperture ownership is now located > in the fbdev drivers. > > Signed-off-by: Thomas Zimmermann > --- > drivers/video/fbdev/core/fbmem.c | 33 -- > drivers/video/fbdev/core/fbsysfs.c | 1 - > include/linux/fb.h | 22 > 3 files changed, 56 deletions(-) Nice patch! Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 17/18] fbdev/vga16fb: Do not use struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Acquire ownership of the firmware scanout buffer by calling Linux' > aperture helpers. Remove the use of struct fb_info.apertures and do > not set FBINFO_MISC_FIRMWARE; both of which previously configured > buffer ownership. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 16/18] fbdev/vesafb: Do not use struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Acquire ownership of the firmware scanout buffer by calling Linux' > aperture helpers. Remove the use of struct fb_info.apertures and do > not set FBINFO_MISC_FIRMWARE; both of which previously configured > buffer ownership. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 15/18] fbdev/vesafb: Remove trailing whitespaces
On 12/19/22 17:05, Thomas Zimmermann wrote: > Fix coding style. No functional changes. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 14/18] fbdev/simplefb: Do not use struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Acquire ownership of the firmware scanout buffer by calling Linux' > aperture helpers. Remove the use of struct fb_info.apertures and do > not set FBINFO_MISC_FIRMWARE; both of which previously configured > buffer ownership. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 13/18] fbdev/offb: Do not use struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Acquire ownership of the firmware scanout buffer by calling Linux' > aperture helpers. Remove the use of struct fb_info.apertures and do > not set FBINFO_MISC_FIRMWARE; both of which previously configured > buffer ownership. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 12/18] fbdev/offb: Allocate struct offb_par with framebuffer_alloc()
On 12/19/22 17:05, Thomas Zimmermann wrote: > Move the palette array into struct offb_par and allocate both via > framebuffer_alloc(), as intended by fbdev. No functional changes. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 11/18] fbdev/efifb: Do not use struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Acquire ownership of the firmware scanout buffer by calling Linux' > aperture helpers. Remove the use of struct fb_info.apertures and do > not set FBINFO_MISC_FIRMWARE; both of which previously configured > buffer ownership. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 10/18] fbdev/efifb: Add struct efifb_par for driver data
On 12/19/22 17:05, Thomas Zimmermann wrote: > The efifb_par structure holds the palette for efifb. It will also > be useful for storing the device's aperture range. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 09/18] vfio-mdev/mdpy-fb: Do not set struct fb_info.apertures
[adding Kirti Wankhede and k...@vger.kernel.org to Cc list]
On 12/19/22 17:05, Thomas Zimmermann wrote:
> Generic fbdev drivers use the apertures field in struct fb_info to
> control ownership of the framebuffer memory and graphics device. Do
> not set the values in mdpy-fb.
>
> Signed-off-by: Thomas Zimmermann
> ---
> samples/vfio-mdev/mdpy-fb.c | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/samples/vfio-mdev/mdpy-fb.c b/samples/vfio-mdev/mdpy-fb.c
> index 9ec93d90e8a5..1de5801cd2e8 100644
> --- a/samples/vfio-mdev/mdpy-fb.c
> +++ b/samples/vfio-mdev/mdpy-fb.c
> @@ -161,14 +161,6 @@ static int mdpy_fb_probe(struct pci_dev *pdev,
> goto err_release_fb;
> }
>
> - info->apertures = alloc_apertures(1);
> - if (!info->apertures) {
> - ret = -ENOMEM;
> - goto err_unmap;
> - }
> - info->apertures->ranges[0].base = info->fix.smem_start;
> - info->apertures->ranges[0].size = info->fix.smem_len;
> -
> info->fbops = _fb_ops;
> info->flags = FBINFO_DEFAULT;
> info->pseudo_palette = par->palette;
Reviewed-by: Javier Martinez Canillas
But I think an ack from Kirti Wankhede or other virt folk is needed if you
want to merge this through drm-misc-next.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [Intel-gfx] [PATCH] drm/i915/display/dsi: use intel_de_rmw if possible
On Mon, 19 Dec 2022, Andrzej Hajda wrote:
> The helper makes the code more compact and readable.
>
> Signed-off-by: Andrzej Hajda
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 256 -
> 1 file changed, 82 insertions(+), 174 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index ae14c794c4bc09..b02ac9d2b1e4a2 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -207,7 +207,7 @@ void icl_dsi_frame_update(struct intel_crtc_state
> *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - u32 tmp, mode_flags;
> + u32 mode_flags;
> enum port port;
>
> mode_flags = crtc_state->mode_flags;
> @@ -224,9 +224,7 @@ void icl_dsi_frame_update(struct intel_crtc_state
> *crtc_state)
> else
> return;
>
> - tmp = intel_de_read(dev_priv, DSI_CMD_FRMCTL(port));
> - tmp |= DSI_FRAME_UPDATE_REQUEST;
> - intel_de_write(dev_priv, DSI_CMD_FRMCTL(port), tmp);
> + intel_de_rmw(dev_priv, DSI_CMD_FRMCTL(port), 0,
> DSI_FRAME_UPDATE_REQUEST);
> }
>
> static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
> @@ -234,7 +232,7 @@ static void dsi_program_swing_and_deemphasis(struct
> intel_encoder *encoder)
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> enum phy phy;
> - u32 tmp;
> + u32 tmp, mask, val;
> int lane;
>
> for_each_dsi_phy(phy, intel_dsi->phys) {
> @@ -242,56 +240,35 @@ static void dsi_program_swing_and_deemphasis(struct
> intel_encoder *encoder)
>* Program voltage swing and pre-emphasis level values as per
>* table in BSPEC under DDI buffer programing
>*/
> + mask = SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK;
> + val = SCALING_MODE_SEL(0x2) | TAP2_DISABLE | TAP3_DISABLE |
> + RTERM_SELECT(0x6);
> tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
> - tmp &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK);
> - tmp |= SCALING_MODE_SEL(0x2);
> - tmp |= TAP2_DISABLE | TAP3_DISABLE;
> - tmp |= RTERM_SELECT(0x6);
> + tmp &= ~mask;
> + tmp |= val;
> intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
> + intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), mask, val);
>
> - tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
> - tmp &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK);
> - tmp |= SCALING_MODE_SEL(0x2);
> - tmp |= TAP2_DISABLE | TAP3_DISABLE;
> - tmp |= RTERM_SELECT(0x6);
> - intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
> -
> + mask = SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
> +RCOMP_SCALAR_MASK;
> + val = SWING_SEL_UPPER(0x2) | SWING_SEL_LOWER(0x2) |
> + RCOMP_SCALAR(0x98);
> tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_LN(0, phy));
> - tmp &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
> - RCOMP_SCALAR_MASK);
> - tmp |= SWING_SEL_UPPER(0x2);
> - tmp |= SWING_SEL_LOWER(0x2);
> - tmp |= RCOMP_SCALAR(0x98);
> + tmp &= ~mask;
> + tmp |= val;
> intel_de_write(dev_priv, ICL_PORT_TX_DW2_GRP(phy), tmp);
> + intel_de_rmw(dev_priv, ICL_PORT_TX_DW2_AUX(phy), mask, val);
>
> - tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_AUX(phy));
> - tmp &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
> - RCOMP_SCALAR_MASK);
> - tmp |= SWING_SEL_UPPER(0x2);
> - tmp |= SWING_SEL_LOWER(0x2);
> - tmp |= RCOMP_SCALAR(0x98);
> - intel_de_write(dev_priv, ICL_PORT_TX_DW2_AUX(phy), tmp);
> -
> - tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW4_AUX(phy));
> - tmp &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
> - CURSOR_COEFF_MASK);
> - tmp |= POST_CURSOR_1(0x0);
> - tmp |= POST_CURSOR_2(0x0);
> - tmp |= CURSOR_COEFF(0x3f);
> - intel_de_write(dev_priv, ICL_PORT_TX_DW4_AUX(phy), tmp);
> -
> - for (lane = 0; lane <= 3; lane++) {
> - /* Bspec: must not use GRP register for write */
> - tmp = intel_de_read(dev_priv,
> - ICL_PORT_TX_DW4_LN(lane, phy));
> - tmp &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
> - CURSOR_COEFF_MASK);
> - tmp |=
Re: [Intel-gfx] [PATCH 08/18] fbdev/hyperv-fb: Do not set struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Generic fbdev drivers use the apertures field in struct fb_info to > control ownership of the framebuffer memory and graphics device. Do > not set the values in hyperv-fb. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 07/18] fbdev/clps711x-fb: Do not set struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Generic fbdev drivers use the apertures field in struct fb_info to > control ownership of the framebuffer memory and graphics device. Do > not set the values in clps711x-fb. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 06/18] drm/fb-helper: Do not allocate unused apertures structure
On 12/19/22 17:05, Thomas Zimmermann wrote: > The apertures field in struct fb_info is not used by DRM drivers. Do > not allocate it. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 05/18] drm/radeon: Do not set struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Generic fbdev drivers use the apertures field in struct fb_info to > control ownership of the framebuffer memory and graphics device. Do > not set the values in radeon. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 04/18] drm/i915: Do not set struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Generic fbdev drivers use the apertures field in struct fb_info to > control ownership of the framebuffer memory and graphics device. Do > not set the values in i915. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 03/18] drm/gma500: Do not set struct fb_info.apertures
On 12/19/22 17:05, Thomas Zimmermann wrote: > Generic fbdev drivers use the apertures field in struct fb_info to > control ownership of the framebuffer memory and graphics device. Do > not set the values in gma500. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [PATCH 02/18] Revert "fbcon: don't lose the console font across generic->chip driver switch"
On 12/19/22 17:05, Thomas Zimmermann wrote:
> This reverts commit ae1287865f5361fa138d4d3b1b6277908b54eac9.
>
> Always free the console font when deinitializing the framebuffer
> console. Subsequent framebuffer consoles will then use the default
> font. Rely on userspace to load any user-configured font for these
> consoles.
>
> Commit ae1287865f53 ("fbcon: don't lose the console font across
> generic->chip driver switch") was introduced to work around losing
> the font during graphics-device handover. [1][2] It kept a dangling
> pointer with the font data between loading the two consoles, which is
> fairly adventurous hack. It also never covered cases when the other
> consoles, such as VGA text mode, where involved.
>
> The problem has meanwhile been solved in userspace. Systemd comes
> with a udev rule that re-installs the configured font when a console
> comes up. [3] So the kernel workaround can be removed.
>
> This also removes one of the two special cases triggered by setting
> FBINFO_MISC_FIRMWARE in an fbdev driver.
>
> Tested during device handover from efifb and simpledrm to radeon. Udev
> reloads the configured console font for the new driver's terminal.
>
> Signed-off-by: Thomas Zimmermann
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=892340 # 1
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1074624 # 2
> Link:
> https://cgit.freedesktop.org/systemd/systemd/tree/src/vconsole/90-vconsole.rules.in?h=v222
> # 3
> ---
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [Intel-gfx] [PATCH 01/18] fbcon: Remove trailing whitespaces
On 12/19/22 17:04, Thomas Zimmermann wrote: > Fix coding style. No functional changes. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [Intel-gfx] [RESEND PATCH v4] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry
Zhenyu Wang 于2022年12月20日周二 16:25写道：
>
> On 2022.12.19 20:52:04 +0800, Zheng Wang wrote:
> > If intel_gvt_dma_map_guest_page failed, it will call
> > ppgtt_invalidate_spt, which will finally free the spt. But the caller does
> > not notice that, it will free spt again in error path.
> >
>
> It's not clear from this description which caller is actually wrong,
> better to clarify the problem in ppgtt_populate_spt_by_guest_entry() function.
>
Get it, will do in the next fix.
> > PAGE_SIZE, _addr);
> > - if (ret) {
> > - ppgtt_invalidate_spt(spt);
> > - return ret;
> > - }
> > + if (ret)
> > + goto err;
>
> I think it's fine to remove this and leave to upper caller, but again please
> describe the behavior change in commit message as well, e.g to fix the sanity
> of spt destroy that leaving previous invalidate and free of spt to caller
> function
> instead of within callee function.
Sorry for my bad habit. Will do in the next version.
> > sub_se.val64 = se->val64;
> >
> > /* Copy the PAT field from PDE. */
> > @@ -1231,6 +1229,47 @@ static int split_2MB_gtt_entry(struct intel_vgpu
> > *vgpu,
> > ops->set_pfn(se, sub_spt->shadow_page.mfn);
> > ppgtt_set_shadow_entry(spt, se, index);
> > return 0;
> > +err:
> > + /* Undone the existing mappings of DMA addr. */
> > + for_each_present_shadow_entry(spt, , parent_index) {
>
> sub_spt? We're undoing what's mapped for sub_spt right?
Yes, will change it to sub_spt in the next version.
>
> > + switch (e.type) {
> > + case GTT_TYPE_PPGTT_PTE_4K_ENTRY:
> > + gvt_vdbg_mm("invalidate 4K entry\n");
> > + ppgtt_invalidate_pte(spt, );
> > + break;
> > + case GTT_TYPE_PPGTT_PTE_64K_ENTRY:
> > + /* We don't setup 64K shadow entry so far. */
> > + WARN(1, "suspicious 64K gtt entry\n");
> > + continue;
> > + case GTT_TYPE_PPGTT_PTE_2M_ENTRY:
> > + gvt_vdbg_mm("invalidate 2M entry\n");
> > + continue;
> > + case GTT_TYPE_PPGTT_PTE_1G_ENTRY:
> > + WARN(1, "GVT doesn't support 1GB page\n");
> > + continue;
> > + case GTT_TYPE_PPGTT_PML4_ENTRY:
> > + case GTT_TYPE_PPGTT_PDP_ENTRY:
> > + case GTT_TYPE_PPGTT_PDE_ENTRY:
>
> I don't think this all entry type makes sense, as here we just split
> 2M entry for multiple 4K PTE entry.
I got it. I will leave the code for handling 4K PTE entry only.
>
> > + gvt_vdbg_mm("invalidate PMUL4/PDP/PDE entry\n");
> > + ret1 = ppgtt_invalidate_spt_by_shadow_entry(
> > + spt->vgpu, );
> > + if (ret1) {
> > + gvt_vgpu_err("fail: shadow page %p shadow
> > entry 0x%llx type %d\n",
> > + spt, e.val64, e.type);
> > + goto free_spt;
> > + }
>
> for above reason, I don't think this is valid.
Got it.
Thanks for your carefully reviewing. I'll try to fix that in the coming patch.
Best regards,
Zheng Wang
Re: [Intel-gfx] [PATCH v2] drm/i915/hdmi: Go for scrambling only if platform supports TMDS clock > 340MHz
On Tue, 20 Dec 2022, Ankit Nautiyal wrote:
> There are cases, where devices have an HDMI1.4 retimer, and TMDS clock rate
> is capped to 340MHz via VBT. In such cases scrambling might be supported
> by the platform and an HDMI2.0 sink for lower TMDS rates, but not
> supported by the retimer, causing blankouts.
>
> So avoid enabling scrambling, if the TMDS clock is capped to <= 340MHz.
>
> v2: Added comment, documenting the rationale to check for TMDS clock,
> before going for scrambling. (Arun)
>
> Signed-off-by: Ankit Nautiyal
> Reviewed-by: Arun R Murthy
> ---
> drivers/gpu/drm/i915/display/intel_hdmi.c | 21 -
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index efa2da080f62..7603426af9a0 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2244,6 +2244,25 @@ static bool intel_hdmi_is_cloned(const struct
> intel_crtc_state *crtc_state)
> !is_power_of_2(crtc_state->uapi.encoder_mask);
> }
>
> +static bool source_can_support_scrambling(struct intel_encoder *encoder)
"source can support scrambling" reads like "if this function returns
true, source might support scrambing, but also might not, depends on
something else".
So does this mean "source supports scrambling" or "source can support
scrambling"? If the latter, as it is now named, what *else* is required
for source to support scrambling?
BR,
Jani.
> +{
> + /*
> + * Gen 10+ support HDMI 2.0 : the max tmds clock is 594MHz, and
> + * scrambling is supported.
> + * But there seem to be cases where certain platforms that support
> + * HDMI 2.0, have an HDMI1.4 retimer chip, and the max tmds clock is
> + * capped by VBT to less than 340MHz.
> + *
> + * In such cases when an HDMI2.0 sink is connected, it creates a
> + * problem : the platform and the sink both support scrambling but the
> + * HDMI 1.4 retimer chip doesn't.
> + *
> + * So go for scrambling, based on the max tmds clock taking into
> account,
> + * restrictions coming from VBT.
> + */
> + return intel_hdmi_source_max_tmds_clock(encoder) > 34;
> +}
> +
> int intel_hdmi_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config,
> struct drm_connector_state *conn_state)
> @@ -2301,7 +2320,7 @@ int intel_hdmi_compute_config(struct intel_encoder
> *encoder,
>
> pipe_config->lane_count = 4;
>
> - if (scdc->scrambling.supported && DISPLAY_VER(dev_priv) >= 10) {
> + if (scdc->scrambling.supported &&
> source_can_support_scrambling(encoder)) {
> if (scdc->scrambling.low_rates)
> pipe_config->hdmi_scrambling = true;
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v3 2/7] drm/i915/hdcp: Keep cp fw agonstic naming convention
On Wed, 14 Dec 2022, Suraj Kandpal wrote:
> From: Anshuman Gupta
>
> Change the include/drm/i915_mei_hdcp_interface.h to
> include/drm/i915_cp_fw_hdcp_interface.h
This is now stale.
>
> Cc: Tomas Winkler
> Cc: Rodrigo Vivi
> Cc: Uma Shankar
> Cc: Ankit Nautiyal
> Signed-off-by: Anshuman Gupta
> Signed-off-by: Suraj Kandpal
> Acked-by: Tomas Winkler
> ---
> .../drm/i915/display/intel_display_types.h| 2 +-
> drivers/misc/mei/hdcp/mei_hdcp.c | 2 +-
> include/drm/i915_mei_hdcp_interface.h | 184 --
> 3 files changed, 2 insertions(+), 186 deletions(-)
> delete mode 100644 include/drm/i915_mei_hdcp_interface.h
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 32e8b2fc3cc6..81d195ef5e57 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -43,7 +43,7 @@
> #include
> #include
> #include
> -#include
> +#include
> #include
>
> #include "i915_vma.h"
> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
> b/drivers/misc/mei/hdcp/mei_hdcp.c
> index e889a8bd7ac8..cbad27511899 100644
> --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> @@ -23,7 +23,7 @@
> #include
> #include
> #include
> -#include
> +#include
>
> #include "mei_hdcp.h"
>
> diff --git a/include/drm/i915_mei_hdcp_interface.h
> b/include/drm/i915_mei_hdcp_interface.h
> deleted file mode 100644
This patch now *deletes* the file. The next patch adds it back. Please
be more careful before sending.
Please check that all commits build before sending, run something like:
$ git rebase -i $tip --exec="make -j8"
This should be standard practice for all contributions.
BR,
Jani.
> index f441cbcd95a4..
> --- a/include/drm/i915_mei_hdcp_interface.h
> +++ /dev/null
> @@ -1,184 +0,0 @@
> -/* SPDX-License-Identifier: (GPL-2.0+) */
> -/*
> - * Copyright © 2017-2019 Intel Corporation
> - *
> - * Authors:
> - * Ramalingam C
> - */
> -
> -#ifndef _I915_MEI_HDCP_INTERFACE_H_
> -#define _I915_MEI_HDCP_INTERFACE_H_
> -
> -#include
> -#include
> -#include
> -
> -/**
> - * enum hdcp_port_type - HDCP port implementation type defined by ME FW
> - * @HDCP_PORT_TYPE_INVALID: Invalid hdcp port type
> - * @HDCP_PORT_TYPE_INTEGRATED: In-Host HDCP2.x port
> - * @HDCP_PORT_TYPE_LSPCON: HDCP2.2 discrete wired Tx port with LSPCON
> - * (HDMI 2.0) solution
> - * @HDCP_PORT_TYPE_CPDP: HDCP2.2 discrete wired Tx port using the CPDP (DP
> 1.3)
> - *solution
> - */
> -enum hdcp_port_type {
> - HDCP_PORT_TYPE_INVALID,
> - HDCP_PORT_TYPE_INTEGRATED,
> - HDCP_PORT_TYPE_LSPCON,
> - HDCP_PORT_TYPE_CPDP
> -};
> -
> -/**
> - * enum hdcp_wired_protocol - HDCP adaptation used on the port
> - * @HDCP_PROTOCOL_INVALID: Invalid HDCP adaptation protocol
> - * @HDCP_PROTOCOL_HDMI: HDMI adaptation of HDCP used on the port
> - * @HDCP_PROTOCOL_DP: DP adaptation of HDCP used on the port
> - */
> -enum hdcp_wired_protocol {
> - HDCP_PROTOCOL_INVALID,
> - HDCP_PROTOCOL_HDMI,
> - HDCP_PROTOCOL_DP
> -};
> -
> -enum mei_fw_ddi {
> - MEI_DDI_INVALID_PORT = 0x0,
> -
> - MEI_DDI_B = 1,
> - MEI_DDI_C,
> - MEI_DDI_D,
> - MEI_DDI_E,
> - MEI_DDI_F,
> - MEI_DDI_A = 7,
> - MEI_DDI_RANGE_END = MEI_DDI_A,
> -};
> -
> -/**
> - * enum mei_fw_tc - ME Firmware defined index for transcoders
> - * @MEI_INVALID_TRANSCODER: Index for Invalid transcoder
> - * @MEI_TRANSCODER_EDP: Index for EDP Transcoder
> - * @MEI_TRANSCODER_DSI0: Index for DSI0 Transcoder
> - * @MEI_TRANSCODER_DSI1: Index for DSI1 Transcoder
> - * @MEI_TRANSCODER_A: Index for Transcoder A
> - * @MEI_TRANSCODER_B: Index for Transcoder B
> - * @MEI_TRANSCODER_C: Index for Transcoder C
> - * @MEI_TRANSCODER_D: Index for Transcoder D
> - */
> -enum mei_fw_tc {
> - MEI_INVALID_TRANSCODER = 0x00,
> - MEI_TRANSCODER_EDP,
> - MEI_TRANSCODER_DSI0,
> - MEI_TRANSCODER_DSI1,
> - MEI_TRANSCODER_A = 0x10,
> - MEI_TRANSCODER_B,
> - MEI_TRANSCODER_C,
> - MEI_TRANSCODER_D
> -};
> -
> -/**
> - * struct hdcp_port_data - intel specific HDCP port data
> - * @fw_ddi: ddi index as per ME FW
> - * @fw_tc: transcoder index as per ME FW
> - * @port_type: HDCP port type as per ME FW classification
> - * @protocol: HDCP adaptation as per ME FW
> - * @k: No of streams transmitted on a port. Only on DP MST this is != 1
> - * @seq_num_m: Count of RepeaterAuth_Stream_Manage msg propagated.
> - * Initialized to 0 on AKE_INIT. Incremented after every successful
> - * transmission of RepeaterAuth_Stream_Manage message. When it rolls
> - * over re-Auth has to be triggered.
> - * @streams: struct hdcp2_streamid_type[k]. Defines the type and id for the
> - *streams
> - */
> -struct hdcp_port_data {
> - enum mei_fw_ddi fw_ddi;
> - enum mei_fw_tc
Re: [Intel-gfx] [PATCH] [next] i915/gvt: Replace one-element array with flexible-array member
On 2022.12.20 16:41:15 +1300, Paulo Miguel Almeida wrote:
> One-element arrays are deprecated, and we are replacing them with
> flexible array members instead. So, replace one-element array with
> flexible-array member in struct gvt_firmware_header and refactor the
> rest of the code accordingly.
>
> Additionally, previous implementation was allocating 8 bytes more than
> required to represent firmware_header + cfg_space data + mmio data.
>
> This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
> routines on memcpy() and help us make progress towards globally
> enabling -fstrict-flex-arrays=3 [1].
>
> Link: https://github.com/KSPP/linux/issues/79
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
> Signed-off-by: Paulo Miguel Almeida
> ---
> To make reviewing this patch easier, I'm pasting before/after struct
> sizes.
>
> pahole -C gvt_firmware_header before/drivers/gpu/drm/i915/gvt/firmware.o
> struct gvt_firmware_header {
> u64magic;/* 0 8 */
> u32crc32;/* 8 4 */
> u32version; /*12 4 */
> u64cfg_space_size; /*16 8 */
> u64cfg_space_offset; /*24 8 */
> u64mmio_size;/*32 8 */
> u64mmio_offset; /*40 8 */
> unsigned char data[1]; /*48 1 */
>
> /* size: 56, cachelines: 1, members: 8 */
> /* padding: 7 */
> /* last cacheline: 56 bytes */
> };
>
> pahole -C gvt_firmware_header after/drivers/gpu/drm/i915/gvt/firmware.o
> struct gvt_firmware_header {
> u64magic;/* 0 8 */
> u32crc32;/* 8 4 */
> u32version; /*12 4 */
> u64cfg_space_size; /*16 8 */
> u64cfg_space_offset; /*24 8 */
> u64mmio_size;/*32 8 */
> u64mmio_offset; /*40 8 */
> unsigned char data[]; /*48 0 */
>
> /* size: 48, cachelines: 1, members: 8 */
> /* last cacheline: 48 bytes */
> };
>
> As you can see the additional byte of the fake-flexible array (data[1])
> forced the compiler to pad the struct but those bytes aren't actually used
> as first & last bytes (of both cfg_space and mmio) are controlled by the
> <>_size and <>_offset members present in the gvt_firmware_header struct.
> ---
> drivers/gpu/drm/i915/gvt/firmware.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/firmware.c
> b/drivers/gpu/drm/i915/gvt/firmware.c
> index a683c22d5b64..dce93738e98a 100644
> --- a/drivers/gpu/drm/i915/gvt/firmware.c
> +++ b/drivers/gpu/drm/i915/gvt/firmware.c
> @@ -45,7 +45,7 @@ struct gvt_firmware_header {
> u64 cfg_space_offset; /* offset in the file */
> u64 mmio_size;
> u64 mmio_offset;/* offset in the file */
> - unsigned char data[1];
> + unsigned char data[];
> };
>
> #define dev_to_drm_minor(d) dev_get_drvdata((d))
> @@ -77,7 +77,7 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt)
> unsigned long size, crc32_start;
> int ret;
>
> - size = sizeof(*h) + info->mmio_size + info->cfg_space_size;
> + size = offsetof(struct gvt_firmware_header, data) + info->mmio_size +
> info->cfg_space_size;
> firmware = vzalloc(size);
> if (!firmware)
> return -ENOMEM;
> --
Looks good to me.
Reviewed-by: Zhenyu Wang
Thanks!
signature.asc
Description: PGP signature
[Intel-gfx] [PATCH v4 2/2] drm/i915/mtl: Limit scaler input to 4k in plane scaling [RESEND]
From: Animesh Manna
As part of die area reduction max input source modified to 4096
for MTL so modified range check logic of scaler.
Signed-off-by: José Roberto de Souza
Signed-off-by: Animesh Manna
Signed-off-by: Luca Coelho
---
drivers/gpu/drm/i915/display/skl_scaler.c | 31 +--
1 file changed, 23 insertions(+), 8 deletions(-)
In v2:
* No changes;
In v3:
* Removed stray reviewed-by tag;
* Added my s-o-b.
In v4:
* No changes.
For some reason this patch didn't reach the list before. Resending.
diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
b/drivers/gpu/drm/i915/display/skl_scaler.c
index d7390067b7d4..6baa07142b03 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -103,6 +103,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool
force_detach,
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
const struct drm_display_mode *adjusted_mode =
_state->hw.adjusted_mode;
+ int min_src_w, min_src_h, min_dst_w, min_dst_h;
+ int max_src_w, max_src_h, max_dst_w, max_dst_h;
/*
* Src coordinates are already rotated by 270 degrees for
@@ -157,15 +159,28 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
bool force_detach,
return -EINVAL;
}
+ min_src_w = SKL_MIN_SRC_W;
+ min_src_h = SKL_MIN_SRC_H;
+ min_dst_w = SKL_MIN_DST_W;
+ min_dst_h = SKL_MIN_DST_H;
+
+ if (DISPLAY_VER(dev_priv) >= 11 && DISPLAY_VER(dev_priv) < 14) {
+ max_src_w = ICL_MAX_SRC_W;
+ max_src_h = ICL_MAX_SRC_H;
+ max_dst_w = ICL_MAX_DST_W;
+ max_dst_h = ICL_MAX_DST_H;
+ } else {
+ max_src_w = SKL_MAX_SRC_W;
+ max_src_h = SKL_MAX_SRC_H;
+ max_dst_w = SKL_MAX_DST_W;
+ max_dst_h = SKL_MAX_DST_H;
+ }
+
/* range checks */
- if (src_w < SKL_MIN_SRC_W || src_h < SKL_MIN_SRC_H ||
- dst_w < SKL_MIN_DST_W || dst_h < SKL_MIN_DST_H ||
- (DISPLAY_VER(dev_priv) >= 11 &&
-(src_w > ICL_MAX_SRC_W || src_h > ICL_MAX_SRC_H ||
- dst_w > ICL_MAX_DST_W || dst_h > ICL_MAX_DST_H)) ||
- (DISPLAY_VER(dev_priv) < 11 &&
-(src_w > SKL_MAX_SRC_W || src_h > SKL_MAX_SRC_H ||
- dst_w > SKL_MAX_DST_W || dst_h > SKL_MAX_DST_H))) {
+ if (src_w < min_src_w || src_h < min_src_h ||
+ dst_w < min_dst_w || dst_h < min_dst_h ||
+ src_w > max_src_w || src_h > max_src_h ||
+ dst_w > max_dst_w || dst_h > max_dst_h) {
drm_dbg_kms(_priv->drm,
"scaler_user index %u.%u: src %ux%u dst %ux%u "
"size is out of scaler range\n",
Re: [Intel-gfx] [RESEND PATCH v4] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry
On 2022.12.19 20:52:04 +0800, Zheng Wang wrote:
> If intel_gvt_dma_map_guest_page failed, it will call
> ppgtt_invalidate_spt, which will finally free the spt. But the caller does
> not notice that, it will free spt again in error path.
>
It's not clear from this description which caller is actually wrong,
better to clarify the problem in ppgtt_populate_spt_by_guest_entry() function.
> Fix this by undoing the mapping of DMA address and freeing sub_spt.
>
> Fixes: b901b252b6cf ("drm/i915/gvt: Add 2M huge gtt support")
> Signed-off-by: Zheng Wang
> ---
> v4:
> - fix by undo the mapping of DMA address and free sub_spt suggested by Zhi
>
> v3:
> - correct spelling mistake and remove unused variable suggested by Greg
>
> v2: https://lore.kernel.org/all/20221006165845.1735393-1-zyytlz...@163.com/
>
> v1: https://lore.kernel.org/all/20220928033340.1063949-1-zyytlz...@163.com/
> ---
> drivers/gpu/drm/i915/gvt/gtt.c | 53 +-
> 1 file changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 51e5e8fb505b..b472e021e5a4 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1192,11 +1192,11 @@ static int split_2MB_gtt_entry(struct intel_vgpu
> *vgpu,
> {
> const struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
> struct intel_vgpu_ppgtt_spt *sub_spt;
> - struct intel_gvt_gtt_entry sub_se;
> + struct intel_gvt_gtt_entry sub_se, e;
> unsigned long start_gfn;
> dma_addr_t dma_addr;
> - unsigned long sub_index;
> - int ret;
> + unsigned long sub_index, parent_index;
> + int ret, ret1;
>
> gvt_dbg_mm("Split 2M gtt entry, index %lu\n", index);
>
> @@ -1209,10 +1209,8 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
> for_each_shadow_entry(sub_spt, _se, sub_index) {
> ret = intel_gvt_dma_map_guest_page(vgpu, start_gfn + sub_index,
> PAGE_SIZE, _addr);
> - if (ret) {
> - ppgtt_invalidate_spt(spt);
> - return ret;
> - }
> + if (ret)
> + goto err;
I think it's fine to remove this and leave to upper caller, but again please
describe the behavior change in commit message as well, e.g to fix the sanity
of spt destroy that leaving previous invalidate and free of spt to caller
function
instead of within callee function.
> sub_se.val64 = se->val64;
>
> /* Copy the PAT field from PDE. */
> @@ -1231,6 +1229,47 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
> ops->set_pfn(se, sub_spt->shadow_page.mfn);
> ppgtt_set_shadow_entry(spt, se, index);
> return 0;
> +err:
> + /* Undone the existing mappings of DMA addr. */
> + for_each_present_shadow_entry(spt, , parent_index) {
sub_spt? We're undoing what's mapped for sub_spt right?
> + switch (e.type) {
> + case GTT_TYPE_PPGTT_PTE_4K_ENTRY:
> + gvt_vdbg_mm("invalidate 4K entry\n");
> + ppgtt_invalidate_pte(spt, );
> + break;
> + case GTT_TYPE_PPGTT_PTE_64K_ENTRY:
> + /* We don't setup 64K shadow entry so far. */
> + WARN(1, "suspicious 64K gtt entry\n");
> + continue;
> + case GTT_TYPE_PPGTT_PTE_2M_ENTRY:
> + gvt_vdbg_mm("invalidate 2M entry\n");
> + continue;
> + case GTT_TYPE_PPGTT_PTE_1G_ENTRY:
> + WARN(1, "GVT doesn't support 1GB page\n");
> + continue;
> + case GTT_TYPE_PPGTT_PML4_ENTRY:
> + case GTT_TYPE_PPGTT_PDP_ENTRY:
> + case GTT_TYPE_PPGTT_PDE_ENTRY:
I don't think this all entry type makes sense, as here we just split
2M entry for multiple 4K PTE entry.
> + gvt_vdbg_mm("invalidate PMUL4/PDP/PDE entry\n");
> + ret1 = ppgtt_invalidate_spt_by_shadow_entry(
> + spt->vgpu, );
> + if (ret1) {
> + gvt_vgpu_err("fail: shadow page %p shadow entry
> 0x%llx type %d\n",
> + spt, e.val64, e.type);
> + goto free_spt;
> + }
for above reason, I don't think this is valid.
> + break;
> + default:
> + GEM_BUG_ON(1);
> + }
> + }
> + /* Release the new alloced apt. */
> +free_spt:
> + trace_spt_change(sub_spt->vgpu->id, "release", sub_spt,
> + sub_spt->guest_page.gfn, sub_spt->shadow_page.type);
> + ppgtt_free_spt(sub_spt);
> + sub_spt = NULL;
> + return ret;
> }
>
> static int split_64KB_gtt_entry(struct intel_vgpu *vgpu,
> --
> 2.25.1
>
[Intel-gfx] [PATCH v5 2/2] drm/i915/mtl: Limit scaler input to 4k in plane scaling
From: Animesh Manna
As part of die area reduction max input source modified to 4096
for MTL so modified range check logic of scaler.
Signed-off-by: Jose Roberto de Souza
Signed-off-by: Animesh Manna
Signed-off-by: Luca Coelho
---
In v2:
* No changes;
In v3:
* Removed stray reviewed-by tag;
* Added my s-o-b.
In v4:
* No changes.
In v5:
* Just resent with a cover letter.
drivers/gpu/drm/i915/display/skl_scaler.c | 31 +--
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
b/drivers/gpu/drm/i915/display/skl_scaler.c
index d7390067b7d4..6baa07142b03 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -103,6 +103,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool
force_detach,
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
const struct drm_display_mode *adjusted_mode =
_state->hw.adjusted_mode;
+ int min_src_w, min_src_h, min_dst_w, min_dst_h;
+ int max_src_w, max_src_h, max_dst_w, max_dst_h;
/*
* Src coordinates are already rotated by 270 degrees for
@@ -157,15 +159,28 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
bool force_detach,
return -EINVAL;
}
+ min_src_w = SKL_MIN_SRC_W;
+ min_src_h = SKL_MIN_SRC_H;
+ min_dst_w = SKL_MIN_DST_W;
+ min_dst_h = SKL_MIN_DST_H;
+
+ if (DISPLAY_VER(dev_priv) >= 11 && DISPLAY_VER(dev_priv) < 14) {
+ max_src_w = ICL_MAX_SRC_W;
+ max_src_h = ICL_MAX_SRC_H;
+ max_dst_w = ICL_MAX_DST_W;
+ max_dst_h = ICL_MAX_DST_H;
+ } else {
+ max_src_w = SKL_MAX_SRC_W;
+ max_src_h = SKL_MAX_SRC_H;
+ max_dst_w = SKL_MAX_DST_W;
+ max_dst_h = SKL_MAX_DST_H;
+ }
+
/* range checks */
- if (src_w < SKL_MIN_SRC_W || src_h < SKL_MIN_SRC_H ||
- dst_w < SKL_MIN_DST_W || dst_h < SKL_MIN_DST_H ||
- (DISPLAY_VER(dev_priv) >= 11 &&
-(src_w > ICL_MAX_SRC_W || src_h > ICL_MAX_SRC_H ||
- dst_w > ICL_MAX_DST_W || dst_h > ICL_MAX_DST_H)) ||
- (DISPLAY_VER(dev_priv) < 11 &&
-(src_w > SKL_MAX_SRC_W || src_h > SKL_MAX_SRC_H ||
- dst_w > SKL_MAX_DST_W || dst_h > SKL_MAX_DST_H))) {
+ if (src_w < min_src_w || src_h < min_src_h ||
+ dst_w < min_dst_w || dst_h < min_dst_h ||
+ src_w > max_src_w || src_h > max_src_h ||
+ dst_w > max_dst_w || dst_h > max_dst_h) {
drm_dbg_kms(_priv->drm,
"scaler_user index %u.%u: src %ux%u dst %ux%u "
"size is out of scaler range\n",
--
2.38.1
[Intel-gfx] [PATCH v5 1/2] drm/i915/mtl: limit second scaler vertical scaling in ver >= 14
In newer hardware versions (i.e. display version >= 14), the second
scaler doesn't support vertical scaling.
The current implementation of the scaling limits is simplified and
only occurs when the planes are created, so we don't know which scaler
is being used.
In order to handle separate scaling limits for horizontal and vertical
scaling, and different limits per scaler, split the checks in two
phases. We first do a simple check during plane creation and use the
best-case scenario (because we don't know the scaler that may be used
at a later point) and then do a more specific check when the scalers
are actually being set up.
Signed-off-by: Luca Coelho
---
In v2:
* fix DRM_PLANE_NO_SCALING renamed macros;
In v3:
* No changes.
In v4:
* Got rid of the changes in the general planes max scale code;
* Added a couple of FIXMEs;
* Made intel_atomic_setup_scaler() return an int with errors;
In v5:
* Just resent with a cover letter.
drivers/gpu/drm/i915/display/i9xx_plane.c | 4 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 83 ---
.../gpu/drm/i915/display/intel_atomic_plane.c | 30 ++-
.../gpu/drm/i915/display/intel_atomic_plane.h | 2 +-
drivers/gpu/drm/i915/display/intel_cursor.c | 4 +-
drivers/gpu/drm/i915/display/intel_sprite.c | 20 +
.../drm/i915/display/skl_universal_plane.c| 45 +-
7 files changed, 128 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
b/drivers/gpu/drm/i915/display/i9xx_plane.c
index ecaeb7dc196b..390e96f0692b 100644
--- a/drivers/gpu/drm/i915/display/i9xx_plane.c
+++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
@@ -326,9 +326,7 @@ i9xx_plane_check(struct intel_crtc_state *crtc_state,
if (ret)
return ret;
- ret = intel_atomic_plane_check_clipping(plane_state, crtc_state,
- DRM_PLANE_NO_SCALING,
- DRM_PLANE_NO_SCALING,
+ ret = intel_atomic_plane_check_clipping(plane_state, crtc_state, false,
i9xx_plane_has_windowing(plane));
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index 6621aa245caf..bf4761a40675 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -38,6 +38,7 @@
#include "intel_atomic.h"
#include "intel_cdclk.h"
#include "intel_display_types.h"
+#include "intel_fb.h"
#include "intel_global_state.h"
#include "intel_hdcp.h"
#include "intel_psr.h"
@@ -310,11 +311,11 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
kfree(crtc_state);
}
-static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state
*scaler_state,
- int num_scalers_need, struct intel_crtc
*intel_crtc,
- const char *name, int idx,
- struct intel_plane_state *plane_state,
- int *scaler_id)
+static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state
*scaler_state,
+int num_scalers_need, struct intel_crtc
*intel_crtc,
+const char *name, int idx,
+struct intel_plane_state *plane_state,
+int *scaler_id)
{
struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
int j;
@@ -334,7 +335,7 @@ static void intel_atomic_setup_scaler(struct
intel_crtc_scaler_state *scaler_sta
if (drm_WARN(_priv->drm, *scaler_id < 0,
"Cannot find scaler for %s:%d\n", name, idx))
- return;
+ return -EBUSY;
/* set scaler mode */
if (plane_state && plane_state->hw.fb &&
@@ -375,9 +376,69 @@ static void intel_atomic_setup_scaler(struct
intel_crtc_scaler_state *scaler_sta
mode = SKL_PS_SCALER_MODE_DYN;
}
+ /*
+* FIXME: we should also check the scaler factors for pfit, so
+* this shouldn't be tied directly to planes.
+*/
+ if (plane_state && plane_state->hw.fb) {
+ const struct drm_framebuffer *fb = plane_state->hw.fb;
+ struct drm_rect *src = _state->uapi.src;
+ struct drm_rect *dst = _state->uapi.dst;
+ int hscale, vscale, max_vscale, max_hscale;
+
+ /*
+* FIXME: When two scalers are needed, but only one of
+* them needs to downscale, we should make sure that
+* the one that needs downscaling support is assigned
+* as the first scaler, so we don't reject downscaling
+* unnecessarily.
+*/
+
+ if (DISPLAY_VER(dev_priv) >= 14) {
+
[Intel-gfx] [PATCH v5 0/2] drm/i915/mtl: handle some MTL scaler limitations
Hi, I'm resending this series with a cover letter, because the patches didn't seem to arrive in patchwork as they should. The versioning history is in the patches themselves. Please review. Cheers, Luca. Animesh Manna (1): drm/i915/mtl: Limit scaler input to 4k in plane scaling Luca Coelho (1): drm/i915/mtl: limit second scaler vertical scaling in ver >= 14 drivers/gpu/drm/i915/display/i9xx_plane.c | 4 +- drivers/gpu/drm/i915/display/intel_atomic.c | 83 --- .../gpu/drm/i915/display/intel_atomic_plane.c | 30 ++- .../gpu/drm/i915/display/intel_atomic_plane.h | 2 +- drivers/gpu/drm/i915/display/intel_cursor.c | 4 +- drivers/gpu/drm/i915/display/intel_sprite.c | 20 + drivers/gpu/drm/i915/display/skl_scaler.c | 31 +-- .../drm/i915/display/skl_universal_plane.c| 45 +- 8 files changed, 151 insertions(+), 68 deletions(-) -- 2.38.1
